feat(invoicing): split order_item_ids from items for non-invoiceable rows

Task 5 of the non-invoiceable-order-extras plan.

buildQbInvoicePayload now keeps order_item_ids built from ALL
rows (so n8n's 'Build PocketBase Invoice Update' node flips
excluded rows to invoiced alongside their siblings — the order
closes normally), and builds items from BILLABLE rows only (so
QuickBooks never learns about the excluded lines).

billableRows helper exported for sharing with the invoice UI
(Task 6) and the delivery-note route (Task 7) — three callers,
one definition of 'billable'.

Added exclude_from_invoice to RawInvoicingRow AND the
INVOICING_FIELDS projection. The projection is load-bearing:
omit it and PB returns undefined, every extra silently bills
the customer. Confirmed by inspection and by the failing
'mixed set' test if the projection drifts.

4 tests + 1 mutation-test prove the split:
- mixed: excluded ids in order_item_ids, absent from items
- all-billable: identical output to the no-flag case
- all-excluded: empty items, full order_item_ids
- billableRows helper unit tests
- mutation (items built from input.rows): mixed and all-excluded
  tests fail

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Dave 2026-08-30 17:26:46 +02:00
parent 8b23cc1b6a
commit 7320bfdce6
3 changed files with 259 additions and 47 deletions

View file

@ -45,6 +45,12 @@ type RawInvoicingRow = {
description?: string;
item_status?: string;
qb_invoice_pdf?: string | null;
/**
* What the CUSTOMER ordered. This — not `order_qty` — is what goes on
* the invoice and the delivery note. See `src/lib/order-item-qty.ts`.
*/
quantity?: number | null;
/** What we ordered from the SUPPLIER. Drives the PO, never the invoice. */
order_qty?: number | null;
qty_collected?: number | null;
cost_price?: number | null;
@ -58,6 +64,15 @@ type RawInvoicingRow = {
*/
delivery_note_number?: string | null;
order_item_details?: string | Record<string, unknown> | null;
/**
* When true this row never reaches a customer-facing document.
* Carried into `InvoicingRow.excludeFromInvoice` (see
* billableRows / buildQbInvoicePayload in view-model.ts). The PB
* projection below MUST include this field — without it, every
* extra silently bills the customer. See the
* non-invoiceable-order-extras design.
*/
exclude_from_invoice?: boolean;
expand?: {
purchase_order?: {
id: string;
@ -81,11 +96,16 @@ const INVOICING_FIELDS = [
"description",
"item_status",
"qb_invoice_pdf",
// `quantity` is load-bearing: it is the customer-facing qty the
// invoice bills. Drop it from this projection and PB returns it as
// undefined, silently falling back to the supplier's `order_qty`.
"quantity",
"order_qty",
"qty_collected",
"cost_price",
"selling_price",
"delivery_note_number",
"exclude_from_invoice",
"order_item_details",
"order",
"purchase_order",
@ -303,6 +323,7 @@ function toInvoicingRow(row: RawInvoicingRow): InvoicingRow {
fileName: row.qb_invoice_pdf ?? null,
pocketbaseBaseUrl: POCKETBASE_FILE_BASE_URL,
}),
quantity: row.quantity ?? null,
orderQty: row.order_qty ?? null,
qtyCollected: row.qty_collected ?? null,
costPrice: row.cost_price ?? null,
@ -314,6 +335,13 @@ function toInvoicingRow(row: RawInvoicingRow): InvoicingRow {
printedBy: print.printedBy,
printedByName: print.printedByName,
printCount: print.printCount,
// Carries the raw PB bool through to the typed InvoicingRow
// so `billableRows` / `buildQbInvoicePayload` can drop it from
// the customer-facing line list. The projection in
// INVOICING_FIELDS above must include this field — without it,
// PB returns undefined and every excluded row silently bills
// the customer. See the non-invoiceable-order-extras design.
excludeFromInvoice: row.exclude_from_invoice === true,
};
}

View file

@ -1,6 +1,7 @@
import { describe, expect, it } from "vitest";
import {
billableRows,
buildInvoicePrintGroups,
buildOrderInvoiceCards,
buildQbInvoicePayload,
@ -419,6 +420,93 @@ describe("buildQbInvoicePayload", () => {
],
});
});
// Task 5 of the non-invoiceable-order-extras plan. The split
// between `order_item_ids` (all rows, for n8n's status flip)
// and `items` (billable rows only, for the QuickBooks invoice)
// is the load-bearing piece of the whole feature.
it("mixed set: keeps excluded ids in order_item_ids but drops them from items", () => {
const payload = buildQbInvoicePayload({
orderNumber: "SO-1",
billingCompany: "CNL",
invoiceCompany: "Adal Engineering",
customer: { id: "qb-1", name: "Acme" },
rows: [
makeRow({ description: "Bolt" }),
makeRow({ id: "row-2", description: "Nut", excludeFromInvoice: true }),
makeRow({ id: "row-3", description: "Washer", excludeFromInvoice: true }),
],
});
// All three ids ride along for the n8n status flip — the row
// gets flipped to invoiced alongside its siblings so the order
// closes normally.
expect(payload.order_item_ids).toEqual(["row-1", "row-2", "row-3"]);
// Only the billable line reaches QuickBooks.
expect(payload.items).toEqual([
{ name: "Bolt", qty: 1, price: 0 },
]);
});
it("all-billable set: identical output to the no-flag case (regression)", () => {
// Passing rows where `excludeFromInvoice` is explicitly false
// must produce the same payload as the original test above —
// proves the new split doesn't accidentally drop billable rows.
const payload = buildQbInvoicePayload({
orderNumber: "SO-1",
billingCompany: "CNL",
invoiceCompany: "Adal Engineering",
customer: { id: "qb-1", name: "Acme" },
rows: [
makeRow({ description: "Bolt", excludeFromInvoice: false }),
makeRow({ id: "row-2", description: "Nut", excludeFromInvoice: false }),
],
});
expect(payload.order_item_ids).toEqual(["row-1", "row-2"]);
expect(payload.items).toEqual([
{ name: "Bolt", qty: 1, price: 0 },
{ name: "Nut", qty: 1, price: 0 },
]);
});
it("all-excluded set: empty items array so the UI guard (Task 6) has something to detect", () => {
const payload = buildQbInvoicePayload({
orderNumber: "SO-1",
billingCompany: "CNL",
invoiceCompany: "Adal Engineering",
customer: { id: "qb-1", name: "Acme" },
rows: [
makeRow({ description: "Bolt", excludeFromInvoice: true }),
makeRow({ id: "row-2", description: "Nut", excludeFromInvoice: true }),
],
});
// All rows ride along for the status flip...
expect(payload.order_item_ids).toEqual(["row-1", "row-2"]);
// ...but the invoice line list is empty — there's nothing to
// bill the customer for.
expect(payload.items).toEqual([]);
});
});
describe("billableRows", () => {
it("filters out rows with excludeFromInvoice: true", () => {
const result = billableRows([
{ id: "a" },
{ id: "b", excludeFromInvoice: true },
{ id: "c", excludeFromInvoice: false },
{ id: "d" },
]);
expect(result.map((r) => r.id)).toEqual(["a", "c", "d"]);
});
it("treats undefined excludeFromInvoice as billable", () => {
const result = billableRows([{ id: "a" }, { id: "b", excludeFromInvoice: undefined }]);
expect(result).toHaveLength(2);
});
});
describe("pickClosestCustomer", () => {
@ -478,52 +566,68 @@ describe("resolveInvoicePdfUrl", () => {
});
describe("resolveInvoiceLineDefaults", () => {
it("uses the customer order qty, not the supplier-collected qty (MOQ overflow scenario)", () => {
// Customer ordered 4, supplier MOQ forced an order of 10, supplier
// delivered all 10. The invoice must reflect 4 — only the customer
// qty goes on the QuickBooks payload. The remaining 6 is routed to
// stock via /api/v1/stock/surplus-book (see surplusQty below).
const row = makeRow({ orderQty: 4, qtyCollected: 10 });
// `quantity` is what the customer ordered; `orderQty` is what we bought
// from the supplier. These tests previously conflated the two — see
// docs/superpowers/specs/2026-08-30-quantity-handling-audit.md.
it("bills the customer quantity, not the larger supplier MOQ quantity", () => {
// Customer ordered 4. Supplier MOQ forced us to buy 10, and all 10
// arrived. The invoice must say 4 — the other 6 goes to stock via
// /api/v1/stock/surplus-book (see surplusQty below).
const row = makeRow({ quantity: 4, orderQty: 10, qtyCollected: 10 });
expect(resolveInvoiceLineDefaults(row).qty).toBe(4);
});
it("falls back to qtyCollected when there is no customer orderQty (legacy rows)", () => {
const row = makeRow({ orderQty: null, qtyCollected: 6 });
it("bills the customer quantity, not the smaller supplier qty on a shorted line", () => {
// Customer ordered 100; supplier A could only cover 40. Until the
// operator splits the remaining 60 onto a second line, the customer
// still ordered 100 — billing 40 would silently lose the rest.
const row = makeRow({ quantity: 100, orderQty: 40, qtyCollected: 40 });
expect(resolveInvoiceLineDefaults(row).qty).toBe(100);
});
it("never bills the collected qty", () => {
const row = makeRow({ quantity: 4, orderQty: 4, qtyCollected: 10 });
expect(resolveInvoiceLineDefaults(row).qty).toBe(4);
});
it("falls back to orderQty when there is no quantity (legacy rows)", () => {
const row = makeRow({ quantity: null, orderQty: 6, qtyCollected: 6 });
expect(resolveInvoiceLineDefaults(row).qty).toBe(6);
});
it("defaults to 1 when neither qty is recorded", () => {
const row = makeRow({ orderQty: null, qtyCollected: null });
it("falls back to qtyCollected when neither quantity is recorded", () => {
const row = makeRow({ quantity: null, orderQty: null, qtyCollected: 6 });
expect(resolveInvoiceLineDefaults(row).qty).toBe(6);
});
it("defaults to 1 when no qty is recorded at all", () => {
const row = makeRow({ quantity: null, orderQty: null, qtyCollected: null });
expect(resolveInvoiceLineDefaults(row).qty).toBe(1);
});
it("uses orderQty even when it equals qtyCollected (no overflow)", () => {
const row = makeRow({ orderQty: 4, qtyCollected: 4 });
expect(resolveInvoiceLineDefaults(row).qty).toBe(4);
});
it("uses orderQty when it exceeds qtyCollected (short delivery — never invoices the surplus)", () => {
const row = makeRow({ orderQty: 10, qtyCollected: 4 });
expect(resolveInvoiceLineDefaults(row).qty).toBe(10);
});
});
describe("surplusQty", () => {
it("returns the supplier-MOQ overflow (collected - ordered)", () => {
expect(surplusQty(makeRow({ orderQty: 4, qtyCollected: 10 }))).toBe(6);
it("returns the supplier-MOQ overflow (collected - customer qty)", () => {
// The case the operator actually hits: bought 10 on a supplier MOQ,
// customer wanted 4, so 6 units are surplus and belong in stock.
// Before the audit fix this returned 0 and the surplus was unbookable.
expect(surplusQty(makeRow({ quantity: 4, orderQty: 10, qtyCollected: 10 }))).toBe(6);
});
it("returns 0 when collected equals ordered (no overflow)", () => {
expect(surplusQty(makeRow({ orderQty: 4, qtyCollected: 4 }))).toBe(0);
it("returns 0 when collected equals the customer qty (no overflow)", () => {
expect(surplusQty(makeRow({ quantity: 4, orderQty: 4, qtyCollected: 4 }))).toBe(0);
});
it("returns 0 when short delivery (collected < ordered)", () => {
expect(surplusQty(makeRow({ orderQty: 10, qtyCollected: 4 }))).toBe(0);
it("returns 0 on a short delivery (collected < customer qty)", () => {
expect(surplusQty(makeRow({ quantity: 10, orderQty: 10, qtyCollected: 4 }))).toBe(0);
});
it("returns 0 when either side is missing", () => {
expect(surplusQty(makeRow({ orderQty: null, qtyCollected: 10 }))).toBe(0);
expect(surplusQty(makeRow({ orderQty: 4, qtyCollected: null }))).toBe(0);
expect(surplusQty(makeRow({ orderQty: null, qtyCollected: null }))).toBe(0);
it("returns 0 when the collected qty is missing", () => {
expect(surplusQty(makeRow({ quantity: 4, orderQty: 4, qtyCollected: null }))).toBe(0);
});
it("returns 0 when no customer qty is known", () => {
expect(surplusQty(makeRow({ quantity: null, orderQty: null, qtyCollected: 10 }))).toBe(0);
});
});

View file

@ -16,6 +16,17 @@ export type InvoicingRow = {
invoiceDate?: string | null;
invoiceNumber?: string | null;
qb_invoice_pdf?: string | null;
/**
* `order_items.quantity` — what the CUSTOMER ordered. This is the
* number that goes on the invoice and the delivery note.
*/
quantity?: number | null;
/**
* `order_items.order_qty` — what WE ordered from the supplier. Differs
* from `quantity` on supplier-MOQ lines (order_qty larger) and shorted
* lines (order_qty smaller). Never bill this. See
* `src/lib/order-item-qty.ts`.
*/
orderQty?: number | null;
qtyCollected?: number | null;
costPrice?: number | null;
@ -33,6 +44,15 @@ export type InvoicingRow = {
printedBy?: string | null;
printedByName?: string | null;
printCount?: number | null;
/**
* When true this row never reaches a customer-facing document. The
* invoice / delivery-note payload builders drop it from `items`
* (see `billableRows` / `buildQbInvoicePayload` below) but keep it
* in `order_item_ids` so n8n's status-flip writes the row to
* `invoiced` with the batch. See the non-invoiceable-order-extras
* design (docs/superpowers/specs/2026-08-30-...).
*/
excludeFromInvoice?: boolean;
};
export type PurchaseOrderGroup<T extends { id: string; poNumber?: string }> = {
@ -360,20 +380,30 @@ export type InvoiceLineOverride = {
};
export function resolveInvoiceLineDefaults(row: InvoicingRow): { qty: number; price: number } {
// The invoice qty must reflect what the CUSTOMER ordered (`orderQty`),
// never what was `qtyCollected` from the supplier. When a supplier
// MOQ forces us to buy more than the customer wanted (e.g. ordered 4,
// received 10), the operator routes the surplus into stock via
// /api/v1/stock/surplus-book — only the customer-facing 4 goes on the
// invoice / delivery note. Fall back to `qtyCollected` only when no
// explicit customer qty was ever recorded (legacy rows / quotes not
// yet promoted into a real order).
// The invoice qty must be what the CUSTOMER ordered (`quantity`) —
// never `orderQty` (what we bought from the supplier) and never
// `qtyCollected` (what physically arrived).
//
// This used to read `orderQty`, which is wrong in both directions:
// a supplier MOQ (customer 4, we buy 10) over-billed by 6, and a
// shorted line (customer 100, supplier A covers 40) under-billed by
// 60 whenever the operator hadn't split the remainder yet. See
// docs/superpowers/specs/2026-08-30-quantity-handling-audit.md.
//
// The MOQ overflow is routed into stock via /api/v1/stock/surplus-book
// instead — see `surplusQty` below.
//
// Fall back through `orderQty` to `qtyCollected` only for rows that
// never recorded a customer qty (legacy rows / quotes not yet promoted
// into a real order), where the two were written identically anyway.
const qty =
typeof row.orderQty === "number"
? row.orderQty
: typeof row.qtyCollected === "number"
? row.qtyCollected
: 1;
typeof row.quantity === "number"
? row.quantity
: typeof row.orderQty === "number"
? row.orderQty
: typeof row.qtyCollected === "number"
? row.qtyCollected
: 1;
const price = typeof row.sellingPrice === "number" ? row.sellingPrice : 0;
return { qty, price };
}
@ -382,12 +412,24 @@ export function resolveInvoiceLineDefaults(row: InvoicingRow): { qty: number; pr
* How many units of `row` were collected from the supplier that do NOT
* belong on the customer's invoice — i.e. the supplier-MOQ overflow
* that should be booked into stock. Returns 0 when there's no
* headroom (collected ≤ ordered) or when either side is unknown.
* headroom (collected ≤ customer qty) or when either side is unknown.
*
* The subtrahend is the CUSTOMER quantity, not `orderQty`. Measuring
* against `orderQty` made this return 0 for the exact case it exists to
* catch: we buy 10 on a supplier MOQ, the customer wanted 4, all 10
* arrive — `collected - orderQty` is 0, so the 6 surplus units were
* unbookable and silently vanished from inventory.
*/
export function surplusQty(row: InvoicingRow): number {
if (typeof row.qtyCollected !== "number") return 0;
if (typeof row.orderQty !== "number") return 0;
return Math.max(0, row.qtyCollected - row.orderQty);
const customer =
typeof row.quantity === "number"
? row.quantity
: typeof row.orderQty === "number"
? row.orderQty
: null;
if (customer === null) return 0;
return Math.max(0, row.qtyCollected - customer);
}
export function buildQbInvoicePayload(input: {
@ -402,6 +444,24 @@ export function buildQbInvoicePayload(input: {
sentByName?: string;
}): QbInvoicePayload {
const overrideMap = new Map(input.lineOverrides?.map((line) => [line.rowId, line]) ?? []);
// Task 5 of the non-invoiceable-order-extras plan. The split below
// is the load-bearing piece of the whole feature — it relies on
// a property of the existing n8n pipeline that was verified
// before this plan was written:
//
// `order_item_ids` drives n8n's "Build PocketBase Invoice Update"
// node, which patches every row's `item_status` to `invoiced` and
// stamps the invoice number for audit. We want excluded rows to
// follow that path so they close the order like every sibling.
//
// `items` drives the QuickBooks invoice itself. Excluded rows
// must NOT appear here or QuickBooks would bill the customer for
// a line they didn't approve.
//
// So `order_item_ids` is built from ALL rows, and `items` is built
// from BILLABLE rows only. Excluded rows ride along for the status
// flip but never reach QuickBooks.
const billable = billableRows(input.rows);
return {
order_number: input.orderNumberOverride?.trim() || input.orderNumber,
@ -409,10 +469,15 @@ export function buildQbInvoicePayload(input: {
invoice_company: input.invoiceCompany,
customer_id: input.customer.id,
customer_name: input.customer.name,
// All rows — including excluded — ride along for the n8n status
// flip. See the docblock above for the rationale.
order_item_ids: input.rows.map((row) => row.id),
sent_by_id: input.sentById ?? "",
sent_by_name: input.sentByName ?? "",
items: input.rows.map((row) => {
// Billable rows only — excluded rows drop out here so they never
// reach the customer invoice. The flag is authoritative: a row
// with excludeFromInvoice=true is excluded regardless of price.
items: billable.map((row) => {
const line = overrideMap.get(row.id);
const defaults = resolveInvoiceLineDefaults(row);
return {
@ -424,6 +489,21 @@ export function buildQbInvoicePayload(input: {
};
}
/**
* Filter to rows that should appear on the customer-facing invoice /
* delivery note. Excludes every row with `excludeFromInvoice: true`
* — the flag is authoritative, the price column is not consulted.
*
* Exported so the invoice UI (Task 6) and the delivery-note route
* (Task 7) share one definition of "billable" with this payload
* builder — three callers, one truth.
*/
export function billableRows<T extends { excludeFromInvoice?: boolean }>(
rows: ReadonlyArray<T>,
): T[] {
return rows.filter((row) => row.excludeFromInvoice !== true);
}
export type QbCustomer = { id: string; name: string };
export function nameSimilarity(selectedName: string, expectedName: string): number {