From a231a05b7caf20a5e2774ec3cccec3e125e265e4 Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Thu, 28 Aug 2025 16:12:47 -0400 Subject: [PATCH] fix(plugin-nested-docs): prevent phantom breadcrumb row (#13628) When saving a doc and regenerating the breadcrumbs array, a phantom row will append itself to the end of the array on save. This is because of fixes made in #13551 changed the way we merge array and block rows from the server. To fix this we need to ensure that row IDs are consistent across form state invocations, i.e. the hooks that mutate the array rows _cannot_ discard the row IDs. Before: https://github.com/user-attachments/assets/db715801-b4fd-4114-b39b-8d9b37fad979 After: https://github.com/user-attachments/assets/6da63a31-cd5d-43c1-a15e-caddbc540d56 --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1211175452200168 --- next-env.d.ts | 1 + .../src/utilities/formatBreadcrumb.ts | 8 ++++ .../src/utilities/populateBreadcrumbs.ts | 8 ++-- test/form-state/collections/Posts/index.ts | 40 ++++++++++++------- test/form-state/e2e.spec.ts | 14 +++---- test/form-state/payload-types.ts | 18 +++++++-- test/plugin-nested-docs/payload-types.ts | 14 +++++++ 7 files changed, 73 insertions(+), 30 deletions(-) diff --git a/next-env.d.ts b/next-env.d.ts index 1b3be0840f..830fb594ca 100644 --- a/next-env.d.ts +++ b/next-env.d.ts @@ -1,5 +1,6 @@ /// /// +/// // NOTE: This file should not be edited // see https://nextjs.org/docs/app/api-reference/config/typescript for more information. diff --git a/packages/plugin-nested-docs/src/utilities/formatBreadcrumb.ts b/packages/plugin-nested-docs/src/utilities/formatBreadcrumb.ts index 13702b38c4..de98d49409 100644 --- a/packages/plugin-nested-docs/src/utilities/formatBreadcrumb.ts +++ b/packages/plugin-nested-docs/src/utilities/formatBreadcrumb.ts @@ -3,12 +3,19 @@ import type { SanitizedCollectionConfig } from 'payload' import type { Breadcrumb, GenerateLabel, GenerateURL } from '../types.js' type Args = { + /** + * Existing breadcrumb, if any, to base the new breadcrumb on. + * This ensures that row IDs are maintained across updates, etc. + */ + breadcrumb?: Breadcrumb collection: SanitizedCollectionConfig docs: Record[] generateLabel?: GenerateLabel generateURL?: GenerateURL } + export const formatBreadcrumb = ({ + breadcrumb, collection, docs, generateLabel, @@ -32,6 +39,7 @@ export const formatBreadcrumb = ({ } return { + ...(breadcrumb || {}), doc: lastDoc.id as string, label, url, diff --git a/packages/plugin-nested-docs/src/utilities/populateBreadcrumbs.ts b/packages/plugin-nested-docs/src/utilities/populateBreadcrumbs.ts index 86b1431269..782957085d 100644 --- a/packages/plugin-nested-docs/src/utilities/populateBreadcrumbs.ts +++ b/packages/plugin-nested-docs/src/utilities/populateBreadcrumbs.ts @@ -26,10 +26,13 @@ export const populateBreadcrumbs = async ({ req, }: Args): Promise => { const newData = data + const currentDocument = { ...originalDoc, ...data, + id: originalDoc?.id ?? data?.id, } + const allParentDocuments: Document[] = await getAllParentDocuments( req, { @@ -41,14 +44,11 @@ export const populateBreadcrumbs = async ({ currentDocument, ) - if (originalDoc?.id) { - currentDocument.id = originalDoc?.id - } - allParentDocuments.push(currentDocument) const breadcrumbs = allParentDocuments.map((_, i) => formatBreadcrumb({ + breadcrumb: currentDocument[breadcrumbsFieldName]?.[i], collection, docs: allParentDocuments.slice(0, i + 1), generateLabel, diff --git a/test/form-state/collections/Posts/index.ts b/test/form-state/collections/Posts/index.ts index 07a5e8231d..a28b5f8838 100644 --- a/test/form-state/collections/Posts/index.ts +++ b/test/form-state/collections/Posts/index.ts @@ -76,24 +76,10 @@ export const PostsCollection: CollectionConfig = { name: 'array', type: 'array', admin: { - description: 'If there is no value, a default row will be added by a beforeChange hook', components: { RowLabel: './collections/Posts/ArrayRowLabel.js#ArrayRowLabel', }, }, - hooks: { - beforeChange: [ - ({ value }) => - !value?.length - ? [ - { - defaultTextField: 'This is a computed value.', - customTextField: 'This is a computed value.', - }, - ] - : value, - ], - }, fields: [ { name: 'customTextField', @@ -111,5 +97,31 @@ export const PostsCollection: CollectionConfig = { }, ], }, + { + name: 'computedArray', + type: 'array', + admin: { + description: + 'If there is no value, a default row will be added by a beforeChange hook. Otherwise, modifies the rows on save.', + }, + hooks: { + beforeChange: [ + ({ value }) => + !value?.length + ? [ + { + text: 'This is a computed value.', + }, + ] + : value, + ], + }, + fields: [ + { + name: 'text', + type: 'text', + }, + ], + }, ], } diff --git a/test/form-state/e2e.spec.ts b/test/form-state/e2e.spec.ts index cb0ec2ca0f..27fb396099 100644 --- a/test/form-state/e2e.spec.ts +++ b/test/form-state/e2e.spec.ts @@ -312,22 +312,18 @@ test.describe('Form State', () => { // Now test array rows, as their merge logic is different - await page.locator('#field-array #array-row-0').isVisible() + await page.locator('#field-computedArray #computedArray-row-0').isVisible() - await removeArrayRow(page, { fieldName: 'array' }) + await removeArrayRow(page, { fieldName: 'computedArray' }) - await page.locator('#field-array .array-row-0').isHidden() + await page.locator('#field-computedArray #computedArray-row-0').isHidden() await saveDocAndAssert(page) - await expect(page.locator('#field-array #array-row-0')).toBeVisible() + await expect(page.locator('#field-computedArray #computedArray-row-0')).toBeVisible() await expect( - page.locator('#field-array #array-row-0 #field-array__0__customTextField'), - ).toHaveValue('This is a computed value.') - - await expect( - page.locator('#field-array #array-row-0 #field-array__0__defaultTextField'), + page.locator('#field-computedArray #computedArray-row-0 #field-computedArray__0__text'), ).toHaveValue('This is a computed value.') }) diff --git a/test/form-state/payload-types.ts b/test/form-state/payload-types.ts index 165bedbee0..179343a2bb 100644 --- a/test/form-state/payload-types.ts +++ b/test/form-state/payload-types.ts @@ -144,9 +144,6 @@ export interface Post { } )[] | null; - /** - * If there is no value, a default row will be added by a beforeChange hook - */ array?: | { customTextField?: string | null; @@ -154,6 +151,15 @@ export interface Post { id?: string | null; }[] | null; + /** + * If there is no value, a default row will be added by a beforeChange hook. Otherwise, modifies the rows on save. + */ + computedArray?: + | { + text?: string | null; + id?: string | null; + }[] + | null; updatedAt: string; createdAt: string; } @@ -288,6 +294,12 @@ export interface PostsSelect { defaultTextField?: T; id?: T; }; + computedArray?: + | T + | { + text?: T; + id?: T; + }; updatedAt?: T; createdAt?: T; } diff --git a/test/plugin-nested-docs/payload-types.ts b/test/plugin-nested-docs/payload-types.ts index de28d82c89..3079834d06 100644 --- a/test/plugin-nested-docs/payload-types.ts +++ b/test/plugin-nested-docs/payload-types.ts @@ -178,6 +178,13 @@ export interface User { hash?: string | null; loginAttempts?: number | null; lockUntil?: string | null; + sessions?: + | { + id: string; + createdAt?: string | null; + expiresAt: string; + }[] + | null; password?: string | null; } /** @@ -295,6 +302,13 @@ export interface UsersSelect { hash?: T; loginAttempts?: T; lockUntil?: T; + sessions?: + | T + | { + id?: T; + createdAt?: T; + expiresAt?: T; + }; } /** * This interface was referenced by `Config`'s JSON-Schema