From a3a831d183a60242ac1461b88eb5379f8d2681a5 Mon Sep 17 00:00:00 2001 From: Dave Date: Fri, 16 Oct 2026 01:07:50 +0200 Subject: [PATCH] fix(quotes): list renders the customer's company, not the stale copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third and last place that read `quotes.company` directly. The list page showed "Mre" next to Pierre Visser, who works for Harmony — the same drift already reported on the detail page. This one was a deliberate perf shortcut rather than an oversight. `QUOTE_LIST_EXPAND` dropped `customer.company` to "save one IN query per page load" and the page read the quote's own `company` relation instead. That relation is a denormalised copy that PocketBase never re-cascades, so the shortcut read a field nothing keeps correct — and the column it fed is what a rep reads to decide whether to approve, so a stale company there is worse than a slightly larger query. The 3-level expand is back, on both the list and the search path (the search results feed the same columns, so they needed it too). Search now also RANKS on the customer's company rather than the stale copy — otherwise a query for "Mre" would surface quotes whose results then render "Harmony", contradicting themselves on screen. It falls back to the quote's own copy for the ~4% of customers with no company relation, where the two are equal by construction. Applies to both the desktop table column and the mobile card. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/app/(app)/quotes/page.tsx | 32 ++++++++++++++++-- src/lib/server/quotes-search-service.ts | 17 ++++++++-- src/lib/server/quotes-service.test.ts | 44 +++++++++++++++++++++++++ src/lib/server/quotes-service.ts | 21 ++++++++---- 4 files changed, 103 insertions(+), 11 deletions(-) diff --git a/src/app/(app)/quotes/page.tsx b/src/app/(app)/quotes/page.tsx index 64f1dec..0e1b93c 100644 --- a/src/app/(app)/quotes/page.tsx +++ b/src/app/(app)/quotes/page.tsx @@ -676,7 +676,13 @@ export default function QuotesPage() { label: "Customer", render: (item: Quote) => { const customerName = item.expand?.customer?.customer; - const companyName = item.expand?.company?.company; + // From the CUSTOMER, not from the quote's own `company` + // relation. That field is a denormalised copy that is + // never re-cascaded when the customer changes, so it + // drifts — Q202610315 listed "Mre" while its customer sat + // at Harmony. Matches the approvals list and the quote + // PDF, which both already read the customer's company. + const companyName = customerCompanyName(item); if (!customerName && !companyName) { return —; } @@ -1129,9 +1135,9 @@ export default function QuotesPage() {

{item.expand?.customer?.customer || "—"}

- {item.expand?.company?.company ? ( + {customerCompanyName(item) ? (

- {item.expand.company.company} + {customerCompanyName(item)}

) : null} @@ -1299,6 +1305,26 @@ function AgedBadge({ created }: { created: string }) { * Hidden on terminal states (complete / cancelled / rejected) where the * reminder is moot. */ +/** + * Company name for a quote row, read from the CUSTOMER rather than + * from the quote's own `company` relation. + * + * `quotes.company` is a denormalised copy of `customer.company` that + * PocketBase does not re-cascade, so it silently goes stale whenever a + * quote's customer is swapped — Q202610315 showed "Mre" while its + * customer sat at Harmony. The list page used to read that stale copy + * (a deliberate perf shortcut: it skipped the `customer.company` + * expand), which meant the rep deciding whether to approve saw the + * wrong company. The 3-level expand is back and this is the + * authoritative source. Matches the approvals list and the quote PDF. + */ +function customerCompanyName(quote: Quote): string | undefined { + const company = quote.expand?.customer?.expand?.company as + | { company?: string } + | undefined; + return company?.company || undefined; +} + function isFollowUp(quote: Quote): boolean { if (quote.status === "follow_up") { const terminal = new Set(["complete", "cancelled", "rejected"]); diff --git a/src/lib/server/quotes-search-service.ts b/src/lib/server/quotes-search-service.ts index e593b2a..cc5b593 100644 --- a/src/lib/server/quotes-search-service.ts +++ b/src/lib/server/quotes-search-service.ts @@ -414,7 +414,20 @@ export function scoreQuote( breakdown.rfqNumber = awardHeader(quote.rfq_number, needles, WEIGHTS.rfqNumber); breakdown.externalNumber = awardHeader(quote.external_number, needles, WEIGHTS.externalNumber); breakdown.customer = awardHeader(quote.expand?.customer?.customer, needles, WEIGHTS.customer); - breakdown.company = awardHeader(quote.expand?.company?.company, needles, WEIGHTS.company); + // Rank on the CUSTOMER's company, not `quotes.company`. The latter + // is a denormalised copy that is never re-cascaded when a quote's + // customer is swapped, so searching "Mre" would surface quotes whose + // customer actually sits at Harmony — and the list beside it would + // render Harmony, so the results contradicted themselves. Falls back + // to the quote's own copy only for the ~4% of customers that have no + // company relation at all, where the two are equal by construction. + breakdown.company = awardHeader( + (quote.expand?.customer?.expand?.company as { company?: string } | undefined) + ?.company ?? + (quote.expand?.company as { company?: string } | undefined)?.company, + needles, + WEIGHTS.company, + ); // Customer / company sub-fields — null when the parent object is // absent (the quote has no customer / company expanded), so the // score loop never penalises rows for missing relations. @@ -1348,7 +1361,7 @@ const QUOTE_SEARCH_FIELDS = [ "customer.rep.email", ].join(","); -const QUOTE_SEARCH_EXPAND = "customer,customer.rep,company"; +const QUOTE_SEARCH_EXPAND = "customer,customer.company,customer.rep,company"; const QUOTE_ITEM_SEARCH_FIELDS = [ "id", diff --git a/src/lib/server/quotes-service.test.ts b/src/lib/server/quotes-service.test.ts index 34fb72b..36467f2 100644 --- a/src/lib/server/quotes-service.test.ts +++ b/src/lib/server/quotes-service.test.ts @@ -2052,6 +2052,50 @@ describe("getQuotesDashboardData — end-to-end", () => { ]); }); + it("expands customer.company so the list renders the customer's real company", async () => { + // Regression pin. `QUOTE_LIST_EXPAND` used to be + // "customer,customer.rep,company" — the 3-level expand was dropped + // to save one IN query per page load, and the list page read the + // quote's own `company` relation instead. That relation is a + // denormalised copy PocketBase never re-cascades, so it drifts: + // Q202610315 listed "Mre" while its customer sat at Harmony, and + // the rep reads that column to decide whether to approve. + // + // The mock doesn't apply expand, so pin the expand string itself. + await pbMock.collection("quotes").create({ + id: "q-company", quote_number: "Q1", status: "new", + }); + + const seen: string[] = []; + const realCollection = pbMock.collection.bind(pbMock); + const spy = vi + .spyOn(pbMock, "collection") + .mockImplementation((name: string) => { + const coll = realCollection(name); + if (name !== "quotes") return coll; + return { + ...coll, + async getList( + page = 1, + perPage = 25, + opts: { filter?: string; expand?: string } = {}, + ) { + if (opts.expand) seen.push(opts.expand); + return coll.getList(page, perPage, opts); + }, + }; + }); + + try { + await getQuotesDashboardData({ page: 1, perPage: 20, tab: "new" }); + } finally { + spy.mockRestore(); + } + + expect(seen.length).toBeGreaterThan(0); + expect(seen[0]).toContain("customer.company"); + }); + it("can omit archive counts for the main quotes page", async () => { await pbMock.collection("quotes").create({ id: "q-completed", quote_number: "Q1", status: "emailed", diff --git a/src/lib/server/quotes-service.ts b/src/lib/server/quotes-service.ts index bc4a2f1..e66dee5 100644 --- a/src/lib/server/quotes-service.ts +++ b/src/lib/server/quotes-service.ts @@ -377,11 +377,20 @@ export async function getQuotesDashboardData( } /** - * Expand relation for the list-page response. `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. + * Expand relation for the list-page response. + * + * `customer.company` WAS deliberately dropped to save one IN query per + * page load, with the page reading `expand.company.company` (the + * quote's own denormalised copy) instead. That shortcut is why the + * list showed the wrong company: `quotes.company` is not re-cascaded + * when a quote's customer changes, so it goes stale and stays stale — + * Q202610315 listed "Mre" while its customer sat at Harmony. It is + * also the surface a rep reads to decide whether to approve, so a + * stale company there is worse than a slightly larger query. + * + * The 3-level expand is back. If this ever needs trimming, the fix is + * to drop `quote.company` from the schema rather than to read a field + * that nothing keeps correct. * * (No `fields` projection here — see the comment at the call site. * PB 0.26 doesn't support per-expand field limiting without @@ -389,7 +398,7 @@ export async function getQuotesDashboardData( * per-record wire size and rely on schema-level fixes to trim * the heavy text columns.) */ -const QUOTE_LIST_EXPAND = "customer,customer.rep,company"; +const QUOTE_LIST_EXPAND = "customer,customer.company,customer.rep,company"; async function listQuotesForDashboard( params: {