From 1d60fb212604fe528678763ca9be3e7aec0a356a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 13:38:23 +0000 Subject: [PATCH 1/4] =?UTF-8?q?feat(revenue):=20line=5Fnumber=20gets=20its?= =?UTF-8?q?=20writer=20=E2=80=94=20a=20per-parent=20ordinal=20on=20both=20?= =?UTF-8?q?line=20items?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X8U3asekbiC7yWoEPWR4Dg --- src/revenue/objects/_line-item-price-fill.ts | 47 +++++ .../objects/opportunity_line_item.hook.ts | 10 +- src/revenue/objects/quote_line_item.hook.ts | 10 +- test/line-item-line-number.test.ts | 191 ++++++++++++++++++ 4 files changed, 254 insertions(+), 4 deletions(-) create mode 100644 test/line-item-line-number.test.ts diff --git a/src/revenue/objects/_line-item-price-fill.ts b/src/revenue/objects/_line-item-price-fill.ts index 3d81f668c..19f210d5a 100644 --- a/src/revenue/objects/_line-item-price-fill.ts +++ b/src/revenue/objects/_line-item-price-fill.ts @@ -65,3 +65,50 @@ export function createLineItemPriceFill(objectName: string, hookName: string): H }, }; } + +/** + * Shared line-number assigner (#1828) — `line_number` is `readonly`, so this is + * its one writer. Insert: `max under the parent + 1`. Update of a row that still + * has none (created before this hook — `scripts/backfill-line-number.ts` touches + * each once): same stamp. An existing ordinal is never renumbered. The engine + * keeps a payload key a hook wrote through its readonly strip, so a user write + * lands the stamp and a user-supplied value is overwritten; only a no-user + * system insert that supplies one (the seed) keeps its own. `runAs: 'system'` + * elevates the sibling read and nothing else. Body-only rule as above: the + * parent key comes from `ctx.object`, never a factory parameter. + */ +export function createLineItemNumbering(objectName: string, hookName: string): Hook { + return { + name: hookName, + object: objectName, + events: ['beforeInsert', 'beforeUpdate'], + priority: 110, + runAs: 'system', + description: 'Assign line_number = (max under the parent) + 1.', + handler: async (ctx: HookContext) => { + const { event, input } = ctx; + const prev = ctx.previous; + const api = ctx.api as HookApi | undefined; + if (!api) return; + if (event === 'beforeUpdate') { + // D3: a predicate update has ONE payload for every row (forecast.hook.ts). + if (ctx.dispatch?.mode === 'per-row' || typeof prev?.line_number === 'number') return; + } else if (!ctx.user?.id && typeof input.line_number === 'number') { + return; + } + const parentKey = ctx.object === 'crm_quote_line_item' ? 'crm_quote' : 'crm_opportunity'; + const parentId = input[parentKey] ?? prev?.[parentKey]; + if (typeof parentId !== 'string' || !parentId) return; + const siblings = await api.object(ctx.object).find({ + where: { [parentKey]: parentId }, fields: ['line_number'], top: 5000, + }); + let max = 0; + for (const s of siblings) { + if (typeof s.line_number === 'number' && s.line_number > max) max = s.line_number; + } + // A batch insert runs every row's beforeInsert before storing any row. + const offset = event === 'beforeInsert' && ctx.dispatch?.mode === 'per-row' ? ctx.dispatch.index : 0; + input.line_number = max + 1 + offset; + }, + }; +} diff --git a/src/revenue/objects/opportunity_line_item.hook.ts b/src/revenue/objects/opportunity_line_item.hook.ts index b48248e5e..0dd40742a 100644 --- a/src/revenue/objects/opportunity_line_item.hook.ts +++ b/src/revenue/objects/opportunity_line_item.hook.ts @@ -2,7 +2,7 @@ import type { Hook, HookContext } from '@objectstack/spec/data'; import type { HookApi } from '../../sales/objects/_hook-api'; -import { createLineItemPriceFill } from './_line-item-price-fill'; +import { createLineItemNumbering, createLineItemPriceFill } from './_line-item-price-fill'; /** * Opportunity amount rollup. @@ -39,6 +39,12 @@ const opportunityLineItemPriceFill: Hook = createLineItemPriceFill( 'opportunity_line_item_price_fill', ); +/** `line_number` assigner — see `_line-item-price-fill.ts` (#1828). */ +const opportunityLineItemNumbering: Hook = createLineItemNumbering( + 'crm_opportunity_line_item', + 'opportunity_line_item_line_number', +); + const opportunityAmountRollup: Hook = { name: 'opportunity_amount_rollup', object: 'crm_opportunity_line_item', @@ -98,4 +104,4 @@ const opportunityAmountRollup: Hook = { }, }; -export default [opportunityLineItemPriceFill, opportunityAmountRollup]; +export default [opportunityLineItemPriceFill, opportunityLineItemNumbering, opportunityAmountRollup]; diff --git a/src/revenue/objects/quote_line_item.hook.ts b/src/revenue/objects/quote_line_item.hook.ts index 5b4a01db2..50950dd3e 100644 --- a/src/revenue/objects/quote_line_item.hook.ts +++ b/src/revenue/objects/quote_line_item.hook.ts @@ -2,7 +2,7 @@ import type { Hook, HookContext } from '@objectstack/spec/data'; import type { HookApi } from '../../sales/objects/_hook-api'; -import { createLineItemPriceFill } from './_line-item-price-fill'; +import { createLineItemNumbering, createLineItemPriceFill } from './_line-item-price-fill'; /** * Quote total rollup. @@ -42,6 +42,12 @@ const quoteLineItemPriceFill: Hook = createLineItemPriceFill( 'quote_line_item_price_fill', ); +/** `line_number` assigner — see `_line-item-price-fill.ts` (#1828). */ +const quoteLineItemNumbering: Hook = createLineItemNumbering( + 'crm_quote_line_item', + 'quote_line_item_line_number', +); + const quoteTotalRollup: Hook = { name: 'quote_total_rollup', object: 'crm_quote_line_item', @@ -107,4 +113,4 @@ const quoteTotalRollup: Hook = { }, }; -export default [quoteLineItemPriceFill, quoteTotalRollup]; +export default [quoteLineItemPriceFill, quoteLineItemNumbering, quoteTotalRollup]; diff --git a/test/line-item-line-number.test.ts b/test/line-item-line-number.test.ts new file mode 100644 index 000000000..be58dc22d --- /dev/null +++ b/test/line-item-line-number.test.ts @@ -0,0 +1,191 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { ObjectQL, bindHooksToEngine } from '@objectstack/objectql'; +import { InMemoryDriver } from '@objectstack/driver-memory'; +import stack from '../objectstack.config'; +import { extractSandboxBody, makeSandboxEngine, runHookBody } from './helpers/action-sandbox'; + +/** + * `line_number` HAS A WRITER (#1828 — ruling: batch #163 item 2, letter 1). + * + * Both line-item objects declare `line_number: Field.number({ readonly: true })` + * and, until this card, nothing wrote it: since objectql 17.4.0 a static + * readonly column is stripped from a NON-SYSTEM caller's INSERT, so every line a + * rep created carried a null ordinal — while `LINE_ITEM_FIELDS` + * (`src/sales/flows/_billing-endpoint.ts`) ships the key to the billing system + * on every won deal. The seed fixture's `1..n` per parent + * (`test/seed-consistency.test.ts`) was the only evidence of an ordinal, and it + * pinned the FIXTURE. This file is the same pin on the runtime. + * + * Measured on a real `ObjectQL` over the SHIPPED field definitions and the + * SHIPPED hooks, because the one fact everything rests on is an engine rule, not + * a hook rule: a payload key a `beforeInsert` / `beforeUpdate` hook WROTE is + * exempt from the readonly strip (`hookWrittenKeys` in `stripReadonlyFields`), + * while the same key supplied by a user is dropped. The control at the bottom + * shows the strip is live on this exact schema — without it "the hook's stamp + * survived" would prove nothing. + */ + +type AnyRec = Record; + +const objects: AnyRec[] = (stack as AnyRec).objects ?? []; +const hooks: AnyRec[] = (stack as AnyRec).hooks ?? []; +const byName = (n: string) => objects.find((o) => o.name === n) as AnyRec; + +/** The two line-item objects mirror each other; every case runs on both. */ +const PAIRS = [ + { object: 'crm_opportunity_line_item', parentKey: 'crm_opportunity', hook: 'opportunity_line_item_line_number' }, + { object: 'crm_quote_line_item', parentKey: 'crm_quote', hook: 'quote_line_item_line_number' }, +] as const; + +/** A rep: a real user id, and NOT system — the context the readonly strip bites on. */ +const rep = { userId: 'rep_1' }; +/** The seed loader's shape: system, no user. */ +const seed = { isSystem: true }; + +let ql: AnyRec; + +/** + * Boot on the shipped schemas. Only the numbering hooks are bound: the price + * fill and the async rollups are other hooks' behaviour, and a line here carries + * its own price so none of them is needed to make the insert legal. + */ +async function boot(withNumbering: boolean) { + ql = (await ObjectQL.create({ + datasources: { default: new InMemoryDriver({ persistence: false }) }, + objects: { + crm_opportunity_line_item: byName('crm_opportunity_line_item'), + crm_quote_line_item: byName('crm_quote_line_item'), + } as never, + })) as never; + if (withNumbering) { + bindHooksToEngine( + ql as never, + hooks.filter((h) => PAIRS.some((p) => p.hook === h.name)) as never, + ); + } +} + +afterEach(async () => { + await ql?.close(); +}); + +const line = (parentKey: string, parentId: string, extra: AnyRec = {}): AnyRec => ({ + [parentKey]: parentId, crm_product: 'prod_1', quantity: 1, unit_price: 100, ...extra, +}); + +const ordinals = async (object: string, parentKey: string, parentId: string): Promise => { + const rows: AnyRec[] = await ql.find(object, { where: { [parentKey]: parentId }, context: seed }); + return rows.map((r) => r.line_number ?? null).sort((a, b) => a - b); +}; + +describe.each(PAIRS)('$object — the assigner', ({ object, parentKey, hook }) => { + beforeEach(() => boot(true)); + + it('is shipped: registered, elevated, and bound to insert and update', () => { + const h = hooks.find((x) => x.name === hook) as AnyRec; + expect(h?.object).toBe(object); + expect(h?.events).toEqual(['beforeInsert', 'beforeUpdate']); + expect(h?.runAs).toBe('system'); + // The ruling keeps the column platform-assigned. + expect(byName(object).fields.line_number.readonly).toBe(true); + }); + + it('numbers a rep\'s inserts 1..n under each parent, independently per parent', async () => { + for (let i = 0; i < 3; i++) await ql.insert(object, line(parentKey, 'p1'), { context: rep }); + await ql.insert(object, line(parentKey, 'p2'), { context: rep }); + expect(await ordinals(object, parentKey, 'p1')).toEqual([1, 2, 3]); + expect(await ordinals(object, parentKey, 'p2')).toEqual([1]); + }); + + it('ignores an ordinal the rep supplies — it is platform-assigned', async () => { + await ql.insert(object, line(parentKey, 'p1'), { context: rep }); + const row = await ql.insert(object, line(parentKey, 'p1', { line_number: 99 }), { context: rep }); + const stored = await ql.findOne(object, { where: { id: row.id }, context: seed }); + expect(stored.line_number).toBe(2); + }); + + it('continues from the highest ordinal, so a deleted line never hands its number out twice', async () => { + const rows = []; + for (let i = 0; i < 3; i++) rows.push(await ql.insert(object, line(parentKey, 'p1'), { context: rep })); + await ql.delete(object, { where: { id: rows[1].id }, context: seed }); + await ql.insert(object, line(parentKey, 'p1'), { context: rep }); + expect(await ordinals(object, parentKey, 'p1')).toEqual([1, 3, 4]); + }); + + it('numbers a batch insert distinctly — every row\'s beforeInsert runs before any row is stored', async () => { + await ql.insert(object, [line(parentKey, 'p1'), line(parentKey, 'p1'), line(parentKey, 'p1')], { context: rep }); + expect(await ordinals(object, parentKey, 'p1')).toEqual([1, 2, 3]); + }); + + it('keeps the ordinal a no-user system insert supplies (the seed fixture)', async () => { + await ql.insert(object, line(parentKey, 'p1', { line_number: 7 }), { context: seed }); + expect(await ordinals(object, parentKey, 'p1')).toEqual([7]); + }); + + it('never renumbers an existing ordinal on update, whatever the payload says', async () => { + const row = await ql.insert(object, line(parentKey, 'p1'), { context: rep }); + await ql.update(object, { id: row.id, line_number: 42, quantity: 2 }, { where: { id: row.id }, context: rep }); + const stored = await ql.findOne(object, { where: { id: row.id }, context: seed }); + expect(stored.line_number).toBe(1); + expect(stored.quantity).toBe(2); + }); +}); + +describe.each(PAIRS)('$object — rows that predate the assigner (the back-fill path)', ({ object, parentKey }) => { + it('a touch in creation order numbers each null row after the parent\'s highest ordinal', async () => { + // Rows created before this card: no hook, a rep's insert → null ordinal. + await boot(false); + const legacy = []; + for (let i = 0; i < 3; i++) legacy.push(await ql.insert(object, line(parentKey, 'p1'), { context: rep })); + const other = await ql.insert(object, line(parentKey, 'p2'), { context: rep }); + expect(await ordinals(object, parentKey, 'p1')).toEqual([null, null, null]); + + // Same store, the hook now bound — what `scripts/backfill-line-number.ts` + // does over REST: one single-row update per null row, parent by parent, in + // creation order, as a non-system caller. + bindHooksToEngine(ql as never, hooks.filter((h) => h.object === object && /_line_number$/.test(h.name)) as never); + for (const r of [...legacy, other]) { + await ql.update(object, { id: r.id, line_number: null }, { where: { id: r.id }, context: rep }); + } + const p1: AnyRec[] = await ql.find(object, { where: { [parentKey]: 'p1' }, context: seed }); + expect(legacy.map((r) => p1.find((x) => x.id === r.id)?.line_number)).toEqual([1, 2, 3]); + expect(await ordinals(object, parentKey, 'p2')).toEqual([1]); + + // Idempotent: a second pass finds every row numbered and changes nothing. + for (const r of legacy) { + await ql.update(object, { id: r.id, line_number: null }, { where: { id: r.id }, context: rep }); + } + expect(await ordinals(object, parentKey, 'p1')).toEqual([1, 2, 3]); + }); +}); + +describe.each(PAIRS)('$object — CONTROL: the readonly strip is live on this schema', ({ object, parentKey }) => { + it('without the assigner a rep\'s supplied ordinal is dropped and the row stores none', async () => { + // The pre-#1828 state, and the reason the assigner has to be a hook: had + // this landed 99, "the hook's stamp survived" above would measure nothing. + await boot(false); + const row = await ql.insert(object, line(parentKey, 'p1', { line_number: 99 }), { context: rep }); + const stored = await ql.findOne(object, { where: { id: row.id }, context: seed }); + expect(stored.line_number ?? null).toBeNull(); + }); +}); + +describe.each(PAIRS)('$object — the assigner ships body-only', ({ object, parentKey, hook }) => { + const shipped = () => hooks.find((x) => x.name === hook) as AnyRec; + + it('lowers with no free identifiers — the parent key is read from ctx.object', () => { + expect(() => extractSandboxBody(shipped().handler, `hook '${hook}'`)).not.toThrow(); + }); + + it('stamps max + 1 in the VM', async () => { + const engine = makeSandboxEngine({ + [object]: [{ id: 'a', [parentKey]: 'p1', line_number: 1 }, { id: 'b', [parentKey]: 'p1', line_number: 4 }], + }); + const { input } = await runHookBody(shipped(), { + event: 'beforeInsert', input: { [parentKey]: 'p1' }, user: { id: 'rep_1' } as never, engine, + }); + expect(input.line_number).toBe(5); + }); +}); From 23eb7155d9ed8707fa53bb9b083fbc71140228d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 13:45:25 +0000 Subject: [PATCH 2/4] feat(revenue): one-off back-fill for line items that predate the assigner Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X8U3asekbiC7yWoEPWR4Dg --- .changeset/line-item-line-number-writer.md | 21 +++ scripts/backfill-line-number.ts | 171 +++++++++++++++++++ src/revenue/objects/_line-item-price-fill.ts | 11 +- test/line-item-line-number.test.ts | 8 +- 4 files changed, 203 insertions(+), 8 deletions(-) create mode 100644 .changeset/line-item-line-number-writer.md create mode 100644 scripts/backfill-line-number.ts diff --git a/.changeset/line-item-line-number-writer.md b/.changeset/line-item-line-number-writer.md new file mode 100644 index 000000000..d834c2900 --- /dev/null +++ b/.changeset/line-item-line-number-writer.md @@ -0,0 +1,21 @@ +--- +'hotcrm': minor +--- + +**Line items now carry their line number.** `line_number` on opportunity and quote line +items is read-only and platform-assigned, but nothing assigned it: every line a rep added +stored an empty `Line #`, and the billing hand-off sent `"line_number": null` for each of +them on every won deal and every activated contract. A new `beforeInsert` hook on both +line-item objects now stamps `(highest line number under the parent) + 1` — a line added +from the Products or Line Items panel, through the API, or in a batch gets the next number +under its opportunity or quote, a number supplied by the caller is ignored, and an existing +number is never renumbered. The seed's own numbering is kept as authored. + +**Existing deployments: run the one-time back-fill.** Lines created before this release +still have no number. The same hook numbers such a line the next time it is edited; to +number them all now, in creation order under each parent, run +`pnpm exec tsx scripts/backfill-line-number.ts --url https:// --email +--password ` to see the count, then again with `--apply`. It is safe to re-run — a +converged org reports zero rows. Each touched line re-runs the parent rollups, exactly as +an edit would: an open deal's amount and a draft quote's totals are recomputed from their +lines. diff --git a/scripts/backfill-line-number.ts b/scripts/backfill-line-number.ts new file mode 100644 index 000000000..d809a77de --- /dev/null +++ b/scripts/backfill-line-number.ts @@ -0,0 +1,171 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +// +// One-time line-number backfill (#1828) — give every line item created before +// the assigner hook its ordinal. +// +// pnpm exec tsx scripts/backfill-line-number.ts --url https:// --email --password +// pnpm exec tsx scripts/backfill-line-number.ts ... --apply # without this it only reports +// +// `line_number` on `crm_opportunity_line_item` and `crm_quote_line_item` is +// `readonly`: platform-assigned. Until #1828 nothing assigned it, and since +// @objectstack/objectql 17.4.0 a readonly column is stripped from every +// non-system caller's INSERT, so each line a rep created stored a null ordinal — +// which the billing hand-off payload then shipped (`LINE_ITEM_FIELDS`). +// +// The writer is the `*_line_item_line_number` hook +// (`src/revenue/objects/_line-item-price-fill.ts`). It stamps an ordinal on +// insert, and ALSO on an update of a row that still has none — that second +// trigger is what this script pulls. It never writes a number itself (it could +// not: this is a REST caller, and the engine strips a readonly key it +// supplies); it sends `{ line_number: null }` to each null row, one PATCH per +// row, and the hook — whose written keys the engine keeps — stamps +// `(highest ordinal under the parent) + 1`. +// +// Order is the point of running it here rather than waiting for the next edit +// to heal each row: rows are touched parent by parent, oldest first +// (`created_at`, then `id`), so within a parent the ordinals follow creation +// order, after any ordinal the parent already carries. +// +// Safe by construction: +// • report-only unless `--apply` is passed; +// • it only touches rows whose `line_number` is empty; the hook never +// renumbers a row that has one; +// • rerunnable: a second pass over a converged org reports zero rows. +// +// Side effect worth knowing: a line-item write re-runs the parent rollups +// (`opportunity_amount_rollup`, `quote_total_rollup`), exactly as a rep's edit +// would — an open deal's amount / a draft quote's totals are recomputed from +// their lines. Closed deals and accepted / expired quotes are skipped by those +// hooks, as always. + +type Json = Record; + +const DEFAULT_URL = 'http://localhost:4001'; + +/** The two line-item objects and the master-detail field that is each one's parent. */ +const OBJECTS = [ + ['crm_opportunity_line_item', 'crm_opportunity'], + ['crm_quote_line_item', 'crm_quote'], +] as const; + +/** `--flag value` / `--flag=value`, else the env var, else the fallback. */ +function arg(name: string, envName: string, fallback: string): string { + const argv = process.argv.slice(2); + const eq = argv.find((a) => a.startsWith(`--${name}=`)); + if (eq) return eq.slice(name.length + 3); + const i = argv.indexOf(`--${name}`); + if (i !== -1 && argv[i + 1]) return argv[i + 1]; + return process.env[envName]?.trim() || fallback; +} + +const APPLY = process.argv.slice(2).includes('--apply'); + +class Api { + private cookie = ''; + constructor(private readonly base: URL) {} + + private async call(method: string, path: string, body?: Json): Promise<{ status: number; json: Json }> { + const res = await fetch(new URL(path, this.base), { + method, + headers: { + 'Content-Type': 'application/json', + Origin: this.base.origin, + ...(this.cookie ? { Cookie: this.cookie } : {}), + }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + }); + const setCookie = res.headers.getSetCookie?.() ?? []; + if (setCookie.length) this.cookie = setCookie.map((c) => c.split(';')[0]).join('; '); + const text = await res.text(); + let json: Json = {}; + try { json = text ? JSON.parse(text) : {}; } catch { json = { raw: text }; } + return { status: res.status, json }; + } + + private static message(json: Json): string { + return String(json?.error?.message ?? json?.error ?? json?.message ?? JSON.stringify(json)); + } + + async signIn(email: string, password: string): Promise { + const { status, json } = await this.call('POST', '/api/v1/auth/sign-in/email', { email, password }); + if (status !== 200 || !json?.user?.id) { + throw new Error(`sign-in failed for ${email} (${status}: ${json?.message ?? json?.code ?? 'unknown'})`); + } + return json.user as Json; + } + + /** Every row of `object`, paged. */ + async all(object: string, fields: string[]): Promise { + const out: Json[] = []; + for (let skip = 0; ; ) { + const { status, json } = await this.call('POST', `/api/v1/data/${object}/query`, { + filters: [], fields, sort: 'id asc', skip, top: 200, + }); + if (status < 200 || status >= 300) throw new Error(`query ${object} → ${status}: ${Api.message(json)}`); + const rows = (json.records ?? []) as Json[]; + out.push(...rows); + if (rows.length < 200) return out; + skip += rows.length; + } + } + + async patch(object: string, id: string, body: Json): Promise { + const { status, json } = await this.call('PATCH', `/api/v1/data/${object}/${id}`, body); + if (status < 200 || status >= 300) throw new Error(`PATCH ${object}/${id} → ${status}: ${Api.message(json)}`); + } +} + +const unnumbered = (r: Json) => typeof r.line_number !== 'number'; + +/** Parent by parent, oldest first — the order the ordinals should read in. */ +function touchOrder(rows: Json[], parentKey: string): Json[] { + const key = (r: Json) => [String(r[parentKey] ?? ''), String(r.created_at ?? ''), String(r.id)]; + return [...rows].sort((a, b) => { + const [ka, kb] = [key(a), key(b)]; + for (let i = 0; i < ka.length; i++) if (ka[i] !== kb[i]) return ka[i] < kb[i] ? -1 : 1; + return 0; + }); +} + +async function main(): Promise { + const url = arg('url', 'HOTCRM_URL', DEFAULT_URL); + const email = arg('email', 'HOTCRM_ADMIN_EMAIL', 'admin@objectos.ai'); + const password = arg('password', 'HOTCRM_ADMIN_PASSWORD', 'admin123'); + + const api = new Api(new URL(url)); + const me = await api.signIn(email, password); + console.log(`Signed in as ${me.email ?? me.id} at ${url}`); + console.log(APPLY ? 'Mode: APPLY (records will be written)' : 'Mode: REPORT ONLY (pass --apply to write)'); + + let failed = 0; + for (const [object, parentKey] of OBJECTS) { + const fields = ['id', parentKey, 'line_number', 'created_at']; + const rows = await api.all(object, fields); + const todo = touchOrder(rows.filter(unnumbered), parentKey); + const parents = new Set(todo.map((r) => r[parentKey])); + console.log(`\n${object}: ${rows.length} row(s), ${todo.length} without a line number, under ${parents.size} parent(s).`); + if (todo.length === 0 || !APPLY) continue; + + for (const r of todo) { + try { + await api.patch(object, String(r.id), { line_number: null }); + } catch (err) { + failed++; + console.log(` ${(err as Error).message}`); + } + } + // Read back rather than trusting the PATCH answers: the question is whether + // the rows now carry an ordinal, and only a re-read can say so. + const left = (await api.all(object, fields)).filter(unnumbered); + console.log(` Backfilled ${todo.length - left.length}/${todo.length}; ${left.length} still without a line number.`); + if (left.length > 0) failed++; + } + + if (!APPLY) console.log('\nReport only. Re-run with --apply to write.'); + if (failed > 0) process.exitCode = 1; +} + +main().catch((err) => { + console.error(`\nBackfill failed: ${(err as Error).message}`); + process.exitCode = 1; +}); diff --git a/src/revenue/objects/_line-item-price-fill.ts b/src/revenue/objects/_line-item-price-fill.ts index 19f210d5a..a7a480796 100644 --- a/src/revenue/objects/_line-item-price-fill.ts +++ b/src/revenue/objects/_line-item-price-fill.ts @@ -73,7 +73,7 @@ export function createLineItemPriceFill(objectName: string, hookName: string): H * each once): same stamp. An existing ordinal is never renumbered. The engine * keeps a payload key a hook wrote through its readonly strip, so a user write * lands the stamp and a user-supplied value is overwritten; only a no-user - * system insert that supplies one (the seed) keeps its own. `runAs: 'system'` + * system write that supplies one (the seed, and its replay) keeps its own. `runAs: 'system'` * elevates the sibling read and nothing else. Body-only rule as above: the * parent key comes from `ctx.object`, never a factory parameter. */ @@ -90,12 +90,9 @@ export function createLineItemNumbering(objectName: string, hookName: string): H const prev = ctx.previous; const api = ctx.api as HookApi | undefined; if (!api) return; - if (event === 'beforeUpdate') { - // D3: a predicate update has ONE payload for every row (forecast.hook.ts). - if (ctx.dispatch?.mode === 'per-row' || typeof prev?.line_number === 'number') return; - } else if (!ctx.user?.id && typeof input.line_number === 'number') { - return; - } + if (!ctx.user?.id && typeof input.line_number === 'number') return; + // D3: a predicate update has ONE payload for every row (forecast.hook.ts). + if (event === 'beforeUpdate' && (ctx.dispatch?.mode === 'per-row' || typeof prev?.line_number === 'number')) return; const parentKey = ctx.object === 'crm_quote_line_item' ? 'crm_quote' : 'crm_opportunity'; const parentId = input[parentKey] ?? prev?.[parentKey]; if (typeof parentId !== 'string' || !parentId) return; diff --git a/test/line-item-line-number.test.ts b/test/line-item-line-number.test.ts index be58dc22d..927b808c3 100644 --- a/test/line-item-line-number.test.ts +++ b/test/line-item-line-number.test.ts @@ -124,7 +124,13 @@ describe.each(PAIRS)('$object — the assigner', ({ object, parentKey, hook }) = expect(await ordinals(object, parentKey, 'p1')).toEqual([7]); }); - it('never renumbers an existing ordinal on update, whatever the payload says', async () => { + it('keeps the ordinal a no-user system update supplies (the seed replay re-asserting its fixture)', async () => { + const row = await ql.insert(object, line(parentKey, 'p1'), { context: rep }); + await ql.update(object, { id: row.id, line_number: 5 }, { where: { id: row.id }, context: seed }); + expect(await ordinals(object, parentKey, 'p1')).toEqual([5]); + }); + + it('never renumbers an existing ordinal on a rep\'s update, whatever the payload says', async () => { const row = await ql.insert(object, line(parentKey, 'p1'), { context: rep }); await ql.update(object, { id: row.id, line_number: 42, quantity: 2 }, { where: { id: row.id }, context: rep }); const stored = await ql.findOne(object, { where: { id: row.id }, context: seed }); From a1dbe2630d762b2f875151044798b3ab4a28505e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 13:46:24 +0000 Subject: [PATCH 3/4] fix(scripts): the line-number back-fill is a module, not a global script Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X8U3asekbiC7yWoEPWR4Dg --- .changeset/line-item-line-number-writer.md | 2 +- scripts/backfill-line-number.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.changeset/line-item-line-number-writer.md b/.changeset/line-item-line-number-writer.md index d834c2900..5cf480510 100644 --- a/.changeset/line-item-line-number-writer.md +++ b/.changeset/line-item-line-number-writer.md @@ -5,7 +5,7 @@ **Line items now carry their line number.** `line_number` on opportunity and quote line items is read-only and platform-assigned, but nothing assigned it: every line a rep added stored an empty `Line #`, and the billing hand-off sent `"line_number": null` for each of -them on every won deal and every activated contract. A new `beforeInsert` hook on both +them on every won deal and every activated contract. A new hook on both line-item objects now stamps `(highest line number under the parent) + 1` — a line added from the Products or Line Items panel, through the API, or in a batch gets the next number under its opportunity or quote, a number supplied by the caller is ignored, and an existing diff --git a/scripts/backfill-line-number.ts b/scripts/backfill-line-number.ts index d809a77de..70734bbab 100644 --- a/scripts/backfill-line-number.ts +++ b/scripts/backfill-line-number.ts @@ -169,3 +169,6 @@ main().catch((err) => { console.error(`\nBackfill failed: ${(err as Error).message}`); process.exitCode = 1; }); + +// A module, not a global script: `backfill-owner-id.ts` declares the same names. +export {}; From 946e409b33a4beb875d5a6fb552086e231569873 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 13:49:16 +0000 Subject: [PATCH 4/4] test: the line-number assigner's runtime file joins the coverage roster Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X8U3asekbiC7yWoEPWR4Dg --- test/runtime-coverage.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/runtime-coverage.test.ts b/test/runtime-coverage.test.ts index 8529fdbd7..97f932af9 100644 --- a/test/runtime-coverage.test.ts +++ b/test/runtime-coverage.test.ts @@ -93,6 +93,11 @@ const RUNTIME_TEST_FILES = [ 'account-approval-gate.test.ts', 'opportunity-account-capability-gate.test.ts', 'lead-conversion-approval-gate.test.ts', + // #1828 — the `line_number` assigner on both line items. Same precedent: its + // evidence is an ENGINE fact (a hook-written readonly key survives the + // non-system strip, a caller-supplied one does not) measured on a real + // ObjectQL with the strip's own control beside it, plus the lowered body. + 'line-item-line-number.test.ts', ]; /**