From 7ae2da655d08e42a7fafb9c906edb11dd5db6b10 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 00:59:30 +0000 Subject: [PATCH 1/2] fix(grid): withhold a row's Edit/Delete when the host refuses that record (objectui#8674) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ObjectGrid` takes one grid-level `onDelete` and derives `{ update: !!onEdit, delete: !!onDelete }`, so the row action was drawn for every row. A host whose refusal is per RECORD had nowhere to put it and put it in the callback instead: `FieldDesigner`'s delete handler returned early on a system field, after the button had been drawn and clicked — no dialog, no toast, no console message. `readOnly` was honest in the same component (the callback is withheld, so no button is drawn); `isSystem` was not. `ObjectGridComponentProps` gains `rowOperations?: (record) => { update?, delete? }` — the grid-level `operations` vocabulary, resolved for one row — ANDed into the same expression that already folds the #2614 predicates and the #4296 record-level verdict. An INTERSECTION like every layer around it: `false` withholds, and nothing it returns can re-open what the object's bucket, its `userActions`, the server's effective operations, the principal's grant or the record verdict closed. A caller that passes nothing renders what it rendered before. `FieldDesigner` is its first caller; the guard inside `handleDelete` stays for direct callers of the prop value but is no longer the only refusal. Pins are rendered-affordance assertions in both packages — the withheld entry read next to the surviving one on the same row and next to the neighbouring row that keeps both, plus a no-predicate control grid. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ --- .changeset/8674-objectgrid-row-operations.md | 39 ++++ content/docs/plugins/plugin-grid.mdx | 41 ++++ .../plugin-designer/src/FieldDesigner.tsx | 30 +++ ...ldDesigner.systemFieldDelete-8674.test.tsx | 151 ++++++++++++++ packages/plugin-grid/README.md | 41 ++++ packages/plugin-grid/src/ObjectGrid.tsx | 174 +++++++++++----- .../rowOperationsPerRow-8674.test.tsx | 191 ++++++++++++++++++ packages/plugin-grid/src/index.tsx | 2 +- 8 files changed, 613 insertions(+), 56 deletions(-) create mode 100644 .changeset/8674-objectgrid-row-operations.md create mode 100644 packages/plugin-designer/src/__tests__/FieldDesigner.systemFieldDelete-8674.test.tsx create mode 100644 packages/plugin-grid/src/__tests__/rowOperationsPerRow-8674.test.tsx diff --git a/.changeset/8674-objectgrid-row-operations.md b/.changeset/8674-objectgrid-row-operations.md new file mode 100644 index 0000000000..34ff64c436 --- /dev/null +++ b/.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 `` 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. diff --git a/content/docs/plugins/plugin-grid.mdx b/content/docs/plugins/plugin-grid.mdx index a8d13cd13f..d4a24bf5cf 100644 --- a/content/docs/plugins/plugin-grid.mdx +++ b/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 }) => ( + 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: diff --git a/packages/plugin-designer/src/FieldDesigner.tsx b/packages/plugin-designer/src/FieldDesigner.tsx index f857c29910..4ebbc23849 100644 --- a/packages/plugin-designer/src/FieldDesigner.tsx +++ b/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) => { + const field = fields.find((f) => f.name === record.name); + return { delete: !!field && !field.isSystem }; + }, [fields]); + const handleDelete = useCallback(async (record: Record) => { 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} /> diff --git a/packages/plugin-designer/src/__tests__/FieldDesigner.systemFieldDelete-8674.test.tsx b/packages/plugin-designer/src/__tests__/FieldDesigner.systemFieldDelete-8674.test.tsx new file mode 100644 index 0000000000..2db3d95330 --- /dev/null +++ b/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> = {}) { + return render( + + + , + ); +} + +/** 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 }); + }); +}); diff --git a/packages/plugin-grid/README.md b/packages/plugin-grid/README.md index a398a88324..3b2640c678 100644 --- a/packages/plugin-grid/README.md +++ b/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 }) => ( + 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: diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index a9bfc67d7e..9d9d3fceae 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -453,10 +453,58 @@ export interface ObjectGridExternalPaginationProps * The barrel keeps `ObjectGridProps` as a deprecated alias of this type, so no * importer breaks. Tripwire: `__tests__/spec-symbol-4650.test.ts`. */ +/** + * The subset of the grid-level `operations` block that a row may answer for + * ITSELF — the vocabulary {@link ObjectGridComponentProps.rowOperations} speaks. + * + * `update` / `delete` are the two the row kebab renders, and they are spelled + * exactly as the authored `operations` block spells them, so one word means one + * thing whether it is declared for the whole grid or resolved for one row. The + * block's other members (`create`, `export`) are deliberately absent: neither + * is a row affordance, so a per-row answer for them would have nowhere to land. + */ +export interface ObjectGridRowOperations { + update?: boolean; + delete?: boolean; +} + export interface ObjectGridComponentProps extends ObjectGridExternalPaginationProps { schema: ObjectGridSchema; dataSource?: DataSource; className?: string; + /** + * [objectui#8674] Narrow ONE row's generic Edit / Delete entries — the layer + * that lets a host withhold an operation the record itself cannot accept. + * + * ## Why this exists + * + * `operations` and the `onEdit` / `onDelete` wiring are GRID-level: they say + * whether the affordance exists at all, identically for every row. A host + * whose refusal is per RECORD had nowhere to put it, so it put the refusal in + * the callback instead — `FieldDesigner`'s delete handler returned early on a + * system field, after the grid had already drawn the button. The two states + * it was distinguishing (`readOnly`, which withholds the callback, and + * `isSystem`, which swallowed the click) differed in the code and did not + * differ on screen: the author clicked a button drawn as available and got no + * dialog, no toast, no console message. The operation an affordance cannot + * perform is not offered. + * + * ## The contract + * + * Called with a row record; returns the overrides for THAT row. It is an + * INTERSECTION, like every layer around it (the ADR-0103 bucket, the object's + * `userActions`, the server's effective API operations, the principal's own + * grant and the record-level explain verdict): `false` WITHHOLDS, and nothing + * it returns can re-open what those closed. `true`, an omitted member, a + * `null` / `undefined` return, and an absent prop all leave the verdict + * exactly as the grid resolved it — so a caller that passes nothing renders + * what it rendered before this prop existed. + * + * Scope is the row's kebab. Bulk delete rides `onBulkDelete` and the object + * verdict behind the selection bar, which no per-row answer can speak for: + * one selected row's `false` must not silently drop the other rows' action. + */ + rowOperations?: (record: any) => ObjectGridRowOperations | null | undefined; onRowClick?: (record: any) => void; onEdit?: (record: any) => void; onDelete?: (record: any) => void; @@ -1041,6 +1089,7 @@ export const ObjectGrid: React.FC = ({ dataSource, onEdit, onDelete, + rowOperations, onBulkDelete, onRowSelect, onRowClick, @@ -3425,61 +3474,76 @@ export const ObjectGrid: React.FC = ({ // extent and is hidden). Excluded from the frozen-column decision below so // this auto-pin doesn't cancel the default left-freeze of the first column. pinned: 'right', - cell: (_value: any, row: any) => ( -