[CST-5307] Address feedback

This commit is contained in:
Giuseppe Digilio
2022-05-26 12:48:27 +02:00
parent ebbae79854
commit be3fceb185
8 changed files with 62 additions and 64 deletions

View File

@@ -11,7 +11,11 @@ import { HALEndpointService } from '../shared/hal-endpoint.service';
import { RequestService } from '../data/request.service'; import { RequestService } from '../data/request.service';
import { PageInfo } from '../shared/page-info.model'; import { PageInfo } from '../shared/page-info.model';
import { buildPaginatedList } from '../data/paginated-list.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 { RestResponse } from '../cache/response.models';
import { RequestEntry } from '../data/request-entry.model'; import { RequestEntry } from '../data/request-entry.model';
import { ResearcherProfileService } from './researcher-profile.service'; import { ResearcherProfileService } from './researcher-profile.service';
@@ -223,7 +227,7 @@ describe('ResearcherProfileService', () => {
describe('without a related item', () => { describe('without a related item', () => {
beforeEach(() => { 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', () => { 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); 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 result = service.findRelatedItemId(researcherProfile);
const expected = cold('(a|)', { const expected = cold('(a|)', {
a: undefined a: null
}); });
expect(result).toBeObservable(expected); expect(result).toBeObservable(expected);
}); });
@@ -246,7 +250,7 @@ describe('ResearcherProfileService', () => {
describe('setVisibility', () => { describe('setVisibility', () => {
let patchSpy; let patchSpy;
beforeEach(() => { 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)); spyOn((service as any), 'findById').and.returnValue(createSuccessfulRemoteDataObject$(researcherProfilePatched));
}); });
@@ -260,14 +264,14 @@ describe('ResearcherProfileService', () => {
scheduler.schedule(() => service.setVisibility(researcherProfile, true)); scheduler.schedule(() => service.setVisibility(researcherProfile, true));
scheduler.flush(); scheduler.flush();
expect((service as any).patch).toHaveBeenCalledWith(researcherProfile, [replaceOperation]); expect((service as any).dataService.patch).toHaveBeenCalledWith(researcherProfile, [replaceOperation]);
}); });
}); });
describe('createFromExternalSource', () => { describe('createFromExternalSource', () => {
let patchSpy; let patchSpy;
beforeEach(() => { 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)); spyOn((service as any), 'findById').and.returnValue(createSuccessfulRemoteDataObject$(researcherProfilePatched));
}); });

View File

@@ -4,9 +4,9 @@ import { Injectable } from '@angular/core';
import { Router } from '@angular/router'; import { Router } from '@angular/router';
import { Store } from '@ngrx/store'; import { Store } from '@ngrx/store';
import { Operation, ReplaceOperation } from 'fast-json-patch'; import { ReplaceOperation } from 'fast-json-patch';
import { Observable, of as observableOf } from 'rxjs'; import { Observable } from 'rxjs';
import { catchError, find, map, tap } from 'rxjs/operators'; import { find, map, tap } from 'rxjs/operators';
import { NotificationsService } from '../../shared/notifications/notifications.service'; import { NotificationsService } from '../../shared/notifications/notifications.service';
import { dataService } from '../cache/builders/build-decorators'; import { dataService } from '../cache/builders/build-decorators';
@@ -19,11 +19,7 @@ import { RemoteData } from '../data/remote-data';
import { RequestService } from '../data/request.service'; import { RequestService } from '../data/request.service';
import { HALEndpointService } from '../shared/hal-endpoint.service'; import { HALEndpointService } from '../shared/hal-endpoint.service';
import { NoContent } from '../shared/NoContent.model'; import { NoContent } from '../shared/NoContent.model';
import { import { getAllCompletedRemoteData, getFirstCompletedRemoteData } from '../shared/operators';
getAllCompletedRemoteData,
getFirstCompletedRemoteData,
getFirstSucceededRemoteDataPayload
} from '../shared/operators';
import { ResearcherProfile } from './model/researcher-profile.model'; import { ResearcherProfile } from './model/researcher-profile.model';
import { RESEARCHER_PROFILE } from './model/researcher-profile.resource-type'; import { RESEARCHER_PROFILE } from './model/researcher-profile.resource-type';
import { HttpOptions } from '../dspace-rest/dspace-rest.service'; 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 { hasValue } from '../../shared/empty.util';
import { CoreState } from '../core-state.model'; import { CoreState } from '../core-state.model';
import { FollowLinkConfig } from '../../shared/utils/follow-link-config.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. * A private DataService implementation to delegate specific methods to.
@@ -126,14 +123,10 @@ export class ResearcherProfileService {
* @param researcherProfile the profile to find for * @param researcherProfile the profile to find for
*/ */
public findRelatedItemId(researcherProfile: ResearcherProfile): Observable<string> { public findRelatedItemId(researcherProfile: ResearcherProfile): Observable<string> {
return this.itemService.findByHref(researcherProfile._links.item.href, false) return this.itemService.findByHref(researcherProfile._links.item.href, false).pipe(
.pipe(getFirstSucceededRemoteDataPayload(), getFirstCompletedRemoteData(),
catchError((error) => { map((itemRD: RemoteData<Item>) => (itemRD.hasSucceeded && itemRD.payload) ? itemRD.payload.id : null)
console.debug(error); );
return observableOf(null);
}),
map((item) => item?.id)
);
} }
/** /**
@@ -149,7 +142,7 @@ export class ResearcherProfileService {
value: visible 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()); const href$ = this.halService.getEndpoint(this.dataService.getLinkPath());
href$.pipe( href$.pipe(
find((href: string) => hasValue(href)), find((href: string) => hasValue(href))
map((href: string) => { ).subscribe((endpoint: string) => {
const request = new PostRequest(requestId, href, sourceUri, options); const request = new PostRequest(requestId, endpoint, sourceUri, options);
this.requestService.send(request); this.requestService.send(request);
}) });
).subscribe();
return this.rdbService.buildFromRequestUUID(requestId); return this.rdbService.buildFromRequestUUID(requestId);
} }
private patch(researcherProfile: ResearcherProfile, operations: Operation[]): Observable<RemoteData<ResearcherProfile>> {
return this.dataService.patch(researcherProfile, operations).pipe(
getFirstCompletedRemoteData()
);
}
} }

View File

@@ -1,10 +1,13 @@
import { Component } from '@angular/core'; 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 { 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 {
import {TruncatableService} from '../../../../../shared/truncatable/truncatable.service'; ItemSearchResultListElementComponent
import {DSONameService} from '../../../../../core/breadcrumbs/dso-name.service'; } from '../../../../../shared/object-list/search-result-list-element/item-search-result/item-types/item/item-search-result-list-element.component';
import {isNotEmpty} from '../../../../../shared/empty.util'; import { TruncatableService } from '../../../../../shared/truncatable/truncatable.service';
import { DSONameService } from '../../../../../core/breadcrumbs/dso-name.service';
@listableObjectComponent('PersonSearchResult', ViewMode.ListElement) @listableObjectComponent('PersonSearchResult', ViewMode.ListElement)
@Component({ @Component({
@@ -21,11 +24,10 @@ export class PersonSearchResultListElementComponent extends ItemSearchResultList
super(truncatableService, dsoNameService); super(truncatableService, dsoNameService);
} }
/**
* Return the person name
*/
get name() { get name() {
let personName = this.dsoNameService.getName(this.dso); return 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;
} }
} }

View File

@@ -14,6 +14,7 @@ import { ViewMode } from '../../core/shared/view-mode.model';
import { ProfileClaimService } from '../profile-claim/profile-claim.service'; import { ProfileClaimService } from '../profile-claim/profile-claim.service';
import { CollectionElementLinkType } from '../../shared/object-collection/collection-element-link.type'; import { CollectionElementLinkType } from '../../shared/object-collection/collection-element-link.type';
import { SearchObjects } from '../../shared/search/models/search-objects.model'; 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 * 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 * Retrieve suggested profiles, if any
*/ */
ngOnInit(): void { ngOnInit(): void {
this.profileClaimService.searchForSuggestions(this.dso as EPerson).subscribe( this.profileClaimService.searchForSuggestions(this.dso as EPerson).pipe(
getFirstCompletedRemoteData(),
).subscribe(
(result: RemoteData<SearchObjects<DSpaceObject>>) => this.listEntries$.next(result) (result: RemoteData<SearchObjects<DSpaceObject>>) => this.listEntries$.next(result)
); );
} }

View File

@@ -8,7 +8,7 @@ import { SearchService } from '../../core/shared/search/search.service';
import { ItemSearchResult } from '../../shared/object-collection/shared/item-search-result.model'; import { ItemSearchResult } from '../../shared/object-collection/shared/item-search-result.model';
import { SearchObjects } from '../../shared/search/models/search-objects.model'; import { SearchObjects } from '../../shared/search/models/search-objects.model';
import { Item } from '../../core/shared/item.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'; import { EPerson } from '../../core/eperson/models/eperson.model';
describe('ProfileClaimService', () => { describe('ProfileClaimService', () => {
@@ -204,7 +204,7 @@ describe('ProfileClaimService', () => {
it('should return null', () => { it('should return null', () => {
const result = service.searchForSuggestions(null); const result = service.searchForSuggestions(null);
const expected = cold('(a|)', { const expected = cold('(a|)', {
a: null a: createNoContentRemoteDataObject()
}); });
expect(result).toBeObservable(expected); expect(result).toBeObservable(expected);
}); });

View File

@@ -11,6 +11,7 @@ import { isEmpty, isNotEmpty } from '../../shared/empty.util';
import { PaginatedSearchOptions } from '../../shared/search/models/paginated-search-options.model'; import { PaginatedSearchOptions } from '../../shared/search/models/paginated-search-options.model';
import { getFirstCompletedRemoteData } from '../../core/shared/operators'; import { getFirstCompletedRemoteData } from '../../core/shared/operators';
import { SearchObjects } from '../../shared/search/models/search-objects.model'; import { SearchObjects } from '../../shared/search/models/search-objects.model';
import { createNoContentRemoteDataObject } from '../../shared/remote-data.utils';
/** /**
* Service that handle profiles claim. * Service that handle profiles claim.
@@ -28,6 +29,7 @@ export class ProfileClaimService {
*/ */
hasProfilesToSuggest(eperson: EPerson): Observable<boolean> { hasProfilesToSuggest(eperson: EPerson): Observable<boolean> {
return this.searchForSuggestions(eperson).pipe( return this.searchForSuggestions(eperson).pipe(
getFirstCompletedRemoteData(),
map((rd: RemoteData<SearchObjects<DSpaceObject>>) => { map((rd: RemoteData<SearchObjects<DSpaceObject>>) => {
return isNotEmpty(rd) && rd.hasSucceeded && rd.payload?.page?.length > 0; return isNotEmpty(rd) && rd.hasSucceeded && rd.payload?.page?.length > 0;
}) })
@@ -42,7 +44,7 @@ export class ProfileClaimService {
searchForSuggestions(eperson: EPerson): Observable<RemoteData<SearchObjects<DSpaceObject>>> { searchForSuggestions(eperson: EPerson): Observable<RemoteData<SearchObjects<DSpaceObject>>> {
const query = this.personQueryData(eperson); const query = this.personQueryData(eperson);
if (isEmpty(query)) { if (isEmpty(query)) {
return of(null); return of(createNoContentRemoteDataObject() as RemoteData<SearchObjects<DSpaceObject>>);
} }
return this.lookup(query); return this.lookup(query);
} }
@@ -54,14 +56,12 @@ export class ProfileClaimService {
*/ */
private lookup(query: string): Observable<RemoteData<SearchObjects<DSpaceObject>>> { private lookup(query: string): Observable<RemoteData<SearchObjects<DSpaceObject>>> {
if (isEmpty(query)) { if (isEmpty(query)) {
return of(null); return of(createNoContentRemoteDataObject() as RemoteData<SearchObjects<DSpaceObject>>);
} }
return this.searchService.search(new PaginatedSearchOptions({ return this.searchService.search(new PaginatedSearchOptions({
configuration: 'eperson_claims', configuration: 'eperson_claims',
query: query query: query
})).pipe( }));
getFirstCompletedRemoteData()
);
} }
/** /**

View File

@@ -15,6 +15,7 @@ import { ResearcherProfile } from '../../core/profile/model/researcher-profile.m
import { ResearcherProfileService } from '../../core/profile/researcher-profile.service'; import { ResearcherProfileService } from '../../core/profile/researcher-profile.service';
import { ProfileClaimService } from '../profile-claim/profile-claim.service'; import { ProfileClaimService } from '../profile-claim/profile-claim.service';
import { RemoteData } from '../../core/data/remote-data'; import { RemoteData } from '../../core/data/remote-data';
import { isNotEmpty } from '../../shared/empty.util';
@Component({ @Component({
selector: 'ds-profile-page-researcher-form', selector: 'ds-profile-page-researcher-form',
@@ -128,14 +129,15 @@ export class ProfilePageResearcherFormComponent implements OnInit {
* @param researcherProfile the profile to update * @param researcherProfile the profile to update
*/ */
toggleProfileVisibility(researcherProfile: ResearcherProfile): void { toggleProfileVisibility(researcherProfile: ResearcherProfile): void {
this.researcherProfileService.setVisibility(researcherProfile, !researcherProfile.visible) this.researcherProfileService.setVisibility(researcherProfile, !researcherProfile.visible).pipe(
.subscribe((rd: RemoteData<ResearcherProfile>) => { getFirstCompletedRemoteData()
if (rd.hasSucceeded) { ).subscribe((rd: RemoteData<ResearcherProfile>) => {
this.researcherProfile$.next(rd.payload); if (rd.hasSucceeded) {
} else { this.researcherProfile$.next(rd.payload);
this.notificationService.error(null, this.translationService.get('researcher.profile.change-visibility.fail')); } 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)), tap((researcherProfile) => this.researcherProfile$.next(researcherProfile)),
mergeMap((researcherProfile) => this.researcherProfileService.findRelatedItemId(researcherProfile)), mergeMap((researcherProfile) => this.researcherProfileService.findRelatedItemId(researcherProfile)),
).subscribe((itemId: string) => { ).subscribe((itemId: string) => {
this.researcherProfileItemId = itemId; if (isNotEmpty(itemId)) {
this.researcherProfileItemId = itemId;
}
}); });
} }

View File

@@ -55,7 +55,6 @@ export class PersonPageClaimButtonComponent implements OnInit {
this.researcherProfileService.createFromExternalSource(this.object._links.self.href).pipe( this.researcherProfileService.createFromExternalSource(this.object._links.self.href).pipe(
getFirstCompletedRemoteData(), getFirstCompletedRemoteData(),
mergeMap((rd: RemoteData<ResearcherProfile>) => { mergeMap((rd: RemoteData<ResearcherProfile>) => {
console.log(rd);
if (rd.hasSucceeded) { if (rd.hasSucceeded) {
return this.researcherProfileService.findRelatedItemId(rd.payload); return this.researcherProfileService.findRelatedItemId(rd.payload);
} else { } else {