diff --git a/.changeset/27485-studio-formula-hold.md b/.changeset/27485-studio-formula-hold.md new file mode 100644 index 00000000000..742daade7eb --- /dev/null +++ b/.changeset/27485-studio-formula-hold.md @@ -0,0 +1,13 @@ +--- +'@object-ui/app-shell': patch +'@object-ui/i18n': patch +--- + +fix(studio): closing the field inspector keeps a formula that does not parse unsaved, with the field's stored result type, and a switch to another object asks before it drops it (#27485) + +Clause-②: no +Metadata-protocol: none + +- The Data pillar holds a field's draft while its CEL formula or condition does not parse. That hold used to end the moment the field inspector closed (Escape, Close, another field card, another tab), and the autosave then sent the broken expression as the object's draft, which publish refuses. The hold now belongs to the draft: it lasts while the draft still carries the text the inspector judged, and ends when that text is edited, the field is removed or the object reloads. While it lasts, the pillar shows the held line ("Not saved yet: an expression on the field “F Total” has an error to fix…") with a **Show me** that reopens the field, and the Studio's Publish names it. +- Typing a formula that does not parse (`record.est_hours > (`) cleared the field's stored `returnType`, because a broken source proves no type. The stored type now stays while the formula has a lint error; a formula that parses stamps its proven type, or clears it when none is proven, as before. +- With such an edit held, switching to another object in the Data pillar's rail dropped it silently. The switch now asks first; declining keeps the object and the edit. diff --git a/.changeset/27511-studio-rail-switch-sends-pending-edit.md b/.changeset/27511-studio-rail-switch-sends-pending-edit.md new file mode 100644 index 00000000000..9b36a5ec244 --- /dev/null +++ b/.changeset/27511-studio-rail-switch-sends-pending-edit.md @@ -0,0 +1,10 @@ +--- +'@object-ui/app-shell': patch +--- + +fix(studio): switching objects in the Data pillar's rail sends an edit made inside the autosave delay instead of dropping it (#27511) + +Clause-②: no +Metadata-protocol: none + +The Studio autosave sends a draft 1.5 s after the last edit, and sends nothing once another object has been opened. An edit made less than 1.5 s before a click on another object in the Data pillar's rail (a field removed, a label renamed) was therefore dropped, with nothing on screen, and was gone after a reload. The switch now sends that edit to its own object first and opens the next object once the save has landed; a save already in flight is waited out the same way. An edit that cannot be sent (its save was refused, or Studio holds it as unfinished) is not dropped silently: the switch asks first, and declining keeps the open object and its edit. diff --git a/ui/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.returnTypeHeld-27485.test.tsx b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.returnTypeHeld-27485.test.tsx new file mode 100644 index 00000000000..ef39500cb17 --- /dev/null +++ b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.returnTypeHeld-27485.test.tsx @@ -0,0 +1,126 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#27485 — a formula that does not parse keeps the field's stored + * `returnType`. + * + * The inspector stamps `returnType` from the formula's inferred CEL type once + * the author edits it. A source with a parse fault infers no type (`unknown`), + * so every keystroke that broke the formula cleared the stored type, and the + * broken body reached the server without it. The stamp is now settled from the + * editor's two verdicts on one source: a source with a lint error proves no + * type and the stored one stays; a source that lints clean stamps the proven + * type, or clears it when none is proven, as before. + * + * The engine is stubbed so both verdicts are deterministic; the live lint and + * inference are CelPredicateField.test.tsx's job. + */ + +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'; + +vi.mock('../useMetadata', () => ({ + useMetadataClient: () => ({ + list: vi.fn().mockResolvedValue([]), + listDrafts: vi.fn().mockResolvedValue([]), + }), +})); + +vi.mock('../previews/useObjectFields', () => ({ + useObjectFields: () => ({ fields: [], loading: false, error: null }), +})); + +import { ObjectFieldInspector } from './ObjectFieldInspector'; +import { __setCelFormulaLoader } from '../celAuthoring'; + +afterEach(() => { + cleanup(); + __setCelFormulaLoader(undefined); +}); + +/** The card's repro shape: a source left open on a bracket or an operator. */ +const BROKEN = /[*+\-/&|=<>(]\s*$/; + +function stubEngine() { + __setCelFormulaLoader(() => + Promise.resolve({ + validateExpression: (_role: string, input: unknown) => { + const src = typeof input === 'string' ? input : String((input as { source?: string })?.source ?? ''); + return BROKEN.test(src) + ? { ok: false, errors: [{ message: 'Parse error: unexpected end of input' }], warnings: [] } + : { ok: true, errors: [], warnings: [] }; + }, + introspectScope: () => ({ fields: ['est_hours'], roots: ['record'], functions: ['has'] }), + // A broken source proves no type, as the engine answers it. + inferExpressionType: (input: unknown) => { + const src = typeof input === 'string' ? input : ''; + if (BROKEN.test(src)) return 'unknown' as const; + if (src.includes("'")) return 'text' as const; + if (src.includes('+')) return 'unknown' as const; + return 'number' as const; + }, + }), + ); +} + +function Harness({ onDef }: { onDef: (def: Record) => void }) { + const [fields, setFields] = React.useState>>({ + est_hours: { type: 'number' }, + f_total: { type: 'formula', label: 'F Total', expression: 'record.est_hours * 2', returnType: 'number' }, + }); + React.useEffect(() => { + onDef(fields.f_total); + }); + return ( + ) => setFields(patch.fields as typeof fields)} + onClearSelection={() => {}} + onSelectionChange={() => {}} + readOnly={false} + locale={'en-US'} + onBlockingIssuesChange={() => {}} + /> + ); +} + +function renderFormula() { + stubEngine(); + const defs: Array> = []; + render( defs.push(def)} />); + const box = screen.getByText('Formula (CEL)').parentElement!.querySelector('[role="combobox"]') as HTMLTextAreaElement; + const latest = () => defs.at(-1)!; + return { box, latest }; +} + +const settle = () => new Promise((r) => setTimeout(r, 600)); + +describe('ObjectFieldInspector — a broken formula keeps the stored returnType (objectstack#27485)', () => { + it("the card's repro: `record.est_hours > (` leaves returnType as stored", async () => { + const { box, latest } = renderFormula(); + fireEvent.change(box, { target: { value: 'record.est_hours > (' } }); + await waitFor(() => expect(screen.getByText(/Parse error/)).toBeInTheDocument(), { timeout: 3000 }); + await settle(); + expect(latest().expression).toBe('record.est_hours > ('); + expect(latest().returnType).toBe('number'); + }); + + it('a source that parses again stamps its proven type', async () => { + const { box, latest } = renderFormula(); + fireEvent.change(box, { target: { value: 'record.est_hours > (' } }); + await waitFor(() => expect(screen.getByText(/Parse error/)).toBeInTheDocument(), { timeout: 3000 }); + fireEvent.change(box, { target: { value: "'hours: ' + string(record.est_hours)" } }); + await waitFor(() => expect(latest().returnType).toBe('text'), { timeout: 3000 }); + }); + + it('a source that parses and proves no type still clears it, as before', async () => { + const { box, latest } = renderFormula(); + fireEvent.change(box, { target: { value: 'record.est_hours + record.est_hours' } }); + await waitFor(() => expect(latest().returnType).toBeUndefined(), { timeout: 3000 }); + }); +}); diff --git a/ui/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.tsx b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.tsx index f5ff3e8a0dc..8525dca5eff 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.tsx @@ -121,7 +121,7 @@ import { useExternalWriteState, } from '../previews/external-object.js'; import { CelPredicateField } from '../CelPredicateField.js'; -import type { CelLintIssue } from '../celAuthoring.js'; +import type { CelLintIssue, CelValueType } from '../celAuthoring.js'; import { t, tFormat, type SupportedLocale } from '../i18n.js'; import { FilterConditionField } from './FilterConditionField.js'; import type { FilterCondition } from './datasetFilterCondition.js'; @@ -726,6 +726,20 @@ export function ObjectFieldInspector({ React.useEffect(() => { formulaEdited.current = false; }, [name]); + // objectstack#27485 — the formula editor's two verdicts on one source, its + // lint and its inferred type, which arrive separately and in either order. + // The stamp is settled once both are in for the source the draft holds: a + // source with a lint error proves no type, so the stored `returnType` stays + // (it was cleared on every keystroke that broke the formula, and the broken + // body then reached the server without it); a source that lints clean + // stamps the proven type, or clears it, as before. + const formulaVerdictRef = React.useRef<{ + field: string; + source: string; + errors?: number; + typed?: boolean; + inferred?: CelValueType | null; + }>({ field: '', source: '' }); /* ─── Who owns the API name (objectui#7615) ────────────────────────────── * @@ -892,6 +906,20 @@ export function ObjectFieldInspector({ writeView({ shape: view.shape, entries: nextEntries }); }; + /** objectstack#27485 — records one of the formula's verdicts and settles `returnType` (see `formulaVerdictRef`). */ + const settleFormulaType = (verdict: { errors: number } | { typed: true; inferred: CelValueType | null }) => { + const source = readPredicate(def.expression ?? def.formula); + const was = formulaVerdictRef.current; + const same = was.field === entry.name && was.source === source; + const now = { ...(same ? was : { field: entry.name, source }), ...verdict }; + formulaVerdictRef.current = now; + if (!formulaEdited.current || now.errors === undefined || !now.typed || now.errors > 0) return; + // Spec: `returnType` carries only a PROVEN concrete type — + // an ambiguous (`unknown`) or unavailable verdict clears it. + const next = now.inferred && now.inferred !== 'unknown' ? now.inferred : undefined; + if (def.returnType !== next) patchDef({ returnType: next }); + }; + /** * objectui#10202 — "use picklist". Naming a list sets `picklist` and removes * `options` outright (the key is ABSENT, as `unsetDefKey` leaves it): the @@ -1384,14 +1412,11 @@ export function ObjectFieldInspector({ ...(v.trim() ? {} : { returnType: undefined }), }); }} - onLintChange={(issues) => reportCel('formula', issues)} - onInferredTypeChange={(inferred) => { - if (!formulaEdited.current) return; - // Spec: `returnType` carries only a PROVEN concrete type — - // an ambiguous (`unknown`) or unavailable verdict clears it. - const next = inferred && inferred !== 'unknown' ? inferred : undefined; - if (def.returnType !== next) patchDef({ returnType: next }); + onLintChange={(issues) => { + reportCel('formula', issues); + settleFormulaType({ errors: issues.filter((i) => i.severity === 'error').length }); }} + onInferredTypeChange={(inferred) => settleFormulaType({ typed: true, inferred })} disabled={readOnly} placeholder="record.amount * 0.2" objectName={typeof (draft as any).name === 'string' ? ((draft as any).name as string) : undefined} diff --git a/ui/packages/app-shell/src/views/studio-design/DataPillar.celGate.test.tsx b/ui/packages/app-shell/src/views/studio-design/DataPillar.celGate.test.tsx index e0d9c94bb4a..3220cb0af8b 100644 --- a/ui/packages/app-shell/src/views/studio-design/DataPillar.celGate.test.tsx +++ b/ui/packages/app-shell/src/views/studio-design/DataPillar.celGate.test.tsx @@ -15,10 +15,17 @@ * * - the count actually reaches the button and disables it, with the same * message the RLS editor already shows; - * - the host RESETS its own count when the selection changes or the inspector - * unmounts (ruling sub-decision A) — writability must never depend on a - * child's teardown running, or closing the panel on a faulty formula leaves - * Save wedged shut with no editor on screen to fix it. + * - objectstack#27485 — the verdict belongs to the DRAFT, not to the + * selection: closing the inspector (Close, Escape, another field, another + * tab) keeps the hold while the draft still carries the text it judged, and + * a switch to another object asks before it drops the held edit. The + * maintainer's ruling on #27485 (comment 6500685972, 2026-10-29T15:17Z) + * replaced sub-decision A's reset-on-close mechanism with this hold; the + * objectui#4306 ruling's sub-decision A stands as its two invariants: + * writability never depends on a child's teardown running (the kept verdict + * is read off the draft, and an edit, the field's removal or a load ends + * it), and Save is never wedged with nothing on screen to fix (the held line + * names the field, and its "Show me" reopens the editor). * * The engine is stubbed so the verdict is deterministic; the live lint is * CelPredicateField.test.tsx's job. @@ -26,7 +33,7 @@ import '@testing-library/jest-dom/vitest'; import * as React from 'react'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react'; import { MemoryRouter } from 'react-router-dom'; @@ -39,11 +46,18 @@ const objectDef = { ], }; +const siteDef = { name: 'showcase_site', label: 'Site', fields: [{ name: 'address', label: 'Address', type: 'text' }] }; + const mockClient = { save: vi.fn(async () => ({})), - list: vi.fn(async () => [{ name: 'showcase_task', label: 'Task' }]), + list: vi.fn(async () => [ + { name: 'showcase_task', label: 'Task' }, + { name: 'showcase_site', label: 'Site' }, + ]), listDrafts: vi.fn(async () => []), - layered: vi.fn(async () => ({ effective: objectDef, code: objectDef })), + layered: vi.fn(async (_type: string, name: string) => + name === 'showcase_site' ? { effective: siteDef, code: siteDef } : { effective: objectDef, code: objectDef }, + ), getDraft: vi.fn(async () => null), }; @@ -103,6 +117,13 @@ failOnAbsorbedFetchError(); // The right rail resolves the field inspector through the registry. registerBuiltinInspectors(); +// This environment ships no window.confirm: the pillar's discard question. +let confirmSpy: ReturnType; +beforeEach(() => { + confirmSpy = vi.fn(() => false); + window.confirm = confirmSpy as unknown as typeof window.confirm; +}); + afterEach(() => { cleanup(); __setCelFormulaLoader(undefined); @@ -172,23 +193,74 @@ describe('DataPillar — auto-save is gated on the field inspector’s CEL verdi fireEvent.change(box, { target: { value: 'record.est_hours * 2' } }); await waitFor(() => expect(mockClient.save).toHaveBeenCalled(), { timeout: 4000 }); }); +}); - /** - * Sub-decision A. Closing the panel unmounts the inspector, so nothing will - * ever report `0` for it — the host must drop the count on its own, or the - * dirty draft stays wedged UNSAVED forever with no editor on screen to fix. - */ - it('drops the count when the inspector closes, so the draft cannot wedge unsaved', async () => { +/** + * objectstack#27485, the switch arm — shared with objectstack#27511's + * flush-or-ask switch: an edit the CEL verdict holds cannot be sent, so a + * switch to another object asks before its load replaces the buffer. + */ +describe('DataPillar — a switch to another object over a held formula (objectstack#27485)', () => { + it('asks; declined, the edit stays; agreed, it is dropped unsent', async () => { const box = await openFormulaField(); + fireEvent.change(box, { target: { value: 'record.est_hours *' } }); + await new Promise((r) => setTimeout(r, 2300)); + expect(mockClient.save).not.toHaveBeenCalled(); + fireEvent.click(screen.getByRole('button', { name: /Site/ })); + expect(confirmSpy).toHaveBeenCalledTimes(1); + expect(box.value).toBe('record.est_hours *'); + + confirmSpy.mockReturnValue(true); + fireEvent.click(screen.getByRole('button', { name: /Site/ })); + await waitFor(() => expect(screen.queryByText('Est hours')).toBeNull(), { timeout: 3000 }); + await new Promise((r) => setTimeout(r, 2300)); + expect(mockClient.save).not.toHaveBeenCalled(); + }); +}); + +/** The ways the field inspector closes (the card's step 4). */ +const CLOSERS: Array<[string, () => void]> = [ + ['its Close button', () => fireEvent.click(screen.getAllByRole('button', { name: 'Close' })[0])], + ['Escape', () => fireEvent.keyDown(document.body, { key: 'Escape' })], + ['another field card', () => fireEvent.click((screen.getByText('Est hours').closest('.cursor-grab') as HTMLElement))], + ['the Records tab', () => fireEvent.click(screen.getByRole('button', { name: 'Records' }))], +]; + +describe('DataPillar — the CEL hold belongs to the draft, not to the open inspector (objectstack#27485)', () => { + async function breakFormula(): Promise { + const box = await openFormulaField(); fireEvent.change(box, { target: { value: 'record.est_hours *' } }); + await waitFor(() => expect(screen.getByTestId('studio-held-message')).toHaveTextContent('Total'), { timeout: 3000 }); + return box; + } + + it.each(CLOSERS)('closing the inspector by %s keeps the hold, and the held line names the field', async (_how, close) => { + await breakFormula(); + close(); await new Promise((r) => setTimeout(r, 2300)); expect(mockClient.save).not.toHaveBeenCalled(); + expect(screen.getByTestId('studio-held-message')).toHaveTextContent('Total'); + }); - // Two "Close" buttons dismiss the panel — the rail header's and the - // inspector shell's own; either clears the selection (and the block). + it('"Show me" reopens the field, and fixing it there releases the hold', async () => { + await breakFormula(); fireEvent.click(screen.getAllByRole('button', { name: 'Close' })[0]); await waitFor(() => expect(screen.queryByText('Formula (CEL)')).toBeNull(), { timeout: 3000 }); + fireEvent.click(screen.getByRole('button', { name: 'Show me' })); + const label = await screen.findByText('Formula (CEL)'); + const box = label.parentElement!.querySelector('[role="combobox"]') as HTMLTextAreaElement; + expect(box.value).toBe('record.est_hours *'); + fireEvent.change(box, { target: { value: 'record.est_hours * 2' } }); + await waitFor(() => expect(mockClient.save).toHaveBeenCalled(), { timeout: 4000 }); + expect(screen.queryByTestId('studio-held')).toBeNull(); + }); + + it('removing the field ends the hold: nothing judged is left in the draft', async () => { + await breakFormula(); + fireEvent.click(screen.getByRole('button', { name: 'Remove “Total”' })); await waitFor(() => expect(mockClient.save).toHaveBeenCalled(), { timeout: 4000 }); + const body = (mockClient.save.mock.calls.at(-1) as unknown[] | undefined)?.[2] as { fields: Array<{ name: string }> }; + expect(body.fields.map((f) => f.name)).toEqual(['est_hours']); }); }); diff --git a/ui/packages/app-shell/src/views/studio-design/DataPillar.railSwitchFlush-27511.test.tsx b/ui/packages/app-shell/src/views/studio-design/DataPillar.railSwitchFlush-27511.test.tsx new file mode 100644 index 00000000000..964de997444 --- /dev/null +++ b/ui/packages/app-shell/src/views/studio-design/DataPillar.railSwitchFlush-27511.test.tsx @@ -0,0 +1,256 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#27511 — an object switch in the Data pillar's rail never drops + * an edit still inside the autosave's debounce. + * + * The autosave sends 1.5 s after the last edit, and its timer sends nothing + * once the open object has changed (objectui#11232): the rail's switch reloads + * the one buffer the edit lives in. So an edit made less than 1.5 s before a + * click on another object was dropped with nothing on screen, and reverted on + * the next visit. The switch now sends the pending edit first with the + * autosave's own `flush` and opens the next object once its save has landed; a + * save already in flight is waited out the same way; an edit that cannot be + * sent (its save was refused) is the author's to drop, and the switch asks. + * + * The client is a server double that records every save and can hold or + * refuse one; the records grid is a double that draws only its "+" column. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +// Module scope as well as in the `vi.mock` factory below: the cold transform is paid +// during collection, which no test or hook timeout clocks (AGENTS.md, flaky-test discipline). +import '@object-ui/components'; +import { render, screen, fireEvent, cleanup, waitFor, act } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; + +const PKG = 'com.acme.app'; + +const TASK = { + name: 'acme_task', + label: 'Task', + fields: [ + { name: 'title', label: 'Title', type: 'text' }, + { name: 'status', label: 'Status', type: 'text' }, + ], +}; +const NOTE = { name: 'acme_note', label: 'Note', fields: [{ name: 'body', label: 'Body', type: 'text' }] }; + +const server = vi.hoisted(() => ({ + active: new Map>(), + drafts: new Map>(), + /** Every save, as the server received it. */ + saves: [] as Array<{ name: string; body: Record }>, + /** While set, the next save waits on it (a save held in flight). */ + saveHold: null as Promise | null, + /** While true, every save is refused. */ + refuse: false, +})); + +// A server double: a save lands as the object's draft, and a later load serves it. +const mockClient = vi.hoisted(() => ({ + list: vi.fn(async () => [...server.active.values()].map((row) => ({ name: row.name, label: row.label }))), + listDrafts: vi.fn(async () => []), + listTypes: vi.fn(async () => ({ entries: [] })), + get: vi.fn(async () => null), + withPreviewDrafts: vi.fn((): unknown => mockClient), + references: vi.fn(async () => []), + layered: vi.fn(async (_type: string, name: string) => { + const eff = server.active.get(name) ?? null; + return { code: null, overlay: eff, overlayScope: eff ? 'env' : null, effective: eff, editable: true, deletable: true, resettable: false, lock: 'none' }; + }), + getDraft: vi.fn(async (type: string, name: string) => { + const draft = server.drafts.get(name); + if (draft) return { item: JSON.parse(JSON.stringify(draft)) as Record }; + throw Object.assign(new Error(`No pending draft exists for ${type}/${name}.`), { code: 'NO_DRAFT', status: 404 }); + }), + save: vi.fn(async (_type: string, name: string, item: unknown) => { + const body = JSON.parse(JSON.stringify(item)) as Record; + server.saves.push({ name, body }); + const hold = server.saveHold; + server.saveHold = null; + if (hold) await hold; + if (server.refuse) throw Object.assign(new Error('The server refused this draft.'), { status: 500 }); + server.drafts.set(name, body); + return { type: 'object', name, item }; + }), +})); + +vi.mock('../metadata-admin/useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, useMetadataClient: () => mockClient, useMetadataTypes: () => ({ loading: false, error: null, entries: [] }) }; +}); + +vi.mock('./packages-io', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, fetchPackages: vi.fn(async () => [{ id: PKG, name: 'Acme', writable: true, namespace: 'acme' }]) }; +}); + +vi.mock('@object-ui/react', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, useAdapter: () => dataSource }; +}); + +// The records grid: only its "+" column, drawn from the grid's authoring +// context the way the real grid draws it (objectui#12511). +vi.mock('@object-ui/plugin-view', async (importOriginal) => { + const mod = await importOriginal>(); + const { useGridFieldAuthoring } = await import('@object-ui/components'); + function GridDouble() { + const authoring = useGridFieldAuthoring(); + return authoring?.onAddColumn ? ( + + ) : null; + } + return { ...mod, ObjectView: GridDouble }; +}); + +vi.mock('sonner', () => ({ + toast: { success: vi.fn(), error: vi.fn(), info: vi.fn(), dismiss: vi.fn() }, + Toaster: () => null, +})); + +import { DataPillar } from './StudioDesignSurface'; +import { addFieldFromGrid } from './__tests__/dataPillarAddField'; +import { createEmptyDataSource } from './__tests__/emptyDataSource'; +import { readFields } from '../metadata-admin/previews/object-fields-io'; + +const dataSource = createEmptyDataSource(); + +window.matchMedia = ((query: string) => ({ + matches: false, + media: query, + onchange: null, + addEventListener: () => {}, + removeEventListener: () => {}, + addListener: () => {}, + removeListener: () => {}, + dispatchEvent: () => false, +})) as unknown as typeof window.matchMedia; +(globalThis as unknown as { ResizeObserver: unknown }).ResizeObserver = + (globalThis as unknown as { ResizeObserver?: unknown }).ResizeObserver ?? + class { + observe() {} + unobserve() {} + disconnect() {} + }; + +// This environment ships no window.confirm: the pillar's discard question. +let confirmSpy: ReturnType; + +beforeEach(() => { + server.active.clear(); + server.drafts.clear(); + server.saves.length = 0; + server.saveHold = null; + server.refuse = false; + server.active.set(TASK.name, JSON.parse(JSON.stringify(TASK))); + server.active.set(NOTE.name, JSON.parse(JSON.stringify(NOTE))); + confirmSpy = vi.fn(() => false); + window.confirm = confirmSpy as unknown as typeof window.confirm; + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response('null', { status: 404, headers: { 'content-type': 'application/json' } })), + ); +}); + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); + +const SLOW = { timeout: 8000 }; + +const fieldNamesOf = (body: Record) => readFields(body.fields).entries.map((e) => e.name); +const savesRead = () => server.saves.map((s) => [s.name, fieldNamesOf(s.body)]); +const fieldCount = () => screen.getByText(/^\d+ fields$/); + +async function openTask(): Promise { + render( + + + , + ); + await waitFor(() => expect(fieldCount()).toHaveTextContent('2 fields'), SLOW); +} + +/** Waits past the autosave's debounce: what was sent by then is what will be. */ +async function pastDebounce(ms = 2500): Promise { + await act(async () => { + await new Promise((r) => setTimeout(r, ms)); + }); +} + +describe('Data pillar — an object switch inside the autosave debounce (objectstack#27511)', () => { + it('sends the pending edit to its own object first, then opens the next one', async () => { + await openTask(); + await addFieldFromGrid(); + // Inside the debounce: nothing has been sent yet. + expect(server.saves).toEqual([]); + fireEvent.click(screen.getByRole('button', { name: /Note/ })); + + await waitFor(() => expect(fieldCount()).toHaveTextContent('1 fields'), SLOW); + await pastDebounce(); + // Exactly one save, to Task, carrying the edit; nothing was sent as Note's. + expect(savesRead()).toEqual([['acme_task', ['title', 'status', 'new_field']]]); + expect(confirmSpy).not.toHaveBeenCalled(); + + // The edit is Task's draft: back on Task, it is there. + fireEvent.click(screen.getByRole('button', { name: /Task/ })); + await waitFor(() => expect(fieldCount()).toHaveTextContent('3 fields'), SLOW); + }, 30000); + + it('waits out a save in flight, and opens the next object once it lands', async () => { + await openTask(); + let release!: () => void; + server.saveHold = new Promise((r) => (release = r)); + await addFieldFromGrid(); + // The timer sends it, and the save is held in flight. + await waitFor(() => expect(server.saves).toHaveLength(1), SLOW); + + fireEvent.click(screen.getByRole('button', { name: /Note/ })); + await pastDebounce(300); + // Still on Task while its save is in flight. + expect(fieldCount()).toHaveTextContent('3 fields'); + + await act(async () => { + release(); + await new Promise((r) => setTimeout(r, 0)); + }); + await waitFor(() => expect(fieldCount()).toHaveTextContent('1 fields'), SLOW); + await pastDebounce(); + expect(savesRead()).toEqual([['acme_task', ['title', 'status', 'new_field']]]); + }, 30000); + + it('an edit whose save was refused is not dropped silently: the switch asks, and a declined ask keeps it', async () => { + await openTask(); + server.refuse = true; + await addFieldFromGrid(); + await waitFor(() => expect(server.saves).toHaveLength(1), SLOW); + await pastDebounce(300); + + fireEvent.click(screen.getByRole('button', { name: /Note/ })); + expect(confirmSpy).toHaveBeenCalledTimes(1); + await pastDebounce(300); + expect(fieldCount()).toHaveTextContent('3 fields'); + + // Agreed to: the edit is dropped and Note opens; nothing more was sent. + confirmSpy.mockReturnValue(true); + fireEvent.click(screen.getByRole('button', { name: /Note/ })); + await waitFor(() => expect(fieldCount()).toHaveTextContent('1 fields'), SLOW); + expect(server.saves).toHaveLength(1); + }, 30000); + + it('a switch with nothing pending opens the next object at once and sends nothing', async () => { + await openTask(); + fireEvent.click(screen.getByRole('button', { name: /Note/ })); + await waitFor(() => expect(fieldCount()).toHaveTextContent('1 fields'), SLOW); + await pastDebounce(); + expect(server.saves).toEqual([]); + expect(confirmSpy).not.toHaveBeenCalled(); + }, 30000); +}); diff --git a/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.autosaveSwitch-11232.test.tsx b/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.autosaveSwitch-11232.test.tsx index 76a1f7c252a..ed11684ee71 100644 --- a/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.autosaveSwitch-11232.test.tsx +++ b/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.autosaveSwitch-11232.test.tsx @@ -18,9 +18,11 @@ * After a switch it sends nothing until the caller's dirty flag has fallen * (the new item's load installs its own buffer and clears it), and a save's * claim on its buffer reads moved once the item has changed. The previous - * item's pending edit is dropped, which is what the Data pillar's switch has - * always done, and what every pillar did whenever the new load landed inside - * the debounce. The Data pillar is the control. + * item's pending edit is dropped by the hook, which is what every pillar did + * whenever the new load landed inside the debounce. The Data pillar is the + * control: its rail sends the pending edit to its own object with the hook's + * `flush` before it switches (objectstack#27511), so nothing of it reaches the + * object opened next either. * * objectui#11203 (folded in): a package switch keeps the Interfaces pillar * mounted, so the nav editor's dirty flag and open editing survived into the @@ -431,7 +433,7 @@ describe('Interfaces page inspector — a save in flight across a page switch (o }); describe('Data pillar — an object switch inside the debounce, the control (objectui#11232)', () => { - it('sends nothing to the newly opened object, as it always has', async () => { + it('sends nothing to the newly opened object; the pending edit goes to its own (objectstack#27511)', async () => { render( @@ -445,7 +447,11 @@ describe('Data pillar — an object switch inside the debounce, the control (obj await pastDebounce(); expect(savesOf('object', 'acme_note')).toEqual([]); - expect(server.saves).toEqual([]); + // objectstack#27511 — the switch sent the edit to Task before it opened Note. + expect(savesOf('object', 'acme_task').map((b) => readFields(b.fields).entries.map((e) => e.name))).toEqual([ + ['title', 'status', 'new_field'], + ]); + server.saves.length = 0; await release(); await waitFor(() => expect(screen.getByText(/^\d+ fields$/)).toHaveTextContent('1 fields'), SLOW); 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 112c2439faa..6c56242317e 100644 --- a/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx +++ b/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx @@ -6279,6 +6279,21 @@ function formFieldNames(fields: unknown): string[] { .filter((n) => !STUDIO_SYSTEM_FIELD_NAMES.has(n)); } +/** + * objectstack#27485 — what the field inspector's CEL verdict on `field` was + * judged from, as the object draft holds it: the field's type, its CEL texts + * (`expression`, the legacy `formula`, `visibleWhen`, `readonlyWhen`, + * `requiredWhen`) and the object's field names its lint resolves references + * against. The same stamp is the same verdict. `null` when the field is gone. + */ +function fieldCelStamp(draft: Record, field: string): string | null { + const { entries } = readFields(draft.fields); + const def = entries.find((e) => e.name === field)?.def; + if (!def) return null; + const cel = [def.type, def.expression, def.formula, def.visibleWhen, def.readonlyWhen, def.requiredWhen]; + return JSON.stringify([cel.map((v) => v ?? null), 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']); @@ -6415,13 +6430,54 @@ export function DataPillar({ // that does not parse must not be saveable, let alone publishable as the live // field definition (objectui#4306). // - // The count is STAMPED with the selection it came from, so it expires by - // construction when the selection changes or the panel closes: an unmounted - // inspector can never retract its last verdict, and a host that waited for - // one would leave Save wedged shut with nothing on screen to fix. + // The report is STAMPED with the selection it came from: it is the open + // inspector's live verdict, and never reads as another selection's. const [blockingReport, setBlockingReport] = React.useState({ key: '', count: 0 }); const fieldSelKey = fieldSel ? `${current?.name ?? ''}:${fieldSel.kind}:${fieldSel.id}` : ''; - const inspectorBlocking = blockingReport.key === fieldSelKey ? blockingReport.count : 0; + // objectstack#27485 — the verdict belongs to the draft, not to the selection. + // Expiring it with the selection released the hold the moment the inspector + // closed (Escape, Close, another field, another tab) while the broken text + // stayed in the draft, and the autosave sent it. So each field's last live + // verdict is kept with the stamp of what it judged (`fieldCelStamp`, taken + // while the inspector is open on it) and holds, with the inspector closed, + // while the draft still carries that stamp: an edit to the text, the field's + // removal or the object's next load ends it. Nothing is left shut with + // nothing to fix: the held line names the field and its "Show me" opens it, + // and the open inspector's live verdict always wins. + const [celVerdicts, setCelVerdicts] = React.useState>({}); + const liveFieldReport = fieldSel?.kind === 'field' && blockingReport.key === fieldSelKey; + const openFieldStamp = fieldSel?.kind === 'field' ? fieldCelStamp(objDraft, fieldSel.id) : null; + React.useEffect(() => { + if (!liveFieldReport || fieldSel === null) return; + const kept = { field: fieldSel.id, count: blockingReport.count, stamp: openFieldStamp }; + setCelVerdicts((prev) => { + const was = prev[fieldSelKey]; + if (was && was.count === kept.count && was.stamp === kept.stamp) return prev; + return { ...prev, [fieldSelKey]: kept }; + }); + }, [liveFieldReport, fieldSel, fieldSelKey, blockingReport.count, openFieldStamp]); + // The open object's fields whose verdict holds, and their summed count. + const celHeldFields: string[] = []; + let inspectorBlocking = 0; + const celFieldPrefix = `${current?.name ?? ''}:field:`; + for (const [key, kept] of Object.entries(celVerdicts)) { + if (!key.startsWith(celFieldPrefix) || key === fieldSelKey) continue; + if (kept.count > 0 && kept.stamp === fieldCelStamp(objDraft, kept.field)) { + celHeldFields.push(kept.field); + inspectorBlocking += kept.count; + } + } + if (blockingReport.key === fieldSelKey && blockingReport.count > 0) { + if (fieldSel?.kind === 'field') celHeldFields.push(fieldSel.id); + inspectorBlocking += blockingReport.count; + } else if (fieldSelKey !== '' && !liveFieldReport) { + // Opened, and its inspector has not reported yet: its kept verdict stands. + const kept = celVerdicts[fieldSelKey]; + if (kept && kept.count > 0 && kept.stamp === openFieldStamp) { + celHeldFields.push(kept.field); + inspectorBlocking += kept.count; + } + } // The pillar's PANEL family reports separately (objectui#4527): the // validations, actions and settings panels all write through this same draft // and own no Save of their own, so their CEL faults have to reach this @@ -6691,6 +6747,8 @@ export function DataPillar({ setLoading(true); setError(null); setFieldSel(null); + // objectstack#27485 — kept CEL verdicts judged the buffer this load replaces. + setCelVerdicts({}); setDirty(false); (async () => { try { @@ -6958,7 +7016,7 @@ export function DataPillar({ // objectui#5813 — auto-save replaces the Save draft button; the blocked guard // is the button's old disabled-condition verbatim, plus the hold above. - const { sending: sendingObjDraft, loaded: objLoaded } = useDraftAutoSave({ + const { flush: flushObjSave, sending: sendingObjDraft, loaded: objLoaded } = useDraftAutoSave({ // objectui#11232 — the object `doSave` addresses. target: `object:${current?.name ?? ''}`, loadedFor: objDraftFor, @@ -6967,8 +7025,27 @@ export function DataPillar({ snapshot: objDraft, save: doSave, }); + // 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. + 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 + ? { + clause: tFormat('engine.studio.held.celInvalid', locale, { field: celHeldLabel }), + target: { kind: 'field', id: celHeldField }, + } + : null, + [dirty, celHeldField, celHeldLabel, locale], + ); // Only the open object's own buffer is held (objectui#11272). - const objHeld = objLoaded ? objIncomplete : null; + const objHeld = objLoaded ? (objIncomplete ?? objCelHeld) : null; // objectstack#26917 — the refusal's way out: the object holding the // identifier is on this rail, so it opens instead of a second one being // made. The New button already asked about a held edit (objectstack#26845). @@ -6985,9 +7062,47 @@ export function DataPillar({ const fieldAgentMark = objLoaded && fieldSel?.kind === 'field' ? agentDraftMarkOf(objDraftChanges, ['fields', fieldSel.id]) : null; useHeldEditReport(onHeldEditChange, objHeld?.clause ?? null); + // objectstack#27511 — opening another object replaces the one buffer the + // open object's edits live in, and the autosave's timer sends nothing once + // the open object has changed (objectui#11232). So an edit still inside the + // debounce was dropped by the switch with nothing on screen. The switch now + // sends it first with the autosave's own `flush` and opens the object once + // the buffer is clean, as the Interfaces nav's "Done" closes (objectui#11189); + // a save already in flight is waited out the same way. An edit that cannot + // be sent (held, or its save refused with the refusal on screen) is the + // author's to drop: the switch asks (objectstack#26705). + const [switchTo, setSwitchTo] = React.useState(null); + const openObject = React.useCallback( + (next: Surface): boolean => { + if (next.name === current?.name) { + setSwitchTo(null); + setCurrent(next); + return true; + } + if (saving || (dirty && objHeld === null && flushObjSave())) { + setSwitchTo(next); + return true; + } + if (!confirmDropUnsavedWork(dirty, locale)) return false; + setSwitchTo(null); + setCurrent(next); + return true; + }, + [current, saving, dirty, objHeld, flushObjSave, locale], + ); + React.useEffect(() => { + if (!switchTo || saving) return; + if (dirty && objHeld === null && flushObjSave()) return; + setSwitchTo(null); + // Still dirty: the save was refused (on screen) or an edit taken meanwhile + // is held. The author stays; the next switch asks. + if (dirty) return; + setCurrent(switchTo); + }, [switchTo, saving, dirty, objHeld, flushObjSave]); // The live request's apply (see `requestedSurface` above). objectstack#26845 // — another object's load replaces the buffer a held edit lives in, so it - // asks first, as the rail does; declined, the request is spent and the held + // goes through the rail's own switch (`openObject`): an unsent edit is sent + // first, or the author is asked; declined, the request is spent and the // edit stays. A request for the open object drops nothing and asks nothing. React.useEffect(() => { if (!requestedSurface || !objectsLoaded) return; @@ -6995,9 +7110,8 @@ export function DataPillar({ appliedRequestRef.current = requestedSurface.id; const match = resolveSurfaceDeepLink(objects, requestedSurface.target, 'object'); if (!match) return; - if (match.name !== current?.name && !confirmDropUnsavedWork(objHeld !== null, locale)) return; - setCurrent(match); - }, [requestedSurface, objects, objectsLoaded, current, objHeld, locale]); + openObject(match); + }, [requestedSurface, objects, objectsLoaded, openObject]); // objectstack#26845 — a created object opens at once, and its load replaces // the buffer a held edit lives in: asked before the dialog opens, so Cancel // costs nothing and the create itself asks nothing. @@ -7283,11 +7397,10 @@ export function DataPillar({ key={o.name} active={current?.name === o.name} onClick={() => { - // objectstack#26705 — another object's load replaces the - // buffer the held edit lives in; the open one keeps it. - if (current?.name !== o.name && !confirmDropUnsavedWork(objHeld !== null, locale)) return; - setCurrent(o); - if (isMobile) setRailOpen(false); + // objectstack#26705, objectstack#27511 — another object's load + // replaces the buffer the open one's edits live in: they are + // sent first, or the author is asked (`openObject`). + if (openObject(o) && isMobile) setRailOpen(false); }} icon={} > diff --git a/ui/packages/app-shell/src/views/studio-design/useDraftAutoSave.ts b/ui/packages/app-shell/src/views/studio-design/useDraftAutoSave.ts index 9a33e927534..16a18f51783 100644 --- a/ui/packages/app-shell/src/views/studio-design/useDraftAutoSave.ts +++ b/ui/packages/app-shell/src/views/studio-design/useDraftAutoSave.ts @@ -43,9 +43,11 @@ import * as React from 'react'; // load installs its own; so a period that began on another item is never // sent, by the timer or by `flush`, and it ends only when the caller's dirty // flag falls. -// The previous item's pending edit is dropped: what the Data pillar's switch -// has always done, and what every pillar did whenever the new load landed -// inside the debounce. A claim also reads moved once the target has +// The previous item's pending edit is dropped by the hook: what every +// pillar did whenever the new load landed inside the debounce. A caller +// that must not lose it sends it with `flush` BEFORE it switches, and +// switches once its save has landed: the Data pillar's object rail does +// (objectstack#27511). A claim also reads moved once the target has // changed, so a save that lands after a switch never clears the dirty flag // of the item opened since. // - objectui#11272 — the buffer belongs to the item it was loaded for. The diff --git a/ui/packages/i18n/src/namespaces/studio/en.ts b/ui/packages/i18n/src/namespaces/studio/en.ts index dea9ae057c2..cdf15d8911d 100644 --- a/ui/packages/i18n/src/namespaces/studio/en.ts +++ b/ui/packages/i18n/src/namespaces/studio/en.ts @@ -3010,6 +3010,8 @@ const STUDIO_EN: Record = { // objectui#11910 — a dashboard widget held until it is bound; `input` is the // widget inspector's own label for it (Dataset, Values (measures)). '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', '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 3f772523ddb..cf2fd8251ae 100644 --- a/ui/packages/i18n/src/namespaces/studio/zh.ts +++ b/ui/packages/i18n/src/namespaces/studio/zh.ts @@ -2679,6 +2679,7 @@ const STUDIO_ZH: Record = { 'engine.studio.held.needsRuleField': '规则「{rule}」需要选择一个字段', 'engine.studio.held.needsRuleMessage': '规则「{rule}」需要一条错误消息', 'engine.studio.held.widgetNeedsInput': '组件「{widget}」的「{input}」需要填写', + 'engine.studio.held.celInvalid': '字段「{field}」的表达式有错误需要修正', 'engine.studio.held.line': '尚未保存：{clause}。更改已保留在此处，补全后会自动保存。', 'engine.studio.held.publish': '未发布：{clause}，该更改尚未保存。请先补全，再发布。', 'engine.studio.held.inputHint': '必填。填写后才会保存更改。',