fix(po-send): stop auto-flipping order_items.item_status to collection_pool

Previously, when recordPurchaseOrderSendOutcome received a "success"
outcome from the n8n workflow, it auto-flipped every linked
order_items.item_status from "po_generated" to "collection_pool".

This dropped items off the buyer-tasks PO Sent tab immediately on
send — before the supplier had even confirmed receipt. The operator
had no chance to mark items ready-to-collect manually.

New flow: send mail → items stay "po_generated" and remain visible
in buyer-tasks PO Sent → operator manually marks items ready →
items drop off buyer-tasks.

The back-link sync (purchase_orders.order_item) still runs and now
reads all items with purchase_order = poId regardless of status, so
the array stays accurate as items progress through their lifecycle.

Adds src/lib/server/purchase-orders-service.test.ts with 4 cases:
- success does NOT auto-flip linked items
- success flips PO to sent + stamps email_message_id
- success keeps the PO's order_item back-link in sync
- failure flips PO to mail_failed + stamps error (also no item writes)

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

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

src/lib/server/purchase-orders-service.test.ts

@@ -0,0 +1,155 @@
+import { describe, it, expect, beforeEach, vi } from "vitest";
+
+vi.mock("@/lib/server/pocketbase-admin", () => ({
+  getAdminPocketBase: vi.fn(),
+}));
+
+import { getAdminPocketBase } from "./pocketbase-admin";
+import { recordPurchaseOrderSendOutcome } from "./purchase-orders-service";
+
+/**
+ * Build a mock admin client wired for the outcome-recording flow:
+ *  - purchase_orders.getOne   → the PO with email_payload
+ *  - purchase_orders.update   → echoes the payload as the new row,
+ *                               captures the call for assertions
+ *  - order_items.getFullList  → the linked items
+ *  - order_items.update       → captures any per-item writes
+ */
+const makeAdmin = ({
+  existingItems = [],
+  poOverrides = {},
+}: {
+  existingItems?: Array<Record<string, unknown>>;
+  poOverrides?: Record<string, unknown>;
+} = {}) => {
+  const po = {
+    id: "po-1",
+    status: "sending",
+    email_payload: { subject: "PO-001", body: "<p>x</p>", bodyText: "x" },
+    ...poOverrides,
+  };
+
+  const poUpdate = vi
+    .fn()
+    .mockImplementation((_id: string, payload: Record<string, unknown>) =>
+      Promise.resolve({ ...po, ...payload }),
+    );
+  const orderItemUpdate = vi
+    .fn()
+    .mockImplementation((id: string, payload: Record<string, unknown>) =>
+      Promise.resolve({ id, ...payload }),
+    );
+
+  const admin = {
+    collection: vi.fn().mockImplementation((name: string) => {
+      if (name === "purchase_orders") {
+        return {
+          getOne: vi.fn().mockResolvedValue(po),
+          update: poUpdate,
+          getFullList: vi.fn().mockResolvedValue([]),
+        };
+      }
+      if (name === "order_items") {
+        return {
+          getFullList: vi.fn().mockResolvedValue(existingItems),
+          update: orderItemUpdate,
+          getOne: vi.fn(),
+        };
+      }
+      return { getOne: vi.fn(), update: vi.fn(), getFullList: vi.fn() };
+    }),
+  };
+
+  return { admin, po, poUpdate, orderItemUpdate };
+};
+
+beforeEach(() => {
+  vi.clearAllMocks();
+});
+
+describe("recordPurchaseOrderSendOutcome", () => {
+  it("does NOT auto-flip linked order_items.item_status to collection_pool on success (operator must mark them ready manually)", async () => {
+    const { admin, orderItemUpdate } = makeAdmin({
+      existingItems: [
+        { id: "item-1", purchase_order: "po-1", item_status: "po_generated" },
+        { id: "item-2", purchase_order: "po-1", item_status: "po_generated" },
+      ],
+    });
+    vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
+
+    await recordPurchaseOrderSendOutcome({
+      poId: "po-1",
+      outcome: "success",
+      messageId: "<abc@wfsales.co.za>",
+    });
+
+    // The service should NOT touch order_items on success. The buyer-tasks
+    // po_sent tab should keep showing the items until the operator manually
+    // marks them ready to collect.
+    expect(orderItemUpdate).not.toHaveBeenCalled();
+  });
+
+  it("flips PO status to sent + stamps email_message_id on success", async () => {
+    const { admin, poUpdate } = makeAdmin();
+    vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
+
+    await recordPurchaseOrderSendOutcome({
+      poId: "po-1",
+      outcome: "success",
+      messageId: "<abc@wfsales.co.za>",
+    });
+
+    expect(poUpdate).toHaveBeenCalled();
+    const firstCall = poUpdate.mock.calls[0]!;
+    const payload = firstCall[1] as Record<string, unknown>;
+    expect(payload).toMatchObject({
+      status: "sent",
+      email_message_id: "<abc@wfsales.co.za>",
+      email_error: "",
+    });
+  });
+
+  it("still keeps the purchase_orders.order_item back-link in sync with the linked items on success", async () => {
+    const { admin, poUpdate } = makeAdmin({
+      existingItems: [
+        { id: "item-1", purchase_order: "po-1", item_status: "po_generated" },
+        { id: "item-2", purchase_order: "po-1", item_status: "po_generated" },
+      ],
+    });
+    vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
+
+    await recordPurchaseOrderSendOutcome({
+      poId: "po-1",
+      outcome: "success",
+      messageId: "<abc@wfsales.co.za>",
+    });
+
+    // The second poUpdate call carries the back-link array.
+    const backLinkCall = poUpdate.mock.calls.find((call) => {
+      const payload = call[1] as Record<string, unknown>;
+      return Array.isArray(payload.order_item);
+    });
+    expect(backLinkCall).toBeDefined();
+    const payload = backLinkCall![1] as Record<string, unknown>;
+    expect(payload.order_item).toEqual(["item-1", "item-2"]);
+  });
+
+  it("flips PO to mail_failed + stamps errorMessage on failure (does not touch order_items)", async () => {
+    const { admin, poUpdate, orderItemUpdate } = makeAdmin();
+    vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
+
+    await recordPurchaseOrderSendOutcome({
+      poId: "po-1",
+      outcome: "failure",
+      errorMessage: "SMTP 550 mailbox unavailable",
+    });
+
+    expect(orderItemUpdate).not.toHaveBeenCalled();
+    expect(poUpdate).toHaveBeenCalledTimes(1);
+    const payload = poUpdate.mock.calls[0]![1] as Record<string, unknown>;
+    expect(payload).toMatchObject({
+      status: "mail_failed",
+      email_error: "[smtp] SMTP 550 mailbox unavailable",
+    });
+  });
+});

src/lib/server/purchase-orders-service.ts

@@ -348,9 +348,12 @@ async function markPreSendFailure(
  * `Purchase Order Outbound Send` workflow (or any other worker that's
  * been handed the send).
  *
- * `outcome = "success"` flips status to `sent` + stamps delivery fields
- * + flips linked `order_items.item_status` from `po_generated` to
- * `collection_pool` (so the driver-app can pick them up).
+ * `outcome = "success"` flips status to `sent` + stamps delivery fields.
+ * It deliberately does NOT touch linked `order_items.item_status` — the
+ * operator marks items ready-to-collect manually from the order page /
+ * stock-orders page after the supplier confirms, which then drops them
+ * off the buyer-tasks PO Sent tab. Auto-flipping on send would hide the
+ * item before the supplier has even confirmed.
  *
  * `outcome = "failure"` flips to `mail_failed` + stamps error.
  *
@@ -392,46 +395,28 @@ export async function recordPurchaseOrderSendOutcome(
       patch
     )) as PurchaseOrder;
 
-    // Flip linked order_items from `po_generated` to `collection_pool`.
-    // We don't touch `drv_id` / `drv_status` — those are set later by the
-    // assignment page. Status flip here just signals "PO is in the
-    // supplier pipeline; ready for the driver to collect when stock
-    // arrives".
+    // Read the linked order_items to keep the back-link on the PO in
+    // sync. `order_items.purchase_order` is the canonical direction;
+    // `purchase_orders.order_item` is the mirror that surfaces in the
+    // PB admin UI and lets any future PO-side render read the items
+    // without an extra query.
     //
-    // Status name note: the UI surface (`item-status-ui.ts`) calls this
-    // state "Collection Pool" — matching what the operator sees when
-    // they look at the line item after sending the PO. The internal
-    // `item_status` enum value is `collection_pool` to keep code and
-    // UI in lockstep; historically this was written as `driver_pool`
-    // which matched a separate (now-removed) `collection_pool` table.
-    // The migration that renamed the table to `DO_NOT_USE_*` did NOT
-    // rename the `item_status` value on existing rows, so any POs sent
-    // before that migration wrote `driver_pool` — the migration step in
-    // this batch updates those rows.
+    // We deliberately do NOT touch `item_status` here. Previously this
+    // function auto-flipped linked items from `po_generated` to
+    // `collection_pool` on send, which dropped them off buyer-tasks
+    // immediately — before the supplier had even confirmed. The
+    // operator now marks items ready-to-collect from the order page /
+    // stock-orders page after the supplier commits, and only that
+    // explicit flip removes them from the buyer-tasks PO Sent tab.
+    //
+    // The filter intentionally doesn't restrict by `item_status` —
+    // we want every item linked to this PO regardless of where it is
+    // in the lifecycle, so the back-link array stays accurate.
     const linked = await pb.collection(ORDER_ITEMS_COLLECTION).getFullList({
-      filter: `purchase_order = "${input.poId}" && item_status = "po_generated"`,
+      filter: `purchase_order = "${input.poId}"`,
       fields: "id",
     });
-    // Newly-entered pool rows start as `ready_for_collection = false` —
-    // the supplier hasn't been confirmed yet. The operator marks them
-    // ready from the dashboard's collection-pool / collection-map pages
-    // once the supplier commits, which then unblocks the driver app's
-    // `getPoolItems` filter to surface them.
-    for (const it of linked) {
-      await pb
-        .collection(ORDER_ITEMS_COLLECTION)
-        .update(it.id, {
-          item_status: "collection_pool",
-          ready_for_collection: false,
-        });
-    }
 
-    // Keep the back-link on the PO in sync. `order_items.purchase_order`
-    // is the canonical direction; `purchase_orders.order_item` is the
-    // mirror that surfaces in the PB admin UI and lets any future
-    // PO-side render read the items without an extra query. We write
-    // the full set after the flip so a later assignment page can
-    // trust the array matches `purchase_order = "X"`.
     await pb.collection(PO_COLLECTION).update(input.poId, {
       order_item: linked.map((it) => it.id),
     });