David/WFDASH-WEBAPP
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(orders): hide cost and markup from review modal items table

Browse source 
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
SJCoetz 2026-07-29 19:02:49 +02:00
parent 74c73e9f17
commit d307e8d400
2 changed files with 104 additions and 177 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

145
src/components/orders-review-modal.test.tsx
View file

@ -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<X>" segment was removed in the same change).
render(<OrdersReviewModal {...baseProps()} />);
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(<OrdersReviewModal {...baseProps()} items={items} />);
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(<OrdersReviewModal {...baseProps()} />);
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(<OrdersReviewModal {...baseProps()} />);
// 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(<OrdersReviewModal {...baseProps()} />);
// 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(<OrdersReviewModal {...baseProps()} />);
// 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(
<OrdersReviewModal
@ -428,14 +402,16 @@ describe("OrdersReviewModal", () => {
});
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(<OrdersReviewModal {...baseProps()} />);
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(<OrdersReviewModal {...baseProps()} />);
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)", () => {

136
src/components/orders-review-modal.tsx
View file

@ -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({
<h3 className="text-xs font-semibold uppercase tracking-wide text-muted-foreground">
Order items
</h3>
{/* 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. */}
<p
className="text-xs text-muted-foreground tabular-nums truncate"
data-testid="review-items-totals"
>
{headerTotals.count} item
{headerTotals.count === 1 ? "" : "s"} · Cost{" "}
{formatCurrency(headerTotals.cost)} · Selling{" "}
{headerTotals.count === 1 ? "" : "s"} · Selling{" "}
{formatCurrency(headerTotals.selling)}
</p>
</div>
@ -842,7 +828,7 @@ export function OrdersReviewModal({
)
)}
</div>
<div className="grid grid-cols-4 gap-2">
<div className="grid grid-cols-2 gap-2">
<div className="space-y-1">
<Label className="text-[10px] text-muted-foreground">
Qty
@ -857,35 +843,6 @@ export function OrdersReviewModal({
className="h-8 text-xs tabular-nums"
/>
</div>
<div className="space-y-1">
<Label className="text-[10px] text-muted-foreground">
Cost
</Label>
<Input
type="number"
value={d.cost_price ?? ""}
onChange={(e) => 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",
)}
/>
</div>
<div className="space-y-1">
<Label className="text-[10px] text-muted-foreground">
Markup %
</Label>
<Input
type="number"
value={d.markup ?? ""}
onChange={(e) =>
onMarkupChange(it.id, e.target.value)
}
disabled={!editingItems || saving || d.pendingRemoval}
className="h-8 text-xs tabular-nums"
/>
</div>
<div className="space-y-1">
<Label className="text-[10px] text-muted-foreground">
Selling
@ -897,10 +854,7 @@ export function OrdersReviewModal({
onSellingChange(it.id, e.target.value)
}
disabled={!editingItems || saving || d.pendingRemoval}
className={cn(
"h-8 text-xs tabular-nums",
d.lock === "selling" && "border-warn/40 bg-warn/10",
)}
className="h-8 text-xs tabular-nums"
/>
</div>
</div>
@ -963,7 +917,7 @@ export function OrdersReviewModal({
)
)}
</div>
<div className="grid grid-cols-4 gap-2">
<div className="grid grid-cols-2 gap-2">
<div className="space-y-1">
<Label className="text-[10px] text-muted-foreground">
Qty
@ -976,33 +930,6 @@ export function OrdersReviewModal({
className="h-8 text-xs tabular-nums"
/>
</div>
<div className="space-y-1">
<Label className="text-[10px] text-muted-foreground">
Cost
</Label>
<Input
type="number"
value={d.cost_price ?? ""}
onChange={(e) => 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",
)}
/>
</div>
<div className="space-y-1">
<Label className="text-[10px] text-muted-foreground">
Markup %
</Label>
<Input
type="number"
value={d.markup ?? ""}
onChange={(e) => onMarkupChange(id, e.target.value)}
disabled={!editingItems || saving || removed}
className="h-8 text-xs tabular-nums"
/>
</div>
<div className="space-y-1">
<Label className="text-[10px] text-muted-foreground">
Selling
@ -1012,10 +939,7 @@ export function OrdersReviewModal({
value={d.selling_price ?? ""}
onChange={(e) => onSellingChange(id, e.target.value)}
disabled={!editingItems || saving || removed}
className={cn(
"h-8 text-xs tabular-nums",
d.lock === "selling" && "border-warn/40 bg-warn/10",
)}
className="h-8 text-xs tabular-nums"
/>
</div>
</div>