From 8db3be0910b393f60c4ebe7c4048141759221031 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 07:50:37 +0000 Subject: [PATCH] fix(app-shell): all CEL-hosting inspectors block Save on parse faults (#4527) CelPredicateField has always reported its lint verdict through onLintChange, but ConditionBuilder and ConditionalFormattingEditor passed no listener, so a predicate that does not parse rendered its inline error and Save still saved -- the objectui#4306 defect, one editor family over. Both editors gain an optional callback surfacing their blocking-error count (severity 'error' only), and the inspectors above them aggregate and report through MetadataInspectorProps.onBlockingIssuesChange, the channel PR #4536 shipped. PageBlockInspector gates on a page block's visibleWhen; ViewVariantInspector gates on a view's conditionalFormatting rules, forwarded through ViewInspector on the scoped path. Counts are DERIVED from what they describe, never repaired by reset effects. ConditionBuilder's CEL editor exists only in raw mode and can vanish while its last verdict was an error -- the adopt effect flips back to rows in the same commit, unmounting it and cancelling its pending lint -- so the count is read as 0 whenever that editor is not mounted. ConditionalFormattingEditor keys a per-rule map and counts only rules that still exist: a shared counter would let whichever rule linted last overwrite the others, and a deleted rule's remembered error would wedge Save shut with no editor left to fix it in. Three sites named in the report are NOT wired: HookDefaultInspector, ActionDefaultInspector and the view's home panel are MetadataDefaultInspectorProps components whose contract has no blocking-issues member, and widgets.tsx's condition widget is a SchemaForm widget rather than an inspector. Wiring those needs a second contract decision plus edits to the hosts, so they are escalated rather than guessed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- .../inspector-family-cel-save-gate-4527.md | 13 ++ ...nditionalFormattingEditor.celGate.test.tsx | 172 ++++++++++++++++ .../ConditionalFormattingEditor.tsx | 51 +++++ .../ResourceEditPage.celGate.test.tsx | 153 ++++++++++++++ .../ConditionBuilder.celGate.test.tsx | 187 ++++++++++++++++++ .../inspectors/ConditionBuilder.tsx | 46 ++++- .../PageBlockInspector.celGate.test.tsx | 117 +++++++++++ .../inspectors/PageBlockInspector.tsx | 41 +++- .../inspectors/ViewInspector.tsx | 5 + .../ViewVariantInspector.celGate.test.tsx | 169 ++++++++++++++++ .../inspectors/ViewVariantInspector.tsx | 47 +++++ 11 files changed, 999 insertions(+), 2 deletions(-) create mode 100644 .changeset/inspector-family-cel-save-gate-4527.md create mode 100644 packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.celGate.test.tsx create mode 100644 packages/app-shell/src/views/metadata-admin/ResourceEditPage.celGate.test.tsx create mode 100644 packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.celGate.test.tsx create mode 100644 packages/app-shell/src/views/metadata-admin/inspectors/PageBlockInspector.celGate.test.tsx create mode 100644 packages/app-shell/src/views/metadata-admin/inspectors/ViewVariantInspector.celGate.test.tsx diff --git a/.changeset/inspector-family-cel-save-gate-4527.md b/.changeset/inspector-family-cel-save-gate-4527.md new file mode 100644 index 0000000000..0c78d103ba --- /dev/null +++ b/.changeset/inspector-family-cel-save-gate-4527.md @@ -0,0 +1,13 @@ +--- +'@object-ui/app-shell': patch +--- + +All CEL-hosting inspectors block Save on parse faults — a page block or formatting rule whose condition does not parse no longer saves + +`CelPredicateField` has always reported its lint verdict upward through `onLintChange`, but only the RLS policy editor listened. #4306 fixed the field inspector and shipped the channel the rest of the console needed — `MetadataInspectorProps.onBlockingIssuesChange`, named for blocking issues rather than for CEL precisely so the remaining sites could be wired against it. This wires the two shared editors that discarded the verdict: `ConditionBuilder` (the no-code predicate builder's raw-expression mode) and `ConditionalFormattingEditor` (one condition per formatting rule). Each gains an optional callback surfacing its blocking-error count, and the inspectors above them aggregate and report through the contract, so the host that owns Save refuses to write. + +Both counts are DERIVED from what they describe rather than repaired by reset effects, because the editors can vanish while their last verdict was "1 error" and nothing would ever retract it. `ConditionBuilder`'s CEL editor exists only in raw mode — an externally-changed value that round-trips as a simple predicate flips the builder back to rows in the same commit, unmounting the editor and cancelling its pending lint — so the count is read as 0 whenever the raw editor is not mounted. `ConditionalFormattingEditor` keys a per-rule map and counts only rules that still exist: a shared counter would let whichever rule linted last overwrite the others, so fixing one of two broken rules would hand back a writable Save while the other was still malformed, and a deleted rule's remembered error would wedge Save shut with no editor on screen to fix it. Only `severity: 'error'` counts; advisory warnings never block. + +Wired this way, `PageBlockInspector` gates the metadata editor's Save on a page block's `visibleWhen`, and `ViewVariantInspector` gates it on a view's `conditionalFormatting` rules (forwarded through `ViewInspector`, the scoped router — without that hop the channel stops one component short of the editor and the wiring is inert). + +Three of the sites named in the original report are NOT wired here, because the channel does not reach them: `HookDefaultInspector`, `ActionDefaultInspector` and the view's home panel are `MetadataDefaultInspectorProps` components, whose contract has no blocking-issues member, and `widgets.tsx`'s condition widget is a `SchemaForm` widget rather than an inspector at all. Wiring those needs a second contract decision and edits to the hosts, so they are left for a follow-up rather than guessed at. diff --git a/packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.celGate.test.tsx b/packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.celGate.test.tsx new file mode 100644 index 0000000000..587e6f4a29 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.celGate.test.tsx @@ -0,0 +1,172 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `ConditionalFormattingEditor` must REPORT its CEL verdicts upward, so the + * inspector above it can aggregate and the host that owns Save can refuse to + * publish a parse fault — objectui#4527, the same ungated-Save family as #4306. + * + * This editor mounts one `CelPredicateField` PER RULE, and passed no + * `onLintChange` to any of them: a formatting condition that does not parse + * rendered its inline error and Save stayed writable. `ViewVariantInspector` + * reaches this file. + * + * ## The two cases that decide the implementation + * + * - **Per-rule map, not a running total.** Rules lint independently and + * asynchronously, so one shared counter lets whichever reported last + * overwrite the others: fixing one of two broken rules would hand back a + * writable Save while the other was still malformed. A shared counter + * passes every single-rule case and fails only this one. + * - **Prune by derivation.** Deleting a rule unmounts its editor, which can + * never report `0` afterwards, so a remembered count would hold Save shut + * with no editor on screen to fix it. The total counts only rules that + * still exist (`i < drafts.length`) rather than being repaired by a reset + * effect (#4527 ruling item 2, mirroring #4306). + * + * The engine is stubbed deterministically — the live lint is + * `CelPredicateField.test.tsx`'s job; this suite tests WIRING. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react'; + +import { + ConditionalFormattingEditor, + type ConditionalFormattingRuleDraft, +} from './ConditionalFormattingEditor'; +import { __setCelFormulaLoader } from './celAuthoring'; + +afterEach(() => { + cleanup(); + __setCelFormulaLoader(undefined); +}); + +const DANGLING = /[*+\-/&|=<>]\s*$/; + +function stubEngine() { + __setCelFormulaLoader(() => + Promise.resolve({ + validateExpression: (_role: string, input: unknown) => { + const src = typeof input === 'string' ? input : String((input as { source?: string })?.source ?? ''); + return DANGLING.test(src) + ? { ok: false, errors: [{ message: 'Parse error: expression ends after an operator' }], warnings: [] } + : { ok: true, errors: [], warnings: [] }; + }, + introspectScope: () => ({ fields: ['status', 'amount'], roots: ['record'], functions: ['has'] }), + inferExpressionType: () => 'boolean' as const, + }), + ); +} + +const t = (k: string) => k; + +/** Controlled harness — the editor is controlled, so edits must round-trip. */ +function Harness({ + initial, + report, +}: { + initial: ConditionalFormattingRuleDraft[]; + report: (count: number) => void; +}) { + const [rules, setRules] = React.useState(initial); + return ( + + ); +} + +function renderEditor(initial: ConditionalFormattingRuleDraft[]) { + const report = vi.fn(); + render(); + const current = () => report.mock.calls.at(-1)?.[0] as number | undefined; + return { report, current }; +} + +/** The rule `i`'s CEL box (`CelPredicateField` renders a combobox TEXTAREA). */ +const ruleBox = (i: number) => + screen + .getByTestId(`cf-rule-${i}`) + .querySelector('[role="combobox"]') as HTMLTextAreaElement; + +describe('ConditionalFormattingEditor — blocking CEL issues are reported upward (#4527)', () => { + it('counts a formatting condition that does not parse', async () => { + stubEngine(); + const { current } = renderEditor([{ condition: "record.status == 'a'", style: {} }]); + fireEvent.change(ruleBox(0), { target: { value: 'record.status ==' } }); + await waitFor(() => expect(current()).toBe(1), { timeout: 3000 }); + }); + + it('reports a clean rule as zero, so a valid condition never blocks Save', async () => { + stubEngine(); + const { current } = renderEditor([{ condition: '', style: {} }]); + fireEvent.change(ruleBox(0), { target: { value: "record.status == 'overdue'" } }); + await waitFor(() => expect(current()).toBe(0), { timeout: 3000 }); + }); + + /** + * DECISIVE — the per-rule map. One shared counter passes both cases above + * and fails here: clearing ONE of two faulty rules would drop the total to 0 + * and hand back a Save button that still publishes the other fault. + */ + it('keeps each rule independent — fixing one leaves the other counted', async () => { + stubEngine(); + const { current } = renderEditor([ + { condition: '', style: {} }, + { condition: '', style: {} }, + ]); + + fireEvent.change(ruleBox(0), { target: { value: 'record.status ==' } }); + await waitFor(() => expect(current()).toBe(1), { timeout: 3000 }); + + fireEvent.change(ruleBox(1), { target: { value: 'record.amount >' } }); + await waitFor(() => expect(current()).toBe(2), { timeout: 3000 }); + + // Fix only the first — the second must still hold Save closed. + fireEvent.change(ruleBox(0), { target: { value: "record.status == 'overdue'" } }); + await waitFor(() => expect(current()).toBe(1), { timeout: 3000 }); + }); + + /** + * DECISIVE — the prune. Deleting the faulty rule unmounts its editor, so + * nothing will ever report `0` for it; a remembered count wedges Save shut. + */ + it('drops the count when the faulty rule is deleted, so Save cannot wedge shut', async () => { + stubEngine(); + const { current } = renderEditor([ + { condition: "record.status == 'ok'", style: {} }, + { condition: '', style: {} }, + ]); + + fireEvent.change(ruleBox(1), { target: { value: 'record.amount >' } }); + await waitFor(() => expect(current()).toBe(1), { timeout: 3000 }); + + fireEvent.click(screen.getByTestId('cf-remove-1')); + await waitFor(() => expect(screen.queryByTestId('cf-rule-1')).toBeNull(), { timeout: 3000 }); + await waitFor(() => expect(current()).toBe(0), { timeout: 3000 }); + }); + + it('stays mountable with no reporter attached — the prop is optional', async () => { + stubEngine(); + render( + {}} + objectName="invoice" + fieldNames={['status']} + t={t} + />, + ); + await waitFor( + () => expect(screen.getByText(/Parse error: expression ends after an operator/)).toBeInTheDocument(), + { timeout: 3000 }, + ); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.tsx b/packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.tsx index 447c6d1970..80ac1a3859 100644 --- a/packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.tsx +++ b/packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.tsx @@ -28,6 +28,7 @@ import * as React from 'react'; import { Button, Input, cn } from '@object-ui/components'; import { Plus, Trash2, ChevronUp, ChevronDown } from 'lucide-react'; import { CelPredicateField } from './CelPredicateField'; +import type { CelLintIssue } from './celAuthoring'; /** * Scope roots bound at RUNTIME for a row predicate, advertised to autocomplete. @@ -132,6 +133,17 @@ export interface ConditionalFormattingEditorProps { disabled?: boolean; /** i18n resolver (`(key) => string`). */ t: (key: string) => string; + /** + * Report how many BLOCKING author-time issues this editor is showing — rule + * conditions that do not parse (objectui#4527). The inspector above + * aggregates and hands the total to the host through + * `MetadataInspectorProps.onBlockingIssuesChange`, because the Save button + * belongs to the host, not here. + * + * Optional: mount sites with no Save to gate simply omit it. Fires whenever + * the aggregate changes, `0` when every rule is clean. + */ + onBlockingIssuesChange?: (count: number) => void; } /** A native color swatch + free-text value (hex / CSS / Tailwind), like the @@ -178,6 +190,7 @@ export function ConditionalFormattingEditor({ fieldNames, disabled, t, + onBlockingIssuesChange, }: ConditionalFormattingEditorProps) { // Normalize the persisted rules to the authoring shape once per input change. const drafts = React.useMemo( @@ -185,6 +198,43 @@ export function ConditionalFormattingEditor({ [rules], ); + /* ─── Blocking CEL verdicts → the inspector's aggregate (objectui#4527) ─── + * + * Errors are counted PER RULE rather than into one running total: each rule + * mounts its own `CelPredicateField`, and they lint independently and + * asynchronously, so a shared counter would let whichever reported last + * overwrite the others — fixing one of two broken rules would hand back a + * writable Save while the other was still malformed. + * + * The total is DERIVED against the rule list rather than repaired by a reset + * effect: a deleted rule's editor is gone and can never report `0` for + * itself, so counting a verdict it left behind would wedge Save shut with no + * editor on screen to fix it. Indices at or past the current length are + * therefore simply not counted. */ + const [celErrors, setCelErrors] = React.useState>({}); + const reportCel = React.useCallback((index: number, issues: CelLintIssue[]) => { + // Only `error` blocks Save; `warning` is advisory, matching #4306. + const errs = issues.filter((i) => i.severity === 'error').length; + setCelErrors((prev) => (prev[index] === errs ? prev : { ...prev, [index]: errs })); + }, []); + const ruleCount = drafts.length; + const blockingIssues = React.useMemo(() => { + let total = 0; + for (const [index, count] of Object.entries(celErrors)) { + if (Number(index) >= ruleCount) continue; // pruned: the rule is gone + total += count; + } + return total; + }, [celErrors, ruleCount]); + // Held in a ref so an unmemoized parent callback cannot re-fire the effect. + const onBlockingIssuesChangeRef = React.useRef(onBlockingIssuesChange); + React.useEffect(() => { + onBlockingIssuesChangeRef.current = onBlockingIssuesChange; + }); + React.useEffect(() => { + onBlockingIssuesChangeRef.current?.(blockingIssues); + }, [blockingIssues]); + const commit = (next: ConditionalFormattingRuleDraft[]) => onChange(next); const setRule = (i: number, patch: Partial) => { @@ -283,6 +333,7 @@ export function ConditionalFormattingEditor({ scope="flattened" roots={ROW_PREDICATE_ROOTS} onChange={(v) => setRule(i, { condition: v })} + onLintChange={(issues) => reportCel(i, issues)} t={t} id={`cf-condition-${i}`} /> diff --git a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.celGate.test.tsx b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.celGate.test.tsx new file mode 100644 index 0000000000..5e42beae84 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.celGate.test.tsx @@ -0,0 +1,153 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Save must refuse a page block whose `visibleWhen` does not parse — the host + * half of objectui#4527, end to end in the metadata editor that owns the + * button. + * + * #4536 wired this host to `MetadataInspectorProps.onBlockingIssuesChange` and + * proved the mechanism in the OTHER host (`DataPillar`, Studio's Data pillar). + * Nothing proved it here: no test in this repo renders + * `MetadataResourceEditPage` at all. So this suite pins the path this card + * actually creates — block inspector -> ConditionBuilder verdict -> the + * contract -> this host's Save button — for a non-`object` type. + * + * ## What is stubbed, and why that keeps the assertion honest + * + * Only the page CANVAS is stubbed, and only to emit the selection: turning a + * click on a rendered block into a `MetadataSelection` is `PageBlockCanvas`'s + * own concern and is tested there. Everything this card touches is real — the + * registered `PageBlockInspector`, the real `ConditionBuilder`, the real + * contract prop, and this host's real gating — so a regression anywhere along + * that path fails here. + * + * The CEL engine is stubbed deterministically, as in the #4306 suites. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; +import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; + +const PAGE = { + name: 'home', + label: 'Home', + type: 'home', + template: 'default', + regions: [{ name: 'main', components: [{ type: 'text', id: 'b1', visibleWhen: 'record.amount > 10' }] }], +}; + +const mockClient = { + list: vi.fn(async () => []), + listDrafts: vi.fn(async () => []), + layered: vi.fn(async () => ({ effective: PAGE, code: PAGE, editable: true })), + getDraft: vi.fn(async () => null), + get: vi.fn(async () => null), + saveDraft: vi.fn(async () => ({})), +}; + +vi.mock('./useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + useMetadataClient: () => mockClient, + // `allowOrgOverride` is what makes the item writable, and therefore what + // makes a Save button exist at all. + useMetadataTypes: () => ({ + entries: [{ type: 'page', name: 'page', label: 'Page', allowOrgOverride: true }], + }), + }; +}); + +import { MetadataResourceEditPage } from './ResourceEditPage'; +import { registerBuiltinInspectors } from './inspectors'; +import { registerMetadataPreview, getMetadataPreview, type MetadataSelection } from './preview-registry'; +import { __setCelFormulaLoader } from './celAuthoring'; + +registerBuiltinInspectors(); + +const BLOCK_PATH = 'regions[0].components[0]'; +const DANGLING = /[*+\-/&|=<>]\s*$/; + +function stubEngine() { + __setCelFormulaLoader(() => + Promise.resolve({ + validateExpression: (_role: string, input: unknown) => { + const src = typeof input === 'string' ? input : String((input as { source?: string })?.source ?? ''); + return DANGLING.test(src) + ? { ok: false, errors: [{ message: 'Parse error: expression ends after an operator' }], warnings: [] } + : { ok: true, errors: [], warnings: [] }; + }, + introspectScope: () => ({ fields: ['amount'], roots: ['record'], functions: ['has'] }), + inferExpressionType: () => 'boolean' as const, + }), + ); +} + +/** Canvas stand-in: emits the block selection the real canvas emits on click. */ +function StubPageCanvas({ + onSelectionChange, +}: { + onSelectionChange?: (next: MetadataSelection | null) => void; +}) { + return ( + + ); +} + +const realPagePreview = getMetadataPreview('page'); + +beforeEach(() => { + stubEngine(); + registerMetadataPreview('page', StubPageCanvas as never); +}); + +afterEach(() => { + cleanup(); + __setCelFormulaLoader(undefined); + if (realPagePreview) registerMetadataPreview('page', realPagePreview); +}); + +/** The Save button is an icon button identified by its title. */ +const saveButton = () => + screen.getByRole('button', { name: /Save \(⌘S\)|Fix the CEL syntax errors before saving\./ }); + +/** Open the editor, select the block, and switch its visibility control to raw CEL. */ +async function openBlockCel() { + render( + + + , + ); + fireEvent.click(await screen.findByRole('button', { name: 'select the block' })); + fireEvent.click(await screen.findByText('Expression')); + return screen.getAllByRole('combobox').find((el) => el.tagName === 'TEXTAREA') as HTMLTextAreaElement; +} + +describe('MetadataResourceEditPage — Save is gated on the block inspector’s CEL verdict (#4527)', () => { + it("refuses the card's repro: a dangling operator disables Save", async () => { + const box = await openBlockCel(); + + // A valid predicate first — this dirties the draft (so Save is live at all) + // and pins the must-not-change half: a good predicate never blocks. + fireEvent.change(box, { target: { value: 'record.amount > 20' } }); + await waitFor(() => expect(saveButton()).toBeEnabled(), { timeout: 4000 }); + + fireEvent.change(box, { target: { value: 'record.amount >' } }); + await waitFor(() => expect(saveButton()).toBeDisabled(), { timeout: 4000 }); + expect(saveButton()).toHaveAttribute('title', 'Fix the CEL syntax errors before saving.'); + }); + + it('re-enables Save once the predicate parses again', async () => { + const box = await openBlockCel(); + + fireEvent.change(box, { target: { value: 'record.amount >' } }); + await waitFor(() => expect(saveButton()).toBeDisabled(), { timeout: 4000 }); + + fireEvent.change(box, { target: { value: 'record.amount > 20' } }); + await waitFor(() => expect(saveButton()).toBeEnabled(), { timeout: 4000 }); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.celGate.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.celGate.test.tsx new file mode 100644 index 0000000000..ae06c3d42e --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.celGate.test.tsx @@ -0,0 +1,187 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `ConditionBuilder` must REPORT its raw-editor CEL verdict upward, so the + * inspector above it can aggregate and the host that owns Save can refuse to + * publish a parse fault — objectui#4527, the same ungated-Save family as #4306. + * + * `CelPredicateField` has always reported through `onLintChange`; this mount + * site passed no listener at all, so a predicate that does not parse rendered + * its inline error and Save stayed writable. Four inspectors reach this file. + * + * ## Why the PRUNE case is the decisive one + * + * The raw CEL editor is mounted ONLY in raw mode — the row builder has no + * `CelPredicateField` at all. So the editor can vanish while its last verdict + * was "1 error": the adopt effect (an externally-changed `value` that round + * trips as a simple predicate) flips `raw` false in the same commit, which + * unmounts the editor and cancels its pending debounced lint. Nothing will + * ever report `0` for it again. A naive implementation that just remembers the + * last count therefore WEDGES Save shut with no editor left on screen to fix + * it — which is why the count is DERIVED (`raw ? count : 0`) rather than + * repaired by a reset effect (#4527 ruling item 2, mirroring #4306). + * + * The engine is stubbed deterministically — linting against the REAL engine is + * `CelPredicateField.test.tsx`'s job; this suite tests WIRING, so the stub + * treats "ends on a dangling binary operator" as the parse fault. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react'; + +import { ConditionBuilder } from './ConditionBuilder'; +import { __setCelFormulaLoader } from '../celAuthoring'; + +afterEach(() => { + cleanup(); + __setCelFormulaLoader(undefined); +}); + +/** The card's repro shape: a source left hanging on a binary operator. */ +const DANGLING = /[*+\-/&|=<>]\s*$/; + +function stubEngine(opts: { asWarning?: boolean } = {}) { + __setCelFormulaLoader(() => + Promise.resolve({ + validateExpression: (_role: string, input: unknown) => { + const src = typeof input === 'string' ? input : String((input as { source?: string })?.source ?? ''); + if (!DANGLING.test(src)) return { ok: true, errors: [], warnings: [] }; + const issue = { message: 'Parse error: expression ends after an operator' }; + // The count is severity-gated: only `error` blocks Save, matching the + // RLS editor and #4306. A warning-only verdict must report 0. + return opts.asWarning + ? { ok: true, errors: [], warnings: [issue] } + : { ok: false, errors: [issue], warnings: [] }; + }, + introspectScope: () => ({ + fields: ['status', 'amount'], + roots: ['record', 'user'], + functions: ['has'], + }), + inferExpressionType: () => 'boolean' as const, + }), + ); +} + +/** `CelPredicateField` renders its editor as a combobox TEXTAREA. */ +const rawEditor = () => + screen.getAllByRole('combobox').find((el) => el.tagName === 'TEXTAREA') as HTMLTextAreaElement; + +/** + * Controlled harness — `ConditionBuilder` is a controlled editor, so the + * committed CEL must round-trip or the second keystroke reverts the first. + * `setExternally` simulates the parent replacing the value from somewhere + * OTHER than this editor (a selection change, an edit elsewhere), which is + * what drives the adopt effect and, with it, the prune case. + */ +function Harness({ + initial = '', + report, + external, + pass = true, +}: { + initial?: string; + report: (count: number) => void; + external?: string; + pass?: boolean; +}) { + const [value, setValue] = React.useState(initial); + return ( + <> + {external !== undefined && ( + + )} + + + ); +} + +/** Render, switch into the raw CEL editor, and expose the count a host would hold. */ +function renderRaw(opts: { initial?: string; external?: string; pass?: boolean } = {}) { + const report = vi.fn(); + render(); + fireEvent.click(screen.getByText('Expression')); + const current = () => report.mock.calls.at(-1)?.[0] as number | undefined; + return { report, current }; +} + +describe('ConditionBuilder — the raw CEL editor reports blocking issues (#4527)', () => { + it("counts the card's repro: a dangling operator in the expression editor", async () => { + stubEngine(); + const { current } = renderRaw(); + fireEvent.change(rawEditor(), { target: { value: 'record.status ==' } }); + await waitFor(() => expect(current()).toBe(1), { timeout: 3000 }); + }); + + it('reports a clean predicate as zero, so a valid expression never blocks Save', async () => { + stubEngine(); + const { current } = renderRaw(); + fireEvent.change(rawEditor(), { target: { value: "record.status == 'open'" } }); + await waitFor(() => expect(current()).toBe(0), { timeout: 3000 }); + }); + + it('re-enables Save when the author fixes the expression', async () => { + stubEngine(); + const { current } = renderRaw(); + const box = rawEditor(); + fireEvent.change(box, { target: { value: 'record.amount >' } }); + await waitFor(() => expect(current()).toBe(1), { timeout: 3000 }); + fireEvent.change(box, { target: { value: 'record.amount > 10' } }); + await waitFor(() => expect(current()).toBe(0), { timeout: 3000 }); + }); + + it('counts only ERRORS — an advisory warning never blocks Save', async () => { + stubEngine({ asWarning: true }); + const { current } = renderRaw(); + fireEvent.change(rawEditor(), { target: { value: 'record.status ==' } }); + // Prove a verdict was actually produced and rendered, so the `0` below is + // "the warning was not counted" rather than "no lint ever ran". + await waitFor( + () => expect(screen.getByText(/Parse error: expression ends after an operator/)).toBeInTheDocument(), + { timeout: 3000 }, + ); + expect(current()).toBe(0); + }); + + /** + * DECISIVE — the prune. An externally-replaced value that round-trips as a + * simple predicate flips the builder out of raw mode in the same commit, + * unmounting the CEL editor and cancelling its pending lint. Nothing reports + * `0` afterwards, so a remembered count would hold Save shut forever. + */ + it('drops the verdict when the raw editor unmounts, so Save cannot wedge shut', async () => { + stubEngine(); + const { current } = renderRaw({ external: "record.status == 'open'" }); + + fireEvent.change(rawEditor(), { target: { value: 'record.status ==' } }); + await waitFor(() => expect(current()).toBe(1), { timeout: 3000 }); + + fireEvent.click(screen.getByRole('button', { name: 'set externally' })); + // The raw editor is gone — the row builder took over. + await waitFor(() => expect(screen.queryByText('Expression')).toBeInTheDocument(), { timeout: 3000 }); + await waitFor(() => expect(current()).toBe(0), { timeout: 3000 }); + }); + + it('stays mountable with no reporter attached — the prop is optional', async () => { + stubEngine(); + const { report } = renderRaw({ pass: false }); + fireEvent.change(rawEditor(), { target: { value: 'record.status ==' } }); + // The inline error still renders; nothing throws, nothing is reported. + await waitFor( + () => expect(screen.getByText(/Parse error: expression ends after an operator/)).toBeInTheDocument(), + { timeout: 3000 }, + ); + expect(report).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx index 724a01e962..48dfcd46ff 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx @@ -23,6 +23,7 @@ import { import { Plus, X, Code2, ListFilter } from 'lucide-react'; import { useObjectFields } from '../previews/useObjectFields'; import { CelPredicateField } from '../CelPredicateField'; +import type { CelLintIssue } from '../celAuthoring'; import { t, useMetadataLocale } from '../i18n'; type Op = '==' | '!=' | '>' | '<' | '>=' | '<=' | 'truthy' | 'falsy'; @@ -109,7 +110,7 @@ function initFrom(value: string): { rows: Row[]; join: '&&' | '||'; raw: boolean return { rows: [], join: '&&', raw: !!value }; } -export function ConditionBuilder({ label, value, onCommit, objectName, fields: fieldsProp, disabled }: { +export function ConditionBuilder({ label, value, onCommit, objectName, fields: fieldsProp, disabled, onBlockingIssuesChange }: { label?: string; value: string; onCommit: (cel: string) => void; @@ -118,6 +119,17 @@ export function ConditionBuilder({ label, value, onCommit, objectName, fields: f * when omitted, fields are loaded from `objectName`. */ fields?: Array<{ name: string; label?: string; hidden?: boolean }>; disabled?: boolean; + /** + * Report how many BLOCKING author-time issues this editor is showing — a CEL + * predicate that does not parse (objectui#4527). The inspector above + * aggregates these and hands the total to the host through + * `MetadataInspectorProps.onBlockingIssuesChange`, because the Save button + * belongs to the host, not here. + * + * Optional: mount sites with no Save to gate simply omit it. Fires whenever + * the aggregate changes, `0` when everything is clean. + */ + onBlockingIssuesChange?: (count: number) => void; }) { const { fields: hookFields } = useObjectFields(objectName); const fields = fieldsProp ?? hookFields; @@ -165,6 +177,37 @@ export function ConditionBuilder({ label, value, onCommit, objectName, fields: f emit(nextRows, nextJoin); }; + /* ─── Blocking CEL verdicts → the inspector's aggregate (objectui#4527) ─── + * + * The raw-expression editor is this component's ONLY CEL site — the row + * builder compiles rows itself and mounts no `CelPredicateField` — so the + * aggregate is that one editor's error count, DERIVED against the mode + * rather than repaired by a reset effect. + * + * Deriving is what prevents the wedge. The raw editor exists only while + * `raw` is true, and it can vanish while its last verdict was "1 error": + * the adopt effect above flips `raw` false in the same commit when an + * externally-changed value round-trips as a simple predicate, which + * unmounts the editor and cancels its pending debounced lint. Nothing will + * ever report `0` for it again, so a remembered count would hold Save shut + * with no editor left on screen to fix it. */ + const [celErrors, setCelErrors] = React.useState(0); + const reportCel = React.useCallback((issues: CelLintIssue[]) => { + // Only `error` blocks Save; `warning` is advisory (typo / blast-radius), + // matching the RLS editor and objectui#4306. + const errs = issues.filter((i) => i.severity === 'error').length; + setCelErrors((prev) => (prev === errs ? prev : errs)); + }, []); + const blockingIssues = raw ? celErrors : 0; + // Held in a ref so an unmemoized parent callback cannot re-fire the effect. + const onBlockingIssuesChangeRef = React.useRef(onBlockingIssuesChange); + React.useEffect(() => { + onBlockingIssuesChangeRef.current = onBlockingIssuesChange; + }); + React.useEffect(() => { + onBlockingIssuesChangeRef.current?.(blockingIssues); + }, [blockingIssues]); + const compiled = compile(rows, join); if (raw) { @@ -185,6 +228,7 @@ export function ConditionBuilder({ label, value, onCommit, objectName, fields: f label={tLocal('engine.condition.celLabel')} value={value} onChange={(v) => { lastEmitted.current = v; onCommit(v); }} + onLintChange={reportCel} disabled={disabled} placeholder="record.status != 'done' && user.isAdmin" objectName={objectName} diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/PageBlockInspector.celGate.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/PageBlockInspector.celGate.test.tsx new file mode 100644 index 0000000000..275044ccc4 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/PageBlockInspector.celGate.test.tsx @@ -0,0 +1,117 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The Page block inspector must REPORT its blocking CEL verdict through the + * `MetadataInspectorProps.onBlockingIssuesChange` channel #4536 shipped, so the + * host that owns Save can refuse to publish a parse fault — objectui#4527. + * + * The block inspector authors `visibleWhen` through {@link ConditionBuilder} + * (#3229), whose raw-expression editor is a `CelPredicateField`. That mount + * site passed no `onLintChange`, so `record.amount >` rendered its inline parse + * error and Save stayed writable — the #4306 defect, one inspector over. + * + * What is pinned here is the INSPECTOR half: the count reaching the contract. + * The engine is stubbed deterministically; the live lint is + * `CelPredicateField.test.tsx`'s job, and the editor-side derivation is pinned + * in {@link file://./ConditionBuilder.celGate.test.tsx}. + * + * FIXTURE DISCIPLINE (the sibling suite's method): no envelope is hand-written + * — the fixture is authored input fed through `PageSchema.parse`, so it cannot + * drift from the spec. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react'; +import { PageSchema } from '@objectstack/spec/ui'; + +import { PageBlockInspector } from './PageBlockInspector'; +import { __setCelFormulaLoader } from '../celAuthoring'; + +afterEach(() => { + cleanup(); + __setCelFormulaLoader(undefined); +}); + +const BLOCK_PATH = 'regions[0].components[0]'; +const DANGLING = /[*+\-/&|=<>]\s*$/; + +function stubEngine() { + __setCelFormulaLoader(() => + Promise.resolve({ + validateExpression: (_role: string, input: unknown) => { + const src = typeof input === 'string' ? input : String((input as { source?: string })?.source ?? ''); + return DANGLING.test(src) + ? { ok: false, errors: [{ message: 'Parse error: expression ends after an operator' }], warnings: [] } + : { ok: true, errors: [], warnings: [] }; + }, + introspectScope: () => ({ fields: ['amount'], roots: ['record'], functions: ['has'] }), + inferExpressionType: () => 'boolean' as const, + }), + ); +} + +function pageDraft(block: Record): Record { + return PageSchema.parse({ + name: 'home', + label: 'Home', + type: 'home', + template: 'default', + regions: [{ name: 'main', components: [{ type: 'text', id: 'b1', ...block }] }], + }) as unknown as Record; +} + +/** Stateful harness — the inspector is controlled, so patches must round-trip. */ +function Harness({ initial, report }: { initial: Record; report: (n: number) => void }) { + const [draft, setDraft] = React.useState(initial); + return ( + setDraft((d) => ({ ...d, ...patch }))} + onClearSelection={() => {}} + onSelectionChange={() => {}} + onBlockingIssuesChange={report} + readOnly={false} + locale={'en-US'} + /> + ); +} + +/** Render and switch the visibility builder into its raw CEL editor. */ +function renderRaw(block: Record = { visibleWhen: 'record.amount > 10' }) { + const report = vi.fn(); + render(); + fireEvent.click(screen.getByText('Expression')); + const box = screen.getAllByRole('combobox').find((el) => el.tagName === 'TEXTAREA') as HTMLTextAreaElement; + const current = () => report.mock.calls.at(-1)?.[0] as number | undefined; + return { report, current, box }; +} + +describe('PageBlockInspector — blocking CEL issues reach the host (#4527)', () => { + it("counts the card's repro: a dangling operator in `visibleWhen`", async () => { + stubEngine(); + const { current, box } = renderRaw(); + fireEvent.change(box, { target: { value: 'record.amount >' } }); + await waitFor(() => expect(current()).toBe(1), { timeout: 3000 }); + }); + + it('reports a clean predicate as zero, so a valid condition never blocks Save', async () => { + stubEngine(); + const { current, box } = renderRaw(); + fireEvent.change(box, { target: { value: 'record.amount > 20' } }); + await waitFor(() => expect(current()).toBe(0), { timeout: 3000 }); + }); + + it('re-enables Save when the author fixes the predicate', async () => { + stubEngine(); + const { current, box } = renderRaw(); + fireEvent.change(box, { target: { value: 'record.amount >' } }); + await waitFor(() => expect(current()).toBe(1), { timeout: 3000 }); + fireEvent.change(box, { target: { value: 'record.amount > 20' } }); + await waitFor(() => expect(current()).toBe(0), { timeout: 3000 }); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/PageBlockInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/PageBlockInspector.tsx index f67fe58fb2..adfd0655e1 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/PageBlockInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/PageBlockInspector.tsx @@ -306,7 +306,7 @@ export function writeSiblings(root: Record, hops: Hop[], nextSi return { [topKey]: next[topKey] }; } -export function PageBlockInspector({ selection, draft, onPatch, onClearSelection, onSelectionChange, locale, readOnly }: MetadataInspectorProps) { +export function PageBlockInspector({ selection, draft, onPatch, onClearSelection, onSelectionChange, onBlockingIssuesChange, locale, readOnly }: MetadataInspectorProps) { // Slotted record page: selection ids are `slot::` and address // `draft.slots.` (a single component is normalised to a 1-element array). // `slot::` optionally followed by a nested sub-path within the @@ -349,6 +349,44 @@ export function PageBlockInspector({ selection, draft, onPatch, onClearSelection ? readSiblings(draft, hops) : null; + /* ─── Blocking CEL verdicts → the host's Save gate (objectui#4527) ───── + * + * The visibility builder below authors `visibleWhen` through a + * `CelPredicateField`; a predicate that does not parse must not be + * saveable, let alone publishable as the live page definition (#4306's + * defect, one inspector over). + * + * The count is STAMPED with the block it describes and the mismatch is + * read as 0 at aggregation time, so a verdict that lands after the + * selection moved cannot gate the block now on screen. Declared above the + * `!block` early return — these are hooks, and their order must not depend + * on the selection resolving to a live block. */ + const [celErrors, setCelErrors] = React.useState<{ block: string; count: number }>({ + block: selection.id, + count: 0, + }); + const reportCel = React.useCallback( + (count: number) => { + setCelErrors((prev) => { + // A verdict that arrives after the selection moved describes the block + // now on screen, not the one it was queued for. + if (prev.block !== selection.id) return { block: selection.id, count }; + if (prev.count === count) return prev; + return { block: selection.id, count }; + }); + }, + [selection.id], + ); + const blockingIssues = celErrors.block === selection.id ? celErrors.count : 0; + // Held in a ref so an unmemoized host callback cannot re-fire the effect. + const onBlockingIssuesChangeRef = React.useRef(onBlockingIssuesChange); + React.useEffect(() => { + onBlockingIssuesChangeRef.current = onBlockingIssuesChange; + }); + React.useEffect(() => { + onBlockingIssuesChangeRef.current?.(blockingIssues); + }, [blockingIssues]); + if ((!slotMatch && !hops) || !block) { return ( @@ -614,6 +652,7 @@ export function PageBlockInspector({ selection, draft, onPatch, onClearSelection onCommit={(v) => patch({ visibleWhen: writeExpressionSource(block.visibleWhen, v) })} objectName={pageObject} disabled={readOnly} + onBlockingIssuesChange={reportCel} /> {blockHasConfig(block.type) && ( diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ViewInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ViewInspector.tsx index 7ffa18afae..ae6be5df1b 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/ViewInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ViewInspector.tsx @@ -39,6 +39,11 @@ export function ViewInspector(props: MetadataInspectorProps) { onPatch={props.onPatch} onSelectionChange={props.onSelectionChange} onClearSelection={props.onClearSelection} + // The host's blocking-issues channel must survive this router, or the + // variant inspector's CEL verdicts stop one component short of Save + // and the wiring is inert (objectui#4527). The default (home) path + // below has no such prop to forward — see ViewVariantInspectorProps. + onBlockingIssuesChange={props.onBlockingIssuesChange} readOnly={props.readOnly} locale={props.locale} variantKey={binding?.storeKey ?? 'list'} diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ViewVariantInspector.celGate.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ViewVariantInspector.celGate.test.tsx new file mode 100644 index 0000000000..c592c74e5f --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ViewVariantInspector.celGate.test.tsx @@ -0,0 +1,169 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The View variant inspector must REPORT its blocking CEL verdicts through the + * `MetadataInspectorProps.onBlockingIssuesChange` channel #4536 shipped, so the + * host that owns Save can refuse to publish a parse fault — objectui#4527. + * + * The variant inspector authors `conditionalFormatting` through + * {@link ConditionalFormattingEditor}, one `CelPredicateField` per rule. That + * mount site passed no `onLintChange`, so a condition that does not parse + * rendered its inline error and Save stayed writable. + * + * ## Reachability note (the reason `ViewInspector` is exercised here too) + * + * `ViewVariantInspector` is reached two ways: SCOPED, through `ViewInspector` + * (a `MetadataInspectorProps` component, which therefore carries the channel), + * and HOME, through `ViewDefaultInspector` (a `MetadataDefaultInspectorProps` + * component, whose contract has no such channel — see #4527's report). Only the + * scoped path can report today, so the forwarding through `ViewInspector` is + * pinned explicitly: without it the channel stops one component short of the + * editor and the whole wiring is inert. + * + * The editor-side per-rule map and prune are pinned in + * {@link file://../ConditionalFormattingEditor.celGate.test.tsx}. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react'; + +import { ViewVariantInspector } from './ViewVariantInspector'; +import { ViewInspector } from './ViewInspector'; +import { __setCelFormulaLoader } from '../celAuthoring'; + +afterEach(() => { + cleanup(); + __setCelFormulaLoader(undefined); +}); + +const DANGLING = /[*+\-/&|=<>]\s*$/; + +function stubEngine() { + __setCelFormulaLoader(() => + Promise.resolve({ + validateExpression: (_role: string, input: unknown) => { + const src = typeof input === 'string' ? input : String((input as { source?: string })?.source ?? ''); + return DANGLING.test(src) + ? { ok: false, errors: [{ message: 'Parse error: expression ends after an operator' }], warnings: [] } + : { ok: true, errors: [], warnings: [] }; + }, + introspectScope: () => ({ fields: ['status', 'amount'], roots: ['record'], functions: ['has'] }), + inferExpressionType: () => 'boolean' as const, + }), + ); +} + +const FIELDS = [ + { name: 'status', label: 'Status', type: 'text', hidden: false }, + { name: 'amount', label: 'Amount', type: 'number', hidden: false }, +]; + +/** A `view` draft whose primary (list) variant carries formatting rules. */ +function viewDraft(rules: unknown[]): Record { + return { + name: 'invoices', + label: 'Invoices', + list: { + type: 'grid', + object: 'invoice', + columns: ['status', 'amount'], + conditionalFormatting: rules, + }, + }; +} + +/** Controlled harness — patches must round-trip or the second edit reverts. */ +function Harness({ + initial, + report, + viaViewInspector = false, +}: { + initial: Record; + report: (n: number) => void; + viaViewInspector?: boolean; +}) { + const [draft, setDraft] = React.useState(initial); + const onPatch = (patch: Record) => setDraft((d) => ({ ...d, ...patch })); + + if (viaViewInspector) { + // The SCOPED path exactly as the host mounts it. + return ( + {}} + onSelectionChange={() => {}} + onBlockingIssuesChange={report} + readOnly={false} + locale={'en-US'} + /> + ); + } + return ( + {}} + onClearSelection={() => {}} + onBlockingIssuesChange={report} + readOnly={false} + locale={'en-US'} + variantKey="list" + familyKey="list" + isHome={false} + objectFieldsOverride={FIELDS} + /> + ); +} + +function renderInspector(rules: unknown[], viaViewInspector = false) { + const report = vi.fn(); + render(); + const current = () => report.mock.calls.at(-1)?.[0] as number | undefined; + return { report, current }; +} + +const ruleBox = (i: number) => + screen.getByTestId(`cf-rule-${i}`).querySelector('[role="combobox"]') as HTMLTextAreaElement; + +describe('ViewVariantInspector — blocking CEL issues reach the host (#4527)', () => { + it('counts a formatting condition that does not parse', async () => { + stubEngine(); + const { current } = renderInspector([{ condition: "record.status == 'a'", style: {} }]); + fireEvent.change(ruleBox(0), { target: { value: 'record.amount >' } }); + await waitFor(() => expect(current()).toBe(1), { timeout: 3000 }); + }); + + it('reports a clean rule as zero, so a valid condition never blocks Save', async () => { + stubEngine(); + const { current } = renderInspector([{ condition: '', style: {} }]); + fireEvent.change(ruleBox(0), { target: { value: "record.status == 'overdue'" } }); + await waitFor(() => expect(current()).toBe(0), { timeout: 3000 }); + }); + + it('drops the count when the faulty rule is deleted, so Save cannot wedge shut', async () => { + stubEngine(); + const { current } = renderInspector([{ condition: '', style: {} }]); + fireEvent.change(ruleBox(0), { target: { value: 'record.amount >' } }); + await waitFor(() => expect(current()).toBe(1), { timeout: 3000 }); + + fireEvent.click(screen.getByTestId('cf-remove-0')); + await waitFor(() => expect(screen.queryByTestId('cf-rule-0')).toBeNull(), { timeout: 3000 }); + await waitFor(() => expect(current()).toBe(0), { timeout: 3000 }); + }); + + /** The channel must survive the scoped router, or the wiring is inert. */ + it('is reported through ViewInspector on the scoped path', async () => { + stubEngine(); + const { current } = renderInspector([{ condition: '', style: {} }], true); + fireEvent.change(ruleBox(0), { target: { value: 'record.amount >' } }); + await waitFor(() => expect(current()).toBe(1), { timeout: 3000 }); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ViewVariantInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ViewVariantInspector.tsx index 9f6454bc97..81a079da1a 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/ViewVariantInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ViewVariantInspector.tsx @@ -107,6 +107,17 @@ export interface ViewVariantInspectorProps extends MetadataDefaultInspectorProps isHome: boolean; /** Clear the current selection (scoped mode only). */ onClearSelection?: () => void; + /** + * Report how many BLOCKING author-time issues this inspector is showing — + * conditional-formatting rules whose CEL does not parse (objectui#4527). + * + * Only the SCOPED path can carry this today: `ViewInspector` is a + * `MetadataInspectorProps` component and forwards the host's callback, while + * `ViewDefaultInspector` (the home panel) is a + * `MetadataDefaultInspectorProps` component whose contract has no such + * channel — so on that path it is simply absent and nothing is reported. + */ + onBlockingIssuesChange?: (count: number) => void; /** * Pre-resolved field catalog for the bound object. When supplied, both * this inspector and its {@link FieldsListEditor} skip the network fetch @@ -193,6 +204,7 @@ export function ViewVariantInspector({ readOnly, onClearSelection, onSelectionChange, + onBlockingIssuesChange, objectFieldsOverride, locale, serverSchema, @@ -232,6 +244,40 @@ export function ViewVariantInspector({ const cfRules = Array.isArray(variant.conditionalFormatting) ? (variant.conditionalFormatting as unknown[]) : []; + + /* ─── Blocking CEL verdicts → the host's Save gate (objectui#4527) ───── + * + * The conditional-formatting editor below mounts one `CelPredicateField` + * per rule and owns the per-rule map; this inspector only stamps that + * editor's aggregate with the variant it describes, so a verdict that + * lands after the author switched variants cannot gate the one now on + * screen. Mismatch is read as 0 at aggregation time rather than repaired + * by a reset effect. The formatting editor is rendered for list families + * only, so a form variant never reports anything but 0. */ + const [celErrors, setCelErrors] = React.useState<{ variant: string; count: number }>({ + variant: variantKey, + count: 0, + }); + const reportCel = React.useCallback( + (count: number) => { + setCelErrors((prev) => { + if (prev.variant !== variantKey) return { variant: variantKey, count }; + if (prev.count === count) return prev; + return { variant: variantKey, count }; + }); + }, + [variantKey], + ); + const blockingIssues = + celErrors.variant === variantKey && !isFormFamily ? celErrors.count : 0; + // Held in a ref so an unmemoized host callback cannot re-fire the effect. + const onBlockingIssuesChangeRef = React.useRef(onBlockingIssuesChange); + React.useEffect(() => { + onBlockingIssuesChangeRef.current = onBlockingIssuesChange; + }); + React.useEffect(() => { + onBlockingIssuesChangeRef.current?.(blockingIssues); + }, [blockingIssues]); const widgetContext = React.useMemo( () => ({ objectFields: objectFields.map((f) => ({ @@ -359,6 +405,7 @@ export function ViewVariantInspector({ onChange={(rules) => writeVariant({ conditionalFormatting: rules.length > 0 ? rules : undefined }) } + onBlockingIssuesChange={reportCel} /> )}