diff --git a/.changeset/27629-studio-panel-cel-hold.md b/.changeset/27629-studio-panel-cel-hold.md new file mode 100644 index 00000000000..959d47e493a --- /dev/null +++ b/.changeset/27629-studio-panel-cel-hold.md @@ -0,0 +1,12 @@ +--- +'@object-ui/app-shell': patch +'@object-ui/i18n': patch +--- + +fix(studio): leaving the Validations or Actions tab, or the metadata editor's selection, keeps an expression that does not parse unsaved and names it (#27629) + +Clause-②: no +Metadata-protocol: none + +- The Data pillar's Validations and Actions panels held the object's draft while a rule's guard or an action's predicate did not parse, but only while their tab was open: leaving the tab ended the hold, and the autosave sent the broken expression as the object's draft. The hold now belongs to the draft, as the field inspector's does since #27485: it lasts while the draft still carries the rule or action the editor judged, and ends when that item is edited, removed or the object reloads. While it lasts, the pillar shows the held line ("Not saved yet: an expression on the rule “Rule A” has an error to fix…") with a **Show me** that reopens the rule or action, and a switch to another object asks first. +- The metadata editor (`ResourceEditPage`) did the same with its selection: clearing or changing it re-enabled Save and the autosave over a broken predicate in the item it had open. The hold now stays with the draft slice the inspector judged, the editor shows a held line naming the item, and **Show me** reselects it. diff --git a/ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.celHold-27629.test.tsx b/ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.celHold-27629.test.tsx new file mode 100644 index 00000000000..f3bae2a644d --- /dev/null +++ b/ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.celHold-27629.test.tsx @@ -0,0 +1,179 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#27629 — the metadata editor keeps a scoped inspector's CEL hold + * with the DRAFT, not with the selection. + * + * The maintainer's ruling on #27485 (comment 6500685972, 2026-10-29T15:17Z, + * letter A) makes a CEL parse-fault hold belong to the draft: it lasts while + * the draft still carries the text the editor judged; editing that text, + * removing the item or reloading ends it; while it holds, a held line names the + * item with a "Show me" that reopens it. Item 3 asks for the sibling surfaces; + * this suite pins `MetadataResourceEditPage`. Before this card the inspector's + * count was stamped with the selection, so clearing or changing the selection + * released Save and the autosave while the broken predicate stayed in the draft. + * + * Set up as {@link file://./ResourceEditPage.celGate.test.tsx}: only the page + * canvas is stubbed (to emit selections); the block inspector, its + * ConditionBuilder and this host's gating are real, the CEL engine is stubbed. + */ + +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' }, + { type: 'text', id: 'b2' }, + ], + }, + ], +}; + +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), + save: vi.fn(async () => ({})), + saveDraft: vi.fn(async () => ({})), +}; + +vi.mock('./useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + useMetadataClient: () => mockClient, + useMetadataTypes: () => ({ + entries: [{ type: 'page', name: 'page', label: 'Page', allowEnvironmentOverlay: true }], + }), + }; +}); + +import { MetadataResourceEditPage } from './ResourceEditPage'; +import { registerBuiltinInspectors } from './inspectors'; +import { registerMetadataPreview, getMetadataPreview, type MetadataSelection } from './preview-registry'; +import { __setCelFormulaLoader } from './celAuthoring'; + +registerBuiltinInspectors(); + +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 selections the real canvas emits on click and on an empty 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); + mockClient.save.mockClear(); + mockClient.saveDraft.mockClear(); + if (realPagePreview) registerMetadataPreview('page', realPagePreview); +}); + +const saveButton = () => + screen.getByRole('button', { name: /Save \(⌘S\)|Fix the issues shown in the inspector before saving\./ }); + +/** Past the autosave's 1.5 s debounce. */ +const outlastDebounce = () => new Promise((r) => setTimeout(r, 2300)); + +async function openFirstBlockCel(): Promise { + fireEvent.click(await screen.findByRole('button', { name: 'select the first block' })); + fireEvent.click(await screen.findByText('Expression')); + return screen.getAllByRole('combobox').find((el) => el.tagName === 'TEXTAREA') as HTMLTextAreaElement; +} + +async function breakFirstBlock(): Promise { + render( + + + , + ); + const box = await openFirstBlockCel(); + fireEvent.change(box, { target: { value: 'record.amount >' } }); + await waitFor(() => expect(saveButton()).toBeDisabled(), { timeout: 4000 }); +} + +const LEAVERS: Array<[string, () => void]> = [ + ['clearing the selection', () => fireEvent.click(screen.getByRole('button', { name: 'clear the selection' }))], + ['selecting another block', () => fireEvent.click(screen.getByRole('button', { name: 'select the second block' }))], +]; + +describe('MetadataResourceEditPage — the CEL hold belongs to the draft, not to the selection (objectstack#27629)', () => { + it.each(LEAVERS)('%s keeps Save and the autosave held, and the held line names the block', async (_how, leave) => { + await breakFirstBlock(); + leave(); + await outlastDebounce(); + expect(mockClient.save).not.toHaveBeenCalled(); + expect(mockClient.saveDraft).not.toHaveBeenCalled(); + expect(saveButton()).toBeDisabled(); + expect(screen.getByTestId('inspector-held-message')).toHaveTextContent('Intro text'); + }); + + it('"Show me" reopens the block with its text, and fixing it there releases the hold', async () => { + await breakFirstBlock(); + fireEvent.click(screen.getByRole('button', { name: 'clear the selection' })); + await waitFor(() => expect(screen.getByTestId('inspector-held')).toBeInTheDocument(), { timeout: 3000 }); + + fireEvent.click(screen.getByRole('button', { name: 'Show me' })); + const box = await waitFor(() => { + const el = screen.queryAllByRole('combobox').find((e) => e.tagName === 'TEXTAREA') as HTMLTextAreaElement | undefined; + expect(el).toBeTruthy(); + return el!; + }); + expect(box.value).toBe('record.amount >'); + fireEvent.change(box, { target: { value: 'record.amount > 20' } }); + await waitFor(() => expect(saveButton()).toBeEnabled(), { timeout: 4000 }); + expect(screen.queryByTestId('inspector-held')).toBeNull(); + }); +}); diff --git a/ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx b/ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx index 498de5737e3..d6f6691741c 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx @@ -478,6 +478,75 @@ interface SaveRefusal { summary: string; } +/** The value at a dot-joined path (numeric segments index arrays), or `undefined`. */ +function valueAtPath(root: unknown, path: string): unknown { + let cursor: unknown = root; + for (const seg of path === '' ? [] : path.split('.')) { + if (Array.isArray(cursor) && /^\d+$/.test(seg)) cursor = cursor[Number(seg)]; + else if (cursor && typeof cursor === 'object') cursor = (cursor as Record)[seg]; + else return undefined; + } + return cursor; +} + +const SELECTION_IDENTITY_KEYS = ['id', 'name', 'key'] as const; + +/** The path of the first object under `node` that a selection `id` names, depth-first. */ +function findSelectionPath(node: unknown, id: string, path: string[], depth: number): string | null { + if (!node || typeof node !== 'object' || depth > 8) return null; + const entries: Array<[string, unknown]> = Array.isArray(node) + ? node.map((v, i) => [String(i), v]) + : Object.entries(node as Record); + for (const [seg, child] of entries) { + if (!child || typeof child !== 'object' || Array.isArray(child)) continue; + const named = + (!Array.isArray(node) && seg === id) || + SELECTION_IDENTITY_KEYS.some((k) => (child as Record)[k] === id); + if (named) return [...path, seg].join('.'); + } + for (const [seg, child] of entries) { + const found = findSelectionPath(child, id, [...path, seg], depth + 1); + if (found !== null) return found; + } + return null; +} + +/** + * objectstack#27629 — the draft slice a scoped inspector's selection edits, as + * a path `issueSliceFingerprint` reads: the selection id read as a path + * (a page block's `regions[0].components[0]`), else the first object whose + * `id` / `name` / `key`, or map key, is the id (an object's field). `''`, the + * whole document, for the default inspector and for an id no draft value + * carries: any edit then ends the hold, so it can release early but can never + * wedge Save. + */ +function selectionSlicePath(draft: Record, selection: MetadataSelection | null): string { + if (!selection || selection.id === '') return ''; + const asPath = selection.id.replace(/\[(\d+)\]/g, '.$1').replace(/^\./, ''); + const atPath = valueAtPath(draft, asPath); + if (atPath && typeof atPath === 'object') return asPath; + return findSelectionPath(draft, selection.id, [], 0) ?? ''; +} + +/** What the held line calls a kept selection: its own label, the item's identity, or its id. */ +function heldSelectionLabel( + draft: Record, + held: { selection: MetadataSelection | null; slicePath: string }, + itemName: string, +): string { + const { selection } = held; + if (!selection) return itemName; + if (selection.label && selection.label.trim() !== '') return selection.label.trim(); + const item = valueAtPath(draft, held.slicePath); + if (held.slicePath !== '' && item && typeof item === 'object') { + for (const k of [...SELECTION_IDENTITY_KEYS, 'label']) { + const v = (item as Record)[k]; + if (typeof v === 'string' && v.trim() !== '') return v.trim(); + } + } + return selection.id; +} + interface MetadataResourceEditPageImplProps { type: string; name: string; @@ -899,10 +968,10 @@ function MetadataResourceEditPageImpl({ // formula that does not parse) — Save must refuse them rather than publish a // malformed definition (objectui#4306). // - // The count is STAMPED with the selection it describes, so it expires by - // construction when the selection changes or the inspector unmounts: a - // component that has gone away cannot retract its last verdict, and a host - // that waited for one would wedge Save shut. + // The count is STAMPED with the selection it describes: it is the open + // inspector's live verdict, and never reads as another selection's. A + // component that has gone away cannot retract its last verdict, so the host + // never waits for one. const [blockingReport, setBlockingReport] = React.useState({ key: '', count: 0 }); // Covers BOTH inspector branches. With no selection the editor renders the // registered DEFAULT inspector instead, and that surface hosts CEL too (a @@ -912,7 +981,42 @@ function MetadataResourceEditPageImpl({ const selectionKey = selection ? `${type}:${name}:${selection.kind}:${selection.id}` : `${type}:${name}:default`; - const inspectorBlocking = blockingReport.key === selectionKey ? blockingReport.count : 0; + // objectstack#27629 — the verdict belongs to the DRAFT, not to the selection + // (the ruling on objectstack#27485, ported here from Studio's Data pillar). + // Expiring it with the selection released Save and the autosave the moment + // the author cleared or changed it, while the broken text stayed in the + // draft. So each selection's last live verdict is kept with the stamp of the + // draft slice it judged (`selectionSlicePath`, fingerprinted the way a server + // refusal's slice is) and holds, with the inspector closed, while the draft + // still carries that slice: an edit to it, the item's removal or the next + // load ends it. Nothing is left shut with nothing to fix: the held line names + // the item and its "Show me" reopens it, and the open inspector's live + // verdict always wins. + const [inspectorVerdicts, setInspectorVerdicts] = React.useState< + Record + >({}); + const liveInspectorReport = blockingReport.key === selectionKey; + const openSlicePath = React.useMemo(() => selectionSlicePath(draft, selection), [draft, selection]); + const openSliceStamp = issueSliceFingerprint(draft, openSlicePath); + React.useEffect(() => { + if (!liveInspectorReport) return; + const kept = { selection, slicePath: openSlicePath, count: blockingReport.count, stamp: openSliceStamp }; + setInspectorVerdicts((prev) => { + const was = prev[selectionKey]; + if (was && was.count === kept.count && was.stamp === kept.stamp && was.slicePath === kept.slicePath) return prev; + return { ...prev, [selectionKey]: kept }; + }); + }, [liveInspectorReport, selection, selectionKey, openSlicePath, blockingReport.count, openSliceStamp]); + // The kept verdicts that hold, and the open inspector's live one. + const inspectorHeld: Array<{ selection: MetadataSelection | null; slicePath: string }> = []; + let inspectorBlocking = liveInspectorReport ? blockingReport.count : 0; + for (const [key, kept] of Object.entries(inspectorVerdicts)) { + if (key === selectionKey && liveInspectorReport) continue; + if (kept.count > 0 && kept.stamp !== undefined && issueSliceFingerprint(draft, kept.slicePath) === kept.stamp) { + inspectorHeld.push({ selection: kept.selection, slicePath: kept.slicePath }); + inspectorBlocking += kept.count; + } + } // ── The SERVER's refusal is not advisory — and it is DOCUMENT-scoped ── // @@ -935,9 +1039,10 @@ function MetadataResourceEditPageImpl({ // ⭐ Why it is DOCUMENT-scoped where `inspectorBlocking` is SELECTION-scoped. // The measured wedge (`9073cf018`) is an author who adds a Lookup field // with an empty target, then selects an unrelated already-saved field and - // renames it. `inspectorBlocking` above expires when the selection changes — - // BY DESIGN, and that design is right for what it gates — so it is already - // gone by that second step, and every later auto-save PUTs the half-filled + // renames it. `inspectorBlocking` above expired with the selection then (it + // keeps a verdict with the draft since objectstack#27629, but only one an + // inspector reported, and the server's refusal is not one) — so it was + // already gone by that second step, and every later auto-save PUTs the half-filled // document again and is refused again, while the designer renders the rename // as applied and the server has none of it. ⛔ The fix is NOT to widen that // stamp: a selection-scoped term cannot carry a document-scoped invariant. @@ -973,6 +1078,7 @@ function MetadataResourceEditPageImpl({ // because the state it clears is declared here. React.useEffect(() => { setSaveRefusal(null); + setInspectorVerdicts({}); }, [type, name]); React.useEffect(() => { if (!editing) setSelection(null); @@ -1235,6 +1341,8 @@ function MetadataResourceEditPageImpl({ const initial = config.toDraft ? config.toDraft(rawInitial) : rawInitial; setDraft(initial); draftSnapshotRef.current = initial; + // objectstack#27629 — kept inspector verdicts judged the buffer this load replaces. + setInspectorVerdicts({}); // objectui#11773 — the next save is pinned to what the draft read // served: its version, or "no draft row" when none was pending. recordDraftRead(type, name, draftPackage, draftRead.pin); @@ -2583,8 +2691,9 @@ function MetadataResourceEditPageImpl({ still neither of them — that asymmetry is on purpose. The discriminator is who refused, not how bad it looks: - `inspectorBlocking` — an author-time fault the server does NOT - catch (a CEL predicate that does not parse), SELECTION-scoped - (objectui#4306 / #6980); + catch (a CEL predicate that does not parse), live for the open + selection and kept with the draft slice it judged after that + (objectui#4306 / #6980, objectstack#27629); - `refusalBlocking` — a 422 the server DID return for this exact document, DOCUMENT-scoped so it survives a selection change (`9073cf018`). @@ -2971,6 +3080,34 @@ function MetadataResourceEditPageImpl({ ); })()} + {/* objectstack#27629 — the held line: an inspector verdict kept with + the draft after its inspector closed names the item, and "Show me" + reopens it (the field inspector's rule in Studio, objectstack#27485). */} + {inspectorHeld.length > 0 && (isDirty || createMode) && ( +
+

+ {tFormat('engine.studio.held.line', locale, { + clause: tFormat('engine.studio.held.inspectorInvalid', locale, { + item: heldSelectionLabel(draft, inspectorHeld[0], name || type), + }), + })} +

+ +
+ )} {PreviewComponent ? (
{ + it('keeps the hold when the author leaves the Rules tab, and names the rule', async () => { const box = await openRuleGuard(); fireEvent.change(box, { target: { value: 'record.status ==' } }); @@ -184,6 +187,8 @@ describe('DataPillar — auto-save is gated on the validations panel’s CEL ver expect(mockClient.save).not.toHaveBeenCalled(); fireEvent.click(screen.getByRole('button', { name: 'Form' })); - await waitFor(() => expect(mockClient.save).toHaveBeenCalled(), { timeout: 4000 }); + await new Promise((r) => setTimeout(r, 2300)); + expect(mockClient.save).not.toHaveBeenCalled(); + expect(screen.getByTestId('studio-held-message')).toHaveTextContent('Rule A'); }); }); diff --git a/ui/packages/app-shell/src/views/studio-design/DataPillar.panelHold-27629.test.tsx b/ui/packages/app-shell/src/views/studio-design/DataPillar.panelHold-27629.test.tsx new file mode 100644 index 00000000000..caf692cc528 --- /dev/null +++ b/ui/packages/app-shell/src/views/studio-design/DataPillar.panelHold-27629.test.tsx @@ -0,0 +1,254 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#27629 — the Data pillar's PANEL family keeps a CEL hold with the + * draft, the same rule the field inspector follows since objectstack#27485. + * + * The maintainer's ruling on #27485 (comment 6500685972, 2026-10-29T15:17Z, + * letter A) makes a CEL parse-fault hold belong to the draft, not to the open + * editor: it lasts while the draft still carries the text the editor judged; + * editing that text, removing the item or reloading ends it; while it holds, the + * held line names the item with a "Show me" that reopens it. Item 3 of that + * ruling asks for the sibling surfaces, which this suite pins for the two panels + * that mount a CEL editor: Validations (a rule's guard) and Actions (an action's + * predicates). Before this card the panel count was stamped with the panel TAB, + * so leaving the tab released the hold and the autosave sent the broken text + * ({@link file://./DataPillar.panelGate.test.tsx} pinned that, and is flipped). + * + * The Settings panel is not here: `ObjectDefaultInspector` mounts no CEL editor, + * so it has no verdict to keep (`ObjectSettingsPanel`'s own header). + * + * The engine is stubbed so the verdict is deterministic, 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, within } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; + +const ARCHIVE = { + name: 'showcase_task_archive', + label: 'Archive', + objectName: 'showcase_task', + locations: ['record_more'], + type: 'script', + body: { language: 'expression', source: 'true' }, +}; + +const objectDef = { + name: 'showcase_task', + label: 'Task', + fields: [{ name: 'status', label: 'Status', type: 'text' }], + validations: [ + { + type: 'script', + name: 'rule_a', + label: 'Rule A', + message: 'nope', + severity: 'error', + active: true, + }, + ], + actions: [ARCHIVE], +}; + +const mockClient: Record & { save: ReturnType } = { + save: vi.fn(async () => ({})), + // The action inspector's field pickers read the object through the preview-draft door. + get: vi.fn(async () => objectDef), + list: vi.fn(async () => [{ name: 'showcase_task', label: 'Task' }]), + listDrafts: vi.fn(async () => []), + layered: vi.fn(async () => ({ effective: objectDef, code: objectDef })), + getDraft: vi.fn(async () => null), +}; +mockClient.withPreviewDrafts = () => mockClient; + +vi.mock('../metadata-admin/useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + useMetadataClient: () => mockClient, + useMetadataTypes: () => ({ entries: [] }), + }; +}); + +vi.mock('./packages-io', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, fetchPackages: vi.fn(async () => []) }; +}); + +// The advanced tabs live in a Radix DropdownMenu; rendered as passthroughs, as +// in DataPillar.panelGate.test.tsx. +type PassthroughProps = { children?: React.ReactNode; onSelect?: () => void }; +vi.mock('@object-ui/components', async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + DropdownMenu: (p: PassthroughProps) =>
{p.children}
, + DropdownMenuTrigger: (p: PassthroughProps) =>
{p.children}
, + DropdownMenuContent: (p: PassthroughProps) =>
{p.children}
, + DropdownMenuItem: (p: PassthroughProps) => ( + + ), + DropdownMenuRadioGroup: (p: PassthroughProps) =>
{p.children}
, + DropdownMenuRadioItem: (p: PassthroughProps) => ( + + ), + }; +}); + +vi.mock('@object-ui/react', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, useAdapter: () => dataSource }; +}); + +import { DataPillar } from './StudioDesignSurface'; +import { createEmptyDataSource, failOnAbsorbedFetchError } from './__tests__/emptyDataSource'; +import { registerBuiltinInspectors } from '../metadata-admin/inspectors'; +import { __setCelFormulaLoader } from '../metadata-admin/celAuthoring'; + +const dataSource = createEmptyDataSource(); +failOnAbsorbedFetchError(); + +registerBuiltinInspectors(); + +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'], roots: ['record'], functions: ['has'] }), + inferExpressionType: () => 'boolean' as const, + }), + ); +} + +beforeEach(stubEngine); +afterEach(() => { + cleanup(); + __setCelFormulaLoader(undefined); + mockClient.save.mockClear(); +}); + +/** Past the autosave's 1.5 s debounce. */ +const outlastDebounce = () => new Promise((r) => setTimeout(r, 2300)); + +const rawEditorIn = (root: HTMLElement) => + within(root).getAllByRole('combobox').find((el) => el.tagName === 'TEXTAREA') as HTMLTextAreaElement; + +function mount() { + render( + + + , + ); +} + +/** The selected rule's guard, in raw CEL. The panel selects the first rule. */ +async function openRuleGuard(): Promise { + fireEvent.click(await screen.findByTestId('data-tab-flat-rules')); + fireEvent.click(await screen.findByText('Expression')); + return rawEditorIn(document.body); +} + +/** The selected action's "Visible when" predicate, in raw CEL. */ +async function openActionVisible(): Promise { + fireEvent.click(await screen.findByTestId('data-tab-flat-actions')); + const root = (await screen.findByText('Visible when')).parentElement!.parentElement as HTMLElement; + fireEvent.click(within(root).getByText('Expression')); + return rawEditorIn(root); +} + +const leaveToForm = () => fireEvent.click(screen.getByRole('button', { name: 'Form' })); + +describe('DataPillar — a rule guard that does not parse holds with the draft (objectstack#27629)', () => { + it('leaving the Validations tab keeps the hold, and the held line names the rule', async () => { + mount(); + const box = await openRuleGuard(); + fireEvent.change(box, { target: { value: 'record.status ==' } }); + await outlastDebounce(); + expect(mockClient.save).not.toHaveBeenCalled(); + + leaveToForm(); + await outlastDebounce(); + expect(mockClient.save).not.toHaveBeenCalled(); + expect(screen.getByTestId('studio-held-message')).toHaveTextContent('Rule A'); + }); + + it('"Show me" reopens the rule with its text, and fixing it there releases the hold', async () => { + mount(); + fireEvent.change(await openRuleGuard(), { target: { value: 'record.status ==' } }); + await outlastDebounce(); + leaveToForm(); + await waitFor(() => expect(screen.getByTestId('studio-held-message')).toHaveTextContent('Rule A'), { timeout: 3000 }); + + fireEvent.click(screen.getByRole('button', { name: 'Show me' })); + // The guard does not parse, so its builder reopens straight in raw CEL. + const box = await waitFor(() => { + const el = screen.queryAllByRole('combobox').find((e) => e.tagName === 'TEXTAREA') as HTMLTextAreaElement | undefined; + expect(el).toBeTruthy(); + return el!; + }); + expect(box.value).toBe('record.status =='); + fireEvent.change(box, { target: { value: "record.status == 'open'" } }); + await waitFor(() => expect(mockClient.save).toHaveBeenCalled(), { timeout: 4000 }); + expect(screen.queryByTestId('studio-held')).toBeNull(); + }); + + it('removing the rule ends the hold: nothing judged is left in the draft', async () => { + mount(); + fireEvent.change(await openRuleGuard(), { target: { value: 'record.status ==' } }); + await outlastDebounce(); + leaveToForm(); + await waitFor(() => expect(screen.getByTestId('studio-held')).toBeInTheDocument(), { timeout: 3000 }); + + fireEvent.click(screen.getByTestId('data-tab-flat-rules')); + const detail = (await screen.findAllByRole('button', { name: 'Delete' })).at(-1)!; + fireEvent.click(detail); + await waitFor(() => expect(mockClient.save).toHaveBeenCalled(), { timeout: 4000 }); + const body = (mockClient.save.mock.calls.at(-1) as unknown[] | undefined)?.[2] as { validations?: unknown[] }; + expect(body.validations ?? []).toEqual([]); + }); +}); + +describe('DataPillar — an action predicate that does not parse holds with the draft (objectstack#27629)', () => { + it('leaving the Actions tab keeps the hold, and the held line names the action', async () => { + mount(); + const box = await openActionVisible(); + fireEvent.change(box, { target: { value: 'record.status ==' } }); + await outlastDebounce(); + expect(mockClient.save).not.toHaveBeenCalled(); + + leaveToForm(); + await outlastDebounce(); + expect(mockClient.save).not.toHaveBeenCalled(); + expect(screen.getByTestId('studio-held-message')).toHaveTextContent('Archive'); + }); + + it('"Show me" reopens the action with its text, and fixing it there releases the hold', async () => { + mount(); + fireEvent.change(await openActionVisible(), { target: { value: 'record.status ==' } }); + await outlastDebounce(); + leaveToForm(); + await waitFor(() => expect(screen.getByTestId('studio-held-message')).toHaveTextContent('Archive'), { timeout: 3000 }); + + fireEvent.click(screen.getByRole('button', { name: 'Show me' })); + const root = (await screen.findByText('Visible when')).parentElement!.parentElement as HTMLElement; + const box = await waitFor(() => { + const el = within(root).getAllByRole('combobox').find((e) => e.tagName === 'TEXTAREA') as HTMLTextAreaElement | undefined; + expect(el).toBeTruthy(); + return el!; + }); + expect(box.value).toBe('record.status =='); + fireEvent.change(box, { target: { value: "record.status == 'open'" } }); + await waitFor(() => expect(mockClient.save).toHaveBeenCalled(), { timeout: 4000 }); + expect(screen.queryByTestId('studio-held')).toBeNull(); + }); +}); diff --git a/ui/packages/app-shell/src/views/studio-design/ObjectActionsPanel.tsx b/ui/packages/app-shell/src/views/studio-design/ObjectActionsPanel.tsx index f22a6ded259..2521ac03c8a 100644 --- a/ui/packages/app-shell/src/views/studio-design/ObjectActionsPanel.tsx +++ b/ui/packages/app-shell/src/views/studio-design/ObjectActionsPanel.tsx @@ -122,6 +122,7 @@ export function ObjectActionsPanel({ disabled, actionSchema, onBlockingIssuesChange, + onCelVerdictsChange, focus, }: { draft: Record; @@ -140,6 +141,12 @@ export function ObjectActionsPanel({ * writes the actions, so the Data pillar holds this count and refuses. */ onBlockingIssuesChange?: (count: number) => void; + /** + * objectstack#27629 — the same verdicts per action NAME (only actions in the + * draft, `0` included), so the Data pillar can keep each one with the draft + * once this panel unmounts and name the action it holds. + */ + onCelVerdictsChange?: (verdicts: Readonly>) => void; /** * The live server JSONSchema for the `action` type (`/meta/types`). Handed to * ActionDefaultInspector as `serverSchema` so its "More fields" section can @@ -239,21 +246,28 @@ export function ObjectActionsPanel({ () => new Set(actions.map((a) => String(a.name ?? ''))), [actions], ); - const blockingIssues = React.useMemo(() => { - let total = 0; + const liveCelErrors = React.useMemo(() => { + const live: Record = {}; for (const [name, count] of Object.entries(celErrors)) { - if (!liveActionNames.has(name)) continue; // pruned: the action is gone - total += count; + if (liveActionNames.has(name)) live[name] = count; // else pruned: the action is gone } - return total; + return live; }, [celErrors, liveActionNames]); + const blockingIssues = Object.values(liveCelErrors).reduce((total, count) => total + count, 0); const onBlockingIssuesChangeRef = React.useRef(onBlockingIssuesChange); + const onCelVerdictsChangeRef = React.useRef(onCelVerdictsChange); React.useEffect(() => { onBlockingIssuesChangeRef.current = onBlockingIssuesChange; + onCelVerdictsChangeRef.current = onCelVerdictsChange; }); React.useEffect(() => { onBlockingIssuesChangeRef.current?.(blockingIssues); }, [blockingIssues]); + // Keyed on the verdicts' text, not on the memo's identity (ui/AGENTS.md #10). + const celVerdictsKey = JSON.stringify(liveCelErrors); + React.useEffect(() => { + onCelVerdictsChangeRef.current?.(JSON.parse(celVerdictsKey) as Record); + }, [celVerdictsKey]); // objectui#11861 — the blank action, under New's "Advanced": what New wrote // before the presets, unchanged. Its name is unique among the held actions diff --git a/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx b/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx index 8bfe689bb39..84f0b2498c4 100644 --- a/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx +++ b/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx @@ -863,6 +863,8 @@ export function ObjectValidationsPanel({ onPatch, disabled, onBlockingIssuesChange, + onCelVerdictsChange, + focus, }: { draft: Record; onPatch: (patch: Record) => void; @@ -874,6 +876,18 @@ export function ObjectValidationsPanel({ * and refuses to write. */ onBlockingIssuesChange?: (count: number) => void; + /** + * objectstack#27629 — the same verdicts per rule NAME (only rules in the + * draft, `0` included), so the Data pillar can keep each one with the draft + * once this panel unmounts and name the rule it holds. + */ + onCelVerdictsChange?: (verdicts: Readonly>) => void; + /** + * objectstack#27629 — the rule the Data pillar's "Show me" asks for, on a + * held edit. Each new object selects it once; the author's own picks are + * free after that. + */ + focus?: { name: string } | null; }) { const locale = useMetadataLocale(); const rules = React.useMemo(() => readRules(draft.validations), [draft.validations]); @@ -882,7 +896,12 @@ export function ObjectValidationsPanel({ // in the header). The first edit that gives one its guard writes it. const [unsaved, setUnsaved] = React.useState([]); const listed = React.useMemo(() => [...rules, ...unsaved], [rules, unsaved]); - const [selected, setSelected] = React.useState(null); + const [selected, setSelected] = React.useState(() => focus?.name ?? null); + const [focusSeen, setFocusSeen] = React.useState(focus); + if (focus !== focusSeen) { + setFocusSeen(focus); + if (focus) setSelected(focus.name); + } const [addOpen, setAddOpen] = React.useState(false); // Default to the first rule so the detail pane isn't a dead "pick one" empty // state whenever rules already exist. Falls back automatically when `selected` @@ -952,22 +971,29 @@ export function ObjectValidationsPanel({ () => new Set(rules.map((r) => r.name ?? '')), [rules], ); - const blockingIssues = React.useMemo(() => { - let total = 0; + const liveCelErrors = React.useMemo(() => { + const live: Record = {}; for (const [name, count] of Object.entries(celErrors)) { - if (!liveRuleNames.has(name)) continue; // pruned: the rule is gone - total += count; + if (liveRuleNames.has(name)) live[name] = count; // else pruned: the rule is gone } - return total; + return live; }, [celErrors, liveRuleNames]); - // Held in a ref so an unmemoized host callback cannot re-fire the effect. + const blockingIssues = Object.values(liveCelErrors).reduce((total, count) => total + count, 0); + // Held in refs so an unmemoized host callback cannot re-fire the effects. const onBlockingIssuesChangeRef = React.useRef(onBlockingIssuesChange); + const onCelVerdictsChangeRef = React.useRef(onCelVerdictsChange); React.useEffect(() => { onBlockingIssuesChangeRef.current = onBlockingIssuesChange; + onCelVerdictsChangeRef.current = onCelVerdictsChange; }); React.useEffect(() => { onBlockingIssuesChangeRef.current?.(blockingIssues); }, [blockingIssues]); + // Keyed on the verdicts' text, not on the memo's identity (ui/AGENTS.md #10). + const celVerdictsKey = JSON.stringify(liveCelErrors); + React.useEffect(() => { + onCelVerdictsChangeRef.current?.(JSON.parse(celVerdictsKey) as Record); + }, [celVerdictsKey]); const commit = (next: ValidationRuleDraft[]) => onPatch({ validations: next }); diff --git a/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx b/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx index f8db0991e7d..5f23b54d28d 100644 --- a/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx +++ b/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx @@ -6307,6 +6307,32 @@ function fieldCelStamp(draft: Record, field: string): string | return JSON.stringify([cel.map((v) => v ?? null), entries.map((e) => e.name)]); } +/** objectstack#27629 — the Data pillar panels that mount a CEL editor and report per item. */ +type CelPanelTab = 'rules' | 'actions'; +const isCelPanelTab = (v: string): v is CelPanelTab => v === 'rules' || v === 'actions'; + +/** The rule (`rules`) or action (`actions`) named `item` in the object draft. */ +function panelItemOf(draft: Record, tab: CelPanelTab, item: string): Record | undefined { + const list = tab === 'rules' ? draft.validations : draft.actions; + if (!Array.isArray(list)) return undefined; + return list.find( + (e): e is Record => !!e && typeof e === 'object' && String((e as { name?: unknown }).name ?? '') === item, + ); +} + +/** + * objectstack#27629 — what a panel's CEL verdict on `item` was judged from, as + * the object draft holds it: the rule or action itself (its guard or + * predicates among its keys) and the object's field names its lint resolves + * references against. `fieldCelStamp`'s counterpart for the panel family; the + * same stamp is the same verdict. `null` when the item is gone. + */ +function panelItemCelStamp(draft: Record, tab: CelPanelTab, item: string): string | null { + const def = panelItemOf(draft, tab, item); + if (!def) return null; + return JSON.stringify([def, readFields(draft.fields).entries.map((e) => e.name)]); +} + /** The data-source writes the Form tab's Preview refuses (objectui#12515). */ const PREVIEW_REFUSED_WRITES = new Set(['create', 'update', 'delete', 'bulk', 'bulkUpdate', 'bulkDelete']); @@ -6499,8 +6525,10 @@ export function DataPillar({ // // Stamped with the panel TAB: exactly one panel is mounted at a time, so // leaving the tab unmounts the reporter and it can never retract its last - // verdict — deriving against the live tab is what stops a fault authored - // under Validations from wedging Save on a tab with no CEL editor at all. + // verdict. objectstack#27629 — only Settings still reports through this + // count (its inspector mounts no CEL editor today); Validations and Actions + // report per item and keep their verdicts with the draft (`panelCelVerdicts` + // below), so leaving their tab no longer releases the hold. const [panelBlockingReport, setPanelBlockingReport] = React.useState({ key: '', count: 0 }); const [dirty, setDirty] = React.useState(false); const [hasDraft, setHasDraft] = React.useState(false); @@ -6528,12 +6556,19 @@ export function DataPillar({ // its "Show me": a fresh object per press, so pressing it again re-selects // the action after the author picked another one. const [actionFocus, setActionFocus] = React.useState<{ name: string } | null>(null); + // objectstack#27629 — the rule a held line names, opened the same way. + const [ruleFocus, setRuleFocus] = React.useState<{ name: string } | null>(null); const showObjTarget = (target: MetadataSelection) => { if (target.kind === 'action') { selectViewMode('actions'); setActionFocus({ name: target.id }); return; } + if (target.kind === 'rule') { + selectViewMode('rules'); + setRuleFocus({ name: target.id }); + return; + } setFieldSel({ kind: target.kind, id: target.id }); }; // objectstack#26730 — a refusal's "Show me" also ends on the input it names: @@ -6584,9 +6619,63 @@ export function DataPillar({ (count: number) => setPanelBlockingReport({ key: panelKey, count }), [panelKey], ); + // objectstack#27629 — the panel family's verdicts belong to the draft, as the + // field inspector's do (`celVerdicts` above, the ruling on objectstack#27485). + // Validations and Actions report a verdict per rule or action; each is kept + // with the stamp of what it judged (`panelItemCelStamp`, retaken while its + // panel is mounted) and holds, with the tab left, while the draft still + // carries that stamp: an edit to the item, its removal or the object's next + // load ends it. While the panel is mounted its own report is the live verdict + // and always wins; the held line names the item and its "Show me" reopens it. + const [livePanelVerdicts, setLivePanelVerdicts] = React.useState<{ + key: string; + verdicts: Readonly>; + }>({ key: '', verdicts: {} }); + const reportPanelVerdicts = React.useCallback( + (verdicts: Readonly>) => setLivePanelVerdicts({ key: panelKey, verdicts }), + [panelKey], + ); + const [panelCelVerdicts, setPanelCelVerdicts] = React.useState< + Record + >({}); + const livePanel = isCelPanelTab(viewMode) && livePanelVerdicts.key === panelKey ? livePanelVerdicts.verdicts : null; + React.useEffect(() => { + if (livePanel === null || !isCelPanelTab(viewMode)) return; + setPanelCelVerdicts((prev) => { + let next = prev; + for (const [item, count] of Object.entries(livePanel)) { + const kept = { tab: viewMode, item, count, stamp: panelItemCelStamp(objDraft, viewMode, item) }; + const key = `${panelKey}:${item}`; + const was = prev[key]; + if (was && was.count === kept.count && was.stamp === kept.stamp) continue; + if (next === prev) next = { ...prev }; + next[key] = kept; + } + return next; + }); + }, [livePanel, viewMode, panelKey, objDraft]); + // The open object's panel items whose verdict holds, and their summed count. + const celHeldPanelItems: Array<{ tab: CelPanelTab; item: string }> = []; + let panelCelBlocking = 0; + const celPanelPrefix = `${current?.name ?? ''}:`; + for (const [key, kept] of Object.entries(panelCelVerdicts)) { + if (!key.startsWith(celPanelPrefix)) continue; + if (livePanel !== null && kept.tab === viewMode && Object.prototype.hasOwnProperty.call(livePanel, kept.item)) continue; + if (kept.count > 0 && kept.stamp !== null && kept.stamp === panelItemCelStamp(objDraft, kept.tab, kept.item)) { + celHeldPanelItems.push({ tab: kept.tab, item: kept.item }); + panelCelBlocking += kept.count; + } + } + if (livePanel !== null && isCelPanelTab(viewMode)) { + for (const [item, count] of Object.entries(livePanel)) { + if (count <= 0) continue; + celHeldPanelItems.push({ tab: viewMode, item }); + panelCelBlocking += count; + } + } // Either source closes the door: a malformed field formula and a malformed // rule guard are both unsaveable, and neither excuses the other. - const saveBlocking = inspectorBlocking + panelBlocking; + const saveBlocking = inspectorBlocking + panelBlocking + panelCelBlocking; // Within the Form view: Layout (WYSIWYG drag/section designer) ⇄ Preview // (live form). const [formMode, setFormMode] = React.useState<'layout' | 'preview'>('layout'); @@ -6762,6 +6851,7 @@ export function DataPillar({ setFieldSel(null); // objectstack#27485 — kept CEL verdicts judged the buffer this load replaces. setCelVerdicts({}); + setPanelCelVerdicts({}); setDirty(false); (async () => { try { @@ -7041,22 +7131,41 @@ export function DataPillar({ // objectstack#27485 — an unsent edit the CEL verdict holds is named like an // incomplete one: the held line, the surface's Publish and the rail's // question all read it, so it outlives the inspector without going unseen. + // objectstack#27629 — then a rule or action the panel family holds. const celHeldField = celHeldFields[0] ?? null; const celHeldLabel = React.useMemo(() => { if (celHeldField === null) return null; const label = readFields(objDraft.fields).entries.find((e) => e.name === celHeldField)?.def.label; return typeof label === 'string' && label.trim() !== '' ? label.trim() : celHeldField; }, [celHeldField, objDraft.fields]); - const objCelHeld = React.useMemo( - () => - dirty && celHeldField !== null && celHeldLabel !== null + const celHeldPanelTab = celHeldPanelItems[0]?.tab ?? null; + const celHeldPanelItem = celHeldPanelItems[0]?.item ?? null; + const celHeldPanelLabel = React.useMemo(() => { + if (celHeldPanelTab === null || celHeldPanelItem === null) return null; + const label = panelItemOf(objDraft, celHeldPanelTab, celHeldPanelItem)?.label; + return typeof label === 'string' && label.trim() !== '' ? label.trim() : celHeldPanelItem; + }, [celHeldPanelTab, celHeldPanelItem, objDraft]); + const objCelHeld = React.useMemo(() => { + if (!dirty) return null; + if (celHeldField !== null && celHeldLabel !== null) { + return { + clause: tFormat('engine.studio.held.celInvalid', locale, { field: celHeldLabel }), + target: { kind: 'field', id: celHeldField }, + }; + } + if (celHeldPanelTab !== null && celHeldPanelItem !== null && celHeldPanelLabel !== null) { + return celHeldPanelTab === 'rules' ? { - clause: tFormat('engine.studio.held.celInvalid', locale, { field: celHeldLabel }), - target: { kind: 'field', id: celHeldField }, + clause: tFormat('engine.studio.held.celInvalidRule', locale, { rule: celHeldPanelLabel }), + target: { kind: 'rule', id: celHeldPanelItem }, } - : null, - [dirty, celHeldField, celHeldLabel, locale], - ); + : { + clause: tFormat('engine.studio.held.celInvalidAction', locale, { action: celHeldPanelLabel }), + target: { kind: 'action', id: celHeldPanelItem }, + }; + } + return null; + }, [dirty, celHeldField, celHeldLabel, celHeldPanelTab, celHeldPanelItem, celHeldPanelLabel, locale]); // Only the open object's own buffer is held (objectui#11272). const objHeld = objLoaded ? (objIncomplete ?? objCelHeld) : null; // objectstack#26917 — the refusal's way out: the object holding the @@ -7562,7 +7671,8 @@ export function DataPillar({ draft={objDraft} onPatch={onPatch} disabled={readOnly} - onBlockingIssuesChange={reportPanelBlocking} + focus={ruleFocus} + onCelVerdictsChange={reportPanelVerdicts} /> ) : viewMode === 'settings' ? (
) : viewMode === 'api' ? ( diff --git a/ui/packages/i18n/src/namespaces/studio/en.ts b/ui/packages/i18n/src/namespaces/studio/en.ts index 3e629cc6686..5a9740f0b83 100644 --- a/ui/packages/i18n/src/namespaces/studio/en.ts +++ b/ui/packages/i18n/src/namespaces/studio/en.ts @@ -3014,6 +3014,10 @@ const STUDIO_EN: Record = { 'engine.studio.held.widgetNeedsInput': '{input} on the widget “{widget}” needs a value', // objectstack#27485 — a field CEL expression the inspector judged broken, held after it closes. 'engine.studio.held.celInvalid': 'an expression on the field “{field}” has an error to fix', + // objectstack#27629 — the same hold for a validation rule, an action, and an item in the metadata editor. + 'engine.studio.held.celInvalidRule': 'an expression on the rule “{rule}” has an error to fix', + 'engine.studio.held.celInvalidAction': 'an expression on the action “{action}” has an error to fix', + 'engine.studio.held.inspectorInvalid': '“{item}” has an error to fix in its inspector', 'engine.studio.held.line': 'Not saved yet: {clause}. Your changes are kept here and saved once it is filled in.', 'engine.studio.held.publish': 'Not published: {clause}, so that change is not saved yet. Finish it, then publish.', 'engine.studio.held.inputHint': 'Required. Changes are saved once this has a value.', diff --git a/ui/packages/i18n/src/namespaces/studio/zh.ts b/ui/packages/i18n/src/namespaces/studio/zh.ts index a7f9835faff..4c3bab37785 100644 --- a/ui/packages/i18n/src/namespaces/studio/zh.ts +++ b/ui/packages/i18n/src/namespaces/studio/zh.ts @@ -2682,6 +2682,9 @@ const STUDIO_ZH: Record = { 'engine.studio.held.needsRuleMessage': '规则「{rule}」需要一条错误消息', 'engine.studio.held.widgetNeedsInput': '组件「{widget}」的「{input}」需要填写', 'engine.studio.held.celInvalid': '字段「{field}」的表达式有错误需要修正', + 'engine.studio.held.celInvalidRule': '规则「{rule}」的表达式有错误需要修正', + 'engine.studio.held.celInvalidAction': '动作「{action}」的表达式有错误需要修正', + 'engine.studio.held.inspectorInvalid': '「{item}」在检查器中有错误需要修正', 'engine.studio.held.line': '尚未保存：{clause}。更改已保留在此处，补全后会自动保存。', 'engine.studio.held.publish': '未发布：{clause}，该更改尚未保存。请先补全，再发布。', 'engine.studio.held.inputHint': '必填。填写后才会保存更改。',