[CST-14903] Orcid Synchronization improvements

feat:
 - Introduces reactive states derived from item inside orcid-sync page
 - Removes unnecessary navigation
ref:
  - Introduces catchError operator and handles failures with error messages

src/app/item-page/orcid-page/orcid-auth/orcid-auth.component.ts

@@ -1,8 +1,8 @@
 import { Component, EventEmitter, Inject, Input, OnChanges, OnInit, Output, SimpleChanges } from '@angular/core';
 
 import { TranslateService } from '@ngx-translate/core';
-import { BehaviorSubject, Observable } from 'rxjs';
+import { BehaviorSubject, Observable, of } from 'rxjs';
-import { map } from 'rxjs/operators';
+import { catchError, map } from 'rxjs/operators';
 import { NativeWindowRef, NativeWindowService } from '../../../core/services/window.service';
 import { Item } from '../../../core/shared/item.model';
 import { NotificationsService } from '../../../shared/notifications/notifications.service';
@@ -10,6 +10,8 @@ import { RemoteData } from '../../../core/data/remote-data';
 import { ResearcherProfile } from '../../../core/profile/model/researcher-profile.model';
 import { getFirstCompletedRemoteData } from '../../../core/shared/operators';
 import { OrcidAuthService } from '../../../core/orcid/orcid-auth.service';
+import { createFailedRemoteDataObject } from '../../../shared/remote-data.utils';
+import { HttpErrorResponse } from '@angular/common/http';
 
 @Component({
   selector: 'ds-orcid-auth',
@@ -170,14 +172,15 @@ export class OrcidAuthComponent implements OnInit, OnChanges {
   unlinkOrcid(): void {
     this.unlinkProcessing.next(true);
     this.orcidAuthService.unlinkOrcidByItem(this.item).pipe(
-      getFirstCompletedRemoteData()
+      getFirstCompletedRemoteData(),
+      catchError((err: HttpErrorResponse) => of(createFailedRemoteDataObject(err.message, err.status)))
     ).subscribe((remoteData: RemoteData<ResearcherProfile>) => {
       this.unlinkProcessing.next(false);
-      if (remoteData.isSuccess) {
+      if (remoteData.hasFailed) {
+        this.notificationsService.error(this.translateService.get('person.page.orcid.unlink.error'));
+      } else {
         this.notificationsService.success(this.translateService.get('person.page.orcid.unlink.success'));
         this.unlink.emit();
-      } else {
-        this.notificationsService.error(this.translateService.get('person.page.orcid.unlink.error'));
       }
     });
   }

src/app/item-page/orcid-page/orcid-page.component.ts

@@ -3,7 +3,7 @@ import { ActivatedRoute, ParamMap, Router } from '@angular/router';
 import { isPlatformBrowser } from '@angular/common';
 
 import { BehaviorSubject, combineLatest } from 'rxjs';
-import { map, take } from 'rxjs/operators';
+import { filter, map, take } from 'rxjs/operators';
 
 import { OrcidAuthService } from '../../core/orcid/orcid-auth.service';
 import { getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload } from '../../core/shared/operators';
@@ -147,7 +147,19 @@ export class OrcidPageComponent implements OnInit {
    */
   private clearRouteParams(): void {
     // update route removing the code from query params
-    const redirectUrl = this.router.url.split('?')[0];
+    this.route.queryParamMap
-    this.router.navigate([redirectUrl]);
+      .pipe(
+        filter((paramMap: ParamMap) => isNotEmpty(paramMap.keys)),
+        map(_ => Object.assign({})),
+        take(1),
+      ).subscribe(queryParams =>
+        this.router.navigate(
+          [],
+          {
+            relativeTo: this.route,
+            queryParams
+          }
+        )
+    );
   }
 }

src/app/item-page/orcid-page/orcid-sync-settings/orcid-sync-settings.component.spec.ts

@@ -1,6 +1,6 @@
 import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core';
 import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
-import { UntypedFormControl, UntypedFormGroup, FormsModule, ReactiveFormsModule } from '@angular/forms';
+import { FormsModule, ReactiveFormsModule, UntypedFormControl, UntypedFormGroup } from '@angular/forms';
 import { RouterTestingModule } from '@angular/router/testing';
 import { By } from '@angular/platform-browser';
 
@@ -24,8 +24,8 @@ describe('OrcidSyncSettingsComponent test suite', () => {
   let comp: OrcidSyncSettingsComponent;
   let fixture: ComponentFixture<OrcidSyncSettingsComponent>;
   let scheduler: TestScheduler;
-  let researcherProfileService: jasmine.SpyObj<ResearcherProfileDataService>;
   let notificationsService;
+  let researcherProfileService: jasmine.SpyObj<ResearcherProfileDataService>;
   let formGroup: UntypedFormGroup;
 
   const mockResearcherProfile: ResearcherProfile = Object.assign(new ResearcherProfile(), {
@@ -161,6 +161,7 @@ describe('OrcidSyncSettingsComponent test suite', () => {
     scheduler = getTestScheduler();
     fixture = TestBed.createComponent(OrcidSyncSettingsComponent);
     comp = fixture.componentInstance;
+    researcherProfileService.findByRelatedItem.and.returnValue(createSuccessfulRemoteDataObject$(mockResearcherProfile));
     comp.item = mockItemLinkedToOrcid;
     fixture.detectChanges();
   }));
@@ -197,7 +198,6 @@ describe('OrcidSyncSettingsComponent test suite', () => {
     });
 
     it('should call updateByOrcidOperations properly', () => {
-      researcherProfileService.findByRelatedItem.and.returnValue(createSuccessfulRemoteDataObject$(mockResearcherProfile));
       researcherProfileService.patch.and.returnValue(createSuccessfulRemoteDataObject$(mockResearcherProfile));
       const expectedOps: Operation[] = [
         {
@@ -226,7 +226,6 @@ describe('OrcidSyncSettingsComponent test suite', () => {
     });
 
     it('should show notification on success', () => {
-      researcherProfileService.findByRelatedItem.and.returnValue(createSuccessfulRemoteDataObject$(mockResearcherProfile));
       researcherProfileService.patch.and.returnValue(createSuccessfulRemoteDataObject$(mockResearcherProfile));
 
       scheduler.schedule(() => comp.onSubmit(formGroup));
@@ -238,6 +237,8 @@ describe('OrcidSyncSettingsComponent test suite', () => {
 
     it('should show notification on error', () => {
       researcherProfileService.findByRelatedItem.and.returnValue(createFailedRemoteDataObject$());
+      comp.item = mockItemLinkedToOrcid;
+      fixture.detectChanges();
 
       scheduler.schedule(() => comp.onSubmit(formGroup));
       scheduler.flush();
@@ -247,7 +248,6 @@ describe('OrcidSyncSettingsComponent test suite', () => {
     });
 
     it('should show notification on error', () => {
-      researcherProfileService.findByRelatedItem.and.returnValue(createSuccessfulRemoteDataObject$(mockResearcherProfile));
       researcherProfileService.patch.and.returnValue(createFailedRemoteDataObject$());
 
       scheduler.schedule(() => comp.onSubmit(formGroup));

src/app/item-page/orcid-page/orcid-sync-settings/orcid-sync-settings.component.ts

@@ -1,80 +1,97 @@
-import { Component, EventEmitter, Input, OnInit, Output } from '@angular/core';
+import { Component, EventEmitter, Input, OnDestroy, OnInit, Output } from '@angular/core';
 import { UntypedFormGroup } from '@angular/forms';
 
 import { TranslateService } from '@ngx-translate/core';
 import { Operation } from 'fast-json-patch';
-import { of } from 'rxjs';
+import { BehaviorSubject, Observable, of } from 'rxjs';
-import { switchMap } from 'rxjs/operators';
+import { catchError, filter, map, switchMap, take, takeUntil } from 'rxjs/operators';
 
 import { RemoteData } from '../../../core/data/remote-data';
 import { ResearcherProfileDataService } from '../../../core/profile/researcher-profile-data.service';
 import { Item } from '../../../core/shared/item.model';
-import { getFirstCompletedRemoteData } from '../../../core/shared/operators';
+import { getFirstCompletedRemoteData, getRemoteDataPayload } from '../../../core/shared/operators';
 import { NotificationsService } from '../../../shared/notifications/notifications.service';
 import { ResearcherProfile } from '../../../core/profile/model/researcher-profile.model';
+import { hasValue } from '../../../shared/empty.util';
+import { HttpErrorResponse } from '@angular/common/http';
+import { createFailedRemoteDataObject } from '../../../shared/remote-data.utils';
 
 @Component({
   selector: 'ds-orcid-sync-setting',
   templateUrl: './orcid-sync-settings.component.html',
   styleUrls: ['./orcid-sync-settings.component.scss']
 })
-export class OrcidSyncSettingsComponent implements OnInit {
+export class OrcidSyncSettingsComponent implements OnInit, OnDestroy {
-
-  /**
-   * The item for which showing the orcid settings
-   */
-  @Input() item: Item;
 
   /**
    * The prefix used for i18n keys
    */
   messagePrefix = 'person.page.orcid';
-
   /**
    * The current synchronization mode
    */
   currentSyncMode: string;
-
   /**
    * The current synchronization mode for publications
    */
   currentSyncPublications: string;
-
   /**
    * The current synchronization mode for funding
    */
   currentSyncFunding: string;
-
   /**
    * The synchronization options
    */
   syncModes: { value: string, label: string }[];
-
   /**
    * The synchronization options for publications
    */
   syncPublicationOptions: { value: string, label: string }[];
-
   /**
    * The synchronization options for funding
    */
   syncFundingOptions: { value: string, label: string }[];
-
   /**
    * The profile synchronization options
    */
   syncProfileOptions: { value: string, label: string, checked: boolean }[];
-
   /**
    * An event emitted when settings are updated
    */
   @Output() settingsUpdated: EventEmitter<void> = new EventEmitter<void>();
+  /**
+   * Emitter that triggers onDestroy lifecycle
+   * @private
+   */
+  readonly #destroy$ = new EventEmitter<void>();
+  /**
+   * {@link BehaviorSubject} that reflects {@link item} input changes
+   * @private
+   */
+  readonly #item$ = new BehaviorSubject<Item>(null);
+  /**
+   * {@link Observable} that contains {@link ResearcherProfile} linked to the {@link #item$}
+   * @private
+   */
+  #researcherProfile$: Observable<ResearcherProfile>;
 
   constructor(private researcherProfileService: ResearcherProfileDataService,
               private notificationsService: NotificationsService,
               private translateService: TranslateService) {
   }
 
+  /**
+   * The item for which showing the orcid settings
+   */
+  @Input()
+  set item(item: Item) {
+    this.#item$.next(item);
+  }
+
+  ngOnDestroy(): void {
+    this.#destroy$.next();
+  }
+
   /**
    * Init orcid settings form
    */
@@ -106,20 +123,21 @@ export class OrcidSyncSettingsComponent implements OnInit {
         };
       });
 
-    const syncProfilePreferences = this.item.allMetadataValues('dspace.orcid.sync-profile');
+    this.updateSyncProfileOptions(this.#item$.asObservable());
+    this.updateSyncPreferences(this.#item$.asObservable());
 
-    this.syncProfileOptions = ['BIOGRAPHICAL', 'IDENTIFIERS']
+    this.#researcherProfile$ =
-      .map((value) => {
+      this.#item$.pipe(
-        return {
+        switchMap(item =>
-          label: this.messagePrefix + '.sync-profile.' + value.toLowerCase(),
+          this.researcherProfileService.findByRelatedItem(item)
-          value: value,
+            .pipe(
-          checked: syncProfilePreferences.includes(value)
+              getFirstCompletedRemoteData(),
-        };
+              catchError((err: HttpErrorResponse) => of(createFailedRemoteDataObject<ResearcherProfile>(err.message, err.status))),
-      });
+              getRemoteDataPayload(),
-
+            )
-    this.currentSyncMode = this.getCurrentPreference('dspace.orcid.sync-mode', ['BATCH', 'MANUAL'], 'MANUAL');
+        ),
-    this.currentSyncPublications = this.getCurrentPreference('dspace.orcid.sync-publications', ['DISABLED', 'ALL'], 'DISABLED');
+        takeUntil(this.#destroy$)
-    this.currentSyncFunding = this.getCurrentPreference('dspace.orcid.sync-fundings', ['DISABLED', 'ALL'], 'DISABLED');
+      );
   }
 
   /**
@@ -144,37 +162,84 @@ export class OrcidSyncSettingsComponent implements OnInit {
       return;
     }
 
-    this.researcherProfileService.findByRelatedItem(this.item).pipe(
+    this.#researcherProfile$
-      getFirstCompletedRemoteData(),
+      .pipe(
-      switchMap((profileRD: RemoteData<ResearcherProfile>) => {
+        switchMap(researcherProfile => this.researcherProfileService.patch(researcherProfile, operations)),
-        if (profileRD.hasSucceeded) {
+        getFirstCompletedRemoteData(),
-          return this.researcherProfileService.patch(profileRD.payload, operations).pipe(
+        catchError((err: HttpErrorResponse) => of(createFailedRemoteDataObject(err.message, err.status))),
-            getFirstCompletedRemoteData(),
+        take(1)
-          );
+      )
+      .subscribe((remoteData: RemoteData<ResearcherProfile>) => {
+        if (remoteData.hasFailed) {
+          this.notificationsService.error(this.translateService.get(this.messagePrefix + '.synchronization-settings-update.error'));
         } else {
-          return of(profileRD);
+          this.notificationsService.success(this.translateService.get(this.messagePrefix + '.synchronization-settings-update.success'));
+          this.settingsUpdated.emit();
         }
-      }),
+      });
-    ).subscribe((remoteData: RemoteData<ResearcherProfile>) => {
+  }
-      if (remoteData.isSuccess) {
+
-        this.notificationsService.success(this.translateService.get(this.messagePrefix + '.synchronization-settings-update.success'));
+  /**
-        this.settingsUpdated.emit();
+   *
-      } else {
+   * Handles subscriptions to populate sync preferences
-        this.notificationsService.error(this.translateService.get(this.messagePrefix + '.synchronization-settings-update.error'));
+   *
-      }
+   * @param item observable that emits update on item changes
-    });
+   * @private
+   */
+  private updateSyncPreferences(item: Observable<Item>) {
+    item.pipe(
+      filter(hasValue),
+      map(i => this.getCurrentPreference(i, 'dspace.orcid.sync-mode', ['BATCH', 'MANUAL'], 'MANUAL')),
+      takeUntil(this.#destroy$)
+    ).subscribe(val => this.currentSyncMode = val);
+    item.pipe(
+      filter(hasValue),
+      map(i => this.getCurrentPreference(i, 'dspace.orcid.sync-publications', ['DISABLED', 'ALL'], 'DISABLED')),
+      takeUntil(this.#destroy$)
+    ).subscribe(val => this.currentSyncPublications = val);
+    item.pipe(
+      filter(hasValue),
+      map(i => this.getCurrentPreference(i, 'dspace.orcid.sync-fundings', ['DISABLED', 'ALL'], 'DISABLED')),
+      takeUntil(this.#destroy$)
+    ).subscribe(val => this.currentSyncFunding = val);
+  }
+
+  /**
+   * Handles subscription to populate the {@link syncProfileOptions} field
+   *
+   * @param item observable that emits update on item changes
+   * @private
+   */
+  private updateSyncProfileOptions(item: Observable<Item>) {
+    item.pipe(
+      filter(hasValue),
+      map(i => i.allMetadataValues('dspace.orcid.sync-profile')),
+      map(metadata =>
+        ['BIOGRAPHICAL', 'IDENTIFIERS']
+          .map((value) => {
+            return {
+              label: this.messagePrefix + '.sync-profile.' + value.toLowerCase(),
+              value: value,
+              checked: metadata.includes(value)
+            };
+          })
+      ),
+      takeUntil(this.#destroy$)
+    )
+      .subscribe(value => this.syncProfileOptions = value);
   }
 
   /**
    * Retrieve setting saved in the item's metadata
    *
+   * @param item The item from which retrieve settings
    * @param metadataField The metadata name that contains setting
    * @param allowedValues The allowed values
    * @param defaultValue  The default value
    * @private
    */
-  private getCurrentPreference(metadataField: string, allowedValues: string[], defaultValue: string): string {
+  private getCurrentPreference(item: Item, metadataField: string, allowedValues: string[], defaultValue: string): string {
-    const currentPreference = this.item.firstMetadataValue(metadataField);
+    const currentPreference = item.firstMetadataValue(metadataField);
     return (currentPreference && allowedValues.includes(currentPreference)) ? currentPreference : defaultValue;
   }