fix(ui): phantom fields when duplicating rows with rows (#14068)

jacobsfletch · web-flow · commit 08f6d99e4b80 · 2025-10-06T16:11:12.000Z
Fixes #14032. When duplicating arrays that contains nested arrays, the newly duplicated row will include twice as many fields within the nested array as the original. This is because duplicated row ids don't match 1:1 with their parent's `rows` property. For example: `array.0.nestedArray.0.id` should match `array.0.nestedArray.rows[0].id`. The problem is that when we duplicate the row, we regenerate all nested id fields, but we fail to sync them its corresponding row in its parent field. When we go to build form state on the server, we build new fields for this stale row. Then when we merge server form state back into local state, those new fields are simply _merged_ with the local state without replacing them. This is how we determine whether local changes to a row took place while a form state request was pending. It is important to prevent the merge strategy from overriding your active changes. --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1211543194775558
diff --git a/packages/ui/src/forms/Form/fieldReducer.ts b/packages/ui/src/forms/Form/fieldReducer.ts
@@ -131,48 +131,75 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState {
       return newState
     }
 
+    /**
+     * Duplicates a row in an array or blocks field.
+     * It needs to manipulate two distinct parts of the form state:
+     *   - The `rows` property of the parent field, e.g. `array.rows`, `blocks.rows`, etc.
+     *   - The row's state, e.g. `array.0.id`, `array.0.text`, etc.
+     */
     case 'DUPLICATE_ROW': {
       const { path, rowIndex } = action
       const { remainingFields, rows } = separateRows(path, state)
-      const rowsWithDuplicate = [...(state[path].rows || [])]
 
-      const newRow = deepCopyObjectSimpleWithoutReactComponents(rowsWithDuplicate[rowIndex])
+      // 1. Duplicate the `rows` property of the parent field, e.g. `array.rows`, `blocks.rows`, etc.
+      const newRows = [...(state[path].rows || [])]
+
+      const newRow = deepCopyObjectSimpleWithoutReactComponents(newRows[rowIndex])
 
       const newRowID = new ObjectId().toHexString()
 
       if (newRow.id) {
         newRow.id = newRowID
       }
 
-      if (rowsWithDuplicate[rowIndex]?.customComponents?.RowLabel) {
+      if (newRows[rowIndex]?.customComponents?.RowLabel) {
         newRow.customComponents = {
-          RowLabel: rowsWithDuplicate[rowIndex].customComponents.RowLabel,
+          RowLabel: newRows[rowIndex].customComponents.RowLabel,
         }
       }
 
-      const duplicateRowState = deepCopyObjectSimpleWithoutReactComponents(rows[rowIndex])
+      // 2. Duplicate the row's state, e.g. `array.0.id`, `array.0.text`, etc.
+      const newRowState = deepCopyObjectSimpleWithoutReactComponents(rows[rowIndex])
 
-      if (duplicateRowState.id) {
-        duplicateRowState.id.value = newRowID
-        duplicateRowState.id.initialValue = newRowID
+      // Ensure that `id` in form state exactly matches the row id on the parent field
+      if (newRowState.id) {
+        newRowState.id.value = newRowID
+        newRowState.id.initialValue = newRowID
       }
 
-      for (const key of Object.keys(duplicateRowState).filter((key) => key.endsWith('.id'))) {
-        const idState = duplicateRowState[key]
+      // Generate new ids for all nested id fields, e.g. `array.0.nestedArray.0.id`
+      for (const key of Object.keys(newRowState).filter((key) => key.endsWith('.id'))) {
+        const idState = newRowState[key]
 
         const newNestedFieldID = new ObjectId().toHexString()
 
         if (idState && typeof idState.value === 'string' && ObjectId.isValid(idState.value)) {
-          duplicateRowState[key].value = newNestedFieldID
-          duplicateRowState[key].initialValue = newNestedFieldID
+          newRowState[key].value = newNestedFieldID
+          newRowState[key].initialValue = newNestedFieldID
+
+          // Apply the ID to its corresponding parent field's rows, e.g. `array.0.nestedArray.rows[0].id`
+          const segments = key.split('.')
+          const rowIndex = parseInt(segments[segments.length - 2], 10)
+          const parentFieldPath = segments.slice(0, segments.length - 2).join('.')
+          const parentFieldRows = newRowState?.[parentFieldPath]?.rows
+
+          if (newRowState[parentFieldPath] && Array.isArray(parentFieldRows)) {
+            if (!parentFieldRows[rowIndex]) {
+              parentFieldRows[rowIndex] = {
+                id: newNestedFieldID,
+              }
+            } else {
+              parentFieldRows[rowIndex].id = newNestedFieldID
+            }
+          }
         }
       }
 
       // If there are subfields
-      if (Object.keys(duplicateRowState).length > 0) {
+      if (Object.keys(newRowState).length > 0) {
         // Add new object containing subfield names to unflattenedRows array
-        rows.splice(rowIndex + 1, 0, duplicateRowState)
-        rowsWithDuplicate.splice(rowIndex + 1, 0, newRow)
+        rows.splice(rowIndex + 1, 0, newRowState)
+        newRows.splice(rowIndex + 1, 0, newRow)
       }
 
       const newState = {
@@ -181,7 +208,7 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState {
         [path]: {
           ...state[path],
           disableFormData: true,
-          rows: rowsWithDuplicate,
+          rows: newRows,
           value: rows.length,
         },
       }
diff --git a/test/fields/collections/Array/e2e.spec.ts b/test/fields/collections/Array/e2e.spec.ts
@@ -5,6 +5,7 @@ import { expect, test } from '@playwright/test'
 import { assertToastErrors } from 'helpers/assertToastErrors.js'
 import { copyPasteField } from 'helpers/e2e/copyPasteField.js'
 import { addArrayRow, duplicateArrayRow, removeArrayRow } from 'helpers/e2e/fields/array/index.js'
+import { scrollEntirePage } from 'helpers/e2e/scrollEntirePage.js'
 import { toggleBlockOrArrayRow } from 'helpers/e2e/toggleCollapsible.js'
 import path from 'path'
 import { wait } from 'payload/shared'
@@ -191,76 +192,136 @@ describe('Array', () => {
   })
 
   describe('row manipulation', () => {
-    test('should add, remove and duplicate rows', async () => {
-      const assertText0 = 'array row 1'
-      const assertGroupText0 = 'text in group in row 1'
-      const assertText1 = 'array row 2'
-      const assertText3 = 'array row 3'
-      const assertGroupText3 = 'text in group in row 3'
+    test('should add rows', async () => {
+      const row1Text = 'Array row 1'
+      const row2Text = 'Array row 2'
+      const row3Text = 'Array row 3'
+
+      const row1GroupText = 'text in group in row 1'
+
       await loadCreatePage()
-      await page.mouse.wheel(0, 1750)
-      await page.locator('#field-potentiallyEmptyArray').scrollIntoViewIfNeeded()
-      await wait(300)
+      await scrollEntirePage(page)
 
-      // Add 3 rows
       await addArrayRow(page, { fieldName: 'potentiallyEmptyArray' })
       await addArrayRow(page, { fieldName: 'potentiallyEmptyArray' })
       await addArrayRow(page, { fieldName: 'potentiallyEmptyArray' })
 
-      // Fill out row 1
-      await page.locator('#field-potentiallyEmptyArray__0__text').fill(assertText0)
+      await page.locator('#field-potentiallyEmptyArray__0__text').fill(row1Text)
+      await page.locator('#field-potentiallyEmptyArray__1__text').fill(row2Text)
+      await page.locator('#field-potentiallyEmptyArray__2__text').fill(row3Text)
 
-      await page
-        .locator('#field-potentiallyEmptyArray__0__groupInRow__textInGroupInRow')
-        .fill(assertGroupText0)
+      await page.locator('#field-potentiallyEmptyArray__0__group__text').fill(row1GroupText)
+
+      await saveDocAndAssert(page)
+      await scrollEntirePage(page)
 
-      // Fill out row 2
-      await page.locator('#field-potentiallyEmptyArray__1__text').fill(assertText1)
+      await expect(page.locator('#field-potentiallyEmptyArray__0__text')).toHaveValue(row1Text)
+      await expect(page.locator('#field-potentiallyEmptyArray__1__text')).toHaveValue(row2Text)
+      await expect(page.locator('#field-potentiallyEmptyArray__2__text')).toHaveValue(row3Text)
 
-      // Fill out row 3
-      await page.locator('#field-potentiallyEmptyArray__2__text').fill(assertText3)
+      const input = page.locator('#field-potentiallyEmptyArray__0__group__text')
 
+      await expect(input).toHaveValue(row1GroupText)
+    })
+
+    test('should duplicate rows', async () => {
+      const row1Text = 'Array row 1'
+      const row2Text = 'Array row 2'
+      const row3Text = 'Array row 3'
+
+      await loadCreatePage()
+      await scrollEntirePage(page)
+
+      await addArrayRow(page, { fieldName: 'potentiallyEmptyArray' })
+      await addArrayRow(page, { fieldName: 'potentiallyEmptyArray' })
+      await addArrayRow(page, { fieldName: 'potentiallyEmptyArray' })
+
+      await page.locator('#field-potentiallyEmptyArray__0__text').fill(row1Text)
+      await page.locator('#field-potentiallyEmptyArray__1__text').fill(row2Text)
+      await page.locator('#field-potentiallyEmptyArray__2__text').fill(row3Text)
+
+      await page.locator('#field-potentiallyEmptyArray__0__text').fill(row1Text)
+
+      // Mark the first row with some unique values to assert against later
       await page
-        .locator('#field-potentiallyEmptyArray__2__groupInRow__textInGroupInRow')
-        .fill(assertGroupText3)
+        .locator('#field-potentiallyEmptyArray__0__group__text')
+        .fill(`${row1Text} duplicate`)
 
-      await removeArrayRow(page, { fieldName: 'potentiallyEmptyArray', rowIndex: 1 })
-      await removeArrayRow(page, { fieldName: 'potentiallyEmptyArray', rowIndex: 0 })
+      await duplicateArrayRow(page, { fieldName: 'potentiallyEmptyArray' })
 
-      // Save document
       await saveDocAndAssert(page)
+      await scrollEntirePage(page)
+
+      await page.locator('#field-potentiallyEmptyArray__0__text').fill(row1Text)
+      await page.locator('#field-potentiallyEmptyArray__1__text').fill(row1Text)
+      await page.locator('#field-potentiallyEmptyArray__2__text').fill(row2Text)
+      await page.locator('#field-potentiallyEmptyArray__3__text').fill(row3Text)
+
+      await expect(page.locator('#field-potentiallyEmptyArray__1__group__text')).toHaveValue(
+        `${row1Text} duplicate`,
+      )
+    })
+
+    test('should duplicate rows with nested arrays', async () => {
+      await loadCreatePage()
+      await scrollEntirePage(page)
+
+      await addArrayRow(page, { fieldName: 'potentiallyEmptyArray' })
+      await addArrayRow(page, { fieldName: 'potentiallyEmptyArray__0__array' })
+
+      await page.locator('#field-potentiallyEmptyArray__0__array__0__text').fill('Row 1')
 
-      // Scroll to array row (fields are not rendered in DOM until on screen)
-      await page.locator('#field-potentiallyEmptyArray__0__groupInRow').scrollIntoViewIfNeeded()
+      // There should be 2 fields in the nested array row: the text field and the row id
+      const fieldsInRow = page
+        .locator('#field-potentiallyEmptyArray__0__array')
+        .locator('.render-fields')
+        .first()
 
-      // Expect the remaining row to be the third row
-      const input = page.locator('#field-potentiallyEmptyArray__0__groupInRow__textInGroupInRow')
-      await expect(input).toHaveValue(assertGroupText3)
+      await expect(fieldsInRow.locator('> *')).toHaveCount(2)
 
       await duplicateArrayRow(page, { fieldName: 'potentiallyEmptyArray' })
 
-      // Update duplicated row group field text
-      await page
-        .locator('#field-potentiallyEmptyArray__1__groupInRow__textInGroupInRow')
-        .fill(`${assertGroupText3} duplicate`)
+      // There should still only be 2 fields in the duplicated row
+      const fieldsInDuplicatedRow = page
+        .locator('#field-potentiallyEmptyArray__1__array')
+        .locator('.render-fields')
+        .first()
 
-      // Save document
-      await saveDocAndAssert(page)
+      await expect(fieldsInDuplicatedRow.locator('> *')).toHaveCount(2)
+    })
 
-      // Expect the second row to be a duplicate of the remaining row
-      await expect(
-        page.locator('#field-potentiallyEmptyArray__1__groupInRow__textInGroupInRow'),
-      ).toHaveValue(`${assertGroupText3} duplicate`)
+    test('should remove rows', async () => {
+      const row1Text = 'Array row 1'
+      const row2Text = 'Array row 2'
+      const row3Text = 'Array row 3'
 
+      const assertGroupText3 = 'text in group in row 3'
+
+      await loadCreatePage()
+      await scrollEntirePage(page)
+
+      await addArrayRow(page, { fieldName: 'potentiallyEmptyArray' })
+      await addArrayRow(page, { fieldName: 'potentiallyEmptyArray' })
+      await addArrayRow(page, { fieldName: 'potentiallyEmptyArray' })
+
+      await page.locator('#field-potentiallyEmptyArray__0__text').fill(row1Text)
+      await page.locator('#field-potentiallyEmptyArray__1__text').fill(row2Text)
+      await page.locator('#field-potentiallyEmptyArray__2__text').fill(row3Text)
+
+      // Mark the third row with some unique values to assert against later
+      await page.locator('#field-potentiallyEmptyArray__2__group__text').fill(assertGroupText3)
+
+      // Remove all rows one by one, except the last one
+      await removeArrayRow(page, { fieldName: 'potentiallyEmptyArray', rowIndex: 1 })
       await removeArrayRow(page, { fieldName: 'potentiallyEmptyArray', rowIndex: 0 })
 
-      // Save document
       await saveDocAndAssert(page)
+      await scrollEntirePage(page)
 
-      // Expect the remaining row to be the copy of the duplicate row
-      await expect(
-        page.locator('#field-potentiallyEmptyArray__0__groupInRow__textInGroupInRow'),
-      ).toHaveValue(`${assertGroupText3} duplicate`)
+      // Expect the remaining row to be the third row, now first
+      await expect(page.locator('#field-potentiallyEmptyArray__0__group__text')).toHaveValue(
+        assertGroupText3,
+      )
     })
   })
 
diff --git a/test/fields/collections/Array/index.ts b/test/fields/collections/Array/index.ts
@@ -136,15 +136,25 @@ const ArrayFields: CollectionConfig = {
           type: 'text',
         },
         {
-          name: 'groupInRow',
+          name: 'group',
           fields: [
             {
-              name: 'textInGroupInRow',
+              name: 'text',
               type: 'text',
             },
           ],
           type: 'group',
         },
+        {
+          name: 'array',
+          fields: [
+            {
+              name: 'text',
+              type: 'text',
+            },
+          ],
+          type: 'array',
+        },
       ],
       type: 'array',
     },
diff --git a/test/helpers/e2e/fields/array/duplicateArrayRow.ts b/test/helpers/e2e/fields/array/duplicateArrayRow.ts
@@ -14,7 +14,8 @@ export const duplicateArrayRow = async (
   popupContentLocator: Locator
   rowActionsButtonLocator: Locator
 }> => {
-  const rowLocator = page.locator(`#field-${fieldName} .array-field__row`)
+  const rowLocator = page.locator(`#field-${fieldName} > .array-field__draggable-rows > *`)
+
   const numberOfPrevRows = await rowLocator.count()
 
   const { popupContentLocator, rowActionsButtonLocator } = await openArrayRowActions(page, {
