Skip to content

Commit 19ceb5d

Browse files
committed
fix(studio): a broken formula keeps its stored returnType; a switch over a held formula asks (#27485)
The inspector settles returnType from the formula editor's lint and inferred type on one source: a source with a lint error proves no type, so the stored one stays. The object switch over an edit the CEL verdict holds asks first (the #27511 switch). Claude-Session: https://claude.ai/code/session_01Utp6CbCrKUjGeTReNEmUJx Co-authored-by: Claude <noreply@anthropic.com>
1 parent e72223f commit 19ceb5d

3 files changed

Lines changed: 181 additions & 4 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(studio): a formula that does not parse keeps the field's stored result type, and a switch to another object asks before it drops the held edit (#27485)
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
- 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.
11+
- 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.
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectstack#27485 — a formula that does not parse keeps the field's stored
5+
* `returnType`.
6+
*
7+
* The inspector stamps `returnType` from the formula's inferred CEL type once
8+
* the author edits it. A source with a parse fault infers no type (`unknown`),
9+
* so every keystroke that broke the formula cleared the stored type, and the
10+
* broken body reached the server without it. The stamp is now settled from the
11+
* editor's two verdicts on one source: a source with a lint error proves no
12+
* type and the stored one stays; a source that lints clean stamps the proven
13+
* type, or clears it when none is proven, as before.
14+
*
15+
* The engine is stubbed so both verdicts are deterministic; the live lint and
16+
* inference are CelPredicateField.test.tsx's job.
17+
*/
18+
19+
import '@testing-library/jest-dom/vitest';
20+
import * as React from 'react';
21+
import { describe, it, expect, vi, afterEach } from 'vitest';
22+
import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react';
23+
24+
vi.mock('../useMetadata', () => ({
25+
useMetadataClient: () => ({
26+
list: vi.fn().mockResolvedValue([]),
27+
listDrafts: vi.fn().mockResolvedValue([]),
28+
}),
29+
}));
30+
31+
vi.mock('../previews/useObjectFields', () => ({
32+
useObjectFields: () => ({ fields: [], loading: false, error: null }),
33+
}));
34+
35+
import { ObjectFieldInspector } from './ObjectFieldInspector';
36+
import { __setCelFormulaLoader } from '../celAuthoring';
37+
38+
afterEach(() => {
39+
cleanup();
40+
__setCelFormulaLoader(undefined);
41+
});
42+
43+
/** The card's repro shape: a source left open on a bracket or an operator. */
44+
const BROKEN = /[*+\-/&|=<>(]\s*$/;
45+
46+
function stubEngine() {
47+
__setCelFormulaLoader(() =>
48+
Promise.resolve({
49+
validateExpression: (_role: string, input: unknown) => {
50+
const src = typeof input === 'string' ? input : String((input as { source?: string })?.source ?? '');
51+
return BROKEN.test(src)
52+
? { ok: false, errors: [{ message: 'Parse error: unexpected end of input' }], warnings: [] }
53+
: { ok: true, errors: [], warnings: [] };
54+
},
55+
introspectScope: () => ({ fields: ['est_hours'], roots: ['record'], functions: ['has'] }),
56+
// A broken source proves no type, as the engine answers it.
57+
inferExpressionType: (input: unknown) => {
58+
const src = typeof input === 'string' ? input : '';
59+
if (BROKEN.test(src)) return 'unknown' as const;
60+
if (src.includes("'")) return 'text' as const;
61+
if (src.includes('+')) return 'unknown' as const;
62+
return 'number' as const;
63+
},
64+
}),
65+
);
66+
}
67+
68+
function Harness({ onDef }: { onDef: (def: Record<string, unknown>) => void }) {
69+
const [fields, setFields] = React.useState<Record<string, Record<string, unknown>>>({
70+
est_hours: { type: 'number' },
71+
f_total: { type: 'formula', label: 'F Total', expression: 'record.est_hours * 2', returnType: 'number' },
72+
});
73+
React.useEffect(() => {
74+
onDef(fields.f_total);
75+
});
76+
return (
77+
<ObjectFieldInspector
78+
type="object"
79+
name="work_order"
80+
draft={{ name: 'work_order', fields }}
81+
selection={{ kind: 'field', id: 'f_total' }}
82+
onPatch={(patch: Record<string, unknown>) => setFields(patch.fields as typeof fields)}
83+
onClearSelection={() => {}}
84+
onSelectionChange={() => {}}
85+
readOnly={false}
86+
locale={'en-US'}
87+
onBlockingIssuesChange={() => {}}
88+
/>
89+
);
90+
}
91+
92+
function renderFormula() {
93+
stubEngine();
94+
const defs: Array<Record<string, unknown>> = [];
95+
render(<Harness onDef={(def) => defs.push(def)} />);
96+
const box = screen.getByText('Formula (CEL)').parentElement!.querySelector('[role="combobox"]') as HTMLTextAreaElement;
97+
const latest = () => defs.at(-1)!;
98+
return { box, latest };
99+
}
100+
101+
const settle = () => new Promise((r) => setTimeout(r, 600));
102+
103+
describe('ObjectFieldInspector — a broken formula keeps the stored returnType (objectstack#27485)', () => {
104+
it("the card's repro: `record.est_hours > (` leaves returnType as stored", async () => {
105+
const { box, latest } = renderFormula();
106+
fireEvent.change(box, { target: { value: 'record.est_hours > (' } });
107+
await waitFor(() => expect(screen.getByText(/Parse error/)).toBeInTheDocument(), { timeout: 3000 });
108+
await settle();
109+
expect(latest().expression).toBe('record.est_hours > (');
110+
expect(latest().returnType).toBe('number');
111+
});
112+
113+
it('a source that parses again stamps its proven type', async () => {
114+
const { box, latest } = renderFormula();
115+
fireEvent.change(box, { target: { value: 'record.est_hours > (' } });
116+
await waitFor(() => expect(screen.getByText(/Parse error/)).toBeInTheDocument(), { timeout: 3000 });
117+
fireEvent.change(box, { target: { value: "'hours: ' + string(record.est_hours)" } });
118+
await waitFor(() => expect(latest().returnType).toBe('text'), { timeout: 3000 });
119+
});
120+
121+
it('a source that parses and proves no type still clears it, as before', async () => {
122+
const { box, latest } = renderFormula();
123+
fireEvent.change(box, { target: { value: 'record.est_hours + record.est_hours' } });
124+
await waitFor(() => expect(latest().returnType).toBeUndefined(), { timeout: 3000 });
125+
});
126+
});

‎ui/packages/app-shell/src/views/studio-design/DataPillar.celGate.test.tsx‎

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,17 @@
1818
* - the host RESETS its own count when the selection changes or the inspector
1919
* unmounts (ruling sub-decision A) — writability must never depend on a
2020
* child's teardown running, or closing the panel on a faulty formula leaves
21-
* Save wedged shut with no editor on screen to fix it.
21+
* Save wedged shut with no editor on screen to fix it;
22+
* - objectstack#27485, the switch arm: a switch to another object over a
23+
* held formula asks before it drops the edit.
2224
*
2325
* The engine is stubbed so the verdict is deterministic; the live lint is
2426
* CelPredicateField.test.tsx's job.
2527
*/
2628

2729
import '@testing-library/jest-dom/vitest';
2830
import * as React from 'react';
29-
import { afterEach, describe, expect, it, vi } from 'vitest';
31+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
3032
import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react';
3133
import { MemoryRouter } from 'react-router-dom';
3234

@@ -39,11 +41,18 @@ const objectDef = {
3941
],
4042
};
4143

44+
const siteDef = { name: 'showcase_site', label: 'Site', fields: [{ name: 'address', label: 'Address', type: 'text' }] };
45+
4246
const mockClient = {
4347
save: vi.fn(async () => ({})),
44-
list: vi.fn(async () => [{ name: 'showcase_task', label: 'Task' }]),
48+
list: vi.fn(async () => [
49+
{ name: 'showcase_task', label: 'Task' },
50+
{ name: 'showcase_site', label: 'Site' },
51+
]),
4552
listDrafts: vi.fn(async () => []),
46-
layered: vi.fn(async () => ({ effective: objectDef, code: objectDef })),
53+
layered: vi.fn(async (_type: string, name: string) =>
54+
name === 'showcase_site' ? { effective: siteDef, code: siteDef } : { effective: objectDef, code: objectDef },
55+
),
4756
getDraft: vi.fn(async () => null),
4857
};
4958

@@ -103,6 +112,13 @@ failOnAbsorbedFetchError();
103112
// The right rail resolves the field inspector through the registry.
104113
registerBuiltinInspectors();
105114

115+
// This environment ships no window.confirm: the pillar's discard question.
116+
let confirmSpy: ReturnType<typeof vi.fn>;
117+
beforeEach(() => {
118+
confirmSpy = vi.fn(() => false);
119+
window.confirm = confirmSpy as unknown as typeof window.confirm;
120+
});
121+
106122
afterEach(() => {
107123
cleanup();
108124
__setCelFormulaLoader(undefined);
@@ -192,3 +208,27 @@ describe('DataPillar — auto-save is gated on the field inspector’s CEL verdi
192208
await waitFor(() => expect(mockClient.save).toHaveBeenCalled(), { timeout: 4000 });
193209
});
194210
});
211+
212+
/**
213+
* objectstack#27485, the switch arm — shared with objectstack#27511's
214+
* flush-or-ask switch: an edit the CEL verdict holds cannot be sent, so a
215+
* switch to another object asks before its load replaces the buffer.
216+
*/
217+
describe('DataPillar — a switch to another object over a held formula (objectstack#27485)', () => {
218+
it('asks; declined, the edit stays; agreed, it is dropped unsent', async () => {
219+
const box = await openFormulaField();
220+
fireEvent.change(box, { target: { value: 'record.est_hours *' } });
221+
await new Promise((r) => setTimeout(r, 2300));
222+
expect(mockClient.save).not.toHaveBeenCalled();
223+
224+
fireEvent.click(screen.getByRole('button', { name: /Site/ }));
225+
expect(confirmSpy).toHaveBeenCalledTimes(1);
226+
expect(box.value).toBe('record.est_hours *');
227+
228+
confirmSpy.mockReturnValue(true);
229+
fireEvent.click(screen.getByRole('button', { name: /Site/ }));
230+
await waitFor(() => expect(screen.queryByText('Est hours')).toBeNull(), { timeout: 3000 });
231+
await new Promise((r) => setTimeout(r, 2300));
232+
expect(mockClient.save).not.toHaveBeenCalled();
233+
});
234+
});

0 commit comments

Comments
 (0)