David/WFDASH-WEBAPP
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(stock): a catalogue mirror error no longer aborts the batch

Browse source 
The price_list mirror could reject — a bad filter, a 400, a network
blip — and the rejection escaped setStockQuantitiesBulk. The
quantities had already been written and their ledger rows were in
place, so the caller got an exception instead of a summary: the
remaining lines were never reported, and a retry re-applied the
quantities and wrote a second set of ledger rows, double-counting the
audit trail.

The mirror is now guarded. The quantity landed, so the row stays
"applied" — reporting it as "failed" would say the quantity did not
change, which is false. The row carries price_list_rows_touched: 0
and a message saying the catalogue snapshot failed and why. The batch
always returns its summary and results.

Also: "ok" now means what it says — summary.failed === 0 — instead of
always being true. The price-list page is unbounded by design at this
data volume (about 2,900 catalogue rows over about 2,700 SKUs), and
the comment now says so, with the caveat that the filter's raw-SKU
LIKE clause treats % and _ as wildcards, so the page is sized by
substring noise rather than by the SKU's own rows.
This commit is contained in:
Dave 2026-11-05 22:28:18 +02:00
parent acaec3f5c7
commit 7dd70577a7
2 changed files with 107 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

85
src/lib/server/stock-quantities-bulk-service.test.ts
View file

@ -35,7 +35,7 @@ const STOCK_ITEMS = [
function stubPb(
priceListRows: Array<Record<string, unknown>> = [],
opts: { failUpdateIds?: string[] } = {},
opts: { failUpdateIds?: string[]; failPriceListForSku?: string } = {},
) {
const calls: string[] = [];
const ledgerCreates: Array<Record<string, unknown>> = [];
@ -82,10 +82,26 @@ function stubPb(
}
if (name === "price_list") {
return {
getList: vi.fn().mockResolvedValue({
items: priceListRows,
totalItems: priceListRows.length,
}),
getList: vi.fn().mockImplementation(
async (
_page: number,
_perPage: number,
query?: Record<string, unknown>,
) => {
// One SKU's catalogue query blows up — a rejected
// filter, a 400, a network blip.
if (
opts.failPriceListForSku &&
String(query?.filter ?? "").includes(opts.failPriceListForSku)
) {
throw new Error("400: invalid filter");
}
return {
items: priceListRows,
totalItems: priceListRows.length,
};
},
),
update: vi.fn().mockImplementation(async (id: string, patch: Record<string, unknown>) => {
priceListUpdates.push({ id, patch });
return { id, ...patch };
@ -131,6 +147,7 @@ describe("setStockQuantitiesBulk — applying", () => {
actor: { id: "u1", name: "Dave" },
});
expect(res.summary.applied).toBe(1);
expect(res.ok).toBe(true);
expect(calls[0]).toBe("stock_ledger.create");
expect(calls[1]).toBe("stock_items.update");
expect(ledgerCreates[0].change_qty).toBe(38);
@ -207,7 +224,7 @@ describe("setStockQuantitiesBulk — applying", () => {
});
it("keeps the rest of the batch going when one write fails", async () => {
const { pb } = stubPb([], { failUpdateIds: ["id-def"] });
const { pb, ledgerCreates } = stubPb([], { failUpdateIds: ["id-def"] });
const res = await setStockQuantitiesBulk({
pb,
lines: [
@ -228,6 +245,20 @@ describe("setStockQuantitiesBulk — applying", () => {
expect(res.summary.unmatched).toBe(0);
expect(res.results[1].status).toBe("failed");
expect(res.results[1].message).toContain("Failed to update");
// The ledger row for the failed line is compensated, so the two
// rows net to zero — the orphan cannot double-count on a retry.
const failedLineRows = ledgerCreates.filter(
(r) => r.stock_item === "id-def",
);
expect(failedLineRows.map((r) => r.change_qty)).toEqual([4, -4]);
expect(
failedLineRows.reduce((sum, r) => sum + Number(r.change_qty), 0),
).toBe(0);
// The failed row says the ledger was compensated, so a retry is
// safe.
expect(res.results[1].message).toContain("reversed");
// A failed write makes the batch a partial success.
expect(res.ok).toBe(false);
// Every line is counted once, in exactly one bucket.
const { applied, unchanged, unmatched, ambiguous, failed } = res.summary;
expect(res.summary.total).toBe(
@ -293,6 +324,48 @@ describe("setStockQuantitiesBulk — summary", () => {
});
});
describe("setStockQuantitiesBulk — catalogue failure", () => {
it("keeps the batch going when the catalogue mirror fails", async () => {
const { pb, ledgerCreates } = stubPb(
[{ id: "pl1", sku: "M16-FLAT-WASHER", sku_normalised: "M16FLATWASHER" }],
{ failPriceListForSku: "M16-FLAT-WASHER" },
);
const res = await setStockQuantitiesBulk({
pb,
lines: [
{ sku: "M16-FLAT-WASHER", qty: 50 },
{ sku: "BOLT-M6-40", qty: 9 },
],
mode: "set",
dryRun: false,
actor: { id: "u1", name: "Dave" },
});
// The call returns normally — no rejection, no lost rows.
expect(res.results).toHaveLength(2);
expect(res.summary.applied).toBe(2);
expect(res.summary.failed).toBe(0);
expect(res.ok).toBe(true);
// The row whose mirror failed still applied — the quantity DID
// move — and the row says why the catalogue did not follow.
expect(res.results[0].status).toBe("applied");
expect(res.results[0].new_qty).toBe(50);
expect(res.results[0].price_list_rows_touched).toBe(0);
expect(res.results[0].message).toContain(
"price-list catalogue snapshot failed",
);
expect(res.results[0].message).toContain("invalid filter");
// The next line was still processed.
expect(res.results[1].status).toBe("applied");
expect(res.results[1].new_qty).toBe(9);
// The quantities really landed.
expect(ledgerCreates).toHaveLength(2);
});
});
describe("setStockQuantitiesBulk — price_list mirror", () => {
it("mirrors the new quantity onto every catalogue row for that SKU", async () => {
const { pb, priceListUpdates } = stubPb([

36
src/lib/server/stock-quantities-bulk-service.ts
View file

@ -193,8 +193,13 @@ async function mirrorPriceListQty(
const norm = normaliseSku(rawSku);
if (!norm) return 0;
// One page is enough at the catalogue's size: a SKU has one
// row per supplier, so a handful at most.
// One page is unbounded by design at this data volume: the
// catalogue holds about 2,900 rows over about 2,700 SKUs, so a SKU
// has one row per supplier and a handful at most.
//
// Caveat: the filter carries a raw-SKU LIKE clause, and PocketBase
// treats `%` and `_` inside a LIKE value as wildcards — so the page
// is sized by substring noise rather than by the SKU's own rows.
const page = await pb
.collection("price_list")
.getList<PriceListRow>(1, 200, {
@ -404,12 +409,22 @@ export async function setStockQuantitiesBulk(
summary.applied += 1;
let priceListRowsTouched = 0;
let message: string | undefined;
if (line.item.sku) {
priceListRowsTouched = await mirrorPriceListQty(
pb,
line.item.sku,
p.target,
);
try {
priceListRowsTouched = await mirrorPriceListQty(
pb,
line.item.sku,
p.target,
);
} catch (err) {
// The quantity is already written and its ledger row is in
// place. The catalogue row is a snapshot of our quantity, not
// the quantity itself, so a catalogue error must not fail the
// row — and must not abort the batch. The operator still gets
// the whole summary, and the reason is on the row.
message = `The quantity was applied but the price-list catalogue snapshot failed: ${err instanceof Error ? err.message : String(err)}`;
}
}
results.push({
@ -422,11 +437,16 @@ export async function setStockQuantitiesBulk(
new_qty: p.target,
delta,
price_list_rows_touched: priceListRowsTouched,
...(message ? { message } : {}),
});
}
return {
ok: true,
// A failed write makes the batch a partial success: the rows that
// applied are applied, and the rows that failed are on their own.
// Read `summary` and `results[]` — this flag only says whether
// anything failed.
ok: summary.failed === 0,
mode,
dry_run: dryRun,
summary,