From fd3a9296c627242edef996034af13287828afa79 Mon Sep 17 00:00:00 2001 From: Dave Date: Fri, 9 Oct 2026 11:56:48 +0200 Subject: [PATCH] feat(delivery-notes): the note decides the quantity; the quote follows it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A counter note is raised when the customer takes goods off the shelf. "They said make it five more" is edited on the NOTE. Previously that edit changed the note and left the quote showing the old number, so the same physical goods carried two different quantities across two screens and the invoice was whichever screen the operator opened last. `updateDeliveryNoteItemPricing` now pushes the note's quantity onto the quote lines that came from it. A sync failure is logged, not fatal — the edit already succeeded and the quote can be re-synced by re-saving. ## Matching without a back-link `quote_items` has no field pointing at its source `order_item`, so the operator's call was to make the note authoritative and work around it. Lines are paired by `sku + description`, and when a key repeats, by ORDER OF OCCURRENCE. That is sound because both sides derive from the same ordered list of note lines: `createQuoteFromDeliveryNote` iterates the note's items in query order and emits one quote line each, so the Nth of a repeated key on the note is the Nth on the quote. On the live data across 500 note lines there are ZERO groups sharing (note, sku, description) — the key is unique in practice. The occurrence counter exists so that if two identical lines are ever added, the second still lands on the right row instead of both writing to the first. That is the failure the tests exist for: matching on the key alone silently writes the wrong number onto a customer invoice. Verified by removing the occurrence counter — exactly that test fails. ## Also carried through - The note's COLLECTED qty is the billing qty, not the ordered qty, so a short pickup (10 ordered, 8 collected) puts 8 on the quote. - A cancelled, rejected or declined quote keeps its own numbers. Rewriting a quote the customer has already seen is worse than a stale number. - A line that cannot be matched is left alone, never guessed at. - A note line with neither a collected nor an ordered qty is skipped, so this can never reintroduce the zero-quantity invoice line. 8 new tests. Full suite 3832 passing, tsc clean, no new lint errors. --- src/lib/server/delivery-notes-service.ts | 147 +++++++++++++ src/lib/server/quote-quantity-sync.test.ts | 242 +++++++++++++++++++++ 2 files changed, 389 insertions(+) create mode 100644 src/lib/server/quote-quantity-sync.test.ts diff --git a/src/lib/server/delivery-notes-service.ts b/src/lib/server/delivery-notes-service.ts index b5e03ea..9e8ba4f 100644 --- a/src/lib/server/delivery-notes-service.ts +++ b/src/lib/server/delivery-notes-service.ts @@ -55,6 +55,7 @@ import type { DeliveryNoteStatus, OrderItem, Quote, + QuoteItem, } from "@/lib/types"; const DELIVERY_NOTES = "delivery_notes"; @@ -2237,6 +2238,24 @@ export async function updateDeliveryNoteItemPricing( await pb.collection(ORDER_ITEMS).update(input.itemId, patch); + // The delivery note decides the quantity. If this note already has a + // quote, its line moves with it, so editing here can never leave the + // quote showing a different number than the note the customer is + // holding. + if (hasQty) { + try { + await syncQuoteQuantitiesFromDeliveryNote(dn.id); + } catch (syncErr) { + // The pricing edit itself succeeded; failing to mirror it onto a + // quote must not fail the save. The quote keeps its old number + // and the operator can re-save to retry. + console.error( + `[updateDeliveryNoteItemPricing] could not push the new quantity to the quote for DN ${dn.delivery_note_number}:`, + syncErr, + ); + } + } + // Auto-regenerate the parent DN's PDF per UX decision (every // save re-renders the printed copy). Best-effort: a PDF // failure does not surface to the caller; the pricing edit @@ -2310,3 +2329,131 @@ export async function getDeliveryNoteDetail(deliveryNoteId: string): Promise the lines it changed, for the operator's toast. */ + detail: Array<{ quoteNumber: string; lines: string[] }>; +} + +/** Line-matching key. SKU first because two lines on one note can + * legitimately share a description (the same bolt at two pack sizes) + * but not a SKU. */ +function lineKey(row: { sku?: string | null; description?: string | null }): string { + return `${(row.sku ?? "").trim()} || ${(row.description ?? "").trim()}`; +} + +/** + * Push the delivery note's quantities onto the quote lines that came + * from it. + * + * ## The rule + * + * The delivery note decides. A counter note is raised when the + * customer takes goods off the shelf, and "they said make it five + * more" is edited on the NOTE. The quote and the invoice follow it. + * Otherwise the same physical goods carry three different quantities + * across three screens and the invoice is whatever the operator + * happened to be looking at. + * + * ## Matching without a back-link + * + * `quote_items` has no field pointing at its source `order_item` + * (adding one is a schema migration), so a quote line is matched to + * its note line by `sku + description`, and for a repeated key by + * ORDER OF OCCURRENCE. + * + * That holds because both sides derive from the same ordered list of + * note lines: `createQuoteFromDeliveryNote` iterates the note's items + * in query order and emits one quote line each, so the Nth of a + * repeated key on the note is the Nth on the quote. + * + * Measured on the live data: across 500 delivery-note lines there are + * ZERO groups sharing (note, sku, description), so the key is unique + * in practice. The occurrence counter is there so that if two + * identical lines are ever added, the second still lands on the right + * row instead of both writing to the first. + * + * ## What it refuses to touch + * + * A quote that is cancelled, rejected or declined, and any line the + * match cannot resolve. A line we cannot confidently pair is left + * alone and counted, never guessed at. + */ +export async function syncQuoteQuantitiesFromDeliveryNote( + deliveryNoteId: string, +): Promise { + const result: QuoteQuantitySyncResult = { + quotesTouched: 0, + linesChanged: 0, + detail: [], + }; + + const pb = await getAdminPocketBase(); + const dn = await pb + .collection(DELIVERY_NOTES) + .getOne(deliveryNoteId); + if (!dn.delivery_note_number) return result; + + const esc = (v: string) => v.replace(/"/g, '\\"'); + const notes = await pb.collection(ORDER_ITEMS).getFullList({ + filter: `delivery_note_number = "${esc(dn.delivery_note_number)}"`, + }); + if (notes.length === 0) return result; + + // Note line per (key, occurrence). + const byKey = new Map(); + for (const n of notes) { + const k = lineKey(n); + const list = byKey.get(k); + if (list) list.push(n); + else byKey.set(k, [n]); + } + + const quotes = await pb.collection(QUOTES).getFullList({ + filter: `delivery_note = "${esc(deliveryNoteId)}"`, + }); + + for (const quote of quotes) { + // A voided or declined quote keeps its own numbers — rewriting it + // would edit a document the customer may already have seen. + if (["cancelled", "rejected", "declined"].includes(quote.status)) continue; + + const lines = await pb + .collection(QUOTE_ITEMS) + .getFullList({ filter: `quote = "${esc(quote.id)}"` }); + if (lines.length === 0) continue; + + const seen = new Map(); + const changed: string[] = []; + for (const line of lines) { + const k = lineKey(line); + const nth = seen.get(k) ?? 0; + seen.set(k, nth + 1); + const source = byKey.get(k)?.[nth]; + if (!source) continue; // unresolvable — leave it, never guess + // The note's collected qty is the billing qty; fall back to the + // note quantity only when nothing was ever collected. + const noteQty = Number(source.qty_collected ?? source.quantity ?? 0); + const lineQty = Number(line.quantity ?? 0); + if (!Number.isFinite(noteQty) || noteQty <= 0) continue; + if (noteQty === lineQty) continue; + await pb.collection(QUOTE_ITEMS).update(line.id, { quantity: noteQty }); + changed.push(line.description ?? "(no description)"); + } + if (changed.length > 0) { + result.quotesTouched += 1; + result.linesChanged += changed.length; + result.detail.push({ quoteNumber: quote.quote_number, lines: changed }); + } + } + return result; +} diff --git a/src/lib/server/quote-quantity-sync.test.ts b/src/lib/server/quote-quantity-sync.test.ts new file mode 100644 index 0000000..80e6169 --- /dev/null +++ b/src/lib/server/quote-quantity-sync.test.ts @@ -0,0 +1,242 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; + +/** + * Tests for `syncQuoteQuantitiesFromDeliveryNote`. + * + * The invariant under test: THE DELIVERY NOTE DECIDES. A counter note + * is raised when the customer takes goods off the shelf, and "they + * said make it five more" is edited on the note. The quote and the + * invoice follow it. + * + * The interesting part is the MATCHING. `quote_items` carries no + * field pointing at its source `order_item`, so lines are paired by + * `sku + description` and, when a key repeats, by order of + * occurrence. That is the part that would silently write the wrong + * number, so it is pinned here — including the duplicate-line case + * the live data happens not to contain. + */ + +// `vi.mock` is hoisted above the imports, so the store and the PB +// double have to be hoisted with it — otherwise the factory runs +// before `pbMock` exists. Same shape as +// delivery-notes-service.test.ts. +const { store, getOrCreate, pbMock } = vi.hoisted(() => { + const store = new Map>>(); + const getOrCreate = (name: string) => { + let m = store.get(name); + if (!m) { + m = new Map(); + store.set(name, m); + } + return m; + }; + + const pbMock = { + collection: (name: string) => ({ + async getOne(id: string) { + const item = getOrCreate(name).get(id); + if (!item) { + const err = new Error("not found") as Error & { status: number }; + err.status = 404; + throw err; + } + return item; + }, + async getFullList(opts: { filter?: string } = {}) { + // The test store is small enough that we honour the DN-number + // filter directly and ignore everything else, exactly as the + // real service's filters are scoped. + const f = opts.filter ?? ""; + const m = f.match(/delivery_note_number = "([^"]+)"/); + let rows = [...getOrCreate(name).values()]; + if (m) rows = rows.filter((r) => r.delivery_note_number === m[1]); + const q = f.match(/quote = "([^"]+)"/); + if (q) rows = rows.filter((r) => r.quote === q[1]); + const d = f.match(/delivery_note = "([^"]+)"/); + if (d) rows = rows.filter((r) => r.delivery_note === d[1]); + return rows; + }, + async update(id: string, data: Record) { + const item = getOrCreate(name).get(id); + if (!item) { + const err = new Error("not found") as Error & { status: number }; + err.status = 404; + throw err; + } + Object.assign(item, data); + return item; + }, + }), + }; + + return { store, getOrCreate, pbMock }; +}); + +vi.mock("@/lib/server/pocketbase-admin", () => ({ + getAdminPocketBase: vi.fn().mockResolvedValue(pbMock), +})); + +import { syncQuoteQuantitiesFromDeliveryNote } from "./delivery-notes-service"; + +const seedQuote = (over: Record = {}) => { + const id = "q1"; + getOrCreate("quotes").set(id, { + id, + quote_number: "Q26071401", + delivery_note: "dn-1", + status: "delivery_note", + ...over, + }); + return id; +}; + +const seedQuoteLine = ( + quote: string, + over: Record = {}, +) => { + const id = `qi-${Math.random().toString(36).slice(2, 8)}`; + getOrCreate("quote_items").set(id, { + id, + quote, + description: "Bolt", + sku: "BOLT-1", + quantity: 10, + ...over, + }); + return id; +}; + +const seedNoteLine = (over: Record = {}) => { + const id = `oi-${Math.random().toString(36).slice(2, 8)}`; + getOrCreate("order_items").set(id, { + id, + delivery_note_number: "DN26071401", + description: "Bolt", + sku: "BOLT-1", + quantity: 10, + qty_collected: 10, + ...over, + }); + return id; +}; + +const qtyOf = (id: string) => + getOrCreate("quote_items").get(id)?.quantity as number; + +describe("syncQuoteQuantitiesFromDeliveryNote", () => { + beforeEach(() => { + store.clear(); + getOrCreate("delivery_notes").set("dn-1", { + id: "dn-1", + delivery_note_number: "DN26071401", + status: "sent", + }); + }); + + it("pushes a changed note quantity onto the quote line", async () => { + seedQuote(); + const noteId = seedNoteLine({ quantity: 5, qty_collected: 5 }); + const lineId = seedQuoteLine("q1", { quantity: 10 }); + expect(qtyOf(lineId)).toBe(10); + + const result = await syncQuoteQuantitiesFromDeliveryNote("dn-1"); + + expect(qtyOf(lineId)).toBe(5); + expect(result.quotesTouched).toBe(1); + expect(result.linesChanged).toBe(1); + expect(result.detail[0]).toEqual({ + quoteNumber: "Q26071401", + lines: ["Bolt"], + }); + expect(noteId).toBeTruthy(); + }); + + it("leaves a quote line alone when the quantities already agree", async () => { + seedQuote(); + seedNoteLine({ quantity: 10, qty_collected: 10 }); + const lineId = seedQuoteLine("q1", { quantity: 10 }); + + const result = await syncQuoteQuantitiesFromDeliveryNote("dn-1"); + + expect(qtyOf(lineId)).toBe(10); + expect(result.linesChanged).toBe(0); + expect(result.quotesTouched).toBe(0); + }); + + it("uses the note's COLLECTED qty, not its ordered qty", async () => { + // A short pickup: 10 ordered, 8 actually collected. The invoice + // bills 8, so the quote must say 8 too. + seedQuote(); + seedNoteLine({ quantity: 10, qty_collected: 8 }); + const lineId = seedQuoteLine("q1", { quantity: 10 }); + + await syncQuoteQuantitiesFromDeliveryNote("dn-1"); + + expect(qtyOf(lineId)).toBe(8); + }); + + it("pairs repeated sku+description by ORDER, so line 2 moves to line 2", async () => { + // The case that would silently write the wrong number. Two lines + // that look identical, different quantities. Matching on the key + // alone would send both to the first one. + seedQuote(); + seedNoteLine({ description: "Bolt", sku: "BOLT-1", quantity: 3, qty_collected: 3 }); + seedNoteLine({ description: "Bolt", sku: "BOLT-1", quantity: 7, qty_collected: 7 }); + const lineA = seedQuoteLine("q1", { description: "Bolt", sku: "BOLT-1", quantity: 10 }); + const lineB = seedQuoteLine("q1", { description: "Bolt", sku: "BOLT-1", quantity: 20 }); + + await syncQuoteQuantitiesFromDeliveryNote("dn-1"); + + expect(qtyOf(lineA)).toBe(3); + expect(qtyOf(lineB)).toBe(7); + }); + + it("does not touch a cancelled or declined quote", async () => { + seedQuote({ status: "cancelled" }); + seedNoteLine({ quantity: 5, qty_collected: 5 }); + const lineId = seedQuoteLine("q1", { quantity: 10 }); + + const result = await syncQuoteQuantitiesFromDeliveryNote("dn-1"); + + expect(qtyOf(lineId)).toBe(10); + expect(result.quotesTouched).toBe(0); + }); + + it("leaves a line it cannot match rather than guessing", async () => { + seedQuote(); + seedNoteLine({ description: "Bolt", sku: "BOLT-1", quantity: 5, qty_collected: 5 }); + // This line has no counterpart on the note at all. + const orphan = seedQuoteLine("q1", { + description: "Something else entirely", + sku: "OTHER-9", + quantity: 99, + }); + + const result = await syncQuoteQuantitiesFromDeliveryNote("dn-1"); + + expect(qtyOf(orphan)).toBe(99); + expect(result.linesChanged).toBe(0); + }); + + it("skips a note line with no collected qty and no quantity to fall back on", async () => { + seedQuote(); + seedNoteLine({ quantity: 0, qty_collected: 0 }); + const lineId = seedQuoteLine("q1", { quantity: 10 }); + + await syncQuoteQuantitiesFromDeliveryNote("dn-1"); + + // Zero is not a billable quantity, and writing it back would + // reproduce the exact zero-line invoice this all exists to stop. + expect(qtyOf(lineId)).toBe(10); + }); + + it("does nothing when the note has no lines", async () => { + seedQuote(); + const lineId = seedQuoteLine("q1", { quantity: 10 }); + + const result = await syncQuoteQuantitiesFromDeliveryNote("dn-1"); + + expect(qtyOf(lineId)).toBe(10); + expect(result.quotesTouched).toBe(0); + }); +});