From be3fceb185450c9d7cb19bfb72670dbdb0c570bb Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 26 May 2022 12:48:27 +0200 Subject: [PATCH] [CST-5307] Address feedback --- .../researcher-profile.service.spec.ts | 18 +++++--- .../profile/researcher-profile.service.ts | 44 +++++++------------ ...on-search-result-list-element.component.ts | 22 +++++----- .../profile-claim-item-modal.component.ts | 5 ++- .../profile-claim.service.spec.ts | 4 +- .../profile-claim/profile-claim.service.ts | 10 ++--- .../profile-page-researcher-form.component.ts | 22 ++++++---- .../person-page-claim-button.component.ts | 1 - 8 files changed, 62 insertions(+), 64 deletions(-) diff --git a/src/app/core/profile/researcher-profile.service.spec.ts b/src/app/core/profile/researcher-profile.service.spec.ts index 103bae2719..3a8d7b6017 100644 --- a/src/app/core/profile/researcher-profile.service.spec.ts +++ b/src/app/core/profile/researcher-profile.service.spec.ts @@ -11,7 +11,11 @@ import { HALEndpointService } from '../shared/hal-endpoint.service'; import { RequestService } from '../data/request.service'; import { PageInfo } from '../shared/page-info.model'; import { buildPaginatedList } from '../data/paginated-list.model'; -import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { + createNoContentRemoteDataObject$, + createSuccessfulRemoteDataObject, + createSuccessfulRemoteDataObject$ +} from '../../shared/remote-data.utils'; import { RestResponse } from '../cache/response.models'; import { RequestEntry } from '../data/request-entry.model'; import { ResearcherProfileService } from './researcher-profile.service'; @@ -223,7 +227,7 @@ describe('ResearcherProfileService', () => { describe('without a related item', () => { beforeEach(() => { - (service as any).itemService.findByHref.and.returnValue(createSuccessfulRemoteDataObject$(null)); + (service as any).itemService.findByHref.and.returnValue(createNoContentRemoteDataObject$()); }); it('should proxy the call to dataservice.findById with eperson UUID', () => { @@ -233,10 +237,10 @@ describe('ResearcherProfileService', () => { expect((service as any).itemService.findByHref).toHaveBeenCalledWith(researcherProfile._links.item.href, false); }); - it('should return a ResearcherProfile object with the given id', () => { + it('should not return a ResearcherProfile object with the given id', () => { const result = service.findRelatedItemId(researcherProfile); const expected = cold('(a|)', { - a: undefined + a: null }); expect(result).toBeObservable(expected); }); @@ -246,7 +250,7 @@ describe('ResearcherProfileService', () => { describe('setVisibility', () => { let patchSpy; beforeEach(() => { - spyOn((service as any), 'patch').and.returnValue(createSuccessfulRemoteDataObject$(researcherProfilePatched)); + spyOn((service as any).dataService, 'patch').and.returnValue(createSuccessfulRemoteDataObject$(researcherProfilePatched)); spyOn((service as any), 'findById').and.returnValue(createSuccessfulRemoteDataObject$(researcherProfilePatched)); }); @@ -260,14 +264,14 @@ describe('ResearcherProfileService', () => { scheduler.schedule(() => service.setVisibility(researcherProfile, true)); scheduler.flush(); - expect((service as any).patch).toHaveBeenCalledWith(researcherProfile, [replaceOperation]); + expect((service as any).dataService.patch).toHaveBeenCalledWith(researcherProfile, [replaceOperation]); }); }); describe('createFromExternalSource', () => { let patchSpy; beforeEach(() => { - spyOn((service as any), 'patch').and.returnValue(createSuccessfulRemoteDataObject$(researcherProfilePatched)); + spyOn((service as any).dataService, 'patch').and.returnValue(createSuccessfulRemoteDataObject$(researcherProfilePatched)); spyOn((service as any), 'findById').and.returnValue(createSuccessfulRemoteDataObject$(researcherProfilePatched)); }); diff --git a/src/app/core/profile/researcher-profile.service.ts b/src/app/core/profile/researcher-profile.service.ts index 0c39396950..23656f3f71 100644 --- a/src/app/core/profile/researcher-profile.service.ts +++ b/src/app/core/profile/researcher-profile.service.ts @@ -4,9 +4,9 @@ import { Injectable } from '@angular/core'; import { Router } from '@angular/router'; import { Store } from '@ngrx/store'; -import { Operation, ReplaceOperation } from 'fast-json-patch'; -import { Observable, of as observableOf } from 'rxjs'; -import { catchError, find, map, tap } from 'rxjs/operators'; +import { ReplaceOperation } from 'fast-json-patch'; +import { Observable } from 'rxjs'; +import { find, map, tap } from 'rxjs/operators'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { dataService } from '../cache/builders/build-decorators'; @@ -19,11 +19,7 @@ import { RemoteData } from '../data/remote-data'; import { RequestService } from '../data/request.service'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { NoContent } from '../shared/NoContent.model'; -import { - getAllCompletedRemoteData, - getFirstCompletedRemoteData, - getFirstSucceededRemoteDataPayload -} from '../shared/operators'; +import { getAllCompletedRemoteData, getFirstCompletedRemoteData } from '../shared/operators'; import { ResearcherProfile } from './model/researcher-profile.model'; import { RESEARCHER_PROFILE } from './model/researcher-profile.resource-type'; import { HttpOptions } from '../dspace-rest/dspace-rest.service'; @@ -31,6 +27,7 @@ import { PostRequest } from '../data/request.models'; import { hasValue } from '../../shared/empty.util'; import { CoreState } from '../core-state.model'; import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; +import { Item } from '../shared/item.model'; /** * A private DataService implementation to delegate specific methods to. @@ -126,14 +123,10 @@ export class ResearcherProfileService { * @param researcherProfile the profile to find for */ public findRelatedItemId(researcherProfile: ResearcherProfile): Observable { - return this.itemService.findByHref(researcherProfile._links.item.href, false) - .pipe(getFirstSucceededRemoteDataPayload(), - catchError((error) => { - console.debug(error); - return observableOf(null); - }), - map((item) => item?.id) - ); + return this.itemService.findByHref(researcherProfile._links.item.href, false).pipe( + getFirstCompletedRemoteData(), + map((itemRD: RemoteData) => (itemRD.hasSucceeded && itemRD.payload) ? itemRD.payload.id : null) + ); } /** @@ -149,7 +142,7 @@ export class ResearcherProfileService { value: visible }; - return this.patch(researcherProfile, [replaceOperation]); + return this.dataService.patch(researcherProfile, [replaceOperation]); } /** @@ -166,19 +159,12 @@ export class ResearcherProfileService { const href$ = this.halService.getEndpoint(this.dataService.getLinkPath()); href$.pipe( - find((href: string) => hasValue(href)), - map((href: string) => { - const request = new PostRequest(requestId, href, sourceUri, options); - this.requestService.send(request); - }) - ).subscribe(); + find((href: string) => hasValue(href)) + ).subscribe((endpoint: string) => { + const request = new PostRequest(requestId, endpoint, sourceUri, options); + this.requestService.send(request); + }); return this.rdbService.buildFromRequestUUID(requestId); } - - private patch(researcherProfile: ResearcherProfile, operations: Operation[]): Observable> { - return this.dataService.patch(researcherProfile, operations).pipe( - getFirstCompletedRemoteData() - ); - } } diff --git a/src/app/entity-groups/research-entities/item-list-elements/search-result-list-elements/person/person-search-result-list-element.component.ts b/src/app/entity-groups/research-entities/item-list-elements/search-result-list-elements/person/person-search-result-list-element.component.ts index 32785d7b8a..3b222ce27c 100644 --- a/src/app/entity-groups/research-entities/item-list-elements/search-result-list-elements/person/person-search-result-list-element.component.ts +++ b/src/app/entity-groups/research-entities/item-list-elements/search-result-list-elements/person/person-search-result-list-element.component.ts @@ -1,10 +1,13 @@ import { Component } from '@angular/core'; -import { listableObjectComponent } from '../../../../../shared/object-collection/shared/listable-object/listable-object.decorator'; +import { + listableObjectComponent +} from '../../../../../shared/object-collection/shared/listable-object/listable-object.decorator'; import { ViewMode } from '../../../../../core/shared/view-mode.model'; -import { ItemSearchResultListElementComponent } from '../../../../../shared/object-list/search-result-list-element/item-search-result/item-types/item/item-search-result-list-element.component'; -import {TruncatableService} from '../../../../../shared/truncatable/truncatable.service'; -import {DSONameService} from '../../../../../core/breadcrumbs/dso-name.service'; -import {isNotEmpty} from '../../../../../shared/empty.util'; +import { + ItemSearchResultListElementComponent +} from '../../../../../shared/object-list/search-result-list-element/item-search-result/item-types/item/item-search-result-list-element.component'; +import { TruncatableService } from '../../../../../shared/truncatable/truncatable.service'; +import { DSONameService } from '../../../../../core/breadcrumbs/dso-name.service'; @listableObjectComponent('PersonSearchResult', ViewMode.ListElement) @Component({ @@ -21,11 +24,10 @@ export class PersonSearchResultListElementComponent extends ItemSearchResultList super(truncatableService, dsoNameService); } + /** + * Return the person name + */ get name() { - let personName = this.dsoNameService.getName(this.dso); - if (isNotEmpty(this.firstMetadataValue('person.familyName')) && isNotEmpty(this.firstMetadataValue('person.givenName'))) { - personName = this.firstMetadataValue('person.familyName') + ', ' + this.firstMetadataValue('person.givenName'); - } - return personName; + return this.dsoNameService.getName(this.dso); } } diff --git a/src/app/profile-page/profile-claim-item-modal/profile-claim-item-modal.component.ts b/src/app/profile-page/profile-claim-item-modal/profile-claim-item-modal.component.ts index 01138bb07f..6838704560 100644 --- a/src/app/profile-page/profile-claim-item-modal/profile-claim-item-modal.component.ts +++ b/src/app/profile-page/profile-claim-item-modal/profile-claim-item-modal.component.ts @@ -14,6 +14,7 @@ import { ViewMode } from '../../core/shared/view-mode.model'; import { ProfileClaimService } from '../profile-claim/profile-claim.service'; import { CollectionElementLinkType } from '../../shared/object-collection/collection-element-link.type'; import { SearchObjects } from '../../shared/search/models/search-objects.model'; +import { getFirstCompletedRemoteData } from '../../core/shared/operators'; /** * Component representing a modal that show a list of suggested profile item to claim @@ -63,7 +64,9 @@ export class ProfileClaimItemModalComponent extends DSOSelectorModalWrapperCompo * Retrieve suggested profiles, if any */ ngOnInit(): void { - this.profileClaimService.searchForSuggestions(this.dso as EPerson).subscribe( + this.profileClaimService.searchForSuggestions(this.dso as EPerson).pipe( + getFirstCompletedRemoteData(), + ).subscribe( (result: RemoteData>) => this.listEntries$.next(result) ); } diff --git a/src/app/profile-page/profile-claim/profile-claim.service.spec.ts b/src/app/profile-page/profile-claim/profile-claim.service.spec.ts index a06934231b..e2a9ff1461 100644 --- a/src/app/profile-page/profile-claim/profile-claim.service.spec.ts +++ b/src/app/profile-page/profile-claim/profile-claim.service.spec.ts @@ -8,7 +8,7 @@ import { SearchService } from '../../core/shared/search/search.service'; import { ItemSearchResult } from '../../shared/object-collection/shared/item-search-result.model'; import { SearchObjects } from '../../shared/search/models/search-objects.model'; import { Item } from '../../core/shared/item.model'; -import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; +import { createNoContentRemoteDataObject, createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; import { EPerson } from '../../core/eperson/models/eperson.model'; describe('ProfileClaimService', () => { @@ -204,7 +204,7 @@ describe('ProfileClaimService', () => { it('should return null', () => { const result = service.searchForSuggestions(null); const expected = cold('(a|)', { - a: null + a: createNoContentRemoteDataObject() }); expect(result).toBeObservable(expected); }); diff --git a/src/app/profile-page/profile-claim/profile-claim.service.ts b/src/app/profile-page/profile-claim/profile-claim.service.ts index 806f48a2f6..ee54d0c3be 100644 --- a/src/app/profile-page/profile-claim/profile-claim.service.ts +++ b/src/app/profile-page/profile-claim/profile-claim.service.ts @@ -11,6 +11,7 @@ import { isEmpty, isNotEmpty } from '../../shared/empty.util'; import { PaginatedSearchOptions } from '../../shared/search/models/paginated-search-options.model'; import { getFirstCompletedRemoteData } from '../../core/shared/operators'; import { SearchObjects } from '../../shared/search/models/search-objects.model'; +import { createNoContentRemoteDataObject } from '../../shared/remote-data.utils'; /** * Service that handle profiles claim. @@ -28,6 +29,7 @@ export class ProfileClaimService { */ hasProfilesToSuggest(eperson: EPerson): Observable { return this.searchForSuggestions(eperson).pipe( + getFirstCompletedRemoteData(), map((rd: RemoteData>) => { return isNotEmpty(rd) && rd.hasSucceeded && rd.payload?.page?.length > 0; }) @@ -42,7 +44,7 @@ export class ProfileClaimService { searchForSuggestions(eperson: EPerson): Observable>> { const query = this.personQueryData(eperson); if (isEmpty(query)) { - return of(null); + return of(createNoContentRemoteDataObject() as RemoteData>); } return this.lookup(query); } @@ -54,14 +56,12 @@ export class ProfileClaimService { */ private lookup(query: string): Observable>> { if (isEmpty(query)) { - return of(null); + return of(createNoContentRemoteDataObject() as RemoteData>); } return this.searchService.search(new PaginatedSearchOptions({ configuration: 'eperson_claims', query: query - })).pipe( - getFirstCompletedRemoteData() - ); + })); } /** diff --git a/src/app/profile-page/profile-page-researcher-form/profile-page-researcher-form.component.ts b/src/app/profile-page/profile-page-researcher-form/profile-page-researcher-form.component.ts index b0a2c90de4..bffec746d3 100644 --- a/src/app/profile-page/profile-page-researcher-form/profile-page-researcher-form.component.ts +++ b/src/app/profile-page/profile-page-researcher-form/profile-page-researcher-form.component.ts @@ -15,6 +15,7 @@ import { ResearcherProfile } from '../../core/profile/model/researcher-profile.m import { ResearcherProfileService } from '../../core/profile/researcher-profile.service'; import { ProfileClaimService } from '../profile-claim/profile-claim.service'; import { RemoteData } from '../../core/data/remote-data'; +import { isNotEmpty } from '../../shared/empty.util'; @Component({ selector: 'ds-profile-page-researcher-form', @@ -128,14 +129,15 @@ export class ProfilePageResearcherFormComponent implements OnInit { * @param researcherProfile the profile to update */ toggleProfileVisibility(researcherProfile: ResearcherProfile): void { - this.researcherProfileService.setVisibility(researcherProfile, !researcherProfile.visible) - .subscribe((rd: RemoteData) => { - if (rd.hasSucceeded) { - this.researcherProfile$.next(rd.payload); - } else { - this.notificationService.error(null, this.translationService.get('researcher.profile.change-visibility.fail')); - } - }); + this.researcherProfileService.setVisibility(researcherProfile, !researcherProfile.visible).pipe( + getFirstCompletedRemoteData() + ).subscribe((rd: RemoteData) => { + if (rd.hasSucceeded) { + this.researcherProfile$.next(rd.payload); + } else { + this.notificationService.error(null, this.translationService.get('researcher.profile.change-visibility.fail')); + } + }); } /** @@ -183,7 +185,9 @@ export class ProfilePageResearcherFormComponent implements OnInit { tap((researcherProfile) => this.researcherProfile$.next(researcherProfile)), mergeMap((researcherProfile) => this.researcherProfileService.findRelatedItemId(researcherProfile)), ).subscribe((itemId: string) => { - this.researcherProfileItemId = itemId; + if (isNotEmpty(itemId)) { + this.researcherProfileItemId = itemId; + } }); } diff --git a/src/app/shared/dso-page/person-page-claim-button/person-page-claim-button.component.ts b/src/app/shared/dso-page/person-page-claim-button/person-page-claim-button.component.ts index d2e8e888e1..f317b88c8d 100644 --- a/src/app/shared/dso-page/person-page-claim-button/person-page-claim-button.component.ts +++ b/src/app/shared/dso-page/person-page-claim-button/person-page-claim-button.component.ts @@ -55,7 +55,6 @@ export class PersonPageClaimButtonComponent implements OnInit { this.researcherProfileService.createFromExternalSource(this.object._links.self.href).pipe( getFirstCompletedRemoteData(), mergeMap((rd: RemoteData) => { - console.log(rd); if (rd.hasSucceeded) { return this.researcherProfileService.findRelatedItemId(rd.payload); } else {