From 2903486974ad193bbee10a79fd3a237998dd7db5 Mon Sep 17 00:00:00 2001 From: Jarrod Flesch <30633324+JarrodMFlesch@users.noreply.github.com> Date: Fri, 1 Aug 2025 11:04:51 -0400 Subject: [PATCH] fix(ui): group/array error paths persisting when valid (#13347) Fields such as groups and arrays would not always reset errorPaths when there were no more errors. The server and client state was not being merged safely and the client state was always persisting when the server sent back no errorPaths, i.e. itterable fields with fully valid children. This change ensures errorPaths is defaulted to an empty array if it is not present on the incoming field. Likely a regression from https://github.com/payloadcms/payload/pull/9388. Adds e2e test. --- .../ui/src/forms/Form/mergeServerFormState.ts | 8 ++++ test/field-error-states/e2e.spec.ts | 44 +++++++++++++++++++ test/field-error-states/shared.ts | 1 + test/form-state/int.spec.ts | 2 + 4 files changed, 55 insertions(+) diff --git a/packages/ui/src/forms/Form/mergeServerFormState.ts b/packages/ui/src/forms/Form/mergeServerFormState.ts index ae6900dad..5d0bd2a2f 100644 --- a/packages/ui/src/forms/Form/mergeServerFormState.ts +++ b/packages/ui/src/forms/Form/mergeServerFormState.ts @@ -40,6 +40,14 @@ export const mergeServerFormState = ({ ...incomingField, } + if ( + currentState[path] && + 'errorPaths' in currentState[path] && + !('errorPaths' in incomingField) + ) { + newState[path].errorPaths = [] + } + /** * Intelligently merge the rows array to ensure changes to local state are not lost while the request was pending * For example, the server response could come back with a row which has been deleted on the client diff --git a/test/field-error-states/e2e.spec.ts b/test/field-error-states/e2e.spec.ts index 49bb09b55..810158bf8 100644 --- a/test/field-error-states/e2e.spec.ts +++ b/test/field-error-states/e2e.spec.ts @@ -23,6 +23,7 @@ describe('Field Error States', () => { let validateDraftsOnAutosave: AdminUrlUtil let prevValue: AdminUrlUtil let prevValueRelation: AdminUrlUtil + let errorFieldsURL: AdminUrlUtil beforeAll(async ({ browser }, testInfo) => { testInfo.setTimeout(TEST_TIMEOUT_LONG) @@ -32,6 +33,7 @@ describe('Field Error States', () => { validateDraftsOnAutosave = new AdminUrlUtil(serverURL, collectionSlugs.validateDraftsOnAutosave) prevValue = new AdminUrlUtil(serverURL, collectionSlugs.prevValue) prevValueRelation = new AdminUrlUtil(serverURL, collectionSlugs.prevValueRelation) + errorFieldsURL = new AdminUrlUtil(serverURL, collectionSlugs.errorFields) const context = await browser.newContext() page = await context.newPage() initPageConsoleErrorCatch(page) @@ -124,4 +126,46 @@ describe('Field Error States', () => { await expect(page.locator('#field-title')).toHaveValue('original value 2') }) }) + + describe('error field types', () => { + async function prefillBaseRequiredFields() { + const homeTabLocator = page.locator('.tabs-field__tab-button', { + hasText: 'Home', + }) + const heroTabLocator = page.locator('.tabs-field__tab-button', { + hasText: 'Hero', + }) + + await homeTabLocator.click() + // fill out all required fields in the home tab + await page.locator('#field-home__text').fill('Home Collapsible Text') + await page.locator('#field-home__tabText').fill('Home Tab Text') + + await page.locator('#field-group__text').fill('Home Group Text') + await heroTabLocator.click() + // fill out all required fields in the hero tab + await page.locator('#field-tabText').fill('Hero Tab Text') + await page.locator('#field-text').fill('Hero Tab Collapsible Text') + } + test('group errors', async () => { + await page.goto(errorFieldsURL.create) + await prefillBaseRequiredFields() + + // clear group and save + await page.locator('#field-group__text').fill('') + await saveDocAndAssert(page, '#action-save', 'error') + + // should show the error pill and count + const groupFieldErrorPill = page.locator('#field-group .group-field__header .error-pill', { + hasText: '1 error', + }) + await expect(groupFieldErrorPill).toBeVisible() + + // finish filling out the group + await page.locator('#field-group__text').fill('filled out') + + await expect(page.locator('#field-group .group-field__header .error-pill')).toBeHidden() + await saveDocAndAssert(page, '#action-save') + }) + }) }) diff --git a/test/field-error-states/shared.ts b/test/field-error-states/shared.ts index 9941f1bcd..7ba9fc7c9 100644 --- a/test/field-error-states/shared.ts +++ b/test/field-error-states/shared.ts @@ -8,6 +8,7 @@ export const collectionSlugs: { validateDraftsOnAutosave: 'validate-drafts-on-autosave', prevValue: 'prev-value', prevValueRelation: 'prev-value-relation', + errorFields: 'error-fields', } export const globalSlugs: { diff --git a/test/form-state/int.spec.ts b/test/form-state/int.spec.ts index eb614e002..3b1b4a896 100644 --- a/test/form-state/int.spec.ts +++ b/test/form-state/int.spec.ts @@ -309,6 +309,7 @@ describe('Form State', () => { it('should merge array rows without losing rows added to local state', () => { const currentState: FormState = { array: { + errorPaths: [], rows: [ { id: '1', @@ -358,6 +359,7 @@ describe('Form State', () => { // Row 2 should still exist expect(newState).toStrictEqual({ array: { + errorPaths: [], passesCondition: true, valid: true, rows: [