diff --git a/.changeset/bulk-enabled-is-a-tombstone.md b/.changeset/bulk-enabled-is-a-tombstone.md new file mode 100644 index 000000000..695e57d96 --- /dev/null +++ b/.changeset/bulk-enabled-is-a-tombstone.md @@ -0,0 +1,42 @@ +--- +"@object-ui/plugin-grid": patch +"@object-ui/types": patch +"@object-ui/app-shell": patch +--- + +fix(grid): drop the `bulkEnabled` derivation — the spec key is a tombstone + +Follow-up to objectui#3002 / #3031. That change folded two sources into the +selection bar: a view's `bulkActions` names resolved against +`objectDef.actions`, and object actions declaring `ActionSchema.bulkEnabled`. +The second source is dead. + +`@objectstack/spec` 17.0.0 retired `action.bulkEnabled` in the #3896 audit +close-out (framework#4054, landed while #3031 was in flight — the spec source +still carried the key when its design was settled). It is now a `retiredKey()` +tombstone, so it is not merely ignored: `defineStack` **hard-rejects** a config +that sets it, and the backend refuses to boot. Browser verification against a +real showcase backend is what surfaced this — the derivation branch could never +run, and #3031's changeset pointed authors at a key that breaks their app. + +The tombstone's own prescription is the path that survives: + +> the multi-select toolbar is driven by the LIST VIEW's `bulkActions` / +> `bulkActionDefs`, never by this flag … declare the action in the view's +> `bulkActions` instead. + +So `resolveBulkActions` now folds exactly two vocabularies — inline-authored +`bulkActionDefs`, and `bulkActions` names promoted to their declared object +action — which is what #3031's other half already did and what the end-to-end +run exercised: naming `showcase_mark_done` in the view's `bulkActions` issued +one `POST /api/v1/actions/showcase_task/showcase_mark_done` per selected +record (10/10 → `done: true, progress: 100` server-side). Everything downstream +of the fold is unchanged: promoted defs still carry the action's label, icon, +`visible`, confirm text and params; still run through `BulkActionDialog` +(params → confirm → progress → result); still dispatch per record with +`_rowRecord` attached; still attribute failures per record. + +A stale `bulkEnabled: true` on an object action is now inert rather than a +second path into the bar. Note tsc cannot catch this class of drift here — the +fold reads a loosely-typed `NamedActionDef` with an index signature, so the +retired key never surfaces as `never`. diff --git a/packages/app-shell/src/views/ObjectView.tsx b/packages/app-shell/src/views/ObjectView.tsx index fb416a8a1..1215b5799 100644 --- a/packages/app-shell/src/views/ObjectView.tsx +++ b/packages/app-shell/src/views/ObjectView.tsx @@ -1421,13 +1421,12 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an : []), /** * Selection-bar actions. Unlike `rowActionDefs` above, these are - * NOT derived here: `ObjectGrid` is the single convergence point of - * all three list callers (this view, plugin-view's ObjectView and - * plugin-list's ListView), so it folds `objectDef.actions` — the - * spec's `bulkEnabled: true` declaration, plus any legacy - * `bulkActions` name that resolves to a declared action — into the - * def list itself (`resolveBulkActions`, objectui#3002). What we - * pass through is the view author's own inline declaration. + * NOT resolved here: `ObjectGrid` is the single convergence point + * of all three list callers (this view, plugin-view's ObjectView + * and plugin-list's ListView), so it resolves each `bulkActions` + * name against `objectDef.actions` and promotes it into the def + * list itself (`resolveBulkActions`, objectui#3002). What we pass + * through is the view author's own declaration. */ bulkActions: viewDef.bulkActions ?? listSchema.bulkActions, bulkActionDefs: (viewDef as any).bulkActionDefs ?? (listSchema as any).bulkActionDefs, diff --git a/packages/plugin-grid/demo/bulk-actions.tsx b/packages/plugin-grid/demo/bulk-actions.tsx index 2c0c47e36..2cf3f0c3b 100644 --- a/packages/plugin-grid/demo/bulk-actions.tsx +++ b/packages/plugin-grid/demo/bulk-actions.tsx @@ -11,15 +11,14 @@ * on-page panel instead of hitting a server). * * What it exercises: - * 1. `bulk_mark_done` is declared on the OBJECT with `bulkEnabled: true` and - * is named by no view — before #3002 an object simply could not express a - * bulk action, so this button did not exist. - * 2. `bulk_recalc_estimate` is declared on the object WITHOUT the flag; the - * view names it in legacy `bulkActions: [...]`. That name used to be - * dispatched as `{ type: 'bulk_recalc_estimate' }` and never ran. - * 3. `legacy_only_handler` resolves to no declared action and stays a + * 1. `bulk_mark_done` and `bulk_recalc_estimate` are declared on the object + * and NAMED in the view's `bulkActions` — the only way to declare a bulk + * 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. - * 4. Record `t3` fails server-side (422), proving per-record attribution: the + * 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. */ import * as React from 'react'; @@ -53,9 +52,8 @@ const ROWS = [ ]; /** - * `bulkEnabled: true` is the spec's object-level declaration — "this action can - * be applied to multiple selected records". It also carries `locations: - * ['list_item']`, so the same action is a row entry AND a bulk entry. + * Also carries `locations: ['list_item']`, so naming it in the view's + * `bulkActions` makes the same action a row entry AND a bulk entry. */ const MARK_DONE = { name: 'bulk_mark_done', @@ -65,10 +63,9 @@ const MARK_DONE = { target: '/api/demo/mark-done', recordIdParam: 'taskId', locations: ['list_item'], - bulkEnabled: true, }; -/** No `bulkEnabled` — the VIEW names it in legacy `bulkActions` instead. */ +/** Surfaces only on the record overflow menu — until the view names it. */ const RECALC = { name: 'bulk_recalc_estimate', label: 'Recalculate Estimate', @@ -171,8 +168,8 @@ const schema: any = { { field: 'progress', label: 'Progress' }, ], pagination: { pageSize: 50 }, - // Legacy bare names: one resolves to a declared action, one doesn't. - bulkActions: ['bulk_recalc_estimate', 'legacy_only_handler'], + // Bare names: two resolve to declared actions, one doesn't. + bulkActions: ['bulk_mark_done', 'bulk_recalc_estimate', 'legacy_only_handler'], }; function Demo() { @@ -185,9 +182,9 @@ function Demo() { Object-declared bulk actions — objectui#3002

- Select rows → the bar shows Mark Done (derived from the object's - bulkEnabled: true), Recalculate Estimate (legacy name promoted to - its declared action) and Legacy Only Handler (unresolvable, dispatched by name). + Select rows → the bar shows Mark Done and Recalculate Estimate (names + from the view's bulkActions, promoted to their declared object + actions) and Legacy Only Handler (unresolvable, dispatched by name).

diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 799882070..a925f1346 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -1634,10 +1634,9 @@ export const ObjectGrid: React.FC = ({ const explicitBulkActions = objectCanDelete ? declaredBulkActions : declaredBulkActions?.filter((a: unknown) => String(a).toLowerCase() !== 'delete'); - // Legacy `bulkActions` carry a bare action NAME, which the runner cannot - // execute on its own, and the object's own `bulkEnabled: true` actions were - // never surfaced at all — resolve both against `objectDef.actions` so they - // dispatch as real defs. See `resolveBulkActions` (objectui#3002). The + // `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 // 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`. @@ -1746,7 +1745,7 @@ export const ObjectGrid: React.FC = ({ })(); }; - // Per-record executor for a DERIVED bulk action (objectui#3002) — one + // Per-record executor for a PROMOTED bulk action (objectui#3002) — one // declared object action applied to each selected record through the action // runner. The dialog already collected params and took the confirmation, so // this dispatch strips both from the def: leaving them on would make the diff --git a/packages/plugin-grid/src/__tests__/objectBulkActionDispatch.test.tsx b/packages/plugin-grid/src/__tests__/objectBulkActionDispatch.test.tsx index b44f672fe..e4aa71927 100644 --- a/packages/plugin-grid/src/__tests__/objectBulkActionDispatch.test.tsx +++ b/packages/plugin-grid/src/__tests__/objectBulkActionDispatch.test.tsx @@ -7,16 +7,20 @@ */ /** - * An object-declared bulk action must reach the ActionRunner as a real action + * A view's `bulkActions` name must reach the ActionRunner as a real action * DEF, applied to every selected record (objectui#3002). * * Before this, `bulkActions: ['push_down']` dispatched the action NAME in the * runner's `type` slot — `{ type: 'push_down', params: { records } }` — which * matches no built-in type and no handler, so it fell through the runner's * schema fallback (green success toast pre-#2996, a loud failure after it). - * Either way the action never ran. And an object could not declare a bulk - * action at all: `bulkActionDefs` was passed through from view JSON verbatim, - * never derived from `objectDef.actions`. + * Either way the action never ran, and the button carried none of the def's + * label / icon / params: `bulkActionDefs` was passed through from view JSON + * verbatim, never resolved against `objectDef.actions`. + * + * Naming the action in the view is the ONLY way to declare a bulk action — + * spec 17 retired `action.bulkEnabled` as a tombstone (framework#4054) and + * prescribes exactly this. See `resolveBulkActions`. * * These drive the REAL ObjectGrid through a real ActionProvider and assert the * user-visible outcome: selecting rows and clicking the button issues one @@ -51,9 +55,10 @@ const ROWS = [ ]; /** - * The object's own bulk action. The label is deliberately NOT the humanization + * The object's declared action. The label is deliberately NOT the humanization * of the name ("Push Down"), so an assertion on it distinguishes the resolved - * def from the legacy path — which could only ever render `formatActionLabel`. + * def from the by-name path — which could only ever render + * `formatActionLabel`. */ const PUSH_DOWN = { name: 'push_down', @@ -61,7 +66,6 @@ const PUSH_DOWN = { type: 'api', target: '/api/v1/plans/push', recordIdParam: 'planId', - bulkEnabled: true, }; let fetchMock: ReturnType; @@ -121,17 +125,32 @@ afterEach(() => { vi.unstubAllGlobals(); }); -describe('object-declared bulk actions (objectui#3002)', () => { - it('surfaces a bulkEnabled object action with no view declaration at all', async () => { - await renderAndSelectAll({ objectActions: [PUSH_DOWN] }); +describe('view-declared bulk actions (objectui#3002)', () => { + it('renders the declared action label, not the humanized name', async () => { + await renderAndSelectAll({ + objectActions: [PUSH_DOWN], + schema: { bulkActions: ['push_down'] }, + }); const button = await screen.findByTestId('bulk-action-push_down'); // The object action's own label, not `formatActionLabel('push_down')`. expect(button).toHaveTextContent('下推'); }); + it('surfaces nothing for an object action the view never names', async () => { + // `action.bulkEnabled` is a spec-17 tombstone, so there is no object-level + // opt-in: an unnamed action must not reach the selection bar. + renderGrid({ objectActions: [{ ...PUSH_DOWN, bulkEnabled: true }] }); + await waitFor(() => expect(screen.getByText('Plan A')).toBeInTheDocument()); + + expect(screen.queryByTestId('bulk-action-push_down')).not.toBeInTheDocument(); + }); + it('issues one request per selected record instead of no-opping', async () => { - await renderAndSelectAll({ objectActions: [PUSH_DOWN] }); + await renderAndSelectAll({ + objectActions: [PUSH_DOWN], + schema: { bulkActions: ['push_down'] }, + }); fireEvent.click(await screen.findByTestId('bulk-action-push_down')); // No params on this action → the dialog opens straight on confirm. @@ -148,22 +167,6 @@ describe('object-declared bulk actions (objectui#3002)', () => { expect(sentIds).toEqual(['r1', 'r2']); }); - it('resolves a legacy bulkActions name to the declared action', async () => { - // The reported shape: the object never flagged the action, the view names it. - const unflagged = { ...PUSH_DOWN, bulkEnabled: undefined }; - await renderAndSelectAll({ - objectActions: [unflagged], - schema: { bulkActions: ['push_down'] }, - }); - - const button = await screen.findByTestId('bulk-action-push_down'); - expect(button).toHaveTextContent('下推'); - - fireEvent.click(button); - fireEvent.click(await screen.findByRole('button', { name: 'Run' })); - await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)); - }); - it('reports per-record failures instead of counting them as successes', async () => { fetchMock.mockResolvedValue({ ok: false, @@ -172,7 +175,10 @@ describe('object-declared bulk actions (objectui#3002)', () => { headers: { get: () => 'application/json' }, json: async () => ({ error: 'precondition not met' }), }); - await renderAndSelectAll({ objectActions: [PUSH_DOWN] }); + await renderAndSelectAll({ + objectActions: [PUSH_DOWN], + schema: { bulkActions: ['push_down'] }, + }); fireEvent.click(await screen.findByTestId('bulk-action-push_down')); fireEvent.click(await screen.findByRole('button', { name: 'Run' })); @@ -184,27 +190,27 @@ describe('object-declared bulk actions (objectui#3002)', () => { await waitFor(() => expect(screen.getByText(/Succeeded 0 \/ 2/)).toBeInTheDocument()); expect(screen.getByTestId('bulk-error-row-r1')).toHaveTextContent('HTTP 422'); expect(screen.getByTestId('bulk-error-row-r2')).toBeInTheDocument(); - // A derived def re-dispatches for one record, so the row keeps its Retry. + // A promoted def re-dispatches for one record, so the row keeps its Retry. expect(screen.getByTestId('bulk-error-retry-r1')).toBeInTheDocument(); }); - it('renders one button when a legacy name repeats a derived action', async () => { + it('renders one button for a repeated name', async () => { await renderAndSelectAll({ objectActions: [PUSH_DOWN], - schema: { bulkActions: ['push_down'] }, + schema: { bulkActions: ['push_down', 'push_down'] }, }); await waitFor(() => expect(screen.getAllByTestId('bulk-action-push_down')).toHaveLength(1)); }); - it('keeps an unresolvable legacy name alongside the derived defs', async () => { + 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. const handler = vi.fn(async () => ({ success: true })); await renderAndSelectAll({ objectActions: [PUSH_DOWN], - schema: { bulkActions: ['crm_only_handler'] }, + schema: { bulkActions: ['push_down', 'crm_only_handler'] }, handlers: { crm_only_handler: handler }, }); diff --git a/packages/plugin-grid/src/__tests__/resolveBulkActions.test.ts b/packages/plugin-grid/src/__tests__/resolveBulkActions.test.ts index 75a2683fd..386173234 100644 --- a/packages/plugin-grid/src/__tests__/resolveBulkActions.test.ts +++ b/packages/plugin-grid/src/__tests__/resolveBulkActions.test.ts @@ -7,7 +7,7 @@ */ import { describe, it, expect } from 'vitest'; -import { resolveBulkActions, DERIVED_BULK_BATCH_SIZE } from '../resolveBulkActions'; +import { resolveBulkActions, PROMOTED_BULK_BATCH_SIZE } from '../resolveBulkActions'; import type { BulkActionDef } from '@object-ui/types'; const PUSH_DOWN = { @@ -15,7 +15,6 @@ const PUSH_DOWN = { label: '下推', type: 'api', target: '/api/v1/plans/push', - bulkEnabled: true, }; const AUTHORED: BulkActionDef = { @@ -26,8 +25,14 @@ const AUTHORED: BulkActionDef = { }; describe('resolveBulkActions', () => { - it('derives a def from an object action declaring bulkEnabled', () => { - const { defs, unresolved } = resolveBulkActions({ objectActions: [PUSH_DOWN] }); + it('promotes a named action to its declared def', () => { + // 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({ + bulkActions: ['push_down'], + objectActions: [PUSH_DOWN], + }); expect(unresolved).toEqual([]); expect(defs).toHaveLength(1); @@ -36,53 +41,38 @@ describe('resolveBulkActions', () => { label: '下推', operation: 'custom', actionDef: PUSH_DOWN, - batchSize: DERIVED_BULK_BATCH_SIZE, + batchSize: PROMOTED_BULK_BATCH_SIZE, }); }); - it('leaves an action alone that never opted into bulk', () => { + 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] }); + const { defs, unresolved } = resolveBulkActions({ objectActions: [rowOnly, PUSH_DOWN] }); expect(defs).toEqual([]); expect(unresolved).toEqual([]); }); - it('promotes a legacy name to its declared action even without bulkEnabled', () => { - // Naming the action in the view's `bulkActions` IS the declaration of - // intent at the view level — same rule the row fold applies to `rowActions`. - const noFlag = { name: 'dispatch_job', label: '派工', type: 'api', target: '/x' }; - const { defs, unresolved } = resolveBulkActions({ - bulkActions: ['dispatch_job'], - objectActions: [noFlag], - }); + 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] }); + expect(defs).toEqual([]); expect(unresolved).toEqual([]); - expect(defs).toHaveLength(1); - expect(defs[0]).toMatchObject({ name: 'dispatch_job', operation: 'custom', actionDef: noFlag }); }); it('keeps a name that matches no declared action for by-name dispatch', () => { const { defs, unresolved } = resolveBulkActions({ - bulkActions: ['crm_only_handler'], + bulkActions: ['push_down', 'crm_only_handler'], objectActions: [PUSH_DOWN], }); - // push_down still derives from its own flag; the unknown name stays legacy. expect(defs.map(d => d.name)).toEqual(['push_down']); expect(unresolved).toEqual(['crm_only_handler']); }); - it('does not render a legacy name twice when it also derives from bulkEnabled', () => { - const { defs, unresolved } = resolveBulkActions({ - bulkActions: ['push_down'], - objectActions: [PUSH_DOWN], - }); - - expect(defs.map(d => d.name)).toEqual(['push_down']); - expect(unresolved).toEqual([]); - }); - it('lets an inline-authored def win over the object action of the same name', () => { const authoredPush: BulkActionDef = { name: 'push_down', operation: 'update', patch: { s: 1 } }; const { defs } = resolveBulkActions({ @@ -94,7 +84,7 @@ describe('resolveBulkActions', () => { expect(defs).toEqual([authoredPush]); }); - it('drops a repeated legacy name', () => { + it('drops a repeated name', () => { const { defs, unresolved } = resolveBulkActions({ bulkActions: ['dispatch_job', 'dispatch_job', 'ghost', 'ghost'], objectActions: [{ name: 'dispatch_job' }], @@ -114,7 +104,6 @@ describe('resolveBulkActions', () => { it('maps spec param keys onto the dialog vocabulary', () => { const withParams = { name: 'reassign', - bulkEnabled: true, params: [ { field: 'owner', @@ -129,7 +118,10 @@ describe('resolveBulkActions', () => { { label: 'orphan' }, ], }; - const { defs } = resolveBulkActions({ objectActions: [withParams] }); + const { defs } = resolveBulkActions({ + bulkActions: ['reassign'], + objectActions: [withParams], + }); expect(defs[0].params).toEqual([ expect.objectContaining({ @@ -145,8 +137,11 @@ describe('resolveBulkActions', () => { }); it('drops a non-string I18nLabel rather than handing an object to the renderer', () => { - const mapLabel = { name: 'push_down', bulkEnabled: true, label: { en: 'Push', 'zh-CN': '下推' } }; - const { defs } = resolveBulkActions({ objectActions: [mapLabel] }); + const mapLabel = { name: 'push_down', label: { en: 'Push', 'zh-CN': '下推' } }; + const { defs } = resolveBulkActions({ + bulkActions: ['push_down'], + objectActions: [mapLabel], + }); expect(defs[0].label).toBeUndefined(); }); @@ -154,13 +149,15 @@ describe('resolveBulkActions', () => { it('carries confirm text, icon and visible from the action', () => { const rich = { name: 'push_down', - bulkEnabled: true, icon: 'send', variant: 'danger', confirm: { message: '确认下推?' }, visible: { dialect: 'cel', source: 'current_user.is_admin' }, }; - const { defs } = resolveBulkActions({ objectActions: [rich] }); + const { defs } = resolveBulkActions({ + bulkActions: ['push_down'], + objectActions: [rich], + }); expect(defs[0]).toMatchObject({ icon: 'send', @@ -171,14 +168,18 @@ describe('resolveBulkActions', () => { }); it('drops an action variant the bulk bar cannot render', () => { - const linkVariant = { name: 'push_down', bulkEnabled: true, variant: 'link' }; - const { defs } = resolveBulkActions({ objectActions: [linkVariant] }); + const linkVariant = { name: 'push_down', variant: 'link' }; + const { defs } = resolveBulkActions({ + bulkActions: ['push_down'], + objectActions: [linkVariant], + }); expect(defs[0].variant).toBeUndefined(); }); - it('localizes derived labels through the supplied resolver', () => { + it('localizes promoted labels through the supplied resolver', () => { const { defs } = resolveBulkActions({ + bulkActions: ['push_down'], objectActions: [PUSH_DOWN], localizeLabel: (name, fallback) => (name === 'push_down' ? 'Push Down' : fallback), }); diff --git a/packages/plugin-grid/src/components/BulkActionDialog.tsx b/packages/plugin-grid/src/components/BulkActionDialog.tsx index 79dc3f419..7d7809d0f 100644 --- a/packages/plugin-grid/src/components/BulkActionDialog.tsx +++ b/packages/plugin-grid/src/components/BulkActionDialog.tsx @@ -73,7 +73,7 @@ export interface BulkActionDialogProps { */ objectFields?: Record; /** - * Per-record dispatcher for a def DERIVED from an object action + * Per-record dispatcher for a def PROMOTED from an object action * (objectui#3002) — see {@link BulkExecutorOptions.runAction}. The dialog's * params → confirm steps are what let one action run over N records without * the runner re-prompting per record. @@ -432,7 +432,7 @@ export const BulkActionDialog: React.FC = ({ {/* A plain `custom` callout has nothing to re-run, but - a DERIVED def (#3002) re-dispatches its object + a PROMOTED def (#3002) re-dispatches its object action for that one record — same as update/delete. */} {(def.operation !== 'custom' || !!def.actionDef) && (