Merge pull request #764 from Luca-Timo/fix/dunning-backfill-on-enable

fix(workflows): backfill existing invoices + anchor dunning grace to due date when enabled (#750)

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

backend/src/routes/adminWorkflows.js

@@ -254,6 +254,19 @@ 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). 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(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 });
+      }
+    }
     res.json({ id, enabled });
   } catch (e) { next(e); }
 });

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',

backend/src/services/workflows/engine.js

@@ -46,11 +46,18 @@ 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. 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();
 }
 
 function gateTimeout(config = {}) {
@@ -421,6 +428,51 @@ 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.
+ *
+ * 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(targetWorkflowId) {
+  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,
+        targetWorkflowId,
+        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 +597,7 @@ async function testRun(workflowId, { entityType = null, entityId = null, payload
 module.exports = {
   emitWorkflowEvent,
   isBuiltinFlowActive,
+  backfillDunningRuns,
   runDueWaits,
   emitDueEventReminders,
   recoverStaleRuns,