feat(workflows): implement remaining stub actions (prepare_quote, prepare_gallery, reserve_date)

These were the last guard-stubbed actions — offered in the builder palette but
refused on enable. Now all three are real, backed by existing converters:

- prepare_gallery: alias of prepare_event (a gallery IS an event in picpeak).
- reserve_date: convertToEvent({ skipInvoices: true }) — a pure draft date hold
  with no money documents (new skipInvoices option on convertToEvent).
- prepare_quote: createQuote (customer entity) or duplicateQuote (quote entity),
  producing a status='draft' quote; idempotent via ctx.vars.preparedQuoteId.

With no stubs left, the enable-guard switches from a hardcoded DOCUMENT_ACTIONS
list to a registry lookup: an action node whose config.action has no registered
handler is unimplementable. This can't drift from what the engine can run and
also catches typo'd/future actions. (Fixes the enable-route node mapping to
carry node.type so the action-node filter matches.)

Extends the single-connection SQLite in-trx deadlock fixes to the quote-create
path (prepare_quote runs unattended): nextQuoteNumber reads getAppSetting
through trx, createQuote logs via trx and hoists its hasColumnCached schema-drift
checks before the transaction.

Adds tests for reserve_date (no invoices), prepare_quote (draft, no deadlock),
and registry coverage; retargets the enable-guard refusal test at a genuinely
unregistered action. Full backend suite: 985 passed, 1 skipped.
This commit is contained in:
Luca
2026-06-27 00:49:35 +02:00
parent 4faf5a344a
commit 9414b42b7f
5 changed files with 134 additions and 83 deletions
@@ -76,6 +76,31 @@ describe('booking cutover — draft invoices on hold', () => {
expect(q.converted_event_id).toBe(res.eventId); expect(q.converted_event_id).toBe(res.eventId);
}); });
it('reserve_date path (convertToEvent skipInvoices) creates a draft event with NO invoices', async () => {
const quoteId = await acceptedQuote();
const res = await quoteService.convertToEvent(quoteId, adminId, { hold: true, skipInvoices: true });
expect(res.eventId).toBeGreaterThanOrEqual(1);
expect(res.invoiceIds).toEqual([]);
const invCount = await db('invoices').where({ event_id: res.eventId }).count({ c: '*' }).first();
expect(Number(invCount.c)).toBe(0); // pure date hold — no money documents
});
it('prepare_quote path (duplicateQuote) creates a new DRAFT quote — no in-trx deadlock', async () => {
const quoteId = await acceptedQuote();
const newId = await quoteService.duplicateQuote(quoteId, adminId);
expect(newId).toBeGreaterThanOrEqual(1);
expect(newId).not.toBe(quoteId);
const q = await db('quotes').where({ id: newId }).first();
expect(q.status).toBe('draft');
});
it('registers prepare_gallery / reserve_date / prepare_quote as real actions', () => {
const { registry } = require('../../src/services/workflows'); // loads actions.js (side-effect registration)
for (const a of ['prepare_gallery', 'reserve_date', 'prepare_quote', 'prepare_event', 'prepare_invoice', 'send_document']) {
expect(typeof registry.getAction(a)).toBe('function');
}
});
it('prepare_contract path (createFromQuote) completes under SQLite — no in-trx deadlock', async () => { it('prepare_contract path (createFromQuote) completes under SQLite — no in-trx deadlock', async () => {
const contractService = require('../../src/services/contractService'); const contractService = require('../../src/services/contractService');
const quoteId = await acceptedQuote(); const quoteId = await acceptedQuote();
@@ -71,16 +71,16 @@ describe('admin workflows API', () => {
expect(res.body.error).toMatch(/unknown node type/i); expect(res.body.error).toMatch(/unknown node type/i);
}); });
test('refuses to enable a flow that uses an unimplemented action', async () => { test('refuses to enable a flow that uses an unregistered action', async () => {
const create = await request(app).post('/api/admin/workflows').set(auth(token)).send({ const create = await request(app).post('/api/admin/workflows').set(auth(token)).send({
name: 'Stub flow', trigger_type: 'quote.accepted', enabled: false, name: 'Stub flow', trigger_type: 'quote.accepted', enabled: false,
nodes: [{ node_key: 't', type: 'trigger' }, { node_key: 'a', type: 'action', config: { action: 'prepare_gallery' } }], nodes: [{ node_key: 't', type: 'trigger' }, { node_key: 'a', type: 'action', config: { action: 'totally_not_a_real_action' } }],
edges: [{ from_node: 't', to_node: 'a' }], edges: [{ from_node: 't', to_node: 'a' }],
}); });
expect(create.status).toBe(201); expect(create.status).toBe(201);
const res = await request(app).patch(`/api/admin/workflows/${create.body.id}/enabled`).set(auth(token)).send({ enabled: true }); const res = await request(app).patch(`/api/admin/workflows/${create.body.id}/enabled`).set(auth(token)).send({ enabled: true });
expect(res.status).toBe(409); expect(res.status).toBe(409);
expect(res.body.error).toMatch(/not.*implemented|prepare_gallery/i); expect(res.body.error).toMatch(/not.*implemented|totally_not_a_real_action/i);
}); });
test('allows enabling a flow using the now-implemented booking invoice actions', async () => { test('allows enabling a flow using the now-implemented booking invoice actions', async () => {
+7 -9
View File
@@ -24,7 +24,6 @@ const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions'); const { requirePermission } = require('../middleware/permissions');
const { requireFeatureFlag } = require('../middleware/requireFeatureFlag'); const { requireFeatureFlag } = require('../middleware/requireFeatureFlag');
const workflows = require('../services/workflows'); const workflows = require('../services/workflows');
const { DOCUMENT_ACTIONS } = require('../services/workflows/actions');
const { hasColumnCached } = require('../utils/schemaCache'); const { hasColumnCached } = require('../utils/schemaCache');
router.use(adminAuth, requireFeatureFlag('workflows')); router.use(adminAuth, requireFeatureFlag('workflows'));
@@ -35,9 +34,6 @@ const MAX_NODES = 200;
const MAX_EDGES = 500; const MAX_EDGES = 500;
const MAX_NODE_CONFIG_BYTES = 16 * 1024; const MAX_NODE_CONFIG_BYTES = 16 * 1024;
const VALID_NODE_TYPES = new Set(['trigger', 'action', 'condition', 'branch', 'loop', 'wait', 'gate', 'webhook']); const VALID_NODE_TYPES = new Set(['trigger', 'action', 'condition', 'branch', 'loop', 'wait', 'gate', 'webhook']);
// Actions registered but not yet wired (return {skipped:true}); a flow that
// uses any of these can't be meaningfully enabled.
const UNIMPLEMENTED_ACTIONS = new Set(DOCUMENT_ACTIONS);
function parseJson(v, fallback) { function parseJson(v, fallback) {
if (v == null) return fallback; if (v == null) return fallback;
@@ -65,13 +61,15 @@ function validateGraph(body) {
return null; return null;
} }
// The unimplemented (stub) actions a graph references — used to refuse enabling // The unimplemented actions a graph references — any action node whose
// a flow that would silently no-op (e.g. the booking built-ins' prepare_*/send). // `config.action` has no registered handler. Used to refuse enabling a flow
// that would silently no-op at runtime (typo'd or future-but-unwired actions).
// Registry-driven so it can't drift from what the engine can actually run.
function unimplementedActionsIn(nodes = []) { function unimplementedActionsIn(nodes = []) {
const found = new Set(); const found = new Set();
for (const n of nodes) { for (const n of nodes) {
const action = n && n.config && n.config.action; const action = n && n.type === 'action' && n.config && n.config.action;
if (action && UNIMPLEMENTED_ACTIONS.has(action)) found.add(action); if (action && !workflows.registry.getAction(action)) found.add(action);
} }
return [...found]; return [...found];
} }
@@ -246,7 +244,7 @@ router.patch('/:id/enabled', requirePermission('workflows.manage'), async (req,
// prepare_*/send_document stubs). Concern #5 from review. // prepare_*/send_document stubs). Concern #5 from review.
if (enabled) { if (enabled) {
const rows = await db('workflow_nodes').where({ workflow_id: id, version: wf.version }); const rows = await db('workflow_nodes').where({ workflow_id: id, version: wf.version });
const stubs = unimplementedActionsIn(rows.map((n) => ({ config: parseJson(n.config, {}) }))); const stubs = unimplementedActionsIn(rows.map((n) => ({ type: n.type, config: parseJson(n.config, {}) })));
if (stubs.length) { if (stubs.length) {
return res.status(409).json({ error: `This flow can't be enabled yet — it uses actions that aren't implemented: ${stubs.join(', ')}.` }); return res.status(409).json({ error: `This flow can't be enabled yet — it uses actions that aren't implemented: ${stubs.join(', ')}.` });
} }
+26 -7
View File
@@ -299,7 +299,10 @@ function formatNumberInTemplate(format, year, seq) {
// The previous SELECT-MAX-then-INSERT path raced under concurrent // The previous SELECT-MAX-then-INSERT path raced under concurrent
// admin creates and could emit `Q-2026-AB12C3` after 5 retries. // admin creates and could emit `Q-2026-AB12C3` after 5 retries.
async function nextQuoteNumber(trx) { async function nextQuoteNumber(trx) {
const format = (await getAppSetting('crm_quotes_number_format')) || 'Q-{YEAR}-{SEQ:04d}'; // Read through `trx` when present — getAppSetting on the global db inside an
// open transaction deadlocks the single-connection SQLite pool (prepare_quote
// runs createQuote unattended from a workflow).
const format = (await getAppSetting('crm_quotes_number_format', null, trx || db)) || 'Q-{YEAR}-{SEQ:04d}';
const year = new Date().getFullYear(); const year = new Date().getFullYear();
const seq = await claimNextSequence('quote', year, trx); const seq = await claimNextSequence('quote', year, trx);
return formatNumberInTemplate(format, year, seq); return formatNumberInTemplate(format, year, seq);
@@ -521,6 +524,15 @@ async function createQuote(payload, adminId) {
// Resolve bank account for the chosen currency. // Resolve bank account for the chosen currency.
const bank = await businessProfileService.resolveBankAccountForCurrency(currency, payload.businessBankAccountId); const bank = await businessProfileService.resolveBankAccountForCurrency(currency, payload.businessBankAccountId);
// Resolve schema-drift column checks BEFORE the transaction — a cold
// hasColumnCached lookup hits the global db, which deadlocks the single-
// connection SQLite pool if issued inside the trx (prepare_quote runs this
// unattended from a workflow).
const hasProjectId = await hasColumnCached('quotes', 'project_id');
const hasVatCode = await hasColumnCached('quotes', 'vat_code');
const hasEventType = await hasColumnCached('quotes', 'event_type');
const hasBookingWorkflowId = await hasColumnCached('quotes', 'booking_workflow_id');
return await db.transaction(async (trx) => { return await db.transaction(async (trx) => {
// SQLite's 1-connection default deadlocks when claimNextSequence // SQLite's 1-connection default deadlocks when claimNextSequence
// opens its own micro-transaction inside this outer one — thread // opens its own micro-transaction inside this outer one — thread
@@ -574,21 +586,21 @@ async function createQuote(payload, adminId) {
updated_at: new Date(), updated_at: new Date(),
}; };
// Migration 121 — optional link to a Project Overview project. // Migration 121 — optional link to a Project Overview project.
if (payload.projectId !== undefined && await hasColumnCached('quotes', 'project_id')) { if (payload.projectId !== undefined && hasProjectId) {
row.project_id = payload.projectId || null; row.project_id = payload.projectId || null;
} }
// Migration 130 — snapshot the chosen output VAT code (immutable; the export // Migration 130 — snapshot the chosen output VAT code (immutable; the export
// emits exactly this rather than re-deriving from the mutable rate→code map). // emits exactly this rather than re-deriving from the mutable rate→code map).
if (payload.vatCode !== undefined && await hasColumnCached('quotes', 'vat_code')) { if (payload.vatCode !== undefined && hasVatCode) {
row.vat_code = payload.vatCode ? String(payload.vatCode).slice(0, 16) : null; row.vat_code = payload.vatCode ? String(payload.vatCode).slice(0, 16) : null;
} }
// Migration 146 — event type (event_types.slug_prefix). Drives the type of // Migration 146 — event type (event_types.slug_prefix). Drives the type of
// the event the quote converts into, instead of the old hardcoded 'wedding'. // the event the quote converts into, instead of the old hardcoded 'wedding'.
if (payload.eventType !== undefined && await hasColumnCached('quotes', 'event_type')) { if (payload.eventType !== undefined && hasEventType) {
row.event_type = payload.eventType ? String(payload.eventType).slice(0, 64) : null; row.event_type = payload.eventType ? String(payload.eventType).slice(0, 64) : null;
} }
// Migration 147 — the booking workflow this quote runs on acceptance. // Migration 147 — the booking workflow this quote runs on acceptance.
if (payload.bookingWorkflowId !== undefined && await hasColumnCached('quotes', 'booking_workflow_id')) { if (payload.bookingWorkflowId !== undefined && hasBookingWorkflowId) {
row.booking_workflow_id = payload.bookingWorkflowId || null; row.booking_workflow_id = payload.bookingWorkflowId || null;
} }
const inserted = await trx('quotes').insert(row).returning('id'); const inserted = await trx('quotes').insert(row).returning('id');
@@ -620,7 +632,9 @@ async function createQuote(payload, adminId) {
} }
try { try {
await logActivity('quote_created', { quoteId, quoteNumber, customerAccountId: payload.customerAccountId }, null, `admin:${adminId}`); // Pass `trx` so the audit insert rides the transaction's connection —
// the global db here deadlocks the single-connection SQLite pool.
await logActivity('quote_created', { quoteId, quoteNumber, customerAccountId: payload.customerAccountId }, null, `admin:${adminId}`, trx);
} catch (_) {} } catch (_) {}
logger.info('Quote created', { adminId, quoteId, quoteNumber }); logger.info('Quote created', { adminId, quoteId, quoteNumber });
@@ -1610,7 +1624,12 @@ async function convertToEvent(quoteId, adminId, options = {}) {
? paymentTermSnapshot.installments ? paymentTermSnapshot.installments
: [{ percent: 100, trigger: 'after_delivery', offset_days: 0, label: 'Total' }]; : [{ percent: 100, trigger: 'after_delivery', offset_days: 0, label: 'Total' }];
const spawnResult = await invoiceService.scheduleInvoicesForEvent({ // `skipInvoices` (workflow reserve_date): create the event as a pure date
// hold — no invoices scheduled at all. The other booking actions handle
// money documents separately.
const spawnResult = options.skipInvoices === true
? { invoiceIds: [] }
: await invoiceService.scheduleInvoicesForEvent({
trx, trx,
eventId, eventId,
quoteId: quote.id, quoteId: quote.id,
+39 -30
View File
@@ -16,16 +16,6 @@
*/ */
const registry = require('./registry'); const registry = require('./registry');
// Document actions still awaiting wiring (the enable-guard refuses flows that use
// any of these). prepare_contract / prepare_event / prepare_invoice /
// send_document are now implemented below (draft-seam booking cutover), so
// they're off this list — which makes booking_full / booking_simple enableable.
const DOCUMENT_ACTIONS = [
'prepare_quote',
'prepare_gallery',
'reserve_date',
];
// --- Conditions --- // --- Conditions ---
// True once the run's invoice entity is settled (paid_at set, status paid, or // True once the run's invoice entity is settled (paid_at set, status paid, or
@@ -241,26 +231,54 @@ registry.registerAction('prepare_contract', async (ctx) => {
return { contract_prepared: res.contractId, alreadyConverted: !!res.alreadyConverted }; return { contract_prepared: res.contractId, alreadyConverted: !!res.alreadyConverted };
}); });
// Prepare a DRAFT event/gallery from the accepted quote. convertToEvent creates // Create a DRAFT event/gallery from the accepted quote. convertToEvent creates
// the event as is_draft=true AND schedules its invoices — on HOLD here so they // the event as is_draft=true AND (unless skipInvoices) schedules its invoices —
// wait for the review gate + send_document. The created invoice ids are stashed // on HOLD here so they wait for the review gate + send_document. The created
// so the flow's downstream prepare_invoice ADOPTS them (instead of double- // invoice ids are stashed so the flow's downstream prepare_invoice ADOPTS them
// creating, which would also throw ALREADY_CONVERTED_TO_EVENT). Idempotent: a // (instead of double-creating, which would also throw ALREADY_CONVERTED_TO_EVENT).
// re-entry returns the existing event + invoices. // Shared by prepare_event, prepare_gallery (alias — a gallery IS an event in
registry.registerAction('prepare_event', async (ctx) => { // picpeak), and reserve_date (skipInvoices: a pure date hold, no money docs).
async function doPrepareEvent(ctx, label, { skipInvoices = false } = {}) {
const quoteId = ctx.run.entity_id; const quoteId = ctx.run.entity_id;
if (ctx.run.entity_type !== 'quote' || !quoteId) return { skipped: true, reason: 'prepare_event needs a quote entity' }; if (ctx.run.entity_type !== 'quote' || !quoteId) return { skipped: true, reason: `${label} needs a quote entity` };
if (ctx.vars?.__dryRun) return { dryRun: true, would: 'prepare_event', quoteId }; if (ctx.vars?.__dryRun) return { dryRun: true, would: label, quoteId };
if (ctx.vars.preparedEventId) { if (ctx.vars.preparedEventId) {
return { already: true, eventId: ctx.vars.preparedEventId, invoiceIds: ctx.vars.preparedInvoiceIds || [] }; return { already: true, eventId: ctx.vars.preparedEventId, invoiceIds: ctx.vars.preparedInvoiceIds || [] };
} }
const adminId = await resolveActor(ctx); const adminId = await resolveActor(ctx);
const res = await require('../quoteService').convertToEvent(quoteId, adminId, { hold: true }); const res = await require('../quoteService').convertToEvent(quoteId, adminId, { hold: true, skipInvoices });
ctx.vars.preparedEventId = res.eventId; ctx.vars.preparedEventId = res.eventId;
// The flow's prepare_invoice short-circuits on a populated preparedInvoiceIds, // The flow's prepare_invoice short-circuits on a populated preparedInvoiceIds,
// so the event's held invoices flow straight through to send_document. // so the event's held invoices flow straight through to send_document.
ctx.vars.preparedInvoiceIds = res.invoiceIds || []; ctx.vars.preparedInvoiceIds = res.invoiceIds || [];
return { event_prepared: res.eventId, invoiceIds: ctx.vars.preparedInvoiceIds, alreadyConverted: !!res.alreadyConverted }; return { event_prepared: res.eventId, invoiceIds: ctx.vars.preparedInvoiceIds, alreadyConverted: !!res.alreadyConverted };
}
registry.registerAction('prepare_event', (ctx) => doPrepareEvent(ctx, 'prepare_event'));
// A gallery IS an event in picpeak — same draft-seam behaviour.
registry.registerAction('prepare_gallery', (ctx) => doPrepareEvent(ctx, 'prepare_gallery'));
// Reserve the date only: create the draft event as a pure calendar hold with NO
// invoices. A flow can invoice later (or never).
registry.registerAction('reserve_date', (ctx) => doPrepareEvent(ctx, 'reserve_date', { skipInvoices: true }));
// Create a DRAFT quote. From a quote entity (e.g. quote.declined → re-quote) it
// duplicates that quote; from a customer entity (customer.created) it opens a
// blank draft quote for them. Idempotent via ctx.vars.preparedQuoteId.
registry.registerAction('prepare_quote', async (ctx) => {
if (ctx.vars?.__dryRun) return { dryRun: true, would: 'prepare_quote', entity: ctx.run.entity_type };
if (ctx.vars.preparedQuoteId) return { already: true, quoteId: ctx.vars.preparedQuoteId };
const adminId = await resolveActor(ctx);
const quoteService = require('../quoteService');
let quoteId;
if (ctx.run.entity_type === 'quote' && ctx.run.entity_id) {
quoteId = await quoteService.duplicateQuote(ctx.run.entity_id, adminId);
} else if (ctx.run.entity_type === 'customer' && ctx.run.entity_id) {
quoteId = await quoteService.createQuote({ customerAccountId: ctx.run.entity_id }, adminId);
} else {
return { skipped: true, reason: 'prepare_quote needs a quote or customer entity' };
}
ctx.vars.preparedQuoteId = quoteId;
return { quote_prepared: quoteId };
}); });
// Prepare DRAFT invoice(s) from the accepted quote — created on HOLD (no // Prepare DRAFT invoice(s) from the accepted quote — created on HOLD (no
@@ -315,13 +333,4 @@ registry.registerAction('send_document', async (ctx) => {
return { skipped: true, reason: `send_document for '${doc}' not implemented yet` }; return { skipped: true, reason: `send_document for '${doc}' not implemented yet` };
}); });
// Create/prepare-document actions — registered so flows referencing them are module.exports = {};
// valid; service wiring is a follow-up. Records a skipped step (observable).
for (const key of DOCUMENT_ACTIONS) {
registry.registerAction(key, async (ctx) => {
ctx.logger?.warn?.('[workflow] document action not yet wired', { action: key, runId: ctx.run.id });
return { skipped: true, reason: `action ${key} not yet implemented` };
});
}
module.exports = { DOCUMENT_ACTIONS };