From c50b9506a6b43fee5aefa30ddd331f3cee0a044b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 06:40:20 +0000 Subject: [PATCH] =?UTF-8?q?fix(approvals):=20one=20decision,=20one=20dialo?= =?UTF-8?q?g=20=E2=80=94=20carry=20reject/recall=20confirm=20questions=20o?= =?UTF-8?q?n=20`description`=20(#7278)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sys_approval_request.approval_reject` and `approval_recall` declared both `confirmText` and `params`. The console action runner chains confirmation THEN param collection, both awaited, so one decision opened a confirm prompt and then a second dialog the approver never asked for — with nothing sent until the second Confirm, while the first prompt already read as "the action is running". Per the maintainer's 2026-08-10 ruling on #7278 (Option 1), each question moves to the action's top-level `description` — the key #7367 / PR #7430 added for exactly this — which objectui's `ActionParamDialog` renders under the dialog title. The wording is carried verbatim, finality warning included. The four generated locale bundles were regenerated (`os i18n extract` merge mode). Because a renamed key reads as a new gap, `--fill=default` seeded the new `description` leaf with English in zh-CN / ja-JP / es-ES, silently discarding the curated translations of the very same sentence; those were carried across by hand and pinned, since `check:i18n` cannot see that loss — an English string in a non-English locale is exactly what a fresh extract produces. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015fkdTyGmMD5s8ZtEifvuGy --- .changeset/approval-decision-one-dialog.md | 18 +++++ .../src/sys-approval-request.object.test.ts | 52 ++++++++++++- .../src/sys-approval-request.object.ts | 14 +++- .../decision-question-i18n.test.ts | 74 +++++++++++++++++++ .../src/translations/en.objects.generated.ts | 4 +- .../translations/es-ES.objects.generated.ts | 4 +- .../translations/ja-JP.objects.generated.ts | 4 +- .../translations/zh-CN.objects.generated.ts | 4 +- 8 files changed, 163 insertions(+), 11 deletions(-) create mode 100644 .changeset/approval-decision-one-dialog.md create mode 100644 packages/plugins/plugin-approvals/src/translations/decision-question-i18n.test.ts diff --git a/.changeset/approval-decision-one-dialog.md b/.changeset/approval-decision-one-dialog.md new file mode 100644 index 0000000000..2ec2586f36 --- /dev/null +++ b/.changeset/approval-decision-one-dialog.md @@ -0,0 +1,18 @@ +--- +'@objectstack/plugin-approvals': patch +--- + +approvals: rejecting or recalling a request now opens ONE dialog instead of two + +`sys_approval_request`'s `approval_reject` and `approval_recall` actions declared +both `confirmText` and `params`. The console action runner chains confirmation +**then** param collection, both awaited, so a single decision opened a confirm +prompt, then a second dialog the approver never asked for — and nothing was sent +until that second Confirm, while the first prompt already read as "the action is +running". + +Each action now carries its confirm question in the action's top-level +`description` (the key added in #7367), which the param dialog renders under its +title. The wording is unchanged in all four shipped locales — including the +finality warning "A rejection is final for every approver." — so one decision is +one condition, one wording, one dialog, and nothing is sent until its own Confirm. diff --git a/packages/plugins/plugin-approvals/src/sys-approval-request.object.test.ts b/packages/plugins/plugin-approvals/src/sys-approval-request.object.test.ts index 0b3c6f0d86..73fc5c7c21 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-request.object.test.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-request.object.test.ts @@ -89,7 +89,57 @@ describe('sys_approval_request declared actions', () => { it('recall stays available while a returned request is still the submitter\'s to abandon', () => { expect(vis('approval_recall')).toContain('record.status == "returned"'); - expect(byName('approval_recall').confirmText).toBeTruthy(); + }); + + // ── One decision, one dialog (#7278) ────────────────────────────── + // The console action runner chains confirm THEN param collection, both + // awaited, so an action declaring `confirmText` *and* `params` shows two + // sequential dialogs for a single decision — and the first one already reads + // as "the action ran". The maintainer's 2026-08-10 ruling on #7278: carry the + // confirm question in the action's top-level `description` (#7367), which the + // param dialog renders under its title, and drop `confirmText`. Nothing is + // sent until that one dialog's own Confirm. + // + // These pin the WORDING, not just the shape: the previous round rejected + // dropping `confirmText` outright precisely because it would have deleted the + // finality warning from the most irreversible surface in the product. + it('the confirm question rides `description`, not `confirmText`, so one decision opens one dialog (#7278)', () => { + const reject = byName('approval_reject'); + expect(reject.confirmText).toBeUndefined(); + expect(reject.description).toBe( + 'Reject this request? A rejection is final for every approver.', + ); + // the finality warning is the half that must survive the move + expect(reject.description).toContain('final for every approver'); + + const recall = byName('approval_recall'); + expect(recall.confirmText).toBeUndefined(); + expect(recall.description).toBe( + 'Recall this request? Approvers can no longer act on it and the record is unlocked.', + ); + expect(recall.description).toContain('can no longer act on it'); + }); + + it('no declared action pairs `confirmText` with `params` (#7278 ruling, object-wide)', () => { + const doubled = actions + .filter((a) => a.confirmText && Array.isArray(a.params) && a.params.length > 0) + .map((a) => a.name); + expect( + doubled, + 'these actions would open a confirm dialog and then a param dialog for one decision — ' + + 'move the question to the action\'s top-level `description` (#7278). ' + + 'NB: the top-level key, never `ai.description` (the LLM-facing tool contract).', + ).toEqual([]); + }); + + it('the confirm question is human dialog copy, never armed as an AI tool description', () => { + // `ActionAiSchema.description` is the LLM-facing contract (min 40 chars, + // required when `ai.exposed`) — the same name one level down. Putting the + // question there would arm a tool description while the dialog fell back to + // its generic line. + for (const name of ['approval_reject', 'approval_recall']) { + expect(byName(name).ai?.description, `${name}.ai.description`).toBeUndefined(); + } }); it('reassign collects the new approver via a field-backed sys_user picker keyed as `to`', () => { diff --git a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts index 60f674bec2..fd23c9f345 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts @@ -276,6 +276,15 @@ export const SysApprovalRequest = ObjectSchema.create({ { name: 'approval_reject', label: 'Reject', + // The confirm question lives HERE, not in `confirmText`: this action + // collects params, so the console would otherwise chain a confirm dialog + // and then the param dialog for one decision — and the first one already + // reads as "the action ran" (#7278, maintainer ruling 2026-08-10). The + // param dialog renders this as its description, and nothing is POSTed + // until its own Confirm: one condition, one wording, one dialog. + // NB: the top-level action `description` (#7367), never `ai.description` + // — that one is the LLM-facing tool contract and is not shown to anyone. + description: 'Reject this request? A rejection is final for every approver.', icon: 'x-circle', // Destructive decision — rendered in the console's danger styling so it // reads as the irreversible action it is (objectui#2762 P1-5). @@ -288,7 +297,6 @@ export const SysApprovalRequest = ObjectSchema.create({ { name: 'attachments', label: 'Attachments', type: 'file', multiple: true, required: false }, ], visible: 'record.viewer.can_act || record.viewer.can_override', - confirmText: 'Reject this request? A rejection is final for every approver.', locations: ['record_section', 'list_item'], successMessage: 'Rejected.', refreshAfter: true, @@ -374,6 +382,9 @@ export const SysApprovalRequest = ObjectSchema.create({ { name: 'approval_recall', label: 'Recall', + // Confirm question as the param dialog's description, not `confirmText` + // — same one-decision-one-dialog rule as `approval_reject` above (#7278). + description: 'Recall this request? Approvers can no longer act on it and the record is unlocked.', icon: 'undo-2', type: 'api', method: 'POST', @@ -384,7 +395,6 @@ export const SysApprovalRequest = ObjectSchema.create({ // Recall applies while the request is live for the submitter — pending // (withdraw) or returned (abandon the revision instead of resubmitting). visible: '(record.status == "pending" || record.status == "returned") && record.viewer.is_submitter', - confirmText: 'Recall this request? Approvers can no longer act on it and the record is unlocked.', locations: ['record_section'], successMessage: 'Recalled.', refreshAfter: true, diff --git a/packages/plugins/plugin-approvals/src/translations/decision-question-i18n.test.ts b/packages/plugins/plugin-approvals/src/translations/decision-question-i18n.test.ts new file mode 100644 index 0000000000..587f3db00a --- /dev/null +++ b/packages/plugins/plugin-approvals/src/translations/decision-question-i18n.test.ts @@ -0,0 +1,74 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Translation-carryover guard for the two decision questions (#7278). +// +// `approval_reject` / `approval_recall` used to ask their confirm question via +// `confirmText`; the maintainer's 2026-08-10 ruling moved it to the action's +// top-level `description` so one decision opens one dialog instead of two. +// The wording did not change — but the KEY did, and `os i18n extract` treats a +// renamed key as a brand-new gap: with `--fill=default` it seeds the new leaf +// from the English source in every locale, so the curated zh-CN / ja-JP / es-ES +// strings for the very same sentence were overwritten with English by the +// regeneration that accompanied the move. They were carried across by hand. +// +// Nothing else would notice. `check:i18n` compares the bundles against a fresh +// merge-mode extract, and English-in-a-non-English-locale is perfectly "in +// sync" — the bundle is what the extractor produces. So the loss is invisible +// to the drift gate by construction, on the most irreversible surface in the +// product, in three of the four shipped locales. +// +// This turns that into a red test: each locale's decision question must be +// translated, not the English literal. + +import { describe, it, expect } from 'vitest'; +import { zhCNObjects } from './zh-CN.objects.generated.js'; +import { jaJPObjects } from './ja-JP.objects.generated.js'; +import { esESObjects } from './es-ES.objects.generated.js'; +import { enObjects } from './en.objects.generated.js'; + +const LOCALES = [ + ['zh-CN', zhCNObjects], + ['ja-JP', jaJPObjects], + ['es-ES', esESObjects], +] as const; + +const ACTIONS = ['approval_reject', 'approval_recall'] as const; + +const question = (bundle: any, action: string): string | undefined => + bundle?.sys_approval_request?._actions?.[action]?.description; + +describe('sys_approval_request decision questions stay translated (#7278)', () => { + it('the English bundle carries the question on `description`, not `confirmText`', () => { + for (const action of ACTIONS) { + const node = (enObjects as any).sys_approval_request._actions[action]; + expect(node.confirmText, `${action}.confirmText`).toBeUndefined(); + expect(node.description, `${action}.description`).toBeTruthy(); + } + expect(question(enObjects, 'approval_reject')).toContain('final for every approver'); + expect(question(enObjects, 'approval_recall')).toContain('can no longer act on it'); + }); + + it('every non-English locale translates it instead of echoing the English source', () => { + for (const [locale, bundle] of LOCALES) { + for (const action of ACTIONS) { + const translated = question(bundle, action); + expect(translated, `${locale} ${action}.description missing`).toBeTruthy(); + expect( + translated, + `${locale} ${action}.description is the untranslated English source — a re-run of ` + + '`os i18n extract` seeds new keys from English, so the curated string was lost. ' + + 'Restore the translation (the wording is unchanged from the old `confirmText`).', + ).not.toBe(question(enObjects, action)); + } + } + }); + + it('no locale left the retired `confirmText` behind on these two actions', () => { + for (const [locale, bundle] of LOCALES) { + for (const action of ACTIONS) { + const node = (bundle as any).sys_approval_request._actions[action]; + expect(node.confirmText, `${locale} ${action}.confirmText`).toBeUndefined(); + } + } + }); +}); diff --git a/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts index f526fc7027..28d00b1362 100644 --- a/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts @@ -118,7 +118,7 @@ export const enObjects: NonNullable = { }, approval_reject: { label: "Reject", - confirmText: "Reject this request? A rejection is final for every approver.", + description: "Reject this request? A rejection is final for every approver.", successMessage: "Rejected.", params: { comment: { @@ -171,7 +171,7 @@ export const enObjects: NonNullable = { }, approval_recall: { label: "Recall", - confirmText: "Recall this request? Approvers can no longer act on it and the record is unlocked.", + description: "Recall this request? Approvers can no longer act on it and the record is unlocked.", successMessage: "Recalled.", params: { comment: { diff --git a/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts index a535ed3e63..6094a97e52 100644 --- a/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts @@ -118,7 +118,7 @@ export const esESObjects: NonNullable = { }, approval_reject: { label: "Rechazar", - confirmText: "¿Rechazar esta solicitud? Un rechazo es definitivo para todos los aprobadores.", + description: "¿Rechazar esta solicitud? Un rechazo es definitivo para todos los aprobadores.", successMessage: "Rechazada.", params: { comment: { @@ -171,7 +171,7 @@ export const esESObjects: NonNullable = { }, approval_recall: { label: "Retirar", - confirmText: "¿Retirar esta solicitud? Los aprobadores ya no podrán actuar sobre ella y el registro se desbloqueará.", + description: "¿Retirar esta solicitud? Los aprobadores ya no podrán actuar sobre ella y el registro se desbloqueará.", successMessage: "Retirada.", params: { comment: { diff --git a/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts index fe0295140e..7274f2c975 100644 --- a/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts @@ -118,7 +118,7 @@ export const jaJPObjects: NonNullable = { }, approval_reject: { label: "却下", - confirmText: "このリクエストを却下しますか？却下はすべての承認者に対して最終決定になります。", + description: "このリクエストを却下しますか？却下はすべての承認者に対して最終決定になります。", successMessage: "却下しました。", params: { comment: { @@ -171,7 +171,7 @@ export const jaJPObjects: NonNullable = { }, approval_recall: { label: "取り下げ", - confirmText: "このリクエストを取り下げますか？承認者は操作できなくなり、レコードのロックが解除されます。", + description: "このリクエストを取り下げますか？承認者は操作できなくなり、レコードのロックが解除されます。", successMessage: "取り下げました。", params: { comment: { diff --git a/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts index 5e8f2a6445..603095302a 100644 --- a/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts @@ -118,7 +118,7 @@ export const zhCNObjects: NonNullable = { }, approval_reject: { label: "拒绝", - confirmText: "拒绝该请求？拒绝对所有审批人立即生效。", + description: "拒绝该请求？拒绝对所有审批人立即生效。", successMessage: "已拒绝。", params: { comment: { @@ -171,7 +171,7 @@ export const zhCNObjects: NonNullable = { }, approval_recall: { label: "撤回", - confirmText: "撤回该请求？撤回后审批人将无法继续处理，记录随即解锁。", + description: "撤回该请求？撤回后审批人将无法继续处理，记录随即解锁。", successMessage: "已撤回。", params: { comment: {