Skip to content

fix(sales): scale a line's default and list price to its unit - #1166

Merged
mforce merged 8 commits into
mainfrom
fix/1160-unit-scaled-price
Oct 10, 2026
Merged

mforce merged 8 commits into
mainfrom
fix/1160-unit-scaled-price

Conversation

@mforce

@mforce mforce commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

Closes #1160

Why

A product's default price is per its own selling unit (Sold per on Products), but a sales line sold in another unit through Per used that price unscaled. Large Eggs at $0.45 per egg, sold Per: Tray, prefilled $0.45 a tray: 8 trays (240 eggs) came to $3.60 instead of $108.00. The line also recorded $0.45 as its list price, so its discount was measured against a per-egg number. The reverse, a tray-priced product sold per egg, overcharged 30 times. The server had the same gap, so an API caller that omitted the price got the wrong default too. Present since #100; #734 added the unscaled list-price snapshot on top.

Scope

  • AddOrderItemHandler: a line's default price and list price are the product's default price × (line unit eggs-per-unit ÷ product unit eggs-per-unit), rounded up to a whole minor unit, in exact Int128 math. The stale-list-price check (SalesOrder.ListPriceChanged) compares and prints the scaled value. A line in the product's own unit, or for an unpriced product, takes the old path with no extra lookup. A line in another unit needs an active definition for the product's unit too (SalesOrder.NoUnitConversion, naming that unit). A scaled price beyond long refuses with SalesOrder.LineTotalTooLarge.
  • SalesPage.tsx: one helper, listPriceFor, feeds the prefill, the Per change (which now re-prefills the price, as a product change already did), the below/above-list hint and the expectedListUnitPriceMinorUnits expectation. It computes the same value in BigInt. When a factor is unknown on the client, or the result is beyond Number.MAX_SAFE_INTEGER, the price stays blank and no list-price expectation is sent; the server then applies its own scaled default.
  • Docs: docs/decisions/1160-unit-scaled-list-price.md, the GLOSSARY's Sales line and List price entries, and the Help page and in-app glossary in en/es/tl.

Recorded lines are not rewritten. There is no migration or backfill; #720 snapshots are history. UpdateOrderItem is unchanged because a line's unit cannot change after creation.

Tradeoffs

Rounding up instead of recording uneven scaling as NotComparable. #727 exempts a line whose list-price basis says the discount is not computable from the discount ceiling, so NotComparable would let a ceiling-bound seller escape the ceiling by picking a unit that does not divide evenly. Rounding up rather than half up, because half up can scale a positive price to zero: $1.30 a case of 360 sold per egg became a list price of 0, and a free line then read as "at list" under a 0% ceiling. Rounded up, a positive price never scales to zero and the default never undercuts the bulk price. The cost is that a customer pays at most one minor unit more per selling unit than the exact share: a $13.00 tray sold per egg is $0.44.

Blast Radius

  • Every add-line call where the line's unit differs from the product's selling unit now gets a different default price and list price. Same-unit lines are byte-for-byte unchanged.
  • One existing test pinned the bug. PackedUnit_FactorSnapshots_RedefineOnlyAffectsFutureLines sold 3 cartons of a 100-per-egg product and expected 300. It now expects 2 × 1200 + 1 × 3000 = 5400.
  • Non-CI callers are unaffected: both seeders and k6 bundles.js omit the unit, so they sell in the product's own unit. No Playwright spec touches the Per picker.
  • New failure mode, for a priced product sold in another unit only: an inactive definition for the product's own unit now refuses the add.

Verification

Red at origin/main, green at the head:

Suite Before After
AddOrderItemHandlerTests (new) 9 failed, 1 passed (same-unit case) 12 passed
SalesProductTests.AddLine_InAnotherUnit_ScalesTheDefaultAndListPrice + PackedUnit_FactorSnapshots_… 3 failed (Expected (1350, 1350), Actual (45, 45); Expected 5400, Actual 300) passed
SalesPage.test.tsx, four new cases 4 failed (expectedListUnitPriceMinorUnits: 750 received 300) 276 passed
Round 1 cases (f306ac07, red at 6e54752e) handler 2 failed (Expected 44 / Actual 43, Expected 1 / Actual 0); AFreePerEggLineFromACasePrice_IsRefusedUnderAZeroCeiling failed (Expected 1 / Actual 0); Vitest 2 failed (…396 received …395; the beyond-safe price was prefilled) all passed

Also green locally: the integration classes SalesProductTests, EggLedgerTests, SalesDiscountCeilingTests, SalesDiscountReasonTests, SalesOrderAuditPayloadTests and CustomerAndOrderTests (104 passed with the new ceiling case, no other test needed a change); src/i18n and the help tests (425 passed); npm run typecheck; and a warnings-as-errors solution build.

In the running app (isolated stacks, readme-farm, Large Eggs Per: Tray, quantity 8): before at bd4e5eca the line prefills and records $0.45 a tray for a $3.60 total; after at 6e54752e it prefills and records $13.50 a tray for $108.00. Before/after screenshots.

Failing at the base: the handler, the endpoint and the SPA all use the
product's default price unscaled, whatever unit the line is in.
A product's default price is per its own selling unit. A line in another
unit now defaults to, and snapshots as its list price, that price scaled
by the two eggs-per-unit factors, rounded half up to a whole minor unit.
The stale-list-price check compares the scaled value. The SPA mirrors it
for the prefill, the Per change, the list-price hint and the expectation.

Rounding rather than NotComparable keeps the #727 discount ceiling from
being escaped by picking a unit that does not divide evenly.

Closes #1160
@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

The issue's line on the demo farm (readme-farm): Hotel Paraíso, Large Eggs (sold per egg at $0.45), Per: Tray, quantity 8. 1280×800, 1:1. Before is bd4e5eca (this PR's base on main), after is 6e54752e (this head).

Before. The form prefills $0.45 a tray. The added line records List price $0.45 and Unit price $0.45, so 240 eggs total $3.60.

before, form
before, line added

After. The form prefills $13.50 a tray. The added line records List price $13.50 and Unit price $13.50, so 240 eggs total $108.00, the same as selling them per egg.

after, form
after, line added

Failing at the previous head: half-up turns 1.30 a case into a free
per-egg line that passes a zero ceiling, and the SPA's float quotient
lands a minor unit off near the safe-integer edge.
…ides

Half-up let a positive bulk price scale to a zero per-egg list price, so a
free line read as at list and passed a zero discount ceiling. Rounding up
never yields zero from a positive price and never undercuts the bulk
price. The SPA now computes the same value in BigInt and treats a result
beyond the safe-integer range as unknown.
@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

Review round 1 came from Codex (gpt-6-astra). All three findings are fixed in new commits. Head is 2fe64db2.

1. P2: a positive price could scale to a free line that passed the ceiling. The maintainer decided to round up instead of half up. The server now computes (price × lineFactor + productFactor − 1) / productFactor in Int128 and still refuses with LineTotalTooLarge above long.MaxValue. The SPA computes the same value. The decision record now says why: a positive price never scales to zero, the default never undercuts the bulk price, and splitting a bulk price this way is common practice. The cost is at most one minor unit per selling unit, so a $13.00 tray sold per egg is $0.44. The $1.30-a-case line is recorded there as the evidence that changed the rule. The NotComparable rejection stays. The handler cases now cover 1300 ÷ 30 → 44, 130 ÷ 360 → 1, and a free product staying free. SalesDiscountCeilingTests.AFreePerEggLineFromACasePrice_IsRefusedUnderAZeroCeiling adds 360 eggs at 0 under a 0% ceiling and expects DiscountCeilingExceeded with the order still a draft. Tests in f306ac07 (red at 6e54752e: Expected 44 / Actual 43, Expected 1 / Actual 0), fix in be62c830.

2. P2: SPA float rounding differed from the server. listPriceFor now uses BigInt. A result beyond Number.MAX_SAFE_INTEGER is treated as unknown: no prefill and no list-price expectation, the same as the unknown-factor path. New Vitest cases: your 3-egg Carton at 9007199254740991 sold per Egg (→ 3002399751580331); 9007199254740988 per Tray sold per Dozen, where floats gave …395 and the exact answer is …396 (red at 6e54752e); and an Egg product at MAX_SAFE_INTEGER sold per Tray, which prefills nothing and sends no expectation. The code comment and the decision record no longer make the safe-integer claim. Tests in f306ac07, fix in be62c830.

3. P3: the glossary said "nearest cent". The GLOSSARY and the in-app glossary in en, es and tl now say the price is rounded up to the currency's smallest unit ("redondeado hacia arriba a la unidad más pequeña de la moneda", "ni-round pataas sa pinakamaliit na yunit ng pera"). Fixed in 2fe64db2.

Rerun locally at 2fe64db2: handler and Sales unit tests 45 passed; the six sales integration classes 104 passed; SalesPage.test.tsx 276 passed; i18n and help 425 passed; typecheck and a warnings-as-errors build are clean.

The screenshots stay valid. $0.45 an egg to $13.50 a tray is an exact multiply, so rounding up gives the same numbers.

@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

Review round 2 came from Codex (gpt-6-astra). It found no product defect and one documentation fix.

P3: the decision record claimed the money parser refuses typed prices beyond Number.MAX_SAFE_INTEGER. It does not: parseMoneyToMinorUnits goes through parseFloat. The sentence is gone. The record now says a typed price still loses precision above 2^53 minor units, and that this predates the PR and is out of scope here. The code comment near listPriceFor made no such claim. Fixed in 8c1fbe7c; the parser is unchanged.

@mforce
mforce merged commit 5c2b0c0 into main Oct 10, 2026
20 checks passed
@mforce
mforce deleted the fix/1160-unit-scaled-price branch October 10, 2026 05:58
mforce added a commit that referenced this pull request Oct 10, 2026
… split

#1166 (5c2b0c0) changed web/src/routes/SalesPage.tsx, which this branch
moved. Its four production hunks now live in the files that own that code:
listPriceFor in sales/SalesPage.tsx beside eggsPerUnit, the add-line
expectation in sales/useOrderCommands.tsx, and the Per onChange re-prefill
and list-price hint in sales/AddLineForm.tsx. Its five tests merged into
sales/SalesPage.test.tsx through rename detection, verbatim.
mforce added a commit that referenced this pull request Oct 10, 2026
The merge of origin/main brought #1166's tests into sales/SalesPage.test.tsx
but not its production change, which targeted the deleted SalesPage.tsx.
listPriceFor joins eggsPerUnit in sales/SalesPage.tsx; the add-line
expectation in sales/useOrderCommands.tsx and the Per re-prefill and
list-price hint in sales/AddLineForm.tsx use it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sales line priced per tray keeps the per-egg default price

1 participant