diff --git a/.changeset/30169-undeclared-bulk-action-name-draws-nothing.md b/.changeset/30169-undeclared-bulk-action-name-draws-nothing.md new file mode 100644 index 00000000000..0c834bb3c33 --- /dev/null +++ b/.changeset/30169-undeclared-bulk-action-name-draws-nothing.md @@ -0,0 +1,19 @@ +--- +'@object-ui/plugin-grid': minor +--- + +fix(plugin-grid)!: a list's selection bar and row menu draw only actions the object declares; an undeclared bare `bulkActions` / `rowActions` name draws nothing and warns once in development (#30169) + +Clause-②: no (narrowing) — the grid stops drawing a bare `bulkActions` or legacy `rowActions` name that matches no action the object declares, which it drew before by humanized name. `packages/spec/src/ui/bulk-action.zod.ts` already says the member "names an action the object declares", and lint `action-name-undefined` (ADR-0049) refuses any other at authoring. Nothing is widened. +Metadata-protocol: narrowed +Skills-docs: no move — `skills/**` teaches no handler-only bulk or row action name (searched for `bulkActions` / `rowActions` examples and runner-handler text); `content/docs/ui/**` already names only declared actions in its `bulkActions` / `rowActions` examples. + + + +**BREAKING** for a host that registered a runner handler under a bare name the object does not declare (`ActionProvider handlers` / `runner.registerHandler('NAME', …)`) and listed that name in a view's `bulkActions` or `rowActions`. + +- **Before.** `resolveBulkActions` and `resolveLegacyRowActions` kept a name that matched no declared action, and `ObjectGrid` drew it by humanized name and dispatched it to the runner by name. A name nothing handled toasted "not executable" on click. This is how a list whose app had a package disabled kept a ghost button: the server withholds that package's actions from the served object, so their names matched nothing. +- **Now.** Such a name gets no selection-bar button and no row-menu entry, and in development the console logs one warning per list naming the key, the block and each name. The resolvers return those names as `undeclared` (renamed from `unresolved`), used only for that warning. The canonical `'delete'` (and the row menu's `'edit'` / `'delete'`) keep their own paths, unchanged. +- **Unchanged.** A name that matches a declared action is promoted to that action's def exactly as before, under the action's own label, icon, params, `visible` and `requiredPermissions`. + +**Remedy.** No rewrite is derived, because which fix an undeclared name needs is a judgement about the view: declare the named action on the object, or remove the name from `bulkActions` / `rowActions`. The development warning names each one. An action withheld by a disabled package needs nothing: it draws again when the package is enabled. diff --git a/ui/packages/plugin-grid/README.md b/ui/packages/plugin-grid/README.md index a4163bd4eae..e468d26dab6 100644 --- a/ui/packages/plugin-grid/README.md +++ b/ui/packages/plugin-grid/README.md @@ -549,7 +549,8 @@ const schema: ObjectGridBlockNode = { objectName: 'users', columns: ['name', 'email'], selection: { type: 'multiple' }, - bulkActions: ['delete', 'export'], + // `deactivate` is an action the `users` object declares. + bulkActions: ['delete', 'deactivate'], } satisfies ObjectGridProps, }; ``` @@ -757,6 +758,13 @@ const schema: ObjectGridBlockNode = { themselves live in the object's action set, so the same action behaves identically wherever it is offered. They are `string[]`, not inline definitions with callbacks. +Each name must name an action the object declares; `'edit'` and `'delete'` are the +built-in exceptions. A name the object does not declare draws nothing: no row-menu +entry and no selection-bar button, and in development the console logs one warning +naming it. That includes a misspelling and an action a disabled package withholds, +which the server leaves out of the object. A runner handler registered under a bare +name is not a declaration and does not bring the entry back. + ```typescript import type { ObjectGridBlockNode } from '@object-ui/types'; import type { ObjectGridProps } from '@objectstack/spec/ui'; @@ -766,9 +774,10 @@ const schema: ObjectGridBlockNode = { properties: { objectName: 'users', columns: ['name', 'email'], - rowActions: ['view', 'edit', 'delete'], + // `deactivate` is an action the `users` object declares. + rowActions: ['deactivate', 'edit', 'delete'], selection: { type: 'multiple' }, - bulkActions: ['delete', 'export'], + bulkActions: ['delete', 'deactivate'], } satisfies ObjectGridProps, }; ``` diff --git a/ui/packages/plugin-grid/demo/bulk-actions.tsx b/ui/packages/plugin-grid/demo/bulk-actions.tsx index 53b88015334..e48c917b8dc 100644 --- a/ui/packages/plugin-grid/demo/bulk-actions.tsx +++ b/ui/packages/plugin-grid/demo/bulk-actions.tsx @@ -16,8 +16,11 @@ * action (spec 17 retired `action.bulkEnabled` as a tombstone). Both * names used to dispatch as `{ type: '' }` and never run; the * buttons also carried none of the def's label / icon / params. - * 2. `legacy_only_handler` resolves to no declared action and stays a - * by-name dispatch — it must still render ALONGSIDE the two defs above. + * 2. `not_declared` names no action the object declares, so the bar draws + * NO button for it (the spec: a bare name "names an action the object + * declares"), and the browser console logs one warning naming it. An + * action a disabled package withholds reads the same way: it leaves the + * served object, so its name draws nothing (objectstack#30169). * 3. Record `t3` fails server-side (422), proving per-record attribution: the * result panel reports 4/5 with an error row rather than a blanket success. * 4. Rich `bulkActionDefs` with picker params (#3064 / ADR-0059): `Set @@ -237,8 +240,9 @@ const schema: any = { { field: 'progress', label: 'Progress' }, ], pagination: { pageSize: 50 }, - // Bare names: two resolve to declared actions, one doesn't. - bulkActions: ['bulk_mark_done', 'bulk_recalc_estimate', 'legacy_only_handler'], + // Bare names: two resolve to declared actions; `not_declared` names nothing + // the object declares, so it draws no button and warns once in the console. + bulkActions: ['bulk_mark_done', 'bulk_recalc_estimate', 'not_declared'], // Rich defs with picker params — the shared-field-widget surface (#3064). bulkActionDefs: [ // Aggregate mode (#3139): names the declared action, ONE dispatch for the @@ -285,8 +289,9 @@ function Demo() { Select rows → the bar shows Mark Done and Recalculate Estimate (names from the view's bulkActions, promoted to their declared object actions), Export Zip (an execution: 'aggregate' def — - ONE dispatch carrying every selected id, #3139) and Legacy Only - Handler (unresolvable, dispatched by name). + ONE dispatch carrying every selected id, #3139). The view also names{' '} + not_declared, which the object does not declare: no button, and one + warning in the browser console.

@@ -324,16 +329,7 @@ function Demo() { createRoot(document.getElementById('root')!).render( - {/* A handler registered under a bare NAME — the compat path that legacy - `bulkActions` entries must keep working for. */} - { - pushLog({ url: '(runner handler)', recordId: 'all-selected', params: '{}', status: 200 }); - return { success: true }; - }, - }} - > + , diff --git a/ui/packages/plugin-grid/src/ObjectGrid.tsx b/ui/packages/plugin-grid/src/ObjectGrid.tsx index 2545cc4a07c..bb609bebb5e 100644 --- a/ui/packages/plugin-grid/src/ObjectGrid.tsx +++ b/ui/packages/plugin-grid/src/ObjectGrid.tsx @@ -73,6 +73,7 @@ import { resolveRowCrudAffordances, resolveRowRecordCrudAffordance } from './row import { useRecordCrudVerdicts } from './hooks/useRecordCrudVerdicts'; import { resolveLegacyRowActions } from './resolveLegacyRowActions'; import { resolveBulkActions, resolvePromotedBulkParams } from './resolveBulkActions'; +import { useUndeclaredActionNamesWarning } from './undeclaredActionNames'; import { partitionBulkRows } from './bulkEligibility'; import { RowActionMenu, formatActionLabel } from './components/RowActionMenu'; import { BulkActionBar } from './components/BulkActionBar'; @@ -701,6 +702,10 @@ export const ObjectGrid: React.FC = ({ // getter, which reads the live cache at call time. const metadata = useMetadata(); const [objectSchema, setObjectSchema] = useState(null); + // The inline-data schema read failed (the non-inline load says so through + // `error`). Read only by the undeclared-name warning's readiness below, so a + // failed read reports the names instead of dropping them silently. + const [objectSchemaReadFailed, setObjectSchemaReadFailed] = useState(false); const [useCardView, setUseCardView] = useState(false); // objectstack#28434 — the record URL a row links to, for its context menu's // "Open in new tab": the host's own builder, the one `LinkCell` draws the @@ -997,9 +1002,10 @@ export const ObjectGrid: React.FC = ({ // Legacy `rowActions` carry a bare action NAME, which the runner cannot // execute on its own — resolve each against the object's declared actions so // it dispatches as a real def (and so a name that duplicates an existing - // `list_item` def stops rendering a dead twin of it). See + // `list_item` def stops rendering a dead twin of it). A name the object does + // not declare draws nothing and is reported below, in development. See // `resolveLegacyRowActions` for the full rationale (objectui#2960). - const { defs: resolvedRowActionDefs, unresolved: customRowActions } = resolveLegacyRowActions({ + const { defs: resolvedRowActionDefs, undeclared: undeclaredRowActions } = resolveLegacyRowActions({ rowActions: rowActionsList.filter(a => a !== 'edit' && a !== 'delete'), rowActionDefs: rowActionDefsList, objectActions: (objectSchema as any)?.actions, @@ -1129,6 +1135,7 @@ export const ObjectGrid: React.FC = ({ // mask, handled as masked), because it could be a `password` / // `secret` field — fail closed (objectui#10657, `objectTypesPending`). console.warn(`[ObjectGrid] Failed to fetch objectSchema for inline data (objectName: ${objectName}):`, err); + if (!cancelled) setObjectSchemaReadFailed(true); } }; @@ -1250,14 +1257,16 @@ export const ObjectGrid: React.FC = ({ : declaredBulkActions?.filter((a: unknown) => String(a).toLowerCase() !== 'delete'); // `bulkActions` carries a bare action NAME, which the runner cannot execute // on its own — resolve each against `objectDef.actions` so it dispatches as a - // real def. See `resolveBulkActions` (objectui#3002). The + // real def. See `resolveBulkActions` (objectui#3002). A name the object does + // not declare gets no button (spec: a bare name "names an action the object + // declares") and is reported below, in development. The // canonical `'delete'` is held back: it routes to `onBulkDelete` (which owns // confirm + refresh), not the runner, even if the object declares an action // that happens to be named `delete`. const legacyBulkNames = (explicitBulkActions ?? []).filter( (a: unknown) => String(a).toLowerCase() !== 'delete', ); - const { defs: resolvedBulkDefs, unresolved: unresolvedBulkActions } = resolveBulkActions({ + const { defs: resolvedBulkDefs, undeclared: undeclaredBulkActions } = resolveBulkActions({ bulkActions: legacyBulkNames, bulkActionDefs: Array.isArray(schema.bulkActionDefs) ? schema.bulkActionDefs : [], objectActions: (objectSchema as any)?.actions, @@ -1277,18 +1286,45 @@ export const ObjectGrid: React.FC = ({ (objectCanDelete || def?.operation !== 'delete') && (rowEditVerdict.allowed || def?.operation !== 'update'), ); - // Names still dispatched by string: the unresolved ones, plus `'delete'` - // when the author asked for it — or the implicit bulk-delete affordance. + // The one name still dispatched by string: `'delete'` when the author asked + // for it — or the implicit bulk-delete affordance. An undeclared name is not + // among them: it draws nothing. const keptDeleteAction = (explicitBulkActions ?? []).filter( (a: unknown) => String(a).toLowerCase() === 'delete', ); const effectiveBulkActions: string[] = explicitBulkActions && explicitBulkActions.length > 0 - ? [...unresolvedBulkActions, ...keptDeleteAction] + ? keptDeleteAction : canDelete && onBulkDelete && bulkActionDefs.length === 0 ? ['delete'] : []; const hasBulkActions = effectiveBulkActions.length > 0 || bulkActionDefs.length > 0; + // The names the two folds above did not draw, said once in development, and + // only once there is nothing left to wait for. Keyed on the RESOLVED + // `objectName` (a grid may name its object only through + // `data: { provider: 'object', object }`), and mirroring `objectTypesPending`: + // a schema is awaited only when there is an object to ask about and a data + // source that can answer `getObjectSchema`, and a read that failed (inline: + // `objectSchemaReadFailed`; the row load: `error`) is not awaited either. + // Reported with no schema in hand, the names are said to be unread, never + // undeclared. + const objectSchemaAwaited = + !!objectName && typeof dataSource?.getObjectSchema === 'function' && objectSchema == null + && !objectSchemaReadFailed && error == null; + const undeclaredNamesBasis = objectName && objectSchema == null ? 'unread' : 'declared'; + const undeclaredNamesAddress = { + blockType: (schema as { type?: unknown }).type, + objectName, + label: schema.label ?? (schema as { title?: unknown }).title, + }; + useUndeclaredActionNamesWarning({ + key: 'bulkActions', names: undeclaredBulkActions, ready: !objectSchemaAwaited, + basis: undeclaredNamesBasis, address: undeclaredNamesAddress, + }); + useUndeclaredActionNamesWarning({ + key: 'rowActions', names: undeclaredRowActions, ready: !objectSchemaAwaited, + basis: undeclaredNamesBasis, address: undeclaredNamesAddress, + }); let selectionMode: 'none' | 'single' | 'multiple' | boolean = false; if (schema.selection !== undefined) { // "presence enables; an explicit off wins" — the ONE rule both @@ -1676,7 +1712,7 @@ export const ObjectGrid: React.FC = ({ columnsWithActions, hasActions, hasPinnedColumns, hasUserPinnedColumns, pinnedLeftCols, pinnedRightCols, unpinnedCols, rightPinnedClasses, rightPinnedCellClasses, userLeftPinnedCount, } = resolveObjectGridActionsColumn({ - schema, data, objectSchema, persistedColumns, operations, customRowActions, resolvedRowActionDefs, + schema, data, objectSchema, persistedColumns, operations, resolvedRowActionDefs, wantEditAction, wantDeleteAction, mayInvokeRowAction, rowOperations, canEdit, canDelete, recordVerdict, predicateScope, onEdit, onDelete, editPredicates, deletePredicates, executeAction, setRefreshKey, t, diff --git a/ui/packages/plugin-grid/src/__tests__/actionsColumnAndChips-12708.test.tsx b/ui/packages/plugin-grid/src/__tests__/actionsColumnAndChips-12708.test.tsx index a429ac3c791..7a7d80ff1e5 100644 --- a/ui/packages/plugin-grid/src/__tests__/actionsColumnAndChips-12708.test.tsx +++ b/ui/packages/plugin-grid/src/__tests__/actionsColumnAndChips-12708.test.tsx @@ -45,6 +45,8 @@ function renderGrid(opts: { schema?: Record; objectFields?: Record; wired?: boolean; + /** Serves the object's schema (the rows stay inline), e.g. to declare actions. */ + dataSource?: Record; } = {}) { const schema: any = { type: 'object-grid', @@ -59,6 +61,7 @@ function renderGrid(opts: { @@ -122,9 +125,21 @@ describe('objectui#12708 · CONTROL — a list whose rows offer actions keeps it }); it('draws it for a legacy row action that is not Edit or Delete, with nothing else wired', async () => { - renderGrid({ wired: false, schema: { rowActions: ['archive'] } }); + // `archive` is declared on the object: a bare `rowActions` name draws only + // when it names an action the object declares (objectstack#30169). + renderGrid({ + wired: false, + schema: { rowActions: ['archive'] }, + dataSource: { + getObjectSchema: async (name: string) => ({ + name, + fields: { name: { type: 'text' }, status: { type: 'text' } }, + actions: [{ name: 'archive', label: 'Archive' }], + }), + }, + }); await settle(); - expect(actionsHeader()).not.toBeNull(); + await waitFor(() => expect(actionsHeader()).not.toBeNull()); }); }); diff --git a/ui/packages/plugin-grid/src/__tests__/bulkActionMembers-8071.test.tsx b/ui/packages/plugin-grid/src/__tests__/bulkActionMembers-8071.test.tsx index 421176a0021..c9832f9e1d4 100644 --- a/ui/packages/plugin-grid/src/__tests__/bulkActionMembers-8071.test.tsx +++ b/ui/packages/plugin-grid/src/__tests__/bulkActionMembers-8071.test.tsx @@ -76,12 +76,13 @@ * pass without ever reaching the bar. */ -import { describe, it, expect, vi, beforeAll } from 'vitest'; +import { describe, it, expect, vi, beforeAll, beforeEach } 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 { resetUndeclaredActionNamesWarnings } from '../undeclaredActionNames'; import { registerAllFields } from '@object-ui/fields'; import { ActionProvider } from '@object-ui/react'; @@ -177,6 +178,9 @@ function bulkDefWarnings(warn: ConsoleSpy): string[] { .filter((line: string) => line.includes('bulkActionDefs')); } +// The warning is said once per message per module; each row starts unwarned. +beforeEach(() => resetUndeclaredActionNamesWarnings()); + describe('object-grid `bulkActions` members are BARE ACTION NAMES (objectui#8071)', () => { it('1. a name member is resolved against `objectDef.actions` and promoted', async () => { await renderAndSelectAll({ bulkActions: ['approve'] }); @@ -199,16 +203,30 @@ describe('object-grid `bulkActions` members are BARE ACTION NAMES (objectui#8071 } }); - it('3. a name matching no declared action still reaches the bar, by name', async () => { - // Not a hole: `resolveBulkActions` keeps unresolved names so a consumer's - // `runner.registerHandler('', …)` stays reachable. Pinned because it - // is the one case where the member is read as a name AND stays one — which - // is what makes row 1's promotion an observable event rather than the only - // path a string could take. - await renderAndSelectAll({ bulkActions: ['no_such_action'] }); - expect(await screen.findByTestId('bulk-action-no_such_action')).toHaveTextContent( - 'No Such Action', - ); + it('3. a name matching no declared action draws NOTHING, and says so once (objectstack#30169)', async () => { + // Trigger objectstack#30169: an action a disabled package withholds leaves + // the served object, so its bare name matched nothing — and this row used + // to pin that the bar still drew it by humanized name ("No Such Action"), + // a button whose click toasted "not executable". The spec says the member + // "names an action the object declares"; one that does not draws nothing. + // The declared sibling keeps the bar up, so the absence is observed on a + // rendered bar, not on a missing one. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + await renderAndSelectAll({ bulkActions: ['approve', 'no_such_action'] }); + await screen.findByTestId('bulk-action-approve'); + expect(renderedBulkActionIds()).toEqual(['bulk-action-approve']); + expect(screen.queryByText('No Such Action')).not.toBeInTheDocument(); + // The drop is not silent: one development warning, naming the member. + const named = () => + warn.mock.calls + .map((args: unknown[]) => String(args[0])) + .filter((line: string) => line.includes('ObjectGrid bulkActions')); + await waitFor(() => expect(named()).toHaveLength(1)); + expect(named()[0]).toContain("'no_such_action'"); + } finally { + warn.mockRestore(); + } }); }); diff --git a/ui/packages/plugin-grid/src/__tests__/bulkActionRefresh.test.tsx b/ui/packages/plugin-grid/src/__tests__/bulkActionRefresh.test.tsx index 11139cef065..fd64883f3d9 100644 --- a/ui/packages/plugin-grid/src/__tests__/bulkActionRefresh.test.tsx +++ b/ui/packages/plugin-grid/src/__tests__/bulkActionRefresh.test.tsx @@ -75,8 +75,12 @@ function renderGrid(dataSource: any, handlers: Record) { const schema: any = { type: 'object-grid', objectName: OBJECT, - // String bulk action — the path 下推 / 派工 travels. - batchActions: ['approve'], + // String bulk action. Since objectstack#30169 a bare name the object does + // not declare draws nothing, and a declared one is promoted to a def and + // runs through the dialog — so the one name still dispatched by string is + // the canonical `'delete'` with no `onBulkDelete` wired, which reaches the + // runner by name (the `delete` handler each test registers). + batchActions: ['delete'], columns: [ { field: 'name', label: 'Name' }, { field: 'status', label: 'Status' }, @@ -84,7 +88,7 @@ function renderGrid(dataSource: any, handlers: Record) { pagination: { pageSize: 50 }, }; return render( - + , ); @@ -110,7 +114,7 @@ describe('ObjectGrid — string bulk action refreshes the list on success', () = expect(headerCheckbox).toBeTruthy(); fireEvent.click(headerCheckbox); - const approveBtn = await screen.findByTestId('bulk-action-approve'); + const approveBtn = await screen.findByTestId('bulk-action-delete'); fireEvent.click(approveBtn); // The handler ran against the selected records. @@ -140,7 +144,7 @@ describe('ObjectGrid — string bulk action refreshes the list on success', () = const headerCheckbox = document.querySelector('thead [role="checkbox"]') as HTMLElement; fireEvent.click(headerCheckbox); - const approveBtn = await screen.findByTestId('bulk-action-approve'); + const approveBtn = await screen.findByTestId('bulk-action-delete'); fireEvent.click(approveBtn); await waitFor(() => expect(approve).toHaveBeenCalledTimes(1)); @@ -301,8 +305,10 @@ describe('ObjectGrid — bulk dialog Done keeps selection in sync', () => { * * 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'`. + * reaches `dispatchBulkAction` is the canonical `'delete'`: through + * `onBulkDelete` when it is wired, and to the runner by name when it is not + * (the `delete` handler `renderGrid` registers). A bare name the object does + * not declare no longer reaches it at all (objectstack#30169). */ describe('ObjectGrid — dispatchBulkAction keeps selection in sync (#3056)', () => { const headerChecked = () => @@ -317,7 +323,7 @@ describe('ObjectGrid — dispatchBulkAction keeps selection in sync (#3056)', () fireEvent.click(document.querySelector('thead [role="checkbox"]') as HTMLElement); await waitFor(() => expect(headerChecked()).toBe('checked')); - fireEvent.click(await screen.findByTestId('bulk-action-approve')); + fireEvent.click(await screen.findByTestId('bulk-action-delete')); await waitFor(() => expect(approve).toHaveBeenCalledTimes(1)); await waitFor(() => @@ -335,7 +341,7 @@ describe('ObjectGrid — dispatchBulkAction keeps selection in sync (#3056)', () fireEvent.click(document.querySelector('thead [role="checkbox"]') as HTMLElement); await waitFor(() => expect(headerChecked()).toBe('checked')); - fireEvent.click(await screen.findByTestId('bulk-action-approve')); + fireEvent.click(await screen.findByTestId('bulk-action-delete')); await waitFor(() => expect(approve).toHaveBeenCalledTimes(1)); // Failure keeps BOTH sources, so the user can retry the same rows. diff --git a/ui/packages/plugin-grid/src/__tests__/bulkActionsSpecKey.test.tsx b/ui/packages/plugin-grid/src/__tests__/bulkActionsSpecKey.test.tsx index ef2baf94465..084b4a5ea2a 100644 --- a/ui/packages/plugin-grid/src/__tests__/bulkActionsSpecKey.test.tsx +++ b/ui/packages/plugin-grid/src/__tests__/bulkActionsSpecKey.test.tsx @@ -69,6 +69,12 @@ function makeDataSource() { name: { type: 'text' }, status: { type: 'text' }, }, + // Declared, because a bare bulk-action name draws only when the object + // declares the action it names (objectstack#30169). + actions: [ + { name: 'approve', label: 'Approve' }, + { name: 'reject', label: 'Reject' }, + ], }), } as any; } diff --git a/ui/packages/plugin-grid/src/__tests__/gridPagerSelectionMembers-8071.test.tsx b/ui/packages/plugin-grid/src/__tests__/gridPagerSelectionMembers-8071.test.tsx index 29adeb2365b..b6163bd4d5e 100644 --- a/ui/packages/plugin-grid/src/__tests__/gridPagerSelectionMembers-8071.test.tsx +++ b/ui/packages/plugin-grid/src/__tests__/gridPagerSelectionMembers-8071.test.tsx @@ -88,7 +88,20 @@ const ROWS = Array.from({ length: 12 }, (_, i) => ({ status: 'open', })); -function renderGrid(opts: Record) { +/** + * A data source that serves only the object's schema, with `approve` declared: + * a bare `bulkActions` name auto-enables selection only once it resolves to an + * action the object declares (objectstack#30169). The rows stay inline. + */ +const DECLARES_APPROVE: any = { + getObjectSchema: async (name: string) => ({ + name, + fields: { id: { type: 'text' }, name: { type: 'text' }, status: { type: 'text' } }, + actions: [{ name: 'approve', label: 'Approve' }], + }), +}; + +function renderGrid(opts: Record, dataSource?: any) { const schema: any = { type: 'object-grid', objectName: 'task', @@ -101,7 +114,7 @@ function renderGrid(opts: Record) { }; return render( - + , ); } @@ -236,15 +249,15 @@ describe('object-grid `selection`: presence enables, an explicit off wins (objec // `bulkActions` alone auto-enables multi-select (the row below proves it), // so this is the member deciding against a declaration that asks for the // opposite — and the author is told nothing. - renderGrid({ ...paged, selection: { type: 'none' }, bulkActions: ['approve'] }); + renderGrid({ ...paged, selection: { type: 'none' }, bulkActions: ['approve'] }, DECLARES_APPROVE); await waitFor(() => expect(screen.getByText('Row 01')).toBeInTheDocument()); expect(screen.queryAllByRole('checkbox')).toHaveLength(0); }); it('CONTROL — the same schema WITHOUT the member auto-enables multi-select', async () => { - renderGrid({ ...paged, bulkActions: ['approve'] }); + renderGrid({ ...paged, bulkActions: ['approve'] }, DECLARES_APPROVE); await waitFor(() => expect(screen.getByText('Row 01')).toBeInTheDocument()); - expect(screen.getAllByRole('checkbox')).toHaveLength(6); + await waitFor(() => expect(screen.getAllByRole('checkbox')).toHaveLength(6)); }); it('`none` also beats the deprecated `selectable: true`', async () => { diff --git a/ui/packages/plugin-grid/src/__tests__/legacyRowActionDispatch.test.tsx b/ui/packages/plugin-grid/src/__tests__/legacyRowActionDispatch.test.tsx index e4493e8a035..4b55cc33a28 100644 --- a/ui/packages/plugin-grid/src/__tests__/legacyRowActionDispatch.test.tsx +++ b/ui/packages/plugin-grid/src/__tests__/legacyRowActionDispatch.test.tsx @@ -38,6 +38,7 @@ vi.mock('@object-ui/permissions', async (importOriginal) => { }); import { ObjectGrid } from '../ObjectGrid'; +import { resetUndeclaredActionNamesWarnings } from '../undeclaredActionNames'; import { registerAllFields } from '@object-ui/fields'; import { ActionProvider, SchemaRendererProvider } from '@object-ui/react'; @@ -108,6 +109,7 @@ async function openRowMenu(opts: Parameters[0]) { } beforeEach(() => { + resetUndeclaredActionNamesWarnings(); fetchMock = vi.fn().mockResolvedValue({ ok: true, status: 200, @@ -160,19 +162,31 @@ describe('legacy string rowActions dispatch (objectui#2960)', () => { expect(screen.getAllByTestId('row-action-convert_lead')).toHaveLength(1); }); - it('leaves a name with no declared action to the by-name handler path', async () => { - // Consumers may register a runner handler under exactly this name, so the - // entry must still render and still dispatch. - await openRowMenu({ - objectActions: [CONVERT_LEAD], - schema: { rowActions: ['crm_only_handler'] }, - }); + it('draws no entry for a name the object does not declare, and warns once naming it (objectstack#30169)', async () => { + // Trigger objectstack#30169: an action a disabled package withholds is + // absent from the served object, so its bare name matched nothing and the + // menu drew a humanized entry that toasted "not executable". The spec says + // a bare name "names an action the object declares"; one that does not + // draws nothing, and development says which name it dropped. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + await openRowMenu({ + objectActions: [CONVERT_LEAD], + schema: { rowActions: ['convert_lead', 'crm_only_handler'] }, + }); - expect(screen.getByTestId('row-action-crm_only_handler')).toBeInTheDocument(); - // Nothing resolvable to call — and with the runner's fallback now failing - // loudly, no request and no success are reported. - await userEvent.click(screen.getByTestId('row-action-crm_only_handler')); - await waitFor(() => expect(fetchMock).not.toHaveBeenCalled()); + // The declared sibling still draws, so the menu is open and populated. + expect(screen.getByTestId('row-action-convert_lead')).toBeInTheDocument(); + expect(screen.queryByTestId('row-action-crm_only_handler')).not.toBeInTheDocument(); + const lines = warn.mock.calls + .map((args) => String(args[0])) + .filter((line) => line.includes('ObjectGrid rowActions')); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain("'crm_only_handler'"); + expect(lines[0]).not.toContain("'convert_lead'"); + } finally { + warn.mockRestore(); + } }); }); diff --git a/ui/packages/plugin-grid/src/__tests__/objectBulkActionDispatch.test.tsx b/ui/packages/plugin-grid/src/__tests__/objectBulkActionDispatch.test.tsx index 6e1ee327b02..cad94a83eba 100644 --- a/ui/packages/plugin-grid/src/__tests__/objectBulkActionDispatch.test.tsx +++ b/ui/packages/plugin-grid/src/__tests__/objectBulkActionDispatch.test.tsx @@ -41,6 +41,7 @@ vi.mock('@object-ui/permissions', async (importOriginal) => { }); import { ObjectGrid } from '../ObjectGrid'; +import { resetUndeclaredActionNamesWarnings } from '../undeclaredActionNames'; import { registerAllFields } from '@object-ui/fields'; import { ActionProvider } from '@object-ui/react'; @@ -80,16 +81,24 @@ function renderGrid(opts: { handlers?: Record; /** Override the seed records — e.g. to give a `visible` predicate something to bite on. */ rows?: Array>; + /** `'absent'`: the data source has no `getObjectSchema`; `'fails'`: it rejects. */ + schemaRead?: 'absent' | 'fails'; + /** Mount under React StrictMode, as the Console does. */ + strict?: boolean; }) { const rows = opts.rows ?? ROWS; const dataSource: any = { find: vi.fn(async () => ({ data: rows.map(r => ({ ...r })), total: rows.length, hasMore: false, pageSize: 50 })), - getObjectSchema: async (name: string) => ({ + }; + if (opts.schemaRead === 'fails') { + dataSource.getObjectSchema = async () => { throw new Error('schema read refused'); }; + } else if (opts.schemaRead !== 'absent') { + dataSource.getObjectSchema = async (name: string) => ({ name, fields: { id: { type: 'text' }, name: { type: 'text', label: 'Name' }, done: { type: 'boolean' } }, ...(opts.objectActions ? { actions: opts.objectActions } : {}), - }), - }; + }); + } const schema: any = { type: 'object-grid', objectName: OBJECT, @@ -97,11 +106,13 @@ function renderGrid(opts: { pagination: { pageSize: 50 }, ...opts.schema, }; - render( + const tree = ( - , + ); + const result = render(opts.strict ? {tree} : tree); + dataSource.unmount = result.unmount; return dataSource; } @@ -117,6 +128,7 @@ async function renderAndSelectAll(opts: Parameters[0]) { } beforeEach(() => { + resetUndeclaredActionNamesWarnings(); fetchMock = vi.fn().mockResolvedValue({ ok: true, status: 200, @@ -210,10 +222,11 @@ describe('view-declared bulk actions (objectui#3002)', () => { await waitFor(() => expect(screen.getAllByTestId('bulk-action-push_down')).toHaveLength(1)); }); - it('keeps an unresolvable name alongside the promoted defs', async () => { - // A name the object never declared may still have a runner handler - // registered under it; hiding it because some OTHER def exists dropped - // half the bar's buttons. + it('draws no button for a name the object does not declare, even with a runner handler under it (objectstack#30169)', async () => { + // Trigger objectstack#30169. The spec says a bare `bulkActions` name "names + // an action the object declares", and lint `action-name-undefined` refuses + // any other. A runner handler registered under the name is client code, + // not a declaration, so it does not bring the button back. const handler = vi.fn(async () => ({ success: true })); await renderAndSelectAll({ objectActions: [PUSH_DOWN], @@ -222,9 +235,126 @@ describe('view-declared bulk actions (objectui#3002)', () => { }); expect(await screen.findByTestId('bulk-action-push_down')).toBeInTheDocument(); - const legacy = await screen.findByTestId('bulk-action-crm_only_handler'); - fireEvent.click(legacy); - await waitFor(() => expect(handler).toHaveBeenCalledTimes(1)); + expect(screen.queryByTestId('bulk-action-crm_only_handler')).not.toBeInTheDocument(); + expect(handler).not.toHaveBeenCalled(); + }); +}); + +/** + * objectstack#30169 — the selection bar and a disabled package. With the + * package disabled the server withholds its actions: they leave the served + * object's `actions`, exactly as if never declared. The view still names one + * in `bulkActions` as a bare string (the spec accepts it). Before the fix the + * bar drew it anyway, under its humanized name ("Create Campaign"), and the + * click toasted "not executable". Now the bar draws only what the object + * declares: withheld, no button and one development warning naming it; + * served, the button under the action's own label. + */ +describe('a bulk action a disabled package withholds (objectstack#30169)', () => { + /** The package's action. Its label is NOT the humanized name, on purpose. */ + const CREATE_CAMPAIGN = { + name: 'create_campaign', + label: 'Add to Campaign', + type: 'api', + target: '/api/v1/campaigns/add', + }; + const VIEW = { bulkActions: ['push_down', 'create_campaign'] }; + + const bulkActionWarnings = (warn: { mock: { calls: unknown[][] } }) => + warn.mock.calls + .map((args) => String(args[0])) + .filter((line) => line.includes('ObjectGrid bulkActions')); + + it('withheld: the served object lacks it, so the bar draws no button and warns once naming it', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + await renderAndSelectAll({ objectActions: [PUSH_DOWN], schema: VIEW }); + + // The declared sibling is drawn, so the bar is up and populated. + expect(await screen.findByTestId('bulk-action-push_down')).toBeInTheDocument(); + expect(screen.queryByTestId('bulk-action-create_campaign')).not.toBeInTheDocument(); + expect(screen.queryByText('Create Campaign')).not.toBeInTheDocument(); + await waitFor(() => expect(bulkActionWarnings(warn)).toHaveLength(1)); + expect(bulkActionWarnings(warn)[0]).toContain("'create_campaign'"); + expect(bulkActionWarnings(warn)[0]).not.toContain("'push_down'"); + } finally { + warn.mockRestore(); + } + }); + + it('served: the same view draws it under the action label, and stays quiet', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + await renderAndSelectAll({ objectActions: [PUSH_DOWN, CREATE_CAMPAIGN], schema: VIEW }); + + expect(await screen.findByTestId('bulk-action-create_campaign')).toHaveTextContent('Add to Campaign'); + expect(bulkActionWarnings(warn)).toEqual([]); + } finally { + warn.mockRestore(); + } + }); + + it('an object named only through `data.object` waits for that object, and the warning names it', async () => { + // The grid's object is `resolveRecordSourceObjectName(schema, data)`, not + // `schema.objectName`. Keyed on the top-level key, readiness was true on + // the first render, before the schema landed, so the DECLARED `push_down` + // was reported too, under "no objectName". + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + await renderAndSelectAll({ + objectActions: [PUSH_DOWN], + schema: { ...VIEW, objectName: undefined, data: { provider: 'object', object: OBJECT } }, + }); + + expect(await screen.findByTestId('bulk-action-push_down')).toBeInTheDocument(); + await waitFor(() => expect(bulkActionWarnings(warn)).toHaveLength(1)); + expect(bulkActionWarnings(warn)[0]).toContain(`objectName: '${OBJECT}'`); + expect(bulkActionWarnings(warn)[0]).toContain("'create_campaign'"); + expect(bulkActionWarnings(warn)[0]).not.toContain("'push_down'"); + } finally { + warn.mockRestore(); + } + }); + + it.each([ + ['has no getObjectSchema', 'absent'], + ['rejects the schema read', 'fails'], + ] as const)('inline rows whose data source %s still report the names, as unread', async (_label, schemaRead) => { + // Nothing will ever land, so nothing is awaited: the names are reported, + // and the message says the object's actions are unknown rather than that + // the names are undeclared. Before, readiness never flipped and the drop + // was silent. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + // (No wait on the rows: with a failed read their columns stay masked, + // objectui#10657, so 'Plan A' is never drawn in the `fails` leg.) + renderGrid({ schemaRead, schema: { ...VIEW, data: { provider: 'value', items: ROWS } } }); + + await waitFor(() => expect(bulkActionWarnings(warn)).toHaveLength(1)); + expect(bulkActionWarnings(warn)[0]).toContain('could not be read'); + expect(bulkActionWarnings(warn)[0]).toContain("'push_down', 'create_campaign'"); + expect(screen.queryByTestId('bulk-action-create_campaign')).not.toBeInTheDocument(); + } finally { + warn.mockRestore(); + } + }); + + it('warns once under StrictMode and across a remount of the same list', async () => { + // StrictMode runs every mount effect setup → cleanup → setup; this shape is + // ready on its first render, so an effect-only guard said it twice. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const opts = { schemaRead: 'absent', strict: true, schema: { ...VIEW, data: { provider: 'value', items: ROWS } } } as const; + const ds = renderGrid(opts); + await waitFor(() => expect(screen.getByText('Plan A')).toBeInTheDocument()); + await waitFor(() => expect(bulkActionWarnings(warn).length).toBeGreaterThan(0)); + ds.unmount(); + renderGrid(opts); + await waitFor(() => expect(screen.getByText('Plan A')).toBeInTheDocument()); + expect(bulkActionWarnings(warn)).toHaveLength(1); + } finally { + warn.mockRestore(); + } }); }); diff --git a/ui/packages/plugin-grid/src/__tests__/resolveBulkActions.test.ts b/ui/packages/plugin-grid/src/__tests__/resolveBulkActions.test.ts index 096e555ef2c..15b08cc816c 100644 --- a/ui/packages/plugin-grid/src/__tests__/resolveBulkActions.test.ts +++ b/ui/packages/plugin-grid/src/__tests__/resolveBulkActions.test.ts @@ -29,12 +29,12 @@ describe('resolveBulkActions', () => { // Naming the action in the view's `bulkActions` is the ONLY declaration — // spec 17 retired `action.bulkEnabled` (framework#4054) and its tombstone // prescribes exactly this. - const { defs, unresolved } = resolveBulkActions({ + const { defs, undeclared } = resolveBulkActions({ bulkActions: ['push_down'], objectActions: [PUSH_DOWN], }); - expect(unresolved).toEqual([]); + expect(undeclared).toEqual([]); expect(defs).toHaveLength(1); expect(defs[0]).toMatchObject({ name: 'push_down', @@ -47,30 +47,35 @@ describe('resolveBulkActions', () => { it('surfaces nothing for an object action the view never names', () => { const rowOnly = { name: 'convert_lead', label: 'Convert', locations: ['list_item'] }; - const { defs, unresolved } = resolveBulkActions({ objectActions: [rowOnly, PUSH_DOWN] }); + const { defs, undeclared } = resolveBulkActions({ objectActions: [rowOnly, PUSH_DOWN] }); expect(defs).toEqual([]); - expect(unresolved).toEqual([]); + expect(undeclared).toEqual([]); }); it('ignores a stale bulkEnabled flag on an object action', () => { // The key is a retiredKey() tombstone: `defineStack` hard-rejects a config // that sets it, so it must never be a path into the bar on its own. const stale = { name: 'push_down', label: '下推', bulkEnabled: true }; - const { defs, unresolved } = resolveBulkActions({ objectActions: [stale] }); + const { defs, undeclared } = resolveBulkActions({ objectActions: [stale] }); expect(defs).toEqual([]); - expect(unresolved).toEqual([]); + expect(undeclared).toEqual([]); }); - it('keeps a name that matches no declared action for by-name dispatch', () => { - const { defs, unresolved } = resolveBulkActions({ + it('gives a name the object does not declare no def, and reports it as undeclared (objectstack#30169)', () => { + // Trigger objectstack#30169: a disabled package's action is withheld from + // the served object, so its bare name matched nothing and the bar drew a + // humanized ghost button that toasted "not executable". The spec says a + // bare name "names an action the object declares"; a name that does not + // resolves to no def. `undeclared` feeds the development warning only. + const { defs, undeclared } = resolveBulkActions({ bulkActions: ['push_down', 'crm_only_handler'], objectActions: [PUSH_DOWN], }); expect(defs.map(d => d.name)).toEqual(['push_down']); - expect(unresolved).toEqual(['crm_only_handler']); + expect(undeclared).toEqual(['crm_only_handler']); }); it('lets an inline-authored def win over the object action of the same name', () => { @@ -85,13 +90,13 @@ describe('resolveBulkActions', () => { }); it('drops a repeated name', () => { - const { defs, unresolved } = resolveBulkActions({ + const { defs, undeclared } = resolveBulkActions({ bulkActions: ['dispatch_job', 'dispatch_job', 'ghost', 'ghost'], objectActions: [{ name: 'dispatch_job' }], }); expect(defs).toHaveLength(1); - expect(unresolved).toEqual(['ghost']); + expect(undeclared).toEqual(['ghost']); }); it('returns the authored array by reference when nothing folds in', () => { @@ -255,12 +260,12 @@ describe('resolveBulkActions', () => { operation: 'custom', execution: 'aggregate', }; - const { defs, unresolved } = resolveBulkActions({ + const { defs, undeclared } = resolveBulkActions({ bulkActionDefs: [authored], objectActions: [PUSH_DOWN], }); - expect(unresolved).toEqual([]); + expect(undeclared).toEqual([]); expect(defs).toHaveLength(1); expect(defs[0]).toMatchObject({ name: 'push_down', diff --git a/ui/packages/plugin-grid/src/__tests__/resolveLegacyRowActions.test.ts b/ui/packages/plugin-grid/src/__tests__/resolveLegacyRowActions.test.ts index 8b2e6e70ca1..a08e6507fc2 100644 --- a/ui/packages/plugin-grid/src/__tests__/resolveLegacyRowActions.test.ts +++ b/ui/packages/plugin-grid/src/__tests__/resolveLegacyRowActions.test.ts @@ -19,7 +19,7 @@ const CONVERT_LEAD = { describe('resolveLegacyRowActions', () => { it('promotes a legacy name to the object action it names', () => { - const { defs, unresolved } = resolveLegacyRowActions({ + const { defs, undeclared } = resolveLegacyRowActions({ rowActions: ['convert_lead'], rowActionDefs: [], objectActions: [CONVERT_LEAD], @@ -28,12 +28,12 @@ describe('resolveLegacyRowActions', () => { // The dead `{ type: 'convert_lead' }` dispatch is gone: the row now // carries the real def, so it routes through the api branch (objectui#2960). expect(defs).toEqual([CONVERT_LEAD]); - expect(unresolved).toEqual([]); + expect(undeclared).toEqual([]); }); it('drops a legacy name that duplicates an existing list_item def', () => { const listItemDef = { ...CONVERT_LEAD, locations: ['list_item'], label: '转换线索' }; - const { defs, unresolved } = resolveLegacyRowActions({ + const { defs, undeclared } = resolveLegacyRowActions({ rowActions: ['convert_lead'], rowActionDefs: [listItemDef], objectActions: [listItemDef], @@ -42,33 +42,46 @@ describe('resolveLegacyRowActions', () => { // One working entry, not a working entry plus a dead twin — and the // already-localized def wins over the raw object action. expect(defs).toEqual([listItemDef]); - expect(unresolved).toEqual([]); + expect(undeclared).toEqual([]); }); - it('keeps a name that matches no declared action for by-name dispatch', () => { - // Consumers may register a runner handler under exactly this name; that - // path stays intact. - const { defs, unresolved } = resolveLegacyRowActions({ + it('gives a name the object does not declare no def, and reports it as undeclared (objectstack#30169)', () => { + // Trigger objectstack#30169: the same rule as the selection bar. A bare + // name that names no declared action (a misspelling, or an action a + // disabled package withholds) draws nothing; a runner handler registered + // under the name is not a declaration. `undeclared` feeds the development + // warning only. + const { defs, undeclared } = resolveLegacyRowActions({ rowActions: ['crm_only_handler'], rowActionDefs: [], objectActions: [CONVERT_LEAD], }); expect(defs).toEqual([]); - expect(unresolved).toEqual(['crm_only_handler']); + expect(undeclared).toEqual(['crm_only_handler']); + }); + + it('reports a repeated undeclared name once', () => { + const { undeclared } = resolveLegacyRowActions({ + rowActions: ['ghost', 'ghost'], + rowActionDefs: [], + objectActions: [CONVERT_LEAD], + }); + + expect(undeclared).toEqual(['ghost']); }); it('appends promotions after the declared defs and preserves order', () => { const archive = { name: 'archive', locations: ['list_item'] }; const merge = { name: 'merge', locations: ['record_header'] }; - const { defs, unresolved } = resolveLegacyRowActions({ + const { defs, undeclared } = resolveLegacyRowActions({ rowActions: ['merge', 'unknown_one', 'convert_lead'], rowActionDefs: [archive], objectActions: [CONVERT_LEAD, merge], }); expect(defs).toEqual([archive, merge, CONVERT_LEAD]); - expect(unresolved).toEqual(['unknown_one']); + expect(undeclared).toEqual(['unknown_one']); }); it('promotes a repeated legacy name only once', () => { @@ -98,13 +111,13 @@ describe('resolveLegacyRowActions', () => { it('tolerates a missing object schema and malformed entries', () => { // `objectSchema` is fetched async, so the first paint has no actions. - const { defs, unresolved } = resolveLegacyRowActions({ + const { defs, undeclared } = resolveLegacyRowActions({ rowActions: ['convert_lead', '', null as unknown as string], rowActionDefs: null, objectActions: undefined, }); expect(defs).toEqual([]); - expect(unresolved).toEqual(['convert_lead']); + expect(undeclared).toEqual(['convert_lead']); }); }); diff --git a/ui/packages/plugin-grid/src/__tests__/rowCrudEffectiveOps.test.tsx b/ui/packages/plugin-grid/src/__tests__/rowCrudEffectiveOps.test.tsx index 04d4057b6dc..dd9d9a72ca7 100644 --- a/ui/packages/plugin-grid/src/__tests__/rowCrudEffectiveOps.test.tsx +++ b/ui/packages/plugin-grid/src/__tests__/rowCrudEffectiveOps.test.tsx @@ -129,6 +129,9 @@ function renderGrid(c: Case) { ...(c.managedBy ? { managedBy: c.managedBy } : {}), ...(c.userActions ? { userActions: c.userActions } : {}), fields: { id: { type: 'text' }, name: { type: 'text', label: 'Name' } }, + // `notify` is declared: a bare bulk-action name draws only when the + // object declares the action it names (objectstack#30169). + actions: [{ name: 'notify', label: 'Notify' }], }), }; const schema: any = { diff --git a/ui/packages/plugin-grid/src/__tests__/undeclaredActionNames.test.ts b/ui/packages/plugin-grid/src/__tests__/undeclaredActionNames.test.ts new file mode 100644 index 00000000000..35da6e636ad --- /dev/null +++ b/ui/packages/plugin-grid/src/__tests__/undeclaredActionNames.test.ts @@ -0,0 +1,65 @@ +// @vitest-environment node +/** + * 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. + */ + +/** + * objectstack#30169 — the undeclared-name warning's own contract. + * + * The production gate is pinned against the BUILD, not the runtime: under + * vitest `process` is node's, so both spellings of the gate read the same + * `NODE_ENV` and a runtime assertion cannot tell them apart. What differs is + * what the Console's production build does to them. Vite replaces the literal + * `process.env.NODE_ENV` with `"production"`; an optional chain off + * `globalThis.process` survives the build and then reads the Console's + * `window.process` shim (`ui/apps/console/index.html`), which says + * `development` unconditionally. Measured on vite 8.3.5 with the Console's own + * `define`: the literal folds to a constant, the chain stays live. + */ + +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { describe, it, expect } from 'vitest'; +import { transformWithOxc } from 'vite'; +import { describeUndeclaredActionNames } from '../undeclaredActionNames'; + +const SOURCE_PATH = fileURLToPath(new URL('../undeclaredActionNames.ts', import.meta.url)); + +describe('the development gate folds away in a production build (objectstack#30169)', () => { + it('reads only the bundler-replaced literal, so the Console build leaves no NODE_ENV read behind', async () => { + // The Console's `define` (`ui/apps/console/vite.config.ts`) plus the + // `process.env.NODE_ENV` Vite adds for a production build. + const { code } = await transformWithOxc(readFileSync(SOURCE_PATH, 'utf8'), SOURCE_PATH, { + lang: 'ts', + define: { 'process.env.NODE_ENV': '"production"', 'process.env': '{}' }, + }); + expect(code).not.toMatch(/NODE_ENV/); + expect(code).toContain('"production" !== "production"'); + }); +}); + +describe('describeUndeclaredActionNames', () => { + const ADDRESS = { blockType: 'object-grid', objectName: 'lead', label: 'All Leads' }; + + it('says the names are undeclared when the object was read', () => { + const line = describeUndeclaredActionNames('bulkActions', ['create_campaign'], ADDRESS); + expect(line).toContain("object-grid (objectName: 'lead', label: 'All Leads')"); + expect(line).toContain("1 name matches no action the object declares"); + expect(line).toContain("'create_campaign'"); + }); + + it('says the actions are unknown, not undeclared, when the schema was never read', () => { + const line = describeUndeclaredActionNames('rowActions', ['a', 'b'], ADDRESS, 'unread'); + expect(line).toContain('could not be read'); + expect(line).toContain("the row menu draws no entry for 2 names: 'a', 'b'"); + expect(line).not.toContain('matches no action'); + }); + + it('says nothing for no names', () => { + expect(describeUndeclaredActionNames('bulkActions', [], ADDRESS)).toBeNull(); + }); +}); diff --git a/ui/packages/plugin-grid/src/objectGridActionsColumn.tsx b/ui/packages/plugin-grid/src/objectGridActionsColumn.tsx index 198bd743b4c..b722ab44e1d 100644 --- a/ui/packages/plugin-grid/src/objectGridActionsColumn.tsx +++ b/ui/packages/plugin-grid/src/objectGridActionsColumn.tsx @@ -27,7 +27,7 @@ type RowCrudAffordances = ReturnType; * `ObjectGrid` calls it where these lines used to stand. */ export function resolveObjectGridActionsColumn({ - schema, data, objectSchema, persistedColumns, operations, customRowActions, resolvedRowActionDefs, + schema, data, objectSchema, persistedColumns, operations, resolvedRowActionDefs, wantEditAction, wantDeleteAction, mayInvokeRowAction, rowOperations, canEdit, canDelete, recordVerdict, predicateScope, onEdit, onDelete, editPredicates, deletePredicates, executeAction, setRefreshKey, t, @@ -37,7 +37,6 @@ export function resolveObjectGridActionsColumn({ objectSchema: any; persistedColumns: ReturnType['persistedColumns']; operations: ObjectGridSchema['operations'] | { update: boolean; delete: boolean }; - customRowActions: ReturnType['unresolved']; resolvedRowActionDefs: ReturnType['defs']; wantEditAction: boolean; wantDeleteAction: boolean; @@ -61,7 +60,7 @@ export function resolveObjectGridActionsColumn({ // record-level verdict they feed is fetched by a hook, which may not sit // behind a conditional return. const hasActions = !!(operations && (operations.update || operations.delete)); - const hasRowActions = customRowActions.length > 0 || resolvedRowActionDefs.length > 0 || wantEditAction || wantDeleteAction; + const hasRowActions = resolvedRowActionDefs.length > 0 || wantEditAction || wantDeleteAction; // objectui#12708 — the Actions column is drawn only when a row on screen // OFFERS something: its plan, the one `RowActionMenu` renders the row's cell @@ -86,7 +85,6 @@ export function resolveObjectGridActionsColumn({ scope: predicateScope, actionDefs: rowActionDefsGated, maxInlineActions: maxInlineRowActions, - rowActions: customRowActions, ...rowCrudVerdicts(row), onEdit, onDelete, @@ -116,7 +114,6 @@ export function resolveObjectGridActionsColumn({ const rowVerdicts = rowCrudVerdicts(row); return { row, - rowActions: customRowActions, rowActionDefs: resolvedRowActionDefs as any[], objectFields: objectSchema?.fields, // NON-AUTHOR SURFACE — `maxInlineRowActions` is deliberately diff --git a/ui/packages/plugin-grid/src/resolveBulkActions.ts b/ui/packages/plugin-grid/src/resolveBulkActions.ts index e3925d0790a..14941657c24 100644 --- a/ui/packages/plugin-grid/src/resolveBulkActions.ts +++ b/ui/packages/plugin-grid/src/resolveBulkActions.ts @@ -75,11 +75,20 @@ * historical no-op-per-row meaning (callout handled by the consumer's * `onComplete`). * - * Names that resolve to nothing stay in `unresolved` and are still dispatched - * by name: consumers may have registered a runner handler under exactly that - * name (`runner.registerHandler('my_action', …)`), and that path must keep - * working. When no such handler exists the runner reports the dead end rather - * than toasting success (#2996). + * ## A name the object does not declare draws nothing + * + * `bulkActions: ['']` "names an action the object declares" + * (`packages/spec/src/ui/bulk-action.zod.ts`), and lint `action-name-undefined` + * (ADR-0049) refuses any other name at authoring. The selection bar holds the + * same line at render: a name with no match in `objectDef.actions` gets no + * button. That covers a misspelling and an action a disabled package withholds + * (the server leaves it out of the served object, so it reads exactly like a + * name that never existed). The name comes back in `undeclared` only so the + * grid can say, once and in development, which names it did not draw + * (`describeUndeclaredActionNames`). A runner handler registered under a bare + * name is not a declaration and does not bring the button back. The canonical + * `'delete'` is not a name this fold sees: the grid routes it to + * `onBulkDelete` before the call. */ import type { BulkActionDef, BulkActionParam } from '@object-ui/types'; @@ -417,8 +426,11 @@ export function resolveBulkActions(opts: { * its identity across renders. */ defs: readonly BulkActionDef[]; - /** Names that matched no declared action; dispatched by name. */ - unresolved: string[]; + /** + * Names that matched no declared action. NOT drawn and NOT dispatched: they + * exist for the development warning only. + */ + undeclared: string[]; } { // [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 @@ -473,25 +485,25 @@ export function resolveBulkActions(opts: { ); const promoted: BulkActionDef[] = []; - const unresolved: string[] = []; + const undeclared: string[] = []; for (const name of names) { if (typeof name !== 'string' || name === '') continue; // Already surfaced (an authored def of the same name, or an earlier pass // over a repeated entry) — drop the duplicate rather than render a twin. - // Both outcomes claim the name: the bar keys its buttons on it, so even - // two dead-name entries would collide. + // Both outcomes claim the name, so a repeated undeclared name is reported + // once. if (claimed.has(name)) continue; claimed.add(name); const match = objectActions.find(a => a?.name === name); if (match) { promoted.push(toBulkActionDef(match, opts.localizeLabel)); } else { - unresolved.push(name); + undeclared.push(name); } } return { defs: promoted.length > 0 ? [...authored, ...promoted] : authored, - unresolved, + undeclared, }; } diff --git a/ui/packages/plugin-grid/src/resolveLegacyRowActions.ts b/ui/packages/plugin-grid/src/resolveLegacyRowActions.ts index e5122aade8f..80909f21e63 100644 --- a/ui/packages/plugin-grid/src/resolveLegacyRowActions.ts +++ b/ui/packages/plugin-grid/src/resolveLegacyRowActions.ts @@ -32,11 +32,16 @@ * declared elsewhere on the object — the case the legacy form exists to serve — * is PROMOTED to that def so it dispatches for real. * - * Names that resolve to nothing stay in `unresolved` and are still dispatched - * by name: consumers may have registered a runner handler under exactly that - * name (`runner.registerHandler('my_action', …)`), and that path must keep - * working. When no such handler exists the runner now reports the dead end - * rather than toasting success. + * A name that resolves to nothing draws nothing. The spec's own sentence is + * the rule: a bare `rowActions` / `bulkActions` name "names an action the + * object declares" (`bulk-action.zod.ts`), and lint `action-name-undefined` + * (ADR-0049) refuses an undeclared one at authoring. An action a disabled + * package withholds is absent from the served `objectDef.actions` for the same + * reason, so it leaves the row menu with the rest of that package. The name + * is returned in `undeclared` only so the grid can say, once and in + * development, which names it did not draw (`describeUndeclaredActionNames`). + * A handler a host registers under a bare name is not a declaration and does + * not bring the entry back. */ /** The subset of an `ActionDef` this fold needs: everything is passed through. */ @@ -63,12 +68,15 @@ export function resolveLegacyRowActions(opts: { * every render of a view that has no legacy names to fold. */ defs: readonly NamedActionDef[]; - /** Legacy names that matched no declared action; dispatched by name. */ - unresolved: string[]; + /** + * Legacy names that matched no declared action. NOT drawn and NOT + * dispatched: they exist for the development warning only. + */ + undeclared: string[]; } { const defs = Array.isArray(opts.rowActionDefs) ? opts.rowActionDefs : []; const names = Array.isArray(opts.rowActions) ? opts.rowActions : []; - if (names.length === 0) return { defs, unresolved: [] }; + if (names.length === 0) return { defs, undeclared: [] }; const objectActions = Array.isArray(opts.objectActions) ? opts.objectActions : []; const claimed = new Set( @@ -76,7 +84,7 @@ export function resolveLegacyRowActions(opts: { ); const promoted: NamedActionDef[] = []; - const unresolved: string[] = []; + const undeclared: string[] = []; for (const name of names) { if (typeof name !== 'string' || name === '') continue; @@ -87,13 +95,13 @@ export function resolveLegacyRowActions(opts: { if (match) { claimed.add(name); promoted.push(match); - } else { - unresolved.push(name); + } else if (!undeclared.includes(name)) { + undeclared.push(name); } } return { defs: promoted.length > 0 ? [...defs, ...promoted] : defs, - unresolved, + undeclared, }; } diff --git a/ui/packages/plugin-grid/src/undeclaredActionNames.ts b/ui/packages/plugin-grid/src/undeclaredActionNames.ts new file mode 100644 index 00000000000..20bd63af6a3 --- /dev/null +++ b/ui/packages/plugin-grid/src/undeclaredActionNames.ts @@ -0,0 +1,156 @@ +/** + * 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. + */ + +/** + * The development warning for a bare `bulkActions` / `rowActions` name that + * matches no action the object declares. + * + * `resolveBulkActions` and `resolveLegacyRowActions` draw nothing for such a + * name: the spec says a bare name "names an action the object declares" + * (`packages/spec/src/ui/bulk-action.zod.ts`), and lint + * `action-name-undefined` (ADR-0049) refuses any other at authoring. This + * module is the half that keeps the drop from being silent. It warns once per + * distinct list, only in development, and only once there is nothing left to + * wait for: the object's schema has landed, or it never will (no + * `getObjectSchema`, or the read failed). A list still loading its object is + * never reported; a list whose object could not be read is, and the message + * says the object's actions are unknown rather than that the name is + * undeclared. + */ + +import { useEffect } from 'react'; + +/** + * The bundler-replaced `process.env.NODE_ENV` idiom, declared for THIS module + * only (the same stance as `@object-ui/fields`' `resolveActionParams.ts`): the + * package's source program carries no node typings, and a module-scoped + * declaration cannot shadow the real node global anywhere else. + * + * ⛔ Spell the gate as the literal `process.env.NODE_ENV`, never as an optional + * chain off `globalThis.process`. Only the literal is a token the bundler + * replaces: the Console's production build turns it into `"production"`, while + * the chain survives the build and reads the Console's `window.process` shim + * (`ui/apps/console/index.html`), which says `development` unconditionally, so + * a production Console would log the warning too. Pinned by + * `__tests__/undeclaredActionNames.test.ts`. + */ +declare const process: { env: { NODE_ENV?: string } }; + +/** The two bare-name keys whose undeclared names this module reports. */ +export type UndeclaredActionNameKey = 'bulkActions' | 'rowActions'; + +/** Where the offending block lives, for the first line of the message. */ +export interface UndeclaredActionNamesAddress { + /** 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. */ + label?: unknown; +} + +/** + * Whether the object's actions were known when the names were judged. + * `'unread'`: the grid names an object but its schema never arrived (no + * `getObjectSchema`, or the read failed), so every bare name went undrawn + * without the object being asked. + */ +export type UndeclaredActionNamesBasis = 'declared' | 'unread'; + +const SURFACE: Record = { + bulkActions: 'the selection bar draws no button', + rowActions: 'the row menu draws no entry', +}; + +function describeAddress({ blockType, objectName, label }: UndeclaredActionNamesAddress): 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(', ')})`; +} + +/** + * The message for the names one fold did not draw, or `null` when there are + * none. It names the key, the block, and every name, and says what to write. + */ +export function describeUndeclaredActionNames( + key: UndeclaredActionNameKey, + names: readonly string[], + address: UndeclaredActionNamesAddress, + basis: UndeclaredActionNamesBasis = 'declared', +): string | null { + if (names.length === 0) return null; + const them = names.length === 1 ? 'it' : 'them'; + const listed = names.map((n) => `'${n}'`).join(', '); + const rule = ` A bare \`${key}\` name must name an action declared on the object.`; + if (basis === 'unread') { + const count = names.length === 1 ? '1 name' : `${names.length} names`; + return `[ObjectUI] ObjectGrid ${key}: ${describeAddress(address)} — the object's schema could not ` + + `be read (no getObjectSchema on the data source, or the read failed), so its declared actions ` + + `are unknown and ${SURFACE[key]} for ${count}: ${listed}.\n` + + `${rule} Give the grid a data source that answers getObjectSchema, or check why the read failed.`; + } + const count = names.length === 1 ? '1 name matches' : `${names.length} names match`; + return `[ObjectUI] ObjectGrid ${key}: ${describeAddress(address)} — ${count} no action the object ` + + `declares, so ${SURFACE[key]} for ${them}: ${listed}.\n` + + `${rule} Declare the action on the ` + + 'object, or remove the name. An action that a disabled package withholds is left out of the ' + + 'object the same way, so it is not drawn either.'; +} + +/** Development only; see the `process` declaration above for the spelling. */ +function isDevelopment(): boolean { + return process.env.NODE_ENV !== 'production'; +} + +// Warn once per message, not once per effect run: under React StrictMode every +// effect runs setup → cleanup → setup in development, and a list that remounts +// (navigating away and back) would say the same thing again. Keyed by the +// message, which already carries the key, the block's address and every name, +// so a list that changes its names, or a second list, still reports. The same +// memo shape as `@object-ui/core`'s `column-identity.ts`. +const warnedMessages = new Set(); + +/** Reset the warn-once memo. Exported for tests. */ +export function resetUndeclaredActionNamesWarnings(): void { + warnedMessages.clear(); +} + +/** + * Logs {@link describeUndeclaredActionNames} once per distinct message, in + * development only. `ready` is false while the object's schema is still on its + * way: before that every name is undeclared and none is worth reporting. + * `basis` says whether the object's actions were read at all. + */ +export function useUndeclaredActionNamesWarning(input: { + key: UndeclaredActionNameKey; + names: readonly string[]; + ready: boolean; + basis?: UndeclaredActionNamesBasis; + address: UndeclaredActionNamesAddress; +}): void { + const { key, ready } = input; + const basis = input.basis ?? 'declared'; + // Keyed on the names' VALUES, never on the array the fold returns, which is + // a fresh one on every render. + const namesKey = ready ? JSON.stringify(input.names) : ''; + const { blockType, objectName, label } = input.address; + useEffect(() => { + if (!namesKey || !isDevelopment()) return; + const message = describeUndeclaredActionNames(key, JSON.parse(namesKey) as string[], { + blockType, + objectName, + label, + }, basis); + if (!message || warnedMessages.has(message)) return; + warnedMessages.add(message); + console.warn(message); + }, [key, namesKey, basis, blockType, objectName, label]); +}