fix(workflows): ship built-ins disabled for first beta + enabled-based mutex + admin sentinel

Per review: the four cutover built-ins (dunning, gallery_expiring,
gallery_expired, pre_event_email) now ship enabled:false. The mutual-exclusion
guards revert to ENABLED-based (isBuiltinFlowActive, not existence) so the
legacy paths keep running until the admin enables a built-in — enabling cuts
over, disabling reverts to legacy (fixes concern #4's "disable = silent dark"
foot-gun; no automation goes dark on upgrade).

admin_toggled_at sentinel (migration 148) marks admin ownership; the boot
re-seeder applies a shipped default-flip (enabled→disabled) only to
never-touched built-ins and never overwrites an admin's enable/disable/edit
(nit #1). SEED_VERSIONs bumped so the disabled default propagates.

Nit: applyReminder unlinks the just-rendered Mahnung PDF if queueEmail throws
(no orphan file).
This commit is contained in:
Luca
2026-06-23 23:35:56 +02:00
parent 98ab717043
commit 5893ecb27a
5 changed files with 104 additions and 67 deletions
@@ -0,0 +1,24 @@
/**
* Migration 148: mark when an admin has taken ownership of a (built-in) workflow.
*
* The boot seeder re-seeds a built-in on a SEED_VERSION bump and applies the new
* default `enabled` state. Without a sentinel that would re-flip a flow the
* admin had deliberately enabled/disabled. `admin_toggled_at` is stamped on any
* admin enable/disable or edit; the seeder then leaves that flow alone. Nullable
* → existing rows are treated as never-touched (seed defaults apply once).
*/
exports.up = async function (knex) {
if (!(await knex.schema.hasTable('workflows'))) return;
if (!(await knex.schema.hasColumn('workflows', 'admin_toggled_at'))) {
await knex.schema.alterTable('workflows', (t) => {
t.timestamp('admin_toggled_at');
});
}
};
exports.down = async function (knex) {
if (!(await knex.schema.hasTable('workflows'))) return;
if (await knex.schema.hasColumn('workflows', 'admin_toggled_at')) {
await knex.schema.alterTable('workflows', (t) => t.dropColumn('admin_toggled_at'));
}
};
+38 -35
View File
@@ -206,16 +206,20 @@ function buildGalleryExpiredGraph() {
// Built-in registry. `version` is the SEED_VERSION — bump when a graph changes // Built-in registry. `version` is the SEED_VERSION — bump when a graph changes
// (or to re-assert the default `enabled` state) so a never-admin-touched copy is // (or to re-assert the default `enabled` state) so a never-admin-touched copy is
// re-seeded on boot. `enabled` is the cutover default: the live automations // re-seeded on boot. `enabled` is the seed default.
// (dunning, gallery expiry, pre-event) ship ENABLED and their legacy hardcoded //
// paths stand down (isBuiltinFlowActive guards), so behaviour is preserved with // FIRST-BETA POSTURE (review feedback): all built-ins ship DISABLED. The legacy
// zero double-send. Illustrative/stub flows (booking) ship disabled. // hardcoded paths keep running by default (the mutual-exclusion guards are
// invoice_dunning v5 = enabled-by-default cutover (was v4: collections handoff). // enabled-based, so they only stand down once the admin ENABLES the matching
// built-in — a deliberate, per-install cutover). Enabling reverts to legacy.
// Once the prefetch-safe approval interstitial has soaked in beta, flip the
// three notification built-ins back to enabled-by-default in a follow-up.
// invoice_dunning v6 = ship disabled (was v5 enabled-by-default).
const BUILTINS = [ const BUILTINS = [
{ {
key: DUNNING_KEY, key: DUNNING_KEY,
version: 5, version: 6,
enabled: true, enabled: false,
name: 'Invoice dunning (built-in)', name: 'Invoice dunning (built-in)',
trigger_type: 'invoice.sent', trigger_type: 'invoice.sent',
trigger_config: {}, trigger_config: {},
@@ -223,9 +227,9 @@ const BUILTINS = [
'Drives overdue dunning through the engine: wait to the due date, then up to ' 'Drives overdue dunning through the engine: wait to the due date, then up to '
+ 'three payment-check cycles. Each cycle fires the existing admin confirm-payment ' + 'three payment-check cycles. Each cycle fires the existing admin confirm-payment '
+ 'email (the gate), which applies reminders + Mahngebühr via the proven payment-check ' + 'email (the gate), which applies reminders + Mahngebühr via the proven payment-check '
+ 'flow; after the cycles exhaust it hands the case to collections. ENABLED by default; ' + 'flow; after the cycles exhaust it hands the case to collections. DISABLED by default '
+ 'while it is enabled the hardcoded reminder ladder is skipped automatically, so the two ' + 'the hardcoded reminder ladder keeps running until you enable this; enabling cuts over to '
+ 'never double-send. Reminder timing is now edited here (no longer in Settings → CRM).', + 'the engine (the ladder then stands down so the two never double-send), disabling reverts.',
build: async () => { build: async () => {
const firstDays = Number(await getAppSetting('crm_invoices_reminder_first_days')) || 14; const firstDays = Number(await getAppSetting('crm_invoices_reminder_first_days')) || 14;
const secondDays = Number(await getAppSetting('crm_invoices_reminder_second_days')) || 30; const secondDays = Number(await getAppSetting('crm_invoices_reminder_second_days')) || 30;
@@ -235,50 +239,49 @@ const BUILTINS = [
}, },
{ {
key: 'gallery_expiring', key: 'gallery_expiring',
version: 1, version: 2,
enabled: true, enabled: false,
name: 'Gallery expiring (built-in)', name: 'Gallery expiring (built-in)',
trigger_type: 'gallery.expiring', trigger_type: 'gallery.expiring',
trigger_config: {}, trigger_config: {},
description: description:
'When a gallery is approaching its expiry date, email the customer the expiration warning. ' 'When a gallery is approaching its expiry date, email the customer the expiration warning. '
+ 'ENABLED by default; it delegates to the same email the hourly expiration checker used to ' + 'DISABLED by default; the hourly expiration checker keeps sending the warning until you '
+ 'send, and that legacy email stands down while this flow is on (no double-send). Edit or ' + 'enable this, which delegates to the identical email and stands the legacy send down. '
+ 'extend it here (e.g. add a final-download nudge).', + 'Edit or extend it here (e.g. add a final-download nudge).',
build: async () => buildGalleryExpiringGraph(), build: async () => buildGalleryExpiringGraph(),
}, },
{ {
key: 'gallery_expired', key: 'gallery_expired',
version: 1, version: 2,
enabled: true, enabled: false,
name: 'Gallery expired (built-in)', name: 'Gallery expired (built-in)',
trigger_type: 'gallery.expired', trigger_type: 'gallery.expired',
trigger_config: {}, trigger_config: {},
description: description:
'When a gallery passes its expiry, email the customer (and admin) that it has expired. ' 'When a gallery passes its expiry, email the customer (and admin) that it has expired. '
+ 'ENABLED by default; delegates to the same email the expiration checker used to send, ' + 'DISABLED by default; the expiration checker keeps sending it until you enable this, which '
+ 'and that legacy email stands down while this flow is on. The gallery is still archived ' + 'delegates to the identical email and stands the legacy send down. The gallery is still '
+ 'automatically regardless of this flow.', + 'archived automatically regardless of this flow.',
build: async () => buildGalleryExpiredGraph(), build: async () => buildGalleryExpiredGraph(),
}, },
{ {
key: 'pre_event_email', key: 'pre_event_email',
version: 2, version: 3,
enabled: true, enabled: false,
name: 'Pre-event reminder (built-in)', name: 'Pre-event reminder (built-in)',
trigger_type: 'event.date_approaching', trigger_type: 'event.date_approaching',
// daysBefore seeds the scheduler emitter from the current global setting so // daysBefore seeds the scheduler emitter from the current global setting so
// upgrades preserve timing; per-event offset overrides still win. This flow // enabling preserves timing; per-event offset overrides still win.
// is now the source of truth for the lead time (was Settings → Reminder emails).
trigger_config: async () => { trigger_config: async () => {
const d = Number(await getAppSetting('crm_event_reminders_days_before')); const d = Number(await getAppSetting('crm_event_reminders_days_before'));
return { daysBefore: Number.isFinite(d) && d >= 0 ? d : 2 }; return { daysBefore: Number.isFinite(d) && d >= 0 ? d : 2 };
}, },
description: description:
'A few days before the event date, send the customer the pre-event reminder. ENABLED by ' 'A few days before the event date, send the customer the pre-event reminder. DISABLED by '
+ 'default; the notify_pre_event action delegates to the proven reminder logic (per-type ' + 'default; the legacy reminder pass keeps running until you enable this, which delegates to '
+ 'template, per-event override, send-once), and the legacy reminder pass stands down while ' + 'the proven reminder logic (per-type template, per-event override, send-once) and stands '
+ 'this flow is on. Lead time = daysBefore in the trigger config (seeded from your old ' + 'the legacy pass down. Lead time = daysBefore in the trigger config (seeded from your old '
+ 'global setting); per-event overrides on the event page still apply.', + 'global setting); per-event overrides on the event page still apply.',
build: async () => buildPreEventEmailGraph(), build: async () => buildPreEventEmailGraph(),
}, },
@@ -363,14 +366,14 @@ async function seedOneBuiltin(db, logger, def) {
const existing = await db('workflows').where({ builtin_key: def.key }).first(); const existing = await db('workflows').where({ builtin_key: def.key }).first();
if (existing) { if (existing) {
// Re-seed only a never-admin-activated copy whose SEED_VERSION moved on. An // Never touch a built-in the admin has taken ownership of (enabled/disabled
// already-ENABLED built-in is the admin's live (possibly customised) flow — // or edited it) — admin_toggled_at is the sentinel (migration 148). For a
// never overwrite it. The version bump carries the cutover default (incl. // never-touched copy, re-seed on a SEED_VERSION bump and (re-)apply the seed
// flipping a still-disabled flow to enabled); the cutover targets flows that // default `enabled`, so a shipped default flip (e.g. enabled→disabled for
// shipped disabled and were never touched, so this leaves admin choices alone. // first beta) propagates to installs the admin hasn't customised.
const storedVersion = Number(parseSeedConfig(existing.trigger_config).seedVersion) || 0; const storedVersion = Number(parseSeedConfig(existing.trigger_config).seedVersion) || 0;
const isEnabled = existing.enabled === true || existing.enabled === 1; const adminOwned = !!existing.admin_toggled_at;
if (isEnabled || storedVersion >= def.version) return; if (adminOwned || storedVersion >= def.version) return;
const newVersion = (existing.version || 1) + 1; const newVersion = (existing.version || 1) + 1;
await db.transaction(async (trx) => { await db.transaction(async (trx) => {
+6 -6
View File
@@ -128,13 +128,13 @@ async function runEventReminderPass() {
return { scanned: 0, sent: 0, skipped: 0, disabled: true }; return { scanned: 0, sent: 0, skipped: 0, disabled: true };
} }
// Mutual exclusion with the workflow engine: once the pre_event_email built-in // Mutual exclusion with the workflow engine: the legacy pass stands down only
// is seeded (flag on), the engine OWNS the reminder — it sends via the // when the pre_event_email built-in is ENABLED (then the engine sends via the
// notify_pre_event action when the flow is enabled, or nothing when the admin // notify_pre_event action). If the flow is disabled, this legacy pass keeps
// disabled it. Either way the legacy pass stands down so the two never // running — so the built-ins can ship disabled without going dark, and
// double-send. Fails closed → legacy pass keeps running if the subsystem is down. // disabling a built-in cleanly reverts to the legacy path. Fails closed.
try { try {
if (await require('./workflows').isBuiltinFlowPresent('pre_event_email')) { if (await require('./workflows').isBuiltinFlowActive('pre_event_email')) {
return { scanned: 0, sent: 0, skipped: 0, byWorkflow: true }; return { scanned: 0, sent: 0, skipped: 0, byWorkflow: true };
} }
} catch (_) { /* workflow subsystem down → keep the legacy pass running */ } } catch (_) { /* workflow subsystem down → keep the legacy pass running */ }
+8 -6
View File
@@ -26,12 +26,14 @@ async function checkExpirations() {
// but skip the LEGACY email so the two never double-send. State transitions // but skip the LEGACY email so the two never double-send. State transitions
// (is_active=false, archive) always run regardless — they're the expiry // (is_active=false, archive) always run regardless — they're the expiry
// mechanic, not the notification. // mechanic, not the notification.
// Existence-based: once the built-in is seeded the engine OWNS the email, so // Enabled-based mutual exclusion: the legacy email stands down only when the
// the legacy send stands down whether the flow is enabled (it sends) or // matching built-in is ENABLED (then its action sends the identical mail). A
// disabled (admin turned it off). The trigger is still emitted regardless. // disabled built-in leaves the legacy send running — so the flows can ship
const { isBuiltinFlowPresent } = require('./workflows'); // disabled without galleries going un-notified, and disabling a flow reverts
const warningFlowOwns = await isBuiltinFlowPresent('gallery_expiring'); // to legacy. The trigger is still emitted regardless (for any custom flows).
const expiredFlowOwns = await isBuiltinFlowPresent('gallery_expired'); const { isBuiltinFlowActive } = require('./workflows');
const warningFlowOwns = await isBuiltinFlowActive('gallery_expiring');
const expiredFlowOwns = await isBuiltinFlowActive('gallery_expired');
// Check for events needing warning emails // Check for events needing warning emails
// Skip events with null expires_at (they never expire) // Skip events with null expires_at (they never expire)
+28 -20
View File
@@ -2765,20 +2765,27 @@ async function applyReminder(invoice, lineItems, level, adminId) {
attachments.push({ filename: `${fresh.invoice_number}_Mahnung.pdf`, contentPath: mahnungPath, contentType: 'application/pdf' }); attachments.push({ filename: `${fresh.invoice_number}_Mahnung.pdf`, contentPath: mahnungPath, contentType: 'application/pdf' });
const { to: reminderTo, cc: reminderCc } = resolveBillingRecipients(customer, invoice.cc_pdf_email); const { to: reminderTo, cc: reminderCc } = resolveBillingRecipients(customer, invoice.cc_pdf_email);
await emailProcessor.queueEmail(invoice.event_id || null, reminderTo, templateKey, { try {
invoice_number: invoice.invoice_number, await emailProcessor.queueEmail(invoice.event_id || null, reminderTo, templateKey, {
customer_name: customer.display_name || customer.first_name || customer.email.split('@')[0], invoice_number: invoice.invoice_number,
total_amount: formatMajor(invoice.total_amount_minor, invoice.currency, locale), customer_name: customer.display_name || customer.first_name || customer.email.split('@')[0],
new_total_amount: formatMajor(newTotal, invoice.currency, locale), total_amount: formatMajor(invoice.total_amount_minor, invoice.currency, locale),
outstanding_amount: formatMajor(outstandingMinor, invoice.currency, locale), new_total_amount: formatMajor(newTotal, invoice.currency, locale),
paid_amount: formatMajor(invoice.paid_amount_minor, invoice.currency, locale), outstanding_amount: formatMajor(outstandingMinor, invoice.currency, locale),
late_fee_amount: formatMajor(lateFeeGross, invoice.currency, locale), paid_amount: formatMajor(invoice.paid_amount_minor, invoice.currency, locale),
due_date: formatShortDate(invoice.due_date), late_fee_amount: formatMajor(lateFeeGross, invoice.currency, locale),
days_overdue: daysOverdue, due_date: formatShortDate(invoice.due_date),
cc: reminderCc, days_overdue: daysOverdue,
attachments, cc: reminderCc,
// Dunning reminders are relationship mail — hold to business hours. attachments,
}, { respectBusinessHours: true }); // Dunning reminders are relationship mail — hold to business hours.
}, { respectBusinessHours: true });
} catch (err) {
// Don't leave the just-rendered Mahnung PDF orphaned on disk if queueing the
// email failed — it would only be reachable via the next reminder anyway.
try { fs.unlinkSync(mahnungPath); } catch (_) { /* best-effort cleanup */ }
throw err;
}
try { try {
await logActivity('invoice_reminder_sent', { invoiceId: invoice.id, level, lateFeeMinor: lateFeeGross }, await logActivity('invoice_reminder_sent', { invoiceId: invoice.id, level, lateFeeMinor: lateFeeGross },
@@ -3447,14 +3454,15 @@ async function runScheduledTasks() {
// Throttled to one email per 24h per invoice via // Throttled to one email per 24h per invoice via
// invoices.last_payment_check_at. // invoices.last_payment_check_at.
const remindersEnabled = await getAppSetting('crm_invoices_reminders_enabled'); const remindersEnabled = await getAppSetting('crm_invoices_reminders_enabled');
// Mutual exclusion with the workflow engine: once the invoice_dunning built-in // Mutual exclusion with the workflow engine: the hardcoded ladder stands down
// is seeded (flag on), the engine OWNS dunning and is the single switch — it // only when the invoice_dunning built-in is ENABLED (then the engine fires the
// fires the payment-check emails when the flow is enabled, or nothing when the // payment-check emails). A disabled built-in leaves this ladder running — so
// admin disabled it. Either way the hardcoded ladder stands down so the two // the flow can ship disabled without dunning going dark, and disabling the
// never double-send. Fails closed → ladder stays on if the subsystem is down. // flow reverts to the ladder. Fails closed → ladder stays on if the subsystem
// is down.
let engineDrivesDunning = false; let engineDrivesDunning = false;
try { try {
engineDrivesDunning = await require('./workflows').isBuiltinFlowPresent('invoice_dunning'); engineDrivesDunning = await require('./workflows').isBuiltinFlowActive('invoice_dunning');
} catch (_) { /* workflows tables absent / flag system down → ladder stays on */ } } catch (_) { /* workflows tables absent / flag system down → ladder stays on */ }
if (remindersEnabled !== false && !engineDrivesDunning) { if (remindersEnabled !== false && !engineDrivesDunning) {
const firstDays = ensureInt(await getAppSetting('crm_invoices_reminder_first_days')) || 14; const firstDays = ensureInt(await getAppSetting('crm_invoices_reminder_first_days')) || 14;