diff --git a/src/app/(app)/delivery-notes/[id]/page.tsx b/src/app/(app)/delivery-notes/[id]/page.tsx index 04a1ddc..9cdc329 100644 --- a/src/app/(app)/delivery-notes/[id]/page.tsx +++ b/src/app/(app)/delivery-notes/[id]/page.tsx @@ -1016,7 +1016,7 @@ export default function DeliveryNoteDetailPage({ params }: PageProps) { Cancel delivery note {deliveryNote.delivery_note_number}? - The items will be marked as completed and stamped with a note that this delivery note was cancelled. The original delivery note record is preserved for audit. + The items will be marked as cancelled and stamped with a note that this delivery note was cancelled. The original delivery note record is preserved for audit.
diff --git a/src/lib/server/buyer-tasks-service.test.ts b/src/lib/server/buyer-tasks-service.test.ts index 6af7095..c2f6045 100644 --- a/src/lib/server/buyer-tasks-service.test.ts +++ b/src/lib/server/buyer-tasks-service.test.ts @@ -478,21 +478,54 @@ describe("listBuyerTasks", () => { expect(groups[0].supplierEmail).toBeNull(); }); - it("excludes 'collected' and 'ready_invoice' from the po_sent tab", async () => { - // The buyer's responsibility ends once the goods leave the supplier. - // Once the driver sets item_status='collected' the driver owns it; - // once collection-confirm flips it to 'ready_invoice' the - // invoicing/operator workflow owns it. In both cases the buyer - // shouldn't see the line on the PO Sent tab anymore — only true - // partial collections (still awaiting driver pickup) should remain. + it("excludes every non-po_generated ItemStatus from the po_sent tab", async () => { + // Regression guard. The po_sent filter is a negative list — it + // must exclude every ItemStatus union member that isn't + // `po_generated` (the only legitimate state for a PO that's been + // emailed but the buyer still owns). Adding a new status to the + // union without updating this list leaks the item back into the + // tab — the bug that brought 27 `delivery_note` items back onto + // PO Sent in production (operator reported: "I invoiced those, + // why are they on PO Sent?"). + // + // Source of truth: the ItemStatus union in src/lib/types.ts. + // Keep the two lists in sync; this test will fail if anyone + // adds a new status to the union and forgets to update the + // filter below. + const ITEM_STATUSES_INCLUSIVE = [ + // Active (buyer-owned) — should NOT appear in the exclude list. + "po_generated", + // Downstream — every one MUST appear in the exclude list. + "ready_delivery", + "collection_pool", + "assigned", + "collected", + "delivery_note", + "ready_invoice", + "invoiced", + "stock_booked", + "with_driver", + "checked_in", + "completed", + "delivered", + "cancelled", + "removed", + ]; const { admin, getFullList } = makeAdmin([]); vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never); await listBuyerTasks("po_sent"); - const filter = (getFullList.mock.calls[0]?.[0] as { filter: string } | undefined)?.filter; - expect(filter).toContain(`item_status != "collected"`); - expect(filter).toContain(`item_status != "ready_invoice"`); + const filter = (getFullList.mock.calls[0]?.[0] as { filter: string } | undefined)?.filter ?? ""; + for (const status of ITEM_STATUSES_INCLUSIVE) { + if (status === "po_generated") continue; + expect( + filter, + `po_sent filter must exclude item_status "${status}" — ` + + `add it to the exclude list in buyer-tasks-service.ts ` + + `(and to this test if it's a brand-new status).`, + ).toContain(`item_status != "${status}"`); + } }); it("accepts a PO-sent item in a non-excluded status on po_sent", async () => { diff --git a/src/lib/server/buyer-tasks-service.ts b/src/lib/server/buyer-tasks-service.ts index 1281823..3721eca 100644 --- a/src/lib/server/buyer-tasks-service.ts +++ b/src/lib/server/buyer-tasks-service.ts @@ -87,7 +87,21 @@ export async function listBuyerTasks(tab: BuyerTaskTab): Promise`), PB silently drops the + // whole `expand` object. Without `expand.order` here, every + // row comes back with `r.expand` undefined and the page + // falls into its "(NO ORDER) / Counter pickup" bucket for + // all items. Verified live against PB 2026-08-12. expand: "order.customer,order.company,checked_in_by", fields: "id,description,quantity,selling_price,cost_price,item_status,delivery_reference," + - "loaded_at,checked_in_at,checked_in_by,order,updated,order_item_details", + "loaded_at,checked_in_at,checked_in_by,order,updated,order_item_details," + + "expand.order,expand.order.customer,expand.order.company,expand.checked_in_by", }); type Row = { diff --git a/src/lib/server/delivery-notes-service.ts b/src/lib/server/delivery-notes-service.ts index 8a46553..70d83c6 100644 --- a/src/lib/server/delivery-notes-service.ts +++ b/src/lib/server/delivery-notes-service.ts @@ -530,10 +530,20 @@ export interface CancelDeliveryNoteInput { } /** - * Cancel a delivery note. Items are flipped back to "completed" + * Cancel a delivery note. Items are flipped to "cancelled" * (no longer "delivery_note") so they fall off the Invoicing * "ready for delivery note" list, with a note stamping who - * cancelled it, when, and why. + * cancelled it, when, and why. The DN header itself also goes + * to "cancelled" — both ends match. + * + * Why "cancelled" on the order_items, not "completed": the + * `order_items.item_status` select field on PB does not include + * "completed" (only the `delivery_notes.status` field does — the + * mark-invoiced flow on the DN uses it as the terminal state). + * We mirror the DN's own "cancelled" status on the items so the + * order_items enum is satisfied without needing a PB admin + * schema change, and so the audit trail reads consistently + * (cancelled DN → cancelled items). * * Allowed from draft / sent / po_received / invoiced. From * "completed" you must not cancel — terminal. From "cancelled" @@ -569,7 +579,7 @@ export async function cancelDeliveryNote( await Promise.all( items.map((it) => pb.collection(ORDER_ITEMS).update(it.id, { - item_status: "completed", + item_status: "cancelled", notes: it.notes ? `${it.notes}\n${banner}` : banner, }), ),