From 17d1f2e6acbe683961d89b775e8f8acee5afc84f Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Thu, 11 Apr 2024 17:51:59 +0200 Subject: [PATCH 01/12] also add relationships one by one on the edit item relationships tab (cherry picked from commit 8e59b7d0b0798ed7f7f7fdc952dbcfe1c9f773b6) --- .../core/data/relationship-data.service.ts | 24 ++- .../item-relationships.component.ts | 173 ++++++++++-------- 2 files changed, 120 insertions(+), 77 deletions(-) diff --git a/src/app/core/data/relationship-data.service.ts b/src/app/core/data/relationship-data.service.ts index 17d0fa6634..f0164b839d 100644 --- a/src/app/core/data/relationship-data.service.ts +++ b/src/app/core/data/relationship-data.service.ts @@ -155,8 +155,11 @@ export class RelationshipDataService extends IdentifiableDataService> { + deleteRelationship(id: string, copyVirtualMetadata: string, shouldRefresh = true): Observable> { return this.getRelationshipEndpoint(id).pipe( isNotEmptyOperator(), take(1), @@ -167,7 +170,11 @@ export class RelationshipDataService extends IdentifiableDataService this.rdbService.buildFromRequestUUID(restRequest.uuid)), getFirstCompletedRemoteData(), - tap(() => this.refreshRelationshipItemsInCacheByRelationship(id)), + tap(() => { + if (shouldRefresh) { + this.refreshRelationshipItemsInCacheByRelationship(id); + } + }), ); } @@ -178,8 +185,11 @@ export class RelationshipDataService extends IdentifiableDataService> { + addRelationship(typeId: string, item1: Item, item2: Item, leftwardValue?: string, rightwardValue?: string, shouldRefresh = true): Observable> { const options: HttpOptions = Object.create({}); let headers = new HttpHeaders(); headers = headers.append('Content-Type', 'text/uri-list'); @@ -194,8 +204,12 @@ export class RelationshipDataService extends IdentifiableDataService this.rdbService.buildFromRequestUUID(restRequest.uuid)), getFirstCompletedRemoteData(), - tap(() => this.refreshRelationshipItemsInCache(item1)), - tap(() => this.refreshRelationshipItemsInCache(item2)), + tap(() => { + if (shouldRefresh) { + this.refreshRelationshipItemsInCache(item1); + this.refreshRelationshipItemsInCache(item2); + } + }), ) as Observable>; } diff --git a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts index ea9b571cd6..3cf6601f0e 100644 --- a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -17,16 +17,17 @@ import { TranslateService, } from '@ngx-translate/core'; import { + BehaviorSubject, combineLatest as observableCombineLatest, + EMPTY, Observable, - of as observableOf, - zip as observableZip, } from 'rxjs'; import { + concatMap, map, - startWith, switchMap, take, + toArray, } from 'rxjs/operators'; import { ObjectCacheService } from '../../../core/cache/object-cache.service'; @@ -91,7 +92,7 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { /** * The item's entity type as an observable */ - entityType$: Observable; + entityType$: BehaviorSubject = new BehaviorSubject(undefined); constructor( public itemService: ItemDataService, @@ -123,13 +124,13 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { map((relationshipTypes: PaginatedList) => relationshipTypes.page), ); - this.entityType$ = this.entityTypeService.getEntityTypeByLabel(label).pipe( + this.entityTypeService.getEntityTypeByLabel(label).pipe( getFirstSucceededRemoteData(), getRemoteDataPayload(), - ); + ).subscribe((type) => this.entityType$.next(type)); } else { - this.entityType$ = observableOf(undefined); + this.entityType$.next(undefined); } } @@ -147,8 +148,7 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { public submit(): void { // Get all the relationships that should be removed - const removedRelationshipIDs$: Observable = this.relationshipService.getItemRelationshipsArray(this.item).pipe( - startWith([]), + const removeUpdates$: Observable = this.relationshipService.getItemRelationshipsArray(this.item).pipe( map((relationships: Relationship[]) => relationships.map((relationship) => Object.assign(new Relationship(), relationship, { uuid: relationship.id }), )), @@ -157,84 +157,113 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { }), map((fieldUpdates: FieldUpdates) => Object.values(fieldUpdates) - .filter((fieldUpdate: FieldUpdate) => fieldUpdate.changeType === FieldChangeType.REMOVE) - .map((fieldUpdate: FieldUpdate) => fieldUpdate.field as DeleteRelationship), + .filter((fieldUpdate: FieldUpdate) => fieldUpdate.changeType === FieldChangeType.REMOVE), ), + take(1), ); - const addRelatedItems$: Observable = this.objectUpdatesService.getFieldUpdates(this.url, []).pipe( + const addUpdates$: Observable = this.objectUpdatesService.getFieldUpdates(this.url, []).pipe( map((fieldUpdates: FieldUpdates) => Object.values(fieldUpdates) .filter((fieldUpdate: FieldUpdate) => hasValue(fieldUpdate)) - .filter((fieldUpdate: FieldUpdate) => fieldUpdate.changeType === FieldChangeType.ADD) - .map((fieldUpdate: FieldUpdate) => fieldUpdate.field as RelationshipIdentifiable), + .filter((fieldUpdate: FieldUpdate) => fieldUpdate.changeType === FieldChangeType.ADD), ), + take(1), ); - observableCombineLatest( - removedRelationshipIDs$, - addRelatedItems$, - ).pipe( + observableCombineLatest([ + removeUpdates$, + addUpdates$, + ]).pipe( take(1), - ).subscribe(([removeRelationshipIDs, addRelatedItems]) => { - const actions = [ - this.deleteRelationships(removeRelationshipIDs), - this.addRelationships(addRelatedItems), - ]; - actions.forEach((action) => - action.subscribe((response) => { - if (response.length > 0) { - this.initializeOriginalFields(); - this.cdr.detectChanges(); - this.displayNotifications(response); - this.modalService.dismissAll(); - } - }), - ); + switchMap(([removeUpdates, addUpdates]) => [...removeUpdates, ...addUpdates]), + concatMap((update: FieldUpdate) => { + if (update.changeType === FieldChangeType.REMOVE) { + return this.deleteRelationship(update.field as DeleteRelationship).pipe(take(1)); + } else if (update.changeType === FieldChangeType.ADD) { + return this.addRelationship(update.field as RelationshipIdentifiable).pipe( + take(1), + switchMap((relationshipRD: RemoteData) => { + if (relationshipRD.hasSucceeded) { + // Set the newly related item to stale, so its relationships will update to include + // the new one. Only set the current item to stale at the very end so we only do it + // once + const { leftItem, rightItem } = relationshipRD.payload._links; + if (leftItem.href === this.item.self) { + return this.itemService.invalidateByHref(rightItem.href).pipe( + // when it's invalidated, emit the original relationshipRD for use in the pipe below + map(() => relationshipRD), + ); + } else { + return this.itemService.invalidateByHref(leftItem.href).pipe( + // when it's invalidated, emit the original relationshipRD for use in the pipe below + map(() => relationshipRD), + ); + } + } else { + return [relationshipRD]; + } + }), + ); + } else { + return EMPTY; + } + }), + toArray(), + switchMap((responses) => { + // once all relationships are made and all related items have been invalidated, invalidate + // the current item + return this.itemService.invalidateByHref(this.item.self).pipe( + map(() => responses), + ); + }), + ).subscribe((responses) => { + if (responses.length > 0) { + this.initializeOriginalFields(); + this.displayNotifications(responses); + this.modalService.dismissAll(); + } }); } - deleteRelationships(deleteRelationshipIDs: DeleteRelationship[]): Observable[]> { - return observableZip(...deleteRelationshipIDs.map((deleteRelationship) => { - let copyVirtualMetadata: string; - if (deleteRelationship.keepLeftVirtualMetadata && deleteRelationship.keepRightVirtualMetadata) { - copyVirtualMetadata = 'all'; - } else if (deleteRelationship.keepLeftVirtualMetadata) { - copyVirtualMetadata = 'left'; - } else if (deleteRelationship.keepRightVirtualMetadata) { - copyVirtualMetadata = 'right'; - } else { - copyVirtualMetadata = 'none'; - } - return this.relationshipService.deleteRelationship(deleteRelationship.uuid, copyVirtualMetadata); - }, - )); + deleteRelationship(deleteRelationship: DeleteRelationship): Observable> { + let copyVirtualMetadata: string; + if (deleteRelationship.keepLeftVirtualMetadata && deleteRelationship.keepRightVirtualMetadata) { + copyVirtualMetadata = 'all'; + } else if (deleteRelationship.keepLeftVirtualMetadata) { + copyVirtualMetadata = 'left'; + } else if (deleteRelationship.keepRightVirtualMetadata) { + copyVirtualMetadata = 'right'; + } else { + copyVirtualMetadata = 'none'; + } + + return this.relationshipService.deleteRelationship(deleteRelationship.uuid, copyVirtualMetadata, false); } - addRelationships(addRelatedItems: RelationshipIdentifiable[]): Observable[]> { - return observableZip(...addRelatedItems.map((addRelationship) => - this.entityType$.pipe( - switchMap((entityType) => this.entityTypeService.isLeftType(addRelationship.type, entityType)), - switchMap((isLeftType) => { - let leftItem: Item; - let rightItem: Item; - let leftwardValue: string; - let rightwardValue: string; - if (isLeftType) { - leftItem = this.item; - rightItem = addRelationship.relatedItem; - leftwardValue = null; - rightwardValue = addRelationship.nameVariant; - } else { - leftItem = addRelationship.relatedItem; - rightItem = this.item; - leftwardValue = addRelationship.nameVariant; - rightwardValue = null; - } - return this.relationshipService.addRelationship(addRelationship.type.id, leftItem, rightItem, leftwardValue, rightwardValue); - }), - ), - )); + addRelationship(addRelationship: RelationshipIdentifiable): Observable> { + return this.entityType$.pipe( + switchMap((entityType) => this.entityTypeService.isLeftType(addRelationship.type, entityType)), + switchMap((isLeftType) => { + let leftItem: Item; + let rightItem: Item; + let leftwardValue: string; + let rightwardValue: string; + if (isLeftType) { + leftItem = this.item; + rightItem = addRelationship.relatedItem; + leftwardValue = null; + rightwardValue = addRelationship.nameVariant; + } else { + leftItem = addRelationship.relatedItem; + rightItem = this.item; + leftwardValue = addRelationship.nameVariant; + rightwardValue = null; + } + return this.relationshipService.addRelationship(addRelationship.type.id, leftItem, rightItem, leftwardValue, rightwardValue, false); + }), + ); + } /** From 814c81b7b7b82b1890f4b006e58150bf723a5acd Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Thu, 25 Apr 2024 10:56:54 +0200 Subject: [PATCH 02/12] update test (cherry picked from commit e4b098e64dd98af3d87c8f9f2f28f9c3ea3c0336) --- .../item-relationships/item-relationships.component.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts index 106edb08ef..4cd3061744 100644 --- a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts +++ b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts @@ -285,7 +285,7 @@ describe('ItemRelationshipsComponent', () => { }); it('it should delete the correct relationship', () => { - expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationships[1].uuid, 'left'); + expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationships[1].uuid, 'left', false); }); }); From b8a4c83353aa2be8b27daae03e892cbadf5a9eed Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 8 May 2024 17:48:33 +0200 Subject: [PATCH 03/12] fix issue where a submit emitted from the edit relationship modal wouldn't arrive in the edit relationships component (cherry picked from commit 9f3ee328580719006a08b3bfd8bcbedf35944d12) --- src/app/app.component.ts | 6 +- .../object-updates/object-updates.reducer.ts | 2 + .../core/data/relationship-data.service.ts | 8 +- .../edit-item-relationships.service.spec.ts | 16 ++ .../edit-item-relationships.service.ts | 196 ++++++++++++++++++ .../edit-relationship-list.component.ts | 134 ++++++------ .../edit-relationship.component.ts | 6 +- .../item-relationships.component.html | 3 +- .../item-relationships.component.ts | 166 +-------------- 9 files changed, 302 insertions(+), 235 deletions(-) create mode 100644 src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.spec.ts create mode 100644 src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.ts diff --git a/src/app/app.component.ts b/src/app/app.component.ts index cdf45f50a8..b87073c034 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -32,6 +32,7 @@ import { Observable, } from 'rxjs'; import { + delay, distinctUntilChanged, take, withLatestFrom, @@ -136,7 +137,10 @@ export class AppComponent implements OnInit, AfterViewInit { } ngAfterViewInit() { - this.router.events.subscribe((event) => { + this.router.events.pipe( + // delay(0) to prevent "Expression has changed after it was checked" errors + delay(0), + ).subscribe((event) => { if (event instanceof NavigationStart) { distinctNext(this.isRouteLoading$, true); } else if ( diff --git a/src/app/core/data/object-updates/object-updates.reducer.ts b/src/app/core/data/object-updates/object-updates.reducer.ts index e014889850..cadae9ae83 100644 --- a/src/app/core/data/object-updates/object-updates.reducer.ts +++ b/src/app/core/data/object-updates/object-updates.reducer.ts @@ -61,6 +61,8 @@ export interface VirtualMetadataSource { export interface RelationshipIdentifiable extends Identifiable { nameVariant?: string; + originalItem: Item; + originalIsLeft: boolean relatedItem: Item; relationship: Relationship; type: RelationshipType; diff --git a/src/app/core/data/relationship-data.service.ts b/src/app/core/data/relationship-data.service.ts index f0164b839d..43c56f1a32 100644 --- a/src/app/core/data/relationship-data.service.ts +++ b/src/app/core/data/relationship-data.service.ts @@ -350,7 +350,13 @@ export class RelationshipDataService extends IdentifiableDataService { + let service: EditItemRelationshipsService; + + beforeEach(() => { + TestBed.configureTestingModule({}); + service = TestBed.inject(EditItemRelationshipsService); + }); + + it('should be created', () => { + expect(service).toBeTruthy(); + }); +}); diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.ts b/src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.ts new file mode 100644 index 0000000000..c2dab99756 --- /dev/null +++ b/src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.ts @@ -0,0 +1,196 @@ +import { Injectable } from '@angular/core'; +import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; +import { TranslateService } from '@ngx-translate/core'; +import { + EMPTY, + Observable, +} from 'rxjs'; +import { + concatMap, + map, + switchMap, + take, + toArray, +} from 'rxjs/operators'; + +import { EntityTypeDataService } from '../../../core/data/entity-type-data.service'; +import { ItemDataService } from '../../../core/data/item-data.service'; +import { FieldChangeType } from '../../../core/data/object-updates/field-change-type.model'; +import { FieldUpdate } from '../../../core/data/object-updates/field-update.model'; +import { FieldUpdates } from '../../../core/data/object-updates/field-updates.model'; +import { + DeleteRelationship, + RelationshipIdentifiable, +} from '../../../core/data/object-updates/object-updates.reducer'; +import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service'; +import { RelationshipDataService } from '../../../core/data/relationship-data.service'; +import { RemoteData } from '../../../core/data/remote-data'; +import { Item } from '../../../core/shared/item.model'; +import { Relationship } from '../../../core/shared/item-relationships/relationship.model'; +import { NoContent } from '../../../core/shared/NoContent.model'; +import { hasValue } from '../../../shared/empty.util'; +import { NotificationsService } from '../../../shared/notifications/notifications.service'; + +@Injectable({ + providedIn: 'root', +}) +export class EditItemRelationshipsService { + public notificationsPrefix = 'static-pages.form.notification'; + + constructor( + public itemService: ItemDataService, + public objectUpdatesService: ObjectUpdatesService, + public notificationsService: NotificationsService, + protected modalService: NgbModal, + public relationshipService: RelationshipDataService, + public entityTypeService: EntityTypeDataService, + public translateService: TranslateService, + ) { } + + + /** + * Resolve the currently selected related items back to relationships and send a delete request for each of the relationships found + * Make sure the lists are refreshed afterwards and notifications are sent for success and errors + */ + public submit(item: Item, url: string): void { + this.objectUpdatesService.getFieldUpdates(url, [], true).pipe( + map((fieldUpdates: FieldUpdates) => + Object.values(fieldUpdates) + .filter((fieldUpdate: FieldUpdate) => hasValue(fieldUpdate)) + .filter((fieldUpdate: FieldUpdate) => fieldUpdate.changeType === FieldChangeType.ADD || fieldUpdate.changeType === FieldChangeType.REMOVE), + ), + take(1), + // emit each update in the array separately + switchMap((updates) => updates), + // process each update one by one, while waiting for the previous to finish + concatMap((update: FieldUpdate) => { + if (update.changeType === FieldChangeType.REMOVE) { + return this.deleteRelationship(update.field as DeleteRelationship).pipe(take(1)); + } else if (update.changeType === FieldChangeType.ADD) { + return this.addRelationship(update.field as RelationshipIdentifiable).pipe( + take(1), + switchMap((relationshipRD: RemoteData) => { + if (relationshipRD.hasSucceeded) { + // Set the newly related item to stale, so its relationships will update to include + // the new one. Only set the current item to stale at the very end so we only do it + // once + const { leftItem, rightItem } = relationshipRD.payload._links; + if (leftItem.href === item.self) { + return this.itemService.invalidateByHref(rightItem.href).pipe( + // when it's invalidated, emit the original relationshipRD for use in the pipe below + map(() => relationshipRD), + ); + } else { + return this.itemService.invalidateByHref(leftItem.href).pipe( + // when it's invalidated, emit the original relationshipRD for use in the pipe below + map(() => relationshipRD), + ); + } + } else { + return [relationshipRD]; + } + }), + ); + } else { + return EMPTY; + } + }), + toArray(), + switchMap((responses) => { + // once all relationships are made and all related items have been invalidated, invalidate + // the current item + return this.itemService.invalidateByHref(item.self).pipe( + map(() => responses), + ); + }), + ).subscribe((responses) => { + if (responses.length > 0) { + this.initializeOriginalFields(item, url); + this.displayNotifications(responses); + this.modalService.dismissAll(); + } + }); + } + + /** + * Sends all initial values of this item to the object updates service + */ + public initializeOriginalFields(item: Item, url: string) { + return this.relationshipService.getRelatedItems(item).pipe( + take(1), + ).subscribe((items: Item[]) => { + this.objectUpdatesService.initialize(url, items, item.lastModified); + }); + } + + deleteRelationship(deleteRelationship: DeleteRelationship): Observable> { + let copyVirtualMetadata: string; + if (deleteRelationship.keepLeftVirtualMetadata && deleteRelationship.keepRightVirtualMetadata) { + copyVirtualMetadata = 'all'; + } else if (deleteRelationship.keepLeftVirtualMetadata) { + copyVirtualMetadata = 'left'; + } else if (deleteRelationship.keepRightVirtualMetadata) { + copyVirtualMetadata = 'right'; + } else { + copyVirtualMetadata = 'none'; + } + + return this.relationshipService.deleteRelationship(deleteRelationship.uuid, copyVirtualMetadata, false); + } + + addRelationship(addRelationship: RelationshipIdentifiable): Observable> { + let leftItem: Item; + let rightItem: Item; + let leftwardValue: string; + let rightwardValue: string; + if (addRelationship.originalIsLeft) { + leftItem = addRelationship.originalItem; + rightItem = addRelationship.relatedItem; + leftwardValue = null; + rightwardValue = addRelationship.nameVariant; + } else { + leftItem = addRelationship.relatedItem; + rightItem = addRelationship.originalItem; + leftwardValue = addRelationship.nameVariant; + rightwardValue = null; + } + return this.relationshipService.addRelationship(addRelationship.type.id, leftItem, rightItem, leftwardValue, rightwardValue, false); + } + + /** + * Display notifications + * - Error notification for each failed response with their message + * - Success notification in case there's at least one successful response + * @param responses + */ + displayNotifications(responses: RemoteData[]) { + const failedResponses = responses.filter((response: RemoteData) => response.hasFailed); + const successfulResponses = responses.filter((response: RemoteData) => response.hasSucceeded); + + failedResponses.forEach((response: RemoteData) => { + this.notificationsService.error(this.getNotificationTitle('failed'), response.errorMessage); + }); + if (successfulResponses.length > 0) { + this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved')); + } + } + + + + /** + * Get translated notification title + * @param key + */ + getNotificationTitle(key: string) { + return this.translateService.instant(this.notificationsPrefix + key + '.title'); + } + + /** + * Get translated notification content + * @param key + */ + getNotificationContent(key: string) { + return this.translateService.instant(this.notificationsPrefix + key + '.content'); + + } +} diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts index d9029a7af4..740d95e473 100644 --- a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts +++ b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts @@ -21,11 +21,13 @@ import { TranslateModule } from '@ngx-translate/core'; import { BehaviorSubject, combineLatest as observableCombineLatest, + EMPTY, from as observableFrom, Observable, Subscription, } from 'rxjs'; import { + concatMap, defaultIfEmpty, map, mergeMap, @@ -78,6 +80,7 @@ import { FollowLinkConfig } from '../../../../shared/utils/follow-link-config.mo import { ObjectValuesPipe } from '../../../../shared/utils/object-values-pipe'; import { itemLinksToFollow } from '../../../../shared/utils/relation-query.utils'; import { VarDirective } from '../../../../shared/utils/var.directive'; +import { EditItemRelationshipsService } from '../edit-item-relationships.service'; import { EditRelationshipComponent } from '../edit-relationship/edit-relationship.component'; @Component({ @@ -141,7 +144,7 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { * Observable that emits true if {@link itemType} is on the left-hand side of {@link relationshipType}, * false if it is on the right-hand side and undefined in the rare case that it is on neither side. */ - private currentItemIsLeftItem$: Observable; + private currentItemIsLeftItem$: BehaviorSubject = new BehaviorSubject(undefined); private relatedEntityType$: Observable; @@ -206,6 +209,7 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { protected modalService: NgbModal, protected paginationService: PaginationService, protected selectableListService: SelectableListService, + protected editItemRelationshipsService: EditItemRelationshipsService, @Inject(APP_CONFIG) protected appConfig: AppConfig, ) { this.fetchThumbnail = this.appConfig.browseBy.showThumbnails; @@ -240,7 +244,6 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { * Open the dynamic lookup modal to search for items to add as relationships */ openLookup() { - this.modalRef = this.modalService.open(DsDynamicLookupRelationModalComponent, { size: 'lg', }); @@ -306,51 +309,59 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { modalComp.submitEv = () => { - - const subscriptions = []; - - modalComp.toAdd.forEach((searchResult: SearchResult) => { - const relatedItem = searchResult.indexableObject; - subscriptions.push(this.relationshipService.getNameVariant(this.listId, relatedItem.uuid).pipe( - map((nameVariant) => { - const update = { - uuid: this.relationshipType.id + '-' + searchResult.indexableObject.uuid, - nameVariant, - type: this.relationshipType, - relatedItem, - } as RelationshipIdentifiable; - this.objectUpdatesService.saveAddFieldUpdate(this.url, update); - return update; - }), - )); - }); - - modalComp.toRemove.forEach( (searchResult) => { - subscriptions.push(this.relationshipService.getNameVariant(this.listId, searchResult.indexableObjectuuid).pipe( - switchMap((nameVariant) => { - return this.getRelationFromId(searchResult.indexableObject).pipe( - map( (relationship: Relationship) => { + modalComp.isPending = true; + const isLeft = this.currentItemIsLeftItem$.getValue(); + const addOperations = modalComp.toAdd.map((searchResult: any) => ({ type: 'add', searchResult })); + const removeOperations = modalComp.toRemove.map((searchResult: any) => ({ type: 'remove', searchResult })); + observableFrom([...addOperations, ...removeOperations]).pipe( + concatMap(({ type, searchResult }: { type: string, searchResult: any }) => { + if (type === 'add') { + const relatedItem = searchResult.indexableObject; + return this.relationshipService.getNameVariant(this.listId, relatedItem.uuid).pipe( + map((nameVariant) => { const update = { - uuid: relationship.id, + uuid: this.relationshipType.id + '-' + searchResult.indexableObject.uuid, nameVariant, type: this.relationshipType, - relationship, + originalIsLeft: isLeft, + originalItem: this.item, + relatedItem, } as RelationshipIdentifiable; - this.objectUpdatesService.saveRemoveFieldUpdate(this.url,update); + this.objectUpdatesService.saveAddFieldUpdate(this.url, update); return update; }), + take(1), ); - }), - )); - }); - - observableCombineLatest(subscriptions).subscribe( (res) => { - // Wait until the states changes since there are multiple items - setTimeout( () => { + } else if (type === 'remove') { + return this.relationshipService.getNameVariant(this.listId, searchResult.indexableObjectuuid).pipe( + switchMap((nameVariant) => { + return this.getRelationFromId(searchResult.indexableObject).pipe( + map( (relationship: Relationship) => { + const update = { + uuid: relationship.id, + nameVariant, + type: this.relationshipType, + originalIsLeft: isLeft, + originalItem: this.item, + relationship, + } as RelationshipIdentifiable; + this.objectUpdatesService.saveRemoveFieldUpdate(this.url,update); + return update; + }), + ); + }), + take(1), + ); + } else { + return EMPTY; + } + }), + toArray(), + ).subscribe({ + complete: () => { + this.editItemRelationshipsService.submit(this.item, this.url); this.submit.emit(); - },1000); - - modalComp.isPending = true; + }, }); }; @@ -384,27 +395,12 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { } getRelationFromId(relatedItem) { - return this.currentItemIsLeftItem$.pipe( - take(1), - switchMap( isLeft => { - let apiCall; - if (isLeft) { - apiCall = this.relationshipService.searchByItemsAndType( this.relationshipType.id, this.item.uuid, this.relationshipType.leftwardType ,[relatedItem.id] ).pipe( - getFirstSucceededRemoteData(), - getRemoteDataPayload(), - ); - } else { - apiCall = this.relationshipService.searchByItemsAndType( this.relationshipType.id, this.item.uuid, this.relationshipType.rightwardType ,[relatedItem.id] ).pipe( - getFirstSucceededRemoteData(), - getRemoteDataPayload(), - ); - } - - return apiCall.pipe( - map( (res: PaginatedList) => res.page[0]), - ); - }, - )); + const relationshipLabel = this.currentItemIsLeftItem$.getValue() ? this.relationshipType.leftwardType : this.relationshipType.rightwardType; + return this.relationshipService.searchByItemsAndType( this.relationshipType.id, this.item.uuid, relationshipLabel ,[relatedItem.id] ).pipe( + getFirstSucceededRemoteData(), + getRemoteDataPayload(), + map( (res: PaginatedList) => res.page[0]), + ); } @@ -469,7 +465,6 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { } ngOnInit(): void { - // store the left and right type of the relationship in a single observable this.relationshipLeftAndRightType$ = observableCombineLatest([ this.relationshipType.leftType, @@ -490,7 +485,7 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { (relatedEntityType) => this.listId = `edit-relationship-${this.itemType.id}-${relatedEntityType.id}`, ); - this.currentItemIsLeftItem$ = this.relationshipLeftAndRightType$.pipe( + this.subs.push(this.relationshipLeftAndRightType$.pipe( map(([leftType, rightType]: [ItemType, ItemType]) => { if (leftType.id === this.itemType.id) { return true; @@ -504,7 +499,9 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { console.warn(`The item ${this.item.uuid} is not on the right or the left side of relationship type ${this.relationshipType.uuid}`); return undefined; }), - ); + ).subscribe((nextValue: boolean) => { + this.currentItemIsLeftItem$.next(nextValue); + })); this.getRelationshipMessageKey$ = observableCombineLatest( this.getLabel(), @@ -547,19 +544,20 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { currentPagination$, this.currentItemIsLeftItem$, ]).pipe( - switchMap(([currentPagination, currentItemIsLeftItem]: [PaginationComponentOptions, boolean]) => + switchMap(([currentPagination, currentItemIsLeftItem]: [PaginationComponentOptions, boolean]) => { // get the relationships for the current item, relationshiptype and page - this.relationshipService.getItemRelationshipsByLabel( + return this.relationshipService.getItemRelationshipsByLabel( this.item, currentItemIsLeftItem ? this.relationshipType.leftwardType : this.relationshipType.rightwardType, { elementsPerPage: currentPagination.pageSize, currentPage: currentPagination.currentPage, }, - false, + true, true, ...linksToFollow, - )), + ); + }), ).subscribe((rd: RemoteData>) => { this.relationshipsRd$.next(rd); }), @@ -595,6 +593,8 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { uuid: relationship.id, type: this.relationshipType, relationship, + originalIsLeft: isLeftItem, + originalItem: this.item, nameVariant, } as RelationshipIdentifiable; }), diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.ts b/src/app/item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.ts index 5f213327e3..97cfb7a16b 100644 --- a/src/app/item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.ts +++ b/src/app/item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.ts @@ -167,9 +167,9 @@ export class EditRelationshipComponent implements OnChanges { ) as DeleteRelationship; }), take(1), - ).subscribe((deleteRelationship: DeleteRelationship) => - this.objectUpdatesService.saveRemoveFieldUpdate(this.url, deleteRelationship), - ); + ).subscribe((deleteRelationship: DeleteRelationship) => { + this.objectUpdatesService.saveRemoveFieldUpdate(this.url, deleteRelationship); + }); } openVirtualMetadataModal(content: any) { diff --git a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.html b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.html index c3556962d4..e413b5f8ad 100644 --- a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.html +++ b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.html @@ -27,8 +27,7 @@ [item]="item" [itemType]="entityType$ | async" [relationshipType]="relationshipType" - [hasChanges] = hasChanges() - (submit) = submit() + [hasChanges]="hasChanges()" > diff --git a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts index 3cf6601f0e..6c013c8d60 100644 --- a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -18,49 +18,30 @@ import { } from '@ngx-translate/core'; import { BehaviorSubject, - combineLatest as observableCombineLatest, - EMPTY, Observable, } from 'rxjs'; -import { - concatMap, - map, - switchMap, - take, - toArray, -} from 'rxjs/operators'; +import { map } from 'rxjs/operators'; import { ObjectCacheService } from '../../../core/cache/object-cache.service'; import { EntityTypeDataService } from '../../../core/data/entity-type-data.service'; import { ItemDataService } from '../../../core/data/item-data.service'; -import { FieldChangeType } from '../../../core/data/object-updates/field-change-type.model'; -import { FieldUpdate } from '../../../core/data/object-updates/field-update.model'; -import { FieldUpdates } from '../../../core/data/object-updates/field-updates.model'; -import { - DeleteRelationship, - RelationshipIdentifiable, -} from '../../../core/data/object-updates/object-updates.reducer'; import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service'; import { PaginatedList } from '../../../core/data/paginated-list.model'; import { RelationshipDataService } from '../../../core/data/relationship-data.service'; import { RelationshipTypeDataService } from '../../../core/data/relationship-type-data.service'; -import { RemoteData } from '../../../core/data/remote-data'; import { RequestService } from '../../../core/data/request.service'; -import { Item } from '../../../core/shared/item.model'; import { ItemType } from '../../../core/shared/item-relationships/item-type.model'; -import { Relationship } from '../../../core/shared/item-relationships/relationship.model'; import { RelationshipType } from '../../../core/shared/item-relationships/relationship-type.model'; -import { NoContent } from '../../../core/shared/NoContent.model'; import { getFirstSucceededRemoteData, getRemoteDataPayload, } from '../../../core/shared/operators'; -import { hasValue } from '../../../shared/empty.util'; import { ThemedLoadingComponent } from '../../../shared/loading/themed-loading.component'; import { NotificationsService } from '../../../shared/notifications/notifications.service'; import { followLink } from '../../../shared/utils/follow-link-config.model'; import { VarDirective } from '../../../shared/utils/var.directive'; import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component'; +import { EditItemRelationshipsService } from './edit-item-relationships.service'; import { EditRelationshipListComponent } from './edit-relationship-list/edit-relationship-list.component'; @Component({ @@ -108,6 +89,7 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { protected relationshipTypeService: RelationshipTypeDataService, public cdr: ChangeDetectorRef, protected modalService: NgbModal, + protected editItemRelationshipsService: EditItemRelationshipsService, ) { super(itemService, objectUpdatesService, router, notificationsService, translateService, route); } @@ -146,152 +128,14 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { * Make sure the lists are refreshed afterwards and notifications are sent for success and errors */ public submit(): void { - - // Get all the relationships that should be removed - const removeUpdates$: Observable = this.relationshipService.getItemRelationshipsArray(this.item).pipe( - map((relationships: Relationship[]) => relationships.map((relationship) => - Object.assign(new Relationship(), relationship, { uuid: relationship.id }), - )), - switchMap((relationships: Relationship[]) => { - return this.objectUpdatesService.getFieldUpdatesExclusive(this.url, relationships) as Observable; - }), - map((fieldUpdates: FieldUpdates) => - Object.values(fieldUpdates) - .filter((fieldUpdate: FieldUpdate) => fieldUpdate.changeType === FieldChangeType.REMOVE), - ), - take(1), - ); - - const addUpdates$: Observable = this.objectUpdatesService.getFieldUpdates(this.url, []).pipe( - map((fieldUpdates: FieldUpdates) => - Object.values(fieldUpdates) - .filter((fieldUpdate: FieldUpdate) => hasValue(fieldUpdate)) - .filter((fieldUpdate: FieldUpdate) => fieldUpdate.changeType === FieldChangeType.ADD), - ), - take(1), - ); - - observableCombineLatest([ - removeUpdates$, - addUpdates$, - ]).pipe( - take(1), - switchMap(([removeUpdates, addUpdates]) => [...removeUpdates, ...addUpdates]), - concatMap((update: FieldUpdate) => { - if (update.changeType === FieldChangeType.REMOVE) { - return this.deleteRelationship(update.field as DeleteRelationship).pipe(take(1)); - } else if (update.changeType === FieldChangeType.ADD) { - return this.addRelationship(update.field as RelationshipIdentifiable).pipe( - take(1), - switchMap((relationshipRD: RemoteData) => { - if (relationshipRD.hasSucceeded) { - // Set the newly related item to stale, so its relationships will update to include - // the new one. Only set the current item to stale at the very end so we only do it - // once - const { leftItem, rightItem } = relationshipRD.payload._links; - if (leftItem.href === this.item.self) { - return this.itemService.invalidateByHref(rightItem.href).pipe( - // when it's invalidated, emit the original relationshipRD for use in the pipe below - map(() => relationshipRD), - ); - } else { - return this.itemService.invalidateByHref(leftItem.href).pipe( - // when it's invalidated, emit the original relationshipRD for use in the pipe below - map(() => relationshipRD), - ); - } - } else { - return [relationshipRD]; - } - }), - ); - } else { - return EMPTY; - } - }), - toArray(), - switchMap((responses) => { - // once all relationships are made and all related items have been invalidated, invalidate - // the current item - return this.itemService.invalidateByHref(this.item.self).pipe( - map(() => responses), - ); - }), - ).subscribe((responses) => { - if (responses.length > 0) { - this.initializeOriginalFields(); - this.displayNotifications(responses); - this.modalService.dismissAll(); - } - }); + this.editItemRelationshipsService.submit(this.item, this.url); } - deleteRelationship(deleteRelationship: DeleteRelationship): Observable> { - let copyVirtualMetadata: string; - if (deleteRelationship.keepLeftVirtualMetadata && deleteRelationship.keepRightVirtualMetadata) { - copyVirtualMetadata = 'all'; - } else if (deleteRelationship.keepLeftVirtualMetadata) { - copyVirtualMetadata = 'left'; - } else if (deleteRelationship.keepRightVirtualMetadata) { - copyVirtualMetadata = 'right'; - } else { - copyVirtualMetadata = 'none'; - } - - return this.relationshipService.deleteRelationship(deleteRelationship.uuid, copyVirtualMetadata, false); - } - - addRelationship(addRelationship: RelationshipIdentifiable): Observable> { - return this.entityType$.pipe( - switchMap((entityType) => this.entityTypeService.isLeftType(addRelationship.type, entityType)), - switchMap((isLeftType) => { - let leftItem: Item; - let rightItem: Item; - let leftwardValue: string; - let rightwardValue: string; - if (isLeftType) { - leftItem = this.item; - rightItem = addRelationship.relatedItem; - leftwardValue = null; - rightwardValue = addRelationship.nameVariant; - } else { - leftItem = addRelationship.relatedItem; - rightItem = this.item; - leftwardValue = addRelationship.nameVariant; - rightwardValue = null; - } - return this.relationshipService.addRelationship(addRelationship.type.id, leftItem, rightItem, leftwardValue, rightwardValue, false); - }), - ); - - } - - /** - * Display notifications - * - Error notification for each failed response with their message - * - Success notification in case there's at least one successful response - * @param responses - */ - displayNotifications(responses: RemoteData[]) { - const failedResponses = responses.filter((response: RemoteData) => response.hasFailed); - const successfulResponses = responses.filter((response: RemoteData) => response.hasSucceeded); - - failedResponses.forEach((response: RemoteData) => { - this.notificationsService.error(this.getNotificationTitle('failed'), response.errorMessage); - }); - if (successfulResponses.length > 0) { - this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved')); - } - } /** * Sends all initial values of this item to the object updates service */ public initializeOriginalFields() { - return this.relationshipService.getRelatedItems(this.item).pipe( - take(1), - ).subscribe((items: Item[]) => { - this.objectUpdatesService.initialize(this.url, items, this.item.lastModified); - }); + return this.editItemRelationshipsService.initializeOriginalFields(this.item, this.url); } From 65a629387aaa78add36b3b2eea7797d6953c0022 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Thu, 9 May 2024 13:40:03 +0200 Subject: [PATCH 04/12] Merge remote-tracking branch 'alexandre/fix-create-relationship-not-working-between-same-types_contribute-7.6' into w2p-113560_edit-item-add-relationships-one-by-one (cherry picked from commit 1c50dbf4c6199aecc22d8339174d95dfc4992785) --- .../edit-relationship-list.component.html | 2 +- .../edit-relationship-list.component.ts | 129 ++++++------------ .../item-relationships.component.html | 84 ++++++------ .../item-relationships.component.ts | 7 + ...dynamic-lookup-relation-modal.component.ts | 5 +- 5 files changed, 94 insertions(+), 133 deletions(-) diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html index 0a99a5820f..e32938590f 100644 --- a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html +++ b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html @@ -1,5 +1,5 @@

- {{getRelationshipMessageKey$ | async | translate}} + {{relationshipMessageKey$ | async | translate}} + + + +
+
+ +
+
+ + + +
+
+ -
- - -
- -
- - - -
-
- - - -
-
- +
diff --git a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts index de52246cc3..cd2d82087f 100644 --- a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -75,6 +75,10 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { */ entityType$: BehaviorSubject = new BehaviorSubject(undefined); + get isSaving$(): BehaviorSubject { + return this.editItemRelationshipsService.isSaving$; + } + constructor( public itemService: ItemDataService, public objectUpdatesService: ObjectUpdatesService, diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html index e467cc3328..111e2d9994 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html @@ -87,7 +87,7 @@ (click)="submitEv()"> - +  {{"item.edit.metadata.save-button" | translate}} From 5f74c295d3127a5759ede88b66d1d6d0f63dcabd Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Thu, 9 May 2024 14:55:37 +0200 Subject: [PATCH 06/12] fix issue where relationship lists wouldn't update automatically from the second submit onwards (cherry picked from commit 4541788a34500b7d23fed6d1016172eb63c5a8fe) --- src/app/core/data/relationship-data.service.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/app/core/data/relationship-data.service.ts b/src/app/core/data/relationship-data.service.ts index 43c56f1a32..b71ec0b677 100644 --- a/src/app/core/data/relationship-data.service.ts +++ b/src/app/core/data/relationship-data.service.ts @@ -350,13 +350,19 @@ export class RelationshipDataService extends IdentifiableDataService + this.getItemRelationshipsByLabel(item, label, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)), + ); } /** From 71d033bf5054da113403c6a2a424dfb3614f512c Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Thu, 9 May 2024 15:25:37 +0200 Subject: [PATCH 07/12] decrease the number of unnecessary rerenders by checking whether the relationship types have changed (cherry picked from commit a658bf4531e97737e0ace703b042a8e4951212cc) --- .../item-relationships.component.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts index cd2d82087f..2539c3a37d 100644 --- a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -20,7 +20,10 @@ import { BehaviorSubject, Observable, } from 'rxjs'; -import { map } from 'rxjs/operators'; +import { + distinctUntilChanged, + map, +} from 'rxjs/operators'; import { ObjectCacheService } from '../../../core/cache/object-cache.service'; import { EntityTypeDataService } from '../../../core/data/entity-type-data.service'; @@ -40,6 +43,7 @@ import { ThemedLoadingComponent } from '../../../shared/loading/themed-loading.c import { NotificationsService } from '../../../shared/notifications/notifications.service'; import { followLink } from '../../../shared/utils/follow-link-config.model'; import { VarDirective } from '../../../shared/utils/var.directive'; +import { compareArraysUsingIds } from '../../simple/item-types/shared/item-relationships-utils'; import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component'; import { EditItemRelationshipsService } from './edit-item-relationships.service'; import { EditRelationshipListComponent } from './edit-relationship-list/edit-relationship-list.component'; @@ -105,10 +109,10 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { const label = this.item.firstMetadataValue('dspace.entity.type'); if (label !== undefined) { - this.relationshipTypes$ = this.relationshipTypeService.searchByEntityType(label, true, true, ...this.getRelationshipTypeFollowLinks()) - .pipe( - map((relationshipTypes: PaginatedList) => relationshipTypes.page), - ); + this.relationshipTypes$ = this.relationshipTypeService.searchByEntityType(label, true, true, ...this.getRelationshipTypeFollowLinks()).pipe( + map((relationshipTypes: PaginatedList) => relationshipTypes.page), + distinctUntilChanged(compareArraysUsingIds()), + ); this.entityTypeService.getEntityTypeByLabel(label).pipe( getFirstSucceededRemoteData(), From 1338712048368f8c4a7da73eaa82bab6bff53bec Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Tue, 14 May 2024 17:46:43 +0200 Subject: [PATCH 08/12] 115046: Fixed failing tests & added new test to cover added code (cherry picked from commit 479adf6519dcf88b539d525fa3703671a6fe99a3) --- .../core/data/relationship-data.service.ts | 2 +- .../edit-item-relationships.service.spec.ts | 293 +++++++++++++- .../edit-item-relationships.service.ts | 9 +- .../edit-relationship-list.component.spec.ts | 362 +++++++++--------- .../edit-relationship-list.component.ts | 4 +- .../item-relationships.component.spec.ts | 10 +- .../shared/testing/base-data-service.stub.ts | 7 + .../edit-item-relationships.service.stub.ts | 48 +++ .../testing/entity-type-data.service.stub.ts | 5 + .../shared/testing/item-data.service.stub.ts | 8 + .../testing/object-updates.service.stub.ts | 24 ++ .../testing/relationship-data.service.stub.ts | 86 +++++ 12 files changed, 668 insertions(+), 190 deletions(-) create mode 100644 src/app/shared/testing/edit-item-relationships.service.stub.ts create mode 100644 src/app/shared/testing/entity-type-data.service.stub.ts create mode 100644 src/app/shared/testing/item-data.service.stub.ts create mode 100644 src/app/shared/testing/object-updates.service.stub.ts create mode 100644 src/app/shared/testing/relationship-data.service.stub.ts diff --git a/src/app/core/data/relationship-data.service.ts b/src/app/core/data/relationship-data.service.ts index b71ec0b677..8514ab3e2a 100644 --- a/src/app/core/data/relationship-data.service.ts +++ b/src/app/core/data/relationship-data.service.ts @@ -237,7 +237,7 @@ export class RelationshipDataService extends IdentifiableDataService { let service: EditItemRelationshipsService; + let itemService: ItemDataServiceStub; + let objectUpdatesService: ObjectUpdatesServiceStub; + let notificationsService: NotificationsServiceStub; + let relationshipService: RelationshipDataServiceStub; + let entityTypeDataService: EntityTypeDataServiceStub; + + let currentItem: Item; + + let relationshipItem1: Item; + let relationshipIdentifiable1: RelationshipIdentifiable; + let relationship1: Relationship; + + let relationshipItem2: Item; + let relationshipIdentifiable2: RelationshipIdentifiable; + let relationship2: Relationship; + + let orgUnitType: ItemType; + let orgUnitToOrgUnitType: RelationshipType; + beforeEach(() => { - TestBed.configureTestingModule({}); + itemService = new ItemDataServiceStub(); + objectUpdatesService = new ObjectUpdatesServiceStub(); + notificationsService = new NotificationsServiceStub(); + relationshipService = new RelationshipDataServiceStub(); + entityTypeDataService = new EntityTypeDataServiceStub(); + + TestBed.configureTestingModule({ + imports: [ + TranslateModule.forRoot(), + ], + providers: [ + { provide: ItemDataService, useValue: itemService }, + { provide: ObjectUpdatesService, useValue: objectUpdatesService }, + { provide: NotificationsService, useValue: notificationsService }, + { provide: RelationshipDataService, useValue: relationshipService }, + { provide: EntityTypeDataService, useValue: entityTypeDataService }, + ], + }); service = TestBed.inject(EditItemRelationshipsService); }); - it('should be created', () => { - expect(service).toBeTruthy(); + beforeEach(() => { + currentItem = Object.assign(new Item(), { + uuid: uuidv4(), + metadata: { + 'dspace.entity.type': 'OrgUnit', + }, + _links: { + self: { + href: 'selfLink1', + }, + }, + }); + + relationshipItem1 = Object.assign(new Item(), { + uuid: uuidv4(), + metadata: { + 'dspace.entity.type': 'OrgUnit', + }, + _links: { + self: { + href: 'selfLink2', + }, + }, + }); + relationshipIdentifiable1 = { + originalItem: currentItem, + relatedItem: relationshipItem1, + type: orgUnitToOrgUnitType, + uuid: `1-${relationshipItem1.uuid}`, + } as RelationshipIdentifiable; + relationship1 = Object.assign(new Relationship(), { + _links: { + leftItem: currentItem._links.self, + rightItem: relationshipItem1._links.self, + }, + }); + + relationshipItem2 = Object.assign(new Item(), { + uuid: uuidv4(), + metadata: { + 'dspace.entity.type': 'OrgUnit', + }, + _links: { + self: { + href: 'selfLink3', + }, + }, + }); + relationshipIdentifiable2 = { + originalItem: currentItem, + relatedItem: relationshipItem2, + type: orgUnitToOrgUnitType, + uuid: `1-${relationshipItem2.uuid}`, + } as RelationshipIdentifiable; + relationship2 = Object.assign(new Relationship(), { + _links: { + leftItem: currentItem._links.self, + rightItem: relationshipItem2._links.self, + }, + }); + + orgUnitType = Object.assign(new ItemType(), { + id: '2', + label: 'OrgUnit', + }); + orgUnitToOrgUnitType = Object.assign(new RelationshipType(), { + id: '1', + leftMaxCardinality: null, + leftMinCardinality: 0, + leftType: createSuccessfulRemoteDataObject$(orgUnitType), + leftwardType: 'isOrgUnitOfOrgUnit', + rightMaxCardinality: null, + rightMinCardinality: 0, + rightType: createSuccessfulRemoteDataObject$(orgUnitType), + rightwardType: 'isOrgUnitOfOrgUnit', + uuid: 'relationshiptype-1', + }); + }); + + describe('submit', () => { + let fieldUpdateAddRelationship1: FieldUpdate; + let fieldUpdateRemoveRelationship2: FieldUpdate; + + beforeEach(() => { + fieldUpdateAddRelationship1 = { + changeType: FieldChangeType.ADD, + field: relationshipIdentifiable1, + }; + fieldUpdateRemoveRelationship2 = { + changeType: FieldChangeType.REMOVE, + field: relationshipIdentifiable2, + }; + + spyOn(service, 'addRelationship').withArgs(relationshipIdentifiable1).and.returnValue(createSuccessfulRemoteDataObject$(relationship1)); + spyOn(service, 'deleteRelationship').withArgs(relationshipIdentifiable2 as DeleteRelationship).and.returnValue(createSuccessfulRemoteDataObject$({})); + spyOn(itemService, 'invalidateByHref').and.callThrough(); + }); + + it('should support performing multiple relationships manipulations in one submit() call', () => { + spyOn(objectUpdatesService, 'getFieldUpdates').and.returnValue(observableOf({ + [`1-${relationshipItem1.uuid}`]: fieldUpdateAddRelationship1, + [`1-${relationshipItem2.uuid}`]: fieldUpdateRemoveRelationship2, + } as FieldUpdates)); + service.submit(currentItem, `/entities/orgunit/${currentItem.uuid}/edit/relationships`); + + expect(service.addRelationship).toHaveBeenCalledWith(relationshipIdentifiable1); + expect(service.deleteRelationship).toHaveBeenCalledWith(relationshipIdentifiable2 as DeleteRelationship); + + expect(itemService.invalidateByHref).toHaveBeenCalledWith(currentItem.self); + expect(itemService.invalidateByHref).toHaveBeenCalledWith(relationshipItem1.self); + // TODO currently this isn't done yet + // expect(itemService.invalidateByHref).toHaveBeenCalledWith(relationshipItem2.self); + + expect(notificationsService.success).toHaveBeenCalledTimes(1); + }); + }); + + describe('deleteRelationship', () => { + beforeEach(() => { + spyOn(relationshipService, 'deleteRelationship').and.callThrough(); + }); + + it('should pass "all" as copyVirtualMetadata when the user want to keep the data on both sides', () => { + service.deleteRelationship({ + uuid: relationshipItem1.uuid, + keepLeftVirtualMetadata: true, + keepRightVirtualMetadata: true, + } as DeleteRelationship); + + expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationshipItem1.uuid, 'all', false); + }); + + it('should pass "left" as copyVirtualMetadata when the user only want to keep the data on the left side', () => { + service.deleteRelationship({ + uuid: relationshipItem1.uuid, + keepLeftVirtualMetadata: true, + keepRightVirtualMetadata: false, + } as DeleteRelationship); + + expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationshipItem1.uuid, 'left', false); + }); + + it('should pass "right" as copyVirtualMetadata when the user only want to keep the data on the right side', () => { + service.deleteRelationship({ + uuid: relationshipItem1.uuid, + keepLeftVirtualMetadata: false, + keepRightVirtualMetadata: true, + } as DeleteRelationship); + + expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationshipItem1.uuid, 'right', false); + }); + + it('should pass "none" as copyVirtualMetadata when the user doesn\'t want to keep the virtual metadata', () => { + service.deleteRelationship({ + uuid: relationshipItem1.uuid, + keepLeftVirtualMetadata: false, + keepRightVirtualMetadata: false, + } as DeleteRelationship); + + expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationshipItem1.uuid, 'none', false); + }); + }); + + describe('addRelationship', () => { + beforeEach(() => { + spyOn(relationshipService, 'addRelationship').and.callThrough(); + }); + + it('should call the addRelationship from relationshipService correctly when original item is on the right', () => { + service.addRelationship({ + originalItem: currentItem, + originalIsLeft: false, + relatedItem: relationshipItem1, + type: orgUnitToOrgUnitType, + uuid: `1-${relationshipItem1.uuid}`, + } as RelationshipIdentifiable); + expect(relationshipService.addRelationship).toHaveBeenCalledWith(orgUnitToOrgUnitType.id, relationshipItem1, currentItem, undefined, null, false); + }); + + it('should call the addRelationship from relationshipService correctly when original item is on the left', () => { + service.addRelationship({ + originalItem: currentItem, + originalIsLeft: true, + relatedItem: relationshipItem1, + type: orgUnitToOrgUnitType, + uuid: `1-${relationshipItem1.uuid}`, + } as RelationshipIdentifiable); + + expect(relationshipService.addRelationship).toHaveBeenCalledWith(orgUnitToOrgUnitType.id, currentItem, relationshipItem1, null, undefined, false); + }); + }); + + describe('displayNotifications', () => { + it('should show one success notification when multiple requests succeeded', () => { + service.displayNotifications([ + createSuccessfulRemoteDataObject({}), + createSuccessfulRemoteDataObject({}), + ]); + + expect(notificationsService.success).toHaveBeenCalledTimes(1); + }); + + it('should show one success notification even when some requests failed', () => { + service.displayNotifications([ + createSuccessfulRemoteDataObject({}), + createFailedRemoteDataObject('Request Failed'), + createSuccessfulRemoteDataObject({}), + ]); + + expect(notificationsService.success).toHaveBeenCalledTimes(1); + expect(notificationsService.error).toHaveBeenCalledTimes(1); + }); + + it('should show a separate error notification for each failed request', () => { + service.displayNotifications([ + createSuccessfulRemoteDataObject({}), + createFailedRemoteDataObject('Request Failed 1'), + createSuccessfulRemoteDataObject({}), + createFailedRemoteDataObject('Request Failed 2'), + ]); + + expect(notificationsService.success).toHaveBeenCalledTimes(1); + expect(notificationsService.error).toHaveBeenCalledTimes(2); + }); }); }); diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.ts b/src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.ts index 8a5204020c..14fa226bf4 100644 --- a/src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.ts +++ b/src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.ts @@ -5,6 +5,7 @@ import { BehaviorSubject, EMPTY, Observable, + Subscription, } from 'rxjs'; import { concatMap, @@ -120,7 +121,7 @@ export class EditItemRelationshipsService { /** * Sends all initial values of this item to the object updates service */ - public initializeOriginalFields(item: Item, url: string) { + public initializeOriginalFields(item: Item, url: string): Subscription { return this.relationshipService.getRelatedItems(item).pipe( take(1), ).subscribe((items: Item[]) => { @@ -168,7 +169,7 @@ export class EditItemRelationshipsService { * - Success notification in case there's at least one successful response * @param responses */ - displayNotifications(responses: RemoteData[]) { + displayNotifications(responses: RemoteData[]): void { const failedResponses = responses.filter((response: RemoteData) => response.hasFailed); const successfulResponses = responses.filter((response: RemoteData) => response.hasSucceeded); @@ -186,7 +187,7 @@ export class EditItemRelationshipsService { * Get translated notification title * @param key */ - getNotificationTitle(key: string) { + getNotificationTitle(key: string): string { return this.translateService.instant(this.notificationsPrefix + key + '.title'); } @@ -194,7 +195,7 @@ export class EditItemRelationshipsService { * Get translated notification content * @param key */ - getNotificationContent(key: string) { + getNotificationContent(key: string): string { return this.translateService.instant(this.notificationsPrefix + key + '.content'); } diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts index e28a3633c4..42296a1d71 100644 --- a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts +++ b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts @@ -10,19 +10,17 @@ import { import { By } from '@angular/platform-browser'; import { ActivatedRoute, - Router, + RouterModule, } from '@angular/router'; import { provideMockStore } from '@ngrx/store/testing'; import { TranslateModule } from '@ngx-translate/core'; +import { cold } from 'jasmine-marbles'; import { of as observableOf } from 'rxjs'; -import { AuthRequestService } from 'src/app/core/auth/auth-request.service'; -import { CookieService } from 'src/app/core/services/cookie.service'; -import { HardRedirectService } from 'src/app/core/services/hard-redirect.service'; -import { ActivatedRouteStub } from 'src/app/shared/testing/active-router.stub'; -import { AuthRequestServiceStub } from 'src/app/shared/testing/auth-request-service.stub'; import { APP_CONFIG } from '../../../../../config/app-config.interface'; +import { environment } from '../../../../../environments/environment.test'; import { REQUEST } from '../../../../../express.tokens'; +import { AuthRequestService } from '../../../../core/auth/auth-request.service'; import { LinkService } from '../../../../core/cache/builders/link.service'; import { ConfigurationDataService } from '../../../../core/data/configuration-data.service'; import { FieldChangeType } from '../../../../core/data/object-updates/field-change-type.model'; @@ -31,6 +29,8 @@ import { RelationshipDataService } from '../../../../core/data/relationship-data import { RelationshipTypeDataService } from '../../../../core/data/relationship-type-data.service'; import { GroupDataService } from '../../../../core/eperson/group-data.service'; import { PaginationService } from '../../../../core/pagination/pagination.service'; +import { CookieService } from '../../../../core/services/cookie.service'; +import { HardRedirectService } from '../../../../core/services/hard-redirect.service'; import { LinkHeadService } from '../../../../core/services/link-head.service'; import { ConfigurationProperty } from '../../../../core/shared/configuration-property.model'; import { Item } from '../../../../core/shared/item.model'; @@ -40,51 +40,55 @@ import { RelationshipType } from '../../../../core/shared/item-relationships/rel import { SearchConfigurationService } from '../../../../core/shared/search/search-configuration.service'; import { XSRFService } from '../../../../core/xsrf/xsrf.service'; import { HostWindowService } from '../../../../shared/host-window.service'; -import { RouterMock } from '../../../../shared/mocks/router.mock'; import { SelectableListService } from '../../../../shared/object-list/selectable-list/selectable-list.service'; import { PaginationComponent } from '../../../../shared/pagination/pagination.component'; import { PaginationComponentOptions } from '../../../../shared/pagination/pagination-component-options.model'; import { createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils'; +import { ActivatedRouteStub } from '../../../../shared/testing/active-router.stub'; +import { AuthRequestServiceStub } from '../../../../shared/testing/auth-request-service.stub'; +import { EditItemRelationshipsServiceStub } from '../../../../shared/testing/edit-item-relationships.service.stub'; import { HostWindowServiceStub } from '../../../../shared/testing/host-window-service.stub'; import { PaginationServiceStub } from '../../../../shared/testing/pagination-service.stub'; import { SearchConfigurationServiceStub } from '../../../../shared/testing/search-configuration-service.stub'; import { createPaginatedList } from '../../../../shared/testing/utils.test'; +import { EditItemRelationshipsService } from '../edit-item-relationships.service'; import { EditRelationshipListComponent } from './edit-relationship-list.component'; -let comp: EditRelationshipListComponent; -let fixture: ComponentFixture; -let de: DebugElement; - -let linkService; -let objectUpdatesService; -let relationshipService; -let selectableListService; -let paginationService; -let hostWindowService; -let hardRedirectService; -const relationshipTypeService = {}; - -const url = 'http://test-url.com/test-url'; - -let item; -let entityType; -let relatedEntityType; -let author1; -let author2; -let fieldUpdate1; -let fieldUpdate2; -let relationships; -let relationshipType; -let paginationOptions; - describe('EditRelationshipListComponent', () => { + let comp: EditRelationshipListComponent; + let fixture: ComponentFixture; + let de: DebugElement; + + let linkService; + let objectUpdatesService; + let relationshipService; + let selectableListService; + let paginationService: PaginationServiceStub; + let hostWindowService: HostWindowServiceStub; + let hardRedirectService; + const relationshipTypeService = {}; + let editItemRelationshipsService: EditItemRelationshipsServiceStub; + + const url = 'http://test-url.com/test-url'; + + let itemLeft: Item; + let entityTypeLeft: ItemType; + let entityTypeRight: ItemType; + let itemRight1: Item; + let itemRight2: Item; + let fieldUpdate1; + let fieldUpdate2; + let relationships: Relationship[]; + let relationshipType: RelationshipType; + let paginationOptions: PaginationComponentOptions; + const resetComponent = () => { fixture = TestBed.createComponent(EditRelationshipListComponent); comp = fixture.componentInstance; de = fixture.debugElement; - comp.item = item; - comp.itemType = entityType; + comp.item = itemLeft; + comp.itemType = entityTypeLeft; comp.url = url; comp.relationshipType = relationshipType; comp.hasChanges = observableOf(false); @@ -101,29 +105,26 @@ describe('EditRelationshipListComponent', () => { }, }; - hardRedirectService = jasmine.createSpyObj('hardRedirectService', ['redirect']); - - beforeEach(waitForAsync(() => { - - entityType = Object.assign(new ItemType(), { - id: 'Publication', - uuid: 'Publication', - label: 'Publication', + function init(leftType: string, rightType: string): void { + entityTypeLeft = Object.assign(new ItemType(), { + id: leftType, + uuid: leftType, + label: leftType, }); - relatedEntityType = Object.assign(new ItemType(), { - id: 'Author', - uuid: 'Author', - label: 'Author', + entityTypeRight = Object.assign(new ItemType(), { + id: rightType, + uuid: rightType, + label: rightType, }); relationshipType = Object.assign(new RelationshipType(), { id: '1', uuid: '1', - leftType: createSuccessfulRemoteDataObject$(entityType), - rightType: createSuccessfulRemoteDataObject$(relatedEntityType), - leftwardType: 'isAuthorOfPublication', - rightwardType: 'isPublicationOfAuthor', + leftType: createSuccessfulRemoteDataObject$(entityTypeLeft), + rightType: createSuccessfulRemoteDataObject$(entityTypeRight), + leftwardType: `is${rightType}Of${leftType}`, + rightwardType: `is${leftType}Of${rightType}`, }); paginationOptions = Object.assign(new PaginationComponentOptions(), { @@ -132,13 +133,13 @@ describe('EditRelationshipListComponent', () => { currentPage: 1, }); - author1 = Object.assign(new Item(), { - id: 'author1', - uuid: 'author1', + itemRight1 = Object.assign(new Item(), { + id: `${rightType}-1`, + uuid: `${rightType}-1`, }); - author2 = Object.assign(new Item(), { - id: 'author2', - uuid: 'author2', + itemRight2 = Object.assign(new Item(), { + id: `${rightType}-2`, + uuid: `${rightType}-2`, }); relationships = [ @@ -147,25 +148,25 @@ describe('EditRelationshipListComponent', () => { id: '2', uuid: '2', relationshipType: createSuccessfulRemoteDataObject$(relationshipType), - leftItem: createSuccessfulRemoteDataObject$(item), - rightItem: createSuccessfulRemoteDataObject$(author1), + leftItem: createSuccessfulRemoteDataObject$(itemLeft), + rightItem: createSuccessfulRemoteDataObject$(itemRight1), }), Object.assign(new Relationship(), { self: url + '/3', id: '3', uuid: '3', relationshipType: createSuccessfulRemoteDataObject$(relationshipType), - leftItem: createSuccessfulRemoteDataObject$(item), - rightItem: createSuccessfulRemoteDataObject$(author2), + leftItem: createSuccessfulRemoteDataObject$(itemLeft), + rightItem: createSuccessfulRemoteDataObject$(itemRight2), }), ]; - item = Object.assign(new Item(), { + itemLeft = Object.assign(new Item(), { _links: { self: { href: 'fake-item-url/publication' }, }, - id: 'publication', - uuid: 'publication', + id: `1-${leftType}`, + uuid: `1-${leftType}`, relationships: createSuccessfulRemoteDataObject$(createPaginatedList(relationships)), }); @@ -197,7 +198,7 @@ describe('EditRelationshipListComponent', () => { relationshipService = jasmine.createSpyObj('relationshipService', { - getRelatedItemsByLabel: createSuccessfulRemoteDataObject$(createPaginatedList([author1, author2])), + getRelatedItemsByLabel: createSuccessfulRemoteDataObject$(createPaginatedList([itemRight1, itemRight2])), getItemRelationshipsByLabel: createSuccessfulRemoteDataObject$(createPaginatedList(relationships)), isLeftItem: observableOf(true), }, @@ -233,14 +234,14 @@ describe('EditRelationshipListComponent', () => { })), }); - const environmentUseThumbs = { - browseBy: { - showThumbnails: true, - }, - }; + editItemRelationshipsService = new EditItemRelationshipsServiceStub(); TestBed.configureTestingModule({ - imports: [TranslateModule.forRoot(), EditRelationshipListComponent], + imports: [ + EditRelationshipListComponent, + RouterModule.forRoot([]), + TranslateModule.forRoot(), + ], providers: [ provideMockStore({ initialState }), { provide: ObjectUpdatesService, useValue: objectUpdatesService }, @@ -251,15 +252,15 @@ describe('EditRelationshipListComponent', () => { { provide: HostWindowService, useValue: hostWindowService }, { provide: RelationshipTypeDataService, useValue: relationshipTypeService }, { provide: GroupDataService, useValue: groupDataService }, - { provide: Router, useValue: new RouterMock() }, { provide: LinkHeadService, useValue: linkHeadService }, { provide: ConfigurationDataService, useValue: configurationDataService }, { provide: SearchConfigurationService, useValue: new SearchConfigurationServiceStub() }, + { provide: EditItemRelationshipsService, useValue: editItemRelationshipsService }, { provide: ActivatedRoute, useValue: new ActivatedRouteStub() }, { provide: AuthRequestService, useValue: new AuthRequestServiceStub() }, { provide: HardRedirectService, useValue: hardRedirectService }, { provide: XSRFService, useValue: {} }, - { provide: APP_CONFIG, useValue: environmentUseThumbs }, + { provide: APP_CONFIG, useValue: environment }, { provide: REQUEST, useValue: {} }, CookieService, ], schemas: [ @@ -268,114 +269,127 @@ describe('EditRelationshipListComponent', () => { }).compileComponents(); resetComponent(); - })); + } - describe('changeType is REMOVE', () => { - beforeEach(() => { - fieldUpdate1.changeType = FieldChangeType.REMOVE; - fixture.detectChanges(); - }); - it('the div should have class alert-danger', () => { - const element = de.queryAll(By.css('.relationship-row'))[1].nativeElement; - expect(element.classList).toContain('alert-danger'); - }); - }); + describe('Publication - Author relationship', () => { + beforeEach(waitForAsync(() => init('Publication', 'Author'))); - describe('pagination component', () => { - let paginationComp: PaginationComponent; - - beforeEach(() => { - paginationComp = de.query(By.css('ds-pagination')).componentInstance; - }); - - it('should receive the correct pagination config', () => { - expect(paginationComp.paginationOptions).toEqual(paginationOptions); - }); - - it('should receive correct collection size', () => { - expect(paginationComp.collectionSize).toEqual(relationships.length); - }); - - }); - - describe('relationshipService.getItemRelationshipsByLabel', () => { - it('should receive the correct pagination info', () => { - expect(relationshipService.getItemRelationshipsByLabel).toHaveBeenCalledTimes(1); - - const callArgs = relationshipService.getItemRelationshipsByLabel.calls.mostRecent().args; - const findListOptions = callArgs[2]; - const linksToFollow = callArgs[5]; - expect(findListOptions.elementsPerPage).toEqual(paginationOptions.pageSize); - expect(findListOptions.currentPage).toEqual(paginationOptions.currentPage); - expect(linksToFollow.linksToFollow[0].name).toEqual('thumbnail'); - - }); - - describe('when the publication is on the left side of the relationship', () => { + describe('changeType is REMOVE', () => { beforeEach(() => { - relationshipType = Object.assign(new RelationshipType(), { - id: '1', - uuid: '1', - leftType: createSuccessfulRemoteDataObject$(entityType), // publication - rightType: createSuccessfulRemoteDataObject$(relatedEntityType), // author - leftwardType: 'isAuthorOfPublication', - rightwardType: 'isPublicationOfAuthor', - }); - relationshipService.getItemRelationshipsByLabel.calls.reset(); - resetComponent(); - }); - - it('should fetch isAuthorOfPublication', () => { - expect(relationshipService.getItemRelationshipsByLabel).toHaveBeenCalledTimes(1); - - const callArgs = relationshipService.getItemRelationshipsByLabel.calls.mostRecent().args; - const label = callArgs[1]; - - expect(label).toEqual('isAuthorOfPublication'); - }); - }); - - describe('when the publication is on the right side of the relationship', () => { - beforeEach(() => { - relationshipType = Object.assign(new RelationshipType(), { - id: '1', - uuid: '1', - leftType: createSuccessfulRemoteDataObject$(relatedEntityType), // author - rightType: createSuccessfulRemoteDataObject$(entityType), // publication - leftwardType: 'isPublicationOfAuthor', - rightwardType: 'isAuthorOfPublication', - }); - relationshipService.getItemRelationshipsByLabel.calls.reset(); - resetComponent(); - }); - - it('should fetch isAuthorOfPublication', () => { - expect(relationshipService.getItemRelationshipsByLabel).toHaveBeenCalledTimes(1); - - const callArgs = relationshipService.getItemRelationshipsByLabel.calls.mostRecent().args; - const label = callArgs[1]; - - expect(label).toEqual('isAuthorOfPublication'); - }); - }); - - - - describe('changes managment for add buttons', () => { - - it('should show enabled add buttons', () => { - const element = de.query(By.css('.btn-success')); - expect(element.nativeElement?.disabled).toBeFalse(); - }); - - it('after hash changes changed', () => { - comp.hasChanges = observableOf(true); + fieldUpdate1.changeType = FieldChangeType.REMOVE; fixture.detectChanges(); - const element = de.query(By.css('.btn-success')); - expect(element.nativeElement?.disabled).toBeTrue(); + }); + it('the div should have class alert-danger', () => { + const element = de.queryAll(By.css('.relationship-row'))[1].nativeElement; + expect(element.classList).toContain('alert-danger'); }); }); + describe('pagination component', () => { + let paginationComp: PaginationComponent; + + beforeEach(() => { + paginationComp = de.query(By.css('ds-pagination')).componentInstance; + }); + + it('should receive the correct pagination config', () => { + expect(paginationComp.paginationOptions).toEqual(paginationOptions); + }); + + it('should receive correct collection size', () => { + expect(paginationComp.collectionSize).toEqual(relationships.length); + }); + + }); + + describe('relationshipService.getItemRelationshipsByLabel', () => { + it('should receive the correct pagination info', () => { + expect(relationshipService.getItemRelationshipsByLabel).toHaveBeenCalledTimes(1); + + const callArgs = relationshipService.getItemRelationshipsByLabel.calls.mostRecent().args; + const findListOptions = callArgs[2]; + const linksToFollow = callArgs[5]; + expect(findListOptions.elementsPerPage).toEqual(paginationOptions.pageSize); + expect(findListOptions.currentPage).toEqual(paginationOptions.currentPage); + expect(linksToFollow.linksToFollow[0].name).toEqual('thumbnail'); + + }); + + describe('when the publication is on the left side of the relationship', () => { + beforeEach(() => { + relationshipType = Object.assign(new RelationshipType(), { + id: '1', + uuid: '1', + leftType: createSuccessfulRemoteDataObject$(entityTypeLeft), // publication + rightType: createSuccessfulRemoteDataObject$(entityTypeRight), // author + leftwardType: 'isAuthorOfPublication', + rightwardType: 'isPublicationOfAuthor', + }); + relationshipService.getItemRelationshipsByLabel.calls.reset(); + resetComponent(); + }); + + it('should fetch isAuthorOfPublication', () => { + expect(relationshipService.getItemRelationshipsByLabel).toHaveBeenCalledTimes(1); + + const callArgs = relationshipService.getItemRelationshipsByLabel.calls.mostRecent().args; + const label = callArgs[1]; + + expect(label).toEqual('isAuthorOfPublication'); + }); + }); + + describe('when the publication is on the right side of the relationship', () => { + beforeEach(() => { + relationshipType = Object.assign(new RelationshipType(), { + id: '1', + uuid: '1', + leftType: createSuccessfulRemoteDataObject$(entityTypeRight), // author + rightType: createSuccessfulRemoteDataObject$(entityTypeLeft), // publication + leftwardType: 'isPublicationOfAuthor', + rightwardType: 'isAuthorOfPublication', + }); + relationshipService.getItemRelationshipsByLabel.calls.reset(); + resetComponent(); + }); + + it('should fetch isAuthorOfPublication', () => { + expect(relationshipService.getItemRelationshipsByLabel).toHaveBeenCalledTimes(1); + + const callArgs = relationshipService.getItemRelationshipsByLabel.calls.mostRecent().args; + const label = callArgs[1]; + + expect(label).toEqual('isAuthorOfPublication'); + }); + }); + + + + describe('changes managment for add buttons', () => { + + it('should show enabled add buttons', () => { + const element = de.query(By.css('.btn-success')); + expect(element.nativeElement?.disabled).toBeFalse(); + }); + + it('after hash changes changed', () => { + comp.hasChanges = observableOf(true); + fixture.detectChanges(); + const element = de.query(By.css('.btn-success')); + expect(element.nativeElement?.disabled).toBeTrue(); + }); + }); + + }); }); + describe('OrgUnit - OrgUnit relationship', () => { + beforeEach(waitForAsync(() => init('OrgUnit', 'OrgUnit'))); + + it('should emit the relatedEntityType$ even for same entity relationships', () => { + expect(comp.relatedEntityType$).toBeObservable(cold('(a|)', { + a: entityTypeRight, + })); + }); + }); }); diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts index 3ae342e14e..6c3b8d556a 100644 --- a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts +++ b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts @@ -50,7 +50,6 @@ import { RelationshipIdentifiable } from '../../../../core/data/object-updates/o import { ObjectUpdatesService } from '../../../../core/data/object-updates/object-updates.service'; import { PaginatedList } from '../../../../core/data/paginated-list.model'; import { RelationshipDataService } from '../../../../core/data/relationship-data.service'; -import { RelationshipTypeDataService } from '../../../../core/data/relationship-type-data.service'; import { RemoteData } from '../../../../core/data/remote-data'; import { PaginationService } from '../../../../core/pagination/pagination.service'; import { Collection } from '../../../../core/shared/collection.model'; @@ -146,7 +145,7 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { */ private currentItemIsLeftItem$: BehaviorSubject = new BehaviorSubject(undefined); - private relatedEntityType$: Observable; + relatedEntityType$: Observable; /** * The translation key for the entity type @@ -208,7 +207,6 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { protected objectUpdatesService: ObjectUpdatesService, protected linkService: LinkService, protected relationshipService: RelationshipDataService, - protected relationshipTypeService: RelationshipTypeDataService, protected modalService: NgbModal, protected paginationService: PaginationService, protected selectableListService: SelectableListService, diff --git a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts index 4cd3061744..05dade88ba 100644 --- a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts +++ b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts @@ -44,6 +44,7 @@ import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$, } from '../../../shared/remote-data.utils'; +import { ItemDataServiceStub } from '../../../shared/testing/item-data.service.stub'; import { relationshipTypes } from '../../../shared/testing/relationship-types.mock'; import { RouterStub } from '../../../shared/testing/router.stub'; import { createPaginatedList } from '../../../shared/testing/utils.test'; @@ -72,7 +73,7 @@ const notificationsService = jasmine.createSpyObj('notificationsService', const router = new RouterStub(); let relationshipTypeService; let routeStub; -let itemService; +let itemService: ItemDataServiceStub; const url = 'http://test-url.com/test-url'; router.url = url; @@ -157,10 +158,7 @@ describe('ItemRelationshipsComponent', () => { changeType: FieldChangeType.REMOVE, }; - itemService = jasmine.createSpyObj('itemService', { - findByHref: createSuccessfulRemoteDataObject$(item), - findById: createSuccessfulRemoteDataObject$(item), - }); + itemService = new ItemDataServiceStub(); routeStub = { data: observableOf({}), parent: { @@ -251,6 +249,8 @@ describe('ItemRelationshipsComponent', () => { })); beforeEach(() => { + spyOn(itemService, 'findByHref').and.returnValue(item); + spyOn(itemService, 'findById').and.returnValue(item); fixture = TestBed.createComponent(ItemRelationshipsComponent); comp = fixture.componentInstance; de = fixture.debugElement; diff --git a/src/app/shared/testing/base-data-service.stub.ts b/src/app/shared/testing/base-data-service.stub.ts index 3df661ba5b..65b761ba71 100644 --- a/src/app/shared/testing/base-data-service.stub.ts +++ b/src/app/shared/testing/base-data-service.stub.ts @@ -4,12 +4,19 @@ import { } from 'rxjs'; import { CacheableObject } from '../../core/cache/cacheable-object.model'; +import { RemoteData } from '../../core/data/remote-data'; +import { createSuccessfulRemoteDataObject$ } from '../remote-data.utils'; +import { FollowLinkConfig } from '../utils/follow-link-config.model'; /** * Stub class for {@link BaseDataService} */ export abstract class BaseDataServiceStub { + findByHref(_href$: string | Observable, _useCachedVersionIfAvailable = true, _reRequestOnStale = true, ..._linksToFollow: FollowLinkConfig[]): Observable> { + return createSuccessfulRemoteDataObject$(undefined); + } + invalidateByHref(_href: string): Observable { return observableOf(true); } diff --git a/src/app/shared/testing/edit-item-relationships.service.stub.ts b/src/app/shared/testing/edit-item-relationships.service.stub.ts new file mode 100644 index 0000000000..1d29569793 --- /dev/null +++ b/src/app/shared/testing/edit-item-relationships.service.stub.ts @@ -0,0 +1,48 @@ +/* eslint-disable no-empty, @typescript-eslint/no-empty-function */ +import { + Observable, + Subscription, +} from 'rxjs'; + +import { + DeleteRelationship, + RelationshipIdentifiable, +} from '../../core/data/object-updates/object-updates.reducer'; +import { RemoteData } from '../../core/data/remote-data'; +import { Item } from '../../core/shared/item.model'; +import { Relationship } from '../../core/shared/item-relationships/relationship.model'; +import { NoContent } from '../../core/shared/NoContent.model'; +import { createSuccessfulRemoteDataObject$ } from '../remote-data.utils'; + +/** + * Stub class of {@link EditItemRelationshipsService} + */ +export class EditItemRelationshipsServiceStub { + + submit(_item: Item, _url: string): void { + } + + initializeOriginalFields(_item: Item, _url: string): Subscription { + return new Subscription(); + } + + deleteRelationship(_deleteRelationship: DeleteRelationship): Observable> { + return createSuccessfulRemoteDataObject$({}); + } + + addRelationship(_addRelationship: RelationshipIdentifiable): Observable> { + return createSuccessfulRemoteDataObject$(undefined); + } + + displayNotifications(_responses: RemoteData[]): void { + } + + getNotificationTitle(_key: string): string { + return ''; + } + + getNotificationContent(_key: string): string { + return ''; + } + +} diff --git a/src/app/shared/testing/entity-type-data.service.stub.ts b/src/app/shared/testing/entity-type-data.service.stub.ts new file mode 100644 index 0000000000..367eeb5f74 --- /dev/null +++ b/src/app/shared/testing/entity-type-data.service.stub.ts @@ -0,0 +1,5 @@ +/** + * Stub class of {@link EntityTypeDataService} + */ +export class EntityTypeDataServiceStub { +} diff --git a/src/app/shared/testing/item-data.service.stub.ts b/src/app/shared/testing/item-data.service.stub.ts new file mode 100644 index 0000000000..eed5d4bb11 --- /dev/null +++ b/src/app/shared/testing/item-data.service.stub.ts @@ -0,0 +1,8 @@ +import { Item } from '../../core/shared/item.model'; +import { IdentifiableDataServiceStub } from './identifiable-data-service.stub'; + +/** + * Stub class of {@link ItemDataService} + */ +export class ItemDataServiceStub extends IdentifiableDataServiceStub { +} diff --git a/src/app/shared/testing/object-updates.service.stub.ts b/src/app/shared/testing/object-updates.service.stub.ts new file mode 100644 index 0000000000..c66cc3aa0d --- /dev/null +++ b/src/app/shared/testing/object-updates.service.stub.ts @@ -0,0 +1,24 @@ +/* eslint-disable no-empty, @typescript-eslint/no-empty-function */ +import { + Observable, + of as observableOf, +} from 'rxjs'; + +import { FieldUpdates } from '../../core/data/object-updates/field-updates.model'; +import { Identifiable } from '../../core/data/object-updates/identifiable.model'; +import { PatchOperationService } from '../../core/data/object-updates/patch-operation-service/patch-operation.service'; +import { GenericConstructor } from '../../core/shared/generic-constructor'; + +/** + * Stub class of {@link ObjectUpdatesService} + */ +export class ObjectUpdatesServiceStub { + + initialize(_url: string, _fields: Identifiable[], _lastModified: Date, _patchOperationService?: GenericConstructor): void { + } + + getFieldUpdates(_url: string, _initialFields: Identifiable[], _ignoreStates?: boolean): Observable { + return observableOf({}); + } + +} diff --git a/src/app/shared/testing/relationship-data.service.stub.ts b/src/app/shared/testing/relationship-data.service.stub.ts new file mode 100644 index 0000000000..f0463b3e6c --- /dev/null +++ b/src/app/shared/testing/relationship-data.service.stub.ts @@ -0,0 +1,86 @@ +/* eslint-disable no-empty, @typescript-eslint/no-empty-function */ +import { + Observable, + of as observableOf, +} from 'rxjs'; + +import { FindListOptions } from '../../core/data/find-list-options.model'; +import { PaginatedList } from '../../core/data/paginated-list.model'; +import { RemoteData } from '../../core/data/remote-data'; +import { DSpaceObject } from '../../core/shared/dspace-object.model'; +import { Item } from '../../core/shared/item.model'; +import { Relationship } from '../../core/shared/item-relationships/relationship.model'; +import { MetadataValue } from '../../core/shared/metadata.models'; +import { MetadataRepresentation } from '../../core/shared/metadata-representation/metadata-representation.model'; +import { NoContent } from '../../core/shared/NoContent.model'; +import { createSuccessfulRemoteDataObject$ } from '../remote-data.utils'; +import { FollowLinkConfig } from '../utils/follow-link-config.model'; + +/** + * Stub class of {@link RelationshipDataService} + */ +export class RelationshipDataServiceStub { + + deleteRelationship(_id: string, _copyVirtualMetadata: string, _shouldRefresh = true): Observable> { + return createSuccessfulRemoteDataObject$({}); + } + + addRelationship(_typeId: string, _item1: Item, _item2: Item, _leftwardValue?: string, _rightwardValue?: string, _shouldRefresh = true): Observable> { + return createSuccessfulRemoteDataObject$(new Relationship()); + } + + refreshRelationshipItemsInCache(_item: Item): void { + } + + getItemRelationshipsArray(_item: Item, ..._linksToFollow: FollowLinkConfig[]): Observable { + return observableOf([]); + } + + getRelatedItems(_item: Item): Observable { + return observableOf([]); + } + + getRelatedItemsByLabel(_item: Item, _label: string, _options?: FindListOptions): Observable>> { + return createSuccessfulRemoteDataObject$(new PaginatedList()); + } + + getItemRelationshipsByLabel(_item: Item, _label: string, _options?: FindListOptions, _useCachedVersionIfAvailable = true, _reRequestOnStale = true, ..._linksToFollow: FollowLinkConfig[]): Observable>> { + return createSuccessfulRemoteDataObject$(new PaginatedList()); + } + + getRelationshipByItemsAndLabel(_item1: Item, _item2: Item, _label: string, _options?: FindListOptions): Observable { + return observableOf(new Relationship()); + } + + setNameVariant(_listID: string, _itemID: string, _nameVariant: string): void { + } + + getNameVariant(_listID: string, _itemID: string): Observable { + return observableOf(''); + } + + updateNameVariant(_item1: Item, _item2: Item, _relationshipLabel: string, _nameVariant: string): Observable> { + return createSuccessfulRemoteDataObject$(new Relationship()); + } + + isLeftItem(_relationship: Relationship, _item: Item): Observable { + return observableOf(false); + } + + update(_object: Relationship): Observable> { + return createSuccessfulRemoteDataObject$(new Relationship()); + } + + searchByItemsAndType(_typeId: string, _itemUuid: string, _relationshipLabel: string, _arrayOfItemIds: string[]): Observable>> { + return createSuccessfulRemoteDataObject$(new PaginatedList()); + } + + searchBy(_searchMethod: string, _options?: FindListOptions, _useCachedVersionIfAvailable?: boolean, _reRequestOnStale?: boolean, ..._linksToFollow: FollowLinkConfig[]): Observable>> { + return createSuccessfulRemoteDataObject$(new PaginatedList()); + } + + resolveMetadataRepresentation(_metadatum: MetadataValue, _parentItem: DSpaceObject, _itemType: string): Observable { + return observableOf({} as MetadataRepresentation); + } + +} From df80c33e4e86d49c603d79d934c5aa19e125c404 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Fri, 17 May 2024 14:34:24 +0200 Subject: [PATCH 09/12] 115046: Fixed multiple edit relationship bugs - Fixed issue making it impossible to add new relationships until the page is refreshed after deleting an existing one (only when you refreshed the page after creating the initial relationship) - Fixed NPE in DsDynamicLookupRelationModalComponent - Grouped buttons on relationship page in order to assure that they always have the same behaviour --- .../abstract-item-update.component.ts | 11 ++- .../item-bitstreams.component.html | 16 ++-- .../edit-item-relationships.service.spec.ts | 2 - .../item-relationships.component.html | 82 +++++++++---------- .../item-relationships.component.spec.ts | 11 ++- .../item-relationships.component.ts | 13 ++- ...namic-lookup-relation-modal.component.html | 2 +- ...dynamic-lookup-relation-modal.component.ts | 7 +- ...ic-lookup-relation-search-tab.component.ts | 6 +- ...ic-lookup-relation-search-tab.component.ts | 4 +- ...lookup-relation-selection-tab.component.ts | 6 +- src/assets/i18n/en.json5 | 2 + 12 files changed, 86 insertions(+), 76 deletions(-) diff --git a/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts b/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts index 77c3fdaafb..69b234fbaf 100644 --- a/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts +++ b/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts @@ -55,6 +55,10 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl */ updates$: Observable; + hasChanges$: Observable; + + isReinstatable$: Observable; + /** * Route to the item's page */ @@ -101,10 +105,9 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl } this.discardTimeOut = environment.item.edit.undoTimeout; - this.url = this.router.url; - if (this.url.indexOf('?') > 0) { - this.url = this.url.substr(0, this.url.indexOf('?')); - } + this.url = this.router.url.split('?')[0]; + this.hasChanges$ = this.hasChanges(); + this.isReinstatable$ = this.isReinstatable(); this.hasChanges().pipe(first()).subscribe((hasChanges) => { if (!hasChanges) { this.initializeOriginalFields(); diff --git a/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html b/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html index bb0c3e3760..88d984c19f 100644 --- a/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html +++ b/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html @@ -6,21 +6,21 @@ class="fas fa-upload">  {{"item.edit.bitstreams.upload-button" | translate}} - - - - - - - + +
+
@@ -26,36 +10,46 @@ [item]="item" [itemType]="entityType" [relationshipType]="relationshipType" - [hasChanges]="hasChanges()" + [hasChanges]="hasChanges$" >
- - -
-
- - - +
+
-
+ + + + {{ 'item.edit.relationships.no-entity-type' | translate }} + + + + + + + + +
+ + + +
+ diff --git a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts index 05dade88ba..28380b3f27 100644 --- a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts +++ b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts @@ -13,12 +13,10 @@ import { Router, } from '@angular/router'; import { TranslateModule } from '@ngx-translate/core'; -import { getTestScheduler } from 'jasmine-marbles'; import { combineLatest as observableCombineLatest, of as observableOf, } from 'rxjs'; -import { TestScheduler } from 'rxjs/testing'; import { ObjectCacheService } from '../../../core/cache/object-cache.service'; import { RestResponse } from '../../../core/cache/response.models'; @@ -33,6 +31,7 @@ import { Item } from '../../../core/shared/item.model'; import { ItemType } from '../../../core/shared/item-relationships/item-type.model'; import { Relationship } from '../../../core/shared/item-relationships/relationship.model'; import { RelationshipType } from '../../../core/shared/item-relationships/relationship-type.model'; +import { AlertComponent } from '../../../shared/alert/alert.component'; import { getMockThemeService } from '../../../shared/mocks/theme-service.mock'; import { INotification, @@ -78,7 +77,6 @@ let itemService: ItemDataServiceStub; const url = 'http://test-url.com/test-url'; router.url = url; -let scheduler: TestScheduler; let item; let author1; let author2; @@ -226,7 +224,6 @@ describe('ItemRelationshipsComponent', () => { }, ); - scheduler = getTestScheduler(); TestBed.configureTestingModule({ imports: [TranslateModule.forRoot(), ItemRelationshipsComponent], providers: [ @@ -245,6 +242,12 @@ describe('ItemRelationshipsComponent', () => { ], schemas: [ NO_ERRORS_SCHEMA, ], + }).overrideComponent(ItemRelationshipsComponent, { + remove: { + imports: [ + AlertComponent, + ], + }, }).compileComponents(); })); diff --git a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts index 2539c3a37d..051c4db136 100644 --- a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -2,6 +2,7 @@ import { AsyncPipe, NgForOf, NgIf, + NgTemplateOutlet, } from '@angular/common'; import { ChangeDetectorRef, @@ -39,6 +40,8 @@ import { getFirstSucceededRemoteData, getRemoteDataPayload, } from '../../../core/shared/operators'; +import { AlertComponent } from '../../../shared/alert/alert.component'; +import { AlertType } from '../../../shared/alert/alert-type'; import { ThemedLoadingComponent } from '../../../shared/loading/themed-loading.component'; import { NotificationsService } from '../../../shared/notifications/notifications.service'; import { followLink } from '../../../shared/utils/follow-link-config.model'; @@ -53,12 +56,14 @@ import { EditRelationshipListComponent } from './edit-relationship-list/edit-rel styleUrls: ['./item-relationships.component.scss'], templateUrl: './item-relationships.component.html', imports: [ - ThemedLoadingComponent, + AlertComponent, AsyncPipe, - TranslateModule, - NgIf, EditRelationshipListComponent, NgForOf, + NgIf, + NgTemplateOutlet, + ThemedLoadingComponent, + TranslateModule, VarDirective, ], standalone: true, @@ -83,6 +88,8 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { return this.editItemRelationshipsService.isSaving$; } + readonly AlertType = AlertType; + constructor( public itemService: ItemDataService, public objectUpdatesService: ObjectUpdatesService, diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html index 111e2d9994..112175d806 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html @@ -32,7 +32,7 @@
  • - {{'submission.sections.describe.relationship-lookup.search-tab.tab-title.' + source.id | translate : { count: (totalExternal$ | async)[idx] } }} + {{'submission.sections.describe.relationship-lookup.search-tab.tab-title.' + source.id | translate : { count: (totalExternal$ | async)?.[idx] } }} []) { + select(...selectableObjects: SearchResult[]) { this.zone.runOutsideAngular( () => { const obs: Observable = observableCombineLatest([...selectableObjects.map((sri: SearchResult) => { @@ -326,11 +327,11 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy * Deselect (a list of) objects and remove them from the store * @param selectableObjects */ - deselect(...selectableObjects: SearchResult[]) { + deselect(...selectableObjects: SearchResult[]) { this.zone.runOutsideAngular( () => selectableObjects.forEach((object) => { this.subMap[object.indexableObject.uuid].unsubscribe(); - this.store.dispatch(new RemoveRelationshipAction(this.item, object.indexableObject, this.relationshipOptions.relationshipType, this.submissionId)); + this.store.dispatch(new RemoveRelationshipAction(this.item, object.indexableObject as Item, this.relationshipOptions.relationshipType, this.submissionId)); }), ); } diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.ts index 06b4946e12..e2654d079d 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.ts @@ -132,12 +132,12 @@ export class DsDynamicLookupRelationSearchTabComponent implements OnInit, OnDest /** * Send an event to deselect an object from the list */ - @Output() deselectObject: EventEmitter = new EventEmitter(); + @Output() deselectObject: EventEmitter> = new EventEmitter(); /** * Send an event to select an object from the list */ - @Output() selectObject: EventEmitter = new EventEmitter(); + @Output() selectObject: EventEmitter> = new EventEmitter(); /** * Search results @@ -214,7 +214,7 @@ export class DsDynamicLookupRelationSearchTabComponent implements OnInit, OnDest this.selection$ .pipe(take(1)) .subscribe((selection: SearchResult[]) => { - const filteredPage = page.filter((pageItem) => selection.findIndex((selected) => selected.equals(pageItem)) < 0); + const filteredPage: SearchResult[] = page.filter((pageItem: SearchResult) => selection.findIndex((selected: SearchResult) => selected.equals(pageItem)) < 0); this.selectObject.emit(...filteredPage); }); this.selectableListService.select(this.listId, page); diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/themed-dynamic-lookup-relation-search-tab.component.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/themed-dynamic-lookup-relation-search-tab.component.ts index 71d55e6494..756fee2017 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/themed-dynamic-lookup-relation-search-tab.component.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/themed-dynamic-lookup-relation-search-tab.component.ts @@ -51,9 +51,9 @@ export class ThemedDynamicLookupRelationSearchTabComponent extends ThemedCompone @Input() isEditRelationship: boolean; - @Output() deselectObject: EventEmitter = new EventEmitter(); + @Output() deselectObject: EventEmitter> = new EventEmitter(); - @Output() selectObject: EventEmitter = new EventEmitter(); + @Output() selectObject: EventEmitter> = new EventEmitter(); @Output() resultFound: EventEmitter> = new EventEmitter(); diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/selection-tab/dynamic-lookup-relation-selection-tab.component.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/selection-tab/dynamic-lookup-relation-selection-tab.component.ts index d1e516513b..01cf844e06 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/selection-tab/dynamic-lookup-relation-selection-tab.component.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/selection-tab/dynamic-lookup-relation-selection-tab.component.ts @@ -24,6 +24,7 @@ import { import { RemoteData } from '../../../../../../core/data/remote-data'; import { PaginationService } from '../../../../../../core/pagination/pagination.service'; import { Context } from '../../../../../../core/shared/context.model'; +import { DSpaceObject } from '../../../../../../core/shared/dspace-object.model'; import { PageInfo } from '../../../../../../core/shared/page-info.model'; import { SearchConfigurationService } from '../../../../../../core/shared/search/search-configuration.service'; import { SEARCH_CONFIG_SERVICE } from '../../../../../../my-dspace-page/my-dspace-configuration.service'; @@ -33,6 +34,7 @@ import { PageSizeSelectorComponent } from '../../../../../page-size-selector/pag import { PaginationComponentOptions } from '../../../../../pagination/pagination-component-options.model'; import { createSuccessfulRemoteDataObject } from '../../../../../remote-data.utils'; import { PaginatedSearchOptions } from '../../../../../search/models/paginated-search-options.model'; +import { SearchResult } from '../../../../../search/models/search-result.model'; @Component({ selector: 'ds-dynamic-lookup-relation-selection-tab', @@ -91,12 +93,12 @@ export class DsDynamicLookupRelationSelectionTabComponent { /** * Send an event to deselect an object from the list */ - @Output() deselectObject: EventEmitter = new EventEmitter(); + @Output() deselectObject: EventEmitter> = new EventEmitter(); /** * Send an event to select an object from the list */ - @Output() selectObject: EventEmitter = new EventEmitter(); + @Output() selectObject: EventEmitter> = new EventEmitter(); /** * The initial pagination to use diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index a9529900c2..7a9b87cb4b 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3664,6 +3664,8 @@ "orgunit.page.ror": "ROR Identifier", + "orgunit.search.results.head": "Organizational Unit Search Results", + "pagination.options.description": "Pagination options", "pagination.results-per-page": "Results Per Page", From 58bcb9b8dabf1281622c8fc436ab33627731060d Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Thu, 30 May 2024 10:58:23 +0200 Subject: [PATCH 10/12] 115046: Fixed performance issues in virtual metadata popup --- .../virtual-metadata.component.html | 12 ++--- .../virtual-metadata.component.spec.ts | 1 + .../virtual-metadata.component.ts | 54 +++++++++++++++---- 3 files changed, 51 insertions(+), 16 deletions(-) diff --git a/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.html b/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.html index b83b93d8f1..927f7dbb48 100644 --- a/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.html +++ b/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.html @@ -5,9 +5,9 @@
    - -
    +
    - + -
    +
    {{metadata.metadataField}}
    @@ -31,7 +31,7 @@
    diff --git a/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.spec.ts b/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.spec.ts index 66727871c0..e02df167f5 100644 --- a/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.spec.ts +++ b/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.spec.ts @@ -77,6 +77,7 @@ describe('VirtualMetadataComponent', () => { comp.url = url; comp.leftItem = item; comp.rightItem = relatedItem; + comp.ngOnChanges(); comp.relationshipId = relationshipId; fixture.detectChanges(); diff --git a/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.ts b/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.ts index 11c9a291f8..e8f42c8fc0 100644 --- a/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.ts +++ b/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.ts @@ -8,11 +8,17 @@ import { EventEmitter, Inject, Input, + OnChanges, + OnDestroy, OnInit, Output, } from '@angular/core'; import { TranslateModule } from '@ngx-translate/core'; -import { Observable } from 'rxjs'; +import { + BehaviorSubject, + Observable, + Subscription, +} from 'rxjs'; import { APP_CONFIG, @@ -21,9 +27,18 @@ import { import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service'; import { Item } from '../../../core/shared/item.model'; import { MetadataValue } from '../../../core/shared/metadata.models'; +import { hasValue } from '../../../shared/empty.util'; import { ListableObjectComponentLoaderComponent } from '../../../shared/object-collection/shared/listable-object/listable-object-component-loader.component'; import { VarDirective } from '../../../shared/utils/var.directive'; +interface ItemDTO { + + item: Item; + + isSelectedVirtualMetadataItem$: Observable; + +} + @Component({ selector: 'ds-virtual-metadata', templateUrl: './virtual-metadata.component.html', @@ -42,7 +57,7 @@ import { VarDirective } from '../../../shared/utils/var.directive'; * The component is shown when a relationship is marked to be deleted. * Each item has a checkbox to indicate whether its virtual metadata should be saved as real metadata. */ -export class VirtualMetadataComponent implements OnInit { +export class VirtualMetadataComponent implements OnInit, OnChanges, OnDestroy { /** * The current url of this page @@ -83,9 +98,9 @@ export class VirtualMetadataComponent implements OnInit { /** * Get an array of the left and the right item of the relationship to be deleted. */ - get items() { - return [this.leftItem, this.rightItem]; - } + itemDTOs$: BehaviorSubject = new BehaviorSubject([]); + + subs: Subscription[] = []; public virtualMetadata: Map = new Map(); @@ -137,14 +152,33 @@ export class VirtualMetadataComponent implements OnInit { /** * Prevent unnecessary rerendering so fields don't lose focus */ - trackItem(index, item: Item) { - return item && item.uuid; + trackItemDTO(index, itemDTO: ItemDTO): string { + return itemDTO?.item?.uuid; } ngOnInit(): void { - this.items.forEach((item) => { - this.virtualMetadata.set(item.uuid, this.getVirtualMetadata(item)); - }); + this.subs.push(this.itemDTOs$.subscribe((itemDTOs: ItemDTO[]) => { + itemDTOs.forEach((itemDTO: ItemDTO) => this.virtualMetadata.set(itemDTO.item.uuid, this.getVirtualMetadata(itemDTO.item))); + })); + } + + ngOnChanges(): void { + if (hasValue(this.leftItem) && hasValue(this.rightItem)) { + this.itemDTOs$.next([ + { + item: this.leftItem, + isSelectedVirtualMetadataItem$: this.isSelectedVirtualMetadataItem(this.leftItem), + }, + { + item: this.rightItem, + isSelectedVirtualMetadataItem$: this.isSelectedVirtualMetadataItem(this.rightItem), + }, + ]); + } + } + + ngOnDestroy(): void { + this.subs.forEach((sub: Subscription) => sub.unsubscribe()); } } From a596af03a48ad6d2738acd45df0ba1d1f4a287f2 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Tue, 28 May 2024 13:38:23 +0200 Subject: [PATCH 11/12] 115046: Fixed incorrect name variant being saved in the store when marking relationship ready for deletion --- .../edit-relationship-list.component.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts index 6c3b8d556a..15fec85766 100644 --- a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts +++ b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts @@ -335,16 +335,16 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { modalComp.submitEv = () => { modalComp.isPending = true; const isLeft = this.currentItemIsLeftItem$.getValue(); - const addOperations = modalComp.toAdd.map((searchResult: any) => ({ type: 'add', searchResult })); - const removeOperations = modalComp.toRemove.map((searchResult: any) => ({ type: 'remove', searchResult })); + const addOperations = modalComp.toAdd.map((searchResult: ItemSearchResult) => ({ type: 'add', searchResult })); + const removeOperations = modalComp.toRemove.map((searchResult: ItemSearchResult) => ({ type: 'remove', searchResult })); observableFrom([...addOperations, ...removeOperations]).pipe( - concatMap(({ type, searchResult }: { type: string, searchResult: any }) => { + concatMap(({ type, searchResult }: { type: string, searchResult: ItemSearchResult }) => { + const relatedItem: Item = searchResult.indexableObject; if (type === 'add') { - const relatedItem = searchResult.indexableObject; return this.relationshipService.getNameVariant(this.listId, relatedItem.uuid).pipe( map((nameVariant) => { const update = { - uuid: this.relationshipType.id + '-' + searchResult.indexableObject.uuid, + uuid: `${this.relationshipType.id}-${relatedItem.uuid}`, nameVariant, type: this.relationshipType, originalIsLeft: isLeft, @@ -357,7 +357,7 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { take(1), ); } else if (type === 'remove') { - return this.relationshipService.getNameVariant(this.listId, searchResult.indexableObjectuuid).pipe( + return this.relationshipService.getNameVariant(this.listId, relatedItem.uuid).pipe( switchMap((nameVariant) => { return this.getRelationFromId(searchResult.indexableObject).pipe( map( (relationship: Relationship) => { From 62e07e73357c73a8ff5a8a4bee8db409317b7439 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Thu, 30 May 2024 16:55:35 +0200 Subject: [PATCH 12/12] 115046: Fixed issue where store operations would sometimes be performed in incorrect order --- .../object-updates/object-updates.service.ts | 17 +++++++++++++++-- .../edit-relationship-list.component.ts | 8 +++----- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/app/core/data/object-updates/object-updates.service.ts b/src/app/core/data/object-updates/object-updates.service.ts index 1f036ddfb1..75b554d87b 100644 --- a/src/app/core/data/object-updates/object-updates.service.ts +++ b/src/app/core/data/object-updates/object-updates.service.ts @@ -15,6 +15,7 @@ import { filter, map, switchMap, + take, } from 'rxjs/operators'; import { @@ -212,8 +213,14 @@ export class ObjectUpdatesService { * @param url The page's URL for which the changes are saved * @param field An updated field for the page's object */ - saveAddFieldUpdate(url: string, field: Identifiable) { + saveAddFieldUpdate(url: string, field: Identifiable): Observable { + const update$: Observable = this.getFieldUpdatesExclusive(url, [field]).pipe( + filter((fieldUpdates: FieldUpdates) => fieldUpdates[field.uuid].changeType === FieldChangeType.ADD), + take(1), + map(() => true), + ); this.saveFieldUpdate(url, field, FieldChangeType.ADD); + return update$; } /** @@ -221,8 +228,14 @@ export class ObjectUpdatesService { * @param url The page's URL for which the changes are saved * @param field An updated field for the page's object */ - saveRemoveFieldUpdate(url: string, field: Identifiable) { + saveRemoveFieldUpdate(url: string, field: Identifiable): Observable { + const update$: Observable = this.getFieldUpdatesExclusive(url, [field]).pipe( + filter((fieldUpdates: FieldUpdates) => fieldUpdates[field.uuid].changeType === FieldChangeType.REMOVE), + take(1), + map(() => true), + ); this.saveFieldUpdate(url, field, FieldChangeType.REMOVE); + return update$; } /** diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts index 15fec85766..e396c5ee92 100644 --- a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts +++ b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts @@ -342,7 +342,7 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { const relatedItem: Item = searchResult.indexableObject; if (type === 'add') { return this.relationshipService.getNameVariant(this.listId, relatedItem.uuid).pipe( - map((nameVariant) => { + switchMap((nameVariant) => { const update = { uuid: `${this.relationshipType.id}-${relatedItem.uuid}`, nameVariant, @@ -351,8 +351,7 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { originalItem: this.item, relatedItem, } as RelationshipIdentifiable; - this.objectUpdatesService.saveAddFieldUpdate(this.url, update); - return update; + return this.objectUpdatesService.saveAddFieldUpdate(this.url, update); }), take(1), ); @@ -369,8 +368,7 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy { originalItem: this.item, relationship, } as RelationshipIdentifiable; - this.objectUpdatesService.saveRemoveFieldUpdate(this.url,update); - return update; + return this.objectUpdatesService.saveRemoveFieldUpdate(this.url,update); }), ); }),