diff --git a/.changeset/8069-visiblewhen-fault-refuses-submit.md b/.changeset/8069-visiblewhen-fault-refuses-submit.md new file mode 100644 index 0000000000..a23cf50015 --- /dev/null +++ b/.changeset/8069-visiblewhen-fault-refuses-submit.md @@ -0,0 +1,56 @@ +--- +'@object-ui/types': minor +'@object-ui/core': minor +'@object-ui/i18n': minor +'@object-ui/components': patch +'@object-ui/plugin-form': patch +'@object-ui/console': patch +--- + +fix(types,components,plugin-form,console,core): a faulted `visibleWhen` refuses the submit, naming the field and the rule; a blank field rule is refused; blank gates are diagnosed (objectui#8069) + +⚠️ **User-visible, and a narrowing.** A form whose field `visibleWhen` cannot be +evaluated — a typo in a column name, a syntax error, an unbound root — used to +render the field (fail-open) and submit as if the rule had said "show". It now +still renders the field, and **refuses the submit** with a message that names +the field and the rule (`form.visibleWhenFaulted`). This is ADR-0137 D2 as +ruled for objectui#8069 (Q1 = B, one judge per rule): no server evaluates a +field's `visibleWhen`, so its fail-open render direction (D3) was a silent grant +— a field the working rule would have hidden, drawn, edited and written. The +refusal applies on the record form renderer (`form.tsx`, every `ObjectForm` +layout), the console's `/forms/:name` and `/f/:slug` page, and the wizard's +cross-step gate at final submit. + +- `requiredWhen` / `readonlyWhen` are **unchanged on the client**: the server + evaluates both and refuses a faulted one itself (ADR-0137 D2), and the form + renderer shows that field-attributed refusal beside the input. +- **Accepted residuals:** a `visibleWhen` reading `previous` cannot be + evaluated on a CREATE form, so such a form is refused on every submit; and + the wizard's cross-step gate binds no `previous` in either mode, so the same + rule is refused at the final submit of an EDIT wizard too. +- A **blank** field rule (`''`, whitespace, an envelope whose `source` is + blank) is a fault, not "no rule" (ADR-0137 D2). A STORED blank `visibleWhen` + is refused at submit on the same three paths; a stored blank `requiredWhen` / + `readonlyWhen` is the server's to refuse. + +⚠️ **Narrowing (`@object-ui/types`): `FormFieldSchema` refuses a blank field +rule at parse.** `visibleWhen`, `readonlyWhen` and `requiredWhen` on a form +field now refuse a predicate that is blank after trimming, with the spec's own +sentence (`EVALUATED_EXPRESSION_SOURCE_REQUIRED`) — ADR-0137 D1, the same +refusal `@objectstack/spec` makes on `FieldSchema`. The accepted SHAPE is +unchanged (a string or `{ dialect?, source }`, `ExpressionWireSchema`'s own +arms): only the blank value is taken out. A blank GATE — `BaseSchema`'s +`visible` / `hidden` / `disabled`, a form field's view-level `visibleOn`, an +option's `visibleWhen` — still parses and is still read as "no gate". + +**Added (`@object-ui/core`):** `resolveFieldRuleState` returns `faults` beside +its three verdicts — the per-rule fault report the submit paths read, filled +from the same evaluation — and its type, `FieldRuleFaults`, is exported. +**Added (`@object-ui/i18n`):** the `form.visibleWhenFaulted` key in all ten +packs. + +**Diagnosed, no verdict changed (ADR-0137 D4):** a blank CEL gate reaching +`ExpressionEvaluator.evaluateCondition`, and a blank gate folded to "no gate" by +`hasDeclaredPredicate`, now each report once through the same `[blank]` channel +field-rule faults use. Both verdicts (objectui#3850 / #3960) are unchanged, +`throwOnError` included. diff --git a/apps/console/src/components/FormPage.conditionalRules.test.tsx b/apps/console/src/components/FormPage.conditionalRules.test.tsx index 34c0d29d6b..a284f8b365 100644 --- a/apps/console/src/components/FormPage.conditionalRules.test.tsx +++ b/apps/console/src/components/FormPage.conditionalRules.test.tsx @@ -685,6 +685,8 @@ describe('objectui#5627 — the keys reach the rows, in their own slots', () => visible: true, readonly: false, required: false, + // No rule, no fault — the report the submit check reads (objectui#8069). + faults: {}, }); expect(resolveRowState(byName.viewHidden, values, null, isCreate).visible).toBe(false); expect(resolveRowState(byName.objectHidden, values, null, isCreate).visible).toBe(false); diff --git a/apps/console/src/components/FormPage.sharedWidgets-10179.test.tsx b/apps/console/src/components/FormPage.sharedWidgets-10179.test.tsx index db75007d21..3b406f6584 100644 --- a/apps/console/src/components/FormPage.sharedWidgets-10179.test.tsx +++ b/apps/console/src/components/FormPage.sharedWidgets-10179.test.tsx @@ -30,7 +30,7 @@ * * The shared widgets announce required on the state channel and never arm the * native attribute the hand-rolled controls carried, so the page refuses a - * submit that leaves a required row empty itself (`findMissingRequired`). The + * submit that leaves a required row empty itself (`findSubmitRefusals`). The * refusal is pinned with its control: the same form, filled, submits. */ diff --git a/apps/console/src/components/FormPage.tsx b/apps/console/src/components/FormPage.tsx index f4561d34db..efd897f790 100644 --- a/apps/console/src/components/FormPage.tsx +++ b/apps/console/src/components/FormPage.tsx @@ -128,6 +128,7 @@ import { isRuntimeDefault, isServerOwnedValue, resolveFieldRuleState, + type FieldRuleFaults, type FieldRulePredicate, } from '@object-ui/core'; import { omitServerResolvedDefaults, resolveSectionGroupReferences } from '@object-ui/plugin-form'; @@ -994,7 +995,7 @@ export function resolveRowState( previous: Record | null | undefined, isCreateForm: boolean, predicateScope?: Record, -): { visible: boolean; readonly: boolean; required: boolean } { +): { visible: boolean; readonly: boolean; required: boolean; faults: FieldRuleFaults } { const ruleState = resolveFieldRuleState( field.rules ?? {}, values, @@ -1018,6 +1019,10 @@ export function resolveRowState( visible: ruleState.visible && viewVisible, readonly: ruleState.readonly, required: ruleState.required, + // The OBJECT-level rules' fault report, passed through untouched for the + // submit check (objectui#8069). The view-level predicate has none: it is + // a layout gate, evaluated by `isFieldVisible`, and not a field rule. + faults: ruleState.faults, }; } @@ -1783,7 +1788,7 @@ function needsDependentValues(widgetKey: string): boolean { * attribute. The widget contract refuses a `required` boolean by name so the * asterisk keeps one author, and arming the browser's constraint bubble * would put a second validator beside this page's own submit check (see - * {@link findMissingRequired}); + * {@link findSubmitRefusals}); * - readonly as `disabled`: the control stays on screen, labelled and inert, * which is what this page's rows have always done. A widget's `readonly` * branch renders a replacement display that drops the host id, and the @@ -1846,8 +1851,31 @@ function FieldInput({ field, state, value, onChange, values, onUploadingChange } } /** - * The rows that would submit EMPTY while their effective verdict says required - * — the page's own required check (objectui#10179). + * What this page's own submit check refuses, read off ONE {@link resolveRowState} + * call per row — so the refusal is decided by the evaluation that drew the row, + * never by a second one. + * + * ## `faultedVisibleWhen` — the rule no server judges (objectui#8069) + * + * The rows whose OBJECT-level `visibleWhen` could not be evaluated. ADR-0137 + * D2, as ruled for objectui#8069 (Q1 = B, one judge per rule): the client + * refuses the submit on this one rule, because no server evaluates it and its + * fail-open render direction (D3) would otherwise be a silent grant. + * `requiredWhen` / `readonlyWhen` faults are the server's to refuse; this page + * keeps their render direction and warning unchanged. A stored BLANK + * `visibleWhen` is a fault like any other (ADR-0137 D2, `FieldRuleFaults`); the + * view-level predicate is not a field rule at all but a layout gate, and is + * not judged. + * + * Judged over the rows the required check below walks — a hidden SECTION is + * skipped by both. On this page visibility decides what is DRAWN and nothing + * else (a hidden row's value still submits, unchanged), so a row a hidden + * section already keeps off screen cannot be shown by its own broken rule, and + * there is no grant to refuse. + * + * ## `missing` — the rows that would submit EMPTY while required + * + * The page's own required check (objectui#10179). * * The hand-rolled controls this page used to render carried the NATIVE * `required` attribute, so the browser refused such a submit on the text-like @@ -1859,24 +1887,26 @@ function FieldInput({ field, state, value, onChange, values, onUploadingChange } * `0` are values), and skips exactly what a browser skips — a row that is not * on screen, and a locked one. */ -function findMissingRequired( +function findSubmitRefusals( sections: RenderableSection[], values: Record, previous: Record | null | undefined, isCreateForm: boolean, predicateScope?: Record, -): RenderableField[] { +): { faultedVisibleWhen: RenderableField[]; missing: RenderableField[] } { + const faultedVisibleWhen: RenderableField[] = []; const missing: RenderableField[] = []; for (const sec of sections) { if (!isSectionVisible(sec, values, previous, predicateScope)) continue; for (const f of sec.fields) { const state = resolveRowState(f, values, previous, isCreateForm, predicateScope); + if (state.faults.visibleWhen !== undefined) faultedVisibleWhen.push(f); if (state.visible && state.required && !state.readonly && isMissingForRequired(values[f.name])) { missing.push(f); } } } - return missing; + return { faultedVisibleWhen, missing }; } // ─── Main component ───────────────────────────────────────────────── @@ -2117,12 +2147,32 @@ export function FormPage({ mode, recordPath }: FormPageProps) { const handleSubmit = async (e: FormEvent) => { e.preventDefault(); if (!loaded) return; + // The client-side refusals — see `findSubmitRefusals`. Both go out on the + // page's failure channel and the outcome toast id, so a retry that + // succeeds supersedes them exactly as it supersedes a refused write + // (objectui#7252). + const { faultedVisibleWhen, missing } = findSubmitRefusals( + sections, + values, + loaded.record, + isCreateForm, + predicateScope, + ); + // A faulted `visibleWhen` first (objectui#8069): it names the field and + // the rule, and nothing typed into the form can clear it, so the required + // check behind it would only ask for work that is refused anyway. + if (faultedVisibleWhen.length > 0) { + const msg = t('form.visibleWhenFaulted', { + fields: faultedVisibleWhen + .map((f) => fieldLabel(loaded.object, f.name, f.label)) + .join(t('validation.formInvalidJoiner')), + }); + setError(msg); + toast.error(msg, { id: outcomeToastId }); + return; + } // The client-side required refusal the native attribute used to give the - // hand-rolled controls (objectui#10179) — see `findMissingRequired`. It - // goes out on the page's failure channel and the outcome toast id, so a - // retry that succeeds supersedes it exactly as it supersedes a refused - // write (objectui#7252). - const missing = findMissingRequired(sections, values, loaded.record, isCreateForm, predicateScope); + // hand-rolled controls (objectui#10179). if (missing.length > 0) { // One catalogue frame around the labels (objectui#11071), joined with // the pack's own list separator: the colon, its spacing and the order of diff --git a/apps/console/src/components/FormPage.visibleWhenFault-8069.test.tsx b/apps/console/src/components/FormPage.visibleWhenFault-8069.test.tsx new file mode 100644 index 0000000000..7a1e60c71f --- /dev/null +++ b/apps/console/src/components/FormPage.visibleWhenFault-8069.test.tsx @@ -0,0 +1,211 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#8069 — `FormPage` refuses a submit whose object-level `visibleWhen` + * could not be evaluated, naming the field and the rule (ADR-0137 D2, ruled as + * Q1 = B: one judge per rule). + * + * ## Why only `visibleWhen` + * + * No server evaluates a field's `visibleWhen`, so its fail-open render + * direction (ADR-0137 D3: a faulting rule SHOWS the field) is the only verdict + * there is — without a submit refusal it is a silent grant. `requiredWhen` and + * `readonlyWhen` are evaluated and refused by the server itself (D2), so the + * client keeps their render direction and warning unchanged; the pin below + * shows a faulted `requiredWhen` still reaching the write. + * + * ## What each case measures + * + * The WRITE, read off the stubbed transport — a refused submit issues none, an + * accepted one issues exactly one — and the message the page shows. Every + * refusal has a control that differs from it in one respect only. + */ + +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; +import { toast } from 'sonner'; +import { createI18n } from '@object-ui/i18n'; +import { FormPage } from './FormPage'; + +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +const BASE_FIELDS: Record> = { + title: { type: 'text', label: 'Title' }, + priority: { type: 'text', label: 'Priority', defaultValue: 'low' }, + notes: { type: 'text', label: 'Notes' }, + status: { type: 'text', label: 'Status' }, +}; + +/** The object with extra keys merged onto named object FIELDS. */ +function objectWith(rules: Record>) { + const fields: Record> = {}; + for (const [name, def] of Object.entries(BASE_FIELDS)) fields[name] = { ...def, ...(rules[name] ?? {}) }; + return { name: 'showcase_task', label: 'Task', fields }; +} + +interface Call { + method: string; + url: string; +} +let calls: Call[] = []; + +const EDIT_RECORD = { id: 'task-42', title: 'T', priority: 'low', notes: 'n', status: 'open' }; + +function renderForm( + objectSchema: unknown, + opts: { sections?: unknown[]; edit?: boolean } = {}, +) { + const sections = opts.sections ?? [{ label: 'Basics', fields: ['title', 'priority', 'notes', 'status'] }]; + const routes: Array<{ method: string; match: string; body: unknown }> = [ + { + method: 'GET', + match: '/meta/view/', + body: { + name: 'showcase_task.edit', + object: 'showcase_task', + viewKind: 'form', + label: 'Task', + config: { type: 'simple', sections }, + }, + }, + { method: 'GET', match: '/meta/object/', body: objectSchema }, + { + method: 'GET', + match: '/data/showcase_task/task-42', + body: { object: 'showcase_task', id: 'task-42', record: EDIT_RECORD }, + }, + { method: 'POST', match: '/data/showcase_task', body: { object: 'showcase_task', id: 'new-1' } }, + { + method: 'PATCH', + match: '/data/showcase_task/task-42', + body: { object: 'showcase_task', id: 'task-42' }, + }, + ]; + vi.stubGlobal( + 'fetch', + vi.fn(async (url: string, init?: RequestInit) => { + const method = (init?.method ?? 'GET').toUpperCase(); + calls.push({ method, url: String(url) }); + const route = routes.find((r) => r.method === method && String(url).includes(r.match)); + if (!route) throw new Error(`unstubbed fetch: ${method} ${url}`); + return { + ok: true, + status: 200, + statusText: 'OK', + json: async () => route.body, + text: async () => JSON.stringify(route.body), + } as unknown as Response; + }), + ); + return render( + + + } /> + + , + ); +} + +const writes = () => calls.filter((c) => c.method === 'POST' || c.method === 'PATCH'); + +async function submit() { + await waitFor(() => expect(screen.getByLabelText('Priority')).toBeInTheDocument()); + await userEvent.click(screen.getByRole('button', { name: /submit/i })); +} + +let warn: ReturnType; + +beforeEach(() => { + createI18n({ defaultLanguage: 'en', detectBrowserLanguage: false }); + calls = []; + vi.mocked(toast.error).mockClear(); + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + warn.mockRestore(); +}); + +describe('objectui#8069 — FormPage refuses a faulted visibleWhen at submit', () => { + it('refuses, writes nothing, and names the field and the rule', async () => { + renderForm(objectWith({ notes: { visibleWhen: 'record.priority ==' } })); + await submit(); + + const message = await screen.findByText(/visibleWhen rule of Notes could not be evaluated/); + expect(message).toBeInTheDocument(); + const toasts = vi.mocked(toast.error).mock.calls; + expect(String(toasts[toasts.length - 1]?.[0])).toMatch(/visibleWhen rule of Notes/); + expect(writes()).toHaveLength(0); + // Fail-open at RENDER is unchanged (ADR-0137 D3): the field is on screen. + expect(screen.getByLabelText('Notes')).toBeInTheDocument(); + }); + + it('CONTROL — the same field with a rule that evaluates submits', async () => { + renderForm(objectWith({ notes: { visibleWhen: "record.priority == 'low'" } })); + await submit(); + await waitFor(() => expect(writes()).toHaveLength(1)); + expect(writes()[0].method).toBe('POST'); + }); + + it('a STORED blank visibleWhen is refused too — ADR-0137 D2', async () => { + renderForm(objectWith({ notes: { visibleWhen: '' } })); + await submit(); + await screen.findByText(/visibleWhen rule of Notes could not be evaluated/); + expect(writes()).toHaveLength(0); + expect(screen.getByLabelText('Notes')).toBeInTheDocument(); + expect(warn.mock.calls.some((c: unknown[]) => String(c[0]).includes('[blank]'))).toBe(true); + }); + + it('control — a blank VIEW-level visibleWhen is a layout gate, not a field rule: it submits', async () => { + // The view's section entry is normalized before this page builds its rows, + // and a blank view-level predicate does not survive that step — "no gate", + // and nothing on this path refuses it. + renderForm(objectWith({}), { + sections: [{ label: 'Basics', fields: ['title', 'priority', { field: 'notes', visibleWhen: '' }, 'status'] }], + }); + await waitFor(() => expect(screen.getByLabelText('Notes')).toBeInTheDocument()); + await submit(); + await waitFor(() => expect(writes()).toHaveLength(1)); + }); + + it('a faulted requiredWhen is NOT refused on the client — it is the server’s to refuse (D2)', async () => { + renderForm(objectWith({ notes: { requiredWhen: 'record.priority ==' } })); + await submit(); + await waitFor(() => expect(writes()).toHaveLength(1)); + }); + + it('a faulted readonlyWhen is NOT refused on the client either', async () => { + renderForm(objectWith({ notes: { readonlyWhen: 'record.priority ==' } }), { edit: true }); + await submit(); + await waitFor(() => expect(writes()).toHaveLength(1)); + expect(writes()[0].method).toBe('PATCH'); + }); + + it('ACCEPTED RESIDUAL — a visibleWhen reading previous is refused on a CREATE form', async () => { + renderForm(objectWith({ notes: { visibleWhen: "previous.status == 'open'" } })); + await submit(); + await screen.findByText(/visibleWhen rule of Notes could not be evaluated/); + expect(writes()).toHaveLength(0); + }); + + it('CONTROL — the same previous-reading visibleWhen submits on an EDIT form', async () => { + renderForm(objectWith({ notes: { visibleWhen: "previous.status == 'open'" } }), { edit: true }); + await submit(); + await waitFor(() => expect(writes()).toHaveLength(1)); + expect(writes()[0].method).toBe('PATCH'); + }); + + it('a row inside a HIDDEN section is not judged: its own rule cannot put it on screen', async () => { + renderForm(objectWith({ notes: { visibleWhen: 'record.priority ==' } }), { + sections: [ + { label: 'Basics', fields: ['title', 'priority'] }, + { label: 'Escalation', visibleWhen: "record.priority == 'urgent'", fields: ['notes'] }, + ], + }); + await submit(); + await waitFor(() => expect(writes()).toHaveLength(1)); + }); +}); diff --git a/content/docs/guide/metadata-diagnostics.md b/content/docs/guide/metadata-diagnostics.md index 145f674984..675b992991 100644 --- a/content/docs/guide/metadata-diagnostics.md +++ b/content/docs/guide/metadata-diagnostics.md @@ -161,13 +161,35 @@ A predicate that slips past authoring is still not silent at **runtime**: when a conditional rule (`visibleWhen` / `readonlyWhen` / `requiredWhen`, view-level `visibleOn`, per-option `visibleWhen`, list conditional formatting) fails to evaluate, the renderer applies the rule's safe default -(fail-open — a broken predicate never hides a field or blocks a submit) and +(fail-open — a broken predicate never hides a field) and logs **one `console.warn` per predicate** with the predicate source, the engine's failure reason, and the field it was attached to. A rule that never fires while its field stays visible is the classic symptom — open the browser console and the broken predicate identifies itself (most often a bare field name where `record.` was meant). +At **submit**, a broken field rule is refused rather than passed (ADR-0137 +D2). A field-level `visibleWhen` that cannot be evaluated refuses the submit +on the client — the record form, the `/forms/:name` and `/f/:slug` page, and a +wizard's final step — with a message naming the field and the rule, because no +server evaluates `visibleWhen` and its fail-open render would otherwise be a +silent grant. A broken `requiredWhen` / `readonlyWhen` is refused by the server, +and the form shows that refusal beside the input. A `visibleWhen` that reads +`previous` cannot be evaluated on a create form, so such a form is always +refused (and on an edit wizard too, whose final cross-step check binds no +`previous`). + +A **blank** field rule (`''`, whitespace, or an envelope whose `source` is +blank) is not "no rule" — it is refused, at the first place that can see it +(ADR-0137 D1 / D2). A new one is refused at authoring: the form schema's +`visibleWhen` / `readonlyWhen` / `requiredWhen` reject a blank predicate at +parse, as the protocol's field schema does. One already stored is refused at +submit, like any rule that cannot be evaluated: the client refuses a blank +`visibleWhen`, the server a blank `requiredWhen` / `readonlyWhen`. A blank +**gate** is different: a blank `visible`, `hidden` or `disabled` is still read +as "no gate", with a one-time `[blank]` warning in the console, and a form +view's own blank field `visibleWhen` is likewise read as no gate, never refused. + The same is now true of a **component node's own gate** — `visibleWhen` on a page component (and its `visible` / `visibleOn` / `visibility` / `hidden` / `hiddenOn` siblings), plus a `page:tabs` item's `visibleWhen`. These used to diff --git a/content/docs/plugins/plugin-form.mdx b/content/docs/plugins/plugin-form.mdx index 2b184830aa..ddbc336fb1 100644 --- a/content/docs/plugins/plugin-form.mdx +++ b/content/docs/plugins/plugin-form.mdx @@ -93,7 +93,7 @@ has **23 declared keys**, and `name` is the only **required** one: `type` and | `options` | `SelectOption[] \| RadioOption[]` | choices for `select` / radio fields | | `validation` | `FieldValidationRules` | **an object keyed by rule name** — see below | | `condition` | `FieldCondition` | legacy `{ field, equals, notEquals, in }` matcher (`custom` is retired, objectui#7759: nothing ran it) | -| `visibleWhen` / `readonlyWhen` / `requiredWhen` | `string \| { dialect?, source }` | CEL predicates over the live record, evaluated by `@objectstack/formula` — the same engine and dialect the server enforces. They fail open | +| `visibleWhen` / `readonlyWhen` / `requiredWhen` | `string \| { dialect?, source }` | CEL predicates over the live record, evaluated by `@objectstack/formula` — the same engine and dialect the server enforces. They fail open on screen; a `visibleWhen` that cannot be evaluated refuses the submit, naming the field and the rule | | `visibleOn` | `string \| { dialect?, source }` | view-level visibility predicate (spec `FormField.visibleOn`) | | `dependsOn` | `DependsOnInput` | cascading parent(s): a bare name, a list of names, or `{ field, param }` entries | | `span` | `'auto' \| 'full'` | relative field width, independent of the column count (preferred) | diff --git a/packages/app-shell/src/views/ActionParamDialog.test.tsx b/packages/app-shell/src/views/ActionParamDialog.test.tsx index 3ff37ac0bd..d569fb3bde 100644 --- a/packages/app-shell/src/views/ActionParamDialog.test.tsx +++ b/packages/app-shell/src/views/ActionParamDialog.test.tsx @@ -153,11 +153,13 @@ describe('filterVisibleParams — faults are fail-open and LOUD (objectui#4640)' expect(warnings).toHaveLength(1); }); - it('a blank or absent predicate is NOT a fault — kept, and silent', () => { + it('a blank predicate is NOT a fault — kept, never evaluated, and diagnosed as blank (objectui#8069)', () => { // `''`, whitespace, and the empty `{ dialect, source: '' }` envelope a // spec-normalized empty predicate compiles to all mean "no gate declared" // (objectui#3850 / #3960) — they must never reach the evaluator to be - // reported as broken. + // reported as broken. Since ADR-0137 D4 the "no gate" fold is no longer + // SILENT: `hasDeclaredPredicate` reports each blank spelling once through + // core's `[blank]` channel, and nothing else speaks. const params: ActionParamDef[] = [ p('a', ''), p('b', ' '), @@ -166,6 +168,17 @@ describe('filterVisibleParams — faults are fail-open and LOUD (objectui#4640)' ]; const { result, warnings } = withWarnings(() => filterVisibleParams(params, {}, 'Create user')); expect(result.map((x) => x.name)).toEqual(['a', 'b', 'c', 'd']); + // `p('a', '')` authors no key at all (the helper drops a falsy predicate); + // `' '`, `{ source: '' }` and `{ source: ' ' }` are three blank + // spellings, one line each. + expect(warnings).toHaveLength(3); + expect(warnings.every((w) => w.includes('[blank]'))).toBe(true); + }); + + it('control — an ABSENT predicate is kept and silent: there is nothing declared to report', () => { + const params: ActionParamDef[] = [p('a'), { name: 'n', label: 'N', type: 'text', visible: null as never }]; + const { result, warnings } = withWarnings(() => filterVisibleParams(params, {}, 'Create user')); + expect(result.map((x) => x.name)).toEqual(['a', 'n']); expect(warnings).toEqual([]); }); diff --git a/packages/components/src/renderers/form/__tests__/visibleWhen-fault-refuses-submit-8069.test.tsx b/packages/components/src/renderers/form/__tests__/visibleWhen-fault-refuses-submit-8069.test.tsx new file mode 100644 index 0000000000..b7b28249dd --- /dev/null +++ b/packages/components/src/renderers/form/__tests__/visibleWhen-fault-refuses-submit-8069.test.tsx @@ -0,0 +1,237 @@ +/** + * 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#8069 — the form renderer refuses a submit whose field `visibleWhen` + * could not be evaluated, naming the field and the rule; `requiredWhen` / + * `readonlyWhen` faults go to the server, whose ADR-0137 D2 refusal lands + * beside the input. + * + * ## The ruling this pins (Q1 = B, one judge per rule) + * + * No server evaluates a field's `visibleWhen`, so its fail-open render + * direction (ADR-0137 D3: a faulting rule SHOWS the field) is the only verdict + * anywhere — without a refusal at submit it is a silent grant. The server DOES + * evaluate `requiredWhen` / `readonlyWhen` and refuses a faulted one itself + * (D2), so the client keeps their render direction and warning, lets the + * submit through, and shows the server's field-attributed refusal inline + * through the existing `extractFieldErrors` → `form.setError` path. + * + * ## The server end, transcribed + * + * `SERVER_D2_REFUSAL` is the envelope `@objectstack/objectql`'s rule validator + * builds for an unevaluable field rule at the spec 17.5.0 tag (objectstack + * `0f6dcac5e9`, `unevaluableFieldRuleError` → `unevaluableRuleError`): + * `code: 'rule_violation'`, a message of the shape "Field '' requiredWhen + * could not be evaluated (…) — write rejected.", and `constraint.rule` naming + * the SLOT with `constraint.reason: 'unevaluable'` — served as + * `VALIDATION_FAILED` with `fields[]`, as `@objectstack/client` decorates it. + * objectql is not a dependency of this repository, so the pin carries the wire + * shape rather than calling the validator. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render, screen, fireEvent, waitFor, cleanup, within } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +import { toast } from '../../../ui/sonner'; +// Registers the renderers at module scope (object-ui/no-dynamic-import-in-test-hook). +import '../../../renderers'; + +let toastError: ReturnType; +let warn: ReturnType; + +beforeEach(() => { + toastError = vi.spyOn(toast, 'error').mockImplementation(() => 'id' as never); + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const proto = Element.prototype as unknown as { scrollIntoView?: () => void }; + if (!proto.scrollIntoView) proto.scrollIntoView = () => {}; +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +/** The server's ADR-0137 D2 refusal for one field rule, as the client sees it. */ +function SERVER_D2_REFUSAL(field: string, slot: 'requiredWhen' | 'readonlyWhen') { + const message = `Field '${field}' ${slot} could not be evaluated (the predicate names a key this record does not have) — write rejected.`; + const fields = [ + { + field, + code: 'rule_violation', + message, + constraint: { rule: slot, reason: 'unevaluable', fault: 'the predicate names a key this record does not have' }, + }, + ]; + return Object.assign(new Error(message), { + code: 'VALIDATION_FAILED', + httpStatus: 400, + details: { error: message, code: 'VALIDATION_FAILED', fields, object: 'crm_deal' }, + }); +} + +function renderForm( + fields: Array>, + onSubmit: (data: unknown) => Promise, + extra: Record = {}, +) { + const Form = ComponentRegistry.get('form')!; + return render( +
, + ); +} + +const submit = () => fireEvent.click(screen.getByRole('button', { name: /create/i })); +const lastToast = () => String(toastError.mock.calls.at(-1)?.[0] ?? ''); + +/** A predicate the engine cannot parse: a fault whatever the record holds. */ +const BROKEN = 'record.stage =='; + +const base = (discount: Record) => [ + { name: 'stage', label: 'Stage', type: 'input' }, + { name: 'discount', label: 'Discount', type: 'input', ...discount }, +]; + +describe('objectui#8069 — a faulted visibleWhen refuses the submit', () => { + it('refuses, calls no host, and names the field and the rule — the field stays drawn (D3)', async () => { + const onSubmit = vi.fn().mockResolvedValue(undefined); + renderForm(base({ visibleWhen: BROKEN }), onSubmit); + expect(screen.getByLabelText('Discount')).toBeTruthy(); + + submit(); + + await waitFor(() => expect(lastToast()).toMatch(/visibleWhen rule of Discount could not be evaluated/)); + expect(screen.getByText(/visibleWhen rule of Discount could not be evaluated/)).toBeTruthy(); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it('refuses BEFORE client validation: a required empty field is not asked for first', async () => { + const onSubmit = vi.fn().mockResolvedValue(undefined); + renderForm( + [ + { name: 'stage', label: 'Stage', type: 'input', required: true }, + { name: 'discount', label: 'Discount', type: 'input', visibleWhen: BROKEN }, + ], + onSubmit, + ); + + submit(); + + await waitFor(() => expect(lastToast()).toMatch(/visibleWhen rule of Discount/)); + expect(toastError.mock.calls.some((c: unknown[]) => /check the highlighted fields/.test(String(c[0])))).toBe(false); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it('CONTROL — the same field with a rule that evaluates submits', async () => { + const onSubmit = vi.fn().mockResolvedValue(undefined); + renderForm(base({ visibleWhen: "record.stage == null" }), onSubmit); + submit(); + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + }); + + it('a STORED blank visibleWhen is refused too — ADR-0137 D2: "a blank predicate takes this path too"', async () => { + // A new one cannot be authored (the form schema's triad wire refuses it at + // parse, D1); one already stored meets the submit refusal. The field is + // still drawn — a blank rule renders like no rule (D3). + const onSubmit = vi.fn().mockResolvedValue(undefined); + renderForm(base({ visibleWhen: '' }), onSubmit); + expect(screen.getByLabelText('Discount')).toBeTruthy(); + submit(); + await waitFor(() => expect(lastToast()).toMatch(/visibleWhen rule of Discount could not be evaluated/)); + expect(onSubmit).not.toHaveBeenCalled(); + expect(warn.mock.calls.some((c: unknown[]) => String(c[0]).includes('[blank]'))).toBe(true); + }); + + it('control — a blank view-level visibleOn is a layout GATE: no gate plus a diagnostic, never a refusal (D4)', async () => { + const onSubmit = vi.fn().mockResolvedValue(undefined); + renderForm(base({ visibleOn: '' }), onSubmit); + expect(screen.getByLabelText('Discount')).toBeTruthy(); + submit(); + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + expect(warn.mock.calls.some((c: unknown[]) => String(c[0]).includes('[blank]'))).toBe(true); + }); + + it('a section-divider row’s visibleWhen is a layout gate, not a field rule: it does not refuse', async () => { + const onSubmit = vi.fn().mockResolvedValue(undefined); + renderForm( + [ + { name: '__section_extra', label: 'Extra', type: 'section-divider', visibleWhen: BROKEN }, + { name: 'stage', label: 'Stage', type: 'input' }, + ], + onSubmit, + ); + submit(); + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + }); + + it('ACCEPTED RESIDUAL — a visibleWhen reading previous is refused on a CREATE form', async () => { + const onSubmit = vi.fn().mockResolvedValue(undefined); + renderForm(base({ visibleWhen: "previous.stage == 'won'" }), onSubmit); + submit(); + await waitFor(() => expect(lastToast()).toMatch(/visibleWhen rule of Discount/)); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it('CONTROL — the same previous-reading visibleWhen submits on an EDIT form', async () => { + const onSubmit = vi.fn().mockResolvedValue(undefined); + renderForm(base({ visibleWhen: "previous.stage == 'won'" }), onSubmit, { + mode: 'edit', + defaultValues: { stage: 'won', discount: '5' }, + previousValues: { stage: 'won', discount: '5' }, + }); + submit(); + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + }); +}); + +describe('objectui#8069 — requiredWhen / readonlyWhen faults are the SERVER’s to refuse', () => { + it('a faulted requiredWhen is not refused on the client; the server D2 refusal lands beside the input', async () => { + const onSubmit = vi.fn().mockRejectedValue(SERVER_D2_REFUSAL('discount', 'requiredWhen')); + const { container } = renderForm(base({ requiredWhen: BROKEN }), onSubmit); + + submit(); + + // The client let it through — one judge per rule. + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + // …and the server's refusal is written under the input it names. + const discount = container.querySelector('[data-field="discount"]') as HTMLElement; + await waitFor(() => + expect(within(discount).getByText(/requiredWhen could not be evaluated/)).toBeTruthy(), + ); + // Fully attributed, so no page-level banner repeats it. + expect(container.querySelector('[role="alert"]')?.textContent ?? '').not.toMatch(/write rejected/); + }); + + it('a faulted readonlyWhen on an EDIT form is not refused on the client either', async () => { + const onSubmit = vi.fn().mockRejectedValue(SERVER_D2_REFUSAL('discount', 'readonlyWhen')); + const { container } = renderForm(base({ readonlyWhen: BROKEN }), onSubmit, { + mode: 'edit', + defaultValues: { stage: 'open', discount: '5' }, + previousValues: { stage: 'open', discount: '5' }, + }); + + submit(); + + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + const discount = container.querySelector('[data-field="discount"]') as HTMLElement; + await waitFor(() => + expect(within(discount).getByText(/readonlyWhen could not be evaluated/)).toBeTruthy(), + ); + }); +}); diff --git a/packages/components/src/renderers/form/__tests__/visiblewhen-clear-on-hide-6958.test.tsx b/packages/components/src/renderers/form/__tests__/visiblewhen-clear-on-hide-6958.test.tsx index ccd3fa7e0e..a2a05e5661 100644 --- a/packages/components/src/renderers/form/__tests__/visiblewhen-clear-on-hide-6958.test.tsx +++ b/packages/components/src/renderers/form/__tests__/visiblewhen-clear-on-hide-6958.test.tsx @@ -265,7 +265,14 @@ describe('#6958 — a field its own `visibleWhen` hides is cleared, not carried' expect((await payloadOf(onSubmit)).source).toBe('import'); }); - it('BOUNDARY — a BROKEN predicate fails open, so nothing is cleared', async () => { + it('BOUNDARY — a BROKEN predicate fails open, so nothing is cleared — and nothing is written either', async () => { + // The constraint ADR-0137 carries verbatim from this card: a broken + // predicate must NEVER silently null a stored column. It holds in two + // halves since objectui#8069 (D3 + D2): at RENDER the rule fails open, so + // the clear-on-hide never fires and the value stays in the form; at SUBMIT + // the unevaluable `visibleWhen` refuses the write, so neither a null nor + // the unchecked value reaches the host. Before #8069 the second half was + // "the value is written as it stood", which this pin used to assert. vi.spyOn(console, 'warn').mockImplementation(() => {}); vi.spyOn(console, 'error').mockImplementation(() => {}); const onSubmit = vi.fn(); @@ -284,8 +291,14 @@ describe('#6958 — a field its own `visibleWhen` hides is cleared, not carried' }); fireEvent.change(screen.getByLabelText(/plan/i), { target: { value: 'basic' } }); + // Not cleared: the field is still drawn and still holds its value. + expect((screen.getByLabelText(/contact/i) as HTMLInputElement).value).toBe('AEwPffbk'); save(); - expect((await payloadOf(onSubmit)).crm_contact).toBe('AEwPffbk'); + await waitFor(() => + expect(screen.getByText(/visibleWhen rule of Contact could not be evaluated/)).toBeTruthy(), + ); + expect(onSubmit).not.toHaveBeenCalled(); + expect((screen.getByLabelText(/contact/i) as HTMLInputElement).value).toBe('AEwPffbk'); }); it('BOUNDARY — an already-empty hidden field is never written to (no invented null)', async () => { diff --git a/packages/components/src/renderers/form/form.tsx b/packages/components/src/renderers/form/form.tsx index a594c2831e..7ca24fbdb9 100644 --- a/packages/components/src/renderers/form/form.tsx +++ b/packages/components/src/renderers/form/form.tsx @@ -289,6 +289,10 @@ const useSafeFormTranslation = createSafeTranslation( 'form.noPermissionToSave': "You don't have permission to save this record.", 'form.submitFailed': 'Could not save. Please try again.', 'form.clearedOnHide': 'Cleared — no longer applicable given the current values: {{fields}}', + // objectui#8069 — the submit refusal for a field `visibleWhen` that could + // not be evaluated (see `handleSubmit`). Byte-identical to the `en` pack. + 'form.visibleWhenFaulted': + "Can't submit: the visibleWhen rule of {{fields}} could not be evaluated. The rule must be fixed before this form can be submitted.", }, 'common.selectOption', ); @@ -1548,8 +1552,26 @@ ComponentRegistry.register('form', // a payload. Re-evaluating the predicates the render path also evaluates // cannot double-warn: `warnPredicateFailure` dedupes by predicate source, // which `readonlyFieldNames` above already leans on. - const conditionallyHiddenFieldNames = React.useMemo(() => { + // + // ── …and the fields whose `visibleWhen` could NOT be evaluated + // (objectui#8069, ADR-0137 D2 as ruled) ───────────────────────────────── + // The same `resolveFieldRuleState` call reports them (`faults`), so the + // submit refusal below reads the verdict this memo already drew and never + // evaluates a predicate a second time. Every declared field is judged, not + // only the drawn ones: a faulted `visibleWhen` fails OPEN, so the field is + // never cleared on hide (`6a449fc49`) and its value reaches the payload + // whether or not a section or tab happens to hide it — the rule changes + // the write either way, so the write is refused either way. + // + // One exclusion, about WHAT the slot holds rather than about the fault: a + // `section-divider` row carries its SECTION's predicate in this same + // `visibleWhen` slot, which is a layout gate and not a field rule. A + // stored BLANK field rule is a fault like any other (ADR-0137 D2; see + // `FieldRuleFaults`) and is refused; a new one is refused earlier, at + // authoring, by the form schema's triad wire. + const { conditionallyHiddenFieldNames, faultedVisibleWhenFieldNames } = React.useMemo(() => { const hidden = new Set(); + const faulted: string[] = []; for (const f of fields as FormFieldConfig[]) { const name = f?.name; if (!name) continue; @@ -1571,6 +1593,7 @@ ComponentRegistry.register('form', // predicate is reported once, against the field. `field '${name}'`, ); + if (st.faults.visibleWhen !== undefined && f.type !== 'section-divider') faulted.push(name); if (!st.visible) { hidden.add(name); continue; @@ -1585,7 +1608,7 @@ ComponentRegistry.register('form', hidden.add(name); } } - return hidden; + return { conditionallyHiddenFieldNames: hidden, faultedVisibleWhenFieldNames: faulted }; }, [fields, ruleRecord, previousRecord, isCreateForm, predicateScope]); // ── The section grouping contract (objectui#6236, maintainer ruling @@ -2394,8 +2417,10 @@ ComponentRegistry.register('form', revealField(firstName); }; - // Handle form submission - const handleSubmit = form.handleSubmit(async (data) => { + // Handle form submission — react-hook-form's validation, then the write. + // Reached only through `handleSubmit` below, which refuses first on a + // faulted `visibleWhen`. + const submitThroughValidation = form.handleSubmit(async (data) => { setIsSubmitting(true); setSubmitError(null); // …and the toast twin of that banner (objectui#7252). Both belong to the @@ -2542,6 +2567,58 @@ ComponentRegistry.register('form', announceFieldErrors(Object.keys(validationErrors || {})); }); + /** + * The submit entry: a faulted `visibleWhen` refuses the write, naming the + * field and the rule (objectui#8069 — ADR-0137 D2, ruled as Q1 = B: one + * judge per rule). + * + * ## Why `visibleWhen` alone + * + * It is the one field rule no server evaluates, so its fail-open render + * direction (D3: a faulting `visibleWhen` SHOWS the field) would otherwise + * be a silent grant — a field the working rule would have hidden, drawn, + * edited and written, with nothing anywhere saying the rule did not run. + * `requiredWhen` / `readonlyWhen` keep their render direction and warning + * here unchanged: the server evaluates both and refuses a fault itself + * (D2), field-attributed, and that refusal lands beside the input through + * the `extractFieldErrors` → `form.setError` path above. A second judge on + * the client, with less of the record in hand than the server has, would + * refuse writes the server accepts. + * + * ## Why BEFORE react-hook-form's validation + * + * Nothing the person filling the form can type clears a broken rule, so + * asking them to satisfy the other rules first only to refuse them anyway + * would be wasted work. The refusal therefore runs first and ends the + * attempt: no validation, no `onAction` / `onSubmit`, no write. + * + * ## The accepted residual, named rather than worked around + * + * A `visibleWhen` reading `previous` cannot be evaluated on a CREATE form + * (there is no stored row), so such a form is refused on every submit. The + * ruling accepted that; it is pinned, not patched. + * + * The verdict is read from `faultedVisibleWhenFieldNames` — the same + * evaluation that decided which fields are hidden — never re-derived here. + */ + const handleSubmit = (event?: React.BaseSyntheticEvent) => { + if (faultedVisibleWhenFieldNames.length > 0) { + event?.preventDefault?.(); + toast.dismiss(outcomeToastId); + const message = t('form.visibleWhenFaulted', { + fields: faultedVisibleWhenFieldNames + .map((n) => fieldLabelByName[n] || n) + .join(t('validation.formInvalidJoiner')), + }); + setSubmitError(message); + toast.error(message, { id: outcomeToastId }); + // Marks the tabs that hold the fields, as a rejected submit does. + setRejectedFieldNames(faultedVisibleWhenFieldNames); + return Promise.resolve(); + } + return submitThroughValidation(event); + }; + // Handle cancel const handleCancel = () => { form.reset(); diff --git a/packages/core/src/evaluator/ExpressionEvaluator.ts b/packages/core/src/evaluator/ExpressionEvaluator.ts index 4a6737fddd..bcf875eeb5 100644 --- a/packages/core/src/evaluator/ExpressionEvaluator.ts +++ b/packages/core/src/evaluator/ExpressionEvaluator.ts @@ -20,6 +20,7 @@ import { ExpressionContext } from './ExpressionContext.js'; import { ExpressionCache } from './ExpressionCache.js'; import { FormulaFunctions } from './FormulaFunctions.js'; import { evalFieldPredicate } from './fieldRules.js'; +import { isBlankPredicateText } from './declaredPredicate.js'; /** * Options for expression evaluation @@ -419,7 +420,29 @@ export class ExpressionEvaluator { * syntax) throws; a genuine `false` never throws. */ private evaluateCelCondition(source: string, options: EvaluationOptions): boolean { - if (!source.trim()) return true; // no predicate → visible/enabled + if (isBlankPredicateText(source)) { + // A BLANK gate: no predicate → visible/enabled, exactly as before + // (objectui#3850 / #3960's verdict), in EVERY mode — `throwOnError` + // included, since the author wrote nothing that could fault. What + // changed is that it is no longer SILENT (ADR-0137 D4: a blank gate + // predicate is "diagnosed, never a silent `true`"; objectui#8069). The + // report is `evalFieldPredicate`'s own `[blank]` one — the channel every + // other predicate fault already uses — so a caller that passed `onFault` + // receives it there (and prints its own, node-named line), and every + // other caller gets the deduped built-in warning. No engine call is made: + // a blank predicate never reaches the engine. + evalFieldPredicate( + source, + {}, + true, + undefined, + undefined, + options.onFault + ? { warn: false, onFault: options.onFault } + : { context: 'a CEL gate predicate, read as no gate' }, + ); + return true; + } const bag = this.context.toObject(); const rec = bag.record; const record = (rec && typeof rec === 'object' && !Array.isArray(rec)) diff --git a/packages/core/src/evaluator/__tests__/fieldRuleFaults-8069.test.ts b/packages/core/src/evaluator/__tests__/fieldRuleFaults-8069.test.ts new file mode 100644 index 0000000000..1b07eb1c5b --- /dev/null +++ b/packages/core/src/evaluator/__tests__/fieldRuleFaults-8069.test.ts @@ -0,0 +1,251 @@ +/** + * 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#8069 — the core half of ADR-0137 D2 and D4. + * + * ## D2: the fault REPORT a submit path reads + * + * `resolveFieldRuleState` used to return three verdicts and nothing else, so a + * submit path that wanted to refuse a broken rule had only one way to learn + * about it: evaluate the predicate a second time, with its own copy of the + * record assembly. It now returns `faults` beside the verdicts, filled from the + * very evaluation that drew them. The verdicts themselves do not move — a + * faulting `visibleWhen` still SHOWS the field (ADR-0137 D3). + * + * ## A stored BLANK field rule is a fault (D2), a blank GATE is not (D4) + * + * ADR-0137 makes a declared-but-blank field rule a third state: "A blank + * predicate takes this path too, wherever one is already stored" (D2). So it + * enters the report under its rule, with the `[blank]` reason, and a submit + * path refuses it. The authoring half (D1) is `@object-ui/types`' form wire, + * which refuses a blank triad key at parse, so no author reaches this state by + * writing it; its pin is `base-schema-predicate-envelope-7530.test.ts`. A blank + * GATE never reaches this function — the silencer cases below are its control. + * + * ## D4: the two blank-GATE silencers, diagnosed with no verdict moved + * + * 1. `ExpressionEvaluator.evaluateCelCondition` answered `true` for a blank + * CEL source before any evaluator could say so. + * 2. `hasDeclaredPredicate` folded blank predicate text to "no gate". + * + * Both verdicts are objectui#3850 / #3960's and stay; both now report through + * `evalFieldPredicate`'s one `[blank]` channel. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +/** + * The one-time warning dedupe is MODULE state, and this project runs with + * `isolate: false`: without a reset, a blank spelling another file already + * warned about reads as silence here and a case passes having checked nothing. + * `vi.resetModules()` + fresh imports give every case its own dedupe `Set` — + * the shape `ExpressionEvaluator.faultWarnDedupe.test.ts` uses and proves. + */ +let resolveFieldRuleState: typeof import('../fieldRules.js').resolveFieldRuleState; +let hasDeclaredPredicate: typeof import('../declaredPredicate.js').hasDeclaredPredicate; +let ExpressionEvaluator: typeof import('../ExpressionEvaluator.js').ExpressionEvaluator; +let warn: ReturnType; + +beforeEach(async () => { + vi.resetModules(); + ({ resolveFieldRuleState } = await import('../fieldRules.js')); + ({ hasDeclaredPredicate } = await import('../declaredPredicate.js')); + ({ ExpressionEvaluator } = await import('../ExpressionEvaluator.js')); + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); +afterEach(() => warn.mockRestore()); + +const warnings = (): string[] => warn.mock.calls.map((call: unknown[]) => String(call[0])); + +/** Unique per case: the one-time warning dedupes on predicate text. */ +const unbound = (tag: string) => `record.no_such_column_${tag}_8069 == 1`; + +describe('resolveFieldRuleState — the fault report (objectui#8069, ADR-0137 D2)', () => { + it('reports a faulted visibleWhen with the reason the warning prints, and still SHOWS the field', () => { + const st = resolveFieldRuleState({ visibleWhen: unbound('vis') }, {}, {}, undefined, undefined, "field 'a'"); + expect(st.visible).toBe(true); + expect(st.faults.visibleWhen).toMatch(/no_such_column_vis_8069/); + expect(st.faults.readonlyWhen).toBeUndefined(); + expect(st.faults.requiredWhen).toBeUndefined(); + // The same reason the one-time warning carries, from the same evaluation. + const line = warnings().find((w) => w.includes("visibleWhen of field 'a'")); + expect(line).toBeDefined(); + expect(line).toContain(st.faults.visibleWhen!); + }); + + it('reports each rule under its own key — readonlyWhen and requiredWhen too', () => { + const st = resolveFieldRuleState( + { readonlyWhen: unbound('ro'), requiredWhen: unbound('req') }, + {}, + {}, + ); + expect(st.readonly).toBe(false); + expect(st.required).toBe(false); + expect(st.faults.readonlyWhen).toMatch(/no_such_column_ro_8069/); + expect(st.faults.requiredWhen).toMatch(/no_such_column_req_8069/); + expect(st.faults.visibleWhen).toBeUndefined(); + }); + + it('a visibleWhen reading previous with no previous bound is a fault — the create-form residual', () => { + // ADR-0137 D2 as ruled for objectui#8069: nothing anywhere can evaluate + // `previous` on an INSERT, so this is the accepted residual the submit + // paths refuse. + const st = resolveFieldRuleState({ visibleWhen: "previous.status_8069 == 'approved'" }, {}, {}); + expect(st.visible).toBe(true); + expect(st.faults.visibleWhen).toMatch(/previous/); + }); + + it('control — the same previous-reading visibleWhen is NOT a fault once previous is bound (an edit)', () => { + const st = resolveFieldRuleState( + { visibleWhen: "previous.status_8069 == 'approved'" }, + {}, + {}, + { status_8069: 'approved' }, + ); + expect(st.visible).toBe(true); + expect(st.faults).toEqual({}); + }); + + it('control — healthy predicates, TRUE and FALSE alike, report nothing', () => { + const rules = { + visibleWhen: "record.stage_8069 == 'open'", + readonlyWhen: "record.stage_8069 == 'closed'", + requiredWhen: "record.stage_8069 == 'open'", + }; + expect(resolveFieldRuleState(rules, { stage_8069: 'open' }, {}).faults).toEqual({}); + const hidden = resolveFieldRuleState(rules, { stage_8069: 'closed' }, {}); + expect(hidden.visible).toBe(false); + expect(hidden.faults).toEqual({}); + expect(warn).not.toHaveBeenCalled(); + }); + + it('control — no rules at all report nothing', () => { + expect(resolveFieldRuleState({}, {}, {}).faults).toEqual({}); + }); + + it('a rule the verdict did not need never runs, so it cannot fault', () => { + const st = resolveFieldRuleState( + { readonlyWhen: unbound('ro_static'), requiredWhen: unbound('req_owned') }, + {}, + { readonly: true, serverOwnedValue: true }, + ); + expect(st.readonly).toBe(true); + expect(st.required).toBe(false); + expect(st.faults).toEqual({}); + }); +}); + +describe('a stored BLANK field rule is reported as a fault — ADR-0137 D2 (objectui#8069)', () => { + it.each([ + ['empty string', ''], + ['whitespace string', ' '], + ['blank envelope', { dialect: 'cel', source: ' ' }], + ] as const)('%s visibleWhen: still SHOWN (D3), [blank] warned, and in the report as [blank]', (_label, pred) => { + const st = resolveFieldRuleState( + { visibleWhen: pred as never }, + {}, + {}, + undefined, + undefined, + `field 'blank_${_label.replace(/\W/g, '_')}_8069'`, + ); + expect(st.visible).toBe(true); + expect(st.faults.visibleWhen).toMatch(/^\[blank\]/); + expect(warnings().some((w) => w.includes('[blank]'))).toBe(true); + }); + + it('a blank requiredWhen / readonlyWhen is reported too — which rules a path refuses on is the path\u2019s ruling', () => { + const st = resolveFieldRuleState({ readonlyWhen: '', requiredWhen: { source: ' ' } }, {}, {}); + expect(st.readonly).toBe(false); + expect(st.required).toBe(false); + expect(st.faults.readonlyWhen).toMatch(/^\[blank\]/); + expect(st.faults.requiredWhen).toMatch(/^\[blank\]/); + }); + + it('control — an ABSENT rule is not a fault: no key, no report', () => { + const st = resolveFieldRuleState({ visibleWhen: undefined }, {}, {}); + expect(st.faults).toEqual({}); + expect(warn).not.toHaveBeenCalled(); + }); +}); + +describe('ADR-0137 D4 — silencer 1: evaluateCelCondition on a blank CEL gate', () => { + const blank = { dialect: 'cel', source: ' ' }; + + it('answers true, as before, and says so once', () => { + const ev = new ExpressionEvaluator({ record: {} }); + expect(ev.evaluateCondition(blank)).toBe(true); + expect(ev.evaluateCondition(blank)).toBe(true); + const lines = warnings().filter((w) => w.includes('[blank]')); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain('a CEL gate predicate, read as no gate'); + }); + + it('hands the [blank] reason to onFault instead of warning, when the caller supplies one', () => { + const reasons: string[] = []; + const ev = new ExpressionEvaluator({ record: {} }); + expect(ev.evaluateCondition({ dialect: 'cel', source: '' }, { onFault: (r) => reasons.push(r) })).toBe(true); + expect(reasons).toHaveLength(1); + expect(reasons[0]).toContain('[blank]'); + expect(warn).not.toHaveBeenCalled(); + }); + + it('throwOnError does NOT throw on a blank — nothing was authored that could fault', () => { + const ev = new ExpressionEvaluator({ record: {} }); + expect(ev.evaluateCondition({ dialect: 'cel', source: '\t' }, { throwOnError: true })).toBe(true); + }); + + it('control — a healthy CEL gate is silent and a broken one still throws under throwOnError', () => { + const ev = new ExpressionEvaluator({ record: { stage_8069: 'open' } }); + expect(ev.evaluateCondition({ dialect: 'cel', source: "record.stage_8069 == 'open'" })).toBe(true); + expect(warn).not.toHaveBeenCalled(); + expect(() => + ev.evaluateCondition({ dialect: 'cel', source: unbound('gate_throw') }, { throwOnError: true }), + ).toThrow(/CEL predicate failed to evaluate/); + }); +}); + +describe('ADR-0137 D4 — silencer 2: hasDeclaredPredicate folding a blank gate', () => { + it.each([ + ['whitespace string', ' \n '], + ['whitespace envelope', { dialect: 'cel', source: ' \t' }], + ['dialect-less blank envelope', { source: ' ' }], + ] as const)('%s: still "not declared", now diagnosed', (_label, value) => { + expect(hasDeclaredPredicate(value)).toBe(false); + const lines = warnings().filter((w) => w.includes('[blank]')); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain('a declared gate, read as no gate'); + }); + + it("'' and the empty envelope are one blank spelling: not declared, and ONE line between them", () => { + // The locator is fixed at this key-neutral layer, so the dedupe is per + // blank spelling across the app — the limit `reportBlankGate` documents. + expect(hasDeclaredPredicate('')).toBe(false); + expect(hasDeclaredPredicate({ dialect: 'cel', source: '' })).toBe(false); + expect(hasDeclaredPredicate('')).toBe(false); + expect(warnings().filter((w) => w.includes('[blank]'))).toHaveLength(1); + }); + + it.each([ + ['undefined', undefined], + ['null', null], + ['a number', 0], + ['an object without source', {}], + ] as const)('control — %s is not blank TEXT: not declared, and silent', (_label, value) => { + expect(hasDeclaredPredicate(value)).toBe(false); + expect(warn).not.toHaveBeenCalled(); + }); + + it('control — a declared gate is declared and silent', () => { + expect(hasDeclaredPredicate("record.stage == 'open'")).toBe(true); + expect(hasDeclaredPredicate(false)).toBe(true); + expect(hasDeclaredPredicate({ dialect: 'cel', source: 'true' })).toBe(true); + expect(warn).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/src/evaluator/__tests__/fieldRules.test.ts b/packages/core/src/evaluator/__tests__/fieldRules.test.ts index 19be61d89e..e5f4779924 100644 --- a/packages/core/src/evaluator/__tests__/fieldRules.test.ts +++ b/packages/core/src/evaluator/__tests__/fieldRules.test.ts @@ -396,13 +396,24 @@ describe('fault directions — what a BROKEN rule decides (objectui#8069)', () = it('all three compose from ONE broken predicate: shows more, locks less, demands less', () => { // The card's thesis in one assertion — the three faults do not cancel. + // Since ADR-0137 the verdicts are unchanged (D3) and each fault is also + // REPORTED, under its own rule, for a submit path to read (D2). expect( resolveFieldRuleState( { visibleWhen: broken, readonlyWhen: broken, requiredWhen: broken }, {}, {}, ), - ).toEqual({ visible: true, readonly: false, required: false }); + ).toEqual({ + visible: true, + readonly: false, + required: false, + faults: { + visibleWhen: expect.any(String), + readonlyWhen: expect.any(String), + requiredWhen: expect.any(String), + }, + }); }); }); diff --git a/packages/core/src/evaluator/declaredPredicate.ts b/packages/core/src/evaluator/declaredPredicate.ts index 5b19815ba5..748c4f1709 100644 --- a/packages/core/src/evaluator/declaredPredicate.ts +++ b/packages/core/src/evaluator/declaredPredicate.ts @@ -7,6 +7,10 @@ */ import { toPredicateInput } from './predicateInput.js'; +// A cycle with `fieldRules.ts`, which imports `isBlankPredicateText` from here. +// Benign by construction: each side reads the other only inside a function +// body, never while its module is being evaluated. +import { evalFieldPredicate, type FieldRulePredicate } from './fieldRules.js'; /** * Is the predicate TEXT blank — in EITHER spelling? (objectui#3960) @@ -21,10 +25,10 @@ import { toPredicateInput } from './predicateInput.js'; * * This is not a fourth dialect of "empty": it is the SAME rule core's evaluation * entries already apply on the value side — `evaluateCondition` - * (`if (!trimmed) return true`), `evaluateCelCondition` (`if (!source.trim()) - * return true`), `evalRowPredicate` (`listConditional.ts`) — brought to the one - * place that answers "is there a condition at all?", so the two halves cannot - * disagree about the same blank. + * (`if (!trimmed) return true`), `evaluateCelCondition` (which asks THIS + * function since objectui#8069), `evalRowPredicate` (`listConditional.ts`) — + * brought to the one place that answers "is there a condition at all?", so the + * two halves cannot disagree about the same blank. * * Exported since objectui#8069 for its second consumer, `evalFieldPredicate` * (`evaluator/fieldRules.ts`), which is the entry this docblock's list did NOT @@ -77,8 +81,9 @@ export function isBlankPredicateText(value: unknown): boolean { * see {@link isBlankPredicateText} for why this is the layer that says it. * Core's other predicate entries already treat both as blank: * `evaluateCondition` (`if (!trimmed) return true`), `evaluateCelCondition` - * (`if (!source.trim()) return true`) and `evalRowPredicate` - * (`evaluator/listConditional.ts`, `if (!source.trim())`). + * (`isBlankPredicateText(source)`, then `true` — diagnosed since + * objectui#8069) and `evalRowPredicate` (`evaluator/listConditional.ts`, + * `if (!source.trim())`). * - `{ dialect, source: '' }` — the empty ENVELOPE. This is not an exotic * spelling: `@objectstack/spec`'s `ExpressionInputSchema` normalizes every * authored predicate into an envelope, so "author left the predicate empty" @@ -89,6 +94,12 @@ export function isBlankPredicateText(value: unknown): boolean { * action refuses to run. Fail-open on junk, which is the posture * `ActionRunner` already committed to (`catch { isDisabled = false }`). * + * The three BLANK shapes above (`''`, whitespace-only text, and an envelope + * whose `source` is blank) are still folded to "not declared" — that is the + * verdict — but no longer in silence: each is reported once (objectui#8069, + * ADR-0137 D4), see {@link reportBlankGate}. `null` / `undefined` (no key) and + * junk are not blank TEXT and stay silent: nothing was declared there to report. + * * A declared-and-`false` gate is DECLARED (`toPredicateInput` returns the * boolean unchanged): `disabled: false` / `visible: false` are verdicts, and * routing them to the evaluator — which short-circuits booleans — is the point @@ -155,6 +166,40 @@ export function isBlankPredicateText(value: unknown): boolean { * repo now asks "is a gate declared?" anywhere but here. */ export function hasDeclaredPredicate(value: unknown): boolean { - if (isBlankPredicateText(value)) return false; + if (isBlankPredicateText(value)) { + reportBlankGate(value as FieldRulePredicate); + return false; + } return toPredicateInput(value) !== undefined; } + +/** + * Say out loud that a DECLARED gate was blank — the one side effect + * {@link hasDeclaredPredicate} has (objectui#8069). + * + * The fold above is objectui#3850 / #3960's ruled verdict and it stands: a + * blank gate is "no gate". What ADR-0137 D4 removed is the SILENCE of it — "a + * gate predicate that is blank or faulting is diagnosed, never a silent + * `true`" — and this fold is where a blank `visible` / `hidden` / `enabled` / + * `disabled` / `condition` stopped before it could reach any evaluator that + * might have said so. + * + * Reported through `evalFieldPredicate`'s own `[blank]` report — the channel + * every other predicate fault already uses, with its once-per-(text, locator) + * dedupe — and not through a warner of its own. It makes no engine call: a + * blank predicate never reaches the engine. The `false` it is handed is this + * function's answer ("not declared"); nothing reads it back. + * + * ⚠️ The locator is FIXED, because this question is key-neutral and receives + * nothing that names the node. So the dedupe is per blank SPELLING across the + * app: `''`, `' '` and a blank envelope each warn once, and a second node + * with the same blank spelling does not warn again. That is weaker than the + * field-rule path, which joins the field's locator to a blank's key, and it is + * the price of diagnosing at the one definition rather than at each of its + * callers. + */ +function reportBlankGate(value: FieldRulePredicate): void { + evalFieldPredicate(value, {}, false, undefined, undefined, { + context: 'a declared gate, read as no gate', + }); +} diff --git a/packages/core/src/evaluator/fieldRules.ts b/packages/core/src/evaluator/fieldRules.ts index 83605f713b..4d2bb2781e 100644 --- a/packages/core/src/evaluator/fieldRules.ts +++ b/packages/core/src/evaluator/fieldRules.ts @@ -202,9 +202,10 @@ function warnPredicateFailure( * disagreement into `throw new Error('CEL predicate failed to evaluate: …')`. * * ⚠️ The last two policies exist **because** `fallback` is free to specify: they - * detect a fault by disagreeing with themselves. Any proposal to fix a - * direction *inside* this helper — objectui#8069's open question — removes the - * mechanism they are built on, so read them before writing one. + * detect a fault by disagreeing with themselves. Fixing a direction *inside* + * this helper would remove the mechanism they are built on, which is why + * ADR-0137 D5 keeps `fallback` free and declares the direction at the + * field-rule and gate layer instead. * * @param pred The `visibleWhen` / `readonlyWhen` / `requiredWhen` predicate. * @param record The live form values (overlays prior persisted record). @@ -216,8 +217,9 @@ function warnPredicateFailure( * call, as {@link resolveFieldRuleState} does. The historic * advice is to pick the *safe* default (`false` for * readonly/required — don't lock/block on error; `true` for - * visibility — don't hide on error); whether "safe" is the - * right axis is objectui#8069's open question. + * visibility — don't hide on error). ADR-0137 kept those + * directions for RENDER (D3) and put the answer to a broken + * field rule at SUBMIT (D2) — see {@link FieldRuleFaults}. * @param previous The prior persisted record, if any (for `previous.*` refs). * @param scope Extra top-level scope variables bound alongside `record` — * e.g. `{ parent }` so an inline line-item cell can reference @@ -301,8 +303,11 @@ export function evalFieldPredicate( * * All three directions are the PERMISSIVE one, so a single mistyped column in * one authored predicate yields a form that shows more, locks less and demands - * less — three faults from one typo, composing rather than cancelling. That is - * objectui#8069's open question and it is deliberately NOT decided here. + * less — three faults from one typo, composing rather than cancelling. + * ADR-0137 answered that without moving any of the three: they stay the RENDER + * direction (D3 — a faulting `visibleWhen` shows the field), and the composite + * is closed at SUBMIT instead (D2), from the `faults` report + * {@link resolveFieldRuleState} returns beside the verdicts. * * **What the history records** (read on full, unshallowed history — a shallow * clone answers this with one commit and no warning): the direction *was* @@ -315,10 +320,9 @@ export function evalFieldPredicate( * never put. objectstack#5149 then removed the SILENCE and left the direction * explicitly undecided ("appeal 1"). * - * ⛔ These values are shipped behaviour. Changing one is not a refactor, it is - * objectui#8069's decision — and `6a449fc49` leans on the `visibleWhen` half - * staying fail-open (a broken predicate must never silently null a stored - * column). + * ⛔ These values are shipped behaviour, declared by ADR-0137 D3. Changing one + * is not a refactor — and `6a449fc49` leans on the `visibleWhen` half staying + * fail-open (a broken predicate must never silently null a stored column). */ const VISIBLE_WHEN_FAULTED = true; const READONLY_WHEN_FAULTED = false; @@ -336,6 +340,46 @@ const VISIBLE_WHEN_ABSENT = true; const READONLY_WHEN_ABSENT = false; const REQUIRED_WHEN_ABSENT = false; +/** + * Which of {@link resolveFieldRuleState}'s rules could NOT be evaluated, and + * why — keyed by rule, valued by the reason the one-time warning prints after + * `Reason:` (`"[kind] message"`). A key is present only for a rule that ran and + * faulted (objectui#8069). + * + * This is the report a SUBMIT path reads — ADR-0137 D2, "at submit time, a + * field-rule predicate that cannot be evaluated refuses the write and names the + * field and the rule" — so that the refusal is decided by the very evaluation + * that drew the verdict, never by a second one with its own record assembly. + * Which rules a submit path refuses on is that path's ruling, not this + * function's: under objectui#8069's ruling (Q1 = B, one judge per rule) the + * client refuses `visibleWhen` only, because no server evaluates it, and leaves + * `readonlyWhen` / `requiredWhen` to the server's own D2 refusal. The other two + * keys are reported all the same, so the report does not encode that ruling. + * + * A BLANK predicate IS in it, as the `[blank]` reason. ADR-0137 makes a + * declared-but-blank field rule a third state, not a spelling of "no rule": + * "A blank predicate takes this path too, wherever one is already stored — D1 + * keeps new ones from being authored, and D2 is what a stored one meets" (D2), + * and its Alternatives refuse "blank means no rule" for this triad by name. So + * a stored blank `visibleWhen` is refused at submit like any other fault. The + * trap that would otherwise make — an authorable value that no submit can ever + * pass — is closed at AUTHORING: the triad's form wire refuses a blank at parse + * (`FieldRulePredicateWireSchema` in `@object-ui/types`' `form.zod.ts`, D1). + * The render verdict of a blank rule is unchanged (D3: the permissive one). + * A blank GATE is a different thing: `hasDeclaredPredicate` still folds it to + * "no gate" and only diagnoses it (D4) — it never reaches this function. + * + * Deliberately NOT in it: a rule the verdict did not need. `readonlyWhen` under + * a static `readonly: true`, and `requiredWhen` under `serverOwnedValue` or a + * static `required: true`, are short-circuited and never run, so they cannot + * fault. + */ +export interface FieldRuleFaults { + visibleWhen?: string; + readonlyWhen?: string; + requiredWhen?: string; +} + /** * Resolve the effective `{ visible, readonly, required }` state for a field * given its conditional rules and the live record. Each `*When` rule, when @@ -365,6 +409,8 @@ const REQUIRED_WHEN_ABSENT = false; * `"field 'amount'"`. The warning then reads * `visibleWhen of field 'amount' …`; without it, the rule * kind alone is reported. + * @returns The three verdicts, plus `faults` — see {@link FieldRuleFaults} for + * what a submit path reads from it and what it leaves out. */ export function resolveFieldRuleState( rules: { @@ -377,9 +423,16 @@ export function resolveFieldRuleState( previous?: Record, scope?: Record, fieldContext?: string, -): { visible: boolean; readonly: boolean; required: boolean } { - const diag = (rule: string): FieldPredicateDiagnostic => ({ +): { visible: boolean; readonly: boolean; required: boolean; faults: FieldRuleFaults } { + const faults: FieldRuleFaults = {}; + // `onFault` fires beside the built-in warning, not instead of it (`warn` + // stays on), so recording the report costs no diagnostic and no second + // engine call. + const diag = (rule: keyof FieldRuleFaults): FieldPredicateDiagnostic => ({ context: fieldContext ? `${rule} of ${fieldContext}` : rule, + onFault: (reason) => { + faults[rule] = reason; + }, }); const visible = @@ -427,5 +480,5 @@ export function resolveFieldRuleState( ) : REQUIRED_WHEN_ABSENT); - return { visible, readonly, required }; + return { visible, readonly, required, faults }; } diff --git a/packages/i18n/src/locales/ar.ts b/packages/i18n/src/locales/ar.ts index 6869358d5a..58f5d4a8f6 100644 --- a/packages/i18n/src/locales/ar.ts +++ b/packages/i18n/src/locales/ar.ts @@ -188,6 +188,7 @@ const ar = { submitFailed: "تعذّر الحفظ. يرجى المحاولة مرة أخرى.", uploadInFlight: "انتظر حتى ينتهي الرفع قبل الحفظ.", clearedOnHide: "تم مسح ما لم يعد ينطبق على القيم الحالية: {{fields}}", + visibleWhenFaulted: "تعذّر الإرسال: تعذّر تقييم قاعدة visibleWhen لـ {{fields}}. يجب إصلاح القاعدة قبل أن يصبح إرسال هذا النموذج ممكنًا.", discardTitle: "تجاهل التغييرات؟", discardMessage: "لديك تغييرات غير محفوظة. إذا أغلقت هذا النموذج الآن، ستفقد تعديلاتك.", // objectui#4024 — the create/edit dialog's `sr-only` accessible diff --git a/packages/i18n/src/locales/de.ts b/packages/i18n/src/locales/de.ts index cc5e8b1f01..1e13ee2f18 100644 --- a/packages/i18n/src/locales/de.ts +++ b/packages/i18n/src/locales/de.ts @@ -177,6 +177,7 @@ const de = { submitFailed: "Speichern fehlgeschlagen. Bitte versuchen Sie es erneut.", uploadInFlight: "Warten Sie, bis der Upload abgeschlossen ist, bevor Sie speichern.", clearedOnHide: "Geleert, was für die aktuellen Werte nicht mehr gilt: {{fields}}", + visibleWhenFaulted: "Absenden nicht möglich: Die visibleWhen-Regel von {{fields}} konnte nicht ausgewertet werden. Die Regel muss korrigiert werden, bevor dieses Formular abgesendet werden kann.", discardTitle: "Änderungen verwerfen?", discardMessage: "Sie haben ungespeicherte Änderungen. Wenn Sie dieses Formular jetzt schließen, gehen Ihre Bearbeitungen verloren.", // objectui#4024 — the create/edit dialog's `sr-only` accessible diff --git a/packages/i18n/src/locales/en.ts b/packages/i18n/src/locales/en.ts index 35b3923928..e9ba7ee249 100644 --- a/packages/i18n/src/locales/en.ts +++ b/packages/i18n/src/locales/en.ts @@ -231,6 +231,13 @@ const en = { // with `validation.formInvalidJoiner`. A list after a colon, so no word has // to agree with how many fields were cleared. clearedOnHide: 'Cleared — no longer applicable given the current values: {{fields}}', + // The refusal a record form raises at SUBMIT when a field's own + // `visibleWhen` could not be evaluated (objectui#8069, ADR-0137 D2 as ruled: + // the client refuses the one rule no server judges). It names the rule and + // the fields — `{{fields}}` is their labels joined with + // `validation.formInvalidJoiner` — because the fix is to the metadata, not + // to anything the person filling the form can type. + visibleWhenFaulted: "Can't submit: the visibleWhen rule of {{fields}} could not be evaluated. The rule must be fixed before this form can be submitted.", removeItem: 'Remove item', fieldRequired: 'This field is required', invalidFormat: 'Invalid format', diff --git a/packages/i18n/src/locales/es.ts b/packages/i18n/src/locales/es.ts index ed166ef06b..f39b9564d5 100644 --- a/packages/i18n/src/locales/es.ts +++ b/packages/i18n/src/locales/es.ts @@ -181,6 +181,7 @@ const es = { submitFailed: "No se pudo guardar. Inténtalo de nuevo.", uploadInFlight: "Espere a que finalice la carga antes de guardar.", clearedOnHide: "Se borró lo que ya no corresponde a los valores actuales: {{fields}}", + visibleWhenFaulted: "No se puede enviar: no se pudo evaluar la regla visibleWhen de {{fields}}. Hay que corregir la regla antes de poder enviar este formulario.", discardTitle: "¿Descartar los cambios?", discardMessage: "Tiene cambios sin guardar. Si cierra este formulario ahora, sus ediciones se perderán.", // objectui#4024 — the create/edit dialog's `sr-only` accessible diff --git a/packages/i18n/src/locales/fr.ts b/packages/i18n/src/locales/fr.ts index 29ba3867cb..e82132fe4d 100644 --- a/packages/i18n/src/locales/fr.ts +++ b/packages/i18n/src/locales/fr.ts @@ -177,6 +177,7 @@ const fr = { submitFailed: "Échec de l'enregistrement. Veuillez réessayer.", uploadInFlight: "Attendez la fin du téléversement avant de sauvegarder.", clearedOnHide: "Ce qui ne s'applique plus aux valeurs actuelles a été effacé : {{fields}}", + visibleWhenFaulted: "Envoi impossible : la règle visibleWhen de {{fields}} n'a pas pu être évaluée. La règle doit être corrigée avant que ce formulaire puisse être envoyé.", discardTitle: "Abandonner les modifications ?", discardMessage: "Vous avez des modifications non enregistrées. Si vous fermez ce formulaire maintenant, vos modifications seront perdues.", // objectui#4024 — the create/edit dialog's `sr-only` accessible diff --git a/packages/i18n/src/locales/ja.ts b/packages/i18n/src/locales/ja.ts index 60289fdb32..cfed49fce2 100644 --- a/packages/i18n/src/locales/ja.ts +++ b/packages/i18n/src/locales/ja.ts @@ -177,6 +177,7 @@ const ja = { submitFailed: "保存できませんでした。もう一度お試しください。", uploadInFlight: "アップロードが完了してから保存してください。", clearedOnHide: "現在の値に該当しなくなった項目をクリアしました: {{fields}}", + visibleWhenFaulted: "送信できません: {{fields}} の visibleWhen ルールを評価できませんでした。このフォームを送信するには、先にルールを修正する必要があります。", discardTitle: "変更を破棄しますか?", discardMessage: "保存されていない変更があります。このままフォームを閉じると編集内容は失われます。", // objectui#4024 — the create/edit dialog's `sr-only` accessible diff --git a/packages/i18n/src/locales/ko.ts b/packages/i18n/src/locales/ko.ts index a39c56ec33..5f022c7f3f 100644 --- a/packages/i18n/src/locales/ko.ts +++ b/packages/i18n/src/locales/ko.ts @@ -177,6 +177,7 @@ const ko = { submitFailed: "저장하지 못했습니다. 다시 시도해 주세요.", uploadInFlight: "업로드가 완료된 후에 저장하세요.", clearedOnHide: "현재 값에 더 이상 해당하지 않는 항목을 비웠습니다: {{fields}}", + visibleWhenFaulted: "제출할 수 없습니다: {{fields}}의 visibleWhen 규칙을 평가할 수 없습니다. 이 양식을 제출하려면 먼저 규칙을 수정해야 합니다.", discardTitle: "변경 내용을 버릴까요?", discardMessage: "저장하지 않은 변경 내용이 있습니다. 지금 이 양식을 닫으면 편집 내용이 사라집니다.", // objectui#4024 — the create/edit dialog's `sr-only` accessible diff --git a/packages/i18n/src/locales/pt.ts b/packages/i18n/src/locales/pt.ts index 3f208e15b9..0b0651d24b 100644 --- a/packages/i18n/src/locales/pt.ts +++ b/packages/i18n/src/locales/pt.ts @@ -176,6 +176,7 @@ const pt = { submitFailed: "Não foi possível salvar. Tente novamente.", uploadInFlight: "Aguarde a conclusão do upload antes de salvar.", clearedOnHide: "O que não se aplica mais aos valores atuais foi limpo: {{fields}}", + visibleWhenFaulted: "Não é possível enviar: não foi possível avaliar a regra visibleWhen de {{fields}}. A regra precisa ser corrigida antes que este formulário possa ser enviado.", discardTitle: "Descartar as alterações?", discardMessage: "Você tem alterações não salvas. Se fechar este formulário agora, suas edições serão perdidas.", // objectui#4024 — the create/edit dialog's `sr-only` accessible diff --git a/packages/i18n/src/locales/ru.ts b/packages/i18n/src/locales/ru.ts index 13004505dc..dd8fc5b877 100644 --- a/packages/i18n/src/locales/ru.ts +++ b/packages/i18n/src/locales/ru.ts @@ -187,6 +187,7 @@ const ru = { submitFailed: "Не удалось сохранить. Попробуйте ещё раз.", uploadInFlight: "Дождитесь завершения загрузки, прежде чем сохранять.", clearedOnHide: "Очищено то, что больше не применимо к текущим значениям: {{fields}}", + visibleWhenFaulted: "Невозможно отправить: не удалось вычислить правило visibleWhen для {{fields}}. Правило нужно исправить, прежде чем эту форму можно будет отправить.", discardTitle: "Отменить изменения?", discardMessage: "Есть несохранённые изменения. Если закрыть форму сейчас, правки будут потеряны.", // objectui#4024 — the create/edit dialog's `sr-only` accessible diff --git a/packages/i18n/src/locales/zh.ts b/packages/i18n/src/locales/zh.ts index 4c7b073954..165b4edbc8 100644 --- a/packages/i18n/src/locales/zh.ts +++ b/packages/i18n/src/locales/zh.ts @@ -184,6 +184,7 @@ const zh = { submitFailed: '保存失败,请重试。', uploadInFlight: '请等待上传完成后再保存。', clearedOnHide: '已清空不再适用于当前取值的字段:{{fields}}', + visibleWhenFaulted: '无法提交:{{fields}} 的 visibleWhen 规则无法求值。需先修正该规则,才能提交此表单。', removeItem: '移除项目', fieldRequired: '此字段为必填项', invalidFormat: '格式不正确', diff --git a/packages/plugin-form/src/WizardForm.tsx b/packages/plugin-form/src/WizardForm.tsx index a5758d5d6b..3bacaf4ba0 100644 --- a/packages/plugin-form/src/WizardForm.tsx +++ b/packages/plugin-form/src/WizardForm.tsx @@ -226,6 +226,12 @@ const useWizardTranslation = createSafeTranslation( 'publicForm.thankYouTitle': 'Thank you!', 'publicForm.loading': 'Loading form…', 'form.errorLoading': 'Error loading form', + // objectui#8069 — the cross-step gate's refusal of a field `visibleWhen` + // that could not be evaluated, and the locale's list separator it joins + // the field labels with. The same keys and values the form renderer reads. + 'form.visibleWhenFaulted': + "Can't submit: the visibleWhen rule of {{fields}} could not be evaluated. The rule must be fixed before this form can be submitted.", + 'validation.formInvalidJoiner': ', ', }, 'wizard.missingRequired', ); @@ -673,7 +679,7 @@ export const WizardForm: React.FC = ({ mode: schema.mode, // Feeds the "no persisted record" test that decides whether a runtime // `defaultValue` excuses a field from `required` (#4069). The wizard's - // own final-submit gate (`missingRequiredByStep`) reads the `required` + // own final-submit gate (`gateFinalSubmit`) reads the `required` // this produces, so it agrees with the renderer for free. recordId: schema.recordId, fieldLabel, @@ -764,10 +770,36 @@ export const WizardForm: React.FC = ({ * form renderer and the server's rule-validator use, so a conditionally * required/hidden field gets the same verdict from all three rather than a * second, divergent dialect. + * + * ## `faultedVisibleWhen` — the same pass, one more answer (objectui#8069) + * + * The fields whose `visibleWhen` could not be evaluated, grouped the same + * way, read off the very `resolveFieldRuleState` call that decides the + * required verdict (its `faults` report) — never a second evaluation. + * ADR-0137 D2 as ruled (Q1 = B, one judge per rule): a faulted `visibleWhen` + * refuses the final submit, because no server evaluates it; `requiredWhen` + * / `readonlyWhen` keep their direction here and are the server's to refuse. + * A step whose form the user never opened never ran the renderer's own + * refusal, which is why the cross-step gate asks as well. A stored BLANK + * `visibleWhen` is a fault like any other (ADR-0137 D2, `FieldRuleFaults`); + * a `section-divider` row's `visibleWhen` is its section's layout gate, not + * a field rule, and is not judged. + * + * ⚠️ This gate binds no `previous` in either mode (see the `serverOwnedValue` + * note below), so a `visibleWhen` reading `previous` is unevaluable here even + * on an EDIT wizard, and is refused. objectui#8069's ruling declined new + * binding for `previous` ("no new binding pipelines", its option D) and + * accepted the create-form half of that as the residual; the edit-wizard + * half is the same mechanism, accepted as a residual under the same refusal + * of option D on objectui#8069 — ⛔ so `persistedRecord` is deliberately NOT + * bound here. */ - const missingRequiredByStep = useCallback( - (record: Record): Map => { + const gateFinalSubmit = useCallback( + ( + record: Record, + ): { missing: Map; faultedVisibleWhen: Map } => { const out = new Map(); + const faultedVisibleWhen = new Map(); schema.sections.forEach((section, index) => { for (const field of buildSectionFields(section)) { const name = field?.name; @@ -800,6 +832,12 @@ export const WizardForm: React.FC = ({ predicateScope, `field '${name}'`, ); + if (state.faults.visibleWhen !== undefined && field.type !== 'section-divider') { + faultedVisibleWhen.set(index, [ + ...(faultedVisibleWhen.get(index) ?? []), + field.label || name, + ]); + } // View-level FormField.visibleOn hides the field the same way a // field-level visibleWhen does — fold it into the verdict exactly // like the form renderer (form.tsx) does, or the gate demands a @@ -816,7 +854,7 @@ export const WizardForm: React.FC = ({ out.set(index, [...(out.get(index) ?? []), (field as any).label || name]); } }); - return out; + return { missing: out, faultedVisibleWhen }; }, [schema.sections, buildSectionFields, isCreateWizard, predicateScope], ); @@ -860,10 +898,22 @@ export const WizardForm: React.FC = ({ return; } // Gate the submit on the FULL field set, not just this step's (see - // missingRequiredByStep) — then point the user at the first step that is + // gateFinalSubmit) — then point the user at the first step that is // short something, instead of letting the server answer with a 400 that // names fields the user cannot even see. - const missing = missingRequiredByStep(mergedData); + const { missing, faultedVisibleWhen } = gateFinalSubmit(mergedData); + // A faulted `visibleWhen` first (objectui#8069): it names the field and + // the rule, and nothing typed on any step can clear it. + if (faultedVisibleWhen.size > 0) { + setInvalidSteps(new Set(faultedVisibleWhen.keys())); + toast.error( + t('form.visibleWhenFaulted', { + fields: [...faultedVisibleWhen.values()].flat().join(t('validation.formInvalidJoiner')), + }), + ); + goToStep(Math.min(...faultedVisibleWhen.keys())); + return; + } if (missing.size > 0) { setInvalidSteps(new Set(missing.keys())); const labels = [...missing.values()].flat(); @@ -1070,7 +1120,7 @@ export const WizardForm: React.FC = ({ // Move to next step goToStep(currentStep + 1); } - }, [closedAffordance, formData, currentStep, isLastStep, schema, objectSchema, dataSource, perms, missingRequiredByStep, t, saveWithOcc, uploadGate.uploading, uploadGate.reason, reportUnsavedInput, recordSaved]); + }, [closedAffordance, formData, currentStep, isLastStep, schema, objectSchema, dataSource, perms, gateFinalSubmit, t, saveWithOcc, uploadGate.uploading, uploadGate.reason, reportUnsavedInput, recordSaved]); // Navigation const goToStep = useCallback((step: number) => { diff --git a/packages/plugin-form/src/wizardPredicateScope.test.tsx b/packages/plugin-form/src/wizardPredicateScope.test.tsx index 7a62f29fa5..f7d1ec1283 100644 --- a/packages/plugin-form/src/wizardPredicateScope.test.tsx +++ b/packages/plugin-form/src/wizardPredicateScope.test.tsx @@ -13,7 +13,7 @@ * * ## What was broken, and why it is worse than a render-path gap * - * `missingRequiredByStep` is not a render path. It re-checks the WHOLE declared + * `gateFinalSubmit` (was `missingRequiredByStep`) is not a render path. It re-checks the WHOLE declared * field set at final submit, because `allowSkip` lets the user jump past a step * whose fields were therefore never mounted, never registered and never * validated. Its own docstring states the contract it is built to keep: @@ -142,7 +142,7 @@ async function skipPastOwnerAndSubmit( await waitFor(() => expect(document.body.querySelector('[data-field="subject"]')).toBeTruthy()); fill('subject', 'S1'); // Straight to the last step — step 2 is never mounted, so react-hook-form has - // no rule registered for `owner` and only `missingRequiredByStep` can speak. + // no rule registered for `owner` and only `gateFinalSubmit` can speak. fireEvent.click(stepIndicator(2)); await waitFor(() => expect(document.body.querySelector('[data-field="notes"]')).toBeTruthy()); fill('notes', 'S3'); diff --git a/packages/plugin-form/src/wizardVisibleWhenFault-8069.test.tsx b/packages/plugin-form/src/wizardVisibleWhenFault-8069.test.tsx new file mode 100644 index 0000000000..e9cd794204 --- /dev/null +++ b/packages/plugin-form/src/wizardVisibleWhenFault-8069.test.tsx @@ -0,0 +1,145 @@ +/** + * 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#8069 — the WIZARD's cross-step gate refuses the final submit when a + * field's `visibleWhen` could not be evaluated, naming the field and the rule + * (ADR-0137 D2, ruled as Q1 = B: one judge per rule). + * + * The field under test sits on the MIDDLE step and the user jumps past it + * (`allowSkip`), so its step form never mounts and the renderer's own submit + * refusal never runs for it: only the cross-step gate can see the fault, which + * is exactly why the ruling names the gate. The observable is the one the + * sibling pin `wizardPredicateScope.test.tsx` uses — whether + * `dataSource.create` was called — plus the step the wizard lands on and the + * message it raises. + */ + +import React from 'react'; +import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; +import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react'; +import { registerAllFields } from '@object-ui/fields'; +import { toast } from '@object-ui/components'; +import { WizardForm } from './WizardForm'; + +registerAllFields(); + +let toastError: ReturnType; + +beforeEach(() => { + toastError = vi.spyOn(toast, 'error').mockImplementation(() => 'id' as never); + // Muted: the faults these cases provoke warn once each by design. + vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +const makeDataSource = (ownerRules: Record) => ({ + getObjectSchema: vi.fn().mockResolvedValue({ + name: 'case', + fields: { + subject: { type: 'text', label: 'Subject' }, + owner: { type: 'text', label: 'Owner', ...ownerRules }, + notes: { type: 'text', label: 'Notes' }, + }, + }), + create: vi.fn().mockResolvedValue({ id: 'case-1' }), + update: vi.fn(), + findOne: vi.fn(), +}); + +const fill = (name: string, value: string) => { + const input = document.body.querySelector(`[data-field="${name}"] input`); + if (!input) throw new Error(`field not rendered: ${name}`); + fireEvent.change(input, { target: { value } }); +}; + +const stepIndicator = (index: number) => + document.body.querySelectorAll('nav[aria-label="Progress"] button')[index]; + +/** 3-step create wizard; skip step 2 (`owner`), fill step 3, press Create. */ +async function skipPastOwnerAndSubmit( + ownerRules: Record, + ownerField: string | Record = 'owner', +) { + const dataSource = makeDataSource(ownerRules); + render( + , + ); + await waitFor(() => expect(document.body.querySelector('[data-field="subject"]')).toBeTruthy()); + fill('subject', 'S1'); + fireEvent.click(stepIndicator(2)); + await waitFor(() => expect(document.body.querySelector('[data-field="notes"]')).toBeTruthy()); + fill('notes', 'S3'); + expect(document.body.querySelector('[data-field="owner"]')).toBeNull(); + fireEvent.click(screen.getByRole('button', { name: /Create/i })); + return dataSource; +} + +async function expectSubmitted(dataSource: ReturnType) { + await waitFor(() => expect(dataSource.create).toHaveBeenCalledTimes(1)); +} + +describe('objectui#8069 — the wizard gate refuses a faulted visibleWhen', () => { + it('refuses the final submit, lands on the step holding the field, and names field and rule', async () => { + const ds = await skipPastOwnerAndSubmit({ visibleWhen: "'x' in no_such_root_8069.tags" }); + await waitFor(() => expect(document.body.querySelector('[data-field="owner"]')).toBeTruthy()); + expect(ds.create).not.toHaveBeenCalled(); + const messages: string[] = toastError.mock.calls.map((c: unknown[]) => String(c[0])); + expect(messages.some((m) => /visibleWhen rule of Owner could not be evaluated/.test(m))).toBe(true); + }); + + it('CONTROL — the same field with a rule that evaluates lets the submit through', async () => { + const ds = await skipPastOwnerAndSubmit({ visibleWhen: "record.subject == 'S1'" }); + await expectSubmitted(ds); + }); + + it('a STORED blank visibleWhen is refused by the gate too — ADR-0137 D2', async () => { + const ds = await skipPastOwnerAndSubmit({ visibleWhen: ' ' }); + await waitFor(() => expect(document.body.querySelector('[data-field="owner"]')).toBeTruthy()); + expect(ds.create).not.toHaveBeenCalled(); + const messages: string[] = toastError.mock.calls.map((c: unknown[]) => String(c[0])); + expect(messages.some((m) => /visibleWhen rule of Owner could not be evaluated/.test(m))).toBe(true); + }); + + it('control — a blank VIEW-level visibleWhen is a layout gate, not a field rule: the submit goes through', async () => { + // `sectionFields` drops a blank view-level predicate before it reaches the + // runtime field (`attachVisibility`), so the gate never sees it at all — + // "no gate", and nothing on this path refuses it. + const ds = await skipPastOwnerAndSubmit({}, { field: 'owner', visibleWhen: '' }); + await expectSubmitted(ds); + }); + + it('a faulted requiredWhen is NOT refused by the gate — the server refuses it (D2)', async () => { + const ds = await skipPastOwnerAndSubmit({ requiredWhen: "'x' in no_such_root_8069.tags" }); + await expectSubmitted(ds); + }); + + it('ACCEPTED RESIDUAL — a visibleWhen reading previous is refused on a CREATE wizard', async () => { + const ds = await skipPastOwnerAndSubmit({ visibleWhen: "previous.status == 'open'" }); + await waitFor(() => expect(document.body.querySelector('[data-field="owner"]')).toBeTruthy()); + expect(ds.create).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/react/src/__tests__/SchemaRenderer.disabledGateFaultDiagnostic.test.tsx b/packages/react/src/__tests__/SchemaRenderer.disabledGateFaultDiagnostic.test.tsx index b66f0e04a7..3843d146e0 100644 --- a/packages/react/src/__tests__/SchemaRenderer.disabledGateFaultDiagnostic.test.tsx +++ b/packages/react/src/__tests__/SchemaRenderer.disabledGateFaultDiagnostic.test.tsx @@ -533,10 +533,14 @@ describe('#6445 group 3 — nothing else became loud', () => { }); }); - it('the objectui#3862 EMPTY shapes stay silent AND stay enabled — an undeclared gate never reaches the evaluator', async () => { + it('the objectui#3862 EMPTY shapes stay enabled and never reach the evaluator — the BLANK ones are now diagnosed (objectui#8069)', async () => { // `hasDeclaredPredicate` still decides this one line earlier, so these rows - // are not faults; they are non-gates. A wiring that reported them would be - // reporting metadata that says nothing, once per empty spelling in the app. + // are not FAULTS: nothing reaches this file's enablement reporter. What + // changed is ADR-0137 D4 ("a gate predicate that is blank ... is + // diagnosed, never a silent `true`"): the fold itself now reports a BLANK + // one through core's `[blank]` channel — once per blank spelling, since + // that key-neutral fold names no node. `null` is not blank text and stays + // silent. The verdicts are objectui#3862's, unchanged. await inProduction((mount) => { const warn = spyWarn(); mount([ @@ -548,7 +552,11 @@ describe('#6445 group 3 — nothing else became loud', () => { ]); expect(shownCount()).toBe(5); expect(disabledProp()).toBe('absent'); - expect(allWarnings(warn)).toHaveLength(0); + expect(reports(warn)).toHaveLength(0); + const blank = allWarnings(warn).filter((m) => m.includes('[blank]')); + // `''` and `{ source: '' }` are one blank spelling; `' '` is another. + expect(blank).toHaveLength(2); + expect(allWarnings(warn)).toHaveLength(blank.length); }); }); diff --git a/packages/types/src/__tests__/base-schema-predicate-envelope-7530.test.ts b/packages/types/src/__tests__/base-schema-predicate-envelope-7530.test.ts index e03d8e7ffd..ace47cec20 100644 --- a/packages/types/src/__tests__/base-schema-predicate-envelope-7530.test.ts +++ b/packages/types/src/__tests__/base-schema-predicate-envelope-7530.test.ts @@ -40,11 +40,21 @@ * `ExpressionWireSchema` (zod) are the same union, so the two faces cannot * drift apart the way `hidden` did between #4581 and #7455. * 3. Reuse by REFERENCE -- the object arm each key's zod union carries IS - * `ExpressionWireSchema`, and so is the one `FormFieldSchema.visibleWhen` - * carries after the hoist. A faithful copy passes every value comparison; - * identity is the only check that distinguishes reuse from the second - * envelope type the ruling forbids (the insight `spec-subschema-parity` - * already wrote down for spec re-exports). + * `ExpressionWireSchema`, and so are the form's GATE legs (`visibleOn`, + * an option's `visibleWhen`). A faithful copy passes every value + * comparison; identity is the only check that distinguishes reuse from + * the second envelope type the ruling forbids (the insight + * `spec-subschema-parity` already wrote down for spec re-exports). + * The form's field-rule TRIAD (`visibleWhen` / `readonlyWhen` / + * `requiredWhen`) carries the SAME wire -- the very option schemas of + * `ExpressionWireSchema`, by reference -- with one blank check added + * (objectui#8069): ADR-0137 D1 refuses an authored blank predicate in a + * field-rule slot, as the spec's `EvaluatedExpressionInputSchema` does on + * `FieldSchema`, and the objectui#8069 ruling (card comment 5910115531, + * as the seat applied it after contract review 5915380177) handles + * objectui's own wire in the same round. What this pin protects is the + * ONE accepted SHAPE across the platform; a refused VALUE is not a second + * shape, and the case below pins exactly that line. * 4. Runtime -- the envelope `safeParse`s GREEN in full on all three keys, * with and without `dialect`, beside the string and boolean controls; and * the anti-overshoot guards: `{}` (no `source`), `{ dialect: 'cel' }`, @@ -86,6 +96,7 @@ import type { ExpressionWire } from '../expression'; import { BaseSchema as Mirror } from '../zod/base.zod'; import { ExpressionWireSchema } from '../zod/expression.zod'; import { FormFieldSchema, SelectOptionSchema } from '../zod/form.zod'; +import { EVALUATED_EXPRESSION_SOURCE_REQUIRED } from '@objectstack/spec/shared'; /* -- Type-level helpers ---------------------------------------------------- */ @@ -143,6 +154,8 @@ export const dialectlessEnvelopeIsAuthorable: BaseSchema = { /* -- Runtime companion (the zod mirror) ------------------------------------ */ const KEYS = ['visible', 'hidden', 'disabled'] as const; +/** The form's field-rule triad (objectui#8069) -- the three keys ADR-0137 D1 narrows. */ +const TRIAD = ['visibleWhen', 'readonlyWhen', 'requiredWhen'] as const; type PredicateKey = (typeof KEYS)[number]; const CEL_ENVELOPE = { dialect: 'cel', source: 'data.status == "draft"' }; @@ -196,14 +209,23 @@ describe('one envelope type across the package -- reuse pinned by reference (obj expect(envelopeArmOf(key)).toBe(ExpressionWireSchema); }); - it('the hoisted form legs carry the same object', () => { - expect(FormFieldSchema.shape.visibleWhen.unwrap()).toBe(ExpressionWireSchema); + it('the form GATE legs carry the same object', () => { expect(FormFieldSchema.shape.visibleOn.unwrap()).toBe(ExpressionWireSchema); - expect(FormFieldSchema.shape.readonlyWhen.unwrap()).toBe(ExpressionWireSchema); - expect(FormFieldSchema.shape.requiredWhen.unwrap()).toBe(ExpressionWireSchema); expect(SelectOptionSchema.shape.visibleWhen.unwrap()).toBe(ExpressionWireSchema); }); + it.each(TRIAD)('FormFieldSchema.shape.%s is the same wire SHAPE -- the same option schemas by reference -- plus a blank check (objectui#8069)', (key) => { + const wire = FormFieldSchema.shape[key].unwrap(); + // Not the bare const: it carries the ADR-0137 D1 check... + expect(wire).not.toBe(ExpressionWireSchema); + // ...over the SAME accepted shape: the union's arms are the const's own + // schemas, not copies -- which is what "one wire type" means. + expect(wire.options).toBe(ExpressionWireSchema.options); + expect(wire.options[0]).toBe(ExpressionWireSchema.options[0]); + expect(wire.options[1]).toBe(ExpressionWireSchema.options[1]); + expect(wire.constructor).toBe(ExpressionWireSchema.constructor); + }); + it('... and the form leg still parses the envelope -- the hoist moved the const, not its verdict', () => { expect(SelectOptionSchema.safeParse({ label: 'Draft', value: 'draft', visibleWhen: CEL_ENVELOPE }).success).toBe(true); expect(SelectOptionSchema.safeParse({ label: 'Draft', value: 'draft', visibleWhen: PREDICATE }).success).toBe(true); @@ -228,3 +250,60 @@ describe('one envelope type across the package -- reuse pinned by reference (obj expect(dialectlessEnvelopeIsAuthorable.visible).toEqual({ source: '${data.role === "admin"}' }); }); }); + +/* -- objectui#8069: the triad refuses a BLANK predicate at parse (ADR-0137 D1) -- */ + +/** The blank spellings -- a string, and an envelope whose `source` is one. */ +const BLANKS: Array<{ label: string; value: unknown }> = [ + { label: "''", value: '' }, + { label: 'whitespace', value: ' ' }, + { label: "{ source: '' }", value: { source: '' } }, + { label: "{ dialect: 'cel', source: ' ' }", value: { dialect: 'cel', source: ' ' } }, +]; + +const field = (key: (typeof TRIAD)[number], value: unknown) => ({ name: 'amount', [key]: value }); + +describe.each(TRIAD)('FormFieldSchema.%s refuses a blank predicate (objectui#8069, ADR-0137 D1)', (key) => { + it.each(BLANKS)('$label is refused AT THE KEY, with the spec\u2019s own sentence', ({ value }) => { + const result = FormFieldSchema.safeParse(field(key, value)); + expect(result.success).toBe(false); + if (!result.success) { + const atKey = result.error.issues.filter((issue) => issue.path.join('.') === key); + expect(atKey.map((issue) => issue.message)).toEqual([EVALUATED_EXPRESSION_SOURCE_REQUIRED]); + } + }); + + it('a non-blank string, a CEL envelope and a dialect-less envelope all parse in full', () => { + expect(FormFieldSchema.safeParse(field(key, PREDICATE)).success).toBe(true); + expect(FormFieldSchema.safeParse(field(key, "record.stage == 'won'")).success).toBe(true); + expect(FormFieldSchema.safeParse(field(key, CEL_ENVELOPE)).success).toBe(true); + expect(FormFieldSchema.safeParse(field(key, DIALECTLESS_ENVELOPE)).success).toBe(true); + }); + + it('the parsed value is the authored one, unchanged -- the check narrows the VALUE set, not the shape', () => { + const parsed = FormFieldSchema.parse(field(key, "record.stage == 'won'")); + expect(parsed[key]).toBe("record.stage == 'won'"); + expect(FormFieldSchema.parse(field(key, CEL_ENVELOPE))[key]).toEqual(CEL_ENVELOPE); + }); + + it('junk is still refused at the key -- the anti-overshoot guard, unchanged', () => { + expect(FormFieldSchema.safeParse(field(key, {})).success).toBe(false); + expect(FormFieldSchema.safeParse(field(key, 42)).success).toBe(false); + }); + + it('control -- an ABSENT rule is no rule and parses', () => { + expect(FormFieldSchema.safeParse({ name: 'amount' }).success).toBe(true); + }); +}); + +describe('control -- a blank GATE keeps the plain wire: "no gate", not a refusal (ADR-0137 D4, objectui#3850)', () => { + it.each(KEYS)("BaseSchema.%s: '' and a blank envelope still parse", (key) => { + expect(Mirror.safeParse({ type: 'test-component', [key]: '' }).success).toBe(true); + expect(Mirror.safeParse({ type: 'test-component', [key]: { dialect: 'cel', source: ' ' } }).success).toBe(true); + }); + + it("FormFieldSchema.visibleOn (the view-level gate) and an option's visibleWhen still parse a blank", () => { + expect(FormFieldSchema.safeParse({ name: 'amount', visibleOn: '' }).success).toBe(true); + expect(SelectOptionSchema.safeParse({ label: 'Draft', value: 'draft', visibleWhen: ' ' }).success).toBe(true); + }); +}); diff --git a/packages/types/src/__tests__/imported-defaults-8317.test.ts b/packages/types/src/__tests__/imported-defaults-8317.test.ts index d1f26daa99..f2e75946ec 100644 --- a/packages/types/src/__tests__/imported-defaults-8317.test.ts +++ b/packages/types/src/__tests__/imported-defaults-8317.test.ts @@ -119,6 +119,7 @@ import { checkDashboardWidgetMetricMeasureArity, } from '@objectstack/spec/ui'; import { FieldSchema as SpecFieldSchema, SelectOptionSchema as SpecSelectOptionSchema } from '@objectstack/spec/data'; +import { EvaluatedExpressionInputSchema as SpecEvaluatedExpressionInputSchema } from '@objectstack/spec/shared'; import { stripImportedDefaults } from '../zod/imported-defaults.js'; const HERE = dirname(fileURLToPath(import.meta.url)); @@ -221,6 +222,10 @@ const IMPORTED: Array = [ // objectui#8885: `ObjectChartSchema.drillDown` crosses this boundary. ['ChartDrillDownSchema', SpecChartDrillDownSchema], ['SelectOptionSchema', SpecSelectOptionSchema], + // objectui#8069: the form field-rule triad's blank check hands a predicate's + // TEXT to this schema and takes its verdict and sentence (ADR-0137 D1), so + // the crossing is measured here like every other one. + ['EvaluatedExpressionInputSchema', SpecEvaluatedExpressionInputSchema], // objectui#7265, the @object-ui/types slice: `UserFiltersSchema.fields[]` // stopped being a hand copy of the spec's field shape and now derives from // it, so that crossing is measured here like every other one. diff --git a/packages/types/src/__tests__/terminal-unknown-key-refusal-11073.test.ts b/packages/types/src/__tests__/terminal-unknown-key-refusal-11073.test.ts index 9b8ae2ebb4..a45c828d9f 100644 --- a/packages/types/src/__tests__/terminal-unknown-key-refusal-11073.test.ts +++ b/packages/types/src/__tests__/terminal-unknown-key-refusal-11073.test.ts @@ -154,7 +154,15 @@ function readFace(nodes: Set, specOwned: Set): FaceReading { }); // The OPEN twin: every closed arm rebuilt as an ordinary ZodObject from the same def. - const twin = z.union((d.options as Z[]).map((o) => (isStrict(o) && isClosed(o) ? new (z.ZodObject as any)({ ...o._zod.def }) : o)) as any); + // The union itself is rebuilt from ITS def too, so a check the union carries + // (objectui#8069: the form field-rule triad's blank refusal, a `.superRefine` + // over the shared predicate wire) stays on both sides and the differential + // still measures the arms alone — `z.union(options)` would drop `def.checks`, + // the loss `node-derivation.ts`'s `cloneWithDef` exists to prevent. + const twin = new (z.ZodUnion as any)({ + ...d, + options: (d.options as Z[]).map((o) => (isStrict(o) && isClosed(o) ? new (z.ZodObject as any)({ ...o._zod.def }) : o)), + }); const corpus: unknown[] = [undefined, null, '', 'x', 0, 1, true, [], {}, { zzUnknown: 1 }]; const samples = arms.filter((o) => o._zod.def.type === 'object').map((o) => sample(o) as Record); diff --git a/packages/types/src/zod/form.zod.ts b/packages/types/src/zod/form.zod.ts index b2e5156753..53a98c5b78 100644 --- a/packages/types/src/zod/form.zod.ts +++ b/packages/types/src/zod/form.zod.ts @@ -26,6 +26,7 @@ import { BaseSchema, SchemaNodeSchema } from './base.zod.js'; // and the form predicate keys below read ONE definition. Its docblock and // rationale moved with it. import { ExpressionWireSchema } from './expression.zod.js'; +import { EvaluatedExpressionInputSchema as SpecEvaluatedExpressionInputSchema } from '@objectstack/spec/shared'; import { stripImportedDefaults } from './imported-defaults.js'; import { closeStrictUnionArms } from './node-derivation.js'; @@ -924,6 +925,53 @@ function unresolvableFieldWidgetNamespaceMessage(id: string): string { ); } +/** + * The wire of the field-rule TRIAD — `visibleWhen` / `readonlyWhen` / + * `requiredWhen` on a form field: `ExpressionWireSchema` itself, with ONE check + * added. A predicate whose text is blank after trimming — `''`, whitespace, or + * an envelope whose `source` is either — is refused at parse (objectui#8069). + * + * This is ADR-0137 D1 ("a predicate slot accepts only what the engine can + * run"; an authored blank predicate is "refused at authoring") on objectui's + * own form wire, mirroring what `@objectstack/spec` already does to the same + * three keys on `FieldSchema` with `EvaluatedExpressionInputSchema`. The + * blankness is not restated here: the predicate's TEXT (the string itself, or + * an envelope's `source`) is handed to that spec schema, and its verdict and + * its sentence (`EVALUATED_EXPRESSION_SOURCE_REQUIRED`) are the refusal — so + * the two refusals of one mistake read alike and cannot drift apart (the + * objectui#8563 reason for chaining a spec rule rather than copying it). It is + * the authoring half of a pair: D2 refuses a blank that is + * already STORED at submit (the form renderer, the console's form page and the + * wizard's final gate refuse a blank `visibleWhen`; the server refuses a blank + * `requiredWhen` / `readonlyWhen`), so a blank is refused at the first place + * that can see it, never silently read as "no rule" (ADR-0137 refuses that + * reading for the triad in its Alternatives). + * + * ⚠️ Deliberately a REFINEMENT and not a second schema. The accepted SHAPE is + * exactly the one wire type objectui#7530 ruled for the whole platform — the + * very two option schemas `ExpressionWireSchema` holds, by reference, with the + * same output — and only the blank VALUE is taken out of it. The spec's + * `EvaluatedExpressionInputSchema` is not reused because it is a different + * shape: it rewrites a string into an envelope and narrows `dialect` to the + * spec enum. The pin is `base-schema-predicate-envelope-7530.test.ts`. + * + * ⛔ Not for gates. `BaseSchema`'s `visible` / `hidden` / `disabled`, this + * form's view-level `visibleOn` and an option's `visibleWhen` keep the plain + * wire: a blank GATE stays "no gate" plus a one-time diagnostic (ADR-0137 D4, + * objectui#3850 / #3960), and none of them is refused at submit. + */ +const FieldRulePredicateWireSchema = ExpressionWireSchema.superRefine((predicate, ctx) => { + // Only the TEXT is judged by the spec schema — never the envelope itself, + // whose `dialect` enum is narrower than this wire's and would refuse shapes + // objectui#7530 admits. A non-blank text always passes its string arm. + const verdict = stripImportedDefaults(SpecEvaluatedExpressionInputSchema).safeParse( + typeof predicate === 'string' ? predicate : predicate.source, + ); + if (!verdict.success) { + for (const issue of verdict.error.issues) ctx.addIssue({ code: 'custom', message: issue.message }); + } +}); + export const FormFieldSchema = z.object({ id: z.string().optional().describe('Field ID'), name: z.string().describe('Field name (form data path)'), @@ -949,11 +997,11 @@ export const FormFieldSchema = z.object({ readonly: z.boolean().optional().describe('Whether the field is read-only'), visibleOn: ExpressionWireSchema.optional() .describe('View-level visibility predicate (CEL) — ANDed with visibleWhen'), - visibleWhen: ExpressionWireSchema.optional() + visibleWhen: FieldRulePredicateWireSchema.optional() .describe('Field-level visibility rule (CEL) — field shown only when TRUE'), - readonlyWhen: ExpressionWireSchema.optional() + readonlyWhen: FieldRulePredicateWireSchema.optional() .describe('Field-level read-only rule (CEL)'), - requiredWhen: ExpressionWireSchema.optional() + requiredWhen: FieldRulePredicateWireSchema.optional() .describe('Field-level required rule (CEL)'), colSpan: z.number().optional().describe('Column span in grid layout (legacy — prefer span)'), span: z.enum(['auto', 'full']).optional().describe('Relative field width'),