Sales: record list/old/new price in the order-line audit payload

[mforce]

Amendment (2026-09-10): this issue is backend-only and shipped in #742. The payload it records was invisible on screen until #747/#748 (snapshot the product name) and #745/#749 (render it as a Details column). See this comment for what landed where and what this issue never claimed. Body below left as written.

Part of #719. Blocked by #720 (needs the list-price snapshot).

What

Put the actual prices into the sales-line audit events.

AddOrderItemHandler.cs:106 and UpdateOrderItemHandler.cs:30 already write
SalesOrder.AddItem / SalesOrder.UpdateItem audit events — but with no
payload. The trail today shows someone touched a line, never what price
they set. That is why the discounts that triggered #719 are not reconstructible
from stored data.

Scope

Carry list price, previous unit price and new unit price in the audit payload for
both actions. AuditWriter already resolves the actor (#500) and AuditEvents
already has a durable global ordering key (#508), so this one change makes "who
discounted what, in what order" answerable from data already being written.

Guard to read before writing the call site

AuditVocabularyCoverageTests accepts only a literal AuditActions.X — or a
ternary of two such references — as the action argument of an
IAuditWriter.WriteAsync call, and fails closed on everything else. The obvious
tidy-up here (forwarding both calls through one shared private helper) goes red
on a test you would never have opened. Keep the two call sites literal.

Acceptance

• Adding a line records list price and unit price.
• Editing a line records the price it changed from and to.
• Events are attributable to the acting user and orderable across a same-second
  burst (fix(api): order same-instant audit events by a durable monotonic key, not a random Guid #508's Sequence tiebreak).
• AuditVocabularyCoverageTests stays green.

Note

Audit rows are not a reporting surface — the report in #719 reads the order
lines, not this. This slice exists so a specific past incident can be
investigated after the fact, which is the thing that was impossible in the case
that opened #719.

[mforce]

Mockup

This slice is much smaller than the issue implies, and it should ship first. IAuditWriter.WriteAsync already has the parameter:

Task WriteAsync(string action, string entityType, Guid entityId,
    string? reason = null, object? details = null, CancellationToken ct = default);

AuditWriter serialises details into AuditEvent.DetailsJson, and ~30 call sites already pass it — UpdateFarmSettingsHandler.cs:148 passes details: new { before, after = Snapshot(account) }, which is this slice's exact shape. No signature change, no new column, no migration.

DetailsJson already reaches the API response (AuditEndpoints.cs:45), the CSV export (ExportQueries.cs:259) and the SPA (AuditPage.tsx:409 renders it as the row's title). So no i18n, no glossary, no catalog parity. The house test idiom is one Assert.Contains("\"amountMinorUnits\":500", row.DetailsJson) in AuditTests.cs.

The guard named in this issue does not apply the way I wrote it: AuditVocabularyCoverageTests splits the arguments and checks only args[0] and args[1]. A details: argument is invisible to it. The rule that still holds is the other half — reuse the existing AuditActions constants; minting a new one drags web/src/i18n/enums.ts and three locales.

~2 src files, 1 test file, hours. Note the caveat drawn on the artboard: today the SPA shows detailsJson as a raw JSON tooltip with no per-key renderer, so the readable table in the mockup needs a small follow-up. And this slice need not wait for #720 if it logs old/new unit price only — UpdateOrderItemHandler holds only the order, not the product, so the list price there arrives with #720.

[mforce]

Shipped, and what was deliberately left out

Merged as 97c866f (PR #742). All four acceptance criteria delivered:

Criterion	Where
Adding a line records list price and unit price	AddItem_RecordsListPriceBasisByName
Editing records the price it changed from and to	UpdateItem_RecordsThePriceItChangedFromAndTo
Attributable and orderable across a same-second burst	already satisfied by shipped code (#500 actor guard, #508 Sequence) — this slice does not regress it
AuditVocabularyCoverageTests stays green	in the 2384-test suite, green

The Add payload carries salesOrderItemId, productId, quantity, unitPriceMinorUnits, listUnitPriceMinorUnits, listPriceBasis, currencyCode, currencyMinorUnit. The Update payload adds a before/after pair.

One thing this issue's body got wrong, worth recording: it says the Add path cannot know the line id. It can, but only after the save that assigns it — so the handler's tail now runs inside ExecuteInTransactionAsync, saving to get the id, then writing the audit row, then committing together. Mutation row M3 proves it: move the audit write back above the save and the payload carries Guid.Empty. That mattered because the owner supplied a fact the issue did not have — duplicate products on one order are the common case here, so identifying a line by product would not have identified it.

Deliberately NOT in this slice, now filed

• SPA: render audit detailsJson as a readable per-key table instead of a raw tooltip #745 — the SPA still renders detailsJson as a raw title tooltip (AuditPage.tsx:409), so the readable table in this issue's mockup does not exist yet. Scoped out by this issue's own comment; the work is mostly i18n, since rendering the keys as labels makes them translatable strings in three locales.
• Catalog: two audit payloads store enums as ordinals, so reordering an enum silently rewrites history #746 — two catalog audit payloads store enums as ordinals rather than names, so reordering an enum would silently rewrite stored history. Same defect class this slice guarded against for listPriceBasis; found while designing it.
• Sales: AddOrderItemHandler saves inside its transaction delegate with no rollback cleanup for tracked entities #743 — AddOrderItemHandler saves inside its transaction delegate with no DiscardChanges-style cleanup, unlike UpdateFarmSettingsHandler. Unreachable today; a trap for a future return false below the inner save.

Not reopening this issue — its stated acceptance is met, and the repo's rule is that unimplemented criteria become follow-up issues rather than bullets left open. The three above are that. Filed late and only after the owner asked, which is the miss: the design recorded #745 and #746 as "file at Phase 13b" and close-out ran without them.

[mforce]

One more, found after the fact by opening this issue's mockup rather than reading the comment about it: #747 — the payloads carry productId but no product name, while the artboard's Details column renders "Medium Eggs". A renderer resolving the id live gets today's name, so a renamed product would re-render history under a name the seller never saw — the same hazard #720's list-price snapshot exists to prevent, one field over. The house idiom already snapshots names in three sibling catalog handlers.

My design diffed this slice's block against CreateProductHandler field by field and matched its transaction shape while missing that it snapshots a name. Five review seats did not catch it either, because every one of them was briefed on code, tests or repo rules — none looked at the rendered intent.

[mforce]

Amendment — where this actually landed, and what it did not cover

Written after the fact so the next reader of this issue is not misled by its closure. The body above is left exactly as written; this comment is the correction.

What shipped, and where

#742 shipped the whole of this issue's Scope and met all four Acceptance bullets:

• adding a line records list price and unit price — AddOrderItemHandler
• editing a line records the price it changed from and to — UpdateOrderItemHandler
• rows are actor-attributed (fix(seed): seeded audit events carry "(unresolved)" as the actor, now visible on record History columns #500) and orderable across a same-second burst (fix(api): order same-instant audit events by a durable monotonic key, not a random Guid #508)
• AuditVocabularyCoverageTests stayed green, with both call sites kept literal per the guard warning above

Pinned by tests/Cluckwork.Api.IntegrationTests/SalesOrderAuditPayloadTests.cs.

One design fact worth carrying forward, because it is not obvious from the scope: the AddItem payload names the line, not just the order. SalesOrderItem.Id is assigned by EF during SaveChanges, so the audit write had to move inside ExecuteInTransactionAsync after an interleaved save. Without that, an order carrying the same product on several lines — the normal case here — has no attributable discount, because productId + quantity does not identify a line.

What this issue did NOT cover, and where that went

This issue is backend-only, deliberately. Its acceptance list contains no UI bullet, and its own closing Note says audit rows are not a reporting surface. That is accurate as written — but it means closing it left the payload invisible on every screen: the SPA's audit table rendered a Reason column that was an em dash on every sales-line row, so nothing a user could open showed the prices this issue had just started recording.

Two follow-ups close that, and neither is a criterion this issue failed — both are scope it never claimed:

• Sales: audit payloads snapshot productId but not the product name, so history re-renders under today's name #747 (→ #748, merged) — the payload stored productId and no name. The audit screen's own mockup renders product names, and resolving an id live would re-render history under today's name, so a renamed product would show the seller a name they never saw. The name and unit are now snapshotted at write time, the same rule Sales: snapshot the list price on the order line (discount foundation) #720 applies to the list price.
• SPA: render audit detailsJson as a readable per-key table instead of a raw tooltip #745 (→ #749) — the SPA now renders the payload as a readable Details column: Large Eggs $0.40 → $0.38 (list $0.45), with an explicit no-list-price wording rather than a zero.

The honest note

The gap between "this issue is complete" and "a person can see a discount" was one full slice wide, and it was the owner who caught it, not the delivery process — the process had already closed this issue and moved on. If you are writing a follow-up to this one, the transferable lesson is that a payload-only slice is not done in any sense a user can observe until something renders it, and that is worth stating in the issue rather than discovering after it closes.