Repository navigation
feat(api,web): show who created and last changed a record, inline on its own page (#494) - #503
Conversation
…its own page (#494) Admins and workers can now see, on a record's own list page, who created it and when — and who last changed it, if anyone has. Covers Flocks, Egg grades, Daily entry history, Sales and Expenses. Derived from the existing append-only audit trail, so there is no schema change and no new endpoint. A gap had to be closed first: none of the five create paths wrote an audit event at all, so "earliest event" would have resolved to the first correction. All five now write one, with matching vocabulary entries on both the server registry and the SPA's list. - AuditEventRepository.GetProvenanceAsync: two raw-SQL DISTINCT ON queries per batch of ids, earliest event as the creation and latest as the last change. Raw SQL means EF's tenant query filter is opted out of, so the hand-written AccountId predicate is the only thing scoping the read — both halves are load-bearing and covered by a test that fails if either goes. - Oversized inputs are chunked rather than refused. The egg-grade list has no pagination and grades are never deleted, so that catalog outgrows any fixed cap eventually; refusing would have 500'd the whole endpoint. - The server decides whether a change happened, comparing event IDENTITY rather than timestamps, and sends nulls when nothing has. Two distinct events can share an instant, so a timestamp comparison would hide a real edit. - Visibility inherits each endpoint's existing read access — no new authorization branch. This does newly surface a coworker's email on screens that showed none before, which was the deliberate product decision. - Records predating this get no backfill and show a blank column. Docs per the sync rule: a "Record history" glossary entry and two Help page bullets, in en/es/tl. Verified: 1547 backend tests, 1708 web tests, build clean under warnings-as-errors, typecheck clean, SPA coverage gate passing. Five mutation checks confirmed red-then-green — the create-vs-append gate, the tenant predicate, the SQL ordering, the batch chunking, and the page-level column wiring. Follow-up filed as #500: seeded audit events carry "(unresolved)" as the actor, which this change makes visible on the new columns for the first time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@codex please review. Note two things you flagged in a local pre-review round are already addressed in this branch:
Worth your attention on this round: the manual tenant predicate in the raw-SQL path, the chunking loop, and whether the new nullable |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de6ebb336b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .ToListAsync(ct); | ||
| } | ||
|
|
||
| // #494 — earliest event per id is the creation, latest is the last change. |
There was a problem hiding this comment.
For post-#494 records, this assumption is false for two of the five extended resources: re-recording or submitting a DailyEntry, and adding/updating/removing lines, confirming, or cancelling a SalesOrder, mutate the displayed record without writing an audit event in their handlers. Consequently, ordinary workflows such as creating an order and then adding its first item still return lastChangedByEmail: null, while the UI claims the record has never changed since creation; these mutations need corresponding entity audit events or provenance must come from a source that observes every mutation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct finding. Partially addressed in c91edb9 — one of the two resources is fixed, the other is an open product decision with the repo owner, so I am flagging rather than closing this.
Daily entries — done. SubmitDailyEntryHandler now writes DailyEntry.Submit into its own unit of work, so it commits with the lots and movements or not at all.
That surfaced a product question your finding does not cover. Auditing submit alone makes the common case read wrong: one person saves a draft and submits it, changing nothing in between, and the row would then claim somebody changed their work. So "last changed" is now the latest event that is neither the creation nor a submit by the creator. Written as an exclusion from the candidate set rather than as "if the last event is a self-submit, report nothing" — the latter also discards an earlier edit that the submit happens to sit on top of, which would hide a real change by a different person. The exclusion is keyed on the action, never the actor alone, so correcting a locked entry always shows even when the corrector created it.
Draft re-recording — deliberately not audited. Re-saving a draft is not a stock-altering change and the owner does not want it on the trail. The consequence is stated rather than hidden: a draft edited by a second person before submission leaves no event.
Sales orders — open, and genuinely not decided. AddOrderItem / UpdateOrderItem / RemoveOrderItem / ConfirmSale / CancelSalesOrder still write nothing, so your "create an order, add its first item, still reports null" example stands unfixed today. Orders are created as Draft, so the same question applies as for daily entries and the answer is not mechanical — it is the repo owner's call, not mine to make inside a review round. I would rather leave this thread open and honest than mark it resolved.
Verification for what did land: 1554 backend tests, 1708 web tests, typecheck clean. Three mutants confirmed red-then-green — dropping the exclusion, reducing it to an actor-only rule, and writing the wrong submit action. A fourth mutant (deleting the audit write outright) was discarded as unusable: it leaves the primary-constructor parameter unread, and warnings-as-errors turns that into a build failure, which produces no test result at all rather than a kill.
There was a problem hiding this comment.
Sales orders are now done too — dbb6240. This thread is fully addressed, with one deliberate exception stated at the end.
Rather than work from the five handlers you listed, I walked every mutation handler across all five extended resources and classified each. That found Sales was the only resource with gaps, and that it had six unaudited handlers, not five — RecordPayment was missing from the list. It turns out not to belong here (it inserts a Payment row without touching the order aggregate), but Payment.Void is audited while Payment.Create is not, which is a real asymmetry your finding did not reach.
The order state machine then decided the rest, since AddItem / UpdateItem / RemoveItem / Cancel are all Draft-only and Confirm is what allocates:
| handler | applies to | allocates? | audited |
|---|---|---|---|
| AddItem / UpdateItem / RemoveItem | Draft | no | no |
| Confirm | Draft to Confirmed | yes, FIFO | yes |
| Cancel | Draft to Cancelled | no, but terminal | yes |
| Void | Confirmed to Voided | yes | already was |
Confirm is a promotion — the action that turns a draft into the official record, alongside DailyEntry.Submit — so the self-promotion exclusion now covers both and confirming your own order reports no change. Cancel is deliberately not a promotion: it kills a draft rather than making it official, so cancelling your own order stays reportable. A mutant that adds Cancel to the promotion list pins that distinction rather than leaving it to a comment.
The write sits inside the existing FIFO transaction, so the event commits with the allocations or rolls back with them.
The deliberate exception, stated plainly: your worked example still reports null. Create an order, add its first item, and the History column still says nothing has changed — because draft line edits are not audited, exactly as re-recording a draft daily entry is not. Assembling a draft is not a change to report. That is the rule the owner chose, applied consistently; it is not an oversight, and I would rather say so than quietly leave you to re-find it.
Verification: 1560 backend tests, 1708 web tests, typecheck clean. Seven mutants confirmed red-then-green, each against a named test — no exclusion, actor-only rule, promotion list missing Confirm, promotion list over-including Cancel, and a wrong action at each of the three capture points.
There was a problem hiding this comment.
Addressed across 01d7573e (drafting writes) and d0447929 (this round).
01d7573e added the six missing mutations to the trail — DailyEntry.Update, SalesOrder.AddItem / UpdateItem / RemoveItem, plus Confirm and Cancel — so a draft edit is no longer invisible to the last-changed column.
d0447929 closes the consequence that landed with them: hiding the creator's own drafting is right while the draft is theirs alone, but it was still applied after a colleague had edited it, so the column named the colleague whose numbers had since been overwritten. A shared CTE now marks records carrying a drafting event by an actor other than the creator, and the exclusion lifts for those.
Verification: 1577 backend tests green (325 domain / 145 application / 1107 integration), 1713 web, typecheck clean. Mutation set is 27 mutants, 26 killed by a named test; the one survivor is recorded in the source with why no fixture can currently distinguish it.
| WHERE "AccountId" = {accountId} | ||
| AND "EntityType" = {entityType} | ||
| AND "EntityId" = ANY({ids}) | ||
| ORDER BY "EntityId", "OccurredAtUtc" ASC, "Id" ASC |
There was a problem hiding this comment.
When two events share OccurredAtUtc, ordering by Id does not preserve their chronology because AuditWriter assigns each event a random Guid.NewGuid(). The smaller UUID is therefore reported as the creator and the larger as the last changer regardless of which event occurred first, so the actors can be reversed; the matching same-instant test is also probabilistic for the same reason. Use a durable monotonic ordering value rather than a random UUID as the tiebreaker.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real, and worse than the wording suggests — the test I wrote for this case was itself a coin flip. Fixed in c91edb9.
You are right that Id carries no chronology: AuditWriter mints a random v4 Guid, so the tiebreaker named whichever event happened to sort first and could reverse creator and changer. The fix does not try to recover the true order — it stops depending on order for that question. Creation is now identified by its action ("Action" LIKE '%.Create'), of which there is at most one per entity, so a same-instant pair resolves the same way every time regardless of how it sorts.
Ordering still picks the last change, and a same-instant pair there is still arbitrary — but every candidate in that set genuinely is a change, so naming either is true. That is a materially weaker claim than naming a corrector as the author, and it is the one the code now makes.
Verification, including the part that embarrasses me: the original same-instant test passed 4 runs in 10 against the old ordering. I had run it once, seen green, and called it mutation-verified. It now passes 10/10. Full suite 1554 backend / 1708 web, typecheck clean.
…ng as a change Two codex findings on #503, and the product decision behind them. Creation is now identified by its ACTION rather than by its position on the trail. Position could not answer it: events sharing an OccurredAtUtc have no knowable order, because AuditWriter mints a random v4 Guid, so the Id tiebreaker named whichever event happened to sort first — reversing creator and changer at random. The test written for that case was itself a coin flip: 4 passes in 10 runs. It is now deterministic, verified 10/10. Submitting a daily entry is now audited. It mints the egg lots, so it alters stock on its own merits, and it is the only record of who made a day's numbers official. That alone would have made the common case read wrong: one person saves a draft and submits it, changing nothing in between, and the row would claim somebody changed their work. So the last change is now the latest event that is neither the creation NOR a submit by the creator. Stated as an exclusion from the candidate set, deliberately — the naive "if the last event is a self-submit, report nothing" also discards an earlier edit the submit sits on top of, hiding a real change by a different person. The exclusion is keyed on the action, never on the actor alone: correcting a locked entry always shows, including when the person correcting it is the one who created it. Verification: 1554 backend tests, 1708 web tests, typecheck clean. Three mutants confirmed red-then-green — dropping the exclusion, reducing it to an actor-only rule, and writing the wrong submit action. Refs #494.
|
@codex both findings answered in c91edb9 — one closed, one deliberately left open. "Preserve actual event order when timestamps collide" — closed. Creation no longer depends on trail position at all; it is identified by its action ( "Audit every mutation used by last-changed provenance" — half closed. Worth a fresh look, since it is where new defects would live: the last change is now the latest event that is neither the creation nor a submit by the creator. That is stated as an exclusion from the candidate set rather than as a check on the last event — the naive form would hide an earlier edit by a different person that a self-submit happens to sit on top of. It is keyed on the action, not the actor, so a self-adjust of a locked entry still shows. Verification: 1554 backend, 1708 web, typecheck clean, three mutants confirmed red-then-green. |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…d history Closes the sales-order half of codex's "audit every mutation used by last-changed provenance" on #503. A walk of every mutation handler across all five resources — rather than the list codex gave — found Sales was the only one with gaps, and that it had six unaudited handlers rather than five. The order state machine then decided which of them belong on the trail: AddItem / UpdateItem / RemoveItem Draft-only, allocate nothing -> no Confirm Draft to Confirmed, FIFO -> yes Cancel Draft to Cancelled, terminal -> yes Void already audited That is the daily-entry rule applied unchanged: draft edits are not stock movements and stay off the trail, exactly as re-recording a draft entry does. Confirm is a PROMOTION — the action that turns a draft into the official record, alongside DailyEntry.Submit — so the self-promotion exclusion now covers both, and confirming your own order reports no change. Cancel is deliberately NOT a promotion: it kills a draft rather than making it official, so cancelling your own order stays a reportable change. A mutant that adds Cancel to the promotion list pins that distinction. Confirm writes inside the existing FIFO transaction, so the event commits with the allocations or rolls back with them. Known and intended: creating an order and adding its first line still reports no change, because assembling a draft is not a change to report. Verification: 1560 backend tests, 1708 web tests, typecheck clean. Seven mutants confirmed red-then-green, each against a named test. Refs #494.
|
@codex sales-order auditing landed in dbb6240 — the half of "audit every mutation used by last-changed provenance" I left open is now closed. Worth re-checking, since it is new logic rather than a repeat of what you already passed:
One thing your finding did not reach, found by walking every handler rather than the listed five: Verification: 1560 backend, 1708 web, typecheck clean, seven mutants confirmed red-then-green. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbb62408e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| AND e."Action" NOT LIKE {createAction} | ||
| AND NOT (e."Action" LIKE ANY({promotionActions}) | ||
| AND e."ActorUserId" IS NOT DISTINCT FROM c."ActorUserId") | ||
| ORDER BY e."EntityId", e."OccurredAtUtc" DESC, e."Id" DESC |
There was a problem hiding this comment.
When two reportable changes for the same entity have an identical OccurredAtUtc—for example, an update and archive recorded within the database timestamp precision—the selected last changer still depends on Id, although AuditWriter generates random v4 GUIDs. Fresh evidence after c91edb9 is that creation is now identified correctly by action, but this new latest-change query still orders equal-timestamp changes by random ID, so it can attribute the earlier change as the latest one; use a durable monotonic ordering value for this tie-break instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
You are right, and I checked the reachability rather than arguing from the code shape. Documented in 859eca0; split out as #508.
Confirmed real. AuditWriter mints Guid.NewGuid(), a random v4, so the "Id" tiebreak carries no chronology and a same-instant pair of reportable changes resolves arbitrarily. The timestamp displayed is correct either way; the name is what can be wrong.
Where I expected you to be wrong, and was not. My first instinct was that two changes to one record cannot truly collide: the aggregate Version token serialises them, and the loser 409s having written nothing, since audit shares the transaction. That argument holds for most handlers — and then fails. RecordBirdMovementHandler writes an audit event keyed to the flocks id while only inserting a BirdMovement row; it never mutates Flock, so there is no Version bump and it does not serialise against Flock.Update. Two such requests can land in the same microsecond against the same flock. That is the concrete path, and it is why I am not arguing this one away.
Why it is not fixed here. Your remedy — a durable monotonic ordering value — does not exist on this table, and creating one is a migration. #494s stated premise is no schema change, so the correct fix is out of scope for this PR by construction rather than by preference. #508 carries it, including the two alternatives that do not work so they are not re-proposed: UUIDv7 orders only to millisecond granularity and keeps a random tail (and would mix with existing v4 rows), and ctid is not durable across VACUUM FULL.
No test pins the current outcome, deliberately. Pinning a known loss would turn "did not fix" into "spec", and the next person would read it as intended behaviour.
For completeness: ListAsync shares the tiebreak. There it is merely arbitrary rather than wrong, since paging only needs a deterministic order, but the Audit page can still show a same-instant pair in the wrong order. #508 covers both call sites.
There was a problem hiding this comment.
Still open by design, and now tracked as #508 rather than left implicit.
The ordering is only arbitrary when two reportable changes share an OccurredAtUtc, and fixing it properly needs a durable monotonic column on AuditEvents — a migration, which #494 was specified not to do. The timestamp shown is correct either way; only the name is at risk, and the path is narrow (the aggregate Version token serialises most concurrent writes, so the loser 409s having written nothing; RecordBirdMovement is the exception, and it writes against the flock without touching Flock.Version).
The reasoning and the reachable path are both in the source comment at the query that has it, so the next person meets it where the risk is. There is deliberately no test pinning the current outcome — that would promote a known loss into a specification.
… has it Codex's third round on PR #503 is correct: the last-change tiebreak still orders by "Id", and AuditWriter mints a random v4 Guid, so two reportable changes sharing an OccurredAtUtc resolve arbitrarily and the wrong actor can be named. The timestamp shown is right either way; only the name is at risk. Reachable, not theoretical, and the reason matters. Most concurrent changes to one record cannot collide: the aggregate Version token serialises them and the loser 409s having written nothing. RecordBirdMovement escapes that argument — it writes an audit event against the FLOCK's id while only inserting a movement row, never touching Flock.Version, so it does not serialise against Flock.Update. Fixing it properly needs a durable monotonic column on AuditEvents, which is a migration — the one thing #494 was specified not to do. Tracked in #508 rather than smuggled into a PR whose premise was no schema change. No test pins the current arbitrary outcome. Pinning a known loss would turn "did not fix" into "spec". Refs #494, #508.
|
@codex the same-instant ordering finding is accepted, not fixed here — 859eca0 documents it and #508 carries the fix. Short version: you are right that The remedy you propose needs a durable monotonic column on Everything else on this PR is landed and verified: 1560 backend tests, 1708 web, typecheck clean, seven mutants confirmed red-then-green. Two things I would genuinely value a fresh look at, both new since your last clean pass:
|
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…ated Local adversarial review of PR #503 found, by mutation rather than argument, that three predicates on the new last-change query were unguarded. All three survived deletion against the entire suite: outer last-change query loses AccountId 28/28 still pass creator CTE loses AccountId 28/28 still pass creator CTE loses the ".Create" filter 28/28 still pass The cause is that Provenance_IsScopedToTheTenant gives each account a DISTINCT entityId. The foreign row is therefore excluded before those predicates are ever consulted, so the test that the file's own comment cited as holding them in place did no such thing. That comment was the defect: a guard that reads as safety while enforcing nothing. Two tests now hold them, both mutation-verified red-then-green: Provenance_LastChange_IsScopedToTheTenant shares ONE entityId across two accounts, with the rival's Create dated EARLIER and its Adjust LATER, so dropping either AccountId predicate changes the reported answer. Provenance_TheCreatorForTheExclusion_IsFoundByActionNotByPosition places a correction before the creation, the only arrangement where position and action disagree. Neither is a live leak: entity ids are unique across accounts and callers pass their own already-filtered ids. The fix is to the guard, not to a leak. Also from the same pass: - The promotion set is now the exact constants rather than a "%.Submit" / "%.Confirm" suffix pattern. Creation rightly keeps its wildcard, since any ".Create" IS a creation, but promotion is a closed set and a pattern would silently swallow a future action that merely ends the same way. - The EntityProvenance doc comment described the position-based algorithm this branch abandoned, and claimed LastChanged is null only when both ends of the trail are the same event. Both false since c91edb9. - The IS NOT DISTINCT FROM justification claimed to prevent something that cannot be observed, since such records are dropped by the keying first. The operator stays; the claim is now honest about not being load-bearing. - Three end-to-end tests discarded the status of the call they depend on. A submit or add-item that started 422ing would have left them green and empty. Verification: 1562 backend tests, and 10 mutants confirmed red-then-green, each against a named test. Refs #494.
|
@codex 6c9318f — a local adversarial pass found, by mutation rather than argument, that three predicates on the new last-change query were unguarded. All three survived deletion against the entire suite:
Cause: Two new tests hold them now, both mutation-verified: one shares a single Also: the promotion set is now the exact constants instead of a 10 mutants confirmed red-then-green, each against a named test. 1562 backend tests green. Three behavioural consequences surfaced by that pass are with the repo owner as product decisions, not defects — I will not close them silently: a co-author who only edits a draft leaves no trace when the creator promotes; the promotion timestamp (when stock is actually minted) is no longer visible on the records own page; and a pre-#494 record with real changes renders fully blank rather than showing the change we do have attribution for. |
|
The adversarial additions at
I found no additional defect requiring a code change. The three surfaced behavioral questions are appropriately left as explicit product decisions rather than silently encoded here. Because the working tree remains clean, I did not create a commit or a follow-up PR. Testing
|
…ed it Excluding a self-promotion from "last changed by" removed the noise but also removed the only visible trace of WHEN the record became official — and that instant is when stock is minted or allocated. A daily entry drafted Monday and submitted Friday read as "created Monday" and nothing else; Friday existed only in the admin audit log, because DailyEntry stores no submission timestamp of its own, just LockedAtUtc. The same applied to a sales order's confirm, which is the FIFO allocation moment. Raised by the local adversarial review of #503. EntityProvenance now carries MadeOfficialAtUtc, reported whoever performed the promotion, so the suppression costs a name and not a time. It is null for a draft still awaiting promotion and for resources with no promotion step at all. Read by a third query rather than folded into the two above it. Those two are the ones under mutation guard and are already dense; this is a plain indexed lookup on the same (AccountId, EntityId) index, and at most one promotion exists per entity since neither DailyEntry nor SalesOrder has a path back to Draft. Only the two resources that have a promotion expose the field. On the SPA the caller declares which concept applies — "submitted" for daily entries, "confirmed" for sales orders — so flocks, egg grades and expenses cannot render the line at all, and each gets a full translated phrase rather than a verb composed with a date, which would put word order beyond a translator's reach. Its tenant predicate got its own guard immediately, rather than repeating the mistake the previous commit fixed: two predicates had sat unguarded because every tenant test used distinct ids per account. Verification: 1566 backend tests, 1712 web tests, typecheck clean. 12 mutants confirmed red-then-green, each against a named test. Refs #494.
|
@codex baba9a2 adds the promotion instant to the record-history column. The gap: excluding a self-promotion from "last changed by" removed the noise but also removed the only visible trace of when the record became official — the moment stock is minted or allocated. A daily entry drafted Monday and submitted Friday read as "created Monday" and nothing else, because New surface worth a fresh look:
12 mutants confirmed red-then-green, each against a named test. 1566 backend, 1712 web, typecheck clean. Two findings from that same local pass remain open as product decisions, deliberately not fixed: a co-author who only edits a draft leaves no trace when the creator promotes (draft edits are unaudited by design), and a pre-#494 record with real changes renders fully blank because the result is keyed off the creation event. |
|
No additional defect found in
The acknowledged pre-#494 blank-record behavior remains unchanged: The working tree remained clean, so I made no commit and did not create a follow-up PR. Testing
|
…redates #494 Two findings from the local adversarial review, both owner-decided. FIRST — a co-author on a shared draft was invisible. ana records an entry, bo re-records it changing 100 eggs to 500, ana submits. The 500 came from bo, and the column read "created by ana, never changed": ana was credited with bo's numbers. Same shape for an order whose lines somebody else assembled. Draft edits are now on the trail — DailyEntry.Update and the three SalesOrder line actions. They move no stock, which is why they were left off, but they are the only thing binding a person to the numbers. The noise that omission was avoiding stays avoided, because the exclusion widened with it: the last change is now the latest event that is neither the creation nor the creator's own DRAFTING of it, drafting being draft edits plus the promotion that ends them. Editing your own draft is part of writing it and stays hidden; a different person's edit surfaces. Keyed on the actor, so the same event is hidden for one person and shown for another. SECOND — a record whose creation predates #494 rendered completely blank even when it had a change with real attribution. The result was keyed off the creation event, so a flock archived by somebody yesterday showed nothing at all. Refusing to invent a creator and discarding a change we can prove are separate decisions; only the first was intended. The two halves are now independent and CreatedBy* is nullable. DailyEntry_ReRecordedOnTheSameKey_WritesOnlyOneCreateEvent asserted the re-record left NO event. Its real invariant — never a second CREATION, which would give an entry two candidate authors — is intact and now asserted directly, rather than as a side effect of a total count. Verification: 1568 backend tests, 1713 web tests, typecheck clean. 16 mutants confirmed red-then-green, each against a named test. Three patterns in the mutation script had gone stale against the reshaped code and reported themselves as inert rather than passing silently; they were repaired and the whole set re-run. Refs #494.
|
@codex 01d7573 closes the two behavioural findings I had deliberately left open. Both were owner decisions, now made — this is the round where a fresh look is worth most, because the rule itself changed rather than an implementation detail. Draft edits are now audited — The exclusion widened with them, so the noise stays suppressed. Last change is now the latest event that is neither the creation nor the creators own drafting of it — drafting being draft edits plus the promotion that ends them. Editing your own draft is part of writing it and stays hidden; a different persons edit surfaces. Keyed on the actor, so the same action is hidden for one person and shown for another — that asymmetry is the part I would attack first. A record whose creation predates #494 no longer renders blank when it has a change with real attribution. The result was keyed off the creation event, so a flock archived by somebody yesterday showed nothing at all. Refusing to invent a creator and discarding a provable change were being made as one decision; only the first was intended. Also in range, from the previous rounds: Worth checking hardest:
Verification: 1568 backend, 1713 web, typecheck clean, 16 mutants confirmed red-then-green. Three of those mutant patterns had gone stale against the reshaped code and reported themselves inert rather than passing silently; they were repaired and the whole set re-run. |
Summary
Testing
|
… predicate Local adversarial review of #503, round two. Three findings, each proven by mutation against the full 1104-test integration suite rather than argued. 1. SalesOrder.UpdateItem and SalesOrder.RemoveItem could both be deleted from the drafting set with the entire suite still green, while their two siblings were held. Two of four covered is worse than none: the pattern reads as complete. ana adding a line, correcting its quantity and confirming — the most common order-editing path — would have reported "last changed by ana", the exact noise this feature exists to remove. 2. Three of the four EntityType predicates likewise survived deletion. Same cause as the tenant miss this file already documents: the obvious test gives each type a distinct entityId, so the later predicates never run in it. The most visible consequence was a FLOCK reporting MadeOfficialAtUtc — an entity with no promotion step claiming it had been made official. 3. The comment asserting IS NOT DISTINCT FROM was "NOT currently load-bearing" became false in the same branch that wrote it. Replacing the creation-keyed result with a three-way union means a legacy record whose only event is a drafting action now reaches the output; under a plain "=", Guid = NULL yields SQL NULL and WHERE drops the row, losing a real attributable change. That sentence would have licensed the next person to simplify the line. Also: DailyEntry_ReRecordedOnTheSameKey compared an ordered sequence against an unordered query, which Postgres never promised. Verification: 1572 backend tests, 1713 web tests, typecheck clean. 22 mutants confirmed red-then-green, each against a named test. Refs #494.
…ed the draft The History column hid every edit the creator made to their own draft. That is right while the draft is theirs alone — writing a record and submitting it is one act, not a change — but it was still applied after a colleague had edited it. The column then named the colleague, whose numbers had since been overwritten, and dated the record to an edit that no longer existed in it, while the numbers that actually went into stock were the creator's. The exclusion now lifts for a record somebody else drafted: a `shared` CTE marks entities carrying a drafting event by an actor other than the creator, and the creator's own edits are only suppressed when that flag is absent. Their promotion stays suppressed either way — it has its own "Submitted" line, and repeating it as the last change would say that making a record official changed it. The last-change query now reads its creator and its shared set from ONE `scoped` CTE rather than restating the account/type/id triple per CTE. That triple was twice shipped mostly unguarded, both times because a duplicate predicate does no work in a test that gives each account a distinct entity id; stating it once removes the place that failure hides. 27 mutants, 26 killed by a named test. The survivor is recorded in the source: `shared` counts drafting by somebody else rather than any event by somebody else, and no fixture can currently tell those apart, because every foreign non-drafting action on these aggregates only becomes reachable once drafting has ended.
|
@codex two commits since your last look, and the second changes a product rule rather than an implementation detail — that is where a fresh read is worth most.
The same commit restructures the last-change query onto a single 1577 backend / 1713 web green, typecheck clean, 26 of 27 mutants killed by a named test. The survivor is documented in the source: Worth your attention: whether the |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Closes #494.
Admins and workers can now see, on a record's own list page, who created it and when — and who last changed it, if anyone has. Covers Flocks, Egg grades, Daily entry history, Sales and Expenses.
Derived entirely from the existing append-only audit trail: no schema change, no migration, no new endpoint.
The gap that had to be closed first
None of the five create paths wrote an audit event at all, so "earliest event = created" would have silently resolved to the first correction. All five now write one, with matching vocabulary entries on both the server registry and the SPA's list, which an existing coverage test holds in lockstep.
Design notes
Tenant scoping is manual and load-bearing.
GetProvenanceAsyncruns two raw-SQLDISTINCT ONqueries, because PostgresDISTINCT ONhas no LINQ translation. Raw SQL opts out of EF's tenant query filter, so the hand-writtenAccountIdpredicate is the only thing scoping the read. A test fails if either half is removed.Oversized batches are chunked, not refused. The egg-grade list has no pagination and grades are never deleted, so that catalog outgrows any fixed cap eventually. An earlier revision threw above 500 ids, which would have 500'd the whole endpoint the daily-capture flow depends on.
The server decides whether a change happened, and sends nulls when nothing has. It compares event identity, not timestamps — two distinct events can share an instant, and a timestamp comparison hides a real edit. The client must not re-derive this.
Visibility inherits each endpoint's existing read access. No new authorization branch. This does newly surface a coworker's email address on screens that previously showed none — the deliberate product decision recorded in the issue, not an oversight.
No backfill. Records created before this show a blank column, as the issue specifies.
Verification
Non-CI callers checked, per the write-contract rule
"(unresolved)"as the actor, which this change makes visible for the first time. Filed separately as fix(seed): seeded audit events carry "(unresolved)" as the actor, now visible on record History columns #500 rather than widening this PR.graphify-out/is deliberately not included, per the reasoning in #497.