Skip to content

Commit 7abdd74

Browse files
os-helpclaude
andauthored
fix(approvals): one decision, one dialog — carry reject/recall confirm questions on description (#7278) (#7592)
`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. Claude-Session: https://claude.ai/code/session_015fkdTyGmMD5s8ZtEifvuGy Co-authored-by: Claude <noreply@anthropic.com>
1 parent 744b8f5 commit 7abdd74

8 files changed

Lines changed: 163 additions & 11 deletions
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
'@objectstack/plugin-approvals': patch
3+
---
4+
5+
approvals: rejecting or recalling a request now opens ONE dialog instead of two
6+
7+
`sys_approval_request`'s `approval_reject` and `approval_recall` actions declared
8+
both `confirmText` and `params`. The console action runner chains confirmation
9+
**then** param collection, both awaited, so a single decision opened a confirm
10+
prompt, then a second dialog the approver never asked for — and nothing was sent
11+
until that second Confirm, while the first prompt already read as "the action is
12+
running".
13+
14+
Each action now carries its confirm question in the action's top-level
15+
`description` (the key added in #7367), which the param dialog renders under its
16+
title. The wording is unchanged in all four shipped locales — including the
17+
finality warning "A rejection is final for every approver." — so one decision is
18+
one condition, one wording, one dialog, and nothing is sent until its own Confirm.

packages/plugins/plugin-approvals/src/sys-approval-request.object.test.ts

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,57 @@ describe('sys_approval_request declared actions', () => {
8989

9090
it('recall stays available while a returned request is still the submitter\'s to abandon', () => {
9191
expect(vis('approval_recall')).toContain('record.status == "returned"');
92-
expect(byName('approval_recall').confirmText).toBeTruthy();
92+
});
93+
94+
// ── One decision, one dialog (#7278) ──────────────────────────────
95+
// The console action runner chains confirm THEN param collection, both
96+
// awaited, so an action declaring `confirmText` *and* `params` shows two
97+
// sequential dialogs for a single decision — and the first one already reads
98+
// as "the action ran". The maintainer's 2026-08-10 ruling on #7278: carry the
99+
// confirm question in the action's top-level `description` (#7367), which the
100+
// param dialog renders under its title, and drop `confirmText`. Nothing is
101+
// sent until that one dialog's own Confirm.
102+
//
103+
// These pin the WORDING, not just the shape: the previous round rejected
104+
// dropping `confirmText` outright precisely because it would have deleted the
105+
// finality warning from the most irreversible surface in the product.
106+
it('the confirm question rides `description`, not `confirmText`, so one decision opens one dialog (#7278)', () => {
107+
const reject = byName('approval_reject');
108+
expect(reject.confirmText).toBeUndefined();
109+
expect(reject.description).toBe(
110+
'Reject this request? A rejection is final for every approver.',
111+
);
112+
// the finality warning is the half that must survive the move
113+
expect(reject.description).toContain('final for every approver');
114+
115+
const recall = byName('approval_recall');
116+
expect(recall.confirmText).toBeUndefined();
117+
expect(recall.description).toBe(
118+
'Recall this request? Approvers can no longer act on it and the record is unlocked.',
119+
);
120+
expect(recall.description).toContain('can no longer act on it');
121+
});
122+
123+
it('no declared action pairs `confirmText` with `params` (#7278 ruling, object-wide)', () => {
124+
const doubled = actions
125+
.filter((a) => a.confirmText && Array.isArray(a.params) && a.params.length > 0)
126+
.map((a) => a.name);
127+
expect(
128+
doubled,
129+
'these actions would open a confirm dialog and then a param dialog for one decision — '
130+
+ 'move the question to the action\'s top-level `description` (#7278). '
131+
+ 'NB: the top-level key, never `ai.description` (the LLM-facing tool contract).',
132+
).toEqual([]);
133+
});
134+
135+
it('the confirm question is human dialog copy, never armed as an AI tool description', () => {
136+
// `ActionAiSchema.description` is the LLM-facing contract (min 40 chars,
137+
// required when `ai.exposed`) — the same name one level down. Putting the
138+
// question there would arm a tool description while the dialog fell back to
139+
// its generic line.
140+
for (const name of ['approval_reject', 'approval_recall']) {
141+
expect(byName(name).ai?.description, `${name}.ai.description`).toBeUndefined();
142+
}
93143
});
94144

95145
it('reassign collects the new approver via a field-backed sys_user picker keyed as `to`', () => {

packages/plugins/plugin-approvals/src/sys-approval-request.object.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,15 @@ export const SysApprovalRequest = ObjectSchema.create({
276276
{
277277
name: 'approval_reject',
278278
label: 'Reject',
279+
// The confirm question lives HERE, not in `confirmText`: this action
280+
// collects params, so the console would otherwise chain a confirm dialog
281+
// and then the param dialog for one decision — and the first one already
282+
// reads as "the action ran" (#7278, maintainer ruling 2026-08-10). The
283+
// param dialog renders this as its description, and nothing is POSTed
284+
// until its own Confirm: one condition, one wording, one dialog.
285+
// NB: the top-level action `description` (#7367), never `ai.description`
286+
// — that one is the LLM-facing tool contract and is not shown to anyone.
287+
description: 'Reject this request? A rejection is final for every approver.',
279288
icon: 'x-circle',
280289
// Destructive decision — rendered in the console's danger styling so it
281290
// reads as the irreversible action it is (objectui#2762 P1-5).
@@ -288,7 +297,6 @@ export const SysApprovalRequest = ObjectSchema.create({
288297
{ name: 'attachments', label: 'Attachments', type: 'file', multiple: true, required: false },
289298
],
290299
visible: 'record.viewer.can_act || record.viewer.can_override',
291-
confirmText: 'Reject this request? A rejection is final for every approver.',
292300
locations: ['record_section', 'list_item'],
293301
successMessage: 'Rejected.',
294302
refreshAfter: true,
@@ -374,6 +382,9 @@ export const SysApprovalRequest = ObjectSchema.create({
374382
{
375383
name: 'approval_recall',
376384
label: 'Recall',
385+
// Confirm question as the param dialog's description, not `confirmText`
386+
// — same one-decision-one-dialog rule as `approval_reject` above (#7278).
387+
description: 'Recall this request? Approvers can no longer act on it and the record is unlocked.',
377388
icon: 'undo-2',
378389
type: 'api',
379390
method: 'POST',
@@ -384,7 +395,6 @@ export const SysApprovalRequest = ObjectSchema.create({
384395
// Recall applies while the request is live for the submitter — pending
385396
// (withdraw) or returned (abandon the revision instead of resubmitting).
386397
visible: '(record.status == "pending" || record.status == "returned") && record.viewer.is_submitter',
387-
confirmText: 'Recall this request? Approvers can no longer act on it and the record is unlocked.',
388398
locations: ['record_section'],
389399
successMessage: 'Recalled.',
390400
refreshAfter: true,
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// Translation-carryover guard for the two decision questions (#7278).
4+
//
5+
// `approval_reject` / `approval_recall` used to ask their confirm question via
6+
// `confirmText`; the maintainer's 2026-08-10 ruling moved it to the action's
7+
// top-level `description` so one decision opens one dialog instead of two.
8+
// The wording did not change — but the KEY did, and `os i18n extract` treats a
9+
// renamed key as a brand-new gap: with `--fill=default` it seeds the new leaf
10+
// from the English source in every locale, so the curated zh-CN / ja-JP / es-ES
11+
// strings for the very same sentence were overwritten with English by the
12+
// regeneration that accompanied the move. They were carried across by hand.
13+
//
14+
// Nothing else would notice. `check:i18n` compares the bundles against a fresh
15+
// merge-mode extract, and English-in-a-non-English-locale is perfectly "in
16+
// sync" — the bundle is what the extractor produces. So the loss is invisible
17+
// to the drift gate by construction, on the most irreversible surface in the
18+
// product, in three of the four shipped locales.
19+
//
20+
// This turns that into a red test: each locale's decision question must be
21+
// translated, not the English literal.
22+
23+
import { describe, it, expect } from 'vitest';
24+
import { zhCNObjects } from './zh-CN.objects.generated.js';
25+
import { jaJPObjects } from './ja-JP.objects.generated.js';
26+
import { esESObjects } from './es-ES.objects.generated.js';
27+
import { enObjects } from './en.objects.generated.js';
28+
29+
const LOCALES = [
30+
['zh-CN', zhCNObjects],
31+
['ja-JP', jaJPObjects],
32+
['es-ES', esESObjects],
33+
] as const;
34+
35+
const ACTIONS = ['approval_reject', 'approval_recall'] as const;
36+
37+
const question = (bundle: any, action: string): string | undefined =>
38+
bundle?.sys_approval_request?._actions?.[action]?.description;
39+
40+
describe('sys_approval_request decision questions stay translated (#7278)', () => {
41+
it('the English bundle carries the question on `description`, not `confirmText`', () => {
42+
for (const action of ACTIONS) {
43+
const node = (enObjects as any).sys_approval_request._actions[action];
44+
expect(node.confirmText, `${action}.confirmText`).toBeUndefined();
45+
expect(node.description, `${action}.description`).toBeTruthy();
46+
}
47+
expect(question(enObjects, 'approval_reject')).toContain('final for every approver');
48+
expect(question(enObjects, 'approval_recall')).toContain('can no longer act on it');
49+
});
50+
51+
it('every non-English locale translates it instead of echoing the English source', () => {
52+
for (const [locale, bundle] of LOCALES) {
53+
for (const action of ACTIONS) {
54+
const translated = question(bundle, action);
55+
expect(translated, `${locale} ${action}.description missing`).toBeTruthy();
56+
expect(
57+
translated,
58+
`${locale} ${action}.description is the untranslated English source — a re-run of `
59+
+ '`os i18n extract` seeds new keys from English, so the curated string was lost. '
60+
+ 'Restore the translation (the wording is unchanged from the old `confirmText`).',
61+
).not.toBe(question(enObjects, action));
62+
}
63+
}
64+
});
65+
66+
it('no locale left the retired `confirmText` behind on these two actions', () => {
67+
for (const [locale, bundle] of LOCALES) {
68+
for (const action of ACTIONS) {
69+
const node = (bundle as any).sys_approval_request._actions[action];
70+
expect(node.confirmText, `${locale} ${action}.confirmText`).toBeUndefined();
71+
}
72+
}
73+
});
74+
});

packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ export const enObjects: NonNullable<TranslationData['objects']> = {
118118
},
119119
approval_reject: {
120120
label: "Reject",
121-
confirmText: "Reject this request? A rejection is final for every approver.",
121+
description: "Reject this request? A rejection is final for every approver.",
122122
successMessage: "Rejected.",
123123
params: {
124124
comment: {
@@ -171,7 +171,7 @@ export const enObjects: NonNullable<TranslationData['objects']> = {
171171
},
172172
approval_recall: {
173173
label: "Recall",
174-
confirmText: "Recall this request? Approvers can no longer act on it and the record is unlocked.",
174+
description: "Recall this request? Approvers can no longer act on it and the record is unlocked.",
175175
successMessage: "Recalled.",
176176
params: {
177177
comment: {

packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ export const esESObjects: NonNullable<TranslationData['objects']> = {
118118
},
119119
approval_reject: {
120120
label: "Rechazar",
121-
confirmText: "¿Rechazar esta solicitud? Un rechazo es definitivo para todos los aprobadores.",
121+
description: "¿Rechazar esta solicitud? Un rechazo es definitivo para todos los aprobadores.",
122122
successMessage: "Rechazada.",
123123
params: {
124124
comment: {
@@ -171,7 +171,7 @@ export const esESObjects: NonNullable<TranslationData['objects']> = {
171171
},
172172
approval_recall: {
173173
label: "Retirar",
174-
confirmText: "¿Retirar esta solicitud? Los aprobadores ya no podrán actuar sobre ella y el registro se desbloqueará.",
174+
description: "¿Retirar esta solicitud? Los aprobadores ya no podrán actuar sobre ella y el registro se desbloqueará.",
175175
successMessage: "Retirada.",
176176
params: {
177177
comment: {

packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ export const jaJPObjects: NonNullable<TranslationData['objects']> = {
118118
},
119119
approval_reject: {
120120
label: "却下",
121-
confirmText: "このリクエストを却下しますか?却下はすべての承認者に対して最終決定になります。",
121+
description: "このリクエストを却下しますか?却下はすべての承認者に対して最終決定になります。",
122122
successMessage: "却下しました。",
123123
params: {
124124
comment: {
@@ -171,7 +171,7 @@ export const jaJPObjects: NonNullable<TranslationData['objects']> = {
171171
},
172172
approval_recall: {
173173
label: "取り下げ",
174-
confirmText: "このリクエストを取り下げますか?承認者は操作できなくなり、レコードのロックが解除されます。",
174+
description: "このリクエストを取り下げますか?承認者は操作できなくなり、レコードのロックが解除されます。",
175175
successMessage: "取り下げました。",
176176
params: {
177177
comment: {

packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ export const zhCNObjects: NonNullable<TranslationData['objects']> = {
118118
},
119119
approval_reject: {
120120
label: "拒绝",
121-
confirmText: "拒绝该请求?拒绝对所有审批人立即生效。",
121+
description: "拒绝该请求?拒绝对所有审批人立即生效。",
122122
successMessage: "已拒绝。",
123123
params: {
124124
comment: {
@@ -171,7 +171,7 @@ export const zhCNObjects: NonNullable<TranslationData['objects']> = {
171171
},
172172
approval_recall: {
173173
label: "撤回",
174-
confirmText: "撤回该请求?撤回后审批人将无法继续处理,记录随即解锁。",
174+
description: "撤回该请求?撤回后审批人将无法继续处理,记录随即解锁。",
175175
successMessage: "已撤回。",
176176
params: {
177177
comment: {

0 commit comments

Comments
 (0)