fix(delivery-notes): correct DN cancellation and filter issues

Update the cancel delivery note dialog description to correctly state items are marked as cancelled
Fix PocketBase expand field whitelisting for delivery items to avoid missing related entity data
Change order item status to "cancelled" instead of "completed" when cancelling a delivery note, with updated JSDoc explaining the choice of status
Update the buyer tasks PO sent filter and its regression test to cover all current ItemStatus values

src/app/(app)/delivery-notes/[id]/page.tsx

@@ -1016,7 +1016,7 @@ export default function DeliveryNoteDetailPage({ params }: PageProps) {
           <DialogHeader>
             <DialogTitle>Cancel delivery note {deliveryNote.delivery_note_number}?</DialogTitle>
             <DialogDescription>
-              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.
             </DialogDescription>
           </DialogHeader>
           <div className="space-y-2">

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 () => {

src/lib/server/buyer-tasks-service.ts

@@ -87,7 +87,21 @@ export async function listBuyerTasks(tab: BuyerTaskTab): Promise<BuyerTaskGroup[
     ? `item_status = "processed" && purchase_order = ""`
     : tab === "po_draft"
       ? `item_status = "po_generated" && purchase_order != ""`
-      : `purchase_order != "" && item_status != "ready_delivery" && item_status != "collection_pool" && item_status != "assigned" && item_status != "collected" && item_status != "ready_invoice" && item_status != "invoiced" && item_status != "delivered" && item_status != "cancelled"`;
+      // PO Sent: PO emailed, awaiting supplier. Every status outside
+      // `po_generated` represents a state the buyer no longer owns:
+      //   • supplier confirmed ready → ready_delivery
+      //   • marked ready for collection → collection_pool
+      //   • driver assigned → assigned
+      //   • driver picked up → collected
+      //   • delivery-note issued → delivery_note
+      //   • awaiting invoice / invoiced / delivered / cancelled → terminal
+      //
+      // This is a negative list by design (PB can't express "all
+      // statuses except these" positively), so it MUST be kept in
+      // lockstep with the ItemStatus union. The regression test in
+      // `buyer-tasks-service.test.ts` walks every union member and
+      // asserts the post-PB exclusion list covers it.
+      : `purchase_order != "" && item_status != "ready_delivery" && item_status != "collection_pool" && item_status != "assigned" && item_status != "collected" && item_status != "delivery_note" && item_status != "ready_invoice" && item_status != "invoiced" && item_status != "stock_booked" && item_status != "with_driver" && item_status != "checked_in" && item_status != "completed" && item_status != "delivered" && item_status != "cancelled" && item_status != "removed"`;
 
   const items = await admin.collection("order_items").getFullList({
     filter: itemFilter,

src/lib/server/deliveries-service.ts

@@ -208,10 +208,20 @@ export async function listDeliveryItems(params: {
       // render "Customer X — Order ORD-..." without a second
       // round trip per row. PB's expand syntax is dot-separated
       // paths; commas separate siblings.
+      //
+      // IMPORTANT: when `fields=` is set, PB also whitelists
+      // top-level response keys — and the `expand` block is a
+      // top-level key. If the expanded relation isn't added to
+      // the whitelist (as `expand.<path>`), 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 = {

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,
       }),
     ),