diff --git a/SeaHaven.Services/Implementation/WorkOrderBoardCancelService.cs b/SeaHaven.Services/Implementation/WorkOrderBoardCancelService.cs index f0347f2..94fe959 100644 --- a/SeaHaven.Services/Implementation/WorkOrderBoardCancelService.cs +++ b/SeaHaven.Services/Implementation/WorkOrderBoardCancelService.cs @@ -35,41 +35,39 @@ namespace SeaHaven.Services.Implementation ClaimsPrincipal user, string? actorId) { - return await _upliftData.ExecuteWorkOrderMutationAsync( + // 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, - ct => CancelLockedAsync(workOrderId, user, actorId, ct), + async ct => + { + var workOrder = await _mutationData.GetTrackedWorkOrderAsync(workOrderId, ct); + + if (workOrder == null) + 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."); + + 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, ct); + await _auditService.StageStatusChangedAsync(workOrderId, oldStatus, LifecycleStatus.Canceled.ToString(), actorId); + await _mutationData.SaveAsync(ct); + return true; + }, CancellationToken.None); - } - - private async Task CancelLockedAsync( - int workOrderId, - ClaimsPrincipal user, - string? actorId, - CancellationToken cancellationToken) - { - var workOrder = await _mutationData.GetTrackedWorkOrderAsync(workOrderId, cancellationToken); - - 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."); - } - - 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; - - await _upliftService.WithdrawPendingForWorkOrderAsync(workOrderId, actorId, cancellationToken); - await _auditService.StageStatusChangedAsync(workOrderId, oldStatus, LifecycleStatus.Canceled.ToString(), actorId); - await _mutationData.SaveAsync(cancellationToken); 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 d53b7b9..6a2c388 100644 --- a/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs +++ b/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs @@ -101,7 +101,7 @@ namespace SeaHaven.Services.Implementation if (workOrder.LifecycleStatus is LifecycleStatus.Completed or LifecycleStatus.Canceled) { throw new InvalidOperationException( - $"Cannot change uplifts on a '{workOrder.LifecycleStatus}' work order"); + $"Cannot create an uplift on a '{workOrder.LifecycleStatus}' work order"); } var dispatch = await _dispatchData.GetByIdAsync(dispatchId); @@ -166,8 +166,6 @@ namespace SeaHaven.Services.Implementation if (!await HasWorkOrderAccessAsync(workOrderId, user, cancellationToken)) return null; - await EnsureWorkOrderAllowsUpliftMutationAsync(workOrderId, user, cancellationToken); - var req = await _upliftData.GetByIdAndWorkOrderAsync(upliftId, workOrderId, cancellationToken); if (req == null) throw new KeyNotFoundException("Uplift request not found"); @@ -179,6 +177,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); @@ -213,8 +215,6 @@ namespace SeaHaven.Services.Implementation if (!await HasWorkOrderAccessAsync(workOrderId, user, cancellationToken)) return null; - await EnsureWorkOrderAllowsUpliftMutationAsync(workOrderId, user, cancellationToken); - var req = await _upliftData.GetByIdAndWorkOrderAsync(upliftId, workOrderId, cancellationToken); if (req == null) throw new KeyNotFoundException("Uplift request not found"); @@ -247,9 +247,20 @@ 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; + // 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; @@ -359,24 +370,24 @@ namespace SeaHaven.Services.Implementation return WorkOrderUpliftContractMapper.MapItem(created, requesterName, null); } - private async Task EnsureWorkOrderAllowsUpliftMutationAsync( + /// + /// 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); - if (workOrder == null) - throw new KeyNotFoundException("Work order not found"); + var workOrder = await _detailData.GetWorkOrderForMediaAsync(workOrderId, cancellationToken, accountFilter); + var lifecycle = workOrder?.LifecycleStatus; - if (workOrder.LifecycleStatus is LifecycleStatus.Completed or LifecycleStatus.Canceled) - { + if (lifecycle == LifecycleStatus.Completed || lifecycle == LifecycleStatus.Canceled) throw new InvalidOperationException( - $"Cannot change uplifts on a '{workOrder.LifecycleStatus}' work order"); - } + $"Cannot {action} an uplift on a '{lifecycle}' work order"); } private async Task HasWorkOrderAccessAsync( diff --git a/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs b/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs index 42d1fa6..5bbec79 100644 --- a/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs @@ -26,12 +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(), - new UpliftDataService(context)); + var cancel = new WorkOrderBoardCancelService(mutationData, boardService, audit, new NoOpUpliftService(), new PassThroughUpliftData()); var update = new WorkOrderBoardUpdateService(boardData, mutationData, audit); return (context, cancel, update); } @@ -210,7 +205,7 @@ public class WorkOrderBoardCancelServiceTests new UserDataService(context), TimeProvider.System, Options.Create(new ApprovalsOptions())); - var cancel = new WorkOrderBoardCancelService(mutationData, boardService, audit, uplifts, new UpliftDataService(context)); + var cancel = new WorkOrderBoardCancelService(mutationData, boardService, audit, uplifts, new PassThroughUpliftData()); var result = await cancel.CancelAsync(1, WorkOrderAccountTestHelpers.OrgWideAdmin(), "actor-1"); @@ -276,7 +271,7 @@ public class WorkOrderBoardCancelServiceTests boardService, audit, uplifts, - new UpliftDataService(context)); + new PassThroughUpliftData()); await Assert.ThrowsAsync(() => cancel.CancelAsync(1, WorkOrderAccountTestHelpers.OrgWideAdmin(), "actor-1")); @@ -419,6 +414,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 5b7d6f9..7ec8856 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() {