Skip to content

Commit 19e9fa0

Browse files
os-zhuangclaude
andauthored
fix(grid): drop the bulkEnabled derivation — the spec key is a tombstone (#3002) (#3053)
Follow-up to #3031. `@objectstack/spec` 17.0.0 retired `action.bulkEnabled` in the #3896 audit close-out (framework#4054), which 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. The derivation branch could never run, and #3031's changeset pointed authors at a key that breaks their app. Browser verification against a real showcase backend is what surfaced it. The tombstone prescribes the surviving path — "declare the action in the view's `bulkActions` instead" — which is #3031's other half, and the half the end-to-end run exercised: naming `showcase_mark_done` in the view 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. A stale `bulkEnabled: true` is now inert rather than a second path into the bar. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 2307b52 commit 19e9fa0

10 files changed

Lines changed: 208 additions & 172 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@object-ui/plugin-grid": patch
3+
"@object-ui/types": patch
4+
"@object-ui/app-shell": patch
5+
---
6+
7+
fix(grid): drop the `bulkEnabled` derivation — the spec key is a tombstone
8+
9+
Follow-up to objectui#3002 / #3031. That change folded two sources into the
10+
selection bar: a view's `bulkActions` names resolved against
11+
`objectDef.actions`, and object actions declaring `ActionSchema.bulkEnabled`.
12+
The second source is dead.
13+
14+
`@objectstack/spec` 17.0.0 retired `action.bulkEnabled` in the #3896 audit
15+
close-out (framework#4054, landed while #3031 was in flight — the spec source
16+
still carried the key when its design was settled). It is now a `retiredKey()`
17+
tombstone, so it is not merely ignored: `defineStack` **hard-rejects** a config
18+
that sets it, and the backend refuses to boot. Browser verification against a
19+
real showcase backend is what surfaced this — the derivation branch could never
20+
run, and #3031's changeset pointed authors at a key that breaks their app.
21+
22+
The tombstone's own prescription is the path that survives:
23+
24+
> the multi-select toolbar is driven by the LIST VIEW's `bulkActions` /
25+
> `bulkActionDefs`, never by this flag … declare the action in the view's
26+
> `bulkActions` instead.
27+
28+
So `resolveBulkActions` now folds exactly two vocabularies — inline-authored
29+
`bulkActionDefs`, and `bulkActions` names promoted to their declared object
30+
action — which is what #3031's other half already did and what the end-to-end
31+
run exercised: naming `showcase_mark_done` in the view's `bulkActions` issued
32+
one `POST /api/v1/actions/showcase_task/showcase_mark_done` per selected
33+
record (10/10 → `done: true, progress: 100` server-side). Everything downstream
34+
of the fold is unchanged: promoted defs still carry the action's label, icon,
35+
`visible`, confirm text and params; still run through `BulkActionDialog`
36+
(params → confirm → progress → result); still dispatch per record with
37+
`_rowRecord` attached; still attribute failures per record.
38+
39+
A stale `bulkEnabled: true` on an object action is now inert rather than a
40+
second path into the bar. Note tsc cannot catch this class of drift here — the
41+
fold reads a loosely-typed `NamedActionDef` with an index signature, so the
42+
retired key never surfaces as `never`.

packages/app-shell/src/views/ObjectView.tsx

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1421,13 +1421,12 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an
14211421
: []),
14221422
/**
14231423
* Selection-bar actions. Unlike `rowActionDefs` above, these are
1424-
* NOT derived here: `ObjectGrid` is the single convergence point of
1425-
* all three list callers (this view, plugin-view's ObjectView and
1426-
* plugin-list's ListView), so it folds `objectDef.actions` — the
1427-
* spec's `bulkEnabled: true` declaration, plus any legacy
1428-
* `bulkActions` name that resolves to a declared action — into the
1429-
* def list itself (`resolveBulkActions`, objectui#3002). What we
1430-
* pass through is the view author's own inline declaration.
1424+
* NOT resolved here: `ObjectGrid` is the single convergence point
1425+
* of all three list callers (this view, plugin-view's ObjectView
1426+
* and plugin-list's ListView), so it resolves each `bulkActions`
1427+
* name against `objectDef.actions` and promotes it into the def
1428+
* list itself (`resolveBulkActions`, objectui#3002). What we pass
1429+
* through is the view author's own declaration.
14311430
*/
14321431
bulkActions: viewDef.bulkActions ?? listSchema.bulkActions,
14331432
bulkActionDefs: (viewDef as any).bulkActionDefs ?? (listSchema as any).bulkActionDefs,

packages/plugin-grid/demo/bulk-actions.tsx

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,14 @@
1111
* on-page panel instead of hitting a server).
1212
*
1313
* What it exercises:
14-
* 1. `bulk_mark_done` is declared on the OBJECT with `bulkEnabled: true` and
15-
* is named by no view — before #3002 an object simply could not express a
16-
* bulk action, so this button did not exist.
17-
* 2. `bulk_recalc_estimate` is declared on the object WITHOUT the flag; the
18-
* view names it in legacy `bulkActions: [...]`. That name used to be
19-
* dispatched as `{ type: 'bulk_recalc_estimate' }` and never ran.
20-
* 3. `legacy_only_handler` resolves to no declared action and stays a
14+
* 1. `bulk_mark_done` and `bulk_recalc_estimate` are declared on the object
15+
* and NAMED in the view's `bulkActions` — the only way to declare a bulk
16+
* action (spec 17 retired `action.bulkEnabled` as a tombstone). Both
17+
* names used to dispatch as `{ type: '<name>' }` and never run; the
18+
* buttons also carried none of the def's label / icon / params.
19+
* 2. `legacy_only_handler` resolves to no declared action and stays a
2120
* by-name dispatch — it must still render ALONGSIDE the two defs above.
22-
* 4. Record `t3` fails server-side (422), proving per-record attribution: the
21+
* 3. Record `t3` fails server-side (422), proving per-record attribution: the
2322
* result panel reports 4/5 with an error row rather than a blanket success.
2423
*/
2524
import * as React from 'react';
@@ -53,9 +52,8 @@ const ROWS = [
5352
];
5453

5554
/**
56-
* `bulkEnabled: true` is the spec's object-level declaration — "this action can
57-
* be applied to multiple selected records". It also carries `locations:
58-
* ['list_item']`, so the same action is a row entry AND a bulk entry.
55+
* Also carries `locations: ['list_item']`, so naming it in the view's
56+
* `bulkActions` makes the same action a row entry AND a bulk entry.
5957
*/
6058
const MARK_DONE = {
6159
name: 'bulk_mark_done',
@@ -65,10 +63,9 @@ const MARK_DONE = {
6563
target: '/api/demo/mark-done',
6664
recordIdParam: 'taskId',
6765
locations: ['list_item'],
68-
bulkEnabled: true,
6966
};
7067

71-
/** No `bulkEnabled` — the VIEW names it in legacy `bulkActions` instead. */
68+
/** Surfaces only on the record overflow menu — until the view names it. */
7269
const RECALC = {
7370
name: 'bulk_recalc_estimate',
7471
label: 'Recalculate Estimate',
@@ -171,8 +168,8 @@ const schema: any = {
171168
{ field: 'progress', label: 'Progress' },
172169
],
173170
pagination: { pageSize: 50 },
174-
// Legacy bare names: one resolves to a declared action, one doesn't.
175-
bulkActions: ['bulk_recalc_estimate', 'legacy_only_handler'],
171+
// Bare names: two resolve to declared actions, one doesn't.
172+
bulkActions: ['bulk_mark_done', 'bulk_recalc_estimate', 'legacy_only_handler'],
176173
};
177174

178175
function Demo() {
@@ -185,9 +182,9 @@ function Demo() {
185182
Object-declared bulk actions — objectui#3002
186183
</h1>
187184
<p style={{ fontSize: 12, color: 'hsl(var(--muted-foreground))', margin: '4px 0 0' }}>
188-
Select rows → the bar shows <b>Mark Done</b> (derived from the object&apos;s
189-
<code> bulkEnabled: true</code>), <b>Recalculate Estimate</b> (legacy name promoted to
190-
its declared action) and <b>Legacy Only Handler</b> (unresolvable, dispatched by name).
185+
Select rows → the bar shows <b>Mark Done</b> and <b>Recalculate Estimate</b> (names
186+
from the view&apos;s <code>bulkActions</code>, promoted to their declared object
187+
actions) and <b>Legacy Only Handler</b> (unresolvable, dispatched by name).
191188
</p>
192189
</header>
193190

packages/plugin-grid/src/ObjectGrid.tsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1634,10 +1634,9 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
16341634
const explicitBulkActions = objectCanDelete
16351635
? declaredBulkActions
16361636
: declaredBulkActions?.filter((a: unknown) => String(a).toLowerCase() !== 'delete');
1637-
// Legacy `bulkActions` carry a bare action NAME, which the runner cannot
1638-
// execute on its own, and the object's own `bulkEnabled: true` actions were
1639-
// never surfaced at all — resolve both against `objectDef.actions` so they
1640-
// dispatch as real defs. See `resolveBulkActions` (objectui#3002). The
1637+
// `bulkActions` carries a bare action NAME, which the runner cannot execute
1638+
// on its own — resolve each against `objectDef.actions` so it dispatches as a
1639+
// real def. See `resolveBulkActions` (objectui#3002). The
16411640
// canonical `'delete'` is held back: it routes to `onBulkDelete` (which owns
16421641
// confirm + refresh), not the runner, even if the object declares an action
16431642
// that happens to be named `delete`.
@@ -1746,7 +1745,7 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
17461745
})();
17471746
};
17481747

1749-
// Per-record executor for a DERIVED bulk action (objectui#3002) — one
1748+
// Per-record executor for a PROMOTED bulk action (objectui#3002) — one
17501749
// declared object action applied to each selected record through the action
17511750
// runner. The dialog already collected params and took the confirmation, so
17521751
// this dispatch strips both from the def: leaving them on would make the

packages/plugin-grid/src/__tests__/objectBulkActionDispatch.test.tsx

Lines changed: 39 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,20 @@
77
*/
88

99
/**
10-
* An object-declared bulk action must reach the ActionRunner as a real action
10+
* A view's `bulkActions` name must reach the ActionRunner as a real action
1111
* DEF, applied to every selected record (objectui#3002).
1212
*
1313
* Before this, `bulkActions: ['push_down']` dispatched the action NAME in the
1414
* runner's `type` slot — `{ type: 'push_down', params: { records } }` — which
1515
* matches no built-in type and no handler, so it fell through the runner's
1616
* schema fallback (green success toast pre-#2996, a loud failure after it).
17-
* Either way the action never ran. And an object could not declare a bulk
18-
* action at all: `bulkActionDefs` was passed through from view JSON verbatim,
19-
* never derived from `objectDef.actions`.
17+
* Either way the action never ran, and the button carried none of the def's
18+
* label / icon / params: `bulkActionDefs` was passed through from view JSON
19+
* verbatim, never resolved against `objectDef.actions`.
20+
*
21+
* Naming the action in the view is the ONLY way to declare a bulk action —
22+
* spec 17 retired `action.bulkEnabled` as a tombstone (framework#4054) and
23+
* prescribes exactly this. See `resolveBulkActions`.
2024
*
2125
* These drive the REAL ObjectGrid through a real ActionProvider and assert the
2226
* user-visible outcome: selecting rows and clicking the button issues one
@@ -51,17 +55,17 @@ const ROWS = [
5155
];
5256

5357
/**
54-
* The object's own bulk action. The label is deliberately NOT the humanization
58+
* The object's declared action. The label is deliberately NOT the humanization
5559
* of the name ("Push Down"), so an assertion on it distinguishes the resolved
56-
* def from the legacy path — which could only ever render `formatActionLabel`.
60+
* def from the by-name path — which could only ever render
61+
* `formatActionLabel`.
5762
*/
5863
const PUSH_DOWN = {
5964
name: 'push_down',
6065
label: '下推',
6166
type: 'api',
6267
target: '/api/v1/plans/push',
6368
recordIdParam: 'planId',
64-
bulkEnabled: true,
6569
};
6670

6771
let fetchMock: ReturnType<typeof vi.fn>;
@@ -121,17 +125,32 @@ afterEach(() => {
121125
vi.unstubAllGlobals();
122126
});
123127

124-
describe('object-declared bulk actions (objectui#3002)', () => {
125-
it('surfaces a bulkEnabled object action with no view declaration at all', async () => {
126-
await renderAndSelectAll({ objectActions: [PUSH_DOWN] });
128+
describe('view-declared bulk actions (objectui#3002)', () => {
129+
it('renders the declared action label, not the humanized name', async () => {
130+
await renderAndSelectAll({
131+
objectActions: [PUSH_DOWN],
132+
schema: { bulkActions: ['push_down'] },
133+
});
127134

128135
const button = await screen.findByTestId('bulk-action-push_down');
129136
// The object action's own label, not `formatActionLabel('push_down')`.
130137
expect(button).toHaveTextContent('下推');
131138
});
132139

140+
it('surfaces nothing for an object action the view never names', async () => {
141+
// `action.bulkEnabled` is a spec-17 tombstone, so there is no object-level
142+
// opt-in: an unnamed action must not reach the selection bar.
143+
renderGrid({ objectActions: [{ ...PUSH_DOWN, bulkEnabled: true }] });
144+
await waitFor(() => expect(screen.getByText('Plan A')).toBeInTheDocument());
145+
146+
expect(screen.queryByTestId('bulk-action-push_down')).not.toBeInTheDocument();
147+
});
148+
133149
it('issues one request per selected record instead of no-opping', async () => {
134-
await renderAndSelectAll({ objectActions: [PUSH_DOWN] });
150+
await renderAndSelectAll({
151+
objectActions: [PUSH_DOWN],
152+
schema: { bulkActions: ['push_down'] },
153+
});
135154

136155
fireEvent.click(await screen.findByTestId('bulk-action-push_down'));
137156
// No params on this action → the dialog opens straight on confirm.
@@ -148,22 +167,6 @@ describe('object-declared bulk actions (objectui#3002)', () => {
148167
expect(sentIds).toEqual(['r1', 'r2']);
149168
});
150169

151-
it('resolves a legacy bulkActions name to the declared action', async () => {
152-
// The reported shape: the object never flagged the action, the view names it.
153-
const unflagged = { ...PUSH_DOWN, bulkEnabled: undefined };
154-
await renderAndSelectAll({
155-
objectActions: [unflagged],
156-
schema: { bulkActions: ['push_down'] },
157-
});
158-
159-
const button = await screen.findByTestId('bulk-action-push_down');
160-
expect(button).toHaveTextContent('下推');
161-
162-
fireEvent.click(button);
163-
fireEvent.click(await screen.findByRole('button', { name: 'Run' }));
164-
await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2));
165-
});
166-
167170
it('reports per-record failures instead of counting them as successes', async () => {
168171
fetchMock.mockResolvedValue({
169172
ok: false,
@@ -172,7 +175,10 @@ describe('object-declared bulk actions (objectui#3002)', () => {
172175
headers: { get: () => 'application/json' },
173176
json: async () => ({ error: 'precondition not met' }),
174177
});
175-
await renderAndSelectAll({ objectActions: [PUSH_DOWN] });
178+
await renderAndSelectAll({
179+
objectActions: [PUSH_DOWN],
180+
schema: { bulkActions: ['push_down'] },
181+
});
176182

177183
fireEvent.click(await screen.findByTestId('bulk-action-push_down'));
178184
fireEvent.click(await screen.findByRole('button', { name: 'Run' }));
@@ -184,27 +190,27 @@ describe('object-declared bulk actions (objectui#3002)', () => {
184190
await waitFor(() => expect(screen.getByText(/Succeeded 0 \/ 2/)).toBeInTheDocument());
185191
expect(screen.getByTestId('bulk-error-row-r1')).toHaveTextContent('HTTP 422');
186192
expect(screen.getByTestId('bulk-error-row-r2')).toBeInTheDocument();
187-
// A derived def re-dispatches for one record, so the row keeps its Retry.
193+
// A promoted def re-dispatches for one record, so the row keeps its Retry.
188194
expect(screen.getByTestId('bulk-error-retry-r1')).toBeInTheDocument();
189195
});
190196

191-
it('renders one button when a legacy name repeats a derived action', async () => {
197+
it('renders one button for a repeated name', async () => {
192198
await renderAndSelectAll({
193199
objectActions: [PUSH_DOWN],
194-
schema: { bulkActions: ['push_down'] },
200+
schema: { bulkActions: ['push_down', 'push_down'] },
195201
});
196202

197203
await waitFor(() => expect(screen.getAllByTestId('bulk-action-push_down')).toHaveLength(1));
198204
});
199205

200-
it('keeps an unresolvable legacy name alongside the derived defs', async () => {
206+
it('keeps an unresolvable name alongside the promoted defs', async () => {
201207
// A name the object never declared may still have a runner handler
202208
// registered under it; hiding it because some OTHER def exists dropped
203209
// half the bar's buttons.
204210
const handler = vi.fn(async () => ({ success: true }));
205211
await renderAndSelectAll({
206212
objectActions: [PUSH_DOWN],
207-
schema: { bulkActions: ['crm_only_handler'] },
213+
schema: { bulkActions: ['push_down', 'crm_only_handler'] },
208214
handlers: { crm_only_handler: handler },
209215
});
210216

0 commit comments

Comments
 (0)