Skip to content

Commit d15ab40

Browse files
authored
fix(ui): presentational fields skipped in form state when admin.condition is false (#17222)
### What Presentational fields (`row`,`collapsible`, or unnamed `group`) with `admin.condition` set to `false` were dropped entirely from form state. ### Why Regression introduced in #16819 (backport of #16780). It added a short circuit in `addFieldStatePromise` to skip rendering hidden fields. For a presentational field (which holds no value of its own), the early return fired **before** recursing into `field.fields`, so nested field state was never built. ### Fix Excludes presentational fields from the early return, letting them fall through to the `fieldHasSubFields` logic. This allows nested fields to preserve to their values and still skip rendering (they short circuit themselves). ### Tests Added to `test/form-state` NOTE: 3.x equivalent of this change [here](#17224).
1 parent 87e83b9 commit d15ab40

4 files changed

Lines changed: 149 additions & 4 deletions

File tree

packages/ui/src/forms/fieldSchemasToFormState/addFieldStatePromise.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,13 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
186186
fieldState.fieldSchema = field
187187
}
188188

189-
// Short-circuit to prevent hidden fields from recursing and rendering.
190-
// Note: `tab` is excluded bc tab visibility is keyed by `field.id` rather than `path`.
191-
// The tab branch below owns that write and the skip-recursion.
192-
if (passesCondition === false && field.type !== 'tab') {
189+
// Short-circuit hidden fields to prevent recursing and rendering. Two exclusions:
190+
// - `tab`: visibility is keyed by `field.id` (not `path`); the tab branch owns that write.
191+
// - presentational containers (row, collapsible, unnamed group): they hold no value, so
192+
// returning here drops their nested fields' values.
193+
const isPresentationalWithSubFields = fieldHasSubFields(field) && !fieldAffectsData(field)
194+
195+
if (passesCondition === false && field.type !== 'tab' && !isPresentationalWithSubFields) {
193196
if (fieldAffectsData(field) && data?.[field.name] !== undefined) {
194197
fieldState.value = data[field.name]
195198
fieldState.initialValue = data[field.name]
@@ -834,6 +837,13 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
834837
state[path] = {
835838
disableFormData: true,
836839
}
840+
841+
// Presentational containers are hidden client-side via `withCondition`, which reads
842+
// `passesCondition` from their own state entry. Must be set here since these fields
843+
// are excluded from the short-circuit above (which would otherwise carry the flag).
844+
if (passesCondition === false) {
845+
state[path].passesCondition = false
846+
}
837847
}
838848

839849
await iterateFields({

test/form-state/collections/Conditions/index.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,31 @@ export const ConditionsCollection: CollectionConfig = {
1919
},
2020
},
2121
},
22+
{
23+
type: 'row',
24+
admin: {
25+
condition: (data) => data?.showField === true,
26+
},
27+
fields: [
28+
{
29+
name: 'conditionalRowField',
30+
type: 'text',
31+
},
32+
],
33+
},
34+
{
35+
type: 'collapsible',
36+
label: 'Conditional Collapsible',
37+
admin: {
38+
condition: (data) => data?.showField === true,
39+
},
40+
fields: [
41+
{
42+
name: 'conditionalCollapsibleField',
43+
type: 'text',
44+
},
45+
],
46+
},
2247
],
2348
versions: false,
2449
}

test/form-state/int.spec.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,77 @@ describe('Form State', () => {
299299
await payload.delete({ collection: conditionsSlug, id: visibleDoc.id })
300300
})
301301

302+
it('should preserve values of fields nested inside a row hidden by admin.condition', async () => {
303+
const req = await createLocalReq({ user }, payload)
304+
305+
const hiddenDoc = await payload.create({
306+
collection: conditionsSlug,
307+
data: {
308+
showField: false,
309+
conditionalRowField: 'value in db',
310+
},
311+
})
312+
313+
const { state: stateHidden } = await buildFormState({
314+
mockRSCs: true,
315+
id: hiddenDoc.id,
316+
collectionSlug: conditionsSlug,
317+
data: hiddenDoc,
318+
docPermissions: undefined,
319+
docPreferences: {
320+
fields: {},
321+
},
322+
documentFormState: undefined,
323+
operation: 'update',
324+
renderAllFields: true,
325+
req,
326+
schemaPath: conditionsSlug,
327+
})
328+
329+
expect(stateHidden?.conditionalRowField).toBeDefined()
330+
expect(stateHidden?.conditionalRowField?.value).toBe('value in db')
331+
332+
// The row itself must still carry `passesCondition: false` so the client hides it via
333+
// `withCondition` (rather than rendering an empty, visible row).
334+
expect(stateHidden?.['_index-2']?.passesCondition).toBe(false)
335+
336+
await payload.delete({ collection: conditionsSlug, id: hiddenDoc.id })
337+
})
338+
339+
it('should preserve values of fields nested inside a collapsible hidden by admin.condition', async () => {
340+
const req = await createLocalReq({ user }, payload)
341+
342+
const hiddenDoc = await payload.create({
343+
collection: conditionsSlug,
344+
data: {
345+
showField: false,
346+
conditionalCollapsibleField: 'collapsible db value',
347+
},
348+
})
349+
350+
const { state: stateHidden } = await buildFormState({
351+
mockRSCs: true,
352+
id: hiddenDoc.id,
353+
collectionSlug: conditionsSlug,
354+
data: hiddenDoc,
355+
docPermissions: undefined,
356+
docPreferences: {
357+
fields: {},
358+
},
359+
documentFormState: undefined,
360+
operation: 'update',
361+
renderAllFields: true,
362+
req,
363+
schemaPath: conditionsSlug,
364+
})
365+
366+
// Same regression class as `row`: a collapsible is a presentational container, so its
367+
// nested field's value must survive even though the collapsible is hidden.
368+
expect(stateHidden?.conditionalCollapsibleField?.value).toBe('collapsible db value')
369+
370+
await payload.delete({ collection: conditionsSlug, id: hiddenDoc.id })
371+
})
372+
302373
it('should render custom Field component when admin.condition flips from false to true via onChange', async () => {
303374
const req = await createLocalReq({ user }, payload)
304375

test/form-state/payload-types.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,8 @@ export interface Config {
9696
locale: null;
9797
widgets: {
9898
collections: CollectionsWidget;
99+
'collection-query': CollectionQueryWidget;
100+
activity: ActivityWidget;
99101
};
100102
user: User;
101103
jobs: {
@@ -202,6 +204,8 @@ export interface Condition {
202204
id: string;
203205
showField?: boolean | null;
204206
conditionalCustomField?: string | null;
207+
conditionalRowField?: string | null;
208+
conditionalCollapsibleField?: string | null;
205209
updatedAt: string;
206210
createdAt: string;
207211
}
@@ -383,6 +387,8 @@ export interface AutosavePostsSelect<T extends boolean = true> {
383387
export interface ConditionsSelect<T extends boolean = true> {
384388
showField?: T;
385389
conditionalCustomField?: T;
390+
conditionalRowField?: T;
391+
conditionalCollapsibleField?: T;
386392
updatedAt?: T;
387393
createdAt?: T;
388394
}
@@ -458,6 +464,39 @@ export interface CollectionsWidget {
458464
};
459465
width: 'full';
460466
}
467+
/**
468+
* This interface was referenced by `Config`'s JSON-Schema
469+
* via the `definition` "collection-query_widget".
470+
*/
471+
export interface CollectionQueryWidget {
472+
data?: {
473+
title?: string | null;
474+
relatedCollection: 'posts' | 'autosave-posts' | 'conditions' | 'users';
475+
where?:
476+
| {
477+
[k: string]: unknown;
478+
}
479+
| unknown[]
480+
| string
481+
| number
482+
| boolean
483+
| null;
484+
sortField?: string | null;
485+
sortDirection?: ('asc' | 'desc') | null;
486+
limit?: number | null;
487+
};
488+
width: 'x-small' | 'small' | 'medium' | 'large' | 'x-large' | 'full';
489+
}
490+
/**
491+
* This interface was referenced by `Config`'s JSON-Schema
492+
* via the `definition` "activity_widget".
493+
*/
494+
export interface ActivityWidget {
495+
data?: {
496+
excludedCollections?: ('posts' | 'autosave-posts' | 'conditions' | 'users')[] | null;
497+
};
498+
width: 'x-small' | 'small' | 'medium' | 'large' | 'x-large' | 'full';
499+
}
461500
/**
462501
* This interface was referenced by `Config`'s JSON-Schema
463502
* via the `definition` "auth".

0 commit comments

Comments
 (0)