Dave
2387a71cfe
Quoteers reported markup changing to a random number after saving or clicking away, and lines carrying markup = -100 that nobody had edited. 51 quote_items rows were affected (40 of them exactly -100), all with a real cost and selling_price = 0. All 51 reproduced exactly from ((selling - cost) / cost * 100) — none were typed by a human. Two independent defects, both price-rounding: 1. The approvals screen priced with Math.round(cost * (1 + m/100)) and no precision argument — round to WHOLE RAND. A 30% markup on a R0.07 washer became R0.00, and a selling price of 0 re-derives as exactly -100. Rounding is now a 2dp ceiling: R0.07 @ 30% -> R0.10. A ceiling can never collapse a positive price to zero, so that failure mode is structurally unreachable rather than merely patched. 2. Round-to-nearest also undercut cheap lines generally: 30% on R0.07 rounded to R0.09, which re-derives as 28.6% — the "random number". Rounding up means the price can only drift in the customer's favour. Markup now floors at MIN_MARKUP_PERCENT (18) once a line has a cost, applied in one place so every screen inherits it. Scoped to priced lines: an unpriced draft (no cost) is undecided, not 0% margin, and stays writable — 1,181 live rows depend on that. Applied at derivation (not just on save) so -100 can no longer be computed, plus server-side in review-approve as the authoritative gate. order_items already carried markup min=0 plus a Math.max(0, …) clamp, which is why no order row was ever affected. quote_items had neither. Verified: 3,817 tests pass, typecheck clean, lint delta zero, and all 16 of the real broken rows re-priced through the new logic resolve to a valid markup and a non-zero selling price (was 16/16 broken). Existing negative rows are left untouched — re-pricing live quotes is a separate, deliberate decision.
449 lines
17 KiB
TypeScript
449 lines
17 KiB
TypeScript
import { describe, it, expect } from "vitest";
|
||
import {
|
||
applyMarkupFloor,
|
||
buildLinePricing,
|
||
DEFAULT_MARKUP_PERCENT,
|
||
hasCostPrice,
|
||
MIN_MARKUP_PERCENT,
|
||
markupFromCostAndSelling,
|
||
repriceLine,
|
||
repriceWithCostEntry,
|
||
round2,
|
||
round2Up,
|
||
sellingFromCostAndMarkup,
|
||
totalsForLines,
|
||
} from "./quote-pricing";
|
||
|
||
/**
|
||
* A wrong selling price here is invisible: it renders as a plausible
|
||
* number, the total still sums, and no screen says "error". These
|
||
* tests pin the arithmetic that the review modal's totals depend on.
|
||
*/
|
||
|
||
describe("round2", () => {
|
||
it("kills float drift rather than surfacing it", () => {
|
||
// 100 * 1.15 is 114.99999999999999 in IEEE-754. That has to reach
|
||
// the quote as 115, not as a 14-decimal number.
|
||
expect(sellingFromCostAndMarkup(100, 15)).toBe(115);
|
||
expect(round2(0.1 + 0.2)).toBe(0.3);
|
||
});
|
||
|
||
it("rounds half away from zero, not to even", () => {
|
||
expect(round2(1.005)).toBe(1.01);
|
||
expect(round2(2.675)).toBe(2.68);
|
||
});
|
||
|
||
it("coerces non-finite input to 0 rather than NaN", () => {
|
||
expect(round2(Number.NaN)).toBe(0);
|
||
expect(round2(Number.POSITIVE_INFINITY)).toBe(0);
|
||
});
|
||
});
|
||
|
||
describe("markup convention", () => {
|
||
it("treats markup as a PERCENTAGE, not a multiplier", () => {
|
||
// 25 means 25%, so cost 100 -> 125. Getting this backwards turns
|
||
// a 25% margin into a 25x price and nobody would notice until
|
||
// someone tried to pay it.
|
||
expect(sellingFromCostAndMarkup(100, 25)).toBe(125);
|
||
expect(sellingFromCostAndMarkup(100, 0)).toBe(100);
|
||
expect(sellingFromCostAndMarkup(100, 100)).toBe(200);
|
||
});
|
||
|
||
it("round-trips cost -> selling -> markup", () => {
|
||
for (const [cost, markup] of [
|
||
[100, 25],
|
||
[57.5, 33.3],
|
||
[1234.56, 18.75],
|
||
[1, 99],
|
||
] as const) {
|
||
const selling = sellingFromCostAndMarkup(cost, markup);
|
||
const back = markupFromCostAndSelling(cost, selling);
|
||
expect(Math.abs(back - markup)).toBeLessThan(0.02);
|
||
}
|
||
});
|
||
|
||
it("reports zero markup for a zero cost instead of dividing by zero", () => {
|
||
expect(markupFromCostAndSelling(0, 500)).toBe(0);
|
||
});
|
||
});
|
||
|
||
describe("hasCostPrice", () => {
|
||
it("requires a positive finite number", () => {
|
||
expect(hasCostPrice(0)).toBe(false);
|
||
expect(hasCostPrice(null)).toBe(false);
|
||
expect(hasCostPrice(undefined)).toBe(false);
|
||
expect(hasCostPrice(Number.NaN)).toBe(false);
|
||
expect(hasCostPrice(0.01)).toBe(true);
|
||
});
|
||
});
|
||
|
||
/**
|
||
* ── The minus-one-hundred regression ──────────────────────────────
|
||
*
|
||
* 40 live quote_items rows were found holding `markup = -100`, every
|
||
* one of them with a real cost (R0.04 … R0.38) and `selling_price = 0`.
|
||
* Nobody typed -100. The derivation `(selling - cost) / cost * 100`
|
||
* returns exactly -100 whenever selling is 0 and cost is positive, so
|
||
* any screen that derived markup from a line that had a cost but no
|
||
* selling price yet produced -100 and persisted it.
|
||
*
|
||
* These tests pin the fix at each of the three layers: derivation,
|
||
* the floor helper, and the round-up that stops selling collapsing
|
||
* to zero in the first place.
|
||
*/
|
||
describe("round2Up", () => {
|
||
it("never rounds DOWN", () => {
|
||
expect(round2Up(1.201)).toBe(1.21);
|
||
expect(round2Up(1.209)).toBe(1.21);
|
||
expect(round2Up(1.2)).toBe(1.2);
|
||
});
|
||
|
||
it("can never turn a positive number into zero — the structural guarantee", () => {
|
||
// This is the whole point. `round2` (to nearest) sends 0.07 × 1.30
|
||
// = 0.091 to 0.09, but on a 4-cent washer the same operation
|
||
// collapses the SELLING price to R0.00, and a selling price of 0
|
||
// is precisely what re-derives as -100%.
|
||
for (const cost of [0.04, 0.06, 0.07, 0.16, 0.38, 1.19]) {
|
||
for (const markup of [15, 20, 25, 30, 45]) {
|
||
const selling = round2Up(cost * (1 + markup / 100));
|
||
expect(selling).toBeGreaterThan(0);
|
||
}
|
||
}
|
||
});
|
||
|
||
it("keeps an exact 2dp boundary from being bumped a whole cent by float drift", () => {
|
||
// 0.056 × 1.25 computes as 0.07000000000000001, which a naive
|
||
// ceil takes to R0.08 — a whole cent overcharge on every such line.
|
||
// The EPSILON guard pulls it back to the R0.07 it actually is.
|
||
expect((0.056 * 1.25) * 100).toBeGreaterThan(Math.round(0.056 * 1.25 * 100)); // drift
|
||
expect(round2Up(0.056 * 1.25)).toBe(0.07);
|
||
expect(round2Up(0.112 * 1.25)).toBe(0.14);
|
||
expect(round2Up(0.168 * 1.25)).toBe(0.21);
|
||
});
|
||
|
||
it("coerces non-finite input to 0 rather than NaN", () => {
|
||
expect(round2Up(Number.NaN)).toBe(0);
|
||
expect(round2Up(Number.POSITIVE_INFINITY)).toBe(0);
|
||
});
|
||
});
|
||
|
||
describe("the -100 bug: markup derived from a zero selling price", () => {
|
||
it("used to return exactly -100, and must never do so again", () => {
|
||
// The raw formula, unguarded, for the real costs found in the DB.
|
||
for (const cost of [0.04, 0.06, 0.07, 0.16, 0.25, 0.38, 1.19]) {
|
||
expect(((0 - cost) / cost) * 100).toBe(-100);
|
||
// And the fix:
|
||
expect(markupFromCostAndSelling(cost, 0)).toBe(MIN_MARKUP_PERCENT);
|
||
}
|
||
});
|
||
|
||
it("floors a selling price set BELOW cost rather than reporting a loss", () => {
|
||
// "KEY STEEL, 18 X 11mm" sat at cost 321 / selling 182.95 = -43.01%.
|
||
expect(markupFromCostAndSelling(321, 182.95)).toBe(MIN_MARKUP_PERCENT);
|
||
// The 1.19 / 1.00 family that produced -15.97, -6.54, -4.76, -7.41.
|
||
expect(markupFromCostAndSelling(1.19, 1)).toBe(MIN_MARKUP_PERCENT);
|
||
expect(markupFromCostAndSelling(1.07, 1)).toBe(MIN_MARKUP_PERCENT);
|
||
});
|
||
|
||
it("still reports the truth above the floor", () => {
|
||
expect(markupFromCostAndSelling(100, 125)).toBe(25);
|
||
expect(markupFromCostAndSelling(100, 120)).toBe(20);
|
||
// Exactly on the floor passes through untouched.
|
||
expect(markupFromCostAndSelling(100, 118)).toBe(MIN_MARKUP_PERCENT);
|
||
});
|
||
|
||
it("does NOT round DOWN into the floor", () => {
|
||
// 17.4% is under the floor and must read as the floor, not 17.
|
||
expect(markupFromCostAndSelling(100, 117.4)).toBe(MIN_MARKUP_PERCENT);
|
||
});
|
||
});
|
||
|
||
describe("applyMarkupFloor", () => {
|
||
it("raises a sub-floor markup on a priced line", () => {
|
||
expect(applyMarkupFloor(0, 100)).toBe(MIN_MARKUP_PERCENT);
|
||
expect(applyMarkupFloor(8, 100)).toBe(MIN_MARKUP_PERCENT);
|
||
expect(applyMarkupFloor(-100, 0.07)).toBe(MIN_MARKUP_PERCENT);
|
||
});
|
||
|
||
it("leaves an above-floor markup exactly as chosen", () => {
|
||
expect(applyMarkupFloor(18, 100)).toBe(18);
|
||
expect(applyMarkupFloor(22, 100)).toBe(22);
|
||
expect(applyMarkupFloor(50, 100)).toBe(50);
|
||
});
|
||
|
||
it("does NOT floor an unpriced draft — 1,181 live rows depend on this", () => {
|
||
// cost 0 + markup 0 means "nothing decided yet", not "0% margin".
|
||
// Flooring these would block creating a quote line at all.
|
||
expect(applyMarkupFloor(0, 0)).toBe(0);
|
||
expect(applyMarkupFloor(0, null)).toBe(0);
|
||
expect(applyMarkupFloor(0, undefined)).toBe(0);
|
||
});
|
||
|
||
it("floors a NEGATIVE markup even on a cheap line", () => {
|
||
expect(applyMarkupFloor(-100, 0.07)).toBe(MIN_MARKUP_PERCENT);
|
||
expect(applyMarkupFloor(-43.01, 321)).toBe(MIN_MARKUP_PERCENT);
|
||
});
|
||
|
||
it("coerces garbage to the floor rather than propagating NaN", () => {
|
||
expect(applyMarkupFloor(Number.NaN, 100)).toBe(MIN_MARKUP_PERCENT);
|
||
});
|
||
});
|
||
|
||
describe("selling price cannot collapse to zero on a cheap line", () => {
|
||
it("keeps a 30% markup on the real washer/nut costs as a real price", () => {
|
||
// The approvals screen used `Math.round(cost * (1 + m/100))` with
|
||
// no precision argument, which is round-to-WHOLE-RAND: every one of
|
||
// these became R0.00 and then re-derived as -100%.
|
||
for (const cost of [0.04, 0.06, 0.07, 0.16, 0.38]) {
|
||
const selling = sellingFromCostAndMarkup(cost, 30);
|
||
expect(selling).toBeGreaterThan(0);
|
||
expect(selling).toBeGreaterThan(cost);
|
||
}
|
||
});
|
||
|
||
it("rounds UP so the quoted markup is never undercut", () => {
|
||
// 30% on R0.07 is R0.091. Ceiling → R0.10 (a real 42.9% margin, so
|
||
// the customer is never under-quoted). Rounding to nearest would
|
||
// give R0.09, which re-derives as 28.6% — the "my number changed to
|
||
// something random" complaint, and an UNDERQUOTE besides.
|
||
expect(sellingFromCostAndMarkup(0.07, 30)).toBe(0.1);
|
||
expect(sellingFromCostAndMarkup(0.06, 25)).toBe(0.08);
|
||
// Proof the old value was an underquote:
|
||
expect(Math.round(0.07 * 1.3 * 100) / 100).toBeLessThan(0.07 * 1.3);
|
||
});
|
||
|
||
it("leaves a price that is already clean alone", () => {
|
||
expect(sellingFromCostAndMarkup(100, 25)).toBe(125);
|
||
expect(sellingFromCostAndMarkup(12.5, 25)).toBe(15.63);
|
||
});
|
||
});
|
||
|
||
describe("repriceLine honours the floor", () => {
|
||
const base = {
|
||
orderItemId: "i1",
|
||
description: "M10 FLAT WASHER BLK",
|
||
quantity: 1,
|
||
costPrice: 0.07,
|
||
markupPercent: 30,
|
||
sellingPrice: 0.1,
|
||
};
|
||
|
||
it("floors a sub-floor markup instead of writing it through", () => {
|
||
const r = repriceLine(base, { markupPercent: 0 });
|
||
expect(r.markupPercent).toBe(MIN_MARKUP_PERCENT);
|
||
expect(r.sellingPrice).toBeGreaterThan(0);
|
||
});
|
||
|
||
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 });
|
||
expect(r.markupPercent).toBe(MIN_MARKUP_PERCENT);
|
||
});
|
||
|
||
it("still allows the deliberate thin margins reps actually use", () => {
|
||
expect(repriceLine(base, { markupPercent: 18 }).markupPercent).toBe(18);
|
||
expect(repriceLine(base, { markupPercent: 20 }).markupPercent).toBe(20);
|
||
expect(repriceLine(base, { markupPercent: 22 }).markupPercent).toBe(22);
|
||
});
|
||
});
|
||
|
||
describe("buildLinePricing", () => {
|
||
it("applies the default markup to a cost-only line", () => {
|
||
const [line] = buildLinePricing([
|
||
{ id: "i1", description: "Bolt", quantity: 2, cost_price: 100 },
|
||
]);
|
||
expect(line).toMatchObject({
|
||
orderItemId: "i1",
|
||
quantity: 2,
|
||
costPrice: 100,
|
||
markupPercent: DEFAULT_MARKUP_PERCENT,
|
||
sellingPrice: 125,
|
||
});
|
||
});
|
||
|
||
it("respects an EXISTING selling price instead of re-marking it", () => {
|
||
// A human priced this before the delivery note existed. Silently
|
||
// replacing it with the default markup would overwrite a decision.
|
||
const [line] = buildLinePricing([
|
||
{ id: "i1", cost_price: 100, selling_price: 160 },
|
||
]);
|
||
expect(line.sellingPrice).toBe(160);
|
||
expect(line.markupPercent).toBe(60);
|
||
});
|
||
|
||
it("leaves a no-cost line at zero rather than quoting a fake R0.00", () => {
|
||
// cost 0 x (1 + 25%) is still 0 — and a selling price of 0 looks
|
||
// like a deliberate free-of-charge item. It has to read as
|
||
// "no cost on file" in the UI, which means a real 0 here.
|
||
const [line] = buildLinePricing([{ id: "i1", cost_price: 0 }], 25);
|
||
expect(line.costPrice).toBe(0);
|
||
expect(line.markupPercent).toBe(0);
|
||
expect(line.sellingPrice).toBe(0);
|
||
expect(hasCostPrice(line.costPrice)).toBe(false);
|
||
});
|
||
|
||
it("defaults a missing quantity to 1", () => {
|
||
const [line] = buildLinePricing([{ id: "i1", cost_price: 50 }]);
|
||
expect(line.quantity).toBe(1);
|
||
});
|
||
|
||
it("maps 1:1 in the order it was given", () => {
|
||
const lines = buildLinePricing([
|
||
{ id: "a", cost_price: 10 },
|
||
{ id: "b", cost_price: 20 },
|
||
{ id: "c", cost_price: 0 },
|
||
]);
|
||
expect(lines.map((l) => l.orderItemId)).toEqual(["a", "b", "c"]);
|
||
});
|
||
});
|
||
|
||
describe("repriceLine", () => {
|
||
const base = {
|
||
orderItemId: "i1",
|
||
description: "Bolt",
|
||
quantity: 1,
|
||
costPrice: 100,
|
||
markupPercent: 25,
|
||
sellingPrice: 125,
|
||
};
|
||
|
||
it("re-derives selling when the markup changes", () => {
|
||
expect(repriceLine(base, { markupPercent: 40 }).sellingPrice).toBe(140);
|
||
});
|
||
|
||
it("re-derives selling when the cost changes", () => {
|
||
const r = repriceLine(base, { costPrice: 200 });
|
||
expect(r.sellingPrice).toBe(250);
|
||
expect(r.markupPercent).toBe(25);
|
||
});
|
||
|
||
it("collapses to zero when the cost is zeroed out", () => {
|
||
// Editing cost to 0 must NOT leave the old selling price hanging
|
||
// on the line — that is a R125 charge on a free item.
|
||
const r = repriceLine(base, { costPrice: 0 });
|
||
expect(r.sellingPrice).toBe(0);
|
||
});
|
||
|
||
it("does not mutate the input line", () => {
|
||
repriceLine(base, { markupPercent: 99 });
|
||
expect(base.sellingPrice).toBe(125);
|
||
expect(base.markupPercent).toBe(25);
|
||
});
|
||
|
||
it("leaves untouched fields alone", () => {
|
||
const r = repriceLine(base, { markupPercent: 50 });
|
||
expect(r.orderItemId).toBe("i1");
|
||
expect(r.description).toBe("Bolt");
|
||
expect(r.quantity).toBe(1);
|
||
});
|
||
});
|
||
|
||
describe("totalsForLines", () => {
|
||
it("multiplies by quantity and sums both sides", () => {
|
||
// 100 @ 25% x3 = 375, and 50 @ 25% x2 = 125.
|
||
const totals = totalsForLines([
|
||
{ ...buildLinePricing([{ id: "a", cost_price: 100, quantity: 3 }])[0] },
|
||
{ ...buildLinePricing([{ id: "b", cost_price: 50, quantity: 2 }])[0] },
|
||
]);
|
||
expect(totals.costTotal).toBe(400);
|
||
expect(totals.sellingTotal).toBe(500);
|
||
expect(totals.marginValue).toBe(100);
|
||
});
|
||
|
||
it("reports margin as a share of REVENUE, not of cost", () => {
|
||
// cost 100 -> selling 125 is a 25% markup but only a 20% margin.
|
||
// Conflating the two is the classic pricing mistake.
|
||
const totals = totalsForLines([
|
||
buildLinePricing([{ id: "a", cost_price: 100, selling_price: 125 }])[0],
|
||
]);
|
||
expect(totals.marginPercent).toBe(20);
|
||
});
|
||
|
||
it("is zero-safe on an empty set and on no-revenue lines", () => {
|
||
expect(totalsForLines([])).toEqual({
|
||
costTotal: 0,
|
||
sellingTotal: 0,
|
||
marginValue: 0,
|
||
marginPercent: 0,
|
||
});
|
||
const noCost = totalsForLines([
|
||
buildLinePricing([{ id: "a", cost_price: 0 }])[0],
|
||
]);
|
||
expect(noCost.marginPercent).toBe(0);
|
||
});
|
||
|
||
it("keeps cost and selling totals in step after an edit", () => {
|
||
// The modal recomputes these live on every keystroke, so an
|
||
// inconsistency here shows up as footer numbers that don't match
|
||
// the visible rows.
|
||
let lines = buildLinePricing([{ id: "a", cost_price: 100, quantity: 4 }]);
|
||
lines = [repriceLine(lines[0], { markupPercent: 30 })];
|
||
const totals = totalsForLines(lines);
|
||
expect(totals.costTotal).toBe(400);
|
||
expect(totals.sellingTotal).toBe(520);
|
||
});
|
||
});
|
||
|
||
describe("repriceWithCostEntry", () => {
|
||
const noCost = {
|
||
orderItemId: "i1",
|
||
description: "1.6MM BRAZING ROD",
|
||
quantity: 10,
|
||
costPrice: 0,
|
||
markupPercent: 0,
|
||
sellingPrice: 0,
|
||
};
|
||
|
||
it("seeds the default markup when the FIRST cost is typed", () => {
|
||
// Found in the browser, not here. A counter note opens with no cost
|
||
// on file, so the line starts at markup 0; typing a cost used to
|
||
// leave it there and derive a selling price EQUAL to the cost —
|
||
// a line that looks priced and earns nothing.
|
||
const r = repriceWithCostEntry(noCost, { costPrice: 100 });
|
||
expect(r.markupPercent).toBe(DEFAULT_MARKUP_PERCENT);
|
||
expect(r.sellingPrice).toBe(125);
|
||
});
|
||
|
||
it("yields a margin, not a zero-margin line", () => {
|
||
const r = repriceWithCostEntry(noCost, { costPrice: 100 });
|
||
const totals = totalsForLines([r]);
|
||
expect(totals.sellingTotal).toBeGreaterThan(totals.costTotal);
|
||
expect(totals.marginValue).toBeGreaterThan(0);
|
||
});
|
||
|
||
it("does NOT reseed when the operator is editing an already-costed line", () => {
|
||
const costed = { ...noCost, costPrice: 100, markupPercent: 40, sellingPrice: 140 };
|
||
const r = repriceWithCostEntry(costed, { costPrice: 200 });
|
||
expect(r.markupPercent).toBe(40);
|
||
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("seeds nothing when the cost is cleared back to zero", () => {
|
||
const costed = { ...noCost, costPrice: 100, markupPercent: 0, sellingPrice: 100 };
|
||
const r = repriceWithCostEntry(costed, { costPrice: 0 });
|
||
expect(r.sellingPrice).toBe(0);
|
||
});
|
||
|
||
it("is a pure no-op when the patch carries no cost", () => {
|
||
const costed = { ...noCost, costPrice: 100, markupPercent: 25, sellingPrice: 125 };
|
||
expect(repriceWithCostEntry(costed, { markupPercent: 30 })).toEqual({
|
||
...costed,
|
||
markupPercent: 30,
|
||
sellingPrice: 130,
|
||
});
|
||
});
|
||
});
|