Skip to content

Commit a4e7014

Browse files
fix(studio): field inspector API name refuses a taken name and drops the trailing underscore (#27591)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgNbC31RXiaPbk8mUf7e5LXa)_ Fixes #27517 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:Studio 字段检查器的「API 名称」输入框改为显示作者实际键入的内容;键入其他字段已用的名称时,给出与「添加字段」弹窗完全相同的拒绝提示,字段保留编辑前的名称;写入的名称按新建对象对话框的规则规范化(末尾标点不再存成下划线)。 - 为什么改:原来键入已存在的名称会无声吞掉最后一个按键并把半截名称(`statu`)存进草稿;`Estimated Hours!` 会存成 `estimated_hours_`。 - 风险与回滚:仅 Studio 字段检查器一个输入框与「添加字段」弹窗的一处提示取值(抽成共享函数,行为不变);不改元数据协议;回滚即 revert 本 PR。 - 先看:`ObjectFieldInspector.tsx` 的 `FieldApiNameInput`(键入、拒绝回退到编辑前名称、失焦复位);`AddFieldPopover.tsx` 的 `fieldNameTakenRefusal`;新测试文件。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.apiNameRefusal-27517.test.tsx`,预期 6 个用例全绿。 - 需要你做的:无,席位落地。 ## What changes - **`ObjectFieldInspector`** — the API name box is now `FieldApiNameInput`, a box that keeps what the author typed as its own text and still writes the field's name on every keystroke, as before: - The text is normalised as the new-object dialog (`CreateItemDialog`) normalises an identifier: `toFieldNameLoose` while typing, so an `_` can be typed mid-word, and the strict rule (`toFieldNameOrEmpty`, which is `toFieldName` without its `field` placeholder) for the name written. `Estimated Hours!` saves `estimated_hours`. - A name another field of the object holds is refused with the Add field popover's own sentence. The box keeps showing what was typed (no keystroke dropped), marks itself `aria-invalid` and points `aria-describedby` at the refusal, and the field is left carrying the name it had when the edit began, not the last free prefix of the refused name. An emptied box writes no name either. - Leaving the box shows the name the field carries (the refusal goes with it). - A name arriving from outside the box (another field selected, the Label deriving one) replaces the text; the box's own write echoing back does not. - `setKey` now receives the already-normalised name; its collision guard reads the same `takenNames` list the box refuses against. The objectui#7615 ownership stamp is unchanged: every keystroke still stamps the author as owner. - **`AddFieldPopover`** — the duplicate-name refusal is extracted as the exported `fieldNameTakenRefusal(name, takenNames, locale)` and the popover reads it; its hint logic is otherwise unchanged (same branches, same key `engine.studio.data.addFieldApiNameTaken`). No second validator, no new string. - Root changeset: `@object-ui/app-shell` patch. ## Tests - New `ObjectFieldInspector.apiNameRefusal-27517.test.tsx` (6 cases), typing one character at a time through a host that feeds each patch back and follows the rename with the selection: the refusal equals `fieldNameTakenRefusal` for the same name; the field keeps `estimated_hours`; typing past the refusal (`status_2`) takes the name; blur restores; an emptied box saves nothing; `Estimated Hours!` saves `estimated_hours`; `repair_ticket` still types its underscore. - Ablation, one leg per behaviour, each via `scripts/ablation-replace.mjs` (anchor hit 1 → 0, blob changed, restored, `git diff HEAD` empty afterwards); app-shell tests import the source, no build in the path: | Leg | Mutation | Result | |---|---|---| | rollback | refused name written instead of the edit-start name | 2 of 6 red (keeps previous name; blur restore) | | normaliser | loose text written as the name | 1 of 6 red (`Estimated Hours!`) | | display | box shows the stored name, not the typed text | 3 of 6 red (refusal keystroke, emptied box, trailing `_`) | - Existing inspector, popover, Data pillar and external-object suites green before the main merge (28 files, 297 tests); the full `metadata-admin/` and `studio-design/` directories are in the gate run below. ## Acceptance notes - The Label → API name derivation has the same prefix shape on a collision: deriving `Status` letter by letter on a new field whose object already has `status` leaves the derived `statu`. That path is `deriveNameFor`, not the API name box, and is outside this card; noted, not changed. - The refusal is shown while the author is in the box; leaving the box restores the stored name and drops the refusal, which is the "previous name kept" the card's Expected section describes. - Docs-driven rule (ui/AGENTS.md #2): no docs page describes the inspector's API name box; nothing to update. ## Verification Gate readings are recorded in the report comment on #27517. --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CYtNGSzW4fU2cRSbut)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1e45a4e commit a4e7014

4 files changed

Lines changed: 293 additions & 21 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(studio): the field inspector's API name box refuses a name another field holds, and saves no trailing underscore
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
Typing another field's API name into the field inspector used to drop the last keystroke without a message and save the half-typed name (`status` saved as `statu`), and a trailing punctuation mark saved a trailing underscore (`Estimated Hours!` saved as `estimated_hours_`). The box now shows what the author typed, says the Add field popover's own refusal ("Another field is already named status. Type a different API name."), and keeps the name the field had before the edit. The name written is normalised the way the new-object dialog normalises an identifier, so a trailing underscore is never saved; an underscore can still be typed mid-word. Leaving the box shows the name the field carries.
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* ObjectFieldInspector — the API name box refuses a name another field holds
5+
* and normalises trailing punctuation (objectstack#27517).
6+
*
7+
* Measured on the release-candidate walkthrough: selecting all of an Estimated
8+
* Hours field's API name and typing `status` (another field's name) left the
9+
* box reading `statu` with no message, and `statu` was saved; typing
10+
* `Estimated Hours!` saved `estimated_hours_`. The box now shows what the
11+
* author typed, refuses a taken name in the Add field popover's own sentence,
12+
* keeps the name the field had when the edit began, and writes the name the
13+
* new-object dialog's rule derives.
14+
*
15+
* Every case types ONE CHARACTER AT A TIME through a host that feeds each
16+
* patch back in and follows the rename with the selection, as the Studio host
17+
* does: the dropped keystroke only exists in that loop.
18+
*/
19+
20+
import * as React from 'react';
21+
import { describe, it, expect, vi, afterEach } from 'vitest';
22+
import { render, screen, fireEvent, cleanup } 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 { fieldNameTakenRefusal } from '../../studio-design/AddFieldPopover';
37+
38+
afterEach(cleanup);
39+
40+
type Fields = Record<string, Record<string, unknown>>;
41+
42+
const WORK_ORDER: Fields = {
43+
estimated_hours: { type: 'number', label: 'Estimated Hours' },
44+
status: { type: 'select', label: 'Status', options: [{ value: 'new', label: 'New Request' }] },
45+
};
46+
47+
/** The host's draft as last patched, read by the assertions. */
48+
let saved: Fields = {};
49+
50+
function Host({ fields: initial, id }: { fields: Fields; id: string }) {
51+
const [fields, setFields] = React.useState(initial);
52+
const [selection, setSelection] = React.useState<any>({ kind: 'field', id });
53+
return (
54+
<ObjectFieldInspector
55+
type="object"
56+
name="work_order"
57+
draft={{ name: 'work_order', fields }}
58+
selection={selection}
59+
onPatch={(patch: any) => {
60+
saved = patch.fields;
61+
setFields(patch.fields);
62+
}}
63+
onClearSelection={() => {}}
64+
onSelectionChange={(next: any) => next && setSelection(next)}
65+
readOnly={false}
66+
locale={'en-US'}
67+
/>
68+
);
69+
}
70+
71+
function apiNameBox(): HTMLInputElement {
72+
return screen.getByTestId('field-apiname-input') as HTMLInputElement;
73+
}
74+
75+
/** Select all, then one `change` event per character — a person at a keyboard. */
76+
function typeOver(text: string): void {
77+
fireEvent.focus(apiNameBox());
78+
for (let i = 1; i <= text.length; i += 1) {
79+
fireEvent.change(apiNameBox(), { target: { value: text.slice(0, i) } });
80+
}
81+
}
82+
83+
function renderWorkOrder(): void {
84+
saved = WORK_ORDER;
85+
render(<Host fields={WORK_ORDER} id="estimated_hours" />);
86+
}
87+
88+
describe('ObjectFieldInspector — API name box refuses a taken name (#27517)', () => {
89+
it('shows every keystroke and the Add field popover refusal, and keeps the previous name', () => {
90+
renderWorkOrder();
91+
typeOver('status');
92+
// Pre-fix the box read `statu`: the refused keystroke was dropped silently.
93+
expect(apiNameBox()).toHaveValue('status');
94+
const refusal = screen.getByTestId('field-apiname-refusal');
95+
// The same sentence the Add field popover says for the same name.
96+
expect(refusal.textContent).toBe(fieldNameTakenRefusal('status', ['status'], 'en-US'));
97+
expect(apiNameBox()).toHaveAttribute('aria-invalid', 'true');
98+
expect(apiNameBox().getAttribute('aria-describedby')).toBe(refusal.id);
99+
// Saves nothing: the field still carries the name it had before the edit,
100+
// not the last free prefix (`statu`) of the refused one.
101+
expect(Object.keys(saved).sort()).toEqual(['estimated_hours', 'status']);
102+
});
103+
104+
it('takes the name once the author types past the refusal', () => {
105+
renderWorkOrder();
106+
typeOver('status_2');
107+
expect(apiNameBox()).toHaveValue('status_2');
108+
expect(screen.queryByTestId('field-apiname-refusal')).toBeNull();
109+
expect(Object.keys(saved).sort()).toEqual(['status', 'status_2']);
110+
});
111+
112+
it('shows the name the field carries once the author leaves a refused box', () => {
113+
renderWorkOrder();
114+
typeOver('status');
115+
fireEvent.blur(apiNameBox());
116+
expect(apiNameBox()).toHaveValue('estimated_hours');
117+
expect(screen.queryByTestId('field-apiname-refusal')).toBeNull();
118+
});
119+
120+
it('saves nothing for an emptied box', () => {
121+
renderWorkOrder();
122+
fireEvent.focus(apiNameBox());
123+
fireEvent.change(apiNameBox(), { target: { value: '' } });
124+
expect(apiNameBox()).toHaveValue('');
125+
expect(Object.keys(saved).sort()).toEqual(['estimated_hours', 'status']);
126+
fireEvent.blur(apiNameBox());
127+
expect(apiNameBox()).toHaveValue('estimated_hours');
128+
});
129+
});
130+
131+
describe('ObjectFieldInspector — API name box normalises as the new-object dialog does (#27517)', () => {
132+
it('saves `Estimated Hours!` as estimated_hours, without the trailing underscore', () => {
133+
saved = {};
134+
render(<Host fields={{ field_3: { type: 'number', label: '' } }} id="field_3" />);
135+
typeOver('Estimated Hours!');
136+
// While typing, the box keeps the `_` so the next letter can follow it.
137+
expect(apiNameBox()).toHaveValue('estimated_hours_');
138+
// Pre-fix this read `estimated_hours_`.
139+
expect(Object.keys(saved)).toEqual(['estimated_hours']);
140+
fireEvent.blur(apiNameBox());
141+
expect(apiNameBox()).toHaveValue('estimated_hours');
142+
});
143+
144+
it('still lets an underscore be typed mid-word', () => {
145+
saved = {};
146+
render(<Host fields={{ field_4: { type: 'text', label: '' } }} id="field_4" />);
147+
typeOver('repair_ticket');
148+
expect(apiNameBox()).toHaveValue('repair_ticket');
149+
expect(Object.keys(saved)).toEqual(['repair_ticket']);
150+
});
151+
});

‎ui/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.tsx‎

Lines changed: 107 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ import {
9898
readFields,
9999
writeFields,
100100
toFieldNameLoose,
101+
toFieldNameOrEmpty,
101102
indexOfField,
102103
type FieldsView,
103104
type FieldEntry,
@@ -126,8 +127,99 @@ import { FilterConditionField } from './FilterConditionField.js';
126127
import type { FilterCondition } from './datasetFilterCondition.js';
127128
import { usePickerLoad, type LoadState } from '../loadState.js';
128129
import { isMainContentList } from '../../../utils/deriveRelatedLists.js';
130+
import { fieldNameTakenRefusal } from '../../studio-design/AddFieldPopover.js';
129131

130132

133+
/**
134+
* The field inspector's API name box (objectstack#27517).
135+
*
136+
* The box still writes the field's name on every keystroke, but it shows what
137+
* the author TYPED, not the name the field carries. Showing the stored name
138+
* dropped keystrokes without a word: a keystroke whose name another field
139+
* held was refused, the box snapped back to the previous keystroke (`status`
140+
* read `statu`), and that half-typed name was the one saved. Now:
141+
*
142+
* - the text is normalised the way the new-object dialog normalises an
143+
* identifier: `toFieldNameLoose` while typing, so an `_` can still be typed
144+
* mid-word, and the strict rule (`toFieldNameOrEmpty`, `toFieldName` less
145+
* its placeholder) for the name written, so `Estimated Hours!` is saved as
146+
* `estimated_hours`, never `estimated_hours_`;
147+
* - a name another field holds is refused in the Add field popover's own
148+
* sentence (`fieldNameTakenRefusal`), and the field keeps the name it had
149+
* when the author began typing, not the last free prefix of the refused one;
150+
* an emptied box writes no name either;
151+
* - leaving the box shows the name the field carries.
152+
*/
153+
function FieldApiNameInput({
154+
label,
155+
value,
156+
takenNames,
157+
onRename,
158+
disabled,
159+
locale,
160+
}: {
161+
label: string;
162+
/** The name the field carries. */
163+
value: string;
164+
/** The names the object's OTHER fields carry. */
165+
takenNames: readonly string[];
166+
/** Called on every keystroke with the name the field is to carry: normalised, never a refused one. */
167+
onRename: (name: string) => void;
168+
disabled?: boolean;
169+
locale: SupportedLocale;
170+
}) {
171+
const id = React.useId();
172+
const hintId = React.useId();
173+
const [text, setText] = React.useState(value);
174+
// What a refused or emptied box leaves the field named.
175+
const [startName, setStartName] = React.useState(value);
176+
// A name arriving from outside (another field selected, the Label deriving
177+
// one) replaces the text; this box's own write echoing back does not.
178+
const [seen, setSeen] = React.useState(value);
179+
const [written, setWritten] = React.useState<string | null>(null);
180+
if (value !== seen) {
181+
setSeen(value);
182+
setWritten(null);
183+
if (value !== written) {
184+
setText(value);
185+
setStartName(value);
186+
}
187+
}
188+
189+
const refusal = fieldNameTakenRefusal(toFieldNameOrEmpty(text), takenNames, locale);
190+
const change = (raw: string) => {
191+
const typed = toFieldNameLoose(raw);
192+
setText(typed);
193+
const typedName = toFieldNameOrEmpty(typed);
194+
const next = !typedName || fieldNameTakenRefusal(typedName, takenNames, locale) ? startName : typedName;
195+
if (next !== value) setWritten(next);
196+
onRename(next);
197+
};
198+
199+
return (
200+
<div className="space-y-1">
201+
<Label htmlFor={id} className="text-xs text-muted-foreground">{label}</Label>
202+
<Input
203+
id={id}
204+
value={text}
205+
onChange={(e) => change(e.target.value)}
206+
onFocus={() => setStartName(value)}
207+
onBlur={() => setText(value)}
208+
disabled={disabled}
209+
aria-invalid={refusal ? true : undefined}
210+
aria-describedby={refusal ? hintId : undefined}
211+
data-testid="field-apiname-input"
212+
className="h-8 text-sm font-mono"
213+
/>
214+
{refusal && (
215+
<p id={hintId} className="text-xs text-destructive" data-testid="field-apiname-refusal">
216+
{refusal}
217+
</p>
218+
)}
219+
</div>
220+
);
221+
}
222+
131223
/**
132224
* One row of the picklist option editor.
133225
*
@@ -879,18 +971,19 @@ export function ObjectFieldInspector({
879971
writeView({ shape: view.shape, entries: nextEntries });
880972
};
881973

882-
const setKey = (rawNext: string) => {
883-
const nextName = toFieldNameLoose(rawNext);
884-
const rejected =
885-
!nextName ||
886-
nextName === entry.name ||
887-
// Disallow collision
888-
view.entries.some((e, i) => i !== idx && e.name === nextName);
974+
/** The names the object's other fields carry: what the API name box refuses. */
975+
const takenNames = view.entries.filter((_, i) => i !== idx).map((e) => e.name);
976+
977+
// `nextName` comes from `FieldApiNameInput`, already normalised and checked
978+
// against `takenNames` (objectstack#27517).
979+
const setKey = (nextName: string) => {
980+
const rejected = !nextName || nextName === entry.name || takenNames.includes(nextName);
889981
// Every keystroke in the API name box is the author claiming that value —
890982
// recorded even when the edit is REJECTED just above (cleared, unchanged,
891-
// or colliding), because a rejected keystroke is still an authorial edit
892-
// and the Label must not resume overwriting what they are typing. The
893-
// stamp names whichever value the field is left carrying (objectui#7615).
983+
// or refused by the box), because a rejected keystroke is still an
984+
// authorial edit and the Label must not resume overwriting what they are
985+
// typing. The stamp names whichever value the field is left carrying
986+
// (objectui#7615).
894987
apiNameOwnerRef.current = {
895988
object: objectName,
896989
field: rejected ? entry.name : nextName,
@@ -1107,13 +1200,13 @@ export function ObjectFieldInspector({
11071200
<ExternalFieldBadge binding={external} state={externalWriteState} locale={locale} />
11081201
</div>
11091202
)}
1110-
<InspectorTextField
1203+
<FieldApiNameInput
11111204
label={tr('designer.field.apiName')}
11121205
value={entry.name}
1113-
onCommit={setKey}
1206+
takenNames={takenNames}
1207+
onRename={setKey}
11141208
disabled={ddlLocked}
1115-
mono
1116-
testId="field-apiname-input"
1209+
locale={locale}
11171210
/>
11181211
<InspectorTextField
11191212
label={tr('designer.field.label')}

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

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,26 @@ import {
3838
} from '@object-ui/components';
3939
import type { FieldTypeId } from '../metadata-admin/previews/field-types.js';
4040
import { FieldTypePicker } from '../metadata-admin/inspectors/FieldTypePicker.js';
41-
import { t, tFormat, useMetadataLocale } from '../metadata-admin/i18n.js';
41+
import { t, tFormat, useMetadataLocale, type SupportedLocale } from '../metadata-admin/i18n.js';
4242
import { toFieldNameLoose } from '../metadata-admin/previews/object-fields-io.js';
4343

44+
/**
45+
* The refusal for an api name another field of the object already holds, or
46+
* `null` when `name` is free. The one duplicate-name refusal Studio has: this
47+
* popover and the field inspector's API name box both say it, so the author
48+
* reads the same sentence whichever door they rename through
49+
* (objectstack#27517).
50+
*/
51+
export function fieldNameTakenRefusal(
52+
name: string,
53+
takenNames: readonly string[],
54+
locale: SupportedLocale,
55+
): string | null {
56+
return name && takenNames.includes(name)
57+
? tFormat('engine.studio.data.addFieldApiNameTaken', locale, { name })
58+
: null;
59+
}
60+
4461
export interface AddFieldPopoverProps {
4562
/**
4663
* Create the field: the label as typed (trimmed), the chosen type, and the
@@ -98,7 +115,8 @@ export function AddFieldPopover({
98115
// The label alone cannot name the field: the author types the api name.
99116
const asksName = !!trimmed && (!derived || derivedTaken);
100117
const typed = apiName.replace(/_+$/, '');
101-
const typedTaken = !!typed && takenNames.includes(typed);
118+
const typedRefusal = fieldNameTakenRefusal(typed, takenNames, locale);
119+
const typedTaken = !!typedRefusal;
102120
const canCreate = !!trimmed && (!asksName || (!!typed && !typedTaken));
103121
const submit = (e: React.FormEvent) => {
104122
e.preventDefault();
@@ -108,11 +126,11 @@ export function AddFieldPopover({
108126
};
109127
const nameHint = !asksName
110128
? null
111-
: typedTaken || (!typed && derivedTaken)
112-
? tFormat('engine.studio.data.addFieldApiNameTaken', locale, { name: typed || derived })
113-
: !typed
114-
? t('engine.studio.data.addFieldApiNameNotDerived', locale)
115-
: null;
129+
: typed
130+
? typedRefusal
131+
: derivedTaken
132+
? fieldNameTakenRefusal(derived, takenNames, locale)
133+
: t('engine.studio.data.addFieldApiNameNotDerived', locale);
116134

117135
return (
118136
<Popover open={open} onOpenChange={setOpen}>

0 commit comments

Comments
 (0)