diff --git a/src/lib/server/collection-confirm-service.test.ts b/src/lib/server/collection-confirm-service.test.ts index 1fea614..ea2a995 100644 --- a/src/lib/server/collection-confirm-service.test.ts +++ b/src/lib/server/collection-confirm-service.test.ts @@ -940,7 +940,13 @@ describe("confirmStockRows", () => { expect(lineUpdate).not.toHaveBeenCalled(); }); - it("throws and skips the row when quantity is already fully received", async () => { + it("is idempotent: a line already at full quantity_received is omitted from updatedIds (no throw)", async () => { + // Old behaviour threw a plain Error here → 500. Once the qty + // pencil landed, snapping quantity_received to the full qty + // could leave the line "stuck" — the confirm-stock button + // would 500 forever. Now it's a no-op success: the row is + // dropped from updatedIds so the route returns 200 with the + // remaining rows only. const line = { id: "spi-1", stock_item: "st-1", @@ -953,13 +959,45 @@ describe("confirmStockRows", () => { }); vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never); - await expect( - confirmStockRows(["spi-1"], { actor: { id: "u-1", name: "sj" } }), - ).rejects.toThrow(/already fully confirmed/i); + const result = await confirmStockRows(["spi-1"], { + actor: { id: "u-1", name: "sj" }, + }); + expect(result).toEqual([]); expect(lineUpdate).not.toHaveBeenCalled(); }); + it("processes a mixed batch: full-qty rows are skipped, partial rows are advanced", async () => { + const lines = { + "spi-1": { + id: "spi-1", + stock_item: "st-1", + quantity: 10, + quantity_received: 10, // already full + purchase: "sp-1", + }, + "spi-2": { + id: "spi-2", + stock_item: "st-2", + quantity: 4, + quantity_received: 2, // partial + purchase: "sp-1", + }, + }; + const { admin, lineUpdate } = makeConfirmAdmin({ + lineById: lines, + }); + vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never); + + const result = await confirmStockRows(["spi-1", "spi-2"], { + actor: { id: "u-1", name: "sj" }, + }); + + expect(result).toEqual(["spi-2"]); + expect(lineUpdate).toHaveBeenCalledTimes(1); + expect(lineUpdate).toHaveBeenCalledWith("spi-2", { quantity_received: 4 }); + }); + it("throws and skips the row when the parent purchase id is missing", async () => { const line = { id: "spi-1", diff --git a/src/lib/server/collection-confirm-service.ts b/src/lib/server/collection-confirm-service.ts index 25974d4..bc81c46 100644 --- a/src/lib/server/collection-confirm-service.ts +++ b/src/lib/server/collection-confirm-service.ts @@ -744,6 +744,20 @@ export async function confirmStockRows( * * `pb` is the admin PocketBase client; the function reads / updates * the relevant collections directly. + * + * ## Idempotency + * + * If the line is already fully received (`quantity_received >= + * quantity`), `confirmStockRowForId` returns `false` so the caller + * drops it from the `updatedIds` list. The route surfaces this as a + * 200 with the row omitted from `updatedIds` — the warehouse-side + * "Confirm collection" button is the operator's explicit close-of- + * day action; a previous confirm (or the qty pencil that snapped + * `quantity_received` up to the full qty) means there's nothing + * left to advance. The previous behaviour threw a plain Error → + * 500, which was a foot-gun once the qty pencil arrived: setting + * the qty to the full value would leave the confirm-stock button + * inoperable for that card. */ async function confirmStockRowForId( pb: Awaited>, @@ -767,7 +781,11 @@ async function confirmStockRowForId( } const alreadyReceived = Number(line.quantity_received ?? 0); if (alreadyReceived >= qty) { - throw new Error(`Stock line ${id} is already fully confirmed.`); + // Idempotent no-op: the line is already at full received. The + // qty pencil is one way to land here (operator snapped the qty + // up to the full qty before clicking confirm); a previous + // confirm-stock is another. Either way there's nothing to do. + return false; } if (!line.purchase) { throw new Error(