From da3a77dac40a892158167aec939a1458d488a951 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Tue, 7 Jul 2026 10:33:01 +0200 Subject: [PATCH] fix(workflows): scope dunning backfill to its own flow via targetWorkflowId Address review on #764: backfillDunningRuns emitted invoice.sent without a target, so enabling dunning would also enroll every historical open invoice into any custom invoice.sent flow. Pass the enabled flow's id through to emitWorkflowEvent so the backfill only touches dunning. Also note the computeWakeAt both-fields (untilVar + delay) behaviour change in its comment. --- backend/src/routes/adminWorkflows.js | 6 ++++-- backend/src/services/workflows/engine.js | 13 +++++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/backend/src/routes/adminWorkflows.js b/backend/src/routes/adminWorkflows.js index 56735ca5..c1fd94a3 100644 --- a/backend/src/routes/adminWorkflows.js +++ b/backend/src/routes/adminWorkflows.js @@ -256,10 +256,12 @@ router.patch('/:id/enabled', requirePermission('workflows.manage'), async (req, await db('workflows').where({ id }).update(patch); // Turning dunning ON enrolls existing open/unpaid invoices (anchored to // their due date) so it starts chasing current debtors, not only invoices - // sent after enabling (#750). Best-effort — never fail the toggle over it. + // sent after enabling (#750). Scoped to this flow's id so the backfill only + // enrolls dunning, not any custom invoice.sent flow. Best-effort — never + // fail the toggle over it. if (enabled && wf.builtin_key === 'invoice_dunning') { try { - const n = await require('../services/workflows').backfillDunningRuns(); + const n = await require('../services/workflows').backfillDunningRuns(id); require('../utils/logger').info('[workflow] dunning enabled — enrolled existing invoices', { enrolled: n }); } catch (e) { require('../utils/logger').warn('[workflow] dunning backfill failed', { error: e.message }); diff --git a/backend/src/services/workflows/engine.js b/backend/src/services/workflows/engine.js index 4172cdcc..d1766a1a 100644 --- a/backend/src/services/workflows/engine.js +++ b/backend/src/services/workflows/engine.js @@ -52,7 +52,10 @@ function computeWakeAt(config = {}, vars = {}) { // Anchor to a context var when given (e.g. dueDate), plus any delay offset — // so `{ untilVar: 'dueDate', delayDays: 7 }` means "due date + 7 days" // (absolute), and an already-past anchor resumes immediately. Backward - // compatible: untilVar-only → the var; delay-only → now + delay. + // compatible: untilVar-only → the var; delay-only → now + delay. Behaviour + // change for the both-fields case (untilVar + delay): previously the delay was + // ignored and only the var returned; now they add (this is the intended + // waitGrace semantics — no seeded node relied on the old both-fields path). const base = (cfg.untilVar && vars[cfg.untilVar]) ? new Date(vars[cfg.untilVar]) : new Date(); return new Date(base.getTime() + ms).toISOString(); } @@ -432,8 +435,13 @@ async function isBuiltinFlowActive(builtinKey) { * Idempotent: emitWorkflowEvent's per-(flow, entity) dedup means at most one * run per invoice, so re-enabling is safe. Paired with the due-date-anchored * grace wait, already-overdue invoices dun on their real timeline immediately. + * + * Scoped to `targetWorkflowId` (the dunning flow being enabled) so the backfill + * only enrolls invoices into dunning — never into unrelated custom `invoice.sent` + * flows an admin may have built, which would fire their actions for every + * historical invoice. */ -async function backfillDunningRuns() { +async function backfillDunningRuns(targetWorkflowId) { let enrolled = 0; try { if (!(await db.schema.hasTable('invoices'))) return 0; @@ -445,6 +453,7 @@ async function backfillDunningRuns() { const ids = await emitWorkflowEvent('invoice.sent', { entityType: 'invoice', entityId: inv.id, + targetWorkflowId, payload: { invoiceId: inv.id, invoiceNumber: inv.invoice_number,