diff --git a/.changeset/2083-quote-header-recompute.md b/.changeset/2083-quote-header-recompute.md new file mode 100644 index 00000000..18e97702 --- /dev/null +++ b/.changeset/2083-quote-header-recompute.md @@ -0,0 +1,7 @@ +--- +'hotcrm': patch +--- + +Editing a quote's own **Discount %**, **Tax** or **Shipping & Handling** now updates its **Discount Amount** and **Total Price** in the same save. Before this fix the two totals stayed at their old figures until a line item was added, changed or removed. A quote accepted in that window drafted its contract at the old total. For example, a quote with a 95,000 subtotal, 7,220 tax and 2,500 shipping had its discount raised from 5% to 10% and was then accepted: its contract was drafted at 99,970 instead of 95,220. + +The totals follow one formula, whichever field changed: Discount Amount = Subtotal × Discount %, and Total Price = Subtotal − Discount Amount + Tax + Shipping & Handling. An accepted or expired quote is still frozen, and editing one is refused exactly as before. A quote with no Subtotal keeps the totals it has. diff --git a/src/revenue/objects/quote.hook.ts b/src/revenue/objects/quote.hook.ts index 3c35e0b8..604b8eea 100644 --- a/src/revenue/objects/quote.hook.ts +++ b/src/revenue/objects/quote.hook.ts @@ -13,6 +13,8 @@ import type { HookApi } from '../../sales/objects/_hook-api'; * a line to a settled quote nor change one of its lines. * - Refuses a user's acceptance while the linked opportunity is held by an * approval, because acceptance closes that deal as won (#2014). + * - Derives `discount_amount` / `total_price` on an unsettled quote's update + * that carries a pricing input — the quote header's one formula (#2083). * - On `accepted`, drafts a contract — carrying the quote's negotiated * `payment_terms` onto it, and filling what the quote cannot express from * `DRAFT_CONTRACT_DEFAULTS`, declared placeholders rather than decisions @@ -34,7 +36,7 @@ const quoteValidation: Hook = { object: ['crm_quote', 'crm_quote_line_item'], events: ['beforeInsert', 'beforeUpdate'], priority: 200, - description: 'Default expiration date; freeze accepted/expired quotes and their lines.', + description: 'Default expiration date; freeze accepted/expired quotes and their lines; derive the header totals.', handler: async (ctx: HookContext) => { // The refusal envelope. ⚠️ Mirrored from `src/sales/objects/_refusal.ts` because a lowered // body has no module scope and `extractHookBody` THROWS on an import; @@ -283,6 +285,37 @@ const quoteValidation: Hook = { } } } + + // ─── The header totals follow their inputs (#2083) ────────────────── + // + // The quote header's ONE formula, written here and nowhere else: + // discount_amount = subtotal × discount% + // total_price = subtotal − discount_amount + tax + shipping_handling + // Two writers reach it in the same write they make: a person editing + // `discount`, `tax` or `shipping_handling` — measured on 18.0.0-next.1, + // that left both figures stale until some line was written, and a quote + // accepted then drafted its contract at the stale total — and + // `quote_total_rollup`, which writes only `subtotal`. + // + // Any writer, a user or not: the rollup's write is how lines reach the + // header, and seeds replay. ⛔ Never on a quote settled before this write — + // the freeze above is not reopened, and the rollup skips one too — nor + // with no `subtotal` to derive from, nor on the predicate path, because + // it reads `previous` (D3, see `forecast.hook.ts`). + const settled = previous?.status === 'accepted' || previous?.status === 'expired'; + if (event === 'beforeUpdate' && previous && !settled && ctx.dispatch?.mode !== 'per-row' + && ['subtotal', 'discount', 'tax', 'shipping_handling'].some((k) => k in input)) { + const now = (k: string): number => { + const v = k in input ? input[k] : previous[k]; + return typeof v === 'number' ? v : 0; + }; + const subtotal = 'subtotal' in input ? input.subtotal : previous.subtotal; + if (typeof subtotal === 'number') { + const discountAmount = Math.round(subtotal * (now('discount') / 100) * 100) / 100; + input.discount_amount = discountAmount; + input.total_price = Math.round((subtotal - discountAmount + now('tax') + now('shipping_handling')) * 100) / 100; + } + } }, }; diff --git a/src/revenue/objects/quote_line_item.hook.ts b/src/revenue/objects/quote_line_item.hook.ts index 1d6b20d0..99239c23 100644 --- a/src/revenue/objects/quote_line_item.hook.ts +++ b/src/revenue/objects/quote_line_item.hook.ts @@ -7,21 +7,19 @@ import { createLineItemNumbering, createLineItemPriceFill } from './_line-item-p /** * Quote total rollup. * - * Keeps a quote's pricing (`subtotal` / `discount_amount` / `total_price`) in - * sync with the sum of its line items. Fires after a quote line item is - * inserted / updated / deleted (the console surfaces these as the quote's - * "Line Items" related list). + * Keeps a quote's `subtotal` in sync with the sum of its line items. Fires + * after a quote line item is inserted / updated / deleted (the console + * surfaces these as the quote's "Line Items" related list). * * Why this exists: `quote_generation` seeds the quote totals from the source * opportunity's amount at create time, but nothing kept them in step once a * rep edited the line items — the quote total and its itemization could drift. * - * Total model (mirrors the quote_generation flow): - * subtotal = Σ line (quantity × unit_price × (1 − line_discount/100)) - * discount_amount = subtotal × quote.discount% (quote-level) - * total_price = subtotal − discount_amount + tax + shipping_handling - * Quote-level `tax` and `shipping_handling` stay manual; only the line-derived - * figures are rolled up. + * subtotal = Σ line (quantity × unit_price × (1 − line_discount/100)) + * is all this hook writes. `discount_amount` / `total_price` are derived from + * it IN THE SAME WRITE by `quote_workflow` (`quote.hook.ts`), the header's one + * formula — which is also what keeps them in step when a person edits the + * quote's own discount, tax or shipping (#2083). * * Design decisions mirror opportunity_line_item.hook: * - recompute from raw `quantity/unit_price/discount` (not the `subtotal` @@ -55,7 +53,7 @@ const quoteTotalRollup: Hook = { priority: 300, async: true, onError: 'log', - description: 'Recompute parent quote subtotal/discount/total from its line items.', + description: 'Recompute parent quote subtotal from its line items.', handler: async (ctx: HookContext) => { const api = ctx.api as HookApi | undefined; if (!api) return; @@ -71,7 +69,7 @@ const quoteTotalRollup: Hook = { for (const quoteId of quoteIds) { const quote = await api.object('crm_quote').findOne({ where: { id: quoteId }, - fields: ['id', 'status', 'discount', 'tax', 'shipping_handling'], + fields: ['id', 'status'], }); if (!quote) continue; const status = typeof quote.status === 'string' ? quote.status : undefined; @@ -89,13 +87,7 @@ const quoteTotalRollup: Hook = { for (const l of lines) subtotal += num(l.quantity) * num(l.unit_price) * (1 - num(l.discount) / 100); subtotal = Math.round(subtotal * 100) / 100; - const discountAmount = Math.round(subtotal * (num(quote.discount) / 100) * 100) / 100; - const total = Math.round((subtotal - discountAmount + num(quote.tax) + num(quote.shipping_handling)) * 100) / 100; - - await api.object('crm_quote').update( - { id: quoteId, subtotal, discount_amount: discountAmount, total_price: total }, - { where: { id: quoteId } }, - ); + await api.object('crm_quote').update({ id: quoteId, subtotal }, { where: { id: quoteId } }); } }, }; diff --git a/test/hooks-runtime.test.ts b/test/hooks-runtime.test.ts index 7a99210f..c0922a5d 100644 --- a/test/hooks-runtime.test.ts +++ b/test/hooks-runtime.test.ts @@ -248,6 +248,42 @@ describe('quote_total_rollup', () => { await settle(); expect((await stored('crm_quote', quote.id)).total_price).toBe(111); }); + + describe("quote_workflow · the quote's own pricing edit re-derives its totals in that write (#2083)", () => { + // QTE-0005's shape, where #2083 measured the stale total: 80,000 of lines, 6,400 tax. + const priced = { subtotal: 80_000, discount: 0, discount_amount: 0, tax: 6_400, shipping_handling: 0, total_price: 86_400 }; + + it.each([ + [{ tax: 7_000 }, { discount_amount: 0, total_price: 87_000 }], + [{ discount: 10 }, { discount_amount: 8_000, total_price: 78_400 }], // 80,000 − 8,000 + 6,400 + [{ shipping_handling: 500 }, { discount_amount: 0, total_price: 86_900 }], + ])('a rep editing %o stores %o', async (edit, totals) => { + const quote = await quoteOf({ status: 'draft', ...priced }); + await repWrites('crm_quote', 'update', { id: quote.id, ...edit }); + expect(await stored('crm_quote', quote.id)).toMatchObject({ ...edit, ...totals }); + }); + + it('re-derives nothing on a settled quote: a rep is refused as before, a system write passes as written', async () => { + const quote = await quoteOf({ status: 'accepted', ...priced }); + const refused = await repWrites('crm_quote', 'update', { id: quote.id, discount: 10 }) + .then(() => null, (e: Rec) => [e.code, e.status]); + expect(refused, 'the freeze admitted a rep edit').toEqual(['RECORD_LOCKED', 409]); + await systemUpdate(verify, 'crm_quote', { id: quote.id, tax: 7_000 }); + expect(await stored('crm_quote', quote.id)).toMatchObject({ ...priced, tax: 7_000 }); + }); + + it('drafts the contract at the re-derived total when the rep accepts right after an edit', async () => { + const quote = await quoteOf({ status: 'presented', ...priced }); + await repWrites('crm_quote', 'update', { id: quote.id, discount: 10 }); + await repWrites('crm_quote', 'update', { id: quote.id, status: 'accepted' }); + const contract = await vi.waitFor(async () => { + const [row] = await verify.rows('crm_contract', { crm_opportunity: quote.crm_opportunity }); + expect(row, 'no contract drafted').toBeTruthy(); + return row!; + }, { timeout: 10_000, interval: 50 }); + expect(contract.contract_value).toBe(78_400); + }); + }); }); describe('opportunity_line_item_price_fill', () => {