From bb770ba65b7941f936d8e657771a88cb11994aeb Mon Sep 17 00:00:00 2001 From: Vincenzo Mecca Date: Wed, 10 Jul 2024 11:34:18 +0200 Subject: [PATCH] [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 --- .../orcid-auth/orcid-auth.component.ts | 15 +- .../orcid-page/orcid-page.component.ts | 18 +- .../orcid-sync-settings.component.spec.ts | 10 +- .../orcid-sync-settings.component.ts | 165 ++++++++++++------ 4 files changed, 144 insertions(+), 64 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 index ea970e7d31..73b4a7b4e1 100644 --- 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 @@ -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 { map } from 'rxjs/operators'; +import { BehaviorSubject, Observable, of } from 'rxjs'; +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) => { 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 index f3dbb569d9..1d62c9691c 100644 --- a/src/app/item-page/orcid-page/orcid-page.component.ts +++ b/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.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 index f2fa9d2440..38a6df909e 100644 --- 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 @@ -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; let scheduler: TestScheduler; - let researcherProfileService: jasmine.SpyObj; let notificationsService; + let researcherProfileService: jasmine.SpyObj; 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)); 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 index 0bcbc295ac..422041d340 100644 --- 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 @@ -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 { switchMap } from 'rxjs/operators'; +import { BehaviorSubject, Observable, of } from 'rxjs'; +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 { - - /** - * The item for which showing the orcid settings - */ - @Input() item: Item; +export class OrcidSyncSettingsComponent implements OnInit, OnDestroy { /** * 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 = new EventEmitter(); + /** + * Emitter that triggers onDestroy lifecycle + * @private + */ + readonly #destroy$ = new EventEmitter(); + /** + * {@link BehaviorSubject} that reflects {@link item} input changes + * @private + */ + readonly #item$ = new BehaviorSubject(null); + /** + * {@link Observable} that contains {@link ResearcherProfile} linked to the {@link #item$} + * @private + */ + #researcherProfile$: Observable; 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'] - .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((err: HttpErrorResponse) => of(createFailedRemoteDataObject(err.message, err.status))), + getRemoteDataPayload(), + ) + ), + takeUntil(this.#destroy$) + ); } /** @@ -144,37 +162,84 @@ export class OrcidSyncSettingsComponent implements OnInit { return; } - this.researcherProfileService.findByRelatedItem(this.item).pipe( - getFirstCompletedRemoteData(), - switchMap((profileRD: RemoteData) => { - if (profileRD.hasSucceeded) { - return this.researcherProfileService.patch(profileRD.payload, operations).pipe( - getFirstCompletedRemoteData(), - ); + this.#researcherProfile$ + .pipe( + switchMap(researcherProfile => this.researcherProfileService.patch(researcherProfile, operations)), + getFirstCompletedRemoteData(), + catchError((err: HttpErrorResponse) => of(createFailedRemoteDataObject(err.message, err.status))), + take(1) + ) + .subscribe((remoteData: RemoteData) => { + 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) => { - 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.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.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; }