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

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
2025-10-06 12:11:12 -04:00
parent db6ec3026c
commit 08f6d99e4b
4 changed files with 167 additions and 68 deletions
--- a/packages/ui/src/forms/Form/fieldReducer.ts
+++ b/packages/ui/src/forms/Form/fieldReducer.ts
@@ -131,12 +131,20 @@ 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()

@@ -144,35 +152,54 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState {
        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,
        },
      }
--- 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)

-      // Fill out row 2
-      await page.locator('#field-potentiallyEmptyArray__1__text').fill(assertText1)
-
-      // Fill out row 3
-      await page.locator('#field-potentiallyEmptyArray__2__text').fill(assertText3)
-
-      await page
-        .locator('#field-potentiallyEmptyArray__2__groupInRow__textInGroupInRow')
-        .fill(assertGroupText3)
-
-      await removeArrayRow(page, { fieldName: 'potentiallyEmptyArray', rowIndex: 1 })
-      await removeArrayRow(page, { fieldName: 'potentiallyEmptyArray', rowIndex: 0 })
-
-      // Save document
      await saveDocAndAssert(page)
+      await scrollEntirePage(page)

-      // Scroll to array row (fields are not rendered in DOM until on screen)
-      await page.locator('#field-potentiallyEmptyArray__0__groupInRow').scrollIntoViewIfNeeded()
+      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)

-      // Expect the remaining row to be the third row
-      const input = page.locator('#field-potentiallyEmptyArray__0__groupInRow__textInGroupInRow')
-      await expect(input).toHaveValue(assertGroupText3)
+      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__0__group__text')
+        .fill(`${row1Text} duplicate`)

      await duplicateArrayRow(page, { fieldName: 'potentiallyEmptyArray' })

-      // Update duplicated row group field text
-      await page
-        .locator('#field-potentiallyEmptyArray__1__groupInRow__textInGroupInRow')
-        .fill(`${assertGroupText3} duplicate`)
-
-      // Save document
      await saveDocAndAssert(page)
+      await scrollEntirePage(page)

-      // Expect the second row to be a duplicate of the remaining row
-      await expect(
-        page.locator('#field-potentiallyEmptyArray__1__groupInRow__textInGroupInRow'),
-      ).toHaveValue(`${assertGroupText3} duplicate`)
+      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')
+
+      // 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()
+
+      await expect(fieldsInRow.locator('> *')).toHaveCount(2)
+
+      await duplicateArrayRow(page, { fieldName: 'potentiallyEmptyArray' })
+
+      // There should still only be 2 fields in the duplicated row
+      const fieldsInDuplicatedRow = page
+        .locator('#field-potentiallyEmptyArray__1__array')
+        .locator('.render-fields')
+        .first()
+
+      await expect(fieldsInDuplicatedRow.locator('> *')).toHaveCount(2)
+    })
+
+    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,
+      )
    })
  })

--- 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',
    },
--- 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, {