diff --git a/src/app/(app)/orders/page.tsx b/src/app/(app)/orders/page.tsx index ec97b14..1ef1245 100644 --- a/src/app/(app)/orders/page.tsx +++ b/src/app/(app)/orders/page.tsx @@ -62,10 +62,14 @@ import type { OrderItem, ItemStatus } from "@/lib/types"; * row is expanded, items are pre-filtered to the active tab group, with * a "Show all items" toggle to reveal every line. * - * Search matches across order_number, company name, customer name, any - * line item description / po_description, and (if the query is a plain - * number) any line's selling_price or cost_price. See - * `getOrdersDashboardData` in the server orders service for the field set. + * Search matches across order_number, company, customer, supplier name, + * SKU, contact name, rep, RFQ number, line item description / + * po_description / notes / invoice number, and — when the query looks + * numeric — any line's selling_price or cost_price. The candidate set is + * the ACTIVE order set: not-completed items plus anything completed in + * the last 30 days (`getActiveOrderIds`). See `getOrdersDashboardData` + * in the server orders service for the full field set and the + * price-matching rules. * ========================================================================== */ const EMPTY_COUNTS: Record = { @@ -409,11 +413,13 @@ export default function OrdersPage() { tabKeys={["new", "processing", "post_processing", "collection_pool", "collected", "completed"]} /> - {/* Search — matches order number, company, customer, item description, or item price */} + {/* Search — order/company/customer/supplier/SKU/description/price. + Covers live items plus anything completed in the last 30 days; + older work lives on /orders/archive. */}
handleSearchChange(e.target.value)} className="pl-9 pr-9" diff --git a/src/lib/server/orders-service.test.ts b/src/lib/server/orders-service.test.ts index edf717f..b4cd4fe 100644 --- a/src/lib/server/orders-service.test.ts +++ b/src/lib/server/orders-service.test.ts @@ -1629,6 +1629,183 @@ describe("listOrdersWithItems — search scope + per-tab match counts", () => { }); }); +describe("listOrdersWithItems — widened search field set", () => { + const daysAgo = (n: number) => + new Date(Date.now() - n * 24 * 60 * 60 * 1000).toISOString(); + + async function seedSupplier(name = "Afrisam", supplierId = "sup-afrisam") { + await pbMock.collection("suppliers").create({ id: supplierId, suppliername: name }); + } + + async function seedOrderWithItem( + orderId: string, + item: Record, + orderExtra: Record = {}, + ) { + await pbMock.collection("orders").create({ + id: orderId, + order_number: `ORD-${orderId}`, + quote: `q-${orderId}`, + created: daysAgo(2), + ...orderExtra, + }); + await pbMock.collection("order_items").create({ + id: `oi-${orderId}`, + order: orderId, + description: "Unobtainium fitting", + item_status: "processed", + ...item, + }); + } + + it("matches on supplier NAME, not the relation id (the headline ask)", async () => { + await seedSupplier(); + await seedOrderWithItem("o-sup", { supplier: "sup-afrisam" }); + await seedOrderWithItem("o-other", {}); + + const byName = await listOrdersWithItems({ search: "afrisam" }); + expect(byName.items.map((i) => i.order.id)).toEqual(["o-sup"]); + + // Partial supplier name — operators type fragments, not legal names. + const partial = await listOrdersWithItems({ search: "fris" }); + expect(partial.items.map((i) => i.order.id)).toEqual(["o-sup"]); + + // Case-insensitive, like every other text field. + const upper = await listOrdersWithItems({ search: "AFRISAM" }); + expect(upper.items.map((i) => i.order.id)).toEqual(["o-sup"]); + + // The bare PB record id must NOT match — that's not a workflow. + const byId = await listOrdersWithItems({ search: "sup-afrisam" }); + expect(byId.items).toHaveLength(0); + }); + + it("scopes supplier search to not-completed + completed in last 30 days", async () => { + await seedSupplier(); + // Live item — always in scope. + await seedOrderWithItem("o-live", { supplier: "sup-afrisam" }); + // Completed 5 days ago — inside the window. + await pbMock.collection("orders").create({ + id: "o-recent", order_number: "ORD-o-recent", quote: "q-r", created: daysAgo(9), + }); + await pbMock.collection("order_items").create({ + id: "oi-o-recent", order: "o-recent", description: "Fitting", + item_status: "delivered", delivered_at: daysAgo(5), supplier: "sup-afrisam", + }); + // Completed 40 days ago — outside the window, belongs to /orders/archive. + await pbMock.collection("orders").create({ + id: "o-stale", order_number: "ORD-o-stale", quote: "q-s", created: daysAgo(44), + }); + await pbMock.collection("order_items").create({ + id: "oi-o-stale", order: "o-stale", description: "Fitting", + item_status: "delivered", delivered_at: daysAgo(40), supplier: "sup-afrisam", + }); + + const found = await listOrdersWithItems({ search: "afrisam" }); + const ids = found.items.map((i) => i.order.id).sort(); + + expect(ids).toEqual(["o-live", "o-recent"]); + expect(ids).not.toContain("o-stale"); + }); + + it("matches on SKU", async () => { + await seedOrderWithItem("o-sku", { sku: "NSS-M16-A2" }); + + expect((await listOrdersWithItems({ search: "M16" })).items.map((i) => i.order.id)) + .toEqual(["o-sku"]); + expect((await listOrdersWithItems({ search: "nss-m16" })).items.map((i) => i.order.id)) + .toEqual(["o-sku"]); + }); + + it("matches prices forgivingly — prefixes, currency and separators", async () => { + await seedOrderWithItem("o-p1", { selling_price: 12.22, cost_price: 0 }); + await seedOrderWithItem("o-p2", { selling_price: 0, cost_price: 0 }); + + await seedOrderWithItem("o-p3", { selling_price: 1250, cost_price: 0 }); + await seedOrderWithItem("o-p4", { selling_price: 0, cost_price: 340 }); + + const ids = async (q: string) => + (await listOrdersWithItems({ search: q })).items.map((i) => i.order.id).sort(); + + // Truncated cents: "12.2" must find 12.22. + expect(await ids("12.2")).toEqual(["o-p1"]); + // Full precision still works. + expect(await ids("12.22")).toEqual(["o-p1"]); + // Currency symbol + thousands separator. + expect(await ids("R 1 250")).toEqual(["o-p3"]); + expect(await ids("1250")).toEqual(["o-p3"]); + // cost_price is searched too. + expect(await ids("340")).toEqual(["o-p4"]); + }); + + it("ignores 1–2 char numeric queries so a price substring doesn't match everything", async () => { + await seedOrderWithItem("o-noise", { selling_price: 1250, cost_price: 0 }); + + // "50" is a substring of 1250 — without the MIN_NUMERIC_SEARCH_CHARS + // guard every price on the board becomes a hit. + const short = await listOrdersWithItems({ search: "50" }); + expect(short.items).toHaveLength(0); + + // Three characters is enough to probe. + const long = await listOrdersWithItems({ search: "250" }); + expect(long.items.map((i) => i.order.id)).toEqual(["o-noise"]); + }); + + it("never matches zero-priced lines on a price probe", async () => { + await seedOrderWithItem("o-zero", { selling_price: 0, cost_price: 0 }); + const found = await listOrdersWithItems({ search: "1250" }); + expect(found.items).toHaveLength(0); + }); + + it("matches contact name, order description, RFQ number and rep", async () => { + await seedOrderWithItem("o-meta", {}, { + contact_name: "Mr Petrus Dreyer", + description: "Roof anchor retrofit", + rfq_number: "RFQ-99812", + rep: "Gary", + }); + + const ids = async (q: string) => + (await listOrdersWithItems({ search: q })).items.map((i) => i.order.id); + + expect(await ids("dreyer")).toEqual(["o-meta"]); + expect(await ids("roof anchor")).toEqual(["o-meta"]); + expect(await ids("rfq-99812")).toEqual(["o-meta"]); + expect(await ids("gary")).toEqual(["o-meta"]); + }); + + it("matches quantity exactly, never as a substring", async () => { + await seedOrderWithItem("o-q", { quantity: 15 }); + + const ids = async (q: string) => + (await listOrdersWithItems({ search: q })).items.map((i) => i.order.id); + + expect(await ids("15")).toEqual(["o-q"]); + // "5" is inside 15 — quantity is an exact compare so this must miss. + expect(await ids("5")).toEqual([]); + }); + + it("does not fetch supplier rows when no search is active", async () => { + await seedSupplier(); + await seedOrderWithItem("o-quiet", { supplier: "sup-afrisam" }); + + const original = pbMock.collection; + let supplierQueries = 0; + try { + pbMock.collection = ((name: string) => { + if (name === "suppliers") supplierQueries++; + return original(name); + }) as typeof pbMock.collection; + await listOrdersWithItems({ page: 1, perPage: 20 }); + } finally { + pbMock.collection = original; + } + + // Regression guard for the latency risk: the supplier lookup is + // gated behind an active search so plain page loads don't pay for it. + expect(supplierQueries).toBe(0); + }); +}); + describe("listOrdersWithItems — tab taxonomy completeness", () => { /** * Regression guard. The `ItemStatus` union and `TAB_GROUP_ITEM_STATUSES` diff --git a/src/lib/server/orders-service.ts b/src/lib/server/orders-service.ts index 0031acd..60d2101 100644 --- a/src/lib/server/orders-service.ts +++ b/src/lib/server/orders-service.ts @@ -13,7 +13,7 @@ import { } from "../item-status-ui"; import { resolveRepDisplayName } from "../order-rep"; import { summarizeOrderInvoices, type OrderInvoiceSummary } from "../order-invoices"; -import type { Customer, ItemStatus, Order, OrderItem, User } from "../types"; +import type { Company, Customer, ItemStatus, Order, OrderItem, Supplier, User } from "../types"; const ORDER_COLLECTION = "orders"; const QUOTE_COLLECTION = "quotes"; @@ -340,7 +340,9 @@ export async function listOrdersWithItems( const orderIds = ordersPage.items.map((order) => order.id); const [allItems, users] = await Promise.all([ - fetchOrderItemsForOrders(pb, orderIds), + // Search-only columns (sku / supplier / notes) ride along on the + // search path only — see fetchOrderItemsForOrders. + fetchOrderItemsForOrders(pb, orderIds, { includeSearchFields: Boolean(search) }), // Targeted rep lookup: only the user ids the loaded orders actually // reference. Replaces a `users.getFullList` (which pulled every // user every page load) with a small `id = ...` filter — typically @@ -348,6 +350,11 @@ export async function listOrdersWithItems( fetchRepUsersForOrders(pb, ordersPage.items), ]); + // Supplier ids → names, only while searching. Without this the + // operator can see a PO went to Afrisam but has no way to find the + // order by typing "afrisam" — the item column holds a PB record id. + const supplierNames = search ? await fetchSupplierNamesForItems(pb, allItems) : new Map(); + const usersById = new Map(users.map((user) => [user.id, user])); const itemsByOrder: Record = {}; for (const item of allItems) { @@ -356,7 +363,7 @@ export async function listOrdersWithItems( } const searchLower = search.toLowerCase(); - const numericMatch = isNumericOrderSearch(search) ? Number(search) : null; + const numericQuery = normalizeNumericSearch(search); const items: OrderWithItems[] = []; /** * Per-tab counts of DISTINCT orders matching the search — "how many @@ -383,7 +390,9 @@ export async function listOrdersWithItems( order, orderItems, searchLower, - numericMatch, + numericQuery, + supplierNames, + repName, ) ) { continue; @@ -638,6 +647,57 @@ async function fetchRepUsersForOrders( }); } +/** + * Resolves `order_items.supplier` relation ids to lower-cased supplier + * names for the search predicate. Returns a Map of supplierId → name. + * + * Why a targeted lookup and not `expand` on the order_items query: + * - `order_items.supplier` is a relation id, never a name, so a + * substring match against the raw column would only ever hit + * operators who paste a PB record id — not a real workflow. + * - Adding `supplier.suppliername` to the existing `expand` string + * makes PB issue an extra IN query per page load AND per realtime + * refetch, whether or not anyone is searching. + * So the fetch is scoped to the supplier ids actually present on the + * loaded items, and only runs while a search is active. + * + * Returns an empty Map when no items carry a supplier, which makes the + * whole lookup a no-op for orders that were never bought from a supplier. + */ +async function fetchSupplierNamesForItems( + pb: AdminPocketBase, + items: OrderItem[], +): Promise> { + const needed = new Set(); + for (const item of items) { + if (item.supplier) needed.add(item.supplier); + } + if (needed.size === 0) return new Map(); + + // Chunked for the same reason `fetchOrdersFilteredByIds` chunks: PB + // 400s on an inline `id = ...` filter past ~106 ids / ~3.5 KB. + const ids = [...needed]; + const chunks: string[][] = []; + for (let index = 0; index < ids.length; index += ID_FILTER_CHUNK_SIZE) { + chunks.push(ids.slice(index, index + ID_FILTER_CHUNK_SIZE)); + } + + const batches = await Promise.all( + chunks.map((chunk) => + pb.collection("suppliers").getFullList({ + filter: chunk.map((id) => `id = "${id.replace(/"/g, '\\"')}"`).join(" || "), + fields: "id,suppliername", + }), + ), + ); + + const names = new Map(); + for (const supplier of batches.flat()) { + names.set(supplier.id, (supplier.suppliername || "").toLowerCase()); + } + return names; +} + export async function getOrderItemStatusTabCounts( pbArg?: AdminPocketBase, options: { bypassCache?: boolean; includeArchivedTabs?: boolean } = {}, @@ -1145,6 +1205,7 @@ export async function getOrderReviewData(orderId: string): Promise<{ async function fetchOrderItemsForOrders( pb: AdminPocketBase, orderIds: string[], + options: { includeSearchFields?: boolean } = {}, ): Promise { if (orderIds.length === 0) return []; @@ -1152,10 +1213,16 @@ async function fetchOrderItemsForOrders( // predicate read. The `order_items` schema has many unbounded text // columns (description 5k, notes 5k, problem_notes 5k, ai_item_insights // 64k json, etc.) that the list view never displays but would all - // be pulled over the wire without this. `supplier` was an expand - // here historically but the list view doesn't render the supplier - // per line (that's the order-level PO concern, not the per-line - // card), so we drop it. + // be pulled over the wire without this. + // + // The list view doesn't render the supplier per line (that's the + // order-level PO concern, not the per-line card), so `supplier` used + // to be dropped outright. Search DOES match on it — an operator + // hunting "what did Afrisam supply for us?" has no other handle — + // so `sku` / `supplier` / `notes` are now fetched, but ONLY on the + // search path. Every normal page load and every realtime refetch + // keeps the lean field list, so the widened payload is paid for once + // per keystroke-debounce instead of on every render. const ITEM_FIELDS = [ "id", "order", @@ -1169,6 +1236,7 @@ async function fetchOrderItemsForOrders( // only carries the small `invoice` / `print` / `split` blobs. "customer_invoice_number", "order_item_details", + ...(options.includeSearchFields ? ["sku", "supplier", "notes"] : []), ].join(","); const chunks: string[][] = []; @@ -1190,37 +1258,139 @@ async function fetchOrderItemsForOrders( return batches.flat(); } -function isNumericOrderSearch(search: string): boolean { - return /^\d+(\.\d+)?$/.test(search) && !Number.isNaN(Number(search)); +/** + * Minimum characters for a bare numeric query to be treated as a PRICE + * probe. + * + * Price matching is substring-based on the number's plain digit string, + * which is what makes "12.2" find 12.22 and "1250" find 1250.00 — but it + * also means "50" is a substring of 1250, 350 and 50. A 1–2 character + * query would turn every price on the board into a hit, so short queries + * fall through to the text paths only (order_number, description, …), + * where short fragments are already normal and expected. + */ +const MIN_NUMERIC_SEARCH_CHARS = 3; + +/** + * Normalises a search box value into a price-probe string, or null when + * the query shouldn't be treated as numeric. + * + * Strips currency symbols, spaces and thousands separators so the shapes + * an operator actually types all land on the same value: + * + * "R 1 250.00" → "1250.00" "1 250" → "1250" + * "12.22" → "12.22" "R1250" → "1250" + * + * Returns null for a query with no digits ("afrisam" → null) and for + * anything shorter than MIN_NUMERIC_SEARCH_CHARS. + */ +function normalizeNumericSearch(search: string): string | null { + const stripped = search.replace(/[^0-9.]/g, ""); + if (!/\d/.test(stripped)) return null; + if (stripped.length < MIN_NUMERIC_SEARCH_CHARS) return null; + return stripped; } +/** + * Does a single price column match the normalised price probe? + * + * Substring on the plain digit string first — that is the forgiving + * behaviour operators expect ("12.2" should find 12.22). The float + * tolerance is kept as a fallback for values PB persisted with binary + * representation drift (12.220000000000001), where the string form + * wouldn't contain the query. + * + * Zero is excluded deliberately: zero-priced placeholder lines are + * common on un-costed orders and would match every price probe. + */ +function priceMatchesSearch( + value: number | null | undefined, + numericQuery: string, +): boolean { + if (typeof value !== "number" || Number.isNaN(value) || value === 0) return false; + if (String(value).includes(numericQuery)) return true; + const asNumber = Number(numericQuery); + if (Number.isNaN(asNumber)) return false; + return Math.abs(value - asNumber) < 0.001; +} + +/** + * The `/orders` search predicate — an in-memory OR across every field an + * operator can reasonably type. Anything added here must be present on + * the item/order row that was actually fetched; see + * `fetchOrderItemsForOrders`'s `includeSearchFields` gate for the + * item-level columns. + * + * Scope note: this runs over the ACTIVE order candidate set + * (`getActiveOrderIds`) — items that aren't finished, plus anything + * completed within COMPLETED_WINDOW_DAYS. So a supplier search inherits + * that same window for free: searching a supplier returns the orders we + * still have live with them plus the last 30 days of completed work, + * and nothing older (that lives on /orders/archive). + */ function orderMatchesDashboardSearch( order: Order, items: OrderItem[], searchLower: string, - numericMatch: number | null, + numericQuery: string | null, + supplierNames: Map, + repName: string, ): boolean { if ((order.order_number || "").toLowerCase().includes(searchLower)) return true; - if ((order.expand?.company?.company || "").toLowerCase().includes(searchLower)) { - return true; - } - if ((order.expand?.customer?.customer || "").toLowerCase().includes(searchLower)) { - return true; - } + + const company = order.expand?.company as Company | undefined; + const customer = order.expand?.customer as Customer | undefined; + + if ((company?.company || "").toLowerCase().includes(searchLower)) return true; + if ((company?.address || "").toLowerCase().includes(searchLower)) return true; + if ((company?.vendor || "").toLowerCase().includes(searchLower)) return true; + + if ((customer?.customer || "").toLowerCase().includes(searchLower)) return true; + if ((customer?.email || "").toLowerCase().includes(searchLower)) return true; + if ((customer?.alternative_email || "").toLowerCase().includes(searchLower)) return true; + if ((customer?.buyer_email || "").toLowerCase().includes(searchLower)) return true; + if ((customer?.number || "").toLowerCase().includes(searchLower)) return true; + + // `contact_name` is the person the order is actually for, which is + // often NOT the customer record's name — so a rep can search the name + // printed on the card. + if ((order.contact_name || "").toLowerCase().includes(searchLower)) return true; + if ((order.description || "").toLowerCase().includes(searchLower)) return true; + if ((order.notes || "").toLowerCase().includes(searchLower)) return true; + if ((order.rfq_number || "").toLowerCase().includes(searchLower)) return true; + if ((repName || "").toLowerCase().includes(searchLower)) return true; + + // Quantity is an EXACT compare, not a substring: "5" matching every + // line with 15 / 25 / 50 units is noise, not a search result. + const quantityProbe = searchLower.trim(); for (const item of items) { if ((item.description || "").toLowerCase().includes(searchLower)) return true; if ((item.po_description || "").toLowerCase().includes(searchLower)) return true; + if ((item.sku || "").toLowerCase().includes(searchLower)) return true; + if ((item.customer_invoice_number || "").toLowerCase().includes(searchLower)) { + return true; + } + if ((item.notes || "").toLowerCase().includes(searchLower)) return true; + + if (item.quantity !== null && item.quantity !== undefined) { + if (String(item.quantity).toLowerCase() === quantityProbe) return true; + } + + // Supplier is a relation id on the item; match against the resolved + // name. An item with a supplier id we couldn't resolve (deleted + // supplier) simply doesn't match — better than surfacing a record id. + if (item.supplier) { + const supplierName = supplierNames.get(item.supplier); + if (supplierName && supplierName.includes(searchLower)) return true; + } + + if (numericQuery !== null) { + if (priceMatchesSearch(item.selling_price, numericQuery)) return true; + if (priceMatchesSearch(item.cost_price, numericQuery)) return true; + } } - if (numericMatch !== null) { - const epsilon = 0.001; - return items.some( - (item) => - Math.abs(Number(item.selling_price || 0) - numericMatch) < epsilon || - Math.abs(Number(item.cost_price || 0) - numericMatch) < epsilon, - ); - } return false; }