fix: repopulate corrupts repeat groups when rows are deleted on the server (issue #1940)

The Value Restoration step in getFilteredItemsToRepopulate spliced accepted
deletions out of serverQRItems inside the same forward loop that fills
unselected slots from current values. Splicing shifted unvisited deletion
markers into already-visited indices (so they leaked into the
QuestionnaireResponse) and re-opened their vacated tail slots to be re-filled
from current values, resurrecting the deleted rows as duplicates.

Split the loop into two passes: fill unselected slots first, then filter out
all items marked with the mark-as-deleted extension. This also strips the
internal marker extension from the final response and drops sparse-array holes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: clinnygee

apps/smart-forms-app/src/features/repopulate/test/itemsToRepopulateSelector.test.ts

@@ -238,6 +238,86 @@ describe('getFilteredItemsToRepopulate', () => {
     // currentQRItem should stay undefined / as per original
     expect(filtered['63fe14f3-2374-4382-bce7-180e2747c97f'].currentQRItem).toBeUndefined();
   });
+
+  // Regression tests for https://github.com/aehrc/smart-forms/issues/1940
+  // Rows deleted on the server between repopulations: the dialog correctly shows the
+  // deletions, but applying them corrupted the result (deleted rows resurrected/duplicated
+  // and the internal mark-as-deleted extension leaked into the QuestionnaireResponse).
+  describe('server-side deletions in repeat groups', () => {
+    const markAsDeletedUrl =
+      'https://smartforms.csiro.au/custom-functionality/repopulation/mark-as-deleted';
+
+    function buildRepeatGroupRow(value: string) {
+      return {
+        linkId: 'problem',
+        item: [{ linkId: 'problem-name', answer: [{ valueString: value }] }]
+      };
+    }
+
+    const rowA = buildRepeatGroupRow('A');
+    const rowB = buildRepeatGroupRow('B');
+    const rowC = buildRepeatGroupRow('C');
+
+    function buildDeletionScenario(): {
+      tuplesMap: Map<string, [string, ItemToRepopulate][]>;
+      originalItems: Record<string, ItemToRepopulate>;
+    } {
+      // Form has rows [A, B, C]; rows A and B were deleted on the server, so it returns [C].
+      // Detection (retrieveRepeatGroupCurrentQRItems) aligns matched pairs first:
+      // currentQRItems = [C, A, B], serverQRItems = [C]
+      // → dialog child 0 hidden (C unchanged), child 1 "A removed", child 2 "B removed"
+      const itemToRepopulate: ItemToRepopulate = {
+        qItem: { linkId: 'problem', text: 'Recorded problems', type: 'group', repeats: true },
+        sectionItemText: 'Clinical details',
+        parentItemText: 'Clinical details',
+        isInGrid: false,
+        currentQRItems: [rowC, rowA, rowB],
+        serverQRItems: [rowC]
+      };
+
+      return {
+        tuplesMap: new Map([['Clinical details', [['problem', itemToRepopulate]]]]),
+        originalItems: { problem: itemToRepopulate }
+      };
+    }
+
+    it('removes all accepted deletions instead of resurrecting/duplicating rows', () => {
+      const { tuplesMap, originalItems } = buildDeletionScenario();
+
+      // Accept both deletions, as shown in the dialog
+      const selectedKeys = new Set(['heading-0-parent-0-child-1', 'heading-0-parent-0-child-2']);
+
+      const filtered = getFilteredItemsToRepopulate(tuplesMap, selectedKeys, originalItems);
+
+      // The dialog promised only row C remains
+      expect(filtered['problem'].serverQRItems).toEqual([rowC]);
+    });
+
+    it('does not leak the mark-as-deleted extension into the result', () => {
+      const { tuplesMap, originalItems } = buildDeletionScenario();
+
+      const selectedKeys = new Set(['heading-0-parent-0-child-1', 'heading-0-parent-0-child-2']);
+
+      const filtered = getFilteredItemsToRepopulate(tuplesMap, selectedKeys, originalItems);
+
+      const leakedMarkers = (filtered['problem'].serverQRItems ?? []).filter((serverQRItem) =>
+        serverQRItem.extension?.some((ext) => ext.url === markAsDeletedUrl)
+      );
+      expect(leakedMarkers).toEqual([]);
+    });
+
+    it('keeps unaccepted deletions in the form', () => {
+      const { tuplesMap, originalItems } = buildDeletionScenario();
+
+      // Accept only the deletion of row A; leave row B's deletion unselected
+      const selectedKeys = new Set(['heading-0-parent-0-child-1']);
+
+      const filtered = getFilteredItemsToRepopulate(tuplesMap, selectedKeys, originalItems);
+
+      // Row A removed, row B preserved from current values
+      expect(filtered['problem'].serverQRItems).toEqual([rowC, rowB]);
+    });
+  });
 });
 
 describe('getChipColorByValueChangeMode', () => {

apps/smart-forms-app/src/features/repopulate/utils/itemsToRepopulateSelector.ts

@@ -251,22 +251,22 @@ export function getFilteredItemsToRepopulate(
           filteredItem.serverQRItems = [];
         }
 
+        // Fill unselected slots with current values first, then drop accepted deletions in a
+        // separate pass. Doing both in a single loop corrupts the result: splicing shifts later
+        // items into already-visited indices (so a deletion marker can survive) and re-opens
+        // their vacated tail slots to be re-filled from current values (resurrecting deleted items).
         for (let i = 0; i < maxNumberOfItems; i++) {
           if (filteredItem.serverQRItems[i] === undefined && originalItem.currentQRItems[i]) {
             filteredItem.serverQRItems[i] = originalItem.currentQRItems[i];
           }
-
-          // If the original item is marked as deleted (has the https://smartforms.csiro.au/custom-functionality/repopulation/mark-as-deleted extension), remove the deleted item
-          if (filteredItem.serverQRItems[i]) {
-            const hasDeletedExtension = itemHasMarkAsDeletedExtension(
-              filteredItem.serverQRItems[i]
-            );
-
-            if (hasDeletedExtension) {
-              filteredItem.serverQRItems.splice(i, 1);
-            }
-          }
         }
+
+        // Remove items accepted as deletions (marked with the https://smartforms.csiro.au/custom-functionality/repopulation/mark-as-deleted extension),
+        // so the marker never leaks into the final QuestionnaireResponse. Also drops any holes left in the sparse array.
+        filteredItem.serverQRItems = filteredItem.serverQRItems.filter(
+          (serverQRItem) =>
+            serverQRItem !== undefined && !itemHasMarkAsDeletedExtension(serverQRItem)
+        );
       }
     }
   }