From acaec3f5c7448649dc9f7770a8775ec7746f1d62 Mon Sep 17 00:00:00 2001 From: Dave Date: Thu, 5 Nov 2026 22:25:03 +0200 Subject: [PATCH] fix(stock): compensate the ledger row when the quantity write fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The write service puts the stock_ledger row first and then patches current_qty. When that second write failed, the ledger row was left behind: the ledger sat one change ahead of the quantity, and a retry wrote a second row for the same change, so SUM(change_qty) no longer rebuilt current_qty — the premise a drift-repair job rests on. The failed write is now compensated with a ledger row for the same item carrying the opposite change_qty, stamped with the same actor and noted as a reversal of the adjustment. If the compensation itself fails, the row is still reported as failed with a message that says the ledger could not be reversed. The failed row's message says when the ledger was compensated, so a retry is safe. The ledger-first ordering is unchanged — it is what makes a crash between the two writes leave the audit trail ahead of the cached total. --- .../server/stock-quantities-service.test.ts | 122 +++++++++++++++++- src/lib/server/stock-quantities-service.ts | 41 +++++- 2 files changed, 155 insertions(+), 8 deletions(-) diff --git a/src/lib/server/stock-quantities-service.test.ts b/src/lib/server/stock-quantities-service.test.ts index 26f0faa..f935039 100644 --- a/src/lib/server/stock-quantities-service.test.ts +++ b/src/lib/server/stock-quantities-service.test.ts @@ -312,11 +312,10 @@ describe("updateStockItemsQuantities — multi-row batch", () => { // The service writes the ledger row BEFORE the cache update // (audit-before-cache, same pattern as stock-surplus / // stock-booking-service), so the bad row's ledger entry lands - // before the stock_items update fails. Two ledger rows are - // the correct outcome — the second one is a "drift" that a - // future drift-repair job can rebuild current_qty from. - // Operators see the failure in the toast and can re-apply. - expect(ledgerCreates).toHaveLength(2); + // before the stock_items update fails. The failed write is then + // compensated with the opposite row, so the ledger stays level + // with the cached quantity — a retry cannot double-count it. + expect(ledgerCreates).toHaveLength(3); expect(ledgerCreates[0]).toMatchObject({ stock_item: "siok1aaaaaaaaaaa", change_qty: 5, @@ -325,6 +324,119 @@ describe("updateStockItemsQuantities — multi-row batch", () => { stock_item: "sibad2aaaaaaaaa", change_qty: 5, }); + expect(ledgerCreates[2]).toMatchObject({ + stock_item: "sibad2aaaaaaaaa", + change_qty: -5, + }); + // The failed row says the ledger was compensated, so the operator + // knows a retry is safe. + const failedRow = result.results[1]; + expect(failedRow.ok === false && failedRow.message).toContain("reversed"); + }); +}); + +describe("updateStockItemsQuantities — ledger compensation", () => { + it("reverses the ledger row when the quantity write fails", async () => { + const before = { id: "abcdefghijklmno", current_qty: 5 }; + const ledgerCreates: Array> = []; + const pb = { + collection: (name: string) => { + if (name === "stock_items") { + return { + getOne: vi.fn().mockResolvedValue(before), + update: vi.fn().mockRejectedValue(new Error("PB write failed")), + create: vi.fn(), + }; + } + if (name === "stock_ledger") { + return { + create: vi.fn().mockImplementation(async (row: Record) => { + ledgerCreates.push(row); + return { id: "ledger1", ...row }; + }), + }; + } + throw new Error(`unexpected ${name}`); + }, + }; + + const result = await updateStockItemsQuantities({ + pb: pb as never, + updates: [{ id: "abcdefghijklmno", currentQty: 10 }], + actor: { id: "u1", name: "Dave" }, + } as UpdateStockItemsQuantitiesInput); + + expect(result.summary.failed).toBe(1); + const row = result.results[0]; + expect(row.ok).toBe(false); + + // The original row (+5) and the compensation (-5). + expect(ledgerCreates).toHaveLength(2); + expect(ledgerCreates[0]).toMatchObject({ + stock_item: "abcdefghijklmno", + change_qty: 5, + created_by: "u1", + }); + expect(ledgerCreates[1]).toMatchObject({ + stock_item: "abcdefghijklmno", + change_qty: -5, + reason: "adjustment", + created_by: "u1", + }); + expect(ledgerCreates[1].notes).toContain("reversed"); + expect(ledgerCreates[1].notes).toContain("PB write failed"); + + // The ledger nets to zero, so SUM(change_qty) still rebuilds + // current_qty. + const net = ledgerCreates.reduce( + (sum, r) => sum + Number(r.change_qty), + 0, + ); + expect(net).toBe(0); + + // The message says the ledger was compensated — a retry is safe. + expect(row.ok === false && row.message).toContain("reversed"); + expect(row.ok === false && row.message).toContain("retrying is safe"); + }); + + it("still reports the failed row when the compensating write fails", async () => { + const before = { id: "abcdefghijklmno", current_qty: 5 }; + let creates = 0; + const pb = { + collection: (name: string) => { + if (name === "stock_items") { + return { + getOne: vi.fn().mockResolvedValue(before), + update: vi.fn().mockRejectedValue(new Error("PB write failed")), + create: vi.fn(), + }; + } + if (name === "stock_ledger") { + return { + create: vi.fn().mockImplementation(async () => { + creates += 1; + // The first row lands; the compensation does not. + if (creates > 1) throw new Error("PB write failed"); + return { id: "ledger1" }; + }), + }; + } + throw new Error(`unexpected ${name}`); + }, + }; + + const result = await updateStockItemsQuantities({ + pb: pb as never, + updates: [{ id: "abcdefghijklmno", currentQty: 10 }], + actor: { id: "u1", name: "Dave" }, + } as UpdateStockItemsQuantitiesInput); + + // The row is still reported as failed, and nothing threw. + expect(result.summary.failed).toBe(1); + const row = result.results[0]; + expect(row.ok).toBe(false); + expect(row.ok === false && row.message).toContain("could NOT be reversed"); + expect(creates).toBe(2); }); }); diff --git a/src/lib/server/stock-quantities-service.ts b/src/lib/server/stock-quantities-service.ts index a8cf136..31e85f1 100644 --- a/src/lib/server/stock-quantities-service.ts +++ b/src/lib/server/stock-quantities-service.ts @@ -36,8 +36,18 @@ import type { StockItem } from "@/lib/types"; * crash between the two writes leaves the audit trail ahead of * the cached total — the same pattern `stock-surplus.ts` and * `stock-booking-service.ts` use. A future drift-repair job can - * rebuild `current_qty` from `SUM(change_qty)` if the second - * write ever fails. + * rebuild `current_qty` from `SUM(change_qty)`. + * + * When the SECOND write fails — the ledger row landed but the quantity + * did not — we write a compensating ledger row with the opposite + * `change_qty` before reporting the failure. Without it the ledger + * would sit one change ahead of the cached quantity, and a retry would + * write a second row for the same change and double-count it. The + * compensating row uses `reason: "adjustment"` (the only fitting + * value in STOCK_REASON_VALUES — a new one would need a PocketBase + * schema change) and is stamped with the same actor. If the + * compensation itself fails we still report the row as failed, with a + * message that says the ledger could not be reversed. */ export type StockQuantityUpdate = { @@ -251,8 +261,12 @@ export async function updateStockItemsQuantities( // (idempotent re-apply) — emitting a zero-delta ledger row // would pollute the audit log with no-ops. let ledgerId: string | undefined; + // The change the ledger row recorded. Kept here so the catch + // below can reverse it when the quantity write fails. + let ledgerDelta = 0; if (currentMoved && requestedCurrent !== undefined) { const delta = requestedCurrent - beforeCurrent; + ledgerDelta = delta; try { const created = await input.pb.collection("stock_ledger").create({ stock_item: update.id, @@ -291,11 +305,32 @@ export async function updateStockItemsQuantities( .update(update.id, patch)) as StockItem; } catch (err) { const message = err instanceof Error ? err.message : String(err); + // The ledger row landed but the quantity did not. Reverse the + // ledger row so SUM(change_qty) still rebuilds current_qty — + // a blind retry would otherwise write a second row for the + // same change and double-count it. + let compensation = ""; + if (ledgerId) { + try { + await input.pb.collection("stock_ledger").create({ + stock_item: update.id, + change_qty: -ledgerDelta, + reason: "adjustment", + reference_type: "manual", + notes: `${input.actor.name} reversed the qty ${beforeCurrent} → ${requestedCurrent} ${noteContext} adjustment because the quantity write failed: ${message}`, + created_by: input.actor.id, + }); + compensation = + " The stock_ledger row was reversed, so the quantity did not change — retrying is safe."; + } catch (compensationErr) { + compensation = ` The stock_ledger row could NOT be reversed: ${compensationErr instanceof Error ? compensationErr.message : String(compensationErr)} — the ledger is ahead of the quantity.`; + } + } results.push({ id: update.id, ok: false, reason: "pb_error", - message: `Failed to update: ${message}`, + message: `Failed to update: ${message}.${compensation}`, }); continue; }