Skip to content

Commit 73b0ad8

Browse files
committed
fix(studio): closing the field inspector keeps the CEL hold on the draft (#27485)
The inspector's CEL verdict was stamped with the selection, so closing it (Close, Escape, another field, another tab) released the hold and the autosave sent the broken expression. Each field's last verdict is now kept with a stamp of the CEL text it judged and holds while the draft carries that text; the held line names the field with a Show me that reopens it. Reverses the mechanism of objectui#4306 sub-decision A (reset on close) while keeping its invariants; separable on purpose. Claude-Session: https://claude.ai/code/session_01Utp6CbCrKUjGeTReNEmUJx Co-authored-by: Claude <noreply@anthropic.com>
1 parent 19ceb5d commit 73b0ad8

5 files changed

Lines changed: 144 additions & 32 deletions

File tree

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
---
22
'@object-ui/app-shell': patch
3+
'@object-ui/i18n': patch
34
---
45

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+
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)
67

78
Clause-②: no
89
Metadata-protocol: none
910

11+
- 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.
1012
- 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.
1113
- 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.

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

Lines changed: 55 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,15 @@
1515
*
1616
* - the count actually reaches the button and disables it, with the same
1717
* message the RLS editor already shows;
18-
* - the host RESETS its own count when the selection changes or the inspector
19-
* unmounts (ruling sub-decision A) — writability must never depend on a
20-
* 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;
22-
* - objectstack#27485, the switch arm: a switch to another object over a
23-
* held formula asks before it drops the edit.
18+
* - objectstack#27485 — the verdict belongs to the DRAFT, not to the
19+
* selection: closing the inspector (Close, Escape, another field, another
20+
* tab) keeps the hold while the draft still carries the text it judged, and
21+
* a switch to another object asks before it drops the held edit. The
22+
* objectui#4306 ruling's sub-decision A stands as its two invariants:
23+
* writability never depends on a child's teardown running (the kept verdict
24+
* is read off the draft, and an edit, the field's removal or a load ends
25+
* it), and Save is never wedged with nothing on screen to fix (the held line
26+
* names the field, and its "Show me" reopens the editor).
2427
*
2528
* The engine is stubbed so the verdict is deterministic; the live lint is
2629
* CelPredicateField.test.tsx's job.
@@ -188,25 +191,6 @@ describe('DataPillar — auto-save is gated on the field inspector’s CEL verdi
188191
fireEvent.change(box, { target: { value: 'record.est_hours * 2' } });
189192
await waitFor(() => expect(mockClient.save).toHaveBeenCalled(), { timeout: 4000 });
190193
});
191-
192-
/**
193-
* Sub-decision A. Closing the panel unmounts the inspector, so nothing will
194-
* ever report `0` for it — the host must drop the count on its own, or the
195-
* dirty draft stays wedged UNSAVED forever with no editor on screen to fix.
196-
*/
197-
it('drops the count when the inspector closes, so the draft cannot wedge unsaved', async () => {
198-
const box = await openFormulaField();
199-
200-
fireEvent.change(box, { target: { value: 'record.est_hours *' } });
201-
await new Promise((r) => setTimeout(r, 2300));
202-
expect(mockClient.save).not.toHaveBeenCalled();
203-
204-
// Two "Close" buttons dismiss the panel — the rail header's and the
205-
// inspector shell's own; either clears the selection (and the block).
206-
fireEvent.click(screen.getAllByRole('button', { name: 'Close' })[0]);
207-
await waitFor(() => expect(screen.queryByText('Formula (CEL)')).toBeNull(), { timeout: 3000 });
208-
await waitFor(() => expect(mockClient.save).toHaveBeenCalled(), { timeout: 4000 });
209-
});
210194
});
211195

212196
/**
@@ -232,3 +216,49 @@ describe('DataPillar — a switch to another object over a held formula (objects
232216
expect(mockClient.save).not.toHaveBeenCalled();
233217
});
234218
});
219+
220+
/** The ways the field inspector closes (the card's step 4). */
221+
const CLOSERS: Array<[string, () => void]> = [
222+
['its Close button', () => fireEvent.click(screen.getAllByRole('button', { name: 'Close' })[0])],
223+
['Escape', () => fireEvent.keyDown(document.body, { key: 'Escape' })],
224+
['another field card', () => fireEvent.click((screen.getByText('Est hours').closest('.cursor-grab') as HTMLElement))],
225+
['the Records tab', () => fireEvent.click(screen.getByRole('button', { name: 'Records' }))],
226+
];
227+
228+
describe('DataPillar — the CEL hold belongs to the draft, not to the open inspector (objectstack#27485)', () => {
229+
async function breakFormula(): Promise<HTMLTextAreaElement> {
230+
const box = await openFormulaField();
231+
fireEvent.change(box, { target: { value: 'record.est_hours *' } });
232+
await waitFor(() => expect(screen.getByTestId('studio-held-message')).toHaveTextContent('Total'), { timeout: 3000 });
233+
return box;
234+
}
235+
236+
it.each(CLOSERS)('closing the inspector by %s keeps the hold, and the held line names the field', async (_how, close) => {
237+
await breakFormula();
238+
close();
239+
await new Promise((r) => setTimeout(r, 2300));
240+
expect(mockClient.save).not.toHaveBeenCalled();
241+
expect(screen.getByTestId('studio-held-message')).toHaveTextContent('Total');
242+
});
243+
244+
it('"Show me" reopens the field, and fixing it there releases the hold', async () => {
245+
await breakFormula();
246+
fireEvent.click(screen.getAllByRole('button', { name: 'Close' })[0]);
247+
await waitFor(() => expect(screen.queryByText('Formula (CEL)')).toBeNull(), { timeout: 3000 });
248+
fireEvent.click(screen.getByRole('button', { name: 'Show me' }));
249+
const label = await screen.findByText('Formula (CEL)');
250+
const box = label.parentElement!.querySelector('[role="combobox"]') as HTMLTextAreaElement;
251+
expect(box.value).toBe('record.est_hours *');
252+
fireEvent.change(box, { target: { value: 'record.est_hours * 2' } });
253+
await waitFor(() => expect(mockClient.save).toHaveBeenCalled(), { timeout: 4000 });
254+
expect(screen.queryByTestId('studio-held')).toBeNull();
255+
});
256+
257+
it('removing the field ends the hold: nothing judged is left in the draft', async () => {
258+
await breakFormula();
259+
fireEvent.click(screen.getByRole('button', { name: 'Remove “Total”' }));
260+
await waitFor(() => expect(mockClient.save).toHaveBeenCalled(), { timeout: 4000 });
261+
const body = (mockClient.save.mock.calls.at(-1) as unknown[] | undefined)?.[2] as { fields: Array<{ name: string }> };
262+
expect(body.fields.map((f) => f.name)).toEqual(['est_hours']);
263+
});
264+
});

‎ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx‎

Lines changed: 83 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6279,6 +6279,21 @@ function formFieldNames(fields: unknown): string[] {
62796279
.filter((n) => !STUDIO_SYSTEM_FIELD_NAMES.has(n));
62806280
}
62816281

6282+
/**
6283+
* objectstack#27485 — what the field inspector's CEL verdict on `field` was
6284+
* judged from, as the object draft holds it: the field's type, its CEL texts
6285+
* (`expression`, the legacy `formula`, `visibleWhen`, `readonlyWhen`,
6286+
* `requiredWhen`) and the object's field names its lint resolves references
6287+
* against. The same stamp is the same verdict. `null` when the field is gone.
6288+
*/
6289+
function fieldCelStamp(draft: Record<string, unknown>, field: string): string | null {
6290+
const { entries } = readFields(draft.fields);
6291+
const def = entries.find((e) => e.name === field)?.def;
6292+
if (!def) return null;
6293+
const cel = [def.type, def.expression, def.formula, def.visibleWhen, def.readonlyWhen, def.requiredWhen];
6294+
return JSON.stringify([cel.map((v) => v ?? null), entries.map((e) => e.name)]);
6295+
}
6296+
62826297
/** The data-source writes the Form tab's Preview refuses (objectui#12515). */
62836298
const PREVIEW_REFUSED_WRITES = new Set<PropertyKey>(['create', 'update', 'delete', 'bulk', 'bulkUpdate', 'bulkDelete']);
62846299

@@ -6415,13 +6430,54 @@ export function DataPillar({
64156430
// that does not parse must not be saveable, let alone publishable as the live
64166431
// field definition (objectui#4306).
64176432
//
6418-
// The count is STAMPED with the selection it came from, so it expires by
6419-
// construction when the selection changes or the panel closes: an unmounted
6420-
// inspector can never retract its last verdict, and a host that waited for
6421-
// one would leave Save wedged shut with nothing on screen to fix.
6433+
// The report is STAMPED with the selection it came from: it is the open
6434+
// inspector's live verdict, and never reads as another selection's.
64226435
const [blockingReport, setBlockingReport] = React.useState({ key: '', count: 0 });
64236436
const fieldSelKey = fieldSel ? `${current?.name ?? ''}:${fieldSel.kind}:${fieldSel.id}` : '';
6424-
const inspectorBlocking = blockingReport.key === fieldSelKey ? blockingReport.count : 0;
6437+
// objectstack#27485 — the verdict belongs to the draft, not to the selection.
6438+
// Expiring it with the selection released the hold the moment the inspector
6439+
// closed (Escape, Close, another field, another tab) while the broken text
6440+
// stayed in the draft, and the autosave sent it. So each field's last live
6441+
// verdict is kept with the stamp of what it judged (`fieldCelStamp`, taken
6442+
// while the inspector is open on it) and holds, with the inspector closed,
6443+
// while the draft still carries that stamp: an edit to the text, the field's
6444+
// removal or the object's next load ends it. Nothing is left shut with
6445+
// nothing to fix: the held line names the field and its "Show me" opens it,
6446+
// and the open inspector's live verdict always wins.
6447+
const [celVerdicts, setCelVerdicts] = React.useState<Record<string, { field: string; count: number; stamp: string | null }>>({});
6448+
const liveFieldReport = fieldSel?.kind === 'field' && blockingReport.key === fieldSelKey;
6449+
const openFieldStamp = fieldSel?.kind === 'field' ? fieldCelStamp(objDraft, fieldSel.id) : null;
6450+
React.useEffect(() => {
6451+
if (!liveFieldReport || fieldSel === null) return;
6452+
const kept = { field: fieldSel.id, count: blockingReport.count, stamp: openFieldStamp };
6453+
setCelVerdicts((prev) => {
6454+
const was = prev[fieldSelKey];
6455+
if (was && was.count === kept.count && was.stamp === kept.stamp) return prev;
6456+
return { ...prev, [fieldSelKey]: kept };
6457+
});
6458+
}, [liveFieldReport, fieldSel, fieldSelKey, blockingReport.count, openFieldStamp]);
6459+
// The open object's fields whose verdict holds, and their summed count.
6460+
const celHeldFields: string[] = [];
6461+
let inspectorBlocking = 0;
6462+
const celFieldPrefix = `${current?.name ?? ''}:field:`;
6463+
for (const [key, kept] of Object.entries(celVerdicts)) {
6464+
if (!key.startsWith(celFieldPrefix) || key === fieldSelKey) continue;
6465+
if (kept.count > 0 && kept.stamp === fieldCelStamp(objDraft, kept.field)) {
6466+
celHeldFields.push(kept.field);
6467+
inspectorBlocking += kept.count;
6468+
}
6469+
}
6470+
if (blockingReport.key === fieldSelKey && blockingReport.count > 0) {
6471+
if (fieldSel?.kind === 'field') celHeldFields.push(fieldSel.id);
6472+
inspectorBlocking += blockingReport.count;
6473+
} else if (fieldSelKey !== '' && !liveFieldReport) {
6474+
// Opened, and its inspector has not reported yet: its kept verdict stands.
6475+
const kept = celVerdicts[fieldSelKey];
6476+
if (kept && kept.count > 0 && kept.stamp === openFieldStamp) {
6477+
celHeldFields.push(kept.field);
6478+
inspectorBlocking += kept.count;
6479+
}
6480+
}
64256481
// The pillar's PANEL family reports separately (objectui#4527): the
64266482
// validations, actions and settings panels all write through this same draft
64276483
// and own no Save of their own, so their CEL faults have to reach this
@@ -6691,6 +6747,8 @@ export function DataPillar({
66916747
setLoading(true);
66926748
setError(null);
66936749
setFieldSel(null);
6750+
// objectstack#27485 — kept CEL verdicts judged the buffer this load replaces.
6751+
setCelVerdicts({});
66946752
setDirty(false);
66956753
(async () => {
66966754
try {
@@ -6967,8 +7025,27 @@ export function DataPillar({
69677025
snapshot: objDraft,
69687026
save: doSave,
69697027
});
7028+
// objectstack#27485 — an unsent edit the CEL verdict holds is named like an
7029+
// incomplete one: the held line, the surface's Publish and the rail's
7030+
// question all read it, so it outlives the inspector without going unseen.
7031+
const celHeldField = celHeldFields[0] ?? null;
7032+
const celHeldLabel = React.useMemo(() => {
7033+
if (celHeldField === null) return null;
7034+
const label = readFields(objDraft.fields).entries.find((e) => e.name === celHeldField)?.def.label;
7035+
return typeof label === 'string' && label.trim() !== '' ? label.trim() : celHeldField;
7036+
}, [celHeldField, objDraft.fields]);
7037+
const objCelHeld = React.useMemo<StudioHeld | null>(
7038+
() =>
7039+
dirty && celHeldField !== null && celHeldLabel !== null
7040+
? {
7041+
clause: tFormat('engine.studio.held.celInvalid', locale, { field: celHeldLabel }),
7042+
target: { kind: 'field', id: celHeldField },
7043+
}
7044+
: null,
7045+
[dirty, celHeldField, celHeldLabel, locale],
7046+
);
69707047
// Only the open object's own buffer is held (objectui#11272).
6971-
const objHeld = objLoaded ? objIncomplete : null;
7048+
const objHeld = objLoaded ? (objIncomplete ?? objCelHeld) : null;
69727049
// objectstack#26917 — the refusal's way out: the object holding the
69737050
// identifier is on this rail, so it opens instead of a second one being
69747051
// made. The New button already asked about a held edit (objectstack#26845).

‎ui/packages/i18n/src/namespaces/studio/en.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2999,6 +2999,8 @@ const STUDIO_EN: Record<string, string> = {
29992999
// objectui#11910 — a dashboard widget held until it is bound; `input` is the
30003000
// widget inspector's own label for it (Dataset, Values (measures)).
30013001
'engine.studio.held.widgetNeedsInput': '{input} on the widget “{widget}” needs a value',
3002+
// objectstack#27485 — a field CEL expression the inspector judged broken, held after it closes.
3003+
'engine.studio.held.celInvalid': 'an expression on the field “{field}” has an error to fix',
30023004
'engine.studio.held.line': 'Not saved yet: {clause}. Your changes are kept here and saved once it is filled in.',
30033005
'engine.studio.held.publish': 'Not published: {clause}, so that change is not saved yet. Finish it, then publish.',
30043006
'engine.studio.held.inputHint': 'Required. Changes are saved once this has a value.',

‎ui/packages/i18n/src/namespaces/studio/zh.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2668,6 +2668,7 @@ const STUDIO_ZH: Record<string, string> = {
26682668
'engine.studio.held.needsRuleField': '规则「{rule}」需要选择一个字段',
26692669
'engine.studio.held.needsRuleMessage': '规则「{rule}」需要一条错误消息',
26702670
'engine.studio.held.widgetNeedsInput': '组件「{widget}」的「{input}」需要填写',
2671+
'engine.studio.held.celInvalid': '字段「{field}」的表达式有错误需要修正',
26712672
'engine.studio.held.line': '尚未保存:{clause}。更改已保留在此处,补全后会自动保存。',
26722673
'engine.studio.held.publish': '未发布:{clause},该更改尚未保存。请先补全,再发布。',
26732674
'engine.studio.held.inputHint': '必填。填写后才会保存更改。',

0 commit comments

Comments
 (0)