From 292cf2468ef9bd5fa9b0a656a76cdd2db82c4d38 Mon Sep 17 00:00:00 2001 From: SJCoetz Date: Tue, 4 Aug 2026 19:27:02 +0200 Subject: [PATCH] fix(orders): remove broken fields param + show contact_name + REP label # Regression: orders/quotes list lost customer/company/rep data Last commit added a `fields` parameter to the orders and quotes list queries to trim the wire payload. Measured on a stale test result, looked like a 16x win. Actual: PB 0.26's `fields` parameter is a HARD ALLOWLIST of MAIN record columns only \u2014 specifying it makes PB drop the entire `expand` payload from the response. Result: customer, company, rep, quote names disappeared from every order/quote card. Verified against the live PB: - With `fields`: expand is dropped, just relation IDs returned - Without `fields`: expand works, full records (55-57 KB for 20 rows with deep expand) Fix: remove the `fields` parameter from both list queries. The slim-payload intent has to be paid at the schema level (cap the unbounded text columns) rather than via PB query params. The earlier '12x smaller' / '16x smaller' benchmarks were measurements of the BROKEN response, not the working one. Real wire sizes are unchanged from before the perf work \u2014 but the other wins (server-side tab filter, combined status counts, targeted users fetch, 60s counts cache) all still apply. # Bonus: show contact_name on the order card ~76 of ~4.5k orders have `contact_name` set (the free-text person the order is for, captured during email-intake when the customer record is just the company). Added to the OrderCard header between customer and rep, with a docstring explaining the distinction from the customer relation and email_sender. Added contact_name to the Order type and pinned the regression with a test that asserts the orders list query does NOT pass `fields` (so a future 'let's slim the payload' attempt doesn't reintroduce the bug). # REP label The rep column on the order card now reads 'REP' (uppercase) instead of 'rep' (lowercase). All 1312 tests pass. Co-authored-by: Orca --- src/app/(app)/orders/page.tsx | 19 +++++- src/lib/server/orders-service.test.ts | 36 ++++++----- src/lib/server/orders-service.ts | 88 ++++++++------------------- src/lib/server/quotes-service.ts | 59 ++++++------------ src/lib/types.ts | 10 +++ 5 files changed, 89 insertions(+), 123 deletions(-) diff --git a/src/app/(app)/orders/page.tsx b/src/app/(app)/orders/page.tsx index 2f66e5e..fdc5c40 100644 --- a/src/app/(app)/orders/page.tsx +++ b/src/app/(app)/orders/page.tsx @@ -787,11 +787,28 @@ function OrderCard({ )} + {/* + * `contact_name` is the free-text person the order is + * for — sometimes the customer record is just the + * company, and the actual contact is captured here + * during the email-intake. ~76 of ~4.5k orders have + * it set; render when present so the operator can + * see "this goes to Dreyer, Mr. Petrus" without + * opening the detail page. + */} + {order.contact_name && ( + <> + · + + {order.contact_name} + + + )} {repName && ( <> · - rep: {repName} + REP: {repName} )} diff --git a/src/lib/server/orders-service.test.ts b/src/lib/server/orders-service.test.ts index a103881..4a69443 100644 --- a/src/lib/server/orders-service.test.ts +++ b/src/lib/server/orders-service.test.ts @@ -776,13 +776,15 @@ describe("listOrdersWithItems — tab filter", () => { expect(result.items.length).toBe(2); }); - it("passes a `fields` parameter to the orders list (limits wire payload on the deep expand)", async () => { - // The orders list query asks PB to limit the main record AND each - // expanded record to just the fields the list view reads. Without - // this, every order card drags the full `quote` record along - // (quotes has unbounded `email_body` / `email_message` columns), - // so 20 orders with deep expand = multi-MB JSON payload. PB - // honours `expand.field` syntax in the `fields` query param. + it("does not pass a `fields` parameter on the orders list (PB 0.26 breaks expand when fields is set)", async () => { + // Regression guard: earlier I tried to limit the wire payload + // by passing a `fields` list with per-expand syntax. PB 0.26 + // treats `fields` as a hard allowlist of MAIN record columns, + // and the moment you set it the `expand` payload is dropped + // from the response — so customer/company/rep/quote names + // vanish from every order card. The orders list deliberately + // omits `fields`; this test pins that behaviour so a future + // "let's slim the payload" change doesn't reintroduce the bug. let capturedFields: string | undefined; let capturedExpand: string | undefined; const originalCollection = pbMock.collection; @@ -807,15 +809,17 @@ describe("listOrdersWithItems — tab filter", () => { try { await listOrdersWithItems({ page: 1, perPage: 20 }); - // Main record fields - expect(capturedFields).toContain("order_number"); - expect(capturedFields).toContain("created"); - // Per-expand field limits — the actual perf win - expect(capturedFields).toContain("quote.quote_number"); - expect(capturedFields).toContain("company.company"); - expect(capturedFields).toContain("customer.customer"); - expect(capturedFields).toContain("customer.rep.name"); - // Confirm we dropped potential_quote from the expand + // `fields` must NOT be sent — PB 0.26 drops the expand payload + // when it sees it. + expect(capturedFields).toBeUndefined(); + // Expand IS sent, and includes the four relations the list view + // reads (company, customer, customer.rep, quote). Importantly + // it does NOT include `potential_quote` — the list view only + // checks the field's existence, never the expanded record. + expect(capturedExpand).toContain("company"); + expect(capturedExpand).toContain("customer"); + expect(capturedExpand).toContain("customer.rep"); + expect(capturedExpand).toContain("quote"); expect(capturedExpand).not.toContain("potential_quote"); } finally { pbMock.collection = originalCollection; diff --git a/src/lib/server/orders-service.ts b/src/lib/server/orders-service.ts index 1e744ae..bb9ea1e 100644 --- a/src/lib/server/orders-service.ts +++ b/src/lib/server/orders-service.ts @@ -144,63 +144,24 @@ export async function listOrdersWithItems( // the field's existence (the "Suggested" pill in the header), and // PB issues a separate IN query per expanded relation. Without it, we // save one round-trip per page load and per realtime refetch. + // + // NOTE on the `fields` parameter (and why it isn't here): earlier + // I tried to limit wire payload by passing a `fields` list with + // per-expand syntax like `company.company,customer.customer,...`. + // PB 0.26 doesn't support that — the `fields` parameter is a hard + // allowlist of MAIN record columns, and specifying it makes PB + // drop the `expand` payload entirely. Result: customer/company/ + // rep/quote names disappeared from every order card. Restored by + // removing the parameter; the page now works and the wire payload + // is bigger than I'd like, but the deep expand works. + // + // Future: either upgrade PB (0.22+ supports per-expand field + // limits properly) or move the heavy fields off the orders/quotes + // collections (PB has a free `notes` text field with no max, and + // unbounded `email_body` / `email_message` on quotes — these are + // the dominant wire-payload cost). The schema-level fix would + // permanently trim the payload. const expand = "company,customer,customer.rep,quote"; - // `fields` has two purposes here: - // - // 1. Limit the MAIN record's wire payload to only the columns the - // list view actually reads. The orders schema is wide (notes, - // email_body, ai_insights, migration_meta, etc.) and most rows - // carry KBs of content we never render. - // - // 2. Limit each EXPANDED record's fields via the `expand.field` - // syntax PB supports in the `fields` query param. This is the - // big win: `quotes` has unbounded text columns (email_body, - // email_message) that, without limiting, would drag the full - // email body of every linked quote into the list response — - // 20 orders with deep expand can easily become a 1-5 MB - // JSON payload. We only need `quote.quote_number` for the - // pill, so we ask for that and nothing else. - // - // - `company.company` — the company name on the header row - // - `customer.customer` — the customer name on the header row - // - `customer.rep.id/name/email` — rep label, via - // `resolveRepDisplayName` - // - `quote.quote_number` — the linked-quote pill - // Top-level fields = main record (orders) columns. Dotted fields = - // per-expand field limits using PB's `expandField.subfield` syntax. - // The relation field ids (`company`, `customer`, `customer.rep`, - // `quote`) must be present in the main list so PB knows which - // relations to expand; once listed, the dotted forms below limit - // what comes back from each expanded record. - const fields = - [ - // main record - "id", - "order_number", - "company", - "customer", - "quote", - "potential_quote", - "potential_quote_confidence", - "quote_match_confidence", - "rep", - "created", - "updated", - // expanded `company` — just the name - "company.id", - "company.company", - // expanded `customer` — just the name + the rep relation id - "customer.id", - "customer.customer", - "customer.rep", - // expanded `customer.rep` (users) — just the label fields - "customer.rep.id", - "customer.rep.name", - "customer.rep.email", - // expanded `quote` — just the number for the pill - "quote.id", - "quote.quote_number", - ].join(","); // Tab filter path — three sub-cases: // (a) no tab, no search → original PB-paginated path @@ -225,14 +186,13 @@ export async function listOrdersWithItems( perPage, sort, expand, - fields, }); } } else if (search && !tabGroup) { ordersPage = await pb.collection(ORDER_COLLECTION).getList( 1, ORDER_SEARCH_HARD_CAP, - { sort, expand, fields }, + { sort, expand }, ); // search returns a single page; totalPages/totalItems are derived // from the filtered list size once we know how many survived the @@ -243,7 +203,7 @@ export async function listOrdersWithItems( const rawPage = await pb.collection(ORDER_COLLECTION).getList( 1, ORDER_SEARCH_HARD_CAP, - { sort, expand, fields }, + { sort, expand }, ); ordersPage = { items: rawPage.items, @@ -257,7 +217,7 @@ export async function listOrdersWithItems( ordersPage = await pb.collection(ORDER_COLLECTION).getList( page, perPage, - { sort, expand, fields }, + { sort, expand }, ); } @@ -379,15 +339,15 @@ export async function getOrderIdsForTabGroup( async function fetchOrdersFilteredByIds( pb: AdminPocketBase, ids: string[], - opts: { page: number; perPage: number; sort: string; expand: string; fields: string }, + opts: { page: number; perPage: number; sort: string; expand: string }, ): Promise<{ items: Order[]; totalItems: number; totalPages: number; page: number; perPage: number }> { - const { page, perPage, sort, expand, fields } = opts; + const { page, perPage, sort, expand } = opts; const filter = ids.map((id) => `id = "${id.replace(/"/g, '\\"')}"`).join(" || "); if (ids.length <= ID_LIST_INLINE_LIMIT) { const result = await pb .collection(ORDER_COLLECTION) - .getList(page, perPage, { filter, sort, expand, fields }); + .getList(page, perPage, { filter, sort, expand }); return { items: result.items, totalItems: result.totalItems, @@ -399,7 +359,7 @@ async function fetchOrdersFilteredByIds( const all = await pb .collection(ORDER_COLLECTION) - .getFullList({ filter, sort, expand, fields }); + .getFullList({ filter, sort, expand }); const totalItems = all.length; const totalPages = Math.max(1, Math.ceil(totalItems / perPage)); const start = (page - 1) * perPage; diff --git a/src/lib/server/quotes-service.ts b/src/lib/server/quotes-service.ts index bc749e9..54ed7ad 100644 --- a/src/lib/server/quotes-service.ts +++ b/src/lib/server/quotes-service.ts @@ -286,49 +286,18 @@ export async function getQuotesDashboardData( } /** - * 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 + * 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. + * + * (No `fields` projection here — see the comment at the call site. + * PB 0.26 doesn't support per-expand field limiting without + * dropping the entire `expand` payload, so we accept the larger + * per-record wire size and rely on schema-level fixes to trim + * the heavy text columns.) */ -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( @@ -393,7 +362,13 @@ async function listQuotesForDashboard( filter: clauses, sort, expand: QUOTE_LIST_EXPAND, - fields: QUOTE_LIST_FIELDS, + // No `fields` here: PB 0.26 treats `fields` as a hard allowlist + // of MAIN record columns, and specifying it makes PB drop the + // `expand` payload entirely — so the customer/company/rep names + // disappear from the list. The expand chain is what we actually + // need; the per-record wire cost has to be paid until the + // schema's unbounded text columns (notes, email_body, etc.) + // are trimmed at the field level. }); if (search) { diff --git a/src/lib/types.ts b/src/lib/types.ts index 2e24b2a..eb1569c 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -704,6 +704,16 @@ export interface Order extends BaseRecord { * monitoring pipeline; null for orders created without one. */ email_message_id?: string; + /** + * Free-text person the order is for. Sometimes the customer + * record is just the company (legal entity), and the actual + * contact captured during the email-intake lives here. Distinct + * from `customer` (the relation) and `email_sender` (the email + * From: address) — this is the operator-readable contact name + * shown on the order card so the office can see "this goes to + * Dreyer, Mr. Petrus" without opening the detail page. + */ + contact_name?: string; expand?: { company?: Company; customer?: Customer;