fix(quotes): list renders the customer's company, not the stale copy
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) <noreply@anthropic.com>
This commit is contained in:
parent
b50362e1f3
commit
a3a831d183
4 changed files with 103 additions and 11 deletions
|
|
@ -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 <span className="text-muted-foreground">—</span>;
|
||||
}
|
||||
|
|
@ -1129,9 +1135,9 @@ export default function QuotesPage() {
|
|||
<p className="text-sm font-medium truncate">
|
||||
{item.expand?.customer?.customer || "—"}
|
||||
</p>
|
||||
{item.expand?.company?.company ? (
|
||||
{customerCompanyName(item) ? (
|
||||
<p className="text-xs text-muted-foreground truncate">
|
||||
{item.expand.company.company}
|
||||
{customerCompanyName(item)}
|
||||
</p>
|
||||
) : null}
|
||||
</div>
|
||||
|
|
@ -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"]);
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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: {
|
||||
|
|
|
|||
Loading…
Reference in a new issue