From f017a7f9858fb2d051883ca5649ac43e732e32b9 Mon Sep 17 00:00:00 2001 From: Dave Date: Mon, 19 Oct 2026 15:08:12 +0200 Subject: [PATCH] fix: number customer POs one at a time (duplicate PO numbers) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PO numbers were "read the month's highest, add one", so POs for several suppliers on one order created together got the same number — 34 numbers by 2026-10-19 (e.g. PO202610008LIF for BarloWorld CAT and John Ratcliffe, created 71 ms apart). - createPurchaseOrderWithNumber (po-number.ts): numbers and creates POs one at a time in the server, retries (up to 5) when the unique index rejects a clash; ignores any po_number passed in. - buyer-tasks / MCP generate_po use it. - The Process page's Send PO and in-stock PO buttons now call POST /api/v1/orders/[id]/purchase-orders (browser session) instead of numbering in the browser; the client generatePONumber is removed from pb-helpers. - npm run pb:migrate:po-number-unique adds a partial unique index on purchase_orders.po_number for POs created from then on; the 34 existing duplicates are left as they are. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 11 +++ package.json | 1 + .../2026-10-19-po-number-unique-index.ts | 64 +++++++++++++ src/app/(app)/orders/[id]/process/page.tsx | 57 ++++++----- .../orders/[id]/purchase-orders/route.test.ts | 63 ++++++++++++ .../v1/orders/[id]/purchase-orders/route.ts | 55 +++++++++++ src/lib/pb-helpers.ts | 65 +------------ src/lib/server/buyer-tasks-service.ts | 9 +- src/lib/server/po-number-create.test.ts | 96 +++++++++++++++++++ src/lib/server/po-number.ts | 60 ++++++++++-- 10 files changed, 382 insertions(+), 99 deletions(-) create mode 100644 scripts/migrations/2026-10-19-po-number-unique-index.ts create mode 100644 src/app/api/v1/orders/[id]/purchase-orders/route.test.ts create mode 100644 src/app/api/v1/orders/[id]/purchase-orders/route.ts create mode 100644 src/lib/server/po-number-create.test.ts diff --git a/AGENTS.md b/AGENTS.md index 70d0ea6..3e2c5a8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -205,3 +205,14 @@ Runbook: `docs/collection-ai-checks.md`. Files: `src/lib/server/collection-ai-{r - The AI writes **only** `ai_check`, `collection_doc_reads` rows, an **empty** SKU when it is sure, and — only when a "sure" tax-invoice line shows a discount — the cost set to the price after discount (via `updateOrderItemCost`, with a note). It never overwrites a person's SKU, confirms a collection, or changes status. - The service refuses to run until `ai_check` exists (PB drops unknown fields silently); `ai_check` is in `IGNORED_FIELDS` of the audit hook. - Manual live steps (like select values and pb_hooks): run `npm run pb:migrate:collection-ai` against the live PB, redeploy `pocketbase/pb_hooks/audit_log_utils.js`, deploy, activate the n8n workflow "WFDASH Collection check tick" (every 5 min), then switch Collection checks on in Settings → AI. See the runbook. + +# PO numbers: create customer POs only through `createPurchaseOrderWithNumber` + +`src/lib/server/po-number.ts` numbers a PO by reading the month's highest +number and adding one. Called in parallel (one order, several suppliers) +that gave 34 duplicate PO numbers by 2026-10-19. `createPurchaseOrderWithNumber` +numbers and creates one PO at a time and retries when the unique index on +`po_number` rejects a clash. Never call `generatePONumber` and then +`purchase_orders.create` yourself; the browser goes through +`POST /api/v1/orders/[id]/purchase-orders`. The partial unique index is a +manual live step: `npm run pb:migrate:po-number-unique` (after deploying). diff --git a/package.json b/package.json index 13fc657..b11be48 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "pb:migrate:rep-trip-legs": "npx tsx scripts/migrations/2026-09-28-create-rep-trip-legs.ts", "pb:migrate:quote-assistant": "npx tsx scripts/migrations/2026-10-16-add-quote-assistant-fields.ts", "pb:migrate:collection-ai": "npx tsx scripts/migrations/2026-10-17-add-collection-ai.ts", + "pb:migrate:po-number-unique": "npx tsx scripts/migrations/2026-10-19-po-number-unique-index.ts", "pb:migrate:giellie-chats": "npx tsx scripts/migrations/2026-09-30-create-giellie-chats.ts", "pb:migrate:place-overrides": "npx tsx scripts/migrations/2026-09-28-create-place-overrides.ts", "pb:migrate:drop-orders-status": "npx tsx scripts/migrations/2026-06-27-drop-orders-status-and-wipe.ts", diff --git a/scripts/migrations/2026-10-19-po-number-unique-index.ts b/scripts/migrations/2026-10-19-po-number-unique-index.ts new file mode 100644 index 0000000..835758d --- /dev/null +++ b/scripts/migrations/2026-10-19-po-number-unique-index.ts @@ -0,0 +1,64 @@ +/** + * One-shot migration: a unique index on purchase_orders.po_number for POs + * created from now on. + * + * Why: PO numbers were read-highest-then-+1, so POs for several suppliers + * on one order, created together, shared a number (34 numbers by + * 2026-10-19, e.g. PO202610008LIF for BarloWorld CAT and John Ratcliffe). + * The app now numbers POs one at a time (createPurchaseOrderWithNumber); + * this index is the safety net if two servers ever race. + * + * It's a PARTIAL index (`WHERE created >= `) because the 34 + * existing duplicates were already sent to suppliers and are left as they + * are — a full unique index would refuse to build over them. + * + * Run with: + * + * npm run pb:migrate:po-number-unique + * + * Idempotent — skips when the index already exists. Manual step on the + * live PB (like select values and pb_hooks); run it after deploying the + * app change. Rollback: delete the index in the PB admin (purchase_orders + * → Edit collection → Indexes). + */ +import "./_env-loader"; +import PocketBase from "pocketbase"; + +const PB_URL = process.env.NEXT_PUBLIC_POCKETBASE_URL || process.env.PB_URL || "http://127.0.0.1:8090"; +const ADMIN_EMAIL = process.env.PB_ADMIN_EMAIL; +const ADMIN_PASSWORD = process.env.PB_ADMIN_PASSWORD; +const TAG = "[po-number-unique]"; +const INDEX_NAME = "idx_purchase_orders_po_number_unique_new"; + +async function main() { + if (!ADMIN_EMAIL || !ADMIN_PASSWORD) throw new Error("PB_ADMIN_EMAIL and PB_ADMIN_PASSWORD are required"); + const pb = new PocketBase(PB_URL); + pb.autoCancellation(false); + await pb.collection("_superusers").authWithPassword(ADMIN_EMAIL, ADMIN_PASSWORD); + + const collection = await pb.collections.getOne("purchase_orders"); + const indexes: string[] = Array.isArray(collection.indexes) ? collection.indexes : []; + if (indexes.some((i) => i.includes(INDEX_NAME))) { + console.log(`${TAG} ${INDEX_NAME} already exists — nothing to do.`); + return; + } + + // PB stores `created` as "YYYY-MM-DD HH:MM:SS.sssZ"; compare in that form. + const since = new Date().toISOString().replace("T", " "); + const all = await pb.collection("purchase_orders").getFullList<{ po_number: string; created: string }>({ + fields: "po_number,created", + }); + const counts = new Map(); + for (const p of all) counts.set(p.po_number, (counts.get(p.po_number) ?? 0) + 1); + const dupes = [...counts.values()].filter((n) => n > 1).length; + console.log(`${TAG} ${all.length} POs, ${dupes} duplicated numbers (left as they are; the index starts at ${since}).`); + + const sql = `CREATE UNIQUE INDEX \`${INDEX_NAME}\` ON \`purchase_orders\` (\`po_number\`) WHERE \`created\` >= '${since}'`; + await pb.collections.update(collection.id, { indexes: [...indexes, sql] }); + console.log(`${TAG} added: ${sql}`); +} + +main().catch((err) => { + console.error(`${TAG} failed:`, err?.response ?? err); + process.exit(1); +}); diff --git a/src/app/(app)/orders/[id]/process/page.tsx b/src/app/(app)/orders/[id]/process/page.tsx index c72ed48..52c8c03 100644 --- a/src/app/(app)/orders/[id]/process/page.tsx +++ b/src/app/(app)/orders/[id]/process/page.tsx @@ -51,7 +51,6 @@ import { getOrderItems, getAllSuppliers, getActiveStockItems, - generatePONumber, getQuoteItems, formatCurrency, } from "@/lib/pb-helpers"; @@ -273,6 +272,26 @@ function supplierStripeColor(id: string | undefined): string | null { return `hsl(${hue}, 65%, 55%)`; } +/** + * Create a numbered draft PO through the server, which numbers POs one at + * a time (see `createPurchaseOrderWithNumber`). Throws with the server's + * message on failure. + */ +async function createNumberedPo( + orderId: string, + body: { supplier: string; total_amount: number; order_item: string[] }, +): Promise { + const token = getPocketBase().authStore.token; + const res = await fetch(`/api/v1/orders/${encodeURIComponent(orderId)}/purchase-orders`, { + method: "POST", + headers: { "content-type": "application/json", ...(token ? { Authorization: `Bearer ${token}` } : {}) }, + body: JSON.stringify(body), + }); + const data = (await res.json().catch(() => ({}))) as PurchaseOrder & { error?: string }; + if (!res.ok) throw new Error(data.error || `Creating the PO failed (HTTP ${res.status})`); + return data; +} + export default function ProcessOrderPage({ params }: { params: Promise<{ id: string }> }) { const { id } = use(params); const router = useRouter(); @@ -1260,7 +1279,6 @@ export default function ProcessOrderPage({ params }: { params: Promise<{ id: str setSendingSupplier(supplierId); const pb = getPocketBase(); try { - const poNumber = await generatePONumber(id); // `it.quantity` is now `number | null` (rows can save with no // qty yet). Treat null as 0 for the PO total — a line with // no qty contributes nothing. Qty uses the supplier-facing @@ -1275,24 +1293,19 @@ export default function ProcessOrderPage({ params }: { params: Promise<{ id: str numOrZero(it.cost_price) * numOrZero(it.order_qty ?? it.quantity), 0 ); - const po = await pb.collection("purchase_orders").create({ - po_number: poNumber, - order: id, + // Created as draft — the PDF route flips to "sent" on successful + // v1 generation. If the PDF fails, the PO stays as draft so the + // operator can re-trigger from the order detail page's + // "Regenerate PO" button. `order_item` back-links the lines so + // the PB admin row on the PO shows them. Numbered server-side, + // one PO at a time: two suppliers sent together used to get the + // same PO number (34 times by 2026-10-19). + const po = await createNumberedPo(id, { supplier: supplierId, - // Create as draft — the PDF route flips to "sent" on - // successful v1 generation. If the PDF fails, the PO - // stays as draft so the operator can re-trigger from the - // order detail page's "Regenerate PO" button. - status: "draft", total_amount: total, - // Back-link the order_items that belong to this PO. - // Mirrors `order_items.purchase_order` on the inverse - // direction so the PB admin UI on the PO row shows the - // related lines, and any future PO-side render (history, - // totals, supplier list) can read them without a separate - // getList call. order_item: nonInStock.map((it) => it.id), }); + const poNumber = po.po_number; // Link each item to the PO and set its status. Surface // per-item failures explicitly — a silent drop here is how @@ -1472,24 +1485,18 @@ export default function ProcessOrderPage({ params }: { params: Promise<{ id: str // in_stock=true. Total uses `cost_price * order_qty ?? quantity` // — matches the PDF route's subtotal so the PO record's // total_amount agrees with what the rendered document shows. - const poNumber = await generatePONumber(id); const total = inStock.reduce( (sum, it) => sum + numOrZero(it.cost_price) * numOrZero(it.order_qty ?? it.quantity), 0, ); - const po = await pb.collection("purchase_orders").create({ - po_number: poNumber, - order: id, + // Draft, numbered server-side — same contract as Send PO. + const po = await createNumberedPo(id, { supplier: supplierId, - // `draft` — the PDF route flips to "sent" on v1 success. - // If the PDF step fails, the PO stays in draft so the - // operator can re-trigger from the order detail page's - // "Regenerate PO" button. Same contract as Send PO. - status: "draft", total_amount: total, order_item: inStock.map((it) => it.id), }); + const poNumber = po.po_number; // ── Per-line: link to the new PO + flip to ready_invoice ── // The pre-existing `purchase_order` (if any) is captured in diff --git a/src/app/api/v1/orders/[id]/purchase-orders/route.test.ts b/src/app/api/v1/orders/[id]/purchase-orders/route.test.ts new file mode 100644 index 0000000..a8f5aad --- /dev/null +++ b/src/app/api/v1/orders/[id]/purchase-orders/route.test.ts @@ -0,0 +1,63 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { NextRequest } from "next/server"; + +const createPurchaseOrderWithNumber = vi.fn(); +vi.mock("@/lib/server/po-number", () => ({ + createPurchaseOrderWithNumber: (...a: unknown[]) => createPurchaseOrderWithNumber(...a), +})); +const getBrowserSessionUser = vi.fn(); +vi.mock("@/lib/server/browser-session", () => ({ + getBrowserSessionUser: (...a: unknown[]) => getBrowserSessionUser(...a), +})); + +import { POST } from "./route"; + +const call = (body: unknown) => + POST( + new NextRequest("http://localhost/api/v1/orders/ord-1/purchase-orders", { + method: "POST", + headers: { authorization: "Bearer tok", "content-type": "application/json" }, + body: JSON.stringify(body), + }), + { params: Promise.resolve({ id: "ord-1" }) }, + ); + +beforeEach(() => { + vi.clearAllMocks(); + getBrowserSessionUser.mockResolvedValue({ id: "u1", name: "Sam" }); +}); + +describe("POST /api/v1/orders/[id]/purchase-orders", () => { + it("401s without a session", async () => { + getBrowserSessionUser.mockRejectedValue(Object.assign(new Error("Not authenticated."), { status: 401 })); + expect((await call({ supplier: "s1" })).status).toBe(401); + expect(createPurchaseOrderWithNumber).not.toHaveBeenCalled(); + }); + + it.each([[{}], [{ supplier: "" }], [{ supplier: "s1", order_item: "x" }], [{ supplier: "s1", total_amount: "lots" }]])( + "400s for a bad body %#", + async (b) => { + expect((await call(b)).status).toBe(400); + expect(createPurchaseOrderWithNumber).not.toHaveBeenCalled(); + }, + ); + + it("creates a numbered draft PO with only the allowed fields", async () => { + createPurchaseOrderWithNumber.mockResolvedValue({ id: "po-1", po_number: "PO202610009LIF" }); + const res = await call({ + supplier: "s1", + total_amount: 120.5, + order_item: ["oi-1", "oi-2"], + po_number: "PO-FAKE", + status: "sent", + }); + expect(createPurchaseOrderWithNumber).toHaveBeenCalledWith("ord-1", { + supplier: "s1", + status: "draft", + total_amount: 120.5, + order_item: ["oi-1", "oi-2"], + }); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ id: "po-1", po_number: "PO202610009LIF" }); + }); +}); diff --git a/src/app/api/v1/orders/[id]/purchase-orders/route.ts b/src/app/api/v1/orders/[id]/purchase-orders/route.ts new file mode 100644 index 0000000..d2a87c6 --- /dev/null +++ b/src/app/api/v1/orders/[id]/purchase-orders/route.ts @@ -0,0 +1,55 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getBrowserSessionUser } from "@/lib/server/browser-session"; +import { createPurchaseOrderWithNumber } from "@/lib/server/po-number"; + +/** + * POST /api/v1/orders/[id]/purchase-orders + * + * Create a numbered draft PO for one supplier on an order (the Process + * page's Send PO / in-stock buttons). Numbering happens server-side, one + * PO at a time, so POs for several suppliers on the same order no longer + * share a number. Body: `{ supplier, total_amount?, order_item?: string[] }`. + * + * Auth: browser session (PB token forwarded in `Authorization`). + * Response: the created `purchase_orders` record. + */ +export const runtime = "nodejs"; +export const dynamic = "force-dynamic"; + +export async function POST(req: NextRequest, { params }: { params: Promise<{ id: string }> }) { + try { + await getBrowserSessionUser(req); + const { id } = await params; + const body = (await req.json().catch(() => ({}))) as { + supplier?: unknown; + total_amount?: unknown; + order_item?: unknown; + }; + const supplier = typeof body.supplier === "string" ? body.supplier.trim() : ""; + const total = body.total_amount ?? 0; + const items = body.order_item ?? []; + if ( + !supplier || + typeof total !== "number" || + !Number.isFinite(total) || + !Array.isArray(items) || + !items.every((i) => typeof i === "string" && i !== "") + ) { + return NextResponse.json( + { error: "supplier is required; total_amount must be a number and order_item a list of ids" }, + { status: 400 }, + ); + } + const po = await createPurchaseOrderWithNumber(id, { + supplier, + status: "draft", + total_amount: total, + order_item: items, + }); + return NextResponse.json(po); + } catch (err) { + const message = err instanceof Error ? err.message : "Creating the PO failed"; + const status = (err as { status?: number }).status || 500; + return NextResponse.json({ error: message }, { status }); + } +} diff --git a/src/lib/pb-helpers.ts b/src/lib/pb-helpers.ts index 17262f5..18b5873 100644 --- a/src/lib/pb-helpers.ts +++ b/src/lib/pb-helpers.ts @@ -1480,69 +1480,8 @@ export async function generateOrderNumber(): Promise { return `${prefix}${String(seq).padStart(3, "0")}`; } -/** - * Mirror of the server-side `companyPrefix` in `src/lib/server/po-number.ts`. - * Kept inline here because the client form also needs to render PO - * numbers as it types (e.g. for the email subject line previews). - */ -function companyPrefix(companyName: string | null | undefined): string { - if (!companyName) return "GEN"; - const cleaned = companyName.replace(/[^a-zA-Z0-9\s]/g, "").trim(); - const firstWord = cleaned.split(/\s+/)[0] ?? ""; - return firstWord.slice(0, 3).toUpperCase().padEnd(3, "X"); -} - -/** - * Client-side mirror of `src/lib/server/po-number.ts` `generatePONumber`. - * - * Format: `PO{YYYYMM}{NNN}{XXX}` (no dashes) where XXX is the - * 3-letter company prefix and NNN is the per-company per-month - * sequence. Mirrors the server variant field-by-field so a preview - * in the email composer matches the actual PO number the buyer - * tasks flow will write. - */ -export async function generatePONumber(orderId: string): Promise { - const pb = getPocketBase(); - const now = new Date(); - const year = now.getFullYear(); - const month = String(now.getMonth() + 1).padStart(2, "0"); - const dateKey = `${year}${month}`; - - // Resolve the order's company name. Fallback to "GEN" if the - // lookup fails or the order has no company — the format stays - // valid and the operator can rename in PB admin if needed. - let companyName = ""; - try { - const order = await pb.collection("orders").getOne(orderId, { - expand: "company", - }); - companyName = order.expand?.company?.company ?? ""; - } catch { - // Order not found or no company — fall through to GEN. - } - const prefix = companyPrefix(companyName); - const dateSegment = `PO${dateKey}`; - - const existing = await pb.collection("purchase_orders").getList(1, 500, { - filter: `po_number ~ "${dateSegment}"`, - sort: "-po_number", - }); - - // Post-filter on the suffix — the new format is fixed-length, so - // the company prefix is the last 3 chars of `po_number`. - let maxSeq = 0; - for (const row of existing.items) { - if (!row.po_number.endsWith(prefix)) continue; - const seqMatch = row.po_number.match(/(\d{3})(?=[A-Z]{3}$)/); - if (seqMatch) { - const n = parseInt(seqMatch[1], 10); - if (n > maxSeq) maxSeq = n; - } - } - - const seq = maxSeq + 1; - return `${dateSegment}${String(seq).padStart(3, "0")}${prefix}`; -} +// PO numbers are generated server-side only: see createPurchaseOrderWithNumber +// in src/lib/server/po-number.ts (the client copy raced, 2026-10-19). export function calculateTotals(items: { selling_price: number; quantity: number }[]) { const subtotal = Number(items.reduce((sum, item) => sum + item.selling_price * item.quantity, 0).toFixed(2)); diff --git a/src/lib/server/buyer-tasks-service.ts b/src/lib/server/buyer-tasks-service.ts index 01bcad0..955cabe 100644 --- a/src/lib/server/buyer-tasks-service.ts +++ b/src/lib/server/buyer-tasks-service.ts @@ -1,5 +1,5 @@ import { getAdminPocketBase } from "@/lib/server/pocketbase-admin"; -import { generatePONumber } from "@/lib/server/po-number"; +import { createPurchaseOrderWithNumber } from "@/lib/server/po-number"; import { generatePurchaseOrderPdf, PurchaseOrderPdfValidationError, @@ -459,19 +459,18 @@ export async function generatePoForGroup( // back-link the items to the PO so the PB admin row shows the // related lines without an extra getList call. Mirrors the // canonical /orders/[id]/process create payload. - const poNumber = await generatePONumber(orderId); const totalAmount = items.reduce( (sum, it) => sum + it.costPrice * it.quantity, 0, ); - const po = await admin.collection("purchase_orders").create({ - po_number: poNumber, - order: orderId, + // Numbered and created one at a time (see po-number.ts race note). + const po = await createPurchaseOrderWithNumber(orderId, { supplier: supplierId, status: "draft", total_amount: totalAmount, order_item: items.map((it) => it.id), }); + const poNumber = po.po_number; await Promise.all( items.map((item) => diff --git a/src/lib/server/po-number-create.test.ts b/src/lib/server/po-number-create.test.ts new file mode 100644 index 0000000..d0364a2 --- /dev/null +++ b/src/lib/server/po-number-create.test.ts @@ -0,0 +1,96 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// Fake PB with real async gaps, so concurrent calls interleave like they +// do in production (read highest number → … → create). +type Rec = Record & { id: string; po_number?: string }; +let pos: Rec[] = []; +let createCalls = 0; +let failNext: Error | null = null; +const tick = () => new Promise((r) => setTimeout(r, 5)); +const pb = { + collection: (name: string) => ({ + getOne: async () => { + await tick(); + return { id: "ord-1", expand: { company: { company: "Lifelong Resources" } } }; + }, + getList: async (_p: number, _n: number, opts: { filter?: string }) => { + await tick(); + const seg = /"(.+)"/.exec(opts.filter ?? "")?.[1] ?? ""; + // `hidden` rows were written by another server and aren't visible yet. + return { items: pos.filter((p) => !p.hidden && String(p.po_number).includes(seg)) }; + }, + create: async (data: Record) => { + if (name !== "purchase_orders") throw new Error(name); + createCalls += 1; + await tick(); + if (failNext) { + const e = failNext; + failNext = null; + throw e; + } + const clash = pos.find((p) => p.po_number === data.po_number); + if (clash) { + // By the time PB reports the clash, the other server's PO is committed and visible. + clash.hidden = false; + throw Object.assign(new Error("Failed to create record."), { + status: 400, + response: { data: { po_number: { code: "validation_not_unique", message: "Value must be unique." } } }, + }); + } + const rec = { id: `po-${pos.length + 1}`, ...data } as Rec; + pos.push(rec); + return rec; + }, + }), +}; +vi.mock("@/lib/server/pocketbase-admin", () => ({ getAdminPocketBase: async () => pb })); + +import { createPurchaseOrderWithNumber } from "./po-number"; + +const month = (() => { + const d = new Date(); + return `${d.getFullYear()}${String(d.getMonth() + 1).padStart(2, "0")}`; +})(); + +beforeEach(() => { + pos = []; + createCalls = 0; + failNext = null; +}); + +describe("createPurchaseOrderWithNumber", () => { + it("gives POs created at the same moment different numbers (one per supplier)", async () => { + const [a, b, c] = await Promise.all([ + createPurchaseOrderWithNumber("ord-1", { supplier: "barloworld", status: "draft" }), + createPurchaseOrderWithNumber("ord-1", { supplier: "ratcliffe", status: "draft" }), + createPurchaseOrderWithNumber("ord-1", { supplier: "nss", status: "draft" }), + ]); + expect([a.po_number, b.po_number, c.po_number]).toEqual([ + `PO${month}001LIF`, + `PO${month}002LIF`, + `PO${month}003LIF`, + ]); + expect(a).toMatchObject({ order: "ord-1", supplier: "barloworld", status: "draft" }); + }); + + it("takes the next number when the unique index rejects a clash", async () => { + pos.push({ id: "elsewhere", po_number: `PO${month}001LIF`, hidden: true }); + const po = await createPurchaseOrderWithNumber("ord-1", { supplier: "s" }); + expect(createCalls).toBe(2); + expect(po.po_number).toBe(`PO${month}002LIF`); + }); + + it("doesn't retry other errors, and a failure doesn't block the next PO", async () => { + failNext = new Error("boom"); + await expect(createPurchaseOrderWithNumber("ord-1", { supplier: "s" })).rejects.toThrow("boom"); + expect(createCalls).toBe(1); + await expect(createPurchaseOrderWithNumber("ord-1", { supplier: "s" })).resolves.toMatchObject({ + po_number: `PO${month}001LIF`, + }); + }); + + it("always numbers the PO itself, ignoring a po_number in the fields", async () => { + const po = await createPurchaseOrderWithNumber("ord-1", { supplier: "s", po_number: "PO-FAKE" }); + expect(po.po_number).toBe(`PO${month}001LIF`); + }); +}); diff --git a/src/lib/server/po-number.ts b/src/lib/server/po-number.ts index a8290b5..74a84db 100644 --- a/src/lib/server/po-number.ts +++ b/src/lib/server/po-number.ts @@ -42,11 +42,11 @@ export function companyPrefix(companyName: string | null | undefined): string { * `pb-helpers.ts`. Lives here per the data-access discipline rules * (server-side business logic stays out of pb-helpers.ts). * - * Race note: two concurrent calls could read the same highest seq - * and both write N+1, producing duplicate PO numbers. The call sites - * that need stronger guarantees should wrap this in a transaction or - * retry-on-conflict — for the buyer-tasks flow the user sees the - * duplicate and can regenerate, so we keep the simple version. + * Race note: two concurrent calls read the same highest seq and both + * return N+1. That happened 34 times by 2026-10-19 (one order, several + * suppliers, POs created within a second). Don't create a PO with this + * directly — use `createPurchaseOrderWithNumber`, which numbers and + * creates one PO at a time and retries on the unique index. */ export async function generatePONumber(orderId: string): Promise { const admin = await getAdminPocketBase(); @@ -96,4 +96,52 @@ export async function generatePONumber(orderId: string): Promise { const seq = maxSeq + 1; return `${dateSegment}${String(seq).padStart(3, "0")}${prefix}`; -} \ No newline at end of file +} + +// ─── Numbered create ──────────────────────────────────────────────── + +/** One PO numbered + created at a time in this server (see the race note). */ +let poQueue: Promise = Promise.resolve(); +function oneAtATime(fn: () => Promise): Promise { + const run = poQueue.then(fn, fn); + poQueue = run.catch(() => undefined); + return run; +} + +const MAX_ATTEMPTS = 5; + +/** PB's answer when the unique index on po_number rejects a create. */ +function isPoNumberClash(err: unknown): boolean { + const e = err as { response?: { data?: Record }; data?: { data?: Record } }; + const data = e?.response?.data ?? e?.data?.data; + return data?.po_number?.code === "validation_not_unique"; +} + +/** + * Number a new customer PO and create it — the only way POs should be + * created. Calls run one after another, so the next call reads the + * number this one just wrote; if another server got there first, the + * unique index on `po_number` rejects the create and we take the next + * number (up to 5 tries). Any `po_number` in `fields` is ignored. + */ +export async function createPurchaseOrderWithNumber( + orderId: string, + fields: Record, +): Promise & { id: string; po_number: string }> { + return oneAtATime(async () => { + const admin = await getAdminPocketBase(); + for (let attempt = 1; ; attempt += 1) { + const poNumber = await generatePONumber(orderId); + try { + return await admin.collection("purchase_orders").create & { id: string; po_number: string }>({ + ...fields, + order: orderId, + po_number: poNumber, + }); + } catch (err) { + if (!isPoNumberClash(err) || attempt >= MAX_ATTEMPTS) throw err; + } + } + }); +} +