Skip to content

Commit 5ac50e4

Browse files
fix(app-shell): optional choice rows in Studio metadata forms can be cleared again (#29805)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9UbbCo1bBWKzRoP44fkJR8d4fe)_ Fixes #29733 Clause-②: no Metadata-protocol: none Claim: 6595203212 (amendment 6595600429) ## 维护者速读 - 改了什么:Studio 元数据表单里「可选」的下拉选项(枚举字段)现在第一项是「— 无 —」,选它即把该键从元数据里删掉(恢复为「未声明」)。必填的下拉、以及列表里的单个元素不变,不出现这一项。另外修了同一条链路上的第二个原因:动作、仪表盘、钩子、页面、报表五个编辑器的「更多字段」区,清空任何一项(选「无」或清空文本框)以前会被宿主的合并立刻还原,现在清空能落到草稿并保存为「无此键」。 - 为什么改:卡片 #29733 指出可选枚举选过一次就回不去「未声明」;对 `user` 字段的 `deleteBehavior` 来说「未声明」本身就是一种生效的语义(不进删除级联)。实测发现 Studio 的字段面板目前根本不显示 `deleteBehavior` 这一行(席位另立卡),但缺陷本身在「更多字段」区可复现(动作的 `openIn` 只能在 self / new-tab 之间选)。 - 风险与回滚:只改界面,不动任何元数据契约;必填项、无值时的占位文字、合并逻辑本身都不变。清空的键以 `undefined` 进入草稿,保存时 JSON 自然省略。回滚即 revert 本 PR。 - 先看:`ui/packages/app-shell/src/views/metadata-admin/SchemaForm.tsx` 的 `offerUnset` 与两处 Select;`inspectors/fallbackFormPatch.ts`;五个 DefaultInspector 各一行 `onChange`。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/views/metadata-admin/SchemaForm.optionalEnumUnset-29733.test.tsx packages/app-shell/src/views/metadata-admin/inspectors/fallbackFormPatch-29733.test.tsx`,预期 12 个用例全部通过。 - 需要你做的:无,席位落地。 ## What changes **1. An optional enum select offers "— None —".** Both builtin select paths of the metadata-admin `SchemaForm` (a form row's `options`, and a schema property's own `enum`) listed only the enum values, so an optional key, once picked, could never return to absent. `FieldControl` now leads an optional row's list with a "— None —" item: the existing `engine.form.none` key (en "— None —", zh "— 无 —"), the word the metadata-admin pickers already use, so no new i18n key. Radix refuses an item whose value is the empty string, so the item carries a sentinel that `onValueChange` maps back to `undefined`, the form's existing clear convention (`setField` deletes the key; a repeater row keeps an `undefined` that serialisation drops). With nothing set, the trigger still shows the "Select…" placeholder and nothing is written on render. | Row | Before | Now | |---|---|---| | optional enum, either path | values only | "— None —" first, then the values | | required enum (schema `required`, a form row's `required`, a grid column's `required`) | values only | unchanged | | one element of a scalar list of an enum | values only | unchanged (an element is removed with its own row button) | **2. Clears in the "More fields" fallback form reach the draft.** The action, dashboard, hook, page and report default inspectors render a fallback `SchemaForm` and wired it `onChange={(next) => onPatch(next)}`. The host's `onPatch` merges the patch over the draft, and `SchemaForm` reports a clear by deleting the key from `next`, so the merge kept the old value: a None pick, an emptied text box or an emptied number box there was undone at once. The new pure helper `fallbackFormPatch(draft, next)` returns `next` plus every key the draft had and `next` dropped, set to `undefined`; the five call sites route through it. `onPatch`'s merge itself is unchanged, and a key the form did not touch is never turned into `undefined`. ## Tests - `SchemaForm.optionalEnumUnset-29733.test.tsx` (new, 8), over the real `SchemaForm`: the schema-enum path and the form-options path each emit an object without the key after "— None —" (nothing else moves); a repeater card row (the object form's `fields` rows, where `deleteBehavior` sits) serialises without the key; the placeholder still shows and nothing is emitted on render. CONTROLS: a required enum on each path offers no None item; in a grid repeater a required column offers none while an optional one does; a scalar list element offers none. - `inspectors/fallbackFormPatch-29733.test.tsx` (new, 4): the helper through a merge leaves a cleared key absent and an edited key at its new value; CONTROL: untouched keys pass through and nothing becomes `undefined`; the real `ActionDefaultInspector` under a merging host: "— None —" on `openIn` and an emptied `description` each leave the draft without the key, with the curated keys untouched. - Ablations (`scripts/ablation-replace.mjs`, each restored to the HEAD blob with `git diff HEAD` empty): A1 `offerUnset` forced false, 4 of 8 red (the three positive pins and the grid optional column), the 4 controls green. A2 the helper's undefined-for-dropped-key line removed, 3 of 4 red, the control green. - Touched suites, under the verify lock at `--maxWorkers=2`: the 144 ui test files that name `SchemaForm`, one of the five inspectors or `engine.form.none` (`git grep -l`), 1464 tests, all green. ## Acceptance notes - **The card's named door is not reachable in Studio today (measured).** The Studio object designer's field panel (`ObjectFieldInspector`) offers no Delete Behavior row on a `user` field nor on a `lookup` field; the `deleteBehavior` rows live in `object.form.ts` / `field.form.ts`, which `SchemaForm` renders, and for `object` the editor shows the designer and its inspectors instead. So the named consequence (a cascade or restrict left in force on a `user` field) is prevented here for the moment a form that renders those rows is reached. The missing row in the designer's field panel is filed by the seat as its own finding. - **Surface:** widened by claim amendment 6595600429 to the five default-inspector `onChange` call sites and one helper, after the live run showed a None item alone would be a dead control there. Clearing a text box or a number box in those fallback forms is fixed by the same change. - **Defaults:** an enum whose schema carries a `default` is listed in the served schema's `required` and so offers no None item (hook `onError`, `runAs`). That is the existing required signal, not changed here. - **Not changed:** the action inspector's own curated `variant` select (an `InspectorSelectField`, not `SchemaForm`) still has no unset item; it is outside this card. - Narrowing declared: the full `packages/app-shell` suite is CI's (Test UI); locally the 144-file grep set above ran. ## Verification Real backend (`examples/app-crm`, `objectstack dev --fresh`) and the source Console (Vite on `ui/apps/console`), admin session, a scratch package with an object `udb_ticket` (a `user` field `assignee`, a `lookup` field `parent_ticket`) and an action `udb_escalate` (`type: url`, `openIn: self`). Screenshots are in the project folder (`issue-29733/`), not on GitHub. <details><summary>Before (main 6f09464), the "More fields" Open In select on the action</summary> - Options read `["self","new-tab"]`: no way back to absent. - With only the SchemaForm half applied, picking "— None —" left the trigger at `self`: the merging host kept the key. - The field panel for `assignee` (user) and `parent_ticket` (lookup) shows no Delete Behavior row. </details> <details><summary>After, on this branch</summary> - Options read `["— None —","self","new-tab"]`. - Pick "— None —": the trigger shows the "Select…" placeholder; Save (draft), reload: still unset; Publish. - `GET /api/v1/meta/action/udb_escalate` after publish: the item carries no `openIn` (before it read `"openIn":"self"`). </details> <details><summary>Local gates</summary> On head `74e47d1e` (main merged at `25450217`; 11 later main commits touch none of this PR's files): `node scripts/pm/dispatch-gates.mjs --commands` derives 35 commands; each ran with exit 0 (`check:type-check-debt` and `pnpm --dir ui exec vitest run scripts/` under the verify lock), and `--ran` reconciles 35 derived, 35 run, 0 NOT-MEASURED. Also exit 0: `pnpm --filter @object-ui/app-shell run type-check`; `eslint` on the 9 touched files (0 errors; the warnings are on untouched lines); `check-partof-closing-keyword`; `check-closing-keyword-parity --body` (binds only this card). The two new test files rerun on this head: 12/12. `dispatch-gates --tier`: no path-derived mandate (default tier); 520 changed lines before the main merge, under 3000. The repo-wide sweep (`pnpm lint` and the rest of the farm) is CI's. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jUDMc41NruhHkjaN5X)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 138dacf commit 5ac50e4

10 files changed

Lines changed: 512 additions & 8 deletions
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(app-shell): an optional choice row in a Studio metadata form can be cleared again
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
- **A "— None —" item on optional enum selects.** Both builtin select paths of the metadata-admin `SchemaForm` (a form row's `options`, and a schema property's own `enum`) listed only the enum values, so once an author picked a value on an optional key, the form had no way back to "declares nothing". An optional row now leads with "— None —" (the `engine.form.none` word the metadata-admin pickers already use). Choosing it writes the key as absent, through the form's existing clear convention. A required row, and one element of a list of choices, are unchanged: they offer no None item.
11+
- **Clears in an inspector's "More fields" form are kept.** The action, dashboard, hook, page and report editors pass their "More fields" form's changes to a host that merges them over the draft. A key the form cleared was merely missing from that change, so the merge kept the old value: a None pick, or an emptied text box, was undone at once. The cleared keys now reach the draft as absent, and the saved metadata no longer carries them.
Lines changed: 297 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,297 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* An OPTIONAL enum row in the metadata-admin `SchemaForm` can be returned to
5+
* "declares nothing".
6+
*
7+
* Both builtin select paths (`fieldSpec.options` and the schema's own `enum`)
8+
* used to list only the enum's values, so once an author picked one the key
9+
* could never be removed again from the form. On a `user` field that is not
10+
* cosmetic: a `user` field that declares no `deleteBehavior` stays out of the
11+
* delete walk, so an author who tried `cascade` and changed their mind was
12+
* left with it in force.
13+
*
14+
* The cure is a "— None —" item (the `engine.form.none` word the metadata-admin
15+
* pickers already use) that writes the key as ABSENT, through the form's own
16+
* clear convention: `onChange(undefined)`, which a form row deletes from the
17+
* object and a repeater row stores as `undefined` (dropped on the wire).
18+
*
19+
* Pins:
20+
* - each path: a set value, choose None, the emitted object lacks the key;
21+
* - the repeater card row (the `object` form's `fields` rows, where the
22+
* `deleteBehavior` row lives): the row serialises without the key;
23+
* - CONTROL: a REQUIRED enum offers no None item, on both paths and in a grid
24+
* cell whose column is required;
25+
* - CONTROL: a scalar list row of an enum offers no None item — an array
26+
* element is removed with its own button, never "unset".
27+
*/
28+
29+
import { describe, it, expect, afterEach, vi } from 'vitest';
30+
import { render, screen, cleanup, fireEvent, waitFor, within } from '@testing-library/react';
31+
import { SchemaForm } from './SchemaForm';
32+
import type { FormViewSpec } from './form-spec';
33+
import { t } from './i18n';
34+
35+
afterEach(cleanup);
36+
37+
const NONE = t('engine.form.none', 'en-US');
38+
39+
/** Open a Radix Select trigger the way the other suites in this repo do. */
40+
async function openSelect(trigger: HTMLElement) {
41+
fireEvent.keyDown(trigger, { key: 'ArrowDown' });
42+
await waitFor(() => expect(screen.queryAllByRole('option').length).toBeGreaterThan(0));
43+
}
44+
45+
/** The last object a `SchemaForm` emitted. */
46+
function lastEmitted(onChange: ReturnType<typeof vi.fn>): Record<string, unknown> {
47+
expect(onChange).toHaveBeenCalled();
48+
return onChange.mock.calls[onChange.mock.calls.length - 1][0] as Record<string, unknown>;
49+
}
50+
51+
const DELETE_OPTIONS = [
52+
{ label: 'Set null', value: 'set_null' },
53+
{ label: 'Cascade (delete children)', value: 'cascade' },
54+
{ label: 'Restrict (block the delete)', value: 'restrict' },
55+
];
56+
57+
const fieldSchema = (required: string[] = []) =>
58+
({
59+
type: 'object',
60+
properties: {
61+
type: { type: 'string', title: 'Type' },
62+
deleteBehavior: {
63+
type: 'string',
64+
title: 'Delete Behavior',
65+
enum: ['set_null', 'cascade', 'restrict'],
66+
},
67+
},
68+
required,
69+
}) as never;
70+
71+
describe('an optional enum row offers "— None —", which writes the key as absent', () => {
72+
it('⭐ the schema-enum path (no form options): choosing None deletes the key', async () => {
73+
const onChange = vi.fn();
74+
render(
75+
<SchemaForm
76+
schema={fieldSchema()}
77+
value={{ type: 'user', deleteBehavior: 'cascade' }}
78+
onChange={onChange}
79+
/>,
80+
);
81+
await openSelect(screen.getByRole('combobox', { name: 'Delete Behavior' }));
82+
fireEvent.click(screen.getByRole('option', { name: NONE }));
83+
84+
const next = lastEmitted(onChange);
85+
expect('deleteBehavior' in next).toBe(false);
86+
// Nothing else moved.
87+
expect(next).toEqual({ type: 'user' });
88+
});
89+
90+
it('⭐ the form-options path (`fieldSpec.options`): choosing None deletes the key', async () => {
91+
const onChange = vi.fn();
92+
const form: FormViewSpec = {
93+
sections: [
94+
{
95+
label: 'Relational',
96+
fields: [
97+
{ field: 'type', label: 'Type' },
98+
{ field: 'deleteBehavior', label: 'Delete Behavior', type: 'select', options: DELETE_OPTIONS },
99+
],
100+
},
101+
],
102+
};
103+
render(
104+
<SchemaForm
105+
schema={fieldSchema()}
106+
form={form}
107+
value={{ type: 'user', deleteBehavior: 'restrict' }}
108+
onChange={onChange}
109+
/>,
110+
);
111+
await openSelect(screen.getByRole('combobox', { name: 'Delete Behavior' }));
112+
// The enum's own values are still all offered, after the None item.
113+
const offered = screen.getAllByRole('option').map((o) => o.textContent);
114+
expect(offered).toEqual([NONE, ...DELETE_OPTIONS.map((o) => o.label)]);
115+
fireEvent.click(screen.getByRole('option', { name: NONE }));
116+
117+
const next = lastEmitted(onChange);
118+
expect('deleteBehavior' in next).toBe(false);
119+
expect(next).toEqual({ type: 'user' });
120+
});
121+
122+
it('⭐ a repeater card row (the object form `fields` rows): the row serialises without the key', async () => {
123+
const onChange = vi.fn();
124+
const form: FormViewSpec = {
125+
sections: [
126+
{
127+
label: 'Fields',
128+
fields: [
129+
{
130+
field: 'fields',
131+
type: 'repeater',
132+
fields: [
133+
{ field: 'name', label: 'Name', required: true },
134+
{ field: 'deleteBehavior', label: 'Delete Behavior', type: 'select', options: DELETE_OPTIONS },
135+
],
136+
},
137+
],
138+
},
139+
],
140+
};
141+
render(
142+
<SchemaForm
143+
schema={
144+
{
145+
type: 'object',
146+
properties: {
147+
fields: {
148+
type: 'array',
149+
title: 'Fields',
150+
items: {
151+
type: 'object',
152+
properties: {
153+
name: { type: 'string' },
154+
deleteBehavior: { type: 'string', enum: ['set_null', 'cascade', 'restrict'] },
155+
},
156+
},
157+
},
158+
},
159+
} as never
160+
}
161+
form={form}
162+
value={{ fields: [{ name: 'owner', deleteBehavior: 'cascade' }] }}
163+
onChange={onChange}
164+
/>,
165+
);
166+
fireEvent.click(screen.getByText(/#1/));
167+
await openSelect(screen.getByRole('combobox', { name: 'Delete Behavior' }));
168+
fireEvent.click(screen.getByRole('option', { name: NONE }));
169+
170+
const next = lastEmitted(onChange);
171+
// What a save sends is the JSON of the draft.
172+
expect(JSON.parse(JSON.stringify(next))).toEqual({ fields: [{ name: 'owner' }] });
173+
});
174+
175+
it('with nothing set, the trigger still reads the placeholder (no value is written on render)', () => {
176+
const onChange = vi.fn();
177+
render(<SchemaForm schema={fieldSchema()} value={{ type: 'user' }} onChange={onChange} />);
178+
expect(screen.getByRole('combobox', { name: 'Delete Behavior' })).toHaveTextContent(
179+
t('engine.form.selectEllipsis', 'en-US'),
180+
);
181+
expect(onChange).not.toHaveBeenCalled();
182+
});
183+
});
184+
185+
describe('CONTROL — no None item where the value cannot be absent', () => {
186+
it('a REQUIRED enum (schema-enum path) offers only its values', async () => {
187+
render(
188+
<SchemaForm
189+
schema={fieldSchema(['deleteBehavior'])}
190+
value={{ type: 'user', deleteBehavior: 'cascade' }}
191+
onChange={() => {}}
192+
/>,
193+
);
194+
await openSelect(screen.getByRole('combobox', { name: 'Delete Behavior' }));
195+
expect(screen.queryByRole('option', { name: NONE })).toBeNull();
196+
expect(screen.getAllByRole('option')).toHaveLength(3);
197+
});
198+
199+
it('a REQUIRED enum (form-options path) offers only its values', async () => {
200+
const form: FormViewSpec = {
201+
sections: [
202+
{
203+
label: 'Relational',
204+
fields: [
205+
{ field: 'deleteBehavior', label: 'Delete Behavior', type: 'select', required: true, options: DELETE_OPTIONS },
206+
],
207+
},
208+
],
209+
};
210+
render(
211+
<SchemaForm
212+
schema={fieldSchema()}
213+
form={form}
214+
value={{ deleteBehavior: 'cascade' }}
215+
onChange={() => {}}
216+
/>,
217+
);
218+
await openSelect(screen.getByRole('combobox', { name: 'Delete Behavior' }));
219+
expect(screen.queryByRole('option', { name: NONE })).toBeNull();
220+
expect(screen.getAllByRole('option')).toHaveLength(3);
221+
});
222+
223+
it('a grid repeater cell: an optional column offers None, a required column does not', async () => {
224+
const form: FormViewSpec = {
225+
sections: [
226+
{
227+
label: 'Rules',
228+
fields: [
229+
{
230+
field: 'rules',
231+
type: 'repeater',
232+
widget: 'grid',
233+
fields: [
234+
{ field: 'mode', label: 'Mode', type: 'select', required: true, options: [
235+
{ label: 'Allow', value: 'allow' },
236+
{ label: 'Deny', value: 'deny' },
237+
] },
238+
{ field: 'kind', label: 'Kind', type: 'select', options: [
239+
{ label: 'Open', value: 'open' },
240+
{ label: 'Won', value: 'won' },
241+
] },
242+
],
243+
},
244+
],
245+
},
246+
],
247+
};
248+
render(
249+
<SchemaForm
250+
schema={
251+
{
252+
type: 'object',
253+
properties: {
254+
rules: {
255+
type: 'array',
256+
title: 'Rules',
257+
items: { type: 'object', properties: { mode: { type: 'string' }, kind: { type: 'string' } } },
258+
},
259+
},
260+
} as never
261+
}
262+
form={form}
263+
value={{ rules: [{ mode: 'allow', kind: 'open' }] }}
264+
onChange={() => {}}
265+
/>,
266+
);
267+
const [modeCell, kindCell] = Array.from(document.querySelectorAll('td')) as HTMLElement[];
268+
await openSelect(within(modeCell).getByRole('combobox'));
269+
expect(screen.queryByRole('option', { name: NONE })).toBeNull();
270+
fireEvent.keyDown(document.activeElement ?? document.body, { key: 'Escape' });
271+
await waitFor(() => expect(screen.queryAllByRole('option')).toHaveLength(0));
272+
273+
await openSelect(within(kindCell).getByRole('combobox'));
274+
expect(screen.getByRole('option', { name: NONE })).toBeInTheDocument();
275+
});
276+
277+
it('a scalar list row of an enum offers no None item', async () => {
278+
render(
279+
<SchemaForm
280+
schema={
281+
{
282+
type: 'object',
283+
properties: {
284+
modes: { type: 'array', title: 'Modes', items: { type: 'string', enum: ['a', 'b'] } },
285+
},
286+
} as never
287+
}
288+
form={{ sections: [{ label: 'S', fields: [{ field: 'modes', type: 'repeater' }] }] }}
289+
value={{ modes: ['a'] }}
290+
onChange={() => {}}
291+
/>,
292+
);
293+
await openSelect(screen.getByRole('combobox'));
294+
expect(screen.queryByRole('option', { name: NONE })).toBeNull();
295+
expect(screen.getAllByRole('option')).toHaveLength(2);
296+
});
297+
});

0 commit comments

Comments
 (0)