mirror of
https://github.com/Sea-Haven-Industries/shoc-backend.git
synced 2026-09-30 04:53:11 +00:00
fix(work-orders): symmetric NTE release, terminal guard, serialized cancel (SH-196)
Three contract gaps found reviewing the frontend consumer: - Revoking an auto-approved uplift never restored the dispatch NTE. Create raises NTE for both auto-approved and approved requests, but revoke restored it only for Approved, so the allowance was freed while the NTE stayed raised and every create -> auto-approve -> revoke cycle compounded the inflation. Revoke now compensates for NoApprovalRequired symmetrically. - Revoke and cancel had no work-order lifecycle check, so a direct API call could still mutate uplifts on a Completed or Canceled work order; the board dialog's read-only state is UX only. Both now reject terminal work orders in the service. - WorkOrderBoardCancelService read the pending-uplift list outside any gate, so an in-flight create could commit after that read and leave a pending uplift on a Canceled work order. The cancel flow now runs inside the same per-work-order gate as create, so the pending read, withdrawal and status audit serialize against it.
This commit is contained in:
parent
aeface594a
commit
2327097d5e
4 changed files with 213 additions and 23 deletions
|
|
@ -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<WorkOrderBoardRowDto> 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.");
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
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<bool> HasWorkOrderAccessAsync(
|
||||
int workOrderId,
|
||||
ClaimsPrincipal user,
|
||||
|
|
|
|||
|
|
@ -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<InvalidOperationException>(() =>
|
||||
cancel.CancelAsync(1, WorkOrderAccountTestHelpers.OrgWideAdmin(), "actor-1"));
|
||||
|
|
@ -327,6 +328,41 @@ public class WorkOrderBoardCancelServiceTests
|
|||
=> throw new InvalidOperationException("forced late failure");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
private sealed class PassThroughUpliftData : IUpliftDataService
|
||||
{
|
||||
public Task<T> ExecuteWorkOrderMutationAsync<T>(
|
||||
int workOrderId,
|
||||
Func<CancellationToken, Task<T>> work,
|
||||
CancellationToken cancellationToken) => work(cancellationToken);
|
||||
|
||||
public Task<(int TotalCount, IReadOnlyList<UpliftListItemData> Items)> GetPagedAsync(
|
||||
string? status, int? tier, int page, int pageSize, CancellationToken cancellationToken) =>
|
||||
throw new NotSupportedException();
|
||||
public Task<IReadOnlyList<UpliftForDispatchData>> GetForDispatchAsync(int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<IReadOnlyList<UpliftForWorkOrderData>> GetForWorkOrderAsync(int workOrderId, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<DispatchUpliftRequest?> GetByIdAndWorkOrderAsync(int requestId, int workOrderId, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<IReadOnlyList<PortalUpliftData>> GetForVendorDispatchAsync(int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<DispatchUpliftRequest?> GetByIdAsync(int id, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<DispatchUpliftRequest?> GetByIdAndDispatchAsync(int requestId, int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<UpliftEvidenceDownloadData?> GetEvidenceForInternalDownloadAsync(int upliftRequestId, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<bool> HasPendingAsync(int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<bool> HasPendingForWorkOrderAsync(int workOrderId, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<decimal> SumAutoApprovedAmountForWorkOrderAsync(int workOrderId, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<List<DispatchUpliftRequest>> GetPendingForWorkOrderAsync(int workOrderId, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<bool> HasActiveAsync(int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<DispatchUpliftRequest?> GetActiveRequestAsync(int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<DispatchUpliftRequest?> GetByRequestKeyAsync(int dispatchId, string requestKey, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<List<DispatchUpliftRequest>> GetDueForExpiryAsync(DateTime utcNow, int count, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<List<DispatchUpliftRequest>> GetDueForInitialNotificationAsync(int count, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<List<DispatchUpliftRequest>> 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<IReadOnlyList<WorkOrderUpliftDto>?> ListAsync(
|
||||
|
|
|
|||
|
|
@ -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<InvalidOperationException>(() => 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<InvalidOperationException>(() => 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()
|
||||
{
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue