diff --git a/.changeset/8730-bulk-action-defs-unusable-member.md b/.changeset/8730-bulk-action-defs-unusable-member.md new file mode 100644 index 0000000000..7eeb38faf8 --- /dev/null +++ b/.changeset/8730-bulk-action-defs-unusable-member.md @@ -0,0 +1,55 @@ +--- +'@object-ui/plugin-grid': patch +--- + +`object-grid`: a `bulkActionDefs` member that is not a usable def is skipped and +diagnosed, instead of taking the whole selection bar down (objectui#8730). + +`bulkActions` and `bulkActionDefs` are one affordance authored in two vocabularies — +`bulkActions` members are bare action NAMES resolved against `objectDef.actions`, +`bulkActionDefs` members are full `BulkActionDef` OBJECTS used as authored — and +nothing refused a member written in the other one. Both keys are registered +`type: 'array'` with no `of`, both spec rows are `z.array(z.unknown())`, and a JSON +view is invisible to `tsc`. + +Writing a bare name into `bulkActionDefs` did not fail quietly, it crashed: +`Array.isArray(schema.bulkActionDefs)` is true, the string travelled into the authored +list untouched, `BulkActionBar` rendered a button for it, and +`def.label ?? formatActionLabel(def.name)` threw +`TypeError: Cannot read properties of undefined (reading 'replace')` **during render**. +The author's first multi-row selection lost the entire selection bar — count, Clear and +every well-formed sibling def with it. `key={def.name}` was `undefined` too, so React +logged a duplicate-key warning on the way down. + +`resolveBulkActions` now skips any member that is not an object carrying a non-empty +string `name`. "Usable" is defined by what the renderer actually reads: `name` is both +the React `key` and `formatActionLabel`'s argument, so that one test covers the reported +bare string and, identically, `null`, a number, `{}` and `{ name: '' }`. The guard sits +at the single point where the authored array becomes the list the bar maps over, so the +`key` and the label are read off the same validated def. + +The skip is not silent. `ObjectGrid` reports it once per authored array through the +channel it already uses for "you declared it, the renderer dropped it" — one +`console.warn` prefixed `[ObjectUI] ObjectGrid bulkActionDefs:` — naming the block, the +index, what was seen, and what to write instead: + +``` +[ObjectUI] ObjectGrid bulkActionDefs: object-grid (objectName: 'os_invoice') — 1 of 3 +authored bulk-action defs cannot be rendered and is skipped (2 still render). + • bulkActionDefs[0]: the entry is a string ('approve'), not a def object — this key's + members are full `BulkActionDef` objects, used as authored. Write + `{ name: 'approve', operation: 'custom' }` here, or move the bare name to + `bulkActions`, which resolves it against the object's declared actions and promotes + the match. +``` + +**Not a coercion, deliberately.** A bare `'approve'` is not lifted into +`{ name: 'approve' }` and resolved the way `bulkActions` is. That would make the two +vocabularies interchangeable — a product change to what a `bulkActionDefs` member means +(objectui#3002 / objectui#3139 made them distinct on purpose), not a crash fix. + +**Behaviour changes for authors** beyond the crash: a `{ name: '' }` member used to +render a nameless, unlabelled button with an empty React key, and now renders nothing. +Well-formed defs are untouched — a mixed list renders exactly its usable members, in +order, and a clean array is still returned by reference. The mirror direction +(`bulkActions: [{ name: 'approve' }]`) keeps its existing silent skip. diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 323886ff5d..95d8fbbab5 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -47,7 +47,7 @@ import { resolveRowCrudAffordances, resolveRowRecordCrudAffordance } from './row import { useRecordCrudVerdicts } from './hooks/useRecordCrudVerdicts'; import { resolveLegacyRowActions } from './resolveLegacyRowActions'; import { applyRelationalMeta } from './relationalMetaKeys'; -import { resolveBulkActions } from './resolveBulkActions'; +import { resolveBulkActions, describeUnusableBulkActionDefs } from './resolveBulkActions'; import { partitionBulkRows } from './bulkEligibility'; import { resolvesToDataColumn, describeUnresolvedColumns } from './columnSpellingDiagnostics'; import { RowActionMenu, formatActionLabel } from './components/RowActionMenu'; @@ -2226,6 +2226,30 @@ export const ObjectGrid: React.FC = ({ if (message) console.warn(message); }, [schema.columns, columnDiagnosticBlockType, schema.objectName, columnDiagnosticLabel]); + // [objectui#8730] The same channel, for the sibling failure on + // `bulkActionDefs`. A member that is not a usable def (a bare action name — + // the OTHER key's vocabulary — or `null`, a number, `{}`, `{ name: '' }`) is + // skipped by `resolveBulkActions`; before that guard it reached the bar and + // `formatActionLabel(undefined)` threw during render, so the author's first + // multi-row selection lost the whole selection bar. + // + // The skip alone would only relocate the failure into silence — which is what + // the mirror direction already does (`bulkActions: [{ name: 'approve' }]`, + // stepped over by the `typeof name !== 'string'` guard). Saying which member + // was dropped, and that a bare name belongs in `bulkActions`, is what makes + // this a diagnosis rather than a quieter version of the same defect. One + // `console.warn` per authored array, keyed on it — NOT a second guard: the + // predicate lives once, in `resolveBulkActions`, and this reads it. + const bulkDefsDiagnosticSlice = (schema as { bulkActionDefs?: unknown }).bulkActionDefs; + useEffect(() => { + const message = describeUnusableBulkActionDefs(bulkDefsDiagnosticSlice, { + blockType: columnDiagnosticBlockType, + objectName: schema.objectName, + label: columnDiagnosticLabel, + }); + if (message) console.warn(message); + }, [bulkDefsDiagnosticSlice, columnDiagnosticBlockType, schema.objectName, columnDiagnosticLabel]); + const generateColumns = useCallback((): ObjectGridColumnDraft[] => { // Map field type to column header icon (Airtable-style) const getTypeIcon = (fieldType: string | null): React.ReactNode => { diff --git a/packages/plugin-grid/src/__tests__/bulkActionDefsUnusableMember-8730.test.tsx b/packages/plugin-grid/src/__tests__/bulkActionDefsUnusableMember-8730.test.tsx new file mode 100644 index 0000000000..0ea434e14f --- /dev/null +++ b/packages/plugin-grid/src/__tests__/bulkActionDefsUnusableMember-8730.test.tsx @@ -0,0 +1,322 @@ +/** + * 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. + */ + +/** + * `object-grid.bulkActionDefs` — a member that is not a usable def is SKIPPED + * and DIAGNOSED, never rendered (objectui#8730). + * + * ## What was wrong + * + * `bulkActions` and `bulkActionDefs` are one affordance authored in two + * vocabularies: `bulkActions` members are BARE ACTION NAMES resolved against + * `objectDef.actions`; `bulkActionDefs` members are FULL `BulkActionDef` + * OBJECTS used as authored. Nothing refuses a member written in the other + * vocabulary — both keys are registered `type: 'array'` with no `of`, and both + * spec rows are `z.array(z.unknown())` — so the read site is the whole member + * contract. + * + * Writing a bare name into `bulkActionDefs` did not fail quietly, it CRASHED: + * `Array.isArray(schema.bulkActionDefs)` is true, the string travelled into the + * authored list untouched, `BulkActionBar` rendered a `BulkActionButton` for + * it, and `def.label ?? formatActionLabel(def.name)` threw + * `TypeError: Cannot read properties of undefined (reading 'replace')` DURING + * RENDER — the author's first multi-row selection lost the entire selection + * bar. `key={def.name}` was `undefined` too, so React logged a duplicate-key + * warning on the way down. + * + * ## What is pinned here + * + * "Usable" is defined by what the RENDERER READS, not by a fresh opinion: + * `BulkActionBar` uses `def.name` both as the React `key` and as + * `formatActionLabel`'s argument, so a member must be an object carrying a + * non-empty string `name`. One test covers the whole unusable class — the + * reported bare string, `null`, a number, `{}` and `{ name: '' }`. + * + * ⭐ The load-bearing half is the NON-REGRESSION axis, because "skip the bad + * member" is also satisfied by an implementation that skips EVERYTHING. So the + * class rows below are paired with rows that fail under exactly that caricature: + * + * - reddens under "skip everything": rows 6 (a mixed list renders exactly the + * two good defs, in order, with their labels) and 7 (a clean list is + * untouched — same buttons, and the resolved array is the authored array BY + * REFERENCE). + * - reddens under "skip nothing": rows 1-5 (the class) and row 8's fire leg. + * + * The diagnostic is an assertion too, so it is pinned as a PAIR (row 8): it + * must fire for the bad member and NAME it, and it must NOT fire for a clean + * list. A warning that fires always is as useless as one that never fires. + * + * ## What row 9 inherits + * + * objectui#8071's member pin asserted this crash as the CURRENT SHAPE, so that + * it could not be mistaken for the SILENT drop of the mirror direction + * (`bulkActions: [{ name: 'approve' }]`, stepped over by `resolveBulkActions`'s + * `typeof name !== 'string'` guard). That distinction is not deleted by this + * fix, it is sharpened, and row 9 carries it: both directions now SKIP, but only + * the def direction is diagnosed. Direction one's silence is asserted here as + * well — it is deliberately unchanged (objectui#8730's ruling puts it out of + * scope), and this row is what would notice a diagnostic leaking into it. + * + * `selection` is declared explicitly on every row so the ONLY variable between + * them is the member shape. Left implicit, the grid auto-enables multi-select + * from `hasBulkActions`, which is itself derived from these two keys — so a + * row whose members are all skipped would lose its selection UI for the very + * reason under test and pass without ever reaching the bar. + */ + +import { describe, it, expect, vi, beforeAll, afterEach } from 'vitest'; +import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { ObjectGrid } from '../ObjectGrid'; +import { resolveBulkActions } from '../resolveBulkActions'; +import { registerAllFields } from '@object-ui/fields'; +import { ActionProvider } from '@object-ui/react'; +import type { BulkActionDef } from '@object-ui/types'; + +registerAllFields(); + +beforeAll(() => { + if (!Element.prototype.scrollIntoView) { + Element.prototype.scrollIntoView = vi.fn() as any; + } +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +const OBJECT = 'os_invoice'; + +/** + * The object declares ONE action, `approve`, whose label is NOT the humanized + * form of its name. That gap is the instrument for row 9: a button reading + * "Approve the invoice" can only have come from resolving a member as a NAME. + */ +const OBJECT_ACTIONS = [{ name: 'approve', label: 'Approve the invoice', variant: 'primary' }]; + +/** Two well-formed defs, used wherever a row needs survivors to count. */ +const ARCHIVE: BulkActionDef = { name: 'archive', operation: 'custom', label: 'Put it away' }; +const EXPORT: BulkActionDef = { name: 'export_pdf', operation: 'custom', label: 'Export PDF' }; + +/** + * The whole unusable class, each entry paired with the substring the + * diagnostic must use to address it. `{ name: '' }` is in here for a reason + * worth stating: it was the ONE member of the class that did not throw before + * the fix — it rendered a nameless, unlabelled button with an empty React key. + * Skipping is therefore a change in behaviour for it too, not only for the + * four that crashed. + */ +const UNUSABLE: ReadonlyArray = [ + ['a bare action name (the reported shape)', 'approve'], + ['null', null], + ['a number', 42], + ['an empty object', {}], + ['an empty-string name', { name: '' }], +]; + +function makeDataSource() { + const rows = [ + { id: 'r1', name: 'INV-1', status: 'draft' }, + { id: 'r2', name: 'INV-2', status: 'draft' }, + ]; + return { + find: vi.fn(async () => ({ + data: rows.map((r) => ({ ...r })), + total: rows.length, + hasMore: false, + pageSize: 50, + })), + getObjectSchema: async (name: string) => ({ + name, + fields: { id: { type: 'text' }, name: { type: 'text' }, status: { type: 'text' } }, + actions: OBJECT_ACTIONS, + }), + } as any; +} + +async function renderAndSelectAll(schema: Record) { + render( + + + , + ); + await waitFor(() => expect(screen.getByText('INV-1')).toBeInTheDocument()); + const headerCheckbox = document.querySelector('thead [role="checkbox"]') as HTMLElement; + expect(headerCheckbox, 'the multi-select header checkbox').toBeTruthy(); + fireEvent.click(headerCheckbox); + await waitFor(() => expect(screen.getByTestId('bulk-actions-bar')).toBeInTheDocument()); +} + +/** + * Every bulk-action button in the bar, in DOM order, by `data-testid`. + * + * COUNTING is the point, not "something rendered": a census that navigates with + * `querySelector` never notices a node it did not expect (measured on + * objectui#8596, where `.rounded-full` matched two nodes per avatar). The + * prefix ends in a hyphen so the bar's own `bulk-actions-bar` testid cannot + * match, and the query is scoped to the bar so a second bar would be visible as + * a `getByTestId` failure rather than as doubled counts. + */ +function renderedBulkActionIds(): string[] { + const bar = screen.getByTestId('bulk-actions-bar'); + return Array.from(bar.querySelectorAll('[data-testid^="bulk-action-"]')).map( + (n) => n.getAttribute('data-testid') as string, + ); +} + +/** Just enough of a spy to read its call log, without importing vitest's generics. */ +type ConsoleSpy = { mock: { calls: unknown[][] } }; + +/** The diagnostic channel: `console.warn` lines this key owns. */ +function bulkDefWarnings(warn: ConsoleSpy): string[] { + return warn.mock.calls + .map((args: unknown[]) => String(args[0])) + .filter((line: string) => line.includes('bulkActionDefs')); +} + +/** React's duplicate-key complaint — the second symptom of an undefined `name`. */ +function duplicateKeyErrors(error: ConsoleSpy): string[] { + return error.mock.calls + .map((args: unknown[]) => args.map((a: unknown) => String(a)).join(' ')) + .filter((line: string) => line.includes('unique "key" prop')); +} + +describe('object-grid `bulkActionDefs`: an unusable member is skipped (objectui#8730)', () => { + UNUSABLE.forEach(([label, member], i) => { + it(`${i + 1}. ${label} renders no button, and does not take the bar down`, async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + + // The pre-fix failure was a THROW during render, so reaching this line at + // all is half the assertion; the count is the other half. + await renderAndSelectAll({ bulkActionDefs: [member] }); + + expect(renderedBulkActionIds()).toEqual([]); + // The bar itself survives — that is the affordance the crash destroyed. + expect(screen.getByTestId('bulk-actions-bar')).toBeInTheDocument(); + expect(duplicateKeyErrors(error)).toEqual([]); + // Skipping is never silent on this key (row 8 pins the message itself). + expect(bulkDefWarnings(warn)).toHaveLength(1); + }); + }); + + it('6. a mixed list renders EXACTLY the well-formed defs, in order', async () => { + // ⭐ The discriminating row. "Skip the bad member" is also satisfied by an + // implementation that skips every member, and by one that drops the + // survivors' order or identity. Only an exact, ordered census refuses all + // three at once. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + + await renderAndSelectAll({ bulkActionDefs: ['approve', ARCHIVE, EXPORT] }); + + expect(renderedBulkActionIds()).toEqual(['bulk-action-archive', 'bulk-action-export_pdf']); + expect(screen.getByTestId('bulk-action-archive')).toHaveTextContent('Put it away'); + expect(screen.getByTestId('bulk-action-export_pdf')).toHaveTextContent('Export PDF'); + // The `key` half of "the key and the label read the same validated def": + // the keys are the two surviving names, so React has nothing to complain + // about. An `undefined` key is what the reported defect produced first. + expect(duplicateKeyErrors(error)).toEqual([]); + // One skip, reported once, naming the member that was skipped. + expect(bulkDefWarnings(warn)).toHaveLength(1); + expect(bulkDefWarnings(warn)[0]).toContain('bulkActionDefs[0]'); + }); + + it('7. a list of only well-formed defs is untouched', async () => { + // ⭐ The other half of the discriminating pair, at both levels. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + await renderAndSelectAll({ bulkActionDefs: [ARCHIVE, EXPORT] }); + expect(renderedBulkActionIds()).toEqual(['bulk-action-archive', 'bulk-action-export_pdf']); + expect(bulkDefWarnings(warn)).toEqual([]); + + // Unit level, and the sharper assertion of the two: a clean authored array + // comes back BY REFERENCE. An always-allocating filter would pass the DOM + // census above and fail here — and a view whose defs lose referential + // identity every render is a real cost, not a stylistic one (the array is a + // `useMemo`/`useEffect` dependency downstream). + const authored = [ARCHIVE, EXPORT]; + const { defs } = resolveBulkActions({ bulkActionDefs: authored }); + expect(defs).toBe(authored); + }); + + it('8. the diagnostic fires for the bad member, by name — and not for a clean list', async () => { + // An assertion that a warning EXISTS is worth little on its own: a warning + // that fires always is as useless as one that never fires. Both legs, in + // one row, so the pair cannot drift apart. + const fired = vi.spyOn(console, 'warn').mockImplementation(() => {}); + await renderAndSelectAll({ bulkActionDefs: ['approve'] }); + + const lines = bulkDefWarnings(fired); + expect(lines).toHaveLength(1); + // WHERE: the addressed block, and the index inside the authored array. + expect(lines[0]).toContain("objectName: 'os_invoice'"); + expect(lines[0]).toContain('bulkActionDefs[0]'); + // WHAT: the member itself, quoted — this is the "by name" half. + expect(lines[0]).toContain("'approve'"); + // WHAT TO DO: the other vocabulary is named, since that is where a bare + // action name belongs. + expect(lines[0]).toContain('bulkActions'); + fired.mockRestore(); + + // No-fire leg, on a fresh render of a clean list. + document.body.innerHTML = ''; + const quiet = vi.spyOn(console, 'warn').mockImplementation(() => {}); + await renderAndSelectAll({ bulkActionDefs: [ARCHIVE] }); + expect(bulkDefWarnings(quiet)).toEqual([]); + }); +}); + +describe('the two vocabularies still fail DIFFERENTLY (inherited from objectui#8071)', () => { + it('9. direction one stays silent; direction two skips and says so', async () => { + // What objectui#8071's row 5 was protecting: direction two is NOT direction + // one's silent drop. Before this fix the difference was crash-vs-silence; + // it is now diagnostic-vs-silence. Both are skips, and neither throws. + // + // Direction one — the DEF vocabulary written into `bulkActions`. Stepped + // over by `resolveBulkActions`'s `typeof name !== 'string'` guard, with no + // diagnostic. Deliberately unchanged (objectui#8730 scopes it out); this + // leg is what would notice a diagnostic leaking into it. + const one = vi.spyOn(console, 'warn').mockImplementation(() => {}); + await renderAndSelectAll({ bulkActions: [{ name: 'approve' }] }); + expect(renderedBulkActionIds()).toEqual([]); + expect(one).not.toHaveBeenCalled(); + one.mockRestore(); + + // Direction two — the NAME vocabulary written into `bulkActionDefs`. Same + // disposition, opposite treatment: skipped, and reported. + document.body.innerHTML = ''; + const two = vi.spyOn(console, 'warn').mockImplementation(() => {}); + await renderAndSelectAll({ bulkActionDefs: ['approve'] }); + expect(renderedBulkActionIds()).toEqual([]); + expect(bulkDefWarnings(two)).toHaveLength(1); + }); + + it('10. a name in the RIGHT key still resolves and promotes', async () => { + // The guard must not be reachable from the vocabulary that is supposed to + // carry bare names. The object action's OWN label proves the promotion + // happened rather than a humanized fallback. + await renderAndSelectAll({ bulkActions: ['approve'] }); + expect(renderedBulkActionIds()).toEqual(['bulk-action-approve']); + expect(screen.getByTestId('bulk-action-approve')).toHaveTextContent('Approve the invoice'); + }); +}); diff --git a/packages/plugin-grid/src/resolveBulkActions.ts b/packages/plugin-grid/src/resolveBulkActions.ts index ba993c40bc..c094482097 100644 --- a/packages/plugin-grid/src/resolveBulkActions.ts +++ b/packages/plugin-grid/src/resolveBulkActions.ts @@ -153,6 +153,133 @@ function toBulkActionDef(action: NamedActionDef, localize?: ActionLabelResolver) }; } +/** + * Is this authored `bulkActionDefs` member a def the selection bar can render? + * + * "Usable" is defined by WHAT THE RENDERER READS, not by a fresh opinion about + * what a def ought to carry: `BulkActionBar` uses `def.name` twice — as the + * React `key` of the button it maps to, and as the argument to + * `formatActionLabel` when the def declares no `label`. A member without a + * non-empty string `name` therefore has no identity and no label, and + * `formatActionLabel(undefined)` threw `TypeError: Cannot read properties of + * undefined (reading 'replace')` DURING RENDER — taking the whole selection bar + * down on the author's first multi-row selection (objectui#8730). + * + * Every other `BulkActionDef` key is optional at the read site (an absent + * `operation` falls through to the button's default treatment, an absent + * `visible` means ungated), so `name` is the whole predicate. It covers the + * reported bare string and, by the same test, `null`, a number, `{}` and + * `{ name: '' }`. + * + * ⛔ NOT a coercion. A bare `'approve'` is NOT lifted into `{ name: 'approve' }` + * and resolved the way `bulkActions` is: that would make the two vocabularies + * interchangeable, which is a product change to what a `bulkActionDefs` member + * MEANS (objectui#3002 / objectui#3139 established them as two vocabularies + * deliberately), not a crash fix. The member is skipped, and said out loud. + */ +export function isUsableBulkActionDef(member: unknown): member is BulkActionDef { + if (!member || typeof member !== 'object') return false; + const name = (member as { name?: unknown }).name; + return typeof name === 'string' && name.length > 0; +} + +/** Where the offending block lives, for the first line of the message. */ +export interface BulkActionDefsAddress { + /** The schema node's `type` — `object-grid`, or the `view:grid` alias. */ + blockType?: unknown; + /** The object the grid queries, when it names one. */ + objectName?: unknown; + /** The grid's authored label, when it has one — often the only human name. */ + label?: unknown; +} + +function quote(value: unknown): string { + return typeof value === 'string' ? `'${value}'` : String(value); +} + +function describeAddress({ blockType, objectName, label }: BulkActionDefsAddress): string { + const block = typeof blockType === 'string' && blockType.length > 0 ? blockType : 'object-grid'; + const parts: string[] = []; + if (typeof objectName === 'string' && objectName.length > 0) parts.push(`objectName: '${objectName}'`); + else parts.push('no objectName'); + if (typeof label === 'string' && label.length > 0) parts.push(`label: '${label}'`); + return `${block} (${parts.join(', ')})`; +} + +/** + * Describe ONE skipped member: what it spells, and what to write instead. + * + * Each branch reports only what it checked. The string arm carries the extra + * sentence because a bare action name is not nonsense — it is the OTHER key's + * vocabulary, and naming that key is the whole fix for the author. + */ +function describeMember(member: unknown): string { + if (member === null || member === undefined) { + return `the entry is ${String(member)}, not a def object`; + } + if (typeof member !== 'object') { + const rewrite = typeof member === 'string' && member.length > 0 + ? ` Write \`{ name: '${member}', operation: 'custom' }\` here, or move the bare name to ` + + '`bulkActions`, which resolves it against the object\'s declared actions and promotes the match.' + : ''; + return `the entry is a ${typeof member} (${quote(member)}), not a def object — this key's ` + + `members are full \`BulkActionDef\` objects, used as authored.${rewrite}`; + } + const entry = member as Record; + const keys = Object.keys(entry); + const seen = keys.length > 0 + ? `keys seen: ${keys.map((k) => `\`${k}\``).join(', ')}` + : 'the entry is empty `{}`'; + if (!('name' in entry)) { + return `${seen} — no \`name\` key, so the def has no identity.`; + } + const name = entry.name; + if (typeof name !== 'string') { + return `${seen} — \`name\` is a ${name === null ? 'null' : typeof name} (${quote(name)}), ` + + 'and only a non-empty string names an action.'; + } + return `${seen} — \`name\` is an empty string, so the button would carry no key and no label.`; +} + +/** + * The message for a `bulkActionDefs` array carrying members the bar cannot + * render, or `null` when every authored member is usable. + * + * Naming the ADDRESS is the whole point: which block, which object, which + * index, what was seen, and what to write instead. A message that only said + * something went wrong would leave the author exactly where the crash did. + * + * The channel is `ObjectGrid`'s existing one for "you declared it, the renderer + * dropped it" — a `useEffect` keyed on the schema slice and one `console.warn` + * prefixed `[ObjectUI] ObjectGrid :`, the same shape as the columns + * diagnostic and the export-format warning, rather than a third differently + * shaped one beside them. + */ +export function describeUnusableBulkActionDefs( + members: unknown, + address: BulkActionDefsAddress, +): string | null { + if (!Array.isArray(members) || members.length === 0) return null; + const skipped = members + .map((member, index) => ({ index, member })) + .filter(({ member }) => !isUsableBulkActionDef(member)); + if (skipped.length === 0) return null; + + const survivors = members.length - skipped.length; + const subject = `${skipped.length} of ${members.length} authored bulk-action ` + + `def${members.length === 1 ? '' : 's'} cannot be rendered and ` + + `${skipped.length === 1 ? 'is' : 'are'} skipped`; + const headline = survivors === 0 + ? `${subject}, so this selection bar offers NO bulk-action buttons` + : `${subject} (${survivors} still render)`; + const lines = skipped.map(({ index, member }) => ` • bulkActionDefs[${index}]: ${describeMember(member)}`); + + return `[ObjectUI] ObjectGrid bulkActionDefs: ${describeAddress(address)} — ${headline}.\n` + + `${lines.join('\n')}\n` + + ' A `bulkActionDefs` member must be an object with a non-empty string `name`: the selection ' + + 'bar uses it as the button\'s React key AND as the source of its label.'; +} + export function resolveBulkActions(opts: { /** * The view's `bulkActions` names, already stripped of the canonical @@ -180,7 +307,22 @@ export function resolveBulkActions(opts: { /** Names that matched no declared action; dispatched by name. */ unresolved: string[]; } { - const rawAuthored = Array.isArray(opts.bulkActionDefs) ? opts.bulkActionDefs : []; + // [objectui#8730] THE GUARD SITE. This is the one place where the authored + // `bulkActionDefs` array becomes the list the selection bar maps over, so the + // React `key` and the label are read off the SAME validated def. A member the + // bar cannot render is dropped here rather than refused, coerced, or defended + // against again downstream — `BulkActionBar` keeps reading `def.name` + // unconditionally, because by this line it can. + // + // Referential identity is preserved when nothing is dropped (`every` before + // `filter`): the `defs` contract below promises the authored array BY + // REFERENCE when nothing folds in, and an always-allocating filter would + // break that for every clean view — see `resolveBulkActions.test.ts`'s + // "returns the authored array by reference when nothing folds in". + const authoredMembers = Array.isArray(opts.bulkActionDefs) ? opts.bulkActionDefs : []; + const rawAuthored = authoredMembers.every(isUsableBulkActionDef) + ? authoredMembers + : authoredMembers.filter(isUsableBulkActionDef); const names = Array.isArray(opts.bulkActions) ? opts.bulkActions : []; const objectActions = Array.isArray(opts.objectActions) ? opts.objectActions : [];