Skip to content

Commit d9cdda6

Browse files
baozhoutaoclaude
andauthored
fix(approvals): record-header Reject fires after one dialog again (#3126) (#3128)
Since #2961 made the record header's decision inputs live, Reject carried both `confirmText` and a collectable comment param. The ActionRunner chains confirm before param collection, so rejecting queued TWO dialogs — after "Continue" the request silently waited on a second, unexpected comment dialog, which downstream read as a dead button (zero requests, no toast). Drop `confirmText` from the Reject decision action: the param dialog is the confirmation surface, titled by the action and carrying the old confirm question (same `approvals.rejectConfirm` key) as its description. One decision, one dialog — matching Approve and the Approval Center. Verified end-to-end in a real Chromium harness: Reject → single dialog → Confirm → `reject_request` dispatched. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 1ce750b commit d9cdda6

3 files changed

Lines changed: 47 additions & 2 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
"@object-ui/app-shell": patch
3+
---
4+
5+
fix(approvals): record-header Reject fires after ONE dialog again (objectui#3126)
6+
7+
Since #2961 made the record header's decision inputs live (`actionParams`),
8+
the Reject action carried BOTH `confirmText` and a collectable comment param.
9+
The ActionRunner chains confirmation before param collection, so rejecting
10+
queued two dialogs: the approver answered "Reject this approval request? →
11+
Continue", the alertdialog closed — and no request fired, because it was
12+
waiting on a second, unexpected "Action parameters / Comment (optional)"
13+
dialog. Anyone on the rc.0 contract (one confirm → request) read that as a
14+
silent no-op: zero network traffic, no toast, the flow stuck pending. Approve
15+
never declared `confirmText`, which is why it kept working on the same node.
16+
17+
The Reject action no longer declares `confirmText`. The param dialog is the
18+
confirmation surface: it is titled by the action ("Reject"), carries the old
19+
confirm question as its description (same `approvals.rejectConfirm` i18n key,
20+
so every locale keeps its translation), collects the optional comment and any
21+
declared decision outputs, and nothing is sent until its own Confirm — one
22+
decision, one dialog, matching Approve and the Approval Center.

packages/app-shell/src/views/RecordDetailView.approvalDecisionActions.test.tsx

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,22 @@ describe('buildApprovalDecisionActions — param key (objectui#2955)', () => {
5656
const actions = byName(buildApprovalDecisionActions(pending(), t) as any[]);
5757
expect(actions.approve_request.order).toBe(-100);
5858
expect(actions.reject_request.order).toBe(-99);
59-
expect(actions.reject_request.confirmText).toBeDefined();
59+
});
60+
});
61+
62+
describe('buildApprovalDecisionActions — one dialog per decision (objectui#3126)', () => {
63+
it('never carries `confirmText` — the param dialog is the confirmation', () => {
64+
// The runner chains confirm THEN param collection, so `confirmText` +
65+
// `actionParams` queued TWO dialogs: after "Continue" nothing was sent
66+
// until a second, unexpected comment dialog was also confirmed — the
67+
// #3126 "reject silently does nothing" report. One decision, one dialog.
68+
const actions = buildApprovalDecisionActions(pending(), t) as any[];
69+
for (const a of actions) expect(a.confirmText).toBeUndefined();
70+
});
71+
72+
it('keeps the reject confirm question as the dialog description', () => {
73+
const actions = byName(buildApprovalDecisionActions(pending(), t) as any[]);
74+
expect(actions.reject_request.description).toBe('Reject this approval request?');
6075
});
6176
});
6277

packages/app-shell/src/views/RecordDetailView.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,15 @@ export function buildApprovalDecisionActions(
228228
order: -99,
229229
locations: ['record_header'],
230230
refreshAfter: true,
231-
confirmText: t('approvals.rejectConfirm', { defaultValue: 'Reject this approval request?' }),
231+
// NO `confirmText` here (objectui#3126). The runner chains confirm THEN
232+
// param collection, so carrying both queued two dialogs: the approver
233+
// answered "Reject this approval request? → Continue" and the request
234+
// still didn't fire — it waited on a second, unexpected comment dialog
235+
// (opened since #2961 made `actionParams` live on this surface), which
236+
// reads as a silent no-op. The param dialog IS the confirmation: it is
237+
// titled by this label, carries the confirm question as its description,
238+
// and nothing is sent until its own Confirm.
239+
description: t('approvals.rejectConfirm', { defaultValue: 'Reject this approval request?' }),
232240
actionParams: decisionParams('reject'),
233241
successMessage: t('approvals.rejectSuccess', { defaultValue: 'Rejected' }),
234242
},

0 commit comments

Comments
 (0)