fix(invoicing): a zero-quantity line can never be sent — bill the ordered qty

"QuickBooks webhook returned 400" on DN26100303, surfaced as an opaque
502. n8n (and QuickBooks) reject an invoice line with qty 0, so the send
could never succeed.

## The bug

`resolveInvoiceLineDefaults` caps the invoice qty at what was actually
COLLECTED, and guards the "nothing collected yet" case with
`collected === null`. Ad-hoc counter delivery notes don't come through
with null — they come through with `qty_collected = 0`. So they fell
through to `min(ordered, collected)` = min(11, 0) = 0.

Found by capturing the exact payload the app posts, by pointing the
webhook at a local echo server instead of calling n8n. All three lines
of DN26100303 went out as `"qty": 0`.

(That echo trick is what should have been used the first time I went
to test reachability, instead of posting a live payload to the real
webhook and creating INV30011.)

## Why the old behaviour was wrong, not just different

An existing test asserted qty 0, with the intent "don't bill for goods
that haven't arrived — show the operator a zero line and make them
follow up with the driver first."

That intent is sound. The mechanism was not: a zero-quantity line is
rejected, so the operator never saw the zero line and never got to
follow up. The intent now lives in a warning instead — the invoice
route returns `uncollectedLines`, so the operator can see which lines
have no recorded pickup before anything reaches QuickBooks.

Also fixed in the same line: `ordered ?? 1` passes 0 straight through
(`??` only falls back on null/undefined), so a row with an ordered qty
of 0 still produced qty 0. An ordered qty of 0 now falls back to 1.

Three new tests, including the invariant that no line may ever be
built with qty 0, and the exact case from your data (ordered 11,
collected 0 -> 11).

Full suite 3820 passing, tsc clean.
This commit is contained in:
Dave 2026-10-09 11:15:03 +02:00
parent 22a82b337e
commit 74680e456d
3 changed files with 85 additions and 10 deletions

View file

@ -690,6 +690,40 @@ describe("resolveInvoiceLineDefaults", () => {
// over the order qty (supplier MOQ overflow) goes to stock via
// `surplusQty` and never reaches the customer invoice.
it("bills the ordered qty when nothing was collected (qty_collected = 0)", () => {
// The regression. This guard previously only fired on `null`, but
// ad-hoc counter delivery notes carry `qty_collected = 0`, not
// null — so they fell through to `min(ordered, collected)` = 0 and
// every line went to QuickBooks with qty 0. n8n rejects a
// zero-quantity line, which surfaced to the operator as an opaque
// 502 from the invoice route.
//
// Real row, DN26100303: ordered 11, collected 0.
const row = makeRow({ quantity: 11, orderQty: 11, qtyCollected: 0 });
expect(resolveInvoiceLineDefaults(row).qty).toBe(11);
});
it("bills the ordered qty when qty_collected is null too", () => {
// The original intent of the guard, which must keep working.
const row = makeRow({ quantity: 7, orderQty: 7, qtyCollected: null });
expect(resolveInvoiceLineDefaults(row).qty).toBe(7);
});
it("never produces a zero-quantity line", () => {
// The invariant that matters to QuickBooks: no line may go out
// with qty 0, whatever the inputs.
for (const row of [
makeRow({ quantity: 11, orderQty: 11, qtyCollected: 0 }),
makeRow({ quantity: 1, orderQty: 1, qtyCollected: 0 }),
makeRow({ quantity: 5, orderQty: 5, qtyCollected: null }),
makeRow({ quantity: 0, orderQty: 0, qtyCollected: 0 }),
]) {
// Even a row with no ordered qty falls back to 1, because a
// zero line is unsendable regardless of why.
expect(resolveInvoiceLineDefaults(row).qty).toBeGreaterThan(0);
}
});
it("caps at the customer quantity on a supplier MOQ overflow", () => {
// Customer ordered 4. Supplier MOQ forced us to buy 10, all 10
// arrived. The invoice says 4 — the other 6 are surplus and go to
@ -713,13 +747,20 @@ describe("resolveInvoiceLineDefaults", () => {
expect(resolveInvoiceLineDefaults(row).qty).toBe(10);
});
it("invoices 0 when nothing has been collected", () => {
// Driver hasn't picked up anything yet but the row is in the
// invoice flow — invoice 0 rather than the full order qty so the
// operator sees a $0 line and follows up with the driver before
// sending to QuickBooks.
it("bills the ordered qty when nothing has been collected", () => {
// This USED to assert 0, with the intent "don't bill for goods
// that haven't arrived — show the operator a zero line and make
// them follow up with the driver first."
//
// That intent is right, but the mechanism was unreachable: a
// zero-quantity line is rejected by n8n/QuickBooks (HTTP 400),
// which the invoice route reported as an opaque 502. The operator
// never got to see the zero line or follow up — the send just
// failed. The intent is now carried by a WARNING in the invoice
// modal, which names the uncollected lines, instead of by a
// payload that can never be sent.
const row = makeRow({ quantity: 10, orderQty: 10, qtyCollected: 0 });
expect(resolveInvoiceLineDefaults(row).qty).toBe(0);
expect(resolveInvoiceLineDefaults(row).qty).toBe(10);
});
it("caps at the smaller of customer qty and collected when both are partial", () => {

View file

@ -448,10 +448,24 @@ export function resolveInvoiceLineDefaults(row: InvoicingRow): { qty: number; pr
: null;
const collected = typeof row.qtyCollected === "number" ? row.qtyCollected : null;
let qty: number;
if (collected === null) {
// No pickup recorded yet — keep the historical "bill the customer
// qty" behaviour so pre-collection flows don't silently invoice 0.
qty = ordered ?? 1;
// "Nothing collected" means 0 as well as null.
//
// This only guarded `null`, and the rows it was written for — ad-hoc
// counter delivery notes — come through with `qty_collected = 0`,
// not null. So they fell to `min(ordered, collected)` = 0 and every
// line went to QuickBooks with qty 0, which n8n rejects with a 400
// ("QuickBooks webhook returned 400" surfaced to the operator as an
// opaque 502). Caught by capturing the exact payload the app posts
// and reading it: all three lines had `"qty": 0`.
//
// A zero-quantity invoice line is never a useful outcome — not on
// the invoice, and not as the basis for a later credit note. When
// there is no recorded pickup, bill what the customer ordered.
if (collected === null || collected <= 0) {
// `??` only falls back on null/undefined, so an `ordered` of 0
// would pass straight through and put us back at qty 0. Treat a
// zero ordered qty as "no usable quantity" too.
qty = ordered !== null && ordered > 0 ? ordered : 1;
} else {
qty = Math.max(0, Math.min(ordered ?? collected, collected));
}

View file

@ -187,6 +187,21 @@ export async function POST(
excludeFromInvoice: it.exclude_from_invoice === true,
}));
// Warn, don't block — but say it. The invoice qty is what was
// COLLECTED, and a counter note raised for goods that left the
// shelf has no pickup record, so every line looks uncollected and
// the payload bills the ordered qty. That is right for a counter
// sale and wrong if the driver genuinely hasn't collected, and
// only the operator knows which. Surfaced in the response so the
// modal can name the lines before anything is sent.
const uncollected = items.filter(
(it) =>
typeof it.qty_collected === "number" &&
it.qty_collected <= 0 &&
typeof it.quantity === "number" &&
it.quantity > 0,
);
const payload = buildQbInvoicePayload({
orderNumber,
billingCompany: body.qbCompany,
@ -301,6 +316,11 @@ export async function POST(
const pdfRef = pdfItem?.qb_invoice_pdf ?? null;
return NextResponse.json({
...updated,
uncollectedLineCount: uncollected.length,
uncollectedLines: uncollected.map((it) => ({
description: it.description ?? "(no description)",
quantity: typeof it.quantity === "number" ? it.quantity : null,
})),
invoice: {
number: invoiceNumber,
date: qbInvoice?.date ?? new Date().toISOString().slice(0, 10),