From 46e06836355028d979efc3d1c5542807d91d56cc Mon Sep 17 00:00:00 2001 From: Dave Date: Sun, 16 Aug 2026 11:25:00 +0200 Subject: [PATCH] feat(duplicate-orders): enhance duplicate order detection and UI - Introduced a new `DuplicateKind` type to differentiate between "order_number" and "line_aggregate" duplicates. - Updated the `DuplicateOrdersDialog` component to support tabbed navigation between order number and line aggregate views, improving user experience. - Enhanced the data structure for duplicate records to include `totalQuantity` and `totalSellingPrice`, facilitating more accurate duplicate detection. - Implemented logic to auto-flip to the tab with available content, ensuring operators do not encounter empty views. - Expanded test coverage for the `findDuplicateOrders` function to validate new line-aggregate matching logic and ensure accurate detection of duplicates across different scenarios. - Improved documentation for the duplicate order handling process, clarifying the criteria for identifying duplicates. --- src/components/duplicate-orders-dialog.tsx | 101 +++++++++- src/lib/server/orders-service.test.ts | 223 ++++++++++++++++++++- src/lib/server/orders-service.ts | 216 ++++++++++++++++---- 3 files changed, 481 insertions(+), 59 deletions(-) diff --git a/src/components/duplicate-orders-dialog.tsx b/src/components/duplicate-orders-dialog.tsx index 946978e..dde3702 100644 --- a/src/components/duplicate-orders-dialog.tsx +++ b/src/components/duplicate-orders-dialog.tsx @@ -33,10 +33,16 @@ interface DuplicateRecord { itemCount: number; itemStatusSummary: Record; items: DuplicateItem[]; + totalQuantity: number | null; + totalSellingPrice: number | null; } +type DuplicateKind = "order_number" | "line_aggregate"; + interface DuplicateGroup { + kind: DuplicateKind; orderNumber: string; + aggregateKey?: string; records: DuplicateRecord[]; } @@ -70,6 +76,11 @@ export function DuplicateOrdersDialog({ }) { const [groups, setGroups] = useState([]); const [loading, setLoading] = useState(false); + // "order_number" is the surer signal (same number = certainly + // duplicated at intake); "line_aggregate" is a heuristic. Default + // to the surer tab, then auto-flip if it's empty so the operator + // doesn't see a blank list when only the heuristic tab has hits. + const [activeKind, setActiveKind] = useState("order_number"); // Track which specific record ids are mid-cancel so the operator // sees a spinner on the exact row they're cancelling, not on the // whole dialog. @@ -162,30 +173,61 @@ export function DuplicateOrdersDialog({ } }; + const numberGroups = groups.filter((g) => g.kind === "order_number"); + const aggregateGroups = groups.filter((g) => g.kind === "line_aggregate"); const totalGroups = groups.length; + // Auto-flip to whichever tab has content — the operator shouldn't + // land on an empty first tab when the heuristic tab is the only one + // with hits. The first render after `load()` runs from the same + // effect, so this is safe to compute inline. + const effectiveKind: DuplicateKind = + activeKind === "order_number" && numberGroups.length === 0 && aggregateGroups.length > 0 + ? "line_aggregate" + : activeKind; + const effectiveVisible = + effectiveKind === "order_number" ? numberGroups : aggregateGroups; + return ( - Duplicate order numbers + Duplicate orders {loading ? "Loading…" : totalGroups === 0 - ? "No duplicate order numbers on the order board." - : `${totalGroups} order number${totalGroups === 1 ? "" : "s"} appear on more than one active order. Click Cancel next to the duplicate you want to remove.`} + ? "No duplicate orders on the order board." + : `${numberGroups.length} shared order number${numberGroups.length === 1 ? "" : "s"} · ${aggregateGroups.length} matching line-aggregate${aggregateGroups.length === 1 ? "" : "s"}.`} - {!loading && groups.length > 0 && ( + {!loading && totalGroups > 0 && ( +
+ setActiveKind("order_number")} + /> + setActiveKind("line_aggregate")} + hint="Heuristic — same item count, total qty, and total selling price. Descriptions may differ." + /> +
+ )} + + {!loading && effectiveVisible.length > 0 && (
- {groups.map((group) => ( + {effectiveVisible.map((group) => ( )} + {!loading && totalGroups > 0 && effectiveVisible.length === 0 && ( +

+ No {activeKind === "order_number" ? "shared order numbers" : "matching line-aggregates"} on the order board. +

+ )} + + ); +} + function DuplicateGroupRow({ group, canceling, @@ -219,13 +299,18 @@ function DuplicateGroupRow({ return (
-
+
{group.orderNumber} - {group.records.length} copies + {group.records.length} {group.kind === "order_number" ? "copies" : "matching"} + {group.kind === "line_aggregate" && ( + + heuristic + + )}
    {sorted.map((r, idx) => { diff --git a/src/lib/server/orders-service.test.ts b/src/lib/server/orders-service.test.ts index a8e6fbc..c57a981 100644 --- a/src/lib/server/orders-service.test.ts +++ b/src/lib/server/orders-service.test.ts @@ -2010,23 +2010,232 @@ describe("findDuplicateOrders", () => { }); it("returns groups sorted by size (largest first)", async () => { - for (const [id, num] of [ - ["o-s1", "SOLO"], - ["o-s2", "SOLO"], - ["o-t1", "TRIPLE"], - ["o-t2", "TRIPLE"], - ["o-t3", "TRIPLE"], + // Use distinct quantities per order_number so the cross-number + // line-aggregate pass doesn't accidentally fire here — this test + // is about the order-number pass's sort order. + for (const [id, num, qty] of [ + ["o-s1", "SOLO", 1], + ["o-s2", "SOLO", 1], + ["o-t1", "TRIPLE", 2], + ["o-t2", "TRIPLE", 2], + ["o-t3", "TRIPLE", 2], ]) { await pbMock.collection("orders").create({ id, order_number: num, quote: "q", created: "2025-01-01T00:00:00Z", }); await pbMock.collection("order_items").create({ - id: `oi-${id}`, order: id, description: "x", item_status: "pending", + id: `oi-${id}`, order: id, description: "x", quantity: qty, item_status: "pending", }); } const groups = await findDuplicateOrders(); expect(groups.map((g) => g.orderNumber)).toEqual(["TRIPLE", "SOLO"]); }); + + // ────── line-aggregate ("Same line items") pass ────── + it("flags cross-order_number line-aggregate duplicates even when descriptions differ", async () => { + // Two orders under different order_numbers, same line breakdown + // (1 line, qty 5, R100). Descriptions and created-timestamps + // differ — the kind: "line_aggregate" pass should still flag it. + await pbMock.collection("orders").create({ + id: "o-a", order_number: "ORD-A", quote: "q", + created: "2025-01-01T00:00:00Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-a", order: "o-a", description: "M16 hex bolt 50mm grade 8.8", + quantity: 5, selling_price: 100, item_status: "pending", + }); + await pbMock.collection("orders").create({ + id: "o-b", order_number: "ORD-B", quote: "q", + created: "2025-01-02T00:00:00Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-b", order: "o-b", description: "M16 x 50 hex bolt", // different text + quantity: 5, selling_price: 100, item_status: "pending", + }); + + const groups = await findDuplicateOrders(); + const aggregateGroups = groups.filter((g) => g.kind === "line_aggregate"); + expect(aggregateGroups).toHaveLength(1); + expect(aggregateGroups[0].records.map((r) => r.id).sort()).toEqual(["o-a", "o-b"]); + expect(aggregateGroups[0].orderNumber).toMatch(/^1 line · 5 · R/); + }); + + it("does not double-list a same-number group under line-aggregate (cross-number filter)", async () => { + // Two records under the SAME order_number with matching content + // — already an order-number duplicate. The line-aggregate pass + // must NOT surface them again. + await pbMock.collection("orders").create({ + id: "o-1", order_number: "DUP", quote: "q", created: "2025-01-01T00:00:00Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-1", order: "o-1", description: "x", quantity: 5, selling_price: 100, + item_status: "pending", + }); + await pbMock.collection("orders").create({ + id: "o-2", order_number: "DUP", quote: "q", created: "2025-01-01T00:00:00.100Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-2", order: "o-2", description: "x", quantity: 5, selling_price: 100, + item_status: "invoiced", + }); + + const groups = await findDuplicateOrders(); + expect(groups).toHaveLength(1); + expect(groups[0].kind).toBe("order_number"); + expect(groups[0].records.map((r) => r.id)).toEqual(["o-1", "o-2"]); + }); + + it("excludes orders with zero items from line-aggregate matching", async () => { + // Empty orders would all match each other on (0 lines, 0 qty, R0) + // — must not be flagged. + await pbMock.collection("orders").create({ + id: "o-empty-1", order_number: "EMPTY-A", quote: "q", + }); + await pbMock.collection("orders").create({ + id: "o-empty-2", order_number: "EMPTY-B", quote: "q", + }); + + const groups = await findDuplicateOrders(); + expect(groups).toHaveLength(0); + }); + + it("rounds sub-cent price differences so 1499.998 and 1500.002 both match 1500.00", async () => { + await pbMock.collection("orders").create({ + id: "o-x", order_number: "X", quote: "q", created: "2025-01-01T00:00:00Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-x", order: "o-x", description: "x", quantity: 1, + selling_price: 1499.998, item_status: "pending", + }); + await pbMock.collection("orders").create({ + id: "o-y", order_number: "Y", quote: "q", created: "2025-01-02T00:00:00Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-y", order: "o-y", description: "x", quantity: 1, + selling_price: 1500.002, item_status: "pending", + }); + + const groups = await findDuplicateOrders(); + expect(groups.filter((g) => g.kind === "line_aggregate")).toHaveLength(1); + }); + + it("matches when both sides are entirely zero-priced", async () => { + await pbMock.collection("orders").create({ + id: "o-z1", order_number: "Z1", quote: "q", created: "2025-01-01T00:00:00Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-z1", order: "o-z1", description: "x", quantity: 5, + selling_price: 0, item_status: "pending", + }); + await pbMock.collection("orders").create({ + id: "o-z2", order_number: "Z2", quote: "q", created: "2025-01-02T00:00:00Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-z2", order: "o-z2", description: "x", quantity: 5, + selling_price: 0, item_status: "pending", + }); + + const groups = await findDuplicateOrders(); + expect(groups.filter((g) => g.kind === "line_aggregate")).toHaveLength(1); + }); + + it("does not match when one side is zero-priced and the other is not", async () => { + await pbMock.collection("orders").create({ + id: "o-p1", order_number: "P1", quote: "q", created: "2025-01-01T00:00:00Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-p1", order: "o-p1", description: "x", quantity: 1, + selling_price: 0, item_status: "pending", + }); + await pbMock.collection("orders").create({ + id: "o-p2", order_number: "P2", quote: "q", created: "2025-01-02T00:00:00Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-p2", order: "o-p2", description: "x", quantity: 1, + selling_price: 50, item_status: "pending", + }); + + const groups = await findDuplicateOrders(); + expect(groups).toHaveLength(0); + }); + + it("does not match when total quantity differs (everything else the same)", async () => { + await pbMock.collection("orders").create({ + id: "o-q1", order_number: "Q1", quote: "q", created: "2025-01-01T00:00:00Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-q1", order: "o-q1", description: "x", quantity: 5, + selling_price: 100, item_status: "pending", + }); + await pbMock.collection("orders").create({ + id: "o-q2", order_number: "Q2", quote: "q", created: "2025-01-02T00:00:00Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-q2", order: "o-q2", description: "x", quantity: 6, + selling_price: 100, item_status: "pending", + }); + + const groups = await findDuplicateOrders(); + expect(groups).toHaveLength(0); + }); + + it("does not match when line count differs (everything else the same)", async () => { + await pbMock.collection("orders").create({ + id: "o-c1", order_number: "C1", quote: "q", created: "2025-01-01T00:00:00Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-c1a", order: "o-c1", description: "x", quantity: 5, + selling_price: 100, item_status: "pending", + }); + await pbMock.collection("order_items").create({ + id: "oi-c1b", order: "o-c1", description: "y", quantity: 5, + selling_price: 100, item_status: "pending", + }); + await pbMock.collection("orders").create({ + id: "o-c2", order_number: "C2", quote: "q", created: "2025-01-02T00:00:00Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-c2", order: "o-c2", description: "x", quantity: 10, + selling_price: 100, item_status: "pending", + }); + + const groups = await findDuplicateOrders(); + expect(groups).toHaveLength(0); + }); + + it("computes totalSellingPrice as the sum of selling_price × quantity per line", async () => { + // 2 lines: qty 2 @ R50 = R100, qty 3 @ R100 = R300 → total R400. + // qty 5, R400. Match against another order with same aggregates. + await pbMock.collection("orders").create({ + id: "o-m1", order_number: "M1", quote: "q", created: "2025-01-01T00:00:00Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-m1a", order: "o-m1", description: "a", quantity: 2, selling_price: 50, + item_status: "pending", + }); + await pbMock.collection("order_items").create({ + id: "oi-m1b", order: "o-m1", description: "b", quantity: 3, selling_price: 100, + item_status: "pending", + }); + await pbMock.collection("orders").create({ + id: "o-m2", order_number: "M2", quote: "q", created: "2025-01-02T00:00:00Z", + }); + await pbMock.collection("order_items").create({ + id: "oi-m2a", order: "o-m2", description: "a", quantity: 2, selling_price: 50, + item_status: "pending", + }); + await pbMock.collection("order_items").create({ + id: "oi-m2b", order: "o-m2", description: "b", quantity: 3, selling_price: 100, + item_status: "pending", + }); + + const groups = await findDuplicateOrders(); + const aggs = groups.filter((g) => g.kind === "line_aggregate"); + expect(aggs).toHaveLength(1); + expect(aggs[0].orderNumber).toMatch(/^2 lines · 5 · R/); + expect(aggs[0].records[0].totalQuantity).toBe(5); + expect(aggs[0].records[0].totalSellingPrice).toBeCloseTo(400, 2); + }); }); diff --git a/src/lib/server/orders-service.ts b/src/lib/server/orders-service.ts index a1c3b80..f65dc20 100644 --- a/src/lib/server/orders-service.ts +++ b/src/lib/server/orders-service.ts @@ -2273,10 +2273,40 @@ export interface DuplicateOrderRecord { itemCount: number; itemStatusSummary: Record; items: Array<{ description: string; quantity: number | null; item_status: string }>; + /** + * Sum of `quantity` across all line items on this order. Used by the + * line-aggregate duplicate pass to flag orders where the line totals + * (count + qty + selling-price) are identical even though descriptions + * differ or `order_number` was re-used. Null when the order has no + * items — such orders are excluded from line-aggregate matching. + */ + totalQuantity: number | null; + /** + * Sum of `selling_price * quantity` across all lines, rounded to 2 + * decimal places. Same use as `totalQuantity`. Null when the order + * has no items. + */ + totalSellingPrice: number | null; } +export type DuplicateMatchKind = "order_number" | "line_aggregate"; + export interface DuplicateOrderGroup { + /** + * What the records under this group have in common. + * - "order_number": same `order_number` (the existing intake-race case). + * - "line_aggregate": same `(itemCount, totalQuantity, totalSellingPrice)` + * — flagged as a likely duplicate because the chance of two distinct + * orders sharing an identical line breakdown is small. + */ + kind: DuplicateMatchKind; + /** The shared identifier — order_number, or aggregate label. */ orderNumber: string; + /** + * Human-readable aggregate label for `kind: "line_aggregate"` + * (e.g. "5 lines · 100 · R1,500.00"). Absent for `order_number` groups. + */ + aggregateKey?: string; records: DuplicateOrderRecord[]; } @@ -2339,61 +2369,159 @@ export async function findDuplicateOrders(): Promise { // Group by order_number. Only keep groups with ≥2 records (otherwise // a singleton can't be a duplicate). - const byNum = new Map(); + const out: DuplicateOrderGroup[] = []; + const allRecords: DuplicateOrderRecord[] = []; + + // Fetch items + build a record for every active order up front so + // both the order-number pass and the line-aggregate pass can read + // from the same flat list. Without this, records that don't share + // an order_number (the line-aggregate-only case) would have no + // items fetched and skip the aggregate pass. for (const o of orders) { - const arr = byNum.get(o.order_number) ?? []; - arr.push(o); - byNum.set(o.order_number, arr); + const items = await loadOrderItemsForRecord(pb, o.id); + allRecords.push(buildRecord(o, items)); } - const out: DuplicateOrderGroup[] = []; - for (const [orderNumber, arr] of byNum) { + // Order-number pass. Only groups with ≥2 records qualify as a + // duplicate (a singleton can't be a duplicate). + const byNum = new Map(); + for (const r of allRecords) { + const arr = byNum.get(r.orderNumber) ?? []; + arr.push(r); + byNum.set(r.orderNumber, arr); + } + for (const [orderNumber, records] of byNum) { + if (records.length < 2) continue; + sortRecordsByCreated(records); + out.push({ kind: "order_number", orderNumber, records }); + } + + // Line-aggregate pass. Flags orders whose line breakdown + // (count + total qty + total selling price) matches across ≥2 + // distinct order_numbers. Descriptions may differ but the line + // totals staying identical is rare and a strong dupe signal. + // + // Indexed by `"count|qty|price"` (2-dp on price; qty rounded to int). + // Records with no items are excluded — otherwise every empty draft + // would match every other empty draft. + const byAggregate = new Map(); + for (const r of allRecords) { + if (r.itemCount === 0) continue; + const key = `${r.itemCount}|${r.totalQuantity}|${r.totalSellingPrice}`; + const arr = byAggregate.get(key) ?? []; + arr.push(r); + byAggregate.set(key, arr); + } + for (const [key, arr] of byAggregate) { if (arr.length < 2) continue; - - const records: DuplicateOrderRecord[] = []; - for (const o of arr) { - const items = await pb - .collection(getOrderItemsCollectionName()) - .getFullList<{ description: string; quantity: number | null; item_status: string }>({ - filter: `order = "${o.id}"`, - fields: "description,quantity,item_status", - batch: 500, - }); - const summary: Record = {}; - for (const it of items) { - summary[it.item_status] = (summary[it.item_status] || 0) + 1; - } - records.push({ - id: o.id, - orderNumber: o.order_number, - created: o.created, - customer: o.customer || null, - company: o.company || null, - customerName: o.expand?.customer?.customer ?? null, - companyName: o.expand?.company?.company ?? null, - itemCount: items.length, - itemStatusSummary: summary, - items: items.map((i) => ({ - description: i.description, - quantity: i.quantity, - item_status: i.item_status, - })), - }); - } - - // Sort by created asc — Date.getTime() rather than localeCompare - // because ISO 8601 sorts incorrectly when the fractional-second - // part is missing (".050Z" vs "Z" — code-point bug). - records.sort((a, b) => new Date(a.created).getTime() - new Date(b.created).getTime()); - - out.push({ orderNumber, records }); + // Skip groups where every record shares one `order_number` — + // that's already covered by the order-number pass, and listing + // them again here would double-flag. + const orderNumbers = new Set(arr.map((r) => r.orderNumber)); + if (orderNumbers.size < 2) continue; + const [countStr, qtyStr, priceStr] = key.split("|"); + const aggregateKey = `${countStr} line${countStr === "1" ? "" : "s"} · ${qtyStr} · R${formatRand(Number(priceStr))}`; + sortRecordsByCreated(arr); + out.push({ kind: "line_aggregate", orderNumber: aggregateKey, aggregateKey, records: arr }); } // Largest groups first — the most painful to fix manually. + // Stable so order-number groups keep their relative order before + // line-aggregate groups of the same size. out.sort((a, b) => b.records.length - a.records.length); return out; } +function sortRecordsByCreated(records: DuplicateOrderRecord[]): void { + // Sort by created asc — Date.getTime() rather than localeCompare + // because ISO 8601 sorts incorrectly when the fractional-second + // part is missing (".050Z" vs "Z" — code-point bug). + records.sort( + (a, b) => new Date(a.created).getTime() - new Date(b.created).getTime(), + ); +} + +/** Round to two decimal places without floating-point drift. */ +function round2(n: number): number { + return Math.round(n * 100) / 100; +} + +/** Format a number as Rand — R1,500.00. Used for the aggregate label. */ +function formatRand(n: number): string { + return n.toLocaleString("en-ZA", { + minimumFractionDigits: 2, + maximumFractionDigits: 2, + }); +} + +type AggregateItemShape = { + description: string; + quantity: number | null; + item_status: string; + selling_price?: number | null; +}; + +type OrderHeaderShape = { + id: string; + order_number: string; + created: string; + customer?: string; + company?: string; + expand?: { + customer?: { id: string; customer: string }; + company?: { id: string; company: string }; + }; +}; + +async function loadOrderItemsForRecord( + pb: AdminPocketBase, + orderId: string, +): Promise { + return pb + .collection(getOrderItemsCollectionName()) + .getFullList({ + filter: `order = "${orderId}"`, + fields: "description,quantity,item_status,selling_price", + batch: 500, + }); +} + +function buildRecord( + order: OrderHeaderShape, + items: AggregateItemShape[], +): DuplicateOrderRecord { + const summary: Record = {}; + let totalQty = 0; + let totalSellingPrice = 0; + for (const it of items) { + summary[it.item_status] = (summary[it.item_status] || 0) + 1; + const qty = Number(it.quantity ?? 0); + const price = Number(it.selling_price ?? 0); + if (Number.isFinite(qty)) totalQty += qty; + if (Number.isFinite(price)) totalSellingPrice += price * qty; + } + return { + id: order.id, + orderNumber: order.order_number, + created: order.created, + customer: order.customer || null, + company: order.company || null, + customerName: order.expand?.customer?.customer ?? null, + companyName: order.expand?.company?.company ?? null, + itemCount: items.length, + itemStatusSummary: summary, + items: items.map((i) => ({ + description: i.description, + quantity: i.quantity, + item_status: i.item_status, + })), + // Null totals when the order has no items — those rows are excluded + // from line-aggregate matching. + totalQuantity: items.length === 0 ? null : Math.round(totalQty), + totalSellingPrice: items.length === 0 ? null : round2(totalSellingPrice), + }; +} + // ────────────────────────────────────────────────────────────────────── // Stats & summary helpers (Phase H — orders MCP coverage expansion) // ──────────────────────────────────────────────────────────────────────