From 2c7b35145861557021482c0e92f573236ae2676e Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Mon, 6 Jul 2026 20:38:07 +0200 Subject: [PATCH 1/2] fix(workflows): backfill existing invoices + anchor grace to due date when dunning is enabled (#750) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enabling the invoice-dunning built-in suppressed the legacy reminder ladder but only created runs for invoices sent AFTER enabling — already-sent unpaid invoices got dunned by neither. Now: - Turning dunning ON enrolls every open sent/overdue unpaid invoice via emitWorkflowEvent('invoice.sent') (engine.backfillDunningRuns(), wired into the workflow enable toggle). Idempotent via the per-(flow,entity) dedup. - The grace wait is anchored to the invoice's due date: computeWakeAt now treats { untilVar, delayDays } as "var + offset" (was var-only OR now+offset), and the built-in's waitGrace becomes { untilVar: 'dueDate', delayDays: firstDays } (seed v6 -> v7). An already-overdue invoice duns on its real timeline instead of restarting a fresh grace clock. Note: the due-date-anchored graph applies to freshly seeded built-ins; an already-admin-enabled dunning workflow still enrolls via backfill but keeps its current grace timing until re-seeded. --- .../integration/workflowEngine.test.js | 6 +-- backend/src/routes/adminWorkflows.js | 11 +++++ backend/src/services/_workflowSeedBoot.js | 7 ++- backend/src/services/workflows/engine.js | 48 ++++++++++++++++++- 4 files changed, 65 insertions(+), 7 deletions(-) diff --git a/backend/__tests__/integration/workflowEngine.test.js b/backend/__tests__/integration/workflowEngine.test.js index 84dbc2c9..5881d90e 100644 --- a/backend/__tests__/integration/workflowEngine.test.js +++ b/backend/__tests__/integration/workflowEngine.test.js @@ -239,7 +239,7 @@ describe('workflow engine', () => { expect(again.already).toBe(true); }); - test('seeds the invoice-dunning built-in as the delegation graph (v6, disabled for first beta)', async () => { + test('seeds the invoice-dunning built-in as the delegation graph (disabled for first beta)', async () => { const { seedBuiltinWorkflowsAtBoot, DUNNING_KEY } = require('../../src/services/_workflowSeedBoot'); const noopLogger = { info() {}, warn() {} }; await seedBuiltinWorkflowsAtBoot(db, noopLogger); @@ -248,7 +248,7 @@ describe('workflow engine', () => { expect(wf).toBeTruthy(); expect(!!wf.is_builtin).toBe(true); expect(!!wf.enabled).toBe(false); // first beta: ships disabled; legacy ladder runs until enabled - expect(JSON.parse(wf.trigger_config).seedVersion).toBe(6); + expect(JSON.parse(wf.trigger_config).seedVersion).toBe(7); const nodes = await db('workflow_nodes').where({ workflow_id: wf.id, version: wf.version }); expect(nodes.filter((n) => n.type === 'trigger')).toHaveLength(1); @@ -273,7 +273,7 @@ describe('workflow engine', () => { await seedBuiltinWorkflowsAtBoot(db, noopLogger); const reseeded = await db('workflows').where({ id: wf.id }).first(); expect(reseeded.version).toBe(wf.version + 1); // bumped - expect(JSON.parse(reseeded.trigger_config).seedVersion).toBe(6); + expect(JSON.parse(reseeded.trigger_config).seedVersion).toBe(7); expect(!!reseeded.enabled).toBe(false); // seed default re-applied (not admin-owned → flips enabled→disabled) const newNodes = await db('workflow_nodes').where({ workflow_id: wf.id, version: reseeded.version }); expect(newNodes.some((n) => n.type === 'gate')).toBe(false); // legacy graph replaced diff --git a/backend/src/routes/adminWorkflows.js b/backend/src/routes/adminWorkflows.js index 5823a0ad..56735ca5 100644 --- a/backend/src/routes/adminWorkflows.js +++ b/backend/src/routes/adminWorkflows.js @@ -254,6 +254,17 @@ router.patch('/:id/enabled', requirePermission('workflows.manage'), async (req, // enabled state on the next SEED_VERSION bump (review nit #1). if (await hasColumnCached('workflows', 'admin_toggled_at')) patch.admin_toggled_at = db.fn.now(); 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. + if (enabled && wf.builtin_key === 'invoice_dunning') { + try { + const n = await require('../services/workflows').backfillDunningRuns(); + 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 }); + } + } res.json({ id, enabled }); } catch (e) { next(e); } }); diff --git a/backend/src/services/_workflowSeedBoot.js b/backend/src/services/_workflowSeedBoot.js index 1f0f1d98..bd1970ed 100644 --- a/backend/src/services/_workflowSeedBoot.js +++ b/backend/src/services/_workflowSeedBoot.js @@ -31,7 +31,10 @@ function buildDunningGraph({ firstDays, gapDays, maxReminders }) { const nodes = [ { node_key: 't', type: 'trigger', config: {}, pos_x: 240, pos_y: 0 }, { node_key: 'waitDue', type: 'wait', config: { untilVar: 'dueDate' }, pos_x: 240, pos_y: 110 }, - { node_key: 'waitGrace', type: 'wait', config: { delayDays: firstDays }, pos_x: 240, pos_y: 220 }, + // Anchor the grace period to the invoice's due date (dueDate + firstDays), + // not "now + firstDays" — so an already-overdue invoice enrolled via backfill + // duns on its real timeline instead of restarting a fresh grace clock (#750). + { node_key: 'waitGrace', type: 'wait', config: { untilVar: 'dueDate', delayDays: firstDays }, pos_x: 240, pos_y: 220 }, { node_key: 'loop', type: 'loop', config: { maxIterations: maxReminders }, pos_x: 240, pos_y: 330 }, { node_key: 'checkPaid', type: 'condition', config: { condition: 'invoice_paid' }, pos_x: 240, pos_y: 440 }, { node_key: 'paymentCheck', type: 'action', config: { action: 'queue_payment_check' }, pos_x: 240, pos_y: 550 }, @@ -218,7 +221,7 @@ function buildGalleryExpiredGraph() { const BUILTINS = [ { key: DUNNING_KEY, - version: 6, + version: 7, enabled: false, name: 'Invoice dunning (built-in)', trigger_type: 'invoice.sent', diff --git a/backend/src/services/workflows/engine.js b/backend/src/services/workflows/engine.js index 7d7cb4c5..4172cdcc 100644 --- a/backend/src/services/workflows/engine.js +++ b/backend/src/services/workflows/engine.js @@ -46,11 +46,15 @@ function outEdge(edges, fromNode, handle) { function computeWakeAt(config = {}, vars = {}) { const cfg = config || {}; - if (cfg.untilVar && vars[cfg.untilVar]) return new Date(vars[cfg.untilVar]).toISOString(); const ms = (Number(cfg.delayDays || 0) * 86400000) + (Number(cfg.delayHours || 0) * 3600000) + (Number(cfg.delayMinutes || 0) * 60000); - return new Date(Date.now() + ms).toISOString(); + // 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. + const base = (cfg.untilVar && vars[cfg.untilVar]) ? new Date(vars[cfg.untilVar]) : new Date(); + return new Date(base.getTime() + ms).toISOString(); } function gateTimeout(config = {}) { @@ -421,6 +425,45 @@ async function isBuiltinFlowActive(builtinKey) { } } +/** + * Enroll every open, unpaid invoice into the dunning flow by emitting + * `invoice.sent` for it — called when the dunning built-in is turned ON so it + * starts chasing invoices that were already sent, not only new ones (#750). + * 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. + */ +async function backfillDunningRuns() { + let enrolled = 0; + try { + if (!(await db.schema.hasTable('invoices'))) return 0; + const invoices = await db('invoices') + .whereIn('status', ['sent', 'overdue']) + .whereNotNull('due_date') + .whereRaw('COALESCE(paid_amount_minor, 0) < total_amount_minor'); + for (const inv of invoices) { + const ids = await emitWorkflowEvent('invoice.sent', { + entityType: 'invoice', + entityId: inv.id, + payload: { + invoiceId: inv.id, + invoiceNumber: inv.invoice_number, + eventId: inv.event_id || null, + customerAccountId: inv.customer_account_id, + dueDate: inv.due_date, + issueDate: inv.issue_date, + totalMinor: inv.total_amount_minor, + currency: inv.currency, + }, + }); + if (ids && ids.length) enrolled += 1; + } + } catch (e) { + logger.error('[workflow] dunning backfill failed', { error: e.message }); + } + return enrolled; +} + /** * Emit `event.date_approaching` for events entering an enabled flow's lead * window. This is the trigger source for the pre-event reminder built-in, so it @@ -545,6 +588,7 @@ async function testRun(workflowId, { entityType = null, entityId = null, payload module.exports = { emitWorkflowEvent, isBuiltinFlowActive, + backfillDunningRuns, runDueWaits, emitDueEventReminders, recoverStaleRuns, 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 2/2] 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,