EPIC: Discount visibility and control on sales #719

Why

Discounts were being given on sales without the owner knowing. Investigation of the
current code found the cause is not a missing screen — the system has no discount
concept at all:

• SalesOrderItem (src/Cluckwork.Domain/Sales/SalesOrder.cs) carries UnitPrice
  and nothing else. Product.DefaultPriceMinorUnits is the list price;
  SalesPage.tsx:890 prefills the price field from it and the seller overtypes it.
  The whole "discount" mechanism is an edited number in a text box.
• The line snapshots ProductTypeSnapshot, EggGradeId, BaseUnitFactor and
  UnitPrice per spec §10.5 — but not the list price it was sold against.
  There is no stored fact saying a sale was discounted.
• AddOrderItemHandler.cs:106 and UpdateOrderItemHandler.cs:30 do write audit
  events, but with no payload — the trail shows someone touched a line, never
  what price they set.

So the discounts were not hidden by a UI gap. They were never recorded as discounts,
and they are not reconstructible from what is stored today.

The decision that must land first (blocks everything else)

Snapshot the list price on the sales order line — ListUnitPriceMinorUnits on
SalesOrderItem, captured from Product.DefaultPriceMinorUnits at line creation —
and derive discount as ListUnitPrice − UnitPrice.

Without the snapshot, "was this discounted?" is computed live against today's
catalog price: raise a product's price next month and every past sale retroactively
becomes a discount; drop it and real discounts vanish. The order screen, the history
badge and the report would each drift, and they would drift differently. This is
the same rule already written into §10.5 for BaseUnitFactor — redefining a carton
must not reinterpret recorded orders. Same rule, same field.

Decide before implementing: products with a null DefaultPriceMinorUnits have no
list price, so "discount" is undefined for them. Either require a list price on egg
products, or render those lines as no list price — never as 0% off, which is the
version that lets a discount hide.

Data starts on the day this ships. Orders confirmed before the migration have no
list-price snapshot and must display as blank/unknown, not as zero discount. Every
screen and report in this epic states that explicitly.

Slices, in implementation order

Sized against the running app and the source, 2026-09-08. Blocks means the named
issue cannot start until this one ships.

Progress

• Sales: snapshot the list price on the order line (discount foundation) #720 Snapshot the list price on the order line — shipped 2026-09-09, PR feat(sales): snapshot the list price on the order line and show the discount #734, squashed to cffed5e
• Sales: record list/old/new price in the order-line audit payload #722 Price change in the audit payload — shipped 2026-09-10, PR feat(sales): record list, old and new price in the order-line audit payload (#722) #742, squashed to 97c866f
• Sales: mark discounted lines and show a discount total on the order screen #723 Mark discounted lines on the order screen — shipped 2026-09-10, PR feat(sales): mark discounted lines, total the discount, and show it in the Orders list (#723, #724) #741, squashed to 1a07441
• Sales: discount badge with amount and percent in history and order detail #724 Discount badge in the Orders list — shipped 2026-09-10, PR feat(sales): mark discounted lines, total the discount, and show it in the Orders list (#723, #724) #741 (same PR as Sales: mark discounted lines and show a discount total on the order screen #723), squashed to 1a07441. The Sales: require a discount reason when confirming a below-list order #721 discount reason on order detail is deliberately not met and is deferred to Sales: require a discount reason when confirming a below-list order #721 — amendment on the issue.
• Sales: require a discount reason when confirming a below-list order #721 Require a discount reason at confirm — shipped 2026-09-11, PR feat(sales): require a discount reason when confirming a below-list order #756, squashed to c7864be. All five acceptance criteria met, including history: the SalesOrder.Confirm audit row carries the reason (the earlier "deliberately not met" amendment was withdrawn). Answers epic open question 2 (per order, not per line) and question 4 (no, a markup above list does not trigger it — HasBelowListLine uses a strict <). Also found and filed Audit Details shows the list price as if it were the sale price on every order line #758: the Audit Details cell renders the LIST price as if it were the sale price on every order line, which hides exactly the discounts this epic exists to surface.
• Audit Details shows the list price as if it were the sale price on every order line #758 Audit Details showed the list price as the sale price — shipped 2026-09-11, PR fix(audit): show the price a line sold for, not its list price #759, squashed to e6b37d0. Regression from SPA: render audit detailsJson as a readable per-key table instead of a raw tooltip #745/feat(audit): show the sales-line audit payload as a readable Details column (#745) #749: the SalesOrder.AddItem Details cell discarded unitPriceMinorUnits whenever a list price existed, so every line read at list $X whatever it sold for — hiding the discounts this epic exists to surface, and understating one fixture line sold above list by 25%. An off-list line now renders at $0.35 (list $0.45), with no direction marker, per this epic's open question 4 (a markup is not a discount). Rendering only: the audit payload already carried both prices correctly, verified against the seeded database. Four literal-text tests, each watched red under its own mutation.
• Sales: per-farm discount ceiling, with Owner/Manager approval above it #727 Per-farm ceiling with Owner/Manager approval — shipped 2026-09-11, PR feat(sales): refuse an over-ceiling confirm from a Sales user (#727) #766, squashed to 8c0792a. All five acceptance criteria met, with the fifth amended: a PreDating line is refused rather than waved through, because the Sales: snapshot the list price on the order line (discount foundation) #720 comment naming this issue says the three non-Recorded bases need opposite treatment, and a pre-Sales: snapshot the list price on the order line (discount foundation) #720 draft can still be re-priced today. Answers epic open question 3 (no in-app approval flow: an honest refusal, out of band). Measured per line, Owner and Manager may exceed, 422 not 403, stored as basis points. Four product defects were found in review and fixed before merge, two of them by an independent adversarial reviewer after CodeRabbit had passed the same code twice: a client-side gate that blocked confirms the server would allow, and a zero list price that waived the ceiling entirely. Also filed AGENTS.md #394 says the Playwright specs are not in CI; they have been since 2026-08-08 #767 (AGENTS.md wrongly says the Playwright specs are outside CI) and recorded a coupon-compatibility analysis on Future: coupon codes as the discount mechanism, instead of typed prices #730.
• Reports: discount totals per salesperson and per customer, in report and CSV export #725 Discount totals in report and export
• Customers: discount history and running total on the customer record #726 Discount history on the customer record
• Alerts: raise an alert-centre entry for a large discount at confirm #728 Alert-centre entry — also needs the Phase 1.5 alert centre on EPIC: Phase 1.5 — Egg product hardening #15
• Future: coupon codes as the discount mechanism, instead of typed prices #730 Coupon codes — unscheduled, needs its own scoping decision

What #720 leaves the rest of the epic: ListUnitPriceMinorUnits (nullable, snapshotted at line
creation, never re-resolved) and ListPriceBasis (Recorded | ProductUnpriced | NotComparable | PreDating), so a pre-migration line is permanently distinguishable from one where no comparable list
price existed. Plus a stale-price guard, and the translated terms List price, Discount and
Above list in en/es/tl with glossary entries — reuse them rather than coining new ones.

#720 ─┬─ #722
      │
      ├─ #723 ── #724
      │
      └─ #721 ─┬─ #727 ─┐
               │        ├─ #728  (also needs the alert centre, #15)
               ├─ #725 ─┘
               │    │
               │    └─ #726

#730 is deliberately unnumbered. It is on this epic but not in the sequence:
it needs a scoping decision, it is probably its own epic rather than a slice, and
it is the one item that would change what some of the nine above should be.

Related but outside this epic: #729 moves farm configuration and identity to
Owner-only. It came out of scoping #727 but #727 no longer depends on it.

Why this order

#720 first, because it defines "below list" for everything else. Additive and
nullable, so it breaks zero existing tests: SalesOrderItem.Create is internal
with one caller, and AddOrderItemHandler already loads the Product, so the value
is in hand and no new query is needed. Its real cost is the migration's generated
Designer file and a Docker-bound schema-docs run.

#722 second, and it does not strictly need #720 to compile — but it does to be
complete. AddOrderItemHandler already has the list price
(command.UnitPriceMinorUnits ?? product.DefaultPriceMinorUnits), so the add half
stands alone. UpdateOrderItemHandler does not: its constructor is
(ISalesOrderRepository, IAuditWriter, IUnitOfWork) with no product repository, so
before #720 an update event can carry old and new unit price but no list price.
Shipping it first would write rows where the list price is absent because the
column did not exist yet — indistinguishable from a product that had no list price,
which is exactly the null-means-two-things trap #720 warns about. After #720 the
snapshot is on the item the handler already holds, so the whole payload lands in
one change with no new query. Roughly two source files either way.

#723 before #724 for a reason that is not size. Both need a translated word for
discount, catalogParity compares key sets only, and #688 says nothing enforces
that two screens agree per locale. Ship them adjacent so one person picks the term
once.

#721 before #725, #726 and #728 because the reason is a column those three
display. Building them first means adding the column later and revisiting three
read surfaces.

#727 after #721 because both change the confirm path, and doing them in one
region avoids two passes over ConfirmSaleHandler's 265 lines of ordered locking.

If #730 is close, cut #721 down. A coupon code is a structured reason, so
#721's five-value picklist would be superseded within a release. Ship #721 as free
text in that case — useConfirm.askReason() already provides a required non-blank
reason and is used by Void on the same screen, which makes that version nearly
free. Decide #730's fate before starting #721, not after.

#725 before #726 because #726's per-customer totals must come from #725's query.
Two customer-discount numbers that can disagree is a defect waiting to be filed.

Two slices need a decision before they are scheduled

#725 promises a per-salesperson breakdown that cannot be built. SalesOrders
has 11 columns and none records a creating user. The Orders list's History column
reads Today · admin from audit rows, not from the order. Attribution needs
Action LIKE '%.Create', which has no supporting index and is permanently blank
before #494. Add a CreatedByUserId column as its own slice, or cut that half.

#726 targets a customer detail screen that does not exist, and the app has no
parameterised route in any of its 24 routes. As written it is the app's first
/customers/:id page, 10 to 12 files, most of them new. The cheaper reading is two
columns on the existing list plus a panel, about 4 files.

Repo rules this epic touches

• feat(eggs): make cracked and dirty eggs sellable stock via condition grades (#396) #407 — one new migration per change; never fold a column into InitialCreate.
• chore(schema): generate PostgreSQL schema documentation #417 — regenerate docs/schema/ in the same PR; CI fails a stale one.
• i18n: catalogParity compares key SETS, so a translation that drifts in MEANING passes every gate #688 / i18n — every new string in all three locales, and help/glossary prose
  naming a control must use that control's label in each locale. Nothing enforces
  this; it is review-only.
• Owner directive — specs/product/GLOSSARY.md gains discount, list price
  and discount reason; the SPA Help page and in-app glossary update in the same PR.
• Daily entry: require grading to reconcile sellable eggs before submit #394 — if confirm gains a required reason, that is a write-contract change:
  read the non-CI callers (DemoDataSeeder, SimulationDataSeeder,
  tools/simulation/k6/, tools/simulation/ui/ Playwright specs). A green baseline
  hides them.
• Version++ — any new SalesOrder / SalesOrderItem mutation bumps Version,
  with a parallel-race integration test.
• Visual PRs — slices 4 and 5 attach 1:1 before/after screenshots from a stack
  rebuilt at the head under review (AGENTS.md: three conventions earned while shipping #651 + #652 #662).

Open questions

1. Is a null DefaultPriceMinorUnits allowed to stay, or does an egg product now
   require a list price? (Decides slice 1's null handling.)
2. Should the discount reason be required on every discounted line, or once per
   order? Per order is cheaper and probably enough.
3. Does an above-threshold order need an in-app approval flow, or is "Sales cannot
   confirm it, tell a manager" sufficient for now?
4. Is a markup above list price also worth flagging, or only discounts?