fix(buyer-tasks): mark-ready puts item into collection pool, not collected

When the operator clicks "Mark as ready" on a buyer-tasks po_sent card
and fills in the note, the item should land in the collection pool
(`item_status: "collection_pool"` + `ready_for_collection: true`) so
a driver can be assigned. Previously the service set
`item_status: "ready_delivery"`, which was rendered as the word
"Collected" on /orders and on /orders tabs — operators reasonably
read that as "driver already picked it up", and the item never
appeared on /collection-pool where they expected to assign a driver.

Now:
- markGroupReady writes a single PB update with the same shape as
  /api/v1/collection-map/items/[id]/assign (collection_pool +
  ready_for_collection: true + notes)
- po_sent buyer-tasks filter excludes collection_pool and assigned,
  so marked-ready items drop out of buyer-tasks and surface on
  /collection-pool instead
- ITEM_STATUS_VISUAL.ready_delivery is labelled "Ready for
  Collection" (kept distinct from the actual `collected` state) for
  any other flow that legitimately sets this status (e.g. stock-out)
- new item-status-ui.test.ts locks down the label/tab mapping so
  the regression can't return

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Co-authored-by: Orca <help@stably.ai>

src/lib/item-status-ui.test.ts

@@ -0,0 +1,66 @@
+import { describe, it, expect } from "vitest";
+import {
+  ITEM_STATUS_VISUAL,
+  TAB_GROUP_ITEM_STATUSES,
+  itemStatusToTabGroup,
+  tabGroupFilter,
+  getItemStatusVisual,
+} from "./item-status-ui";
+
+describe("ITEM_STATUS_VISUAL — ready_delivery", () => {
+  it("is labelled 'Ready for Collection' (NOT 'Collected')", () => {
+    // ready_delivery means "supplier confirmed ready for pickup" —
+    // it sits BEFORE the collection-pool flow. Calling it
+    // "Collected" makes the operator think the driver already
+    // picked the goods up.
+    expect(ITEM_STATUS_VISUAL.ready_delivery.label).toBe("Ready for Collection");
+  });
+
+  it("is visually distinct from the actual `collected` status", () => {
+    // Different label and different hex so the per-item pill is
+    // unambiguous in the order row.
+    expect(ITEM_STATUS_VISUAL.ready_delivery.label).not.toBe(
+      ITEM_STATUS_VISUAL.collected.label,
+    );
+    expect(ITEM_STATUS_VISUAL.ready_delivery.hex).not.toBe(
+      ITEM_STATUS_VISUAL.collected.hex,
+    );
+  });
+});
+
+describe("TAB_GROUP_ITEM_STATUSES — ready_delivery belongs to collection_pool", () => {
+  it("is in the collection_pool tab group, not the collected tab group", () => {
+    expect(TAB_GROUP_ITEM_STATUSES.collection_pool).toContain("ready_delivery");
+    expect(TAB_GROUP_ITEM_STATUSES.collected).not.toContain("ready_delivery");
+  });
+
+  it("ready_delivery maps to the collection_pool tab group", () => {
+    expect(itemStatusToTabGroup("ready_delivery")).toBe("collection_pool");
+  });
+});
+
+describe("tabGroupFilter", () => {
+  it("emits a PB-friendly filter for the collection_pool tab", () => {
+    // ready_delivery must show up under the collection_pool filter
+    // so an order containing a marked-ready item appears on that tab.
+    const filter = tabGroupFilter("collection_pool");
+    expect(filter).toContain(`item_status = "ready_delivery"`);
+    expect(filter).toContain(`item_status = "collection_pool"`);
+    expect(filter).toContain(`item_status = "assigned"`);
+  });
+
+  it("does NOT include ready_delivery under the collected filter", () => {
+    expect(tabGroupFilter("collected")).not.toContain("ready_delivery");
+  });
+});
+
+describe("getItemStatusVisual", () => {
+  it("returns the 'Ready for Collection' visual for ready_delivery", () => {
+    expect(getItemStatusVisual("ready_delivery").label).toBe("Ready for Collection");
+  });
+
+  it("falls back to a sane visual for unknown statuses", () => {
+    const v = getItemStatusVisual("definitely-not-a-real-status");
+    expect(v.label).toBe("Unknown");
+  });
+});

src/lib/item-status-ui.ts

@@ -43,12 +43,17 @@ export const ITEM_STATUS_VISUAL: Record<ItemStatus, ItemStatusVisual> = {
   pending:         { hex: "#2563eb", label: "New" },
   processed:       { hex: "#b45309", label: "Processing" },
   po_generated:    { hex: "#b45309", label: "Processing" },
+  // Pre-pickup collection flow. ready_delivery ("supplier confirmed
+  // ready for pickup") sits BEFORE the pool — it shares the purple
+  // hue with collection_pool/assigned so the operator sees the
+  // family, but gets its own label so they don't confuse it with
+  // the actual `collected` status (driver has picked up).
   collection_pool: { hex: "#7c3aed", label: "Collection Pool" },
   assigned:        { hex: "#7c3aed", label: "Collection Pool" },
+  ready_delivery:  { hex: "#7c3aed", label: "Ready for Collection" },
   collected:       { hex: "#0f766e", label: "Collected" },
   stock_verified:  { hex: "#0f766e", label: "Collected" },
   picking:         { hex: "#0f766e", label: "Collected" },
-  ready_delivery:  { hex: "#0f766e", label: "Collected" },
   delivered:       { hex: "#047857", label: "Delivered" },
   cancelled:       { hex: "#e11d48", label: "Cancelled" },
   ready_invoice:   { hex: "#1d4ed8", label: "Ready for Invoice" },
@@ -116,8 +121,13 @@ export const TAB_GROUP_ORDER: TabGroup[] = [
 export const TAB_GROUP_ITEM_STATUSES: Record<TabGroup, ItemStatus[]> = {
   new:              ["pending"],
   processing:       ["processed", "po_generated"],
-  collection_pool:  ["collection_pool", "assigned"],
-  collected:        ["collected", "stock_verified", "picking", "ready_delivery"],
+  // Pre-pickup flow — collection_pool, assigned, AND ready_delivery
+  // (supplier confirmed ready for pickup) all live here. ready_delivery
+  // used to be lumped under "collected" which made it look like the
+  // item had already been picked up by the driver, but it sits
+  // *before* the pool, not after.
+  collection_pool:  ["collection_pool", "assigned", "ready_delivery"],
+  collected:        ["collected", "stock_verified", "picking"],
   delivered:        ["delivered"],
   cancelled:        ["cancelled"],
 };

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

@@ -220,7 +220,14 @@ describe("listBuyerTasks", () => {
     // so they don't pollute the wire payload.
     const filter = (getFullList.mock.calls[0]?.[0] as { filter: string } | undefined)?.filter;
     expect(filter).toContain(`purchase_order != ""`);
+    // Marking ready (from the buyer-tasks MarkReadyDialog) flips
+    // the item into the collection pool with ready_for_collection:
+    // true. That must drop the item out of the buyer-tasks po_sent
+    // tab so the operator doesn't see the same card twice (once on
+    // /buyer-tasks, once on /collection-pool).
     expect(filter).toContain(`item_status != "ready_delivery"`);
+    expect(filter).toContain(`item_status != "collection_pool"`);
+    expect(filter).toContain(`item_status != "assigned"`);
     expect(filter).toContain(`item_status != "invoiced"`);
     expect(filter).toContain(`item_status != "delivered"`);
     expect(filter).toContain(`item_status != "cancelled"`);
@@ -279,16 +286,16 @@ describe("listBuyerTasks", () => {
     expect(groups[0].supplierEmail).toBeNull();
   });
 
-  it("does not include a separate ready_delivery exclusion in po_sent post-filter", async () => {
-    // The ready_delivery exclusion moved to the PB filter (covered by
-    // the filter-content assertions above). Verify the service still
-    // accepts a PO-sent item in a non-ready_delivery status, which
-    // means we don't accidentally re-introduce a redundant
-    // post-filter that would silently drop legitimate groups.
+  it("accepts a PO-sent item in a non-excluded status on po_sent", async () => {
+    // The item_status exclusions for the po_sent tab all live in the
+    // PB filter (covered by the filter-content assertions above) —
+    // verify the service still surfaces a legitimate PO-sent item
+    // so we don't accidentally re-introduce a redundant post-filter
+    // that would silently drop legitimate groups.
     const { admin } = makeAdmin([
       baseItem({
         purchase_order: "po-1",
-        item_status: "collection_pool",
+        item_status: "po_generated",
         expand: {
           order: baseOrder,
           supplier: baseSupplier,
@@ -374,7 +381,7 @@ describe("generatePoForGroup", () => {
 });
 
 describe("markGroupReady", () => {
-  it("updates all items to ready_delivery with the note written to notes", async () => {
+  it("moves items into the collection pool with ready_for_collection=true and the note", async () => {
     const getOne = vi.fn().mockResolvedValue({ order: "order-1" });
     const updateItem = vi.fn().mockResolvedValue({});
     const admin = {
@@ -392,12 +399,17 @@ describe("markGroupReady", () => {
 
     expect(result.updated).toBe(2);
     expect(result.failed).toEqual([]);
+    // Single write must flip both the status (into the collection
+    // pool) and the ready flag (so driver assignment is unblocked)
+    // — same shape the in-pool MarkReadyDialog uses internally.
     expect(updateItem).toHaveBeenCalledWith("item-1", {
-      item_status: "ready_delivery",
+      item_status: "collection_pool",
+      ready_for_collection: true,
       notes: "Spoke to David — pickup tomorrow 09:00",
     });
     expect(updateItem).toHaveBeenCalledWith("item-2", {
-      item_status: "ready_delivery",
+      item_status: "collection_pool",
+      ready_for_collection: true,
       notes: "Spoke to David — pickup tomorrow 09:00",
     });
   });
@@ -414,7 +426,8 @@ describe("markGroupReady", () => {
     await markGroupReady("order-1", "sup-1", [{ id: "item-1" }], "  trimmed  ");
 
     expect(updateItem).toHaveBeenCalledWith("item-1", {
-      item_status: "ready_delivery",
+      item_status: "collection_pool",
+      ready_for_collection: true,
       notes: "trimmed",
     });
   });

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

@@ -50,7 +50,7 @@ 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 != "invoiced" && item_status != "delivered" && item_status != "cancelled"`;
+      : `purchase_order != "" && item_status != "ready_delivery" && item_status != "collection_pool" && item_status != "assigned" && item_status != "invoiced" && item_status != "delivered" && item_status != "cancelled"`;
 
   const items = await admin.collection("order_items").getFullList({
     filter: itemFilter,
@@ -197,7 +197,16 @@ export async function markGroupReady(
         continue;
       }
       await admin.collection("order_items").update(item.id, {
-        item_status: "ready_delivery",
+        // Marking the item ready for collection puts it straight into
+        // the collection-pool queue (`item_status: "collection_pool"`)
+        // with the supplier's ready flag set. The collection-pool
+        // route (`/api/v1/collection-map/items/[id]/assign`) gates
+        // driver assignment on `ready_for_collection === true`, so a
+        // single write here is enough to unblock driver assignment —
+        // no second API call needed. This mirrors what the in-pool
+        // MarkReadyDialog does for the collection-map path.
+        item_status: "collection_pool",
+        ready_for_collection: true,
         // Audit log of who the buyer spoke to / reference number
         // etc. Mirrors the collection-pool MarkReadyDialog flow.
         notes: trimmedNote,