Skip to content

Commit 5340879

Browse files
baozhoutaoclaude
andauthored
fix(plugin-grid): bulk-action params render the shared form field widgets — lookup errors get Retry, sys_user params get the PeoplePicker (#3064, ADR-0059) (#3073)
BulkActionDialog's params step was a hand-rolled 2026-05 MVP: lookup params eagerly fetched find(object, {$top:200}) once per param name, swallowed any fetch error into an empty option list, rendered that emptiness as a permanent "Loading…" placeholder, and cached the failure per param name so reopening the dialog never retried (#3064). It also predated the PeoplePicker (#2112), so a person param looked nothing like the form's person field. Params now render through the SAME field-widget renderer the object form and ActionParamDialog use (completing ADR-0059 for the bulk surface): a new pure bulkParamToField() adapter translates each BulkActionParam to field metadata, getLazyFieldWidget() supplies the lazy widget, and only picker widgets get the grid DataSource threaded in. A sys_user-targeting lookup (or a user-typed param) is promoted to the form's search-first PeoplePicker; other lookups get the searchable LookupField whose useRecordQuery kernel owns loading/error/ empty states and refetches on every open — the #3064 pipeline (eager prefetch → swallowed error → permanent Loading… → poisoned cache) is gone by construction. Confirm-step labels are resolved per selected id via findOne, replacing the removed candidate prefetch; #2204 schema-fallback multi semantics, required gating, and the #2185 popper guard are preserved (all pinned by tests, plus browser-verified via the demo's new picker defs and ?failUsers=1 failure toggle). Closes #3064 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 2bb1809 commit 5340879

7 files changed

Lines changed: 572 additions & 270 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
"@object-ui/plugin-grid": patch
3+
---
4+
5+
fix(grid): bulk-action params render the shared form field widgets — a failed lookup fetch shows an error + Retry instead of a permanent "Loading…" (#3064, ADR-0059)
6+
7+
`BulkActionDialog`'s hand-rolled param controls (a 2026-05 MVP predating the
8+
PeoplePicker and ADR-0059) are replaced by the same field-widget renderer the
9+
object form and `ActionParamDialog` use, via a new pure `bulkParamToField()`
10+
adapter + `getLazyFieldWidget()`:
11+
12+
- `lookup` params get the real searchable `LookupField` (server-side search,
13+
record-picker dialog, loading/error/empty states owned by `useRecordQuery`);
14+
a `sys_user` target — or a `user`-typed param — is promoted to the form's
15+
search-first PeoplePicker (avatar + subtitle rows, recents, banned users
16+
excluded). Every other param type (date/datetime/boolean/select/multiselect/
17+
textarea/number/…) renders its form widget too, so param support can no
18+
longer drift behind the form surface.
19+
- The #3064 failure pipeline is gone by construction: no more eager
20+
`find($top:200)` prefetch on open, no error swallowed into an empty option
21+
list rendering as permanent "Loading…", and no per-param failure cache —
22+
reopening or Retry refetches.
23+
- Preserved semantics: #2204 schema-fallback multi-value detection, required
24+
gating, #2185 nested-popper dismissal guard, and human-readable confirm-step
25+
labels (now resolved per selected id via `findOne`, replacing the removed
26+
candidate prefetch).

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

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,13 @@
2020
* by-name dispatch — it must still render ALONGSIDE the two defs above.
2121
* 3. Record `t3` fails server-side (422), proving per-record attribution: the
2222
* result panel reports 4/5 with an error row rather than a blanket success.
23+
* 4. Rich `bulkActionDefs` with picker params (#3064 / ADR-0059): `Set
24+
* Manager` (lookup → sys_user, single) and `Assign Executors` (lookup →
25+
* sys_user, multiple) must render the form's PeoplePicker; `Set Queue`
26+
* (lookup → demo_queue) the searchable LookupField. Open with
27+
* `?failUsers=1` to make the FIRST sys_user candidate fetch fail — the
28+
* picker must show an error + Retry (never a permanent "Loading…"), and
29+
* Retry / reopening must refetch.
2330
*/
2431
import * as React from 'react';
2532
import { createRoot } from 'react-dom/client';
@@ -90,11 +97,57 @@ const RECALC = {
9097
],
9198
};
9299

100+
/** Candidate directories for the picker-param defs (#3064). */
101+
const USERS = [
102+
{ id: 'u1', name: '现场工人-测试', email: 'worker@demo.dev' },
103+
{ id: 'u2', name: '工厂长-测试', email: 'chief@demo.dev' },
104+
{ id: 'u3', name: '质检员-测试', email: 'qa@demo.dev' },
105+
{ id: 'u4', name: 'Dev Admin', email: 'admin@demo.dev' },
106+
];
107+
const QUEUES = [
108+
{ id: 'q1', title: 'Frontline support' },
109+
{ id: 'q2', title: 'Escalations' },
110+
];
111+
112+
// `?failUsers=1` — the first sys_user candidate fetch rejects, later ones
113+
// succeed: the manual repro for #3064 (error + Retry, no failure caching).
114+
let failNextUserFetch = new URLSearchParams(location.search).has('failUsers');
115+
116+
function searchable(rows: Array<Record<string, any>>, params: any) {
117+
const q = typeof params?.$search === 'string' ? params.$search.trim().toLowerCase() : '';
118+
const data = q
119+
? rows.filter(r => Object.values(r).some(v => String(v).toLowerCase().includes(q)))
120+
: rows;
121+
return { data: data.map(r => ({ ...r })), total: data.length, hasMore: false, pageSize: 50 };
122+
}
123+
93124
const dataSource: any = {
94-
async find() {
125+
async find(resource: string, params: any) {
126+
if (resource === 'sys_user') {
127+
if (failNextUserFetch) {
128+
failNextUserFetch = false;
129+
throw new Error('sys_user directory unavailable (simulated)');
130+
}
131+
return searchable(USERS, params);
132+
}
133+
if (resource === 'demo_queue') return searchable(QUEUES, params);
95134
return { data: ROWS.map(r => ({ ...r })), total: ROWS.length, hasMore: false, pageSize: 50 };
96135
},
136+
async findOne(resource: string, id: string) {
137+
const pool = resource === 'sys_user' ? USERS : resource === 'demo_queue' ? QUEUES : ROWS;
138+
return pool.find(r => r.id === id) ?? null;
139+
},
140+
async update(resource: string, id: string, patch: Record<string, unknown>) {
141+
pushLog({ url: `(update ${resource})`, recordId: id, params: JSON.stringify(patch), status: 200 });
142+
return { id, ...patch };
143+
},
97144
async getObjectSchema(name: string) {
145+
if (name === 'sys_user') {
146+
return { name, label: 'User', fields: { id: { type: 'text' }, name: { type: 'text', label: 'Name' }, email: { type: 'email', label: 'Email' } } };
147+
}
148+
if (name === 'demo_queue') {
149+
return { name, label: 'Queue', fields: { id: { type: 'text' }, title: { type: 'text', label: 'Title' } } };
150+
}
98151
return {
99152
name,
100153
label: 'Task',
@@ -170,6 +223,33 @@ const schema: any = {
170223
pagination: { pageSize: 50 },
171224
// Bare names: two resolve to declared actions, one doesn't.
172225
bulkActions: ['bulk_mark_done', 'bulk_recalc_estimate', 'legacy_only_handler'],
226+
// Rich defs with picker params — the shared-field-widget surface (#3064).
227+
bulkActionDefs: [
228+
{
229+
name: 'set_manager',
230+
label: 'Set Manager',
231+
operation: 'update',
232+
params: [
233+
{ name: 'manager', label: 'Manager', type: 'lookup', object: 'sys_user', required: true },
234+
],
235+
},
236+
{
237+
name: 'assign_executors',
238+
label: 'Assign Executors',
239+
operation: 'update',
240+
params: [
241+
{ name: 'executors', label: 'Executors', type: 'lookup', object: 'sys_user', multiple: true, required: true },
242+
],
243+
},
244+
{
245+
name: 'set_queue',
246+
label: 'Set Queue',
247+
operation: 'update',
248+
params: [
249+
{ name: 'queue', label: 'Queue', type: 'lookup', object: 'demo_queue', labelField: 'title', required: true },
250+
],
251+
},
252+
],
173253
};
174254

175255
function Demo() {

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

Lines changed: 95 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,18 @@
77
*/
88

99
/**
10-
* BulkActionDialog param widgets (#2185): the bulk-edit dialog previously only
11-
* rendered single-value controls, so a multi-value backend field (e.g. a
12-
* multi-user `executors`) could not be set — the picker collapsed to one value
13-
* and overwrote the array. These tests pin the multi-select path end-to-end
14-
* (empty-array required-gating → pick many → array patch → label preview) plus
15-
* the new date widget.
10+
* BulkActionDialog param widgets.
11+
*
12+
* #2185/#2204 pinned the multi-value path (empty-array required-gating → pick
13+
* many → array patch → label preview) and the date widget. Since the ADR-0059
14+
* migration (#3064) params render through the SHARED form field widgets
15+
* (`@object-ui/fields` via `bulkParamToField`), so these tests now assert the
16+
* form-widget DOM (multiselect toggle chips, native date input) — and the new
17+
* lookup suite pins the #3064 contract itself: no eager candidate prefetch,
18+
* fetch errors surfaced with a Retry affordance, and no failure caching
19+
* (reopening the picker refetches).
1620
*/
17-
import { describe, it, expect, vi, beforeAll } from 'vitest';
21+
import { describe, it, expect, vi, beforeAll, beforeEach } from 'vitest';
1822
import { render, screen, waitFor, fireEvent } from '@testing-library/react';
1923
import '@testing-library/jest-dom';
2024
import React from 'react';
@@ -34,6 +38,20 @@ beforeAll(() => {
3438
}
3539
});
3640

41+
beforeEach(() => {
42+
// happy-dom lacks matchMedia; the lookup widgets' useIsMobile needs it.
43+
window.matchMedia = ((query: string) => ({
44+
matches: false,
45+
media: query,
46+
onchange: null,
47+
addEventListener: () => {},
48+
removeEventListener: () => {},
49+
addListener: () => {},
50+
removeListener: () => {},
51+
dispatchEvent: () => false,
52+
})) as any;
53+
});
54+
3755
function makeDataSource() {
3856
const update = vi.fn(async () => ({}));
3957
const del = vi.fn(async () => ({}));
@@ -77,10 +95,10 @@ describe('BulkActionDialog — multi-select param produces an array patch', () =
7795
const next = screen.getByRole('button', { name: 'Next' });
7896
expect(next).toBeDisabled();
7997

80-
// Open the multi-select popover and pick both options.
81-
fireEvent.click(screen.getByRole('combobox'));
82-
fireEvent.click(await screen.findByText('Red'));
83-
fireEvent.click(await screen.findByText('Blue'));
98+
// The shared MultiSelectField renders toggle chips (same as the form) —
99+
// await them through the lazy-widget Suspense boundary, then pick both.
100+
fireEvent.click(await screen.findByTestId('multiselect-option-red'));
101+
fireEvent.click(await screen.findByTestId('multiselect-option-blue'));
84102

85103
// Now valid → advance to confirm.
86104
await waitFor(() => expect(next).toBeEnabled());
@@ -100,7 +118,7 @@ describe('BulkActionDialog — multi-select param produces an array patch', () =
100118
});
101119

102120
describe('BulkActionDialog — date param renders a date picker', () => {
103-
it('renders a native date input rather than a free-text box', () => {
121+
it('renders a native date input rather than a free-text box', async () => {
104122
const ds = makeDataSource();
105123
const def: any = {
106124
name: 'set_due',
@@ -118,8 +136,70 @@ describe('BulkActionDialog — date param renders a date picker', () => {
118136
onClose={() => {}}
119137
/>,
120138
);
121-
// Radix Dialog portals its content to document.body, not the render root.
122-
const dateInput = document.querySelector('input[type="date"]');
123-
expect(dateInput).toBeInTheDocument();
139+
// Radix Dialog portals to document.body, and the widget loads lazily.
140+
await waitFor(() => {
141+
expect(document.querySelector('input[type="date"]')).toBeInTheDocument();
142+
});
143+
});
144+
});
145+
146+
describe('BulkActionDialog — lookup param uses the shared record picker (#3064)', () => {
147+
const lookupDef: any = {
148+
name: 'assign_queue',
149+
label: 'Assign queue',
150+
operation: 'update',
151+
params: [
152+
{ name: 'queue', label: 'Queue', type: 'lookup', object: 'queues', required: true },
153+
],
154+
};
155+
156+
it('does not prefetch candidates when the dialog opens', async () => {
157+
const ds = makeDataSource();
158+
ds.find = vi.fn(async () => ({ data: [], total: 0 }));
159+
render(
160+
<BulkActionDialog
161+
def={lookupDef}
162+
rows={[{ id: 'r1' }]}
163+
resource="thing"
164+
dataSource={ds}
165+
open
166+
onClose={() => {}}
167+
/>,
168+
);
169+
// The picker trigger renders (lazy widget) with no candidate query issued —
170+
// the old dialog fired find(object, {$top: 200}) on open.
171+
await screen.findByTestId('lookup-trigger-queue');
172+
expect(ds.find).not.toHaveBeenCalled();
173+
});
174+
175+
it('surfaces a failed candidate fetch and retries instead of caching the failure', async () => {
176+
const ds = makeDataSource();
177+
ds.find = vi
178+
.fn()
179+
.mockRejectedValueOnce(new Error('boom'))
180+
.mockResolvedValue({ data: [{ id: 'q1', name: 'Support queue' }], total: 1 });
181+
render(
182+
<BulkActionDialog
183+
def={lookupDef}
184+
rows={[{ id: 'r1' }]}
185+
resource="thing"
186+
dataSource={ds}
187+
open
188+
onClose={() => {}}
189+
/>,
190+
);
191+
192+
// Open the picker → the fetch fails → an error state with a Retry
193+
// affordance renders (previously: a permanent "Loading…" placeholder).
194+
fireEvent.click(await screen.findByTestId('lookup-trigger-queue'));
195+
const alert = await screen.findByRole('alert');
196+
expect(alert.textContent).toContain('boom');
197+
expect(ds.find).toHaveBeenCalledTimes(1);
198+
199+
// Retry refetches (previously: the failure was cached per param name and
200+
// no further request was ever issued) and the candidates render.
201+
fireEvent.click(screen.getByRole('button', { name: 'Retry' }));
202+
await screen.findByText('Support queue');
203+
expect(ds.find).toHaveBeenCalledTimes(2);
124204
});
125205
});

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,11 @@ describe('BulkActionDialog — schema fallback when param.multiple is omitted (#
127127
/>,
128128
);
129129

130-
// Schema says multi → the combobox (Popover multi-select) renders, not a
131-
// single-select trigger that collapses to one value.
132-
fireEvent.click(screen.getByRole('combobox'));
133-
fireEvent.click(await screen.findByText('Frontend'));
134-
fireEvent.click(await screen.findByText('Design'));
130+
// Schema says multi → the shared MultiSelectField (toggle chips) renders,
131+
// not a single-select trigger that collapses to one value.
132+
await screen.findByTestId('multiselect-labels');
133+
fireEvent.click(screen.getByTestId('multiselect-option-frontend'));
134+
fireEvent.click(screen.getByTestId('multiselect-option-design'));
135135

136136
const next = screen.getByRole('button', { name: 'Next' });
137137
await waitFor(() => expect(next).toBeEnabled());

0 commit comments

Comments
 (0)