diff --git a/src/components/delivery-notes/delivery-note-editor-dialog.tsx b/src/components/delivery-notes/delivery-note-editor-dialog.tsx index 177267b..2ed4c14 100644 --- a/src/components/delivery-notes/delivery-note-editor-dialog.tsx +++ b/src/components/delivery-notes/delivery-note-editor-dialog.tsx @@ -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({ @@ -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" /> diff --git a/src/components/delivery-notes/quote-review-dialog.tsx b/src/components/delivery-notes/quote-review-dialog.tsx index e5c0fc4..bd7c15d 100644 --- a/src/components/delivery-notes/quote-review-dialog.tsx +++ b/src/components/delivery-notes/quote-review-dialog.tsx @@ -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({ commitFloors(index)} + step="any" className="h-8 text-right tabular-nums" /> diff --git a/src/lib/quote-pricing.test.ts b/src/lib/quote-pricing.test.ts index a53f3bf..4c15069 100644 --- a/src/lib/quote-pricing.test.ts +++ b/src/lib/quote-pricing.test.ts @@ -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", () => { diff --git a/src/lib/quote-pricing.ts b/src/lib/quote-pricing.ts index ca89fa1..5902679 100644 --- a/src/lib/quote-pricing.ts +++ b/src/lib/quote-pricing.ts @@ -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,