fix(deliveries): group by company and show items on delivery-note rows

Two operator-facing bugs on /deliveries:

1. Page grouped rows by customer instead of company. Added a
   CompanyGroup layer above CustomerGroup with a new
   groupByCompanyCustomerOrder() that builds
   Company → Customer → Order → Items + DNs (4-level hierarchy).
   Each level sorted stably so the most recent work sits on top.

2. The delivery-note expand panel said 'No items loaded on this
   delivery note' for every DN, even when the same DN had items
   on the full /delivery-notes/{id} page. Two root causes:

   a. Wrong join key. The page filtered by
      i.delivery_note === dn.id (the never-populated relation
      field), so the comparison was always null === id and
      never matched. The full DN page joins on the text column
      delivery_note_number — switched the page to the same key.

   b. Items missing from the response. The page-level items
      list is filtered by item_status (invoiced / with_driver /
      checked_in) per tab, so items still in the 'delivery_note'
      lifecycle state (paper sent, invoice not yet created)
      weren't in the page-wide list. Added a third PB query in
      the service that runs after the DN list lands and pulls
      every item whose delivery_note_number matches one of the
      page's DNs, regardless of item_status. Returned as a new
      dn_items field and threaded through to the expand panel
      as a 'dnItems' prop. The main items list still uses the
      status-filtered query so bulk actions stay scoped to the
      tab.
This commit is contained in:
David Els 2026-09-14 12:58:43 +02:00
parent 3d009b144f
commit 58b174606b
2 changed files with 345 additions and 51 deletions

View file

@ -22,17 +22,22 @@
*
* Grouping hierarchy on every tab:
*
* Customer X
* Order ORD-2026-01
* ☑ M10 Bolt (qty 50 @ R 1.20)
* ☐ M12 Nut (qty 30 @ R 2.10)
* Order ORD-2026-02
* ☐ M8 Bolt (qty 100 @ R 0.50)
* [Load all] [Load 1 selected]
*
* Customer Y
* Company X
* Customer A
* Order ORD-2026-01
* ☑ M10 Bolt (qty 50 @ R 1.20)
* ☐ M12 Nut (qty 30 @ R 2.10)
* ▾ DN26071401 (combined paper for this order — plus
* items contributed by orders below)
* • M10 Bolt
* • M12 Nut
* • M8 Washer (from order ORD-2026-02)
* Order ORD-2026-02
* ☐ M8 Bolt (qty 100 @ R 0.50)
* Customer B
* ...
* Company Y
* ...
* [Load all selected across all customers (3)]
*
* Search box hits every field the office might type into:
* Server-side (PB filter):
@ -84,6 +89,7 @@ import Link from "next/link";
// and revert when the project-wide lint config catches up.)
import { toast } from "sonner";
import {
Building2,
CheckSquare,
ChevronDown,
ChevronRight,
@ -130,10 +136,13 @@ type DeliveryItem = {
invoice_number: string | null;
loaded_at: string | null;
checked_in_at: string | null;
// DN this item was assembled into (denormalised from the page's
// /api/v1/deliveries response). Used by the per-row expand panel
// to filter items belonging to a specific DN.
delivery_note: string | null;
// Human-readable DN id (e.g. `DN26091405`) this item was
// assembled into. Used by the per-row expand panel to filter
// items belonging to a specific DN. This is the column
// actually populated in PB; the schema also has a relation
// field `delivery_note` that's vestigial — see the join
// comment in `DeliveryNoteRowComponent` below.
delivery_note_number: string | null;
unit: string | null;
order: {
id: string;
@ -173,6 +182,14 @@ type DeliveryNoteRow = {
type DeliveryListResponse = {
items: DeliveryItem[];
delivery_notes: DeliveryNoteRow[];
/**
* Items linked to the page's DNs, regardless of `item_status`.
* Drives the DN expand panel so it can show every item on the
* DN — same scope as the full /delivery-notes/{id} page. Items
* already in `items` are deduped by id, so the main list and
* the expand panel can never disagree about a row's data.
*/
dn_items?: DeliveryItem[];
totalItems: number;
totalDeliveryNotes: number;
page: number;
@ -213,11 +230,25 @@ const TAB_DEFS: Array<{
];
// ────────────────────────────────────────────────────────────────────
// Grouping — by customer, then by order. Ad-hoc rows (no parent
// order) land in a single "(no order)" bucket at the bottom of
// their customer group.
// Grouping — by company, then by customer, then by order. A single
// delivery note can combine items from multiple orders (the
// "combine DNs" flow), so DNs sit under whichever order the API
// reports as their primary order; when the operator expands a DN
// row, the page looks up its items across the entire page's
// visible items list, not just the current order's items.
//
// Ad-hoc rows (no parent order) land in a single "(no order)"
// bucket at the bottom of their customer group; ad-hoc customers
// (order exists but no customer) land in "(no customer)" under
// their company.
// ────────────────────────────────────────────────────────────────────
type CompanyGroup = {
companyId: string;
companyName: string;
customers: CustomerGroup[];
};
type CustomerGroup = {
customerId: string;
customerName: string;
@ -231,11 +262,23 @@ type OrderGroup = {
deliveryNotes: DeliveryNoteRow[];
};
function groupByCustomerOrder(
function groupByCompanyCustomerOrder(
items: DeliveryItem[],
deliveryNotes: DeliveryNoteRow[] = [],
): CustomerGroup[] {
const customerMap = new Map<string, CustomerGroup>();
): CompanyGroup[] {
const companyMap = new Map<string, CompanyGroup>();
const ensureCustomer = (
company: CompanyGroup,
customerId: string,
customerName: string,
): CustomerGroup => {
let customer = company.customers.find((c) => c.customerId === customerId);
if (!customer) {
customer = { customerId, customerName, orders: [] };
company.customers.push(customer);
}
return customer;
};
const ensureOrder = (
customer: CustomerGroup,
orderId: string,
@ -250,15 +293,23 @@ function groupByCustomerOrder(
};
for (const item of items) {
// Fallback chain: company → customer → "(no order)". When an
// item is counter-pickup / ad-hoc with no parent order, we drop
// it under a single "(no order)" bucket at the bottom of its
// customer group; if the customer is also missing, "(no
// customer)" under "(no company)".
const companyId = item.order?.company_id ?? "_no_order_";
const companyName = item.order?.company_name ?? "(no order)";
const customerId = item.order?.customer_id ?? "_no_order_";
const customerName =
item.order?.customer_name ?? item.order?.company_name ?? "(no order)";
let customer = customerMap.get(customerId);
if (!customer) {
customer = { customerId, customerName, orders: [] };
customerMap.set(customerId, customer);
let company = companyMap.get(companyId);
if (!company) {
company = { companyId, companyName, customers: [] };
companyMap.set(companyId, company);
}
const customer = ensureCustomer(company, customerId, customerName);
const orderId = item.order?.id ?? "_no_order_";
const orderNumber = item.order?.order_number ?? "Counter pickup";
@ -267,15 +318,18 @@ function groupByCustomerOrder(
}
for (const dn of deliveryNotes) {
const companyId = dn.order?.company_id ?? "_no_order_";
const companyName = dn.order?.company_name ?? "(no order)";
const customerId = dn.order?.customer_id ?? "_no_order_";
const customerName =
dn.order?.customer_name ?? dn.order?.company_name ?? "(no order)";
let customer = customerMap.get(customerId);
if (!customer) {
customer = { customerId, customerName, orders: [] };
customerMap.set(customerId, customer);
let company = companyMap.get(companyId);
if (!company) {
company = { companyId, companyName, customers: [] };
companyMap.set(companyId, company);
}
const customer = ensureCustomer(company, customerId, customerName);
const orderId = dn.order?.id ?? "_no_order_";
const orderNumber = dn.order?.order_number ?? "Counter pickup";
@ -283,15 +337,24 @@ function groupByCustomerOrder(
order.deliveryNotes.push(dn);
}
// Stable order: customers alphabetical, orders inside customer
// by order_number descending so the newest order sits on top.
return [...customerMap.values()]
.sort((a, b) => a.customerName.localeCompare(b.customerName))
// Stable order: companies alphabetical, customers within a
// company alphabetical, orders within a customer by order_number
// descending so the newest order sits on top. The catch-all
// "(no order)" buckets (no parent order / no customer / no
// company) sort last alphabetically — "(", "(no company)", etc.
// all come after any real name.
return [...companyMap.values()]
.sort((a, b) => a.companyName.localeCompare(b.companyName))
.map((c) => ({
...c,
orders: [...c.orders].sort((a, b) =>
b.orderNumber.localeCompare(a.orderNumber),
),
customers: [...c.customers]
.sort((a, b) => a.customerName.localeCompare(b.customerName))
.map((cu) => ({
...cu,
orders: [...cu.orders].sort((a, b) =>
b.orderNumber.localeCompare(a.orderNumber),
),
})),
}));
}
@ -343,6 +406,11 @@ export default function DeliveriesPage() {
const [debouncedSearch, setDebouncedSearch] = useState("");
const [items, setItems] = useState<DeliveryItem[]>([]);
const [deliveryNotes, setDeliveryNotes] = useState<DeliveryNoteRow[]>([]);
// Items linked to the page's DNs regardless of item_status —
// feeds the DN expand panel so it shows every item on the DN
// (matching the full /delivery-notes/{id} page). See
// `dn_items` in the deliveries-service response shape.
const [dnItems, setDnItems] = useState<DeliveryItem[]>([]);
const [loading, setLoading] = useState(false);
const [error, setError] = useState("");
const [totalItems, setTotalItems] = useState(0);
@ -429,6 +497,13 @@ export default function DeliveriesPage() {
const data = (await res.json()) as DeliveryListResponse;
setItems(data.items);
setDeliveryNotes(data.delivery_notes ?? []);
// `dn_items` is optional in the response shape — older
// deployments may not include it yet. Fall back to `items`
// so the expand panel still has something to look up; that
// path only finds items in the current tab's status, which
// is the old behaviour. Once the new service ships, the
// expand panel finds every item regardless of status.
setDnItems(data.dn_items ?? data.items);
setTotalItems(data.totalItems);
setTotalDeliveryNotes(data.totalDeliveryNotes ?? 0);
} catch (err) {
@ -536,7 +611,7 @@ export default function DeliveriesPage() {
}, [visibleItems]);
const grouped = useMemo(
() => groupByCustomerOrder(visibleItems, visibleDeliveryNotes),
() => groupByCompanyCustomerOrder(visibleItems, visibleDeliveryNotes),
[visibleItems, visibleDeliveryNotes],
);
@ -940,10 +1015,10 @@ export default function DeliveriesPage() {
</Card>
) : (
<div className="space-y-5">
{grouped.map((customer) => (
<CustomerGroupView
key={customer.customerId}
customer={customer}
{grouped.map((company) => (
<CompanyGroupView
key={company.companyId}
company={company}
activeTab={activeTab}
selectedIds={selectedIds}
onToggleOne={toggleOne}
@ -953,6 +1028,7 @@ export default function DeliveriesPage() {
onSignedCopy={openSignedCopyModal}
onMarkLoaded={handleMarkLoaded}
busy={busy}
dnItems={dnItems}
/>
))}
@ -1182,6 +1258,74 @@ export default function DeliveriesPage() {
* One customer block: name header + one or more order groups.
* Pure presentational — receives everything via props.
*/
/**
* One company block: company name header + one or more customer
* groups. Pure presentational — receives everything via props.
*/
function CompanyGroupView({
company,
activeTab,
selectedIds,
onToggleOne,
onToggleAllInOrder,
onLoad,
onCheckIn,
onSignedCopy,
onMarkLoaded,
busy,
dnItems,
}: {
company: CompanyGroup;
activeTab: DeliveryTab;
selectedIds: Set<string>;
onToggleOne: (id: string) => void;
onToggleAllInOrder: (order: OrderGroup) => void;
onLoad: (itemIds: string[]) => void;
onCheckIn: (itemIds: string[]) => void;
onSignedCopy: (deliveryNoteId: string, deliveryNoteNumber: string) => void;
onMarkLoaded: (deliveryNoteId: string, deliveryNoteNumber: string) => void;
busy: boolean;
/**
* Every item linked to one of this company's DNs, regardless
* of `item_status`. Used by the per-DN expand panel so it
* shows every item on the DN — same scope as the full
* /delivery-notes/{id} page. Forwarded all the way down to
* DeliveryNoteRowComponent.
*/
dnItems: DeliveryItem[];
}) {
return (
<div className="space-y-3">
<div className="flex items-center gap-2 text-sm font-semibold uppercase tracking-wide text-muted-foreground">
<Building2 className="h-3.5 w-3.5" />
{company.companyName}
</div>
<div className="space-y-3 pl-1 sm:pl-4">
{company.customers.map((customer) => (
<CustomerGroupView
key={customer.customerId}
customer={customer}
activeTab={activeTab}
selectedIds={selectedIds}
onToggleOne={onToggleOne}
onToggleAllInOrder={onToggleAllInOrder}
onLoad={onLoad}
onCheckIn={onCheckIn}
onSignedCopy={onSignedCopy}
onMarkLoaded={onMarkLoaded}
busy={busy}
dnItems={dnItems}
/>
))}
</div>
</div>
);
}
/**
* One customer block: customer name header + one or more order
* groups. Pure presentational — receives everything via props.
*/
function CustomerGroupView({
customer,
activeTab,
@ -1193,6 +1337,7 @@ function CustomerGroupView({
onSignedCopy,
onMarkLoaded,
busy,
dnItems,
}: {
customer: CustomerGroup;
activeTab: DeliveryTab;
@ -1204,6 +1349,8 @@ function CustomerGroupView({
onSignedCopy: (deliveryNoteId: string, deliveryNoteNumber: string) => void;
onMarkLoaded: (deliveryNoteId: string, deliveryNoteNumber: string) => void;
busy: boolean;
/** Forwarded to OrderGroupView → DeliveryNoteRowComponent. */
dnItems: DeliveryItem[];
}) {
return (
<div className="space-y-3">
@ -1225,6 +1372,7 @@ function CustomerGroupView({
onSignedCopy={onSignedCopy}
onMarkLoaded={onMarkLoaded}
busy={busy}
dnItems={dnItems}
/>
))}
</div>
@ -1248,6 +1396,7 @@ function OrderGroupView({
onSignedCopy,
onMarkLoaded,
busy,
dnItems,
}: {
order: OrderGroup;
activeTab: DeliveryTab;
@ -1259,6 +1408,13 @@ function OrderGroupView({
onSignedCopy: (deliveryNoteId: string, deliveryNoteNumber: string) => void;
onMarkLoaded: (deliveryNoteId: string, deliveryNoteNumber: string) => void;
busy: boolean;
/**
* Every item linked to one of the page's DNs, regardless of
* `item_status`. Forwarded to DeliveryNoteRowComponent so a
* DN's expand panel can find items that belong to the DN but
* live under a different order in the page's grouping.
*/
dnItems: DeliveryItem[];
}) {
// Set of DN ids whose items panel is expanded. Independent state
// per DN; resets on refresh. Spec § Feature 1 — Interaction details.
@ -1384,7 +1540,7 @@ function OrderGroupView({
busy={busy}
expanded={expandedDnIds.has(dn.id)}
onToggle={() => toggleDnExpanded(dn.id)}
items={order.items}
items={dnItems}
/>
))}
</CardContent>
@ -1553,12 +1709,29 @@ function DeliveryNoteRowComponent({
// abandoned DN would be misleading.
const canUploadSignedCopy = dn.status !== "cancelled";
// Items on this DN — drawn from this order group's items
// array. Stale-by-design: a refresh picks up changes. See spec
// § Feature 1 — Data source.
// Items on this DN — drawn from the page-wide visible items
// list passed in via `items`. A DN can combine items from
// multiple orders (the "combine DNs" flow), so the items can
// live under any order in the page's grouping. Looking them up
// here against the full list (rather than restricting to a
// single order's items) is what lets the expand panel show
// every item on the DN.
//
// Join key: `delivery_note_number` (the human-readable DN id,
// e.g. `DN26091405`). This is the column actually populated
// on order_items; the schema also has a relation field
// `delivery_note` but nothing writes to it. Earlier revisions
// of this code compared `i.delivery_note === dn.id` — that's
// why every DN on /deliveries used to show "no items loaded
// on this delivery note" even when the same DN had items
// visible on the full /delivery-notes/{id} page (which joins
// via `delivery_note_number`). The full DN page joins by the
// text column; this filter does the same.
//
// Stale-by-design: a refresh picks up changes.
const dnItems = useMemo(
() => items.filter((i) => i.delivery_note === dn.id),
[items, dn.id],
() => items.filter((i) => i.delivery_note_number === dn.delivery_note_number),
[items, dn.delivery_note_number],
);
// Header-line suffix logic per spec: suffix omitted when all

View file

@ -92,6 +92,15 @@ export type DeliveryListResult = {
cost_price: number | null;
item_status: string;
delivery_reference: string | null;
/**
* Human-readable DN id (e.g. `DN26091405`) this item
* belongs to. The /deliveries page's DN expand panel
* joins on this field against each delivery note's
* `delivery_note_number`. The PB schema's relation
* field `delivery_note` exists but nothing writes to
* it; the text column is the actual join key.
*/
delivery_note_number: string | null;
/**
* QuickBooks invoice number, extracted from the nested
* `order_item_details.invoice.number` JSON. Null for rows
@ -135,6 +144,35 @@ export type DeliveryListResult = {
company_name: string | null;
} | null;
}>;
/**
* Items linked to the page's DNs regardless of `item_status`.
* Feeds the DN expand panel so it shows every item on the DN
* (same scope as the full /delivery-notes/{id} page). The
* main items list on the page uses `items` only — `dn_items`
* is never rendered as a top-level row.
*/
dn_items: Array<{
id: string;
description: string;
quantity: number | null;
selling_price: number | null;
cost_price: number | null;
item_status: string;
delivery_reference: string | null;
delivery_note_number: string | null;
invoice_number: string | null;
loaded_at: string | null;
checked_in_at: string | null;
order: {
id: string;
order_number: string;
customer_id: string;
customer_name: string | null;
company_id: string;
company_name: string | null;
} | null;
checked_in_by: { id: string; name?: string; email?: string } | null;
}>;
totalItems: number;
totalDeliveryNotes: number;
page: number;
@ -302,7 +340,13 @@ export async function listDeliveryItems(params: {
expand: "order.customer,order.company,checked_in_by",
fields:
"id,description,quantity,unit,selling_price,cost_price,item_status,delivery_reference," +
"delivery_note,loaded_at,checked_in_at,checked_in_by,order,updated,order_item_details," +
// `delivery_note_number` is the human-readable DN id (e.g.
// `DN26091405`). The PB schema also has a relation field
// `delivery_note` but nothing currently writes to it — the
// join key everywhere else (the full DN detail page, the
// delivery-notes service) is the text column. We only
// request the column that's actually populated.
"delivery_note_number,loaded_at,checked_in_at,checked_in_by,order,updated,order_item_details," +
"expand.order,expand.order.customer,expand.order.company,expand.checked_in_by",
}),
pb.collection(DELIVERY_NOTES).getList(page, perPage, {
@ -315,6 +359,51 @@ export async function listDeliveryItems(params: {
}),
]);
// Query #3 — every order_item that has a `delivery_note_number`
// set to one of the DNs in this response, regardless of the
// item's own `item_status`. The DN expand panel needs these so
// it can show every item on the DN — same scope as the full
// /delivery-notes/{id} page (which joins by `delivery_note_number`
// without filtering on item_status). Without this, a DN whose
// items are still in `delivery_note` status (paper sent, invoice
// not yet created) shows "no items loaded" on the expand panel
// even though the same DN has items on the full detail page.
// The main items list (query #1) is what drives the page-level
// bulk actions; this third query only feeds the per-DN expand
// panel — so its status-unfiltered scope doesn't leak into the
// bulk-action rows. Runs after the first two so we have the DN
// numbers in hand; the wire cost is one extra PB read on top
// of the first two parallel queries.
//
// PB rejects filter strings longer than ~8 KB. ~40 chars per
// `delivery_note_number = "DN..." || ` — 200 candidates keeps
// us under the limit. Same pattern as
// order-delivery-note-link-service.ts.
const dnNumbers = (dnsResult.items as unknown as Array<{ delivery_note_number?: string }>)
.map((d) => d.delivery_note_number)
.filter((n): n is string => typeof n === "string" && n.length > 0);
let dnItemsResult: { items: Array<Record<string, unknown>>; totalItems: number } = {
items: [],
totalItems: 0,
};
if (dnNumbers.length > 0) {
const batch = dnNumbers.slice(0, 200);
const dnItemsFilter = batch
.map((n) => `delivery_note_number = "${n.replace(/"/g, '\\"')}"`)
.join(" || ");
dnItemsResult = (await pb
.collection(getOrderItemsCollectionName())
.getList(1, perPage, {
filter: dnItemsFilter,
sort: "-updated",
expand: "order.customer,order.company",
fields:
"id,description,quantity,unit,selling_price,cost_price,item_status,delivery_reference," +
"delivery_note_number,loaded_at,checked_in_at,checked_in_by,order,updated,order_item_details," +
"expand.order,expand.order.customer,expand.order.company",
})) as unknown as { items: Array<Record<string, unknown>>; totalItems: number };
}
type Row = {
id: string;
description: string;
@ -323,7 +412,15 @@ export async function listDeliveryItems(params: {
cost_price: number | null;
item_status: string;
delivery_reference: string | null;
delivery_note: string | null;
/**
* Human-readable DN id (e.g. `DN26091405`). The /deliveries
* page's DN expand panel uses this to filter the items that
* belong to a given DN — the same join key the full DN detail
* page uses (see `pb-helpers.getDeliveryNoteItems`). The
* relation field `delivery_note` in the schema is never
* populated; this is the column we actually care about.
*/
delivery_note_number: string | null;
unit: string | null;
loaded_at: string | null;
checked_in_at: string | null;
@ -365,7 +462,7 @@ export async function listDeliveryItems(params: {
return typeof n === "string" && n.length > 0 ? n : null;
}
const items = (itemsResult.items as unknown as Row[]).map((r) => ({
const mapRow = (r: Row) => ({
id: r.id,
description: r.description ?? "",
quantity: r.quantity ?? null,
@ -373,7 +470,7 @@ export async function listDeliveryItems(params: {
cost_price: r.cost_price ?? null,
item_status: r.item_status,
delivery_reference: r.delivery_reference ?? null,
delivery_note: r.delivery_note ?? null,
delivery_note_number: r.delivery_note_number ?? null,
unit: r.unit ?? null,
invoice_number: readInvoiceNumber(r.order_item_details),
loaded_at: r.loaded_at ?? null,
@ -402,7 +499,23 @@ export async function listDeliveryItems(params: {
email: r.expand.checked_in_by.email,
}
: null,
}));
});
const items = (itemsResult.items as unknown as Row[]).map(mapRow);
// Map the third query (DN-linked items, any status) through the
// same mapper. Deduped by item id so the per-DN expand panel
// doesn't render the same row twice when an item already
// appears in `items` (status-filtered) AND `dn_items`
// (DN-linked, any status). A `Map` keyed by item id preserves
// the first occurrence — the status-filtered one, which is the
// version the page renders in the main list. The expand panel
// looks up by item id via `dnItemsById` and never sees a dup.
const dnItemsRaw = (dnItemsResult.items as unknown as Row[]).map(mapRow);
const dnItemsById = new Map<string, (typeof dnItemsRaw)[number]>();
for (const it of items) dnItemsById.set(it.id, it);
for (const it of dnItemsRaw) if (!dnItemsById.has(it.id)) dnItemsById.set(it.id, it);
const dn_items = [...dnItemsById.values()];
// Map DN rows. The expand shape mirrors the items query so the
// page can group both row kinds by customer → order with the
@ -451,6 +564,14 @@ export async function listDeliveryItems(params: {
return {
items,
delivery_notes,
/**
* Items linked to the page's DNs regardless of `item_status`.
* Feeds the DN expand panel so it shows every item on the
* DN (same scope as the full /delivery-notes/{id} page).
* The main items list on the page uses `items` only — this
* field is never rendered as a top-level row.
*/
dn_items,
totalItems: itemsResult.totalItems,
totalDeliveryNotes: dnsResult.totalItems,
page: itemsResult.page,