diff --git a/src/components/orders-review-modal.test.tsx b/src/components/orders-review-modal.test.tsx index 886ee89..9db64ca 100644 --- a/src/components/orders-review-modal.test.tsx +++ b/src/components/orders-review-modal.test.tsx @@ -140,6 +140,36 @@ describe("OrdersReviewModal", () => { expect(screen.getByDisplayValue("50")).toBeInTheDocument(); }); + it("Cost and Markup columns are not rendered", () => { + // Spec: the review modal no longer surfaces cost or markup as + // user-editable inputs — those values are set later in the + // order-processing flow. Assert the labels are not present in + // either the item list OR the header totals (the + // "Cost R" segment was removed in the same change). + render(); + const list = screen.getByTestId("review-items-list"); + expect(list.textContent).not.toMatch(/Cost/); + expect(list.textContent).not.toMatch(/Markup/); + expect(screen.queryByLabelText(/cost/i)).toBeNull(); + expect(screen.queryByLabelText(/markup/i)).toBeNull(); + }); + + it("header totals show only selling total (no cost total)", () => { + // Three items with deterministic totals. Selling sum is 410. + const items: OrderItem[] = [ + { ...baseItems[0], id: "item-1", quantity: 2, cost_price: 50, selling_price: 100 }, + { ...baseItems[0], id: "item-2", quantity: 1, cost_price: 20, selling_price: 30 }, + { ...baseItems[0], id: "item-3", quantity: 3, cost_price: 10, selling_price: 60 }, + ]; + render(); + const totals = screen.getByTestId("review-items-totals"); + expect(totals.textContent).toMatch(/3 items/); + expect(totals.textContent).toMatch(/Selling/); + expect(totals.textContent).toMatch(/410/); + // The header should NOT contain the word "Cost" + "R" prefix. + expect(totals.textContent).not.toMatch(/Cost R/); + }); + it("Save button is disabled until a field is changed", () => { render(); const save = screen.getByRole("button", { name: /save/i }); @@ -193,62 +223,6 @@ describe("OrdersReviewModal", () => { ).toHaveAttribute("href", "/orders/order-other"); }); - it("lock pattern: editing cost recomputes selling and flips lock to 'cost'", () => { - render(); - - // baseItems: cost=25, selling=50, markup=100 → initial lock="cost". - // Edit cost from 25 → 40 (markup=100). Expected: selling = round2(40*2) = 80. - const costInputs = screen.getAllByDisplayValue("25"); - expect(costInputs).toHaveLength(1); - fireEvent.change(costInputs[0], { target: { value: "40" } }); - - // Selling should now show 80. - expect(screen.getByDisplayValue("80")).toBeInTheDocument(); - expect(screen.queryByDisplayValue("50")).not.toBeInTheDocument(); - }); - - it("lock pattern: editing selling recomputes cost and flips lock to 'selling'", () => { - render(); - - // baseItems: cost=25, selling=50, markup=100 → initial lock="cost". - // Edit selling from 50 → 100 (markup=100). Expected: cost = round2(100/2) = 50. - const sellingInputs = screen.getAllByDisplayValue("50"); - expect(sellingInputs).toHaveLength(1); - fireEvent.change(sellingInputs[0], { target: { value: "100" } }); - - // Cost should now show 50. - expect(screen.getByDisplayValue("50")).toBeInTheDocument(); - expect(screen.queryByDisplayValue("25")).not.toBeInTheDocument(); - }); - - it("lock pattern: editing markup with lock='selling' recomputes cost", () => { - render(); - - // baseItems: cost=25, selling=50, markup=100 → initial lock="cost". - // To exercise the markup-edit-costh-recompute branch, first flip - // the lock to "selling" by editing the selling price (which - // recomputes cost from markup, then sets lock="selling"). - // selling 50 → 100, cost recomputes from 25 → 50, lock="selling". - fireEvent.change(screen.getByDisplayValue("50"), { - target: { value: "100" }, - }); - // After: cost=50, selling=100, markup=100, lock="selling". - - // Now edit markup from 100 → 200. With lock="selling", cost - // recomputes: cost = round2(100 / (1 + 200/100)) = round2(100/3) = 33.33. - // After the previous selling edit, both the markup and selling - // inputs display "100" — pick the markup input (index 2 of the - // per-row input array: qty/cost/markup/selling) by scoping to the - // list item for "HSS drill 6mm". - const itemRow = screen.getByText("HSS drill 6mm").closest("li")!; - const rowInputs = itemRow.querySelectorAll("input"); - // rowInputs: [qty, cost, markup, selling] - fireEvent.change(rowInputs[2], { target: { value: "200" } }); - - // Cost should now show 33.33. - expect(screen.getByDisplayValue("33.33")).toBeInTheDocument(); - }); - it("renders PDF preview via iframe when first attachment is a PDF", () => { render( { }); describe("edit-mode confirmation", () => { - // All 4 per-row inputs (qty, cost, markup, selling) should be - // disabled until the operator clicks "Edit items" → confirms. - // This is the safety gate — items are read-only by default. + // The 2 per-row inputs (qty, selling) should be disabled until + // the operator clicks "Edit items" → confirms. This is the + // safety gate — items are read-only by default. Cost and + // markup are no longer editable in this modal — they're set + // later in the order-processing flow. it("items are read-only by default", () => { render(); const itemRow = screen.getByText("HSS drill 6mm").closest("li")!; const rowInputs = itemRow.querySelectorAll("input"); - expect(rowInputs).toHaveLength(4); + expect(rowInputs).toHaveLength(2); for (const input of Array.from(rowInputs)) { expect(input).toBeDisabled(); } @@ -498,13 +474,15 @@ describe("OrdersReviewModal", () => { }); describe("header totals + item count", () => { - it("header shows item count and totals", () => { + it("header shows item count and selling total (no cost)", () => { // Three items with known prices so the totals are // deterministic. - // item-1: qty=2, cost=50, selling=100 → cost=100, selling=200 - // item-2: qty=1, cost=20, selling=30 → cost=20, selling=30 - // item-3: qty=3, cost=10, selling=60 → cost=30, selling=180 - // Total cost = 150, total selling = 410. + // item-1: qty=2, selling=100 → selling=200 + // item-2: qty=1, selling=30 → selling=30 + // item-3: qty=3, selling=60 → selling=180 + // Total selling = 410. Cost intentionally not asserted — + // the review modal no longer surfaces a cost total (cost / + // markup are set later in the order-processing flow). const items: OrderItem[] = [ { ...baseItems[0], id: "item-1", quantity: 2, cost_price: 50, selling_price: 100 }, { ...baseItems[0], id: "item-2", quantity: 1, cost_price: 20, selling_price: 30 }, @@ -514,13 +492,14 @@ describe("OrdersReviewModal", () => { const totals = screen.getByTestId("review-items-totals"); // "1 item" / "N items" — first validate the count verb. expect(totals.textContent).toMatch(/3 items/); - // The cost and selling totals are rendered via formatCurrency - // ("R 150,00" / "R 410,00"). Intl.NumberFormat en-ZA uses a - // non-breaking space between the currency symbol and the - // number, so search for just the digit portion to avoid - // unicode-sensitivity in the test. - expect(totals.textContent).toMatch(/150/); + // The selling total is rendered via formatCurrency + // ("R 410,00"). Intl.NumberFormat en-ZA uses a non-breaking + // space between the currency symbol and the number, so search + // for just the digit portion to avoid unicode-sensitivity. expect(totals.textContent).toMatch(/410/); + // The header should not surface a Cost total anymore. + expect(totals.textContent).not.toMatch(/Cost\b/); + expect(totals.textContent).not.toMatch(/150/); }); it("header shows '1 item' (singular) when only one row exists", () => { @@ -565,6 +544,30 @@ describe("OrdersReviewModal", () => { expect(descInput?.value).toBe(""); expect(newRow.textContent).toMatch(/New/); }); + + it("Add item button creates a new draft row with only qty + selling inputs (no cost/markup fields rendered)", () => { + // Spec: cost and markup are NOT user-editable at the review + // step. The new row should only expose Qty and Selling inputs + // (description is the third input on the row). Verify by + // counting inputs and by ensuring the Cost / Markup % labels + // are absent from the new row. + render(); + fireEvent.click(screen.getByTestId("review-edit-items-btn")); + fireEvent.click(screen.getByTestId("review-edit-confirm-enable")); + fireEvent.click(screen.getByTestId("review-add-item-btn")); + + const listAfter = screen.getByTestId("review-items-list"); + const newRow = listAfter.querySelector('[data-new="true"]')!; + expect(newRow).toBeInTheDocument(); + // Inputs on the new row: description, qty, selling — 3 total + // (not 4 like before, when cost/markup had their own inputs). + const rowInputs = newRow.querySelectorAll("input"); + expect(rowInputs).toHaveLength(3); + // The Cost and Markup labels should NOT exist inside the new + // row (case-insensitive text search). + expect(newRow.textContent).not.toMatch(/Cost/); + expect(newRow.textContent).not.toMatch(/Markup/); + }); }); describe("remove item (soft-flag)", () => { diff --git a/src/components/orders-review-modal.tsx b/src/components/orders-review-modal.tsx index 9d9ba35..ffa98c3 100644 --- a/src/components/orders-review-modal.tsx +++ b/src/components/orders-review-modal.tsx @@ -213,11 +213,12 @@ export function OrdersReviewModal({ if (d.quantity !== it.quantity) return true; if (d.selling_price !== it.selling_price) return true; if (d.cost_price !== it.cost_price) return true; - // NOTE: markup is deliberately not compared here. The save body only - // carries quantity/selling_price/cost_price; the service layer - // recomputes markup server-side from the selling/cost pair. A - // markup-only edit is impossible anyway — onMarkupChange always - // rewrites the lock partner, which *is* part of the dirty body. + // NOTE: markup is deliberately not compared here. The save body + // only carries quantity/selling_price/cost_price; the service + // layer recomputes markup server-side from the selling/cost pair. + // At the review step the operator doesn't edit markup directly, + // but onSellingChange recomputes cost from selling+markup, so a + // cost change here IS captured above. } // Any new draft row (one with no matching persistedItem) is dirty // by default — it must be POSTed to the create endpoint. @@ -237,26 +238,30 @@ export function OrdersReviewModal({ // still counted (they're still on the order until the save lands); // if the operator changes their mind before saving and un-flags, // the totals stay consistent. + // + // Cost is intentionally NOT surfaced as a header total — the + // review modal no longer edits cost or markup, so showing a cost + // total here would be misleading (the cost values are placeholders + // until the order moves through processing). The selling total is + // the operator's "our total" against which the quote email is + // verified. const headerTotals = useMemo(() => { - let cost = 0; let selling = 0; let count = 0; for (const it of persistedItems) { const d = itemDrafts[it.id]; if (!d) continue; const qty = d.quantity ?? 0; - cost += qty * (d.cost_price ?? 0); selling += qty * (d.selling_price ?? 0); count += 1; } - for (const [id, d] of Object.entries(itemDrafts)) { + for (const [, d] of Object.entries(itemDrafts)) { if (!d.isNew) continue; const qty = d.quantity ?? 0; - cost += qty * (d.cost_price ?? 0); selling += qty * (d.selling_price ?? 0); count += 1; } - return { cost, selling, count }; + return { selling, count }; // eslint-disable-next-line react-hooks/exhaustive-deps }, [itemDrafts, persistedItems.map((it) => it.id).join("|")]); @@ -301,18 +306,11 @@ export function OrdersReviewModal({ }); }, []); - const onCostChange = (id: string, raw: string) => { - const value = Number(raw) || 0; - const d = itemDrafts[id]; - if (!d) return; - const newSelling = round2(value * (1 + (d.markup ?? 0) / 100)); - updateItemDraft(id, { - cost_price: value, - selling_price: newSelling, - lock: "cost", - }); - }; - + // Selling is the only mutable price field at the review step — the + // server-side cost/markup recompute happens later, during order + // processing. We still update the cost/markup on the local draft + // (server-side compatibility) so the lock-pattern data model + // remains intact, but the UI doesn't surface those inputs. const onSellingChange = (id: string, raw: string) => { const value = Number(raw) || 0; const d = itemDrafts[id]; @@ -326,20 +324,6 @@ export function OrdersReviewModal({ }); }; - const onMarkupChange = (id: string, raw: string) => { - const value = Number(raw) || 0; - const d = itemDrafts[id]; - if (!d) return; - if (d.lock === "cost") { - const newSelling = round2((d.cost_price ?? 0) * (1 + value / 100)); - updateItemDraft(id, { markup: value, selling_price: newSelling }); - } else { - const newCost = - value > -100 ? round2((d.selling_price ?? 0) / (1 + value / 100)) : 0; - updateItemDraft(id, { markup: value, cost_price: newCost }); - } - }; - const onQuantityChange = (id: string, raw: string) => { const value = raw === "" ? null : Number(raw); updateItemDraft(id, { quantity: value }); @@ -722,17 +706,19 @@ export function OrdersReviewModal({

Order items

- {/* Tier 1 header totals — count + cost + selling. + {/* Tier 1 header totals — count + selling total. Reads from the live draft state so dirty edits update the totals live. Kept on a single subdued - line so the title stays the visual anchor. */} + line so the title stays the visual anchor. Cost + is intentionally omitted because the review modal + no longer surfaces cost/markup as editable + fields. */}

{headerTotals.count} item - {headerTotals.count === 1 ? "" : "s"} · Cost{" "} - {formatCurrency(headerTotals.cost)} · Selling{" "} + {headerTotals.count === 1 ? "" : "s"} · Selling{" "} {formatCurrency(headerTotals.selling)}

@@ -842,7 +828,7 @@ export function OrdersReviewModal({ ) )} -
+
-
- - onCostChange(it.id, e.target.value)} - disabled={!editingItems || saving || d.pendingRemoval} - className={cn( - "h-8 text-xs tabular-nums", - d.lock === "cost" && "border-warn/40 bg-warn/10", - )} - /> -
-
- - - onMarkupChange(it.id, e.target.value) - } - disabled={!editingItems || saving || d.pendingRemoval} - className="h-8 text-xs tabular-nums" - /> -
@@ -963,7 +917,7 @@ export function OrdersReviewModal({ ) )}
-
+
-
- - onCostChange(id, e.target.value)} - disabled={!editingItems || saving || removed} - className={cn( - "h-8 text-xs tabular-nums", - d.lock === "cost" && "border-warn/40 bg-warn/10", - )} - /> -
-
- - onMarkupChange(id, e.target.value)} - disabled={!editingItems || saving || removed} - className="h-8 text-xs tabular-nums" - /> -