fix(orders): review modal review fixes (drop dead markup check, lock tests, link assertion)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
SJCoetz 2026-07-29 15:07:11 +02:00
parent 0e6c3d8f0c
commit c572d08e53
2 changed files with 65 additions and 1 deletions

View file

@ -187,5 +187,65 @@ describe("OrdersReviewModal", () => {
"/api/v1/orders/order-1/review-save",
expect.objectContaining({ method: "POST" }),
);
expect(
screen.getByRole("link", { name: /open existing/i }),
).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();
});
});

View file

@ -122,7 +122,11 @@ 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;
if ((d.markup ?? null) !== (it.markup ?? null)) 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.
}
return false;
// eslint-disable-next-line react-hooks/exhaustive-deps