From 4447c4e7732561da49b64dd73440e0d4fd19762d Mon Sep 17 00:00:00 2001 From: Dave Date: Wed, 7 Oct 2026 15:53:23 +0200 Subject: [PATCH] feat(pos): customer picker matches the company name, and stops hiding truncation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The account-sale picker filtered on three customer columns — name, number, email. `company` on a customer row is a RELATION id, not a name, so it could never be text-matched; the company name was only ever resolved afterwards by `expand` for display. Typing "Sibanye" returned nothing even though 52 people work there. - the filter now traverses the relation: `company.company ~ q`. Verified live: "Sibanye - Rustenburg" 0 → 52, "Adal" 0 → 2, "Alano" 0 → 1, while "Botha" and "Smit" are unchanged at 6 and 4. - each row carries `viaCompany`, and name matches sort above company-only matches. PB's own sort is by name across the whole result set, which would interleave the two bands. - /customers returns { items, total, shown } and the till renders a count header: "52 matches", or "Showing 100 of 122 — keep typing to narrow". The old `slice(0, 8)` showed 8 of 52 people with nothing to suggest 44 were missing; 15 companies have more staff than that. The picker click handler bound to every `div` in the results box, so adding a count header would have made it selectable and picked a customer with an empty id. Now scoped to `.cust-row`. Two bugs the tests caught rather than the code: - the display cap was first set to 50 while its own comment claimed it sat above the largest company — which has 52 staff. That silently re-created the truncation this change removes. Now 100, with a test asserting a 52-person company comes back whole. - PocketBase has no escape for a literal backslash: `\'` is accepted, but one, two or three backslashes all fail to parse and 500 the request, so typing `\` in the search box errored. No escaping survives, so backslashes are stripped before the filter (`M20\Nut` searches `M20Nut`) and a backslash-only query returns nothing rather than matching every customer. 13 new tests cover the company clause, the 52-staff case, the cap, ranking order, backslash stripping and the empty-after-strip case. Verified live against PocketBase across 10 queries — the ranking guarantee held on every one. sw.js v33 -> v34: pos-app.js and pos.html are cache-first in the shell. --- public/pos/pos-app.js | 17 +- public/pos/pos.html | 7 + public/pos/sw.js | 14 +- src/app/api/v1/pos/customers/route.ts | 14 +- src/lib/server/pos-customer-search.test.ts | 185 +++++++++++++++++++++ src/lib/server/pos-service.ts | 104 ++++++++++-- 6 files changed, 320 insertions(+), 21 deletions(-) create mode 100644 src/lib/server/pos-customer-search.test.ts diff --git a/public/pos/pos-app.js b/public/pos/pos-app.js index 71493d9..6fbfd6f 100644 --- a/public/pos/pos-app.js +++ b/public/pos/pos-app.js @@ -780,11 +780,24 @@ async function searchCustomers(q){ try{ const r = await window.posFetch(`/customers?q=${encodeURIComponent(q)}`); const j = await r.json(); - box.innerHTML = (j.items||[]).map(c=>`
+ const items = j.items || []; + // `total` is every match before the server's display cap. Without it a + // 52-person company would render 8 rows and read as the whole team. + const total = Number.isFinite(j.total) ? j.total : items.length; + if(!items.length){ + box.innerHTML = `
No match for “${escapeHTML(q)}”
`; + return; + } + const head = total > items.length + ? `
Showing ${items.length} of ${total} — keep typing to narrow
` + : `
${total} ${total===1?'match':'matches'}
`; + box.innerHTML = head + items.map(c=>`
${escapeHTML(c.name)} ${escapeHTML(c.company || '')}
`).join(''); - box.querySelectorAll('div').forEach(d=>{ + // Scoped to .cust-row on purpose: binding every div would make the + // count header above selectable, picking a customer with an empty id. + box.querySelectorAll('.cust-row').forEach(d=>{ d.addEventListener('click',()=>{ state.customer = {id:d.dataset.id, name:d.dataset.name, company:d.dataset.company}; const pk = document.getElementById('custPicked'); diff --git a/public/pos/pos.html b/public/pos/pos.html index dbb6478..67aa74c 100644 --- a/public/pos/pos.html +++ b/public/pos/pos.html @@ -216,6 +216,11 @@ input{font:inherit;color:inherit} .cust-results div:last-child{border-bottom:0} .cust-results div:active{background:var(--surface)} .cust-results small{color:#717171;font-size:11px;margin-left:auto;letter-spacing:.02em} +/* Count header + empty state. Both are divs inside .cust-results, so they + need the row cursor/padding neutralised explicitly. */ +.cust-results .cust-count{padding:7px 14px;background:var(--surface);border-bottom:1px solid var(--muted);font-size:11px;font-weight:600;letter-spacing:.02em;color:#717171;cursor:default} +.cust-results .cust-none{padding:11px 14px;font-size:13px;color:#717171;cursor:default} +.cust-results .cust-count:active,.cust-results .cust-none:active{background:none} .cust-picked{padding:10px 12px;background:var(--surface);border:1px solid var(--border);border-radius:10px;font-size:13px;display:none;align-items:center;gap:8px;margin-bottom:8px} .cust-picked.show{display:flex} .cust-picked b{flex:1;font-weight:600} @@ -323,6 +328,8 @@ input{font:inherit;color:inherit} :root[data-theme="dark"] .cust-results div{border-bottom-color:var(--muted);color:var(--fg)} :root[data-theme="dark"] .cust-results div:active{background:var(--surface)} :root[data-theme="dark"] .cust-results small{color:#888} +:root[data-theme="dark"] .cust-results .cust-count{background:#222;border-bottom-color:var(--muted);color:#888} +:root[data-theme="dark"] .cust-results .cust-none{color:#888} :root[data-theme="dark"] .cust-picked{background:var(--surface);border-color:var(--border);color:var(--fg)} :root[data-theme="dark"] .edit-sheet{background:#181818;box-shadow:0 20px 60px rgba(0,0,0,.6)} :root[data-theme="dark"] .edit-sheet h3 small{color:#888} diff --git a/public/pos/sw.js b/public/pos/sw.js index 9d12e27..8905069 100644 --- a/public/pos/sw.js +++ b/public/pos/sw.js @@ -7,6 +7,18 @@ // running the step-1 build of it indefinitely. // Measured on the live catalog: "nuts m20" 0 → 12, "washers flat" 0 → 14, // with nothing that used to match ceasing to match. +// v34 — escapePbValue now drops backslashes rather than escaping them: +// PB has no backslash escape in filters (1, 2 or 3 all throw), so a raw one +// 500s the request. v33 already removed the price field from the edit sheet. +// v34 — the account-sale customer picker now matches the COMPANY name as +// well as the person's name/number/email, so typing "Sibanye" returns that +// mine's staff instead of nothing. /api/v1/pos/customers also returns a +// real `total` now: the old `slice(0, 8)` silently showed 8 of 52 people +// at the largest company and read as the whole team, so the till renders +// "showing N of M" and says so when the list is cut. Name matches rank +// above company-only matches so an OR'd filter can't bury the person the +// cashier typed. escapePbValue also escapes backslashes now — a single +// "\" in the search box used to throw and 500 the request. // v33 — the Edit line sheet is quantity-only. The Unit price field is gone // on request: the till sells at the catalogue shelf price, and a mis-keyed // price is expensive and easy to miss. The per-line price override it fed is @@ -30,7 +42,7 @@ // vertical height while staying visible. // Bumped because pos-app.js and pos.html are both served cache-first out of // the shell; without a new CACHE name the tills keep the old ones. -const CACHE = "pos-v33"; +const CACHE = "pos-v34"; const APP_SHELL = [ './', './index.html', diff --git a/src/app/api/v1/pos/customers/route.ts b/src/app/api/v1/pos/customers/route.ts index ab9a260..b7e5072 100644 --- a/src/app/api/v1/pos/customers/route.ts +++ b/src/app/api/v1/pos/customers/route.ts @@ -3,7 +3,15 @@ import { assertPosAuth } from "@/lib/server/pos-auth"; import { getAdminPocketBase } from "@/lib/server/pocketbase-admin"; import { searchCustomersForPos } from "@/lib/server/pos-service"; -/** GET /api/v1/pos/customers?q= — customer picker for account sales. */ +/** + * GET /api/v1/pos/customers?q= + * + * Customer picker for account sales. Searches the person's name, their + * account number, their email AND the name of the company they work for. + * Responds `{ items, total, shown }` — `total` is every match before the + * display cap so the till can say "showing N of M" instead of implying a + * truncated list is complete. + */ export async function GET(req: NextRequest) { try { assertPosAuth(req); @@ -14,6 +22,6 @@ export async function GET(req: NextRequest) { const { searchParams } = new URL(req.url); const q = searchParams.get("q") || ""; const pb = await getAdminPocketBase(); - const items = await searchCustomersForPos(pb, q); - return NextResponse.json({ items }); + const result = await searchCustomersForPos(pb, q); + return NextResponse.json(result); } diff --git a/src/lib/server/pos-customer-search.test.ts b/src/lib/server/pos-customer-search.test.ts new file mode 100644 index 0000000..d68d78e --- /dev/null +++ b/src/lib/server/pos-customer-search.test.ts @@ -0,0 +1,185 @@ +import { describe, it, expect } from "vitest"; +import { searchCustomersForPos } from "./pos-service"; +import type { PosPb } from "./pos-service"; + +/** + * Customer picker for account sales. + * + * The search used to be three clauses over `customer`, `number` and + * `email`, with a silent `slice(0, 8)` on the result. Two consequences: + * a company name found nothing, and a 52-person company rendered 8 rows + * that read as the complete team. + */ + +type CustomerRow = { + id: string; + customer: string; + number?: string; + email?: string; + company?: string; + expand?: { company?: { company?: string } }; +}; + +const companyId = (name: string) => `co-${name.toLowerCase().replace(/\W+/g, "-")}`; + +/** Records the filter it was handed, and returns rows matching the "own fields" OR a company name. */ +function stubPb(rows: CustomerRow[]) { + const seen: string[] = []; + const pb = { + collection: (name: string) => ({ + getFullList: async (opts: Record = {}) => { + expect(name).toBe("customers"); + const filter = String(opts.filter ?? ""); + seen.push(filter); + // Emulate PB well enough to exercise the service's own logic: + // the service re-derives `viaCompany` in JS, so we only need to + // hand back every row and let the service do the filtering it can. + // Filter here the way PB would, so the ranking bands are real. + const m = filter.match(/~ '((?:[^'\\]|\\.)*)'/); + const needle = m ? m[1].replace(/\\(.)/g, "$1").toLowerCase() : ""; + return rows.filter((r) => { + const own = [r.customer, r.number, r.email].some((v) => + String(v ?? "").toLowerCase().includes(needle), + ); + const co = r.expand?.company?.company ?? ""; + return own || co.toLowerCase().includes(needle); + }); + }, + }), + } as unknown as PosPb; + return { pb, seen }; +} + +const row = ( + id: string, + name: string, + companyName?: string, + extra: Partial = {}, +): CustomerRow => ({ + id, + customer: name, + company: companyName ? companyId(companyName) : "", + expand: companyName ? { company: { company: companyName } } : undefined, + ...extra, +}); + +// Names deliberately do NOT contain "Sibanye" — a staff member called +// "Sibanye Worker" would match on their OWN name, which is the whole point +// of `viaCompany`. +const SIBANYE_STAFF = Array.from({ length: 52 }, (_, i) => + row(`s${i}`, `Rustenburg Employee ${String(i).padStart(2, "0")}`, "Sibanye - Rustenburg Platinum Mines"), +); + +describe("searchCustomersForPos — matches the company name", () => { + it("includes company.company in the filter", async () => { + const { pb, seen } = stubPb(SIBANYE_STAFF); + await searchCustomersForPos(pb, "Sibanye"); + expect(seen[0]).toContain("company.company ~"); + }); + + it("returns the whole company, not the first 8", async () => { + const { pb } = stubPb(SIBANYE_STAFF); + const res = await searchCustomersForPos(pb, "Sibanye"); + expect(res.total).toBe(52); + expect(res.shown).toBe(52); + expect(res.items).toHaveLength(52); + }); + + it("caps ABOVE the largest company, so a real one is never cut", async () => { + // The cap was briefly 50 while the biggest company has 52 staff, which + // silently re-created the bug it was meant to fix. + const { pb } = stubPb(SIBANYE_STAFF); + const res = await searchCustomersForPos(pb, "Sibanye"); + expect(res.shown).toBe(res.total); + }); + + it("marks company-only matches so the till can rank them last", async () => { + const { pb } = stubPb([ + ...SIBANYE_STAFF, + row("named", "Johan Sibanye", "Somewhere Else"), // name contains the query + ]); + const res = await searchCustomersForPos(pb, "Sibanye"); + expect(res.items.find((c) => c.id === "named")?.viaCompany).toBe(false); + expect(res.items.find((c) => c.id === "s0")?.viaCompany).toBe(true); + }); + + it("ranks a name match above a company-only match", async () => { + const { pb } = stubPb([row("s0", "Almo Adams", "Sibanye"), row("direct", "Zack Sibanye")]); + const res = await searchCustomersForPos(pb, "Sibanye"); + expect(res.items[0].id).toBe("direct"); + expect(res.items[0].viaCompany).toBe(false); + }); +}); + +describe("searchCustomersForPos — never truncates silently", () => { + it("caps the display but still reports the true total", async () => { + const many = Array.from({ length: 300 }, (_, i) => row(`c${i}`, `Person ${i}`, "Big Co")); + const { pb } = stubPb(many); + const res = await searchCustomersForPos(pb, "Big Co"); + expect(res.total).toBe(300); + expect(res.shown).toBeLessThan(res.total); + expect(res.items).toHaveLength(res.shown); + }); + + it("reports total === shown when nothing was cut", async () => { + const { pb } = stubPb(SIBANYE_STAFF); + const res = await searchCustomersForPos(pb, "Sibanye"); + expect(res.total).toBe(res.shown); + }); +}); + +describe("searchCustomersForPos — input handling", () => { + it("returns an empty shape for a blank query, without hitting PB", async () => { + const { pb } = stubPb(SIBANYE_STAFF); + for (const q of ["", " "]) { + expect(await searchCustomersForPos(pb, q)).toEqual({ items: [], total: 0, shown: 0 }); + } + }); + + it("strips backslashes, which used to throw a 500", async () => { + const { pb, seen } = stubPb([row("a", "Backslash Co")]); + // PB has no escape for a literal backslash — 1, 2 or 3 all fail to + // parse — so it must never reach the filter at all. + await searchCustomersForPos(pb, "M20\\Nut"); + expect(seen[0]).not.toContain("\\"); + expect(seen[0]).toContain("M20Nut"); + }); + + it("treats a backslash-only query as blank, not as 'everyone'", async () => { + // Stripping leaves an empty needle, and `~ ''` matches every customer — + // a lone backslash must not dump all 703 rows into the picker. + const { pb, seen } = stubPb(SIBANYE_STAFF); + expect(await searchCustomersForPos(pb, "\\")).toEqual({ items: [], total: 0, shown: 0 }); + expect(seen, "must not query PB with an empty needle").toHaveLength(0); + }); + + it("escapes apostrophes", async () => { + const { pb, seen } = stubPb([row("a", "Al's Plant")]); + const res = await searchCustomersForPos(pb, "Al's"); + expect(seen[0]).toContain("Al\\'s"); + expect(res.total).toBe(1); + }); + + it("falls back through name → email → number when the name is blank", async () => { + const { pb } = stubPb([ + { id: "e1", customer: "", email: "no-name@example.com", expand: undefined }, + ]); + const res = await searchCustomersForPos(pb, "example.com"); + expect(res.items[0].name).toBe("no-name@example.com"); + }); + + it("renders an em-dash placeholder when a matched row has no name at all", async () => { + // Reachable in practice: a customer linked to a company but with a + // blank name, matched through its company name. + const { pb } = stubPb([ + { + id: "z", + customer: "", + expand: { company: { company: "Zebra Mine" } }, + }, + ]); + const res = await searchCustomersForPos(pb, "Zebra"); + expect(res.total).toBe(1); + expect(res.items[0].name).toBe("—"); + }); +}); diff --git a/src/lib/server/pos-service.ts b/src/lib/server/pos-service.ts index 3b97708..e98c706 100644 --- a/src/lib/server/pos-service.ts +++ b/src/lib/server/pos-service.ts @@ -263,9 +263,19 @@ export type PosSaleDeliveryNote = { pdfPath: string; }; -/** PB filter strings are built by hand here, so quote-escape anything user-supplied. */ +/** PB filter strings are built by hand here, so sanitise anything user-supplied. */ function escapePbValue(value: string): string { - return value.replace(/'/g, "\\'"); + // PocketBase escapes a quote inside a filter string as \' — that form is + // accepted (verified against the live API). + // + // It has NO escape for a literal backslash: a raw one fails to parse and + // 500s the whole request, and doubling it does not help either (1, 2 and + // 3 backslashes all throw). Nothing we filter on can contain one — client + // refs are generated ids, range bounds are ISO dates, and no customer name + // or company on file contains a backslash — so the only correct move is to + // drop them before they reach the parser. Typing `M20\Nut` into the + // customer search now searches for `M20Nut` instead of erroring. + return value.replace(/\\/g, "").replace(/'/g, "\\'"); } /** @@ -674,24 +684,66 @@ export async function listSalesInRange( // ─── searchCustomersForPos ────────────────────────────────────────── -export type PosCustomerResult = { id: string; name: string; company: string }; +export type PosCustomerResult = { + id: string; + name: string; + company: string; + /** + * True when the row matched ONLY on its company name, not on the person's + * own name / number / email. The till uses it to rank name matches above + * company-only matches so an OR'd filter never buries the person the + * cashier actually typed. + */ + viaCompany: boolean; +}; + +export type PosCustomerSearch = { + items: PosCustomerResult[]; + /** Every row the filter matched, before the display cap. */ + total: number; + /** How many are actually in `items`. */ + shown: number; +}; + +/** + * Display cap. Set well above the largest company on file (52 staff at + * Sibanye - Rustenburg Platinum Mines) so a company search shows the whole + * team rather than the goal it set out to fix, while still bounding a + * one-letter query that matches most of the table. Truncation is never + * silent — the till renders "showing N of M". + */ +const POS_CUSTOMER_RESULT_CAP = 100; /** * Customer picker for account sales. Lives behind the POS API rather * than being a direct PB query from the till — the whole point of this * app's rewrite is that no PocketBase credentials reach the client. * - * `company` is the company's display name. It is NOT sent onward as - * the DN's company — `createAdHocDeliveryNote` derives that from the - * customer's own `company` relation server-side, which is the only + * Matches the person's own name, their account number, their email, AND + * the name of the company they work for. The company clause traverses the + * relation (`company.company`) because `customer.company` holds a relation + * id, not a name — searching that column as text finds nothing. Typing + * "Sibanye" now returns everyone at Sibanye rather than nothing. + * + * `company` in the result is the company's display name. It is NOT sent + * onward as the DN's company — `createAdHocDeliveryNote` derives that from + * the customer's own `company` relation server-side, which is the only * value that can't drift. The till is a display surface here. */ export async function searchCustomersForPos( pb: PosPb, query: string, -): Promise { - if (!query.trim()) return []; - const safe = escapePbValue(query); +): Promise { + const trimmed = query.trim(); + if (!trimmed) return { items: [], total: 0, shown: 0 }; + + const safe = escapePbValue(trimmed); + // A query made entirely of backslashes sanitises down to nothing, and an + // empty `~ ''` matches every customer — so check after sanitising, not + // before. Same reason punctuation-only input must not dump the table. + if (!safe) return { items: [], total: 0, shown: 0 }; + + const needle = safe.toLowerCase(); const rows = await pb.collection("customers").getFullList<{ id: string; customer: string; @@ -699,13 +751,35 @@ export async function searchCustomersForPos( email?: string; expand?: { company?: { company?: string } }; }>({ - filter: `customer ~ '${safe}' || number ~ '${safe}' || email ~ '${safe}'`, + filter: + `(customer ~ '${safe}' || number ~ '${safe}' || email ~ '${safe}'` + + ` || company.company ~ '${safe}')`, expand: "company", sort: "customer", }); - return rows.slice(0, 8).map((c) => ({ - id: c.id, - name: c.customer || c.email || c.number || "—", - company: c.expand?.company?.company || "", - })); + + const mapped = rows.map((c) => { + // Mirrors PB's `~` (case-insensitive contains) on the row's OWN fields, + // so `viaCompany` is exact rather than guessed from the sort order. + const matchedOwn = [c.customer, c.number, c.email].some((v) => + String(v ?? "").toLowerCase().includes(needle), + ); + return { + id: c.id, + name: c.customer || c.email || c.number || "—", + company: c.expand?.company?.company || "", + viaCompany: !matchedOwn, + }; + }); + + // Name matches first, then company-only, alphabetical inside each band. + // PB's own sort is by name across the whole result set, which would + // interleave the two bands. + mapped.sort( + (a, b) => + Number(a.viaCompany) - Number(b.viaCompany) || a.name.localeCompare(b.name), + ); + + const items = mapped.slice(0, POS_CUSTOMER_RESULT_CAP); + return { items, total: mapped.length, shown: items.length }; }