diff --git a/src/lib/server/order-quote-link-service.test.ts b/src/lib/server/order-quote-link-service.test.ts index 668b9e8..9cd0085 100644 --- a/src/lib/server/order-quote-link-service.test.ts +++ b/src/lib/server/order-quote-link-service.test.ts @@ -18,23 +18,6 @@ interface CollectionMock { update?: ReturnType; } -function makePb(collections: Record) { - return { - autoCancellation: vi.fn(), - collection: (name: string) => { - const c = collections[name]; - if (!c) throw new Error(`unexpected collection: ${name}`); - return c; - }, - }; -} - -function useP(pb: ReturnType) { - vi.mocked(getAdminPocketBase).mockResolvedValue( - pb as unknown as Awaited>, - ); -} - const ORDER = { id: "ord1", created: "2026-08-29T10:00:00.000Z", @@ -44,22 +27,57 @@ const ORDER = { quote_reference_text: "", }; +/** + * Sensible defaults for every collection this service touches, merged + * with a per-test override map. Two more tasks (order intake, the + * backlog sweep) add tests to this file — without a default map, each + * test has to redeclare the full collection set even when it only + * cares about one of them, and an undeclared *method* on a + * hand-rolled mock fails with an opaque "not a function" rather than + * the collection map's clear "unexpected collection" error. + */ +function makePb(overrides: Record = {}) { + const defaults: Record = { + [ORDERS]: { + getOne: vi.fn().mockResolvedValue(ORDER), + update: vi.fn().mockResolvedValue({}), + }, + [QUOTES]: { getList: vi.fn().mockResolvedValue({ items: [] }) }, + [ORDER_ITEMS]: { getFullList: vi.fn().mockResolvedValue([]) }, + [QUOTE_ITEMS]: { getFullList: vi.fn().mockResolvedValue([]) }, + }; + const merged: Record = { ...defaults, ...overrides }; + return { + autoCancellation: vi.fn(), + collection: (name: string) => { + const c = merged[name]; + if (!c) throw new Error(`unexpected collection: ${name}`); + return c; + }, + }; +} + +function mockAdminPb(pb: ReturnType) { + vi.mocked(getAdminPocketBase).mockResolvedValue( + pb as unknown as Awaited>, + ); +} + describe("suggestQuoteForOrder", () => { beforeEach(() => vi.clearAllMocks()); it("resolves an exact quote_number citation regardless of the date window", async () => { const update = vi.fn().mockResolvedValue({}); const quotesGetList = vi.fn().mockResolvedValue({ - items: [{ id: "q_old", quote_number: "Q-2024-0847", created: "2024-01-05T00:00:00.000Z" }], + items: [{ id: "q_old", quote_number: "Q-2024-0847", company: "co1", created: "2024-01-05T00:00:00.000Z" }], }); - useP( + mockAdminPb( makePb({ [ORDERS]: { getOne: vi.fn().mockResolvedValue({ ...ORDER, quote_reference_text: "Q-2024-0847" }), update, }, [QUOTES]: { getList: quotesGetList }, - [ORDER_ITEMS]: { getFullList: vi.fn().mockResolvedValue([]) }, }), ); @@ -76,26 +94,66 @@ describe("suggestQuoteForOrder", () => { expect(update.mock.calls[0][1]).not.toHaveProperty("quote"); }); - it("constrains fuzzy candidates to the 90-day window before the order date", async () => { - const quotesGetList = vi.fn().mockResolvedValue({ items: [] }); - useP( + it("scopes exact-identifier lookups to the order's own company", async () => { + // Two quotes share the same customer-supplied reference text but + // belong to different companies. The fixture is deliberately + // ordered so an UNscoped lookup would return the wrong one + // (q_other_co, listed first) — only a company-scoped filter + // narrows it down to q_co1. + const quoteRows = [ + { id: "q_other_co", quote_reference: "SHARED-REF", company: "co2", created: "2026-01-05T00:00:00.000Z" }, + { id: "q_co1", quote_reference: "SHARED-REF", company: "co1", created: "2026-01-01T00:00:00.000Z" }, + ]; + const quotesGetList = vi.fn().mockImplementation( + (_page: number, _perPage: number, opts: { filter: string }) => { + const conditions = opts.filter.split(" && ").map((clause) => { + const m = clause.trim().match(/^(\w+) = "(.*)"$/); + return m ? { field: m[1], value: m[2] } : null; + }); + const items = quoteRows.filter((row) => + conditions.every( + (c) => c === null || (row as unknown as Record)[c.field] === c.value, + ), + ); + return Promise.resolve({ items }); + }, + ); + mockAdminPb( + makePb({ + [ORDERS]: { + getOne: vi.fn().mockResolvedValue({ ...ORDER, quote_reference_text: "SHARED-REF" }), + update: vi.fn().mockResolvedValue({}), + }, + [QUOTES]: { getList: quotesGetList }, + }), + ); + + const res = await suggestQuoteForOrder("ord1"); + + expect(res.best?.quoteId).toBe("q_co1"); + }); + + it("constrains fuzzy candidates to the 90-day window before the order date, and projects only scalar fields", async () => { + const quotesGetList = vi.fn().mockResolvedValue({ items: [] }); + mockAdminPb( makePb({ - [ORDERS]: { getOne: vi.fn().mockResolvedValue(ORDER), update: vi.fn().mockResolvedValue({}) }, [QUOTES]: { getList: quotesGetList }, [ORDER_ITEMS]: { getFullList: vi.fn().mockResolvedValue([ { id: "oi1", description: "hex bolt m20 gr8.8 zinc", selling_price: 10, quantity: 5 }, ]), }, - [QUOTE_ITEMS]: { getFullList: vi.fn().mockResolvedValue([]) }, }), ); await suggestQuoteForOrder("ord1"); - const filter = quotesGetList.mock.calls[0][2].filter as string; - expect(filter).toContain('created >= "2026-05-31T10:00:00.000Z"'); - expect(filter).toContain('company = "co1"'); + const opts = quotesGetList.mock.calls[0][2] as { filter: string; fields?: string }; + expect(opts.filter).toContain('created >= "2026-05-31T10:00:00.000Z"'); + expect(opts.filter).toContain('company = "co1"'); + // `quotes` also carries email_body / ai_insights / migration_meta — + // this query must not pull those to read five scalars. + expect(opts.fields).toBe("id,quote_number,company,customer,created"); }); it("widens to 12 months when the 90-day window yields nothing above threshold", async () => { @@ -105,20 +163,18 @@ describe("suggestQuoteForOrder", () => { .mockResolvedValueOnce({ items: [{ id: "q1", quote_number: "Q-1", company: "co1", customer: "cust1", created: "2025-11-01T00:00:00.000Z" }], }); - useP( + const quoteItemsGetFullList = vi.fn().mockResolvedValue([ + { id: "qi1", quote: "q1", description: "hex bolt m20 gr8.8 zinc", selling_price: 10, quantity: 5 }, + ]); + mockAdminPb( makePb({ - [ORDERS]: { getOne: vi.fn().mockResolvedValue(ORDER), update: vi.fn().mockResolvedValue({}) }, [QUOTES]: { getList: quotesGetList }, [ORDER_ITEMS]: { getFullList: vi.fn().mockResolvedValue([ { id: "oi1", description: "hex bolt m20 gr8.8 zinc", selling_price: 10, quantity: 5 }, ]), }, - [QUOTE_ITEMS]: { - getFullList: vi.fn().mockResolvedValue([ - { id: "qi1", quote: "q1", description: "hex bolt m20 gr8.8 zinc", selling_price: 10, quantity: 5 }, - ]), - }, + [QUOTE_ITEMS]: { getFullList: quoteItemsGetFullList }, }), ); @@ -127,6 +183,14 @@ describe("suggestQuoteForOrder", () => { expect(quotesGetList).toHaveBeenCalledTimes(2); expect(res.window).toBe("widened"); expect(res.best?.quoteId).toBe("q1"); + + // The matcher's quote_items projection must include `description` + // and `selling_price` — fetchQuoteItemsForQuotesAdmin's own default + // fields string belongs to a different caller (the duplicate-quote + // scan) and could be trimmed independently of the matcher's needs. + const itemsOpts = quoteItemsGetFullList.mock.calls[0][0] as { fields: string }; + expect(itemsOpts.fields).toContain("description"); + expect(itemsOpts.fields).toContain("selling_price"); }); it("chunks the quote_items fetch instead of issuing one oversized OR-chain filter", async () => { @@ -156,9 +220,8 @@ describe("suggestQuoteForOrder", () => { return Promise.resolve([]); }); - useP( + mockAdminPb( makePb({ - [ORDERS]: { getOne: vi.fn().mockResolvedValue(ORDER), update: vi.fn().mockResolvedValue({}) }, [QUOTES]: { getList: quotesGetList }, [ORDER_ITEMS]: { getFullList: vi.fn().mockResolvedValue([ @@ -194,16 +257,14 @@ describe("suggestQuoteForOrder", () => { it("records no_match and still stamps quote_matching_attempted", async () => { const update = vi.fn().mockResolvedValue({}); - useP( + mockAdminPb( makePb({ [ORDERS]: { getOne: vi.fn().mockResolvedValue(ORDER), update }, - [QUOTES]: { getList: vi.fn().mockResolvedValue({ items: [] }) }, [ORDER_ITEMS]: { getFullList: vi.fn().mockResolvedValue([ { id: "oi1", description: "widget", selling_price: 1, quantity: 1 }, ]), }, - [QUOTE_ITEMS]: { getFullList: vi.fn().mockResolvedValue([]) }, }), ); @@ -220,7 +281,7 @@ describe("suggestQuoteForOrder", () => { it("skips an order that already has a hard quote link", async () => { const update = vi.fn(); - useP( + mockAdminPb( makePb({ [ORDERS]: { getOne: vi.fn().mockResolvedValue({ ...ORDER, quote: "already" }), update }, }), diff --git a/src/lib/server/order-quote-link-service.ts b/src/lib/server/order-quote-link-service.ts index 71695f7..72e5b1b 100644 --- a/src/lib/server/order-quote-link-service.ts +++ b/src/lib/server/order-quote-link-service.ts @@ -18,6 +18,7 @@ * `setOrderQuote` in `orders-service.ts`. */ +import { ClientResponseError } from "pocketbase"; import { getAdminPocketBase } from "@/lib/server/pocketbase-admin"; import { fetchQuoteItemsForQuotesAdmin } from "@/lib/server/quotes-service"; import type { QuoteItem } from "@/lib/types"; @@ -54,8 +55,10 @@ export interface SuggestQuoteResult { best: QuoteMatchCandidate | null; alternatives: QuoteMatchCandidate[]; method: QuoteMatchMethod; - /** Which pass produced the result — for the sweep's reporting. */ - window: "exact" | "90d" | "widened" | "none"; + /** Which pass produced the result — for the sweep's reporting. + * "initial" is the FUZZY_WINDOW_DAYS pass, not a fixed "90d" label + * — retuning that constant must not make this label lie. */ + window: "exact" | "initial" | "widened" | "none"; /** Days from the matched quote's creation to the order's. null when unmatched. */ ageGapDays: number | null; /** Set when the order was deliberately not processed. */ @@ -91,6 +94,22 @@ const ORDER_COLLECTION = "orders"; const QUOTE_COLLECTION = "quotes"; const ORDER_ITEMS_COLLECTION = "order_items"; +/** The scalars `fuzzyPass` reads off each candidate quote. Explicit + * `fields` on the query — `quotes` also carries `email_body`, + * `ai_insights`, `migration_meta`, etc., and pulling those for up to + * FUZZY_CANDIDATE_CAP (200) candidates (twice, if the window widens) + * to read five scalars would dominate the whole operation's cost at + * intake volume. */ +const CANDIDATE_QUOTE_FIELDS = "id,quote_number,company,customer,created"; + +/** What `scoreOrderVsQuote` needs off each quote line. Passed + * explicitly to `fetchQuoteItemsForQuotesAdmin` — its default + * projection belongs to the duplicate-quote scan, a different + * stakeholder with different needs. If that default is ever trimmed, + * this call must not silently lose `description` or `selling_price` + * and start scoring every candidate near zero. */ +const MATCHER_QUOTE_ITEM_FIELDS = "id,quote,description,selling_price,quantity"; + function esc(s: string): string { return (s || "").replace(/"/g, '\\"'); } @@ -103,11 +122,20 @@ function project(rows: ReadonlyArray) { })); } -/** Try the four exact-identifier fields, in priority order. Never date - * limited — a citation is evidence regardless of age. */ +/** + * Try the four exact-identifier fields, in priority order. Never date + * limited — a citation is evidence regardless of age. + * + * Scoped to the order's own company when known: `quote_reference_text` + * and `rfq_number` are customer-supplied free text, and without this + * clause a reference-number collision across two customers would + * return a confidence-1.0 suggestion pointing at a different + * company's quote. + */ async function findExactMatch( pb: Awaited>, ref: string, + companyId?: string, ): Promise<{ quote: QuoteRow; method: QuoteMatchCandidate["method"] } | null> { const trimmed = (ref || "").trim(); if (!trimmed) return null; @@ -118,13 +146,27 @@ async function findExactMatch( ["external_number", "exact_external_number"], ] as const; for (const [field, method] of fields) { + const clauses = [`${field} = "${esc(trimmed)}"`]; + if (companyId) clauses.push(`company = "${esc(companyId)}"`); try { const list = await pb .collection(QUOTE_COLLECTION) - .getList(1, 5, { filter: `${field} = "${esc(trimmed)}"` }); + .getList(1, 5, { filter: clauses.join(" && ") }); if (list.items.length > 0) return { quote: list.items[0], method }; - } catch { - // Field may not exist on older PB deploys — try the next. + } catch (err) { + // A field this deploy's PB schema doesn't have 400s — try the + // next one; all four fields exist per pb-exports/pb_schema-2.json + // so this is a defensive fallback, not the expected path. + // Anything else (expired admin token, connection reset, PB + // restart mid-sweep) is a real failure and must propagate: + // swallowing it here would let the fuzzy path silently run + // instead, and writeSuggestion would then persist + // `quote_matching_attempted: true` / `quote_match_method: + // "no_match"` for an order that actually had a valid citation — + // permanently hiding it from the sweep's "haven't tried yet" + // filter, since nothing revisits an order once that flag is set. + if (err instanceof ClientResponseError && err.status === 400) continue; + throw err; } } return null; @@ -155,6 +197,7 @@ async function fuzzyPass( .getList(1, FUZZY_CANDIDATE_CAP, { filter: filters.join(" && "), sort: "-created", + fields: CANDIDATE_QUOTE_FIELDS, }); if (candidates.items.length === 0) return []; @@ -167,7 +210,7 @@ async function fuzzyPass( // `quotes-service.ts`, which already has this exact chunking logic // for its duplicate-quote scan. const quoteIds = candidates.items.map((q) => q.id); - const quoteItems = await fetchQuoteItemsForQuotesAdmin(pb, quoteIds); + const quoteItems = await fetchQuoteItemsForQuotesAdmin(pb, quoteIds, MATCHER_QUOTE_ITEM_FIELDS); const byQuote = new Map(); for (const it of quoteItems) { @@ -230,7 +273,7 @@ export async function suggestQuoteForOrder( // 1. Exact identifier citation, if the order carries reference text. const ref = (order.quote_reference_text || order.rfq_number || "").trim(); - const exact = await findExactMatch(pb, ref); + const exact = await findExactMatch(pb, ref, order.company); if (exact) { const best: QuoteMatchCandidate = { quoteId: exact.quote.id, @@ -260,7 +303,7 @@ export async function suggestQuoteForOrder( }); let scored = await fuzzyPass(pb, order, orderItems, FUZZY_WINDOW_DAYS); - let window: SuggestQuoteResult["window"] = "90d"; + let window: SuggestQuoteResult["window"] = "initial"; if (scored.length === 0) { scored = await fuzzyPass(pb, order, orderItems, FUZZY_WINDOW_WIDEN_DAYS); window = "widened"; diff --git a/src/lib/server/quotes-service.ts b/src/lib/server/quotes-service.ts index f2f00b5..35563d8 100644 --- a/src/lib/server/quotes-service.ts +++ b/src/lib/server/quotes-service.ts @@ -3232,15 +3232,23 @@ export async function findDuplicateQuotes( * fuzzy matcher's quote-items fetch instead of building its own * unchunked OR-chain (which blows the PB filter length limit past * ~50 candidate quotes). + * + * `fields` defaults to this module's own projection but is a caller + * parameter, not a shared constant, because a second stakeholder now + * depends on specific columns being present: the matcher needs + * `description` and `selling_price` for its scoring, which the + * duplicate-quote scan (the original caller) has no reason to know + * about. Trimming the default for one caller must not silently starve + * the other. */ export async function fetchQuoteItemsForQuotesAdmin( pb: Awaited>, quoteIds: string[], + fields: string = "id,quote,description,sku,quantity,selling_price", ): Promise { if (quoteIds.length === 0) return []; const escape = (id: string) => id.replace(/"/g, '\\"'); const safeIds = quoteIds.map(escape); - const fields = "id,quote,description,sku,quantity,selling_price"; try { return await pb.collection(QUOTE_ITEMS_COLLECTION).getFullList({ filter: `quote IN (${safeIds.map((id) => `"${id}"`).join(",")})`,