fix(grid): a bulk delete / by-name action clears the row checkboxes too (#3056) (#3058)

`ObjectGrid` carries two selection sources that must move together:
`selectedRows` (ours — drives the toolbar) and the data-table's internal
`selectedRowIds` (drives the checkboxes, cleared only when the host bumps
`selectionResetKey`). `handleBulkDialogClose` reset both; `dispatchBulkAction`
reset only the first, on BOTH of its branches — the bulk-delete route through
`onBulkDelete` and the by-name runner dispatch.

So a successful run left the toolbar gone and every row still ticked, with no
affordance to act on them. The invariant was already written down in
`handleBulkDialogClose`'s comment ("ticked rows with no toolbar"); it just was
not upheld on the other path.

All three call sites now go through one `resetSelection()` helper, so the rule
is structural instead of three places remembering to agree. Failure semantics
are unchanged: an action reporting `success: false` keeps both sources so the
user can retry the same rows.

Reverse-control: with only ObjectGrid.tsx reverted, the two new positive
assertions fail (checkboxes stay 'checked' after a successful by-name action
and after a bulk delete).

Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: os-zhuang

.changeset/bulk-selection-reset-both-sources.md

@@ -0,0 +1,22 @@
+---
+"@object-ui/plugin-grid": patch
+---
+
+fix(grid): a bulk delete / by-name action clears the row checkboxes, not just the toolbar — objectui#3056
+
+After a successful bulk delete (or a bulk action dispatched to a
+consumer-registered runner handler), the selection toolbar vanished but every
+row stayed visibly ticked. The user was left on a page of selected rows with no
+toolbar to act on them, and no way back except unticking each row or reloading.
+
+`ObjectGrid` carries two selection sources that must move together:
+`selectedRows` (ours — drives the toolbar) and the data-table's internal
+`selectedRowIds` (drives the checkboxes, cleared only when the host bumps
+`selectionResetKey`). `handleBulkDialogClose` reset both; `dispatchBulkAction`
+reset only the first, on both of its branches.
+
+Both now go through one `resetSelection()` helper — including the dialog path,
+so the invariant is structural rather than three call sites remembering to
+agree. Failure semantics are untouched: a by-name action that reports
+`success: false` still keeps the toolbar AND the checkboxes so the user can fix
+the cause and retry the same rows.

packages/plugin-grid/src/ObjectGrid.tsx

@@ -1711,24 +1711,35 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
   // Bulk action dispatcher — for the implicit 'delete' action, route through
   // the consumer-provided onBulkDelete (which already knows about confirm +
   // refresh). Other actions fall through to the generic action runner.
+  //
+  // [#3056] Both branches must clear BOTH selection sources. `selectedRows` is
+  // ours and drives the toolbar; the row checkboxes live inside the data-table
+  // and only clear when `selectionResetKey` moves. Bumping one without the
+  // other strands the user on a page of ticked rows with no toolbar to act on
+  // them — the exact drift `handleBulkDialogClose` below already guards.
+  const resetSelection = () => {
+    setSelectedRows([]);
+    setSelectAllMatching(false);
+    setSelectionResetKey(k => k + 1);
+  };
+
   const dispatchBulkAction = (action: string, rows: any[]) => {
     void (async () => {
       const expanded = await resolveBulkRows(rows);
       if (action === 'delete' && onBulkDelete) {
         onBulkDelete(expanded);
-        setSelectedRows([]);
-        setSelectAllMatching(false);
+        resetSelection();
         return;
       }
-      // A string bulk action (e.g. 下推 / 派工) mutated the selected records,
-      // usually through a custom API that never touches dataSource.update — so
-      // nothing else signals the grid to refetch. On success, reset the
-      // selection toolbar and refresh so the list reflects the server state
-      // (mirrors the delete branch and handleBulkDialogClose).
+      // A string bulk action (e.g. a consumer-registered runner handler)
+      // mutated the selected records, usually through a custom API that never
+      // touches dataSource.update — so nothing else signals the grid to
+      // refetch. On success, reset the selection and refresh so the list
+      // reflects the server state (mirrors the delete branch and
+      // handleBulkDialogClose).
       const res = await executeAction({ type: action, params: { records: expanded } });
       if (res?.success) {
-        setSelectedRows([]);
-        setSelectAllMatching(false);
+        resetSelection();
         setRefreshKey(k => k + 1);
       }
     })();
@@ -1791,13 +1802,9 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
     // Only reset selection when the run actually changed something. A total
     // failure (0 succeeded — e.g. a "推计划" precondition error) leaves the data
     // untouched, so we keep the selection *and* the toolbar so the user can fix
-    // it and retry the same rows. Both selection sources must move together, or
-    // the checkboxes (table-internal) and the toolbar (our `selectedRows`) drift
-    // out of sync — ticked rows with no toolbar.
+    // it and retry the same rows.
     if (result && result.succeeded > 0) {
-      setSelectedRows([]);
-      setSelectAllMatching(false);
-      setSelectionResetKey(k => k + 1);
+      resetSelection();
       // Trigger refresh via the same path used by single-record mutations.
       setRefreshKey(k => k + 1);
     }

packages/plugin-grid/src/__tests__/bulkActionRefresh.test.tsx

@@ -291,3 +291,85 @@ describe('ObjectGrid — bulk dialog Done keeps selection in sync', () => {
     expect(headerChecked()).toBe('checked');
   });
 });
+
+/**
+ * Same desync, the OTHER dispatcher (#3056). `dispatchBulkAction` — the
+ * by-name path and the bulk-delete path — cleared `selectedRows` (toolbar) but
+ * never bumped `selectionResetKey` (checkboxes), so a successful run left the
+ * user staring at a page of ticked rows with no toolbar to act on them. Only
+ * the rich-def path above upheld the invariant; these pin the other two.
+ *
+ * Both survive objectui#3002: a name that resolves to a declared object action
+ * is now promoted to a def and routes through the dialog instead, so what still
+ * reaches `dispatchBulkAction` is a consumer-registered runner handler
+ * (`approve` here) and the canonical `'delete'`.
+ */
+describe('ObjectGrid — dispatchBulkAction keeps selection in sync (#3056)', () => {
+  const headerChecked = () =>
+    (document.querySelector('thead [role="checkbox"]') as HTMLElement)?.getAttribute('data-state');
+
+  it('clears the row checkboxes (not just the toolbar) after a by-name action', async () => {
+    const ds = makeDataSource();
+    const approve = vi.fn(async () => ({ success: true }));
+    renderGrid(ds, { approve });
+
+    await waitFor(() => expect(screen.getByText('Plan A')).toBeInTheDocument());
+    fireEvent.click(document.querySelector('thead [role="checkbox"]') as HTMLElement);
+    await waitFor(() => expect(headerChecked()).toBe('checked'));
+
+    fireEvent.click(await screen.findByTestId('bulk-action-approve'));
+    await waitFor(() => expect(approve).toHaveBeenCalledTimes(1));
+
+    await waitFor(() =>
+      expect(screen.queryByTestId('bulk-actions-bar')).not.toBeInTheDocument(),
+    );
+    await waitFor(() => expect(headerChecked()).toBe('unchecked'));
+  });
+
+  it('leaves the checkboxes ticked when the by-name action fails', async () => {
+    const ds = makeDataSource();
+    const approve = vi.fn(async () => ({ success: false, error: 'nope' }));
+    renderGrid(ds, { approve });
+
+    await waitFor(() => expect(screen.getByText('Plan A')).toBeInTheDocument());
+    fireEvent.click(document.querySelector('thead [role="checkbox"]') as HTMLElement);
+    await waitFor(() => expect(headerChecked()).toBe('checked'));
+
+    fireEvent.click(await screen.findByTestId('bulk-action-approve'));
+    await waitFor(() => expect(approve).toHaveBeenCalledTimes(1));
+
+    // Failure keeps BOTH sources, so the user can retry the same rows.
+    await new Promise((r) => setTimeout(r, 50));
+    expect(screen.queryByTestId('bulk-actions-bar')).toBeInTheDocument();
+    expect(headerChecked()).toBe('checked');
+  });
+
+  it('clears the row checkboxes after a bulk delete', async () => {
+    const ds = makeDataSource();
+    const onBulkDelete = vi.fn();
+    const schema: any = {
+      type: 'object-grid',
+      objectName: OBJECT,
+      bulkActions: ['delete'],
+      columns: [{ field: 'name', label: 'Name' }],
+      pagination: { pageSize: 50 },
+    };
+    render(
+      <ActionProvider handlers={{}}>
+        <ObjectGrid schema={schema} dataSource={ds} onBulkDelete={onBulkDelete} />
+      </ActionProvider>,
+    );
+
+    await waitFor(() => expect(screen.getByText('Plan A')).toBeInTheDocument());
+    fireEvent.click(document.querySelector('thead [role="checkbox"]') as HTMLElement);
+    await waitFor(() => expect(headerChecked()).toBe('checked'));
+
+    fireEvent.click(await screen.findByTestId('bulk-action-delete'));
+    await waitFor(() => expect(onBulkDelete).toHaveBeenCalledTimes(1));
+
+    await waitFor(() =>
+      expect(screen.queryByTestId('bulk-actions-bar')).not.toBeInTheDocument(),
+    );
+    await waitFor(() => expect(headerChecked()).toBe('unchecked'));
+  });
+});