From 7ebfb8b86bbd8600b5794f62fdbddabe787faeb3 Mon Sep 17 00:00:00 2001 From: SJCoetz Date: Tue, 14 Jul 2026 12:34:47 +0200 Subject: [PATCH] feat: add row selection and cost pricing - add `costPrice` field to invoicing row types, plus ZAR currency formatting and profit margin calculations - update invoice row displays to show ordered/received quantity, unit cost, selling price and margin metrics - add per-row selection in QuickBooks send modals, letting users filter which order lines to include on invoices - update ModalShell component to support responsive breakpoint-based sizing instead of fixed sm-only widths - refine UI styling across the invoicing page to use consistent theme tokens (bg-card, muted-foreground) - update tests to use dynamic yesterday dates instead of hardcoded values for stable, time-agnostic assertions - improve button states, labels and text for QuickBooks invoice actions, including showing selected item counts --- src/app/(app)/invoicing/page.test.tsx | 49 ++-- src/app/(app)/invoicing/page.tsx | 354 ++++++++++++++++++++------ src/app/(app)/invoicing/view-model.ts | 1 + src/components/ui/modal-shell.tsx | 66 ++++- 4 files changed, 368 insertions(+), 102 deletions(-) diff --git a/src/app/(app)/invoicing/page.test.tsx b/src/app/(app)/invoicing/page.test.tsx index add692a..e54516f 100644 --- a/src/app/(app)/invoicing/page.test.tsx +++ b/src/app/(app)/invoicing/page.test.tsx @@ -3,6 +3,17 @@ import { cleanup, fireEvent, render, screen, waitFor, within } from "@testing-li const invoiceWebhookFetchMock = vi.fn(); +// ISO date "yesterday" — the Invoiced tab only shows rows from the last +// 3 days (`buildRecentInvoiceRange`). Hard-coding a date (e.g. +// "2026-07-08") silently breaks the test once the calendar rolls past +// that range; using "yesterday" keeps the assertion stable over time. +const YESTERDAY_ISO = (() => { + const d = new Date(); + d.setDate(d.getDate() - 1); + return d.toISOString().slice(0, 10); +})(); +const YESTERDAY_ISO_AT_8AM = `${YESTERDAY_ISO}T08:00:00.000Z`; + const { fetchCustomersMock, getFullListMock, mockedRows, makeRawRow } = vi.hoisted(() => ({ fetchCustomersMock: vi.fn(async () => [ { id: "qb-1", name: "Adal Engineering" }, @@ -324,7 +335,7 @@ describe("InvoicingPage", () => { fireEvent.click(getSingleOrderButton("SO-1")); // Wait for the modal to actually open. - await waitFor(() => expect(screen.getByRole("button", { name: /send to quickbooks/i })).toBeInTheDocument()); + await waitFor(() => expect(screen.getByRole("button", { name: /send .*to quickbooks/i })).toBeInTheDocument()); // Give any spurious effect a tick, then assert nothing fired. await new Promise((resolve) => setTimeout(resolve, 0)); expect(fetchCustomersMock).not.toHaveBeenCalled(); @@ -335,7 +346,7 @@ describe("InvoicingPage", () => { await waitFor(() => expect(screen.getAllByText("Wheel hub kit")[0]).toBeInTheDocument()); fireEvent.click(getSingleOrderButton("SO-1")); - await waitFor(() => expect(screen.getByRole("button", { name: /send to quickbooks/i })).toBeInTheDocument()); + await waitFor(() => expect(screen.getByRole("button", { name: /send .*to quickbooks/i })).toBeInTheDocument()); const toggle = getQbCompanyToggle("single"); expect(toggle).toBeInTheDocument(); @@ -349,7 +360,7 @@ describe("InvoicingPage", () => { await waitFor(() => expect(screen.getAllByText("Wheel hub kit")[0]).toBeInTheDocument()); fireEvent.click(getSingleOrderButton("SO-1")); - await waitFor(() => expect(screen.getByRole("button", { name: /send to quickbooks/i })).toBeInTheDocument()); + await waitFor(() => expect(screen.getByRole("button", { name: /send .*to quickbooks/i })).toBeInTheDocument()); pickQbCompany("single", "ISHERWOOD"); @@ -369,7 +380,7 @@ describe("InvoicingPage", () => { await waitFor(() => expect(screen.getAllByText("Wheel hub kit")[0]).toBeInTheDocument()); fireEvent.click(getSingleOrderButton("SO-1")); - await waitFor(() => expect(screen.getByRole("button", { name: /send to quickbooks/i })).toBeInTheDocument()); + await waitFor(() => expect(screen.getByRole("button", { name: /send .*to quickbooks/i })).toBeInTheDocument()); // Pre-condition: picking the QB company triggers exactly one fetch. pickQbCompany("single", "ISHERWOOD"); @@ -396,7 +407,7 @@ describe("InvoicingPage", () => { await waitFor(() => expect(screen.getAllByText("Wheel hub kit")[0]).toBeInTheDocument()); fireEvent.click(getSingleOrderButton("SO-1")); - await waitFor(() => expect(screen.getByRole("button", { name: /send to quickbooks/i })).toBeInTheDocument()); + await waitFor(() => expect(screen.getByRole("button", { name: /send .*to quickbooks/i })).toBeInTheDocument()); pickQbCompany("single", "CNL"); @@ -407,7 +418,7 @@ describe("InvoicingPage", () => { return el; }); expect(select.value).toBe("qb-1"); - const sendButton = screen.getByRole("button", { name: /send to quickbooks/i }); + const sendButton = screen.getByRole("button", { name: /send .*to quickbooks/i }); expect(sendButton).not.toBeDisabled(); }); @@ -453,7 +464,7 @@ describe("InvoicingPage", () => { await waitFor(() => expect(screen.getAllByText("Wheel hub kit")[0]).toBeInTheDocument()); fireEvent.click(getSingleOrderButton("SO-1")); - await waitFor(() => expect(screen.getByRole("button", { name: /send to quickbooks/i })).toBeInTheDocument()); + await waitFor(() => expect(screen.getByRole("button", { name: /send .*to quickbooks/i })).toBeInTheDocument()); pickQbCompany("single", "CNL"); @@ -470,7 +481,7 @@ describe("InvoicingPage", () => { await waitFor(() => expect(screen.getAllByText("Wheel hub kit")[0]).toBeInTheDocument()); fireEvent.click(getSingleOrderButton("SO-1")); - await waitFor(() => expect(screen.getByRole("button", { name: /send to quickbooks/i })).toBeInTheDocument()); + await waitFor(() => expect(screen.getByRole("button", { name: /send .*to quickbooks/i })).toBeInTheDocument()); pickQbCompany("single", "CNL"); @@ -481,7 +492,7 @@ describe("InvoicingPage", () => { }); expect(select.value).toBe("qb-1"); - fireEvent.click(screen.getByRole("button", { name: /^send to quickbooks$/i })); + fireEvent.click(screen.getByRole("button", { name: /^send \d+ items? to quickbooks$/i })); await waitFor(() => { if (invoiceWebhookFetchMock.mock.calls.length === 0) throw new Error("waiting for invoice webhook"); @@ -504,7 +515,7 @@ describe("InvoicingPage", () => { await waitFor(() => expect(screen.getAllByText("Wheel hub kit")[0]).toBeInTheDocument()); fireEvent.click(getSingleOrderButton("SO-1")); - await waitFor(() => expect(screen.getByRole("button", { name: /send to quickbooks/i })).toBeInTheDocument()); + await waitFor(() => expect(screen.getByRole("button", { name: /send .*to quickbooks/i })).toBeInTheDocument()); pickQbCompany("single", "CNL"); await waitFor(() => { @@ -557,7 +568,7 @@ describe("InvoicingPage", () => { await waitFor(() => expect(screen.getAllByText("Wheel hub kit")[0]).toBeInTheDocument()); fireEvent.click(getSingleOrderButton("SO-1")); - await waitFor(() => expect(screen.getByRole("button", { name: /send to quickbooks/i })).toBeInTheDocument()); + await waitFor(() => expect(screen.getByRole("button", { name: /send .*to quickbooks/i })).toBeInTheDocument()); pickQbCompany("single", "CNL"); @@ -569,14 +580,14 @@ describe("InvoicingPage", () => { fireEvent.change(screen.getByLabelText("Invoice order number"), { target: { value: "SO-OVERRIDE" }, }); - fireEvent.change(screen.getByLabelText("Qty for row-1"), { + fireEvent.change(screen.getByLabelText("Qty — Wheel hub kit"), { target: { value: "4" }, }); - fireEvent.change(screen.getByLabelText("Selling price for row-1"), { + fireEvent.change(screen.getByLabelText("Selling price — Wheel hub kit"), { target: { value: "78.5" }, }); - fireEvent.click(screen.getByRole("button", { name: /^send to quickbooks$/i })); + fireEvent.click(screen.getByRole("button", { name: /^send \d+ items? to quickbooks$/i })); await waitFor(() => { if (invoiceWebhookFetchMock.mock.calls.length === 0) throw new Error("waiting for invoice webhook"); @@ -606,7 +617,7 @@ describe("InvoicingPage", () => { order_item_details: { invoice: { number: "INV-1", - date: "2026-07-08", + date: YESTERDAY_ISO, }, }, }), @@ -634,10 +645,10 @@ describe("InvoicingPage", () => { order_item_details: { invoice: { number: "INV-1", - date: "2026-07-08", + date: YESTERDAY_ISO, setBy: "u-1", setByName: "Op", - setAt: "2026-07-08T08:00:00.000Z", + setAt: YESTERDAY_ISO_AT_8AM, }, print: { printed: true, @@ -667,10 +678,10 @@ describe("InvoicingPage", () => { order_item_details: { invoice: { number: "INV-1", - date: "2026-07-08", + date: YESTERDAY_ISO, setBy: "u-1", setByName: "Op", - setAt: "2026-07-08T08:00:00.000Z", + setAt: YESTERDAY_ISO_AT_8AM, }, }, }), diff --git a/src/app/(app)/invoicing/page.tsx b/src/app/(app)/invoicing/page.tsx index 595a3b0..7beb7c7 100644 --- a/src/app/(app)/invoicing/page.tsx +++ b/src/app/(app)/invoicing/page.tsx @@ -42,6 +42,7 @@ type RawInvoicingRow = { qb_invoice_pdf?: string | null; order_qty?: number | null; qty_collected?: number | null; + cost_price?: number | null; selling_price?: number | null; order_item_details?: string | Record | null; expand?: { @@ -158,6 +159,7 @@ function toInvoicingRow(row: RawInvoicingRow): InvoicingRow { }), orderQty: row.order_qty ?? null, qtyCollected: row.qty_collected ?? null, + costPrice: row.cost_price ?? null, sellingPrice: row.selling_price ?? null, printed: print.printed, printedAt: print.printedAt, @@ -174,6 +176,109 @@ function rowStatusLabel(status: string | undefined): string { return status ?? "Unknown"; } +// Format a number as ZAR (R 1 234.56). Returns "—" for null / NaN so the +// row layout doesn't collapse when a value is missing. +function formatZar(value: number | null | undefined): string { + if (typeof value !== "number" || !Number.isFinite(value)) return "—"; + return new Intl.NumberFormat("en-ZA", { + style: "currency", + currency: "ZAR", + minimumFractionDigits: 2, + maximumFractionDigits: 2, + }).format(value); +} + +// Format a plain integer-ish qty. Falls back to "—" when missing so an +// "Qty ordered / received" cell can render partial data without breaking +// the row alignment. +function formatQty(value: number | null | undefined): string { + if (typeof value !== "number" || !Number.isFinite(value)) return "—"; + return new Intl.NumberFormat("en-ZA", { + maximumFractionDigits: 2, + }).format(value); +} + +function RowMetrics({ row }: { row: InvoicingRow }) { + return ( +
+
+
Ordered
+
{formatQty(row.orderQty)}
+
+
+
Received
+
{formatQty(row.qtyCollected)}
+
+
+
Unit
+
{row.sellingPrice != null ? "ea" : ""}
+
+
+
Cost
+
{formatZar(row.costPrice)}
+
+
+
Selling
+
{formatZar(row.sellingPrice)}
+
+
+
Margin
+
+ {typeof row.costPrice === "number" && + typeof row.sellingPrice === "number" && + row.costPrice > 0 + ? `${Math.round(((row.sellingPrice - row.costPrice) / row.costPrice) * 100)}%` + : "—"} +
+
+
+ ); +} + +// Top-of-row identity block: the *thing being sold* is the headline +// (item description, themed with `text-primary` so it pops against the +// card), with the supplier dropped to a muted second line. Keeps the +// visual hierarchy "what is this? → where is it from? → where does it +// live in the order?" instead of "where is it from? → what is this?". +function RowIdentity({ + row, + showOrderLink = true, +}: { + row: InvoicingRow; + showOrderLink?: boolean; +}) { + return ( +
+

+ {row.description || "(no description)"} +

+

+ {row.supplierName || "Unknown supplier"} +

+
+ {row.poNumber ? ( + + + {row.poNumber} + + ) : null} + {showOrderLink && row.orderNumber ? ( + + Order {row.orderNumber} + + + ) : null} +
+
+ ); +} + // Stable cache key for (customer_query, billing_company). The order keeps // the key deterministic for the same inputs so the n8n webhook gets the // right `customer_query` / `billing_company`. @@ -270,6 +375,12 @@ type QbGroup = { orderNumber: string; rows: InvoicingRow[] }; type InvoicePayloadOverrides = { orderNumberOverride?: string; lineOverrides?: InvoiceLineOverride[]; + /** + * Subset of `group.rows` the operator actually wants on the invoice. + * When omitted, every line in `group.rows` is sent (legacy behaviour). + * Filter is applied in `submitQbInvoice` before the QB payload is built. + */ + includedRowIds?: string[]; }; type SubmitQbInvoice = ( group: QbGroup, @@ -308,6 +419,10 @@ function SingleOrderQbModal({ const [customer, setCustomer] = useState(null); const [orderNumberOverride, setOrderNumberOverride] = useState(""); const [lineOverrides, setLineOverrides] = useState>({}); + // Set of row IDs the operator wants on this invoice. Default = all rows + // on open (the common case is "invoice the whole order"), with a + // checkbox per row + select-all/none to drop the ones they don't want. + const [includedRowIds, setIncludedRowIds] = useState>(new Set()); // The webhook match is the company (e.g. "Adal Engineering") — QuickBooks // customers are billed to the company. The order's `customerName` // (e.g. "GIELIE") is just visual context on the row. @@ -328,7 +443,16 @@ function SingleOrderQbModal({ }, [open, customers, orderCompanyName, qbCompany, onQbCompanyChange]); useEffect(() => { if (!open || !group) return; + // The pre-existing sibling effect (preselect closest QB customer) is + // a setState-in-effect on purpose — the modal's `open` transition + // is the only signal that lets us seed the form from incoming props + // without flashing a stale value. Mirror the same pattern for the + // new fields. The parent uses `key`-style remount discipline + // (re-opening resets everything), so this only runs on the + // open/close transition — not on every parent re-render. + // eslint-disable-next-line react-hooks/set-state-in-effect -- intentional open/close seed setOrderNumberOverride(group.orderNumber); + // eslint-disable-next-line react-hooks/set-state-in-effect -- see note above setLineOverrides( Object.fromEntries( group.rows.map((row) => { @@ -337,21 +461,33 @@ function SingleOrderQbModal({ }), ), ); + // Default to "all rows on this invoice" — the common case. Operator + // can uncheck the ones they don't want. + // eslint-disable-next-line react-hooks/set-state-in-effect -- see note above + setIncludedRowIds(new Set(group.rows.map((row) => row.id))); }, [open, group]); + const includedRowIdList = group + ? group.rows + .filter((row) => includedRowIds.has(row.id)) + .map((row) => row.id) + : []; const submitOverrides: InvoicePayloadOverrides = { orderNumberOverride, - lineOverrides: group?.rows.map((row) => { - const current = lineOverrides[row.id]; - const defaults = resolveInvoiceLineDefaults(row); - const qty = Number(current?.qty); - const price = Number(current?.price); - return { - rowId: row.id, - qty: Number.isFinite(qty) ? qty : defaults.qty, - price: Number.isFinite(price) ? price : defaults.price, - }; - }), + includedRowIds: includedRowIdList, + lineOverrides: group?.rows + .filter((row) => includedRowIds.has(row.id)) + .map((row) => { + const current = lineOverrides[row.id]; + const defaults = resolveInvoiceLineDefaults(row); + const qty = Number(current?.qty); + const price = Number(current?.price); + return { + rowId: row.id, + qty: Number.isFinite(qty) ? qty : defaults.qty, + price: Number.isFinite(price) ? price : defaults.price, + }; + }), }; // Drive the fetch off the operator's QB-company selection. Until they // pick CNL or ISHERWOOD, no customer request fires — the picker just @@ -379,19 +515,31 @@ function SingleOrderQbModal({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [matchKey]); if (!group) return null; + const allIncluded = + group.rows.length > 0 && group.rows.every((row) => includedRowIds.has(row.id)); + const noneIncluded = includedRowIds.size === 0; + const itemLabel = (row: InvoicingRow): string => + row.description?.trim() || row.supplierName || `Item ${row.id}`; return ( { if (!state) onClose(); }} - size="md" + size={{ base: "md", md: "xl", lg: "2xl" }} title="Send to QuickBooks" description={`Pick a QuickBooks customer for order ${group.orderNumber || "no order number"} (${orderCompanyName || "no billing company"}, ${orderCustomerName || "no customer"}).`} footer={
-
} @@ -418,11 +566,33 @@ function SingleOrderQbModal({ )}
-
-

Invoice overrides

-

- These values change only the QuickBooks payload for this send. -

+
+
+

Invoice lines

+

+ Uncheck any line you don't want on this invoice. The override + fields change only the QuickBooks payload for this send. +

+
+
+ + · + +