Skip to content

Commit 3c25cf8

Browse files
os-zhuangclaude
andcommitted
feat(approvals): recall endpoint + business-readable inbox contract
The Console approvals inbox surfaced four service-level gaps (found via e2e review of the showcase app): - Recall was advertised in the contract (status 'recalled', action 'recall') and rendered in the Console UI, but never implemented — the button always died with HTTP 404. Implement ApprovalService.recall() (submitter-only, audits a 'recall' action, mirrors the status field, resumes the owning flow run down the reject branch with output.decision='recall' since the engine has no run-cancel primitive) and register POST /approvals/requests/:id/recall. - Rows never carried submitted_at; the inbox showed "—" for every request and its newest-first sort compared empty strings. Expose submitted_at as an alias of created_at on the row mapper. - Requests displayed machine names (flow:manager_review, opaque record ids, raw user ids). Seed $flowName/$flowLabel into engine variables, snapshot authored flow/node labels onto node_config_json (no schema migration), and surface process_label/step_label with a prettified fallback for legacy rows. Enrich listRequests/getRequest with record_title (schema displayNameField, payload-snapshot fallback) and submitter_name (sys_user by id or email), batched per object. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 2047fa1 commit 3c25cf8

7 files changed

Lines changed: 397 additions & 3 deletions

File tree

packages/plugins/plugin-approvals/src/approval-node.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,20 @@ describe('Approval node bridge (ADR-0019)', () => {
122122
expect(suspended[0]).toMatchObject({ nodeId: 'approve_step', correlation: requests[0].id });
123123
});
124124

125+
it('carries the flow name + authored labels onto the request row', async () => {
126+
registerDecisionFlow(automation, [{ type: 'user', value: 'u1' }]);
127+
await automation.execute('deal_approval', {
128+
object: 'crm_deal', record: { id: 'd1', amount: 100 }, userId: 'submitter',
129+
});
130+
const [raw] = await fake.find('sys_approval_request', { where: { status: 'pending' } });
131+
// Engine-seeded `$flowName` (not the node id) names the source…
132+
expect(raw.process_name).toBe('flow:deal_approval');
133+
// …and authored labels ride the config snapshot for inbox display.
134+
const req = (await service.listRequests({ status: 'pending' }, { isSystem: true } as any))[0];
135+
expect(req.process_label).toBe('Deal Approval');
136+
expect(req.step_label).toBe('Manager Approval');
137+
});
138+
125139
it('resumes down the approve branch on approval', async () => {
126140
registerDecisionFlow(automation, [{ type: 'user', value: 'u1' }]);
127141
const paused = await automation.execute('deal_approval', {

packages/plugins/plugin-approvals/src/approval-node.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,22 @@ export function registerApprovalNode(
9797
if (!object) return { success: false, error: `Approval node '${node.id}': no target object in context` };
9898
if (!recordId) return { success: false, error: `Approval node '${node.id}': no record id in $record` };
9999

100+
// Flow identity comes from engine-seeded variables (`$flowName` /
101+
// `$flowLabel`) so the request row can carry a human-readable origin;
102+
// `context.flowName` is a legacy fallback for direct callers.
103+
const flowName = (variables.get('$flowName') as string | undefined) ?? context?.flowName;
104+
const flowLabel = variables.get('$flowLabel') as string | undefined;
105+
100106
try {
101107
const request = await service.openNodeRequest({
102108
object,
103109
recordId: String(recordId),
104110
runId: String(runId),
105111
nodeId: node.id,
106112
config,
107-
flowName: context?.flowName,
113+
flowName,
114+
flowLabel,
115+
nodeLabel: typeof node.label === 'string' ? node.label : undefined,
108116
submitterId: context?.userId ?? null,
109117
record,
110118
organizationId: context?.organizationId ?? context?.tenantId ?? null,

packages/plugins/plugin-approvals/src/approval-service.test.ts

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,98 @@ describe('ApprovalService (node era)', () => {
290290
it('getRequest: returns null for an unknown id', async () => {
291291
expect(await svc.getRequest('nope', SYS)).toBeNull();
292292
});
293+
294+
// ── recall ──────────────────────────────────────────────────────
295+
296+
it('recall: submitter withdraws a pending request', async () => {
297+
const req = await svc.openNodeRequest(openInput(['u9']), CTX);
298+
const out = await svc.recall(req.id, { actorId: 'u1', comment: 'changed my mind' }, CTX);
299+
expect(out.request.status).toBe('recalled');
300+
expect(out.request.completed_at).toBeTruthy();
301+
expect(out.request.pending_approvers).toEqual([]);
302+
const actions = await svc.listActions(req.id, SYS);
303+
expect(actions.map(a => a.action)).toEqual(['submit', 'recall']);
304+
expect(actions[1].comment).toBe('changed my mind');
305+
});
306+
307+
it('recall: blocks a non-submitter in a non-system context', async () => {
308+
const req = await svc.openNodeRequest(openInput(['u9']), CTX);
309+
await expect(svc.recall(req.id, { actorId: 'u9' }, { roles: [], permissions: [] } as any))
310+
.rejects.toThrow(/FORBIDDEN/);
311+
});
312+
313+
it('recall: rejects a recall on a non-pending request', async () => {
314+
const req = await svc.openNodeRequest(openInput(['u9']), CTX);
315+
await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, SYS);
316+
await expect(svc.recall(req.id, { actorId: 'u1' }, SYS)).rejects.toThrow(/INVALID_STATE/);
317+
});
318+
319+
it('recall: resumes the owning run down the reject branch with decision=recall', async () => {
320+
const resumed: any[] = [];
321+
svc.attachAutomation({ async resume(runId, signal) { resumed.push({ runId, signal }); } });
322+
const req = await svc.openNodeRequest(openInput(['u9']), CTX);
323+
const out = await svc.recall(req.id, { actorId: 'u1' }, CTX);
324+
expect(out.resumed).toBe(true);
325+
expect(resumed[0]).toMatchObject({
326+
runId: 'run_1',
327+
signal: { branchLabel: 'reject', output: { decision: 'recall' } },
328+
});
329+
});
330+
331+
it('recall: mirrors `recalled` onto the business record when configured', async () => {
332+
engine._tables['opportunity'] = [{ id: 'opp1', amount: 100 }];
333+
const req = await svc.openNodeRequest(openInput(['u9'], {}, { approvalStatusField: 'approval_status' }), CTX);
334+
await svc.recall(req.id, { actorId: 'u1' }, CTX);
335+
expect(engine._tables['opportunity'][0].approval_status).toBe('recalled');
336+
});
337+
338+
// ── inbox display fields ────────────────────────────────────────
339+
340+
it('rows expose submitted_at as an alias of created_at', async () => {
341+
const req = await svc.openNodeRequest(openInput(['u9']), CTX);
342+
expect(req.submitted_at).toBeTruthy();
343+
expect(req.submitted_at).toBe(req.created_at);
344+
const listed = await svc.listRequests({ status: 'pending' }, SYS);
345+
expect(listed[0].submitted_at).toBe(listed[0].created_at);
346+
});
347+
348+
it('rows carry authored flow/node labels when provided', async () => {
349+
const req = await svc.openNodeRequest(
350+
openInput(['u9'], { flowLabel: 'Deal Approval', nodeLabel: 'Manager Review' }), CTX,
351+
);
352+
expect(req.process_label).toBe('Deal Approval');
353+
expect(req.step_label).toBe('Manager Review');
354+
});
355+
356+
it('rows fall back to prettified machine names when labels are absent', async () => {
357+
const req = await svc.openNodeRequest(openInput(['u9']), CTX);
358+
expect(req.process_label).toBe('Deal Approval'); // from `flow:deal_approval`
359+
expect(req.step_label).toBe('Approve Step'); // from `approve_step`
360+
});
361+
362+
it('listRequests enriches record_title and submitter_name', async () => {
363+
engine._tables['opportunity'] = [{ id: 'opp1', name: 'Acme Renewal', amount: 100 }];
364+
engine._tables['sys_user'] = [{ id: 'u1', name: 'Ada Lovelace', email: 'ada@example.com' }];
365+
await svc.openNodeRequest(openInput(['u9']), CTX); // submitter_id = u1 (CTX.userId)
366+
const rows = await svc.listRequests({ status: 'pending' }, SYS);
367+
expect(rows[0].record_title).toBe('Acme Renewal');
368+
expect(rows[0].submitter_name).toBe('Ada Lovelace');
369+
});
370+
371+
it('enrichment falls back to the payload snapshot when the record is gone', async () => {
372+
await svc.openNodeRequest(
373+
openInput(['u9'], { record: { id: 'opp1', name: 'Snapshot Title', amount: 1 } }), CTX,
374+
);
375+
const rows = await svc.listRequests({ status: 'pending' }, SYS);
376+
expect(rows[0].record_title).toBe('Snapshot Title');
377+
});
378+
379+
it('enrichment resolves an email submitter via sys_user.email', async () => {
380+
engine._tables['sys_user'] = [{ id: 'u7', name: 'Grace Hopper', email: 'grace@example.com' }];
381+
await svc.openNodeRequest(openInput(['u9'], { submitterId: 'grace@example.com' }), CTX);
382+
const rows = await svc.listRequests({ status: 'pending' }, SYS);
383+
expect(rows[0].submitter_name).toBe('Grace Hopper');
384+
});
293385
});
294386

295387
describe('record-lock hook (node era)', () => {

0 commit comments

Comments
 (0)