Skip to content

Commit 0892156

Browse files
fix(react): mark the field a unique-index refusal names in create/edit forms (#27043)
<!-- claude-projects-attribution: {"github_login":"os-warren"} --> _Requested by **Warren** · [project thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ8FFTSFXvg3gyNdUibNK8VkN)_ Fixes #26891 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:`@object-ui/react` 的 `extractFieldErrors` 现在识别 409 `UNIQUE_VIOLATION` 里单个 `field`,返回一条带服务端句子的字段错误;表单因此把重复值标在对应输入框下。 - 为什么改:服务端(#26867)已在拒绝里点名字段并用标签造句,但前端只读字段数组,唯一索引冲突仍只显示横幅加 toast,用户要猜是哪个字段重复。 - 风险与回滚:只在 `code` 为 `UNIQUE_VIOLATION` 且带字符串 `field` 时生效;无 `field`(复合索引、索引名)照旧走横幅。回滚 = revert 本 PR。 - 先看:`ui/packages/react/src/utils/error-message.ts` 的 `extractFieldErrors` 新分支;`form-server-field-errors.test.tsx` 新增的 unique-index 两例。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/react/src/utils/error-message.test.ts packages/components/src/renderers/form/__tests__/form-server-field-errors.test.tsx`,预期全绿;在 console 新建一个 SKU 重复的产品,SKU 输入框下出现 "A record with this SKU already exists",无 toast。 - 需要你做的:无,席位落地。 ## What changes - `extractFieldErrors` (`@object-ui/react`): when no field-error list is present and the refusal's code is `UNIQUE_VIOLATION` (read from `code`, then `details.code`, the same pair `isPermissionError` reads), the single string `field` (`details.field` on the raw `@objectstack/client` error, `field` on a hand-rolled one) becomes one `WriteFieldError` whose message is the server's sentence (`extractWriteErrorMessage`). Without `field` the answer stays `null`. - No change to `form.tsx`: its existing `extractFieldErrors` then `form.setError` path marks the input, and a fully attributed refusal shows no banner and no toast. - Changeset: `@object-ui/react` patch. ## Why The server half shipped in #26867: the 409 `UNIQUE_VIOLATION` body is `{ error, code, developerMessage?, field?, object? }`, with `error` naming the field by its label. `@objectstack/client` parks that flat body on `err.details`, and `normaliseClientError` (`@object-ui/data-objectstack`) passes a unique 409 through unchanged. The last hop read only arrays (`validationErrors`, `details.fields`, `fields`), so it answered `null` and the dialog showed the generic banner and toast. Side effect, measured: `InlineEditSaveBar` (`@object-ui/plugin-detail`) calls the same helper, so on the atomic save path a unique refusal with `field` is now attributed to that field (labelled in the bar and published as the per-field hint) instead of the generic text; in callback mode it was already attributed to the in-flight field. Its suites stay green (below). ## Tests - `error-message.test.ts`: a unique refusal with `field` yields one entry carrying the server's sentence (asserts `code` and `httpStatus` 409 on the refusal); a hand-rolled one with top-level `field`; without `field` it answers `null`; an `INVALID_FIELD` carrying `field` still answers `null`. - `error-message.normalisation-boundary.test.ts`: the refusal survives `normaliseClientError` with `code`, `httpStatus` and the field entry intact. - `form-server-field-errors.test.tsx`: a rejected submit marks the `sku` input (`aria-invalid`, sentence under it, no toast); without `field` the toast carries the generic sentence and no input is marked. ## Acceptance notes - `ApiDataSource`, which embeds the body as a JSON tail in the message string, is not read for `field`; the ObjectStack adapter is the console's path. ## Verification Tree: `a17ebf4e` on base `7eb613c3`. <details><summary>Tests and ablation</summary> - Three touched files: Test Files 3 passed, Tests 46 passed. - Ablation via `scripts/ablation-replace.mjs` (unique branch replaced by `return null`): exactly the four new positive tests red (two helper, one boundary, one form render), 42 passed; restored, blob equals HEAD, `git diff HEAD` empty. Tests resolve `@object-ui/react` to `src` through the ui vitest alias, so no dist leg. - Consumer sweep under the lock: `packages/plugin-detail/src/__tests__/`, `packages/react/`, `apps/console/src/pages/settings/`, `apps/console/src/pages/system/`: Test Files 385 passed, 1 skipped; Tests 3607 passed, 18 skipped. - `type-check` for `@object-ui/react` and `@object-ui/components`: exit 0. eslint on the four files: 0 errors. </details> <details><summary>Derived gates</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: 34 families, each run, all exit 0, including `node scripts/check-sdui-manifest.mjs` and `pnpm --dir ui exec vitest run scripts/`. `--ran` reconciliation: 34 derived, 34 run, 0 NOT-MEASURED, 0 UNRUN. </details> ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. (eslint on the touched files: 0 errors; the repo-wide run is CI's) - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: its generated artifacts are regenerated and committed. (not touched) - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. --- _Generated by [Claude Code](https://claude.ai/code/session_01JQwxbZRo35mpCPrKXVXevY)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 85c40fd commit 0892156

5 files changed

Lines changed: 152 additions & 2 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
'@object-ui/react': patch
3+
---
4+
5+
fix(react): a create or edit form marks the field a unique-index refusal names
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
Saving a record whose value collides with a unique index (a product SKU, an account name that must be unique) used to show "A record with this value already exists" as a banner and a toast, with no field marked. The user had to guess which of the form's unique fields collided.
11+
12+
The server's `409 UNIQUE_VIOLATION` answer names the one field on `field`, with a sentence that names it by its label ("A record with this SKU already exists"). `extractFieldErrors` only read lists of field errors, so it answered nothing for this refusal. It now returns one entry for it: the named field, carrying the server's sentence. The form renderer writes that sentence under the input, as it does for a `required` field, and shows no banner. Inline edit on a record page reads the same helper, so its save bar attributes the refusal to that field too.
13+
14+
When the server cannot tell which column collided (an index name, a composite key), the answer has no `field` and nothing changes: the banner stays.
15+
16+
What to do: nothing.

‎ui/packages/components/src/renderers/form/__tests__/form-server-field-errors.test.tsx‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,3 +200,49 @@ describe('form renderer — server-rejected fields', () => {
200200
expect(scrollSpy).not.toHaveBeenCalled();
201201
});
202202
});
203+
204+
/**
205+
* A unique-index refusal names ONE field on `details.field` (not a list): the
206+
* 409 `UNIQUE_VIOLATION` body `@objectstack/rest` sends, as
207+
* `@objectstack/client` decorates it.
208+
*/
209+
function uniqueViolation(field?: string) {
210+
const error = field ? 'A record with this SKU already exists' : 'A record with this value already exists';
211+
return Object.assign(new Error(error), {
212+
code: 'UNIQUE_VIOLATION',
213+
httpStatus: 409,
214+
details: { error, code: 'UNIQUE_VIOLATION', ...(field ? { field } : {}), object: 'product' },
215+
});
216+
}
217+
218+
const PRODUCT_FIELDS = [
219+
{ name: 'name', label: 'Name', type: 'input' },
220+
{ name: 'sku', label: 'SKU', type: 'input' },
221+
];
222+
223+
describe('form renderer — unique-index refusal', () => {
224+
it('marks the field the refusal names, instead of a banner and a toast', async () => {
225+
const onSubmit = vi.fn().mockRejectedValue(uniqueViolation('sku'));
226+
const { container } = renderForm(PRODUCT_FIELDS, onSubmit);
227+
const skuWrap = container.querySelector('[data-field="sku"]') as HTMLElement;
228+
const nameWrap = container.querySelector('[data-field="name"]') as HTMLElement;
229+
230+
submit();
231+
232+
await waitFor(() => expect(skuWrap.textContent).toMatch(/A record with this SKU already exists/));
233+
expect(skuWrap.querySelector('[aria-invalid="true"]')).toBeTruthy();
234+
expect(nameWrap.textContent).not.toMatch(/already exists/);
235+
expect(toastErrorSpy).not.toHaveBeenCalled();
236+
});
237+
238+
it('keeps the banner when the refusal cannot name the field', async () => {
239+
const onSubmit = vi.fn().mockRejectedValue(uniqueViolation());
240+
const { container } = renderForm(PRODUCT_FIELDS, onSubmit);
241+
242+
submit();
243+
244+
await waitFor(() => expect(toastErrorSpy).toHaveBeenCalled());
245+
expect(String(toastErrorSpy.mock.calls[0][0])).toMatch(/A record with this value already exists/);
246+
expect(container.querySelector('[aria-invalid="true"]')).toBeNull();
247+
});
248+
});

‎ui/packages/react/src/utils/error-message.normalisation-boundary.test.ts‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import { describe, it, expect } from 'vitest';
1010

1111
import { normaliseClientError } from '@object-ui/data-objectstack';
12-
import { declaredUserMessage } from './error-message';
12+
import { declaredUserMessage, extractFieldErrors } from './error-message';
1313

1414
/**
1515
* The two halves of the `userMessage` channel, pinned where they meet.
@@ -94,3 +94,27 @@ describe('userMessage survives the adapter boundary', () => {
9494
expect(declaredUserMessage(normaliseClientError(conflict))).toBeNull();
9595
});
9696
});
97+
98+
describe('a UNIQUE_VIOLATION field survives the adapter boundary', () => {
99+
it('reaches extractFieldErrors through the passthrough arm', () => {
100+
// `normaliseClientError` re-wraps only VALIDATION_FAILED and
101+
// CONCURRENT_UPDATE; a unique-index 409 leaves it untouched, so the single
102+
// `details.field` the client parked is what the form reads.
103+
const raw = Object.assign(new Error('A record with this SKU already exists'), {
104+
code: 'UNIQUE_VIOLATION',
105+
httpStatus: 409,
106+
details: {
107+
error: 'A record with this SKU already exists',
108+
code: 'UNIQUE_VIOLATION',
109+
field: 'sku',
110+
object: 'product',
111+
},
112+
});
113+
const normalised = normaliseClientError(raw) as typeof raw;
114+
expect(normalised.code).toBe('UNIQUE_VIOLATION');
115+
expect(normalised.httpStatus).toBe(409);
116+
expect(extractFieldErrors(normalised)).toEqual([
117+
{ field: 'sku', message: 'A record with this SKU already exists' },
118+
]);
119+
});
120+
});

‎ui/packages/react/src/utils/error-message.test.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,55 @@ describe('extractFieldErrors', () => {
8888
expect(extractFieldErrors('a string')).toBeNull();
8989
expect(extractFieldErrors(new Error('plain'))).toBeNull();
9090
});
91+
92+
// A unique-index refusal names ONE field on a single string `field`, not a
93+
// list: `@objectstack/rest` answers 409 `{ error, code: 'UNIQUE_VIOLATION',
94+
// developerMessage, field, object }`, and `@objectstack/client` puts that flat
95+
// body on `error.details`.
96+
function uniqueViolation(field?: string) {
97+
const error = field ? 'A record with this SKU already exists' : 'A record with this value already exists';
98+
return Object.assign(new Error(error), {
99+
code: 'UNIQUE_VIOLATION',
100+
httpStatus: 409,
101+
details: {
102+
error,
103+
code: 'UNIQUE_VIOLATION',
104+
developerMessage: 'Duplicate value for sku on product',
105+
...(field ? { field } : {}),
106+
object: 'product',
107+
},
108+
});
109+
}
110+
111+
it("marks the field a UNIQUE_VIOLATION names, with the server's sentence", () => {
112+
const err = uniqueViolation('sku');
113+
expect(err.code).toBe('UNIQUE_VIOLATION');
114+
expect(err.httpStatus).toBe(409);
115+
expect(extractFieldErrors(err)).toEqual([
116+
{ field: 'sku', message: 'A record with this SKU already exists' },
117+
]);
118+
});
119+
120+
it('reads a hand-rolled UNIQUE_VIOLATION carrying `field` directly', () => {
121+
const err = { code: 'UNIQUE_VIOLATION', field: 'name', error: 'A record with this Name already exists' };
122+
expect(extractFieldErrors(err)).toEqual([
123+
{ field: 'name', message: 'A record with this Name already exists' },
124+
]);
125+
});
126+
127+
it('returns null for a UNIQUE_VIOLATION without `field` — the server could not tell which column', () => {
128+
const err = uniqueViolation();
129+
expect(err.code).toBe('UNIQUE_VIOLATION');
130+
expect(err.httpStatus).toBe(409);
131+
expect(extractFieldErrors(err)).toBeNull();
132+
});
133+
134+
it('reads a single `field` only on a UNIQUE_VIOLATION', () => {
135+
// `INVALID_FIELD` also carries `field`, but it names a field the request
136+
// referenced that does not exist — there is no input to mark.
137+
const invalid = { code: 'INVALID_FIELD', httpStatus: 400, details: { code: 'INVALID_FIELD', field: 'skuu' } };
138+
expect(extractFieldErrors(invalid)).toBeNull();
139+
});
91140
});
92141

93142
/**

‎ui/packages/react/src/utils/error-message.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,9 @@ export function delegationScopeMessage(err: unknown): string | null {
174174
return extractWriteErrorMessage(err);
175175
}
176176

177+
/** The ADR-0112 code a write answers (409) when a unique index refuses it. */
178+
const UNIQUE_VIOLATION = 'UNIQUE_VIOLATION';
179+
177180
/** One field the server rejected, normalised for `form.setError`. */
178181
export interface WriteFieldError {
179182
/** Field name, matching the form control's `name`. */
@@ -204,6 +207,14 @@ export interface WriteFieldError {
204207
*
205208
* Entries without a usable `field` are dropped rather than guessed at: a wrong
206209
* mark on an innocent input is worse than the generic toast we already show.
210+
*
211+
* One refusal is field-scoped without a list: a `409 UNIQUE_VIOLATION` names
212+
* the ONE field whose unique index refused the write, on a single string
213+
* `field` (`details.field` on the raw client error, `field` on a hand-rolled
214+
* one), with the server's end-user sentence on `error`. It becomes a single
215+
* entry carrying that sentence. The server omits `field` whenever it cannot
216+
* tell which column collided (an index name, a composite key), and the same
217+
* rule holds here: no `field`, no entry, and the caller keeps its banner.
207218
*/
208219
export function extractFieldErrors(err: unknown): WriteFieldError[] | null {
209220
const e = err as Record<string, any> | null | undefined;
@@ -214,7 +225,11 @@ export function extractFieldErrors(err: unknown): WriteFieldError[] | null {
214225
(Array.isArray(e.details?.fields) && e.details.fields) ||
215226
(Array.isArray(e.fields) && e.fields) ||
216227
null;
217-
if (!raw || !Array.isArray(raw)) return null;
228+
if (!raw || !Array.isArray(raw)) {
229+
if (!errorCodeIs(e, UNIQUE_VIOLATION) && !errorCodeIs(e.details, UNIQUE_VIOLATION)) return null;
230+
const field = firstString(e.details?.field, e.field);
231+
return field ? [{ field, message: extractWriteErrorMessage(err) ?? '' }] : null;
232+
}
218233

219234
const out: WriteFieldError[] = [];
220235
for (const entry of raw) {

0 commit comments

Comments
 (0)