fix(stock): compensate the ledger row when the quantity write fails
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.
This commit is contained in:
parent
4362eb4201
commit
acaec3f5c7
2 changed files with 155 additions and 8 deletions
|
|
@ -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<Record<string, unknown>> = [];
|
||||
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<string, unknown>) => {
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue