diff --git a/.changeset/25942-bulk-confirm-param-holes.md b/.changeset/25942-bulk-confirm-param-holes.md new file mode 100644 index 00000000000..3b777835aa8 --- /dev/null +++ b/.changeset/25942-bulk-confirm-param-holes.md @@ -0,0 +1,12 @@ +--- +'@object-ui/plugin-grid': patch +--- + +fix(plugin-grid): a bulk action's `confirmText` fills each param hole with that param's display value + +Clause-②: no +Metadata-protocol: none + +A bulk action's `confirmText` holes may name the action's params by `name` (`{{ tier }}`), and the protocol states what such a hole shows: the param's display value. The bulk dialog filled `{{ count }}` only, so a param hole printed empty. It now fills each param root by that rule. An option-bearing param shows the chosen option's label, and a multi-value param shows its labels joined with ', '. A `lookup` or user param shows the picked record's display name (its id until the name has resolved; the sentence updates when the name arrives). Any other param shows the entered value itself, so the template engine's formatters apply (`{{ amount | currency:EUR }}`, `{{ due | date:long }}`). An empty param prints empty, and `{{ count }}` still wins over a param named `count`. The default sentence and the place of the confirm step are unchanged. + +What to do: nothing. diff --git a/scripts/sdui-manifest.sources/plugin-grid.json b/scripts/sdui-manifest.sources/plugin-grid.json index 086cb5905d6..8e53b237242 100644 --- a/scripts/sdui-manifest.sources/plugin-grid.json +++ b/scripts/sdui-manifest.sources/plugin-grid.json @@ -1 +1 @@ -{"package":"ui/packages/plugin-grid","sources":38,"sha256":"d34e7acc9010e7fc875131b416c4a83a2bac115c893c2067d3ccb85fcaf8bcd0","manifestSha256":"1f6e1ccb925a678eaa393f9091dfc7d0285e4e3626e0a5c61a5474687debabb7"} +{"package":"ui/packages/plugin-grid","sources":38,"sha256":"f6f7b1cbd89a1536a6e33febf4db131b8d526bd7281be460c401dccb8e97876a","manifestSha256":"1f6e1ccb925a678eaa393f9091dfc7d0285e4e3626e0a5c61a5474687debabb7"} diff --git a/ui/packages/plugin-grid/src/__tests__/bulkConfirmParams-25942.test.tsx b/ui/packages/plugin-grid/src/__tests__/bulkConfirmParams-25942.test.tsx new file mode 100644 index 00000000000..577199714de --- /dev/null +++ b/ui/packages/plugin-grid/src/__tests__/bulkConfirmParams-25942.test.tsx @@ -0,0 +1,159 @@ +/** + * 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#25942 step 2 — a bulk `confirmText` hole naming one of the def's + * params is filled with that param's DISPLAY value, by the rule the protocol + * states on the slot: the chosen option's label (a multi-value param's labels + * joined with ', '), the picked record's display name for a lookup / user + * param, and the entered value otherwise, so the engine's formatters apply. + * `count` wins over a param of that name. + * + * Two halves: + * - the helper `bulkConfirmText` over a full scope; + * - the real `BulkActionDialog`: an option label, and a lookup name that + * arrives after the confirm step has opened. + */ +import { describe, it, expect, vi, beforeAll } from 'vitest'; +import React from 'react'; +import { render, screen, cleanup, fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import type { BulkActionDef, BulkActionParam } from '@object-ui/types'; + +import { BulkActionDialog } from '../components/BulkActionDialog'; +import { bulkConfirmText } from '../components/bulkConfirmText'; + +beforeAll(() => { + if (!Element.prototype.scrollIntoView) { + Element.prototype.scrollIntoView = vi.fn() as any; + } +}); + +const TIER: BulkActionParam = { + name: 'tier', + label: 'Tier', + type: 'select', + options: [ + { label: 'Gold', value: 'gold' }, + { label: 'Silver', value: 'silver' }, + ], +}; +const TAGS: BulkActionParam = { + name: 'tags', + label: 'Tags', + type: 'multiselect', + options: [ + { label: 'Alpha', value: 'a' }, + { label: 'Beta', value: 'b' }, + ], +}; +const OWNER: BulkActionParam = { name: 'owner', label: 'Owner', type: 'lookup', object: 'sys_user' }; +const AMOUNT: BulkActionParam = { name: 'amount', label: 'Amount', type: 'number' }; +const DUE: BulkActionParam = { name: 'due', label: 'Due', type: 'date' }; +const NOTE: BulkActionParam = { name: 'note', label: 'Note', type: 'text' }; + +function fill(source: string, values: Record, extra: { count?: number; params?: BulkActionParam[]; lookupLabels?: Record> } = {}) { + return bulkConfirmText(source, { + count: extra.count ?? 2, + params: extra.params ?? [TIER, TAGS, OWNER, AMOUNT, DUE, NOTE], + values, + lookupLabels: extra.lookupLabels, + }); +} + +describe('bulkConfirmText fills param holes with display values (objectstack#25942)', () => { + it("shows the chosen option's label, not its value", () => { + expect(fill('Move {{ count }} account(s) to {{ tier }}?', { tier: 'gold' })).toBe('Move 2 account(s) to Gold?'); + }); + + it("joins a multi-value param's labels with ', '", () => { + expect(fill('Tag them {{ tags }}?', { tags: ['a', 'b'] })).toBe('Tag them Alpha, Beta?'); + }); + + it('shows a value no option declares as entered', () => { + expect(fill('Move to {{ tier }}?', { tier: 'bronze' })).toBe('Move to bronze?'); + }); + + it("shows a lookup's resolved record name, and the id until it is resolved", () => { + expect(fill('Assign to {{ owner }}?', { owner: 'u1' }, { lookupLabels: { owner: { u1: 'Ada Lovelace' } } })) + .toBe('Assign to Ada Lovelace?'); + expect(fill('Assign to {{ owner }}?', { owner: 'u1' })).toBe('Assign to u1?'); + }); + + it('hands the entered value to the formatters', () => { + expect(fill('Charge {{ amount | currency:EUR }} each?', { amount: 1234.5 })).toBe('Charge €1,234.50 each?'); + expect(fill('Charge {{ amount | number:2 }} each?', { amount: 3 })).toBe('Charge 3.00 each?'); + expect(fill('Due {{ due | date:iso }}?', { due: '2026-10-26' })).toBe('Due 2026-10-26?'); + }); + + it('prints an empty param empty, so the default formatter can name it', () => { + expect(fill('Note: {{ note }}.', {})).toBe('Note: .'); + expect(fill("Note: {{ note | default:'none' }}.", { note: '' })).toBe('Note: none.'); + expect(fill("Tags: {{ tags | default:'none' }}.", { tags: [] })).toBe('Tags: none.'); + }); + + it('lets count win over a param named count', () => { + const countParam: BulkActionParam = { name: 'count', label: 'Count', type: 'number' }; + expect(fill('Update {{ count }} record(s)?', { count: 99 }, { count: 3, params: [countParam] })) + .toBe('Update 3 record(s)?'); + }); + + it('prints a hole outside the closed scope empty', () => { + expect(fill('Update {{ record.name }} to {{ tier }}?', { tier: 'silver' })).toBe('Update to Silver?'); + }); +}); + +const ROWS = [{ id: 'r1', name: 'Acme' }, { id: 'r2', name: 'Globex' }]; + +function openDialog(def: BulkActionDef, dataSource: Record = {}) { + cleanup(); + render( + ({})), delete: vi.fn(async () => ({})), ...dataSource } as any} + open + onClose={() => {}} + />, + ); +} + +describe('BulkActionDialog fills param holes in its confirm sentence (objectstack#25942)', () => { + it('shows the option label the params step chose', async () => { + openDialog({ + name: 'set_tier', + label: 'Set tier', + operation: 'update', + params: [{ ...TIER, default: 'silver' }], + confirmText: { dialect: 'template', source: 'Move {{ count }} account(s) to {{ tier }}?' }, + }); + fireEvent.click(await screen.findByRole('button', { name: 'Next' })); + expect(await screen.findByText('Move 2 account(s) to Silver?')).toBeInTheDocument(); + }); + + it('shows the lookup record name once it resolves after the step opens', async () => { + let resolveName: (rec: Record) => void = () => {}; + const findOne = vi.fn(() => new Promise>((r) => { resolveName = r; })); + openDialog( + { + name: 'assign', + label: 'Assign', + operation: 'update', + params: [{ ...OWNER, default: 'u1' }], + confirmText: 'Assign {{ count }} account(s) to {{ owner }}?', + }, + { findOne }, + ); + fireEvent.click(await screen.findByRole('button', { name: 'Next' })); + expect(await screen.findByText('Assign 2 account(s) to u1?')).toBeInTheDocument(); + expect(findOne).toHaveBeenCalledWith('sys_user', 'u1'); + resolveName({ id: 'u1', name: 'Ada Lovelace' }); + expect(await screen.findByText('Assign 2 account(s) to Ada Lovelace?')).toBeInTheDocument(); + }); +}); diff --git a/ui/packages/plugin-grid/src/components/BulkActionDialog.tsx b/ui/packages/plugin-grid/src/components/BulkActionDialog.tsx index c1b43e1f66e..76a683a69c7 100644 --- a/ui/packages/plugin-grid/src/components/BulkActionDialog.tsx +++ b/ui/packages/plugin-grid/src/components/BulkActionDialog.tsx @@ -410,7 +410,7 @@ export const BulkActionDialog: React.FC = ({ {step === 'confirm' && ( - {bulkConfirmText(def.confirmText, { count: rows.length }) ?? t('grid.bulk.confirmDefault', { count: rows.length, defaultValue: `This will apply to ${rows.length} record(s).` })} + {bulkConfirmText(def.confirmText, { count: rows.length, params, values, lookupLabels }) ?? t('grid.bulk.confirmDefault', { count: rows.length, defaultValue: `This will apply to ${rows.length} record(s).` })} )} diff --git a/ui/packages/plugin-grid/src/components/bulkConfirmText.ts b/ui/packages/plugin-grid/src/components/bulkConfirmText.ts index 8413290768d..393b2f5ce8b 100644 --- a/ui/packages/plugin-grid/src/components/bulkConfirmText.ts +++ b/ui/packages/plugin-grid/src/components/bulkConfirmText.ts @@ -7,8 +7,9 @@ */ import { fillTemplateHoles, hasTemplateHoles } from '@object-ui/core'; -import type { BulkActionDef } from '@object-ui/types'; -import { BULK_CONFIRM_COUNT_ROOT } from '@objectstack/spec/ui'; +import type { BulkActionDef, BulkActionParam } from '@object-ui/types'; +import { BULK_CONFIRM_COUNT_ROOT, bulkConfirmTemplateRoots } from '@objectstack/spec/ui'; +import { isLookupishParam } from './bulkParamToField'; /** * The text a bulk action's `confirmText` carries: the string itself, or the @@ -39,11 +40,84 @@ export function bulkConfirmTextSource( return text !== undefined && text.trim() !== '' ? text : undefined; } +/** + * What a bulk `confirmText` hole is filled from: the run's record count, and + * the dialog's param values with what it needs to show them. + */ +export interface BulkConfirmScope { + /** The number of records the run covers (the dialog's `rows.length`). */ + count: number; + /** The def's params, in declaration order. */ + params?: readonly BulkActionParam[]; + /** The dialog's in-progress param values, by param `name`. */ + values?: Record; + /** + * The display names the confirm step resolved for picker params: param name + * → record id → name. Filled lazily once the step opens. + */ + lookupLabels?: Record>; +} + +/** + * A param's DISPLAY value for a confirm hole, by the rule the protocol states + * on `confirmText` (`packages/spec/src/ui/bulk-action.zod.ts`): the chosen + * option's `label` for an option-bearing param, the picked record's display + * name for a `lookup` / user param, and the entered value otherwise — returned + * as the value itself, not a string, so the engine's formatters + * (`{{ amount | currency:EUR }}`, `{{ due | date:long }}`) apply to it. A + * multi-value param's entries are joined with ', '. An empty value is + * `undefined`, so it prints empty and `{{ x | default:'—' }}` can name it. + * + * A picker id whose name has not been resolved (yet, or ever) shows the id, + * as the confirm step's own value summary does; the sentence re-renders when + * the name arrives. + */ +export function bulkConfirmParamDisplayValue( + param: BulkActionParam, + value: unknown, + lookupLabels?: Record>, +): unknown { + if (value === undefined || value === null || value === '') return undefined; + if (Array.isArray(value) && value.length === 0) return undefined; + let labelOf: ((entry: unknown) => string) | undefined; + if (isLookupishParam(param)) { + const names = lookupLabels?.[param.name] ?? {}; + labelOf = (entry) => names[String(entry)] ?? String(entry); + } else if (Array.isArray(param.options) && param.options.length > 0) { + const options = param.options; + labelOf = (entry) => { + const option = options.find((o) => String(o.value) === String(entry)); + return option ? String(option.label) : String(entry); + }; + } + if (Array.isArray(value)) return value.map((entry) => (labelOf ? labelOf(entry) : String(entry))).join(', '); + return labelOf ? labelOf(value) : value; +} + +/** + * The roots a bulk `confirmText` is filled over: the protocol's closed scope + * (`bulkConfirmTemplateRoots`), `count` bound to the run's record count and + * each param bound to its display value. `count` is written last, so it wins + * over a param of that name, as the protocol declares. + */ +export function bulkConfirmTemplateScope(scope: BulkConfirmScope): Record { + const params = scope.params ?? []; + const roots: Record = {}; + for (const root of bulkConfirmTemplateRoots({ params })) { + if (root === BULK_CONFIRM_COUNT_ROOT) continue; + const param = params.find((p) => p.name === root); + if (param) roots[root] = bulkConfirmParamDisplayValue(param, scope.values?.[root], scope.lookupLabels); + } + roots[BULK_CONFIRM_COUNT_ROOT] = scope.count; + return roots; +} + /** * The confirm sentence a bulk action's `confirmText` shows, its `{{ }}` holes * filled over the bulk-confirm scope the protocol declares * (`bulkConfirmTemplateRoots`, ruling B on objectstack#24798): `{{ count }}` is - * the number of records the run covers. Returns `undefined` exactly when + * the number of records the run covers, and `{{ }}` that param's + * display value ({@link bulkConfirmParamDisplayValue}). Returns `undefined` exactly when * {@link bulkConfirmTextSource} does, so the caller keeps its default * sentence. * @@ -51,17 +125,12 @@ export function bulkConfirmTextSource( * notifications and the single-record `confirmText` render with, so the * engine's whitelisted formatters apply (`{{ count | number }}`) and no hole * holds logic. A sentence without a `{{ }}` hole is shown as written. - * - * ⚠️ Step 1 of objectstack#25942: only `count` is bound. A hole naming one of - * the def's params resolves to nothing and prints empty, as an unbound path - * does in every template, until the param roots are filled with their display - * values. */ export function bulkConfirmText( confirmText: BulkActionDef['confirmText'] | null | undefined, - scope: { count: number }, + scope: BulkConfirmScope, ): string | undefined { const source = bulkConfirmTextSource(confirmText); if (source === undefined || !hasTemplateHoles(source)) return source; - return fillTemplateHoles(source, { [BULK_CONFIRM_COUNT_ROOT]: scope.count }); + return fillTemplateHoles(source, bulkConfirmTemplateScope(scope)); }