diff --git a/src/app/core/json-patch/json-patch-operations.reducer.spec.ts b/src/app/core/json-patch/json-patch-operations.reducer.spec.ts index 7af2f1377fb..25e08b4f13f 100644 --- a/src/app/core/json-patch/json-patch-operations.reducer.spec.ts +++ b/src/app/core/json-patch/json-patch-operations.reducer.spec.ts @@ -340,7 +340,7 @@ describe('jsonPatchOperationsReducer test suite', () => { }); describe('dedupeOperationEntries', () => { - it('should not remove duplicated keys if operations are not sequential', () => { + it('should keep only the latest duplicated key even when operations are not sequential', () => { initState = { sections: { children: { @@ -407,24 +407,6 @@ describe('jsonPatchOperationsReducer test suite', () => { const newState = jsonPatchOperationsReducer(initState, action); const expectedBody: any = [ - { - 'operation': { - 'op': 'add', - 'path': '/sections/publicationStep/dc.date.issued', - 'value': [ - { - 'value': '2024-06', - 'language': null, - 'authority': null, - 'display': '2024-06', - 'confidence': -1, - 'place': 0, - 'otherInformation': null, - }, - ], - }, - 'timeCompleted': timestampBeforeStart, - }, { 'operation': { 'op': 'replace', @@ -465,7 +447,7 @@ describe('jsonPatchOperationsReducer test suite', () => { }); - it('should remove duplicated keys if operations are sequential', () => { + it('should keep only the latest duplicated key when operations are sequential', () => { initState = { sections: { children: { @@ -550,24 +532,6 @@ describe('jsonPatchOperationsReducer test suite', () => { const newState = jsonPatchOperationsReducer(initState, action); const expectedBody: any = [ - { - 'operation': { - 'op': 'add', - 'path': '/sections/publicationStep/dc.date.issued', - 'value': [ - { - 'value': '2024-06', - 'language': null, - 'authority': null, - 'display': '2024-06', - 'confidence': -1, - 'place': 0, - 'otherInformation': null, - }, - ], - }, - 'timeCompleted': timestampBeforeStart, - }, { 'operation': { 'op': 'replace', diff --git a/src/app/core/json-patch/json-patch-operations.reducer.ts b/src/app/core/json-patch/json-patch-operations.reducer.ts index 9687bdf21dd..f0e70a344e3 100644 --- a/src/app/core/json-patch/json-patch-operations.reducer.ts +++ b/src/app/core/json-patch/json-patch-operations.reducer.ts @@ -356,27 +356,28 @@ function addOperationToList(body: JsonPatchOperationObject[], actionType, target /** * Dedupe operation entries by op and path. This prevents processing unnecessary patches in a single PATCH request. + * For any given op+path combination, only the latest operation is retained (earlier duplicates are discarded). + * + * Note: this function is only called for submission-form patch operations, which always use numeric-indexed or + * bare metadata paths (e.g. /sections/step/dc.title or /sections/step/dc.title/0). The JSON Patch append + * notation (path ending in "/-") is intentionally never produced by this pipeline, so collapsing duplicates + * by op+path is safe here. * * @param body JSON patch operation object entries * @returns deduped JSON patch operation object entries */ function dedupeOperationEntries(body: JsonPatchOperationObject[]): JsonPatchOperationObject[] { - const ops = new Map(); - for (let i = body.length - 1; i >= 0; i--) { - const patch = body[i].operation; - const key = `${patch.op}-${patch.path}`; - if (!ops.has(key)) { - ops.set(key, i); - } else { - const entry = ops.get(key); - if (entry - 1 === i) { - body.splice(i, 1); - ops.set(key, i); - } - } - } + const lastIndexByOpPath = new Map(); + + body.forEach((entry, index) => { + const patch = entry.operation; + lastIndexByOpPath.set(`${patch.op}-${patch.path}`, index); + }); - return body; + return body.filter((entry, index) => { + const patch = entry.operation; + return lastIndexByOpPath.get(`${patch.op}-${patch.path}`) === index; + }); } function makeOperationEntry(operation) { diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/array-group/dynamic-form-array.component.spec.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/array-group/dynamic-form-array.component.spec.ts index e5e86436c20..d78293ca568 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/array-group/dynamic-form-array.component.spec.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/array-group/dynamic-form-array.component.spec.ts @@ -5,7 +5,7 @@ import { inject, TestBed, } from '@angular/core/testing'; -import { ReactiveFormsModule } from '@angular/forms'; +import { ReactiveFormsModule, UntypedFormArray, UntypedFormGroup } from '@angular/forms'; import { By } from '@angular/platform-browser'; import { DynamicFormLayoutService, @@ -34,6 +34,7 @@ import { UUIDService } from '../../../../../../core/shared/uuid.service'; import { TranslateLoaderMock } from '../../../../../mocks/translate-loader.mock'; import { LiveRegionService } from '../../../../../live-region/live-region.service'; import { getLiveRegionServiceStub } from '../../../../../live-region/live-region.service.stub'; +import { FormBuilderService } from '../../../form-builder.service'; describe('DsDynamicFormArrayComponent', () => { const translateServiceStub = { @@ -48,6 +49,15 @@ describe('DsDynamicFormArrayComponent', () => { generate: () => 'fake-id' }; + const formBuilderServiceStub = { + moveFormArrayGroup: (index: number, step: number, formArray: UntypedFormArray, model: DynamicRowArrayModel) => { + const movedControl = formArray.at(index); + formArray.removeAt(index); + formArray.insert(index + step, movedControl); + model.moveGroup(index, step); + } + }; + let component: DsDynamicFormArrayComponent; let fixture: ComponentFixture; @@ -76,6 +86,7 @@ describe('DsDynamicFormArrayComponent', () => { { provide: APP_CONFIG, useValue: environment }, { provide: UUIDService, useValue: uuidServiceStub }, { provide: LiveRegionService, useValue: getLiveRegionServiceStub() }, + { provide: FormBuilderService, useValue: formBuilderServiceStub }, ], }).overrideComponent(DsDynamicFormArrayComponent, { remove: { @@ -160,13 +171,30 @@ describe('DsDynamicFormArrayComponent', () => { it('should cancel keyboard drag and drop', () => { const dropList = fixture.debugElement.query(By.css('[cdkDropList]')).nativeElement; + const formArray = component.group.get(component.model.id) as UntypedFormArray; component.elementBeingSortedStartingIndex = 2; component.elementBeingSorted = dropList.querySelectorAll('[cdkDragHandle]')[2]; - component.model.moveGroup(2, 1); + (component as any).formBuilderService.moveFormArrayGroup(2, 1, formArray, component.model); fixture.detectChanges(); component.cancelKeyboardDragAndDrop(dropList, 1, 3); fixture.detectChanges(); expect(component.elementBeingSorted).toBeNull(); expect(component.elementBeingSortedStartingIndex).toBeNull(); }); + + it('should move model groups and FormArray controls together on drag and drop reorder', () => { + const formArray = component.group.get(component.model.id) as UntypedFormArray; + const valueControl = 'testFormRowArrayGroupInput'; + + ((formArray.at(0) as UntypedFormGroup).controls[valueControl]).setValue('one'); + ((formArray.at(4) as UntypedFormGroup).controls[valueControl]).setValue('five'); + (component.model.groups[0].group[0] as any).value = 'one'; + (component.model.groups[4].group[0] as any).value = 'five'; + + component.moveSelection({ previousIndex: 4, currentIndex: 0 } as any); + + expect((component.model.groups[0].group[0] as any).value).toBe('five'); + expect(((formArray.at(0) as UntypedFormGroup).controls[valueControl]).value).toBe('five'); + expect(component.getControlOfGroup(component.model.groups[0] as any)).toBe(formArray.at(0)); + }); }); diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/array-group/dynamic-form-array.component.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/array-group/dynamic-form-array.component.ts index fb7ee7ac2e8..a9b3427eb96 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/array-group/dynamic-form-array.component.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/array-group/dynamic-form-array.component.ts @@ -1,6 +1,6 @@ import { CdkDragDrop } from '@angular/cdk/drag-drop'; import { Component, EventEmitter, Input, Output, QueryList } from '@angular/core'; -import { UntypedFormGroup } from '@angular/forms'; +import { UntypedFormArray, UntypedFormGroup } from '@angular/forms'; import { DynamicFormArrayComponent, DynamicFormControlCustomEvent, @@ -17,6 +17,7 @@ import { hasValue } from '../../../../../empty.util'; import { DynamicRowArrayModel } from '../ds-dynamic-row-array-model'; import { LiveRegionService } from '../../../../../live-region/live-region.service'; import { TranslateService } from '@ngx-translate/core'; +import { FormBuilderService } from '../../../form-builder.service'; @Component({ selector: 'ds-dynamic-form-array', @@ -48,6 +49,7 @@ export class DsDynamicFormArrayComponent extends DynamicFormArrayComponent { protected validationService: DynamicFormValidationService, protected liveRegionService: LiveRegionService, protected translateService: TranslateService, + protected formBuilderService: FormBuilderService, ) { super(layoutService, validationService); } @@ -59,7 +61,12 @@ export class DsDynamicFormArrayComponent extends DynamicFormArrayComponent { return; } - this.model.moveGroup(event.previousIndex, event.currentIndex - event.previousIndex); + this.formBuilderService.moveFormArrayGroup( + event.previousIndex, + event.currentIndex - event.previousIndex, + this.control as UntypedFormArray, + this.model, + ); const prevIndex = event.previousIndex; const index = event.currentIndex; @@ -90,16 +97,13 @@ export class DsDynamicFormArrayComponent extends DynamicFormArrayComponent { } /** - * Gets the control of the specified group model. It adds the startingIndex property to the group model if it does not - * already have it. This ensures that the controls are always linked to the correct group model. + * Gets the control of the specified group model. + * The control index now stays aligned with group index because reorder operations move both model and controls. * @param groupModel The group model to get the control for. * @returns The form control of the specified group model. */ getControlOfGroup(groupModel: any) { - if (!groupModel.hasOwnProperty('startingIndex')) { - groupModel.startingIndex = groupModel.index; - } - return this.control.get([groupModel.startingIndex]); + return this.control.get([groupModel.index]); } /** @@ -163,7 +167,12 @@ export class DsDynamicFormArrayComponent extends DynamicFormArrayComponent { } if (this.elementBeingSorted) { - this.model.moveGroup(idx, newIndex - idx); + this.formBuilderService.moveFormArrayGroup( + idx, + newIndex - idx, + this.control as UntypedFormArray, + this.model, + ); if (hasValue(this.model.groups[newIndex]) && hasValue((this.control as any).controls[newIndex])) { this.onCustomEvent({ previousIndex: idx, @@ -191,7 +200,12 @@ export class DsDynamicFormArrayComponent extends DynamicFormArrayComponent { } cancelKeyboardDragAndDrop(sortableElement: HTMLDivElement, index: number, length: number) { - this.model.moveGroup(index, this.elementBeingSortedStartingIndex - index); + this.formBuilderService.moveFormArrayGroup( + index, + this.elementBeingSortedStartingIndex - index, + this.control as UntypedFormArray, + this.model, + ); if (hasValue(this.model.groups[this.elementBeingSortedStartingIndex]) && hasValue((this.control as any).controls[this.elementBeingSortedStartingIndex])) { this.onCustomEvent({ previousIndex: index, diff --git a/src/app/shared/form/builder/form-builder.service.spec.ts b/src/app/shared/form/builder/form-builder.service.spec.ts index 9b67c467498..74bd07c1ad1 100644 --- a/src/app/shared/form/builder/form-builder.service.spec.ts +++ b/src/app/shared/form/builder/form-builder.service.spec.ts @@ -535,6 +535,36 @@ describe('FormBuilderService test suite', () => { expect(service.getValueFromModel(formModel)).toEqual(value); }); + it('should preserve place 0 for object-valued fields in arrays', () => { + const arrayModel = new DynamicRowArrayModel({ + id: 'testObjectArray', + initialCount: 1, + notRepeatable: false, + relationshipConfig: undefined, + submissionId, + isDraggable: true, + groupFactory: () => [ + new DynamicInputModel({ id: 'dc_title' }), + ], + required: false, + metadataKey: 'dc.title', + metadataFields: ['dc.title'], + hasSelectableMetadata: true, + showButtons: true, + typeBindRelations: [], + }); + + (arrayModel.groups[0].group[0] as any).name = 'dc.title'; + (arrayModel.groups[0].group[0] as any).value = { + value: 'Title with stale place', + place: 4, + }; + + const value = service.getValueFromModel([arrayModel]); + + expect(value['dc.title'][0].place).toBe(0); + }); + it('should clear all form\'s fields value', () => { const formModel = service.modelFromConfiguration(submissionId, testFormConfiguration, 'testScopeUUID'); const value = {} as any; diff --git a/src/app/shared/form/builder/form-builder.service.ts b/src/app/shared/form/builder/form-builder.service.ts index 8755d774f57..4329607334b 100644 --- a/src/app/shared/form/builder/form-builder.service.ts +++ b/src/app/shared/form/builder/form-builder.service.ts @@ -244,7 +244,8 @@ export class FormBuilderService extends DynamicFormService { return new FormFieldMetadataValueObject(dateToString(controlValue)); } else if (isObject(controlValue)) { const authority = (controlValue as any).authority || (controlValue as any).id || null; - const place = controlModelIndex || (controlValue as any).place; + const hasArrayIndex = controlModelIndex !== null; + const place = hasArrayIndex ? controlModelIndex : (controlValue as any).place; if (isNgbDateStruct(controlValue)) { return new FormFieldMetadataValueObject(controlValue, controlLanguage, authority, controlValue as any, place); } else {