fix(plugin-grid): fill a bulk confirmText's param holes with the params' display values

.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.

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"}

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<string, unknown>, extra: { count?: number; params?: BulkActionParam[]; lookupLabels?: Record<string, Record<string, string>> } = {}) {
+  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<string, unknown> = {}) {
+  cleanup();
+  render(
+    <BulkActionDialog
+      def={def}
+      rows={ROWS}
+      resource="account"
+      dataSource={{ update: vi.fn(async () => ({})), 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<string, unknown>) => void = () => {};
+    const findOne = vi.fn(() => new Promise<Record<string, unknown>>((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();
+  });
+});

ui/packages/plugin-grid/src/components/BulkActionDialog.tsx

@@ -410,7 +410,7 @@ export const BulkActionDialog: React.FC<BulkActionDialogProps> = ({
           </DialogTitle>
           {step === 'confirm' && (
             <DialogDescription>
-              {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).` })}
             </DialogDescription>
           )}
         </DialogHeader>

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,29 +40,97 @@ 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<string, unknown>;
+  /**
+   * The display names the confirm step resolved for picker params: param name
+   * → record id → name. Filled lazily once the step opens.
+   */
+  lookupLabels?: Record<string, Record<string, string>>;
+}
+
+/**
+ * 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<string, Record<string, string>>,
+): 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<string, unknown> {
+  const params = scope.params ?? [];
+  const roots: Record<string, unknown> = {};
+  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 `{{ <param name> }}` that param's
+ * display value ({@link bulkConfirmParamDisplayValue}). Returns `undefined` exactly when
  * {@link bulkConfirmTextSource} does, so the caller keeps its default
  * sentence.
  *
  * Filled through core's `fillTemplateHoles`, the template engine the flow
  * 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));
 }