From c5d82a996a89c29c08d52b67517e8894377ad51d Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Thu, 24 Sep 2026 23:30:16 -0300 Subject: [PATCH 1/2] fix(uplifts): audit uplift decisions on the dispatch's resolved work order Approve, reject, request-changes, expiry and escalation staged their work order audit with WorkOrderId = dispatch.WorkOrderId ?? 0. WorkOrderAuditLogs requires a real work order, so any uplift on a dispatch without an owning work order failed to save: the decision returned a 500 and the request stayed Pending, and the expiry sweep failed on it every run. The audit now goes to the work order the uplift resolves to through the existing owner-or-linked read that revoke already uses. When none resolves, the status change is saved on the request and no audit row is written. --- .../UpliftWorkflowTests.cs | 2 + .../Implementation/UpliftLifecycleService.cs | 22 +- .../Implementation/UpliftService.cs | 25 +- .../UpliftDecisionAuditRelationalTests.cs | 273 ++++++++++++++++++ 4 files changed, 312 insertions(+), 10 deletions(-) create mode 100644 SeaHavenIndustries.Tests/UpliftDecisionAuditRelationalTests.cs diff --git a/Api.SeaHavenIndustries.Tests/UpliftWorkflowTests.cs b/Api.SeaHavenIndustries.Tests/UpliftWorkflowTests.cs index b57d36f..4518910 100644 --- a/Api.SeaHavenIndustries.Tests/UpliftWorkflowTests.cs +++ b/Api.SeaHavenIndustries.Tests/UpliftWorkflowTests.cs @@ -681,6 +681,8 @@ public sealed class UpliftWorkflowTests var upliftData = new Mock(); upliftData.Setup(u => u.GetDueForExpiryAsync(It.IsAny(), It.IsAny(), It.IsAny())) .ReturnsAsync(new List { orphanReq, validReq }); + upliftData.Setup(u => u.GetWorkOrderIdForUpliftAsync(202, It.IsAny())) + .ReturnsAsync(99); upliftData.Setup(u => u.SaveChangesAsync(It.IsAny())) .Returns(Task.CompletedTask); diff --git a/SeaHaven.Services/Implementation/UpliftLifecycleService.cs b/SeaHaven.Services/Implementation/UpliftLifecycleService.cs index 9f1cfbb..2f94678 100644 --- a/SeaHaven.Services/Implementation/UpliftLifecycleService.cs +++ b/SeaHaven.Services/Implementation/UpliftLifecycleService.cs @@ -61,9 +61,9 @@ namespace SeaHaven.Services.Implementation req.DecidedAt = now; req.LastModificationTime = now; - await _dispatchData.StageAuditLogAsync(new WorkOrderAuditLog + await StageAuditAsync(req.Id, workOrderId => new WorkOrderAuditLog { - WorkOrderId = dispatch?.WorkOrderId ?? 0, + WorkOrderId = workOrderId, FieldName = $"Dispatch {dispatch?.DispatchNumber} Uplift", OldValue = previous, NewValue = UpliftStatus.Expired, @@ -136,9 +136,9 @@ namespace SeaHaven.Services.Implementation req.EscalatedAt = now; req.LastModificationTime = now; - await _dispatchData.StageAuditLogAsync(new WorkOrderAuditLog + await StageAuditAsync(req.Id, workOrderId => new WorkOrderAuditLog { - WorkOrderId = dispatch.WorkOrderId ?? 0, + WorkOrderId = workOrderId, FieldName = $"Dispatch {dispatch.DispatchNumber} Uplift", OldValue = "Pending", NewValue = "Escalated", @@ -247,5 +247,19 @@ namespace SeaHaven.Services.Implementation req.LastModificationTime = now; await _upliftData.SaveChangesAsync(cancellationToken); } + + // The audit goes to the work order the uplift's dispatch belongs to (owner, else the + // one it is linked to). With neither, the status change is recorded on the request + // alone; a work-order id of 0 would fail the required audit foreign key and abort + // the sweep on every run. + private async Task StageAuditAsync( + int upliftId, + Func buildEntry, + CancellationToken cancellationToken) + { + var workOrderId = await _upliftData.GetWorkOrderIdForUpliftAsync(upliftId, cancellationToken); + if (workOrderId is int resolved) + await _dispatchData.StageAuditLogAsync(buildEntry(resolved), cancellationToken); + } } } diff --git a/SeaHaven.Services/Implementation/UpliftService.cs b/SeaHaven.Services/Implementation/UpliftService.cs index 3cfed76..134ec66 100644 --- a/SeaHaven.Services/Implementation/UpliftService.cs +++ b/SeaHaven.Services/Implementation/UpliftService.cs @@ -166,9 +166,9 @@ namespace SeaHaven.Services.Implementation req.DecisionNote = string.IsNullOrWhiteSpace(note) ? null : note!.Trim(); req.LastModificationTime = now; - await _dispatchData.StageAuditLogAsync(new WorkOrderAuditLog + await StageDecisionAuditAsync(req.Id, workOrderId => new WorkOrderAuditLog { - WorkOrderId = dispatch.WorkOrderId ?? 0, + WorkOrderId = workOrderId, UserId = userId, FieldName = $"Dispatch {dispatch.DispatchNumber} NTE", OldValue = $"${oldNTE:F2}", @@ -252,9 +252,9 @@ namespace SeaHaven.Services.Implementation req.DecisionNote = note!.Trim(); req.LastModificationTime = now; - await _dispatchData.StageAuditLogAsync(new WorkOrderAuditLog + await StageDecisionAuditAsync(req.Id, workOrderId => new WorkOrderAuditLog { - WorkOrderId = dispatch?.WorkOrderId ?? 0, + WorkOrderId = workOrderId, UserId = userId, FieldName = $"Dispatch {dispatch?.DispatchNumber} Uplift", OldValue = previous, @@ -290,9 +290,9 @@ namespace SeaHaven.Services.Implementation req.DecisionNote = note.Trim(); req.LastModificationTime = now; - await _dispatchData.StageAuditLogAsync(new WorkOrderAuditLog + await StageDecisionAuditAsync(req.Id, workOrderId => new WorkOrderAuditLog { - WorkOrderId = dispatch?.WorkOrderId ?? 0, + WorkOrderId = workOrderId, UserId = userId, FieldName = $"Dispatch {dispatch?.DispatchNumber} Uplift", OldValue = previous, @@ -307,6 +307,19 @@ namespace SeaHaven.Services.Implementation public bool CanApprove(ClaimsPrincipal user, int tier) => UserCanApprove(user, tier); + // The decision audit goes to the work order the uplift's dispatch belongs to (owner, + // else the one it is linked to). A dispatch with neither has no work order to audit + // against, so the decision is recorded on the request alone. + private async Task StageDecisionAuditAsync( + int upliftId, + Func buildEntry, + CancellationToken cancellationToken) + { + var workOrderId = await _upliftData.GetWorkOrderIdForUpliftAsync(upliftId, cancellationToken); + if (workOrderId is int resolved) + await _dispatchData.StageAuditLogAsync(buildEntry(resolved), cancellationToken); + } + // SH-101: authorized internal download of a Passed UpliftEvidence file. The data // service resolves the document by the request's own linkage (no client-supplied // document id or vendor/public path). Only UpliftEvidence documents are exposed; diff --git a/SeaHavenIndustries.Tests/UpliftDecisionAuditRelationalTests.cs b/SeaHavenIndustries.Tests/UpliftDecisionAuditRelationalTests.cs new file mode 100644 index 0000000..b95ffbd --- /dev/null +++ b/SeaHavenIndustries.Tests/UpliftDecisionAuditRelationalTests.cs @@ -0,0 +1,273 @@ +using Data.SeaHavenIndustries; +using Data.SeaHavenIndustries.Enums; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using SeaHaven.DataServices.Implementation; +using SeaHaven.Services.Configuration; +using SeaHaven.Services.Implementation; +using SeaHaven.Services.Interfaces; +using System.Security.Claims; + +namespace SeaHavenIndustries.Tests; + +// Relational (SQLite, foreign keys on) coverage for the work-order audit entry written by +// uplift decisions. WorkOrderAuditLogs.WorkOrderId is a required FK, so the in-memory +// provider cannot prove a decision on a dispatch without an owning work order commits. +// The audit goes to the work order the dispatch belongs to (owner, else the one it is +// linked to through DispatchWorkOrders); when none resolves, the decision is still saved +// and no audit row is written. +public sealed class UpliftDecisionAuditRelationalTests +{ + private const string AdminId = "admin-1"; + + public enum Ownership + { + Owned, + Linked, + Unresolvable + } + + public enum Decision + { + Approve, + Reject, + RequestChanges + } + + [Theory] + [InlineData(Decision.Approve, Ownership.Owned)] + [InlineData(Decision.Approve, Ownership.Linked)] + [InlineData(Decision.Approve, Ownership.Unresolvable)] + [InlineData(Decision.Reject, Ownership.Owned)] + [InlineData(Decision.Reject, Ownership.Linked)] + [InlineData(Decision.Reject, Ownership.Unresolvable)] + [InlineData(Decision.RequestChanges, Ownership.Owned)] + [InlineData(Decision.RequestChanges, Ownership.Linked)] + [InlineData(Decision.RequestChanges, Ownership.Unresolvable)] + public async Task Decision_CommitsAndAuditsResolvedWorkOrder(Decision decision, Ownership ownership) + { + await using var connection = new SqliteConnection("Data Source=:memory:;Foreign Keys=True"); + await connection.OpenAsync(); + var options = new DbContextOptionsBuilder().UseSqlite(connection).Options; + + int upliftId; + int? expectedWorkOrderId; + await using (var seed = new SqliteUpliftDecisionTestDbContext(options)) + { + await seed.Database.EnsureCreatedAsync(); + (upliftId, expectedWorkOrderId) = await SeedAsync(seed, ownership, expiresAt: null); + } + + await using (var context = new SqliteUpliftDecisionTestDbContext(options)) + { + var service = new UpliftService( + new UpliftDataService(context), + new DispatchDataService(context), + new NoDocumentStorage(), + TimeProvider.System, + Options.Create(new ApprovalsOptions())); + var admin = new ClaimsPrincipal(new ClaimsIdentity(new[] + { + new Claim(ClaimTypes.NameIdentifier, AdminId), + new Claim(ClaimTypes.Role, "Admin") + }, "Test")); + + switch (decision) + { + case Decision.Approve: + await service.ApproveAsync(admin, upliftId, null, CancellationToken.None); + break; + case Decision.Reject: + await service.RejectAsync(admin, upliftId, "too high", CancellationToken.None); + break; + case Decision.RequestChanges: + await service.RequestChangesAsync(admin, upliftId, "send a quote", CancellationToken.None); + break; + } + } + + await using var verify = new SqliteUpliftDecisionTestDbContext(options); + var saved = await verify.DispatchUpliftRequests.AsNoTracking().SingleAsync(u => u.Id == upliftId); + var expectedStatus = decision switch + { + Decision.Approve => UpliftStatus.Approved, + Decision.Reject => UpliftStatus.Rejected, + _ => UpliftStatus.ChangesRequested + }; + Assert.Equal(expectedStatus, saved.Status); + Assert.Equal(AdminId, saved.DecidedByUserId); + if (decision == Decision.Approve) + { + var dispatch = await verify.Dispatches.AsNoTracking().SingleAsync(d => d.Id == saved.DispatchId); + Assert.Equal(5_000m, dispatch.NTEAmount); + } + + var audits = await verify.WorkOrderAuditLogs.AsNoTracking().ToListAsync(); + Assert.DoesNotContain(audits, a => a.WorkOrderId == 0); + if (expectedWorkOrderId is int workOrderId) + { + var audit = Assert.Single(audits); + Assert.Equal(workOrderId, audit.WorkOrderId); + Assert.StartsWith("uplift_", audit.Action); + } + else + { + Assert.Empty(audits); + } + } + + [Theory] + [InlineData(Ownership.Linked)] + [InlineData(Ownership.Unresolvable)] + public async Task Expiry_CommitsAndAuditsResolvedWorkOrder(Ownership ownership) + { + await using var connection = new SqliteConnection("Data Source=:memory:;Foreign Keys=True"); + await connection.OpenAsync(); + var options = new DbContextOptionsBuilder().UseSqlite(connection).Options; + + int upliftId; + int? expectedWorkOrderId; + await using (var seed = new SqliteUpliftDecisionTestDbContext(options)) + { + await seed.Database.EnsureCreatedAsync(); + (upliftId, expectedWorkOrderId) = await SeedAsync(seed, ownership, expiresAt: DateTime.UtcNow.AddHours(-1)); + } + + await using (var context = new SqliteUpliftDecisionTestDbContext(options)) + { + var lifecycle = new UpliftLifecycleService( + new UpliftDataService(context), + new DispatchDataService(context), + null!, + new NoEmailSender(), + Options.Create(new FrontendOptions()), + Options.Create(new ApprovalsOptions()), + TimeProvider.System, + NullLogger.Instance); + + Assert.Equal(1, await lifecycle.ExpireDueAsync(CancellationToken.None)); + } + + await using var verify = new SqliteUpliftDecisionTestDbContext(options); + var saved = await verify.DispatchUpliftRequests.AsNoTracking().SingleAsync(u => u.Id == upliftId); + Assert.Equal(UpliftStatus.Expired, saved.Status); + + var audits = await verify.WorkOrderAuditLogs.AsNoTracking().ToListAsync(); + Assert.DoesNotContain(audits, a => a.WorkOrderId == 0); + if (expectedWorkOrderId is int workOrderId) + Assert.Equal(workOrderId, Assert.Single(audits).WorkOrderId); + else + Assert.Empty(audits); + } + + private static async Task<(int UpliftId, int? ExpectedWorkOrderId)> SeedAsync( + ApplicationDbContext context, + Ownership ownership, + DateTime? expiresAt) + { + context.Users.Add(new ApplicationUser + { + Id = AdminId, + UserName = AdminId, + NormalizedUserName = AdminId.ToUpperInvariant(), + Email = "admin@test.local", + NormalizedEmail = "ADMIN@TEST.LOCAL" + }); + var vendor = new Vendor { CompanyName = "Vinewood LLC", IsActive = true }; + // A second work order keeps ids distinct from the first, so an audit landing on + // the wrong work order cannot pass by coincidence. + var unrelated = new WorkOrder { InternalWONumber = "WO-OTHER", WorkerOrderTitle = "Other" }; + var workOrder = new WorkOrder + { + InternalWONumber = "WO-TARGET", + WorkerOrderTitle = "Repair", + LifecycleStatus = LifecycleStatus.Scheduled + }; + context.AddRange(vendor, unrelated, workOrder); + await context.SaveChangesAsync(); + + var dispatch = new Dispatch + { + VendorId = vendor.Id, + WorkOrderId = ownership == Ownership.Owned ? workOrder.Id : null, + DispatchNumber = "DIS-UPLIFT", + Status = "Scheduled", + NTEAmount = 1_000m + }; + context.Dispatches.Add(dispatch); + await context.SaveChangesAsync(); + + if (ownership == Ownership.Linked) + { + context.DispatchWorkOrders.Add(new DispatchWorkOrder + { + DispatchId = dispatch.Id, + WorkOrderId = workOrder.Id + }); + } + + var request = new DispatchUpliftRequest + { + DispatchId = dispatch.Id, + CurrentNTE = 1_000m, + RequestedNTE = 5_000m, + VendorReason = "Scope grew", + Status = UpliftStatus.Pending, + RequiredTier = 2, + CreatedDate = DateTime.UtcNow.AddDays(-4), + ExpiresAt = expiresAt, + NotificationStatus = "Pending" + }; + context.DispatchUpliftRequests.Add(request); + await context.SaveChangesAsync(); + + return (request.Id, ownership == Ownership.Unresolvable ? null : workOrder.Id); + } + + private sealed class NoDocumentStorage : IVendorDocumentStoragePort + { + public Task SaveAsync(int vendorId, int dispatchId, string storedFileName, Stream content, CancellationToken cancellationToken) + => throw new NotSupportedException(); + + public Stream OpenRead(int vendorId, int dispatchId, string storedFileName) + => throw new NotSupportedException(); + + public void Delete(int vendorId, int dispatchId, string storedFileName) + => throw new NotSupportedException(); + } + + private sealed class NoEmailSender : IEmailSender + { + public Task SendEmailAsync(string emailTo, string subject, string body) => Task.FromResult(true); + } + + private sealed class SqliteUpliftDecisionTestDbContext : ApplicationDbContext + { + public SqliteUpliftDecisionTestDbContext(DbContextOptions options) + : base(options) + { + } + + protected override void OnModelCreating(ModelBuilder builder) + { + base.OnModelCreating(builder); + + // SQL Server filtered index syntax is invalid on SQLite. + foreach (var index in builder.Model.GetEntityTypes().SelectMany(e => e.GetIndexes())) + { + if (index.GetFilter() != null) + index.SetFilter(null); + } + + // SQLite has no rowversion type; treat as plain nullable blobs. + foreach (var entityType in new[] { typeof(WorkOrder), typeof(Dispatch) }) + { + var property = builder.Entity(entityType).Property("RowVersion").Metadata; + property.ValueGenerated = Microsoft.EntityFrameworkCore.Metadata.ValueGenerated.Never; + property.IsConcurrencyToken = false; + } + } + } +} From b2b9fc3588ef6c29b72bffea266b055e9bb3d7d3 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Thu, 24 Sep 2026 23:37:58 -0300 Subject: [PATCH 2/2] test(uplifts): cover escalation audit on a dispatch without an owning work order --- .../UpliftDecisionAuditRelationalTests.cs | 65 ++++++++++++++++++- 1 file changed, 62 insertions(+), 3 deletions(-) diff --git a/SeaHavenIndustries.Tests/UpliftDecisionAuditRelationalTests.cs b/SeaHavenIndustries.Tests/UpliftDecisionAuditRelationalTests.cs index b95ffbd..fb07d09 100644 --- a/SeaHavenIndustries.Tests/UpliftDecisionAuditRelationalTests.cs +++ b/SeaHavenIndustries.Tests/UpliftDecisionAuditRelationalTests.cs @@ -162,10 +162,61 @@ public sealed class UpliftDecisionAuditRelationalTests Assert.Empty(audits); } + [Theory] + [InlineData(Ownership.Linked)] + [InlineData(Ownership.Unresolvable)] + public async Task Escalation_CommitsAndAuditsResolvedWorkOrder(Ownership ownership) + { + await using var connection = new SqliteConnection("Data Source=:memory:;Foreign Keys=True"); + await connection.OpenAsync(); + var options = new DbContextOptionsBuilder().UseSqlite(connection).Options; + + int upliftId; + int? expectedWorkOrderId; + await using (var seed = new SqliteUpliftDecisionTestDbContext(options)) + { + await seed.Database.EnsureCreatedAsync(); + (upliftId, expectedWorkOrderId) = await SeedAsync( + seed, + ownership, + expiresAt: null, + initialNotificationSentAt: DateTime.UtcNow.AddDays(-3)); + } + + var emailSender = new NoEmailSender(); + await using (var context = new SqliteUpliftDecisionTestDbContext(options)) + { + var lifecycle = new UpliftLifecycleService( + new UpliftDataService(context), + new DispatchDataService(context), + null!, + emailSender, + Options.Create(new FrontendOptions()), + Options.Create(new ApprovalsOptions { EscalationRecipients = new[] { "escalate@test.local" } }), + TimeProvider.System, + NullLogger.Instance); + + Assert.Equal(1, await lifecycle.EscalateDueAsync(CancellationToken.None)); + } + + await using var verify = new SqliteUpliftDecisionTestDbContext(options); + var saved = await verify.DispatchUpliftRequests.AsNoTracking().SingleAsync(u => u.Id == upliftId); + Assert.NotNull(saved.EscalatedAt); + Assert.Equal(1, emailSender.Calls); + + var audits = await verify.WorkOrderAuditLogs.AsNoTracking().ToListAsync(); + Assert.DoesNotContain(audits, a => a.WorkOrderId == 0); + if (expectedWorkOrderId is int workOrderId) + Assert.Equal(workOrderId, Assert.Single(audits).WorkOrderId); + else + Assert.Empty(audits); + } + private static async Task<(int UpliftId, int? ExpectedWorkOrderId)> SeedAsync( ApplicationDbContext context, Ownership ownership, - DateTime? expiresAt) + DateTime? expiresAt, + DateTime? initialNotificationSentAt = null) { context.Users.Add(new ApplicationUser { @@ -218,7 +269,8 @@ public sealed class UpliftDecisionAuditRelationalTests RequiredTier = 2, CreatedDate = DateTime.UtcNow.AddDays(-4), ExpiresAt = expiresAt, - NotificationStatus = "Pending" + InitialNotificationSentAt = initialNotificationSentAt, + NotificationStatus = initialNotificationSentAt == null ? "Pending" : "Sent" }; context.DispatchUpliftRequests.Add(request); await context.SaveChangesAsync(); @@ -238,9 +290,16 @@ public sealed class UpliftDecisionAuditRelationalTests => throw new NotSupportedException(); } + // Records sends without delivering anything. private sealed class NoEmailSender : IEmailSender { - public Task SendEmailAsync(string emailTo, string subject, string body) => Task.FromResult(true); + public int Calls; + + public Task SendEmailAsync(string emailTo, string subject, string body) + { + Calls++; + return Task.FromResult(true); + } } private sealed class SqliteUpliftDecisionTestDbContext : ApplicationDbContext