diff --git a/backend/__tests__/integration/bookingCutover.test.js b/backend/__tests__/integration/bookingCutover.test.js index 60347196..ce938f23 100644 --- a/backend/__tests__/integration/bookingCutover.test.js +++ b/backend/__tests__/integration/bookingCutover.test.js @@ -76,6 +76,31 @@ describe('booking cutover — draft invoices on hold', () => { 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 () => { const contractService = require('../../src/services/contractService'); const quoteId = await acceptedQuote(); diff --git a/backend/__tests__/integration/workflowRoutes.test.js b/backend/__tests__/integration/workflowRoutes.test.js index 1ae6a831..c5126c84 100644 --- a/backend/__tests__/integration/workflowRoutes.test.js +++ b/backend/__tests__/integration/workflowRoutes.test.js @@ -71,16 +71,16 @@ describe('admin workflows API', () => { 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({ 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' }], }); expect(create.status).toBe(201); 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.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 () => { diff --git a/backend/src/routes/adminWorkflows.js b/backend/src/routes/adminWorkflows.js index 0b17babf..5823a0ad 100644 --- a/backend/src/routes/adminWorkflows.js +++ b/backend/src/routes/adminWorkflows.js @@ -24,7 +24,6 @@ const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); const { requireFeatureFlag } = require('../middleware/requireFeatureFlag'); const workflows = require('../services/workflows'); -const { DOCUMENT_ACTIONS } = require('../services/workflows/actions'); const { hasColumnCached } = require('../utils/schemaCache'); router.use(adminAuth, requireFeatureFlag('workflows')); @@ -35,9 +34,6 @@ const MAX_NODES = 200; const MAX_EDGES = 500; const MAX_NODE_CONFIG_BYTES = 16 * 1024; 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) { if (v == null) return fallback; @@ -65,13 +61,15 @@ function validateGraph(body) { return null; } -// The unimplemented (stub) actions a graph references — used to refuse enabling -// a flow that would silently no-op (e.g. the booking built-ins' prepare_*/send). +// The unimplemented actions a graph references — any action node whose +// `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 = []) { const found = new Set(); for (const n of nodes) { - const action = n && n.config && n.config.action; - if (action && UNIMPLEMENTED_ACTIONS.has(action)) found.add(action); + const action = n && n.type === 'action' && n.config && n.config.action; + if (action && !workflows.registry.getAction(action)) found.add(action); } return [...found]; } @@ -246,7 +244,7 @@ router.patch('/:id/enabled', requirePermission('workflows.manage'), async (req, // prepare_*/send_document stubs). Concern #5 from review. if (enabled) { 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) { return res.status(409).json({ error: `This flow can't be enabled yet — it uses actions that aren't implemented: ${stubs.join(', ')}.` }); } diff --git a/backend/src/services/quoteService.js b/backend/src/services/quoteService.js index 8578d316..ac1b2f2d 100644 --- a/backend/src/services/quoteService.js +++ b/backend/src/services/quoteService.js @@ -299,7 +299,10 @@ function formatNumberInTemplate(format, year, seq) { // The previous SELECT-MAX-then-INSERT path raced under concurrent // admin creates and could emit `Q-2026-AB12C3` after 5 retries. 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 seq = await claimNextSequence('quote', year, trx); return formatNumberInTemplate(format, year, seq); @@ -521,6 +524,15 @@ async function createQuote(payload, adminId) { // Resolve bank account for the chosen currency. 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) => { // SQLite's 1-connection default deadlocks when claimNextSequence // opens its own micro-transaction inside this outer one — thread @@ -574,21 +586,21 @@ async function createQuote(payload, adminId) { updated_at: new Date(), }; // 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; } // Migration 130 — snapshot the chosen output VAT code (immutable; the export // 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; } // Migration 146 — event type (event_types.slug_prefix). Drives the type of // 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; } // 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; } const inserted = await trx('quotes').insert(row).returning('id'); @@ -620,7 +632,9 @@ async function createQuote(payload, adminId) { } 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 (_) {} logger.info('Quote created', { adminId, quoteId, quoteNumber }); @@ -1610,41 +1624,46 @@ async function convertToEvent(quoteId, adminId, options = {}) { ? paymentTermSnapshot.installments : [{ percent: 100, trigger: 'after_delivery', offset_days: 0, label: 'Total' }]; - const spawnResult = await invoiceService.scheduleInvoicesForEvent({ - trx, - eventId, - quoteId: quote.id, - customer, - currency: quote.currency, - language: quote.language, - lineItems, - totals: { - net: quote.net_amount_minor, - vatRate: quote.vat_rate, - vat: quote.vat_amount_minor, - shipping: quote.shipping_amount_minor, - total: quote.total_amount_minor, - }, - installments, - eventDate: quote.event_date, - // Inline event snapshot — same rationale as convertToInvoiceOnly - // above (migration 123). - eventName: quote.event_name, - eventTimeStart: quote.event_time_start, - eventTimeEnd: quote.event_time_end, - adminId, - ccPdfEmail: quote.cc_pdf_email, - // Net 14 / 30 / 60 / 90 carry through from the quote's - // payment-term template (same as convertToInvoiceOnly). - netDays: paymentTermSnapshot?.net_days, - // Migration 140 — propagate the quote's deal_uuid down through - // every spawned invoice (same as convertToInvoiceOnly above). - dealUuid: quote.deal_uuid, - // Workflow draft-seam: the booking flow's prepare_event creates the - // event's invoices on HOLD (no scheduled_send_at) so they wait for the - // review gate + explicit send_document after the event date. - hold: options.hold === true, - }); + // `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, + eventId, + quoteId: quote.id, + customer, + currency: quote.currency, + language: quote.language, + lineItems, + totals: { + net: quote.net_amount_minor, + vatRate: quote.vat_rate, + vat: quote.vat_amount_minor, + shipping: quote.shipping_amount_minor, + total: quote.total_amount_minor, + }, + installments, + eventDate: quote.event_date, + // Inline event snapshot — same rationale as convertToInvoiceOnly + // above (migration 123). + eventName: quote.event_name, + eventTimeStart: quote.event_time_start, + eventTimeEnd: quote.event_time_end, + adminId, + ccPdfEmail: quote.cc_pdf_email, + // Net 14 / 30 / 60 / 90 carry through from the quote's + // payment-term template (same as convertToInvoiceOnly). + netDays: paymentTermSnapshot?.net_days, + // Migration 140 — propagate the quote's deal_uuid down through + // every spawned invoice (same as convertToInvoiceOnly above). + dealUuid: quote.deal_uuid, + // Workflow draft-seam: the booking flow's prepare_event creates the + // event's invoices on HOLD (no scheduled_send_at) so they wait for the + // review gate + explicit send_document after the event date. + hold: options.hold === true, + }); await trx('quotes').where({ id: quote.id }).update({ status: 'converted', diff --git a/backend/src/services/workflows/actions.js b/backend/src/services/workflows/actions.js index f4eb7e18..91412acf 100644 --- a/backend/src/services/workflows/actions.js +++ b/backend/src/services/workflows/actions.js @@ -16,16 +16,6 @@ */ 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 --- // 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 }; }); -// Prepare 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 -// wait for the review gate + send_document. The created invoice ids are stashed -// so the flow's downstream prepare_invoice ADOPTS them (instead of double- -// creating, which would also throw ALREADY_CONVERTED_TO_EVENT). Idempotent: a -// re-entry returns the existing event + invoices. -registry.registerAction('prepare_event', async (ctx) => { +// Create a DRAFT event/gallery from the accepted quote. convertToEvent creates +// the event as is_draft=true AND (unless skipInvoices) schedules its invoices — +// on HOLD here so they wait for the review gate + send_document. The created +// invoice ids are stashed so the flow's downstream prepare_invoice ADOPTS them +// (instead of double-creating, which would also throw ALREADY_CONVERTED_TO_EVENT). +// Shared by prepare_event, prepare_gallery (alias — a gallery IS an event in +// 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; - if (ctx.run.entity_type !== 'quote' || !quoteId) return { skipped: true, reason: 'prepare_event needs a quote entity' }; - if (ctx.vars?.__dryRun) return { dryRun: true, would: 'prepare_event', quoteId }; + if (ctx.run.entity_type !== 'quote' || !quoteId) return { skipped: true, reason: `${label} needs a quote entity` }; + if (ctx.vars?.__dryRun) return { dryRun: true, would: label, quoteId }; if (ctx.vars.preparedEventId) { return { already: true, eventId: ctx.vars.preparedEventId, invoiceIds: ctx.vars.preparedInvoiceIds || [] }; } 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; // The flow's prepare_invoice short-circuits on a populated preparedInvoiceIds, // so the event's held invoices flow straight through to send_document. ctx.vars.preparedInvoiceIds = res.invoiceIds || []; 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 @@ -315,13 +333,4 @@ registry.registerAction('send_document', async (ctx) => { return { skipped: true, reason: `send_document for '${doc}' not implemented yet` }; }); -// Create/prepare-document actions — registered so flows referencing them are -// 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 }; +module.exports = {};