Port #1267 to dtq-dev-9-base: UFAL/stabilize form array reorder/delete behavior (ufal#120) (#1267) (#1502)

Two runtime bugs in repeatable submission fields.

dedupeOperationEntries() collapsed duplicate patch operations only when they sat
next to each other in the body: it walked backwards, remembered the last index per
op+path, and spliced an entry out only when the remembered index was exactly one
ahead of it. After a reorder or a delete the operations for one field are no longer
adjacent, so stale operations survived into the PATCH and the value the user had
just removed or moved came back on save. It is rewritten to keep the latest
operation per op+path and drop every earlier one, which is what the two renamed
specs now say. The two specs previously asserted the stale first operation was
still in the body; both expected bodies lose that entry.

The dedupe key is op+path, which ignores a move's `from`. That is safe here for the
same reason it is safe on dtq-dev: this function only ever sees submission-form
operations, whose paths are bare or numeric-indexed metadata paths, never the JSON
Patch append form. The rewritten doc comment says so.

Second, form-builder.service.ts computed `place = controlModelIndex || value.place`
for object-valued controls. Index 0 is falsy, so the first entry of an array fell
through to whatever stale `place` the value carried. It now tests for the index
explicitly (hasArrayIndex), so position 0 keeps place 0. A new spec pins it.

v9 notes: the source commit also rewrites dynamic-form-array.component.ts and its
spec (66 of its 104 lines) to keep the FormArray in sync with the model on reorder.
Those two files are deliberately NOT ported. Vanilla 9.3 solves the same desync its
own way, with moveFormControlToPosition() (dynamic-form-array.component.ts:274),
and the file is byte-identical with dspace-9.3 on this branch; transplanting the
7.x mechanism (moveFormArrayGroup/getControlOfGroup, neither of which exists on v9)
would be a regression, and its spec tests exactly those absent methods.
`git diff origin/dtq-dev-9-base HEAD --stat -- .../models/array-group/` is empty.

The four ported files are hunk-identical with the source: for each of them
`git show 164c424 -- <f> | grep -E '^[-+][^-+]'` and the same grep over this
branch's diff produce byte-identical output.

Card PB-02 (tranche T3).

Source: 164c424 (dtq-dev PR #1267)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: milanmajchrak

src/app/core/json-patch/json-patch-operations.reducer.spec.ts

@@ -341,7 +341,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: {
@@ -408,24 +408,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',
@@ -466,7 +448,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: {
@@ -551,24 +533,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',

src/app/core/json-patch/json-patch-operations.reducer.ts

@@ -401,27 +401,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<string, number>();
-  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<string, number>();
+
+  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) {

src/app/shared/form/builder/form-builder.service.spec.ts

@@ -543,6 +543,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;

src/app/shared/form/builder/form-builder.service.ts

@@ -287,7 +287,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 {