diff --git a/.changeset/13384-record-action-row-guard.md b/.changeset/13384-record-action-row-guard.md new file mode 100644 index 0000000000..39dd9dcdad --- /dev/null +++ b/.changeset/13384-record-action-row-guard.md @@ -0,0 +1,11 @@ +--- +'@object-ui/core': patch +'@object-ui/i18n': patch +'@object-ui/app-shell': patch +--- + +A single-record action launched with 0 or several rows selected is refused before any dialog opens, and the refusal is translated (objectui#13384). + +`ActionRunner` now asks the handler it will dispatch to for an early refusal (`ActionRunnerHandler`'s optional `refuseBeforeInput`) before the confirmation and the param dialog. The handlers `createServerActionHandler` builds answer it with the row-count check the dispatch already runs, so "select a row first" and "select exactly one row" are shown before the user fills anything in, and nothing is sent. With exactly one row the dialog behaves as before. A host that wraps the built handler in its own function keeps the early refusal only by forwarding `refuseBeforeInput`; the console's `createConsoleServerActionHandler` now forwards it, so on a list page (HotCRM's "Add to Campaign" on My Leads) the refusal toast shows and no dialog opens. + +Both refusals now carry a pack key (`actions.selectRowFirst`, `actions.selectExactlyOneRow`, defined in every shipped pack). `resolveServerActionRecordId` returns it as `errorKey` beside the English `error`, and the runner's early refusal goes through its translator by that key. diff --git a/packages/app-shell/src/utils/__tests__/consoleServerAction.test.tsx b/packages/app-shell/src/utils/__tests__/consoleServerAction.test.tsx index ac8e59f967..eec8547ee0 100644 --- a/packages/app-shell/src/utils/__tests__/consoleServerAction.test.tsx +++ b/packages/app-shell/src/utils/__tests__/consoleServerAction.test.tsx @@ -21,6 +21,7 @@ vi.mock('sonner', () => { }); import { toast } from 'sonner'; +import { ActionRunner } from '@object-ui/core'; import { createConsoleServerActionHandler } from '../consoleServerAction'; /** A pre-openable tab stub with the surface the wrapper drives. */ @@ -419,3 +420,75 @@ describe('a declared onSuccess block defers to the runner (objectui#5221)', () = expect(openSpy).toHaveBeenCalledWith('/app/crm/contacts/rec_42', '_blank'); }); }); + +describe('the early record-id refusal is forwarded to the runner (objectui#13384)', () => { + const ADD_TO_CAMPAIGN = { + type: 'script', + name: 'add_to_campaign', + objectName: 'crm_lead', + locations: ['list_toolbar', 'list_item'], + actionParams: [{ name: 'campaign', label: 'Campaign', type: 'text', required: true }], + }; + const launch = () => JSON.parse(JSON.stringify(ADD_TO_CAMPAIGN)); + + it("the wrapper answers core's refusal, with its pack key, and nothing for one row", () => { + const { handler } = makeHandler(); + + expect(handler.refuseBeforeInput?.(launch(), { selectedRecords: [] })).toEqual({ + error: 'This action runs on a single record — select a row first.', + errorKey: 'actions.selectRowFirst', + }); + expect(handler.refuseBeforeInput?.(launch(), { selectedRecords: [{ id: 'a' }, { id: 'b' }] })).toEqual({ + error: 'This action runs on a single record — select exactly one row.', + errorKey: 'actions.selectExactlyOneRow', + }); + expect(handler.refuseBeforeInput?.(launch(), { selectedRecords: [{ id: 'a' }] })).toBeUndefined(); + }); + + it('an injected resolveRecordId (record-page policy) decides the early refusal too', () => { + const { handler } = makeHandler({ resolveRecordId: () => ({ recordId: 'page_rec_1' }) }); + + expect(handler.refuseBeforeInput?.(launch(), { selectedRecords: [] })).toBeUndefined(); + }); + + it.each([ + ['0 rows', [] as Record[], 'This action runs on a single record — select a row first.'], + ['2 rows', [{ id: 'a' }, { id: 'b' }], 'This action runs on a single record — select exactly one row.'], + ])('through a real runner, %s: refused before the param dialog, with no tab and no request', async (_label, rows, refusal) => { + const openSpy = vi.spyOn(window, 'open'); + const { handler, fetch } = makeHandler(); + const runner = new ActionRunner({ selectedRecords: rows }); + const toastSpy = vi.fn(); + const dialog = vi.fn(async () => ({ campaign: 'cmp_webinar' })); + runner.setToastHandler(toastSpy); + runner.setParamCollectionHandler(dialog); + runner.registerHandler('script', handler); + + const result = await runner.execute({ ...launch(), opensInNewTab: true }); + + expect(result).toEqual({ success: false, error: refusal }); + expect(dialog).not.toHaveBeenCalled(); + expect(fetch).not.toHaveBeenCalled(); + expect(openSpy).not.toHaveBeenCalled(); + expect(toastSpy).toHaveBeenCalledWith(refusal, expect.objectContaining({ type: 'error' })); + }); + + it('through a real runner, 1 row: the param dialog opens and the request carries that row', async () => { + const { handler, fetch } = makeHandler(); + const runner = new ActionRunner({ selectedRecords: [{ id: 'lead_1' }] }); + const dialog = vi.fn(async () => ({ campaign: 'cmp_webinar' })); + runner.setToastHandler(vi.fn()); + runner.setParamCollectionHandler(dialog); + runner.registerHandler('script', handler); + + const result = await runner.execute(launch()); + + expect(result.success).toBe(true); + expect(dialog).toHaveBeenCalledTimes(1); + expect(fetch).toHaveBeenCalledTimes(1); + expect(JSON.parse((fetch.mock.calls[0] as unknown as [string, { body: string }])[1].body)).toEqual({ + recordId: 'lead_1', + params: { campaign: 'cmp_webinar' }, + }); + }); +}); diff --git a/packages/app-shell/src/utils/consoleServerAction.ts b/packages/app-shell/src/utils/consoleServerAction.ts index 6fa5d8be41..9e6656bc43 100644 --- a/packages/app-shell/src/utils/consoleServerAction.ts +++ b/packages/app-shell/src/utils/consoleServerAction.ts @@ -175,7 +175,7 @@ export function createConsoleServerActionHandler(opts: ConsoleServerActionOption const resolveRecordId = opts.resolveRecordId ?? resolveServerActionRecordId; const t = opts.t ?? defaultTranslate; - return async (action: ActionDef, context?: ActionContext) => { + const handler: ServerActionHandler = async (action: ActionDef, context?: ActionContext) => { // ── Zero-roundtrip fast path ──────────────────────────────────────── // `newTabUrl` names a GET endpoint that performs ALL auth/authz itself // (e.g. /sso-open re-runs every check the POST half would have done), @@ -272,4 +272,11 @@ export function createConsoleServerActionHandler(opts: ConsoleServerActionOption return { success: false, error: (error as Error).message }; } }; + // Core's early record-id refusal (objectui#13384), forwarded: the runner + // asks the handler it dispatches to — this wrapper, not core's handler + // inside it — before it shows a confirmation or a param dialog. Without the + // forward a single-record action launched with 0 or several rows selected + // ran its whole dialog and was refused only after Confirm. + handler.refuseBeforeInput = dispatch.refuseBeforeInput; + return handler; } diff --git a/packages/app-shell/src/views/ObjectView.bulkWiredHeaderDuplicate-12462.test.tsx b/packages/app-shell/src/views/ObjectView.bulkWiredHeaderDuplicate-12462.test.tsx index 4d42e70e4e..2686992933 100644 --- a/packages/app-shell/src/views/ObjectView.bulkWiredHeaderDuplicate-12462.test.tsx +++ b/packages/app-shell/src/views/ObjectView.bulkWiredHeaderDuplicate-12462.test.tsx @@ -29,6 +29,7 @@ import * as React from 'react'; import { describe, it, expect, vi, afterEach } from 'vitest'; import { render, cleanup, screen, fireEvent, waitFor, act, within } from '@testing-library/react'; import { MemoryRouter, Routes, Route } from 'react-router-dom'; +import { toast } from 'sonner'; import { createI18n, I18nProvider } from '@object-ui/i18n'; import { NAV_RUN_ACTION_PARAM } from '@object-ui/layout'; // Module scope, per AGENTS.md's test-discipline rule: the param dialog reaches @@ -80,6 +81,7 @@ vi.mock('./metadata-admin/useMetadata', async (importOriginal) => ({ import { ObjectView } from './ObjectView'; import { ExpressionProvider } from '../providers/ExpressionProvider'; +import { ConsoleToaster } from '../chrome/ConsoleToaster'; /** HotCRM's `CreateCampaignAction`, the keys this chain reads: record-scoped. */ const CREATE_CAMPAIGN = { @@ -138,6 +140,7 @@ const OBJECTS = [ ]; afterEach(() => { + act(() => { toast.dismiss(); }); cleanup(); vi.unstubAllGlobals(); window.history.replaceState(null, '', '/'); @@ -157,6 +160,7 @@ function mountLeads(viewName: keyof typeof LIST_VIEWS) { } as never; return render( + @@ -252,6 +256,14 @@ describe('objectui#12462 — a bulk-wired, record-scoped list_toolbar action lea }); }); +/** + * What an ARMED link shows. `create_campaign` is record-scoped and the link + * runs it as the page mounts, with no row selected, so the run is refused for + * its row count before its param dialog opens (objectui#13384). This toast is + * the run's observable: the link armed and the action ran. + */ +const ZERO_ROW_REFUSAL = 'This action runs on a single record — select a row first.'; + describe('objectui#12462 — the deep link arms only what the header draws', () => { const linkTo = (viewName: string) => window.history.replaceState(null, '', `/apps/crm/crm_lead/view/${viewName}?${NAV_RUN_ACTION_PARAM}=create_campaign`); @@ -268,14 +280,18 @@ describe('objectui#12462 — the deep link arms only what the header draws', () await rowBoxes(); expect(screen.queryByRole('dialog')).toBeNull(); + expect(screen.queryByText(ZERO_ROW_REFUSAL)).toBeNull(); expect(linkStillInUrl()).toBe('create_campaign'); }); - it('CONTROL — on a view that does not wire it, the same link arms: the param dialog opens and the link is consumed', async () => { + it('CONTROL — on a view that does not wire it, the same link arms: the action runs and the link is consumed', async () => { linkTo('my_leads'); mountLeads('my_leads'); - expect(await screen.findByRole('dialog', undefined, { timeout: 5000 })).toBeInTheDocument(); + // It ran: with no row selected, the record-scoped action is refused before + // its param dialog (objectui#13384), so no dialog opens. + expect(await screen.findByText(ZERO_ROW_REFUSAL, undefined, { timeout: 5000 })).toBeInTheDocument(); + expect(screen.queryByRole('dialog')).toBeNull(); await waitFor(() => expect(linkStillInUrl()).toBeNull(), { timeout: 5000 }); }); }); diff --git a/packages/app-shell/src/views/ObjectView.toolbarScriptSelection-12331.test.tsx b/packages/app-shell/src/views/ObjectView.toolbarScriptSelection-12331.test.tsx index 5ee9410c6f..1a8c92fef4 100644 --- a/packages/app-shell/src/views/ObjectView.toolbarScriptSelection-12331.test.tsx +++ b/packages/app-shell/src/views/ObjectView.toolbarScriptSelection-12331.test.tsx @@ -34,6 +34,12 @@ * comes after a picker was used. It refuses and reports exactly as the plain * fixture does. * + * objectui#13384 moved the refusal in front of the dialog: a launch the + * dispatch would refuse for its row count is refused before the param dialog + * opens (the runner asks the console handler's forwarded + * `refuseBeforeInput`), so the 0- and 2-row cases below assert that no dialog + * opened, beside the toast and the absent request. One row still opens it. + * * "Record-scoped" is a declaration, not a guess: an action is single-record * when its `locations` include a record placement (`isRecordScopedAction`). An * action declared at `list_toolbar` alone is object-level, so zero rows @@ -214,22 +220,31 @@ async function launchFromHeader(): Promise { await act(async () => { fireEvent.click(within(dialog).getByRole('button', { name: 'Confirm' })); }); } +/** + * Click the HEADER toolbar's "Add to Campaign" for a launch that is refused for + * its row count (objectui#13384): the refusal shows, and no dialog opened. + */ +async function launchRefusedFromHeader(refusal: string): Promise { + const toolbars = await screen.findAllByRole('toolbar', undefined, { timeout: 5000 }); + fireEvent.click(await within(toolbars[0]).findByRole('button', { name: /Add to Campaign/ }, { timeout: 5000 })); + expect(await screen.findByText(refusal, undefined, { timeout: 5000 })).toBeInTheDocument(); + expect(screen.queryByRole('dialog')).toBeNull(); +} + describe('objectui#12331 — a record-scoped script action at list_toolbar, with a param', () => { - it('0 rows: the refusal shows, and no request is sent', async () => { + it('0 rows: the refusal shows before any dialog, and no request is sent', async () => { mountLeads(RECORD_SCOPED); await tickRows(0); - await launchFromHeader(); + await launchRefusedFromHeader(ZERO_ROW_REFUSAL); - expect(await screen.findByText(ZERO_ROW_REFUSAL, undefined, { timeout: 5000 })).toBeInTheDocument(); expect(actionCalls()).toHaveLength(0); }); - it('2 rows: the refusal shows, and no request is sent', async () => { + it('2 rows: the refusal shows before any dialog, and no request is sent', async () => { mountLeads(RECORD_SCOPED); await tickRows(2); - await launchFromHeader(); + await launchRefusedFromHeader(MULTI_ROW_REFUSAL); - expect(await screen.findByText(MULTI_ROW_REFUSAL, undefined, { timeout: 5000 })).toBeInTheDocument(); expect(actionCalls()).toHaveLength(0); }); @@ -254,12 +269,11 @@ describe('objectui#12331 — a record-scoped script action at list_toolbar, with }); describe('objectui#12331 — the refusal follows the `locations` declaration', () => { - it('declared at list_toolbar alone, 2 rows: one dispatch cannot name two records, so it refuses', async () => { + it('declared at list_toolbar alone, 2 rows: one dispatch cannot name two records, so it refuses before any dialog', async () => { mountLeads(TOOLBAR_ONLY); await tickRows(2); - await launchFromHeader(); + await launchRefusedFromHeader(MULTI_ROW_REFUSAL); - expect(await screen.findByText(MULTI_ROW_REFUSAL, undefined, { timeout: 5000 })).toBeInTheDocument(); expect(actionCalls()).toHaveLength(0); }); @@ -387,21 +401,20 @@ async function launchHotcrmFromHeader(): Promise { } describe('objectui#12331 — HotCRM\'s declaration: list_item placement, a field-backed record-picker param', () => { - it('0 rows: the refusal shows, and no request is sent', async () => { + it('0 rows: the refusal shows before any dialog, and no request is sent', async () => { mountHotcrmLeads(); await tickRows(0); - await launchHotcrmFromHeader(); + // `list_item` also draws the action on every row; the header's toolbar is the first. + await launchRefusedFromHeader(ZERO_ROW_REFUSAL); - expect(await screen.findByText(ZERO_ROW_REFUSAL, undefined, { timeout: 5000 })).toBeInTheDocument(); expect(actionCalls()).toHaveLength(0); }); - it('2 rows: the refusal shows, and no request is sent', async () => { + it('2 rows: the refusal shows before any dialog, and no request is sent', async () => { mountHotcrmLeads(); await tickRows(2); - await launchHotcrmFromHeader(); + await launchRefusedFromHeader(MULTI_ROW_REFUSAL); - expect(await screen.findByText(MULTI_ROW_REFUSAL, undefined, { timeout: 5000 })).toBeInTheDocument(); expect(actionCalls()).toHaveLength(0); }); diff --git a/packages/core/src/actions/ActionRunner.ts b/packages/core/src/actions/ActionRunner.ts index 074b624787..1b1d6734ba 100644 --- a/packages/core/src/actions/ActionRunner.ts +++ b/packages/core/src/actions/ActionRunner.ts @@ -520,11 +520,26 @@ export interface ActionDef { * handler — a different calling convention entirely (`(ctx) => unknown`, with * validated params and a trusted engine facade on `ctx`) and a different * execution site. + * + * `refuseBeforeInput` is optional. When the handler the runner will dispatch + * to carries it, the runner asks it BEFORE the confirmation and the param + * dialog (objectui#13384), with the same context the dispatch will get. An + * answer refuses the run there: its `error` is toasted, translated through + * {@link ActionRunner.setTranslator}'s translator when it names an `errorKey`, + * and nothing is dispatched. `undefined` lets the run go on as before. The + * server-action handler uses it for its row-count refusal, so a single-record + * action launched with 0 or several rows selected never opens a dialog only + * to refuse after Confirm. */ -export type ActionRunnerHandler = ( +export type ActionRunnerHandler = (( action: ActionDef, context: ActionContext -) => Promise | ActionResult; +) => Promise | ActionResult) & { + refuseBeforeInput?: ( + action: ActionDef, + context: ActionContext, + ) => { error: string; errorKey?: string } | undefined; +}; /** * Confirmation handler — replaces window.confirm. @@ -595,6 +610,10 @@ export type ToastHandler = (message: string, options?: { * - `undo` — the label the runner hands the toast handler for an undoable * success toast's Undo affordance (objectui#10969). * + * The translator is also asked for one text this table does not hold: a + * handler's early refusal that names an `errorKey` (`ActionRunnerHandler`'s + * `refuseBeforeInput`, objectui#13384). Its `error` is the English source. + * * An author's `outcomeMessages` / `successMessage` / `errorMessage` and an * action's own error are never in this table: the host localizes authored copy * before dispatch, and the runner only fills in its `${result.*}` tokens. @@ -618,7 +637,7 @@ const RUNNER_TEXT = { }, } as const; -type RunnerText = (typeof RUNNER_TEXT)[keyof typeof RUNNER_TEXT]; +type RunnerText = { readonly key: string; readonly defaultValue: string }; /** * Modal handler — consumers provide to render modal dialogs. @@ -1288,7 +1307,9 @@ export class ActionRunner { * reached the runner, and the Undo label of an undoable success toast. An * author's `outcomeMessages` / `successMessage` / `errorMessage` and an * action's own error never go through this translator: the host localizes - * authored copy before dispatch. With no translator the text stays English. + * authored copy before dispatch. The one handler text it does translate is + * an early refusal that names its key (`ActionRunnerHandler`'s + * `refuseBeforeInput`). With no translator the text stays English. */ setTranslator(translate: (key: string, options: { defaultValue: string }) => string): void { this.translate = translate; @@ -1455,6 +1476,19 @@ export class ActionRunner { } } + // The handler's early refusal (objectui#13384), asked before the + // confirmation and the param dialog below. A run the handler would + // refuse for its context — a single-record action launched with 0 or + // several rows selected — is refused here, with the toast and the + // post-execution the dispatch-time refusal gets, and before the user + // has typed anything or anything is sent. + const refusal = this.refusalBeforeInput(action, actionType); + if (refusal !== undefined) { + const refused: ActionResult = { success: false, error: refusal }; + await this.handlePostExecution(action, refused); + return refused; + } + // Confirmation. One spelling: `confirmText` — the only one the // translation bundle can address (`…._actions.{name}.confirmText`). The // structured `confirm.message` read that used to OUTRANK it is retired @@ -1667,7 +1701,24 @@ export class ActionRunner { } /** - * One entry of `RUNNER_TEXT`, in the installed translator's language. An + * The early refusal of the handler {@link dispatchCollected} would route + * this action to, in the translator's language when it names a key — + * `undefined` when there is none. An update-operation action never reaches + * a registered handler, so it is never asked. + */ + private refusalBeforeInput(action: ActionDef, actionType: string): string | undefined { + if (isUpdateOperationAction(action)) return undefined; + const handler = actionType ? this.handlers.get(actionType) : undefined; + const refusal = handler?.refuseBeforeInput?.(action, this.context); + if (!refusal?.error) return undefined; + return refusal.errorKey + ? this.runnerText({ key: refusal.errorKey, defaultValue: refusal.error }) + : refusal.error; + } + + /** + * One entry of `RUNNER_TEXT` (or a handler's keyed early refusal), in the + * installed translator's language. An * empty answer from the translator falls back to the English source rather * than raising an empty toast or an empty label. */ diff --git a/packages/core/src/actions/__tests__/ActionRunner.recordRowGuard-13384.test.ts b/packages/core/src/actions/__tests__/ActionRunner.recordRowGuard-13384.test.ts new file mode 100644 index 0000000000..1cbd8d3a94 --- /dev/null +++ b/packages/core/src/actions/__tests__/ActionRunner.recordRowGuard-13384.test.ts @@ -0,0 +1,211 @@ +/** + * 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. + */ + +/** + * objectui#13384 — a single-record action launched from a list toolbar is + * refused for its row count BEFORE any dialog opens, and the refusal is + * translatable. + * + * Measured on HotCRM's My Leads: "Add to Campaign", a single-record script + * action with a param, placed in `list_toolbar`. With 0 or 2 rows ticked the + * param dialog opened and ran to the end (the user picked a campaign, and + * could even create one), and only after Confirm did the dispatch refuse, + * in English under zh-CN. The runner ran the param dialog before the handler, + * and the refusal strings had no pack key. + * + * What these pin, through the real runner and the real + * `createServerActionHandler` with a stubbed transport: + * - 0 rows and 2 rows: the refusal toast shows, no param dialog and no + * confirmation open, and nothing is sent; + * - 1 row: the dialog opens as before and the request carries that row; + * - the refusal is asked of the runner's translator by its pack key, with + * the English source as its default (the zh-CN words themselves are pinned + * in `@object-ui/i18n`'s `recordSelectionRefusal-13384` suite); + * - an object-level toolbar action with 0 rows still opens its dialog, and a + * handler with its own record resolver (a record page) is not refused. + */ +import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; +import { + ActionRunner, + type ActionDef, + type ParamCollectionHandler, + type ToastHandler, +} from '../ActionRunner'; +import { createServerActionHandler, type ServerActionFetch } from '../serverActionHandler'; + +const MULTI_ROW_REFUSAL = 'This action runs on a single record — select exactly one row.'; +const ZERO_ROW_REFUSAL = 'This action runs on a single record — select a row first.'; + +/** HotCRM's shape: a script action with a param, mounted on rows and on the toolbar. */ +const ADD_TO_CAMPAIGN: ActionDef = { + type: 'script', + name: 'add_to_campaign', + objectName: 'crm_lead', + locations: ['list_toolbar', 'list_item'], + actionParams: [{ name: 'campaign', label: 'Campaign', type: 'text', required: true }], +}; + +function okFetch() { + return vi.fn(async () => ({ + ok: true, + status: 200, + json: async () => ({ success: true, data: { added: 1 } }), + })) as unknown as ServerActionFetch & Mock; +} + +/** A copy per launch: the runner rewrites `params` on the action it runs. */ +const launch = (action: ActionDef): ActionDef => JSON.parse(JSON.stringify(action)); + +describe('objectui#13384 — the row-count refusal comes before the param dialog', () => { + let toast: Mock; + let dialog: Mock; + let confirm: Mock<(message: string) => Promise>; + let fetch: ServerActionFetch & Mock; + + function runnerWithRows(rows: Record[]): ActionRunner { + const runner = new ActionRunner({ selectedRecords: rows }); + runner.setToastHandler(toast); + runner.setParamCollectionHandler(dialog); + runner.setConfirmHandler(confirm); + runner.registerHandler('script', createServerActionHandler({ fetch })); + return runner; + } + + beforeEach(() => { + toast = vi.fn(); + dialog = vi.fn(async () => ({ campaign: 'cmp_webinar' })); + confirm = vi.fn(async () => true); + fetch = okFetch(); + }); + + it('0 rows: refuses with the toast, opens no dialog and sends nothing', async () => { + const result = await runnerWithRows([]).execute(launch(ADD_TO_CAMPAIGN)); + + expect(result).toEqual({ success: false, error: ZERO_ROW_REFUSAL }); + expect(dialog).not.toHaveBeenCalled(); + expect(fetch).not.toHaveBeenCalled(); + expect(toast).toHaveBeenCalledTimes(1); + expect(toast).toHaveBeenCalledWith(ZERO_ROW_REFUSAL, expect.objectContaining({ type: 'error' })); + }); + + it('2 rows: refuses with the toast, opens no dialog and sends nothing', async () => { + const result = await runnerWithRows([{ id: 'lead_1' }, { id: 'lead_2' }]) + .execute(launch(ADD_TO_CAMPAIGN)); + + expect(result).toEqual({ success: false, error: MULTI_ROW_REFUSAL }); + expect(dialog).not.toHaveBeenCalled(); + expect(fetch).not.toHaveBeenCalled(); + expect(toast).toHaveBeenCalledTimes(1); + expect(toast).toHaveBeenCalledWith(MULTI_ROW_REFUSAL, expect.objectContaining({ type: 'error' })); + }); + + it('a confirmation is not shown either before the refusal', async () => { + await runnerWithRows([]).execute(launch({ ...ADD_TO_CAMPAIGN, confirmText: 'Add the lead?' })); + + expect(confirm).not.toHaveBeenCalled(); + expect(dialog).not.toHaveBeenCalled(); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('1 row: the dialog opens, and the one request carries that row and the collected value', async () => { + const result = await runnerWithRows([{ id: 'lead_1' }]).execute(launch(ADD_TO_CAMPAIGN)); + + expect(dialog).toHaveBeenCalledTimes(1); + expect(result.success).toBe(true); + expect(fetch).toHaveBeenCalledTimes(1); + const [url, init] = fetch.mock.calls[0]; + expect(url).toBe('/api/v1/actions/crm_lead/add_to_campaign'); + expect(JSON.parse(init.body)).toEqual({ recordId: 'lead_1', params: { campaign: 'cmp_webinar' } }); + }); + + it('control — an object-level toolbar action with 0 rows still opens its dialog and dispatches', async () => { + const result = await runnerWithRows([]) + .execute(launch({ ...ADD_TO_CAMPAIGN, locations: ['list_toolbar'] })); + + expect(dialog).toHaveBeenCalledTimes(1); + expect(result.success).toBe(true); + expect(fetch).toHaveBeenCalledTimes(1); + }); + + it('control — a handler with its own record resolver (a record page) is not refused', async () => { + const runner = new ActionRunner({ selectedRecords: [] }); + runner.setToastHandler(toast); + runner.setParamCollectionHandler(dialog); + runner.registerHandler('script', createServerActionHandler({ + fetch, + resolveRecordId: () => ({ recordId: 'lead_page' }), + })); + + const result = await runner.execute(launch(ADD_TO_CAMPAIGN)); + + expect(dialog).toHaveBeenCalledTimes(1); + expect(result.success).toBe(true); + expect(JSON.parse(fetch.mock.calls[0][1].body).recordId).toBe('lead_page'); + }); + + it('control — a dialog that collects `recordId` itself is not judged before it runs', async () => { + dialog = vi.fn(async () => ({ recordId: 'lead_typed' })); + const result = await runnerWithRows([]).execute(launch({ + ...ADD_TO_CAMPAIGN, + actionParams: [{ name: 'recordId', label: 'Lead', type: 'text', required: true }], + })); + + expect(dialog).toHaveBeenCalledTimes(1); + expect(result.success).toBe(true); + expect(JSON.parse(fetch.mock.calls[0][1].body).recordId).toBe('lead_typed'); + }); +}); + +describe('objectui#13384 — the refusal goes through the runner\'s translator by its pack key', () => { + const ZH: Record = { + 'actions.selectRowFirst': 'ZH:selectRowFirst', + 'actions.selectExactlyOneRow': 'ZH:selectExactlyOneRow', + }; + + function translatedRunner(rows: Record[], toast: Mock) { + const runner = new ActionRunner({ selectedRecords: rows }); + runner.setToastHandler(toast); + runner.setParamCollectionHandler(vi.fn(async () => ({ campaign: 'cmp_webinar' }))); + runner.registerHandler('script', createServerActionHandler({ fetch: okFetch() })); + const translate = vi.fn((key: string, options: { defaultValue: string }) => ZH[key] ?? options.defaultValue); + runner.setTranslator(translate); + return { runner, translate }; + } + + it("0 rows: asks for 'actions.selectRowFirst' with the English default, and toasts the answer", async () => { + const toast = vi.fn(); + const { runner, translate } = translatedRunner([], toast); + + const result = await runner.execute(launch(ADD_TO_CAMPAIGN)); + + expect(translate).toHaveBeenCalledWith('actions.selectRowFirst', { defaultValue: ZERO_ROW_REFUSAL }); + expect(result.error).toBe('ZH:selectRowFirst'); + expect(toast).toHaveBeenCalledWith('ZH:selectRowFirst', expect.objectContaining({ type: 'error' })); + }); + + it("2 rows: asks for 'actions.selectExactlyOneRow' with the English default, and toasts the answer", async () => { + const toast = vi.fn(); + const { runner, translate } = translatedRunner([{ id: 'a' }, { id: 'b' }], toast); + + const result = await runner.execute(launch(ADD_TO_CAMPAIGN)); + + expect(translate).toHaveBeenCalledWith('actions.selectExactlyOneRow', { defaultValue: MULTI_ROW_REFUSAL }); + expect(result.error).toBe('ZH:selectExactlyOneRow'); + expect(toast).toHaveBeenCalledWith('ZH:selectExactlyOneRow', expect.objectContaining({ type: 'error' })); + }); + + it('an empty answer from the translator falls back to the English source', async () => { + const toast = vi.fn(); + const { runner } = translatedRunner([], toast); + runner.setTranslator(() => ''); + + const result = await runner.execute(launch(ADD_TO_CAMPAIGN)); + + expect(result.error).toBe(ZERO_ROW_REFUSAL); + }); +}); diff --git a/packages/core/src/actions/__tests__/serverActionHandler.test.ts b/packages/core/src/actions/__tests__/serverActionHandler.test.ts index 7690c5cb63..51287f890b 100644 --- a/packages/core/src/actions/__tests__/serverActionHandler.test.ts +++ b/packages/core/src/actions/__tests__/serverActionHandler.test.ts @@ -233,6 +233,40 @@ describe('createServerActionHandler — resolution wiring', () => { }); }); +describe('the row-count refusals carry their pack key, and are asked before input (objectui#13384)', () => { + const SCOPED = { name: 'a', locations: ['list_item', 'list_toolbar'] } as ActionDef; + + it('each refusal names the locale-pack key its English text is the source of', () => { + expect(resolveServerActionRecordId(SCOPED, { selectedRecords: [{ id: 'x' }, { id: 'y' }] })).toEqual({ + error: 'This action runs on a single record — select exactly one row.', + errorKey: 'actions.selectExactlyOneRow', + }); + expect(resolveServerActionRecordId(SCOPED, { selectedRecords: [] })).toEqual({ + error: 'This action runs on a single record — select a row first.', + errorKey: 'actions.selectRowFirst', + }); + }); + + it('the built handler answers the same refusal before input, and nothing for one row', () => { + const handler = createServerActionHandler({ fetch: okFetch() }); + + expect(handler.refuseBeforeInput?.(SCOPED, { selectedRecords: [] })).toEqual({ + error: 'This action runs on a single record — select a row first.', + errorKey: 'actions.selectRowFirst', + }); + expect(handler.refuseBeforeInput?.(SCOPED, { selectedRecords: [{ id: 'x' }] })).toBeUndefined(); + }); + + it('a custom resolveRecordId decides the early refusal too', () => { + const handler = createServerActionHandler({ + fetch: okFetch(), + resolveRecordId: () => ({ recordId: 'page_rec_1' }), + }); + + expect(handler.refuseBeforeInput?.(SCOPED, { selectedRecords: [] })).toBeUndefined(); + }); +}); + describe('createServerActionHandler — envelope rule (objectstack#3913)', () => { it('treats an INNER success:false under HTTP 200 as a failure (no refresh)', async () => { const fetch = okFetch({ diff --git a/packages/core/src/actions/serverActionHandler.ts b/packages/core/src/actions/serverActionHandler.ts index 213c2d7ac1..e29b7db243 100644 --- a/packages/core/src/actions/serverActionHandler.ts +++ b/packages/core/src/actions/serverActionHandler.ts @@ -81,6 +81,13 @@ export type ServerActionFetch = ( export interface ServerActionRecordIdResolution { recordId?: unknown; error?: string; + /** + * The locale-pack key `error` is the English source of, when the refusal is + * one this module writes itself (see `RECORD_SELECTION_REFUSAL`). A host + * that shows `error` to the user translates it through this key; with no + * key, `error` is shown as it is. + */ + errorKey?: string; } export type ServerActionRecordIdResolver = ( @@ -93,10 +100,22 @@ export type ServerActionRecordIdResolver = ( * to `ActionRunnerHandler` — register it under `'script'` (and, where the host * routes them server-side, other action types). */ -export type ServerActionHandler = ( +export type ServerActionHandler = (( action: ActionDef, context?: ActionContext, -) => Promise; +) => Promise) & { + /** + * The record-id refusal, asked BEFORE the runner opens any dialog + * (objectui#13384) — see `ActionRunnerHandler`'s member of the same name. + * Set on every handler {@link createServerActionHandler} builds; optional so + * a host that wraps the handler in its own function still satisfies the + * type, though such a wrapper only keeps the early refusal by forwarding it. + */ + refuseBeforeInput?: ( + action: ActionDef, + context: ActionContext | undefined, + ) => { error: string; errorKey?: string } | undefined; +}; export interface ServerActionHandlerConfig { /** Authenticated transport — see {@link ServerActionFetch}. */ @@ -140,6 +159,40 @@ export function isRecordScopedAction(action: ActionDef): boolean { l === 'list_item' || l === 'record_header' || l === 'record_more' || l === 'record_section'); } +/** + * The two refusals the record-id dance writes itself. `key` names the + * locale-pack entry (`@object-ui/i18n`'s packs define both), `defaultValue` is + * the English source: the text a resolution's `error` carries, which is what + * shows when nothing translates it. + */ +const RECORD_SELECTION_REFUSAL = { + multipleRows: { + key: 'actions.selectExactlyOneRow', + defaultValue: 'This action runs on a single record — select exactly one row.', + }, + noRow: { + key: 'actions.selectRowFirst', + defaultValue: 'This action runs on a single record — select a row first.', + }, +} as const; + +function refuseSelection( + { key, defaultValue }: (typeof RECORD_SELECTION_REFUSAL)[keyof typeof RECORD_SELECTION_REFUSAL], +): ServerActionRecordIdResolution { + return { error: defaultValue, errorKey: key }; +} + +/** + * Whether the action's param dialog itself collects `recordId` — the first + * rung of the record-id dance. Such an action cannot be judged before its + * dialog has run, so it gets no early refusal. + */ +function collectsRecordIdParam(action: ActionDef): boolean { + const defs = Array.isArray(action.actionParams) ? action.actionParams + : (Array.isArray(action.params) ? action.params : undefined); + return !!defs?.some((def) => (def as { name?: unknown } | null)?.name === 'recordId'); +} + /** * The standard record-id resolution dance, shared by the dispatcher and by * consumers that need the id BEFORE dispatching (e.g. app-shell's zero-roundtrip @@ -173,9 +226,9 @@ export function resolveServerActionRecordId( recordId = selected[0]?.[recordIdField]; } else if (selected.length > 1) { // The runner's post-execution hook surfaces `error` as a toast. - return { error: 'This action runs on a single record — select exactly one row.' }; + return refuseSelection(RECORD_SELECTION_REFUSAL.multipleRows); } else if (isRecordScopedAction(action)) { - return { error: 'This action runs on a single record — select a row first.' }; + return refuseSelection(RECORD_SELECTION_REFUSAL.noRow); } } return { recordId: recordId ?? undefined }; @@ -193,7 +246,7 @@ export function createServerActionHandler(config: ServerActionHandlerConfig): Se // span invocations. const inFlight = new Set(); - return async function serverActionDispatch( + const serverActionDispatch = async function serverActionDispatch( action: ActionDef, context?: ActionContext, ): Promise { @@ -262,4 +315,16 @@ export function createServerActionHandler(config: ServerActionHandlerConfig): Se inFlight.delete(inflightKey); } }; + + // The same resolution the dispatch runs, asked before the runner shows a + // confirmation or a param dialog (objectui#13384): a launch the dispatch + // would refuse for its row count is refused before the user types anything, + // not after Confirm. The dispatch keeps its own check for a caller that + // invokes the handler directly. + const refuseBeforeInput: NonNullable = (action, context) => { + if (collectsRecordIdParam(action)) return undefined; + const resolution = resolveRecordId(action, context); + return resolution.error ? { error: resolution.error, errorKey: resolution.errorKey } : undefined; + }; + return Object.assign(serverActionDispatch, { refuseBeforeInput }); } diff --git a/packages/i18n/src/__tests__/recordSelectionRefusal-13384.test.ts b/packages/i18n/src/__tests__/recordSelectionRefusal-13384.test.ts new file mode 100644 index 0000000000..b3d6369640 --- /dev/null +++ b/packages/i18n/src/__tests__/recordSelectionRefusal-13384.test.ts @@ -0,0 +1,52 @@ +/** + * 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. + */ + +/** + * objectui#13384 — the two row-count refusals of a single-record action have a + * pack key in every shipped pack. + * + * `@object-ui/core`'s `resolveServerActionRecordId` writes them, with their + * keys (`actions.selectExactlyOneRow`, `actions.selectRowFirst`), and the + * action runner asks its translator for them by those keys before any dialog + * opens (pinned in core's `ActionRunner.recordRowGuard-13384` suite). Under a + * zh-CN session the refusal used to read English, since neither string had a + * key. The zh-CN words are pinned here; every other pack is held to defining + * both keys with its own text. + */ +import { describe, it, expect } from 'vitest'; +import { builtInLocales } from '../locales/index'; + +const KEYS = ['selectExactlyOneRow', 'selectRowFirst'] as const; + +const actionsOf = (pack: unknown) => (pack as { actions: Record }).actions; + +describe('objectui#13384 — the row-count refusals are translated', () => { + it('zh-CN reads them in Chinese', () => { + expect(actionsOf(builtInLocales.zh).selectExactlyOneRow).toBe('此操作针对单条记录,请只选择一行。'); + expect(actionsOf(builtInLocales.zh).selectRowFirst).toBe('此操作针对单条记录,请先选择一行。'); + }); + + it('en holds the English source core falls back to', () => { + expect(actionsOf(builtInLocales.en).selectExactlyOneRow) + .toBe('This action runs on a single record — select exactly one row.'); + expect(actionsOf(builtInLocales.en).selectRowFirst) + .toBe('This action runs on a single record — select a row first.'); + }); + + for (const [code, pack] of Object.entries(builtInLocales)) { + if (code === 'en') continue; + it(`${code} defines both keys with its own text`, () => { + for (const key of KEYS) { + const value = actionsOf(pack)[key]; + expect(typeof value, `${code} actions.${key}`).toBe('string'); + expect((value as string).trim(), `${code} actions.${key}`).not.toBe(''); + expect(value, `${code} actions.${key}`).not.toBe(actionsOf(builtInLocales.en)[key]); + } + }); + } +}); diff --git a/packages/i18n/src/locales/ar.ts b/packages/i18n/src/locales/ar.ts index 756ca571ca..effa7410e1 100644 --- a/packages/i18n/src/locales/ar.ts +++ b/packages/i18n/src/locales/ar.ts @@ -150,6 +150,8 @@ const ar = { completedSuccessfully: 'اكتمل الإجراء بنجاح', failed: 'فشل الإجراء', parallelFailed: 'فشل إجراء متوازٍ واحد أو أكثر', + selectExactlyOneRow: 'يُنفَّذ هذا الإجراء على سجل واحد — حدّد صفًا واحدًا فقط.', + selectRowFirst: 'يُنفَّذ هذا الإجراء على سجل واحد — حدّد صفًا أولًا.', undo: 'تراجع', undone: 'تم التراجع عن التغيير', undoneOperation: 'تراجع: {{description}}', diff --git a/packages/i18n/src/locales/de.ts b/packages/i18n/src/locales/de.ts index b00a146432..ca1e9b4f39 100644 --- a/packages/i18n/src/locales/de.ts +++ b/packages/i18n/src/locales/de.ts @@ -136,6 +136,8 @@ const de = { completedSuccessfully: 'Aktion erfolgreich abgeschlossen', failed: 'Aktion fehlgeschlagen', parallelFailed: 'Eine oder mehrere parallele Aktionen sind fehlgeschlagen', + selectExactlyOneRow: 'Diese Aktion gilt für einen einzelnen Datensatz – wählen Sie genau eine Zeile aus.', + selectRowFirst: 'Diese Aktion gilt für einen einzelnen Datensatz – wählen Sie zuerst eine Zeile aus.', undo: 'Rückgängig', undone: 'Änderung rückgängig gemacht', undoneOperation: 'Rückgängig: {{description}}', diff --git a/packages/i18n/src/locales/en.ts b/packages/i18n/src/locales/en.ts index 88d28c5040..a069c88151 100644 --- a/packages/i18n/src/locales/en.ts +++ b/packages/i18n/src/locales/en.ts @@ -168,6 +168,12 @@ const en = { // handler for an undoable success toast. failed: 'Action failed', parallelFailed: 'One or more parallel actions failed', + // The two row-count refusals of a single-record action launched from a + // list toolbar (`resolveServerActionRecordId` in `@object-ui/core`); the + // runner asks for them before any confirmation or param dialog opens + // (objectui#13384). + selectExactlyOneRow: 'This action runs on a single record — select exactly one row.', + selectRowFirst: 'This action runs on a single record — select a row first.', undo: 'Undo', // The confirmation toast after that Undo has run — written by the console's // two undo handlers (`useConsoleActionRuntime` and `RecordDetailView`), not diff --git a/packages/i18n/src/locales/es.ts b/packages/i18n/src/locales/es.ts index 39c378039e..8e706014ac 100644 --- a/packages/i18n/src/locales/es.ts +++ b/packages/i18n/src/locales/es.ts @@ -138,6 +138,8 @@ const es = { completedSuccessfully: 'La acción se completó correctamente', failed: 'La acción falló', parallelFailed: 'Una o más acciones paralelas fallaron', + selectExactlyOneRow: 'Esta acción se ejecuta sobre un solo registro: seleccione exactamente una fila.', + selectRowFirst: 'Esta acción se ejecuta sobre un solo registro: seleccione primero una fila.', undo: 'Deshacer', undone: 'Cambio deshecho', undoneOperation: 'Deshacer: {{description}}', diff --git a/packages/i18n/src/locales/fr.ts b/packages/i18n/src/locales/fr.ts index c04374d69b..1af2386510 100644 --- a/packages/i18n/src/locales/fr.ts +++ b/packages/i18n/src/locales/fr.ts @@ -138,6 +138,8 @@ const fr = { completedSuccessfully: 'Action effectuée avec succès', failed: "Échec de l'action", parallelFailed: 'Une ou plusieurs actions parallèles ont échoué', + selectExactlyOneRow: 'Cette action porte sur un seul enregistrement — sélectionnez exactement une ligne.', + selectRowFirst: 'Cette action porte sur un seul enregistrement — sélectionnez d’abord une ligne.', undo: 'Annuler', undone: 'Modification annulée', undoneOperation: 'Annuler : {{description}}', diff --git a/packages/i18n/src/locales/ja.ts b/packages/i18n/src/locales/ja.ts index 72b4b493f7..d264b8b7ee 100644 --- a/packages/i18n/src/locales/ja.ts +++ b/packages/i18n/src/locales/ja.ts @@ -136,6 +136,8 @@ const ja = { completedSuccessfully: '操作が正常に完了しました', failed: '操作に失敗しました', parallelFailed: '1 つ以上の並列操作が失敗しました', + selectExactlyOneRow: 'この操作は 1 件のレコードに対して実行されます。行を 1 つだけ選択してください。', + selectRowFirst: 'この操作は 1 件のレコードに対して実行されます。先に行を選択してください。', undo: '元に戻す', undone: '変更を元に戻しました', undoneOperation: '元に戻す: {{description}}', diff --git a/packages/i18n/src/locales/ko.ts b/packages/i18n/src/locales/ko.ts index e7ad24d104..3b077d5342 100644 --- a/packages/i18n/src/locales/ko.ts +++ b/packages/i18n/src/locales/ko.ts @@ -136,6 +136,8 @@ const ko = { completedSuccessfully: '작업이 완료되었습니다', failed: '작업 실패', parallelFailed: '하나 이상의 병렬 작업이 실패했습니다', + selectExactlyOneRow: '이 작업은 단일 레코드에서 실행됩니다. 행을 하나만 선택하세요.', + selectRowFirst: '이 작업은 단일 레코드에서 실행됩니다. 먼저 행을 선택하세요.', undo: '실행 취소', undone: '변경 실행 취소됨', undoneOperation: '실행 취소: {{description}}', diff --git a/packages/i18n/src/locales/pt.ts b/packages/i18n/src/locales/pt.ts index 0157bc8d4f..8989a14265 100644 --- a/packages/i18n/src/locales/pt.ts +++ b/packages/i18n/src/locales/pt.ts @@ -138,6 +138,8 @@ const pt = { completedSuccessfully: 'A ação foi concluída com sucesso', failed: 'A ação falhou', parallelFailed: 'Uma ou mais ações paralelas falharam', + selectExactlyOneRow: 'Esta ação é executada em um único registro — selecione exatamente uma linha.', + selectRowFirst: 'Esta ação é executada em um único registro — selecione uma linha primeiro.', undo: 'Desfazer', undone: 'Alteração desfeita', undoneOperation: 'Desfazer: {{description}}', diff --git a/packages/i18n/src/locales/ru.ts b/packages/i18n/src/locales/ru.ts index 44a1158273..9976a4413c 100644 --- a/packages/i18n/src/locales/ru.ts +++ b/packages/i18n/src/locales/ru.ts @@ -143,6 +143,8 @@ const ru = { completedSuccessfully: 'Действие успешно выполнено', failed: 'Действие не выполнено', parallelFailed: 'Одно или несколько параллельных действий не выполнены', + selectExactlyOneRow: 'Это действие выполняется для одной записи — выберите ровно одну строку.', + selectRowFirst: 'Это действие выполняется для одной записи — сначала выберите строку.', undo: 'Отменить', undone: 'Изменение отменено', undoneOperation: 'Отменить: {{description}}', diff --git a/packages/i18n/src/locales/zh.ts b/packages/i18n/src/locales/zh.ts index c780da8dcc..b9c686c8af 100644 --- a/packages/i18n/src/locales/zh.ts +++ b/packages/i18n/src/locales/zh.ts @@ -141,6 +141,8 @@ const zh = { completedSuccessfully: '操作已成功完成', failed: '操作失败', parallelFailed: '一个或多个并行操作失败', + selectExactlyOneRow: '此操作针对单条记录,请只选择一行。', + selectRowFirst: '此操作针对单条记录,请先选择一行。', undo: '撤销', undone: '已撤销更改', undoneOperation: '撤销:{{description}}',