From 5387058937857f2afcea5c16bd24096ca853a9f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Jul 2026 16:32:00 +0000 Subject: [PATCH] feat(approvals): declare full decision/continuity action set + wire revise/resubmit routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend half of retiring the approvals inbox's hand-written buttons (objectui#2678 P2-4 follow-up). The console's generic action runtime already renders `sys_approval_request`'s declared actions anywhere the object is surfaced; this completes the declared set so the inbox no longer needs a bespoke button per capability. New declared actions on sys_approval_request (all `type:'api'`, POST, {id}- interpolated, refreshAfter): - approval_send_back → /revise (approver; status==pending) - approval_request_info → /request-info (approver; comment required) - approval_remind → /remind (submitter-gated) - approval_recall → /recall (submitter-gated; pending|returned) - approval_resubmit → /resubmit (submitter-gated; returned) Submitter-only levers gate on `record.submitter_id == ctx.user.id` (the console exposes the current user in the predicate scope); approver actions gate only on `record.status == "pending"` and defer who-can-act to the service, matching the existing approve/reject precedent. approval_reassign's `to` param becomes field-backed on `submitter_id` so the dialog resolves its `sys_user` lookup config and renders a real user picker (keyed as `to`), replacing the free-text user-id box. Also registers the missing `/revise` and `/resubmit` REST routes: the console has been POSTing to them for send-back/resubmit (ADR-0044) but rest-server never wired them, so both 404'd. They now dispatch to the existing ApprovalService.sendBack / resubmit with the standard approval error mapping. Contract test pins the action set, that every target hits a registered route verb, and the submitter-vs-approver gating. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_016ypkQikZ55oWUHUnMebwXA --- .../src/sys-approval-request.object.test.ts | 94 ++++++++++++++++++ .../src/sys-approval-request.object.ts | 97 ++++++++++++++++++- packages/rest/src/rest-server.ts | 51 ++++++++++ 3 files changed, 241 insertions(+), 1 deletion(-) create mode 100644 packages/plugins/plugin-approvals/src/sys-approval-request.object.test.ts 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 new file mode 100644 index 0000000000..4dbb5c52f4 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/sys-approval-request.object.test.ts @@ -0,0 +1,94 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Contract test for the server-declared decision actions on + * `sys_approval_request` (objectui#2678 P2-4 / retire-hardcoded-buttons). + * + * The console's generic action runtime renders + executes these wherever the + * object is surfaced (the approvals inbox included), so the inbox no longer + * hand-writes a button per capability. That only holds if the declared set + * stays faithful to the REST routes it targets and to the who-can-act gating. + * This pins both: + * + * • every `type:'api'` target resolves `{id}` and points at a route that the + * REST server actually registers (approve/reject/reassign/recall/remind/ + * request-info/revise/resubmit) — a typo'd verb would 404 silently in the UI; + * • submitter-only levers (remind/recall/resubmit) gate on + * `submitter_id == ctx.user.id` so a non-submitter never sees them. + */ + +import { describe, it, expect } from 'vitest'; +import { SysApprovalRequest } from './sys-approval-request.object.js'; + +const actions = (SysApprovalRequest as any).actions as any[]; +const byName = (n: string) => actions.find((a) => a.name === n); +/** `ObjectSchema.create` normalizes `visible` strings into a + * `{ dialect: 'cel', source }` envelope — read the source for substring asserts. */ +const vis = (n: string): string => { + const v = byName(n).visible; + return typeof v === 'string' ? v : String(v?.source ?? ''); +}; + +/** Verbs the REST server registers under `/api/v1/approvals/requests/:id/*`. */ +const ROUTE_VERBS = new Set([ + 'approve', 'reject', 'reassign', 'recall', 'remind', 'request-info', 'revise', 'resubmit', +]); + +describe('sys_approval_request declared actions', () => { + it('declares the full decision + continuity set', () => { + expect(actions.map((a) => a.name).sort()).toEqual( + [ + 'approval_approve', + 'approval_recall', + 'approval_reassign', + 'approval_reject', + 'approval_remind', + 'approval_request_info', + 'approval_resubmit', + 'approval_send_back', + ].sort(), + ); + }); + + it('every api target points at a registered approvals route verb and injects {id}', () => { + for (const a of actions) { + expect(a.type).toBe('api'); + expect(a.method).toBe('POST'); + const m = /^\/api\/v1\/approvals\/requests\/\{id\}\/([a-z-]+)$/.exec(a.target); + expect(m, `${a.name} target ${a.target}`).not.toBeNull(); + expect(ROUTE_VERBS.has(m![1]), `${a.name} → ${m![1]}`).toBe(true); + expect(a.refreshAfter).toBe(true); + } + }); + + it('gates submitter-only levers on the current user; approver actions only on pending', () => { + for (const name of ['approval_remind', 'approval_recall', 'approval_resubmit']) { + expect(vis(name)).toContain('record.submitter_id == ctx.user.id'); + } + // Approver-side actions defer who-can-act to the service; they only trim the + // non-pending case in the UI. + for (const name of ['approval_approve', 'approval_reject', 'approval_send_back', 'approval_request_info', 'approval_reassign']) { + expect(vis(name)).toContain('record.status == "pending"'); + expect(vis(name)).not.toContain('ctx.user.id'); + } + }); + + 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(); + }); + + it('reassign collects the new approver via a field-backed sys_user picker keyed as `to`', () => { + const toParam = byName('approval_reassign').params.find((p: any) => p.name === 'to'); + expect(toParam).toMatchObject({ field: 'submitter_id', name: 'to', required: true }); + }); + + it('request-info requires a comment; other params stay optional', () => { + const ri = byName('approval_request_info').params.find((p: any) => p.name === 'comment'); + expect(ri.required).toBe(true); + for (const name of ['approval_send_back', 'approval_remind', 'approval_recall', 'approval_resubmit']) { + const c = byName(name).params.find((p: any) => p.name === 'comment'); + expect(c.required ?? false).toBe(false); + } + }); +}); 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 a91c2e7e2c..4a113320ae 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts @@ -278,7 +278,12 @@ export const SysApprovalRequest = ObjectSchema.create({ method: 'POST', target: '/api/v1/approvals/requests/{id}/reassign', params: [ - { name: 'to', label: 'New approver (user id)', type: 'text', required: true }, + // Field-backed on `submitter_id` (the object's only `sys_user` lookup): + // the console resolves its lookup config (`reference_to: sys_user`) so the + // dialog renders a real user picker, while `name: 'to'` overrides the + // request-body key to the `to` the reassign route expects. This is a + // config-borrow, not a submitter pre-fill (`defaultFromRow` stays off). + { field: 'submitter_id', name: 'to', label: 'New approver', required: true, helpText: 'User to hand this step to' }, { name: 'comment', label: 'Comment', type: 'textarea', required: false }, ], visible: 'record.status == "pending"', @@ -286,5 +291,95 @@ export const SysApprovalRequest = ObjectSchema.create({ successMessage: 'Reassigned.', refreshAfter: true, }, + + // ── Approver secondary decisions ──────────────────────────────── + // Send back for revision / request more info (ADR-0044). Both are approver + // actions on a pending request; the service is the authority on who may act, + // so `visible` only trims the non-pending case (matching approve/reject). + { + name: 'approval_send_back', + label: 'Send back', + icon: 'corner-up-left', + type: 'api', + method: 'POST', + target: '/api/v1/approvals/requests/{id}/revise', + params: [ + { name: 'comment', label: 'Reason', type: 'textarea', required: false }, + ], + visible: 'record.status == "pending"', + locations: ['record_section'], + successMessage: 'Sent back for revision.', + refreshAfter: true, + }, + { + name: 'approval_request_info', + label: 'Request info', + icon: 'help-circle', + type: 'api', + method: 'POST', + target: '/api/v1/approvals/requests/{id}/request-info', + params: [ + { name: 'comment', label: 'What do you need?', type: 'textarea', required: true }, + ], + visible: 'record.status == "pending"', + locations: ['record_section'], + successMessage: 'Information requested.', + refreshAfter: true, + }, + + // ── Submitter continuity actions ──────────────────────────────── + // Remind / recall (pending) and resubmit / recall (returned). These are the + // submitter's own levers, so `visible` gates on `submitter_id == ctx.user.id` + // — the current user is exposed via the console's predicate scope. The + // service re-checks ownership; the predicate keeps a non-submitter from ever + // seeing a button they cannot use. + { + name: 'approval_remind', + label: 'Send reminder', + icon: 'bell-ring', + type: 'api', + method: 'POST', + target: '/api/v1/approvals/requests/{id}/remind', + params: [ + { name: 'comment', label: 'Note', type: 'textarea', required: false }, + ], + visible: 'record.status == "pending" && record.submitter_id == ctx.user.id', + locations: ['record_section'], + successMessage: 'Reminder sent.', + refreshAfter: true, + }, + { + name: 'approval_recall', + label: 'Recall', + icon: 'undo-2', + type: 'api', + method: 'POST', + target: '/api/v1/approvals/requests/{id}/recall', + params: [ + { name: 'comment', label: 'Comment', type: 'textarea', required: false }, + ], + // 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.submitter_id == ctx.user.id', + confirmText: 'Recall this request? Approvers can no longer act on it and the record is unlocked.', + locations: ['record_section'], + successMessage: 'Recalled.', + refreshAfter: true, + }, + { + name: 'approval_resubmit', + label: 'Resubmit', + icon: 'refresh-cw', + type: 'api', + method: 'POST', + target: '/api/v1/approvals/requests/{id}/resubmit', + params: [ + { name: 'comment', label: 'What changed?', type: 'textarea', required: false }, + ], + visible: 'record.status == "returned" && record.submitter_id == ctx.user.id', + locations: ['record_section'], + successMessage: 'Resubmitted.', + refreshAfter: true, + }, ], }); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index da74ee8e7b..c309f618b8 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -5948,6 +5948,57 @@ export class RestServer { metadata: { summary: 'Recall (withdraw) an approval request', tags: ['approvals'] }, }); + // Send back for revision / resubmit (ADR-0044). Both move the flow (the + // request finalizes `returned` and the run parks at a wait point; a + // resubmit re-enters the approval node), so — like recall — they are + // dedicated routes rather than thread interactions. The service enforces + // access (send-back = a pending approver; resubmit = the submitter) and + // returns the flow outcome (`autoRejected` / `resumed`) verbatim. + const flowMoveRoute = ( + action: 'revise' | 'resubmit', + invoke: (svc: any, id: string, body: any, context: any) => Promise, + ) => { + this.routeManager.register({ + method: 'POST', + path: `${dataPath}/approvals/requests/:id/${action}`, + handler: async (req: any, res: any) => { + try { + const environmentId = isScoped ? req.params?.environmentId : undefined; + const context = await this.resolveExecCtx(environmentId, req); + if (this.enforceAuth(req, res, context)) return; + const svc = await resolveService(environmentId); + if (!svc) return respond501(res); + const body = req.body ?? {}; + try { + const out = await invoke(svc, req.params.id, body, context ?? {}); + res.json(out); + } catch (err: any) { + if (handleApprovalError(res, err)) return; + throw err; + } + } catch (error: any) { + logError(`[REST] ${action} approval error:`, error); + res.status(500).json({ code: `APPROVAL_${action.toUpperCase()}_FAILED`, error: String(error?.message ?? error).slice(0, 500) }); + } + }, + metadata: { summary: `${action} an approval request`, tags: ['approvals'] }, + }); + }; + flowMoveRoute('revise', (svc, id, body, context) => { + if (typeof svc.sendBack !== 'function') throw new Error('VALIDATION_FAILED: revise is not supported'); + return svc.sendBack(id, { + actorId: body.actorId ?? body.actor_id ?? context?.userId, + comment: body.comment, + }, context); + }); + flowMoveRoute('resubmit', (svc, id, body, context) => { + if (typeof svc.resubmit !== 'function') throw new Error('VALIDATION_FAILED: resubmit is not supported'); + return svc.resubmit(id, { + actorId: body.actorId ?? body.actor_id ?? context?.userId, + comment: body.comment, + }, context); + }); + // Thread interactions — reassign / remind / request-info / comment. // None of these move the flow; they update approver slots or the // audit thread. Registered generically: the service method enforces