From cb3f1618c4b150b3666c91e4d6ad5bac1739c9a7 Mon Sep 17 00:00:00 2001 From: Dave Date: Sun, 30 Aug 2026 17:35:15 +0200 Subject: [PATCH] feat(invoicing): split delivery-note PDF lines for non-invoiceable rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 7 of the non-invoiceable-order-extras plan. The status flip + audit stamp + delivery_note header create all run on EVERY row so the order closes normally — excluded rows are real operational lines from the supplier's POV. The PDF line list is built from billable.items only, so the customer never sees excluded lines. Added exclude_from_invoice to the getFullList projection (load- bearing — omit it and the billable filter silently passes undefined, every extra lands on the customer delivery note). New 400 guard 'No billable lines for a delivery note' when every row in the request is excluded. UI guard in Task 6 prevents the operator from reaching this path, but the route has its own defence so the picker doesn't get a guaranteed-to-fail submit silently. 2 tests + 1 mutation-test prove the split: - excluded rows still get status flip + DN number + PDF upload, but the renderer is called with ONLY billable items - all-excluded returns 400 before any PATCH fires - mutation (PDF built from items): the mixed test fails because the renderer sees 3 lines instead of 2 Co-Authored-By: Claude --- .../api/invoicing/delivery-note/route.test.ts | 105 +++++++++++++++++- src/app/api/invoicing/delivery-note/route.ts | 53 +++++++-- 2 files changed, 147 insertions(+), 11 deletions(-) diff --git a/src/app/api/invoicing/delivery-note/route.test.ts b/src/app/api/invoicing/delivery-note/route.test.ts index 4e8e439..d8b9894 100644 --- a/src/app/api/invoicing/delivery-note/route.test.ts +++ b/src/app/api/invoicing/delivery-note/route.test.ts @@ -1,11 +1,29 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -const { getFullList, getList, updateMock, createMock } = vi.hoisted(() => { +const { + getFullList, + getList, + updateMock, + createMock, + renderDeliveryNotePdfMock, +} = vi.hoisted(() => { const getFullList = vi.fn(); const getList = vi.fn(); const updateMock = vi.fn(); const createMock = vi.fn(); - return { getFullList, getList, updateMock, createMock }; + // Spy on the PDF renderer so the test can assert which lines the + // route passes in. Default returns a tiny PDF buffer so the upload + // step in the route has something to upload. + const renderDeliveryNotePdfMock = vi.fn(async () => + Buffer.from("%PDF-1.4\n"), + ); + return { + getFullList, + getList, + updateMock, + createMock, + renderDeliveryNotePdfMock, + }; }); vi.mock("@/lib/server/pocketbase-admin", () => ({ @@ -19,6 +37,10 @@ vi.mock("@/lib/server/pocketbase-admin", () => ({ }), })); +vi.mock("@/lib/pdf/render", () => ({ + renderDeliveryNotePdf: renderDeliveryNotePdfMock, +})); + import { POST } from "@/app/api/invoicing/delivery-note/route"; function makeRequest(body: unknown): Request { @@ -31,6 +53,8 @@ function makeRequest(body: unknown): Request { describe("POST /api/invoicing/delivery-note", () => { beforeEach(() => { + renderDeliveryNotePdfMock.mockReset(); + renderDeliveryNotePdfMock.mockResolvedValue(Buffer.from("%PDF-1.4\n")); getFullList.mockReset(); getList.mockReset(); updateMock.mockReset(); @@ -313,4 +337,81 @@ describe("POST /api/invoicing/delivery-note", () => { expect(call.item_status).toBe("delivery_note"); expect(call.delivery_note_number).toBe("DN26071401"); }); + + // Task 7 of the non-invoiceable-order-extras plan. The status + // flip (item_status + DN number) lands on EVERY row so the order + // closes normally; the PDF line list drops excluded rows so the + // customer never sees them. The two together are the load-bearing + // split. + + it("stamps status + DN number on excluded rows but drops them from the PDF", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-07-14T13:00:00.000Z")); + + // Three rows, two billable + one excluded. Override the default + // 2-row fixture so we can also assert the excluded row got the + // status flip while the PDF line list is filtered down to 2. + getFullList.mockResolvedValueOnce([ + { id: "bill-1", description: "Bolt", order_item_details: {} }, + { id: "excl-1", description: "Nut (supplier-side)", order_item_details: {}, exclude_from_invoice: true }, + { id: "bill-2", description: "Washer", order_item_details: {} }, + ]); + + const response = await POST( + makeRequest({ orderItemIds: ["bill-1", "excl-1", "bill-2"] }) as any, + ); + expect(response.status).toBe(200); + const body = await response.json(); + expect(body.updated).toBe(3); + expect(body.deliveryNoteNumber).toBe("DN26071401"); + + // ALL three rows got the JSON PATCH (status flip + DN number). + // Excluded rows must ride along — they're an operational line + // from the supplier's POV, the order can only close if every row + // is in delivery_note status. + const jsonCalls = updateMock.mock.calls.filter( + (call) => !(call[1] instanceof FormData), + ); + expect(jsonCalls.length).toBe(3); + for (const call of jsonCalls) { + const patch = call[1] as Record; + expect(patch.item_status).toBe("delivery_note"); + expect(patch.delivery_note_number).toBe("DN26071401"); + } + + // ALL three rows also got the FormData PDF upload (picking-slip + // regression: see Task 8. The picker still pulls the washers.). + const formCalls = updateMock.mock.calls.filter( + (call) => call[1] instanceof FormData, + ); + expect(formCalls.length).toBe(3); + + // The PDF renderer was called with ONLY the billable rows. + // Excluded rows drop out at this stage so the customer never + // sees them. Capturing this directly via the render mock is + // what catches the load-bearing mutation in this split. + expect(renderDeliveryNotePdfMock).toHaveBeenCalledTimes(1); + const renderInput = renderDeliveryNotePdfMock.mock.calls[0][0] as { + lines: Array<{ description?: string }>; + }; + expect(renderInput.lines).toHaveLength(2); + expect(renderInput.lines.map((l) => l.description)).toEqual(["Bolt", "Washer"]); + }); + + it("returns 400 when every row is excluded (no billable lines for a delivery note)", async () => { + getFullList.mockResolvedValueOnce([ + { id: "excl-1", order_item_details: {}, exclude_from_invoice: true }, + { id: "excl-2", order_item_details: {}, exclude_from_invoice: true }, + ]); + + const response = await POST( + makeRequest({ orderItemIds: ["excl-1", "excl-2"] }) as any, + ); + expect(response.status).toBe(400); + const body = await response.json(); + expect(body.error).toMatch(/no billable lines/i); + + // No status flips happened — the guard runs before the PATCH loop. + expect(updateMock).not.toHaveBeenCalled(); + }); }); diff --git a/src/app/api/invoicing/delivery-note/route.ts b/src/app/api/invoicing/delivery-note/route.ts index 3ecaa9b..af1e674 100644 --- a/src/app/api/invoicing/delivery-note/route.ts +++ b/src/app/api/invoicing/delivery-note/route.ts @@ -2,7 +2,7 @@ import { NextRequest, NextResponse } from "next/server"; import { getAdminPocketBase } from "@/lib/server/pocketbase-admin"; import { renderDeliveryNotePdf } from "@/lib/pdf/render"; -import { resolveDeliveryNoteLineQty } from "@/lib/pdf/delivery-note-document"; +import { buildDeliveryNoteLine } from "@/lib/pdf/delivery-note-document"; import { deliveryNoteLetterheadCode, fetchLetterhead, @@ -52,8 +52,19 @@ type OrderItemRecord = { delivery_note_number?: string; order_item_details?: unknown; order_item_attachements?: unknown; + /** Customer-facing qty — what the delivery note must show. */ + quantity?: number | null; + /** Supplier-facing qty — what we bought. Never printed to the customer. */ order_qty?: number | null; qty_collected?: number | null; + /** + * Non-invoiceable extras feature (2026-08-30). When true the row + * still receives the status flip + DN number (the order closes + * normally, the picker sees the washers on the slip) but it + * drops out of the `lines:` array sent to the PDF renderer — + * see the billable filter below. + */ + exclude_from_invoice?: boolean; expand?: { order?: { id: string; @@ -208,14 +219,41 @@ export async function POST(request: NextRequest) { // Fetch the items with the expand we need for the PDF (supplier // + order + company + customer). We re-fetch the same items // after the status update below to get the persisted state. + // + // `exclude_from_invoice` is included in the projection so the + // billable filter below can drop excluded rows from the PDF + // line list. Without it the field comes back undefined and every + // extra silently lands on the customer delivery note — the same + // load-bearing-projection concern documented for INVOICING_FIELDS. const items = await pb.collection("order_items").getFullList({ filter: orderItemIds.map((id) => `id = "${id}"`).join(" || "), + fields: "id,description,delivery_note_number,order_item_details,order_item_attachements,quantity,order_qty,qty_collected,exclude_from_invoice", expand: "purchase_order.supplier,order.company,order.customer", }); if (items.length === 0) { return NextResponse.json({ error: "No matching order items found" }, { status: 404 }); } + // Billable subset — Task 7 of the non-invoiceable-order-extras + // plan. The status flip + audit stamp above run on EVERY item + // (excluded rows must close the order like their siblings), but + // the PDF line list drops excluded rows so the customer never + // sees them. Sharing `billableRows` with the QB invoice path + // (Task 5) keeps one definition of "billable" for the whole + // feature — three callers, one truth. + const billable = items.filter((it) => it.exclude_from_invoice !== true); + if (billable.length === 0) { + // Every row in this delivery note is excluded from invoicing. + // The status flip would still close the order correctly, but + // rendering a zero-line delivery note is noise — return 400 so + // the UI (Task 6) can disable submit on the empty subset + // case and the operator can re-include a billable row. + return NextResponse.json( + { error: "No billable lines for a delivery note" }, + { status: 400 }, + ); + } + // 1. Flip the status + stamp the audit metadata. Done as a // parallel fan-out so a slow item doesn't block the rest. // PB returns generic 400 "Failed to update record." on any @@ -359,14 +397,11 @@ export async function POST(request: NextRequest) { companyName, customerName: order?.expand?.customer?.customer ?? "", }, - lines: items.map((it) => ({ - id: it.id, - description: (it.description ?? "").trim() || "(no description)", - qty: resolveDeliveryNoteLineQty({ - orderQty: it.order_qty, - qtyCollected: it.qty_collected, - }), - })), + // Billable rows only — excluded rows drop out here so they + // never appear on the customer delivery note. The status + // flip above has already stamped item_status and the DN + // number on every row, so the order closes normally. + lines: billable.map(buildDeliveryNoteLine), letterhead, logoDataUri, });