diff --git a/SeaHaven.Services/Implementation/WorkOrderBoardCancelService.cs b/SeaHaven.Services/Implementation/WorkOrderBoardCancelService.cs index e847b23..94fe959 100644 --- a/SeaHaven.Services/Implementation/WorkOrderBoardCancelService.cs +++ b/SeaHaven.Services/Implementation/WorkOrderBoardCancelService.cs @@ -14,17 +14,20 @@ namespace SeaHaven.Services.Implementation private readonly IWorkOrderBoardService _boardService; private readonly IWorkOrderAuditService _auditService; private readonly IWorkOrderUpliftService _upliftService; + private readonly IUpliftDataService _upliftData; public WorkOrderBoardCancelService( IWorkOrderBoardMutationDataService mutationData, IWorkOrderBoardService boardService, IWorkOrderAuditService auditService, - IWorkOrderUpliftService upliftService) + IWorkOrderUpliftService upliftService, + IUpliftDataService upliftData) { _mutationData = mutationData; _boardService = boardService; _auditService = auditService; _upliftService = upliftService; + _upliftData = upliftData; } public async Task CancelAsync( @@ -32,29 +35,39 @@ namespace SeaHaven.Services.Implementation ClaimsPrincipal user, string? actorId) { - var workOrder = await _mutationData.GetTrackedWorkOrderAsync(workOrderId, CancellationToken.None); + // SH-196: run the whole cancel under the same per-work-order gate that uplift + // create uses. Previously the pending-uplift read happened outside any gate, so + // an in-flight create could commit after that read and leave a pending uplift on + // a Canceled work order, breaking "cancelling a WO with a pending uplift cancels + // that uplift in the same action". + await _upliftData.ExecuteWorkOrderMutationAsync( + workOrderId, + async ct => + { + var workOrder = await _mutationData.GetTrackedWorkOrderAsync(workOrderId, ct); - if (workOrder == null) - throw new WorkOrderBoardValidationException("NotFound", "Work order not found."); + if (workOrder == null) + throw new WorkOrderBoardValidationException("NotFound", "Work order not found."); - if (workOrder.LifecycleStatus == LifecycleStatus.Canceled) - { - var existing = await _boardService.GetBoardRowAsync(workOrderId, user); - return existing ?? throw new WorkOrderBoardValidationException("NotFound", "Work order not found."); - } + // Already canceled: nothing to mutate, the row is returned below. + if (workOrder.LifecycleStatus == LifecycleStatus.Canceled) + return false; - if (workOrder.LifecycleStatus == LifecycleStatus.Completed) - throw new WorkOrderBoardValidationException("CancelNotAllowed", "Work order cannot be canceled in its current status."); + if (workOrder.LifecycleStatus == LifecycleStatus.Completed) + throw new WorkOrderBoardValidationException("CancelNotAllowed", "Work order cannot be canceled in its current status."); - var oldStatus = workOrder.LifecycleStatus?.ToString() ?? workOrder.Status ?? ""; - workOrder.LifecycleStatus = LifecycleStatus.Canceled; - workOrder.Status = WorkOrderDerivedFields.GetLifecycleStatusLabel(LifecycleStatus.Canceled); - if (workOrder.LegacyStatus == null && workOrder.Status != null) - workOrder.LegacyStatus = workOrder.Status; + var oldStatus = workOrder.LifecycleStatus?.ToString() ?? workOrder.Status ?? ""; + workOrder.LifecycleStatus = LifecycleStatus.Canceled; + workOrder.Status = WorkOrderDerivedFields.GetLifecycleStatusLabel(LifecycleStatus.Canceled); + if (workOrder.LegacyStatus == null && workOrder.Status != null) + workOrder.LegacyStatus = workOrder.Status; - await _upliftService.WithdrawPendingForWorkOrderAsync(workOrderId, actorId, CancellationToken.None); - await _auditService.StageStatusChangedAsync(workOrderId, oldStatus, LifecycleStatus.Canceled.ToString(), actorId); - await _mutationData.SaveAsync(CancellationToken.None); + await _upliftService.WithdrawPendingForWorkOrderAsync(workOrderId, actorId, ct); + await _auditService.StageStatusChangedAsync(workOrderId, oldStatus, LifecycleStatus.Canceled.ToString(), actorId); + await _mutationData.SaveAsync(ct); + return true; + }, + CancellationToken.None); var row = await _boardService.GetBoardRowAsync(workOrderId, user); return row ?? throw new WorkOrderBoardValidationException("NotFound", "Work order not found."); diff --git a/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs b/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs index 45ff8ea..e36801b 100644 --- a/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs +++ b/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs @@ -1,5 +1,6 @@ using System.Security.Claims; using Data.SeaHavenIndustries; +using Data.SeaHavenIndustries.Enums; using Microsoft.Extensions.Options; using SeaHaven.DataServices.Interfaces; using SeaHaven.Services.Configuration; @@ -170,6 +171,10 @@ namespace SeaHaven.Services.Implementation if (dispatch == null) throw new KeyNotFoundException("Dispatch not found"); + // SH-196: same terminal-lifecycle guard as revoke — the read-only dialog state + // is UX only and does not stop a direct API call. + await EnsureWorkOrderAcceptsUpliftMutationAsync(workOrderId, user, "cancel", cancellationToken); + var userId = user.FindFirstValue(ClaimTypes.NameIdentifier); var now = _timeProvider.GetUtcNow().UtcDateTime; var previous = UpliftStatus.ToCanonical(req.Status); @@ -236,10 +241,21 @@ namespace SeaHaven.Services.Implementation if (dispatch == null) throw new KeyNotFoundException("Dispatch not found"); + // SH-196: revoking is blocked once the work order is Completed or Canceled. + // The board dialog enforces this in the UI only, so without a service-side + // guard a direct API call could still mutate uplifts on a terminal WO. + await EnsureWorkOrderAcceptsUpliftMutationAsync(workOrderId, user, "revoke", cancellationToken); + var now = _timeProvider.GetUtcNow().UtcDateTime; var previous = canonical; - if (canonical == UpliftStatus.Approved) + // SH-196: revoking must free the work order's uplift capacity again. Create + // raises the dispatch NTE for BOTH auto-approved and approved uplifts, so + // revoke has to compensate symmetrically. Restoring only on Approved left the + // NTE raised while SumAutoApprovedAmountForWorkOrderAsync stopped counting the + // revoked request, so every create -> auto-approve -> revoke cycle compounded + // the inflation and handed back allowance that was never actually released. + if (canonical == UpliftStatus.Approved || canonical == UpliftStatus.NoApprovalRequired) { var oldNte = dispatch.NTEAmount ?? 0m; dispatch.NTEAmount = req.CurrentNTE ?? oldNte; @@ -348,6 +364,26 @@ namespace SeaHaven.Services.Implementation return WorkOrderUpliftContractMapper.MapItem(created, requesterName, null); } + /// + /// SH-196: uplifts may not be mutated once the work order reaches a terminal + /// lifecycle state. Enforced in the service so a direct API call is rejected too, + /// not only the board dialog's read-only state. + /// + private async Task EnsureWorkOrderAcceptsUpliftMutationAsync( + int workOrderId, + ClaimsPrincipal user, + string action, + CancellationToken cancellationToken) + { + var accountFilter = _accountResolver.ResolveAccountFilter(user); + var workOrder = await _detailData.GetWorkOrderForMediaAsync(workOrderId, cancellationToken, accountFilter); + var lifecycle = workOrder?.LifecycleStatus; + + if (lifecycle == LifecycleStatus.Completed || lifecycle == LifecycleStatus.Canceled) + throw new InvalidOperationException( + $"Cannot {action} an uplift on a '{lifecycle}' work order"); + } + private async Task HasWorkOrderAccessAsync( int workOrderId, ClaimsPrincipal user, diff --git a/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs b/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs index 2aae357..d3bd1d9 100644 --- a/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs @@ -26,7 +26,7 @@ public class WorkOrderBoardCancelServiceTests var boardService = new WorkOrderBoardService(boardData, WorkOrderAccountTestHelpers.Resolver(context)); var fieldLocks = new WorkOrderFieldLockService(new WorkOrderFieldLockDataService(context)); var audit = new WorkOrderAuditService(new WorkOrderAuditDataService(context), fieldLocks); - var cancel = new WorkOrderBoardCancelService(mutationData, boardService, audit, new NoOpUpliftService()); + var cancel = new WorkOrderBoardCancelService(mutationData, boardService, audit, new NoOpUpliftService(), new PassThroughUpliftData()); var update = new WorkOrderBoardUpdateService(boardData, mutationData, audit); return (context, cancel, update); } @@ -205,7 +205,7 @@ public class WorkOrderBoardCancelServiceTests new UserDataService(context), TimeProvider.System, Options.Create(new ApprovalsOptions())); - var cancel = new WorkOrderBoardCancelService(mutationData, boardService, audit, uplifts); + var cancel = new WorkOrderBoardCancelService(mutationData, boardService, audit, uplifts, new PassThroughUpliftData()); var result = await cancel.CancelAsync(1, WorkOrderAccountTestHelpers.OrgWideAdmin(), "actor-1"); @@ -270,7 +270,8 @@ public class WorkOrderBoardCancelServiceTests new ThrowingSaveMutationData(mutationData), boardService, audit, - uplifts); + uplifts, + new PassThroughUpliftData()); await Assert.ThrowsAsync(() => cancel.CancelAsync(1, WorkOrderAccountTestHelpers.OrgWideAdmin(), "actor-1")); @@ -327,6 +328,41 @@ public class WorkOrderBoardCancelServiceTests => throw new InvalidOperationException("forced late failure"); } + /// + /// SH-196: the cancel flow now runs inside the per-work-order gate. Tests only need the + /// gate to invoke the work; the real serialization is exercised against SQL Server. + /// + private sealed class PassThroughUpliftData : IUpliftDataService + { + public Task ExecuteWorkOrderMutationAsync( + int workOrderId, + Func> work, + CancellationToken cancellationToken) => work(cancellationToken); + + public Task<(int TotalCount, IReadOnlyList Items)> GetPagedAsync( + string? status, int? tier, int page, int pageSize, CancellationToken cancellationToken) => + throw new NotSupportedException(); + public Task> GetForDispatchAsync(int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task> GetForWorkOrderAsync(int workOrderId, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task GetByIdAndWorkOrderAsync(int requestId, int workOrderId, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task> GetForVendorDispatchAsync(int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task GetByIdAsync(int id, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task GetByIdAndDispatchAsync(int requestId, int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task GetEvidenceForInternalDownloadAsync(int upliftRequestId, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task HasPendingAsync(int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task HasPendingForWorkOrderAsync(int workOrderId, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task SumAutoApprovedAmountForWorkOrderAsync(int workOrderId, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task> GetPendingForWorkOrderAsync(int workOrderId, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task HasActiveAsync(int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task GetActiveRequestAsync(int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task GetByRequestKeyAsync(int dispatchId, string requestKey, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task> GetDueForExpiryAsync(DateTime utcNow, int count, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task> GetDueForInitialNotificationAsync(int count, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task> GetDueForEscalationAsync(int count, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task StageAsync(DispatchUpliftRequest request, CancellationToken cancellationToken) => throw new NotSupportedException(); + public Task SaveChangesAsync(CancellationToken cancellationToken) => throw new NotSupportedException(); + } + private sealed class NoOpUpliftService : IWorkOrderUpliftService { public Task?> ListAsync( diff --git a/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs b/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs index 9ca211e..67e24d4 100644 --- a/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs @@ -340,6 +340,111 @@ public sealed class WorkOrderUpliftServiceTests Assert.Equal(1000m, dispatch.NTEAmount); } + [Fact] + public async Task RevokeAsync_AutoApproved_RestoresNteAndFreesAllowance() + { + await using var context = CreateContext(); + var (workOrder, dispatch) = await SeedWorkOrderAsync(context); + var service = NewService(context); + + // Auto-approved create raises the dispatch NTE by the requested amount. + var created = await service.CreateAsync( + workOrder.Id, + new CreateWorkOrderUpliftRequestDto { Amount = 400m, Notes = "Within limit" }, + Dispatcher(), + CancellationToken.None); + Assert.Equal("auto_approved", created!.Status); + Assert.Equal(1400m, context.Dispatches.Single(d => d.Id == 10).NTEAmount); + + var revoked = await service.RevokeAsync( + workOrder.Id, + created.Id, + new RevokeWorkOrderUpliftRequestDto(), + Dispatcher(), + CancellationToken.None); + + // SH-196: revoking frees the allowance, so it must release the NTE too. Otherwise + // every create -> revoke cycle compounds NTE inflation. + Assert.Equal("revoked", revoked!.Status); + Assert.Equal(1000m, context.Dispatches.Single(d => d.Id == 10).NTEAmount); + + // The freed allowance is reusable at the full cap. + var second = await service.CreateAsync( + workOrder.Id, + new CreateWorkOrderUpliftRequestDto { Amount = 500m, Notes = "Reuses freed allowance" }, + Dispatcher(), + CancellationToken.None); + Assert.Equal("auto_approved", second!.Status); + Assert.Equal(1500m, context.Dispatches.Single(d => d.Id == 10).NTEAmount); + } + + [Theory] + [InlineData(LifecycleStatus.Completed)] + [InlineData(LifecycleStatus.Canceled)] + public async Task RevokeAsync_TerminalWorkOrder_IsRejected(LifecycleStatus lifecycle) + { + await using var context = CreateContext(); + var (workOrder, dispatch) = await SeedWorkOrderAsync(context); + dispatch.NTEAmount = 1500m; + context.DispatchUpliftRequests.Add(new DispatchUpliftRequest + { + Id = 100, + DispatchId = dispatch.Id, + RequestedNTE = 1500m, + CurrentNTE = 1000m, + Status = "Approved", + RequiredTier = 1, + NotificationStatus = "Sent", + }); + workOrder.LifecycleStatus = lifecycle; + await context.SaveChangesAsync(); + + var service = NewService(context); + + // SH-196: the board dialog's read-only state is UX only; a direct API call must + // still be rejected server-side. + await Assert.ThrowsAsync(() => service.RevokeAsync( + workOrder.Id, + 100, + new RevokeWorkOrderUpliftRequestDto { Reason = "Policy change" }, + WorkOrderAccountTestHelpers.OrgWideAdmin("admin-1"), + CancellationToken.None)); + + Assert.Equal(1500m, context.Dispatches.Single(d => d.Id == 10).NTEAmount); + Assert.Equal("Approved", context.DispatchUpliftRequests.Single(r => r.Id == 100).Status); + } + + [Theory] + [InlineData(LifecycleStatus.Completed)] + [InlineData(LifecycleStatus.Canceled)] + public async Task CancelAsync_TerminalWorkOrder_IsRejected(LifecycleStatus lifecycle) + { + await using var context = CreateContext(); + var (workOrder, dispatch) = await SeedWorkOrderAsync(context); + context.DispatchUpliftRequests.Add(new DispatchUpliftRequest + { + Id = 101, + DispatchId = dispatch.Id, + RequestedNTE = 1600m, + CurrentNTE = 1000m, + Status = "Pending", + RequiredTier = 1, + NotificationStatus = "Sent", + }); + workOrder.LifecycleStatus = lifecycle; + await context.SaveChangesAsync(); + + var service = NewService(context); + + await Assert.ThrowsAsync(() => service.CancelAsync( + workOrder.Id, + 101, + Dispatcher(), + CancellationToken.None)); + + Assert.Equal("Pending", context.DispatchUpliftRequests.Single(r => r.Id == 101).Status); + } + [Fact] public async Task CreateAsync_ConcurrentRequests_PreserveOnePendingAndCap() {