diff --git a/.changeset/6111-formsection-visiblewhen.md b/.changeset/6111-formsection-visiblewhen.md new file mode 100644 index 0000000000..d17c7fa6e8 --- /dev/null +++ b/.changeset/6111-formsection-visiblewhen.md @@ -0,0 +1,49 @@ +--- +'@object-ui/plugin-form': minor +'@object-ui/types': minor +--- + +⚠️ **Behaviour change: an authored `FormSection.visibleWhen` that has been doing nothing +will now START HIDING SECTIONS.** Read this before upgrading if any of your metadata +authors a section predicate. + +`@objectstack/spec` declares `FormSection.visibleWhen` and this repo's spec bridge maps it +through, but every plugin-form layout renders a section header as a virtual +`section-divider` pseudo-field and none of them copied the predicate onto it. On the +object-view chain — the create/edit modal, the drawer, the split form, and the full-page +record form — the key was declared, mapped, carried, and then dropped one hop before +anything could evaluate it. The section rendered unconditionally, with no diagnostic +(objectui#6111). + +**Why nobody noticed, and why the fix is felt as a regression.** `visibleWhen` fails OPEN: +a section that renders is what you get when the predicate resolves TRUE, when the predicate +never arrives, *and* when the predicate faults. Those three worlds were indistinguishable, +so an app that authored a section predicate saw its section render and had no way to tell +that the rule was inert. Every such app has been running with the rule switched off, and +some will have been authored — or simply grown used to — that state. After this change the +predicate is evaluated for real, and sections that have always been visible will disappear +for the users the rule excludes. + +This is the intended ADR-0089 contract being delivered, not a new capability: the key was +already declared, already documented, and already honoured by the console form renderer. +The object-view chain was the one that silently ignored it. + +**Before upgrading**, audit any `sections[].visibleWhen` in your form-view metadata and +confirm each predicate says what you actually want, evaluated against `record` + +`current_user`. A predicate that was written speculatively, or left behind after a rework, +now takes effect. + +**Measured scope of the hide.** The predicate gates the section's HEADER row. The renderer +treats `section-divider` as presentational and holds no association between it and the +fields that follow it, so a false predicate removes the heading and the section's fields +keep rendering. The console renderer (`apps/console`) drops the whole `
`, fields +included. That divergence is real, is pinned honestly by this change's tests rather than +implied away, and is filed separately — it needs a renderer-side grouping contract, not +another line in a layout. + +Two hops were dropping the key and both are repaired: `ObjectForm` rebuilds each section +key by key when it delegates to Split/Drawer/Modal (and `ModalForm`'s own `groups` map does +it again), so a key those maps did not copy never reached the layout at all; and the six +`section-divider` synthesis sites across the four layout files. + +`@object-ui/types` gains the matching `ObjectFormSection.visibleWhen` declaration. diff --git a/packages/plugin-form/src/DrawerForm.tsx b/packages/plugin-form/src/DrawerForm.tsx index a1d60e4e5f..79f20f74c3 100644 --- a/packages/plugin-form/src/DrawerForm.tsx +++ b/packages/plugin-form/src/DrawerForm.tsx @@ -90,6 +90,12 @@ export interface DrawerFormSectionConfig { fields: (string | FormField)[]; collapsible?: boolean; collapsed?: boolean; + /** + * ADR-0089 `FormSection.visibleWhen` — conditional visibility for the + * section's divider HEADER, evaluated by the form renderer with the canonical + * engine and the host predicate scope (#6010/#6111). Fails OPEN. + */ + visibleWhen?: string | { dialect?: string; source: string }; /** Custom CSS class for the section's divider header. */ className?: string; } @@ -547,6 +553,9 @@ export const DrawerForm: React.FC = ({ name: `__section_${sectionKey}`, label: section.label || '', type: 'section-divider', + // ADR-0089 section predicate (#6111) — the renderer evaluates it on + // this pseudo-field with the host predicate scope bound (#6010). + visibleWhen: (section as any).visibleWhen, colSpan: 4, collapsible: section.collapsible, collapsed: isCollapsed, @@ -603,6 +612,8 @@ export const DrawerForm: React.FC = ({ name: `__section_${sectionKey}`, label: title, type: 'section-divider', + // ADR-0089 section predicate (#6111). + visibleWhen: (section as any).visibleWhen, colSpan: 4, collapsible: section.collapsible, collapsed: isCollapsed, diff --git a/packages/plugin-form/src/ModalForm.tsx b/packages/plugin-form/src/ModalForm.tsx index 20d4415465..d8d7999a84 100644 --- a/packages/plugin-form/src/ModalForm.tsx +++ b/packages/plugin-form/src/ModalForm.tsx @@ -78,6 +78,12 @@ export interface ModalFormSectionConfig { description?: string; columns?: 1 | 2 | 3 | 4; fields: (string | FormField)[]; + /** + * ADR-0089 `FormSection.visibleWhen` — conditional visibility for the + * section's divider HEADER, evaluated by the form renderer with the canonical + * engine and the host predicate scope (#6010/#6111). Fails OPEN. + */ + visibleWhen?: string | { dialect?: string; source: string }; /** Custom CSS class for the section's header row (stacked layout). */ className?: string; /** @@ -616,6 +622,9 @@ export const ModalForm: React.FC = ({ key: sectionKey(section, index), title: sectionTitle(section), description: section.description, + // Key-by-key rebuild: an uncopied key never reaches the divider + // synthesis below (#6111). + visibleWhen: section.visibleWhen, className: section.className, gridClassName: section.gridClassName, fields: formColumns > 1 @@ -665,6 +674,9 @@ export const ModalForm: React.FC = ({ label: g.title, description: g.description, type: 'section-divider', + // ADR-0089 section predicate (#6111) — the renderer evaluates it on + // this pseudo-field with the host predicate scope bound (#6010). + visibleWhen: g.visibleWhen, colSpan: 4, className: g.className, } as any); @@ -695,6 +707,8 @@ export const ModalForm: React.FC = ({ name: `__section_${section.name || index}`, label: title, type: 'section-divider', + // ADR-0089 section predicate (#6111). + visibleWhen: (section as any).visibleWhen, } as any); } allFields.push(...(columns > 1 ? applyAutoColSpan(body, columns) : body)); diff --git a/packages/plugin-form/src/ObjectForm.tsx b/packages/plugin-form/src/ObjectForm.tsx index ee9d2a8d9f..0b4c170f32 100644 --- a/packages/plugin-form/src/ObjectForm.tsx +++ b/packages/plugin-form/src/ObjectForm.tsx @@ -300,6 +300,9 @@ export const ObjectForm: React.FC = ({ // rebuilds each section key by key, so a key it doesn't copy is // silently dropped — exactly how `visibleOn` once vanished here. pane: s.pane, + // ADR-0089 section predicate (#6111) — same reason as `pane` above: + // a key this map does not copy never reaches the layout at all. + visibleWhen: (s as any).visibleWhen, className: (s as any).className, gridClassName: (s as any).gridClassName, })), @@ -330,6 +333,9 @@ export const ObjectForm: React.FC = ({ fields: s.fields, collapsible: (s as any).collapsible, collapsed: (s as any).collapsed, + // ADR-0089 section predicate (#6111) — key-by-key rebuild, so an + // uncopied key is silently dropped before DrawerForm ever sees it. + visibleWhen: (s as any).visibleWhen, className: (s as any).className, })), open: schema.open, @@ -358,6 +364,9 @@ export const ObjectForm: React.FC = ({ description: s.description, columns: s.columns, fields: s.fields, + // ADR-0089 section predicate (#6111) — key-by-key rebuild, so an + // uncopied key is silently dropped before ModalForm ever sees it. + visibleWhen: (s as any).visibleWhen, className: (s as any).className, gridClassName: (s as any).gridClassName, })), @@ -1195,6 +1204,11 @@ const SimpleObjectForm: React.FC = ({ name: `__section_${sectionKey}`, label, type: 'section-divider', + // ADR-0089 `FormSection.visibleWhen` (#6111). The renderer evaluates + // a `visibleWhen` on this pseudo-field with the host predicate scope + // bound (#6010), so copying it here is what makes the authored + // section predicate reach an evaluator at all. + visibleWhen: (section as any).visibleWhen, colSpan: 4, collapsible: section.collapsible, collapsed: isCollapsed, diff --git a/packages/plugin-form/src/SplitForm.tsx b/packages/plugin-form/src/SplitForm.tsx index 4b3ade1ffa..393416d6d5 100644 --- a/packages/plugin-form/src/SplitForm.tsx +++ b/packages/plugin-form/src/SplitForm.tsx @@ -46,6 +46,12 @@ export interface SplitFormSectionConfig { */ pane?: 'primary' | 'secondary'; fields: (string | FormField)[]; + /** + * ADR-0089 `FormSection.visibleWhen` — conditional visibility for the + * section's divider HEADER, evaluated by the form renderer with the canonical + * engine and the host predicate scope (#6010/#6111). Fails OPEN. + */ + visibleWhen?: string | { dialect?: string; source: string }; /** Custom CSS class for the section's header row. */ className?: string; /** @@ -339,6 +345,9 @@ export const SplitForm: React.FC = ({ label: section.label, description: section.description, type: 'section-divider', + // ADR-0089 section predicate (#6111) — the renderer evaluates it on + // this pseudo-field with the host predicate scope bound (#6010). + visibleWhen: section.visibleWhen, colSpan: 4, className: section.className, } as any); diff --git a/packages/plugin-form/src/__tests__/sectionVisibleWhen-6111.test.tsx b/packages/plugin-form/src/__tests__/sectionVisibleWhen-6111.test.tsx new file mode 100644 index 0000000000..562ca0cd62 --- /dev/null +++ b/packages/plugin-form/src/__tests__/sectionVisibleWhen-6111.test.tsx @@ -0,0 +1,292 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#6111 — an authored `FormSection.visibleWhen` must REACH an evaluator + * on the object-view chain, in every plugin-form layout. + * + * `@objectstack/spec` declares `FormSection.visibleWhen`; this repo's spec + * bridge carries it (`packages/react/src/spec-bridge/bridges/form-view.ts:250`); + * `RecordFormPage` / `resolveFormViewLayout` wire whole `sections` objects into + * the layouts. Every layout then renders a section header as a virtual + * `section-divider` pseudo-field — and none of them copied the predicate onto + * it, so the key was declared, mapped, carried, and dropped one hop before + * anything evaluated it. + * + * The renderer half already works: `packages/components/src/renderers/form/ + * form.tsx` runs EVERY pseudo-field through `resolveFieldRuleState` with the + * host predicate scope bound (#6010) before it reaches the `section-divider` + * branch, so a divider carrying a `visibleWhen` hides exactly like a field + * does. `predicate-scope-parity-6010.test.tsx`'s `sectionSurface` row pins that + * directly, hand-authoring the pseudo-field this file makes the layouts emit. + * + * ## ⚠️ Why every case below asserts HIDDEN, and never merely SHOWN + * + * `visibleWhen` fails OPEN. A section that renders is the outcome of three + * different worlds — the predicate resolved TRUE, the predicate never arrived, + * and the predicate faulted — so **an assertion that a section IS shown + * distinguishes none of them** and is green on unfixed code. The deliverable is + * therefore the DENIED row: the heading is ABSENT while a predicate that + * resolves false is authored on the section. + * + * The ALLOWED row is the control and nothing more. Without it, "hidden" is + * satisfiable by a layout that dropped the heading for some unrelated reason + * (an empty section, a bad label lookup), which would be a worse defect and + * completely invisible to the DENIED row alone. + * + * ## Why one parameterised case would NOT have been enough + * + * There are SIX `section-divider` synthesis sites across FOUR layout files, and + * a fix applied to five of six still passes a suite that exercises five. Each + * layout is therefore mounted BY NAME below, through the entry the product + * actually uses. + * + * Two hops had to be repaired per layout, and only the second was on the card: + * + * 1. `ObjectForm` rebuilds each section KEY BY KEY when it delegates to + * Split/Drawer/Modal (`ObjectForm.tsx` ~296/~331/~388) — a key that map + * does not copy never reaches the layout at all. `ModalForm`'s own + * `groups` map does the same thing again. The file's own comment on the + * split map already recorded the hazard: *"this mapping rebuilds each + * section key by key, so a key it doesn't copy is silently dropped — + * exactly how `visibleOn` once vanished here."* + * 2. The `section-divider` synthesis sites themselves. + * + * Routing the modal/drawer/split rows through `ObjectForm` (which is what + * `RecordFormPage` does) exercises BOTH hops; the direct `ModalForm` row covers + * `resolveFormViewLayout`, which mounts `ModalForm` without passing through + * `ObjectForm` at all. + * + * ## Reverse verification (direction predicted BEFORE running) + * + * Revert the `visibleWhen:` copy at any ONE synthesis site and that layout's + * DENIED row — and only that one — goes red in the SHOWN direction (the heading + * comes back), because the fallback is fail-open. Every ALLOWED row stays green + * everywhere, which is precisely why the ALLOWED rows could never have caught + * this. + * + * ## Scope, measured — what this does NOT claim + * + * The renderer treats `section-divider` as a presentational ROW and holds no + * association between it and the fields that follow it, so a false predicate + * removes the HEADING and leaves the section's fields rendering. The console + * renderer (`apps/console/src/components/FormPage.tsx:1819`) drops the whole + * `
`, fields included. That divergence is real and is filed + * separately — it needs a renderer-side grouping contract, not another line in + * a layout. The `stillRendersItsFields` case below pins the CURRENT behaviour + * honestly rather than letting the file imply a guarantee it does not deliver. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor } from '@testing-library/react'; +import React from 'react'; +import { PredicateScopeProvider } from '@object-ui/react'; +import { registerAllFields } from '@object-ui/fields'; +import { ObjectForm } from '../ObjectForm'; +import { ModalForm } from '../ModalForm'; + +registerAllFields(); + +/** + * The canonical wire shape — `@objectstack/spec` normalizes an authored + * predicate into a `{ dialect: 'cel' }` envelope at parse (ADR-0089 D2), and a + * bare string would route to a different engine on some surfaces (measured in + * `predicate-scope-parity-6010.test.tsx`). Same spelling as that file, on + * purpose: one authored text, one verdict, every surface. + */ +const cel = (source: string) => ({ dialect: 'cel', source }); + +/** THE authored section predicate. One text, asked of every layout below. */ +const GATE = cel("'sales_manager' in current_user.positions"); + +/** The host scope `ExpressionProvider` mounts, transcribed (see #6010's pin). */ +function hostScope(positions: string[]) { + const user = { id: 'u1', name: 'Kim', positions }; + return { current_user: user, user, ctx: { user }, os: { user }, app: {}, data: {}, features: {} }; +} + +const DENIED = hostScope(['sales']); +const ALLOWED = hostScope(['sales_manager']); + +const objectSchema = { + name: 'crm_case', + fields: { + subject: { type: 'text', label: 'Subject' }, + salary: { type: 'text', label: 'Salary' }, + }, +}; + +let dataSource: any; + +beforeEach(() => { + dataSource = { + getObjectSchema: vi.fn().mockResolvedValue(objectSchema), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + }; +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +/** + * Two sections: `Always` carries no predicate and is the paired control that + * proves the form rendered AT ALL — so a missing `Compensation` heading is a + * verdict and not an inability. `Compensation` carries the gate. + */ +const sections = () => [ + { name: 'always', label: 'Always', fields: ['subject'] }, + { name: 'pay', label: 'Compensation', visibleWhen: GATE, fields: ['salary'] }, +]; + +/** Mount through `ObjectForm` — the entry `RecordFormPage` itself uses. */ +const renderObjectForm = async ( + scope: Record, + extra: Record, +) => { + render( + + + , + ); + // The un-gated sibling heading is the readiness signal AND the control. + await waitFor(() => expect(screen.getByText('Always')).toBeTruthy()); +}; + +/** Mount `ModalForm` directly — the shape `resolveFormViewLayout` produces. */ +const renderModalFormDirect = async (scope: Record) => { + render( + + + , + ); + await waitFor(() => expect(screen.getByText('Always')).toBeTruthy()); +}; + +/** The gated section's heading — `null` when the layout hid it. */ +const gatedHeading = () => screen.queryByText('Compensation'); + +/** + * Every layout, by name, with the mount that reaches its own synthesis site. + * Named individually because a fix applied to five of six sites still passes a + * suite that exercises five. + */ +const LAYOUTS: { label: string; mount: (scope: Record) => Promise }[] = [ + { + label: 'ObjectForm — stacked `simple` sections (ObjectForm.tsx section-divider)', + mount: (scope) => renderObjectForm(scope, { formType: 'simple' }), + }, + { + label: 'ModalForm — via ObjectForm delegation (key-by-key remap + ModalForm groups map)', + mount: (scope) => renderObjectForm(scope, { formType: 'modal', open: true }), + }, + { + label: 'ModalForm — mounted directly (the resolveFormViewLayout shape)', + mount: (scope) => renderModalFormDirect(scope), + }, + { + label: 'DrawerForm — via ObjectForm delegation (key-by-key remap + explicit-sections divider)', + mount: (scope) => renderObjectForm(scope, { formType: 'drawer', open: true }), + }, + { + label: 'SplitForm — via ObjectForm delegation (key-by-key remap + paneFields divider)', + mount: (scope) => renderObjectForm(scope, { formType: 'split' }), + }, +]; + +describe('#6111 — an authored section `visibleWhen` reaches an evaluator in every layout', () => { + describe('DENIED — the predicate resolves FALSE, so the section heading is HIDDEN', () => { + // ⚠️ THE deliverable. Green here is the only observation that separates + // "the predicate arrived and was evaluated" from "it never arrived" — + // every other row in this file is green on unfixed code. + for (const layout of LAYOUTS) { + it(layout.label, async () => { + await layout.mount(DENIED); + expect(gatedHeading()).toBeNull(); + }); + } + }); + + describe('ALLOWED — the SAME predicate text, a user it admits ⇒ still SHOWN', () => { + // The control. Without it, "hidden" is satisfied by a layout that dropped + // the heading for an unrelated reason — a worse defect, invisible above. + for (const layout of LAYOUTS) { + it(layout.label, async () => { + await layout.mount(ALLOWED); + expect(gatedHeading()).not.toBeNull(); + }); + } + }); + + describe('FAULTED — a genuinely unbound root still fails OPEN (unchanged)', () => { + // Pinned so the DENIED rows above mean "evaluated and false" rather than + // "could not be evaluated at all". Asserted against the DENIED user on + // purpose: the only difference from the DENIED block is the ROOT the + // predicate names, so a fail-CLOSED regression cannot hide behind the + // membership test. + for (const layout of LAYOUTS) { + it(layout.label, async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); + const unbound = cel("'sales_manager' in no_such_root.positions"); + render( + + + , + ); + await waitFor(() => expect(screen.getByText('Always')).toBeTruthy()); + expect(gatedHeading()).not.toBeNull(); + }); + } + }); + + it('measured scope: a hidden section still renders its FIELDS (objectui#6111 follow-up)', async () => { + // NOT an endorsement — an honest pin of what this change does and does not + // deliver. `section-divider` is a presentational ROW; the renderer holds no + // association between it and the fields after it, so the heading goes and + // the fields stay. The console renderer drops the whole `
`. + // Reconciling the two needs a renderer-side grouping contract and is filed + // separately; this assertion turning red is the SIGNAL that it landed. + await renderObjectForm(DENIED, { formType: 'simple' }); + expect(gatedHeading()).toBeNull(); + expect(screen.getByLabelText(/salary/i)).toBeTruthy(); + }); +}); diff --git a/packages/types/src/objectql.ts b/packages/types/src/objectql.ts index 39d95a26b4..064ce2e41b 100644 --- a/packages/types/src/objectql.ts +++ b/packages/types/src/objectql.ts @@ -940,6 +940,22 @@ export interface ObjectFormSection { */ fields: (string | FormField)[]; + /** + * Conditional visibility for the SECTION HEADER, as an authored predicate. + * Aligns with @objectstack/spec FormSection.visibleWhen (ADR-0089) — the same + * canonical `@object-ui/core` engine and record scope every other + * `visibleWhen` surface uses, so one authored predicate text means one thing + * everywhere (#6010). A broken predicate fails OPEN (the header renders). + * + * ⚠️ Scope, measured: this gates the section's `section-divider` HEADER row. + * The renderer treats that row as presentational and holds no association + * between it and the fields that follow, so a false predicate removes the + * heading and leaves its fields rendering (objectui#6111). The console + * renderer drops the whole `
`; reconciling the two is filed + * separately. + */ + visibleWhen?: string | { dialect?: string; source: string }; + /** * Custom CSS class for the section's wrapper (Card, when the form variant * renders sections as cards; the divider header, for the flat/simple path).