fix(ui): incorrect error states (#11574)

Fixes https://github.com/payloadcms/payload/issues/11568

### What? Out of sync errors states
- Collaspibles & Tabs were not reporting accurate child error counts
- Arrays could get into a state where they would not update their error
states
- Slight issue with toasts 

### Tabs & Collapsibles
The logic for determining matching field paths was not functioning as
intended. Fields were attempting to match with paths such as `_index-0`
which will not work.

### Arrays
The form state was not updating when the server sent back errorPaths.
This PR adds `errorPaths` to `serverPropsToAccept`.

### Toasts
Some toasts could report errors in the form of `my > > error`. This
ensures they will be `my > error`

### Misc
Removes 2 files that were not in use:
- `getFieldStateFromPaths.ts`
- `getNestedFieldState.ts`
This commit is contained in:
Jarrod Flesch
2025-03-06 14:02:10 -05:00
committed by GitHub
parent 7cef8900a7
commit 48115311e7
19 changed files with 141 additions and 165 deletions

View File

@@ -17,7 +17,9 @@ import { traverseFields } from './traverseFields.js'
function buildFieldLabel(parentLabel: string, label: string): string {
const capitalizedLabel = label.charAt(0).toUpperCase() + label.slice(1)
return parentLabel ? `${parentLabel} > ${capitalizedLabel}` : capitalizedLabel
return parentLabel && capitalizedLabel
? `${parentLabel} > ${capitalizedLabel}`
: capitalizedLabel || parentLabel
}
type Args = {

View File

@@ -46,7 +46,8 @@ function createErrorsFromMessage(message: string): {
if (errors.length === 1) {
return {
message: `${intro}: ${errors[0]}`,
errors,
message: `${intro}:`,
}
}

View File

@@ -107,7 +107,12 @@ const CollapsibleFieldComponent: CollapsibleFieldClientComponent = (props) => {
return (
<Fragment>
<WatchChildErrors fields={fields} path={path.split('.')} setErrorCount={setErrorCount} />
<WatchChildErrors
fields={fields}
// removes the 'collapsible' path segment, i.e. `_index-0`
path={path.split('.').slice(0, -1)}
setErrorCount={setErrorCount}
/>
<div
className={[
fieldBaseClass,

View File

@@ -25,15 +25,16 @@ export const TabComponent: React.FC<TabProps> = ({ isActive, parentPath, setIsAc
const [errorCount, setErrorCount] = useState(undefined)
const path = [
...(parentPath ? parentPath.split('.') : []),
// removes parent 'tabs' path segment, i.e. `_index-0`
...(parentPath ? parentPath.split('.').slice(0, -1) : []),
...(tabHasName(tab) ? [tab.name] : []),
].join('.')
]
const fieldHasErrors = errorCount > 0
return (
<React.Fragment>
<WatchChildErrors fields={tab.fields} path={path.split('.')} setErrorCount={setErrorCount} />
<WatchChildErrors fields={tab.fields} path={path} setErrorCount={setErrorCount} />
<button
className={[
baseClass,

View File

@@ -31,6 +31,7 @@ export const mergeServerFormState = ({
'passesCondition',
'valid',
'errorMessage',
'errorPaths',
'rows',
'customComponents',
'requiresRender',

View File

@@ -3,41 +3,34 @@ import type { ClientField } from 'payload'
import { fieldAffectsData } from 'payload/shared'
export const buildPathSegments = (
parentPath: (number | string)[],
fields: ClientField[],
): string[] => {
const pathNames = fields.reduce((acc, field) => {
export const buildPathSegments = (fields: ClientField[]): (`${string}.` | string)[] => {
return fields.reduce((acc: (`${string}.` | string)[], field) => {
const fields: ClientField[] = 'fields' in field ? field.fields : undefined
if (fields) {
if (fieldAffectsData(field)) {
// group, block, array
const name = 'name' in field ? field.name : 'unnamed'
acc.push(...[...parentPath, name])
acc.push(`${field.name}.`)
} else {
// rows, collapsibles, unnamed-tab
acc.push(...buildPathSegments(parentPath, fields))
acc.push(...buildPathSegments(fields))
}
} else if (field.type === 'tabs') {
// tabs
if ('tabs' in field) {
field.tabs?.forEach((tab) => {
let tabPath = parentPath
if ('name' in tab) {
tabPath = [...parentPath, tab.name]
acc.push(`${tab.name}.`)
} else {
acc.push(...buildPathSegments(tab.fields))
}
acc.push(...buildPathSegments(tabPath, tab.fields))
})
}
} else if (fieldAffectsData(field)) {
// text, number, date, etc.
const name = 'name' in field ? field.name : 'unnamed'
acc.push(...[...parentPath, name])
acc.push(field.name)
}
return acc
}, [])
return pathNames
}

View File

@@ -1,38 +0,0 @@
'use client'
import type { FormState } from 'payload'
export const getFieldStateFromPaths = ({
formState,
pathSegments,
}: {
formState: FormState
pathSegments: string[]
}): {
errorCount: number
fieldState: FormState
} => {
const fieldState: FormState = {}
let errorCount = 0
Object.entries(formState).forEach(([key]) => {
const matchingSegment = pathSegments?.some((segment) => {
if (segment.endsWith('.')) {
return key.startsWith(segment)
}
return key === segment
})
if (matchingSegment) {
const pathState = formState[key]
fieldState[key] = pathState
if ('valid' in pathState && !pathState.valid) {
errorCount += 1
}
}
})
return {
errorCount,
fieldState,
}
}

View File

@@ -1,34 +0,0 @@
'use client'
import type { Field, FormState } from 'payload'
// import { buildPathSegments } from './buildPathSegments'
import { getFieldStateFromPaths } from './getFieldStateFromPaths.js'
export const getNestedFieldState = ({
fieldSchema,
formState,
// path,
pathSegments: pathSegmentsFromProps,
}: {
fieldSchema?: Field[]
formState?: FormState
path: string
pathSegments?: string[]
}): {
errorCount: number
fieldState: FormState
pathSegments: string[]
} => {
const pathSegments = pathSegmentsFromProps
if (!pathSegments && fieldSchema) {
// pathSegments = buildPathSegments(path, fieldSchema)
}
const result = getFieldStateFromPaths({ formState, pathSegments })
return {
...result,
pathSegments,
}
}

View File

@@ -5,28 +5,50 @@ import type React from 'react'
import { useThrottledEffect } from '../../hooks/useThrottledEffect.js'
import { useAllFormFields, useFormSubmitted } from '../Form/context.js'
import { buildPathSegments } from './buildPathSegments.js'
import { getFieldStateFromPaths } from './getFieldStateFromPaths.js'
type TrackSubSchemaErrorCountProps = {
fields?: ClientField[]
/**
* This path should only include path segments that affect data
* i.e. it should not include _index-0 type segments
*
* For collapsibles and tabs you can simply pass their parent path
*/
path: (number | string)[]
setErrorCount: (count: number) => void
}
export const WatchChildErrors: React.FC<TrackSubSchemaErrorCountProps> = ({
fields,
path,
path: parentPath,
setErrorCount,
}) => {
const [formState] = useAllFormFields()
const hasSubmitted = useFormSubmitted()
const pathSegments = buildPathSegments(path, fields)
const segmentsToMatch = buildPathSegments(fields)
useThrottledEffect(
() => {
if (hasSubmitted) {
const { errorCount } = getFieldStateFromPaths({ formState, pathSegments })
let errorCount = 0
Object.entries(formState).forEach(([key]) => {
const matchingSegment = segmentsToMatch?.some((segment) => {
const segmentToMatch = [...parentPath, segment].join('.')
// match fields with same parent path
if (segmentToMatch.endsWith('.')) {
return key.startsWith(segmentToMatch)
}
// match fields with same path
return key === segmentToMatch
})
if (matchingSegment) {
const pathState = formState[key]
if ('valid' in pathState && !pathState.valid) {
errorCount += 1
}
}
})
setErrorCount(errorCount)
}
},

View File

@@ -62,6 +62,12 @@ export const testEslintConfig = [
'payload/no-wait-function': 'warn',
// Enable the no-non-retryable-assertions rule ONLY for hunting for flakes
// 'payload/no-non-retryable-assertions': 'error',
'playwright/expect-expect': [
'error',
{
assertFunctionNames: ['assertToastErrors', 'saveDocAndAssert', 'runFilterOptionsTest'],
},
],
},
},
{

View File

@@ -1,6 +1,7 @@
import type { Page } from '@playwright/test'
import { expect, test } from '@playwright/test'
import { assertToastErrors } from 'helpers/assertToastErrors.js'
import { addListFilter } from 'helpers/e2e/addListFilter.js'
import { openDocControls } from 'helpers/e2e/openDocControls.js'
import { openCreateDocDrawer, openDocDrawer } from 'helpers/e2e/toggleDocDrawer.js'
@@ -288,9 +289,10 @@ describe('Relationship Field', () => {
await expect(field).toContainText(anotherRelationOneDoc.id)
await wait(2000) // Need to wait form state to come back before clicking save
await page.locator('#action-save').click()
await expect(page.locator('.payload-toast-container')).toContainText(
`is invalid: ${fieldLabel}`,
)
await assertToastErrors({
page,
errors: [fieldLabel],
})
filteredField = page.locator(`#field-${fieldName} .react-select`)
await filteredField.click({ delay: 100 })
filteredOptions = filteredField.locator('.rs__option')
@@ -303,7 +305,7 @@ describe('Relationship Field', () => {
describe('filterOptions', () => {
// TODO: Flaky test. Fix this! (This is an actual issue not just an e2e flake)
test('should allow dynamic filterOptions', async () => {
await runFilterOptionsTest('relationshipFilteredByID', 'Relationship Filtered')
await runFilterOptionsTest('relationshipFilteredByID', 'Relationship Filtered By ID')
})
// TODO: Flaky test. Fix this! (This is an actual issue not just an e2e flake)

View File

@@ -1,6 +1,7 @@
import type { Page } from '@playwright/test'
import { expect, test } from '@playwright/test'
import { assertToastErrors } from 'helpers/assertToastErrors.js'
import path from 'path'
import { wait } from 'payload/shared'
import { fileURLToPath } from 'url'
@@ -124,9 +125,10 @@ describe('Array', () => {
await page.locator('#field-arrayWithMinRows >> .array-field__add-row').click()
await page.click('#action-save', { delay: 100 })
await expect(page.locator('.payload-toast-container')).toContainText(
'The following field is invalid: Array With Min Rows',
)
await assertToastErrors({
page,
errors: ['Array With Min Rows'],
})
})
test('should show singular label for array rows', async () => {

View File

@@ -13,6 +13,7 @@ import {
saveDocAndAssert,
} from '../../../helpers.js'
import { AdminUrlUtil } from '../../../helpers/adminUrlUtil.js'
import { assertToastErrors } from '../../../helpers/assertToastErrors.js'
import { initPayloadE2ENoConfig } from '../../../helpers/initPayloadE2ENoConfig.js'
import { reInitializeDB } from '../../../helpers/reInitializeDB.js'
import { RESTClient } from '../../../helpers/rest.js'
@@ -274,9 +275,10 @@ describe('Block fields', () => {
await expect(firstRow).toHaveValue('first row')
await page.click('#action-save', { delay: 100 })
await expect(page.locator('.payload-toast-container')).toContainText(
'The following field is invalid: Blocks With Min Rows',
)
await assertToastErrors({
page,
errors: ['Blocks With Min Rows'],
})
})
test('ensure functions passed to blocks field labels property are respected', async () => {

View File

@@ -10,6 +10,7 @@ import type { Config } from '../../payload-types.js'
import { ensureCompilationIsDone, initPageConsoleErrorCatch } from '../../../helpers.js'
import { AdminUrlUtil } from '../../../helpers/adminUrlUtil.js'
import { assertToastErrors } from '../../../helpers/assertToastErrors.js'
import { initPayloadE2ENoConfig } from '../../../helpers/initPayloadE2ENoConfig.js'
import { reInitializeDB } from '../../../helpers/reInitializeDB.js'
import { RESTClient } from '../../../helpers/rest.js'
@@ -96,9 +97,10 @@ describe('Radio', () => {
await page.click('#action-save', { delay: 200 })
// toast error
await expect(page.locator('.payload-toast-container')).toContainText(
'The following field is invalid: uniqueText',
)
await assertToastErrors({
page,
errors: ['uniqueText'],
})
await expect.poll(() => page.url(), { timeout: POLL_TOPASS_TIMEOUT }).toContain('create')
@@ -117,9 +119,10 @@ describe('Radio', () => {
await page.locator('#action-save').click()
// toast error
await expect(page.locator('.payload-toast-container')).toContainText(
'The following field is invalid: group.unique',
)
await assertToastErrors({
page,
errors: ['group.unique'],
})
await expect.poll(() => page.url(), { timeout: POLL_TOPASS_TIMEOUT }).toContain('create')

View File

@@ -24,6 +24,7 @@ import {
saveDocAndAssert,
} from '../../../../../helpers.js'
import { AdminUrlUtil } from '../../../../../helpers/adminUrlUtil.js'
import { assertToastErrors } from '../../../../../helpers/assertToastErrors.js'
import { trackNetworkRequests } from '../../../../../helpers/e2e/trackNetworkRequests.js'
import { initPayloadE2ENoConfig } from '../../../../../helpers/initPayloadE2ENoConfig.js'
import { reInitializeDB } from '../../../../../helpers/reInitializeDB.js'
@@ -570,17 +571,10 @@ describe('lexicalBlocks', () => {
await topLevelDocTextField.fill('invalid')
await saveDocAndAssert(page, '#action-save', 'error')
await expect(
page
.locator('.payload-toast-container li')
.filter({ hasText: 'The following fields are invalid (2):' }),
).toBeVisible()
await expect(
page.locator('.payload-toast-container [data-testid="field-errors"] li').nth(0),
).toHaveText('Lexical With Blocks')
await expect(
page.locator('.payload-toast-container [data-testid="field-errors"] li').nth(1),
).toHaveText('Lexical With Blocks → Group → Text Depends On Doc Data')
await assertToastErrors({
page,
errors: ['Lexical With Blocks', 'Lexical With Blocks → Group → Text Depends On Doc Data'],
})
await expect(page.locator('.payload-toast-container .payload-toast-item')).toBeHidden()
await trackNetworkRequests(
@@ -601,18 +595,13 @@ describe('lexicalBlocks', () => {
await blockGroupTextField.fill('invalid')
await saveDocAndAssert(page, '#action-save', 'error')
await expect(
page
.locator('.payload-toast-container li')
.filter({ hasText: 'The following fields are invalid (2):' }),
).toBeVisible()
await expect(
page.locator('.payload-toast-container [data-testid="field-errors"] li').nth(0),
).toHaveText('Lexical With Blocks')
await expect(
page.locator('.payload-toast-container [data-testid="field-errors"] li').nth(1),
).toHaveText('Lexical With Blocks → Group → Text Depends On Sibling Data')
await assertToastErrors({
page,
errors: [
'Lexical With Blocks',
'Lexical With Blocks → Group → Text Depends On Sibling Data',
],
})
await expect(page.locator('.payload-toast-container .payload-toast-item')).toBeHidden()
await trackNetworkRequests(
@@ -633,18 +622,10 @@ describe('lexicalBlocks', () => {
await blockTextField.fill('invalid')
await saveDocAndAssert(page, '#action-save', 'error')
await expect(
page
.locator('.payload-toast-container li')
.filter({ hasText: 'The following fields are invalid (2):' }),
).toBeVisible()
await expect(
page.locator('.payload-toast-container [data-testid="field-errors"] li').nth(0),
).toHaveText('Lexical With Blocks')
await expect(
page.locator('.payload-toast-container [data-testid="field-errors"] li').nth(1),
).toHaveText('Lexical With Blocks → Group → Text Depends On Block Data')
await assertToastErrors({
page,
errors: ['Lexical With Blocks', 'Lexical With Blocks → Group → Text Depends On Block Data'],
})
await expect(page.locator('.payload-toast-container .payload-toast-item')).toBeHidden()
await trackNetworkRequests(

View File

@@ -1,6 +1,7 @@
import type { Page } from '@playwright/test'
import { expect, test } from '@playwright/test'
import { addListFilter } from 'helpers/e2e/addListFilter.js'
import { openListFilters } from 'helpers/e2e/openListFilters.js'
import path from 'path'
import { wait } from 'payload/shared'
@@ -15,12 +16,12 @@ import {
saveDocAndAssert,
} from '../../../helpers.js'
import { AdminUrlUtil } from '../../../helpers/adminUrlUtil.js'
import { assertToastErrors } from '../../../helpers/assertToastErrors.js'
import { initPayloadE2ENoConfig } from '../../../helpers/initPayloadE2ENoConfig.js'
import { reInitializeDB } from '../../../helpers/reInitializeDB.js'
import { RESTClient } from '../../../helpers/rest.js'
import { TEST_TIMEOUT_LONG } from '../../../playwright.config.js'
import { numberDoc } from './shared.js'
import { addListFilter } from 'helpers/e2e/addListFilter.js'
const filename = fileURLToPath(import.meta.url)
const currentFolder = path.dirname(filename)
@@ -120,9 +121,10 @@ describe('Number', () => {
await page.keyboard.type(String(input))
await page.keyboard.press('Enter')
await page.click('#action-save', { delay: 100 })
await expect(page.locator('.payload-toast-container')).toContainText(
'The following field is invalid: With Min Rows',
)
await assertToastErrors({
page,
errors: ['With Min Rows'],
})
})
test('should keep data removed on save if deleted', async () => {

View File

@@ -20,6 +20,7 @@ import {
saveDocHotkeyAndAssert,
} from '../../../helpers.js'
import { AdminUrlUtil } from '../../../helpers/adminUrlUtil.js'
import { assertToastErrors } from '../../../helpers/assertToastErrors.js'
import { initPayloadE2ENoConfig } from '../../../helpers/initPayloadE2ENoConfig.js'
import { reInitializeDB } from '../../../helpers/reInitializeDB.js'
import { RESTClient } from '../../../helpers/rest.js'
@@ -448,8 +449,6 @@ describe('relationship', () => {
}),
).toBeVisible()
})
test.skip('has many', async () => {})
})
describe('should duplicate document within document drawer', () => {
@@ -509,8 +508,6 @@ describe('relationship', () => {
}),
).toBeVisible()
})
test.skip('has many', async () => {})
})
describe('should delete document within document drawer', () => {
@@ -569,8 +566,6 @@ describe('relationship', () => {
}),
).toBeHidden()
})
test.skip('has many', async () => {})
})
// TODO: Fix this. This test flakes due to react select
@@ -603,9 +598,10 @@ describe('relationship', () => {
.click()
await page.click('#action-save', { delay: 100 })
await expect(page.locator('.payload-toast-container')).toContainText(
'The following field is invalid: Relationship With Min Rows',
)
await assertToastErrors({
page,
errors: ['Relationship With Min Rows'],
})
})
test('should sort relationship options by sortOptions property (ID in ascending order)', async () => {

View File

@@ -0,0 +1,27 @@
import type { Page } from '@playwright/test'
import { expect } from '@playwright/test'
export async function assertToastErrors({
page,
errors,
}: {
errors: string[]
page: Page
}): Promise<void> {
const message =
errors.length === 1
? 'The following field is invalid:'
: `The following fields are invalid (${errors.length}):`
await expect(
page.locator('.payload-toast-container li').filter({ hasText: message }),
).toBeVisible()
for (let i = 0; i < errors.length; i++) {
const error = errors[i]
if (error) {
await expect(
page.locator('.payload-toast-container [data-testid="field-errors"] li').nth(i),
).toHaveText(error)
}
}
}

View File

@@ -16,6 +16,7 @@ import {
saveDocAndAssert,
} from '../helpers.js'
import { AdminUrlUtil } from '../helpers/adminUrlUtil.js'
import { assertToastErrors } from '../helpers/assertToastErrors.js'
import { initPayloadE2ENoConfig } from '../helpers/initPayloadE2ENoConfig.js'
import { reInitializeDB } from '../helpers/reInitializeDB.js'
import { RESTClient } from '../helpers/rest.js'
@@ -440,9 +441,10 @@ describe('Uploads', () => {
// save the document and expect an error
await page.locator('button#action-save').click()
await expect(page.locator('.payload-toast-container .toast-error')).toContainText(
'The following field is invalid: Audio',
)
await assertToastErrors({
page,
errors: ['Audio'],
})
})
test('should restrict uploads in drawer based on filterOptions', async () => {