From 0814d61db06d493f89ef4779b880424c65859f48 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 08:37:42 +0000 Subject: [PATCH] fix(line-items): F/P expression tags, null guard, shared price-fill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #514 (items 8, 3 and 15 — scoped to the two `*_line_item` objects). - `quote_line_item.total_price` was formula-on-formula (`record.subtotal * …`), depending on the platform hydrating another computed field first. Recomposed from the same stored fields `subtotal` reads; arithmetic unchanged. - `opportunity_line_item.unit_price_positive` compared `unit_price < 0` with no null guard. Strict CEL aborts on `null < 0`, so the rule was inert on a blank price — it now carries the `!= null &&` guard its quote-side twin has. - Formula fields were authored with `P` and `campaign_member`'s validation with a raw `{ dialect: 'cel', source }` object (the file never imported `P`). `F`/`P`/`cel` are the same tagged template, so this only ever misled readers. - The two price-fill hooks were near-verbatim copies differing in comments. Both now build from `_line-item-price-fill.ts`; the handler body closes over nothing but `ctx`, so it still lowers to a body-only sandbox callable and the two lowered bodies are byte-identical. Guarded by a new `test/line-item-conventions.test.ts` (its own file, not more surface on the high-churn `metadata-references.test.ts`). All six guards fail on main before these changes. `pnpm verify` passes. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RomdmGNvi3GfNfBW53HJQb --- .../line-item-expressions-and-price-fill.md | 46 ++++ src/objects/_line-item-price-fill.ts | 59 ++++ src/objects/campaign_member.object.ts | 3 +- src/objects/opportunity_line_item.hook.ts | 42 +-- src/objects/opportunity_line_item.object.ts | 12 +- src/objects/quote_line_item.hook.ts | 33 +-- src/objects/quote_line_item.object.ts | 11 +- test/line-item-conventions.test.ts | 256 ++++++++++++++++++ 8 files changed, 394 insertions(+), 68 deletions(-) create mode 100644 .changeset/line-item-expressions-and-price-fill.md create mode 100644 src/objects/_line-item-price-fill.ts create mode 100644 test/line-item-conventions.test.ts diff --git a/.changeset/line-item-expressions-and-price-fill.md b/.changeset/line-item-expressions-and-price-fill.md new file mode 100644 index 00000000..0e18b382 --- /dev/null +++ b/.changeset/line-item-expressions-and-price-fill.md @@ -0,0 +1,46 @@ +--- +'hotcrm': patch +--- + +Fix the line-item expression authoring and de-duplicate the price-fill hook +(#514 items 8, 3 and 15 — the two `*_line_item` objects only). + +`crm_quote_line_item.total_price` was `record.subtotal * (1 + tax_rate/100)`, +and `subtotal` is itself a FORMULA — so the total depended on the platform +hydrating another computed field first, the hazard written up at +`lead.object.ts:61-64`. It is now composed from the same stored fields +`subtotal` reads, with the tax multiplier applied on top. The arithmetic is +unchanged (4 × 100 at 10% line discount and 8% tax is still 388.80); what +changes is that it no longer depends on evaluation order. + +`crm_opportunity_line_item`'s `unit_price_positive` compared `record.unit_price +< 0` with no null guard. Strict CEL ABORTS on `null < 0` instead of evaluating +it false, so the rule was inert on a blank price — it never fired at all. It now +carries the same `!= null &&` guard its quote-side twin has always had. The +guard only narrows the predicate, so no previously-accepted record starts +failing; a blank price is still caught by the field's own `required`. + +Both line-item objects and `crm_campaign_member` also had their expressions +authored with the wrong tag: formula fields used `P` (the predicate alias) and +`campaign_member`'s validation used a raw `{ dialect: 'cel', source }` object +because the file never imported `P`. `F`, `P` and `cel` are all aliases of the +same tagged template, so this was invisible at runtime and only ever misled +readers. Formulas now use `F`, conditions use `P`. + +Finally, `opportunity_line_item.hook.ts` and `quote_line_item.hook.ts` carried +near-verbatim copies of the same price-fill handler, differing only in comments +— the shape that lets a fix land on one object and silently skip the other. Both +now build their hook from `_line-item-price-fill.ts`. The sharing happens at +authoring time only: the handler body closes over nothing but its own `ctx`, so +it still lowers to a body-only sandbox callable (the two lowered bodies in +`dist/objectstack.json` are byte-identical). The rollup hooks next door look +alike but compute genuinely different totals, so they stay separate. + +Guarded by a new `test/line-item-conventions.test.ts` — deliberately its own +file rather than more surface on the high-churn `metadata-references.test.ts`. +It pins all four: formula fields must use `F` and conditions `P` across every +`*.object.ts` (a source-text check, because the tags are runtime-identical), no +line-item formula may read another formula field, both `unit_price_positive` +rules must be null-guarded, and the two price-fill handlers must remain one +implementation — asserted both on handler source and on a shared behavioural +scenario table run against each object. diff --git a/src/objects/_line-item-price-fill.ts b/src/objects/_line-item-price-fill.ts new file mode 100644 index 00000000..86e968d0 --- /dev/null +++ b/src/objects/_line-item-price-fill.ts @@ -0,0 +1,59 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import type { Hook, HookContext } from '@objectstack/spec/data'; +import type { HookApi } from './_hook-api'; + +/** + * Shared line-item price fill. + * + * `list_price` on both line-item objects is documented as "auto-populated from + * product.list_price", and nothing implemented it until this hook. On write, + * when a product is part of THIS change, it stamps the catalog `list_price`; + * on INSERT it also defaults the negotiated `unit_price` to the list price when + * the rep left it blank (they can still override). On update the negotiated + * `unit_price` is left alone, so a re-synced catalog price never clobbers a + * price someone actually negotiated. + * + * Why it lives here rather than twice: `crm_opportunity_line_item` and + * `crm_quote_line_item` carried near-verbatim copies of this handler that + * differed only in comments — the shape that lets a fix land on one object and + * silently skip the other. Only the price fill is shared; the two ROLLUP hooks + * next door look alike but compute genuinely different totals, so they stay + * separate. + * + * Sandbox note: L2 hook bodies run BODY-ONLY in the QuickJS sandbox, so a + * handler cannot reach module scope at runtime. This factory is still safe + * because the sharing happens at authoring time — `objectName`/`hookName` are + * plain metadata on the returned Hook, and the handler body below closes over + * NOTHING but its own `ctx`. Never move a value the body reads into this + * factory's parameters; it would resolve here and be undefined in the sandbox. + */ +export function createLineItemPriceFill(objectName: string, hookName: string): Hook { + return { + name: hookName, + object: objectName, + events: ['beforeInsert', 'beforeUpdate'], + priority: 100, + description: 'Default list_price / unit_price from the chosen product.', + handler: async (ctx: HookContext) => { + const { event, input } = ctx; + const api = ctx.api as HookApi | undefined; + if (!api) return; + // Only act when a product reference is part of THIS write. + const productId = typeof input.crm_product === 'string' ? input.crm_product : undefined; + if (!productId) return; + const product = await api.object('crm_product').findOne({ + filter: { id: productId }, fields: ['id', 'list_price'], + }); + const listPrice = product && typeof product.list_price === 'number' ? product.list_price : undefined; + if (listPrice === undefined) return; + // Catalog reference always tracks the product. + input.list_price = listPrice; + // Negotiated price defaults to list on create when left blank; never + // overwritten on update (respect a rep's negotiated figure). + if (event === 'beforeInsert' && input.unit_price == null) { + input.unit_price = listPrice; + } + }, + }; +} diff --git a/src/objects/campaign_member.object.ts b/src/objects/campaign_member.object.ts index 9ae5199b..fd816d93 100644 --- a/src/objects/campaign_member.object.ts +++ b/src/objects/campaign_member.object.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { P } from '@objectstack/spec'; /** * Campaign Member Object @@ -121,7 +122,7 @@ export const CampaignMember = ObjectSchema.create({ type: 'script', severity: 'error', message: 'A campaign member must reference either a Lead or a Contact', - condition: { dialect: 'cel', source: 'isBlank(record.crm_lead) && isBlank(record.crm_contact)' }, + condition: P`isBlank(record.crm_lead) && isBlank(record.crm_contact)`, }, ], }); diff --git a/src/objects/opportunity_line_item.hook.ts b/src/objects/opportunity_line_item.hook.ts index 390efa3b..3521a724 100644 --- a/src/objects/opportunity_line_item.hook.ts +++ b/src/objects/opportunity_line_item.hook.ts @@ -2,6 +2,7 @@ import type { Hook, HookContext } from '@objectstack/spec/data'; import type { HookApi } from './_hook-api'; +import { createLineItemPriceFill } from './_line-item-price-fill'; /** * Opportunity amount rollup. @@ -30,42 +31,13 @@ import type { HookApi } from './_hook-api'; * old and new parents are recomputed. */ /** - * Line-item price fill. - * - * `list_price` was documented as "Auto-populated from product.list_price" but - * nothing implemented it. On write, when a product is chosen this stamps the - * catalog `list_price`, and on INSERT defaults the negotiated `unit_price` to - * the list price when the rep left it blank (they can still override). On update - * the negotiated `unit_price` is left alone so a re-synced catalog price never - * clobbers a negotiated one. + * Line-item price fill — one implementation shared with the quote line item; + * see `_line-item-price-fill.ts` for the behaviour and the sandbox caveat. */ -const opportunityLineItemPriceFill: Hook = { - name: 'opportunity_line_item_price_fill', - object: 'crm_opportunity_line_item', - events: ['beforeInsert', 'beforeUpdate'], - priority: 100, - description: 'Default list_price / unit_price from the chosen product.', - handler: async (ctx: HookContext) => { - const { event, input } = ctx; - const api = ctx.api as HookApi | undefined; - if (!api) return; - // Only act when a product reference is part of THIS write. - const productId = typeof input.crm_product === 'string' ? input.crm_product : undefined; - if (!productId) return; - const product = await api.object('crm_product').findOne({ - filter: { id: productId }, fields: ['id', 'list_price'], - }); - const listPrice = product && typeof product.list_price === 'number' ? product.list_price : undefined; - if (listPrice === undefined) return; - // Catalog reference always tracks the product. - input.list_price = listPrice; - // Negotiated price defaults to list on create when left blank; never - // overwritten on update (respect a rep's negotiated figure). - if (event === 'beforeInsert' && input.unit_price == null) { - input.unit_price = listPrice; - } - }, -}; +const opportunityLineItemPriceFill: Hook = createLineItemPriceFill( + 'crm_opportunity_line_item', + 'opportunity_line_item_price_fill', +); const opportunityAmountRollup: Hook = { name: 'opportunity_amount_rollup', diff --git a/src/objects/opportunity_line_item.object.ts b/src/objects/opportunity_line_item.object.ts index 79fe55af..0a8c20b3 100644 --- a/src/objects/opportunity_line_item.object.ts +++ b/src/objects/opportunity_line_item.object.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { ObjectSchema, Field } from '@objectstack/spec/data'; -import { P } from '@objectstack/spec'; +import { F, P } from '@objectstack/spec'; /** * Opportunity Line Item @@ -88,7 +88,7 @@ export const OpportunityLineItem = ObjectSchema.create({ total_price: Field.formula({ label: 'Total', - expression: P`record.quantity * record.unit_price * (1 - record.discount / 100)`, + expression: F`record.quantity * record.unit_price * (1 - record.discount / 100)`, }), line_number: Field.number({ @@ -104,7 +104,13 @@ export const OpportunityLineItem = ObjectSchema.create({ type: 'script', severity: 'error', message: 'Sales price cannot be negative', - condition: P`record.unit_price < 0`, + // The `!= null` guard is load-bearing, not defensive noise: strict CEL + // ABORTS on `null < 0` instead of evaluating it false, so the unguarded + // form left this rule inert on a blank price — it never fired at all, + // which is the opposite of what a validation is for. The hazard is + // written up at `account.object.ts:244-245`; the same-named rule in + // `quote_line_item.object.ts` has carried the guard all along. + condition: P`record.unit_price != null && record.unit_price < 0`, }, ], }); diff --git a/src/objects/quote_line_item.hook.ts b/src/objects/quote_line_item.hook.ts index cdebf254..7c8c594e 100644 --- a/src/objects/quote_line_item.hook.ts +++ b/src/objects/quote_line_item.hook.ts @@ -2,6 +2,7 @@ import type { Hook, HookContext } from '@objectstack/spec/data'; import type { HookApi } from './_hook-api'; +import { createLineItemPriceFill } from './_line-item-price-fill'; /** * Quote total rollup. @@ -33,33 +34,13 @@ import type { HookApi } from './_hook-api'; * - handles re-parenting (old + new quote). */ /** - * Line-item price fill (see opportunity_line_item.hook for the rationale). - * Stamps catalog `list_price` from the chosen product and defaults the - * negotiated `unit_price` to it on create when blank. + * Line-item price fill — one implementation shared with the opportunity line + * item; see `_line-item-price-fill.ts` for the behaviour and the sandbox caveat. */ -const quoteLineItemPriceFill: Hook = { - name: 'quote_line_item_price_fill', - object: 'crm_quote_line_item', - events: ['beforeInsert', 'beforeUpdate'], - priority: 100, - description: 'Default list_price / unit_price from the chosen product.', - handler: async (ctx: HookContext) => { - const { event, input } = ctx; - const api = ctx.api as HookApi | undefined; - if (!api) return; - const productId = typeof input.crm_product === 'string' ? input.crm_product : undefined; - if (!productId) return; - const product = await api.object('crm_product').findOne({ - filter: { id: productId }, fields: ['id', 'list_price'], - }); - const listPrice = product && typeof product.list_price === 'number' ? product.list_price : undefined; - if (listPrice === undefined) return; - input.list_price = listPrice; - if (event === 'beforeInsert' && input.unit_price == null) { - input.unit_price = listPrice; - } - }, -}; +const quoteLineItemPriceFill: Hook = createLineItemPriceFill( + 'crm_quote_line_item', + 'quote_line_item_price_fill', +); const quoteTotalRollup: Hook = { name: 'quote_total_rollup', diff --git a/src/objects/quote_line_item.object.ts b/src/objects/quote_line_item.object.ts index f5d6a55d..3f40c014 100644 --- a/src/objects/quote_line_item.object.ts +++ b/src/objects/quote_line_item.object.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { ObjectSchema, Field } from '@objectstack/spec/data'; -import { P } from '@objectstack/spec'; +import { F, P } from '@objectstack/spec'; /** * Quote Line Item @@ -83,7 +83,7 @@ export const QuoteLineItem = ObjectSchema.create({ subtotal: Field.formula({ label: 'Subtotal', - expression: P`record.quantity * record.unit_price * (1 - record.discount / 100)`, + expression: F`record.quantity * record.unit_price * (1 - record.discount / 100)`, }), tax_rate: Field.percent({ @@ -94,9 +94,14 @@ export const QuoteLineItem = ObjectSchema.create({ defaultValue: 0, }), + // Composed from the same STORED fields `subtotal` reads, not from + // `subtotal` itself — a formula reading another formula depends on the + // platform hydrating that field first, which is the hazard warned against + // at `lead.object.ts:61-64`. The arithmetic is unchanged: this is + // `subtotal`'s expression with the tax multiplier applied on top. total_price: Field.formula({ label: 'Total', - expression: P`record.subtotal * (1 + record.tax_rate / 100)`, + expression: F`record.quantity * record.unit_price * (1 - record.discount / 100) * (1 + record.tax_rate / 100)`, }), line_number: Field.number({ diff --git a/test/line-item-conventions.test.ts b/test/line-item-conventions.test.ts new file mode 100644 index 00000000..d507acf5 --- /dev/null +++ b/test/line-item-conventions.test.ts @@ -0,0 +1,256 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { readFileSync, readdirSync } from 'node:fs'; +import { join } from 'node:path'; + +import oppLineItemHooks from '../src/objects/opportunity_line_item.hook'; +import quoteLineItemHooks from '../src/objects/quote_line_item.hook'; +import { OpportunityLineItem } from '../src/objects/opportunity_line_item.object'; +import { QuoteLineItem } from '../src/objects/quote_line_item.object'; +import { hookNamed, makeCtx, makeHarness, type Rec } from './helpers/hook-harness'; +import { REPO_ROOT } from './helpers/repo-root'; + +/** + * Authoring-convention guards for the two line-item objects (#514 items 8, 3, 15). + * + * These live in their own file on purpose: `metadata-references.test.ts` is the + * repo's highest-churn test module, and none of the guards below are about + * dangling UI references. + * + * What makes these guards necessary is that the mistakes they catch are + * INVISIBLE at runtime: + * + * - `F` and `P` are both plain aliases of `cel` in `@objectstack/spec`, so a + * formula authored with the predicate tag produces a byte-identical + * Expression envelope. Only the source text records the author's intent, so + * the tag guard has to read the source. + * - A raw `{ dialect: 'cel', source: '…' }` object literal likewise compiles to + * exactly what the tagged template produces — it just skips the tag's + * escaping and reads as a different dialect of the same codebase. + * - Formula-on-formula and a missing null guard DO change behavior, so those + * two assert against the compiled metadata rather than the source text. + */ + +const OBJECTS_DIR = join(REPO_ROOT, 'src/objects'); + +type AnyRec = Record; + +const objectSources = readdirSync(OBJECTS_DIR) + .filter((f) => f.endsWith('.object.ts')) + .map((file) => ({ file, source: readFileSync(join(OBJECTS_DIR, file), 'utf8') })); + +/** Guard against the glob silently matching nothing (a vacuous test passes). */ +it('the object-source scan actually reads files', () => { + expect(objectSources.length).toBeGreaterThan(10); +}); + +// ──────────────────────────────────────── #514 item 8: expression tags ── + +describe('expression tags record the author intent', () => { + /** + * `Field.formula({ … expression: X`…` })` — X must be `F`. + * + * Matches the `expression:` that follows a `Field.formula(` opener, which is + * the only place a formula expression can be authored. + */ + it('every Field.formula expression uses the F (formula) tag', () => { + const violations: string[] = []; + for (const { file, source } of objectSources) { + const re = /Field\.formula\(\{[\s\S]*?expression:\s*([A-Za-z_$][\w$]*)?\s*[`{]/g; + for (const m of source.matchAll(re)) { + const tag = m[1]; + if (tag === 'F') continue; + const line = source.slice(0, m.index).split('\n').length; + violations.push(`${file}:${line} — formula expression tagged \`${tag ?? '(raw object)'}\``); + } + } + expect(violations, 'formula fields must be authored with F, not P/cel/a raw envelope').toEqual([]); + }); + + /** `validations[].condition` is a boolean predicate — it must be `P`. */ + it('every validation condition uses the P (predicate) tag', () => { + const violations: string[] = []; + for (const { file, source } of objectSources) { + const re = /condition:\s*([A-Za-z_$][\w$]*)?\s*[`{]/g; + for (const m of source.matchAll(re)) { + const tag = m[1]; + if (tag === 'P') continue; + const line = source.slice(0, m.index).split('\n').length; + violations.push(`${file}:${line} — condition tagged \`${tag ?? '(raw object)'}\``); + } + } + expect( + violations, + 'validation conditions must use P; a raw { dialect: "cel" } literal means the file never imported it', + ).toEqual([]); + }); +}); + +// ─────────────────────────── #514 item 8: no formula-on-formula ── + +describe('formulas resolve from stored values, not from other formulas', () => { + /** + * The hazard is documented at `lead.object.ts:61-64`: a formula that reads + * another formula field depends on the platform hydrating that field first, + * which is not guaranteed. Compose from the same SOURCE fields instead. + */ + it.each([ + ['crm_opportunity_line_item', OpportunityLineItem], + ['crm_quote_line_item', QuoteLineItem], + ])('%s has no formula that reads another formula field', (_name, schema) => { + const fields: Record = (schema as AnyRec).fields ?? {}; + const formulaNames = Object.entries(fields) + .filter(([, f]) => f?.type === 'formula') + .map(([n]) => n); + expect(formulaNames.length, 'expected formula fields to exist').toBeGreaterThan(0); + + const violations: string[] = []; + for (const name of formulaNames) { + const source: string = fields[name]?.expression?.source ?? ''; + for (const m of source.matchAll(/record\.(\w+)/g)) { + const ref = m[1]; + if (ref !== name && formulaNames.includes(ref)) { + violations.push(`${name} reads the formula field ${ref}`); + } + } + } + expect(violations).toEqual([]); + }); + + it('quote line total still applies tax on top of the discounted line amount', () => { + // Inlining `subtotal` must not change the arithmetic. 4 × 100 with a 10% + // line discount is 360; an 8% tax rate takes it to 388.80. + const source: string = (QuoteLineItem as AnyRec).fields.total_price.expression.source; + // The expression is pure arithmetic over `record`, so plain JS evaluates it + // identically to CEL — enough to prove the inlining preserved the math. + const evaluate = new Function('record', `return ${source};`) as (r: AnyRec) => number; + const total = evaluate({ quantity: 4, unit_price: 100, discount: 10, tax_rate: 8 }); + expect(total).toBeCloseTo(388.8, 10); + }); +}); + +// ─────────────────────────────── #514 item 3: null-guarded predicates ── + +describe('line-item validation predicates are null-guarded', () => { + /** + * Strict CEL ABORTS on `null < 0` rather than evaluating it false, so an + * unguarded comparison makes the whole rule inert on an empty field — the + * rule silently never fires instead of failing loudly. `quote_line_item` + * carried the guard; `opportunity_line_item` did not. + * + * Scoped to the two line-item objects: `product.object.ts` has the same + * defect and is tracked by the remainder of #514 item 3. + */ + it.each([ + ['crm_opportunity_line_item', OpportunityLineItem], + ['crm_quote_line_item', QuoteLineItem], + ])('%s unit_price_positive guards unit_price against null', (_name, schema) => { + const rule = ((schema as AnyRec).validations ?? []).find( + (v: AnyRec) => v.name === 'unit_price_positive', + ); + expect(rule, 'unit_price_positive rule missing').toBeTruthy(); + + const source: string = rule.condition?.source ?? ''; + expect(source).toContain('record.unit_price < 0'); + expect( + source, + 'unguarded comparison — strict CEL aborts on null < 0 and the rule never fires', + ).toMatch(/record\.unit_price\s*!=\s*null\s*&&/); + }); +}); + +// ───────────────────────────── #514 item 15: one price-fill implementation ── + +describe('the two line-item price-fill hooks share one implementation', () => { + const oppFill = hookNamed(oppLineItemHooks, 'opportunity_line_item_price_fill'); + const quoteFill = hookNamed(quoteLineItemHooks, 'quote_line_item_price_fill'); + + /** + * The two handlers were independent near-verbatim copies, which is how they + * drift: a fix lands on one and the other keeps the bug. Comparing the + * handler SOURCE pins that they are literally the same code — a re-forked + * copy fails here even if it still behaves the same on the cases below. + */ + it('is literally the same handler body on both objects', () => { + expect(String(quoteFill.handler)).toBe(String(oppFill.handler)); + }); + + it('is wired to the right object with the same events and priority', () => { + expect(oppFill.object).toBe('crm_opportunity_line_item'); + expect(quoteFill.object).toBe('crm_quote_line_item'); + for (const h of [oppFill, quoteFill]) { + expect(h.events).toEqual(['beforeInsert', 'beforeUpdate']); + expect(h.priority).toBe(100); + } + }); + + /** Behavioural parity, asserted on both hooks from one scenario table. */ + const scenarios: Array<{ + title: string; + event: string; + input: Rec; + products: Rec[]; + expected: Rec; + }> = [ + { + title: 'stamps list_price and defaults a blank unit_price on insert', + event: 'beforeInsert', + input: { crm_product: 'p1' }, + products: [{ id: 'p1', list_price: 250 }], + expected: { list_price: 250, unit_price: 250 }, + }, + { + title: 'keeps an explicitly entered unit_price on insert', + event: 'beforeInsert', + input: { crm_product: 'p1', unit_price: 199 }, + products: [{ id: 'p1', list_price: 250 }], + expected: { list_price: 250, unit_price: 199 }, + }, + { + title: 're-syncs list_price on update without touching the negotiated price', + event: 'beforeUpdate', + input: { crm_product: 'p1', unit_price: 199 }, + products: [{ id: 'p1', list_price: 250 }], + expected: { list_price: 250, unit_price: 199 }, + }, + { + title: 'is a no-op when the write carries no product', + event: 'beforeInsert', + input: { quantity: 2 }, + products: [{ id: 'p1', list_price: 250 }], + expected: { list_price: undefined, unit_price: undefined }, + }, + { + title: 'is a no-op when the product has no catalog price', + event: 'beforeInsert', + input: { crm_product: 'p1' }, + products: [{ id: 'p1' }], + expected: { list_price: undefined, unit_price: undefined }, + }, + ]; + + for (const s of scenarios) { + it.each([ + ['opportunity', oppFill], + ['quote', quoteFill], + ])(`%s line item ${s.title}`, async (_label, hook) => { + const h = makeHarness({ crm_product: s.products.map((p) => ({ ...p })) }); + const input: Rec = { ...s.input }; + await hook.handler(makeCtx({ event: s.event, input, user: { id: 'u1' }, api: h.api })); + for (const [field, value] of Object.entries(s.expected)) { + expect(input[field], `${field} after ${s.event}`).toBe(value); + } + }); + } + + it('is a no-op on both objects when ctx.api is absent (never blocks the write)', async () => { + for (const hook of [oppFill, quoteFill]) { + const input: Rec = { crm_product: 'p1' }; + await expect( + hook.handler(makeCtx({ event: 'beforeInsert', input, user: { id: 'u1' } })), + ).resolves.toBeUndefined(); + expect(input.list_price).toBeUndefined(); + } + }); +});