From de6ebb336b3b2091589a408c60f48a5cb8567e2a Mon Sep 17 00:00:00 2001 From: mforce <> Date: Mon, 10 Aug 2026 15:11:26 -0700 Subject: [PATCH 1/9] feat(api,web): show who created and last changed a record, inline on its own page (#494) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- specs/product/GLOSSARY.md | 16 + .../DailyEntries/DailyEntryEndpoints.cs | 24 +- .../Endpoints/EggGrades/EggGradeEndpoints.cs | 28 +- .../Endpoints/Expenses/ExpenseEndpoints.cs | 32 +- .../Endpoints/Flocks/FlockEndpoints.cs | 24 +- .../Endpoints/Sales/SaleEndpoints.cs | 26 +- .../Common/AuditActions.cs | 5 + .../Features/Audit/EntityProvenance.cs | 13 + .../Features/Audit/IAuditEventRepository.cs | 11 + .../RecordDailyEntryHandler.cs | 4 + .../CreateEggGrade/CreateEggGradeHandler.cs | 2 + .../CreateExpense/CreateExpenseHandler.cs | 3 + .../Flocks/CreateFlock/CreateFlockHandler.cs | 2 + .../CreateSalesOrderHandler.cs | 3 + .../Repositories/AuditEventRepository.cs | 75 ++++- .../AuditProvenanceTests.cs | 312 ++++++++++++++++++ .../AuditTests.cs | 12 +- .../CustomerAndOrderTests.cs | 22 ++ .../EggGradeManagementTests.cs | 20 ++ .../ExpensesTests.cs | 20 ++ .../FlockManagementTests.cs | 15 + .../SubmitDailyEntryTests.cs | 50 +++ web/src/api/cluckwork.ts | 29 +- web/src/components/ProvenanceCell.test.tsx | 96 ++++++ web/src/components/ProvenanceCell.tsx | 45 +++ web/src/i18n/en.ts | 17 + web/src/i18n/enums.ts | 7 + web/src/i18n/es.ts | 17 + web/src/i18n/tl.ts | 16 + web/src/routes/DailyEntryPage.test.tsx | 12 +- web/src/routes/Dashboard.test.tsx | 3 + web/src/routes/ExpensesPage.test.tsx | 27 +- web/src/routes/ExpensesPage.tsx | 5 +- web/src/routes/FlocksPage.test.tsx | 28 ++ web/src/routes/FlocksPage.tsx | 5 +- web/src/routes/GradesPage.test.tsx | 35 +- web/src/routes/GradesPage.tsx | 3 + web/src/routes/HelpPage.tsx | 2 + web/src/routes/HistoryPage.test.tsx | 30 +- web/src/routes/HistoryPage.tsx | 3 + web/src/routes/InventoryPage.test.tsx | 3 +- web/src/routes/ProductsPage.test.tsx | 6 +- web/src/routes/SalesPage.test.tsx | 28 +- web/src/routes/SalesPage.tsx | 4 +- web/src/routes/UsersPage.test.tsx | 3 +- web/src/routes/WaterPage.test.tsx | 2 + web/src/test/fixtures.ts | 23 +- 47 files changed, 1104 insertions(+), 64 deletions(-) create mode 100644 src/Cluckwork.Application/Features/Audit/EntityProvenance.cs create mode 100644 tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs create mode 100644 web/src/components/ProvenanceCell.test.tsx create mode 100644 web/src/components/ProvenanceCell.tsx diff --git a/specs/product/GLOSSARY.md b/specs/product/GLOSSARY.md index 746e87058..d329aedfd 100644 --- a/specs/product/GLOSSARY.md +++ b/specs/product/GLOSSARY.md @@ -416,6 +416,22 @@ update or delete surface anywhere. Admin-only viewer at /audit. The entity-local snapshots (`AdjustedFromJson` etc.) remain — they are the record's own history; the audit log is the cross-cutting trail. +**Record history (#494)** — the "Created by … / Last changed by …" column on +the Flocks, Egg grades, Daily entry history, Sales and Expenses tables. Not a +stored field: it is **derived from the audit log**, taking that record's +earliest event as its creation and its latest as its last change. Creation +itself became an audited action for these five in the same change (`Flock.Create` +and siblings) — before that only corrections were on the trail. Two consequences +follow from being derived rather than stored. A record created **before #494** +has no creation event and is **never backfilled**, so its history column is +blank. And who can see it is exactly who could already read the record — it adds +no gate of its own, so a worker reading their own daily entry also sees which +manager corrected it. The actor is shown as the plain **email snapshot** the +audit log already keeps, deliberately not a live-joined name: a later rename or +disable must not rewrite what old history displays. Distinct from the audit +log's own admin-only viewer at /audit, which is the full cross-cutting trail — +this is the one-line summary on the record's own page. + **Hen-day % (#91)** — eggs collected ÷ hen-days × 100 (spec §19.3). A hen-day is one bird alive for one day; the day's bird count comes from the bird ledger (placements + movements). The production report shows it per diff --git a/src/Cluckwork.Api/Endpoints/DailyEntries/DailyEntryEndpoints.cs b/src/Cluckwork.Api/Endpoints/DailyEntries/DailyEntryEndpoints.cs index 815f7f6c9..8e7169695 100644 --- a/src/Cluckwork.Api/Endpoints/DailyEntries/DailyEntryEndpoints.cs +++ b/src/Cluckwork.Api/Endpoints/DailyEntries/DailyEntryEndpoints.cs @@ -54,16 +54,21 @@ public static RouteGroupBuilder MapDailyEntryEndpoints(this RouteGroupBuilder gr private static async Task GetDailyEntry( Guid id, Cluckwork.Application.Features.DailyEntries.IDailyEntryRepository entries, + Cluckwork.Application.Features.Audit.IAuditEventRepository audit, TenantContext tenant, CancellationToken ct) { if (!tenant.IsResolved) return Results.Unauthorized(); var entry = await entries.GetReadOnlyAsync(id, ct); - return entry is null ? Results.NotFound() : Results.Ok(ToResponse(entry)); + if (entry is null) return Results.NotFound(); + var provenance = await audit.GetProvenanceAsync( + nameof(Cluckwork.Domain.Eggs.DailyEntry), [id], ct); + return Results.Ok(ToResponse(entry, provenance.GetValueOrDefault(id))); } private static async Task ListDailyEntries( Cluckwork.Application.Features.DailyEntries.IDailyEntryRepository entries, + Cluckwork.Application.Features.Audit.IAuditEventRepository audit, TenantContext tenant, CancellationToken ct, Guid? flockId = null, @@ -78,10 +83,14 @@ private static async Task ListDailyEntries( var skip = Math.Max(offset ?? 0, 0); var list = await entries.ListAsync(flockId, from, to, take, skip, ct); - return Results.Ok(list.Select(ToResponse)); + var provenance = await audit.GetProvenanceAsync( + nameof(Cluckwork.Domain.Eggs.DailyEntry), list.Select(e => e.Id).ToList(), ct); + return Results.Ok(list.Select(e => ToResponse(e, provenance.GetValueOrDefault(e.Id)))); } - private static DailyEntryResponse ToResponse(Cluckwork.Domain.Eggs.DailyEntry e) => new( + private static DailyEntryResponse ToResponse( + Cluckwork.Domain.Eggs.DailyEntry e, + Cluckwork.Application.Features.Audit.EntityProvenance? p) => new( e.Id, e.FarmId, e.HouseId, e.FlockId, e.Date, e.Status.ToString(), e.TotalEggs, e.CrackedEggs, e.DirtyEggs, e.DiscardedEggs, e.MortalityCount, // #396 — which grade each condition counter resolved to when this entry @@ -92,7 +101,8 @@ private static async Task ListDailyEntries( e.Grades.Select(g => new GradeLineResponse(g.EggGradeId, g.Quantity)).ToList(), e.Version, e.AdjustReason, e.VoidReason, e.LockedAtUtc, // The audit snapshot is stored as JSON; embed it as an object, not a string. - e.AdjustedFromJson is null ? null : JsonSerializer.Deserialize(e.AdjustedFromJson)); + e.AdjustedFromJson is null ? null : JsonSerializer.Deserialize(e.AdjustedFromJson), + p?.CreatedByEmail, p?.CreatedAtUtc, p?.LastChangedByEmail, p?.LastChangedAtUtc); private static async Task AdjustDailyEntry( Guid id, @@ -230,7 +240,11 @@ public sealed record DailyEntryResponse( Guid? CrackedGradeId, Guid? DirtyGradeId, IReadOnlyList Grades, int Version, string? AdjustReason, string? VoidReason, DateTimeOffset? LockedAtUtc, - JsonElement? AdjustedFrom); + JsonElement? AdjustedFrom, + // #494 provenance, derived from the audit trail: null together for a + // record created before that shipped (no backfill). + string? CreatedByEmail, DateTimeOffset? CreatedAtUtc, + string? LastChangedByEmail, DateTimeOffset? LastChangedAtUtc); public sealed record GradeLineResponse(Guid EggGradeId, int Quantity); diff --git a/src/Cluckwork.Api/Endpoints/EggGrades/EggGradeEndpoints.cs b/src/Cluckwork.Api/Endpoints/EggGrades/EggGradeEndpoints.cs index 3165d021b..b40ca8202 100644 --- a/src/Cluckwork.Api/Endpoints/EggGrades/EggGradeEndpoints.cs +++ b/src/Cluckwork.Api/Endpoints/EggGrades/EggGradeEndpoints.cs @@ -1,6 +1,7 @@ namespace Cluckwork.Api.Endpoints.EggGrades; using Cluckwork.Api.Validation; +using Cluckwork.Application.Features.Audit; using Cluckwork.Application.Features.EggGrades; using Cluckwork.Application.Features.EggGrades.CreateEggGrade; using Cluckwork.Application.Features.EggGrades.SetEggGradeActive; @@ -47,22 +48,30 @@ public static RouteGroupBuilder MapEggGradeEndpoints(this RouteGroupBuilder grou } private static async Task ListEggGrades( - IEggGradeRepository grades, TenantContext tenant, CancellationToken ct, + IEggGradeRepository grades, IAuditEventRepository audit, + TenantContext tenant, CancellationToken ct, bool includeInactive = false) { if (!tenant.IsResolved) return Results.Unauthorized(); var list = includeInactive ? await grades.ListAllAsync(ct) : await grades.ListActiveAsync(farmId: null, ct); - return Results.Ok(list.Select(ToResponse)); + // This list is unpaginated, so the batch is however many grades the + // farm has; the lookup chunks internally rather than refusing a big one. + var provenance = await audit.GetProvenanceAsync( + nameof(EggGrade), list.Select(g => g.Id).ToList(), ct); + return Results.Ok(list.Select(g => ToResponse(g, provenance.GetValueOrDefault(g.Id)))); } private static async Task GetEggGrade( - Guid id, IEggGradeRepository grades, TenantContext tenant, CancellationToken ct) + Guid id, IEggGradeRepository grades, IAuditEventRepository audit, + TenantContext tenant, CancellationToken ct) { if (!tenant.IsResolved) return Results.Unauthorized(); var grade = await grades.GetByIdAsync(id, ct); - return grade is null ? Results.NotFound() : Results.Ok(ToResponse(grade)); + if (grade is null) return Results.NotFound(); + var provenance = await audit.GetProvenanceAsync(nameof(EggGrade), [id], ct); + return Results.Ok(ToResponse(grade, provenance.GetValueOrDefault(id))); } private static async Task CreateEggGrade( @@ -126,9 +135,10 @@ private static IResult MapFailure(Cluckwork.Domain.Common.Error error) : Results.Problem(error.Description, statusCode: 422, title: error.Code); } - private static EggGradeResponse ToResponse(EggGrade g) => + private static EggGradeResponse ToResponse(EggGrade g, EntityProvenance? p) => new(g.Id, g.FarmId, g.Name, g.GradeType.ToString(), g.SortOrder, g.IsSaleable, - g.DailyEntryKind.ToString(), g.Active); + g.DailyEntryKind.ToString(), g.Active, + p?.CreatedByEmail, p?.CreatedAtUtc, p?.LastChangedByEmail, p?.LastChangedAtUtc); } // FarmId included from day one — grades are farm-scoped (spec §9.1) and a @@ -141,7 +151,11 @@ public sealed record EggGradeResponse( // lets the Grading pane leave the two counter-fed grades out; the server // refuses them regardless, so this is the affordance, not the enforcement. string DailyEntryKind, - bool Active); + bool Active, + // #494 provenance, derived from the audit trail: null together for a + // record created before that shipped (no backfill). + string? CreatedByEmail, DateTimeOffset? CreatedAtUtc, + string? LastChangedByEmail, DateTimeOffset? LastChangedAtUtc); public sealed record CreateEggGradeRequest( string Name, string GradeType, int SortOrder, bool IsSaleable); diff --git a/src/Cluckwork.Api/Endpoints/Expenses/ExpenseEndpoints.cs b/src/Cluckwork.Api/Endpoints/Expenses/ExpenseEndpoints.cs index 93627b140..2e518d087 100644 --- a/src/Cluckwork.Api/Endpoints/Expenses/ExpenseEndpoints.cs +++ b/src/Cluckwork.Api/Endpoints/Expenses/ExpenseEndpoints.cs @@ -2,6 +2,7 @@ namespace Cluckwork.Api.Endpoints.Expenses; using Cluckwork.Api.Validation; using Cluckwork.Application.Features.Accounts; +using Cluckwork.Application.Features.Audit; using Cluckwork.Application.Features.Expenses; using Cluckwork.Application.Features.Expenses.AdjustExpense; using Cluckwork.Application.Features.Expenses.CreateExpense; @@ -114,6 +115,7 @@ private static async Task UpdateCategory( private static async Task ListExpenses( IExpenseRepository expenses, IAccountRepository accounts, + IAuditEventRepository audit, TenantContext tenant, CancellationToken ct, DateOnly? from = null, @@ -132,20 +134,25 @@ private static async Task ListExpenses( // Single-farm MVP: every expense carries the account currency, so one // label fits the total. Multi-currency totals arrive with multi-farm. var account = await accounts.GetCurrentAsync(ct); + var provenance = await audit.GetProvenanceAsync( + nameof(Expense), list.Select(e => e.Id).ToList(), ct); return Results.Ok(new ExpenseListResponse( - list.Select(ToResponse).ToList(), + list.Select(e => ToResponse(e, provenance.GetValueOrDefault(e.Id))).ToList(), total, account?.DefaultCurrencyCode ?? "", account?.DefaultCurrencyMinorUnit ?? 2)); } private static async Task GetExpense( - Guid id, IExpenseRepository expenses, TenantContext tenant, CancellationToken ct) + Guid id, IExpenseRepository expenses, IAuditEventRepository audit, + TenantContext tenant, CancellationToken ct) { if (!tenant.IsResolved) return Results.Unauthorized(); var expense = await expenses.GetByIdAsync(id, ct); - return expense is null ? Results.NotFound() : Results.Ok(ToResponse(expense)); + if (expense is null) return Results.NotFound(); + var provenance = await audit.GetProvenanceAsync(nameof(Expense), [id], ct); + return Results.Ok(ToResponse(expense, provenance.GetValueOrDefault(id))); } private static async Task CreateExpense( @@ -177,6 +184,7 @@ private static async Task AdjustExpense( AdjustExpenseHandler handler, IValidator validator, IExpenseRepository expenses, + IAuditEventRepository audit, TenantContext tenant, CancellationToken ct) { @@ -194,9 +202,12 @@ private static async Task AdjustExpense( if (result.IsFailure) return MapFailure(result.Error); // The corrected row (fresh version) so the client can rebind its edit - // state without a second round trip. + // state without a second round trip. The adjust just wrote its own + // audit event, so this read reports the correction as the last change. var updated = await expenses.GetByIdAsync(id, ct); - return updated is null ? Results.NotFound() : Results.Ok(ToResponse(updated)); + if (updated is null) return Results.NotFound(); + var provenance = await audit.GetProvenanceAsync(nameof(Expense), [id], ct); + return Results.Ok(ToResponse(updated, provenance.GetValueOrDefault(id))); } private static IResult MapFailure(Cluckwork.Domain.Common.Error error) @@ -212,10 +223,11 @@ private static IResult MapFailure(Cluckwork.Domain.Common.Error error) private static ExpenseCategoryResponse ToResponse(ExpenseCategory c) => new(c.Id, c.FarmId, c.Name, c.Active); - private static ExpenseResponse ToResponse(Expense e) => + private static ExpenseResponse ToResponse(Expense e, EntityProvenance? p) => new(e.Id, e.FarmId, e.ExpenseCategoryId, e.Date, e.Description, e.AmountMinorUnits, e.CurrencyCode, e.CurrencyMinorUnit, - e.FlockId, e.Note, e.Version); + e.FlockId, e.Note, e.Version, + p?.CreatedByEmail, p?.CreatedAtUtc, p?.LastChangedByEmail, p?.LastChangedAtUtc); } public sealed record ExpenseCategoryResponse(Guid Id, Guid FarmId, string Name, bool Active); @@ -225,7 +237,11 @@ public sealed record UpdateExpenseCategoryRequest(string Name, bool Active); public sealed record ExpenseResponse( Guid Id, Guid FarmId, Guid ExpenseCategoryId, DateOnly Date, string Description, long AmountMinorUnits, string CurrencyCode, int CurrencyMinorUnit, - Guid? FlockId, string? Note, int Version); + Guid? FlockId, string? Note, int Version, + // #494 provenance, derived from the audit trail: null together for a + // record created before that shipped (no backfill). + string? CreatedByEmail, DateTimeOffset? CreatedAtUtc, + string? LastChangedByEmail, DateTimeOffset? LastChangedAtUtc); public sealed record ExpenseListResponse( List Items, long TotalMinorUnits, string CurrencyCode, int CurrencyMinorUnit); diff --git a/src/Cluckwork.Api/Endpoints/Flocks/FlockEndpoints.cs b/src/Cluckwork.Api/Endpoints/Flocks/FlockEndpoints.cs index 7f9c201ef..66527d0be 100644 --- a/src/Cluckwork.Api/Endpoints/Flocks/FlockEndpoints.cs +++ b/src/Cluckwork.Api/Endpoints/Flocks/FlockEndpoints.cs @@ -1,6 +1,7 @@ namespace Cluckwork.Api.Endpoints.Flocks; using Cluckwork.Api.Validation; +using Cluckwork.Application.Features.Audit; using Cluckwork.Application.Features.Flocks; using Cluckwork.Application.Features.Flocks.ArchiveFlock; using Cluckwork.Application.Features.Flocks.CreateFlock; @@ -92,7 +93,8 @@ private static async Task CreateFlock( } private static async Task ListFlocks( - IFlockRepository flocks, IBirdMovementRepository movements, TenantContext tenant, + IFlockRepository flocks, IBirdMovementRepository movements, IAuditEventRepository audit, + TenantContext tenant, CancellationToken ct, int? limit = null, int? offset = null, bool includeArchived = false) { if (!tenant.IsResolved) return Results.Unauthorized(); @@ -102,19 +104,22 @@ private static async Task ListFlocks( var list = await flocks.ListAsync(take, skip, includeArchived, ct); var removed = await movements.RemovedByFlockAsync(ct); + var provenance = await audit.GetProvenanceAsync( + nameof(Flock), list.Select(f => f.Id).ToList(), ct); return Results.Ok(list.Select(f => - ToResponse(f, removed.GetValueOrDefault(f.Id, 0)))); + ToResponse(f, removed.GetValueOrDefault(f.Id, 0), provenance.GetValueOrDefault(f.Id)))); } private static async Task GetFlock( Guid id, IFlockRepository flocks, IBirdMovementRepository movements, - TenantContext tenant, CancellationToken ct) + IAuditEventRepository audit, TenantContext tenant, CancellationToken ct) { if (!tenant.IsResolved) return Results.Unauthorized(); var flock = await flocks.GetByIdAsync(id, ct); if (flock is null) return Results.NotFound(); var removed = await movements.RemovedForFlockAsync(id, ct); - return Results.Ok(ToResponse(flock, removed)); + var provenance = await audit.GetProvenanceAsync(nameof(Flock), [id], ct); + return Results.Ok(ToResponse(flock, removed, provenance.GetValueOrDefault(id))); } private static async Task ReactivateFlock( @@ -212,9 +217,10 @@ private static IResult MapFailure(Cluckwork.Domain.Common.Error error) : Results.Problem(error.Description, statusCode: 422, title: error.Code); } - private static FlockResponse ToResponse(Flock f, long removed) => new( + private static FlockResponse ToResponse(Flock f, long removed, EntityProvenance? p) => new( f.Id, f.FarmId, f.HouseId, f.Name, f.Breed, - f.PlacementDate, f.InitialCount, f.InitialCount - removed, f.Status.ToString()); + f.PlacementDate, f.InitialCount, f.InitialCount - removed, f.Status.ToString(), + p?.CreatedByEmail, p?.CreatedAtUtc, p?.LastChangedByEmail, p?.LastChangedAtUtc); } public sealed record CreateFlockRequest( @@ -230,9 +236,13 @@ public sealed record UpdateFlockRequest( int InitialCount); // CurrentBirds = InitialCount − Σ movement quantities (bird ledger, #54). +// The four trailing fields are #494 provenance, derived from the audit trail: +// null together for a record created before that shipped (no backfill). public sealed record FlockResponse( Guid Id, Guid FarmId, Guid HouseId, string Name, string Breed, - DateOnly PlacementDate, int InitialCount, long CurrentBirds, string Status); + DateOnly PlacementDate, int InitialCount, long CurrentBirds, string Status, + string? CreatedByEmail, DateTimeOffset? CreatedAtUtc, + string? LastChangedByEmail, DateTimeOffset? LastChangedAtUtc); public sealed record RecordBirdMovementRequest( DateOnly Date, string Type, int Quantity, string? Note = null); diff --git a/src/Cluckwork.Api/Endpoints/Sales/SaleEndpoints.cs b/src/Cluckwork.Api/Endpoints/Sales/SaleEndpoints.cs index eb0039617..c56604233 100644 --- a/src/Cluckwork.Api/Endpoints/Sales/SaleEndpoints.cs +++ b/src/Cluckwork.Api/Endpoints/Sales/SaleEndpoints.cs @@ -1,6 +1,7 @@ namespace Cluckwork.Api.Endpoints.Sales; using Cluckwork.Api.Validation; +using Cluckwork.Application.Features.Audit; using Cluckwork.Application.Features.Sales; using Cluckwork.Application.Features.Sales.AddOrderItem; using Cluckwork.Application.Features.Sales.CancelSalesOrder; @@ -182,15 +183,19 @@ private static async Task CancelSalesOrder( } private static async Task GetSalesOrder( - Guid id, ISalesOrderRepository orders, TenantContext tenant, CancellationToken ct) + Guid id, ISalesOrderRepository orders, IAuditEventRepository audit, + TenantContext tenant, CancellationToken ct) { if (!tenant.IsResolved) return Results.Unauthorized(); var order = await orders.GetReadOnlyAsync(id, ct); - return order is null ? Results.NotFound() : Results.Ok(ToResponse(order)); + if (order is null) return Results.NotFound(); + var provenance = await audit.GetProvenanceAsync(nameof(SalesOrder), [id], ct); + return Results.Ok(ToResponse(order, provenance.GetValueOrDefault(id))); } private static async Task ListSalesOrders( - ISalesOrderRepository orders, TenantContext tenant, CancellationToken ct, + ISalesOrderRepository orders, IAuditEventRepository audit, + TenantContext tenant, CancellationToken ct, string? status = null, Guid? customerId = null, DateOnly? from = null, DateOnly? to = null, int? limit = null, int? offset = null) @@ -214,17 +219,20 @@ private static async Task ListSalesOrders( var skip = Math.Max(offset ?? 0, 0); var list = await orders.ListAsync(statusFilter, customerId, from, to, take, skip, ct); - return Results.Ok(list.Select(ToResponse)); + var provenance = await audit.GetProvenanceAsync( + nameof(SalesOrder), list.Select(o => o.Id).ToList(), ct); + return Results.Ok(list.Select(o => ToResponse(o, provenance.GetValueOrDefault(o.Id)))); } - private static SalesOrderResponse ToResponse(SalesOrder o) => new( + private static SalesOrderResponse ToResponse(SalesOrder o, EntityProvenance? p) => new( o.Id, o.CustomerId, o.ReferenceNumber, o.OrderDate, o.Status.ToString(), o.TotalAmount.MinorUnits, o.TotalAmount.CurrencyCode, o.TotalAmount.CurrencyMinorUnit, o.VoidReason, o.Items.Select(i => new SalesOrderItemResponse( i.Id, i.ProductId, i.EggGradeId, i.Unit.ToString(), i.BaseUnitFactor, i.Quantity, i.QuantityBase, - i.UnitPrice.MinorUnits, i.UnitPrice.CurrencyCode, i.UnitPrice.CurrencyMinorUnit)).ToList()); + i.UnitPrice.MinorUnits, i.UnitPrice.CurrencyCode, i.UnitPrice.CurrencyMinorUnit)).ToList(), + p?.CreatedByEmail, p?.CreatedAtUtc, p?.LastChangedByEmail, p?.LastChangedAtUtc); private static async Task VoidSale( Guid id, @@ -297,7 +305,11 @@ public sealed record SalesOrderResponse( Guid Id, Guid CustomerId, string ReferenceNumber, DateOnly OrderDate, string Status, long TotalMinorUnits, string CurrencyCode, int CurrencyMinorUnit, string? VoidReason, - IReadOnlyList Items); + IReadOnlyList Items, + // #494 provenance, derived from the audit trail: null together for a + // record created before that shipped (no backfill). + string? CreatedByEmail, DateTimeOffset? CreatedAtUtc, + string? LastChangedByEmail, DateTimeOffset? LastChangedAtUtc); public sealed record CreateSalesOrderRequest(Guid CustomerId, DateOnly OrderDate); diff --git a/src/Cluckwork.Application/Common/AuditActions.cs b/src/Cluckwork.Application/Common/AuditActions.cs index 61ff43868..7cd2ab276 100644 --- a/src/Cluckwork.Application/Common/AuditActions.cs +++ b/src/Cluckwork.Application/Common/AuditActions.cs @@ -55,4 +55,9 @@ public static class AuditActions public const string ProductDeactivate = "Product.Deactivate"; public const string EggUnitConversionUpdate = "EggUnitConversion.Update"; public const string EggLotMovement = "EggLot.Movement"; + public const string FlockCreate = "Flock.Create"; + public const string DailyEntryCreate = "DailyEntry.Create"; + public const string SalesOrderCreate = "SalesOrder.Create"; + public const string ExpenseCreate = "Expense.Create"; + public const string EggGradeCreate = "EggGrade.Create"; } diff --git a/src/Cluckwork.Application/Features/Audit/EntityProvenance.cs b/src/Cluckwork.Application/Features/Audit/EntityProvenance.cs new file mode 100644 index 000000000..ee79059b4 --- /dev/null +++ b/src/Cluckwork.Application/Features/Audit/EntityProvenance.cs @@ -0,0 +1,13 @@ +namespace Cluckwork.Application.Features.Audit; + +// #494 — derived from the audit trail, never stored: the earliest AuditEvent +// for an entity id is its creation, the latest is its most recent change. +// +// LastChanged* is null when nothing has happened since creation — i.e. when +// the trail's two ends are the SAME event. That judgement belongs here and not +// to a caller, because only this layer holds the event ids: two DISTINCT events +// can share an instant, so a caller comparing timestamps would call a genuinely +// edited record untouched (codex review of #494). +public sealed record EntityProvenance( + string CreatedByEmail, DateTimeOffset CreatedAtUtc, + string? LastChangedByEmail, DateTimeOffset? LastChangedAtUtc); diff --git a/src/Cluckwork.Application/Features/Audit/IAuditEventRepository.cs b/src/Cluckwork.Application/Features/Audit/IAuditEventRepository.cs index 008e0f4db..c9964eb94 100644 --- a/src/Cluckwork.Application/Features/Audit/IAuditEventRepository.cs +++ b/src/Cluckwork.Application/Features/Audit/IAuditEventRepository.cs @@ -6,7 +6,18 @@ namespace Cluckwork.Application.Features.Audit; // there is intentionally no update/remove surface anywhere. public interface IAuditEventRepository { + // Ids per round trip. NOT a caller contract — the egg-grade list is + // unpaginated and grades are never deleted, so that catalog outgrows any + // fixed cap eventually; a larger input is chunked, never refused. + public const int MaxBatchIds = 500; + Task> ListAsync( string? action, Guid? entityId, DateOnly? from, DateOnly? to, int limit, int offset, CancellationToken ct = default); + + // #494 — created/last-changed per entity id, for one entity type. An id + // with no events is ABSENT from the result rather than present-and-blank: + // records that predate #494 have no creation event and get no backfill. + Task> GetProvenanceAsync( + string entityType, IReadOnlyCollection entityIds, CancellationToken ct = default); } diff --git a/src/Cluckwork.Application/Features/DailyEntries/RecordDailyEntry/RecordDailyEntryHandler.cs b/src/Cluckwork.Application/Features/DailyEntries/RecordDailyEntry/RecordDailyEntryHandler.cs index 2914d3843..3a42bcfc7 100644 --- a/src/Cluckwork.Application/Features/DailyEntries/RecordDailyEntry/RecordDailyEntryHandler.cs +++ b/src/Cluckwork.Application/Features/DailyEntries/RecordDailyEntry/RecordDailyEntryHandler.cs @@ -14,6 +14,7 @@ public sealed class RecordDailyEntryHandler( IEggGradeRepository eggGrades, IFlockRepository flocks, IFlockScopeGuard flockScope, + IAuditWriter audit, IUnitOfWork unitOfWork, ILogger logger) { @@ -86,6 +87,9 @@ public async Task> HandleAsync( command.FarmId, command.HouseId, command.FlockId, command.Date); await repository.AddAsync(entry, ct); + // #494 — only the first record for a natural key is a creation; + // a later call appends to the same entry and is not audited here. + await audit.WriteAsync(AuditActions.DailyEntryCreate, nameof(DailyEntry), entry.Id, ct: ct); } else { diff --git a/src/Cluckwork.Application/Features/EggGrades/CreateEggGrade/CreateEggGradeHandler.cs b/src/Cluckwork.Application/Features/EggGrades/CreateEggGrade/CreateEggGradeHandler.cs index d63e52d5f..1cf19e7b9 100644 --- a/src/Cluckwork.Application/Features/EggGrades/CreateEggGrade/CreateEggGradeHandler.cs +++ b/src/Cluckwork.Application/Features/EggGrades/CreateEggGrade/CreateEggGradeHandler.cs @@ -7,6 +7,7 @@ namespace Cluckwork.Application.Features.EggGrades.CreateEggGrade; public sealed class CreateEggGradeHandler( IEggGradeRepository grades, + IAuditWriter audit, IUnitOfWork unitOfWork) { public async Task> HandleAsync( @@ -28,6 +29,7 @@ public async Task> HandleAsync( command.Name, gradeType, command.SortOrder, command.IsSaleable); await grades.AddAsync(grade, ct); + await audit.WriteAsync(AuditActions.EggGradeCreate, nameof(EggGrade), grade.Id, ct: ct); await unitOfWork.SaveChangesAsync(ct); return Result.Success(grade.Id); } diff --git a/src/Cluckwork.Application/Features/Expenses/CreateExpense/CreateExpenseHandler.cs b/src/Cluckwork.Application/Features/Expenses/CreateExpense/CreateExpenseHandler.cs index 885713239..b220abad3 100644 --- a/src/Cluckwork.Application/Features/Expenses/CreateExpense/CreateExpenseHandler.cs +++ b/src/Cluckwork.Application/Features/Expenses/CreateExpense/CreateExpenseHandler.cs @@ -12,6 +12,7 @@ public sealed class CreateExpenseHandler( IExpenseCategoryRepository categories, IFlockRepository flocks, IAccountRepository accounts, + IAuditWriter audit, IUnitOfWork unitOfWork) { public async Task> HandleAsync( @@ -59,6 +60,8 @@ await unitOfWork.ExecuteInTransactionAsync(async transactionCt => command.FlockId, command.Note); await expenses.AddAsync(expense, transactionCt); + await audit.WriteAsync( + AuditActions.ExpenseCreate, nameof(Expense), expense.Id, ct: transactionCt); outcome = Result.Success(expense.Id); return true; }, ct); diff --git a/src/Cluckwork.Application/Features/Flocks/CreateFlock/CreateFlockHandler.cs b/src/Cluckwork.Application/Features/Flocks/CreateFlock/CreateFlockHandler.cs index 60a7f8b53..df41d9381 100644 --- a/src/Cluckwork.Application/Features/Flocks/CreateFlock/CreateFlockHandler.cs +++ b/src/Cluckwork.Application/Features/Flocks/CreateFlock/CreateFlockHandler.cs @@ -8,6 +8,7 @@ namespace Cluckwork.Application.Features.Flocks.CreateFlock; public sealed class CreateFlockHandler( IFlockRepository flocks, + IAuditWriter audit, IUnitOfWork unitOfWork) { public async Task> HandleAsync( @@ -20,6 +21,7 @@ public async Task> HandleAsync( command.PlacementDate, command.InitialCount); await flocks.AddAsync(flock, ct); + await audit.WriteAsync(AuditActions.FlockCreate, nameof(Flock), flock.Id, ct: ct); await unitOfWork.SaveChangesAsync(ct); return Result.Success(flock.Id); } diff --git a/src/Cluckwork.Application/Features/Sales/CreateSalesOrder/CreateSalesOrderHandler.cs b/src/Cluckwork.Application/Features/Sales/CreateSalesOrder/CreateSalesOrderHandler.cs index 4be315336..07c1cbe32 100644 --- a/src/Cluckwork.Application/Features/Sales/CreateSalesOrder/CreateSalesOrderHandler.cs +++ b/src/Cluckwork.Application/Features/Sales/CreateSalesOrder/CreateSalesOrderHandler.cs @@ -9,6 +9,7 @@ namespace Cluckwork.Application.Features.Sales.CreateSalesOrder; public sealed class CreateSalesOrderHandler( ICustomerRepository customers, IAccountRepository accounts, + IAuditWriter audit, IUnitOfWork unitOfWork, ISalesOrderRepository orders) { @@ -43,6 +44,8 @@ await unitOfWork.ExecuteInTransactionAsync(async transactionCt => account.DefaultCurrencyCode, account.DefaultCurrencyMinorUnit); await orders.AddAsync(order, transactionCt); + await audit.WriteAsync( + AuditActions.SalesOrderCreate, nameof(SalesOrder), order.Id, ct: transactionCt); outcome = Result.Success(order.Id); return true; }, ct); diff --git a/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs b/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs index 817f2535e..70de78313 100644 --- a/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs +++ b/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs @@ -5,7 +5,7 @@ namespace Cluckwork.Infrastructure.Repositories; using Cluckwork.Infrastructure.Persistence; using Microsoft.EntityFrameworkCore; -public sealed class AuditEventRepository(AppDbContext db) : IAuditEventRepository +public sealed class AuditEventRepository(AppDbContext db, TenantContext tenant) : IAuditEventRepository { public async Task> ListAsync( string? action, Guid? entityId, DateOnly? from, DateOnly? to, @@ -34,4 +34,77 @@ public async Task> ListAsync( .Take(limit) .ToListAsync(ct); } + + // #494 — earliest event per id is the creation, latest is the last change. + // + // MaxBatchIds is how many ids go into ONE round trip, not a limit on the + // caller: the egg-grade list has no pagination and grades are never + // deleted, so that catalog outgrows any fixed cap eventually and must not + // fail the whole endpoint when it does. Oversized inputs are chunked. + public async Task> GetProvenanceAsync( + string entityType, IReadOnlyCollection entityIds, CancellationToken ct = default) + { + var all = new Dictionary(); + foreach (var chunk in entityIds.Distinct().Chunk(IAuditEventRepository.MaxBatchIds)) + { + foreach (var (id, provenance) in await GetProvenanceChunkAsync(entityType, chunk, ct)) + all[id] = provenance; + } + return all; + } + + // One chunk, two round trips. Raw SQL because DISTINCT ON is Postgres-only + // and has no LINQ translation. EF would otherwise compose the tenant query + // filter over this — IgnoreQueryFilters opts out of that, so the hand-written + // AccountId predicate is then the ONLY thing scoping the read to the tenant. + // Both must stay: drop the predicate and every account's trail is visible. + private async Task> GetProvenanceChunkAsync( + string entityType, Guid[] ids, CancellationToken ct) + { + var accountId = tenant.AccountId; + + // Id tiebreaker matches ListAsync: same-instant events resolve stably. + var created = await db.AuditEvents.FromSqlInterpolated($""" + SELECT DISTINCT ON ("EntityId") * + FROM "AuditEvents" + WHERE "AccountId" = {accountId} + AND "EntityType" = {entityType} + AND "EntityId" = ANY({ids}) + ORDER BY "EntityId", "OccurredAtUtc" ASC, "Id" ASC + """) + .IgnoreQueryFilters() + .AsNoTracking() + .ToListAsync(ct); + + var latest = await db.AuditEvents.FromSqlInterpolated($""" + SELECT DISTINCT ON ("EntityId") * + FROM "AuditEvents" + WHERE "AccountId" = {accountId} + AND "EntityType" = {entityType} + AND "EntityId" = ANY({ids}) + ORDER BY "EntityId", "OccurredAtUtc" DESC, "Id" DESC + """) + .IgnoreQueryFilters() + .AsNoTracking() + .ToListAsync(ct); + + // Both queries share a WHERE clause and the log is append-only, so every + // id in `created` is in `latest` too — they can only differ in WHICH row + // won per id. + var latestById = latest.ToDictionary(e => e.EntityId); + return created.ToDictionary( + e => e.EntityId, + e => + { + var last = latestById[e.EntityId]; + // Compare EVENT IDENTITY, not timestamps: two distinct events + // can share an instant (hence the Id tiebreaker above), and + // treating those as one would hide a real change. + var unchanged = last.Id == e.Id; + return new EntityProvenance( + e.ActorEmail, e.OccurredAtUtc, + unchanged ? null : last.ActorEmail, + unchanged ? null : last.OccurredAtUtc); + }); + } } diff --git a/tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs b/tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs new file mode 100644 index 000000000..32c64ce86 --- /dev/null +++ b/tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs @@ -0,0 +1,312 @@ +namespace Cluckwork.Api.IntegrationTests; + +using System.Net; +using Cluckwork.Api.IntegrationTests.Infrastructure; +using Cluckwork.Application.Features.Audit; +using Cluckwork.Domain.Auditing; +using Cluckwork.Infrastructure.Persistence; +using Microsoft.Extensions.DependencyInjection; + +// #494 — "who created this, and who last changed it" derived from the +// append-only audit trail rather than from new columns on every aggregate. +// The lookup is raw SQL (DISTINCT ON), so it bypasses the EF tenant query +// filter: the AccountId predicate in that SQL is the only thing scoping it, +// which is what Provenance_IsScopedToTheTenant exists to hold in place. +[Collection(IntegrationCollection.Name)] +public sealed class AuditProvenanceTests(CluckworkWebApplicationFactory factory) +{ + private static readonly DateTimeOffset Base = + new(2026, 5, 1, 8, 0, 0, TimeSpan.Zero); + + private async Task SeedEventsAsync(Guid accountId, params AuditEvent[] events) => + await factory.WithTenantScopeAsync(accountId, async db => + { + db.AuditEvents.AddRange(events); + await db.SaveChangesAsync(); + }); + + private static AuditEvent Event( + Guid accountId, Guid entityId, string action, string email, int minutesFromBase, + string entityType = "Flock") => + AuditEvent.Create( + Guid.NewGuid(), accountId, Base.AddMinutes(minutesFromBase), + Guid.NewGuid(), email, action, entityType, entityId); + + private async Task WithRepositoryAsync( + Guid accountId, Func> action) + { + using var scope = factory.Services.CreateScope(); + scope.ServiceProvider.GetRequiredService().Resolve(accountId); + return await action(scope.ServiceProvider.GetRequiredService()); + } + + // Never changed since creation: the trail's two ends are the SAME event, so + // there is no change to report. Deciding that here rather than in the UI is + // deliberate — only this layer can tell one event from two that happen to + // share an instant (see Provenance_WhenTwoEventsShareAnInstant...). + [Fact] + public async Task Provenance_WithOneEvent_ReportsNoChange() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var entityId = Guid.NewGuid(); + await SeedEventsAsync(accountId, Event(accountId, entityId, "Flock.Create", "ana@farm.test", 0)); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("Flock", [entityId])); + + var provenance = result[entityId]; + Assert.Equal("ana@farm.test", provenance.CreatedByEmail); + Assert.Equal(Base, provenance.CreatedAtUtc); + Assert.Null(provenance.LastChangedByEmail); + Assert.Null(provenance.LastChangedAtUtc); + } + + [Fact] + public async Task Provenance_WithSeveralEvents_ReportsTheEarliestAndTheLatest() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var entityId = Guid.NewGuid(); + // Deliberately inserted out of order: the SQL orders, not the insert. + await SeedEventsAsync(accountId, + Event(accountId, entityId, "Flock.Update", "bo@farm.test", 30), + Event(accountId, entityId, "Flock.Create", "ana@farm.test", 0), + Event(accountId, entityId, "Flock.Archive", "cy@farm.test", 90)); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("Flock", [entityId])); + + var provenance = result[entityId]; + Assert.Equal("ana@farm.test", provenance.CreatedByEmail); + Assert.Equal(Base, provenance.CreatedAtUtc); + Assert.Equal("cy@farm.test", provenance.LastChangedByEmail); + Assert.Equal(Base.AddMinutes(90), provenance.LastChangedAtUtc); + } + + // Two events can share an instant — the queries carry an Id tiebreaker + // precisely because of it, and a seeder writing off a fixed clock produces + // it readily. The change must still be reported: a caller that told them + // apart only by timestamp would call this record untouched since creation. + [Fact] + public async Task Provenance_WhenTwoEventsShareAnInstant_StillReportsTheChange() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var entityId = Guid.NewGuid(); + await SeedEventsAsync(accountId, + Event(accountId, entityId, "Flock.Create", "ana@farm.test", 0), + Event(accountId, entityId, "Flock.Update", "bo@farm.test", 0)); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("Flock", [entityId])); + + var provenance = result[entityId]; + Assert.Equal("ana@farm.test", provenance.CreatedByEmail); + // Distinct events, so this is a real change and must be reported. + Assert.NotNull(provenance.LastChangedByEmail); + Assert.Equal("bo@farm.test", provenance.LastChangedByEmail); + } + + [Fact] + public async Task Provenance_IsScopedToTheTenant() + { + var mine = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var theirs = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var myEntity = Guid.NewGuid(); + var theirEntity = Guid.NewGuid(); + await SeedEventsAsync(mine, Event(mine, myEntity, "Flock.Create", "ana@farm.test", 0)); + await SeedEventsAsync(theirs, Event(theirs, theirEntity, "Flock.Create", "rival@other.test", 0)); + + var result = await WithRepositoryAsync(mine, repo => + repo.GetProvenanceAsync("Flock", [myEntity, theirEntity])); + + Assert.True(result.ContainsKey(myEntity)); + Assert.False(result.ContainsKey(theirEntity)); + } + + [Fact] + public async Task Provenance_IgnoresEventsOfAnotherEntityType() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + // The same id under two entity types: only the asked-for type counts. + var entityId = Guid.NewGuid(); + await SeedEventsAsync(accountId, + Event(accountId, entityId, "Expense.Create", "bo@farm.test", 0, entityType: "Expense")); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("Flock", [entityId])); + + Assert.Empty(result); + } + + [Fact] + public async Task Provenance_ForAnIdWithNoEvents_IsAbsentRatherThanBlank() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var known = Guid.NewGuid(); + var untouched = Guid.NewGuid(); + await SeedEventsAsync(accountId, Event(accountId, known, "Flock.Create", "ana@farm.test", 0)); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("Flock", [known, untouched])); + + Assert.True(result.ContainsKey(known)); + Assert.False(result.ContainsKey(untouched)); + } + + [Fact] + public async Task Provenance_WithNoIds_ReturnsEmptyWithoutQuerying() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("Flock", [])); + + Assert.Empty(result); + } + + // --- over the wire ----------------------------------------------------- + // + // One per extended list endpoint: the point of #494 is that provenance + // reaches the record's OWN page without a second call, so each of these + // asserts the creating user's email came back on the list row itself. + + private sealed record ProvenanceRowDto( + Guid Id, string? CreatedByEmail, DateTimeOffset? CreatedAtUtc, + string? LastChangedByEmail, DateTimeOffset? LastChangedAtUtc); + private sealed record ExpenseListDto(List Items); + private sealed record CreatedDto(Guid Id); + + private async Task<(HttpClient Client, string Email)> AuthedAsync() + { + var email = $"u-{Guid.NewGuid():N}@test.local"; + await factory.SeedAccountWithUserAsync(email); + return (factory.CreateAuthedClient(await factory.LoginForAccessTokenAsync(email)), email); + } + + private static async Task CreatedIdAsync(HttpResponseMessage response) + { + Assert.Equal(HttpStatusCode.Created, response.StatusCode); + return (await response.Content.ReadFromJsonAsync())!.Id; + } + + private static void AssertCreatedBy(ProvenanceRowDto row, string email) + { + Assert.Equal(email, row.CreatedByEmail); + Assert.NotNull(row.CreatedAtUtc); + // Never changed since creation: the trail's earliest and latest event + // are the same one, so the server reports no change at all. + Assert.Null(row.LastChangedByEmail); + Assert.Null(row.LastChangedAtUtc); + } + + [Fact] + public async Task FlockList_CarriesProvenance() + { + var (client, email) = await AuthedAsync(); + var id = await CreatedIdAsync(await client.PostWithKeyAsync( + "/api/v1/flocks", Guid.NewGuid().ToString(), + new { name = "Barn A", breed = "ISA Brown", placementDate = "2026-01-01", initialCount = 200 })); + + var rows = await client.GetFromJsonAsync>("/api/v1/flocks"); + + AssertCreatedBy(rows!.Single(r => r.Id == id), email); + } + + [Fact] + public async Task EggGradeList_CarriesProvenance() + { + var (client, email) = await AuthedAsync(); + var id = await CreatedIdAsync(await client.PostWithKeyAsync( + "/api/v1/egg-grades", Guid.NewGuid().ToString(), + new { name = "Jumbo", gradeType = "Custom", sortOrder = 7, isSaleable = true })); + + var rows = await client.GetFromJsonAsync>("/api/v1/egg-grades"); + + AssertCreatedBy(rows!.Single(r => r.Id == id), email); + } + + [Fact] + public async Task ExpenseList_CarriesProvenance() + { + var (client, email) = await AuthedAsync(); + var categoryId = await CreatedIdAsync(await client.PostWithKeyAsync( + "/api/v1/expense-categories", Guid.NewGuid().ToString(), new { name = "Feed" })); + var id = await CreatedIdAsync(await client.PostWithKeyAsync( + "/api/v1/expenses", Guid.NewGuid().ToString(), new + { + expenseCategoryId = categoryId, + date = DateOnly.FromDateTime(DateTime.UtcNow.Date), + description = "Feed delivery", + amountMinorUnits = 25_00, + })); + + var list = await client.GetFromJsonAsync("/api/v1/expenses"); + + AssertCreatedBy(list!.Items.Single(r => r.Id == id), email); + } + + [Fact] + public async Task SalesOrderList_CarriesProvenance() + { + var (client, email) = await AuthedAsync(); + var customerId = await CreatedIdAsync(await client.PostWithKeyAsync( + "/api/v1/customers", Guid.NewGuid().ToString(), + new { name = "Mercado Central", phone = "555-0100" })); + var id = await CreatedIdAsync(await client.PostWithKeyAsync( + "/api/v1/sales", Guid.NewGuid().ToString(), + new { customerId, orderDate = DateOnly.FromDateTime(DateTime.UtcNow.Date) })); + + var rows = await client.GetFromJsonAsync>("/api/v1/sales"); + + AssertCreatedBy(rows!.Single(r => r.Id == id), email); + } + + [Fact] + public async Task DailyEntryList_CarriesProvenance() + { + var email = $"u-{Guid.NewGuid():N}@test.local"; + var accountId = await factory.SeedAccountWithUserAsync(email); + var farmId = Guid.NewGuid(); + var flockId = await factory.SeedFlockAsync(accountId, farmId); + var client = factory.CreateAuthedClient(await factory.LoginForAccessTokenAsync(email)); + + var id = await CreatedIdAsync(await client.PostWithKeyAsync( + "/api/v1/daily-entries", Guid.NewGuid().ToString(), new + { + farmId, + houseId = Guid.NewGuid(), + flockId, + date = DateOnly.FromDateTime(DateTime.UtcNow.Date), + totalEggs = 100, + crackedEggs = 0, + dirtyEggs = 0, + discardedEggs = 0, + mortalityCount = 0, + })); + + var rows = await client.GetFromJsonAsync>("/api/v1/daily-entries"); + + AssertCreatedBy(rows!.Single(r => r.Id == id), email); + } + + // Most callers hand this a clamped page, but the egg-grade list has no + // pagination at all and grades are never deleted — a farm's catalog grows + // past any fixed cap eventually. So the batch size is an internal chunking + // detail, NOT a caller contract: more ids than fit in one round trip must + // still answer correctly rather than fail the whole list endpoint. + [Fact] + public async Task Provenance_AboveTheBatchSize_StillResolvesEveryId() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var ids = Enumerable.Range(0, IAuditEventRepository.MaxBatchIds + 25) + .Select(_ => Guid.NewGuid()).ToArray(); + // Seed an event for every id so a dropped chunk shows up as a missing key. + await SeedEventsAsync(accountId, + ids.Select(id => Event(accountId, id, "Flock.Create", "ana@farm.test", 0)).ToArray()); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("Flock", ids)); + + Assert.Equal(ids.Length, result.Count); + Assert.All(ids, id => Assert.Equal("ana@farm.test", result[id].CreatedByEmail)); + } +} diff --git a/tests/Cluckwork.Api.IntegrationTests/AuditTests.cs b/tests/Cluckwork.Api.IntegrationTests/AuditTests.cs index e2185dab4..8becca3f2 100644 --- a/tests/Cluckwork.Api.IntegrationTests/AuditTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/AuditTests.cs @@ -56,8 +56,11 @@ public async Task Adjust_WritesEvent_WithActorAndReason_FailedAdjustWritesNothin }); Assert.Equal(HttpStatusCode.Conflict, failed.StatusCode); + // Scoped to the adjust action: recording the entry legitimately left a + // DailyEntry.Create event (#494), and this test is about what the + // FAILED ADJUST wrote, which is nothing. var afterFailure = await client.GetFromJsonAsync>( - $"/api/v1/audit?entityId={entryId}"); + $"/api/v1/audit?entityId={entryId}&action=DailyEntry.Adjust"); Assert.Empty(afterFailure!); // The successful adjust writes exactly one, with actor + reason. @@ -69,7 +72,8 @@ public async Task Adjust_WritesEvent_WithActorAndReason_FailedAdjustWritesNothin }); Assert.Equal(HttpStatusCode.OK, ok.StatusCode); - var rows = await client.GetFromJsonAsync>($"/api/v1/audit?entityId={entryId}"); + var rows = await client.GetFromJsonAsync>( + $"/api/v1/audit?entityId={entryId}&action=DailyEntry.Adjust"); var row = Assert.Single(rows!); Assert.Equal("DailyEntry.Adjust", row.Action); Assert.Equal("DailyEntry", row.EntityType); @@ -136,7 +140,9 @@ public async Task Viewer_NeverCrossesTenants() version, totalEggs = 40, crackedEggs = 0, dirtyEggs = 0, discardedEggs = 0, mortalityCount = 0, reason = "tenant A only" }); - Assert.Single((await clientA.GetFromJsonAsync>($"/api/v1/audit?entityId={entryId}"))!); + // A's own trail is non-empty (create + adjust), which is what makes B's + // emptiness below meaningful rather than vacuous. + Assert.NotEmpty((await clientA.GetFromJsonAsync>($"/api/v1/audit?entityId={entryId}"))!); // Tenant B's admin sees nothing — not by list, not by A's entity id. var (clientB, _, _, _, _, _) = await SetupAsync(); diff --git a/tests/Cluckwork.Api.IntegrationTests/CustomerAndOrderTests.cs b/tests/Cluckwork.Api.IntegrationTests/CustomerAndOrderTests.cs index c259f8720..66c02b3f6 100644 --- a/tests/Cluckwork.Api.IntegrationTests/CustomerAndOrderTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/CustomerAndOrderTests.cs @@ -2,6 +2,7 @@ namespace Cluckwork.Api.IntegrationTests; using System.Net; using Cluckwork.Api.IntegrationTests.Infrastructure; +using Microsoft.EntityFrameworkCore; // #10 + #11 — customers and the order create/add-item surface. The final test // drives the whole MVP loop through the public API alone (no harness seeding @@ -443,4 +444,25 @@ await client.PostWithKeyAsync( Assert.Equal(350, stock.Single(s => s.EggGradeId == grades["Large"]).Available); Assert.Equal(200, stock.Single(s => s.EggGradeId == grades["Medium"]).Available); } + + // #494 — creation wasn't on the audit trail at all; only corrections were. + [Fact] + public async Task SalesOrder_Create_WritesAuditEvent() + { + var (client, accountId, _, _, _) = await SetupAsync("Large"); + + var customerId = await CreatedId(await client.PostWithKeyAsync( + "/api/v1/customers", Guid.NewGuid().ToString(), + new { name = "Mercado Central", phone = "555-0100" })); + var orderId = await CreatedId(await client.PostWithKeyAsync( + "/api/v1/sales", Guid.NewGuid().ToString(), + new { customerId, orderDate = DateOnly.FromDateTime(DateTime.UtcNow.Date) })); + + var events = await factory.WithTenantScopeAsync(accountId, db => db.AuditEvents + .Where(e => e.EntityType == "SalesOrder" && e.EntityId == orderId) + .ToListAsync()); + + var created = Assert.Single(events); + Assert.Equal("SalesOrder.Create", created.Action); + } } diff --git a/tests/Cluckwork.Api.IntegrationTests/EggGradeManagementTests.cs b/tests/Cluckwork.Api.IntegrationTests/EggGradeManagementTests.cs index a52e74ec0..875902da5 100644 --- a/tests/Cluckwork.Api.IntegrationTests/EggGradeManagementTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/EggGradeManagementTests.cs @@ -221,5 +221,25 @@ public async Task Grade_ParallelCreates_SameName_ExactlyOneWins() Assert.Single(all!, g => g.Name.Equals("duplo", StringComparison.OrdinalIgnoreCase)); } + // #494 — creation wasn't on the audit trail at all; only corrections were. + [Fact] + public async Task Grade_Create_WritesAuditEvent() + { + var (client, accountId) = await SetupAsync(); + + var create = await client.PostWithKeyAsync( + "/api/v1/egg-grades", Guid.NewGuid().ToString(), + new { name = "Jumbo", gradeType = "Custom", sortOrder = 7, isSaleable = true }); + Assert.Equal(HttpStatusCode.Created, create.StatusCode); + var id = (await create.Content.ReadFromJsonAsync())!.Id; + + var events = await factory.WithTenantScopeAsync(accountId, db => db.AuditEvents + .Where(e => e.EntityType == "EggGrade" && e.EntityId == id) + .ToListAsync()); + + var created = Assert.Single(events); + Assert.Equal("EggGrade.Create", created.Action); + } + private sealed record FlockDto(Guid Id, Guid FarmId, Guid HouseId, string Name); } diff --git a/tests/Cluckwork.Api.IntegrationTests/ExpensesTests.cs b/tests/Cluckwork.Api.IntegrationTests/ExpensesTests.cs index 8496d7896..a5dcf1e2a 100644 --- a/tests/Cluckwork.Api.IntegrationTests/ExpensesTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/ExpensesTests.cs @@ -2,6 +2,7 @@ namespace Cluckwork.Api.IntegrationTests; using System.Net; using Cluckwork.Api.IntegrationTests.Infrastructure; +using Microsoft.EntityFrameworkCore; // #87 — basic expenses: farm-scoped category catalog (grade pattern) and // money-out records with a snapshotted currency, server-side period totals, @@ -240,4 +241,23 @@ public async Task ParallelAdjusts_SameBaseVersion_ExactlyOneWins() Assert.True(after.AmountMinorUnits is 60_00 or 40_00); Assert.Equal(after.AmountMinorUnits == 60_00 ? "first" : "second", after.Description); } + + // #494 — creation wasn't on the audit trail at all; only corrections were. + [Fact] + public async Task Expense_Create_WritesAuditEvent() + { + var email = $"u-{Guid.NewGuid():N}@test.local"; + var accountId = await factory.SeedAccountWithUserAsync(email); + var client = factory.CreateAuthedClient(await factory.LoginForAccessTokenAsync(email)); + + var categoryId = await CreateCategoryAsync(client, "Feed"); + var id = await CreateExpenseAsync(client, categoryId, 25_00); + + var events = await factory.WithTenantScopeAsync(accountId, db => db.AuditEvents + .Where(e => e.EntityType == "Expense" && e.EntityId == id) + .ToListAsync()); + + var created = Assert.Single(events); + Assert.Equal("Expense.Create", created.Action); + } } diff --git a/tests/Cluckwork.Api.IntegrationTests/FlockManagementTests.cs b/tests/Cluckwork.Api.IntegrationTests/FlockManagementTests.cs index 9d12e8aac..29e949b1f 100644 --- a/tests/Cluckwork.Api.IntegrationTests/FlockManagementTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/FlockManagementTests.cs @@ -97,6 +97,21 @@ public async Task Flock_ArchiveFromActive_Allowed() Assert.Equal(HttpStatusCode.NoContent, archive.StatusCode); } + // #494 — creation wasn't on the audit trail at all; only corrections were. + [Fact] + public async Task Flock_Create_WritesAuditEvent() + { + var (client, accountId) = await SetupAsync(); + var id = await CreateFlockAsync(client); + + var events = await factory.WithTenantScopeAsync(accountId, db => db.AuditEvents + .Where(e => e.EntityType == "Flock" && e.EntityId == id) + .ToListAsync()); + + var created = Assert.Single(events); + Assert.Equal("Flock.Create", created.Action); + } + // #35 review: the 422 lifecycle branch above can no longer be reached with a // future date, so reach it the way real farms do — a flock depleted days ago // refuses an entry dated after that depletion but still in the past. Deplete diff --git a/tests/Cluckwork.Api.IntegrationTests/SubmitDailyEntryTests.cs b/tests/Cluckwork.Api.IntegrationTests/SubmitDailyEntryTests.cs index d6e4437a5..8adf1ad10 100644 --- a/tests/Cluckwork.Api.IntegrationTests/SubmitDailyEntryTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/SubmitDailyEntryTests.cs @@ -279,4 +279,54 @@ public async Task FullLoop_RecordSubmitSellConfirm_StockDecrements() Assert.Equal(600, lot.QuantityProduced); Assert.Equal(350, lot.QuantityAvailable); } + + // #494 — creation wasn't on the audit trail at all; only corrections were. + [Fact] + public async Task DailyEntry_Record_WritesCreateAuditEvent() + { + var (client, accountId, farmId, flockId, grades) = await SetupAsync("Large"); + var entryId = await RecordAsync(client, Body(farmId, flockId, + [new { eggGradeId = grades["Large"], quantity = 600 }])); + + var events = await factory.WithTenantScopeAsync(accountId, db => db.AuditEvents + .Where(e => e.EntityType == "DailyEntry" && e.EntityId == entryId) + .ToListAsync()); + + var created = Assert.Single(events); + Assert.Equal("DailyEntry.Create", created.Action); + } + + // The gate: re-recording against the same natural key APPENDS to the + // existing entry — that is not a creation and must not emit a second event. + [Fact] + public async Task DailyEntry_ReRecordedOnTheSameKey_WritesOnlyOneCreateEvent() + { + var (client, accountId, farmId, flockId, grades) = await SetupAsync("Large"); + var houseId = Guid.NewGuid(); + var date = DateOnly.FromDateTime(DateTime.UtcNow.Date); + object Body2(int total, int quantity) => new + { + farmId, + houseId, + flockId, + date, + totalEggs = total, + crackedEggs = 10, + dirtyEggs = 5, + discardedEggs = 3, + mortalityCount = 0, + grades = new object[] { new { eggGradeId = grades["Large"], quantity } }, + }; + + var entryId = await RecordAsync(client, Body2(618, 600)); + var again = await RecordAsync(client, Body2(718, 700)); + Assert.Equal(entryId, again); + + var events = await factory.WithTenantScopeAsync(accountId, db => db.AuditEvents + .Where(e => e.EntityType == "DailyEntry" && e.EntityId == entryId) + .Select(e => e.Action) + .ToListAsync()); + + Assert.Equal(["DailyEntry.Create"], events); + } } diff --git a/web/src/api/cluckwork.ts b/web/src/api/cluckwork.ts index ff7360240..8451fcfec 100644 --- a/web/src/api/cluckwork.ts +++ b/web/src/api/cluckwork.ts @@ -1,7 +1,26 @@ // Typed wrappers over the Cluckwork JSON API (mirrors the endpoint DTOs). import { apiDelete, apiGet, apiGetBlob, apiPost, apiPut, apiPutBytes, STEP_UP_HEADER } from "./client"; -export interface EggGrade { +/** + * #494 — who created a record and who last changed it, derived server-side + * from the audit trail rather than stored on the record. + * + * `createdBy*` are null for a record created before #494 shipped: there is no + * backfill, so its trail has no creation event to report. + * + * `lastChanged*` are null when nothing has happened since creation. That is the + * SERVER's judgement, made from the audit event ids — do NOT re-derive it by + * comparing the two timestamps, because two distinct events can share an + * instant and that comparison would hide a real edit. + */ +export interface RecordHistory { + createdByEmail: string | null; + createdAtUtc: string | null; + lastChangedByEmail: string | null; + lastChangedAtUtc: string | null; +} + +export interface EggGrade extends RecordHistory { id: string; farmId: string; name: string; @@ -20,7 +39,7 @@ export interface EggGrade { active: boolean; } -export interface Flock { +export interface Flock extends RecordHistory { id: string; farmId: string; houseId: string; @@ -48,7 +67,7 @@ export interface GradeLine { // version = the base an admin correction must send back (stale → 409); // adjustedFrom = audit snapshot of the values the last adjust replaced. -export interface DailyEntry { +export interface DailyEntry extends RecordHistory { id: string; farmId: string; houseId: string; @@ -284,7 +303,7 @@ export interface OrderItem { currencyMinorUnit: number; } -export interface SalesOrder { +export interface SalesOrder extends RecordHistory { id: string; customerId: string; referenceNumber: string; @@ -1016,7 +1035,7 @@ export interface ExpenseCategory { active: boolean; } -export interface Expense { +export interface Expense extends RecordHistory { id: string; farmId: string; expenseCategoryId: string; diff --git a/web/src/components/ProvenanceCell.test.tsx b/web/src/components/ProvenanceCell.test.tsx new file mode 100644 index 000000000..aaf23c6a7 --- /dev/null +++ b/web/src/components/ProvenanceCell.test.tsx @@ -0,0 +1,96 @@ +import { describe, it, expect } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { ProvenanceCell } from "./ProvenanceCell"; + +// #494 — the cell reports the two ends of a record's audit trail. Whether a +// change happened is the SERVER's call: it sends lastChanged* only when the +// latest event differs from the creation event, and null otherwise. So the +// unchanged case arrives as nulls, and the component must never re-derive it by +// comparing timestamps — distinct events can share an instant. +const CREATED = "2026-05-01T08:00:00+00:00"; +const CHANGED = "2026-05-03T14:30:00+00:00"; + +function renderCell(history: Parameters[0]["history"]) { + return render( + + + + + + +
, + ); +} + +describe("ProvenanceCell", () => { + it("names the creator and when", () => { + renderCell({ + createdByEmail: "ana@farm.test", + createdAtUtc: CREATED, + lastChangedByEmail: null, + lastChangedAtUtc: null, + }); + expect(screen.getByText(/ana@farm\.test/)).toBeInTheDocument(); + }); + + it("says nothing about a change when the record has never been changed", () => { + // The server sends nulls for an unchanged record; repeating the creator + // would read as an edit that never happened. + renderCell({ + createdByEmail: "ana@farm.test", + createdAtUtc: CREATED, + lastChangedByEmail: null, + lastChangedAtUtc: null, + }); + expect(screen.queryByText(/Last changed/i)).not.toBeInTheDocument(); + }); + + it("names the last changer separately once the record has been changed", () => { + renderCell({ + createdByEmail: "ana@farm.test", + createdAtUtc: CREATED, + lastChangedByEmail: "bo@farm.test", + lastChangedAtUtc: CHANGED, + }); + expect(screen.getByText(/ana@farm\.test/)).toBeInTheDocument(); + expect(screen.getByText(/bo@farm\.test/)).toBeInTheDocument(); + }); + + it("shows a change by the SAME actor at a later time", () => { + // Same person, second edit: equality of the EMAIL alone must not suppress + // the line — only the instant tells the two events apart. + renderCell({ + createdByEmail: "ana@farm.test", + createdAtUtc: CREATED, + lastChangedByEmail: "ana@farm.test", + lastChangedAtUtc: CHANGED, + }); + expect(screen.getByText(/Last changed/i)).toBeInTheDocument(); + }); + + it("shows a change made in the SAME instant as the creation", () => { + // Two distinct audit events can share a timestamp — the server's queries + // carry an id tiebreaker precisely because of it, and a seeder running off + // a fixed clock produces it readily. Telling the two apart by instant alone + // would call this record untouched and hide a real edit by someone else. + renderCell({ + createdByEmail: "ana@farm.test", + createdAtUtc: CREATED, + lastChangedByEmail: "bo@farm.test", + lastChangedAtUtc: CREATED, + }); + expect(screen.getByText(/bo@farm\.test/)).toBeInTheDocument(); + }); + + it("renders a placeholder for a record that predates the feature", () => { + // No backfill: all four arrive null together and there is nothing to claim. + renderCell({ + createdByEmail: null, + createdAtUtc: null, + lastChangedByEmail: null, + lastChangedAtUtc: null, + }); + expect(screen.queryByText(/Created by/i)).not.toBeInTheDocument(); + expect(screen.getByText("—")).toBeInTheDocument(); + }); +}); diff --git a/web/src/components/ProvenanceCell.tsx b/web/src/components/ProvenanceCell.tsx new file mode 100644 index 000000000..c686187bc --- /dev/null +++ b/web/src/components/ProvenanceCell.tsx @@ -0,0 +1,45 @@ +import { useTranslation } from "react-i18next"; +import type { RecordHistory } from "../api/cluckwork"; + +// #494 — one table cell reporting who created a record and who last changed it. +// +// Whether a change happened at all is the SERVER's call: it sends +// lastChanged* only when the trail's latest event is a different event from the +// creation, and null otherwise. This component must not re-derive that by +// comparing timestamps — two distinct events can share an instant, and that +// comparison would hide a real edit (codex review of #494). +// +// Timestamp rendering follows AuditPage's existing convention (UTC, trimmed to +// seconds) rather than the farm clock: these are audit instants, and the trail +// is displayed in UTC everywhere else it appears. +function formatInstant(iso: string): string { + return iso.replace("T", " ").slice(0, 19); +} + +export function ProvenanceCell({ history }: { history: RecordHistory }) { + const { t } = useTranslation("common"); + const { createdByEmail, createdAtUtc, lastChangedByEmail, lastChangedAtUtc } = history; + + // Null together: a record created before #494 shipped has no creation event + // and gets no backfill, so there is nothing to claim about it. + if (!createdByEmail || !createdAtUtc) { + return —; + } + + const changed = lastChangedByEmail && lastChangedAtUtc; + + return ( + +
{t("recordHistory.createdBy", { + email: createdByEmail, + at: formatInstant(createdAtUtc), + })}
+ {changed && ( +
{t("recordHistory.lastChangedBy", { + email: lastChangedByEmail, + at: formatInstant(lastChangedAtUtc), + })}
+ )} + + ); +} diff --git a/web/src/i18n/en.ts b/web/src/i18n/en.ts index 12e6864e9..22385d166 100644 --- a/web/src/i18n/en.ts +++ b/web/src/i18n/en.ts @@ -4,6 +4,11 @@ // is a build error. export const en = { common: { + // #494 — the record-history cell shown on the list screens. "Last changed" + // appears only when something happened after creation. + "recordHistory.createdBy": "Created by {{email}} on {{at}}", + "recordHistory.lastChangedBy": "Last changed by {{email}} on {{at}}", + recordHistoryHeader: "History", cancel: "Cancel", save: "Save", close: "Close", @@ -2039,6 +2044,11 @@ export const en = { "auditAction.Product.Deactivate": "Product deactivated", "auditAction.EggUnitConversion.Update": "Egg unit conversion updated", "auditAction.EggLot.Movement": "Stock written off / recounted", + "auditAction.Flock.Create": "Flock created", + "auditAction.DailyEntry.Create": "Daily entry created", + "auditAction.SalesOrder.Create": "Sales order created", + "auditAction.Expense.Create": "Expense created", + "auditAction.EggGrade.Create": "Egg grade created", // entity type (AuditPage table entity cell) — AuditEvent.entityType. "entityType.Account": "Account", @@ -2522,6 +2532,13 @@ export const en = { + "it, when (UTC), what it touched, and the reason where one was given. Written together with the " + "change itself — a failed action leaves no trace, a successful one always does — and never editable, " + "by anyone.", + auditRecordHistory: + "Flocks, Egg grades, Daily entry history, Sales and Expenses each carry a History column showing who " + + "created the record and when, plus who last changed it if anyone has. It is read from the same audit " + + "log, so it needs no extra permission: if you can see the record, you can see its history.", + auditRecordHistoryOlder: + "Records created before this was added show a blank History column. Nothing is missing — there is " + + "simply no creation entry in the log for them, and none is invented after the fact.", // Export & backup exportHeading: "Export & backup (admin)", diff --git a/web/src/i18n/enums.ts b/web/src/i18n/enums.ts index 780822e18..950ccde79 100644 --- a/web/src/i18n/enums.ts +++ b/web/src/i18n/enums.ts @@ -290,6 +290,8 @@ export const AUDIT_ACTION_VALUES = [ "Account.SetBanner", "Account.RemoveBanner", "Account.UpdateSettings", "Product.Create", "Product.Update", "Product.Activate", "Product.Deactivate", "EggUnitConversion.Update", "EggLot.Movement", + "Flock.Create", "DailyEntry.Create", "SalesOrder.Create", + "Expense.Create", "EggGrade.Create", ] as const; export type AuditActionValue = (typeof AUDIT_ACTION_VALUES)[number]; const AUDIT_ACTION_KEYS = { @@ -331,6 +333,11 @@ const AUDIT_ACTION_KEYS = { "Product.Deactivate": "enums:auditAction.Product.Deactivate", "EggUnitConversion.Update": "enums:auditAction.EggUnitConversion.Update", "EggLot.Movement": "enums:auditAction.EggLot.Movement", + "Flock.Create": "enums:auditAction.Flock.Create", + "DailyEntry.Create": "enums:auditAction.DailyEntry.Create", + "SalesOrder.Create": "enums:auditAction.SalesOrder.Create", + "Expense.Create": "enums:auditAction.Expense.Create", + "EggGrade.Create": "enums:auditAction.EggGrade.Create", } as const satisfies Record; export function auditActionLabel(value: AuditActionValue | (string & {})): string { const key = AUDIT_ACTION_KEYS[value as AuditActionValue]; diff --git a/web/src/i18n/es.ts b/web/src/i18n/es.ts index 637c0d250..7c9d36562 100644 --- a/web/src/i18n/es.ts +++ b/web/src/i18n/es.ts @@ -5,6 +5,9 @@ // on-screen label, until that screen itself is externalized to the catalog (#182). export const es = { common: { + "recordHistory.createdBy": "Creado por {{email}} el {{at}}", + "recordHistory.lastChangedBy": "Última modificación por {{email}} el {{at}}", + recordHistoryHeader: "Historial", cancel: "Cancelar", save: "Guardar", close: "Cerrar", @@ -1622,6 +1625,11 @@ export const es = { "auditAction.Product.Deactivate": "Producto desactivado", "auditAction.EggUnitConversion.Update": "Conversión de unidad de huevo actualizada", "auditAction.EggLot.Movement": "Existencias dadas de baja / recontadas", + "auditAction.Flock.Create": "Lote creado", + "auditAction.DailyEntry.Create": "Registro diario creado", + "auditAction.SalesOrder.Create": "Pedido de venta creado", + "auditAction.Expense.Create": "Gasto creado", + "auditAction.EggGrade.Create": "Grado de huevo creado", "entityType.Account": "Cuenta", "entityType.DailyEntry": "Entrada diaria", @@ -2118,6 +2126,15 @@ export const es = { + "automáticamente: quién lo hizo, cuándo (UTC), qué afectó, y el motivo cuando se dio uno. Se " + "escribe junto con el cambio mismo — una acción fallida no deja rastro, una exitosa siempre lo hace " + "— y nunca es editable, por nadie.", + auditRecordHistory: + "Lotes, Grados de huevo, Historial de registro diario, Ventas y Gastos tienen una columna Historial " + + "que muestra quién creó el registro y cuándo, más quién lo modificó por última vez si alguien lo " + + "hizo. Se lee del mismo registro de auditoría, así que no requiere permisos adicionales: si puede " + + "ver el registro, puede ver su historial.", + auditRecordHistoryOlder: + "Los registros creados antes de que esto se agregara muestran la columna Historial en blanco. No " + + "falta nada — simplemente no hay una entrada de creación en el registro para ellos, y no se " + + "inventa ninguna después.", exportHeading: "Exportar y respaldo (admin)", exportCsv: diff --git a/web/src/i18n/tl.ts b/web/src/i18n/tl.ts index 03403a185..54d046a1c 100644 --- a/web/src/i18n/tl.ts +++ b/web/src/i18n/tl.ts @@ -10,6 +10,9 @@ // label, until that screen itself is externalized to the catalog (#182). export const tl = { common: { + "recordHistory.createdBy": "Ginawa ni {{email}} noong {{at}}", + "recordHistory.lastChangedBy": "Huling binago ni {{email}} noong {{at}}", + recordHistoryHeader: "Kasaysayan", cancel: "Kanselahin", save: "I-save", close: "Isara", @@ -1685,6 +1688,11 @@ export const tl = { "auditAction.Product.Deactivate": "Na-deactivate ang produkto", "auditAction.EggUnitConversion.Update": "Na-update ang conversion ng yunit ng itlog", "auditAction.EggLot.Movement": "Na-write off / na-recount ang stock", + "auditAction.Flock.Create": "Nagawa ang kawan", + "auditAction.DailyEntry.Create": "Nagawa ang pang-araw-araw na entry", + "auditAction.SalesOrder.Create": "Nagawa ang order ng benta", + "auditAction.Expense.Create": "Nagawa ang gastos", + "auditAction.EggGrade.Create": "Nagawa ang grado ng itlog", "entityType.Account": "Account", "entityType.DailyEntry": "Araw-araw na Entry", @@ -2199,6 +2207,14 @@ export const tl = { + "awtomatiko: sino ang gumawa nito, kailan (UTC), ano ang naapektuhan, at ang dahilan kung mayroon. " + "Isinusulat ito kasabay ng pagbabago mismo — walang naiiwang bakas ang isang nabigong aksyon, " + "palaging may naiiwan ang isang matagumpay — at hindi kailanman ine-edit, ng kahit sino.", + auditRecordHistory: + "Ang Mga kawan, Grado ng itlog, Kasaysayan ng pang-araw-araw na entry, Benta at Gastos ay may " + + "kolum na Kasaysayan na nagpapakita kung sino ang gumawa ng record at kailan, at kung sino ang " + + "huling nagbago kung mayroon man. Galing ito sa parehong audit log, kaya walang kailangang dagdag " + + "na pahintulot: kung nakikita mo ang record, nakikita mo ang kasaysayan nito.", + auditRecordHistoryOlder: + "Ang mga record na ginawa bago ito idagdag ay may blangkong kolum na Kasaysayan. Walang nawawala — " + + "wala lang talagang entry ng paggawa sa log para sa kanila, at walang iniimbentong bago pagkatapos.", exportHeading: "Export at Backup (admin)", exportCsv: diff --git a/web/src/routes/DailyEntryPage.test.tsx b/web/src/routes/DailyEntryPage.test.tsx index 933461eb3..e467ef20a 100644 --- a/web/src/routes/DailyEntryPage.test.tsx +++ b/web/src/routes/DailyEntryPage.test.tsx @@ -11,7 +11,7 @@ import { import type { Flock, EggGrade, DailyEntry } from "../api/cluckwork"; import { todayIso } from "../lib/dates"; import { FarmContext } from "../farm/FarmContext"; -import { account, farmState } from "../test/fixtures"; +import { account, farmState, NO_RECORD_HISTORY } from "../test/fixtures"; import i18n from "../i18n"; // DailyEntry has no auth deps — mock only the API seam it loads from. The @@ -43,16 +43,18 @@ const mockListWaterUsage = vi.mocked(listWaterUsage); const mockCreateFlock = vi.mocked(createFlock); const FLOCK: Flock = { + ...NO_RECORD_HISTORY, id: "f1", farmId: "farm1", houseId: "h1", name: "Hen House 1", breed: "ISA", placementDate: "2026-01-01", initialCount: 100, currentBirds: 98, status: "Active", }; const GRADES: EggGrade[] = [ - { id: "gr1", farmId: "farm1", name: "Grade A", gradeType: "Size", sortOrder: 1, isSaleable: true, dailyEntryKind: "Manual", active: true }, - { id: "gr2", farmId: "farm1", name: "Grade B", gradeType: "Size", sortOrder: 2, isSaleable: true, dailyEntryKind: "Manual", active: true }, + { ...NO_RECORD_HISTORY, id: "gr1", farmId: "farm1", name: "Grade A", gradeType: "Size", sortOrder: 1, isSaleable: true, dailyEntryKind: "Manual", active: true }, + { ...NO_RECORD_HISTORY, id: "gr2", farmId: "farm1", name: "Grade B", gradeType: "Size", sortOrder: 2, isSaleable: true, dailyEntryKind: "Manual", active: true }, ]; // #396 — saleable AND active, so it passes every pre-existing filter. The only // thing that keeps it out of the Grading pane is its kind. const CRACKED: EggGrade = { + ...NO_RECORD_HISTORY, id: "gr-cracked", farmId: "farm1", name: "Cracked", gradeType: "Quality", sortOrder: 3, isSaleable: true, dailyEntryKind: "Cracked", active: true, }; @@ -259,6 +261,7 @@ describe("DailyEntryPage prefill gating", () => { it("locks the form when the day already has a submitted entry", async () => { const existing: DailyEntry = { + ...NO_RECORD_HISTORY, id: "de1", farmId: "farm1", houseId: "h1", flockId: "f1", date: todayIso(), status: "Submitted", totalEggs: 100, crackedEggs: 2, dirtyEggs: 3, discardedEggs: 5, mortalityCount: 1, crackedGradeId: null, dirtyGradeId: null, grades: [{ eggGradeId: "gr1", quantity: 60 }, { eggGradeId: "gr2", quantity: 25 }], @@ -282,6 +285,7 @@ describe("DailyEntryPage prefill gating", () => { // en.ts's `enums` header comment). it("shows the harmonized 'adjusted' label, not the raw status, for a manager-adjusted day", async () => { const existing: DailyEntry = { + ...NO_RECORD_HISTORY, id: "de1", farmId: "farm1", houseId: "h1", flockId: "f1", date: todayIso(), status: "ManagerAdjusted", totalEggs: 100, crackedEggs: 2, dirtyEggs: 3, discardedEggs: 5, mortalityCount: 1, crackedGradeId: null, dirtyGradeId: null, grades: [{ eggGradeId: "gr1", quantity: 60 }, { eggGradeId: "gr2", quantity: 25 }], @@ -463,6 +467,7 @@ describe("DailyEntryPage structure", () => { describe("DailyEntryPage draft badge", () => { const draftFor = (date: string) => ({ + ...NO_RECORD_HISTORY, id: "de1", farmId: "farm1", houseId: "h1", flockId: "f1", date, status: "Draft", totalEggs: 40, crackedEggs: 1, dirtyEggs: 0, discardedEggs: 0, mortalityCount: 0, crackedGradeId: null, dirtyGradeId: null, grades: [], version: 1, adjustReason: null, voidReason: null, @@ -1081,6 +1086,7 @@ describe("DailyEntryPage i18n wiring (#182, Task 11)", () => { // "submitted" here. it("interpolates the enum-labelled status into the entry-locked banner from the catalog", async () => { const existing: DailyEntry = { + ...NO_RECORD_HISTORY, id: "de1", farmId: "farm1", houseId: "h1", flockId: "f1", date: todayIso(), status: "Submitted", totalEggs: 100, crackedEggs: 2, dirtyEggs: 3, discardedEggs: 5, mortalityCount: 1, crackedGradeId: null, dirtyGradeId: null, grades: [], version: 1, adjustReason: null, voidReason: null, lockedAtUtc: "2026-07-20T10:00:00Z", adjustedFrom: null, diff --git a/web/src/routes/Dashboard.test.tsx b/web/src/routes/Dashboard.test.tsx index ab87a5df9..435314950 100644 --- a/web/src/routes/Dashboard.test.tsx +++ b/web/src/routes/Dashboard.test.tsx @@ -7,6 +7,7 @@ import { } from "../api/cluckwork"; import type { DailyEntry, Flock, SalesOrder, StockRow } from "../api/cluckwork"; import i18n from "../i18n"; +import { NO_RECORD_HISTORY } from "../test/fixtures"; // Keep the real formatMoney; stub the five read endpoints the dashboard fans out. vi.mock("../api/cluckwork", async (importOriginal) => { @@ -28,10 +29,12 @@ const mockOrders = vi.mocked(listOrders); const mockCustomers = vi.mocked(listCustomers); const flock = (id: string, status: string): Flock => ({ + ...NO_RECORD_HISTORY, id, farmId: "f", houseId: "h", name: `Flock ${id}`, breed: "ISA", placementDate: "2026-01-01", initialCount: 100, currentBirds: 98, status, }); const entry = (flockId: string, status: string, totalEggs: number): DailyEntry => ({ + ...NO_RECORD_HISTORY, id: `de-${flockId}`, farmId: "f", houseId: "h", flockId, date: "2026-07-21", status, totalEggs, crackedEggs: 0, dirtyEggs: 0, discardedEggs: 0, mortalityCount: 0, crackedGradeId: null, dirtyGradeId: null, grades: [], diff --git a/web/src/routes/ExpensesPage.test.tsx b/web/src/routes/ExpensesPage.test.tsx index b7c48e869..9ac4a3831 100644 --- a/web/src/routes/ExpensesPage.test.tsx +++ b/web/src/routes/ExpensesPage.test.tsx @@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { screen, within, fireEvent, act, waitFor } from "@testing-library/react"; import { ExpensesPage } from "./ExpensesPage"; import { renderWithProviders } from "../test/renderWithProviders"; -import { account } from "../test/fixtures"; +import { account, NO_RECORD_HISTORY, RECORD_HISTORY } from "../test/fixtures"; import { adjustExpense, createExpense, createExpenseCategory, getExpense, listExpenseCategories, listExpenses, listFlocks, updateExpenseCategory, @@ -56,6 +56,7 @@ const CAT_FEED: ExpenseCategory = { id: "cat-feed", farmId: "farm1", name: "Feed const CAT_UTIL: ExpenseCategory = { id: "cat-util", farmId: "farm1", name: "Utilities", active: true }; const CAT_OLD: ExpenseCategory = { id: "cat-old", farmId: "farm1", name: "Legacy", active: false }; const FLOCK: Flock = { + ...NO_RECORD_HISTORY, id: "f1", farmId: "farm1", houseId: "h1", name: "Hen House 1", breed: "ISA", placementDate: "2026-01-01", initialCount: 100, currentBirds: 98, status: "Active", }; @@ -64,6 +65,7 @@ const FLOCK: Flock = { // account/list currency — the exact case the code comments call out. 1500 minor // units @ 3dp renders "1.500 BHD" (a 2dp formatter could not produce it). const EXP_BHD: Expense = { + ...NO_RECORD_HISTORY, id: "e1", farmId: "farm1", expenseCategoryId: "cat-feed", date: "2026-07-05", description: "Layer feed", amountMinorUnits: 1500, currencyCode: "BHD", currencyMinorUnit: 3, flockId: null, note: null, version: 1, @@ -126,6 +128,29 @@ describe("ExpensesPage list + totals", () => { }); }); +// #494 — the record-history column is a shared component, well tested on its +// own; what is NOT tested by that unit suite is the per-page WIRING that hands +// it the CORRECT row's history object. A page passing the wrong variable (a +// different row, or a stray constant) would go uncaught otherwise. +describe("ExpensesPage record history column (#494)", () => { + it("shows the record history column for the row that has one", async () => { + const EXP_HISTORY: Expense = { ...EXP_BHD, ...RECORD_HISTORY, id: "e-hist", description: "Provenance expense" }; + mockListExpenses.mockResolvedValue({ + items: [EXP_BHD, EXP_HISTORY], totalMinorUnits: 3000, currencyCode: "BHD", currencyMinorUnit: 3, + }); + renderWithProviders(, { token: ADMIN }); + + const historyRow = await screen.findByRole("row", { name: /Provenance expense/ }); + expect(within(historyRow).getByText(/ana@farm\.test/)).toBeInTheDocument(); + expect(within(historyRow).getByText(/bo@farm\.test/)).toBeInTheDocument(); + + // The OTHER row must not carry the history row's data — this is what + // catches every row being wired to the same object. + const otherRow = screen.getByRole("row", { name: /Layer feed/ }); + expect(within(otherRow).queryByText(/ana@farm\.test/)).not.toBeInTheDocument(); + }); +}); + describe("ExpensesPage record expense", () => { // Different scales prove the parse honours the loaded currency's minor unit: // "5" is 5 in JPY (0dp) but would be 500 at 2dp; "1.5" is 1500 in BHD (3dp). diff --git a/web/src/routes/ExpensesPage.tsx b/web/src/routes/ExpensesPage.tsx index 2251730f5..e462a601b 100644 --- a/web/src/routes/ExpensesPage.tsx +++ b/web/src/routes/ExpensesPage.tsx @@ -10,6 +10,7 @@ import { ApiError } from "../api/client"; import { BusyButton } from "../components/BusyButton"; import { Dialog } from "../components/Dialog"; import { DialogError } from "../components/DialogError"; +import { ProvenanceCell } from "../components/ProvenanceCell"; import { useDialogErrors } from "../components/useDialogErrors"; import { usePagedList } from "../components/usePagedList"; import { usePendingAction } from "../components/usePendingAction"; @@ -553,7 +554,8 @@ export function ExpensesPage() { {t("dateHeader")}{t("categoryHeader")}{t("descriptionHeader")}{t("amountHeader")} - {t("flockHeader")}{t("noteHeader")} + {t("flockHeader")}{t("noteHeader")} + {tc("recordHistoryHeader")} @@ -565,6 +567,7 @@ export function ExpensesPage() { {formatMoney(x.amountMinorUnits, x.currencyCode, x.currencyMinorUnit)} {flockName(x.flockId)} {x.note ?? "—"} + {/* Opens the correction dialog — non-mutating, so the spinner belongs to the dialog's Save, not here (#242). */} diff --git a/web/src/routes/FlocksPage.test.tsx b/web/src/routes/FlocksPage.test.tsx index 9b41ff0a4..b4ab19fa7 100644 --- a/web/src/routes/FlocksPage.test.tsx +++ b/web/src/routes/FlocksPage.test.tsx @@ -9,6 +9,7 @@ import { import type { BirdMovement, Flock } from "../api/cluckwork"; import { ApiError } from "../api/client"; import i18n from "../i18n"; +import { NO_RECORD_HISTORY, RECORD_HISTORY } from "../test/fixtures"; // Network seam only; ApiError stays real (errorMessage branches on it), and the // date helpers (todayIso/ageWeeks) stay real too — they are pure and not mocked. @@ -33,14 +34,17 @@ const mockReactivate = vi.mocked(reactivateFlock); const mockListMovements = vi.mocked(listBirdMovements); const ACTIVE: Flock = { + ...NO_RECORD_HISTORY, id: "f1", farmId: "farm1", houseId: "h1", name: "Hen House 1", breed: "ISA Brown", placementDate: "2026-01-01", initialCount: 100, currentBirds: 98, status: "Active", }; const DEPLETED: Flock = { + ...NO_RECORD_HISTORY, id: "f2", farmId: "farm1", houseId: "h1", name: "Depleted Flock", breed: "Leghorn", placementDate: "2025-06-01", initialCount: 200, currentBirds: 0, status: "Depleted", }; const ARCHIVED: Flock = { + ...NO_RECORD_HISTORY, id: "f3", farmId: "farm1", houseId: "h1", name: "Old Coop", breed: "Sussex", placementDate: "2024-01-01", initialCount: 50, currentBirds: 0, status: "Archived", }; @@ -550,6 +554,30 @@ describe("FlocksPage role gating", () => { }); }); +// #494 — the record-history column is a shared component, well tested on its +// own; what is NOT tested by that unit suite is the per-page WIRING that hands +// it the CORRECT row's history object. A page passing the wrong variable (a +// different row, or a stray constant) would go uncaught otherwise. +describe("FlocksPage record history column (#494)", () => { + it("shows the record history column for the row that has one", async () => { + const HISTORY_FLOCK: Flock = { + ...RECORD_HISTORY, + id: "f9", farmId: "farm1", houseId: "h1", name: "Provenance Coop", breed: "Orpington", + placementDate: "2026-02-01", initialCount: 50, currentBirds: 50, status: "Active", + }; + await renderReady(ADMIN, [ACTIVE, HISTORY_FLOCK]); + + const historyRow = screen.getByRole("row", { name: /Provenance Coop/ }); + expect(within(historyRow).getByText(/ana@farm\.test/)).toBeInTheDocument(); + expect(within(historyRow).getByText(/bo@farm\.test/)).toBeInTheDocument(); + + // The OTHER row must not carry the history row's data — this is what + // catches every row being wired to the same object. + const otherRow = screen.getByRole("row", { name: /Hen House 1/ }); + expect(within(otherRow).queryByText(/ana@farm\.test/)).not.toBeInTheDocument(); + }); +}); + // --------------------------------------------------------------------------- // i18n wiring (#182, Task 19, batch B3 — the last B3 screen) // --------------------------------------------------------------------------- diff --git a/web/src/routes/FlocksPage.tsx b/web/src/routes/FlocksPage.tsx index fe3d651cf..54f913837 100644 --- a/web/src/routes/FlocksPage.tsx +++ b/web/src/routes/FlocksPage.tsx @@ -12,6 +12,7 @@ import { BusyButton } from "../components/BusyButton"; import { Dialog } from "../components/Dialog"; import { DialogError } from "../components/DialogError"; import { NumberField } from "../components/NumberField"; +import { ProvenanceCell } from "../components/ProvenanceCell"; import { StatusBadge } from "../components/StatusBadge"; import { useConfirm } from "../components/useConfirm"; import { useDialogErrors } from "../components/useDialogErrors"; @@ -344,7 +345,8 @@ export function FlocksPage() { {t("nameHeader")}{t("breedHeader")}{t("placedHeader")}{t("ageHeader")} - {t("birdsHeader")}{t("statusHeader")} + {t("birdsHeader")}{t("statusHeader")} + {tc("recordHistoryHeader")} @@ -360,6 +362,7 @@ export function FlocksPage() { / {f.initialCount}} + ))} From 01d7573eba999e20522d381f92d842698b3fc294 Mon Sep 17 00:00:00 2001 From: mforce <> Date: Mon, 10 Aug 2026 20:19:51 -0700 Subject: [PATCH 7/9] feat(api,web): record draft edits, and keep a change whose creation predates #494 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- specs/product/GLOSSARY.md | 11 +- .../Common/AuditActions.cs | 10 ++ .../Features/Audit/EntityProvenance.cs | 8 +- .../RecordDailyEntryHandler.cs | 10 ++ .../Sales/AddOrderItem/AddOrderItemHandler.cs | 4 + .../RemoveOrderItem/RemoveOrderItemHandler.cs | 6 + .../UpdateOrderItem/UpdateOrderItemHandler.cs | 4 + .../Repositories/AuditEventRepository.cs | 85 +++++++++----- .../AuditProvenanceTests.cs | 106 +++++++++++++++++- .../SubmitDailyEntryTests.cs | 7 +- web/src/components/ProvenanceCell.test.tsx | 18 ++- web/src/components/ProvenanceCell.tsx | 25 +++-- web/src/i18n/en.ts | 9 +- web/src/i18n/enums.ts | 9 +- web/src/i18n/es.ts | 9 +- web/src/i18n/tl.ts | 11 +- 16 files changed, 273 insertions(+), 59 deletions(-) diff --git a/specs/product/GLOSSARY.md b/specs/product/GLOSSARY.md index c2b4809cd..ec8ecff85 100644 --- a/specs/product/GLOSSARY.md +++ b/specs/product/GLOSSARY.md @@ -448,10 +448,13 @@ created it. So does **cancelling** a draft order: `SalesOrder.Cancel` kills the record rather than promoting it, so cancelling your own order stays a reportable change. -Editing a draft is **not** on the trail at all — re-recording a daily entry, or -adding, changing and removing sales-order lines, write no event, because none of -them alters stock. A draft order whose lines you are still assembling therefore -reports no change, which is correct rather than missing. +Editing a draft **is** recorded, even though it alters no stock, because it is +the only thing binding a person to the numbers. Without it, someone who rewrites +a colleague's draft before it is submitted leaves no trace and the submitter is +credited with their work. The rule above is what keeps that quiet in the ordinary +case: your own edits to your own draft are part of writing it, so they are hidden +along with your own submit, and only a **different** person's edit surfaces as a +change. Two further consequences follow from being derived rather than stored. A record created **before #494** diff --git a/src/Cluckwork.Application/Common/AuditActions.cs b/src/Cluckwork.Application/Common/AuditActions.cs index 7da0de351..fccaf0e46 100644 --- a/src/Cluckwork.Application/Common/AuditActions.cs +++ b/src/Cluckwork.Application/Common/AuditActions.cs @@ -62,6 +62,16 @@ public static class AuditActions // numbers official, which #494 shows as the entry's last change whenever // that is someone other than the person who drafted it. public const string DailyEntrySubmit = "DailyEntry.Submit"; + // Editing a DRAFT. Recorded even though it moves no stock, because it is + // the only thing that binds a person to the numbers: without it, a farmhand + // who rewrites somebody else's draft before it is submitted leaves no trace + // at all, and the submitter is credited with their work. #494's record + // history hides these when the editor is the record's own creator, so the + // common case of drafting your own entry stays quiet. + public const string DailyEntryUpdate = "DailyEntry.Update"; + public const string SalesOrderAddItem = "SalesOrder.AddItem"; + public const string SalesOrderUpdateItem = "SalesOrder.UpdateItem"; + public const string SalesOrderRemoveItem = "SalesOrder.RemoveItem"; public const string SalesOrderCreate = "SalesOrder.Create"; // Confirming allocates stock FIFO — the order's equivalent of submitting a // daily entry, and treated identically by #494's record history. diff --git a/src/Cluckwork.Application/Features/Audit/EntityProvenance.cs b/src/Cluckwork.Application/Features/Audit/EntityProvenance.cs index a717db240..118715726 100644 --- a/src/Cluckwork.Application/Features/Audit/EntityProvenance.cs +++ b/src/Cluckwork.Application/Features/Audit/EntityProvenance.cs @@ -24,7 +24,13 @@ namespace Cluckwork.Application.Features.Audit; // else on the record's own page — DailyEntry has no SubmittedAt, only // LockedAtUtc. Null for anything with no promotion step (flocks, egg grades, // expenses) and for a draft still awaiting one. +// Created* is nullable because the two halves are INDEPENDENT. A record from +// before #494 has no creation event and never gets one — but it may well have +// changes with real attribution, and an earlier revision discarded those too by +// keying the whole result off the creation. Refusing to invent a creator and +// throwing away a change we can prove are separate decisions; only the first +// was intended (adversarial review of PR #503). public sealed record EntityProvenance( - string CreatedByEmail, DateTimeOffset CreatedAtUtc, + string? CreatedByEmail, DateTimeOffset? CreatedAtUtc, string? LastChangedByEmail, DateTimeOffset? LastChangedAtUtc, DateTimeOffset? MadeOfficialAtUtc); diff --git a/src/Cluckwork.Application/Features/DailyEntries/RecordDailyEntry/RecordDailyEntryHandler.cs b/src/Cluckwork.Application/Features/DailyEntries/RecordDailyEntry/RecordDailyEntryHandler.cs index 3a42bcfc7..8013bc414 100644 --- a/src/Cluckwork.Application/Features/DailyEntries/RecordDailyEntry/RecordDailyEntryHandler.cs +++ b/src/Cluckwork.Application/Features/DailyEntries/RecordDailyEntry/RecordDailyEntryHandler.cs @@ -108,6 +108,16 @@ public async Task> HandleAsync( if (result.IsFailure) return Result.Failure(result.Error).LogFailure(logger, "RecordDailyEntry"); + // #494 — a later call against the same natural key REWRITES an existing + // draft. Recorded after the failure check above, so a rejected re-record + // leaves no trace. It moves no stock, but it is the only thing binding a + // person to the numbers: without it, someone rewriting a colleague's + // draft before submission is invisible and the submitter is credited + // with their work. Record history hides this when the editor IS the + // creator, so drafting your own entry stays quiet. + if (existing is not null) + await audit.WriteAsync(AuditActions.DailyEntryUpdate, nameof(DailyEntry), entry.Id, ct: ct); + await unitOfWork.SaveChangesAsync(ct); logger.LogInformation( "Daily entry {DailyEntryId} recorded for flock {FlockId} on {EntryDate}: {TotalEggs} eggs", diff --git a/src/Cluckwork.Application/Features/Sales/AddOrderItem/AddOrderItemHandler.cs b/src/Cluckwork.Application/Features/Sales/AddOrderItem/AddOrderItemHandler.cs index 1857fffc2..8264ca230 100644 --- a/src/Cluckwork.Application/Features/Sales/AddOrderItem/AddOrderItemHandler.cs +++ b/src/Cluckwork.Application/Features/Sales/AddOrderItem/AddOrderItemHandler.cs @@ -13,6 +13,7 @@ public sealed class AddOrderItemHandler( IProductRepository products, IEggGradeRepository eggGrades, IEggUnitConversionRepository conversions, + IAuditWriter audit, IUnitOfWork unitOfWork) { public async Task> HandleAsync( @@ -101,6 +102,9 @@ public async Task> HandleAsync( if (result.IsFailure) return Result.Failure(result.Error); + // #494 — see RemoveOrderItemHandler: draft-only, recorded for attribution. + await audit.WriteAsync(AuditActions.SalesOrderAddItem, nameof(SalesOrder), order.Id, ct: ct); + // EF assigns the item id during save (deliberately not client-set). await unitOfWork.SaveChangesAsync(ct); return Result.Success(result.Value.Id); diff --git a/src/Cluckwork.Application/Features/Sales/RemoveOrderItem/RemoveOrderItemHandler.cs b/src/Cluckwork.Application/Features/Sales/RemoveOrderItem/RemoveOrderItemHandler.cs index ebf9ed62d..b61b101d8 100644 --- a/src/Cluckwork.Application/Features/Sales/RemoveOrderItem/RemoveOrderItemHandler.cs +++ b/src/Cluckwork.Application/Features/Sales/RemoveOrderItem/RemoveOrderItemHandler.cs @@ -7,6 +7,7 @@ namespace Cluckwork.Application.Features.Sales.RemoveOrderItem; public sealed class RemoveOrderItemHandler( ISalesOrderRepository orders, + IAuditWriter audit, IUnitOfWork unitOfWork) { public async Task HandleAsync(Guid orderId, Guid itemId, CancellationToken ct) @@ -19,6 +20,11 @@ public async Task HandleAsync(Guid orderId, Guid itemId, CancellationTok if (result.IsFailure) return result; + // #494 — a draft-only edit, recorded so that reworking somebody else's + // order is attributable. Hidden by record history when the editor is the + // order's own creator. + await audit.WriteAsync(AuditActions.SalesOrderRemoveItem, nameof(SalesOrder), order.Id, ct: ct); + await unitOfWork.SaveChangesAsync(ct); return Result.Success(); } diff --git a/src/Cluckwork.Application/Features/Sales/UpdateOrderItem/UpdateOrderItemHandler.cs b/src/Cluckwork.Application/Features/Sales/UpdateOrderItem/UpdateOrderItemHandler.cs index 824b264be..0af463898 100644 --- a/src/Cluckwork.Application/Features/Sales/UpdateOrderItem/UpdateOrderItemHandler.cs +++ b/src/Cluckwork.Application/Features/Sales/UpdateOrderItem/UpdateOrderItemHandler.cs @@ -7,6 +7,7 @@ namespace Cluckwork.Application.Features.Sales.UpdateOrderItem; public sealed class UpdateOrderItemHandler( ISalesOrderRepository orders, + IAuditWriter audit, IUnitOfWork unitOfWork) { public async Task HandleAsync(UpdateOrderItemCommand command, CancellationToken ct) @@ -25,6 +26,9 @@ public async Task HandleAsync(UpdateOrderItemCommand command, Cancellati if (result.IsFailure) return result; + // #494 — see RemoveOrderItemHandler: draft-only, recorded for attribution. + await audit.WriteAsync(AuditActions.SalesOrderUpdateItem, nameof(SalesOrder), order.Id, ct: ct); + await unitOfWork.SaveChangesAsync(ct); return Result.Success(); } diff --git a/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs b/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs index b5549474e..a9eb90cda 100644 --- a/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs +++ b/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs @@ -93,17 +93,22 @@ SELECT DISTINCT ON ("EntityId") * .AsNoTracking() .ToListAsync(ct); - // The last change is the latest event that is neither the creation NOR a - // PROMOTION by the creator. That second exclusion is the whole of #494's - // "drafting a record and making it official is one act, not a change" - // rule: writing the day's numbers down and submitting them, with nothing - // altered in between, should not read as if somebody corrected them. + // The last change is the latest event that is neither the creation NOR + // the creator's OWN DRAFTING of it. That second exclusion is the whole of + // #494's "writing a record and making it official is one act, not a + // change" rule: filling in the day's numbers and submitting them should + // not read as if somebody corrected your work. // - // A promotion is the action that turns a draft into the official record: - // DailyEntry.Submit and SalesOrder.Confirm are the two, and both mint - // stock at that moment. Deliberately NOT every draft-to-terminal move — - // SalesOrder.Cancel kills a draft rather than promoting it, so - // cancelling your own order stays a reportable change. + // Drafting = editing a draft, plus the promotion that ends it + // (DailyEntry.Submit, SalesOrder.Confirm — the moments stock is minted or + // allocated). Excluded ONLY when the actor is the creator, which is what + // makes a shared draft attributable: rewrite a colleague's numbers before + // they submit and your edit is the last change, even though they are + // still the creator and their own submit stays hidden. + // + // Deliberately NOT every draft-to-terminal move — SalesOrder.Cancel kills + // a draft rather than making it official, so cancelling your own order + // stays a reportable change. // // Stated as an EXCLUSION from the candidate set, deliberately — not as // "if the last event is a self-promotion, report nothing". The latter @@ -145,15 +150,26 @@ SELECT DISTINCT ON ("EntityId") * // NO test pinning the current arbitrary outcome: that would promote a // known loss into a specification. // EXACT actions, not a "%.Submit"/"%.Confirm" pattern. Creation rightly - // uses a wildcard — any ".Create" IS a creation — but promotion is a - // closed two-member set, and a pattern would silently swallow a future - // action that merely ends the same way (a Payment.Confirm, say), dropping - // its actor from history with no error (pi review of PR #503). + // uses a wildcard — any ".Create" IS a creation — but these are closed + // sets, and a pattern would silently swallow a future action that merely + // ends the same way (a Payment.Confirm, say), dropping its actor from + // history with no error (pi review of PR #503). + // + // TWO sets, deliberately not one. Promotion alone answers "when did this + // become official"; drafting is what the creator may do to their own + // record without it counting as a change. var promotionActions = new[] { AuditActions.DailyEntrySubmit, AuditActions.SalesOrderConfirm, }; + var draftingActions = promotionActions.Concat(new[] + { + AuditActions.DailyEntryUpdate, + AuditActions.SalesOrderAddItem, + AuditActions.SalesOrderUpdateItem, + AuditActions.SalesOrderRemoveItem, + }).ToArray(); var latest = await db.AuditEvents.FromSqlInterpolated($""" WITH creator AS ( SELECT DISTINCT ON ("EntityId") "EntityId", "ActorUserId" @@ -171,7 +187,7 @@ SELECT DISTINCT ON (e."EntityId") e.* AND e."EntityType" = {entityType} AND e."EntityId" = ANY({ids}) AND e."Action" NOT LIKE {createAction} - AND NOT (e."Action" = ANY({promotionActions}) + AND NOT (e."Action" = ANY({draftingActions}) AND e."ActorUserId" IS NOT DISTINCT FROM c."ActorUserId") ORDER BY e."EntityId", e."OccurredAtUtc" DESC, e."Id" DESC """) @@ -203,20 +219,33 @@ SELECT DISTINCT ON ("EntityId") * .AsNoTracking() .ToListAsync(ct); - // Keyed off `created`: a record whose trail holds no creation event — - // anything predating #494 — reports nothing at all rather than - // attributing it to whoever happened to correct it first. + // Keyed off EVERY id with any event, not off `created` alone. A record + // predating #494 has no creation event and never gets one — but it can + // still have changes with real attribution, and keying off `created` + // discarded those too, rendering the column completely blank for a flock + // somebody archived yesterday (adversarial review of PR #503). + // + // Refusing to invent a creator and throwing away a provable change are + // separate decisions. Only the first is intended: absent creation stays + // null rather than being filled in from the first correction. + var createdById = created.ToDictionary(e => e.EntityId); var latestById = latest.ToDictionary(e => e.EntityId); var promotedById = promoted.ToDictionary(e => e.EntityId); - return created.ToDictionary( - e => e.EntityId, - e => - { - var last = latestById.GetValueOrDefault(e.EntityId); - return new EntityProvenance( - e.ActorEmail, e.OccurredAtUtc, - last?.ActorEmail, last?.OccurredAtUtc, - promotedById.GetValueOrDefault(e.EntityId)?.OccurredAtUtc); - }); + + return createdById.Keys + .Concat(latestById.Keys) + .Concat(promotedById.Keys) + .Distinct() + .ToDictionary( + id => id, + id => + { + var create = createdById.GetValueOrDefault(id); + var last = latestById.GetValueOrDefault(id); + return new EntityProvenance( + create?.ActorEmail, create?.OccurredAtUtc, + last?.ActorEmail, last?.OccurredAtUtc, + promotedById.GetValueOrDefault(id)?.OccurredAtUtc); + }); } } diff --git a/tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs b/tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs index b348d1838..c8b3b4dcd 100644 --- a/tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs @@ -67,10 +67,15 @@ public async Task Provenance_WithOneEvent_ReportsNoChange() // A record that predates #494 has no ".Create" event, only corrections. // Naming its first corrector as its author would be a claim the trail does - // not support — an outright false attribution, not merely missing data. It - // reports no creator at all, and the column renders blank. + // not support — an outright false attribution, not merely missing data. + // + // But refusing to invent a creator is a separate decision from discarding a + // change we DO have attribution for, and an earlier revision made both at + // once by keying the whole result off the creation event: a flock archived + // by cy yesterday rendered completely blank (adversarial review of PR #503). + // Now the two halves are independent — no creator, real last change. [Fact] - public async Task Provenance_ForALegacyRecordWithOnlyCorrections_NamesNoCreator() + public async Task Provenance_ForALegacyRecordWithOnlyCorrections_NamesNoCreatorButKeepsTheChange() { var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); var entityId = Guid.NewGuid(); @@ -81,7 +86,11 @@ await SeedEventsAsync(accountId, var result = await WithRepositoryAsync(accountId, repo => repo.GetProvenanceAsync("Flock", [entityId])); - Assert.False(result.ContainsKey(entityId)); + var provenance = result[entityId]; + Assert.Null(provenance.CreatedByEmail); + Assert.Null(provenance.CreatedAtUtc); + Assert.Equal("cy@farm.test", provenance.LastChangedByEmail); + Assert.Equal(Base.AddMinutes(90), provenance.LastChangedAtUtc); } [Fact] @@ -251,6 +260,57 @@ await SeedEventsAsync(accountId, Assert.Null(result[entityId].MadeOfficialAtUtc); } + // The masked-co-author case. bo rewrites ana's draft and ana submits it: the + // numbers that became stock are bo's, and before draft edits were recorded + // this read "created by ana, never changed" — crediting ana with bo's work + // (adversarial review of PR #503). + // + // ana's own submit is still hidden, because it is her own drafting. bo's + // edit is not, because bo is not the creator. + [Fact] + public async Task Provenance_WhenSomeoneElseEditsTheDraft_NamesThemEvenIfTheCreatorSubmits() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var entityId = Guid.NewGuid(); + var ana = Guid.NewGuid(); + await SeedEventsAsync(accountId, + Event(accountId, entityId, "DailyEntry.Create", "ana@farm.test", 0, "DailyEntry", ana), + Event(accountId, entityId, "DailyEntry.Update", "bo@farm.test", 2, "DailyEntry"), + Event(accountId, entityId, "DailyEntry.Submit", "ana@farm.test", 5, "DailyEntry", ana)); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("DailyEntry", [entityId])); + + var provenance = result[entityId]; + Assert.Equal("ana@farm.test", provenance.CreatedByEmail); + Assert.Equal("bo@farm.test", provenance.LastChangedByEmail); + Assert.Equal(Base.AddMinutes(2), provenance.LastChangedAtUtc); + // The submit still happened, and when it happened is still reported. + Assert.Equal(Base.AddMinutes(5), provenance.MadeOfficialAtUtc); + } + + // The other half of the same rule, and the reason it is keyed on the actor: + // editing your OWN draft before submitting must stay quiet, or every entry + // reads as having been corrected by the person who wrote it. + [Fact] + public async Task Provenance_WhenTheCreatorEditsTheirOwnDraft_ReportsNoChange() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var entityId = Guid.NewGuid(); + var ana = Guid.NewGuid(); + await SeedEventsAsync(accountId, + Event(accountId, entityId, "DailyEntry.Create", "ana@farm.test", 0, "DailyEntry", ana), + Event(accountId, entityId, "DailyEntry.Update", "ana@farm.test", 2, "DailyEntry", ana), + Event(accountId, entityId, "DailyEntry.Submit", "ana@farm.test", 5, "DailyEntry", ana)); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("DailyEntry", [entityId])); + + var provenance = result[entityId]; + Assert.Null(provenance.LastChangedByEmail); + Assert.Null(provenance.LastChangedAtUtc); + } + // The case the whole option exists for: a worker writes the numbers, someone // else makes them official. Both people must be visible — that is the // accountability the submit step is for. @@ -735,6 +795,44 @@ public async Task DailyEntryList_CarriesProvenance() // belongs in the trail on its own merits. It is also the ONLY source for the // "who made this official" half of a daily entry's record history — without // this event, a manager submitting a worker's draft leaves no trace at all. + // Re-recording against the same natural key rewrites an existing draft. It + // moves no stock, but it is the only thing binding a person to the numbers, + // so it is on the trail — see Provenance_WhenSomeoneElseEditsTheDraft_... + [Fact] + public async Task DailyEntryDraftEdit_WritesAnAuditEvent() + { + var email = $"u-{Guid.NewGuid():N}@test.local"; + var accountId = await factory.SeedAccountWithUserAsync(email); + var farmId = Guid.NewGuid(); + var houseId = Guid.NewGuid(); + var grades = await factory.SeedEggGradesAsync(accountId, farmId, "Large"); + var flockId = await factory.SeedFlockAsync(accountId, farmId); + var client = factory.CreateAuthedClient(await factory.LoginForAccessTokenAsync(email)); + + // Same natural key both times, so the second call appends to the first + // entry rather than creating a second one. + object Body(int total) => new + { + farmId, houseId, flockId, + date = DateOnly.FromDateTime(DateTime.UtcNow.Date), + totalEggs = total, crackedEggs = 0, dirtyEggs = 0, discardedEggs = 0, + mortalityCount = 0, + grades = new[] { new { eggGradeId = grades["Large"], quantity = total } }, + }; + + var id = await CreatedIdAsync(await client.PostWithKeyAsync( + "/api/v1/daily-entries", Guid.NewGuid().ToString(), Body(100))); + var again = await client.PostWithKeyAsync( + "/api/v1/daily-entries", Guid.NewGuid().ToString(), Body(500)); + Assert.Equal(HttpStatusCode.Created, again.StatusCode); + + var actions = await ActionsForAsync(accountId, id, "DailyEntry"); + Assert.Contains("DailyEntry.Update", actions); + // Still exactly one creation: two would give the entry two candidate + // authors and let the later one win. + Assert.Single(actions, a => a == "DailyEntry.Create"); + } + [Fact] public async Task DailyEntrySubmit_WritesAnAuditEvent() { diff --git a/tests/Cluckwork.Api.IntegrationTests/SubmitDailyEntryTests.cs b/tests/Cluckwork.Api.IntegrationTests/SubmitDailyEntryTests.cs index 8adf1ad10..2e24a208a 100644 --- a/tests/Cluckwork.Api.IntegrationTests/SubmitDailyEntryTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/SubmitDailyEntryTests.cs @@ -327,6 +327,11 @@ public async Task DailyEntry_ReRecordedOnTheSameKey_WritesOnlyOneCreateEvent() .Select(e => e.Action) .ToListAsync()); - Assert.Equal(["DailyEntry.Create"], events); + // The re-record DOES leave a trace now — a draft edit is attributable, + // so that rewriting a colleague's numbers before submission is visible + // (#494). What must never happen is a second CREATION, which would give + // the entry two candidate authors and let the later one win. + Assert.Equal(["DailyEntry.Create", "DailyEntry.Update"], events); + Assert.Single(events, a => a == "DailyEntry.Create"); } } diff --git a/web/src/components/ProvenanceCell.test.tsx b/web/src/components/ProvenanceCell.test.tsx index 136262fc1..948385acc 100644 --- a/web/src/components/ProvenanceCell.test.tsx +++ b/web/src/components/ProvenanceCell.test.tsx @@ -87,8 +87,7 @@ describe("ProvenanceCell", () => { expect(screen.getByText(/bo@farm\.test/)).toBeInTheDocument(); }); - it("renders a placeholder for a record that predates the feature", () => { - // No backfill: all four arrive null together and there is nothing to claim. + it("renders a placeholder only when there is nothing at all to say", () => { renderCell({ createdByEmail: null, createdAtUtc: null, @@ -99,6 +98,21 @@ describe("ProvenanceCell", () => { expect(screen.getByText("—")).toBeInTheDocument(); }); + it("still reports the change on a record whose creation predates the feature", () => { + // No backfill, so no creator line — but the change has real attribution and + // must not go down with it. An earlier revision rendered this whole cell as + // the placeholder, discarding cy entirely. + renderCell({ + createdByEmail: null, + createdAtUtc: null, + lastChangedByEmail: "cy@farm.test", + lastChangedAtUtc: CHANGED, + }); + expect(screen.getByText(/cy@farm\.test/)).toBeInTheDocument(); + expect(screen.queryByText(/Created by/i)).not.toBeInTheDocument(); + expect(screen.queryByText("—")).not.toBeInTheDocument(); + }); + // #494 — the promotion instant. A self-submit is excluded from "last changed // by", so without this line the moment stock was minted would appear nowhere // on the record's own page. diff --git a/web/src/components/ProvenanceCell.tsx b/web/src/components/ProvenanceCell.tsx index 01b4d62d6..06678c867 100644 --- a/web/src/components/ProvenanceCell.tsx +++ b/web/src/components/ProvenanceCell.tsx @@ -34,21 +34,26 @@ export function ProvenanceCell({ const { t } = useTranslation("common"); const { createdByEmail, createdAtUtc, lastChangedByEmail, lastChangedAtUtc } = history; - // Null together: a record created before #494 shipped has no creation event - // and gets no backfill, so there is nothing to claim about it. - if (!createdByEmail || !createdAtUtc) { - return —; - } - + // The three lines are INDEPENDENT. A record predating #494 has no creation + // event and never gets one, but it can still carry a change with real + // attribution — so a missing creator hides that one line, not the whole cell. + // The placeholder is for a record with nothing at all to say. + const created = createdByEmail && createdAtUtc; const changed = lastChangedByEmail && lastChangedAtUtc; const officialAt = official && history.madeOfficialAtUtc; + if (!created && !changed && !officialAt) { + return —; + } + return ( -
{t("recordHistory.createdBy", { - email: createdByEmail, - at: formatInstant(createdAtUtc), - })}
+ {created && ( +
{t("recordHistory.createdBy", { + email: createdByEmail, + at: formatInstant(createdAtUtc), + })}
+ )} {officialAt && (
{t( official === "submitted" diff --git a/web/src/i18n/en.ts b/web/src/i18n/en.ts index 977b894ce..52d671331 100644 --- a/web/src/i18n/en.ts +++ b/web/src/i18n/en.ts @@ -2051,7 +2051,11 @@ export const en = { "auditAction.EggLot.Movement": "Stock written off / recounted", "auditAction.Flock.Create": "Flock created", "auditAction.DailyEntry.Create": "Daily entry created", + "auditAction.DailyEntry.Update": "Daily entry draft edited", "auditAction.DailyEntry.Submit": "Daily entry submitted", + "auditAction.SalesOrder.AddItem": "Order line added", + "auditAction.SalesOrder.UpdateItem": "Order line changed", + "auditAction.SalesOrder.RemoveItem": "Order line removed", "auditAction.SalesOrder.Create": "Sales order created", "auditAction.SalesOrder.Confirm": "Sales order confirmed", "auditAction.SalesOrder.Cancel": "Sales order cancelled", @@ -2551,8 +2555,9 @@ export const en = { + "Either way the History column tells you WHEN it was submitted, since that is when the eggs " + "entered stock. Confirming a sales order works the same way, and shows when stock was allocated. " + "Correcting an entry after it is locked always shows, " - + "even if you created it, and so does cancelling your own draft order. Editing a draft — re-recording " - + "an entry, or adding and removing order lines — is not tracked, because none of it moves stock.", + + "even if you created it, and so does cancelling your own draft order. Editing your own draft before " + + "submitting it stays hidden — that is just writing it. But if somebody else edits your draft, their " + + "change is shown, so the person whose numbers went into stock is never invisible.", auditRecordHistoryOlder: "Records created before this was added show a blank History column. Nothing is missing — there is " + "simply no creation entry in the log for them, and none is invented after the fact.", diff --git a/web/src/i18n/enums.ts b/web/src/i18n/enums.ts index dfd31fde1..8f87e1796 100644 --- a/web/src/i18n/enums.ts +++ b/web/src/i18n/enums.ts @@ -290,8 +290,9 @@ export const AUDIT_ACTION_VALUES = [ "Account.SetBanner", "Account.RemoveBanner", "Account.UpdateSettings", "Product.Create", "Product.Update", "Product.Activate", "Product.Deactivate", "EggUnitConversion.Update", "EggLot.Movement", - "Flock.Create", "DailyEntry.Create", "DailyEntry.Submit", "SalesOrder.Create", - "SalesOrder.Confirm", "SalesOrder.Cancel", + "Flock.Create", "DailyEntry.Create", "DailyEntry.Update", "DailyEntry.Submit", + "SalesOrder.Create", "SalesOrder.Confirm", "SalesOrder.Cancel", + "SalesOrder.AddItem", "SalesOrder.UpdateItem", "SalesOrder.RemoveItem", "Expense.Create", "EggGrade.Create", ] as const; export type AuditActionValue = (typeof AUDIT_ACTION_VALUES)[number]; @@ -336,7 +337,11 @@ const AUDIT_ACTION_KEYS = { "EggLot.Movement": "enums:auditAction.EggLot.Movement", "Flock.Create": "enums:auditAction.Flock.Create", "DailyEntry.Create": "enums:auditAction.DailyEntry.Create", + "DailyEntry.Update": "enums:auditAction.DailyEntry.Update", "DailyEntry.Submit": "enums:auditAction.DailyEntry.Submit", + "SalesOrder.AddItem": "enums:auditAction.SalesOrder.AddItem", + "SalesOrder.UpdateItem": "enums:auditAction.SalesOrder.UpdateItem", + "SalesOrder.RemoveItem": "enums:auditAction.SalesOrder.RemoveItem", "SalesOrder.Create": "enums:auditAction.SalesOrder.Create", "SalesOrder.Confirm": "enums:auditAction.SalesOrder.Confirm", "SalesOrder.Cancel": "enums:auditAction.SalesOrder.Cancel", diff --git a/web/src/i18n/es.ts b/web/src/i18n/es.ts index f0486e4b1..40e6d2d8b 100644 --- a/web/src/i18n/es.ts +++ b/web/src/i18n/es.ts @@ -1629,7 +1629,11 @@ export const es = { "auditAction.EggLot.Movement": "Existencias dadas de baja / recontadas", "auditAction.Flock.Create": "Lote creado", "auditAction.DailyEntry.Create": "Registro diario creado", + "auditAction.DailyEntry.Update": "Borrador de registro diario editado", "auditAction.DailyEntry.Submit": "Registro diario enviado", + "auditAction.SalesOrder.AddItem": "Línea de pedido agregada", + "auditAction.SalesOrder.UpdateItem": "Línea de pedido modificada", + "auditAction.SalesOrder.RemoveItem": "Línea de pedido eliminada", "auditAction.SalesOrder.Create": "Pedido de venta creado", "auditAction.SalesOrder.Confirm": "Pedido de venta confirmado", "auditAction.SalesOrder.Cancel": "Pedido de venta cancelado", @@ -2144,8 +2148,9 @@ export const es = { + "momento en que los huevos entraron en existencias. Confirmar un pedido de venta funciona igual y " + "muestra cuándo se asignaron las existencias. Corregir un registro ya bloqueado " + "siempre se muestra, aunque tú lo hayas creado, y cancelar tu propio pedido en borrador también. " - + "Editar un borrador — volver a registrar una entrada, o agregar y quitar líneas de un pedido — no " - + "se registra, porque nada de eso mueve existencias.", + + "Editar tu propio borrador antes de enviarlo queda oculto: eso es simplemente escribirlo. Pero si " + + "otra persona edita tu borrador, su cambio sí se muestra, para que nunca sea invisible quien puso " + + "los números que entraron en existencias.", auditRecordHistoryOlder: "Los registros creados antes de que esto se agregara muestran la columna Historial en blanco. No " + "falta nada — simplemente no hay una entrada de creación en el registro para ellos, y no se " diff --git a/web/src/i18n/tl.ts b/web/src/i18n/tl.ts index 0c4f36fa6..7f2614fba 100644 --- a/web/src/i18n/tl.ts +++ b/web/src/i18n/tl.ts @@ -1692,7 +1692,11 @@ export const tl = { "auditAction.EggLot.Movement": "Na-write off / na-recount ang stock", "auditAction.Flock.Create": "Nagawa ang kawan", "auditAction.DailyEntry.Create": "Nagawa ang pang-araw-araw na entry", + "auditAction.DailyEntry.Update": "Na-edit ang draft ng pang-araw-araw na entry", "auditAction.DailyEntry.Submit": "Naisumite ang pang-araw-araw na entry", + "auditAction.SalesOrder.AddItem": "Nagdagdag ng linya sa order", + "auditAction.SalesOrder.UpdateItem": "Binago ang linya ng order", + "auditAction.SalesOrder.RemoveItem": "Inalis ang linya ng order", "auditAction.SalesOrder.Create": "Nagawa ang order ng benta", "auditAction.SalesOrder.Confirm": "Nakumpirma ang order ng benta", "auditAction.SalesOrder.Cancel": "Kinansela ang order ng benta", @@ -2225,9 +2229,10 @@ export const tl = { + "rin ang pagkumpirma ng order ng benta. Sa alinmang paraan, ipinapakita ng column na History kung " + "KAILAN ito naisumite, dahil iyon ang sandaling pumasok sa stock ang mga itlog, at kung kailan " + "inilaan ang stock para sa order. Ang pagwawasto sa naka-lock na entry ay laging ipinapakita, " - + "kahit ikaw ang gumawa nito, gayundin ang pagkansela ng sarili mong draft na order. Ang pag-edit ng " - + "draft — muling pagtatala ng entry, o pagdaragdag at pagtanggal ng linya sa order — ay hindi " - + "sinusubaybayan, dahil wala roong gumagalaw sa stock.", + + "kahit ikaw ang gumawa nito, gayundin ang pagkansela ng sarili mong draft na order. Nakatago ang " + + "pag-edit mo sa sarili mong draft bago isumite — bahagi lang iyon ng pagsulat nito. Ngunit kung iba " + + "ang nag-edit ng draft mo, ipinapakita ang pagbabago nila, kaya hindi kailanman nawawala ang taong " + + "naglagay ng mga bilang na pumasok sa stock.", auditRecordHistoryOlder: "Ang mga record na ginawa bago ito idagdag ay may blangkong kolum na Kasaysayan. Walang nawawala — " + "wala lang talagang entry ng paggawa sa log para sa kanila, at walang iniimbentong bago pagkatapos.", From 4486b1af9fc2ea5c502e80981c4cc5c12a34bef7 Mon Sep 17 00:00:00 2001 From: mforce <> Date: Mon, 10 Aug 2026 20:49:04 -0700 Subject: [PATCH 8/9] fix(api): guard every member of the drafting set and every EntityType predicate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- specs/product/GLOSSARY.md | 20 ++-- .../Repositories/AuditEventRepository.cs | 37 +++++--- .../AuditProvenanceTests.cs | 91 +++++++++++++++++++ .../SubmitDailyEntryTests.cs | 3 + web/src/i18n/en.ts | 5 +- web/src/i18n/es.ts | 7 +- web/src/i18n/tl.ts | 6 +- 7 files changed, 139 insertions(+), 30 deletions(-) diff --git a/specs/product/GLOSSARY.md b/specs/product/GLOSSARY.md index ec8ecff85..77537654c 100644 --- a/specs/product/GLOSSARY.md +++ b/specs/product/GLOSSARY.md @@ -442,11 +442,13 @@ own page: a daily entry stores no submission timestamp of its own, only `LockedAtUtc`. Blank for a draft awaiting promotion, and absent entirely on flocks, egg grades and expenses, which have no promotion step. -The exception is keyed on the action, not on the actor, and covers promotion -only. Correcting a locked entry always shows, including when the corrector -created it. So does **cancelling** a draft order: `SalesOrder.Cancel` kills the -record rather than promoting it, so cancelling your own order stays a reportable -change. +The exception covers **drafting** — the creator's own edits to their own draft, +and the promotion that ends it — and nothing else. Correcting a locked entry +always shows, including when the corrector created it. So does **cancelling** a +draft order: `SalesOrder.Cancel` kills the record rather than making it official, +so cancelling your own order stays a reportable change. It is keyed on the action +*and* the actor together, which is why the same draft edit is hidden for the +person who created the record and shown for anybody else. Editing a draft **is** recorded, even though it alters no stock, because it is the only thing binding a person to the numbers. Without it, someone who rewrites @@ -457,9 +459,11 @@ along with your own submit, and only a **different** person's edit surfaces as a change. Two further consequences follow from being derived rather than stored. A record -created **before #494** -has no creation event and is **never backfilled**, so its history column is -blank. And who can see it is exactly who could already read the record — it adds +created **before #494** has no creation event and is **never backfilled**, so it +shows no "created by" line — but that half is independent of the rest: if it has +since been changed, that change still shows, with the created line simply absent. +The column is entirely blank only for a record with nothing on the trail at all. +And who can see it is exactly who could already read the record — it adds no gate of its own, so a worker reading their own daily entry also sees which manager corrected it. The actor is shown as the plain **email snapshot** the audit log already keeps, deliberately not a live-joined name: a later rename or diff --git a/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs b/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs index a9eb90cda..7ada87b59 100644 --- a/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs +++ b/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs @@ -60,14 +60,16 @@ public async Task> GetProvenanceAsyn // filter over this — IgnoreQueryFilters opts out of that, so the hand-written // AccountId predicate is then the ONLY thing scoping the read to the tenant. // - // There are THREE such predicates below, not one: the `created` query, the - // `creator` CTE, and the outer last-change query. Each is held by a named - // test, and it took an adversarial pass to notice that two of them were not: - // Provenance_IsScopedToTheTenant covers only `created`, because it gives each - // account a DISTINCT entityId, so the other two predicates never ran in it - // and survived deletion against the whole suite. Provenance_LastChange_ - // IsScopedToTheTenant shares ONE id across accounts and is what actually - // holds them. Do not trust this comment over a mutation run. + // There are FOUR such predicates below: the `created` query, the `creator` + // CTE, the outer last-change query, and `promoted`. The same is true of the + // EntityType predicate. Both sets were once mostly unguarded, and BOTH times + // the cause was identical: the obvious test gives each account (or type) a + // DISTINCT entityId, so the later predicates never run in it and survive + // deletion against the whole suite. Provenance_LastChange_IsScopedToTheTenant + // and Provenance_EveryQueryIsScopedToTheEntityType share ONE id across + // accounts and types respectively, which is what actually holds them; the + // fourth tenant predicate is held by Provenance_MadeOfficial_IsScopedToThe- + // Tenant. Do not trust this comment over a mutation run. private async Task> GetProvenanceChunkAsync( string entityType, Guid[] ids, CancellationToken ct) { @@ -122,13 +124,18 @@ SELECT DISTINCT ON ("EntityId") * // Identity is ActorUserId, not the email — the email is a snapshot label. // // IS NOT DISTINCT FROM rather than "=" keeps the predicate total when the - // LEFT JOIN finds no creator. Be honest about its status: it is NOT - // currently load-bearing. A record with no creation row is dropped from - // the result by `created` keying below, before the difference could be - // observed, so mutating this to a plain "=" changes no outcome and no - // test can catch it (adversarial review of PR #503). It stays because a - // NULL-propagating predicate is a trap for whoever next changes that - // keying — not because it prevents something reachable today. + // LEFT JOIN finds no creator, and it IS load-bearing. It was not while + // the result was keyed off the creation event — such records never + // reached the output — but that keying is gone (see below), so a legacy + // record whose only event is a drafting action now does reach it. Under a + // plain "=", Guid = NULL yields SQL NULL, NOT (drafting AND NULL) is NULL, + // and WHERE drops the row outright, losing a real attributable change + // rather than merely failing to exclude it. Held by + // Provenance_ForALegacyRecordWhoseOnlyEventIsDrafting_StillReportsIt. + // + // This comment previously asserted the opposite and was left behind when + // the keying changed in the same branch — the sentence that would have + // licensed the next person to simplify the line. // // KNOWN RESIDUAL, accepted rather than overlooked (#508). Which candidate // wins is still decided by ORDER BY, and the "Id" tiebreak carries no diff --git a/tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs b/tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs index c8b3b4dcd..a6abb7322 100644 --- a/tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs @@ -93,6 +93,32 @@ await SeedEventsAsync(accountId, Assert.Equal(Base.AddMinutes(90), provenance.LastChangedAtUtc); } + // A legacy record whose ONLY event is a drafting action — no ".Create", so + // the LEFT JOIN finds no creator and c."ActorUserId" is NULL. + // + // This is what makes IS NOT DISTINCT FROM load-bearing, which it was NOT + // before the result stopped being keyed off the creation event. With a plain + // "=", NULL comparison yields SQL NULL, NOT (drafting AND NULL) is NULL, and + // WHERE drops the row entirely — so a real, attributable change silently + // disappears rather than merely failing to be excluded (conventions review of + // PR #503, which caught the comment claiming this could not happen). + [Fact] + public async Task Provenance_ForALegacyRecordWhoseOnlyEventIsDrafting_StillReportsIt() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var entityId = Guid.NewGuid(); + await SeedEventsAsync(accountId, + Event(accountId, entityId, "DailyEntry.Submit", "bo@farm.test", 30, "DailyEntry")); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("DailyEntry", [entityId])); + + var provenance = result[entityId]; + Assert.Null(provenance.CreatedByEmail); + Assert.Equal("bo@farm.test", provenance.LastChangedByEmail); + Assert.Equal(Base.AddMinutes(30), provenance.LastChangedAtUtc); + } + [Fact] public async Task Provenance_WithSeveralEvents_ReportsTheEarliestAndTheLatest() { @@ -517,6 +543,71 @@ await SeedEventsAsync(accountId, Assert.Equal(Base.AddMinutes(-5), provenance.LastChangedAtUtc); } + // Every member of the drafting set, not just the two a page-level test + // happens to exercise. UpdateItem and RemoveItem could both be deleted from + // that set with the FULL 1104-test suite still green, while their two + // siblings were held — the "two of four covered" shape that reads as + // complete (adversarial review of PR #503). + [Fact] + public async Task Provenance_WhenTheCreatorReworksTheirOwnOrderLines_ReportsNoChange() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var entityId = Guid.NewGuid(); + var ana = Guid.NewGuid(); + await SeedEventsAsync(accountId, + Event(accountId, entityId, "SalesOrder.Create", "ana@farm.test", 0, "SalesOrder", ana), + Event(accountId, entityId, "SalesOrder.AddItem", "ana@farm.test", 1, "SalesOrder", ana), + Event(accountId, entityId, "SalesOrder.UpdateItem", "ana@farm.test", 2, "SalesOrder", ana), + Event(accountId, entityId, "SalesOrder.RemoveItem", "ana@farm.test", 3, "SalesOrder", ana), + Event(accountId, entityId, "SalesOrder.Confirm", "ana@farm.test", 4, "SalesOrder", ana)); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("SalesOrder", [entityId])); + + var provenance = result[entityId]; + Assert.Equal("ana@farm.test", provenance.CreatedByEmail); + // Assembling your own order is one act. Drop any member of the drafting + // set and that member resurfaces here as "somebody changed your work". + Assert.Null(provenance.LastChangedByEmail); + Assert.Null(provenance.LastChangedAtUtc); + } + + // The EntityType predicate appears FOUR times, and three of them survived + // deletion against the whole suite — the same miss as the tenant predicate, + // in the same file, found the same way. Provenance_IgnoresEventsOfAnother- + // EntityType holds only the `created` query, because its single foreign + // event is a ".Create" that the other queries exclude by action anyway. + // + // One id, four entity types, arranged so each unguarded predicate changes a + // different answer: + // outer last-change — a foreign Archive would become the last change + // creator CTE — a foreign, EARLIER Create would win the creator slot, + // un-suppressing ana's own submit + // promoted — a foreign, EARLIER Confirm would become the instant + // this entry supposedly became official + [Fact] + public async Task Provenance_EveryQueryIsScopedToTheEntityType() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var entityId = Guid.NewGuid(); + var ana = Guid.NewGuid(); + var rival = Guid.NewGuid(); + await SeedEventsAsync(accountId, + Event(accountId, entityId, "SalesOrder.Confirm", "rival@farm.test", -20, "SalesOrder", rival), + Event(accountId, entityId, "Flock.Create", "rival@farm.test", -10, "Flock", rival), + Event(accountId, entityId, "DailyEntry.Create", "ana@farm.test", 0, "DailyEntry", ana), + Event(accountId, entityId, "DailyEntry.Submit", "ana@farm.test", 5, "DailyEntry", ana), + Event(accountId, entityId, "Flock.Archive", "rival@farm.test", 90, "Flock", rival)); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("DailyEntry", [entityId])); + + var provenance = result[entityId]; + Assert.Equal("ana@farm.test", provenance.CreatedByEmail); + Assert.Null(provenance.LastChangedByEmail); + Assert.Equal(Base.AddMinutes(5), provenance.MadeOfficialAtUtc); + } + [Fact] public async Task Provenance_IsScopedToTheTenant() { diff --git a/tests/Cluckwork.Api.IntegrationTests/SubmitDailyEntryTests.cs b/tests/Cluckwork.Api.IntegrationTests/SubmitDailyEntryTests.cs index 2e24a208a..b01c1dc05 100644 --- a/tests/Cluckwork.Api.IntegrationTests/SubmitDailyEntryTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/SubmitDailyEntryTests.cs @@ -322,8 +322,11 @@ public async Task DailyEntry_ReRecordedOnTheSameKey_WritesOnlyOneCreateEvent() var again = await RecordAsync(client, Body2(718, 700)); Assert.Equal(entryId, again); + // Ordered explicitly: the assertion below compares a sequence, and an + // unordered query gives Postgres no obligation to return one. var events = await factory.WithTenantScopeAsync(accountId, db => db.AuditEvents .Where(e => e.EntityType == "DailyEntry" && e.EntityId == entryId) + .OrderBy(e => e.OccurredAtUtc).ThenBy(e => e.Action) .Select(e => e.Action) .ToListAsync()); diff --git a/web/src/i18n/en.ts b/web/src/i18n/en.ts index 52d671331..35e1482eb 100644 --- a/web/src/i18n/en.ts +++ b/web/src/i18n/en.ts @@ -2559,8 +2559,9 @@ export const en = { + "submitting it stays hidden — that is just writing it. But if somebody else edits your draft, their " + "change is shown, so the person whose numbers went into stock is never invisible.", auditRecordHistoryOlder: - "Records created before this was added show a blank History column. Nothing is missing — there is " - + "simply no creation entry in the log for them, and none is invented after the fact.", + "Records created before this was added show no \"created by\" line — there is simply no creation " + + "entry in the log for them, and none is invented after the fact. If such a record has been changed " + + "since, that change still shows; the History column is empty only when there is nothing at all.", // Export & backup exportHeading: "Export & backup (admin)", diff --git a/web/src/i18n/es.ts b/web/src/i18n/es.ts index 40e6d2d8b..5ca3c808e 100644 --- a/web/src/i18n/es.ts +++ b/web/src/i18n/es.ts @@ -2152,9 +2152,10 @@ export const es = { + "otra persona edita tu borrador, su cambio sí se muestra, para que nunca sea invisible quien puso " + "los números que entraron en existencias.", auditRecordHistoryOlder: - "Los registros creados antes de que esto se agregara muestran la columna Historial en blanco. No " - + "falta nada — simplemente no hay una entrada de creación en el registro para ellos, y no se " - + "inventa ninguna después.", + "Los registros creados antes de que esto se agregara no muestran la línea de \"creado por\": " + + "simplemente no hay una entrada de creación en el registro para ellos, y no se inventa ninguna " + + "después. Si un registro así se modificó luego, ese cambio sí se muestra; la columna Historial " + + "queda vacía solo cuando no hay absolutamente nada.", exportHeading: "Exportar y respaldo (admin)", exportCsv: diff --git a/web/src/i18n/tl.ts b/web/src/i18n/tl.ts index 7f2614fba..d9965f765 100644 --- a/web/src/i18n/tl.ts +++ b/web/src/i18n/tl.ts @@ -2234,8 +2234,10 @@ export const tl = { + "ang nag-edit ng draft mo, ipinapakita ang pagbabago nila, kaya hindi kailanman nawawala ang taong " + "naglagay ng mga bilang na pumasok sa stock.", auditRecordHistoryOlder: - "Ang mga record na ginawa bago ito idagdag ay may blangkong kolum na Kasaysayan. Walang nawawala — " - + "wala lang talagang entry ng paggawa sa log para sa kanila, at walang iniimbentong bago pagkatapos.", + "Ang mga record na ginawa bago ito idagdag ay walang linyang \"ginawa ni\" — wala lang talagang " + + "entry ng paggawa sa log para sa kanila, at walang iniimbentong bago pagkatapos. Kung nabago ang " + + "ganoong record pagkatapos, ipinapakita pa rin ang pagbabagong iyon; blangko lang ang kolum na " + + "Kasaysayan kapag wala talagang anuman.", exportHeading: "Export at Backup (admin)", exportCsv: From d04479293426840bcaeb5ac71f18e701a0ce11e0 Mon Sep 17 00:00:00 2001 From: mforce <> Date: Mon, 10 Aug 2026 21:36:45 -0700 Subject: [PATCH 9/9] fix(api,web): show the creator's own edit once someone else has touched the draft MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- specs/product/GLOSSARY.md | 8 ++ .../Repositories/AuditEventRepository.cs | 80 ++++++++--- .../AuditProvenanceTests.cs | 132 ++++++++++++++++++ web/src/i18n/en.ts | 4 +- web/src/i18n/es.ts | 4 +- web/src/i18n/tl.ts | 4 +- 6 files changed, 210 insertions(+), 22 deletions(-) diff --git a/specs/product/GLOSSARY.md b/specs/product/GLOSSARY.md index 77537654c..01110905d 100644 --- a/specs/product/GLOSSARY.md +++ b/specs/product/GLOSSARY.md @@ -458,6 +458,14 @@ case: your own edits to your own draft are part of writing it, so they are hidde along with your own submit, and only a **different** person's edit surfaces as a change. +That hiding stops the moment the draft stops being yours alone. **Once somebody +else has edited it, your own later edits are shown too** — because by then they +are the answer to "whose numbers are these". Hiding them named the colleague +whose work you overwrote and dated the record to an edit that no longer exists +in it, while the numbers that went into stock were yours. Your own *promotion* +stays hidden even here: it has its own "Submitted …" line, and repeating it as +the last change would say that making a record official changed it. + Two further consequences follow from being derived rather than stored. A record created **before #494** has no creation event and is **never backfilled**, so it shows no "created by" line — but that half is independent of the rest: if it has diff --git a/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs b/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs index 7ada87b59..d6bfac908 100644 --- a/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs +++ b/src/Cluckwork.Infrastructure/Repositories/AuditEventRepository.cs @@ -55,21 +55,25 @@ public async Task> GetProvenanceAsyn return all; } - // One chunk, two round trips. Raw SQL because DISTINCT ON is Postgres-only + // One chunk, three round trips. Raw SQL because DISTINCT ON is Postgres-only // and has no LINQ translation. EF would otherwise compose the tenant query // filter over this — IgnoreQueryFilters opts out of that, so the hand-written // AccountId predicate is then the ONLY thing scoping the read to the tenant. // - // There are FOUR such predicates below: the `created` query, the `creator` - // CTE, the outer last-change query, and `promoted`. The same is true of the - // EntityType predicate. Both sets were once mostly unguarded, and BOTH times - // the cause was identical: the obvious test gives each account (or type) a - // DISTINCT entityId, so the later predicates never run in it and survive - // deletion against the whole suite. Provenance_LastChange_IsScopedToTheTenant + // There are THREE such predicates below, one per round trip, and the same is + // true of the EntityType and EntityId predicates. Keeping it to one per query + // is deliberate rather than tidy: this scoping triple was twice shipped + // mostly unguarded, both times for the identical reason — the obvious test + // gives each account (or type) a DISTINCT entityId, so a duplicate predicate + // never runs in it and survives deletion against the whole suite. Copies of a + // predicate are what let that happen, so the last-change query derives its + // creator and its "was this draft shared" set from ONE `scoped` CTE rather + // than re-stating the triple per CTE. + // + // What actually holds the three: Provenance_LastChange_IsScopedToTheTenant // and Provenance_EveryQueryIsScopedToTheEntityType share ONE id across - // accounts and types respectively, which is what actually holds them; the - // fourth tenant predicate is held by Provenance_MadeOfficial_IsScopedToThe- - // Tenant. Do not trust this comment over a mutation run. + // accounts and types respectively; Provenance_MadeOfficial_IsScopedToThe- + // Tenant holds `promoted`. Do not trust this comment over a mutation run. private async Task> GetProvenanceChunkAsync( string entityType, Guid[] ids, CancellationToken ct) { @@ -108,6 +112,33 @@ SELECT DISTINCT ON ("EntityId") * // they submit and your edit is the last change, even though they are // still the creator and their own submit stays hidden. // + // ...and only while the creator is the ONLY person who drafted it, which + // is what the `shared` CTE decides. Once somebody else has edited the + // draft, the creator's own later edits stop being quiet housekeeping and + // become the answer to "whose numbers are these" — hiding them named the + // colleague whose work was overwritten, and dated the record to an edit + // that no longer exists in it. Their PROMOTION stays hidden either way: + // it has its own line, and repeating it as the last change would say that + // making a record official changed it. Held by Provenance_WhenTheCreator- + // RevertsSomeoneElsesEdit_NamesTheCreatorsOwnEdit. + // + // "Somebody else" is IS DISTINCT FROM, so a creatorless legacy record + // counts as shared. Harmless rather than overlooked: the exclusion this + // gates on cannot fire there anyway — nothing IS NOT DISTINCT FROM a + // creator that does not exist. + // + // `shared` counts DRAFTING by somebody else, not any event by somebody + // else, and that narrowing has NO test — deliberately, because no fixture + // can currently tell the two apart without being fiction. Every foreign + // non-drafting action on these two aggregates (Adjust, Void, Cancel) + // only becomes possible once drafting has ended, so the creator has no + // later draft edit left for the wider rule to unhide. The line stays + // because the narrow rule is the one intended, and the day something + // audits an entry it did not draft — a Lock event from the sweep, a + // payment against an order — the wider version would silently start + // reporting the creator's own edits as changes. Reported as a surviving + // mutant rather than papered over with a contrived test. + // // Deliberately NOT every draft-to-terminal move — SalesOrder.Cancel kills // a draft rather than making it official, so cancelling your own order // stays a reportable change. @@ -178,24 +209,35 @@ SELECT DISTINCT ON ("EntityId") * AuditActions.SalesOrderRemoveItem, }).ToArray(); var latest = await db.AuditEvents.FromSqlInterpolated($""" - WITH creator AS ( - SELECT DISTINCT ON ("EntityId") "EntityId", "ActorUserId" + WITH scoped AS ( + SELECT * FROM "AuditEvents" WHERE "AccountId" = {accountId} AND "EntityType" = {entityType} AND "EntityId" = ANY({ids}) - AND "Action" LIKE {createAction} + ), + creator AS ( + SELECT DISTINCT ON ("EntityId") "EntityId", "ActorUserId" + FROM scoped + WHERE "Action" LIKE {createAction} ORDER BY "EntityId", "OccurredAtUtc" ASC, "Id" ASC + ), + shared AS ( + SELECT DISTINCT s."EntityId" + FROM scoped s + LEFT JOIN creator c ON c."EntityId" = s."EntityId" + WHERE s."Action" = ANY({draftingActions}) + AND s."ActorUserId" IS DISTINCT FROM c."ActorUserId" ) SELECT DISTINCT ON (e."EntityId") e.* - FROM "AuditEvents" e + FROM scoped e LEFT JOIN creator c ON c."EntityId" = e."EntityId" - WHERE e."AccountId" = {accountId} - AND e."EntityType" = {entityType} - AND e."EntityId" = ANY({ids}) - AND e."Action" NOT LIKE {createAction} + LEFT JOIN shared sh ON sh."EntityId" = e."EntityId" + WHERE e."Action" NOT LIKE {createAction} AND NOT (e."Action" = ANY({draftingActions}) - AND e."ActorUserId" IS NOT DISTINCT FROM c."ActorUserId") + AND e."ActorUserId" IS NOT DISTINCT FROM c."ActorUserId" + AND (e."Action" = ANY({promotionActions}) + OR sh."EntityId" IS NULL)) ORDER BY e."EntityId", e."OccurredAtUtc" DESC, e."Id" DESC """) .IgnoreQueryFilters() diff --git a/tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs b/tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs index a6abb7322..f9f56bcb0 100644 --- a/tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/AuditProvenanceTests.cs @@ -337,6 +337,115 @@ await SeedEventsAsync(accountId, Assert.Null(provenance.LastChangedAtUtc); } + // ...but only while the creator is the ONLY person who touched the draft. + // Once somebody else has edited it, hiding the creator's own later edit + // misattributes the record: the column named bo, whose numbers ana then + // overwrote, while ana's numbers are the ones that went into stock. Wrong + // person, wrong timestamp — found independently by two reviewers of PR #503 + // and decided by the owner on 2026-08-10. + // + // The submit is still hidden even here: it is reported on its own line, and + // repeating it as "last changed" would say the act of making a record + // official changed it. + [Fact] + public async Task Provenance_WhenTheCreatorRevertsSomeoneElsesEdit_NamesTheCreatorsOwnEdit() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var entityId = Guid.NewGuid(); + var ana = Guid.NewGuid(); + await SeedEventsAsync(accountId, + Event(accountId, entityId, "DailyEntry.Create", "ana@farm.test", 0, "DailyEntry", ana), + Event(accountId, entityId, "DailyEntry.Update", "bo@farm.test", 2, "DailyEntry"), + Event(accountId, entityId, "DailyEntry.Update", "ana@farm.test", 6, "DailyEntry", ana), + Event(accountId, entityId, "DailyEntry.Submit", "ana@farm.test", 8, "DailyEntry", ana)); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("DailyEntry", [entityId])); + + var provenance = result[entityId]; + Assert.Equal("ana@farm.test", provenance.CreatedByEmail); + Assert.Equal("ana@farm.test", provenance.LastChangedByEmail); + // +6, not +8: her edit, not her submit. + Assert.Equal(Base.AddMinutes(6), provenance.LastChangedAtUtc); + Assert.Equal(Base.AddMinutes(8), provenance.MadeOfficialAtUtc); + } + + // The same rule on the sales-order shape, and the reason it is stated over + // the whole drafting set rather than over DailyEntry.Update alone. + [Fact] + public async Task Provenance_WhenTheCreatorReworksLinesAfterSomeoneElse_NamesTheCreatorsOwnEdit() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var entityId = Guid.NewGuid(); + var ana = Guid.NewGuid(); + await SeedEventsAsync(accountId, + Event(accountId, entityId, "SalesOrder.Create", "ana@farm.test", 0, "SalesOrder", ana), + Event(accountId, entityId, "SalesOrder.UpdateItem", "bo@farm.test", 2, "SalesOrder"), + Event(accountId, entityId, "SalesOrder.RemoveItem", "ana@farm.test", 6, "SalesOrder", ana), + Event(accountId, entityId, "SalesOrder.Confirm", "ana@farm.test", 8, "SalesOrder", ana)); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("SalesOrder", [entityId])); + + var provenance = result[entityId]; + Assert.Equal("ana@farm.test", provenance.LastChangedByEmail); + Assert.Equal(Base.AddMinutes(6), provenance.LastChangedAtUtc); + Assert.Equal(Base.AddMinutes(8), provenance.MadeOfficialAtUtc); + } + + // "Somebody else touched this draft" is decided over THIS tenant's trail. + // A foreign account's edit of a colliding id must not unhide ana's own + // edit — which would report a change to a record nobody but ana ever + // touched, sourced from a row she is not allowed to see. + [Fact] + public async Task Provenance_WhetherADraftWasSharedIsScopedToTheTenant() + { + var mine = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var theirs = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var entityId = Guid.NewGuid(); + var ana = Guid.NewGuid(); + + await SeedEventsAsync(theirs, + Event(theirs, entityId, "DailyEntry.Create", "rival@other.test", 0, "DailyEntry"), + Event(theirs, entityId, "DailyEntry.Update", "other@other.test", 1, "DailyEntry")); + await SeedEventsAsync(mine, + Event(mine, entityId, "DailyEntry.Create", "ana@farm.test", 0, "DailyEntry", ana), + Event(mine, entityId, "DailyEntry.Update", "ana@farm.test", 6, "DailyEntry", ana), + Event(mine, entityId, "DailyEntry.Submit", "ana@farm.test", 8, "DailyEntry", ana)); + + var result = await WithRepositoryAsync(mine, repo => + repo.GetProvenanceAsync("DailyEntry", [entityId])); + + var provenance = result[entityId]; + Assert.Null(provenance.LastChangedByEmail); + Assert.Null(provenance.LastChangedAtUtc); + } + + // ...and over THIS entity's trail. Two records in one batch: a shared one + // and a solitary one. The solitary record must stay quiet, or the batch + // size silently changes what a record says about itself. + [Fact] + public async Task Provenance_WhetherADraftWasSharedIsScopedToTheRecord() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var solitary = Guid.NewGuid(); + var shared = Guid.NewGuid(); + var ana = Guid.NewGuid(); + await SeedEventsAsync(accountId, + Event(accountId, solitary, "DailyEntry.Create", "ana@farm.test", 0, "DailyEntry", ana), + Event(accountId, solitary, "DailyEntry.Update", "ana@farm.test", 6, "DailyEntry", ana), + Event(accountId, shared, "DailyEntry.Create", "ana@farm.test", 0, "DailyEntry", ana), + Event(accountId, shared, "DailyEntry.Update", "bo@farm.test", 2, "DailyEntry"), + Event(accountId, shared, "DailyEntry.Update", "ana@farm.test", 6, "DailyEntry", ana)); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("DailyEntry", [solitary, shared])); + + Assert.Null(result[solitary].LastChangedByEmail); + Assert.Equal("ana@farm.test", result[shared].LastChangedByEmail); + Assert.Equal(Base.AddMinutes(6), result[shared].LastChangedAtUtc); + } + // The case the whole option exists for: a worker writes the numbers, someone // else makes them official. Both people must be visible — that is the // accountability the submit step is for. @@ -655,6 +764,29 @@ public async Task Provenance_ForAnIdWithNoEvents_IsAbsentRatherThanBlank() Assert.False(result.ContainsKey(untouched)); } + // The id list is a predicate on the read, not a filter applied afterwards. + // Dropping it left every requested record's answer CORRECT — the creator and + // the shared-draft set are both resolved per EntityId — so nothing else in + // this file noticed, and the dictionary quietly grew a row for every record + // of that type in the account. A caller enumerating it would then hand out + // provenance for records the page never asked about. + [Fact] + public async Task Provenance_ReturnsOnlyTheRecordsItWasAskedFor() + { + var accountId = await factory.SeedAccountWithUserAsync($"u-{Guid.NewGuid():N}@test.local"); + var asked = Guid.NewGuid(); + var notAsked = Guid.NewGuid(); + await SeedEventsAsync(accountId, + Event(accountId, asked, "Flock.Create", "ana@farm.test", 0), + Event(accountId, notAsked, "Flock.Create", "bo@farm.test", 0), + Event(accountId, notAsked, "Flock.Update", "bo@farm.test", 5)); + + var result = await WithRepositoryAsync(accountId, repo => + repo.GetProvenanceAsync("Flock", [asked])); + + Assert.Equal([asked], result.Keys.ToArray()); + } + [Fact] public async Task Provenance_WithNoIds_ReturnsEmptyWithoutQuerying() { diff --git a/web/src/i18n/en.ts b/web/src/i18n/en.ts index 35e1482eb..b31c48c4d 100644 --- a/web/src/i18n/en.ts +++ b/web/src/i18n/en.ts @@ -2557,7 +2557,9 @@ export const en = { + "Correcting an entry after it is locked always shows, " + "even if you created it, and so does cancelling your own draft order. Editing your own draft before " + "submitting it stays hidden — that is just writing it. But if somebody else edits your draft, their " - + "change is shown, so the person whose numbers went into stock is never invisible.", + + "change is shown, so the person whose numbers went into stock is never invisible. And once they " + + "have, your own later edits are shown too: if you put the numbers back the way you wanted them, " + + "the column names you and the time you did it, not the edit you overwrote.", auditRecordHistoryOlder: "Records created before this was added show no \"created by\" line — there is simply no creation " + "entry in the log for them, and none is invented after the fact. If such a record has been changed " diff --git a/web/src/i18n/es.ts b/web/src/i18n/es.ts index 5ca3c808e..d2f98e146 100644 --- a/web/src/i18n/es.ts +++ b/web/src/i18n/es.ts @@ -2150,7 +2150,9 @@ export const es = { + "siempre se muestra, aunque tú lo hayas creado, y cancelar tu propio pedido en borrador también. " + "Editar tu propio borrador antes de enviarlo queda oculto: eso es simplemente escribirlo. Pero si " + "otra persona edita tu borrador, su cambio sí se muestra, para que nunca sea invisible quien puso " - + "los números que entraron en existencias.", + + "los números que entraron en existencias. Y a partir de ese momento también se muestran tus " + + "propias ediciones posteriores: si dejas los números como querías, la columna te nombra a ti y " + + "marca la hora en que lo hiciste, no la edición que reemplazaste.", auditRecordHistoryOlder: "Los registros creados antes de que esto se agregara no muestran la línea de \"creado por\": " + "simplemente no hay una entrada de creación en el registro para ellos, y no se inventa ninguna " diff --git a/web/src/i18n/tl.ts b/web/src/i18n/tl.ts index d9965f765..eb69410ca 100644 --- a/web/src/i18n/tl.ts +++ b/web/src/i18n/tl.ts @@ -2232,7 +2232,9 @@ export const tl = { + "kahit ikaw ang gumawa nito, gayundin ang pagkansela ng sarili mong draft na order. Nakatago ang " + "pag-edit mo sa sarili mong draft bago isumite — bahagi lang iyon ng pagsulat nito. Ngunit kung iba " + "ang nag-edit ng draft mo, ipinapakita ang pagbabago nila, kaya hindi kailanman nawawala ang taong " - + "naglagay ng mga bilang na pumasok sa stock.", + + "naglagay ng mga bilang na pumasok sa stock. At mula noon, ipinapakita na rin ang sarili mong mga " + + "susunod na pag-edit: kung ibinalik mo ang mga bilang sa gusto mo, ikaw ang pinapangalanan ng " + + "column at ang oras na ginawa mo iyon, hindi ang pag-edit na pinalitan mo.", auditRecordHistoryOlder: "Ang mga record na ginawa bago ito idagdag ay walang linyang \"ginawa ni\" — wala lang talagang " + "entry ng paggawa sa log para sa kanila, at walang iniimbentong bago pagkatapos. Kung nabago ang "