Skip to content

Commit 75ecaf4

Browse files
fix(ui): a refused inline-grid save marks its cells and says every reason; a refused form save is said once (#28396)
<!-- claude-projects-attribution: {"github_login":"os-tesla"} --> _Requested by **Tesla** · [project thread](https://claude.ai/code/project/chan_01AXsQkLFKRdQCqQ3RTr6Jao?thread=cmsg_01AXsQkLFKRdQCqQ3RTr6JaoBcbFtjDn1QChVRkBtHrTF3)_ Part of #27520 (rows 6 and 19; every other row stays on the card) Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:表格行内编辑点「Save All」被服务端拒绝时,出错的单元格加红框(悬停显示原因),每条原因完整地单独一行显示在工具栏下方,不再截断;表格自带保存在发送前先按字段规则(必填、最小/最大值、长度、邮箱/网址格式)检查,违规的行不发送,其他行照常保存;「Save All」不再贴在表格右边缘。表单保存被拒(如 Studio 建的对象的校验规则)时只显示一条横幅并滚动到可见处,不再同时弹一条一模一样的 toast。 - **为什么改**:18.3.0 RC 走查第 6、19 行:用户看不出是哪一格错了、原因被截断、明知违反 min 的值还发到服务端;表单被拒时同一句话说两遍。 - **风险与回滚**:发送前检查只读字段自己声明的规则,读不到就不判,服务端仍是最终裁判;表单不再弹错误 toast,这推翻了 objectui#13340 钉住的「横幅+一条 toast」行为(测试已按席位要求改写,保留「重试不叠加」的保证),需要契约复审。回滚即 revert 本 PR。 - **先看**:`ui/packages/components/src/renderers/complex/data-table.tsx`(`saveRefusalLines`、`cellErrors`);`ui/packages/plugin-grid/src/objectGridInlineRules.ts`;`ui/packages/components/src/renderers/form/form.tsx`(`raiseRefusal`)。 - **两分钟验证**:showcase 打开 Tasks → Grid,清空一行的 Title、把 Estimate 改成 -5,点 Save All,预期两格红框、横幅列出两条原因、网络面板没有 PATCH;用一个带 script 校验规则的对象新建记录并触发规则,预期只有横幅、没有 toast。 - **需要你做的**:无,席位落地。 ## What changes **Row 6, a refused inline save says every reason and marks its cells** (`data-table.tsx`, the renderer that owns the grid's save bar). - The refusal was one `truncate` line in the save cluster, with the rest only in a `title` tooltip. It is now its own `role="alert"` row (`data-testid="table-save-refusal"`) under the toolbar, taking the full width (`sm:basis-full`). One reason reads `Save failed: <reason>`; several read `Save failed:` with one list item each. The lines come from the shared `@object-ui/react` readers the form already uses (`unmarkedRefusalLines`, falling back to the error's message), deduplicated across rows. - The cells a refusal names are marked: `aria-invalid`, `data-cell-error` and `title` carry the reason, and an inset destructive ring draws it. The marks come from `extractFieldErrors` (`fields[]` / `details.fields` / `validationErrors`) and are kept per row key, so they follow the record across re-reads. Editing a marked cell clears that cell's mark; a save that lands, Cancel and Cancel All clear the row's or all marks. A whole-batch rejection names no row, so it says its reasons and marks no cell. - The toolbar gets `data-slot="table-toolbar"` and wraps on `sm`. `ObjectGrid` insets it (`px-3 pt-2`), so Save All no longer sits on the grid's right edge. **Row 6, the grid checks a field's own rules before sending** (`plugin-grid`). - New `objectGridInlineRules.ts` (not exported): `inlineEditRuleRefusals` reads each changed field's declaration through `buildValidationRules` from `@object-ui/fields`, the same rules the record form checks: `required` (blank value), `min` / `max` (a staged number or number string), `minLength` / `maxLength` and the email / URL `pattern`. An authored rule message wins; otherwise the `validation.*` key is localized with the header's label. `inlineRuleRefusalError` builds the server's refusal shape (`code: 'VALIDATION_FAILED'`, `fields[]`), so the table marks and says it like a server refusal. - `defaultRowSave` throws that refusal before claiming or writing. `saveRowsClaimed` refuses such a row unsent and still sends the others. A host's own `onRowSave` / `onBatchSave` is unchanged. A field with no declaration, or a rule set that cannot be read, is not judged; validation rules, uniqueness and state machines still reach the server. - `objectGridTranslations.ts` gains the `validation.*` defaults, byte-identical to the en pack. **Row 19, a refused form save is said once** (`form.tsx`). - The three refusal points (a `visibleWhen` fault, `onAction`'s `result.error`, a thrown write) raised the banner and `toast.error(…, { id: outcomeToastId })` together. They now call `raiseRefusal`: dismiss any outcome toast, set the banner, and scroll the banner into view (`block: 'nearest'`). No error toast is raised, so a refusal the banner draws is said once. Every refusal path of this renderer draws the banner, so the seat's "keep the toast where no banner is drawn" case does not arise here. - The field mark for a rule that names a field already worked on main (measured: a `cross_field` rule with `fields: [qty, max_qty]` marks Quantity). A `script` rule declares no fields, so its refusal names only `_record` and there is nothing to mark. Recorded on the card, not filed. ## Tests - New `ui/packages/components/src/__tests__/data-table-refusal-cells-27520.test.tsx` (4): Save All says two reasons as two whole list items and marks the refused row's two cells only; editing a marked cell removes its mark and keeps the other; a single-row save marks, and a later save that lands clears; a whole-batch rejection marks no cell. - New `ui/packages/plugin-grid/src/__tests__/inlineEditRuleCheck-27520.test.tsx` (7): over the real `ObjectGrid` + `data-table`, Save All refuses the row below `min` unsent, marks its cell and saves the other row; the per-row save refuses the same edit unsent; a control edit inside the rules is sent. Unit: one entry per broken rule with the localized message; a staged number string is read as a number and length is judged on text; no declaration judges nothing; an authored message wins. - Adapted, not deleted (the seat's condition): `form-one-toast-per-refusal-13340.test.tsx` now pins one banner, scrolled into view, zero error toasts, and a retry that still leaves one banner. The other refusal pins that asserted the toast now read the banner and assert no error toast: in components `form-invalid-toast-joiner`, `form-refusal-lines-28053`, `form-server-field-errors`, `visibleWhen-fault-refuses-submit-8069`, `fieldtab-visiblewhen-6237`; in plugin-form `MasterDetailForm.outcomeToastSupersede`, `MasterDetailForm.singleRefusalRaiser`, `MasterDetailForm`, `WizardForm.outcomeToastSupersede`, `submitHandlerSeam`, `submitTargetRefusal`. - `pnpm --dir ui exec vitest run --maxWorkers=3 packages/components/ packages/plugin-grid/ packages/plugin-form/` at f77d796: 944 files passed, 1 skipped; 9131 tests passed, 25 skipped (lock VERDICT command-exit 0). The first run at 9edf9af had 25 reds in seven plugin-grid inline-edit files, all `Cannot read properties of undefined (reading 'min')` from the pre-send check. f77d796 makes an unreadable rule set judge nothing, and the rerun above is green. - `type-check` of `@object-ui/components` and `@object-ui/plugin-grid`: exit 0. - The whole app-shell and console suites were not run locally; they are left to CI. A search of both for the old refusal toast and the save-bar text found no pin. ## Acceptance notes - **This reverses a pinned visible behaviour.** objectui#13340 pinned "a refusal raises the banner and exactly one toast". The seat accepted the change on two conditions, both met: the banner is the one message and is scrolled into view, and the #13340 test is adapted, keeping its retry guarantee. Contract review is owed for this. - Row 19's field mark for a `script` rule cannot be drawn: the spec gives a script rule no `fields`, so the server's answer names `_record`. Recorded, not filed. - The pre-send check covers only the built-in saves. A host that supplies `onRowSave` / `onBatchSave` owns its own checking. ## Verification <details><summary>Live before/after (showcase on a --fresh stack, admin, 1440x900)</summary> Base `origin/main` 3ba5206 (before) and this branch (after). Backend `examples/app-showcase` `objectstack dev --seed-admin --fresh`, Console from `ui/apps/console` source through vite, driven with Playwright. - Row 6, Tasks → Grid, Title cleared and Estimate set to -5, Save All. Before: one truncated banner line `Save failed: Title is required and cannot be cleared; Es…` (384x20 px), no cell marked, `PATCH /api/v1/data/showcase_task/<id>` sent with -5 and answered `400 VALIDATION_FAILED` naming `title` and `estimate_hours`, Save All 0 px from the content edge. After: Title and Estimate cells marked, no write sent, banner "Save failed:" with "Title is required" and "Estimate (h) must be at least 0" as two lines, Save All 12 px from the edge. - Row 19, an object authored with `PUT /api/v1/meta/object/t27520_widget` (a `script` rule and a `cross_field` rule), create page `/new` and the list's New dialog. Before: a script-rule refusal (`fields: [{ field: '_record', code: 'rule_violation' }]`) drew the banner and an identical toast, on both. After: the banner only, no toast, on both. The `cross_field` refusal marks Quantity before and after. </details> <details><summary>Derived gates</summary> At f77d796: `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 35 families against the diff. All 35 ran with `</dev/null`, exit codes recorded first. `dispatch-gates --ran`: "35 derived famil(ies) accounted for — 35 run, 0 NOT-MEASURED". - `pnpm check:type-check-debt` first exited 3 (not measured): it queued behind this branch's own test run on the verify lock and timed out. Rerun under the lock: exit 0, "0 raw tsc error(s) total, none above its recorded number". - `pnpm --dir ui exec vitest run scripts/` under the verify lock: 169 files passed, 2 skipped, exit 0. - Also run: `pnpm --dir ui check:new-line-citations` exit 0; `node scripts/check-sdui-manifest.mjs` exit 0. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01L9qibxQtKRyLyL6eK14pgY)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4a7b99f commit 75ecaf4

20 files changed

Lines changed: 814 additions & 141 deletions
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@object-ui/components': patch
3+
'@object-ui/plugin-grid': patch
4+
---
5+
6+
fix(ui): a refused inline-grid save marks its cells and says every reason; a refused form save is said once
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
- **Inline grid edit, Save All.** When the server refused a row, the save bar showed one shortened line, with the rest only in a tooltip, and no cell showed which value was wrong. Now every reason is shown in full on its own line under the toolbar, and each cell the refusal names gets a red outline with its reason on hover. Editing a marked cell removes its outline.
12+
- **Field rules before sending.** The grid's built-in saves now check an edited value against the field's own rules (required, min / max, length, email / URL pattern) before sending, and refuse a row that breaks one in the same way the server would. That row is not sent; the other rows still save.
13+
- **Save All placement.** The grid toolbar is padded, so the Save All / Cancel All buttons no longer sit against the right edge of the grid.
14+
- **Refused form save.** When the server refused a form save, such as one blocked by a validation rule on a Studio-authored object, the form showed a banner and an identical error toast. It now shows the banner only, scrolled into view, and a retry still replaces that banner instead of adding another. This reverses the earlier one-error-toast behaviour.
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* objectstack-ai/objectstack#27520 (row 6) — a refused inline save says WHY,
11+
* WHERE: the save bar says every reason whole, one line each, and the cells
12+
* the refusal names are marked.
13+
*
14+
* The server refuses a row field by field (`VALIDATION_FAILED` with one
15+
* `fields[]` entry per failed rule). The save bar used to cut that to one
16+
* truncated line, the rest only in a native tooltip, and no cell said which
17+
* value was wrong.
18+
*/
19+
import { describe, it, expect, vi } from 'vitest';
20+
import { fireEvent, waitFor, within } from '@testing-library/react';
21+
import '@testing-library/jest-dom';
22+
import { renderComponent } from './test-utils';
23+
// Registers the renderers at module scope, NOT inside a `beforeAll`; see
24+
// object-ui/no-dynamic-import-in-test-hook (objectui#3010/#3021).
25+
import '../renderers';
26+
27+
const schema = {
28+
type: 'data-table' as const,
29+
editable: true,
30+
singleClickEdit: true,
31+
columns: [
32+
{ header: 'Title', accessorKey: 'title' },
33+
{ header: 'Estimate', accessorKey: 'estimate', type: 'number' },
34+
],
35+
data: [
36+
{ id: '1', title: 'Ingest pipeline', estimate: 60 },
37+
{ id: '2', title: 'Warehouse schema', estimate: 33 },
38+
],
39+
} as any;
40+
41+
type Entry = { rowIndex: number; changes: Record<string, any>; row: any };
42+
43+
const TITLE = 'Title is required and cannot be cleared';
44+
const ESTIMATE = 'Estimate (h) must be ≥ 0';
45+
46+
/** The refusal `PATCH /data/:object/:id` answers, as `@objectstack/client` decorates it. */
47+
function fieldRefusal(fields: Array<{ field: string; message: string }>) {
48+
const message = fields.map((f) => f.message).join('; ');
49+
return Object.assign(new Error(message), {
50+
code: 'VALIDATION_FAILED',
51+
httpStatus: 400,
52+
details: { error: message, code: 'VALIDATION_FAILED', fields: fields.map((f) => ({ ...f, code: 'invalid' })) },
53+
});
54+
}
55+
56+
const cellOf = (container: HTMLElement, rowNumber: number, colNumber: number) =>
57+
container.querySelectorAll('tbody tr')[rowNumber].querySelectorAll('td')[colNumber] as HTMLElement;
58+
59+
/** Stage a new value in a body cell. */
60+
function stage(container: HTMLElement, rowNumber: number, colNumber: number, value: string) {
61+
const cell = cellOf(container, rowNumber, colNumber);
62+
fireEvent.click(cell);
63+
const input = cell.querySelector('input') as HTMLInputElement;
64+
fireEvent.change(input, { target: { value } });
65+
fireEvent.blur(input);
66+
}
67+
68+
const refusalBar = (container: HTMLElement) =>
69+
container.querySelector('[data-testid="table-save-refusal"]') as HTMLElement | null;
70+
71+
describe('data-table — a refused save says every reason and marks its cells (#27520)', () => {
72+
it('Save All: one line per reason, whole, and the two named cells of the refused row marked', async () => {
73+
const onBatchSave = vi.fn(async (entries: Entry[]) => ({
74+
failed: entries
75+
.filter((e) => e.row.id === '1')
76+
.map((e) => ({
77+
rowIndex: e.rowIndex,
78+
error: fieldRefusal([{ field: 'title', message: TITLE }, { field: 'estimate', message: ESTIMATE }]),
79+
})),
80+
}));
81+
const { container, getByText } = renderComponent({ ...schema, onBatchSave });
82+
83+
stage(container, 0, 0, '');
84+
stage(container, 0, 1, '-5');
85+
stage(container, 1, 1, '31');
86+
fireEvent.click(getByText(/Save All/));
87+
88+
await waitFor(() => expect(refusalBar(container)).not.toBeNull());
89+
const bar = refusalBar(container)!;
90+
expect(bar).toHaveAttribute('role', 'alert');
91+
expect(within(bar).getAllByRole('listitem').map((li) => li.textContent)).toEqual([TITLE, ESTIMATE]);
92+
// Said whole: not the `; `-joined summary, and not cut.
93+
expect(bar.textContent).not.toContain('; ');
94+
expect(bar.querySelector('.truncate')).toBeNull();
95+
96+
// The refused row's two cells carry their reasons; the saved row's none.
97+
expect(cellOf(container, 0, 0)).toHaveAttribute('aria-invalid', 'true');
98+
expect(cellOf(container, 0, 0)).toHaveAttribute('data-cell-error', TITLE);
99+
expect(cellOf(container, 0, 1)).toHaveAttribute('aria-invalid', 'true');
100+
expect(cellOf(container, 0, 1)).toHaveAttribute('data-cell-error', ESTIMATE);
101+
expect(container.querySelectorAll('tbody tr')[1].querySelector('[aria-invalid="true"]')).toBeNull();
102+
});
103+
104+
it('editing a marked cell takes its mark away and leaves the other', async () => {
105+
const onBatchSave = vi.fn(async (entries: Entry[]) => ({
106+
failed: entries.map((e) => ({
107+
rowIndex: e.rowIndex,
108+
error: fieldRefusal([{ field: 'title', message: TITLE }, { field: 'estimate', message: ESTIMATE }]),
109+
})),
110+
}));
111+
const { container, getByText } = renderComponent({ ...schema, onBatchSave });
112+
113+
stage(container, 0, 0, '');
114+
stage(container, 0, 1, '-5');
115+
fireEvent.click(getByText(/Save All/));
116+
await waitFor(() => expect(cellOf(container, 0, 1)).toHaveAttribute('aria-invalid', 'true'));
117+
118+
stage(container, 0, 1, '5');
119+
120+
await waitFor(() => expect(cellOf(container, 0, 1)).not.toHaveAttribute('aria-invalid'));
121+
expect(cellOf(container, 0, 0)).toHaveAttribute('aria-invalid', 'true');
122+
});
123+
124+
it('a single-row save marks the cell its refusal names, and a later save that lands clears it', async () => {
125+
const onRowSave = vi
126+
.fn()
127+
.mockRejectedValueOnce(fieldRefusal([{ field: 'estimate', message: ESTIMATE }]))
128+
.mockResolvedValueOnce(undefined);
129+
// `rowActions` draws the edit column that holds the row's own save button.
130+
const { container } = renderComponent({ ...schema, rowActions: true, onRowSave });
131+
132+
stage(container, 0, 1, '-5');
133+
// The row's own save button (the edit column's save icon).
134+
const saveRow = () =>
135+
fireEvent.click(within(container.querySelectorAll('tbody tr')[0] as HTMLElement).getByTitle(/^Save row$/i));
136+
saveRow();
137+
138+
await waitFor(() => expect(cellOf(container, 0, 1)).toHaveAttribute('data-cell-error', ESTIMATE));
139+
expect(refusalBar(container)?.textContent).toContain(ESTIMATE);
140+
141+
stage(container, 0, 1, '5');
142+
saveRow();
143+
await waitFor(() => expect(onRowSave).toHaveBeenCalledTimes(2));
144+
await waitFor(() => expect(container.querySelector('tbody [aria-invalid="true"]')).toBeNull());
145+
});
146+
147+
it('a whole-batch rejection says its reasons but marks no cell: no row is named', async () => {
148+
const onBatchSave = vi.fn(async () => {
149+
throw fieldRefusal([{ field: 'title', message: TITLE }]);
150+
});
151+
const { container, getByText } = renderComponent({ ...schema, onBatchSave });
152+
153+
stage(container, 0, 0, '');
154+
stage(container, 1, 1, '31');
155+
fireEvent.click(getByText(/Save All/));
156+
157+
await waitFor(() => expect(refusalBar(container)?.textContent).toContain(TITLE));
158+
expect(container.querySelector('tbody [aria-invalid="true"]')).toBeNull();
159+
});
160+
});

0 commit comments

Comments
 (0)