From d144af6d8e8a1eaf674c7bd082a16828acbfa9e5 Mon Sep 17 00:00:00 2001 From: Alessio Gravili Date: Wed, 27 Mar 2024 11:41:21 -0400 Subject: [PATCH 01/11] fix: baseIDField not included in form states --- packages/payload/src/fields/baseFields/baseIDField.ts | 6 ++---- .../providers/ComponentMap/buildComponentMap/mapFields.tsx | 2 -- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/payload/src/fields/baseFields/baseIDField.ts b/packages/payload/src/fields/baseFields/baseIDField.ts index d66c418ee..eadb170a8 100644 --- a/packages/payload/src/fields/baseFields/baseIDField.ts +++ b/packages/payload/src/fields/baseFields/baseIDField.ts @@ -12,10 +12,8 @@ export const baseIDField: Field = { name: 'id', type: 'text', admin: { - disabled: true, - }, - hooks: { - beforeChange: [generateID], + hidden: true, }, + defaultValue: generateID, label: 'ID', } diff --git a/packages/ui/src/providers/ComponentMap/buildComponentMap/mapFields.tsx b/packages/ui/src/providers/ComponentMap/buildComponentMap/mapFields.tsx index 9c588af5d..ff018bc93 100644 --- a/packages/ui/src/providers/ComponentMap/buildComponentMap/mapFields.tsx +++ b/packages/ui/src/providers/ComponentMap/buildComponentMap/mapFields.tsx @@ -227,7 +227,6 @@ export const mapFields = (args: { disabled: field.admin?.disabled, fieldMap: mapFields({ config, - disableAddingID: true, fieldSchema: field.fields, filter, parentPath: path, @@ -250,7 +249,6 @@ export const mapFields = (args: { const blocks = field.blocks.map((block) => { const blockFieldMap = mapFields({ config, - disableAddingID: true, fieldSchema: block.fields, filter, parentPath: `${path}.${block.slug}`, From 80d290e17812ec405f74dc42d7a6854c8fa73b4b Mon Sep 17 00:00:00 2001 From: Alessio Gravili Date: Wed, 27 Mar 2024 11:42:08 -0400 Subject: [PATCH 02/11] fix: server-generated ID for base ID field is overriden on the client --- packages/ui/src/forms/Form/fieldReducer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ui/src/forms/Form/fieldReducer.ts b/packages/ui/src/forms/Form/fieldReducer.ts index 3f362abe3..a9dd46260 100644 --- a/packages/ui/src/forms/Form/fieldReducer.ts +++ b/packages/ui/src/forms/Form/fieldReducer.ts @@ -114,7 +114,7 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState { const withNewRow = [...(state[path]?.rows || [])] const newRow: Row = { - id: new ObjectId().toHexString(), + id: (subFieldState?.id.value as string) || new ObjectId().toHexString(), blockType: blockType || undefined, collapsed: false, } From 6664535ab932ad6fd6176a21266946ceb871a4cb Mon Sep 17 00:00:00 2001 From: Alessio Gravili Date: Wed, 27 Mar 2024 11:43:35 -0400 Subject: [PATCH 03/11] fix: form onChange using out-of-date old fields state --- packages/ui/src/forms/Form/index.tsx | 9 ++++++--- packages/ui/src/forms/Form/types.ts | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/ui/src/forms/Form/index.tsx b/packages/ui/src/forms/Form/index.tsx index 3b2b70507..c14574862 100644 --- a/packages/ui/src/forms/Form/index.tsx +++ b/packages/ui/src/forms/Form/index.tsx @@ -530,7 +530,7 @@ export const Form: React.FC = (props) => { () => { const executeOnChange = async () => { if (Array.isArray(onChange)) { - let revalidatedFormState: FormState = fields + let revalidatedFormState: FormState = contextRef.current.fields for (const onChangeFn of onChange) { revalidatedFormState = await onChangeFn({ @@ -538,7 +538,10 @@ export const Form: React.FC = (props) => { }) } - const { changed, newState } = mergeServerFormState(fields || {}, revalidatedFormState) + const { changed, newState } = mergeServerFormState( + contextRef.current.fields || {}, + revalidatedFormState, + ) if (changed) { dispatchFields({ type: 'REPLACE_STATE', optimize: false, state: newState }) @@ -549,7 +552,7 @@ export const Form: React.FC = (props) => { if (modified) void executeOnChange() }, 150, - [fields, dispatchFields, onChange], + [contextRef.current.fields, dispatchFields, onChange], ) const actionString = diff --git a/packages/ui/src/forms/Form/types.ts b/packages/ui/src/forms/Form/types.ts index 7009b87f7..590e30cb0 100644 --- a/packages/ui/src/forms/Form/types.ts +++ b/packages/ui/src/forms/Form/types.ts @@ -174,7 +174,7 @@ export type Context = { disabled: boolean dispatchFields: Dispatch /** - * @deprecated Form context fields may be outdated and should not be relied on. Instead, prefer `useFormFields`. + * Form context fields may be outdated and should not be relied on. Instead, prefer `useFormFields`. */ fields: FormState formRef: React.MutableRefObject From 79da297addcff9af5d81379d33aa6ebf5e43a590 Mon Sep 17 00:00:00 2001 From: Alessio Gravili Date: Wed, 27 Mar 2024 11:44:02 -0400 Subject: [PATCH 04/11] fix: form state not replaced if server has different data for rows --- packages/ui/src/forms/Form/mergeServerFormState.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/ui/src/forms/Form/mergeServerFormState.ts b/packages/ui/src/forms/Form/mergeServerFormState.ts index 9e4083e36..b5f55ad2c 100644 --- a/packages/ui/src/forms/Form/mergeServerFormState.ts +++ b/packages/ui/src/forms/Form/mergeServerFormState.ts @@ -1,5 +1,7 @@ import type { FormState } from 'payload/types' +import isDeepEqual from 'deep-equal' + import { arraysHaveSameStrings } from '../../utilities/arraysHaveSameStrings.js' const propsToCheck = ['passesCondition', 'valid', 'errorMessage'] @@ -48,6 +50,13 @@ export const mergeServerFormState = ( changed = true } }) + + if ( + Array.isArray(newFieldState.rows) && + !isDeepEqual(newFieldState.rows, oldState[path].rows) + ) { + changed = true + } }) } From 6f3934f2e563243e50e440836e414fb778eca67f Mon Sep 17 00:00:00 2001 From: Alessio Gravili Date: Wed, 27 Mar 2024 11:44:50 -0400 Subject: [PATCH 05/11] fix: server form-state request wasn't triggered on first onChange --- packages/ui/src/forms/Form/index.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/ui/src/forms/Form/index.tsx b/packages/ui/src/forms/Form/index.tsx index c14574862..80b43a450 100644 --- a/packages/ui/src/forms/Form/index.tsx +++ b/packages/ui/src/forms/Form/index.tsx @@ -552,7 +552,9 @@ export const Form: React.FC = (props) => { if (modified) void executeOnChange() }, 150, - [contextRef.current.fields, dispatchFields, onChange], + // Make sure we trigger this whenever modified changes (not just when `fields` changes), otherwise we will miss merging server form state for the first form update/onChange. Here's why: + // `fields` updates before `modified`, because setModified is in a setTimeout. So on the first change, modified is false, so we don't trigger the effect even though we should. + [contextRef.current.fields, dispatchFields, onChange, modified], ) const actionString = From a7afb1f680830eb79f3083b39b7602060634cab5 Mon Sep 17 00:00:00 2001 From: Alessio Gravili Date: Wed, 27 Mar 2024 11:54:12 -0400 Subject: [PATCH 06/11] chore: enable all lexical e2e tests --- test/fields/lexical.e2e.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fields/lexical.e2e.spec.ts b/test/fields/lexical.e2e.spec.ts index 18d1d4fab..7ab19e703 100644 --- a/test/fields/lexical.e2e.spec.ts +++ b/test/fields/lexical.e2e.spec.ts @@ -718,7 +718,7 @@ describe('lexical', () => { await expect(nestedEditorParagraph).toHaveText('Some text below relationship node 12345') }) - test.skip('should respect row removal in nested array field', async () => { + test('should respect row removal in nested array field', async () => { await navigateToLexicalFields() const richTextField = page.locator('.rich-text-lexical').nth(1) // second await richTextField.scrollIntoViewIfNeeded() From e99b168bd60265010134c15d04683656419b1a4f Mon Sep 17 00:00:00 2001 From: Alessio Gravili Date: Wed, 27 Mar 2024 12:03:38 -0400 Subject: [PATCH 07/11] ci: do not retry failing tests - we shouldn't have flaky tests in the first place --- .github/workflows/main.yml | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 6e2246984..7d5b00b3e 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -254,12 +254,7 @@ jobs: run: pnpm exec playwright install - name: E2E Tests - uses: nick-fields/retry@v3 - with: - retry_on: error - max_attempts: 2 - timeout_minutes: 15 - command: pnpm test:e2e ${{ matrix.suite }} + run: pnpm test:e2e ${{ matrix.suite }} - uses: actions/upload-artifact@v4 if: always() From 08ff286f9a78de8b9398419813a121c0c402c2c5 Mon Sep 17 00:00:00 2001 From: Alessio Gravili Date: Wed, 27 Mar 2024 12:18:17 -0400 Subject: [PATCH 08/11] chore: e2e: replace flaky manual save actions with non-flaky saveDocAndAssert helper --- test/access-control/e2e.spec.ts | 13 +++++++------ test/fields/e2e.spec.ts | 13 ++++--------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/test/access-control/e2e.spec.ts b/test/access-control/e2e.spec.ts index c81e739bb..7f43984cb 100644 --- a/test/access-control/e2e.spec.ts +++ b/test/access-control/e2e.spec.ts @@ -9,10 +9,12 @@ import type { ReadOnlyCollection, RestrictedVersion } from './payload-types.js' import { closeNav, + delayNetwork, exactText, initPageConsoleErrorCatch, openDocControls, openNav, + saveDocAndAssert, } from '../helpers.js' import { AdminUrlUtil } from '../helpers/adminUrlUtil.js' import { initPayloadE2E } from '../helpers/initPayloadE2E.js' @@ -58,6 +60,7 @@ describe('access control', () => { const context = await browser.newContext() page = await context.newPage() initPageConsoleErrorCatch(page) + await delayNetwork({ context, page, delay: 'Slow 4G' }) }) test('field without read access should not show', async () => { @@ -248,7 +251,7 @@ describe('access control', () => { await expect(deleteAction).toBeHidden() await page.locator('#field-approvedForRemoval').check() - await page.locator('#action-save').click() + await saveDocAndAssert(page) await openDocControls(page) const deleteAction2 = page.locator('#action-delete') @@ -272,7 +275,7 @@ describe('access control', () => { // Allow access to test global. await page.locator('.checkbox-input:has(#field-test) input').check() - await page.locator('#action-save').click() + await saveDocAndAssert(page) await openNav(page) @@ -299,8 +302,7 @@ describe('access control', () => { const documentDrawer = page.locator('[id^=doc-drawer_user-restricted_1_]') await expect(documentDrawer).toBeVisible() await documentDrawer.locator('#field-name').fill('anonymous@email.com') - await documentDrawer.locator('#action-save').click() - await expect(page.locator('.Toastify')).toContainText('successfully') + await saveDocAndAssert(page) // ensure user is not allowed to edit this document await expect(documentDrawer.locator('#field-name')).toBeDisabled() @@ -311,8 +313,7 @@ describe('access control', () => { const documentDrawer2 = page.locator('[id^=doc-drawer_user-restricted_1_]') await expect(documentDrawer2).toBeVisible() await documentDrawer2.locator('#field-name').fill('dev@payloadcms.com') - await documentDrawer2.locator('#action-save').click() - await expect(page.locator('.Toastify')).toContainText('successfully') + await saveDocAndAssert(page) // ensure user is allowed to edit this document await expect(documentDrawer2.locator('#field-name')).toBeEnabled() diff --git a/test/fields/e2e.spec.ts b/test/fields/e2e.spec.ts index 73ae36a6d..1b3d3136a 100644 --- a/test/fields/e2e.spec.ts +++ b/test/fields/e2e.spec.ts @@ -1386,7 +1386,8 @@ describe('fields', () => { await expect(dateField).toBeVisible() await dateField.fill('02/07/2023') await expect(dateField).toHaveValue('02/07/2023') - await page.locator('#action-save').click() + await saveDocAndAssert(page) + const clearButton = page.locator('#field-default .date-time-picker__clear-button') await expect(clearButton).toBeVisible() await clearButton.click() @@ -1409,10 +1410,7 @@ describe('fields', () => { // enter date in default date field await dateField.fill('02/07/2023') - await page.locator('#action-save').click() - - // wait for navigation to update route - await expect.poll(() => page.url(), { timeout: 1000 }).not.toContain('create') + await saveDocAndAssert(page) // get the ID of the doc const routeSegments = page.url().split('/') @@ -1473,10 +1471,7 @@ describe('fields', () => { // enter date in default date field await dateField.fill('02/07/2023') - await page.locator('#action-save').click() - - // wait for navigation to update route - await expect.poll(() => page.url(), { timeout: 1000 }).not.toContain('create') + await saveDocAndAssert(page) // get the ID of the doc const routeSegments = page.url().split('/') From db8e805a967ba5f494bab07106f77ca884de1402 Mon Sep 17 00:00:00 2001 From: Alessio Gravili Date: Wed, 27 Mar 2024 15:55:19 -0400 Subject: [PATCH 09/11] fix: improve error path merging from server, make sure no new or removed rows/values coming from the server are being considered outside addFieldRow --- packages/payload/src/admin/forms/Form.ts | 4 +- packages/ui/src/fields/Array/ArrayRow.tsx | 2 +- packages/ui/src/fields/Array/index.tsx | 2 +- packages/ui/src/fields/Blocks/index.tsx | 2 +- packages/ui/src/forms/Form/fieldReducer.ts | 11 +-- packages/ui/src/forms/Form/index.tsx | 8 +- packages/ui/src/forms/Form/mergeErrorPaths.ts | 38 ++++++++ .../ui/src/forms/Form/mergeServerFormState.ts | 89 +++++++++++-------- .../addFieldStatePromise.ts | 10 ++- .../src/forms/buildStateFromSchema/index.tsx | 2 +- .../buildStateFromSchema/iterateFields.ts | 2 +- test/access-control/e2e.spec.ts | 2 - test/fields/lexical.e2e.spec.ts | 2 +- 13 files changed, 115 insertions(+), 59 deletions(-) create mode 100644 packages/ui/src/forms/Form/mergeErrorPaths.ts diff --git a/packages/payload/src/admin/forms/Form.ts b/packages/payload/src/admin/forms/Form.ts index 8b9f65770..8501b5ff2 100644 --- a/packages/payload/src/admin/forms/Form.ts +++ b/packages/payload/src/admin/forms/Form.ts @@ -8,7 +8,7 @@ export type Data = { export type Row = { blockType?: string collapsed?: boolean - errorPaths?: Set + errorPaths?: string[] id: string } @@ -19,7 +19,7 @@ export type FilterOptionsResult = { export type FormField = { disableFormData?: boolean errorMessage?: string - errorPaths?: Set + errorPaths?: string[] fieldSchema?: Field filterOptions?: FilterOptionsResult initialValue: unknown diff --git a/packages/ui/src/fields/Array/ArrayRow.tsx b/packages/ui/src/fields/Array/ArrayRow.tsx index 6540dd423..f592a24a6 100644 --- a/packages/ui/src/fields/Array/ArrayRow.tsx +++ b/packages/ui/src/fields/Array/ArrayRow.tsx @@ -70,7 +70,7 @@ export const ArrayRow: React.FC = ({ '0', )}` - const errorCount = row.errorPaths?.size + const errorCount = row.errorPaths?.length const fieldHasErrors = errorCount > 0 && hasSubmitted const classNames = [ diff --git a/packages/ui/src/fields/Array/index.tsx b/packages/ui/src/fields/Array/index.tsx index 71bdfa541..6fdf02859 100644 --- a/packages/ui/src/fields/Array/index.tsx +++ b/packages/ui/src/fields/Array/index.tsx @@ -181,7 +181,7 @@ export const _ArrayField: React.FC = (props) => { const hasMaxRows = maxRows && rows.length >= maxRows const fieldErrorCount = - rows.reduce((total, row) => total + (row?.errorPaths?.size || 0), 0) + (valid ? 0 : 1) + rows.reduce((total, row) => total + (row?.errorPaths?.length || 0), 0) + (valid ? 0 : 1) const fieldHasErrors = submitted && fieldErrorCount > 0 diff --git a/packages/ui/src/fields/Blocks/index.tsx b/packages/ui/src/fields/Blocks/index.tsx index 4c783a045..57828b27d 100644 --- a/packages/ui/src/fields/Blocks/index.tsx +++ b/packages/ui/src/fields/Blocks/index.tsx @@ -192,7 +192,7 @@ const _BlocksField: React.FC = (props) => { const hasMaxRows = maxRows && rows.length >= maxRows - const fieldErrorCount = rows.reduce((total, row) => total + (row?.errorPaths?.size || 0), 0) + const fieldErrorCount = rows.reduce((total, row) => total + (row?.errorPaths?.length || 0), 0) const fieldHasErrors = submitted && fieldErrorCount + (valid ? 0 : 1) > 0 const showMinRows = rows.length < minRows || (required && rows.length === 0) diff --git a/packages/ui/src/forms/Form/fieldReducer.ts b/packages/ui/src/forms/Form/fieldReducer.ts index a9dd46260..da4af3203 100644 --- a/packages/ui/src/forms/Form/fieldReducer.ts +++ b/packages/ui/src/forms/Form/fieldReducer.ts @@ -17,14 +17,13 @@ const ObjectId = (ObjectIdImport.default || export function fieldReducer(state: FormState, action: FieldAction): FormState { switch (action.type) { case 'REPLACE_STATE': { - const newState = {} - if (action.optimize !== false) { // Only update fields that have changed // by comparing old value / initialValue to new // .. // This is a performance enhancement for saving // large documents with hundreds of fields + const newState = {} Object.entries(action.state).forEach(([path, field]) => { const oldField = state[path] @@ -36,12 +35,10 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState { newState[path] = oldField } }) - } else { - // If we're not optimizing, just set the state to the new state - return action.state + return newState } - - return newState + // If we're not optimizing, just set the state to the new state + return action.state } case 'REMOVE': { diff --git a/packages/ui/src/forms/Form/index.tsx b/packages/ui/src/forms/Form/index.tsx index 80b43a450..598b6655f 100644 --- a/packages/ui/src/forms/Form/index.tsx +++ b/packages/ui/src/forms/Form/index.tsx @@ -5,7 +5,7 @@ import type { FormState } from 'payload/types' import isDeepEqual from 'deep-equal' import { useRouter } from 'next/navigation.js' import { serialize } from 'object-to-formdata' -import { wait } from 'payload/utilities' +import { deepCopyObject, wait } from 'payload/utilities' import QueryString from 'qs' import React, { useCallback, useEffect, useReducer, useRef, useState } from 'react' import { toast } from 'react-toastify' @@ -544,7 +544,11 @@ export const Form: React.FC = (props) => { ) if (changed) { - dispatchFields({ type: 'REPLACE_STATE', optimize: false, state: newState }) + dispatchFields({ + type: 'REPLACE_STATE', + optimize: false, + state: newState, + }) } } } diff --git a/packages/ui/src/forms/Form/mergeErrorPaths.ts b/packages/ui/src/forms/Form/mergeErrorPaths.ts new file mode 100644 index 000000000..32b2148d7 --- /dev/null +++ b/packages/ui/src/forms/Form/mergeErrorPaths.ts @@ -0,0 +1,38 @@ +import { arraysHaveSameStrings } from '@payloadcms/ui/utilities/arraysHaveSameStrings' + +export const mergeErrorPaths = ( + existing?: string[], + incoming?: string[], +): { + changed: boolean + result?: string[] +} => { + if (!existing) { + return { + changed: false, + result: undefined, + } + } + + const existingErrorPaths: string[] = [] + const incomingErrorPaths: string[] = [] + + if (Array.isArray(incoming) && incoming?.length) { + incoming.forEach((path) => incomingErrorPaths.push(path)) + } + + if (Array.isArray(existing) && existing?.length) { + existing.forEach((path) => existingErrorPaths.push(path)) + } + + if (!arraysHaveSameStrings(existingErrorPaths, incomingErrorPaths)) { + return { + changed: true, + result: incomingErrorPaths, + } + } + return { + changed: false, + result: existing, + } +} diff --git a/packages/ui/src/forms/Form/mergeServerFormState.ts b/packages/ui/src/forms/Form/mergeServerFormState.ts index b5f55ad2c..7906736dc 100644 --- a/packages/ui/src/forms/Form/mergeServerFormState.ts +++ b/packages/ui/src/forms/Form/mergeServerFormState.ts @@ -1,10 +1,8 @@ import type { FormState } from 'payload/types' -import isDeepEqual from 'deep-equal' +import { mergeErrorPaths } from './mergeErrorPaths.js' -import { arraysHaveSameStrings } from '../../utilities/arraysHaveSameStrings.js' - -const propsToCheck = ['passesCondition', 'valid', 'errorMessage'] +const serverPropsToAccept = ['passesCondition', 'valid', 'errorMessage'] /** * Merges certain properties from the server state into the client state. These do not include values, @@ -14,51 +12,70 @@ const propsToCheck = ['passesCondition', 'valid', 'errorMessage'] * is the thing we want to keep in sync with the server (where it's calculated) on the client. */ export const mergeServerFormState = ( - oldState: FormState, - newState: FormState, + existingState: FormState, + incomingState: FormState, ): { changed: boolean; newState: FormState } => { let changed = false - if (oldState) { - Object.entries(newState).forEach(([path, newFieldState]) => { - if (!oldState[path]) return + if (existingState) { + Object.entries(existingState).forEach(([path, newFieldState]) => { + if (!incomingState[path]) return - newFieldState.initialValue = oldState[path].initialValue - newFieldState.value = oldState[path].value + /** + * Handle error paths + */ - const oldErrorPaths: string[] = [] - const newErrorPaths: string[] = [] - - if (oldState[path].errorPaths instanceof Set) { - oldState[path].errorPaths.forEach((path) => oldErrorPaths.push(path)) + const errorPathsResult = mergeErrorPaths( + newFieldState.errorPaths, + incomingState[path].errorPaths as unknown as string[], + ) + if (errorPathsResult.result) { + if (errorPathsResult.changed) { + changed = errorPathsResult.changed + } + newFieldState.errorPaths = errorPathsResult.result } - if ( - newFieldState.errorPaths && - !Array.isArray(newFieldState.errorPaths) && - typeof newFieldState.errorPaths - ) { - Object.values(newFieldState.errorPaths).forEach((path) => newErrorPaths.push(path)) - } - - if (!arraysHaveSameStrings(oldErrorPaths, newErrorPaths)) { - changed = true - } - - propsToCheck.forEach((prop) => { - if (newFieldState[prop] != oldState[path][prop]) { + /** + * Handle the rest which is in serverPropsToAccept + */ + serverPropsToAccept.forEach((prop) => { + if (incomingState[path]?.[prop] !== newFieldState[prop]) { changed = true + if (!(prop in incomingState[path])) { + delete newFieldState[prop] + } else { + newFieldState[prop] = incomingState[path][prop] + } } }) - if ( - Array.isArray(newFieldState.rows) && - !isDeepEqual(newFieldState.rows, oldState[path].rows) - ) { - changed = true + /** + * Handle rows + */ + if (Array.isArray(newFieldState.rows) && newFieldState?.rows.length) { + let i = 0 + for (const row of newFieldState.rows) { + const incomingRow = incomingState[path]?.rows?.[i] + if (incomingRow) { + const errorPathsRowResult = mergeErrorPaths( + row.errorPaths, + incomingRow.errorPaths as unknown as string[], + ) + if (errorPathsRowResult.result) { + if (errorPathsResult.changed) { + changed = true + } + incomingRow.errorPaths = errorPathsRowResult.result + } + } + + i++ + } } }) } - return { changed, newState } + // Conditions don't work if we don't memcopy the new state, as the object references would otherwise be the same + return { changed, newState: { ...existingState } } } diff --git a/packages/ui/src/forms/buildStateFromSchema/addFieldStatePromise.ts b/packages/ui/src/forms/buildStateFromSchema/addFieldStatePromise.ts index 33ae11190..67533c4f6 100644 --- a/packages/ui/src/forms/buildStateFromSchema/addFieldStatePromise.ts +++ b/packages/ui/src/forms/buildStateFromSchema/addFieldStatePromise.ts @@ -23,7 +23,7 @@ export type AddFieldStatePromiseArgs = { */ anyParentLocalized?: boolean data: Data - errorPaths: Set + errorPaths: string[] field: NonPresentationalField /** * You can use this to filter down to only `localized` fields that require translation (type: text, textarea, etc.). Another plugin might want to look for only `point` type fields to do some GIS function. With the filter function you can go in like a surgeon. @@ -74,7 +74,7 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom id, anyParentLocalized = false, data, - errorPaths: parentErrorPaths, + errorPaths: parentErrorPaths = [], field, filter, forceFullValue = false, @@ -95,7 +95,7 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom const validate = operation === 'update' ? field.validate : undefined const fieldState: FormField = { - errorPaths: new Set(), + errorPaths: [], fieldSchema: includeSchema ? field : undefined, initialValue: undefined, passesCondition, @@ -144,7 +144,9 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom fieldState.valid = false // TODO: this is unpredictable, need to figure out why // It will sometimes lead to inconsistencies across re-renders - parentErrorPaths.add(`${path}${field.name}`) + if (!parentErrorPaths.includes(`${path}${field.name}`)) { + parentErrorPaths.push(`${path}${field.name}`) + } } else { fieldState.valid = true } diff --git a/packages/ui/src/forms/buildStateFromSchema/index.tsx b/packages/ui/src/forms/buildStateFromSchema/index.tsx index 3908327de..74f616626 100644 --- a/packages/ui/src/forms/buildStateFromSchema/index.tsx +++ b/packages/ui/src/forms/buildStateFromSchema/index.tsx @@ -40,7 +40,7 @@ export const buildStateFromSchema = async (args: Args): Promise => { await iterateFields({ id, data: fullData, - errorPaths: new Set(), + errorPaths: [], fields: fieldSchema, fullData, operation, diff --git a/packages/ui/src/forms/buildStateFromSchema/iterateFields.ts b/packages/ui/src/forms/buildStateFromSchema/iterateFields.ts index 7dbba5ed0..e7455f8d8 100644 --- a/packages/ui/src/forms/buildStateFromSchema/iterateFields.ts +++ b/packages/ui/src/forms/buildStateFromSchema/iterateFields.ts @@ -12,7 +12,7 @@ type Args = { */ anyParentLocalized?: boolean data: Data - errorPaths: Set + errorPaths: string[] fields: FieldSchema[] filter?: (args: AddFieldStatePromiseArgs) => boolean /** diff --git a/test/access-control/e2e.spec.ts b/test/access-control/e2e.spec.ts index 7f43984cb..4da63e281 100644 --- a/test/access-control/e2e.spec.ts +++ b/test/access-control/e2e.spec.ts @@ -9,7 +9,6 @@ import type { ReadOnlyCollection, RestrictedVersion } from './payload-types.js' import { closeNav, - delayNetwork, exactText, initPageConsoleErrorCatch, openDocControls, @@ -60,7 +59,6 @@ describe('access control', () => { const context = await browser.newContext() page = await context.newPage() initPageConsoleErrorCatch(page) - await delayNetwork({ context, page, delay: 'Slow 4G' }) }) test('field without read access should not show', async () => { diff --git a/test/fields/lexical.e2e.spec.ts b/test/fields/lexical.e2e.spec.ts index 7ab19e703..255b22d6c 100644 --- a/test/fields/lexical.e2e.spec.ts +++ b/test/fields/lexical.e2e.spec.ts @@ -32,7 +32,7 @@ async function navigateToLexicalFields() { const url: AdminUrlUtil = new AdminUrlUtil(serverURL, 'lexical-fields') await page.goto(url.list) const linkToDoc = page.locator('tbody tr:first-child .cell-title a').first() - await expect(() => expect(linkToDoc).toBeTruthy()).toPass({ timeout: 45000 }) + await expect(() => expect(linkToDoc).toBeTruthy()).toPass({ timeout: POLL_TOPASS_TIMEOUT }) const linkDocHref = await linkToDoc.getAttribute('href') await linkToDoc.click() From e6d4445a8ad09adb01561f9fc9ac1e19657ec403 Mon Sep 17 00:00:00 2001 From: Alessio Gravili Date: Wed, 27 Mar 2024 16:03:23 -0400 Subject: [PATCH 10/11] chore: fix build --- packages/ui/src/fields/Blocks/BlockRow.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ui/src/fields/Blocks/BlockRow.tsx b/packages/ui/src/fields/Blocks/BlockRow.tsx index 763b9defb..5b6518816 100644 --- a/packages/ui/src/fields/Blocks/BlockRow.tsx +++ b/packages/ui/src/fields/Blocks/BlockRow.tsx @@ -67,7 +67,7 @@ export const BlockRow: React.FC = ({ const { i18n } = useTranslation() const hasSubmitted = useFormSubmitted() - const errorCount = row.errorPaths?.size + const errorCount = row.errorPaths?.length const fieldHasErrors = errorCount > 0 && hasSubmitted const classNames = [ From 8deb19e3ac9186e3d74edcf61bb54e8731a2a5cc Mon Sep 17 00:00:00 2001 From: Alessio Gravili Date: Wed, 27 Mar 2024 16:17:46 -0400 Subject: [PATCH 11/11] chore: replace incorrect usage of saveDocAndAssert helpers --- test/access-control/e2e.spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/access-control/e2e.spec.ts b/test/access-control/e2e.spec.ts index 4da63e281..6c68500c8 100644 --- a/test/access-control/e2e.spec.ts +++ b/test/access-control/e2e.spec.ts @@ -300,7 +300,8 @@ describe('access control', () => { const documentDrawer = page.locator('[id^=doc-drawer_user-restricted_1_]') await expect(documentDrawer).toBeVisible() await documentDrawer.locator('#field-name').fill('anonymous@email.com') - await saveDocAndAssert(page) + await documentDrawer.locator('#action-save').click() + await expect(page.locator('.Toastify')).toContainText('successfully') // ensure user is not allowed to edit this document await expect(documentDrawer.locator('#field-name')).toBeDisabled() @@ -311,7 +312,8 @@ describe('access control', () => { const documentDrawer2 = page.locator('[id^=doc-drawer_user-restricted_1_]') await expect(documentDrawer2).toBeVisible() await documentDrawer2.locator('#field-name').fill('dev@payloadcms.com') - await saveDocAndAssert(page) + await documentDrawer2.locator('#action-save').click() + await expect(page.locator('.Toastify')).toContainText('successfully') // ensure user is allowed to edit this document await expect(documentDrawer2.locator('#field-name')).toBeEnabled()