From a6d88cef1c7369611edc0578923ee8203df658ba Mon Sep 17 00:00:00 2001 From: Dave Date: Sun, 30 Aug 2026 22:18:02 +0200 Subject: [PATCH] feat(orders): widen DN repeat-match to any status, add 60-day window --- .../order-delivery-note-link-service.test.ts | 120 ++++++++++++++++-- .../order-delivery-note-link-service.ts | 35 ++--- 2 files changed, 122 insertions(+), 33 deletions(-) diff --git a/src/lib/server/order-delivery-note-link-service.test.ts b/src/lib/server/order-delivery-note-link-service.test.ts index a3a6171..9cc874d 100644 --- a/src/lib/server/order-delivery-note-link-service.test.ts +++ b/src/lib/server/order-delivery-note-link-service.test.ts @@ -224,6 +224,89 @@ describe("suggestDeliveryNoteForOrder", () => { expect(res.best?.deliveryNoteId).toBe("dn2"); }); + + it("matches a cancelled delivery note (status no longer filters candidates)", async () => { + // Per 2026-08-30 business decision: match against a DN of ANY + // status, including cancelled. The prior exclusion missed real + // repeat orders where the original delivery's DN was cancelled + // for reasons unrelated to whether goods went out. + const dnItem = { description: "M16x50 Bolt", quantity: 300, order: "other-order" }; + const orderItem = { description: "M16x50 Bolt", quantity: 300 }; + + mockAdminPb(makePb({ + [ORDERS]: { + getOne: vi.fn().mockResolvedValue({ id: "ord1", company: "co1" }), + update: vi.fn().mockResolvedValue({}), + }, + [ORDER_ITEMS]: { + getFullList: vi.fn() + .mockResolvedValueOnce([orderItem]) // for the order's own items + .mockResolvedValueOnce([dnItem]), // for the DN's items (matching) + }, + [DELIVERY_NOTES]: { + getFullList: vi.fn().mockResolvedValue([ + { + id: "dn-cancelled", + delivery_note_number: "DN-CANC-1", + company: "co1", + status: "cancelled", + created: new Date().toISOString(), + order: "other-order", + }, + ]), + }, + })); + + const result = await suggestDeliveryNoteForOrder("ord1"); + + expect(result.best?.deliveryNoteId).toBe("dn-cancelled"); + }); + + it("does not match a delivery note older than 60 days", async () => { + const orderItem = { description: "M16x50 Bolt", quantity: 300 }; + const dnItem = { description: "M16x50 Bolt", quantity: 300, order: "other-order" }; + + // Filter-aware mock: honours the `created >= "..."` clause the + // service now sends, so the test actually exercises the cutoff + // rather than passing vacuously because the mock returns all + // rows regardless of filter. + const cutoff = new Date(Date.now() - 60 * 24 * 60 * 60 * 1000) + .toISOString() + .replace("T", " "); + const dnRows = [ + { + id: "dn-old", + delivery_note_number: "DN-OLD-1", + company: "co1", + status: "completed", + // > 60 days before "now" — must be filtered out by the cutoff + created: "2026-05-01 00:00:00.000Z", + order: "other-order", + }, + ]; + const dnGetFullList = vi + .fn() + .mockImplementation((opts: { filter?: string } = {}) => + dnRows.filter((dn) => dn.created >= cutoff), + ); + + mockAdminPb(makePb({ + [ORDERS]: { + getOne: vi.fn().mockResolvedValue({ id: "ord1", company: "co1" }), + update: vi.fn().mockResolvedValue({}), + }, + [ORDER_ITEMS]: { + getFullList: vi.fn() + .mockResolvedValueOnce([orderItem]) // for the order's own items + .mockResolvedValueOnce([dnItem]), // for the DN's items (would match) + }, + [DELIVERY_NOTES]: { getFullList: dnGetFullList }, + })); + + const result = await suggestDeliveryNoteForOrder("ord1"); + + expect(result.best).toBeNull(); + }); }); /* ============================================================================ @@ -265,7 +348,12 @@ describe("suggestDeliveryNoteForOrder — scoping", () => { expect(filter).not.toContain("customer"); }); - it("restricts candidates to statuses whose goods actually went out", async () => { + it("does not filter candidates by status — any status qualifies (cancelled included)", async () => { + // Per 2026-08-30 business decision: match against a DN of ANY + // status, including `cancelled`. The 60-day window below bounds + // the search space instead. Asserting the absence of any `status =` + // clause pins the policy so a future revert (re-introducing a + // status allowlist) would fail here loudly. const dnGetFullList = vi.fn().mockResolvedValue([]); mockAdminPb( makePb({ @@ -281,13 +369,29 @@ describe("suggestDeliveryNoteForOrder — scoping", () => { await suggestDeliveryNoteForOrder("ord1"); const filter = dnGetFullList.mock.calls[0][0].filter as string; - // draft + loaded are real deliveries here — confirmed with the - // business owner. Excluding them made the feature inert (3 of 116). - for (const s of ["draft", "loaded", "sent", "po_received", "invoiced", "completed"]) { - expect(filter).toContain(`status = "${s}"`); - } - // Cancelled is the one deliberate exclusion. - expect(filter).not.toContain('status = "cancelled"'); + expect(filter).not.toMatch(/status\s*=/); + }); + + it("applies a 60-day candidate window on `delivery_notes.created`", async () => { + // The replacement for the prior status allowlist: bound the search + // space with a recency cutoff so the matcher can't surface a DN + // from years ago that happens to share a description. + const dnGetFullList = vi.fn().mockResolvedValue([]); + mockAdminPb( + makePb({ + [ORDER_ITEMS]: { + getFullList: vi.fn().mockResolvedValue([ + { id: "oi1", description: "Widget", quantity: 1 }, + ]), + }, + [DELIVERY_NOTES]: { getFullList: dnGetFullList }, + }), + ); + + await suggestDeliveryNoteForOrder("ord1"); + + const filter = dnGetFullList.mock.calls[0][0].filter as string; + expect(filter).toMatch(/created\s*>=\s*"\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}/); }); it("never flags an order as a repeat of its OWN delivery note", async () => { diff --git a/src/lib/server/order-delivery-note-link-service.ts b/src/lib/server/order-delivery-note-link-service.ts index 70422a9..d858e20 100644 --- a/src/lib/server/order-delivery-note-link-service.ts +++ b/src/lib/server/order-delivery-note-link-service.ts @@ -34,30 +34,11 @@ const DELIVERY_NOTES_COLLECTION = "delivery_notes"; const QUOTES_COLLECTION = "quotes"; const QUOTE_ITEMS_COLLECTION = "quote_items"; -/** DNs representing goods that actually went out to the customer. - * - * `draft` and `loaded` are INCLUDED, corrected from the original spec: - * - `draft` — confirmed with the business owner (2026-08-30) that - * drafts are real deliveries; the status simply never gets - * advanced in the system. Excluding them left a candidate pool of - * 3 DNs out of 116, making the whole feature inert. - * - `loaded` — reachable directly from `draft` - * (`deliveries-service.ts`), so a DN can go out on the truck - * without ever passing through `sent`. Goods physically left. - * - * `cancelled` remains excluded: nominally "abandoned, kept for audit". - * NOTE it is 84 of 116 live DNs, and the owner has said cancel was - * also used to clear out superseded delivery orders — so some of - * those may be real deliveries. Revisit once that is confirmed; - * including them is a one-line change here. */ -const CANDIDATE_STATUSES = [ - "draft", - "loaded", - "sent", - "po_received", - "invoiced", - "completed", -]; +// Per 2026-08-30 business decision: match against a DN of ANY status, +// including `cancelled` — the prior exclusion missed real repeat orders +// where the original delivery's DN had been cancelled for reasons +// unrelated to whether goods went out. A 60-day candidate window +// (computed below) bounds the search space instead. function esc(s: string): string { return (s || "").replace(/"/g, '\\"'); @@ -113,10 +94,14 @@ export async function suggestDeliveryNoteForOrder( quantity: Number(r.quantity) || 0, })); + const WINDOW_DAYS = 60; + const cutoff = new Date(Date.now() - WINDOW_DAYS * 24 * 60 * 60 * 1000); + const cutoffStr = cutoff.toISOString().replace("T", " "); + const candidates = await pb .collection(DELIVERY_NOTES_COLLECTION) .getFullList({ - filter: `company = "${esc(order.company)}" && (${CANDIDATE_STATUSES.map((s) => `status = "${s}"`).join(" || ")})`, + filter: `company = "${esc(order.company)}" && created >= "${cutoffStr}"`, fields: "id,delivery_note_number,company,status,created,order", batch: 1000, });