From bc138bcae7b647cabdb7f5ed5014aaa94dc9a52d Mon Sep 17 00:00:00 2001 From: DEls Date: Mon, 26 Oct 2026 19:27:29 +0200 Subject: [PATCH] fix(dn): over-PO quantities need an explicit accept, never from the assistant Co-Authored-By: MiniMax (Pinchy) --- .../notes/[noteId]/lines/route.test.ts | 12 ++++ src/lib/inbox-three/assistant.test.ts | 20 +++++- src/lib/inbox-three/assistant.ts | 6 ++ src/lib/inbox-three/line-rows.ts | 62 +++++++++++++++---- src/lib/inbox-three/save-lines.test.ts | 33 +++++++++- src/lib/inbox-three/save-lines.ts | 22 +++++-- 6 files changed, 137 insertions(+), 18 deletions(-) diff --git a/src/app/api/inbox-three/[batchId]/notes/[noteId]/lines/route.test.ts b/src/app/api/inbox-three/[batchId]/notes/[noteId]/lines/route.test.ts index 9a05cad..6f59bce 100644 --- a/src/app/api/inbox-three/[batchId]/notes/[noteId]/lines/route.test.ts +++ b/src/app/api/inbox-three/[batchId]/notes/[noteId]/lines/route.test.ts @@ -62,6 +62,18 @@ it("writes nothing when one edit in the batch fails", async () => { expect(m.batch).not.toHaveBeenCalled(); }); +it("books a quantity over the order only when the office ticks it", async () => { + // The order has 70 m of pipe left; 11 × 10 m is 110 m. + const over = { lines: [0], poItemId: "itemabcdefghijk", slipQty: 11, slipUnit: "ea", factor: 10, price: null, usePoRate: true, reason: "The site ordered extra pipe." }; + const refused = await POST(req({ edits: [over], snapshotHash: p.snapshotHash, decisionHash: decisionHash(p) }), params); + expect(refused.status).toBe(409); + expect((await refused.json()).detail).toContain('Line 1: 40 m more than the PO has left. Tick "accept over PO" or change the quantity.'); + expect(m.batch).not.toHaveBeenCalled(); + + const accepted = await POST(req({ edits: [{ ...over, acceptExcess: true }], snapshotHash: p.snapshotHash, decisionHash: decisionHash(p) }), params); + expect(accepted.status).toBe(200); +}); + it("never reaches for the posting function", async () => { const source = await import("node:fs/promises").then((fs) => fs.readFile(new URL("./route.ts", import.meta.url), "utf8")); expect(source).not.toContain("postLineWithinLease"); diff --git a/src/lib/inbox-three/assistant.test.ts b/src/lib/inbox-three/assistant.test.ts index de168d3..5c6f93a 100644 --- a/src/lib/inbox-three/assistant.test.ts +++ b/src/lib/inbox-three/assistant.test.ts @@ -119,7 +119,7 @@ describe("runLineAssistant", () => { expect(saveLineEdits).toHaveBeenCalledWith( expect.anything(), expect.anything(), - [expect.objectContaining({ lines: [2, 3], poItemId: KERB_ITEM, slipQty: 4, slipUnit: "roll", factor: 15, usePoRate: true })], + [expect.objectContaining({ lines: [2, 3], poItemId: KERB_ITEM, slipQty: 4, slipUnit: "roll", factor: 15, usePoRate: true, acceptExcess: false })], "operator", ); // The rows the office sees now carry the combined line. @@ -164,6 +164,18 @@ describe("runLineAssistant", () => { expect(rows[0].numbers).toEqual([1]); }); + it("never lets the model push a quantity over the purchase order", async () => { + const model = vi.fn(async () => ({ proposal: proposal([ + { lines: [3], poItemId: KERB_ITEM, slipQty: 4, slipUnit: "roll", factor: 30, price: null, reason: "a guess" }, + ]), model: "test-model" })); + + const result = await runLineAssistant(pb as never, BATCH, NOTE, "operator", "book 120 m of kerb", { model, images: async () => [{ mediaType: "image/jpeg", data: "" }] }); + + // The office has to say yes on the line; the assistant cannot. + expect(result.reply).toMatch(/accept over PO/i); + expect(state.mutations.filter((mutation) => mutation.collection === "dn_batch_notes")).toHaveLength(0); + }); + it("never reaches for the posting function", async () => { const source = await readFile(new URL("./assistant.ts", import.meta.url), "utf8"); expect(source).not.toContain("postLineWithinLease"); @@ -201,4 +213,10 @@ describe("LineAssistantProposalSchema", () => { it("keeps a proposal with no edits at all", () => { expect(LineAssistantProposalSchema.safeParse({ reply: "ok", question: null }).success).toBe(true); }); + + it("refuses a proposal that tries to accept an excess itself", () => { + expect(LineAssistantProposalSchema.safeParse({ reply: "ok", question: null, edits: [ + { lines: [1], poItemId: null, slipQty: 1, slipUnit: "ea", factor: 1, price: null, reason: "because", acceptExcess: true }, + ] }).success).toBe(false); + }); }); diff --git a/src/lib/inbox-three/assistant.ts b/src/lib/inbox-three/assistant.ts index 2c47c27..18f83e6 100644 --- a/src/lib/inbox-three/assistant.ts +++ b/src/lib/inbox-three/assistant.ts @@ -134,6 +134,9 @@ export async function assistantHistory(pb: PocketBase, batchId: string, noteId: function plainReason(error: unknown): string { const detail = error instanceof Error ? error.message : "The lines could not be changed."; if (/does not belong to this purchase order/i.test(detail)) return "That order line is not on this purchase order."; + // The office cannot tick a box on a line it cannot see: say where the + // decision has to be made. + if (/more than the PO has left/i.test(detail)) return detail.replace('Tick "accept over PO"', 'Tick "accept over PO" on that line'); return detail; } @@ -222,6 +225,9 @@ export async function runLineAssistant( price: edit.price, // No edited price means the order rate is used. usePoRate: edit.price === null, + // The assistant never accepts a quantity over the order. That is the + // office's decision, on the line, where the number is visible. + acceptExcess: false, reason: edit.reason, })); diff --git a/src/lib/inbox-three/line-rows.ts b/src/lib/inbox-three/line-rows.ts index 14fa922..dece48e 100644 --- a/src/lib/inbox-three/line-rows.ts +++ b/src/lib/inbox-three/line-rows.ts @@ -26,6 +26,8 @@ export interface LineEdit { /** The edited unit price, or null to keep the order rate. */ price: number | null; usePoRate: boolean; + /** A quantity over what the order has left. Only a person may accept it. */ + acceptExcess?: boolean; reason: string; } @@ -34,7 +36,7 @@ export interface LineRow { numbers: number[]; allocationIndex: number; slipText: string; slipQty: number | null; slipUnit: string | null; slipPrice: number | null; slipAmount: number | null; - poItemId: string | null; poUnit: string | null; poRate: number | null; ordered: number | null; priorReceived: number; + poItemId: string | null; poUnit: string | null; poRate: number | null; ordered: number | null; priorReceived: number; booked: number; qty: number | null; unit: string | null; factor: number | null; poQty: number | null; price: number | null; value: number | null; leftOnPo: number | null; flags: Array<"unit_differs" | "factor_missing" | "price_differs" | "over_po" | "guard_conflict" | "not_on_po">; @@ -61,6 +63,36 @@ function priorReceivedForItem(context: PreviewContext, poItemId: string | null): return context.priorItems.filter((row) => row.purchase_order_item === poItemId).reduce((sum, row) => sum + (finiteNumber(row.qty_received) ?? 0), 0); } +/** + * The order line's own numbers. + * + * An edit may put a slip line on another order line, and the numbers that + * decide whether a quantity is over the order belong to the line it is + * going on — not to the one it came from. + */ +export function poLineNumbers(context: PreviewContext, poItemId: string | null): Pick { + const item = context.poItems.find((candidate) => candidate.id === poItemId); + return { + poItemId: poItemId ?? null, + poUnit: item && typeof item.unit === "string" ? item.unit : null, + poRate: item ? (finiteNumber(item.rate) ?? null) : null, + ordered: item ? (finiteNumber(item.qty) ?? null) : null, + priorReceived: priorReceivedForItem(context, poItemId ?? null), + booked: bookedForItem(context, poItemId ?? null), + }; +} + +/** + * How much more than the order has left this quantity would book. + * + * The order line, what other notes have already delivered and what this note + * has already booked — the same three numbers the receipt itself checks. + */ +export function overPo(row: LineRow, qty: number | null): number { + if (qty === null || row.ordered === null) return 0; + return Math.max(0, round2(row.priorReceived + row.booked + qty - row.ordered)); +} + function buildRow( context: PreviewContext, preview: ProcessingPreview, @@ -72,12 +104,8 @@ function buildRow( .map((id) => context.lines.find((line) => line.sourceLineId === id)) .filter((line): line is PreviewLine => Boolean(line)); const first = source[0]; - const item = context.poItems.find((candidate) => candidate.id === allocation.purchaseOrderItemId); const poItemId = allocation.purchaseOrderItemId ?? null; - const poUnit = item && typeof item.unit === "string" ? item.unit : null; - const poRate = item ? (finiteNumber(item.rate) ?? null) : null; - const ordered = item ? (finiteNumber(item.qty) ?? null) : null; - const priorReceived = priorReceivedForItem(context, poItemId); + const { poUnit, poRate, ordered, priorReceived, booked } = poLineNumbers(context, poItemId); const slipQty = finiteNumber(first?.qty) ?? null; const slipUnit = first?.unit ?? null; @@ -130,7 +158,7 @@ function buildRow( allocationIndex, slipText: source.map((line) => line.description ?? "Unreadable line").join(" + "), slipQty, slipUnit, slipPrice, slipAmount, - poItemId, poUnit, poRate, ordered, priorReceived, + poItemId, poUnit, poRate, ordered, priorReceived, booked, qty, unit, factor, poQty, price: slipPrice, value, leftOnPo, flags, @@ -151,9 +179,13 @@ export function buildLineRows(context: PreviewContext, preview: ProcessingPrevie * * The quantity booked is the slip's quantity times the factor, in the * order's unit. A factor of one with a different unit means the order's - * unit label is wrong, and the slip's own unit is booked instead. The - * reason is never empty: the arithmetic is written down so the next person - * can see how the number was reached. + * unit label is wrong, and the slip's own unit is booked instead. + * + * A quantity over what the order has left is only booked when the edit says + * so, and then the reason has to show that it was accepted — the office + * cannot see an excess nobody agreed to. The reason is never empty: the + * arithmetic is written down so the next person can see how the number was + * reached. */ export function editToConfirmation(row: LineRow, edit: LineEdit, poUnit: string | null): LineConfirmation { const targetUnit = poUnit ?? row.poUnit ?? null; @@ -169,11 +201,18 @@ export function editToConfirmation(row: LineRow, edit: LineEdit, poUnit: string const unitOverride = differs && factor === 1; const unit = differs && !unitOverride ? targetUnit! : slipUnit; const priceEdited = edit.price !== null && edit.price !== undefined && edit.usePoRate !== true; + const acceptExcess = edit.acceptExcess === true; + const over = overPo(row, poQty); - const reason = edit.reason?.trim() || [ + const written = edit.reason?.trim() || [ `${edit.slipQty} ${slipUnit} × ${factor} = ${poQty} ${unit}`, ...(priceEdited ? [`Price R${edit.price} instead of PO R${row.poRate ?? "?"}`] : []), ].join("; "); + // An accepted excess is written down, so nobody reads the line later + // without knowing that it went over the order. + const reason = acceptExcess && over > 0 && !/over po/i.test(written) + ? `${written}; Over PO by ${over} ${targetUnit ?? "unit"}, accepted` + : written; return { confirmed: true, @@ -182,6 +221,7 @@ export function editToConfirmation(row: LineRow, edit: LineEdit, poUnit: string unit, ...(unitOverride ? { unitOverride: true } : {}), ...(priceEdited ? { netAmount: round2(poQty * edit.price!) } : { usePoRate: true }), + ...(acceptExcess ? { acceptExcess: true } : {}), reason, }; } diff --git a/src/lib/inbox-three/save-lines.test.ts b/src/lib/inbox-three/save-lines.test.ts index b2a1a3e..98a8990 100644 --- a/src/lib/inbox-three/save-lines.test.ts +++ b/src/lib/inbox-three/save-lines.test.ts @@ -16,7 +16,7 @@ const context = (): PreviewContext => ({ supplier: { id: "supplierabcdefg", name: "Vula" }, po: { id: "poabcdefghijklm", purchase_order_number: "MEGCPO9448", total_value: 11748.7 }, poItems: [ - { id: MESH_ITEM, description: "MESH DIAMOND 600 x 2.5 ROLL", unit: "m", qty: 2, rate: 1124.35, disc_percent: 0, vat_percent: 15 }, + { id: MESH_ITEM, description: "MESH DIAMOND 600 x 2.5 ROLL", unit: "m", qty: 100, rate: 1124.35, disc_percent: 0, vat_percent: 15 }, { id: KERB_ITEM, description: "KERB 150", unit: "m", qty: 100, rate: 150, disc_percent: 0, vat_percent: 15 }, { id: BAILING_ITEM, description: "1.6mm Bailing Wire", unit: "ea", qty: 100, rate: 95, disc_percent: 0, vat_percent: 15 }, ], @@ -106,6 +106,37 @@ describe("saveLineEdits", () => { allocation.purchaseOrderItemId === KERB_ITEM && allocation.sourceLineIds.length === 2)).toBe(true); }); + it("refuses a quantity over what the order has left unless the office accepts it", () => { + const c = context(); + const before = preview(c); + const untouched = structuredClone(before); + // KERB has 100 m left on the order; 4 × 30 m is 120 m. + expect(() => saveLineEdits(c, before, [edit({ + lines: [1], + poItemId: KERB_ITEM, + slipQty: 4, + slipUnit: "ea", + factor: 30, + reason: "One kerb is 30 m.", + })], "operator")).toThrow('Line 2: 20 m more than the PO has left. Tick "accept over PO" or change the quantity.'); + expect(before).toEqual(untouched); + }); + + it("books a quantity over the order when the office accepts it, and says so", () => { + const c = context(); + const result = saveLineEdits(c, preview(c), [edit({ + lines: [1], + poItemId: KERB_ITEM, + slipQty: 4, + slipUnit: "ea", + factor: 30, + acceptExcess: true, + reason: "The site ordered extra kerb.", + })], "operator"); + const allocation = result.preview.allocations.find((candidate) => candidate.purchaseOrderItemId === KERB_ITEM); + expect(allocation?.operatorConfirmation?.reason).toContain("Over PO"); + }); + it("saves nothing when one edit in the batch fails", () => { const c = context(); const before = preview(c); const untouched = structuredClone(before); diff --git a/src/lib/inbox-three/save-lines.ts b/src/lib/inbox-three/save-lines.ts index db0de6a..87b8c03 100644 --- a/src/lib/inbox-three/save-lines.ts +++ b/src/lib/inbox-three/save-lines.ts @@ -14,11 +14,18 @@ import { resolveMatch } from "@/lib/dn-pipeline/matching"; import { rememberConnection } from "@/lib/dn-pipeline/remembered-match"; import { stableSourceKey } from "@/lib/dn-pipeline/line-booking"; import { readyIndexes } from "@/lib/inbox-three/readiness"; -import { buildLineRows, editToConfirmation, type LineEdit } from "./line-rows"; +import { buildLineRows, editToConfirmation, overPo, poLineNumbers, type LineEdit, type LineRow } from "./line-rows"; const sameLines = (a: readonly string[], b: readonly string[]) => a.length === b.length && [...a].sort().join("|") === [...b].sort().join("|"); +/** What the office has to decide: this line is over what the order has left. */ +function excessMessage(lineIndex: number, over: number, poUnit: string | null): string { + const line = lineIndex + 1; + if (over > 0) return `Line ${line}: ${over} ${poUnit ?? "unit"} more than the PO has left. Tick "accept over PO" or change the quantity.`; + return `Line ${line}: this line is worth more than the purchase order has left. Tick "accept over PO" or change the quantity.`; +} + /** Has this slip line already been booked? */ function isPosted(context: PreviewContext, preview: ProcessingPreview, sourceLineId: string): boolean { if (preview.allocations.some((allocation) => allocation.posted && allocation.sourceLineIds.includes(sourceLineId))) return true; @@ -84,15 +91,20 @@ export function saveLineEdits( } const row = buildLineRows(working, next)[index]; - const confirmation = editToConfirmation(row, edit, edit.poItemId ? row.poUnit : null); + // An edit may put the line on another order line. The numbers that + // decide whether a quantity is over the order belong to the line it is + // going on, not to the one it came from. + const target = poLineNumbers(working, edit.poItemId ?? row.poItemId); + const effectiveRow: LineRow = { ...row, ...target }; + const confirmation = editToConfirmation(effectiveRow, edit, target.poUnit); try { current = resolveMatch(working, next, { index, sourceLineIds, snapshotHash: next.snapshotHash, confirmation }); } catch (error) { - // The clerk confirmed a quantity that is over what the order has left. - // Their reason is the acknowledgement; without it the line stays open. + // More than the order has left. The system does not decide this and + // neither does the assistant: a person has to accept it on the line. if (!(error instanceof Error) || !/Acknowledge the cumulative excess/.test(error.message)) throw error; - current = resolveMatch(working, next, { index, sourceLineIds, snapshotHash: next.snapshotHash, confirmation: { ...confirmation, acceptExcess: true } }); + throw Error(excessMessage(indexes[0], overPo(effectiveRow, confirmation.quantity ?? null), target.poUnit)); } // A remembered receiving connection is the supplier's rule, not the