From d4cdde7384897300a3a17d22b64d75dd00e1beec Mon Sep 17 00:00:00 2001 From: Dave Date: Sat, 17 Oct 2026 10:16:21 +0200 Subject: [PATCH] fix(collection-ai): Add to POS never duplicates SKU-less rows, failures are per row I5: a customer row without a SKU returns needs_sku and creates nothing; the Add to POS button is disabled for it ("Set a SKU first"). I6: a createStockItem failure is caught per row (status failed + error), the route still 200s, and the page toasts "N added, M failed, K need a SKU". Co-Authored-By: Claude Opus 5.5 --- .../(app)/collection-confirm/page-ai.test.tsx | 25 ++++++- src/app/(app)/collection-confirm/page.tsx | 7 +- .../add-to-pos/route.test.ts | 11 +++ .../v1/collection-confirm/add-to-pos/route.ts | 5 +- .../items-table-ai.test.tsx | 12 +++ .../collection-confirm/items-table.tsx | 4 +- src/lib/collection-ai-card.test.ts | 19 ++++- src/lib/collection-ai-card.ts | 17 +++++ .../server/collection-confirm-service.test.ts | 29 ++++++++ src/lib/server/collection-confirm-service.ts | 74 ++++++++++++------- 10 files changed, 167 insertions(+), 36 deletions(-) diff --git a/src/app/(app)/collection-confirm/page-ai.test.tsx b/src/app/(app)/collection-confirm/page-ai.test.tsx index d478133..856ed5a 100644 --- a/src/app/(app)/collection-confirm/page-ai.test.tsx +++ b/src/app/(app)/collection-confirm/page-ai.test.tsx @@ -6,6 +6,14 @@ import Page from "./page"; import { makeAiRow } from "@/components/collection-confirm/ai-fixtures"; import type { ConfirmRow } from "@/lib/server/collection-confirm-service"; +const toastMock = vi.hoisted(() => ({ + success: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + info: vi.fn(), +})); +vi.mock("sonner", () => ({ toast: toastMock })); + vi.mock("@/lib/pocketbase", () => ({ getPocketBase: () => ({ authStore: { token: "tok-123" } }), })); @@ -43,6 +51,8 @@ const rows: ConfirmRow[] = [ aiRow("row-c", { in_pos: null }), ]; +let addResults: unknown[] = [{ row_id: "row-a", status: "created", stock_item_id: "s" }]; + function setupFetch(opts: { confirmOk: boolean }) { return vi.spyOn(window, "fetch").mockImplementation(async (input) => { const url = fetchUrl(input); @@ -53,7 +63,7 @@ function setupFetch(opts: { confirmOk: boolean }) { : jsonResponse({ error: "boom" }, { ok: false, status: 500 }); } if (url === "/api/v1/collection-confirm/add-to-pos") { - return jsonResponse({ results: [{ row_id: "row-a", status: "created", stock_item_id: "s" }] }); + return jsonResponse({ results: addResults }); } if (url === "/api/v1/collection-confirm/ai/check") { return jsonResponse({ files_read: 1, lines_checked: 3, skus_filled: 0, error: null }); @@ -80,6 +90,19 @@ const addCalls = (spy: ReturnType) => describe("CollectionConfirmPage AI handlers", () => { beforeEach(() => { vi.restoreAllMocks(); + for (const fn of Object.values(toastMock)) fn.mockClear(); + addResults = [{ row_id: "row-a", status: "created", stock_item_id: "s" }]; + }); + + it("toasts added / failed / need-a-SKU counts after Add to POS", async () => { + addResults = [ + { row_id: "row-a", status: "created", stock_item_id: "s" }, + { row_id: "row-c", status: "failed", stock_item_id: null, error: "boom" }, + { row_id: "row-x", status: "needs_sku", stock_item_id: null }, + ]; + setupFetch({ confirmOk: true }); + await openConfirmAndAdd(); + await waitFor(() => expect(toastMock.warning).toHaveBeenCalledWith("1 added, 1 failed, 1 needs a SKU")); }); it("does not add to the POS when the confirm fails", async () => { diff --git a/src/app/(app)/collection-confirm/page.tsx b/src/app/(app)/collection-confirm/page.tsx index 0490f30..7654511 100644 --- a/src/app/(app)/collection-confirm/page.tsx +++ b/src/app/(app)/collection-confirm/page.tsx @@ -10,7 +10,7 @@ import { ConfirmStockDialog } from "@/components/collection-confirm/confirm-stoc import { InvoiceViewer } from "@/components/collection-confirm/invoice-viewer"; import { PoCard } from "@/components/collection-confirm/po-card"; import type { Attachment } from "@/components/collection-confirm/photo-strip"; -import { aiInvoiceDefaults } from "@/lib/collection-ai-card"; +import { aiInvoiceDefaults, posResultToast } from "@/lib/collection-ai-card"; import { getClientIdentity } from "@/lib/dev-identity"; import { getPocketBase } from "@/lib/pocketbase"; import type { ConfirmRow } from "@/lib/server/collection-confirm-service"; @@ -495,8 +495,9 @@ export default function CollectionConfirmPage() { if (!response.ok) { throw new Error(data.error || `Failed to add to the POS (HTTP ${response.status})`); } - const created = (data.results ?? []).filter((r) => r.status === "created").length; - toast.success(`Added ${created} item${created === 1 ? "" : "s"} to the POS`); + const summary = posResultToast(data.results ?? []); + if (summary.level === "success") toast.success(summary.message); + else toast.warning(summary.message); } catch (err) { toast.error(err instanceof Error ? err.message : String(err)); } diff --git a/src/app/api/v1/collection-confirm/add-to-pos/route.test.ts b/src/app/api/v1/collection-confirm/add-to-pos/route.test.ts index 21311c6..f2d69a0 100644 --- a/src/app/api/v1/collection-confirm/add-to-pos/route.test.ts +++ b/src/app/api/v1/collection-confirm/add-to-pos/route.test.ts @@ -56,6 +56,17 @@ describe("POST /api/v1/collection-confirm/add-to-pos", () => { expect(await res.json()).toEqual({ results }); }); + it("200s with per-row failures and needs_sku results", async () => { + const results = [ + { row_id: "a", status: "failed", stock_item_id: null, error: "boom" }, + { row_id: "b", status: "needs_sku", stock_item_id: null }, + ]; + addRowsToPos.mockResolvedValue(results); + const res = await POST(makeReq({ row_ids: ["a", "b"] })); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ results }); + }); + it("500s when the service throws", async () => { addRowsToPos.mockRejectedValue(new Error("boom")); const res = await POST(makeReq({ row_ids: ["a"] })); diff --git a/src/app/api/v1/collection-confirm/add-to-pos/route.ts b/src/app/api/v1/collection-confirm/add-to-pos/route.ts index 451612c..d3f96f5 100644 --- a/src/app/api/v1/collection-confirm/add-to-pos/route.ts +++ b/src/app/api/v1/collection-confirm/add-to-pos/route.ts @@ -10,8 +10,9 @@ import { addRowsToPos } from "@/lib/server/collection-confirm-service"; * * Auth: browser session (PB token forwarded in `Authorization`). * - * Response: `{ results: [{ row_id, status, stock_item_id }] }` with status - * `created` | `already_in_pos` | `not_found`. + * Response (200 even when some rows fail): `{ results: [{ row_id, status, + * stock_item_id, error? }] }` with status `created` | `already_in_pos` | + * `not_found` | `needs_sku` (no SKU, nothing created) | `failed` (+ `error`). */ export const runtime = "nodejs"; export const dynamic = "force-dynamic"; diff --git a/src/components/collection-confirm/items-table-ai.test.tsx b/src/components/collection-confirm/items-table-ai.test.tsx index 0439df2..460309e 100644 --- a/src/components/collection-confirm/items-table-ai.test.tsx +++ b/src/components/collection-confirm/items-table-ai.test.tsx @@ -120,6 +120,18 @@ describe("ItemsTable AI layer", () => { expect(onAdd).toHaveBeenCalledWith("row-1"); }); + it("disables Add to POS on a row without a SKU (Set a SKU first)", async () => { + const onAdd = vi.fn(); + render( + , + ); + const button = screen.getByRole("button", { name: /add to pos/i }); + expect(button).toBeDisabled(); + expect(button).toHaveAttribute("title", "Set a SKU first"); + await userEvent.click(button); + expect(onAdd).not.toHaveBeenCalled(); + }); + it("renders no tick icons for a row without ai_check inside an AI table", () => { render( onAddToPos(row.id)} - className="rounded border px-1.5 py-0.5 text-xs font-medium hover:bg-accent" + disabled={!effectiveSku(row).trim()} + title={effectiveSku(row).trim() ? undefined : "Set a SKU first"} + className="rounded border px-1.5 py-0.5 text-xs font-medium hover:bg-accent disabled:cursor-not-allowed disabled:opacity-50" data-testid={`add-to-pos-${row.id}`} > Add to POS diff --git a/src/lib/collection-ai-card.test.ts b/src/lib/collection-ai-card.test.ts index 3a63128..afe0763 100644 --- a/src/lib/collection-ai-card.test.ts +++ b/src/lib/collection-ai-card.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { makeAiRow, makeCheck } from "@/components/collection-confirm/ai-fixtures"; -import { aiInvoiceDefaults, cardDocument, cardFindings } from "./collection-ai-card"; +import { aiInvoiceDefaults, cardDocument, cardFindings, posResultToast } from "./collection-ai-card"; const doc = (read_id: string, findings: string[], doc_date = "2026-10-16") => ({ read_id, @@ -46,6 +46,23 @@ describe("collection-ai-card", () => { ]); }); + it("summarises Add to POS results for the toast", () => { + const r = (status: string) => ({ status }); + expect(posResultToast([r("created"), r("created")])).toEqual({ + level: "success", + message: "Added 2 items to the POS", + }); + expect(posResultToast([r("created")])).toEqual({ level: "success", message: "Added 1 item to the POS" }); + expect(posResultToast([r("created"), r("failed"), r("failed"), r("needs_sku")])).toEqual({ + level: "warning", + message: "1 added, 2 failed, 1 needs a SKU", + }); + expect(posResultToast([r("needs_sku"), r("needs_sku")])).toEqual({ + level: "warning", + message: "0 added, 2 need a SKU", + }); + }); + it("picks the newest document for the chip", () => { const rows = [ makeAiRow({ id: "1", ai_doc: doc("r1", [], "2026-10-10") }), diff --git a/src/lib/collection-ai-card.ts b/src/lib/collection-ai-card.ts index 8a92f3c..da9a1ac 100644 --- a/src/lib/collection-ai-card.ts +++ b/src/lib/collection-ai-card.ts @@ -54,6 +54,23 @@ export function aiInvoiceDefaults( return undefined; } +/** Toast for an Add to POS response: plain success, or the counts when any row didn't go in. */ +export function posResultToast( + results: Array<{ status: string }>, +): { level: "success" | "warning"; message: string } { + const count = (status: string) => results.filter((r) => r.status === status).length; + const created = count("created"); + const failed = count("failed"); + const needsSku = count("needs_sku"); + if (failed === 0 && needsSku === 0) { + return { level: "success", message: `Added ${created} item${created === 1 ? "" : "s"} to the POS` }; + } + const parts = [`${created} added`]; + if (failed > 0) parts.push(`${failed} failed`); + if (needsSku > 0) parts.push(`${needsSku} need${needsSku === 1 ? "s" : ""} a SKU`); + return { level: "warning", message: parts.join(", ") }; +} + export function docTypeLabel(type: CollectionDocType): string { if (type === "tax_invoice") return "Tax invoice"; if (type === "delivery_note") return "Delivery note"; diff --git a/src/lib/server/collection-confirm-service.test.ts b/src/lib/server/collection-confirm-service.test.ts index 9700eff..8ca6207 100644 --- a/src/lib/server/collection-confirm-service.test.ts +++ b/src/lib/server/collection-confirm-service.test.ts @@ -1504,6 +1504,35 @@ describe("addRowsToPos", () => { expect(createStockItem).not.toHaveBeenCalled(); }); + it("returns needs_sku for a customer row without a SKU and creates nothing", async () => { + vi.mocked(getAdminPocketBase).mockResolvedValue( + makeAdmin({ orderItems: [oi({ sku: " " })] }) as never, + ); + + const results = await addRowsToPos(["oi-1"]); + // A second click is still a no-op. + await addRowsToPos(["oi-1"]); + + expect(results).toEqual([{ row_id: "oi-1", status: "needs_sku", stock_item_id: null }]); + expect(createStockItem).not.toHaveBeenCalled(); + }); + + it("reports a failed create per row and carries on with the rest", async () => { + vi.mocked(createStockItem) + .mockRejectedValueOnce(new Error("PB said no")) + .mockResolvedValueOnce({ id: "si-2" } as never); + vi.mocked(getAdminPocketBase).mockResolvedValue( + makeAdmin({ orderItems: [oi(), oi({ id: "oi-2", sku: "OTHER-2" })] }) as never, + ); + + const results = await addRowsToPos(["oi-1", "oi-2"]); + + expect(results).toEqual([ + { row_id: "oi-1", status: "failed", stock_item_id: null, error: "PB said no" }, + { row_id: "oi-2", status: "created", stock_item_id: "si-2" }, + ]); + }); + it("returns already_in_pos for a stock-purchase row and not_found for an unknown id", async () => { vi.mocked(getAdminPocketBase).mockResolvedValue( makeAdmin({ stockPurchaseItems: [{ id: "spi-1" }] }) as never, diff --git a/src/lib/server/collection-confirm-service.ts b/src/lib/server/collection-confirm-service.ts index 7f8ce05..290909d 100644 --- a/src/lib/server/collection-confirm-service.ts +++ b/src/lib/server/collection-confirm-service.ts @@ -1188,8 +1188,10 @@ export async function clearCollectedRows( export type AddToPosResult = { row_id: string; - status: "created" | "already_in_pos" | "not_found"; + status: "created" | "already_in_pos" | "not_found" | "needs_sku" | "failed"; stock_item_id: string | null; + /** Only on `failed`: why the stock item couldn't be created. */ + error?: string; }; /** @@ -1197,8 +1199,11 @@ export type AddToPosResult = { * become a `stock_items` row keyed by the supplier's code (SKU). A row whose * normalised SKU already matches a stock item is left alone * (`already_in_pos`), as is any id that is a `stock_purchase_items` line - * (those are already POS items). Unknown ids come back `not_found`. Rows are - * handled one at a time so two rows sharing a code create only one item. + * (those are already POS items). Unknown ids come back `not_found`. A + * customer row without a SKU is `needs_sku` (nothing created, so repeated + * clicks never duplicate). A create that throws is `failed` with `error`; + * the other rows still run. Rows are handled one at a time so two rows + * sharing a code create only one item. */ export async function addRowsToPos(rowIds: string[]): Promise { const { createStockItem } = await import("@/lib/server/stock-items-service"); @@ -1250,36 +1255,49 @@ export async function addRowsToPos(rowIds: string[]): Promise const sku = (row.sku ?? "").trim(); const code = normaliseCode(sku); - if (code) { - let existingId = createdByCode.get(code) ?? null; - if (!existingId) { - const found = await pb.collection("stock_items").getFullList<{ id: string; sku?: string }>({ - filter: `sku ~ "${pbQuote(sku)}"`, - fields: "id,sku", - }); - existingId = found.find((s) => normaliseCode(s.sku) === code)?.id ?? null; - } - if (existingId) { - results.push({ row_id: id, status: "already_in_pos", stock_item_id: existingId }); - continue; - } + if (!code) { + // No supplier code: nothing to key the POS item on, and creating one + // per click would duplicate. + results.push({ row_id: id, status: "needs_sku", stock_item_id: null }); + continue; + } + let existingId = createdByCode.get(code) ?? null; + if (!existingId) { + const found = await pb.collection("stock_items").getFullList<{ id: string; sku?: string }>({ + filter: `sku ~ "${pbQuote(sku)}"`, + fields: "id,sku", + }); + existingId = found.find((s) => normaliseCode(s.sku) === code)?.id ?? null; + } + if (existingId) { + results.push({ row_id: id, status: "already_in_pos", stock_item_id: existingId }); + continue; } const description = (row.description ?? "").trim(); const supplier = row.expand?.purchase_order?.expand?.supplier?.id ?? row.supplier ?? undefined; const unit = row.ai_check?.doc_line?.unit?.trim(); - const item = await createStockItem({ - name: description, - description: (row.po_description ?? "").trim() || description, - sku, - ...(supplier ? { supplier } : {}), - ...(typeof row.cost_price === "number" && row.cost_price > 0 - ? { cost_price: row.cost_price } - : {}), - ...(unit ? { unit } : {}), - }); - if (code) createdByCode.set(code, item.id); - results.push({ row_id: id, status: "created", stock_item_id: item.id }); + try { + const item = await createStockItem({ + name: description, + description: (row.po_description ?? "").trim() || description, + sku, + ...(supplier ? { supplier } : {}), + ...(typeof row.cost_price === "number" && row.cost_price > 0 + ? { cost_price: row.cost_price } + : {}), + ...(unit ? { unit } : {}), + }); + createdByCode.set(code, item.id); + results.push({ row_id: id, status: "created", stock_item_id: item.id }); + } catch (err) { + results.push({ + row_id: id, + status: "failed", + stock_item_id: null, + error: err instanceof Error ? err.message : String(err), + }); + } } return results; }