feat(delivery-notes): lock the cost price, markup stays the only input

A delivery note's cost price is what the business paid. Editing it
while setting a margin rewrites that silently, so a stray keystroke
changes every downstream number on the note without ever looking like
a pricing decision.

The rule lives in quote-pricing.ts as a `costLocked` flag captured at
load time, and repriceLine DROPS a cost patch on a locked line. That
is the guarantee; the read-only cell in the UI is only the
convenience, so no caller can quietly rewrite the cost.

- "Edit note" and "Create quote" grids: cost is read-only with a lock
  icon once the line arrived costed; a blank cost stays fillable and
  still seeds the 25% default markup on first entry.
- readOnly, not disabled, so the number can still be selected and
  checked against a supplier invoice.
- Locked at load, not per keystroke: a cost typed in this session
  stays correctable until save (an operator choice), and locks on the
  next open because the row then carries the cost.
- The DN detail page's edit mode is untouched — it stays the
  deliberate "that cost is simply wrong" correction path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Dave 2026-10-15 15:44:24 +02:00
parent c5e6ce63a2
commit c22e9d1bd6
5 changed files with 489 additions and 30 deletions

View file

@ -0,0 +1,214 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { render, screen, waitFor, within } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { DeliveryNoteEditorDialog } from "./delivery-note-editor-dialog";
import { formatCurrency } from "@/lib/pb-helpers";
// ─── Mocks ──────────────────────────────────────────────────────────
//
// The dialog reads the note through `fetch` (the `/detail` route)
// and writes through `fetch` (PATCH .../pricing). Both are stubbed
// so no request reaches the network: `detailItems` drives what the
// grid renders, and `patchBodies` records what Save would send —
// the assertion that actually matters here is on the payload, not on
// the styling.
//
// PocketBase is stubbed because the dialog pulls the session token
// straight off `authStore` and refuses to load without one.
vi.mock("@/lib/pocketbase", () => ({
__esModule: true,
default: () => ({ authStore: { token: "test-token" } }),
}));
const NOTE_ID = "dn_1_2_3";
const NOTE_NUMBER = "DN26071501";
let detailItems: Array<Record<string, unknown>> = [];
let patchBodies: Array<Record<string, unknown>> = [];
function mockFetch() {
return vi.fn(async (url: string, init?: RequestInit) => {
if (String(url).includes("/detail")) {
return {
ok: true,
status: 200,
json: async () => ({ items: detailItems }),
} as unknown as Response;
}
if (init?.method === "PATCH") {
patchBodies.push(JSON.parse(String(init.body)));
return {
ok: true,
status: 200,
json: async () => ({ item: {}, pdfRegenerated: false }),
} as unknown as Response;
}
return {
ok: false,
status: 404,
json: async () => ({ error: "not found" }),
} as unknown as Response;
});
}
function renderDialog() {
return render(
<DeliveryNoteEditorDialog
open
onOpenChange={() => {}}
deliveryNoteId={NOTE_ID}
deliveryNoteNumber={NOTE_NUMBER}
/>,
);
}
const costBox = (i: number) =>
screen.getByTestId(`dn-editor-cost-${i}`) as HTMLInputElement;
const markupBox = (i: number) =>
screen.getByLabelText(`Markup for line ${i + 1}`) as HTMLInputElement;
/**
* Assert a row shows `amount` as its selling price.
*
* Two traps in one helper. `formatCurrency` renders en-ZA as
* "R 140,00" — non-breaking space, comma decimal — so the expected
* string must come from the app's own formatter, never a literal.
* And testing-library normalises the NODE's text but compares the
* matcher raw (see `@testing-library/dom/dist/matches.js`: the
* `String(matcher)` branch skips the normaliser), so the nbsp in our
* matcher has to be collapsed here too or the two never compare equal.
*/
function expectSelling(rowIndex: number, amount: number) {
const row = screen.getByTestId(`dn-editor-line-${rowIndex}`);
const expected = formatCurrency(amount).replace(/\s+/g, " ").trim();
expect(within(row).getByText(expected)).toBeInTheDocument();
}
/** Radix only mounts the dialog body once open, so wait for a row. */
async function waitForGrid() {
await waitFor(() => expect(screen.queryByText("Loading items…")).toBeNull());
}
beforeEach(() => {
detailItems = [];
patchBodies = [];
vi.stubGlobal("fetch", mockFetch());
vi.clearAllMocks();
});
describe("cost price already on file is fixed", () => {
it("renders the cost read-only when the line arrived costed", async () => {
detailItems = [
{ id: "oi1", description: "M16 HEX BOLT", quantity: 4, cost_price: 12.5 },
];
renderDialog();
await waitForGrid();
expect(costBox(0)).toHaveAttribute("readonly");
expect(costBox(0)).toHaveValue(12.5);
expect(markupBox(0)).not.toHaveAttribute("readonly");
});
it("leaves the cost writable when there is no cost on file", async () => {
detailItems = [{ id: "oi1", description: "M16 HEX BOLT", quantity: 4 }];
renderDialog();
await waitForGrid();
expect(costBox(0)).not.toHaveAttribute("readonly");
});
it("lets the operator change the markup on a locked line", async () => {
const user = userEvent.setup();
detailItems = [
{ id: "oi1", description: "M16 HEX BOLT", quantity: 4, cost_price: 100 },
];
renderDialog();
await waitForGrid();
// 25% is seeded because the line is costed but unpriced.
expect(markupBox(0)).toHaveValue(25);
await user.clear(markupBox(0));
await user.type(markupBox(0), "40");
// Selling re-derives off the FIXED cost: 100 x 1.40.
await waitFor(() => expectSelling(0, 140));
});
it("saves the original cost even after the markup moved", async () => {
const user = userEvent.setup();
detailItems = [
{ id: "oi1", description: "M16 HEX BOLT", quantity: 4, cost_price: 100 },
];
renderDialog();
await waitForGrid();
await user.clear(markupBox(0));
await user.type(markupBox(0), "40");
await user.click(screen.getByTestId("dn-editor-save"));
await waitFor(() => expect(patchBodies).toHaveLength(1));
expect(patchBodies[0]).toMatchObject({
cost_price: 100,
selling_price: 140,
});
});
it("does not let a typed value into a locked cost box", async () => {
const user = userEvent.setup();
detailItems = [
{ id: "oi1", description: "M16 HEX BOLT", quantity: 4, cost_price: 100 },
];
renderDialog();
await waitForGrid();
await user.click(costBox(0));
await user.type(costBox(0), "1");
expect(costBox(0)).toHaveValue(100);
});
it("still accepts a first cost on an uncosted line", async () => {
const user = userEvent.setup();
detailItems = [{ id: "oi1", description: "M16 HEX BOLT", quantity: 4 }];
renderDialog();
await waitForGrid();
await user.type(costBox(0), "80");
// Entering the first cost seeds the default markup (25%), so the
// line is no longer a silent zero-margin line.
await waitFor(() => expect(markupBox(0)).toHaveValue(25));
expectSelling(0, 100);
await user.click(screen.getByTestId("dn-editor-save"));
await waitFor(() => expect(patchBodies).toHaveLength(1));
expect(patchBodies[0]).toMatchObject({
cost_price: 80,
selling_price: 100,
});
});
it("explains the lock instead of showing a dead input", async () => {
detailItems = [
{ id: "oi1", description: "M16 HEX BOLT", quantity: 4, cost_price: 100 },
];
renderDialog();
await waitForGrid();
expect(
screen.getByText(/cost price is fixed/i),
).toBeInTheDocument();
});
it("says so when every cost is still open", async () => {
detailItems = [{ id: "oi1", description: "M16 HEX BOLT", quantity: 4 }];
renderDialog();
await waitForGrid();
expect(
screen.getByText(/no cost is on file yet/i),
).toBeInTheDocument();
});
});

View file

@ -25,10 +25,22 @@
* A line that already carries a selling price keeps it — the office may
* have priced it deliberately — and shows the markup that price implies
* so the two can never quietly disagree.
*
* ## A cost already on file is fixed
*
* The cost cell is read-only on any line that arrived with a cost.
* The operator's job on this screen is to set the MARGIN, and letting
* the cost move while they do that means a stray keystroke silently
* rewrites what the business paid — which changes every downstream
* number on the note without ever looking like a pricing decision.
* A line with no cost on file stays an empty input: that one gets costed
* here. See `costLocked` / `isCostEditable` in `quote-pricing.ts`, and
* the delivery note detail page's edit mode for the deliberate
* "that cost is simply wrong" correction.
*/
import { useCallback, useEffect, useMemo, useState } from "react";
import { toast } from "sonner";
import { AlertTriangle, Loader2 } from "lucide-react";
import { AlertTriangle, Loader2, Lock } from "lucide-react";
import { Button } from "@/components/ui/button";
import { Input } from "@/components/ui/input";
import {
@ -42,6 +54,7 @@ import {
import {
applyMarkupFloor,
hasCostPrice,
isCostEditable,
repriceWithCostEntry,
totalsForLines,
type LinePricing,
@ -127,6 +140,9 @@ export function DeliveryNoteEditorDialog({
? 25
: 0,
sellingPrice: selling,
// Read at load, not derived from `costPrice`: a cost typed
// in during this session must stay fixable until save.
costLocked: hasCostPrice(cost),
};
}),
);
@ -150,6 +166,12 @@ export function DeliveryNoteEditorDialog({
const totals = useMemo(() => totalsForLines(lines), [lines]);
/** How many lines arrived with a cost already on file. */
const lockedCount = useMemo(
() => lines.filter((l) => !isCostEditable(l)).length,
[lines],
);
const update = (index: number, patch: { costPrice?: number; markupPercent?: number }) => {
setLines((prev) =>
prev.map((line, i) => (i === index ? repriceWithCostEntry(line, patch) : line)),
@ -276,6 +298,7 @@ export function DeliveryNoteEditorDialog({
<tbody>
{lines.map((line, index) => {
const costed = hasCostPrice(line.costPrice);
const costLocked = !isCostEditable(line);
return (
<tr
key={line.orderItemId}
@ -320,19 +343,48 @@ export function DeliveryNoteEditorDialog({
/>
</td>
<td className="px-3 py-2">
<Input
type="number"
min={0}
step="0.01"
value={line.costPrice === 0 ? "" : line.costPrice}
placeholder="0.00"
onChange={(e) =>
update(index, { costPrice: Number(e.target.value) || 0 })
}
aria-label={`Cost for line ${index + 1}`}
disabled={saving}
className="h-8 text-right tabular-nums"
/>
<div className="flex items-center justify-end gap-1.5">
{costLocked && (
<Lock
className="h-3 w-3 shrink-0 text-muted-foreground"
aria-hidden="true"
/>
)}
<Input
type="number"
min={0}
step="0.01"
value={line.costPrice === 0 ? "" : line.costPrice}
placeholder="0.00"
onChange={(e) =>
update(index, {
costPrice: Number(e.target.value) || 0,
})
}
aria-label={`Cost for line ${index + 1}`}
// `readOnly`, not `disabled`: a disabled
// box drops out of the tab order and can
// no longer be selected or copied, and
// this number is still worth reading
// off the screen and checking against
// the supplier invoice.
readOnly={costLocked}
aria-readonly={costLocked}
title={
costLocked
? "Cost price is fixed — change the markup to change the selling price"
: undefined
}
data-testid={`dn-editor-cost-${index}`}
disabled={saving}
className={
"h-8 text-right tabular-nums " +
(costLocked
? "cursor-not-allowed bg-muted/60 text-muted-foreground"
: "")
}
/>
</div>
</td>
<td className="px-3 py-2">
<Input
@ -372,6 +424,27 @@ export function DeliveryNoteEditorDialog({
</table>
</div>
{/* One sentence beats a tooltip on every locked cell:
the rule is the whole point of this screen, and a
greyed-out number with no explanation reads as a
broken input. */}
<p className="flex items-start gap-2 text-xs text-muted-foreground">
<Lock className="mt-0.5 h-3.5 w-3.5 shrink-0" aria-hidden="true" />
{lockedCount > 0 ? (
<span>
{lockedCount} cost {lockedCount === 1 ? "price is" : "prices are"}{" "}
fixed — change the markup to change the selling price. A line
with no cost on file can still be costed here.
</span>
) : (
<span>
No cost is on file yet, so every cost box above is open. Once a
cost is saved it stays put and the markup becomes the only
thing to adjust.
</span>
)}
</p>
<div className="flex flex-wrap items-center justify-end gap-x-6 gap-y-1 rounded-md border bg-muted/30 px-4 py-3 text-sm">
<span className="text-muted-foreground">
Total cost{" "}

View file

@ -22,11 +22,15 @@
* - It doesn't invent a cost. A line with no cost on file shows a
* blank cost and a "no cost on file" marker rather than a
* plausible-looking zero that reads as free-of-charge.
* - It doesn't let you edit a cost that's already on file. Same
* rule as the "Edit note" grid one button away: reviewing a
* margin must never silently rewrite what was paid. A blank cost
* stays fillable. See `costLocked` in `quote-pricing.ts`.
*/
import { useEffect, useMemo, useState } from "react";
import { useRouter } from "next/navigation";
import { toast } from "sonner";
import { AlertTriangle, Loader2 } from "lucide-react";
import { AlertTriangle, Loader2, Lock } from "lucide-react";
import { Button } from "@/components/ui/button";
import { Input } from "@/components/ui/input";
import { Label } from "@/components/ui/label";
@ -42,6 +46,7 @@ import {
applyMarkupFloor,
buildLinePricing,
hasCostPrice,
isCostEditable,
repriceLine,
repriceWithCostEntry,
totalsForLines,
@ -157,6 +162,12 @@ export function QuoteReviewDialog({
[lines],
);
/** How many lines arrived with a cost already on file. */
const lockedCount = useMemo(
() => lines.filter((l) => !isCostEditable(l)).length,
[lines],
);
/** Raise any markup left under the floor, on blur and on save. */
const commitFloors = (index: number) => {
setLines((prev) =>
@ -352,6 +363,7 @@ export function QuoteReviewDialog({
<tbody>
{lines.map((line, index) => {
const costed = hasCostPrice(line.costPrice);
const costLocked = !isCostEditable(line);
return (
<tr
key={line.orderItemId}
@ -373,20 +385,44 @@ export function QuoteReviewDialog({
{line.quantity}
</td>
<td className="px-3 py-2">
<Input
type="number"
min="0"
step="0.01"
value={line.costPrice === 0 ? "" : line.costPrice}
placeholder="0.00"
aria-label={`Cost for line ${index + 1}`}
onChange={(e) =>
updateLine(index, {
costPrice: Number(e.target.value) || 0,
})
}
className="h-8 text-right tabular-nums"
/>
<div className="flex items-center justify-end gap-1.5">
{costLocked && (
<Lock
className="h-3 w-3 shrink-0 text-muted-foreground"
aria-hidden="true"
/>
)}
<Input
type="number"
min="0"
step="0.01"
value={line.costPrice === 0 ? "" : line.costPrice}
placeholder="0.00"
aria-label={`Cost for line ${index + 1}`}
// Read-only, not disabled — the number
// stays selectable so it can be checked
// against the supplier invoice.
readOnly={costLocked}
aria-readonly={costLocked}
title={
costLocked
? "Cost price is fixed — change the markup to change the selling price"
: undefined
}
data-testid={`dn-quote-cost-${index}`}
onChange={(e) =>
updateLine(index, {
costPrice: Number(e.target.value) || 0,
})
}
className={
"h-8 text-right tabular-nums " +
(costLocked
? "cursor-not-allowed bg-muted/60 text-muted-foreground"
: "")
}
/>
</div>
</td>
<td className="px-3 py-2">
<Input
@ -426,6 +462,25 @@ export function QuoteReviewDialog({
</table>
</div>
{/* Same one-line explanation the "Edit note" grid carries. A locked
cost with no explanation reads as a broken input, not
as a rule. */}
<p className="flex items-start gap-2 text-xs text-muted-foreground">
<Lock className="mt-0.5 h-3.5 w-3.5 shrink-0" aria-hidden="true" />
{lockedCount > 0 ? (
<span>
{lockedCount} cost {lockedCount === 1 ? "price is" : "prices are"}{" "}
fixed — set the margin you want with the markup. A line with no
cost on file can still be costed here.
</span>
) : (
<span>
No cost is on file yet, so every cost box above is open. Enter a
cost and it prices itself at the default markup.
</span>
)}
</p>
{/* Totals. Margin is shown as a share of REVENUE, not of
cost — 25% markup is a 20% margin, and conflating them
is the classic way to quietly under-price a job. */}

View file

@ -4,6 +4,7 @@ import {
buildLinePricing,
DEFAULT_MARKUP_PERCENT,
hasCostPrice,
isCostEditable,
MIN_MARKUP_PERCENT,
markupFromCostAndSelling,
repriceLine,
@ -219,6 +220,10 @@ describe("selling price cannot collapse to zero on a cheap line", () => {
});
describe("repriceLine honours the floor", () => {
// `costLocked: false` = a costed line whose cost is still editable,
// i.e. a cost entered during the current session. These tests are
// about the floor, so the lock is deliberately left off here — the
// locked-line behaviour has its own describe block below.
const base = {
orderItemId: "i1",
description: "M10 FLAT WASHER BLK",
@ -226,6 +231,7 @@ describe("repriceLine honours the floor", () => {
costPrice: 0.07,
markupPercent: 30,
sellingPrice: 0.1,
costLocked: false,
};
it("does NOT floor while the operator is typing", () => {
@ -328,6 +334,7 @@ describe("repriceLine", () => {
costPrice: 100,
markupPercent: 25,
sellingPrice: 125,
costLocked: false,
};
it("re-derives selling when the markup changes", () => {
@ -415,6 +422,7 @@ describe("repriceWithCostEntry", () => {
costPrice: 0,
markupPercent: 0,
sellingPrice: 0,
costLocked: false,
};
it("seeds the default markup when the FIRST cost is typed", () => {
@ -473,3 +481,73 @@ describe("repriceWithCostEntry", () => {
});
});
});
/**
* A cost price that is already on file is fixed. The grids render it
* read-only, but the real guarantee is in `repriceLine`: a cost
* patch on a locked line is dropped, so no caller — a stray edit
* handler, a future bulk action, a keyboard shortcut — can quietly
* rewrite what was paid while the operator believes they only moved
* the markup.
*/
describe("a cost price already on file is fixed", () => {
const locked = buildLinePricing([
{ id: "i1", description: "Bolt", quantity: 2, cost_price: 100 },
])[0];
it("marks a costed line locked at load", () => {
expect(locked.costLocked).toBe(true);
expect(isCostEditable(locked)).toBe(false);
});
it("leaves an uncosted line open", () => {
const [open] = buildLinePricing([{ id: "i2", cost_price: 0 }]);
expect(open.costLocked).toBe(false);
expect(isCostEditable(open)).toBe(true);
});
it("still lets the markup move — that's the point of the screen", () => {
const r = repriceWithCostEntry(locked, { markupPercent: 40 });
expect(r.costPrice).toBe(100);
expect(r.markupPercent).toBe(40);
expect(r.sellingPrice).toBe(140);
});
it("drops a cost patch instead of applying it", () => {
const r = repriceWithCostEntry(locked, { costPrice: 1 });
expect(r.costPrice).toBe(100);
// And therefore the selling price still reflects the real cost —
// a rejected cost must not leave the margin quietly derived from
// the rejected number.
expect(r.sellingPrice).toBe(125);
});
it("drops a cost patch even when it arrives with a markup", () => {
const r = repriceWithCostEntry(locked, { costPrice: 999, markupPercent: 50 });
expect(r.costPrice).toBe(100);
expect(r.markupPercent).toBe(50);
expect(r.sellingPrice).toBe(150);
});
it("cannot be unlocked by re-running the cost-entry path", () => {
const r = repriceWithCostEntry(locked, { costPrice: 0 });
expect(r.costLocked).toBe(true);
expect(r.costPrice).toBe(100);
});
it("leaves the lock standing across a bulk re-markup", () => {
const bulk = locked;
const r = repriceWithCostEntry(bulk, { markupPercent: MIN_MARKUP_PERCENT }, 25);
expect(r.costLocked).toBe(true);
expect(r.sellingPrice).toBe(118);
});
it("keeps the lock across the commit-time floor pass", () => {
// `applyMarkupFloor` + a re-price is what save actually runs. If
// that path could drop the flag, the second open of the dialog
// would quietly unlock a cost.
const committed = repriceLine(locked, { markupPercent: 0 }, { floor: true });
expect(committed.costLocked).toBe(true);
expect(committed.markupPercent).toBe(MIN_MARKUP_PERCENT);
});
});

View file

@ -149,6 +149,19 @@ export function hasCostPrice(cost: number | null | undefined): boolean {
return Number.isFinite(n) && n > 0;
}
/**
* True when the operator may type into this line's cost cell.
*
* The single source of truth for the "cost on file is fixed" rule, so
* the pricing grid and the review grid can't disagree about when a
* cost is locked. Paired with `repriceLine`, which enforces the same
* rule in the state — the UI hiding an input is a convenience, this
* is the guarantee.
*/
export function isCostEditable(line: Pick<LinePricing, "costLocked">): boolean {
return line.costLocked !== true;
}
/**
* Default markup to seed a line with when the operator hasn't chosen
* one. Deliberately a constant rather than a per-customer or
@ -167,6 +180,20 @@ export interface LinePricing {
markupPercent: number;
/** Derived from cost + markup; never typed independently. */
sellingPrice: number;
/**
* True when the line ARRIVED carrying a cost price, so the cost
* cell is read-only and `repriceLine` refuses cost patches.
*
* Captured at load rather than derived from the live value, and
* that is the whole point: a cost the operator types in during
* this session must stay correctable until they save. Locking on
* the live value would mean one mis-typed digit has to be escaped
* by cancelling the dialog and starting over. After a save the row
* carries the cost, so the next load sets this true and the cell is
* fixed — which is the rule as stated: cost on file is fixed, a
* blank cost is fillable.
*/
costLocked: boolean;
}
export interface LineTotals {
@ -235,6 +262,7 @@ export function buildLinePricing(
costPrice,
markupPercent: markupFromCostAndSelling(costPrice, existingSelling),
sellingPrice: existingSelling,
costLocked: hasCost,
};
}
@ -248,6 +276,7 @@ export function buildLinePricing(
sellingPrice: hasCost
? sellingFromCostAndMarkup(costPrice, markupPercent)
: 0,
costLocked: hasCost,
};
});
}
@ -256,6 +285,14 @@ export function buildLinePricing(
* Re-derive one line's selling price after its cost or markup changed.
* Kept as a pure function so the modal's edit handler is a single
* call and the derivation rule lives in exactly one place.
*
* A `costPrice` patch on a LOCKED line is silently dropped rather
* than applied. The grids disable the cost cell, but a disabled
* input is a UI convention, not an invariant — a stray patch (a
* future bulk action, a keyboard shortcut, a half-merged rebase)
* would otherwise be able to rewrite what the business paid while
* the operator believes they only touched the markup. Dropping it
* here means the cost can only ever change on a line that had none.
*/
export function repriceLine(
line: LinePricing,
@ -263,7 +300,9 @@ export function repriceLine(
opts: { floor?: boolean } = {},
): LinePricing {
const costPrice =
patch.costPrice !== undefined ? round2(Number(patch.costPrice) || 0) : line.costPrice;
patch.costPrice !== undefined && isCostEditable(line)
? round2(Number(patch.costPrice) || 0)
: line.costPrice;
const requestedMarkup =
patch.markupPercent !== undefined
? round2(Number(patch.markupPercent) || 0)