test(ui): pin refused-cell marks, the whole save-bar refusal and the grid's pre-send rule check

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9qibxQtKRyLyL6eK14pgY

Author: claude

ui/packages/components/src/__tests__/data-table-refusal-cells-27520.test.tsx

@@ -0,0 +1,160 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * objectstack-ai/objectstack#27520 (row 6) — a refused inline save says WHY,
+ * WHERE: the save bar says every reason whole, one line each, and the cells
+ * the refusal names are marked.
+ *
+ * The server refuses a row field by field (`VALIDATION_FAILED` with one
+ * `fields[]` entry per failed rule). The save bar used to cut that to one
+ * truncated line, the rest only in a native tooltip, and no cell said which
+ * value was wrong.
+ */
+import { describe, it, expect, vi } from 'vitest';
+import { fireEvent, waitFor, within } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import { renderComponent } from './test-utils';
+// Registers the renderers at module scope, NOT inside a `beforeAll`; see
+// object-ui/no-dynamic-import-in-test-hook (objectui#3010/#3021).
+import '../renderers';
+
+const schema = {
+  type: 'data-table' as const,
+  editable: true,
+  singleClickEdit: true,
+  columns: [
+    { header: 'Title', accessorKey: 'title' },
+    { header: 'Estimate', accessorKey: 'estimate', type: 'number' },
+  ],
+  data: [
+    { id: '1', title: 'Ingest pipeline', estimate: 60 },
+    { id: '2', title: 'Warehouse schema', estimate: 33 },
+  ],
+} as any;
+
+type Entry = { rowIndex: number; changes: Record<string, any>; row: any };
+
+const TITLE = 'Title is required and cannot be cleared';
+const ESTIMATE = 'Estimate (h) must be ≥ 0';
+
+/** The refusal `PATCH /data/:object/:id` answers, as `@objectstack/client` decorates it. */
+function fieldRefusal(fields: Array<{ field: string; message: string }>) {
+  const message = fields.map((f) => f.message).join('; ');
+  return Object.assign(new Error(message), {
+    code: 'VALIDATION_FAILED',
+    httpStatus: 400,
+    details: { error: message, code: 'VALIDATION_FAILED', fields: fields.map((f) => ({ ...f, code: 'invalid' })) },
+  });
+}
+
+const cellOf = (container: HTMLElement, rowNumber: number, colNumber: number) =>
+  container.querySelectorAll('tbody tr')[rowNumber].querySelectorAll('td')[colNumber] as HTMLElement;
+
+/** Stage a new value in a body cell. */
+function stage(container: HTMLElement, rowNumber: number, colNumber: number, value: string) {
+  const cell = cellOf(container, rowNumber, colNumber);
+  fireEvent.click(cell);
+  const input = cell.querySelector('input') as HTMLInputElement;
+  fireEvent.change(input, { target: { value } });
+  fireEvent.blur(input);
+}
+
+const refusalBar = (container: HTMLElement) =>
+  container.querySelector('[data-testid="table-save-refusal"]') as HTMLElement | null;
+
+describe('data-table — a refused save says every reason and marks its cells (#27520)', () => {
+  it('Save All: one line per reason, whole, and the two named cells of the refused row marked', async () => {
+    const onBatchSave = vi.fn(async (entries: Entry[]) => ({
+      failed: entries
+        .filter((e) => e.row.id === '1')
+        .map((e) => ({
+          rowIndex: e.rowIndex,
+          error: fieldRefusal([{ field: 'title', message: TITLE }, { field: 'estimate', message: ESTIMATE }]),
+        })),
+    }));
+    const { container, getByText } = renderComponent({ ...schema, onBatchSave });
+
+    stage(container, 0, 0, '');
+    stage(container, 0, 1, '-5');
+    stage(container, 1, 1, '31');
+    fireEvent.click(getByText(/Save All/));
+
+    await waitFor(() => expect(refusalBar(container)).not.toBeNull());
+    const bar = refusalBar(container)!;
+    expect(bar).toHaveAttribute('role', 'alert');
+    expect(within(bar).getAllByRole('listitem').map((li) => li.textContent)).toEqual([TITLE, ESTIMATE]);
+    // Said whole: not the `; `-joined summary, and not cut.
+    expect(bar.textContent).not.toContain('; ');
+    expect(bar.querySelector('.truncate')).toBeNull();
+
+    // The refused row's two cells carry their reasons; the saved row's none.
+    expect(cellOf(container, 0, 0)).toHaveAttribute('aria-invalid', 'true');
+    expect(cellOf(container, 0, 0)).toHaveAttribute('data-cell-error', TITLE);
+    expect(cellOf(container, 0, 1)).toHaveAttribute('aria-invalid', 'true');
+    expect(cellOf(container, 0, 1)).toHaveAttribute('data-cell-error', ESTIMATE);
+    expect(container.querySelectorAll('tbody tr')[1].querySelector('[aria-invalid="true"]')).toBeNull();
+  });
+
+  it('editing a marked cell takes its mark away and leaves the other', async () => {
+    const onBatchSave = vi.fn(async (entries: Entry[]) => ({
+      failed: entries.map((e) => ({
+        rowIndex: e.rowIndex,
+        error: fieldRefusal([{ field: 'title', message: TITLE }, { field: 'estimate', message: ESTIMATE }]),
+      })),
+    }));
+    const { container, getByText } = renderComponent({ ...schema, onBatchSave });
+
+    stage(container, 0, 0, '');
+    stage(container, 0, 1, '-5');
+    fireEvent.click(getByText(/Save All/));
+    await waitFor(() => expect(cellOf(container, 0, 1)).toHaveAttribute('aria-invalid', 'true'));
+
+    stage(container, 0, 1, '5');
+
+    await waitFor(() => expect(cellOf(container, 0, 1)).not.toHaveAttribute('aria-invalid'));
+    expect(cellOf(container, 0, 0)).toHaveAttribute('aria-invalid', 'true');
+  });
+
+  it('a single-row save marks the cell its refusal names, and a later save that lands clears it', async () => {
+    const onRowSave = vi
+      .fn()
+      .mockRejectedValueOnce(fieldRefusal([{ field: 'estimate', message: ESTIMATE }]))
+      .mockResolvedValueOnce(undefined);
+    // `rowActions` draws the edit column that holds the row's own save button.
+    const { container } = renderComponent({ ...schema, rowActions: true, onRowSave });
+
+    stage(container, 0, 1, '-5');
+    // The row's own save button (the edit column's save icon).
+    const saveRow = () =>
+      fireEvent.click(within(container.querySelectorAll('tbody tr')[0] as HTMLElement).getByTitle(/^Save row$/i));
+    saveRow();
+
+    await waitFor(() => expect(cellOf(container, 0, 1)).toHaveAttribute('data-cell-error', ESTIMATE));
+    expect(refusalBar(container)?.textContent).toContain(ESTIMATE);
+
+    stage(container, 0, 1, '5');
+    saveRow();
+    await waitFor(() => expect(onRowSave).toHaveBeenCalledTimes(2));
+    await waitFor(() => expect(container.querySelector('tbody [aria-invalid="true"]')).toBeNull());
+  });
+
+  it('a whole-batch rejection says its reasons but marks no cell: no row is named', async () => {
+    const onBatchSave = vi.fn(async () => {
+      throw fieldRefusal([{ field: 'title', message: TITLE }]);
+    });
+    const { container, getByText } = renderComponent({ ...schema, onBatchSave });
+
+    stage(container, 0, 0, '');
+    stage(container, 1, 1, '31');
+    fireEvent.click(getByText(/Save All/));
+
+    await waitFor(() => expect(refusalBar(container)?.textContent).toContain(TITLE));
+    expect(container.querySelector('tbody [aria-invalid="true"]')).toBeNull();
+  });
+});

ui/packages/plugin-form/src/MasterDetailForm.outcomeToastSupersede.test.tsx

@@ -53,15 +53,11 @@
  *    decided, whether that retry succeeds or fails again. Driven, not assumed:
  *    the second `it` below IS path 2, and it is green post-#7354.
  *
- * The parent `<ObjectForm>`'s own renderer also toasts a rejected write, under
- * its OWN `form-outcome:` id, and retires it at the top of the next submit
- * (objectui#7252). Before objectui#7354 that toast was already superseded and
- * was not what this file pinned; it was visible in the pre-fix registry below
- * as the second error entry. Post-#7354 it is the ONLY error entry — the
- * duplication objectui#7354 fixed is gone, so `distinctOnScreen` below now
- * sees the same one-error reality `onScreen` does; see
- * `MasterDetailForm.singleRefusalRaiser.test.tsx` for the pin that asserts
- * that directly on the raw (undeduplicated) registry.
+ * The parent `<ObjectForm>`'s own renderer says a rejected write once, by
+ * its banner, and raises no error toast for it (objectstack-ai/objectstack#27520);
+ * it retires any outcome toast of its own at the top of the next submit
+ * (objectui#7252). `MasterDetailForm.singleRefusalRaiser.test.tsx` pins the
+ * one-message count directly.
  *
  * ## Why the pin models sonner rather than counting calls
  *
@@ -189,27 +185,16 @@ const save = () => fireEvent.click(screen.getByRole('button', { name: /create/i
 const onScreen = (): ToastEntry[] => [...toastRegistry.values()];
 
 /**
- * What the screen SAYS, with each distinct statement counted once.
- *
- * Before objectui#7354: a refused master-detail save put the SAME refusal on
- * screen twice — `handleError` here raised one, and the parent
- * `<ObjectForm>`'s renderer raised its own for the write it re-threw. That
- * duplication was a separate defect from the one THIS file pins, so the
- * assertions below were written to deliberately not depend on HOW MANY
- * raisers reported an outcome — only on WHICH outcomes remain readable.
- *
- * objectui#7354 fixed that duplication (removed `handleError`'s own toast),
- * so today `distinctOnScreen()` and the raw `onScreen()` agree on a refused
- * save — there is only the one raiser left. The dedup here is kept anyway:
- * this file's OWN subject (a later outcome superseding an earlier one on THIS
- * form) never depended on how many raisers were active, and
- * `MasterDetailForm.singleRefusalRaiser.test.tsx` is where the raw,
- * undeduplicated count is the point of the assertion.
+ * The refusal as the form says it: its banner, the one message a refused save
+ * raises (objectstack-ai/objectstack#27520) — no error toast repeats it.
  */
-const distinctOnScreen = (): ToastEntry[] => {
-  const seen = new Map<string, ToastEntry>();
-  for (const t of onScreen()) seen.set(`${t.type}:${t.message}`, t);
-  return [...seen.values()];
+const refusalBanners = (): HTMLElement[] =>
+  screen.queryAllByRole('alert').filter((el) => (el.textContent ?? '').includes(REFUSAL));
+
+/** The refusal is said once, by its banner, and by no toast. */
+const saidOnceByBanner = () => {
+  expect(refusalBanners()).toHaveLength(1);
+  expect(onScreen().filter((t) => t.type === 'error')).toEqual([]);
 };
 
 beforeEach(() => {
@@ -228,7 +213,7 @@ describe('MasterDetailForm — a later save outcome supersedes this form’s ear
     fireEvent.change(await headerInput(container), { target: { value: 'PO-1' } });
     save();
     await waitFor(() => expect(ds.batchTransaction).toHaveBeenCalledTimes(1));
-    await waitFor(() => expect(distinctOnScreen()).toEqual([{ type: 'error', message: REFUSAL }]));
+    await waitFor(saidOnceByBanner);
 
     // The user corrects the input and saves again inside the refusal's lifetime.
     fireEvent.change(await headerInput(container), { target: { value: 'PO-1b' } });
@@ -237,6 +222,7 @@ describe('MasterDetailForm — a later save outcome supersedes this form’s ear
 
     // The card's own sentence: one form, one outcome on screen.
     await waitFor(() => expect(onScreen()).toEqual([{ type: 'success', message: 'Created' }]));
+    expect(refusalBanners()).toEqual([]);
   });
 
   it('retires the refusal even when the host owns confirmation (no built-in success toast)', async () => {
@@ -251,7 +237,7 @@ describe('MasterDetailForm — a later save outcome supersedes this form’s ear
     fireEvent.change(await headerInput(container), { target: { value: 'PO-2' } });
     save();
     await waitFor(() => expect(ds.batchTransaction).toHaveBeenCalledTimes(1));
-    await waitFor(() => expect(distinctOnScreen()).toEqual([{ type: 'error', message: REFUSAL }]));
+    await waitFor(saidOnceByBanner);
 
     fireEvent.change(await headerInput(container), { target: { value: 'PO-2b' } });
     save();
@@ -260,6 +246,7 @@ describe('MasterDetailForm — a later save outcome supersedes this form’s ear
 
     // Nothing left asserting the save was refused.
     await waitFor(() => expect(onScreen()).toEqual([]));
+    expect(refusalBanners()).toEqual([]);
   });
 
   it('does not stack a second refusal when the retry is refused too', async () => {
@@ -273,15 +260,14 @@ describe('MasterDetailForm — a later save outcome supersedes this form’s ear
     fireEvent.change(await headerInput(container), { target: { value: 'PO-3' } });
     save();
     await waitFor(() => expect(ds.batchTransaction).toHaveBeenCalledTimes(1));
-    await waitFor(() => expect(distinctOnScreen()).toEqual([{ type: 'error', message: REFUSAL }]));
-    // How many raisers reported this refusal is not this file's subject; that
-    // the SECOND attempt adds none of its own is.
+    await waitFor(saidOnceByBanner);
+    // That the SECOND attempt adds no message of its own is this file's subject.
     const afterFirstAttempt = onScreen().length;
 
     fireEvent.change(await headerInput(container), { target: { value: 'PO-3b' } });
     save();
     await waitFor(() => expect(ds.batchTransaction).toHaveBeenCalledTimes(2));
-    await waitFor(() => expect(distinctOnScreen()).toEqual([{ type: 'error', message: REFUSAL }]));
+    await waitFor(saidOnceByBanner);
     expect(onScreen()).toHaveLength(afterFirstAttempt);
   });
 });

ui/packages/plugin-form/src/MasterDetailForm.singleRefusalRaiser.test.tsx

@@ -147,23 +147,24 @@ afterEach(() => {
 });
 
 describe('MasterDetailForm — a refused save reports its refusal exactly once (objectui#7354)', () => {
-  it('leaves exactly one error toast on screen, not one per raiser', async () => {
+  it('says it once, by the form banner, with no error toast from any raiser', async () => {
     const ds = makeDataSource();
     const { container } = masterDetail(ds);
 
     fireEvent.change(await headerInput(container), { target: { value: 'PO-1' } });
     save();
     await waitFor(() => expect(ds.batchTransaction).toHaveBeenCalledTimes(1));
 
-    // Wait for the outcome to settle, then read the raw (undeduplicated)
-    // registry: this is the count that told the two-raiser defect apart from
-    // the fix. Give the throw its full trip through `<ObjectForm>`'s catch
-    // and back up into the form renderer's before asserting.
-    await waitFor(() => expect(onScreen().length).toBeGreaterThan(0));
-
-    const entries = onScreen();
-    // The lit control: the surviving entry must still be the refusal, so this
-    // cannot pass by silencing every raiser instead of collapsing to one.
-    expect(entries).toEqual([{ type: 'error', message: REFUSAL }]);
+    // Wait for the outcome to settle: the throw's full trip through
+    // `<ObjectForm>`'s catch and back up into the form renderer's, which
+    // raises the banner (objectstack-ai/objectstack#27520).
+    await waitFor(() =>
+      expect(screen.queryAllByRole('alert').filter((el) => (el.textContent ?? '').includes(REFUSAL))).toHaveLength(1),
+    );
+
+    // The raw (undeduplicated) registry: no raiser toasts the refusal as well.
+    // The lit control is the banner above, so this cannot pass by silencing
+    // every raiser.
+    expect(onScreen().filter((entry) => entry.type === 'error')).toEqual([]);
   });
 });

ui/packages/plugin-form/src/MasterDetailForm.test.tsx

@@ -128,7 +128,7 @@ describe('MasterDetailForm — submit feedback (never silent)', () => {
     expect(toastSuccess).not.toHaveBeenCalled();
   });
 
-  it('shows an error toast when the atomic write fails (rollback)', async () => {
+  it('says the failure when the atomic write fails (rollback)', async () => {
     const batchTransaction = vi.fn().mockRejectedValue(new Error('BATCH_ERROR: rolled back'));
     const ds = makeDataSource({ batchTransaction });
 
@@ -155,7 +155,8 @@ describe('MasterDetailForm — submit feedback (never silent)', () => {
     fireEvent.click(screen.getByRole('button', { name: /create/i }));
 
     await waitFor(() => expect(batchTransaction).toHaveBeenCalled());
-    await waitFor(() => expect(toastError).toHaveBeenCalled()); // <- failure is surfaced
+    // The failure is surfaced, once, by the form's banner (objectstack-ai/objectstack#27520).
+    await waitFor(() => expect(screen.getByRole('alert')).toBeTruthy());
     expect(toastSuccess).not.toHaveBeenCalled();
   });
 });
@@ -211,7 +212,7 @@ describe('MasterDetailForm — non-atomic fallback (emulation via runBatchTransa
     expect(toastError).not.toHaveBeenCalled();
   });
 
-  it('create: a failing child create compensates by deleting the just-created parent + error toast', async () => {
+  it('create: a failing child create compensates by deleting the just-created parent + says the failure', async () => {
     const create = vi.fn(async (object: string, data: any) => {
       if (object === 'po_line') throw new Error('child create failed');
       return { id: 'po1', ...data };
@@ -226,7 +227,7 @@ describe('MasterDetailForm — non-atomic fallback (emulation via runBatchTransa
     // Best-effort compensation removes the orphan parent that was created before
     // the child failed — the non-atomic path's stand-in for a rollback.
     await waitFor(() => expect(del).toHaveBeenCalledWith('po', 'po1'));
-    await waitFor(() => expect(toastError).toHaveBeenCalled());
+    await waitFor(() => expect(screen.getByRole('alert')).toBeTruthy());
     expect(toastSuccess).not.toHaveBeenCalled();
   });
 });

ui/packages/plugin-form/src/WizardForm.outcomeToastSupersede.test.tsx

@@ -172,6 +172,17 @@ afterEach(() => {
   cleanup();
 });
 
+/**
+ * The refusal as the form says it: its banner, the one message a refused
+ * submit raises (objectstack-ai/objectstack#27520) — no error toast repeats it.
+ */
+const refusalBanners = (): HTMLElement[] =>
+  screen.queryAllByRole('alert').filter((el) => (el.textContent ?? '').includes(REFUSAL));
+const saidOnceByBanner = () => {
+  expect(refusalBanners()).toHaveLength(1);
+  expect(onScreen().filter((t) => t.type === 'error')).toEqual([]);
+};
+
 describe('WizardForm — a later outcome supersedes this form’s earlier one', () => {
   it('leaves only the success toast after a refused submit is retried and accepted', async () => {
     const ds = makeDataSource();
@@ -185,7 +196,7 @@ describe('WizardForm — a later outcome supersedes this form’s earlier one',
     fireEvent.change(await inputNamed(container, 'status'), { target: { value: 'archived' } });
     submitStep(container);
     await waitFor(() => expect(ds.create).toHaveBeenCalledTimes(1));
-    await waitFor(() => expect(onScreen()).toEqual([{ type: 'error', message: REFUSAL }]));
+    await waitFor(saidOnceByBanner);
 
     // Back to step 2 of the reported flow, change the status, and Create again.
     // The navigation matters: it is what proves the id survives step changes —
@@ -200,6 +211,7 @@ describe('WizardForm — a later outcome supersedes this form’s earlier one',
 
     // The card's own sentence: one form, one outcome on screen.
     await waitFor(() => expect(onScreen()).toEqual([{ type: 'success', message: SUCCESS }]));
+    expect(refusalBanners()).toEqual([]);
   });
 
   it('does not stack a second refusal when the retry is refused too', async () => {
@@ -216,10 +228,10 @@ describe('WizardForm — a later outcome supersedes this form’s earlier one',
 
     submitStep(container);
     await waitFor(() => expect(ds.create).toHaveBeenCalledTimes(1));
-    await waitFor(() => expect(onScreen()).toHaveLength(1));
+    await waitFor(saidOnceByBanner);
 
     submitStep(container);
     await waitFor(() => expect(ds.create).toHaveBeenCalledTimes(2));
-    await waitFor(() => expect(onScreen()).toEqual([{ type: 'error', message: REFUSAL }]));
+    await waitFor(saidOnceByBanner);
   });
 });