diff --git a/apps/smart-forms-app/src/features/repopulate/test/itemsToRepopulateSelector.test.ts b/apps/smart-forms-app/src/features/repopulate/test/itemsToRepopulateSelector.test.ts index df308bf09..4497482fb 100644 --- a/apps/smart-forms-app/src/features/repopulate/test/itemsToRepopulateSelector.test.ts +++ b/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; + originalItems: Record; + } { + // 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', () => { diff --git a/apps/smart-forms-app/src/features/repopulate/utils/itemsToRepopulateSelector.ts b/apps/smart-forms-app/src/features/repopulate/utils/itemsToRepopulateSelector.ts index e884dbc86..cfa13867f 100644 --- a/apps/smart-forms-app/src/features/repopulate/utils/itemsToRepopulateSelector.ts +++ b/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) + ); } } }