fix(grid): withhold a row's Edit/Delete when the host refuses that record (objectui#8674)

.changeset/8674-objectgrid-row-operations.md

@@ -0,0 +1,39 @@
+---
+'@object-ui/plugin-grid': minor
+'@object-ui/plugin-designer': patch
+---
+
+Per-row operation gating for `ObjectGrid`, and the Field Designer stops drawing a
+delete action it refuses to run (objectui#8674).
+
+**The defect.** `FieldDesigner`'s `handleDelete` returned early on `field.isSystem` —
+before the confirm dialog — while the affordance was wired at the GRID level
+(`onDelete={readOnly ? undefined : handleDelete}`). `ObjectGrid` takes one grid-level
+`onDelete` and derives `{ update: !!onEdit, delete: !!onDelete }`, so the row action
+was drawn for every row: clicking delete on a system field produced no dialog, no
+toast and no console message. `readOnly` was honest in the same component (the
+callback is withheld, so no button is drawn); `isSystem` drew the button and dropped
+the click. The two states differed in the code and did not differ on screen.
+
+**`@object-ui/plugin-grid` — new, additive, opt-in.** `ObjectGridComponentProps`
+gains `rowOperations?: (record) => { update?: boolean; delete?: boolean }`, with the
+new `ObjectGridRowOperations` type exported from the package root. It speaks the same
+`update` / `delete` vocabulary the authored `operations` block speaks, resolved for
+one row instead of for the grid, and it is an INTERSECTION like every layer around it
+(the ADR-0103 lifecycle bucket, the object's `userActions`, the server's effective API
+operations, the principal's own grant, the record-level explain verdict): `false`
+withholds, and nothing it returns can re-open what those closed. A caller that passes
+no predicate renders exactly what it rendered before — measured, not asserted: the
+rendered DOM of a no-predicate grid is byte-identical across this change, and every
+other `<ObjectGrid>` call site in the repository is such a caller.
+
+It is a function value, so no metadata document can hold it and none is invited to:
+like the nine `on*` callbacks it sits beside, it is a renderer prop and not an
+authorable key.
+
+**`@object-ui/plugin-designer` — the first caller, and the user-visible fix.** The
+Field Designer passes the predicate, so a system field's row no longer offers Delete
+at all. Edit is untouched: the drawer still opens for a system field, with `name` and
+`type` disabled exactly as before — `isSystem` has never meant "this row is
+untouchable". The guard inside `handleDelete` stays as a second line for direct
+callers of the prop value, but it is no longer the only refusal.

content/docs/plugins/plugin-grid.mdx

@@ -485,6 +485,47 @@ The declarative alternative, which *is* metadata and survives a round trip
 through storage, is `navigation`: its `mode` decides what a row click does
 without any host code.
 
+### Withholding a row's Edit or Delete
+
+`operations` and the `onEdit` / `onDelete` wiring are GRID-level: they decide
+whether the generic Edit / Delete entries exist at all, identically for every
+row. When the refusal belongs to one RECORD — a system field a designer may not
+drop — use `rowOperations`, a component prop called with a row record that
+answers for that row alone:
+
+```tsx
+import { ObjectGrid } from '@object-ui/plugin-grid';
+import type { ObjectGridSchema } from '@object-ui/types';
+
+const schema: ObjectGridSchema = {
+  type: 'object-grid',
+  objectName: 'field_definition',
+  columns: ['name', 'label', 'type'],
+};
+
+export const FieldList = ({ isSystem }: { isSystem: (name: string) => boolean }) => (
+  <ObjectGrid
+    schema={schema}
+    onEdit={(record) => console.log('edit', record)}
+    onDelete={(record) => console.log('delete', record)}
+    rowOperations={(record) => ({ delete: !isSystem(String(record.name)) })}
+  />
+);
+```
+
+It speaks the same `update` / `delete` vocabulary as the authored `operations`
+block, and it is an **intersection**, never a union: `false` withholds the
+entry, while `true`, an omitted member, and a `null` / `undefined` return all
+leave the grid's own verdict alone. Nothing it returns can re-open what the
+object's lifecycle bucket, its `userActions`, the server's effective operations,
+the principal's grant or the record-level verdict already closed — and a grid
+that passes no `rowOperations` renders exactly as it did before the prop
+existed.
+
+Prefer this over refusing inside the callback. A refusal that runs after the
+click ships a button that is drawn as available and then does nothing, which is
+indistinguishable from a broken build (objectui#8674).
+
 ### Inline Editing
 
 Enable inline cell editing for quick data updates:

packages/plugin-designer/src/FieldDesigner.tsx

@@ -183,8 +183,37 @@ export function FieldDesigner({
     }
   }, [fields]);
 
+  /**
+   * [objectui#8674] Which generic row operations THIS row may be offered.
+   *
+   * The delete refusal used to live only inside {@link handleDelete}, which
+   * runs after the click: `ObjectGrid` takes ONE grid-level `onDelete` and drew
+   * the row action for every row, so a system field showed a delete button that
+   * answered a click with nothing at all — no dialog, no toast, no console
+   * message. `readOnly` was already honest (the callback is withheld, so no
+   * button is drawn); `isSystem` was not. This is the same refusal, moved to
+   * where it can withhold the affordance instead of swallowing its click.
+   *
+   * `isSystem` is the spelling the drawer form already uses to disable `name`
+   * and `type` on a system field — the one concept, "this field's structure is
+   * not the designer's to change", not a second one.
+   *
+   * An unresolvable row is withheld too, for the same reason and not as a
+   * defensive flourish: `handleDelete` returns early when the lookup misses, so
+   * offering delete for such a row would reproduce the exact defect this card
+   * is about.
+   */
+  const rowOperations = useCallback((record: Record<string, unknown>) => {
+    const field = fields.find((f) => f.name === record.name);
+    return { delete: !!field && !field.isSystem };
+  }, [fields]);
+
   const handleDelete = useCallback(async (record: Record<string, unknown>) => {
     const field = fields.find((f) => f.name === record.name);
+    // Unreachable through the grid now that `rowOperations` withholds the
+    // action for exactly these two cases — kept because this callback is a
+    // published prop value and nothing stops a future caller invoking it
+    // directly. It must never become the ONLY refusal again.
     if (!field || field.isSystem) return;
     const confirmed = await confirmDialog.confirm(
       t('appDesigner.fieldDesigner.deleteConfirmTitle'),
@@ -416,6 +445,7 @@ export function FieldDesigner({
         dataSource={dataSource}
         onEdit={readOnly ? undefined : handleEdit}
         onDelete={readOnly ? undefined : handleDelete}
+        rowOperations={rowOperations}
         onAddRecord={readOnly ? undefined : handleAddField}
       />
 

packages/plugin-designer/src/__tests__/FieldDesigner.systemFieldDelete-8674.test.tsx

@@ -0,0 +1,151 @@
+/**
+ * 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.
+ */
+
+/**
+ * objectui#8674 — the Field Designer offers no delete action on a system field.
+ *
+ * ## The card
+ *
+ * `handleDelete` returned early on `field.isSystem`, before the confirm dialog,
+ * while the affordance was wired at the GRID level
+ * (`onDelete={readOnly ? undefined : handleDelete}`). `ObjectGrid` derives
+ * `{ update: !!onEdit, delete: !!onDelete }` for the whole grid, so the row
+ * action was drawn for EVERY row: clicking delete on a system field produced no
+ * dialog, no toast and no console message. `readOnly` was already honest in the
+ * same component — the operation is withheld, so no button is drawn — and the
+ * card's sentence for why that difference is the defect is that the two states
+ * "differ in the code and do not differ on screen".
+ *
+ * ## Why this file renders the REAL `ObjectGrid`
+ *
+ * Its siblings here mock `@object-ui/plugin-grid` (`__mocks__/plugin-grid`),
+ * and that mock draws a delete button for every row whenever `onDelete` is
+ * wired — i.e. it reproduces the defect by construction and cannot see the fix.
+ * The claim under test is about what the GRID draws for a row, so the grid
+ * under test has to be the real one; a stub grid would make this file green
+ * against the defect and against the fix alike.
+ *
+ * ## Every absence is paired with a presence
+ *
+ * The system row's missing Delete is read next to its surviving Edit (the row
+ * still has a menu, and the designer still opens the drawer for system fields —
+ * where `name` and `type` are disabled and everything else is editable), and
+ * next to the ordinary row that keeps both. A lone `queryBy… === null` would be
+ * green if the grid never rendered or the menu never opened.
+ */
+
+import React from 'react';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { render, screen, waitFor, cleanup } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import '@testing-library/jest-dom';
+import type { DesignerFieldDefinition } from '@object-ui/types';
+import { ActionProvider } from '@object-ui/react';
+
+import { FieldDesigner } from '../FieldDesigner';
+// The REAL record-level explain double and cache reset the grid suites use —
+// imported rather than restated, so this file answers that probe the same way
+// `plugin-grid`'s own suites do instead of hitting the network under happy-dom.
+import { installExplainDouble } from '../../../plugin-grid/src/__tests__/explainDouble';
+import { __clearRecordCrudVerdictCache } from '../../../plugin-grid/src/hooks/useRecordCrudVerdicts';
+
+// No `registerAllFields()`: `@object-ui/fields` is not a dependency of this
+// package, and nothing here needs a field WIDGET — the assertions read row text
+// and the row kebab's entries, both of which the grid renders on its own.
+
+/** An ordinary, author-created field — the presence half of every pair. */
+const CUSTOM: DesignerFieldDefinition = {
+  id: 'fld_custom',
+  name: 'nickname',
+  label: 'Nickname',
+  type: 'text',
+  isSystem: false,
+};
+
+/** A system field: the designer may not drop it, and now does not offer to. */
+const SYSTEM: DesignerFieldDefinition = {
+  id: 'fld_system',
+  name: 'created_at',
+  label: 'Created At',
+  type: 'datetime',
+  isSystem: true,
+};
+
+const FIELDS = [CUSTOM, SYSTEM];
+
+function renderDesigner(props: Partial<React.ComponentProps<typeof FieldDesigner>> = {}) {
+  return render(
+    <ActionProvider>
+      <FieldDesigner objectName="contacts" fields={FIELDS} onFieldsChange={vi.fn()} {...props} />
+    </ActionProvider>,
+  );
+}
+
+/** What the kebab of the row displaying `label` surfaces. Located by ROW. */
+async function rowKebab(label: string): Promise<{ edit: boolean; delete: boolean; trigger: boolean }> {
+  const row = screen.getByText(label).closest('tr');
+  const trigger = row?.querySelector('[data-testid="row-action-trigger"]');
+  if (!trigger) return { edit: false, delete: false, trigger: false };
+  await userEvent.click(trigger);
+  const answer = {
+    edit: screen.queryAllByTestId('row-action-builtin-edit').length > 0,
+    delete: screen.queryAllByTestId('row-action-builtin-delete').length > 0,
+    trigger: true,
+  };
+  await userEvent.keyboard('{Escape}');
+  return answer;
+}
+
+async function settle() {
+  await waitFor(() => expect(screen.getByText(SYSTEM.name)).toBeInTheDocument());
+}
+
+beforeEach(() => {
+  __clearRecordCrudVerdictCache();
+  installExplainDouble();
+});
+afterEach(() => {
+  cleanup();
+  vi.unstubAllGlobals();
+});
+
+describe('objectui#8674 · the system field draws no delete action', () => {
+  it('withholds Delete on the system row and keeps it on the custom row', async () => {
+    // BOTH ARMS in one render. Pre-fix this read `delete: true` on BOTH rows
+    // and the system row's click did nothing at all.
+    renderDesigner();
+    await settle();
+
+    expect(await rowKebab(SYSTEM.name)).toEqual({ edit: true, delete: false, trigger: true });
+    expect(await rowKebab(CUSTOM.name)).toEqual({ edit: true, delete: true, trigger: true });
+  });
+
+  it('keeps the system row editable — the drawer still opens, it is only delete that is refused', async () => {
+    // The narrowing is `delete` only. `isSystem` disables `name` and `type`
+    // inside the drawer form; it has never meant "this row is untouchable", and
+    // a fix that withheld Edit too would be a capability regression wearing a
+    // bug fix's clothes.
+    renderDesigner();
+    await settle();
+
+    const row = screen.getByText(SYSTEM.name).closest('tr');
+    await userEvent.click(row!.querySelector('[data-testid="row-action-trigger"]')!);
+    await userEvent.click(screen.getAllByTestId('row-action-builtin-edit')[0]);
+
+    await waitFor(() => expect(screen.getByDisplayValue(SYSTEM.label)).toBeInTheDocument());
+  });
+
+  it('readOnly stays honest: no row actions at all, for either kind of field', async () => {
+    // The state the card called the honest one, unchanged by this fix.
+    renderDesigner({ readOnly: true });
+    await settle();
+
+    expect(await rowKebab(SYSTEM.name)).toEqual({ edit: false, delete: false, trigger: false });
+    expect(await rowKebab(CUSTOM.name)).toEqual({ edit: false, delete: false, trigger: false });
+  });
+});

packages/plugin-grid/README.md

@@ -587,6 +587,47 @@ The declarative alternative, which *is* metadata and survives a round trip throu
 storage, is `navigation`: `{ mode: 'page' | 'drawer' | 'modal' | 'split' | 'none' }`
 decides what a row click does without any host code.
 
+### Withholding a row's Edit or Delete
+
+`operations` and the `onEdit` / `onDelete` wiring are GRID-level: they decide
+whether the generic Edit / Delete entries exist at all, identically for every
+row. When the refusal belongs to one RECORD — a system field a designer may not
+drop — use `rowOperations`, a component prop called with a row record that
+answers for that row alone:
+
+```tsx
+import { ObjectGrid } from '@object-ui/plugin-grid';
+import type { ObjectGridSchema } from '@object-ui/types';
+
+const schema: ObjectGridSchema = {
+  type: 'object-grid',
+  objectName: 'field_definition',
+  columns: ['name', 'label', 'type'],
+};
+
+export const FieldList = ({ isSystem }: { isSystem: (name: string) => boolean }) => (
+  <ObjectGrid
+    schema={schema}
+    onEdit={(record) => console.log('edit', record)}
+    onDelete={(record) => console.log('delete', record)}
+    rowOperations={(record) => ({ delete: !isSystem(String(record.name)) })}
+  />
+);
+```
+
+It speaks the same `update` / `delete` vocabulary as the authored `operations`
+block, and it is an **intersection**, never a union: `false` withholds the
+entry, while `true`, an omitted member, and a `null` / `undefined` return all
+leave the grid's own verdict alone. Nothing it returns can re-open what the
+object's lifecycle bucket, its `userActions`, the server's effective operations,
+the principal's grant or the record-level verdict already closed — and a grid
+that passes no `rowOperations` renders exactly as it did before the prop
+existed.
+
+Prefer this over refusing inside the callback. A refusal that runs after the
+click ships a button that is drawn as available and then does nothing, which is
+indistinguishable from a broken build (objectui#8674).
+
 ### Inline Editing
 
 Enable inline cell editing for quick updates: