feat(collection-confirm): improve idempotency handling in stock confirmation

- Updated the `confirmStockRows` function to handle cases where a stock line is already fully received, returning a success response without throwing an error. This change prevents the confirm-stock button from becoming inoperable due to previous confirmations.
- Enhanced tests to verify the new behavior, ensuring that full-quantity rows are omitted from updates and partial rows are processed correctly.
- Added documentation to clarify the idempotency behavior and its implications for the stock confirmation process.
This commit is contained in:
Dave 2026-08-17 08:19:38 +02:00
parent fc55613070
commit 9d80d52c17
2 changed files with 61 additions and 5 deletions

View file

@ -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",

View file

@ -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<ReturnType<typeof getAdminPocketBase>>,
@ -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(