Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/24817-later-step-original-submitter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
'@objectstack/plugin-approvals': patch
---

fix(plugin-approvals): every step of a multi-step approval names the person who submitted the run, not the previous step's approver

Clause-②: no

- **What was wrong.** This corrects the unreleased later-step submitter that PR #24527 introduced (an approval decision resumes its flow as the person who decided, `.changeset/24321-resume-as-decider.md`). Because the nodes after a decision now run as the decider, an approval step that opened after another step's decision recorded that decider as its `submitter_id`. On every later step the "I Submitted" list showed the request to the previous approver, recall and resubmit admitted the previous approver and refused the real submitter, and every notice meant for the submitter (`approval.returned`, `approval.request_info`, `approval.sla_breached`, `approval.approved`, `approval.rejected`) went to the previous approver.
- **Now.** Every step of a run records the user who triggered the run as its `submitter_id`, and as the actor of the step's `submit` timeline entry, whoever decided the steps before it. That holds on both `runAs` postures, after any number of decisions by different people, and on a step that a decision node reaches by routing around another. A run whose trigger resolved no user (a schedule, a write that carried no user) opens its later steps with no submitter, as it opens its first one.
- **Unchanged.** The decider still runs the nodes after their decision: their identity, permissions and attribution, as the resume-as-decider change ships them. The first step of every run, and every single-step approval, are unchanged. Compared with the last release nothing moves: later steps named the run's submitter there too, so recall and resubmit on a later step go back to the person they reached before.
- **`ApprovalService.openNodeRequest`.** A caller that passes `submitterId: null` now opens the request with no submitter. Before, `null` fell back to the calling context's user, the same as leaving `submitterId` out, which still falls back.
33 changes: 32 additions & 1 deletion packages/plugins/plugin-approvals/src/approval-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,34 @@ function nestVariables(variables: Map<string, unknown>): Record<string, unknown>
return vars;
}

/**
* The user who TRIGGERED the run — every step's submitter, whoever resumed the
* run onto it (`ApprovalEscalationSchema.notifySubmitter`: "Notify the
* original submitter on escalation").
*
* After an approval decision the engine runs the next segment as the DECIDER
* (`ResumeSignal.resumedBy`, ruling A on `FlowSchema.runAs`), so
* `context.userId` on a later step is the previous step's decider, never the
* submitter. The engine keeps who started the run on the same context: at the
* first switch it records `triggerUserId` (`null` when the trigger resolved no
* user) and every later switch keeps it, persisted through each pause. Absent,
* no switch happened and `userId` is still the trigger user. This is the
* engine's own reading, `runTriggerUserId` in `@objectstack/service-automation`'s
* `runtime-identity.ts`, mirrored here because this plugin deliberately does not
* depend on service-automation (as {@link nestVariables} mirrors its condition
* scope). The two-step pins in `later-step-original-submitter.integration.test.ts`
* run it against the real engine.
*
* ⛔ Not `triggerUserId ?? userId`: a recorded `null` means nobody triggered
* the run, and falling through would name the decider as its submitter.
*/
function runTriggerUser(context: { userId?: string; triggerUserId?: unknown } | undefined): string | null {
if (context && Object.prototype.hasOwnProperty.call(context, 'triggerUserId')) {
return typeof context.triggerUserId === 'string' && context.triggerUserId !== '' ? context.triggerUserId : null;
}
return context?.userId ?? null;
}

/**
* Register the `approval` node executor on the automation engine, plus the
* `approval_revise` window the ADR-0044 send-back parks on. Idempotent at the
Expand Down Expand Up @@ -162,7 +190,10 @@ export function registerApprovalNode(
flowName,
flowLabel,
nodeLabel: typeof node.label === 'string' ? node.label : undefined,
submitterId: context?.userId ?? null,
// The run's trigger user, not `context.userId`: after a decision
// that is the decider, who acts in this segment (the envelope below)
// but did not submit the run.
submitterId: runTriggerUser(context),
record,
organizationId: context?.organizationId ?? context?.tenantId ?? null,
// #3447 P2: flow variables (nested, as CEL conditions see them) — the
Expand Down
12 changes: 10 additions & 2 deletions packages/plugins/plugin-approvals/src/approval-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3450,6 +3450,13 @@ export class ApprovalService implements IApprovalService {
flowLabel?: string;
/** Authored node label, snapshotted for inbox display. */
nodeLabel?: string;
/**
* Who submitted the run this step belongs to — the approval node passes
* its trigger user. `null` is an answer: the trigger resolved no user, so
* the step has no submitter. Only an OMITTED value falls back to the
* acting context's user: on a later step that context is the previous
* step's decider, who did not submit the run.
*/
submitterId?: string | null;
record?: any;
organizationId?: string | null;
Expand Down Expand Up @@ -3730,12 +3737,13 @@ export class ApprovalService implements IApprovalService {
const n = Array.isArray(prior) ? prior.length : 0;
if (n > 0) configSnapshot.__round = n + 1;
} catch { /* round display is best-effort */ }
const submitterId = input.submitterId !== undefined ? input.submitterId : (context.userId ?? null);
const row: any = {
id,
process_name: processName,
object_name: input.object,
record_id: input.recordId,
submitter_id: input.submitterId ?? context.userId ?? null,
submitter_id: submitterId,
status: 'pending',
// [#23889] The display snapshots the approval lists show: the flow's and
// the node's authored labels as the flow engine handed them over, and
Expand All @@ -3762,7 +3770,7 @@ export class ApprovalService implements IApprovalService {
await this.engine.insert('sys_approval_action', await this.actionRow(row, {
id: uid('aact'), request_id: id, organization_id: requestOrg,
step_name: input.nodeId, step_index: 0, action: 'submit',
actor_id: input.submitterId ?? context.userId ?? null, comment: null, created_at: now,
actor_id: submitterId, comment: null, created_at: now,
}), { context: SYSTEM_CTX });

// [#22824] The request's display says why it is with the administrators:
Expand Down
Loading
Loading