Skip to content

Commit 2903486

Browse files
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 #9388. Adds e2e test.
1 parent b965db8 commit 2903486

File tree

4 files changed

+55
-0
lines changed

4 files changed

+55
-0
lines changed

packages/ui/src/forms/Form/mergeServerFormState.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,14 @@ export const mergeServerFormState = ({
4040
...incomingField,
4141
}
4242

43+
if (
44+
currentState[path] &&
45+
'errorPaths' in currentState[path] &&
46+
!('errorPaths' in incomingField)
47+
) {
48+
newState[path].errorPaths = []
49+
}
50+
4351
/**
4452
* Intelligently merge the rows array to ensure changes to local state are not lost while the request was pending
4553
* For example, the server response could come back with a row which has been deleted on the client

test/field-error-states/e2e.spec.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ describe('Field Error States', () => {
2323
let validateDraftsOnAutosave: AdminUrlUtil
2424
let prevValue: AdminUrlUtil
2525
let prevValueRelation: AdminUrlUtil
26+
let errorFieldsURL: AdminUrlUtil
2627

2728
beforeAll(async ({ browser }, testInfo) => {
2829
testInfo.setTimeout(TEST_TIMEOUT_LONG)
@@ -32,6 +33,7 @@ describe('Field Error States', () => {
3233
validateDraftsOnAutosave = new AdminUrlUtil(serverURL, collectionSlugs.validateDraftsOnAutosave)
3334
prevValue = new AdminUrlUtil(serverURL, collectionSlugs.prevValue)
3435
prevValueRelation = new AdminUrlUtil(serverURL, collectionSlugs.prevValueRelation)
36+
errorFieldsURL = new AdminUrlUtil(serverURL, collectionSlugs.errorFields)
3537
const context = await browser.newContext()
3638
page = await context.newPage()
3739
initPageConsoleErrorCatch(page)
@@ -124,4 +126,46 @@ describe('Field Error States', () => {
124126
await expect(page.locator('#field-title')).toHaveValue('original value 2')
125127
})
126128
})
129+
130+
describe('error field types', () => {
131+
async function prefillBaseRequiredFields() {
132+
const homeTabLocator = page.locator('.tabs-field__tab-button', {
133+
hasText: 'Home',
134+
})
135+
const heroTabLocator = page.locator('.tabs-field__tab-button', {
136+
hasText: 'Hero',
137+
})
138+
139+
await homeTabLocator.click()
140+
// fill out all required fields in the home tab
141+
await page.locator('#field-home__text').fill('Home Collapsible Text')
142+
await page.locator('#field-home__tabText').fill('Home Tab Text')
143+
144+
await page.locator('#field-group__text').fill('Home Group Text')
145+
await heroTabLocator.click()
146+
// fill out all required fields in the hero tab
147+
await page.locator('#field-tabText').fill('Hero Tab Text')
148+
await page.locator('#field-text').fill('Hero Tab Collapsible Text')
149+
}
150+
test('group errors', async () => {
151+
await page.goto(errorFieldsURL.create)
152+
await prefillBaseRequiredFields()
153+
154+
// clear group and save
155+
await page.locator('#field-group__text').fill('')
156+
await saveDocAndAssert(page, '#action-save', 'error')
157+
158+
// should show the error pill and count
159+
const groupFieldErrorPill = page.locator('#field-group .group-field__header .error-pill', {
160+
hasText: '1 error',
161+
})
162+
await expect(groupFieldErrorPill).toBeVisible()
163+
164+
// finish filling out the group
165+
await page.locator('#field-group__text').fill('filled out')
166+
167+
await expect(page.locator('#field-group .group-field__header .error-pill')).toBeHidden()
168+
await saveDocAndAssert(page, '#action-save')
169+
})
170+
})
127171
})

test/field-error-states/shared.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export const collectionSlugs: {
88
validateDraftsOnAutosave: 'validate-drafts-on-autosave',
99
prevValue: 'prev-value',
1010
prevValueRelation: 'prev-value-relation',
11+
errorFields: 'error-fields',
1112
}
1213

1314
export const globalSlugs: {

test/form-state/int.spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,7 @@ describe('Form State', () => {
309309
it('should merge array rows without losing rows added to local state', () => {
310310
const currentState: FormState = {
311311
array: {
312+
errorPaths: [],
312313
rows: [
313314
{
314315
id: '1',
@@ -358,6 +359,7 @@ describe('Form State', () => {
358359
// Row 2 should still exist
359360
expect(newState).toStrictEqual({
360361
array: {
362+
errorPaths: [],
361363
passesCondition: true,
362364
valid: true,
363365
rows: [

0 commit comments

Comments
 (0)