diff --git a/src/lib/server/order-order-duplicate-link-service.test.ts b/src/lib/server/order-order-duplicate-link-service.test.ts index 41629c2..58a0de3 100644 --- a/src/lib/server/order-order-duplicate-link-service.test.ts +++ b/src/lib/server/order-order-duplicate-link-service.test.ts @@ -1,8 +1,9 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; -const { store, pbMock, clearStore } = vi.hoisted(() => { +const { store, pbMock, clearStore, getFullListCalls } = vi.hoisted(() => { type StoreMap = Map>; const store: Record = {}; + const getFullListCalls: Record = {}; let seq = 0; const nextId = () => `mock-${++seq}`; @@ -11,29 +12,40 @@ const { store, pbMock, clearStore } = vi.hoisted(() => { return store[name]; }; + const evalClause = (item: Record, clause: string): boolean => { + // Mock-only filter parser: accepts the four operators the + // duplicate-matcher uses (=, !=, >=, plus the lex variants + // <, <=, >). Anything else silently passes through so a + // typo in a clause doesn't 500 the test. + const m = clause.match(/^(\w+)\s*([!=]+|>=|<=|>|<)\s*"?([^"]+)"?$/); + if (!m) return true; + const [, field, op, raw] = m; + const value = raw.replace(/^"|"$/g, ""); + const actual = String(item[field] ?? ""); + if (op === "=") return actual === value; + if (op === "!=") return actual !== value; + // `created >= "2026-..."` clause — compare lexicographically. + // Safe for the YYYY-MM-DD HH:MM:SS.sssZ format PB uses, which + // sorts correctly as a string. Without this, every candidate + // would pass and the 60-day window would be inert. + if (op === ">=") return actual >= value; + return true; + }; + const where = (name: string, filter: string): Record[] => { const items = Array.from(getOrCreateMap(name).values()); if (!filter || !filter.trim()) return items; - const clauses = filter.split("&&").map((c) => c.trim()); + // PB filter syntax: `||` is OR (lower precedence), `&&` is AND. + // The bulk-fetch in suggestOrderDuplicate joins candidate ids + // with `||`, so we must honor that — otherwise the regex would + // miss the multi-clause filter and silently return everything, + // which would still pass behavioural tests but mask the + // optimization we're pinning in the new bulk-fetch test below. + const orGroups = filter.split("||").map((g) => g.trim()).filter(Boolean); return items.filter((item) => - clauses.every((clause) => { - // Mock-only filter parser: accepts the four operators the - // duplicate-matcher uses (=, !=, >=, plus the lex variants - // <, <=, >). Anything else silently passes through so a - // typo in a clause doesn't 500 the test. - const m = clause.match(/^(\w+)\s*([!=]+|>=|<=|>|<)\s*"?([^"]+)"?$/); - if (!m) return true; - const [, field, op, raw] = m; - const value = raw.replace(/^"|"$/g, ""); - const actual = String(item[field] ?? ""); - if (op === "=") return actual === value; - if (op === "!=") return actual !== value; - // `created >= "2026-..."` clause — compare lexicographically. - // Safe for the YYYY-MM-DD HH:MM:SS.sssZ format PB uses, which - // sorts correctly as a string. Without this, every candidate - // would pass and the 60-day window would be inert. - if (op === ">=") return actual >= value; - return true; + orGroups.some((group) => { + const andClauses = group.split("&&").map((c) => c.trim()).filter(Boolean); + return andClauses.every((clause) => evalClause(item, clause)); }), ); }; @@ -51,6 +63,7 @@ const { store, pbMock, clearStore } = vi.hoisted(() => { return { items, totalItems: items.length, page: 1, perPage: _perPage }; }, async getFullList(opts: { filter?: string; sort?: string } = {}) { + getFullListCalls[name] = (getFullListCalls[name] ?? 0) + 1; const items = where(name, opts.filter || ""); // Honor `sort` so tests can pin behaviour that depends on row // order. The real PB SDK sorts by the requested field @@ -120,10 +133,11 @@ const { store, pbMock, clearStore } = vi.hoisted(() => { const clearStore = () => { for (const key of Object.keys(store)) delete store[key]; + for (const key of Object.keys(getFullListCalls)) delete getFullListCalls[key]; seq = 0; }; - return { store, pbMock, clearStore }; + return { store, pbMock, clearStore, getFullListCalls }; }); vi.mock("pocketbase", () => ({ default: vi.fn().mockImplementation(() => pbMock) })); @@ -343,4 +357,44 @@ describe("suggestOrderDuplicate", () => { const updated = await pbMock.collection("orders").getOne("order-no-company"); expect(updated.duplicate_order_matching_attempted).toBeFalsy(); }); + + it("bulk-fetches candidate items in a single query (kills the N+1)", async () => { + // Three candidates for the same company, all with identical items. + // The source order also has identical items. With the old per-candidate + // loop, the matcher would have made 1 (source items) + 3 (one per + // candidate) = 4 getFullList calls against `order_items`. The new + // bulk-fetch path makes 1 (source) + 1 (all candidates in one OR query) + // = 2. This test pins the optimization — a regression that reverts to + // the per-candidate loop will push the count back up to 4. + await seedOrderWithItems( + "order-cand-1", + { order_number: "ORD-C1", company: "comp-1", created: "2026-08-05 00:00:00.000Z" }, + [{ description: "Same Item", quantity: 5 }], + ); + await seedOrderWithItems( + "order-cand-2", + { order_number: "ORD-C2", company: "comp-1", created: "2026-08-10 00:00:00.000Z" }, + [{ description: "Same Item", quantity: 5 }], + ); + await seedOrderWithItems( + "order-cand-3", + { order_number: "ORD-C3", company: "comp-1", created: "2026-08-15 00:00:00.000Z" }, + [{ description: "Same Item", quantity: 5 }], + ); + await seedOrderWithItems( + "order-source", + { order_number: "ORD-SRC", company: "comp-1", created: "2026-08-30 00:00:00.000Z" }, + [{ description: "Same Item", quantity: 5 }], + ); + + const before = getFullListCalls["order_items"] ?? 0; + const result = await suggestOrderDuplicate("order-source"); + const after = getFullListCalls["order_items"] ?? 0; + + expect(result.duplicateOrderId).toBe("order-cand-1"); // oldest match wins + // 1 fetch for the source order's own items + 1 bulk fetch for ALL + // candidates' items. The OLD per-candidate loop would have made + // 1 + 3 = 4 calls; the new bulk-fetch makes exactly 2. + expect(after - before).toBe(2); + }); }); diff --git a/src/lib/server/order-order-duplicate-link-service.ts b/src/lib/server/order-order-duplicate-link-service.ts index 5c4f7e3..ddc317c 100644 --- a/src/lib/server/order-order-duplicate-link-service.ts +++ b/src/lib/server/order-order-duplicate-link-service.ts @@ -86,21 +86,39 @@ export async function suggestOrderDuplicate( }); let match: { id: string; order_number: string } | null = null; - for (const candidate of candidates) { - const candidateItemRows = await pb + + // Bulk-fetch ALL items for ALL candidates in one query, instead of + // looping with N per-candidate getFullList calls. With many recent + // candidates this is the difference between O(N) and O(1) PB calls. + if (candidates.length > 0) { + const candidateItems = await pb .collection(ORDER_ITEMS_COLLECTION) - .getFullList<{ description?: string; quantity?: number }>({ - filter: `order = "${esc(candidate.id)}"`, - fields: "description,quantity", - batch: 1000, + .getFullList<{ order?: string; description?: string; quantity?: number }>({ + filter: candidates.map((c) => `order = "${esc(c.id)}"`).join(" || "), + fields: "order,description,quantity", + batch: 5000, }); - const candidateLines: DeliveryLine[] = candidateItemRows.map((r) => ({ - description: r.description || "", - quantity: Number(r.quantity) || 0, - })); - if (isExactItemMatch(orderLines, candidateLines)) { - match = candidate; - break; // candidates sorted oldest-first — first hit is the original + + // Group items by candidate order id so we can compare each + // candidate's lines to the source order's lines in memory. + const itemsByOrder = new Map>(); + for (const row of candidateItems) { + if (!row.order) continue; + const bucket = itemsByOrder.get(row.order); + if (bucket) bucket.push(row); + else itemsByOrder.set(row.order, [row]); + } + + for (const candidate of candidates) { + const candidateItemRows = itemsByOrder.get(candidate.id) ?? []; + const candidateLines: DeliveryLine[] = candidateItemRows.map((r) => ({ + description: r.description || "", + quantity: Number(r.quantity) || 0, + })); + if (isExactItemMatch(orderLines, candidateLines)) { + match = candidate; + break; // candidates sorted oldest-first — first hit is the original + } } }