[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
DSpace · Jul 25, 2024 · 1a498f8 · 1a498f8
1 parent 02e1e28
commit 1a498f8
Show file tree

Hide file tree

Showing 5 changed files with 177 additions and 49 deletions.
diff --git a/src/app/item-page/orcid-page/orcid-auth/orcid-auth.component.ts b/src/app/item-page/orcid-page/orcid-auth/orcid-auth.component.ts
@@ -19,6 +19,7 @@ import {
 } from '@ngx-translate/core';
 import {
   BehaviorSubject,
+  catchError,
   Observable,
 } from 'rxjs';
 import { map } from 'rxjs/operators';
@@ -34,6 +35,7 @@ import { Item } from '../../../core/shared/item.model';
 import { getFirstCompletedRemoteData } from '../../../core/shared/operators';
 import { AlertComponent } from '../../../shared/alert/alert.component';
 import { NotificationsService } from '../../../shared/notifications/notifications.service';
+import { createFailedRemoteDataObjectFromError$ } from '../../../shared/remote-data.utils';
 
 @Component({
   selector: 'ds-orcid-auth',
@@ -203,13 +205,14 @@ export class OrcidAuthComponent implements OnInit, OnChanges {
     this.unlinkProcessing.next(true);
     this.orcidAuthService.unlinkOrcidByItem(this.item).pipe(
       getFirstCompletedRemoteData(),
+      catchError(createFailedRemoteDataObjectFromError$<ResearcherProfile>),
     ).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'));
       }
     });
   }

diff --git a/src/app/item-page/orcid-page/orcid-page.component.ts b/src/app/item-page/orcid-page/orcid-page.component.ts
@@ -20,6 +20,7 @@ import {
   combineLatest,
 } from 'rxjs';
 import {
+  filter,
   map,
   take,
 } from 'rxjs/operators';
@@ -187,8 +188,20 @@ export class OrcidPageComponent implements OnInit {
    */
   private clearRouteParams(): void {
     // update route removing the code from query params
-    const redirectUrl = this.router.url.split('?')[0];
-    this.router.navigate([redirectUrl]);
+    this.route.queryParamMap
+      .pipe(
+        filter((paramMap: ParamMap) => isNotEmpty(paramMap.keys)),
+        map(_ => Object.assign({})),
+        take(1),
+      ).subscribe(queryParams =>
+        this.router.navigate(
+          [],
+          {
+            relativeTo: this.route,
+            queryParams,
+          },
+        ),
+      );
   }
 
 }
diff --git a/src/app/item-page/orcid-page/orcid-sync-settings/orcid-sync-settings.component.spec.ts b/src/app/item-page/orcid-page/orcid-sync-settings/orcid-sync-settings.component.spec.ts
@@ -180,6 +180,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();
   }));
@@ -216,7 +217,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[] = [
         {
@@ -245,7 +245,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));
@@ -257,6 +256,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();
@@ -266,7 +267,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));

diff --git a/src/app/item-page/orcid-page/orcid-sync-settings/orcid-sync-settings.component.ts b/src/app/item-page/orcid-page/orcid-sync-settings/orcid-sync-settings.component.ts
@@ -3,6 +3,7 @@ import {
   Component,
   EventEmitter,
   Input,
+  OnDestroy,
   OnInit,
   Output,
 } from '@angular/core';
@@ -15,17 +16,32 @@ import {
   TranslateService,
 } from '@ngx-translate/core';
 import { Operation } from 'fast-json-patch';
-import { of } from 'rxjs';
-import { switchMap } from 'rxjs/operators';
+import {
+  BehaviorSubject,
+  Observable,
+} from 'rxjs';
+import {
+  catchError,
+  filter,
+  map,
+  switchMap,
+  take,
+  takeUntil,
+} from 'rxjs/operators';
 
 import { RemoteData } from '../../../core/data/remote-data';
 import { ResearcherProfile } from '../../../core/profile/model/researcher-profile.model';
 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 { AlertComponent } from '../../../shared/alert/alert.component';
 import { AlertType } from '../../../shared/alert/alert-type';
+import { hasValue } from '../../../shared/empty.util';
 import { NotificationsService } from '../../../shared/notifications/notifications.service';
+import { createFailedRemoteDataObjectFromError$ } from '../../../shared/remote-data.utils';
 
 @Component({
   selector: 'ds-orcid-sync-setting',
@@ -39,14 +55,9 @@ import { NotificationsService } from '../../../shared/notifications/notification
   ],
   standalone: true,
 })
-export class OrcidSyncSettingsComponent implements OnInit {
+export class OrcidSyncSettingsComponent implements OnInit, OnDestroy {
   protected readonly AlertType = AlertType;
 
-  /**
-   * The item for which showing the orcid settings
-   */
-  @Input() item: Item;
-
   /**
    * The prefix used for i18n keys
    */
@@ -91,12 +102,39 @@ export class OrcidSyncSettingsComponent implements OnInit {
    * 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
    */
@@ -128,20 +166,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']
-      .map((value) => {
-        return {
-          label: this.messagePrefix + '.sync-profile.' + value.toLowerCase(),
-          value: value,
-          checked: syncProfilePreferences.includes(value),
-        };
-      });
-
-    this.currentSyncMode = this.getCurrentPreference('dspace.orcid.sync-mode', ['BATCH', 'MANUAL'], 'MANUAL');
-    this.currentSyncPublications = this.getCurrentPreference('dspace.orcid.sync-publications', ['DISABLED', 'ALL'], 'DISABLED');
-    this.currentSyncFunding = this.getCurrentPreference('dspace.orcid.sync-fundings', ['DISABLED', 'ALL'], 'DISABLED');
+    this.#researcherProfile$ =
+      this.#item$.pipe(
+        switchMap(item =>
+          this.researcherProfileService.findByRelatedItem(item)
+            .pipe(
+              getFirstCompletedRemoteData(),
+              catchError(createFailedRemoteDataObjectFromError$<ResearcherProfile>),
+              getRemoteDataPayload(),
+            ),
+        ),
+        takeUntil(this.#destroy$),
+      );
   }
 
   /**
@@ -166,37 +205,84 @@ export class OrcidSyncSettingsComponent implements OnInit {
       return;
     }
 
-    this.researcherProfileService.findByRelatedItem(this.item).pipe(
-      getFirstCompletedRemoteData(),
-      switchMap((profileRD: RemoteData<ResearcherProfile>) => {
-        if (profileRD.hasSucceeded) {
-          return this.researcherProfileService.patch(profileRD.payload, operations).pipe(
-            getFirstCompletedRemoteData(),
-          );
+    this.#researcherProfile$
+      .pipe(
+        switchMap(researcherProfile => this.researcherProfileService.patch(researcherProfile, operations)),
+        getFirstCompletedRemoteData(),
+        catchError(createFailedRemoteDataObjectFromError$<ResearcherProfile>),
+        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 {
-        this.notificationsService.error(this.translateService.get(this.messagePrefix + '.synchronization-settings-update.error'));
-      }
-    });
+      });
+  }
+
+  /**
+   *
+   * Handles subscriptions to populate sync preferences
+   *
+   * @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 {
-    const currentPreference = this.item.firstMetadataValue(metadataField);
+  private getCurrentPreference(item: Item, metadataField: string, allowedValues: string[], defaultValue: string): string {
+    const currentPreference = item.firstMetadataValue(metadataField);
     return (currentPreference && allowedValues.includes(currentPreference)) ? currentPreference : defaultValue;
   }
 
@@ -216,3 +302,4 @@ export class OrcidSyncSettingsComponent implements OnInit {
   }
 
 }
+
diff --git a/src/app/shared/remote-data.utils.ts b/src/app/shared/remote-data.utils.ts
@@ -1,3 +1,4 @@
+import { HttpErrorResponse } from '@angular/common/http';
 import {
   Observable,
   of as observableOf,
@@ -107,3 +108,27 @@ export function createNoContentRemoteDataObject<T>(timeCompleted?: number): Remo
 export function createNoContentRemoteDataObject$<T>(timeCompleted?: number): Observable<RemoteData<T>> {
   return createSuccessfulRemoteDataObject$(undefined, timeCompleted);
 }
+
+/**
+ * Method to create a remote data object that has failed starting from a given error
+ *
+ * @param error
+ */
+export function createFailedRemoteDataObjectFromError<T>(error: unknown): RemoteData<T> {
+  const remoteData = createFailedRemoteDataObject<T>();
+  if (error instanceof Error) {
+    remoteData.errorMessage = error.message;
+  }
+  if (error instanceof HttpErrorResponse) {
+    remoteData.statusCode = error.status;
+  }
+  return remoteData;
+}
+
+/**
+ * Method to create a remote data object that has failed starting from a given error
+ * @param error
+ */
+export function createFailedRemoteDataObjectFromError$<T>(error: unknown): Observable<RemoteData<T>> {
+  return observableOf(createFailedRemoteDataObjectFromError<T>(error));
+}