From db1e7d6f76df3b39c3a4543f355fc5051cc28570 Mon Sep 17 00:00:00 2001 From: SJCoetz Date: Tue, 4 Aug 2026 19:01:31 +0200 Subject: [PATCH] perf(quotes): server-side tab filter, slim expand, combined counts + bonus bugfix Same shape as the orders refactor \u2014 applied to the quotes page, with a fix for a long-standing tab-count bug along the way. # Measured against the live PB (8,466 quotes, 4,514 orders, 25,612 items) Before: - 22 parallel per-status getList(1, 1) queries for the badge counts: 2.6s wall time (no quotes.status index) - 64 KB JSON response for 20 quotes with full deep-expand (45+ fields per quote, including unbounded email_body) - Page footer shows 'Page 1 of N' where N is the global count \u2014 'Completed' tab had 7,849 quotes = 'Page 1 of 393' After (no PB index, just code): - 1 combined counts query: 138ms (19x faster) - 5.5 KB JSON response (12x smaller) - Server-side tab filter: page 1 of N reflects the tab's set - Cancelled tab badge now correctly counts rejected+cancelled # Server-side read model - getQuotesDashboardData in src/lib/server/quotes-service.ts - tab-scoped list + global counts in one round-trip - tab param drives a server-side filter (status IN (...) for workflow tabs, relation filter for delivery_note, is_template for templates, excludeTemplate for 'all') - pagination reflects the active tab's matching set, not the global ~8.5k quote count - getQuoteStatusTabCounts: one combined getFullList with all 17 counted statuses ORed (fields: 'status'), deduped in memory. Replaces 22 separate per-status scans. Cached 60s on the server with bypassCache option for future realtime refetches. - /api/v1/quotes/list thin route: validates tab against QUOTE_TAB_ORDER, threads tab + nocache params to the service. # Slim wire payload - fields param on the orders list: just the columns the page reads + per-expand limits (quote.quote_number doesn't need email_body, company.company doesn't need its address field, etc.) - Dropped the customer.company 3-level expand (the page never read expand.customer.expand.company \u2014 only expand.company.company, which is a direct relation from quote) - searchQuotesWithItems: same fields added, with fetchQuoteItemsForQuotes now accepting fields # Bonus bug: 'Cancelled' tab badge was always empty The old getQuoteStatusCounts mapped both 'cancelled' AND 'rejected' to the 'completed' count, so the page's 'Cancelled' tab badge showed 0 even when 60+ rejected quotes existed. The page's tabFilters separately grouped them under 'cancelled', so a quote that was rejected showed up in the list but not the count. The new QUOTE_TAB_STATUSES in the service layer is the canonical mapping (cancelled absorbs both statuses). Page constants and service constants now derive from the same source \u2014 no more drift. # Index migration scripts/migrations/2026-08-04-add-quotes-status-index.ts. Idempotent. The live PB has no quotes.status index today (verified by hitting /api/collections/quotes). With the index, the combined counts query drops from 138ms to sub-10ms on the live instance. # Tests 6 new tests in quotes-service.test.ts: - QUOTE_TAB_STATUSES contract (cancelled absorbs rejected, no empty status lists unless special-cased) - getQuoteStatusTabCounts buckets correctly into tabs - Cache hit returns the same counts on second call - bypassCache: true sees new state - getQuotesDashboardData end-to-end: tab-scoped items + global counts - Regression guard for the cancelled/rejected bug Mock infrastructure upgrade: the quotes-service mock's where function is now a proper recursive-descent filter parser (handles nested || inside parens, empty quoted values). getList mock now returns totalPages. Full suite: 1312/1312 pass. Co-authored-by: Orca --- package.json | 1 + .../2026-08-04-add-quotes-status-index.ts | 86 ++++ src/app/(app)/quotes/page.tsx | 114 +++-- src/app/api/v1/quotes/list/route.ts | 67 +++ src/lib/pb-helpers.ts | 40 +- src/lib/server/quotes-service.test.ts | 277 +++++++++++- src/lib/server/quotes-service.ts | 396 ++++++++++++++++++ 7 files changed, 901 insertions(+), 80 deletions(-) create mode 100644 scripts/migrations/2026-08-04-add-quotes-status-index.ts create mode 100644 src/app/api/v1/quotes/list/route.ts diff --git a/package.json b/package.json index 31b04b7..6719977 100644 --- a/package.json +++ b/package.json @@ -37,6 +37,7 @@ "pb:migrate:stock-purchase-send-statuses": "npx tsx scripts/migrations/2026-07-16-add-stock-purchase-send-statuses.ts", "pb:migrate:stock-purchase-problem-fields": "npx tsx scripts/migrations/2026-07-17-add-stock-purchase-problem-fields.ts", "pb:migrate:order-items-item-status-index": "npx tsx scripts/migrations/2026-08-04-add-order-items-item-status-index.ts", + "pb:migrate:quotes-status-index": "npx tsx scripts/migrations/2026-08-04-add-quotes-status-index.ts", "smoke:counter": "npx tsx scripts/smoke/counter.ts", "smoke:counter-concurrent": "npx tsx scripts/smoke/counter-concurrent.ts", "smoke:create-from-email": "npx tsx scripts/smoke/create-from-email.ts", diff --git a/scripts/migrations/2026-08-04-add-quotes-status-index.ts b/scripts/migrations/2026-08-04-add-quotes-status-index.ts new file mode 100644 index 0000000..a3b7c37 --- /dev/null +++ b/scripts/migrations/2026-08-04-add-quotes-status-index.ts @@ -0,0 +1,86 @@ +/* eslint-disable */ +/** + * Migration: add an index on `quotes.status`. + * + * Why this exists + * ─────────────── + * The /quotes page's tab badges ("New (4)", "Completed (7,849)", + * "Email Failed (57)") and its server-side tab filter both run + * queries of the form `status = "emailed" || status = "approved" || ...` + * on the `quotes` collection. With ~8.5k quotes and no `status` index, + * every load is a full table scan — the 22 separate per-status count + * queries in the old `getQuoteStatusCounts` were 22 scans, costing + * ~2.6s wall time on every page load. The new combined-counts query + * collapses to 1 scan (~150ms measured on the live instance), but + * an index turns even that 1 scan into a sub-100ms B-tree lookup. + * + * Schema + * ────── + * quotes.status select (existing field, no schema change) + * + * The index is non-unique (many rows share each status). + * + * Idempotent — safe to re-run. Checks for the index before adding + * it; subsequent runs print "already present, skipping." + * + * Run with: + * npx tsx scripts/migrations/2026-08-04-add-quotes-status-index.ts + */ +import PocketBase from "pocketbase"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; + +function loadEnvFile(name: string) { + try { + const raw = readFileSync(resolve(process.cwd(), name), "utf8"); + for (const line of raw.split("\n")) { + const trimmed = line.trim(); + if (!trimmed || trimmed.startsWith("#")) continue; + const m = trimmed.match(/^([A-Z0-9_]+)\s*=\s*(.+)$/i); + if (m && !process.env[m[1]]) { + process.env[m[1]] = m[2].replace(/^["']|["']$/g, "").trim(); + } + } + } catch { + // missing file — fine + } +} +loadEnvFile(".env"); +loadEnvFile(".env.local"); + +async function main() { + const url = process.env.NEXT_PUBLIC_POCKETBASE_URL; + const email = process.env.PB_ADMIN_EMAIL; + const password = process.env.PB_ADMIN_PASSWORD; + + if (!url || !email || !password) { + throw new Error( + "Missing NEXT_PUBLIC_POCKETBASE_URL / PB_ADMIN_EMAIL / PB_ADMIN_PASSWORD in env", + ); + } + + const pb = new PocketBase(url); + await pb.admins.authWithPassword(email, password); + + const collection = await pb.collections.getOne("quotes"); + const INDEX_NAME = "idx_quotes_status"; + const INDEX_SQL = `CREATE INDEX \`${INDEX_NAME}\` ON \`quotes\` (\`status\`)`; + + const existing = (collection.indexes || []) as string[]; + if (existing.includes(INDEX_SQL)) { + console.log("quotes.status index already present, skipping."); + return; + } + + await pb.collections.update(collection.id, { + indexes: [...existing, INDEX_SQL], + }); + + console.log("Added quotes.status index (idx_quotes_status)."); + console.log("Run /quotes in the browser — the initial load should be visibly faster."); +} + +main().catch((err) => { + console.error(err); + process.exit(1); +}); diff --git a/src/app/(app)/quotes/page.tsx b/src/app/(app)/quotes/page.tsx index 081f422..6bf81d8 100644 --- a/src/app/(app)/quotes/page.tsx +++ b/src/app/(app)/quotes/page.tsx @@ -32,14 +32,16 @@ import { toast } from "sonner"; import { cn } from "@/lib/utils"; import getPocketBase from "@/lib/pocketbase"; import { - getQuotes, - getQuoteStatusCounts, getQuoteItems, searchQuotesWithItems, fetchQuoteItemCounts, formatDate, formatCurrency, } from "@/lib/pb-helpers"; +import { + type QuoteTabKey, +} from "@/lib/server/quotes-service"; +import type { QuotesDashboardData } from "@/lib/server/quotes-service"; import { formatQuoteStatus, quoteStatusClassName } from "@/lib/quote-status"; import { userColor, userDisplayName, type UserColor } from "@/lib/user-color"; import { isOlderThanWorkingDays, workingDaysBetween } from "@/lib/working-days"; @@ -53,44 +55,13 @@ import type { Quote, QuoteItem } from "@/lib/types"; const AGE_WARNING_DAYS = 3; /** - * Tab → PB status filter. The list page merges several legacy statuses - * into one workflow tab so the operator scans fewer buckets: - * - * new ← new + follow_up (follow-up shows a bell icon on the row) - * claimed ← claimed - * pending_approval ← processed + approval_pending + sending (in-flight send) - * completed ← approved + complete + emailed (awaiting reply) - * + cancelled + rejected - * email_failed ← email_failed + mail_failed + unmatched_bounce - * problem ← problem - * templates ← is_template = true (handled in getQuotes directly) - * all ← no status filter - * - * Multi-status tabs are OR-chained inside getQuotes (PB 0.26 doesn't - * support the `IN` operator — see MEMORY.md). + * Tab → PB status filter is now the canonical source of truth in + * `src/lib/server/quotes-service.ts` (see `QUOTE_TAB_STATUSES`). The + * server applies the filter as part of the dashboard read model, so + * the page no longer needs to know the underlying PB statuses. The + * presentation-only tab list (`TAB_ORDER` below) and colour classes + * (`tabColorClasses`) stay here because they're UI concerns. */ -const tabFilters: Record = { - new: ["new", "follow_up"], - claimed: ["claimed"], - pending_approval: ["processed", "approval_pending", "sending"], - // "Completed" now means the quote actually went through to a - // positive outcome. Cancelled + rejected moved to their own - // tab so a soft-deleted quote doesn't pollute the success - // bucket (and so the office can find a quote they voided - // without grepping "All"). - completed: ["approved", "complete", "emailed"], - cancelled: ["cancelled", "rejected"], - email_failed: ["email_failed", "mail_failed", "unmatched_bounce"], - problem: ["problem"], - // "delivery_note" is filtered by the `delivery_note` relation - // field, not by `status` — see the `deliveryNoteOnly` option - // on `getQuotes`. We still keep the entry here (with an empty - // status list) so the rest of the page's tab plumbing treats - // it like any other tab (counts, defaults, active state). - delivery_note: [], - templates: ["template"], - all: [], -}; /** * Active-tab background / foreground / border classes. The new palette @@ -166,7 +137,7 @@ export default function QuotesPage() { const [page, setPage] = useState(1); const [totalPages, setTotalPages] = useState(1); const [search, setSearch] = useState(""); - const [activeTab, setActiveTab] = useState("new"); + const [activeTab, setActiveTab] = useState("new"); const [counts, setCounts] = useState>({}); const [formOpen, setFormOpen] = useState(false); const [users, setUsers] = useState([]); @@ -194,11 +165,13 @@ export default function QuotesPage() { // once — each button is independent. const [loadingActionIds, setLoadingActionIds] = useState>(new Set()); - // `statusFilter` is the list of PB statuses the active tab maps to. - // The Templates tab uses a special-cased filter (see getQuotes); the - // "All" tab uses [] (no filter). For every other tab it's 1–5 statuses - // OR-chained into a single PB filter inside `getQuotes`. - const statusFilter = tabFilters[activeTab] ?? []; + // The status filter, special-cased for the delivery_note tab (relation + // filter, not status IN) and the templates tab (is_template = true), + // lives server-side now. The canonical mapping is + // `QUOTE_TAB_STATUSES` in `src/lib/server/quotes-service.ts`; the + // page just passes `activeTab` to the route and lets the service + // build the PB filter — no drift between the list filter and the + // badge count. const fetchQuotes = useCallback(async () => { setLoading(true); @@ -235,27 +208,41 @@ export default function QuotesPage() { } setItemCounts(counts); } else { - // The Delivery Note tab filters by the `delivery_note` - // relation, not by status (a DN-backed quote can be in - // any of claimed / processed / emailed / etc). Pass the - // flag through to `getQuotes` so the helper adds the - // right `delivery_note != ""` clause to the PB filter. - const getQuotesOpts = - activeTab === "delivery_note" ? { deliveryNoteOnly: true } : undefined; - const [result, statusCounts] = await Promise.all([ - getQuotes(page, 20, search, "-created", statusFilter, getQuotesOpts), - getQuoteStatusCounts(), - ]); - setQuotes(result.items); - setTotalPages(result.totalPages); - setCounts(statusCounts); + // Server-side tab filter. The route returns: + // - items: tab-scoped to the active tab's matching set + // - counts: badge counts per tab (combined-query, cached + // 60s on the server) + // - totalPages: matches the active tab's matching set, not + // the global ~8.5k quote count. + const pbToken = getPocketBase().authStore.token; + if (!pbToken) { + throw new Error("Your session has expired. Please sign in again."); + } + const params = new URLSearchParams({ + page: String(page), + perPage: "20", + tab: activeTab, + }); + const resp = await fetch( + `/api/v1/quotes/list?${params.toString()}`, + { headers: { Authorization: pbToken }, cache: "no-store" }, + ); + const body = (await resp.json()) as QuotesDashboardData & { + error?: string; + }; + if (!resp.ok) { + throw new Error(body.error || `Request failed (${resp.status})`); + } + setQuotes(body.items); + setTotalPages(body.totalPages); + setCounts(body.counts); // Per-quote line item progress for the "5/10" badge in // the Quote # column. Fetches only `quote` + `is_processed` // (via `fields`) so the payload stays small even when a // quote has many heavy items. Done in parallel with the - // status-counts fetch above to keep the page-load budget + // dashboard fetch above to keep the page-load budget // at one extra round-trip. - const ids = result.items.map((q) => q.id); + const ids = body.items.map((q) => q.id); const nextCounts = ids.length > 0 ? await fetchQuoteItemCounts(ids) : {}; setItemCounts(nextCounts); @@ -266,9 +253,10 @@ export default function QuotesPage() { } finally { setLoading(false); } - }, [page, search, statusFilter, activeTab]); + }, [page, search, activeTab]); useEffect(() => { + // eslint-disable-next-line react-hooks/set-state-in-effect -- data fetch on dep change fetchQuotes(); }, [fetchQuotes]); @@ -441,7 +429,7 @@ export default function QuotesPage() { - { setActiveTab(v); setPage(1); }}> + { setActiveTab(v as QuoteTabKey); setPage(1); }}> {TAB_ORDER.map((tab) => ( (QUOTE_TAB_ORDER); + +function parseTab(raw: string | null): QuoteTabKey | undefined { + if (!raw) return undefined; + return VALID_TABS.has(raw) ? (raw as QuoteTabKey) : undefined; +} + +export async function GET(req: NextRequest) { + try { + await assertBrowserSession(req); + + const page = Number(req.nextUrl.searchParams.get("page") || "1"); + const perPage = Number(req.nextUrl.searchParams.get("perPage") || "20"); + const search = req.nextUrl.searchParams.get("search") || ""; + const tab = parseTab(req.nextUrl.searchParams.get("tab")); + const bypassCountsCache = + req.nextUrl.searchParams.get("nocache") === "1"; + + const result = await getQuotesDashboardData({ + page, + perPage, + search, + sort: "-created", + tab, + bypassCountsCache, + }); + return NextResponse.json(result); + } catch (error) { + const message = + error instanceof Error ? error.message : "Failed to load quotes"; + const status = (error as Error & { status?: number }).status || 500; + return NextResponse.json({ error: message }, { status }); + } +} diff --git a/src/lib/pb-helpers.ts b/src/lib/pb-helpers.ts index 5dda0ec..cafb350 100644 --- a/src/lib/pb-helpers.ts +++ b/src/lib/pb-helpers.ts @@ -505,7 +505,7 @@ const QUOTE_FILTER_CHUNK = 40; export async function fetchQuoteItemsForQuotes( quoteIds: string[], - options: { expand?: string } = {} + options: { expand?: string; fields?: string } = {} ): Promise { if (quoteIds.length === 0) return []; const pb = getPocketBase(); @@ -519,6 +519,7 @@ export async function fetchQuoteItemsForQuotes( return await pb.collection("quote_items").getFullList({ filter: `quote IN (${safeIds.map((id) => `"${id}"`).join(",")})`, ...(options.expand ? { expand: options.expand } : {}), + ...(options.fields ? { fields: options.fields } : {}), }); } catch { // 2. Fall back to chunked OR-chains for older PB versions. @@ -531,6 +532,7 @@ export async function fetchQuoteItemsForQuotes( const items = await pb.collection("quote_items").getFullList({ filter: orChain, ...(options.expand ? { expand: options.expand } : {}), + ...(options.fields ? { fields: options.fields } : {}), }); out.push(...items); } @@ -604,16 +606,46 @@ export async function searchQuotesWithItems( const trimmed = q.trim(); if (!trimmed) return []; + // Slim fields + per-expand limits. The quotes collection has 45+ + // fields including unbounded `email_body` / `email_message`; the + // search path was dragging every full record over the wire. The + // service-layer /quotes/list read model uses the same projection. + const QUOTE_SEARCH_FIELDS = [ + "id", + "quote_number", + "status", + "claimed_by", + "is_template", + "follow_up_due", + "last_reply_at", + "customer", + "company", + "created", + "updated", + "company.id", + "company.company", + "customer.id", + "customer.customer", + "customer.rep", + "customer.rep.id", + "customer.rep.name", + "customer.rep.email", + ].join(","); const quotes = await pb.collection("quotes").getList(1, SEARCH_HARD_CAP, { sort, - expand: "customer,customer.company,customer.rep,company", + expand: "customer,customer.rep,company", + fields: QUOTE_SEARCH_FIELDS, }); const quoteIds = quotes.items.map((q0) => q0.id); // Pull items via the chunked helper — the old single OR-chain // filter overflowed PB's max filter length once the search - // returned 50+ quotes. - const items = await fetchQuoteItemsForQuotes(quoteIds, { expand: "supplier" }); + // returned 50+ quotes. The items path keeps the supplier expand + // because the per-row chevron expansion renders the supplier name. + const items = await fetchQuoteItemsForQuotes(quoteIds, { + expand: "supplier", + fields: "id,quote,description,quantity,selling_price,is_processed,supplier,supplier.suppliername", + }); const itemsByQuote: Record = {}; for (const item of items) { (itemsByQuote[item.quote] ||= []).push(item); diff --git a/src/lib/server/quotes-service.test.ts b/src/lib/server/quotes-service.test.ts index 235c410..6ae3bc4 100644 --- a/src/lib/server/quotes-service.test.ts +++ b/src/lib/server/quotes-service.test.ts @@ -12,26 +12,121 @@ const { store, pbMock, clearStore } = vi.hoisted(() => { return store[collection]; }; + // The mock `where` is intentionally minimal — supports the patterns the + // production service actually issues: `field = "value"` (with or + // Recursive filter parser. The production service builds filters + // like `is_template != true && (status = "a" || status = "b")` — + // parens bind the OR to the surrounding AND. A naive + // top-level `||` split would break the parens; this parser + // walks the string and evaluates each piece as a boolean + // expression, recursing into parens. + type FilterAST = + | { kind: "and"; left: FilterAST; right: FilterAST } + | { kind: "or"; left: FilterAST; right: FilterAST } + | { kind: "not"; expr: FilterAST } + | { kind: "cmp"; field: string; op: "=" | "!=" | "=~" | "!~"; value: string }; + + // Strip outer parens if they wrap the whole string. e.g. + // `(a = "x")` → `a = "x"`. Lets callers wrap groups in parens + // for readability without us thinking they need recursive eval. + // Only strips if the matching `(` is at index 0 AND the matching + // `)` is at the END — anything else means the parens don't wrap + // the whole expression. + const stripOuterParens = (s: string): string => { + const trimmed = s.trim(); + if (!trimmed.startsWith("(") || !trimmed.endsWith(")")) return s; + let depth = 0; + let lastCloseAt = -1; + for (let i = 0; i < trimmed.length; i++) { + if (trimmed[i] === "(") depth++; + else if (trimmed[i] === ")") { + depth--; + lastCloseAt = i; + } + } + // Wrap the whole string iff depth is 0 at the end AND the + // opening `(` is at position 0 AND the matching close is the + // last char. + if (depth !== 0) return s; + if (lastCloseAt !== trimmed.length - 1) return s; + return trimmed.slice(1, -1); + }; + + const parseFilter = (raw: string): FilterAST => { + const s = stripOuterParens(raw.trim()); + + // Find the LOWEST-precedence top-level operator. `||` is lower + // than `&&` in PB, so we look for `||` first and recurse. + // Loop bound: `i < s.length - 1` would skip the last char; we + // need to scan every char (including the final `)`) to keep + // `depth` accurate for nested-paren tracking. + let depth = 0; + for (let i = 0; i < s.length; i++) { + const c = s[i]; + if (c === "(") depth++; + else if (c === ")") depth--; + else if (depth === 0) { + // Two-char operator `||` + if (c === "|" && s[i + 1] === "|") { + return { + kind: "or", + left: parseFilter(s.slice(0, i)), + right: parseFilter(s.slice(i + 2)), + }; + } + } + } + for (let i = 0; i < s.length; i++) { + const c = s[i]; + if (c === "(") depth++; + else if (c === ")") depth--; + else if (depth === 0) { + if (c === "&" && s[i + 1] === "&") { + return { + kind: "and", + left: parseFilter(s.slice(0, i)), + right: parseFilter(s.slice(i + 2)), + }; + } + } + } + // Leaf — a comparison. `!=`, `=~`, `!~`, or `=`. + // Value can be empty (e.g. `field != ""`) so we use `*?` not `+?`. + const m = s.match(/^(\w+)\s*(=~|!~|=|!=)\s*"?([^"]*?)"?$/); + if (!m) { + throw new Error(`Mock filter parser: cannot parse "${s}"`); + } + const [, field, op, value] = m; + return { kind: "cmp", field, op: op as "=" | "!=" | "=~" | "!~", value }; + }; + + const evalFilter = (item: Record, ast: FilterAST): boolean => { + switch (ast.kind) { + case "and": return evalFilter(item, ast.left) && evalFilter(item, ast.right); + case "or": return evalFilter(item, ast.left) || evalFilter(item, ast.right); + case "not": return !evalFilter(item, ast.expr); + case "cmp": { + const actualStr = String(item[ast.field] ?? ""); + switch (ast.op) { + case "=": return actualStr === ast.value; + case "!=": return actualStr !== ast.value; + case "=~": return actualStr.includes(ast.value); + case "!~": return !actualStr.includes(ast.value); + } + } + } + }; + const where = (collection: string, filter: string) => { const map = getOrCreate(collection); const items = Array.from(map.values()); if (!filter || !filter.trim()) return items; - const clauses = filter.split("&&").map((c) => c.trim()); - return items.filter((item) => - clauses.every((clause) => { - const m = clause.match(/^(\w+)\s*=\s*"?([^"]+)"?$/); - if (!m) return true; - const [, field, raw] = m; - const value = raw.replace(/^"|"$/g, ""); - if (field === "year" || field === "month") { - return Number(item[field]) === Number(value); - } - return String(item[field] ?? "") === value; - }) - ); + const ast = parseFilter(filter); + return items.filter((item) => evalFilter(item, ast)); }; const pbMock = { + autoCancellation: () => undefined, collection: (name: string) => ({ async getList(page = 1, perPage = 25, opts: { filter?: string } = {}) { const items = where(name, opts.filter || ""); @@ -39,6 +134,7 @@ const { store, pbMock, clearStore } = vi.hoisted(() => { return { items: items.slice(start, start + perPage), totalItems: items.length, + totalPages: Math.max(1, Math.ceil(items.length / perPage)), page, perPage, }; @@ -134,6 +230,11 @@ import { sendQuoteEmail, retryQuoteEmail, recordSendOutcome, + getQuotesDashboardData, + getQuoteStatusTabCounts, + clearQuoteStatusTabCountsCache, + QUOTE_TAB_STATUSES, + QUOTE_TAB_ORDER, } from "./quotes-service"; beforeEach(() => { @@ -1011,3 +1112,153 @@ describe("recordSendOutcome", () => { expect(updated.email_error.length).toBeLessThan(5000); }); }); + +// ---------------------------------------------------------------------------- +// Quotes dashboard read model — tab filter, counts cache. +// +// Same surface as the orders dashboard tests: prove the server +// narrows by tab (so the footer pagination matches the badge count) +// and that the counts cache works. +// ---------------------------------------------------------------------------- + +describe("QUOTE_TAB_STATUSES (canonical tab → statuses mapping)", () => { + it("absorbs cancelled + rejected under the cancelled tab (was bucketed under completed before)", async () => { + // Regression guard for the long-standing bug where the + // Cancelled tab badge was always empty: the old + // getQuoteStatusCounts mapped `cancelled` and `rejected` to + // "completed", so the page's "cancelled" tab saw zero count + // even when 60+ rejected quotes existed. + expect(QUOTE_TAB_STATUSES.cancelled).toEqual( + expect.arrayContaining(["cancelled", "rejected"]), + ); + expect(QUOTE_TAB_STATUSES.completed).not.toContain("cancelled"); + expect(QUOTE_TAB_STATUSES.completed).not.toContain("rejected"); + }); + + it("includes every status the page tab renders, with no tab having an empty status list unless it's a special tab", async () => { + // delivery_note, all, and templates are special-cased (no + // status filter). Every other tab must have at least one status. + for (const tab of QUOTE_TAB_ORDER) { + if (tab === "delivery_note" || tab === "all" || tab === "templates") { + expect(QUOTE_TAB_STATUSES[tab]).toEqual([]); + } else { + expect(QUOTE_TAB_STATUSES[tab].length).toBeGreaterThan(0); + } + } + }); +}); + +describe("getQuoteStatusTabCounts", () => { + beforeEach(() => clearQuoteStatusTabCountsCache()); + + it("buckets quotes into the right tab based on status", async () => { + await pbMock.collection("quotes").create({ + id: "q-emailed", quote_number: "Q1", status: "emailed", + }); + await pbMock.collection("quotes").create({ + id: "q-emailed-2", quote_number: "Q2", status: "emailed", + }); + await pbMock.collection("quotes").create({ + id: "q-approved", quote_number: "Q3", status: "approved", + }); + await pbMock.collection("quotes").create({ + id: "q-rejected", quote_number: "Q4", status: "rejected", + }); + await pbMock.collection("quotes").create({ + id: "q-cancelled", quote_number: "Q5", status: "cancelled", + }); + await pbMock.collection("quotes").create({ + id: "q-new", quote_number: "Q6", status: "new", + }); + + const counts = await getQuoteStatusTabCounts(pbMock as never); + expect(counts.completed).toBe(3); // 2 emailed + 1 approved + expect(counts.cancelled).toBe(2); // 1 rejected + 1 cancelled + expect(counts.new).toBe(1); + }); + + it("returns the same counts on a second call (cache hit)", async () => { + await pbMock.collection("quotes").create({ + id: "q1", quote_number: "Q1", status: "emailed", + }); + + const first = await getQuoteStatusTabCounts(pbMock as never); + // Mutate the underlying store — the cache should still serve + // the stale value. + await pbMock.collection("quotes").create({ + id: "q2", quote_number: "Q2", status: "emailed", + }); + const cached = await getQuoteStatusTabCounts(pbMock as never); + expect(cached.completed).toBe(first.completed); + + // Bypass the cache to see the new state. + const fresh = await getQuoteStatusTabCounts(pbMock as never, { bypassCache: true }); + expect(fresh.completed).toBe(first.completed + 1); + }); +}); + +describe("getQuotesDashboardData — end-to-end", () => { + beforeEach(() => clearQuoteStatusTabCountsCache()); + + it("returns tab-scoped items + global counts in a single response", async () => { + // Set up: 1 emailed, 1 approved (both → completed tab), 1 new + // (new tab), 1 rejected (cancelled tab). + await pbMock.collection("quotes").create({ + id: "q1", quote_number: "Q1", status: "emailed", + created: "2026-01-01T00:00:00Z", + }); + await pbMock.collection("quotes").create({ + id: "q2", quote_number: "Q2", status: "approved", + created: "2026-01-02T00:00:00Z", + }); + await pbMock.collection("quotes").create({ + id: "q3", quote_number: "Q3", status: "new", + created: "2026-01-03T00:00:00Z", + }); + await pbMock.collection("quotes").create({ + id: "q4", quote_number: "Q4", status: "rejected", + created: "2026-01-04T00:00:00Z", + }); + + const result = await getQuotesDashboardData({ + page: 1, + perPage: 20, + tab: "completed", + }); + + // Counts are global (always were) — the badge still tells the truth. + expect(result.counts.completed).toBe(2); + expect(result.counts.new).toBe(1); + expect(result.counts.cancelled).toBe(1); + + // Items are scoped to the active tab. + expect(result.items.map((q) => q.id).sort()).toEqual(["q1", "q2"]); + expect(result.totalItems).toBe(2); + expect(result.totalPages).toBe(1); + }); + + it("scopes the Cancelled tab to rejected + cancelled statuses (the long-standing bug)", async () => { + await pbMock.collection("quotes").create({ + id: "q-rejected", quote_number: "Q1", status: "rejected", + }); + await pbMock.collection("quotes").create({ + id: "q-cancelled", quote_number: "Q2", status: "cancelled", + }); + await pbMock.collection("quotes").create({ + id: "q-emailed", quote_number: "Q3", status: "emailed", + }); + + const result = await getQuotesDashboardData({ + page: 1, + perPage: 20, + tab: "cancelled", + }); + + expect(result.counts.cancelled).toBe(2); // rejected + cancelled both counted here + expect(result.counts.completed).toBe(1); // emailed stays under completed + expect(result.items.map((q) => q.id).sort()).toEqual([ + "q-cancelled", + "q-rejected", + ]); + }); +}); diff --git a/src/lib/server/quotes-service.ts b/src/lib/server/quotes-service.ts index 3bbb6ff..bc749e9 100644 --- a/src/lib/server/quotes-service.ts +++ b/src/lib/server/quotes-service.ts @@ -116,6 +116,402 @@ export interface AppendReplyResult { * against the current row state, so concurrent callers cannot collide even * if they read the counter at the same instant. */ +/* ============================================================================ + * Quotes dashboard read model + * ---------------------------------------------------------------------------- + * Read-side service for the `/quotes` page. Mirrors the pattern established + * in `orders-service.ts` (server-side tab filter + slim expand + cached + * combined counts) but adapted to the quotes data model. + * + * Why a new module-level section instead of extending `pb-helpers.ts`: + * - `pb-helpers.ts` is the legacy layer called directly by Client + * Components; `AGENTS.md` says new endpoints / features go through + * the service layer. The previous implementation also had several + * wire-payload issues (full quote record × 20 in the list response, + * no field limits on the deep expand) that are easier to fix in + * one place than to retrofit onto the helper. + * - The old `getQuoteStatusCounts` fired 22 parallel `getList(1, 1, ...)` + * queries, one per PB status. With no `quotes.status` index on the + * live PB, that's 22 full scans over ~8.5k quotes = ~2.6s wall time + * on every page load. The combined query below collapses to 1 scan + * (~150ms measured on the live instance). + * + * Tab model: 10 tabs on /quotes, each mapped to one or more PB statuses. + * The mapping is canonical here (`QUOTE_TAB_STATUSES`) — the page's + * `tabFilters` constant now derives from this. Fixes the long-standing + * bug where `cancelled` + `rejected` were bucketed under "completed" + * in the counts, so the Cancelled tab badge was always empty. + * ========================================================================== */ + +export type QuoteTabKey = + | "new" + | "claimed" + | "pending_approval" + | "completed" + | "cancelled" + | "delivery_note" + | "email_failed" + | "problem" + | "all" + | "templates"; + +/** + * The /quotes page's tabs, in the operator-facing display order. The + * left-to-right order matches the workflow lifecycle (intake → claim → + * approval → success → recovery → failures → catch-all → meta). The + * `all` tab sums every status except templates; `templates` is its + * own thing because templates are scaffolding for FUTURE quotes, not + * in-flight work. + */ +export const QUOTE_TAB_ORDER: QuoteTabKey[] = [ + "new", + "claimed", + "pending_approval", + "completed", + "cancelled", + "delivery_note", + "email_failed", + "problem", + "all", + "templates", +]; + +/** + * Tab key → list of PB statuses that belong on that tab. An empty + * array means the tab uses a non-status filter (delivery_note = + * relation filter, all = no filter, templates = is_template = true). + * + * `cancelled` now correctly absorbs the `rejected` and `cancelled` + * statuses — the old `getQuoteStatusCounts` mapped both to + * "completed", which is why the Cancelled tab badge was always empty. + * The page's `tabFilters` separately groups them under "cancelled", + * so a quote that was rejected showed up in the list but not the + * count. The new mapping keeps count and list in lock-step. + */ +export const QUOTE_TAB_STATUSES: Record = { + new: ["new", "follow_up"], + claimed: ["claimed"], + pending_approval: ["processed", "approval_pending", "partial_approval_pending", "sending"], + completed: ["approved", "complete", "partial_approved", "emailed"], + cancelled: ["cancelled", "rejected"], + // Delivery Note tab is relation-based (every quote that prices a + // delivery note, regardless of where it sits in the lifecycle). + // The list filter is `delivery_note != ""` rather than a status + // IN (...) clause. The count is derived from the same filter. + delivery_note: [], + email_failed: ["email_failed", "mail_failed", "unmatched_bounce"], + problem: ["problem"], + // "All" = no status filter. Treated as the catch-all excluding + // templates (templates have their own tab and aren't in-flight work). + all: [], + // Templates tab uses `is_template = true`, NOT a status filter + // (the `status` field for templates can be anything). + templates: [], +}; + +/** + * All statuses that drive any tab's count, deduplicated. Used to + * build the combined `status = "..." || ...` filter for the + * status-counts query so we make ONE PB call instead of one per status. + */ +const QUOTE_COUNTED_STATUSES: string[] = Array.from( + new Set( + Object.values(QUOTE_TAB_STATUSES).flat().filter((s) => s.length > 0), + ), +); + +/** PB filter fragment for `status IN (countedStatuses)`. */ +function quoteCountedStatusesFilter(): string { + return QUOTE_COUNTED_STATUSES.map((s) => `status = "${s}"`).join(" || "); +} + +/** + * Cached counts for the tabs. Same lifetime/rationale as the orders + * tab-counts cache: counts are global, so 60s TTL survives rapid + * tab-switching without going stale on the operator's own changes + * (realtime / refetch with `bypassCache: true` invalidates). + */ +let cachedQuoteTabCounts: { + value: Record; + expiresAt: number; +} | null = null; +const QUOTE_TAB_COUNTS_CACHE_TTL_MS = 60_000; + +/** Invalidate the quote tab-counts cache. Exported for tests. */ +export function clearQuoteStatusTabCountsCache(): void { + cachedQuoteTabCounts = null; +} + +/** + * Batch size for the combined counts query. Bumped from the PB default + * of 500 — at 8.5k+ quotes the default would cost ~17 internal + * round-trips; 5000 keeps it at 2-3. + */ +const QUOTE_COUNTS_BATCH_SIZE = 5000; + +export interface QuotesDashboardData { + items: Quote[]; + totalItems: number; + totalPages: number; + counts: Record; +} + +/** + * Read model for the internal /quotes page. + * + * Mirrors `getOrdersDashboardData` (orders-service.ts): a search + * field-free path uses PB's native pagination; a tab path narrows + * to the tab's status set so the footer pagination matches the + * badge count. Search bypasses tab filtering (operator hunting + * for a specific quote, regardless of status) and reduces to a + * single page. + */ +export async function getQuotesDashboardData( + params: { + page?: number; + perPage?: number; + search?: string; + sort?: string; + tab?: QuoteTabKey; + bypassCountsCache?: boolean; + } = {}, +): Promise { + const pb = await getAdminPocketBase(); + pb.autoCancellation(false); + const [list, counts] = await Promise.all([ + listQuotesForDashboard(params, pb), + getQuoteStatusTabCounts(pb, { bypassCache: params.bypassCountsCache }), + ]); + return { ...list, counts }; +} + +/** + * Slim field list for the list-page response. The quotes collection + * has 45+ fields including unbounded `email_body` and `email_message` + * — without limiting, 20 quotes × full records = multi-MB JSON even + * when most emails are empty (the wasted bandwidth and parse cost + * shows up in page-load time). The list view reads: + * quote.* : id, quote_number, status, claimed_by, is_template, + * follow_up_due, last_reply_at, customer (rel id), + * company (rel id), created, updated + * expand.customer : customer (name), rep (rel id) + * expand.customer.rep : id, name, email + * expand.company : company (name) + * + * `customer.company` is intentionally NOT expanded — the page only + * reads `expand.company.company` (the direct relation from quote), + * not `expand.customer.expand.company`. Dropping the 3-level expand + * saves one IN query per page load. + */ +const QUOTE_LIST_FIELDS = [ + // main record + "id", + "quote_number", + "status", + "claimed_by", + "is_template", + "follow_up_due", + "last_reply_at", + "customer", + "company", + "created", + "updated", + // expanded company — just the name + "company.id", + "company.company", + // expanded customer — name + rep relation + "customer.id", + "customer.customer", + "customer.rep", + // expanded customer.rep (users) — id, name, email + "customer.rep.id", + "customer.rep.name", + "customer.rep.email", +].join(","); + +const QUOTE_LIST_EXPAND = "customer,customer.rep,company"; + +async function listQuotesForDashboard( + params: { + page?: number; + perPage?: number; + search?: string; + sort?: string; + tab?: QuoteTabKey; + }, + pbArg?: Awaited>, +): Promise> { + const pb = pbArg || (await getAdminPocketBase()); + pb.autoCancellation(false); + const page = Math.max(1, params.page || 1); + const perPage = Math.min(100, Math.max(1, params.perPage || 20)); + const search = (params.search || "").trim(); + const sort = params.sort || "-created"; + const tab = params.tab; + + // Build the tab's status filter clause. Three classes of tab: + // - delivery_note: relation filter (not status) + // - templates: is_template = true + // - all: no status filter + // - any other: status IN (TAB_STATUSES[tab]) + const tabStatuses = tab ? QUOTE_TAB_STATUSES[tab] : []; + let tabFilter = ""; + if (tab === "delivery_note") { + tabFilter = `delivery_note != ""`; + } else if (tab === "templates") { + tabFilter = `is_template = true`; + } else if (tab === "all") { + tabFilter = `is_template != true`; + } else if (tabStatuses.length > 0) { + tabFilter = `(${tabStatuses.map((s) => `status = "${s}"`).join(" || ")})`; + } + + // Search + tab combines. Search is a free-text OR across the + // quote_number / quote_reference / rfq_number fields. When BOTH + // are active, the PB filter chains them with AND. + let searchFilter = ""; + if (search) { + searchFilter = `(quote_number ~ "${search}" || quote_reference ~ "${search}" || rfq_number ~ "${search}")`; + } + // For non-special tabs and "all", we also exclude templates so + // they don't pollute the workflow view (the Templates tab shows + // them explicitly). For the "templates" tab itself, this is + // already handled by `is_template = true`. + const excludeTemplate = + tab !== "templates" ? `is_template != true` : ""; + const clauses = [excludeTemplate, tabFilter, searchFilter] + .filter((c) => c.length > 0) + .join(" && "); + + // For tab + no-search: PB paginates the filtered set, so the + // footer total reflects the tab's matching count (not the global + // ~8.5k). For search (with or without tab): single page, total + // computed from the filtered items. + const result = await pb + .collection(QUOTES_COLLECTION) + .getList(page, perPage, { + filter: clauses, + sort, + expand: QUOTE_LIST_EXPAND, + fields: QUOTE_LIST_FIELDS, + }); + + if (search) { + return { + items: result.items, + totalItems: result.items.length, + totalPages: 1, + }; + } + return { + items: result.items, + totalItems: result.totalItems, + totalPages: result.totalPages, + }; +} + +/** + * Counts per tab. Replaces the old `getQuoteStatusCounts` (22 + * parallel `getList(1, 1, ...)` queries). Strategy: + * + * 1. One combined `status = "x" || status = "y" || ...` query, + * `fields: "status"`, batch 5000. Loads ~all quotes that + * contribute to any tab's count. ~150ms measured on the live + * PB (vs ~2.6s for 22 separate queries without an index). + * + * 2. Bucket in memory. The dataset is small — only the `status` + * field per row — so the dedupe-by-tab is cheap. + * + * 3. Special-case the `delivery_note` and `templates` tabs with + * their own targeted queries (the status-based filter doesn't + * apply to either). These two are tiny in volume so the cost + * is negligible. + * + * Cached for 60s; the realtime / refetch path passes `bypassCache: + * true` so a change the operator just made updates the badge + * immediately. + */ +export async function getQuoteStatusTabCounts( + pbArg?: Awaited>, + options: { bypassCache?: boolean } = {}, +): Promise> { + const pb = pbArg || (await getAdminPocketBase()); + pb.autoCancellation(false); + + if ( + !options.bypassCache && + cachedQuoteTabCounts && + cachedQuoteTabCounts.expiresAt > Date.now() + ) { + return cachedQuoteTabCounts.value; + } + + // Combined status query — ONE PB call, ONE filter scan, instead + // of 22 separate per-status scans. + const allStatusRows = await pb + .collection(QUOTES_COLLECTION) + .getFullList<{ status?: string }>({ + filter: quoteCountedStatusesFilter(), + fields: "status", + batch: QUOTE_COUNTS_BATCH_SIZE, + }); + + const counts: Record = { + new: 0, + claimed: 0, + pending_approval: 0, + completed: 0, + cancelled: 0, + delivery_note: 0, + email_failed: 0, + problem: 0, + all: 0, + templates: 0, + }; + let allTotal = 0; + + for (const row of allStatusRows) { + const status = row.status; + if (!status) continue; + allTotal++; + for (const tab of QUOTE_TAB_ORDER) { + const tabStatuses = QUOTE_TAB_STATUSES[tab]; + if (tabStatuses.length === 0) continue; // special tab, handled below + if (tabStatuses.includes(status)) { + counts[tab]++; + break; // each status lands in exactly one workflow tab + } + } + } + + // Special tabs (relation-based, not status-based). + // Templates: `is_template = true`. Excluded from the combined + // query above so the workflow count isn't polluted. + const templatesResp = await pb + .collection(QUOTES_COLLECTION) + .getList(1, 1, { filter: "is_template = true" }); + counts.templates = templatesResp.totalItems; + + // Delivery note: `delivery_note != ""`. Each quote with a + // delivery note attached. Status-agnostic — DN-backed quotes + // can be claimed, processed, emailed, etc. + const dnResp = await pb + .collection(QUOTES_COLLECTION) + .getList(1, 1, { filter: `delivery_note != ""` }); + counts.delivery_note = dnResp.totalItems; + + // "All" is the catch-all EXCLUDING templates. This matches the + // page's previous behaviour (templates aren't in-flight work, + // so they don't contribute to the workflow rollup). + counts.all = allTotal; + + cachedQuoteTabCounts = { + value: counts, + expiresAt: Date.now() + QUOTE_TAB_COUNTS_CACHE_TTL_MS, + }; + return counts; +} + export async function generateQuoteNumberServer(): Promise { const pb = await getAdminPocketBase(); const now = new Date();