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

fix(pricing): the 18% floor was applied on every keystroke

Browse source 
Operator report: "If I double click on it and I put in 20 there, goes
to 18... if I take everything out, I start typing, automatically change
it to 18. I need to be able to add any value there and when I click
save and the value is less than 18, it should go to 18."

The floor (`2387a71`) was correct in principle and wrong in WHERE it
sat. It was applied inside `repriceLine`, which runs on every
keystroke in both pricing grids — so the moment a sub-floor digit was
typed the state was raised to 18 and the box snapped.

Typing "20" is the clearest demonstration: the "2" is sub-floor, so it
became 18 mid-keystroke; the next character then APPENDED to "18" and
produced 180. What the operator saw as "goes to 18" was the first
keystroke, and "it jumps around" was the second.

She asked for the floor on SAVE, not while typing. So:

  - `repriceLine` no longer floors. Markup is also rounded to 2dp on
    the way in, so the box holds `80.06` rather than the float noise
    a back-solved markup produces (`80.05999755859375`).
  - The floor applies on BLUR and on SAVE, in both the delivery-note
    editor and the quote review modal, via `applyMarkupFloor` — the
    same function, still the only thing between a typed value and the
    database, so the -100-row hole stays closed.
  - `step="0.5"` replaced with `step="any"`. A back-solved 80.06 is
    not a multiple of 0.5, which left those boxes `:invalid` — and a
    number input in an invalid state fights the caret. This is very
    likely a second contributor to "I can't do anything there".
  - The bulk "set all markups" field gets the same treatment.

Verified with real keystrokes in the browser, not synthetic events
(which had masked this — they set a value in one shot, so the
mid-keystroke floor never fired):

  type "20"  -> stays 20, selling R605,40 (504,50 x 1,20)
  type "15"  -> stays 15 while typing
  blur       -> 18, selling R595,31 (504,50 x 1,18)

Three existing tests asserted the per-keystroke floor. They are
repointed, not deleted: the commit path is now pinned directly, and
`applyMarkupFloor` has its own tests so the database protection is
still asserted.

No data changed — the editor work above was dialog-local state only.

Full suite 3833 passing, tsc clean.
This commit is contained in:
Dave 2026-10-09 12:49:13 +02:00
parent bc6c4fdb31
commit 6cfb413508
4 changed files with 123 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
src/components/delivery-notes/delivery-note-editor-dialog.tsx
View file

@ -40,6 +40,7 @@ import {
DialogTitle,
} from "@/components/ui/dialog";
import {
applyMarkupFloor,
hasCostPrice,
repriceWithCostEntry,
totalsForLines,
@ -155,12 +156,34 @@ export function DeliveryNoteEditorDialog({
);
};
/**
* Raise any markup the operator left under the floor. Runs on BLUR
* and again on SAVE — never per keystroke, which is what made the
* box snap to 18 while she was still typing it.
*/
const commitFloors = (index: number) => {
setLines((prev) =>
prev.map((line, i) => {
if (i !== index) return line;
const floored = applyMarkupFloor(line.markupPercent, line.costPrice);
return floored === line.markupPercent
? line
: repriceWithCostEntry(line, { markupPercent: floored });
}),
);
};
const save = async (showDocumentAfter = false) => {
setSaving(true);
try {
// The floor is a COMMIT-time rule, not an editing-time one.
const committed = lines.map((l) => ({
...l,
markupPercent: applyMarkupFloor(l.markupPercent, l.costPrice),
}));
// Sequential rather than parallel: these rewrite the same DN
// PDF, and concurrent writes to one record race.
for (const line of lines) {
for (const line of committed) {
const res = await fetch(
`/api/v1/delivery-notes/items/${encodeURIComponent(line.orderItemId)}/pricing`,
{
@ -315,7 +338,6 @@ export function DeliveryNoteEditorDialog({
<Input
type="number"
min={0}
step="0.5"
value={line.markupPercent === 0 ? "" : line.markupPercent}
placeholder="0"
onChange={(e) =>
@ -323,8 +345,15 @@ export function DeliveryNoteEditorDialog({
markupPercent: Number(e.target.value) || 0,
})
}
// Commit the floor when she leaves the field.
onBlur={() => commitFloors(index)}
aria-label={`Markup for line ${index + 1}`}
disabled={saving}
// "any", not 0.5. A step mismatch left these
// boxes :invalid — a back-solved 80.06 is not
// a multiple of 0.5 — and a number input in
// an invalid state fights the caret.
step="any"
className="h-8 text-right tabular-nums"
/>
</td>

33
src/components/delivery-notes/quote-review-dialog.tsx
View file

@ -39,6 +39,7 @@ import {
DialogTitle,
} from "@/components/ui/dialog";
import {
applyMarkupFloor,
buildLinePricing,
hasCostPrice,
repriceLine,
@ -156,6 +157,19 @@ export function QuoteReviewDialog({
[lines],
);
/** Raise any markup left under the floor, on blur and on save. */
const commitFloors = (index: number) => {
setLines((prev) =>
prev.map((line, i) => {
if (i !== index) return line;
const floored = applyMarkupFloor(line.markupPercent, line.costPrice);
return floored === line.markupPercent
? line
: repriceWithCostEntry(line, { markupPercent: floored });
}),
);
};
const updateLine = (
index: number,
patch: { costPrice?: number; markupPercent?: number },
@ -178,6 +192,12 @@ export function QuoteReviewDialog({
};
const handleSubmit = async () => {
// The floor is a COMMIT-time rule, not an editing-time one.
const committed: LinePricing[] = lines.map((l) => ({
...l,
markupPercent: applyMarkupFloor(l.markupPercent, l.costPrice),
}));
if (unpriced.length > 0) {
const proceed = window.confirm(
`${unpriced.length} line${unpriced.length === 1 ? " has" : "s have"} no cost on file, so the selling price is R0.00.\n\n` +
@ -196,7 +216,7 @@ export function QuoteReviewDialog({
headers: { "Content-Type": "application/json", Authorization: token },
body: JSON.stringify({
letterhead,
items: lines.map((l) => ({
items: committed.map((l) => ({
orderItemId: l.orderItemId,
costPrice: l.costPrice,
markupPercent: l.markupPercent,
@ -294,7 +314,7 @@ export function QuoteReviewDialog({
id="dn-quote-bulk-markup"
type="number"
min="0"
step="0.5"
step="any"
placeholder="25"
onKeyDown={(e) => {
if (e.key !== "Enter") return;
@ -372,7 +392,6 @@ export function QuoteReviewDialog({
<Input
type="number"
min="0"
step="0.5"
value={line.markupPercent === 0 ? "" : line.markupPercent}
placeholder="0"
aria-label={`Markup for line ${index + 1}`}
@ -381,6 +400,14 @@ export function QuoteReviewDialog({
markupPercent: Number(e.target.value) || 0,
})
}
// Floor on leaving the field, not per
// keystroke — see `repriceLine`. And
// step="any": a back-solved markup like
// 80.06 is not a multiple of 0.5, which
// left the box :invalid and fighting the
// caret.
onBlur={() => commitFloors(index)}
step="any"
className="h-8 text-right tabular-nums"
/>
</td>

62
src/lib/quote-pricing.test.ts
View file

@ -228,17 +228,39 @@ describe("repriceLine honours the floor", () => {
sellingPrice: 0.1,
};
it("floors a sub-floor markup instead of writing it through", () => {
it("does NOT floor while the operator is typing", () => {
// This function runs on every keystroke in the pricing grids, so
// flooring here made the input fight the person typing in it:
// entering "20", the "2" was raised to 18 mid-keystroke and the
// box snapped. The operator's report was "it just reverts back
// to the old numbers" and "I start typing, automatically change
// it to 18". The floor belongs at COMMIT — see the tests below.
const r = repriceLine(base, { markupPercent: 0 });
expect(r.markupPercent).toBe(MIN_MARKUP_PERCENT);
expect(r.sellingPrice).toBeGreaterThan(0);
expect(r.markupPercent).toBe(0);
const neg = repriceLine(base, { markupPercent: -100 });
expect(neg.markupPercent).toBe(-100);
});
it("floors the bulk 'apply markup to all lines' action", () => {
// This is the path the delivery-note review modal's blur handler
// takes. It used to write whatever it was handed.
const r = repriceLine(base, { markupPercent: -100 });
it("floors when the caller asks for it (the commit path)", () => {
// blur / save pass `floor: true`. This is what keeps a
// sub-floor value out of the database, which is the -100-row
// hole the floor was introduced for.
const r = repriceLine(base, { markupPercent: 0 }, { floor: true });
expect(r.markupPercent).toBe(MIN_MARKUP_PERCENT);
expect(r.sellingPrice).toBeGreaterThan(0);
const bulk = repriceLine(base, { markupPercent: -100 }, { floor: true });
expect(bulk.markupPercent).toBe(MIN_MARKUP_PERCENT);
});
it("applyMarkupFloor is the commit-time rule both grids use", () => {
// Pinned directly, because it is now the ONLY thing standing
// between a typed "5" and a sub-floor row in the database.
expect(applyMarkupFloor(5, 100)).toBe(MIN_MARKUP_PERCENT);
expect(applyMarkupFloor(0, 100)).toBe(MIN_MARKUP_PERCENT);
expect(applyMarkupFloor(-100, 100)).toBe(MIN_MARKUP_PERCENT);
expect(applyMarkupFloor(25, 100)).toBe(25);
// An uncosted draft is not a 0% margin, it is an undecided line.
expect(applyMarkupFloor(0, 0)).toBe(0);
});
it("still allows the deliberate thin margins reps actually use", () => {
@ -419,17 +441,21 @@ describe("repriceWithCostEntry", () => {
expect(r.sellingPrice).toBe(280);
});
it("BEHAVIOUR CHANGE: an explicit 0 markup on a COSTED line is now floored", () => {
// This test used to assert markupPercent === 0 ("explicit intent").
// That is exactly the hole the -100 rows came through: a zero-margin
// line is a line that can be rounded, re-derived or nudged into a
// negative one. The house rule is now "never below MIN_MARKUP_PERCENT
// once a line has a cost", so an explicit 0 is raised like any other
// sub-floor value. Uncosted drafts are untouched — see
// applyMarkupFloor.
const r = repriceWithCostEntry(noCost, { costPrice: 100, markupPercent: 0 });
expect(r.markupPercent).toBe(MIN_MARKUP_PERCENT);
expect(r.sellingPrice).toBe(118);
it("floors an explicit 0 on a COSTED line once committed", () => {
// A zero-margin line is a line that can be rounded, re-derived or
// nudged into a negative one — the hole the -100 rows came
// through. So an explicit 0 is still raised at commit; it is just
// no longer raised WHILE TYPING, which is what broke the input.
// Uncosted drafts stay untouched — see applyMarkupFloor.
const typed = repriceWithCostEntry(noCost, {
costPrice: 100,
markupPercent: 0,
});
expect(typed.markupPercent).toBe(0);
const committed = repriceLine(typed, {}, { floor: true });
expect(committed.markupPercent).toBe(MIN_MARKUP_PERCENT);
expect(committed.sellingPrice).toBe(118);
});
it("seeds nothing when the cost is cleared back to zero", () => {

24
src/lib/quote-pricing.ts
View file

@ -260,18 +260,30 @@ export function buildLinePricing(
export function repriceLine(
line: LinePricing,
patch: { costPrice?: number; markupPercent?: number },
opts: { floor?: boolean } = {},
): LinePricing {
const costPrice =
patch.costPrice !== undefined ? round2(Number(patch.costPrice) || 0) : line.costPrice;
const requestedMarkup =
patch.markupPercent !== undefined
? Number(patch.markupPercent) || 0
? round2(Number(patch.markupPercent) || 0)
: line.markupPercent;
// Floor applies HERE, at reprice, so every caller inherits it. The
// "apply markup to all lines" bulk action and the delivery-note
// review modal both land in this function, and both previously wrote
// whatever markup they were handed straight through.
const markupPercent = applyMarkupFloor(requestedMarkup, costPrice);
// The floor is NOT applied here by default, and that is deliberate.
//
// This function runs on every keystroke in the pricing grids, so
// flooring here made the input fight the person typing in it: enter
// "20", the "2" was raised to 18 mid-keystroke, and the box snapped
// to 18 — the operator's words were "it just reverts back" and
// "automatically change it to 18". Typing a further character then
// appended to "18" and produced 180.
//
// The floor belongs at COMMIT — on blur and on save — which is what
// the operator actually asked for ("add any value there, and when
// I click save and the value is less than 18, it should go to 18").
// Bulk "set all markups" still passes `floor: true`.
const markupPercent = opts.floor
? applyMarkupFloor(requestedMarkup, costPrice)
: requestedMarkup;
return {
...line,
costPrice,