From 392c6e4523343681ae175fbaecfad2a58a3b9591 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 17:28:24 +0000 Subject: [PATCH] fix(plugin-list): harvest row-action predicate fields from the object's userActions only The `$select` predicate harvest read `userActions` view-first, so a spec-legal view-level toolbar block shadowed the object's CRUD-predicate block and dropped its operands from the projection (objectui#3501's fail-closed CEL fault). On the app-shell path the left operand is always an object literal, so the object block was never reached at all. Sibling of objectui#5426, which fixed the same shape in plugin-grid; both read sites now carry the same comment and the same measured reason. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE --- .../list-user-actions-collision-5398.md | 38 +++ packages/plugin-list/src/ListView.tsx | 55 +++- .../ListView.userActionsCollision.test.tsx | 281 ++++++++++++++++++ 3 files changed, 373 insertions(+), 1 deletion(-) create mode 100644 .changeset/list-user-actions-collision-5398.md create mode 100644 packages/plugin-list/src/__tests__/ListView.userActionsCollision.test.tsx diff --git a/.changeset/list-user-actions-collision-5398.md b/.changeset/list-user-actions-collision-5398.md new file mode 100644 index 000000000..62a0d3c77 --- /dev/null +++ b/.changeset/list-user-actions-collision-5398.md @@ -0,0 +1,38 @@ +--- +'@object-ui/plugin-list': patch +--- + +`list-view` harvests row-action predicate fields from the OBJECT's `userActions` block only — a view's toolbar policy can no longer shadow it. + +`userActions` names two different blocks. On a **view** it is toolbar policy — +the spec's `UserActionsConfigSchema` (`sort`, `search`, `filter`, `refresh`, +`rowHeight`, `addRecordForm`, `editInline`, `buttons`), which rejects `edit` by +name. On an **object** it is the CRUD-predicate block (`edit` / `delete` / +`create` carrying `visibleWhen` / `disabledWhen`, objectui#2614) — and that is +the only shape `listViewPredicates` can read, since its loop skips every +non-object value. + +`ListView` read the key view-first when building the `$select` projection +(`(schema as any).userActions ?? (objectDef as any)?.userActions`). A view +carrying a perfectly legal toolbar block therefore shadowed the object's CRUD +predicates, the harvest found none, and the predicate's operand left the +projection. CEL then faults on the absent key, fails closed, and the row +Edit/Delete button disappears for everyone with nothing pointing at the +projection — objectui#3501's failure, reached with a success receipt at every +step. + +This is the sibling of the `plugin-grid` read site fixed in objectui#5426, and +it was the worse of the two: `app-shell`'s `ObjectView` builds the view-level +`userActions` it hands down as an object literal of two spreads, so the left +operand was `{}` at worst — never nullish. The `??` never fell through, and the +object's CRUD predicates were never consumed at all on that path, whether or +not an author wrote any toolbar policy. + +The harvest now reads the object block only. Both `userActions` read sites in +`ListView.tsx` carry a comment naming the collision, and +`__tests__/ListView.userActionsCollision.test.tsx` pins each clause of it: the +two shapes, a producer that manufactures the view one, the harvest's blindness +to it, and the projection that must keep the object's operand with a toolbar +block — or an empty block — present on the view. + +Toolbar policy itself is untouched — it was never read through this path. diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index 00a798729..ae343afda 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -763,6 +763,17 @@ export const ListView = React.forwardRef(({ // off unless turned on. (`hideFields`/`rowColor` default OFF is objectui's // historical behavior, kept deliberately — flipping it would grow two // buttons on every existing view.) + // + // This is the VIEW half of a NAME COLLISION, and the right half here: + // `userActions` on a VIEW is toolbar policy (`UserActionsConfigSchema`), + // while `userActions` on an OBJECT is the CRUD-predicate block + // (`edit`/`delete`/`create` with `visibleWhen`/`disabledWhen`). So this + // read stays `schema`-only and must never gain an + // `?? (objectDef as any)?.userActions` fallback: the object block carries + // no toolbar key, and the mirrored mistake — reading VIEW-first where only + // the object block is interpretable — is exactly what the `$select` + // predicate harvest below was fixed for (objectui#5398, the sibling of + // objectui#5240). Pinned by `__tests__/ListView.userActionsCollision.test.tsx`. const ua = schema.userActions as Record | undefined; const addRecordEnabled = schema.addRecord?.enabled === true && ua?.addRecordForm !== false; const addRecordPlacement = resolveAddRecordPlacement(schema.addRecord?.position); @@ -1588,7 +1599,49 @@ export const ListView = React.forwardRef(({ rowActionDefs: (schema as any).rowActionDefs, bulkActionDefs: (schema as any).bulkActionDefs, objectActions: (objectDef as any)?.actions, - userActions: (schema as any).userActions ?? (objectDef as any)?.userActions, + // KEY COLLISION — `userActions` names TWO different blocks, and + // only the OBJECT's is interpretable here. This read is therefore + // `objectDef`-only; it must never regain a + // `(schema as any).userActions ??` left operand. Maintainer ruling + // of 2026-08-20 on objectui#5240 (Q1=A), whose `plugin-grid` half + // landed as objectui#5426 — this is the sibling read site named in + // that ruling (objectui#5398), carrying the same shape and the same + // reason on purpose rather than a second spelling of one fix. The + // measurements, re-taken here against `@objectstack/spec@17.0.0`: + // + // - VIEW-level `userActions` is TOOLBAR POLICY — + // `UserActionsConfigSchema` (`sort`, `search`, `filter`, + // `refresh`, `rowHeight`, `addRecordForm`, `editInline`, + // `buttons`), which REJECTS `edit` BY NAME + // (`unrecognized_keys`). `ListViewSchema` accepts it, so it is + // spec-legal and really authored — it is the very block + // `toolbarFlags` and `inlineEditOffered` read above, and + // `normalizeListViewSchema` MANUFACTURES one from a legacy + // `show*` view that never wrote the key at all. + // - OBJECT-level `userActions` is the CRUD-PREDICATE block + // (`edit` / `delete` / `create` carrying `visibleWhen` / + // `disabledWhen`, objectui#2614) — what + // `resolveEffectiveCrudAffordances` / `isObjectInlineEditable` + // consume off `objectDef` above, and the only shape + // `listViewPredicates` can read: its loop skips every + // non-object value, so a toolbar block yields ZERO predicates. + // + // Read view-first, a legitimately authored toolbar block therefore + // SHADOWED the object's CRUD predicates and dropped their operands + // from `$select`; CEL then faults `No such key`, fails CLOSED, and + // the row Edit/Delete button vanishes for everyone with nothing + // pointing at the projection (objectui#3501 — the whole reason this + // harvest exists, stated six lines up). + // + // On THIS component the shadowing was TOTAL rather than occasional: + // `app-shell/src/views/ObjectView.tsx` builds the `userActions` it + // hands down as an object literal of two spreads, so the left + // operand was `{}` at worst — never nullish, so `??` never fell + // through and the object's CRUD block was never reached AT ALL on + // that path. + // + // Pinned by `__tests__/ListView.userActionsCollision.test.tsx`. + userActions: (objectDef as any)?.userActions, }))) addPredicateField(f); return Array.from(required); diff --git a/packages/plugin-list/src/__tests__/ListView.userActionsCollision.test.tsx b/packages/plugin-list/src/__tests__/ListView.userActionsCollision.test.tsx new file mode 100644 index 000000000..c3e87670b --- /dev/null +++ b/packages/plugin-list/src/__tests__/ListView.userActionsCollision.test.tsx @@ -0,0 +1,281 @@ +/** + * 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. + */ + +/** + * `userActions` is TWO blocks sharing one name — and only the OBJECT's is + * interpretable by the `$select` predicate harvest (objectui#5398). + * + * ## The collision + * + * - VIEW-level `userActions` is TOOLBAR POLICY — the spec's + * `UserActionsConfigSchema` (`sort`, `search`, `filter`, `refresh`, + * `rowHeight`, `addRecordForm`, `editInline`, `buttons`), which REJECTS + * `edit` BY NAME. `ListViewSchema` accepts it, so it is spec-legal, and it + * is really written: `ListView`'s own `toolbarFlags` and + * `inlineEditOffered` read it, `normalizeListViewSchema` manufactures one + * out of a legacy `show*` view that never wrote the key, and `app-shell`'s + * `ObjectView` builds one unconditionally for every list it renders. + * - OBJECT-level `userActions` is the CRUD-PREDICATE block (`edit` / + * `delete` / `create` carrying `visibleWhen` / `disabledWhen`, + * objectui#2614) — the only shape `listViewPredicates` can read, since its + * loop skips every non-object value. + * + * `ListView` used to read the key VIEW-FIRST when harvesting predicate fields + * for `$select` (`(schema as any).userActions ?? (objectDef as any)?.userActions`), + * so an authored toolbar block SHADOWED the object's CRUD predicates and + * silently dropped their operands from the projection — objectui#3501's + * fail-closed CEL fault (`No such key`), reached with a success receipt at + * every step. That read is now object-only. + * + * ## Why this file exists at all, given objectui#5240 fixed the same line + * + * This is the SIBLING read site of the one objectui#5426 fixed in + * `plugin-grid`. The maintainer's 2026-08-20 ruling (Q2=B) kept the two serial + * rather than batched for one reason: both read sites must end up with ONE + * shape and ONE stated reason, not two spellings of one fix. So the sections + * below are `gridNonAuthorKeys.test.tsx`'s deliberately, clause for clause. + * + * And this instance was WORSE than the grid's. On the app-shell path the left + * operand of the `??` is an object literal of two spreads, so it is `{}` at + * worst — never nullish. `??` therefore never fell through and the object's + * CRUD block was never reached AT ALL there, rather than being shadowed only + * when an author happened to write toolbar policy. The empty-block case below + * is that path. + * + * ## What the pin does NOT claim + * + * It does not assert "no producer writes this key" and it does not call the + * key non-author surface. Both readings were measured false on objectui#5240, + * and Q1=A exists precisely to keep the falsified claim out of the census + * artifact. What it asserts is what the comments in `ListView.tsx` actually + * claim: the two shapes, a producer inside this package's reach, the harvest's + * blindness to the toolbar block, and the projection behaviour that shadowing + * broke. + * + * plugin-grid is not a dependency of plugin-list (avoids a cycle), so — as in + * `ListView.findParamsHandoff.test.tsx` — a stub `object-grid` stands in for + * the child the default view renders. + */ +import { describe, it, expect, vi, beforeAll, afterAll, afterEach } from 'vitest'; +import { cleanup, render, waitFor } from '@testing-library/react'; +import React from 'react'; +import { + ComponentRegistry, + collectPredicateFieldRefs, + listViewPredicates, + normalizeListViewSchema, +} from '@object-ui/core'; +import { ListViewSchema as SpecListViewSchema, UserActionsConfigSchema } from '@objectstack/spec/ui'; +import { SchemaRendererProvider } from '@object-ui/react'; +import type { ListViewSchema } from '@object-ui/types'; + +import { ListView } from '../ListView'; + +const OBJECT = 'test_object'; + +/** What a VIEW author legitimately writes: toolbar policy. */ +const VIEW_TOOLBAR_BLOCK = { sort: true, search: true, filter: false }; +/** What an OBJECT declares under the same name: a CRUD predicate block. */ +const OBJECT_CRUD_BLOCK = { edit: { visibleWhen: 'record.status == "open"' } }; +/** The toolbar vocabulary the comments at both read sites list by name. */ +const TOOLBAR_KEYS = [ + 'sort', 'search', 'filter', 'refresh', 'rowHeight', 'addRecordForm', 'editInline', 'buttons', +]; + +/** Unrecognized KEYS from a failed parse — a key verdict, never a document one. */ +const unrecognizedKeys = (result: { success: boolean; error?: unknown }): string[] => + ((result as { error: { issues: Array<{ code: string; keys?: string[] }> } }).error?.issues ?? []) + .filter((issue) => issue.code === 'unrecognized_keys') + .flatMap((issue) => issue.keys ?? []); + +// --------------------------------------------------------------------------- +// The two shapes +// --------------------------------------------------------------------------- + +describe('`userActions` is two blocks sharing one name — the view one is toolbar policy (objectui#5398)', () => { + it.each(['edit', 'delete', 'create'])('the view-level schema refuses the CRUD key `%s` by name', (key) => { + const result = UserActionsConfigSchema.safeParse({ [key]: { visibleWhen: 'record.status == "open"' } }); + expect( + result.success, + `\`UserActionsConfigSchema\` now ACCEPTS \`${key}\` — the two \`userActions\` blocks no longer` + + ' collide, so re-read the collision comments in `ListView.tsx`: their premise was that a' + + ' CRUD predicate block can never legally arrive on a VIEW.', + ).toBe(false); + expect(unrecognizedKeys(result)).toContain(key); + }); + + it('…and accepts the toolbar vocabulary, so those refusals are about the KEY', () => { + // The control. Without it, "the view schema refuses `edit`" would pass just + // as happily against a schema that refuses everything. + expect(UserActionsConfigSchema.safeParse(VIEW_TOOLBAR_BLOCK).success).toBe(true); + expect( + Object.keys((UserActionsConfigSchema as unknown as { shape: Record }).shape), + 'the toolbar vocabulary changed — the comments at both read sites spell it out by name.', + ).toEqual(expect.arrayContaining(TOOLBAR_KEYS)); + }); +}); + +// --------------------------------------------------------------------------- +// The view-level key is AUTHORED +// --------------------------------------------------------------------------- + +describe('the view-level key is AUTHORED, not an unwritten surface (objectui#5398)', () => { + it('a view document carrying the toolbar block is spec-legal', () => { + // This is the measurement that killed the original "deliberately unlisted, + // zero producers" reading on objectui#5240. If it ever goes false, the + // collision comments describe a repo that no longer exists. + const result = SpecListViewSchema.safeParse({ + name: 'my_view', + label: 'My View', + columns: [{ field: 'name' }], + userActions: VIEW_TOOLBAR_BLOCK, + }); + expect(result.success, JSON.stringify((result as { error?: unknown }).error ?? {})).toBe(true); + }); + + it('…while the CRUD block is refused on a view, at the userActions path', () => { + const result = SpecListViewSchema.safeParse({ + name: 'my_view', + label: 'My View', + columns: [{ field: 'name' }], + userActions: OBJECT_CRUD_BLOCK, + }); + expect(result.success).toBe(false); + expect(unrecognizedKeys(result)).toContain('edit'); + }); + + it('a PRODUCER in this package’s own reach MANUFACTURES the view block', () => { + // The producer IS the evidence: `normalizeListViewSchema` is the fold + // `ListView` runs on its own schema and `app-shell` runs on both halves of + // the view it hands down. A legacy view that never wrote `userActions` + // comes out of it carrying one — so "nobody authors it" was never + // available as a reason for this read, even before counting the hand-built + // literals. + const folded = normalizeListViewSchema({ + type: 'list-view', + objectName: OBJECT, + columns: ['name'], + showSearch: false, + } as Record) as { userActions?: Record }; + expect( + folded.userActions, + 'the legacy `show*` fold stopped emitting `userActions`. If that is deliberate, the' + + ' collision comments in `ListView.tsx` name this producer and are due a re-read —' + + ' do not just delete this assertion.', + ).toEqual({ search: false }); + }); +}); + +// --------------------------------------------------------------------------- +// Only the OBJECT block is interpretable by the harvest +// --------------------------------------------------------------------------- + +describe('only the OBJECT block is interpretable by the predicate harvest (objectui#5398)', () => { + it('the toolbar block yields ZERO predicate fields', () => { + // Every value is a boolean, so `listViewPredicates` skips all of them. This + // is why shadowing was silent: not an error, just nothing harvested. + expect(collectPredicateFieldRefs(listViewPredicates({ userActions: VIEW_TOOLBAR_BLOCK }))).toEqual([]); + }); + + it('the object CRUD block yields its operand', () => { + expect(collectPredicateFieldRefs(listViewPredicates({ userActions: OBJECT_CRUD_BLOCK }))).toContain('status'); + }); +}); + +// --------------------------------------------------------------------------- +// The renderer behaviour shadowing broke +// --------------------------------------------------------------------------- + +let prevObjectGrid: unknown; +beforeAll(() => { + prevObjectGrid = ComponentRegistry.get('object-grid'); + ComponentRegistry.register('object-grid', () =>
); +}); +afterAll(() => { + if (prevObjectGrid) ComponentRegistry.register('object-grid', prevObjectGrid as never); + else ComponentRegistry.unregister('object-grid'); +}); +afterEach(() => { cleanup(); }); + +function makeDataSource() { + return { + find: vi.fn(async () => ({ data: [{ id: 'r-1', name: 'Alice' }], total: 1 })), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn(async (name: string) => ({ + name, + // `status` is deliberately NOT a column: a predicate-only field is the + // ordinary case the harvest exists for. + fields: { id: { type: 'text' }, name: { type: 'text' }, status: { type: 'select' } }, + userActions: OBJECT_CRUD_BLOCK, + })), + }; +} + +/** + * `$select` for a list whose OBJECT declares the CRUD predicate block, with + * whatever the view carries layered on top. + */ +const projectionSelect = async (viewExtra: Record): Promise => { + const ds = makeDataSource(); + render( + + + , + ); + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + return ((ds.find.mock.calls.at(-1) as unknown as [string, { $select?: string[] }])?.[1]?.$select + ?? []) as string[]; +}; + +describe('a view-level toolbar block must not shadow the object CRUD predicates (objectui#5398)', () => { + it('the object block reaches $select — the surviving read is still there', async () => { + // Pins the READ, not the fix: green on both legs of the shadowing change, + // red the moment the `userActions` line is dropped from the harvest. + expect( + await projectionSelect({}), + "the object's `userActions.edit.visibleWhen` operand is missing from `$select` — CEL faults" + + ' on the absent key and the row Edit button fails closed for everyone (objectui#3501).', + ).toContain('status'); + }); + + it('…and a spec-legal toolbar block on the view does not knock it out', async () => { + // The regression itself. Read view-first, the `??` took this block, the + // harvest found no predicates in it, and `status` left the projection — + // with a success receipt at every step. + expect( + await projectionSelect({ userActions: VIEW_TOOLBAR_BLOCK }), + 'a view-level toolbar block is shadowing the object CRUD predicates again: the projection' + + ' lost `status`. The read at the `$select` harvest must stay object-only.', + ).toContain('status'); + }); + + it('…nor does an EMPTY view-level block — the app-shell path, where the left operand is always an object', async () => { + // Why this instance was worse than plugin-grid's. `app-shell`'s + // `ObjectView` builds `userActions` as `{...listSchema, ...viewDef}`, which + // is `{}` when neither side declares a toolbar key — truthy, so the old + // `??` never fell through and the object's CRUD block was never reached AT + // ALL on that path, for every list the app shell renders. + expect( + await projectionSelect({ userActions: {} }), + 'an EMPTY view-level `userActions` is shadowing the object CRUD predicates: this is the' + + ' app-shell path, where the left operand of a restored `??` is an object literal that is' + + ' never nullish, so the object block would never be consumed at all.', + ).toContain('status'); + }); +});