From 27686dc68cafadd2108b2cfc55e5c6d7f12292fd Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 3 Dec 2025 11:41:49 +0100 Subject: [PATCH 1/5] 136580: Submission - Replace plaintext with relationship --- ...ynamic-form-control-container.component.ts | 5 + ...ic-lookup-relation-modal.component.spec.ts | 42 +++++-- ...dynamic-lookup-relation-modal.component.ts | 33 +++++- .../relationship.actions.ts | 44 +++++++ .../relationship.effects.spec.ts | 85 +++++++++++++- .../relationship.effects.ts | 109 ++++++++++++++---- 6 files changed, 283 insertions(+), 35 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.ts index ff5a119b6fc..3648b87a02c 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.ts @@ -455,6 +455,11 @@ export class DsDynamicFormControlContainerComponent extends DynamicFormControlCo modalComp.query = this.model.value; } else if (typeof this.model.value.value === 'string') { modalComp.query = this.model.value.value; + // If the existing value is not virtual, store properties on the modal required to perform a replace operation + if (!this.model.value.isVirtual) { + modalComp.replaceValuePlace = this.model.value.place; + modalComp.replaceValueMetadataField = this.model.name; + } } } diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.spec.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.spec.ts index 9d57296f826..fb2894026ae 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.spec.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.spec.ts @@ -12,7 +12,7 @@ import { Store } from '@ngrx/store'; import { Item } from '../../../../../core/shared/item.model'; import { ItemSearchResult } from '../../../../object-collection/shared/item-search-result.model'; import { RelationshipOptions } from '../../models/relationship-options.model'; -import { AddRelationshipAction, RemoveRelationshipAction } from './relationship.actions'; +import { AddRelationshipAction, RemoveRelationshipAction, ReplaceRelationshipAction } from './relationship.actions'; import { SearchConfigurationService } from '../../../../../core/shared/search/search-configuration.service'; import { PaginatedSearchOptions } from '../../../../search/models/paginated-search-options.model'; import { ExternalSource } from '../../../../../core/shared/external-source.model'; @@ -32,9 +32,11 @@ describe('DsDynamicLookupRelationModalComponent', () => { let item; let item1; let item2; + let item3; let testWSI; let searchResult1; let searchResult2; + let searchResult3; let listID; let selection$; let selectableListService; @@ -68,11 +70,13 @@ describe('DsDynamicLookupRelationModalComponent', () => { item = Object.assign(new Item(), { uuid: '7680ca97-e2bd-4398-bfa7-139a8673dc42', metadata: {} }); item1 = Object.assign(new Item(), { uuid: 'e1c51c69-896d-42dc-8221-1d5f2ad5516e' }); item2 = Object.assign(new Item(), { uuid: 'c8279647-1acc-41ae-b036-951d5f65649b' }); + item3 = Object.assign(new Item(), { uuid: '6264b66f-ae25-4221-b72a-8696536c5ebb' }); testWSI = new WorkspaceItem(); testWSI.item = createSuccessfulRemoteDataObject$(item); testWSI.collection = createSuccessfulRemoteDataObject$(collection); searchResult1 = Object.assign(new ItemSearchResult(), { indexableObject: item1 }); searchResult2 = Object.assign(new ItemSearchResult(), { indexableObject: item2 }); + searchResult3 = Object.assign(new ItemSearchResult(), { indexableObject: item3 }); listID = '6b0c8221-fcb4-47a8-b483-ca32363fffb3'; selection$ = observableOf([searchResult1, searchResult2]); selectableListService = { getSelectableList: () => selection$ }; @@ -172,13 +176,37 @@ describe('DsDynamicLookupRelationModalComponent', () => { spyOn((component as any).store, 'dispatch'); }); - it('should dispatch an AddRelationshipAction for each selected object', () => { - component.select(searchResult1, searchResult2); - const action = new AddRelationshipAction(component.item, searchResult1.indexableObject, relationship.relationshipType, submissionId, nameVariant); - const action2 = new AddRelationshipAction(component.item, searchResult2.indexableObject, relationship.relationshipType, submissionId, nameVariant); + describe('when replace properties are present', () => { + beforeEach(() => { + component.replaceValuePlace = 3; + component.replaceValueMetadataField = 'dc.subject'; + }); - expect((component as any).store.dispatch).toHaveBeenCalledWith(action); - expect((component as any).store.dispatch).toHaveBeenCalledWith(action2); + it('should dispatch a ReplaceRelationshipAction for the first selected object and a AddRelationshipAction for every other selected object', () => { + component.select(searchResult1, searchResult2, searchResult3); + const action1 = new ReplaceRelationshipAction(component.item, searchResult1.indexableObject, true, 3, 'dc.subject', relationship.relationshipType, submissionId, nameVariant); + const action2 = new AddRelationshipAction(component.item, searchResult2.indexableObject, relationship.relationshipType, submissionId, nameVariant); + const action3 = new AddRelationshipAction(component.item, searchResult3.indexableObject, relationship.relationshipType, submissionId, nameVariant); + + expect((component as any).store.dispatch).toHaveBeenCalledWith(action1); + expect((component as any).store.dispatch).toHaveBeenCalledWith(action2); + expect((component as any).store.dispatch).toHaveBeenCalledWith(action3); + expect(component.replaceValuePlace).toBeUndefined(); + expect(component.replaceValueMetadataField).toBeUndefined(); + }); + }); + + describe('when replace properties are missing', () => { + it('should dispatch an AddRelationshipAction for each selected object', () => { + component.select(searchResult1, searchResult2, searchResult3); + const action1 = new AddRelationshipAction(component.item, searchResult1.indexableObject, relationship.relationshipType, submissionId, nameVariant); + const action2 = new AddRelationshipAction(component.item, searchResult2.indexableObject, relationship.relationshipType, submissionId, nameVariant); + const action3 = new AddRelationshipAction(component.item, searchResult3.indexableObject, relationship.relationshipType, submissionId, nameVariant); + + expect((component as any).store.dispatch).toHaveBeenCalledWith(action1); + expect((component as any).store.dispatch).toHaveBeenCalledWith(action2); + expect((component as any).store.dispatch).toHaveBeenCalledWith(action3); + }); }); }); diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts index 446497a74fc..9100793b2a9 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts @@ -13,7 +13,7 @@ import { SearchResult } from '../../../../search/models/search-result.model'; import { Item } from '../../../../../core/shared/item.model'; import { AddRelationshipAction, - RemoveRelationshipAction, + RemoveRelationshipAction, ReplaceRelationshipAction, UpdateRelationshipNameVariantAction, } from './relationship.actions'; import { RelationshipDataService } from '../../../../../core/data/relationship-data.service'; @@ -95,6 +95,17 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy query: string; + /** + * The index of the plain-text value that should be replaced by adding a relationship + */ + replaceValuePlace: number; + + /** + * The metadata field of the value to replace with a relationship + * Undefined if no value needs replacing + */ + replaceValueMetadataField: string; + /** * A map of subscriptions within this component */ @@ -235,9 +246,17 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy ]); obs .subscribe((arr: any[]) => { - return arr.forEach((object: any) => { - const addRelationshipAction = new AddRelationshipAction(this.item, object.item, this.relationshipOptions.relationshipType, this.submissionId, object.nameVariant); - this.store.dispatch(addRelationshipAction); + return arr.forEach((object: any, i: number) => { + let action; + if (i === 0 && hasValue(this.replaceValueMetadataField)) { + // This is the first action this modal performs and "replace" properties are present to replace an existing metadata value + action = new ReplaceRelationshipAction(this.item, object.item, true, this.replaceValuePlace, this.replaceValueMetadataField, this.relationshipOptions.relationshipType, this.submissionId, object.nameVariant); + // Only "replace" once, reset replace properties so future actions become "add" + this.resetReplaceProperties(); + } else { + action = new AddRelationshipAction(this.item, object.item, this.relationshipOptions.relationshipType, this.submissionId, object.nameVariant); + } + this.store.dispatch(action); } ); }); @@ -260,6 +279,7 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy * @param selectableObjects */ deselect(...selectableObjects: SearchResult[]) { + this.resetReplaceProperties(); this.zone.runOutsideAngular( () => selectableObjects.forEach((object) => { this.subMap[object.indexableObject.uuid].unsubscribe(); @@ -297,6 +317,11 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy this.totalInternal$.next(totalPages); } + private resetReplaceProperties() { + this.replaceValueMetadataField = undefined; + this.replaceValuePlace = undefined; + } + ngOnDestroy() { this.router.navigate([], {}); Object.values(this.subMap).forEach((subscription) => subscription.unsubscribe()); diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.actions.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.actions.ts index fb2271224bd..2b317947253 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.actions.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.actions.ts @@ -9,6 +9,7 @@ import { Relationship } from '../../../../../core/shared/item-relationships/rela export const RelationshipActionTypes = { ADD_RELATIONSHIP: type('dspace/relationship/ADD_RELATIONSHIP'), + REPLACE_RELATIONSHIP: type('dspace/relationship/REPLACE_RELATIONSHIP'), REMOVE_RELATIONSHIP: type('dspace/relationship/REMOVE_RELATIONSHIP'), UPDATE_NAME_VARIANT: type('dspace/relationship/UPDATE_NAME_VARIANT'), UPDATE_RELATIONSHIP: type('dspace/relationship/UPDATE_RELATIONSHIP'), @@ -132,10 +133,53 @@ export class RemoveRelationshipAction implements Action { } } +/** + * An ngrx action to replace a plain-text metadata value with a new relationship + */ +export class ReplaceRelationshipAction implements Action { + type = RelationshipActionTypes.REPLACE_RELATIONSHIP; + + payload: { + item1: Item; + item2: Item; + replaceLeftSide: boolean; + place: number; + mdField: string; + relationshipType: string; + submissionId: string; + nameVariant: string; + }; + + /** + * Create a new AddRelationshipAction + * + * @param item1 The first item in the relationship + * @param item2 The second item in the relationship + * @param replaceLeftSide If true, the item on the left side (item1) will have its metadata value replaced + * @param place The index of the metadata value that should be replaced with the new relationship + * @param mdField The metadata field of the value to replace + * @param relationshipType The label of the relationshipType + * @param submissionId The current submissionId + * @param nameVariant The nameVariant of the relationshipType + */ + constructor( + item1: Item, + item2: Item, + replaceLeftSide: boolean, + place: number, + mdField: string, + relationshipType: string, + submissionId: string, + nameVariant?: string, + ) { + this.payload = { item1, item2, replaceLeftSide, place, mdField, relationshipType, submissionId, nameVariant }; + } +} /** * A type to encompass all RelationshipActions */ export type RelationshipAction = AddRelationshipAction + | ReplaceRelationshipAction | RemoveRelationshipAction; diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts index a5b1b00d75e..a00ec490d8d 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts @@ -3,7 +3,12 @@ import { BehaviorSubject, Observable, of as observableOf } from 'rxjs'; import { provideMockActions } from '@ngrx/effects/testing'; import { Store } from '@ngrx/store'; import { RelationshipEffects } from './relationship.effects'; -import { AddRelationshipAction, RelationshipActionTypes, RemoveRelationshipAction } from './relationship.actions'; +import { + AddRelationshipAction, + RelationshipActionTypes, + RemoveRelationshipAction, + ReplaceRelationshipAction +} from './relationship.actions'; import { Item } from '../../../../../core/shared/item.model'; import { MetadataValue } from '../../../../../core/shared/metadata.models'; import { RelationshipTypeDataService } from '../../../../../core/data/relationship-type-data.service'; @@ -23,6 +28,7 @@ import { SelectableListService } from '../../../../object-list/selectable-list/s import { cold, hot } from 'jasmine-marbles'; import { DEBOUNCE_TIME_OPERATOR } from '../../../../../core/shared/operators'; import { last } from 'rxjs/operators'; +import { ItemDataService } from '../../../../../core/data/item-data.service'; describe('RelationshipEffects', () => { let relationEffects: RelationshipEffects; @@ -51,6 +57,7 @@ describe('RelationshipEffects', () => { let notificationsService; let translateService; let selectableListService; + let itemService; function init() { testUUID1 = '20e24c2f-a00a-467c-bdee-c929e79bf08d'; @@ -93,8 +100,8 @@ describe('RelationshipEffects', () => { getRelationshipByItemsAndLabel: () => observableOf(relationship), deleteRelationship: () => observableOf(new RestResponse(true, 200, 'OK')), - addRelationship: () => observableOf(new RestResponse(true, 200, 'OK')) - + addRelationship: () => createSuccessfulRemoteDataObject$(new Relationship()), + update: () => createSuccessfulRemoteDataObject$(new Relationship()), }; mockRelationshipTypeService = { getRelationshipTypeByLabelAndTypes: @@ -108,6 +115,9 @@ describe('RelationshipEffects', () => { findSelectedByCondition: observableOf({}), deselectSingle: {} }); + itemService = jasmine.createSpyObj('itemService', { + patch: createSuccessfulRemoteDataObject$(new Item()), + }); } beforeEach(waitForAsync(() => { @@ -118,6 +128,7 @@ describe('RelationshipEffects', () => { provideMockActions(() => actions), { provide: RelationshipTypeDataService, useValue: mockRelationshipTypeService }, { provide: RelationshipDataService, useValue: mockRelationshipService }, + { provide: ItemDataService, useValue: itemService }, { provide: SubmissionObjectDataService, useValue: { findById: () => createSuccessfulRemoteDataObject$(new WorkspaceItem()) @@ -140,6 +151,7 @@ describe('RelationshipEffects', () => { identifier = (relationEffects as any).createIdentifier(leftItem, rightItem, relationshipType.leftwardType); spyOn((relationEffects as any), 'addRelationship').and.stub(); spyOn((relationEffects as any), 'removeRelationship').and.stub(); + spyOn((relationEffects as any), 'replaceRelationship').and.stub(); }); describe('mapLastActions$', () => { @@ -208,6 +220,73 @@ describe('RelationshipEffects', () => { }); }); + describe('When a REPLACE_RELATIONSHIP action is triggered', () => { + describe('When it\'s the first time for this identifier', () => { + let action; + + it('should set the current value debounceMap and the value of the initialActionMap to REPLACE_RELATIONSHIP', () => { + action = new ReplaceRelationshipAction(leftItem, rightItem, true, 0, 'dc.subject', relationshipType.leftwardType, '1234'); + actions = hot('--a-|', { a: action }); + const expected = cold('--b-|', { b: undefined }); + expect(relationEffects.mapLastActions$).toBeObservable(expected); + + expect((relationEffects as any).initialActionMap[identifier]).toBe(action.type); + expect((relationEffects as any).debounceMap[identifier].value).toBe(action.type); + }); + }); + + describe('When it\'s not the first time for this identifier', () => { + let action; + const testActionType = 'TEST_TYPE'; + beforeEach(() => { + (relationEffects as any).initialActionMap[identifier] = testActionType; + (relationEffects as any).debounceMap[identifier] = new BehaviorSubject(testActionType); + }); + + it('should set the current value debounceMap to REPLACE_RELATIONSHIP but not change the value of the initialActionMap', () => { + action = new ReplaceRelationshipAction(leftItem, rightItem, true, 0, 'dc.subject', relationshipType.leftwardType, '1234'); + actions = hot('--a-|', { a: action }); + + const expected = cold('--b-|', { b: undefined }); + expect(relationEffects.mapLastActions$).toBeObservable(expected); + + expect((relationEffects as any).initialActionMap[identifier]).toBe(testActionType); + expect((relationEffects as any).debounceMap[identifier].value).toBe(action.type); + }); + }); + + describe('When the initialActionMap contains a REPLACE_RELATIONSHIP action', () => { + let action; + describe('When the last value in the debounceMap is also a REPLACE_RELATIONSHIP action', () => { + beforeEach(() => { + spyOn((relationEffects as any).relationshipService, 'update').and.callThrough(); + ((relationEffects as any).debounceTime as jasmine.Spy).and.returnValue((v) => v); + (relationEffects as any).initialActionMap[identifier] = RelationshipActionTypes.REPLACE_RELATIONSHIP; + }); + + it('should call replaceRelationship on the effect', () => { + action = new ReplaceRelationshipAction(leftItem, rightItem, true, 0, 'dc.subject', relationshipType.leftwardType, '1234'); + actions = hot('--a-|', { a: action }); + const expected = cold('--b-|', { b: undefined }); + expect(relationEffects.mapLastActions$).toBeObservable(expected); + expect((relationEffects as any).replaceRelationship).toHaveBeenCalledWith(leftItem, rightItem, true, 0, 'dc.subject', relationshipType.leftwardType, '1234', undefined); + }); + }); + + describe('When the last value in the debounceMap is instead a REMOVE_RELATIONSHIP action', () => { + it('should not call removeRelationship or replaceRelationship on the effect', () => { + const actiona = new ReplaceRelationshipAction(leftItem, rightItem, true, 0, 'dc.subject', relationshipType.leftwardType, '1234'); + const actionb = new RemoveRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); + actions = hot('--ab-|', { a: actiona, b: actionb }); + const expected = cold('--bb-|', { b: undefined }); + expect(relationEffects.mapLastActions$).toBeObservable(expected); + expect((relationEffects as any).replaceRelationship).not.toHaveBeenCalled(); + expect((relationEffects as any).removeRelationship).not.toHaveBeenCalled(); + }); + }); + }); + }); + describe('When an REMOVE_RELATIONSHIP action is triggered', () => { describe('When it\'s the first time for this identifier', () => { let action; diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts index 2b9c1c29737..c1aa3cd6c1b 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts @@ -5,12 +5,15 @@ import { BehaviorSubject, Observable } from 'rxjs'; import { RelationshipDataService } from '../../../../../core/data/relationship-data.service'; import { getRemoteDataPayload, - getFirstSucceededRemoteData, DEBOUNCE_TIME_OPERATOR + getFirstSucceededRemoteData, + DEBOUNCE_TIME_OPERATOR, + getFirstCompletedRemoteData, } from '../../../../../core/shared/operators'; import { AddRelationshipAction, RelationshipAction, RelationshipActionTypes, + ReplaceRelationshipAction, UpdateRelationshipAction, UpdateRelationshipNameVariantAction } from './relationship.actions'; @@ -33,6 +36,8 @@ import { RemoteData } from '../../../../../core/data/remote-data'; import { NotificationsService } from '../../../../notifications/notifications.service'; import { SelectableListService } from '../../../../object-list/selectable-list/selectable-list.service'; import { TranslateService } from '@ngx-translate/core'; +import { ItemDataService } from '../../../../../core/data/item-data.service'; +import { Operation } from 'fast-json-patch'; const DEBOUNCE_TIME = 500; @@ -63,7 +68,7 @@ export class RelationshipEffects { */ mapLastActions$ = createEffect(() => this.actions$ .pipe( - ofType(RelationshipActionTypes.ADD_RELATIONSHIP, RelationshipActionTypes.REMOVE_RELATIONSHIP), + ofType(RelationshipActionTypes.ADD_RELATIONSHIP, RelationshipActionTypes.REPLACE_RELATIONSHIP, RelationshipActionTypes.REMOVE_RELATIONSHIP), map((action: RelationshipAction) => { const { item1, item2, submissionId, relationshipType } = action.payload; const identifier: string = this.createIdentifier(item1, item2, relationshipType); @@ -76,13 +81,18 @@ export class RelationshipEffects { ).subscribe( (type) => { if (this.initialActionMap[identifier] === type) { - if (type === RelationshipActionTypes.ADD_RELATIONSHIP) { + if (type === RelationshipActionTypes.ADD_RELATIONSHIP || type === RelationshipActionTypes.REPLACE_RELATIONSHIP) { let nameVariant = (action as AddRelationshipAction).payload.nameVariant; if (hasValue(this.nameVariantUpdates[identifier])) { nameVariant = this.nameVariantUpdates[identifier]; delete this.nameVariantUpdates[identifier]; } - this.addRelationship(item1, item2, relationshipType, submissionId, nameVariant); + if (type === RelationshipActionTypes.ADD_RELATIONSHIP) { + this.addRelationship(item1, item2, relationshipType, submissionId, nameVariant); + } else { + const replaceAction = action as ReplaceRelationshipAction; + this.replaceRelationship(item1, item2, replaceAction.payload.replaceLeftSide, replaceAction.payload.place, replaceAction.payload.mdField, relationshipType, submissionId, nameVariant); + } } else { this.removeRelationship(item1, item2, relationshipType, submissionId); } @@ -152,6 +162,7 @@ export class RelationshipEffects { constructor(private actions$: Actions, private relationshipService: RelationshipDataService, private relationshipTypeService: RelationshipTypeDataService, + private itemService: ItemDataService, private submissionObjectService: SubmissionObjectDataService, private store: Store, private objectCache: ObjectCacheService, @@ -168,6 +179,12 @@ export class RelationshipEffects { } private addRelationship(item1: Item, item2: Item, relationshipType: string, submissionId: string, nameVariant?: string) { + this.addRelationshipObs(item1, item2, relationshipType, submissionId, nameVariant).pipe( + switchMap(() => this.refreshWorkspaceItemInCache(submissionId)) + ).subscribe((submissionObject: SubmissionObject) => this.store.dispatch(new SaveSubmissionSectionFormSuccessAction(submissionId, [submissionObject], false))); + } + + private addRelationshipObs(item1: Item, item2: Item, relationshipType: string, submissionId: string, nameVariant?: string): Observable { const type1: string = item1.firstMetadataValue('dspace.entity.type'); const type2: string = item2.firstMetadataValue('dspace.entity.type'); return this.relationshipTypeService.getRelationshipTypeByLabelAndTypes(relationshipType, type1, type2) @@ -185,27 +202,77 @@ export class RelationshipEffects { } }), take(1), - switchMap((rd: RemoteData) => { + map((rd: RemoteData) => { if (hasNoValue(rd) || rd.hasFailed) { // An error occurred, deselect the object from the selectable list and display an error notification - const listId = `list-${submissionId}-${relationshipType}`; - this.selectableListService.findSelectedByCondition(listId, (object: any) => hasValue(object.indexableObject) && object.indexableObject.uuid === item2.uuid).pipe( - take(1), - hasValueOperator() - ).subscribe((selected) => { - this.selectableListService.deselectSingle(listId, selected); - }); - let errorContent; - if (hasNoValue(rd)) { - errorContent = this.translateService.instant('relationships.add.error.relationship-type.content', { type: relationshipType }); - } else { - errorContent = this.translateService.instant('relationships.add.error.server.content'); - } - this.notificationsService.error(this.translateService.instant('relationships.add.error.title'), errorContent); + this.deselectAndShowError(item1, item2, relationshipType, submissionId, hasNoValue(rd)); } - return this.refreshWorkspaceItemInCache(submissionId); + return rd.hasSucceeded && hasValue(rd.payload) ? rd.payload : undefined; }), - ).subscribe((submissionObject: SubmissionObject) => this.store.dispatch(new SaveSubmissionSectionFormSuccessAction(submissionId, [submissionObject], false))); + ); + } + + private deselectAndShowError(item1: Item, item2: Item, relationshipType: string, submissionId: string, noMatchFound = false) { + const listId = `list-${submissionId}-${relationshipType}`; + this.selectableListService.findSelectedByCondition(listId, (object: any) => hasValue(object.indexableObject) && object.indexableObject.uuid === item2.uuid).pipe( + take(1), + hasValueOperator() + ).subscribe((selected) => { + this.selectableListService.deselectSingle(listId, selected); + }); + let errorContent; + if (noMatchFound) { + errorContent = this.translateService.instant('relationships.add.error.relationship-type.content', { type: relationshipType }); + } else { + errorContent = this.translateService.instant('relationships.add.error.server.content'); + } + this.notificationsService.error(this.translateService.instant('relationships.add.error.title'), errorContent); + } + + /** + * Perform a "replace" of a metadata value with a new relationship + * A replace happens in three steps: + * - The old metadata value is removed with an item PATCH + * - The new relationship is created + * - The relationship's place is updated to fit the old place of the removed metadata value + * @param item1 First item in the relationship to create + * @param item2 Second item in the relationship to create + * @param replaceLeftSide If true, item1 will have its metadata value replaced, otherwise item2 + * @param place The index of the metadata value to replace + * @param metadataField The metadata field of the metadata value to replace + * @param relationshipType The type of relationship + * @param submissionId The ID of the submission this action is taking place in + * @param nameVariant Optional name variant of the to-be-created relationship + * @private + */ + private replaceRelationship(item1: Item, item2: Item, replaceLeftSide: boolean, place: number, metadataField: string, relationshipType: string, submissionId: string, nameVariant?: string) { + this.itemService.patch(replaceLeftSide ? item1 : item2, [{ op: 'remove', path: `/metadata/${metadataField}/${place}` } as Operation]).pipe( + getFirstCompletedRemoteData(), + switchMap((rd: RemoteData) => { + if (rd.hasSucceeded) { + return this.addRelationshipObs(item1, item2, relationshipType, submissionId, nameVariant); + } else { + this.deselectAndShowError(item1, item2, relationshipType, submissionId); + return [undefined]; + } + }), + switchMap((rel: Relationship) => { + if (hasValue(rel)) { + const updatedRelationship: Relationship = Object.assign(new Relationship(), rel); + if (replaceLeftSide) { + updatedRelationship.leftPlace = place; + } else { + updatedRelationship.rightPlace = place; + } + return this.relationshipService.update(updatedRelationship).pipe( + getFirstCompletedRemoteData(), + ); + } else { + return [undefined]; + } + }), + switchMap(() => this.refreshWorkspaceItemInCache(submissionId)), + ).subscribe((submissionObject: SubmissionObject) => this.store.dispatch(new SaveSubmissionSectionFormSuccessAction(submissionId, [submissionObject], false))); } private removeRelationship(item1: Item, item2: Item, relationshipType: string, submissionId: string) { From 9d45fd04e2e9d92fe16591ea55129a1bc6809679 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 3 Dec 2025 13:36:55 +0100 Subject: [PATCH 2/5] 136580: replace relationship queue post-merge --- .../relationship.effects.spec.ts | 2 ++ .../relationship.effects.ts | 35 ++++++++++++------- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts index 8bcd108c08f..a9a56731d02 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts @@ -261,6 +261,8 @@ describe('RelationshipEffects', () => { let action; describe('When the last value in the debounceMap is also a REPLACE_RELATIONSHIP action', () => { beforeEach(() => { + jasmine.getEnv().allowRespy(true); + spyOn((relationEffects as any), 'replaceRelationship').and.returnValue(createSuccessfulRemoteDataObject$(relationship)); spyOn((relationEffects as any).relationshipService, 'update').and.callThrough(); ((relationEffects as any).debounceTime as jasmine.Spy).and.returnValue((v) => v); (relationEffects as any).initialActionMap[identifier] = RelationshipActionTypes.REPLACE_RELATIONSHIP; diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts index 751ffe84e22..63208baead8 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts @@ -44,6 +44,7 @@ const DEBOUNCE_TIME = 500; enum RelationOperationType { Add, Remove, + Replace, } interface RelationOperation { @@ -53,6 +54,9 @@ interface RelationOperation { relationshipType: string submissionId: string nameVariant?: string + replaceLeftSide?: boolean + place?: number + mdField?: string } /** @@ -118,7 +122,17 @@ export class RelationshipEffects { }); } else { const replaceAction = action as ReplaceRelationshipAction; - this.replaceRelationship(item1, item2, replaceAction.payload.replaceLeftSide, replaceAction.payload.place, replaceAction.payload.mdField, relationshipType, submissionId, nameVariant); + this.requestQueue.next({ + type: RelationOperationType.Replace, + item1, + item2, + relationshipType, + submissionId, + nameVariant, + replaceLeftSide: replaceAction.payload.replaceLeftSide, + place: replaceAction.payload.place, + mdField: replaceAction.payload.mdField, + }); } } else { this.requestQueue.next({ @@ -222,6 +236,10 @@ export class RelationshipEffects { return this.addRelationship(next.item1, next.item2, next.relationshipType, next.submissionId, next.nameVariant).pipe( map(() => next) ); + case RelationOperationType.Replace: + return this.replaceRelationship(next.item1, next.item2, next.replaceLeftSide, next.place, next.mdField, next.relationshipType, next.submissionId, next.nameVariant).pipe( + map(() => next) + ); case RelationOperationType.Remove: return this.removeRelationship(next.item1, next.item2, next.relationshipType).pipe( map(() => next) @@ -244,13 +262,7 @@ export class RelationshipEffects { return `${item1.uuid}-${item2.uuid}-${relationshipType}`; } - private addRelationship(item1: Item, item2: Item, relationshipType: string, submissionId: string, nameVariant?: string) { - this.addRelationshipObs(item1, item2, relationshipType, submissionId, nameVariant).pipe( - switchMap(() => this.refreshWorkspaceItemInCache(submissionId)) - ).subscribe((submissionObject: SubmissionObject) => this.store.dispatch(new SaveSubmissionSectionFormSuccessAction(submissionId, [submissionObject], false))); - } - - private addRelationshipObs(item1: Item, item2: Item, relationshipType: string, submissionId: string, nameVariant?: string): Observable { + private addRelationship(item1: Item, item2: Item, relationshipType: string, submissionId: string, nameVariant?: string): Observable { const type1: string = item1.firstMetadataValue('dspace.entity.type'); const type2: string = item2.firstMetadataValue('dspace.entity.type'); return this.relationshipTypeService.getRelationshipTypeByLabelAndTypes(relationshipType, type1, type2) @@ -312,11 +324,11 @@ export class RelationshipEffects { * @private */ private replaceRelationship(item1: Item, item2: Item, replaceLeftSide: boolean, place: number, metadataField: string, relationshipType: string, submissionId: string, nameVariant?: string) { - this.itemService.patch(replaceLeftSide ? item1 : item2, [{ op: 'remove', path: `/metadata/${metadataField}/${place}` } as Operation]).pipe( + return this.itemService.patch(replaceLeftSide ? item1 : item2, [{ op: 'remove', path: `/metadata/${metadataField}/${place}` } as Operation]).pipe( getFirstCompletedRemoteData(), switchMap((rd: RemoteData) => { if (rd.hasSucceeded) { - return this.addRelationshipObs(item1, item2, relationshipType, submissionId, nameVariant); + return this.addRelationship(item1, item2, relationshipType, submissionId, nameVariant); } else { this.deselectAndShowError(item1, item2, relationshipType, submissionId); return [undefined]; @@ -337,8 +349,7 @@ export class RelationshipEffects { return [undefined]; } }), - switchMap(() => this.refreshWorkspaceItemInCache(submissionId)), - ).subscribe((submissionObject: SubmissionObject) => this.store.dispatch(new SaveSubmissionSectionFormSuccessAction(submissionId, [submissionObject], false))); + ); } private removeRelationship(item1: Item, item2: Item, relationshipType: string) { From 80070a82546049c003a8eb0d4dda47bcd3350248 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 3 Dec 2025 14:04:28 +0100 Subject: [PATCH 3/5] 136580: Lint fix --- .../relation-lookup-modal/relationship.effects.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts index 63208baead8..d41fdfcb4dc 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts @@ -1,6 +1,6 @@ import { Inject, Injectable } from '@angular/core'; import { Actions, createEffect, ofType } from '@ngrx/effects'; -import { filter, map, mergeMap, switchMap, take, tap, concatMap } from 'rxjs/operators'; +import { filter, map, mergeMap, switchMap, take, concatMap } from 'rxjs/operators'; import { BehaviorSubject, Observable, Subject } from 'rxjs'; import { RelationshipDataService } from '../../../../../core/data/relationship-data.service'; import { From b4c244ee344d2fc3836c64092bcca190e504e0d5 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Thu, 4 Dec 2025 23:47:06 +0100 Subject: [PATCH 4/5] 136580: Fixed selectObject being called twice for non-repeatable fields --- .../selectable-list-item-control.component.ts | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/app/shared/object-collection/shared/selectable-list-item-control/selectable-list-item-control.component.ts b/src/app/shared/object-collection/shared/selectable-list-item-control/selectable-list-item-control.component.ts index ebe49331d5c..e0d652ad2b9 100644 --- a/src/app/shared/object-collection/shared/selectable-list-item-control/selectable-list-item-control.component.ts +++ b/src/app/shared/object-collection/shared/selectable-list-item-control/selectable-list-item-control.component.ts @@ -1,8 +1,8 @@ import { Component, EventEmitter, Input, OnInit, Output } from '@angular/core'; import { ListableObject } from '../listable-object.model'; import { SelectableListService } from '../../../object-list/selectable-list/selectable-list.service'; -import { map, skip, take } from 'rxjs/operators'; -import { Observable } from 'rxjs'; +import { map, take } from 'rxjs/operators'; +import { BehaviorSubject } from 'rxjs'; @Component({ selector: 'ds-selectable-list-item-control', @@ -29,7 +29,7 @@ export class SelectableListItemControlComponent implements OnInit { @Output() selectObject: EventEmitter = new EventEmitter(); - selected$: Observable; + selected$: BehaviorSubject = new BehaviorSubject(false); constructor(public selectionService: SelectableListService) { } @@ -38,14 +38,19 @@ export class SelectableListItemControlComponent implements OnInit { * Setup the dynamic child component */ ngOnInit(): void { - this.selected$ = this.selectionService.isObjectSelected(this.selectionConfig.listId, this.object); - this.selected$ - .pipe(skip(1)).subscribe((selected: boolean) => { - if (selected) { - this.selectObject.emit(this.object); - } else { - this.deselectObject.emit(this.object); + let first = true; + // TODO in the future this should be refactored because it creates a memory leak, but without it closing the modal + // too early can prevent authors from being added + this.selectionService.isObjectSelected(this.selectionConfig.listId, this.object).subscribe((selected: boolean) => { + if (!first && this.selected$.value !== selected) { + if (selected) { + this.selectObject.emit(this.object); + } else { + this.deselectObject.emit(this.object); + } } + this.selected$.next(selected); + first = false; }); } @@ -64,6 +69,7 @@ export class SelectableListItemControlComponent implements OnInit { take(1), map((selected) => selected ? selected.selection : []) ).subscribe((selection) => { + this.selected$.next(value); // First deselect any existing selections, this is a radio button selection.forEach((selectedObject) => { this.selectionService.deselectSingle(this.selectionConfig.listId, selectedObject); From df0277b3f1b32f3a800d1c02e1cced952cdc5a8e Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 10 Dec 2025 13:38:31 +0100 Subject: [PATCH 5/5] 136580: Submission select multiple relationships fix --- ...amic-lookup-relation-search-tab.component.spec.ts | 8 ++------ .../dynamic-lookup-relation-search-tab.component.ts | 12 ------------ 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.spec.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.spec.ts index f689743156f..de1d93b52eb 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.spec.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.spec.ts @@ -133,24 +133,20 @@ describe('DsDynamicLookupRelationSearchTabComponent', () => { describe('selectPage', () => { beforeEach(() => { - spyOn(component.selectObject, 'emit'); component.selectPage([searchResult1, searchResult2, searchResult4]); }); - it('should emit the page filtered from already selected objects and call select on the service for all objects', () => { - expect(component.selectObject.emit).toHaveBeenCalledWith(searchResult4); + it('should call select on the service for all objects', () => { expect(selectableListService.select).toHaveBeenCalledWith(listID, [searchResult1, searchResult2, searchResult4]); }); }); describe('deselectPage', () => { beforeEach(() => { - spyOn(component.deselectObject, 'emit'); component.deselectPage([searchResult1, searchResult2, searchResult3]); }); - it('should emit the page filtered from not yet selected objects and call select on the service for all objects', () => { - expect((component.deselectObject as any).emit).toHaveBeenCalledWith(searchResult1, searchResult2); + it('should call deselect on the service for all objects', () => { expect(selectableListService.deselect).toHaveBeenCalledWith(listID, [searchResult1, searchResult2, searchResult3]); }); }); 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 9452918a978..52f0f9a9b52 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 @@ -175,12 +175,6 @@ export class DsDynamicLookupRelationSearchTabComponent implements OnInit, OnDest * @param page The page to select */ selectPage(page: SearchResult[]) { - this.selection$ - .pipe(take(1)) - .subscribe((selection: SearchResult[]) => { - const filteredPage = page.filter((pageItem) => selection.findIndex((selected) => selected.equals(pageItem)) < 0); - this.selectObject.emit(...filteredPage); - }); this.selectableListService.select(this.listId, page); } @@ -190,12 +184,6 @@ export class DsDynamicLookupRelationSearchTabComponent implements OnInit, OnDest */ deselectPage(page: SearchResult[]) { this.allSelected = false; - this.selection$ - .pipe(take(1)) - .subscribe((selection: SearchResult[]) => { - const filteredPage = page.filter((pageItem) => selection.findIndex((selected) => selected.equals(pageItem)) >= 0); - this.deselectObject.emit(...filteredPage); - }); this.selectableListService.deselect(this.listId, page); }