feat(api): Phase 6 — optimistic concurrency (SHOC contract) + atomic audit staging (#226)

This commit is contained in:
Adam Moussa 2026-07-14 01:18:30 -04:00 • committed by GitHub
parent 4629f7a21a
commit cac4384f0d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
50 changed files with 2148 additions and 225 deletions

View file

@ -118,7 +118,7 @@ Six parallel jobs calling org reusable workflows:
| Job | Workflow | What it checks |
|---|---|---|
| .NET Build & Test | `ci-dotnet.yaml` | Restore, build, test the API solution (123 xUnit tests) |
| .NET Build & Test | `ci-dotnet.yaml` | Restore, build, test the API solution (xUnit) |
| Web Frontend Check | `ci-typescript-frontend.yaml` | Prettier `format:check`, build (includes `tsc -b`), vitest suite, Playwright chromium smoke (dev-login → proposal list, API mocked) |
| Mobile Typecheck | `ci-typescript-cdk.yaml` | TypeScript typecheck for mobile |
| Python Lint | `ci-python-sam.yaml` | ruff check + format on lambdas/ |
@ -169,6 +169,10 @@ Two-layer auth architecture with defense-in-depth:
**Internal API key:** Python Lambdas call the .NET API via a Lambda Function URL with AWS_IAM auth (bypasses API Gateway JWT check). The `InternalApiKeyMiddleware` validates the `X-Internal-Api-Key` header and assigns the `admins` role to the synthetic identity. Lambdas cache the API key from Secrets Manager with a 5-minute TTL.
**Optimistic concurrency (ADR 0004):** proposal responses carry an opaque `rowVersion` token; mutations of the proposal aggregate (update, approve, return-to-review, send, revise, bulk line-item update) require `proposalVersion` in the body. Missing token → 422 `ProposalVersionRequired`, malformed → 422 `InvalidRowVersion`, stale → **409 `{ message, currentState }`** with the reloaded proposal embedded (unguarded races → 409 `{ status, message, code }`). Line-item create/delete are token-less but bump the aggregate version. Audit rows commit atomically with their mutation (stage-then-single-SaveChanges).
**Deploy note for schema changes:** EF migrations auto-apply at API startup under a `pg_advisory_lock`. Before deploying a migration: take a manual RDS snapshot; additive-only migrations are backward-compatible with the previous Lambda version. Test the down-script against a snapshot-restored copy before any production rollback.
## Data Flow
1. Dispatcher submits proposal request (web or mobile)

View file

@ -189,8 +189,9 @@ public class FilesController : ControllerBase
};
_db.GeneratedPdfs.Add(generatedPdf);
// Stage-then-single-SaveChanges: the audit row commits atomically with the PDF record.
_audit.Stage(AuditAction.GeneratePDF, proposalId);
await _db.SaveChangesAsync(ct);
await _audit.LogAsync(AuditAction.GeneratePDF, proposalId, null, ct);
return PhysicalFile(filePath, "application/pdf", Path.GetFileName(filePath));
}

View file

@ -1,5 +1,6 @@
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using ProposalSystem.Application.DTOs;
using ProposalSystem.Application.Interfaces;
@ -74,33 +75,45 @@ public class ProposalsController : ControllerBase
[ProducesResponseType(typeof(ProposalResponse), 200)]
[ProducesResponseType(400)]
[ProducesResponseType(404)]
public async Task<ActionResult<ProposalResponse>> Approve(Guid id, CancellationToken ct)
public async Task<ActionResult<ProposalResponse>> Approve(
Guid id,
[FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Allow)] ProposalVersionRequest? request,
CancellationToken ct)
{
var result = await _proposalService.ApproveAsync(id, ct);
var result = await _proposalService.ApproveAsync(id, request?.ProposalVersion, ct);
return Ok(result);
}
[HttpPost("{id:guid}/return-to-review")]
[Authorize(Roles = "admins,sysadmins")]
public async Task<ActionResult<ProposalResponse>> ReturnToReview(Guid id, CancellationToken ct)
public async Task<ActionResult<ProposalResponse>> ReturnToReview(
Guid id,
[FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Allow)] ProposalVersionRequest? request,
CancellationToken ct)
{
var result = await _proposalService.ReturnToReviewAsync(id, ct);
var result = await _proposalService.ReturnToReviewAsync(id, request?.ProposalVersion, ct);
return Ok(result);
}
[HttpPost("{id:guid}/send")]
[Authorize(Roles = "admins,sysadmins")]
public async Task<ActionResult<ProposalResponse>> MarkSent(Guid id, CancellationToken ct)
public async Task<ActionResult<ProposalResponse>> MarkSent(
Guid id,
[FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Allow)] ProposalVersionRequest? request,
CancellationToken ct)
{
var result = await _proposalService.MarkSentAsync(id, ct);
var result = await _proposalService.MarkSentAsync(id, request?.ProposalVersion, ct);
return Ok(result);
}
[HttpPost("{id:guid}/revise")]
[Authorize(Roles = "admins,sysadmins")]
public async Task<ActionResult<ProposalResponse>> Revise(Guid id, CancellationToken ct)
public async Task<ActionResult<ProposalResponse>> Revise(
Guid id,
[FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Allow)] ProposalVersionRequest? request,
CancellationToken ct)
{
var result = await _proposalService.ReviseAsync(id, ct);
var result = await _proposalService.ReviseAsync(id, request?.ProposalVersion, ct);
return Ok(result);
}

View file

@ -45,6 +45,12 @@ public class VendorProposalsController : ControllerBase
proposal.VendorTotalCost = await _db.VendorProposals
.Where(v => v.ProposalId == vendor.ProposalId)
.SumAsync(v => v.TotalVendorCost, ct);
// Aggregate write must bump the concurrency version (scanner sweep
// finding): unguarded like line-item appends, but a lost race then
// surfaces as the fallback 409 instead of silently overwriting a
// concurrent guarded edit.
proposal.Version++;
proposal.UpdatedAt = DateTime.UtcNow;
await _db.SaveChangesAsync(ct);
}

View file

@ -2,6 +2,7 @@ using System.Net;
using System.Text.Json;
using FluentValidation;
using Microsoft.AspNetCore.Mvc;
using Microsoft.EntityFrameworkCore;
using ProposalSystem.Application.Common;
namespace ProposalSystem.Api.Middleware;
@ -27,6 +28,8 @@ public class GlobalExceptionHandler : IMiddleware
}
}
private static string Sanitize(string? value) => (value ?? "").Replace('\r', '_').Replace('\n', '_');
private static ProblemDetails MakeProblem(int status, string title, string detail, string code)
{
var problem = new ProblemDetails
@ -39,8 +42,50 @@ public class GlobalExceptionHandler : IMiddleware
return problem;
}
// Must match the MVC pipeline's serialization (camelCase + string enums):
// the 409 currentState embeds a ProposalResponse, and clients schema-validate
// it against the same shapes their 200 responses use.
private static readonly JsonSerializerOptions CamelCase = new()
{
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
Converters = { new System.Text.Json.Serialization.JsonStringEnumConverter() },
};
private async Task HandleExceptionAsync(HttpContext context, Exception exception)
{
// Concurrency conflicts use SHOC's envelopes verbatim (ADR 0004), NOT
// ProblemDetails: the guarded path embeds the reloaded currentState for
// immediate client refresh; the bare-DbUpdateConcurrencyException shape
// is the safety net for any unguarded write.
if (exception is ProposalConcurrencyException concurrencyEx)
{
_logger.LogWarning("Concurrency conflict on {Method} {Path}",
Sanitize(context.Request.Method), Sanitize(context.Request.Path));
context.Response.StatusCode = StatusCodes.Status409Conflict;
context.Response.ContentType = "application/json";
await context.Response.WriteAsync(JsonSerializer.Serialize(new
{
message = concurrencyEx.Message,
currentState = concurrencyEx.CurrentState,
}, CamelCase));
return;
}
if (exception is DbUpdateConcurrencyException)
{
_logger.LogWarning("Unguarded concurrency conflict on {Method} {Path}",
Sanitize(context.Request.Method), Sanitize(context.Request.Path));
context.Response.StatusCode = StatusCodes.Status409Conflict;
context.Response.ContentType = "application/json";
await context.Response.WriteAsync(JsonSerializer.Serialize(new
{
status = "Conflict",
message = "The record was modified by another user. Refresh and retry.",
code = 409,
}, CamelCase));
return;
}
// Every response carries a machine-readable "code" extension (SHOC
// error-code vocabulary convention) so clients branch on codes, not
// on human-readable text.
@ -77,7 +122,7 @@ public class GlobalExceptionHandler : IMiddleware
};
_logger.LogError(exception, "Exception on {Method} {Path}: {Status}",
context.Request.Method, context.Request.Path, (int)statusCode);
Sanitize(context.Request.Method), Sanitize(context.Request.Path), (int)statusCode);
context.Response.StatusCode = (int)statusCode;
context.Response.ContentType = "application/problem+json";

View file

@ -0,0 +1,21 @@
using ProposalSystem.Application.DTOs;
namespace ProposalSystem.Application.Common;
/// <summary>
/// Optimistic-concurrency conflict on the proposal aggregate. Carries the
/// freshly reloaded current state so the 409 body can embed it for immediate
/// client refresh (SHOC contract: 409 { message, currentState }). Typed as
/// the response DTO so an EF entity (with navigation graphs) can never be
/// serialized into the error body by accident.
/// </summary>
public class ProposalConcurrencyException : Exception
{
public ProposalResponse? CurrentState { get; }
public ProposalConcurrencyException(ProposalResponse? currentState)
: base("The record was modified by another user. Refresh and retry.")
{
CurrentState = currentState;
}
}

View file

@ -0,0 +1,31 @@
using System.Buffers.Binary;
namespace ProposalSystem.Application.Common;
/// <summary>
/// Encodes the integer concurrency version as an opaque base64 token
/// (8 bytes, big-endian), matching SHOC's rowversion wire contract:
/// clients receive "rowVersion" strings and echo them back untouched.
/// </summary>
public static class RowVersionCodec
{
public static string Encode(long version)
{
Span<byte> buffer = stackalloc byte[8];
BinaryPrimitives.WriteInt64BigEndian(buffer, version);
return Convert.ToBase64String(buffer);
}
public static bool TryDecode(string? token, out long version)
{
version = 0;
if (string.IsNullOrEmpty(token)) return false;
Span<byte> buffer = stackalloc byte[8];
if (!Convert.TryFromBase64String(token, buffer, out var written) || written != 8)
return false;
version = BinaryPrimitives.ReadInt64BigEndian(buffer);
return true;
}
}

View file

@ -14,7 +14,8 @@ public record LineItemResponse(
int SortOrder,
LineItemSource Source,
DateTime CreatedAt,
DateTime UpdatedAt
DateTime UpdatedAt,
string RowVersion
);
public record CreateLineItemRequest(
@ -29,7 +30,8 @@ public record CreateLineItemRequest(
);
public record BulkUpdateLineItemsRequest(
List<UpdateLineItemEntry> LineItems
List<UpdateLineItemEntry> LineItems,
string? ProposalVersion = null
);
public record UpdateLineItemEntry(

View file

@ -18,9 +18,14 @@ public record UpdateProposalRequest(
string? Notes,
string? PoNumber,
string? WorkOrderNumber,
Guid? AssignedAdminId
Guid? AssignedAdminId,
string? ProposalVersion = null
);
/// <summary>Body for state-transition endpoints (approve, return-to-review,
/// mark-sent, revise): the expected proposal version token.</summary>
public record ProposalVersionRequest(string? ProposalVersion);
public record ProposalResponse(
Guid Id,
string ProposalNumber,
@ -46,7 +51,8 @@ public record ProposalResponse(
int CurrentRevision,
Guid? ParentProposalId,
DateTime CreatedAt,
DateTime UpdatedAt
DateTime UpdatedAt,
string RowVersion
);
public record ProposalListResponse(
@ -60,7 +66,8 @@ public record ProposalListResponse(
decimal TotalBidAmount,
DateTime SubmittedAt,
string? SubmittedByName,
string? AssignedAdminName
string? AssignedAdminName,
string RowVersion
);
public record ProposalFilterRequest(

View file

@ -4,5 +4,12 @@ namespace ProposalSystem.Application.Interfaces;
public interface IAuditService
{
/// <summary>Writes the audit entry in its own SaveChanges. For standalone
/// events (downloads, role changes) where no other write is in flight.</summary>
Task LogAsync(AuditAction action, Guid? proposalId, string? details = null, CancellationToken ct = default);
/// <summary>Stages the audit entry on the shared DbContext WITHOUT saving.
/// Mutation flows call this before their own SaveChangesAsync so the domain
/// change and its audit row commit atomically or not at all.</summary>
void Stage(AuditAction action, Guid? proposalId, string? details = null);
}

View file

@ -8,10 +8,10 @@ public interface IProposalService
Task<ProposalResponse?> GetByIdAsync(Guid id, CancellationToken ct = default);
Task<PagedResponse<ProposalListResponse>> GetAllAsync(ProposalFilterRequest filter, CancellationToken ct = default);
Task<ProposalResponse> UpdateAsync(Guid id, UpdateProposalRequest request, CancellationToken ct = default);
Task<ProposalResponse> ApproveAsync(Guid id, CancellationToken ct = default);
Task<ProposalResponse> ReturnToReviewAsync(Guid id, CancellationToken ct = default);
Task<ProposalResponse> MarkSentAsync(Guid id, CancellationToken ct = default);
Task<ProposalResponse> ReviseAsync(Guid id, CancellationToken ct = default);
Task<ProposalResponse> ApproveAsync(Guid id, string? proposalVersion, CancellationToken ct = default);
Task<ProposalResponse> ReturnToReviewAsync(Guid id, string? proposalVersion, CancellationToken ct = default);
Task<ProposalResponse> MarkSentAsync(Guid id, string? proposalVersion, CancellationToken ct = default);
Task<ProposalResponse> ReviseAsync(Guid id, string? proposalVersion, CancellationToken ct = default);
Task<IReadOnlyList<ProposalResponse>> GetRevisionHistoryAsync(Guid id, CancellationToken ct = default);
Task<IReadOnlyList<AuditLogResponse>> GetAuditTrailAsync(Guid id, CancellationToken ct = default);
Task<ProposalStatsResponse> GetStatsAsync(CancellationToken ct = default);

View file

@ -29,6 +29,10 @@ public class LineItem
public LineItemSource Source { get; set; }
public DateTime CreatedAt { get; set; }
public DateTime UpdatedAt { get; set; }
/// <summary>Optimistic-concurrency token. Bulk updates are guarded at the
/// proposal level (rows are replaced wholesale); this exists for per-item
/// mutations and client cache keys.</summary>
public long Version { get; set; } = 1;
public Proposal? Proposal { get; set; }
}

View file

@ -52,6 +52,8 @@ public class Proposal
public Guid? ParentProposalId { get; set; }
public DateTime CreatedAt { get; set; }
public DateTime UpdatedAt { get; set; }
/// <summary>Optimistic-concurrency token; bumps on every aggregate mutation (including line-item changes).</summary>
public long Version { get; set; } = 1;
public User? SubmittedBy { get; set; }
public User? AssignedAdmin { get; set; }

View file

@ -0,0 +1,564 @@
// <auto-generated />
using System;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Migrations;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
using Npgsql.EntityFrameworkCore.PostgreSQL.Metadata;
using ProposalSystem.Infrastructure.Data;
#nullable disable
namespace ProposalSystem.Infrastructure.Data.Migrations
{
[DbContext(typeof(ProposalDbContext))]
[Migration("20260714003834_AddProposalLineItemVersion")]
partial class AddProposalLineItemVersion
{
/// <inheritdoc />
protected override void BuildTargetModel(ModelBuilder modelBuilder)
{
#pragma warning disable 612, 618
modelBuilder
.HasAnnotation("ProductVersion", "8.0.28")
.HasAnnotation("Relational:MaxIdentifierLength", 63);
NpgsqlModelBuilderExtensions.UseIdentityByDefaultColumns(modelBuilder);
modelBuilder.Entity("ProposalSystem.Domain.Entities.AuditLog", b =>
{
b.Property<Guid>("Id")
.ValueGeneratedOnAdd()
.HasColumnType("uuid");
b.Property<string>("Action")
.IsRequired()
.HasColumnType("text");
b.Property<string>("Details")
.HasColumnType("jsonb");
b.Property<string>("IpAddress")
.HasColumnType("text");
b.Property<Guid?>("ProposalId")
.HasColumnType("uuid");
b.Property<DateTime>("Timestamp")
.HasColumnType("timestamp with time zone");
b.Property<Guid>("UserId")
.HasColumnType("uuid");
b.HasKey("Id");
b.HasIndex("ProposalId");
b.HasIndex("Timestamp");
b.HasIndex("UserId");
b.ToTable("AuditLogs");
});
modelBuilder.Entity("ProposalSystem.Domain.Entities.Customer", b =>
{
b.Property<Guid>("Id")
.ValueGeneratedOnAdd()
.HasColumnType("uuid");
b.Property<string>("Addresses")
.HasColumnType("jsonb");
b.Property<string>("ContactEmail")
.HasColumnType("text");
b.Property<DateTime>("CreatedAt")
.HasColumnType("timestamp with time zone");
b.Property<string>("Name")
.IsRequired()
.HasColumnType("text");
b.Property<DateTime>("UpdatedAt")
.HasColumnType("timestamp with time zone");
b.HasKey("Id");
b.ToTable("Customers");
});
modelBuilder.Entity("ProposalSystem.Domain.Entities.GeneratedPdf", b =>
{
b.Property<Guid>("Id")
.ValueGeneratedOnAdd()
.HasColumnType("uuid");
b.Property<DateTime>("GeneratedAt")
.HasColumnType("timestamp with time zone");
b.Property<Guid>("GeneratedById")
.HasColumnType("uuid");
b.Property<Guid>("ProposalId")
.HasColumnType("uuid");
b.Property<int>("Revision")
.HasColumnType("integer");
b.Property<string>("S3Key")
.IsRequired()
.HasColumnType("text");
b.HasKey("Id");
b.HasIndex("GeneratedById");
b.HasIndex("ProposalId");
b.ToTable("GeneratedPdfs");
});
modelBuilder.Entity("ProposalSystem.Domain.Entities.LineItem", b =>
{
b.Property<Guid>("Id")
.ValueGeneratedOnAdd()
.HasColumnType("uuid");
b.Property<DateTime>("CreatedAt")
.HasColumnType("timestamp with time zone");
b.Property<string>("Description")
.IsRequired()
.HasColumnType("text");
b.Property<string>("PricingMode")
.IsRequired()
.HasColumnType("text");
b.Property<Guid>("ProposalId")
.HasColumnType("uuid");
b.Property<decimal>("Quantity")
.HasPrecision(18, 4)
.HasColumnType("numeric(18,4)");
b.Property<int>("SortOrder")
.HasColumnType("integer");
b.Property<string>("Source")
.IsRequired()
.HasColumnType("text");
b.Property<decimal>("TotalPrice")
.HasPrecision(18, 2)
.HasColumnType("numeric(18,2)");
b.Property<string>("Unit")
.IsRequired()
.HasColumnType("text");
b.Property<decimal?>("UnitPrice")
.HasPrecision(18, 2)
.HasColumnType("numeric(18,2)");
b.Property<DateTime>("UpdatedAt")
.HasColumnType("timestamp with time zone");
b.Property<long>("Version")
.IsConcurrencyToken()
.ValueGeneratedOnAdd()
.HasColumnType("bigint")
.HasDefaultValue(1L);
b.HasKey("Id");
b.HasIndex("ProposalId");
b.ToTable("LineItems");
});
modelBuilder.Entity("ProposalSystem.Domain.Entities.PricingLibraryItem", b =>
{
b.Property<Guid>("Id")
.ValueGeneratedOnAdd()
.HasColumnType("uuid");
b.Property<DateTime>("CreatedAt")
.HasColumnType("timestamp with time zone");
b.Property<string>("Description")
.HasColumnType("text");
b.Property<string>("Keywords")
.HasColumnType("text");
b.Property<string>("ServiceCategory")
.IsRequired()
.HasColumnType("text");
b.Property<string>("Source")
.IsRequired()
.HasColumnType("text");
b.Property<string>("Title")
.IsRequired()
.HasColumnType("text");
b.Property<string>("Unit")
.HasColumnType("text");
b.Property<decimal?>("UnitPrice")
.HasPrecision(18, 2)
.HasColumnType("numeric(18,2)");
b.Property<DateTime>("UpdatedAt")
.HasColumnType("timestamp with time zone");
b.HasKey("Id");
b.ToTable("PricingLibraryItems");
});
modelBuilder.Entity("ProposalSystem.Domain.Entities.Proposal", b =>
{
b.Property<Guid>("Id")
.ValueGeneratedOnAdd()
.HasColumnType("uuid");
b.Property<DateTime?>("ApprovedAt")
.HasColumnType("timestamp with time zone");
b.Property<Guid?>("ApprovedById")
.HasColumnType("uuid");
b.Property<Guid?>("AssignedAdminId")
.HasColumnType("uuid");
b.Property<DateTime>("CreatedAt")
.HasColumnType("timestamp with time zone");
b.Property<int>("CurrentRevision")
.HasColumnType("integer");
b.Property<string>("CustomerAddress")
.IsRequired()
.HasColumnType("text");
b.Property<string>("CustomerName")
.IsRequired()
.HasColumnType("text");
b.Property<string>("Notes")
.IsRequired()
.HasColumnType("text");
b.Property<Guid?>("ParentProposalId")
.HasColumnType("uuid");
b.Property<string>("PoNumber")
.HasColumnType("text");
b.Property<string>("Priority")
.IsRequired()
.HasColumnType("text");
b.Property<string>("ProposalNumber")
.IsRequired()
.HasColumnType("text");
b.Property<string>("RefinedScope")
.HasColumnType("text");
b.Property<string>("ScopeOfWork")
.IsRequired()
.HasColumnType("text");
b.Property<DateTime?>("SentAt")
.HasColumnType("timestamp with time zone");
b.Property<string>("ServiceCategory")
.IsRequired()
.HasColumnType("text");
b.Property<string>("Status")
.IsRequired()
.HasColumnType("text");
b.Property<DateTime>("SubmittedAt")
.HasColumnType("timestamp with time zone");
b.Property<Guid>("SubmittedById")
.HasColumnType("uuid");
b.Property<decimal>("TotalBidAmount")
.HasPrecision(18, 2)
.HasColumnType("numeric(18,2)");
b.Property<DateTime>("UpdatedAt")
.HasColumnType("timestamp with time zone");
b.Property<decimal?>("VendorTotalCost")
.HasPrecision(18, 2)
.HasColumnType("numeric(18,2)");
b.Property<long>("Version")
.IsConcurrencyToken()
.ValueGeneratedOnAdd()
.HasColumnType("bigint")
.HasDefaultValue(1L);
b.Property<string>("WorkOrderNumber")
.IsRequired()
.HasColumnType("text");
b.HasKey("Id");
b.HasIndex("ApprovedById");
b.HasIndex("AssignedAdminId");
b.HasIndex("ParentProposalId");
b.HasIndex("ProposalNumber")
.IsUnique();
b.HasIndex("SubmittedById");
b.ToTable("Proposals");
});
modelBuilder.Entity("ProposalSystem.Domain.Entities.SimilarProposalReference", b =>
{
b.Property<Guid>("Id")
.ValueGeneratedOnAdd()
.HasColumnType("uuid");
b.Property<Guid>("ProposalId")
.HasColumnType("uuid");
b.Property<DateTime>("ReferencedAt")
.HasColumnType("timestamp with time zone");
b.Property<Guid>("ReferencedById")
.HasColumnType("uuid");
b.Property<string>("ReferencedLibraryItemId")
.IsRequired()
.HasColumnType("text");
b.Property<float>("SimilarityScore")
.HasColumnType("real");
b.HasKey("Id");
b.HasIndex("ProposalId");
b.HasIndex("ReferencedById");
b.ToTable("SimilarProposalReferences");
});
modelBuilder.Entity("ProposalSystem.Domain.Entities.User", b =>
{
b.Property<Guid>("Id")
.ValueGeneratedOnAdd()
.HasColumnType("uuid");
b.Property<string>("CognitoSub")
.IsRequired()
.HasColumnType("text");
b.Property<DateTime>("CreatedAt")
.HasColumnType("timestamp with time zone");
b.Property<string>("DisplayName")
.IsRequired()
.HasColumnType("text");
b.Property<string>("Email")
.IsRequired()
.HasColumnType("text");
b.Property<bool>("IsActive")
.HasColumnType("boolean");
b.Property<string>("Role")
.IsRequired()
.HasColumnType("text");
b.Property<DateTime>("UpdatedAt")
.HasColumnType("timestamp with time zone");
b.HasKey("Id");
b.HasIndex("CognitoSub")
.IsUnique();
b.HasIndex("Email")
.IsUnique();
b.ToTable("Users");
});
modelBuilder.Entity("ProposalSystem.Domain.Entities.VendorProposal", b =>
{
b.Property<Guid>("Id")
.ValueGeneratedOnAdd()
.HasColumnType("uuid");
b.Property<string>("ExtractedData")
.HasColumnType("jsonb");
b.Property<string>("FileName")
.IsRequired()
.HasColumnType("text");
b.Property<string>("ProcessingStatus")
.IsRequired()
.HasColumnType("text");
b.Property<Guid>("ProposalId")
.HasColumnType("uuid");
b.Property<string>("S3Key")
.IsRequired()
.HasColumnType("text");
b.Property<decimal>("TotalVendorCost")
.HasPrecision(18, 2)
.HasColumnType("numeric(18,2)");
b.Property<DateTime>("UploadedAt")
.HasColumnType("timestamp with time zone");
b.Property<string>("VendorName")
.IsRequired()
.HasColumnType("text");
b.HasKey("Id");
b.HasIndex("ProposalId");
b.ToTable("VendorProposals");
});
modelBuilder.Entity("ProposalSystem.Domain.Entities.AuditLog", b =>
{
b.HasOne("ProposalSystem.Domain.Entities.Proposal", "Proposal")
.WithMany()
.HasForeignKey("ProposalId");
b.HasOne("ProposalSystem.Domain.Entities.User", "User")
.WithMany()
.HasForeignKey("UserId")
.OnDelete(DeleteBehavior.Cascade)
.IsRequired();
b.Navigation("Proposal");
b.Navigation("User");
});
modelBuilder.Entity("ProposalSystem.Domain.Entities.GeneratedPdf", b =>
{
b.HasOne("ProposalSystem.Domain.Entities.User", "GeneratedBy")
.WithMany()
.HasForeignKey("GeneratedById")
.OnDelete(DeleteBehavior.Cascade)
.IsRequired();
b.HasOne("ProposalSystem.Domain.Entities.Proposal", "Proposal")
.WithMany()
.HasForeignKey("ProposalId")
.OnDelete(DeleteBehavior.Cascade)
.IsRequired();
b.Navigation("GeneratedBy");
b.Navigation("Proposal");
});
modelBuilder.Entity("ProposalSystem.Domain.Entities.LineItem", b =>
{
b.HasOne("ProposalSystem.Domain.Entities.Proposal", "Proposal")
.WithMany("LineItems")
.HasForeignKey("ProposalId")
.OnDelete(DeleteBehavior.Cascade)
.IsRequired();
b.Navigation("Proposal");
});
modelBuilder.Entity("ProposalSystem.Domain.Entities.Proposal", b =>
{
b.HasOne("ProposalSystem.Domain.Entities.User", "ApprovedBy")
.WithMany()
.HasForeignKey("ApprovedById")
.OnDelete(DeleteBehavior.SetNull);
b.HasOne("ProposalSystem.Domain.Entities.User", "AssignedAdmin")
.WithMany()
.HasForeignKey("AssignedAdminId")
.OnDelete(DeleteBehavior.SetNull);
b.HasOne("ProposalSystem.Domain.Entities.Proposal", "ParentProposal")
.WithMany()
.HasForeignKey("ParentProposalId")
.OnDelete(DeleteBehavior.SetNull);
b.HasOne("ProposalSystem.Domain.Entities.User", "SubmittedBy")
.WithMany()
.HasForeignKey("SubmittedById")
.OnDelete(DeleteBehavior.Restrict)
.IsRequired();
b.Navigation("ApprovedBy");
b.Navigation("AssignedAdmin");
b.Navigation("ParentProposal");
b.Navigation("SubmittedBy");
});
modelBuilder.Entity("ProposalSystem.Domain.Entities.SimilarProposalReference", b =>
{
b.HasOne("ProposalSystem.Domain.Entities.Proposal", "Proposal")
.WithMany()
.HasForeignKey("ProposalId")
.OnDelete(DeleteBehavior.Cascade)
.IsRequired();
b.HasOne("ProposalSystem.Domain.Entities.User", "ReferencedBy")
.WithMany()
.HasForeignKey("ReferencedById")
.OnDelete(DeleteBehavior.Cascade)
.IsRequired();
b.Navigation("Proposal");
b.Navigation("ReferencedBy");
});
modelBuilder.Entity("ProposalSystem.Domain.Entities.VendorProposal", b =>
{
b.HasOne("ProposalSystem.Domain.Entities.Proposal", "Proposal")
.WithMany("VendorProposals")
.HasForeignKey("ProposalId")
.OnDelete(DeleteBehavior.Cascade)
.IsRequired();
b.Navigation("Proposal");
});
modelBuilder.Entity("ProposalSystem.Domain.Entities.Proposal", b =>
{
b.Navigation("LineItems");
b.Navigation("VendorProposals");
});
#pragma warning restore 612, 618
}
}
}

View file

@ -0,0 +1,40 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace ProposalSystem.Infrastructure.Data.Migrations
{
/// <inheritdoc />
public partial class AddProposalLineItemVersion : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.AddColumn<long>(
name: "Version",
table: "Proposals",
type: "bigint",
nullable: false,
defaultValue: 1L);
migrationBuilder.AddColumn<long>(
name: "Version",
table: "LineItems",
type: "bigint",
nullable: false,
defaultValue: 1L);
}
/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropColumn(
name: "Version",
table: "Proposals");
migrationBuilder.DropColumn(
name: "Version",
table: "LineItems");
}
}
}

View file

@ -17,7 +17,7 @@ namespace ProposalSystem.Infrastructure.Data.Migrations
{
#pragma warning disable 612, 618
modelBuilder
.HasAnnotation("ProductVersion", "8.0.27")
.HasAnnotation("ProductVersion", "8.0.28")
.HasAnnotation("Relational:MaxIdentifierLength", 63);
NpgsqlModelBuilderExtensions.UseIdentityByDefaultColumns(modelBuilder);
@ -162,6 +162,12 @@ namespace ProposalSystem.Infrastructure.Data.Migrations
b.Property<DateTime>("UpdatedAt")
.HasColumnType("timestamp with time zone");
b.Property<long>("Version")
.IsConcurrencyToken()
.ValueGeneratedOnAdd()
.HasColumnType("bigint")
.HasDefaultValue(1L);
b.HasKey("Id");
b.HasIndex("ProposalId");
@ -293,6 +299,12 @@ namespace ProposalSystem.Infrastructure.Data.Migrations
.HasPrecision(18, 2)
.HasColumnType("numeric(18,2)");
b.Property<long>("Version")
.IsConcurrencyToken()
.ValueGeneratedOnAdd()
.HasColumnType("bigint")
.HasDefaultValue(1L);
b.Property<string>("WorkOrderNumber")
.IsRequired()
.HasColumnType("text");

View file

@ -30,6 +30,7 @@ public class ProposalDbContext : DbContext
entity.Property(e => e.Status).HasConversion<string>();
entity.Property(e => e.ServiceCategory).HasConversion<string>();
entity.Property(e => e.Priority).HasConversion<string>();
entity.Property(e => e.Version).IsConcurrencyToken().HasDefaultValue(1L);
entity.HasOne(e => e.SubmittedBy).WithMany().HasForeignKey(e => e.SubmittedById).OnDelete(DeleteBehavior.Restrict);
entity.HasOne(e => e.AssignedAdmin).WithMany().HasForeignKey(e => e.AssignedAdminId).OnDelete(DeleteBehavior.SetNull);
@ -45,6 +46,7 @@ public class ProposalDbContext : DbContext
entity.Property(e => e.TotalPrice).HasPrecision(18, 2);
entity.Property(e => e.PricingMode).HasConversion<string>();
entity.Property(e => e.Source).HasConversion<string>();
entity.Property(e => e.Version).IsConcurrencyToken().HasDefaultValue(1L);
entity.HasOne(e => e.Proposal).WithMany(p => p.LineItems).HasForeignKey(e => e.ProposalId).OnDelete(DeleteBehavior.Cascade);
});

View file

@ -17,6 +17,17 @@ public class AuditService : IAuditService
}
public async Task LogAsync(AuditAction action, Guid? proposalId, string? details = null, CancellationToken ct = default)
{
Stage(action, proposalId, details);
await _db.SaveChangesAsync(ct);
}
public void Stage(AuditAction action, Guid? proposalId, string? details = null)
{
_db.AuditLogs.Add(BuildEntry(action, proposalId, details));
}
private AuditLog BuildEntry(AuditAction action, Guid? proposalId, string? details)
{
// Fix: API-M12 — accept pre-serialized JSON from callers that provide structured audit data.
// If the details string is already valid JSON (starts with '{'), use it directly;
@ -31,7 +42,7 @@ public class AuditService : IAuditService
jsonDetails = JsonSerializer.Serialize(new { message = details });
}
var entry = new AuditLog
return new AuditLog
{
Id = Guid.NewGuid(),
ProposalId = proposalId,
@ -41,8 +52,5 @@ public class AuditService : IAuditService
Timestamp = DateTime.UtcNow,
IpAddress = _currentUser.IpAddress,
};
_db.AuditLogs.Add(entry);
await _db.SaveChangesAsync(ct);
}
}

View file

@ -1,6 +1,7 @@
using System.Text.Json;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using ProposalSystem.Application.Common;
using ProposalSystem.Application.DTOs;
using ProposalSystem.Application.Interfaces;
using ProposalSystem.Domain.Entities;
@ -62,31 +63,36 @@ public class LineItemService : ILineItemService
};
_db.LineItems.Add(lineItem);
await _db.SaveChangesAsync(ct);
// Appends are not token-guarded (SHOC precedent: creates are unguarded),
// but they still bump the aggregate version so concurrent guarded edits
// observe the change and clients refresh their token.
proposal.Version++;
proposal.UpdatedAt = now;
// Fix: API-M12 — capture line item details in audit trail
var addAuditDetails = JsonSerializer.Serialize(new
{
action = "add",
lineItem = new { description = request.Description, quantity = request.Quantity, unit = request.Unit, totalPrice = request.TotalPrice }
});
_audit.Stage(AuditAction.EditLineItem, proposalId, addAuditDetails);
await SaveGuardedAsync(proposalId, ct);
_logger.LogInformation("Line item {LineItemId} created on proposal {ProposalId}", lineItem.Id, proposalId);
try
{
// Fix: API-M12 — capture line item details in audit trail
var addAuditDetails = JsonSerializer.Serialize(new
{
action = "add",
lineItem = new { description = request.Description, quantity = request.Quantity, unit = request.Unit, totalPrice = request.TotalPrice }
});
await _audit.LogAsync(AuditAction.EditLineItem, proposalId, addAuditDetails, ct);
}
catch (Exception ex)
{
_logger.LogError(ex, "Failed to write audit log for line item creation on proposal {ProposalId}", proposalId);
}
return MapToResponse(lineItem);
}
public async Task<IReadOnlyList<LineItemResponse>> BulkUpdateAsync(Guid proposalId, BulkUpdateLineItemsRequest request, CancellationToken ct = default)
{
var proposal = await _db.Proposals.FindAsync(new object[] { proposalId }, ct)
// Loaded with the display navigations so a pre-check conflict's
// currentState matches the DB-race reload shape (no nulled names).
var proposal = await _db.Proposals
.Include(p => p.SubmittedBy)
.Include(p => p.AssignedAdmin)
.Include(p => p.ApprovedBy)
.FirstOrDefaultAsync(p => p.Id == proposalId, ct)
?? throw new KeyNotFoundException($"Proposal {proposalId} not found");
if (proposal.Status == ProposalStatus.Approved || proposal.Status == ProposalStatus.Sent)
@ -95,6 +101,11 @@ public class LineItemService : ILineItemService
throw new InvalidOperationException("Cannot modify line items on approved/sent proposals");
}
// Bulk update replaces the line-item set wholesale, so the guard is the
// PROPOSAL token: any concurrent change to the aggregate (fields or items)
// bumped it, and a stale replace would silently discard those edits.
GuardProposalVersion(proposal, request.ProposalVersion);
await using var transaction = await _db.Database.BeginTransactionAsync(ct);
try
{
@ -126,24 +137,17 @@ public class LineItemService : ILineItemService
proposal.TotalBidAmount = newItems.Sum(li => li.TotalPrice);
proposal.UpdatedAt = now;
await _db.SaveChangesAsync(ct);
await transaction.CommitAsync(ct);
// Fix: API-M12 — capture before/after item counts in audit trail;
// staged so it commits atomically with the replace.
var bulkAuditDetails = JsonSerializer.Serialize(new
{
action = "bulkUpdate",
itemCount = new { old = existing.Count, @new = newItems.Count }
});
_audit.Stage(AuditAction.EditLineItem, proposalId, bulkAuditDetails);
try
{
// Fix: API-M12 — capture before/after item counts in audit trail
var bulkAuditDetails = JsonSerializer.Serialize(new
{
action = "bulkUpdate",
itemCount = new { old = existing.Count, @new = newItems.Count }
});
await _audit.LogAsync(AuditAction.EditLineItem, proposalId, bulkAuditDetails, ct);
}
catch (Exception ex)
{
// Fix: API-M11 — log audit failures instead of silently swallowing
_logger.LogError(ex, "Failed to write audit log for bulk update on proposal {ProposalId}", proposalId);
}
await SaveGuardedAsync(proposalId, ct);
await transaction.CommitAsync(ct);
return newItems.OrderBy(li => li.SortOrder).Select(MapToResponse).ToList();
}
@ -168,9 +172,11 @@ public class LineItemService : ILineItemService
}
_db.LineItems.Remove(lineItem);
await _db.SaveChangesAsync(ct);
_logger.LogInformation("Line item {LineItemId} deleted from proposal {ProposalId}", lineItemId, proposalId);
// Deletes are not token-guarded (SHOC precedent) but bump the aggregate
// version so concurrent guarded edits conflict instead of losing the delete.
proposal.Version++;
proposal.UpdatedAt = DateTime.UtcNow;
// Fix: API-M12 — capture deleted line item details in audit trail
var deleteAuditDetails = JsonSerializer.Serialize(new
@ -178,9 +184,18 @@ public class LineItemService : ILineItemService
action = "delete",
lineItem = new { id = lineItemId, description = lineItem.Description, quantity = lineItem.Quantity, unit = lineItem.Unit, totalPrice = lineItem.TotalPrice }
});
await _audit.LogAsync(AuditAction.EditLineItem, proposalId, deleteAuditDetails, ct);
_audit.Stage(AuditAction.EditLineItem, proposalId, deleteAuditDetails);
await SaveGuardedAsync(proposalId, ct);
_logger.LogInformation("Line item {LineItemId} deleted from proposal {ProposalId}", lineItemId, proposalId);
}
private void GuardProposalVersion(Proposal proposal, string? proposalVersion) =>
ProposalConcurrencyGuard.Guard(_db, proposal, proposalVersion);
private Task SaveGuardedAsync(Guid proposalId, CancellationToken ct) =>
ProposalConcurrencyGuard.SaveAsync(_db, proposalId, ct);
private static LineItemResponse MapToResponse(LineItem li) => new(
li.Id,
li.ProposalId,
@ -193,6 +208,7 @@ public class LineItemService : ILineItemService
li.SortOrder,
li.Source,
li.CreatedAt,
li.UpdatedAt
li.UpdatedAt,
RowVersionCodec.Encode(li.Version)
);
}

View file

@ -0,0 +1,63 @@
using Microsoft.EntityFrameworkCore;
using ProposalSystem.Application.Common;
using ProposalSystem.Application.DTOs;
using ProposalSystem.Domain.Entities;
using ProposalSystem.Infrastructure.Data;
namespace ProposalSystem.Infrastructure.Services;
/// <summary>
/// SHOC double-guard for the proposal aggregate (see ADR 0004).
/// Step 1 (Guard): validate the client token against the freshly loaded row and
/// stamp it as EF's original value so the UPDATE's WHERE clause re-enforces it
/// at the database. Step 2 (SaveAsync): on a lost race, reload the current
/// state and throw the conflict exception that becomes the 409 body.
///
/// CALLER CONTRACT (AUTHZ-CG-01): the 409 currentState embeds the full
/// ProposalResponse with NO ownership filtering (unlike GetByIdAsync's
/// dispatcher check). Every endpoint that can reach Guard/SaveAsync MUST be
/// [Authorize(Roles = "admins,sysadmins")]. Enforced by
/// GuardedEndpointAuthorizationTests — extend the guard with an ownership
/// predicate before ever wiring it to a dispatcher-reachable write.
/// </summary>
internal static class ProposalConcurrencyGuard
{
internal static void Guard(ProposalDbContext db, Proposal proposal, string? proposalVersion)
{
if (string.IsNullOrEmpty(proposalVersion))
throw new BusinessRuleException("ProposalVersionRequired", "proposalVersion is required for this operation.");
if (!RowVersionCodec.TryDecode(proposalVersion, out var expected))
throw new BusinessRuleException("InvalidRowVersion", "proposalVersion is not a valid version token.");
if (proposal.Version != expected)
throw new ProposalConcurrencyException(ProposalMapper.ToResponse(proposal));
db.Entry(proposal).Property(p => p.Version).OriginalValue = expected;
proposal.Version++;
}
internal static async Task SaveAsync(ProposalDbContext db, Guid proposalId, CancellationToken ct)
{
try
{
await db.SaveChangesAsync(ct);
}
catch (DbUpdateConcurrencyException)
{
db.ChangeTracker.Clear();
var currentState = await LoadCurrentStateAsync(db, proposalId, ct);
throw new ProposalConcurrencyException(currentState);
}
}
internal static async Task<ProposalResponse?> LoadCurrentStateAsync(
ProposalDbContext db, Guid proposalId, CancellationToken ct)
{
var current = await db.Proposals
.AsNoTracking()
.Include(p => p.SubmittedBy)
.Include(p => p.AssignedAdmin)
.Include(p => p.ApprovedBy)
.FirstOrDefaultAsync(p => p.Id == proposalId, ct);
return current is null ? null : ProposalMapper.ToResponse(current);
}
}

View file

@ -0,0 +1,39 @@
using ProposalSystem.Application.Common;
using ProposalSystem.Application.DTOs;
using ProposalSystem.Domain.Entities;
namespace ProposalSystem.Infrastructure.Services;
/// <summary>Canonical Proposal → ProposalResponse mapping, shared by the
/// proposal/line-item services and the concurrency guard's currentState reload.</summary>
internal static class ProposalMapper
{
internal static ProposalResponse ToResponse(Proposal p) => new(
p.Id,
p.ProposalNumber,
p.WorkOrderNumber,
p.PoNumber,
p.CustomerName,
p.CustomerAddress,
p.ScopeOfWork,
p.RefinedScope,
p.ServiceCategory,
p.Priority,
p.Status,
p.TotalBidAmount,
p.VendorTotalCost,
p.Notes,
p.SubmittedById,
p.SubmittedBy?.DisplayName,
p.SubmittedAt,
p.AssignedAdminId,
p.ApprovedById,
p.ApprovedAt,
p.SentAt,
p.CurrentRevision,
p.ParentProposalId,
p.CreatedAt,
p.UpdatedAt,
RowVersionCodec.Encode(p.Version)
);
}

View file

@ -2,6 +2,7 @@ using System.Text.Json;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Logging;
using ProposalSystem.Application.Common;
using ProposalSystem.Application.DTOs;
using ProposalSystem.Application.Interfaces;
using ProposalSystem.Domain.Entities;
@ -71,21 +72,13 @@ public class ProposalService : IProposalService
};
_db.Proposals.Add(proposal);
_audit.Stage(AuditAction.Submit, proposal.Id);
await _db.SaveChangesAsync(ct);
await transaction.CommitAsync(ct);
_logger.LogInformation("Proposal {ProposalId} created with number {ProposalNumber} by user {UserId}",
proposal.Id, proposalNumber, _currentUser.UserId);
try
{
await _audit.LogAsync(AuditAction.Submit, proposal.Id, null, ct);
}
catch (Exception ex)
{
_logger.LogError(ex, "Failed to write audit log for proposal submission {ProposalId}", proposal.Id);
}
try
{
await _jobPublisher.PublishAsync("suggestions", new { proposalId = proposal.Id, trigger = "generate" }, ct);
@ -173,7 +166,8 @@ public class ProposalService : IProposalService
p.TotalBidAmount,
p.SubmittedAt,
p.SubmittedBy != null ? p.SubmittedBy.DisplayName : null,
p.AssignedAdmin != null ? p.AssignedAdmin.DisplayName : null
p.AssignedAdmin != null ? p.AssignedAdmin.DisplayName : null,
RowVersionCodec.Encode(p.Version)
))
.ToListAsync(ct);
@ -189,6 +183,8 @@ public class ProposalService : IProposalService
.FirstOrDefaultAsync(p => p.Id == id, ct)
?? throw new KeyNotFoundException($"Proposal {id} not found");
GuardProposalVersion(proposal, request.ProposalVersion);
// Fix: API-M12 — capture before/after values for audit trail
var changes = new Dictionary<string, object>();
@ -223,19 +219,21 @@ public class ProposalService : IProposalService
}
proposal.UpdatedAt = DateTime.UtcNow;
await _db.SaveChangesAsync(ct);
// Stage-then-single-SaveChanges: the audit row commits atomically with the mutation.
var auditDetails = changes.Count > 0 ? JsonSerializer.Serialize(changes) : null;
await _audit.LogAsync(AuditAction.Edit, id, auditDetails, ct);
_audit.Stage(AuditAction.Edit, id, auditDetails);
await SaveGuardedAsync(id, ct);
return MapToResponse(proposal);
}
public async Task<ProposalResponse> ApproveAsync(Guid id, CancellationToken ct = default)
public async Task<ProposalResponse> ApproveAsync(Guid id, string? proposalVersion, CancellationToken ct = default)
{
var proposal = await _db.Proposals
.Include(p => p.LineItems)
.Include(p => p.SubmittedBy)
.Include(p => p.AssignedAdmin)
.Include(p => p.ApprovedBy)
.FirstOrDefaultAsync(p => p.Id == id, ct)
?? throw new KeyNotFoundException($"Proposal {id} not found");
@ -259,6 +257,8 @@ public class ProposalService : IProposalService
throw new InvalidOperationException("Cannot approve proposal without priced line items");
}
GuardProposalVersion(proposal, proposalVersion);
// Fix: API-M12 — capture before/after status for audit trail
var previousStatus = proposal.Status;
proposal.Status = ProposalStatus.Approved;
@ -267,9 +267,9 @@ public class ProposalService : IProposalService
proposal.TotalBidAmount = proposal.LineItems.Sum(li => li.TotalPrice);
proposal.UpdatedAt = DateTime.UtcNow;
await _db.SaveChangesAsync(ct);
var approveAuditDetails = JsonSerializer.Serialize(new { status = new { old = previousStatus.ToString(), @new = ProposalStatus.Approved.ToString() } });
await _audit.LogAsync(AuditAction.Approve, id, approveAuditDetails, ct);
_audit.Stage(AuditAction.Approve, id, approveAuditDetails);
await SaveGuardedAsync(id, ct);
_logger.LogInformation("Proposal {ProposalId} approved by user {UserId}, total bid {TotalBidAmount}",
id, _currentUser.UserId, proposal.TotalBidAmount);
@ -277,10 +277,11 @@ public class ProposalService : IProposalService
return MapToResponse(proposal);
}
public async Task<ProposalResponse> ReturnToReviewAsync(Guid id, CancellationToken ct = default)
public async Task<ProposalResponse> ReturnToReviewAsync(Guid id, string? proposalVersion, CancellationToken ct = default)
{
var proposal = await _db.Proposals
.Include(p => p.SubmittedBy)
.Include(p => p.AssignedAdmin)
.Include(p => p.ApprovedBy)
.FirstOrDefaultAsync(p => p.Id == id, ct)
?? throw new KeyNotFoundException($"Proposal {id} not found");
@ -291,6 +292,8 @@ public class ProposalService : IProposalService
if (proposal.Status != ProposalStatus.Approved)
throw new InvalidOperationException("Only approved proposals can be returned to review");
GuardProposalVersion(proposal, proposalVersion);
// Fix: API-M12 — capture before/after status for audit trail
var previousStatus = proposal.Status;
proposal.Status = ProposalStatus.InReview;
@ -298,17 +301,18 @@ public class ProposalService : IProposalService
proposal.ApprovedAt = null;
proposal.UpdatedAt = DateTime.UtcNow;
await _db.SaveChangesAsync(ct);
var returnAuditDetails = JsonSerializer.Serialize(new { status = new { old = previousStatus.ToString(), @new = ProposalStatus.InReview.ToString() } });
await _audit.LogAsync(AuditAction.ReturnToReview, id, returnAuditDetails, ct);
_audit.Stage(AuditAction.ReturnToReview, id, returnAuditDetails);
await SaveGuardedAsync(id, ct);
return MapToResponse(proposal);
}
public async Task<ProposalResponse> MarkSentAsync(Guid id, CancellationToken ct = default)
public async Task<ProposalResponse> MarkSentAsync(Guid id, string? proposalVersion, CancellationToken ct = default)
{
var proposal = await _db.Proposals
.Include(p => p.SubmittedBy)
.Include(p => p.AssignedAdmin)
.Include(p => p.ApprovedBy)
.FirstOrDefaultAsync(p => p.Id == id, ct)
?? throw new KeyNotFoundException($"Proposal {id} not found");
@ -326,15 +330,17 @@ public class ProposalService : IProposalService
throw new InvalidOperationException("Only approved proposals can be marked as sent");
}
GuardProposalVersion(proposal, proposalVersion);
// Fix: API-M12 — capture before/after status for audit trail
var previousStatus = proposal.Status;
proposal.Status = ProposalStatus.Sent;
proposal.SentAt = DateTime.UtcNow;
proposal.UpdatedAt = DateTime.UtcNow;
await _db.SaveChangesAsync(ct);
var sentAuditDetails = JsonSerializer.Serialize(new { status = new { old = previousStatus.ToString(), @new = ProposalStatus.Sent.ToString() } });
await _audit.LogAsync(AuditAction.MarkSent, id, sentAuditDetails, ct);
_audit.Stage(AuditAction.MarkSent, id, sentAuditDetails);
await SaveGuardedAsync(id, ct);
_logger.LogInformation("Proposal {ProposalId} marked as sent by user {UserId}", id, _currentUser.UserId);
@ -424,10 +430,13 @@ public class ProposalService : IProposalService
}
}
public async Task<ProposalResponse> ReviseAsync(Guid id, CancellationToken ct = default)
public async Task<ProposalResponse> ReviseAsync(Guid id, string? proposalVersion, CancellationToken ct = default)
{
var proposal = await _db.Proposals
.Include(p => p.LineItems)
.Include(p => p.SubmittedBy)
.Include(p => p.AssignedAdmin)
.Include(p => p.ApprovedBy)
.FirstOrDefaultAsync(p => p.Id == id, ct)
?? throw new KeyNotFoundException($"Proposal {id} not found");
@ -450,6 +459,8 @@ public class ProposalService : IProposalService
throw new InvalidOperationException("Only sent proposals can be revised");
}
GuardProposalVersion(proposal, proposalVersion);
var revision = new Proposal
{
Id = Guid.NewGuid(),
@ -499,14 +510,13 @@ public class ProposalService : IProposalService
proposal.UpdatedAt = DateTime.UtcNow;
_db.Proposals.Add(revision);
await _db.SaveChangesAsync(ct);
var reviseAuditDetails = JsonSerializer.Serialize(new
{
status = new { old = previousReviseStatus.ToString(), @new = ProposalStatus.Revised.ToString() },
message = $"Revised from {proposal.Id}"
});
await _audit.LogAsync(AuditAction.CreateRevision, revision.Id, reviseAuditDetails, ct);
_audit.Stage(AuditAction.CreateRevision, revision.Id, reviseAuditDetails);
await SaveGuardedAsync(id, ct);
_logger.LogInformation("Proposal {ProposalId} revised to {RevisionId} (revision {RevisionNumber}) by user {UserId}",
id, revision.Id, revision.CurrentRevision, _currentUser.UserId);
@ -576,31 +586,11 @@ public class ProposalService : IProposalService
: new ProposalStatsResponse(counts.Total, counts.InReview, counts.Approved, counts.Sent);
}
private static ProposalResponse MapToResponse(Proposal p) => new(
p.Id,
p.ProposalNumber,
p.WorkOrderNumber,
p.PoNumber,
p.CustomerName,
p.CustomerAddress,
p.ScopeOfWork,
p.RefinedScope,
p.ServiceCategory,
p.Priority,
p.Status,
p.TotalBidAmount,
p.VendorTotalCost,
p.Notes,
p.SubmittedById,
p.SubmittedBy?.DisplayName,
p.SubmittedAt,
p.AssignedAdminId,
p.ApprovedById,
p.ApprovedAt,
p.SentAt,
p.CurrentRevision,
p.ParentProposalId,
p.CreatedAt,
p.UpdatedAt
);
private void GuardProposalVersion(Proposal proposal, string? proposalVersion) =>
ProposalConcurrencyGuard.Guard(_db, proposal, proposalVersion);
private Task SaveGuardedAsync(Guid proposalId, CancellationToken ct) =>
ProposalConcurrencyGuard.SaveAsync(_db, proposalId, ct);
private static ProposalResponse MapToResponse(Proposal p) => ProposalMapper.ToResponse(p);
}

View file

@ -0,0 +1,37 @@
using FluentAssertions;
using ProposalSystem.Application.Common;
using Xunit;
namespace ProposalSystem.Tests.Common;
public class RowVersionCodecTests
{
[Theory(DisplayName = "Encode/TryDecode round-trips")]
[InlineData(1L)]
[InlineData(2L)]
[InlineData(long.MaxValue)]
public void RoundTrips(long version)
{
var token = RowVersionCodec.Encode(version);
RowVersionCodec.TryDecode(token, out var decoded).Should().BeTrue();
decoded.Should().Be(version);
}
[Fact(DisplayName = "Version 1 encodes to the SHOC-style opaque token")]
public void EncodesOpaqueBase64()
{
RowVersionCodec.Encode(1).Should().Be("AAAAAAAAAAE=");
}
[Theory(DisplayName = "TryDecode rejects missing/malformed/wrong-length tokens")]
[InlineData(null)]
[InlineData("")]
[InlineData("not base64!!")]
[InlineData("AAA=")] // valid base64 but not 8 bytes
[InlineData("AAAAAAAAAAAAAAAAAAAAAA==")] // 16 bytes
public void RejectsInvalid(string? token)
{
RowVersionCodec.TryDecode(token, out _).Should().BeFalse();
}
}

View file

@ -0,0 +1,44 @@
using System.Reflection;
using FluentAssertions;
using Microsoft.AspNetCore.Authorization;
using ProposalSystem.Api.Controllers;
using Xunit;
namespace ProposalSystem.Tests.Controllers;
/// <summary>
/// AUTHZ-CG-01 tripwire: ProposalConcurrencyGuard's 409 currentState embeds the
/// full ProposalResponse with no ownership filtering, so every endpoint that can
/// reach the guard must stay admin-gated. If a new/changed endpoint wires a
/// guarded mutation to a dispatcher-reachable route, this test fails the build
/// until the guard gains an ownership predicate.
/// </summary>
public class GuardedEndpointAuthorizationTests
{
public static readonly TheoryData<Type, string> GuardedEndpoints = new()
{
{ typeof(ProposalsController), "Update" },
{ typeof(ProposalsController), "Approve" },
{ typeof(ProposalsController), "ReturnToReview" },
{ typeof(ProposalsController), "MarkSent" },
{ typeof(ProposalsController), "Revise" },
{ typeof(LineItemsController), "BulkUpdate" },
};
[Theory(DisplayName = "Guard-reaching endpoints require admins/sysadmins")]
[MemberData(nameof(GuardedEndpoints))]
public void GuardedEndpoint_RequiresAdminRole(Type controller, string actionName)
{
var action = controller.GetMethod(actionName, BindingFlags.Public | BindingFlags.Instance);
action.Should().NotBeNull($"{controller.Name}.{actionName} should exist — update GuardedEndpoints if renamed");
var authorize = action!.GetCustomAttributes<AuthorizeAttribute>(inherit: true)
.FirstOrDefault(a => a.Roles != null);
authorize.Should().NotBeNull(
$"{controller.Name}.{actionName} reaches ProposalConcurrencyGuard and must carry a role-restricted [Authorize]");
authorize!.Roles.Should().Contain("admins").And.Contain("sysadmins");
authorize.Roles.Should().NotContain("dispatchers",
"the guard's 409 currentState has no ownership filter — see ProposalConcurrencyGuard caller contract");
}
}

View file

@ -37,6 +37,29 @@ public static class SqliteDbContextFactory
return context;
}
/// <summary>
/// Opens a shared in-memory database and returns the connection plus a factory
/// for additional contexts over the SAME database — needed by optimistic-
/// concurrency tests that simulate two competing writers. Callers dispose the
/// connection after the contexts.
/// </summary>
public static (SqliteConnection Connection, Func<ProposalDbContext> ContextFactory) CreateShared()
{
var connection = new SqliteConnection("DataSource=:memory:");
connection.Open();
RegisterPostgresStubs(connection);
var options = new DbContextOptionsBuilder<ProposalDbContext>()
.UseSqlite(connection)
.Options;
var first = new ProposalDbContext(options);
first.Database.EnsureCreated();
first.Dispose();
return (connection, () => new ProposalDbContext(options));
}
private static void RegisterPostgresStubs(SqliteConnection connection)
{
// hashtext(text) -> integer — returns a constant; only needed so SQL parses

View file

@ -93,4 +93,61 @@ public class GlobalExceptionHandlerTests
context.Response.ContentType.Should().Be("application/problem+json");
}
// ── Concurrency 409 envelopes (ADR 0004) ─────────────────────────────────
// These are SHOC's shapes verbatim, NOT ProblemDetails. Both bodies are wire
// contract: web/mobile branch on message + currentState / status + code.
private static ProposalSystem.Application.DTOs.ProposalResponse SampleState(Guid id) => new(
id, "P-001", "WO-1", null, "Customer", "Addr", "Scope", null,
ProposalSystem.Domain.Entities.ServiceCategory.HVAC,
ProposalSystem.Domain.Entities.Priority.Standard,
ProposalSystem.Domain.Entities.ProposalStatus.InReview,
100m, null, "winner", Guid.NewGuid(), "Submitter", DateTime.UtcNow,
null, null, null, null, 1, null, DateTime.UtcNow, DateTime.UtcNow,
"AAAAAAAAAAI=");
[Fact(DisplayName = "ProposalConcurrencyException maps to 409 { message, currentState } (SHOC envelope)")]
public async Task ConcurrencyException_Maps409WithCurrentState()
{
var id = Guid.NewGuid();
var (status, body) = await InvokeWith(new ProposalConcurrencyException(SampleState(id)));
status.Should().Be(409);
body.GetProperty("message").GetString()
.Should().Be("The record was modified by another user. Refresh and retry.");
body.GetProperty("currentState").GetProperty("id").GetString().Should().Be(id.ToString());
body.GetProperty("currentState").GetProperty("notes").GetString().Should().Be("winner");
body.GetProperty("currentState").GetProperty("rowVersion").GetString().Should().Be("AAAAAAAAAAI=");
// Enums must serialize as strings (matching the MVC pipeline), or client
// schema validation of currentState fails and the state is discarded.
body.GetProperty("currentState").GetProperty("serviceCategory").GetString().Should().Be("HVAC");
body.GetProperty("currentState").GetProperty("status").GetString().Should().Be("InReview");
// The guarded envelope is NOT the fallback shape.
body.TryGetProperty("status", out _).Should().BeFalse();
body.TryGetProperty("code", out _).Should().BeFalse();
}
[Fact(DisplayName = "ProposalConcurrencyException with no reloadable state carries currentState: null")]
public async Task ConcurrencyException_NullState_SerializesNull()
{
var (status, body) = await InvokeWith(new ProposalConcurrencyException(null));
status.Should().Be(409);
body.GetProperty("currentState").ValueKind.Should().Be(JsonValueKind.Null);
}
[Fact(DisplayName = "Bare DbUpdateConcurrencyException maps to the 409 fallback { status, message, code }")]
public async Task DbUpdateConcurrencyException_Maps409Fallback()
{
var (status, body) = await InvokeWith(
new Microsoft.EntityFrameworkCore.DbUpdateConcurrencyException("boom"));
status.Should().Be(409);
body.GetProperty("status").GetString().Should().Be("Conflict");
body.GetProperty("message").GetString()
.Should().Be("The record was modified by another user. Refresh and retry.");
body.GetProperty("code").GetInt32().Should().Be(409);
body.TryGetProperty("currentState", out _).Should().BeFalse();
}
}

View file

@ -121,7 +121,7 @@ public class LineItemServiceStateGuardTests : IDisposable
});
// Act
var act = () => _sut.BulkUpdateAsync(proposal.Id, request);
var act = () => _sut.BulkUpdateAsync(proposal.Id, request with { ProposalVersion = Ver(proposal.Id) });
// Assert
await act.Should().ThrowAsync<InvalidOperationException>()
@ -143,7 +143,7 @@ public class LineItemServiceStateGuardTests : IDisposable
});
// Act
var result = await _sut.BulkUpdateAsync(proposal.Id, request);
var result = await _sut.BulkUpdateAsync(proposal.Id, request with { ProposalVersion = Ver(proposal.Id) });
// Assert
result.Should().HaveCount(2);
@ -165,7 +165,7 @@ public class LineItemServiceStateGuardTests : IDisposable
});
// Act
await _sut.BulkUpdateAsync(proposal.Id, request);
await _sut.BulkUpdateAsync(proposal.Id, request with { ProposalVersion = Ver(proposal.Id) });
// Assert
var updated = await _db.Proposals.FindAsync(proposal.Id);
@ -199,7 +199,7 @@ public class LineItemServiceStateGuardTests : IDisposable
});
// Act
var result = await _sut.BulkUpdateAsync(proposal.Id, request);
var result = await _sut.BulkUpdateAsync(proposal.Id, request with { ProposalVersion = Ver(proposal.Id) });
// Assert
result.Should().HaveCount(1);
@ -214,7 +214,7 @@ public class LineItemServiceStateGuardTests : IDisposable
public async Task BulkUpdateAsync_NonexistentProposal_ThrowsKeyNotFound()
{
var request = new BulkUpdateLineItemsRequest(new List<UpdateLineItemEntry>());
var act = () => _sut.BulkUpdateAsync(Guid.NewGuid(), request);
var act = () => _sut.BulkUpdateAsync(Guid.NewGuid(), request with { ProposalVersion = Ver(Guid.NewGuid()) });
await act.Should().ThrowAsync<KeyNotFoundException>();
}
@ -317,11 +317,9 @@ public class LineItemServiceStateGuardTests : IDisposable
await _sut.CreateAsync(proposal.Id, request);
// Assert
await _audit.Received(1).LogAsync(
AuditAction.EditLineItem,
_audit.Received(1).Stage(AuditAction.EditLineItem,
proposal.Id,
Arg.Is<string?>(s => s != null && s.Contains(request.Description)),
Arg.Any<CancellationToken>());
Arg.Is<string?>(s => s != null && s.Contains(request.Description)));
}
[Fact(DisplayName = "QA-C2: BulkUpdateAsync logs audit event on success")]
@ -338,14 +336,12 @@ public class LineItemServiceStateGuardTests : IDisposable
});
// Act
await _sut.BulkUpdateAsync(proposal.Id, request);
await _sut.BulkUpdateAsync(proposal.Id, request with { ProposalVersion = Ver(proposal.Id) });
// Assert — audit detail may be plain text ("Bulk update: N items") or JSON
await _audit.Received(1).LogAsync(
AuditAction.EditLineItem,
_audit.Received(1).Stage(AuditAction.EditLineItem,
proposal.Id,
Arg.Is<string?>(s => s != null),
Arg.Any<CancellationToken>());
Arg.Is<string?>(s => s != null));
}
[Fact(DisplayName = "QA-C2: DeleteAsync logs audit event on success")]
@ -374,11 +370,9 @@ public class LineItemServiceStateGuardTests : IDisposable
await _sut.DeleteAsync(proposal.Id, lineItem.Id);
// Assert
await _audit.Received(1).LogAsync(
AuditAction.EditLineItem,
_audit.Received(1).Stage(AuditAction.EditLineItem,
proposal.Id,
Arg.Is<string?>(s => s != null && s.Contains("Audit test item")),
Arg.Any<CancellationToken>());
Arg.Is<string?>(s => s != null && s.Contains("Audit test item")));
}
#endregion
@ -418,4 +412,7 @@ public class LineItemServiceStateGuardTests : IDisposable
Source: LineItemSource.Manual
);
}
private string Ver(Guid id) =>
ProposalSystem.Application.Common.RowVersionCodec.Encode(_db.Proposals.Find(id)?.Version ?? 1);
}

View file

@ -0,0 +1,249 @@
using FluentAssertions;
using Microsoft.Data.Sqlite;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Logging;
using NSubstitute;
using ProposalSystem.Application.Common;
using ProposalSystem.Application.DTOs;
using ProposalSystem.Application.Interfaces;
using ProposalSystem.Domain.Entities;
using ProposalSystem.Infrastructure.Data;
using ProposalSystem.Infrastructure.Services;
using ProposalSystem.Tests.Helpers;
using Xunit;
namespace ProposalSystem.Tests.Services;
/// <summary>
/// Optimistic-concurrency tests for the proposal aggregate (ADR 0004, SHOC
/// double-guard). Covers the pre-check path, both 422 token codes, the
/// DB-level race window (two writers over one SQLite database), version
/// bumping, and audit atomicity (staged rows must not survive a failed save).
/// </summary>
public class ProposalConcurrencyTests : IDisposable
{
private readonly SqliteConnection _connection;
private readonly Func<ProposalDbContext> _contextFactory;
private readonly ProposalDbContext _db;
private readonly ICurrentUserService _currentUser;
private readonly ProposalService _sut;
private readonly LineItemService _lineItems;
private readonly Guid _userId = Guid.NewGuid();
public ProposalConcurrencyTests()
{
(_connection, _contextFactory) = SqliteDbContextFactory.CreateShared();
_db = _contextFactory();
_currentUser = Substitute.For<ICurrentUserService>();
_currentUser.UserId.Returns(_userId);
_currentUser.Role.Returns(UserRole.Admin);
_db.Users.Add(new User
{
Id = _userId,
CognitoSub = $"sub-{_userId}",
Email = "admin@test.com",
DisplayName = "Test Admin",
Role = UserRole.Admin,
CreatedAt = DateTime.UtcNow,
UpdatedAt = DateTime.UtcNow,
});
_db.SaveChanges();
var config = new ConfigurationBuilder()
.AddInMemoryCollection(new Dictionary<string, string?> { { "GENERATED_BUCKET", "test-bucket" } })
.Build();
// Real AuditService on the SAME context so staged-audit atomicity is exercised.
var audit = new AuditService(_db, _currentUser);
_sut = new ProposalService(_db, _currentUser, Substitute.For<IProposalNumberGenerator>(), audit,
Substitute.For<IJobPublisher>(), Substitute.For<IEmailService>(), Substitute.For<IS3Service>(),
config, Substitute.For<ILogger<ProposalService>>());
_lineItems = new LineItemService(_db, audit, Substitute.For<ILogger<LineItemService>>());
}
public void Dispose()
{
_db.Dispose();
_connection.Dispose();
}
private Proposal SeedProposal(ProposalStatus status = ProposalStatus.InReview)
{
var proposal = new Proposal
{
Id = Guid.NewGuid(),
ProposalNumber = $"P-{Guid.NewGuid():N}"[..12],
WorkOrderNumber = "WO-001",
CustomerName = "Test Customer",
CustomerAddress = "123 Test St",
ScopeOfWork = "Test scope of work",
ServiceCategory = ServiceCategory.HVAC,
Priority = Priority.Standard,
Status = status,
Notes = "",
SubmittedById = _userId,
SubmittedAt = DateTime.UtcNow.AddDays(-1),
CurrentRevision = 1,
CreatedAt = DateTime.UtcNow.AddDays(-1),
UpdatedAt = DateTime.UtcNow.AddDays(-1),
};
_db.Proposals.Add(proposal);
_db.SaveChanges();
return proposal;
}
private static UpdateProposalRequest UpdateNotes(string notes, string? version) =>
new(null, notes, null, null, null, version);
[Fact(DisplayName = "Missing proposalVersion -> 422 ProposalVersionRequired")]
public async Task MissingVersion_ThrowsVersionRequired()
{
var proposal = SeedProposal();
var act = () => _sut.UpdateAsync(proposal.Id, UpdateNotes("x", null));
var ex = await act.Should().ThrowAsync<BusinessRuleException>();
ex.Which.Code.Should().Be("ProposalVersionRequired");
}
[Fact(DisplayName = "Malformed proposalVersion -> 422 InvalidRowVersion")]
public async Task MalformedVersion_ThrowsInvalidRowVersion()
{
var proposal = SeedProposal();
var act = () => _sut.UpdateAsync(proposal.Id, UpdateNotes("x", "not-a-token!"));
var ex = await act.Should().ThrowAsync<BusinessRuleException>();
ex.Which.Code.Should().Be("InvalidRowVersion");
}
[Fact(DisplayName = "Stale token fails the pre-check and carries currentState")]
public async Task StaleToken_PreCheck_ThrowsWithCurrentState()
{
var proposal = SeedProposal();
var act = () => _sut.UpdateAsync(proposal.Id, UpdateNotes("x", RowVersionCodec.Encode(99)));
var ex = await act.Should().ThrowAsync<ProposalConcurrencyException>();
var state = ex.Which.CurrentState.Should().BeOfType<ProposalResponse>().Subject;
state.Id.Should().Be(proposal.Id);
state.RowVersion.Should().Be(RowVersionCodec.Encode(1));
}
[Fact(DisplayName = "DB-level race: concurrent writer wins, loser gets 409 state with the winner's values")]
public async Task LostRace_ReloadsCurrentState()
{
var proposal = SeedProposal();
var token = RowVersionCodec.Encode(1);
// Load into the service context first (pre-check will pass against version 1)...
var tracked = await _db.Proposals.FirstAsync(p => p.Id == proposal.Id);
tracked.Should().NotBeNull();
// ...then a second writer commits between the pre-check read and our save.
await using (var other = _contextFactory())
{
var competing = await other.Proposals.FirstAsync(p => p.Id == proposal.Id);
other.Entry(competing).Property(p => p.Version).OriginalValue = 1L;
competing.Notes = "the winner's notes";
competing.Version = 2;
await other.SaveChangesAsync();
}
var act = () => _sut.UpdateAsync(proposal.Id, UpdateNotes("the loser's notes", token));
var ex = await act.Should().ThrowAsync<ProposalConcurrencyException>();
var state = ex.Which.CurrentState.Should().BeOfType<ProposalResponse>().Subject;
state.Notes.Should().Be("the winner's notes");
state.RowVersion.Should().Be(RowVersionCodec.Encode(2));
}
[Fact(DisplayName = "Audit atomicity: a save lost to a concurrent writer persists no audit row")]
public async Task LostRace_PersistsNoAuditRow()
{
var proposal = SeedProposal();
await _db.Proposals.FirstAsync(p => p.Id == proposal.Id);
await using (var other = _contextFactory())
{
var competing = await other.Proposals.FirstAsync(p => p.Id == proposal.Id);
competing.Notes = "winner";
competing.Version = 2;
await other.SaveChangesAsync();
}
var act = () => _sut.UpdateAsync(proposal.Id, UpdateNotes("loser", RowVersionCodec.Encode(1)));
await act.Should().ThrowAsync<ProposalConcurrencyException>();
await using var check = _contextFactory();
(await check.AuditLogs.CountAsync()).Should().Be(0);
}
[Fact(DisplayName = "Successful update bumps the version and commits exactly one audit row atomically")]
public async Task SuccessfulUpdate_BumpsVersionAndCommitsAudit()
{
var proposal = SeedProposal();
var result = await _sut.UpdateAsync(proposal.Id, UpdateNotes("new notes", RowVersionCodec.Encode(1)));
result.RowVersion.Should().Be(RowVersionCodec.Encode(2));
await using var check = _contextFactory();
(await check.AuditLogs.CountAsync(a => a.ProposalId == proposal.Id)).Should().Be(1);
(await check.Proposals.SingleAsync(p => p.Id == proposal.Id)).Version.Should().Be(2);
}
[Fact(DisplayName = "Bulk line-item update is guarded by the proposal token")]
public async Task BulkUpdate_StaleProposalToken_Conflicts()
{
var proposal = SeedProposal();
var request = new BulkUpdateLineItemsRequest(
new List<UpdateLineItemEntry>
{
new(null, "Item", 1, "each", null, 100m, PricingMode.TotalPrice, 0, LineItemSource.Manual),
},
RowVersionCodec.Encode(42));
var act = () => _lineItems.BulkUpdateAsync(proposal.Id, request);
await act.Should().ThrowAsync<ProposalConcurrencyException>();
}
[Fact(DisplayName = "Line-item create bumps the proposal aggregate version")]
public async Task LineItemCreate_BumpsProposalVersion()
{
var proposal = SeedProposal();
var created = await _lineItems.CreateAsync(proposal.Id,
new CreateLineItemRequest("Item", 1, "each", null, 50m, PricingMode.TotalPrice, 0, LineItemSource.Manual));
created.RowVersion.Should().Be(RowVersionCodec.Encode(1));
await using var check = _contextFactory();
(await check.Proposals.SingleAsync(p => p.Id == proposal.Id)).Version.Should().Be(2);
}
[Fact(DisplayName = "Approve with the current token succeeds and returns the bumped token")]
public async Task Approve_WithCurrentToken_Succeeds()
{
var proposal = SeedProposal();
_db.LineItems.Add(new LineItem
{
Id = Guid.NewGuid(),
ProposalId = proposal.Id,
Description = "Priced item",
Quantity = 1,
Unit = "each",
TotalPrice = 500m,
PricingMode = PricingMode.TotalPrice,
Source = LineItemSource.Manual,
});
_db.SaveChanges();
var result = await _sut.ApproveAsync(proposal.Id, RowVersionCodec.Encode(1));
result.Status.Should().Be(ProposalStatus.Approved);
result.RowVersion.Should().Be(RowVersionCodec.Encode(2));
}
}

View file

@ -108,7 +108,7 @@ public class ProposalDeliveryTests : IDisposable
.Throws(new InvalidOperationException("SES is down"));
// Act — should NOT throw
var result = await _sut.MarkSentAsync(proposal.Id);
var result = await _sut.MarkSentAsync(proposal.Id, Ver(proposal.Id));
// Assert — transition completed despite email failure
result.Status.Should().Be(ProposalStatus.Sent);
@ -146,7 +146,7 @@ public class ProposalDeliveryTests : IDisposable
.Returns("https://s3.example.com/acme.pdf");
// Act
var result = await _sut.MarkSentAsync(proposal.Id);
var result = await _sut.MarkSentAsync(proposal.Id, Ver(proposal.Id));
// Assert
result.Status.Should().Be(ProposalStatus.Sent);
@ -176,7 +176,7 @@ public class ProposalDeliveryTests : IDisposable
await _db.SaveChangesAsync();
// Act
var result = await _sut.MarkSentAsync(proposal.Id);
var result = await _sut.MarkSentAsync(proposal.Id, Ver(proposal.Id));
// Assert
result.Status.Should().Be(ProposalStatus.Sent);
@ -193,7 +193,7 @@ public class ProposalDeliveryTests : IDisposable
await _db.SaveChangesAsync();
// Act
var result = await _sut.MarkSentAsync(proposal.Id);
var result = await _sut.MarkSentAsync(proposal.Id, Ver(proposal.Id));
// Assert
result.Status.Should().Be(ProposalStatus.Sent);
@ -219,7 +219,7 @@ public class ProposalDeliveryTests : IDisposable
await _db.SaveChangesAsync();
// Act
var result = await _sut.MarkSentAsync(proposal.Id);
var result = await _sut.MarkSentAsync(proposal.Id, Ver(proposal.Id));
// Assert
result.Status.Should().Be(ProposalStatus.Sent);
@ -248,4 +248,7 @@ public class ProposalDeliveryTests : IDisposable
UpdatedAt = DateTime.UtcNow.AddDays(-1),
};
}
private string Ver(Guid id) =>
ProposalSystem.Application.Common.RowVersionCodec.Encode(_db.Proposals.Find(id)?.Version ?? 1);
}

View file

@ -92,7 +92,7 @@ public class ProposalStateMachineTests : IDisposable
await _db.SaveChangesAsync();
// Act
var result = await _sut.ApproveAsync(proposal.Id);
var result = await _sut.ApproveAsync(proposal.Id, Ver(proposal.Id));
// Assert
result.Status.Should().Be(ProposalStatus.Approved);
@ -109,7 +109,7 @@ public class ProposalStateMachineTests : IDisposable
await _db.SaveChangesAsync();
// Act
var result = await _sut.MarkSentAsync(proposal.Id);
var result = await _sut.MarkSentAsync(proposal.Id, Ver(proposal.Id));
// Assert
result.Status.Should().Be(ProposalStatus.Sent);
@ -137,7 +137,7 @@ public class ProposalStateMachineTests : IDisposable
await _db.SaveChangesAsync();
// Act
var result = await _sut.ReviseAsync(proposal.Id);
var result = await _sut.ReviseAsync(proposal.Id, Ver(proposal.Id));
// Assert
result.Status.Should().Be(ProposalStatus.InReview);
@ -159,7 +159,7 @@ public class ProposalStateMachineTests : IDisposable
await _db.SaveChangesAsync();
// Act
var result = await _sut.ApproveAsync(proposal.Id);
var result = await _sut.ApproveAsync(proposal.Id, Ver(proposal.Id));
// Assert
result.Status.Should().Be(ProposalStatus.Approved);
@ -174,7 +174,7 @@ public class ProposalStateMachineTests : IDisposable
await _db.SaveChangesAsync();
// Act
var result = await _sut.MarkSentAsync(proposal.Id);
var result = await _sut.MarkSentAsync(proposal.Id, Ver(proposal.Id));
// Assert
result.Status.Should().Be(ProposalStatus.Sent);
@ -193,7 +193,7 @@ public class ProposalStateMachineTests : IDisposable
await _db.SaveChangesAsync();
// Act
var act = () => _sut.MarkSentAsync(proposal.Id);
var act = () => _sut.MarkSentAsync(proposal.Id, Ver(proposal.Id));
// Assert
await act.Should().ThrowAsync<InvalidOperationException>()
@ -209,7 +209,7 @@ public class ProposalStateMachineTests : IDisposable
await _db.SaveChangesAsync();
// Act
var act = () => _sut.ReviseAsync(proposal.Id);
var act = () => _sut.ReviseAsync(proposal.Id, Ver(proposal.Id));
// Assert
await act.Should().ThrowAsync<InvalidOperationException>()
@ -236,7 +236,7 @@ public class ProposalStateMachineTests : IDisposable
await _db.SaveChangesAsync();
// Act
var act = () => _sut.ApproveAsync(proposal.Id);
var act = () => _sut.ApproveAsync(proposal.Id, Ver(proposal.Id));
// Assert
await act.Should().ThrowAsync<InvalidOperationException>()
@ -252,7 +252,7 @@ public class ProposalStateMachineTests : IDisposable
await _db.SaveChangesAsync();
// Act
var act = () => _sut.ReviseAsync(proposal.Id);
var act = () => _sut.ReviseAsync(proposal.Id, Ver(proposal.Id));
// Assert
await act.Should().ThrowAsync<InvalidOperationException>()
@ -267,7 +267,7 @@ public class ProposalStateMachineTests : IDisposable
_db.Proposals.Add(proposal);
await _db.SaveChangesAsync();
var act = () => _sut.ApproveAsync(proposal.Id);
var act = () => _sut.ApproveAsync(proposal.Id, Ver(proposal.Id));
await act.Should().ThrowAsync<InvalidOperationException>()
.WithMessage("*in review*");
@ -282,7 +282,7 @@ public class ProposalStateMachineTests : IDisposable
await _db.SaveChangesAsync();
// Act
var act = () => _sut.ApproveAsync(proposal.Id);
var act = () => _sut.ApproveAsync(proposal.Id, Ver(proposal.Id));
// Assert
await act.Should().ThrowAsync<InvalidOperationException>()
@ -309,7 +309,7 @@ public class ProposalStateMachineTests : IDisposable
await _db.SaveChangesAsync();
// Act
var act = () => _sut.ApproveAsync(proposal.Id);
var act = () => _sut.ApproveAsync(proposal.Id, Ver(proposal.Id));
// Assert
await act.Should().ThrowAsync<InvalidOperationException>()
@ -321,9 +321,9 @@ public class ProposalStateMachineTests : IDisposable
{
var missingId = Guid.NewGuid();
var approveAct = () => _sut.ApproveAsync(missingId);
var sendAct = () => _sut.MarkSentAsync(missingId);
var reviseAct = () => _sut.ReviseAsync(missingId);
var approveAct = () => _sut.ApproveAsync(missingId, Ver(missingId));
var sendAct = () => _sut.MarkSentAsync(missingId, Ver(missingId));
var reviseAct = () => _sut.ReviseAsync(missingId, Ver(missingId));
await approveAct.Should().ThrowAsync<KeyNotFoundException>();
await sendAct.Should().ThrowAsync<KeyNotFoundException>();
@ -368,7 +368,7 @@ public class ProposalStateMachineTests : IDisposable
await _db.SaveChangesAsync();
// Act
var result = await _sut.ReviseAsync(proposal.Id);
var result = await _sut.ReviseAsync(proposal.Id, Ver(proposal.Id));
// Assert
var revision = await _db.Proposals.FindAsync(result.Id);
@ -398,10 +398,10 @@ public class ProposalStateMachineTests : IDisposable
await _db.SaveChangesAsync();
// Act
await _sut.ApproveAsync(proposal.Id);
await _sut.ApproveAsync(proposal.Id, Ver(proposal.Id));
// Assert
await _audit.Received(1).LogAsync(AuditAction.Approve, proposal.Id, Arg.Any<string?>(), Arg.Any<CancellationToken>());
_audit.Received(1).Stage(AuditAction.Approve, proposal.Id, Arg.Any<string?>());
}
[Fact(DisplayName = "QA-C2: MarkSent publishes library-ingest job")]
@ -413,7 +413,7 @@ public class ProposalStateMachineTests : IDisposable
await _db.SaveChangesAsync();
// Act
await _sut.MarkSentAsync(proposal.Id);
await _sut.MarkSentAsync(proposal.Id, Ver(proposal.Id));
// Assert
await _jobPublisher.Received(1).PublishAsync("library-ingest", Arg.Any<object>(), Arg.Any<CancellationToken>());
@ -442,4 +442,7 @@ public class ProposalStateMachineTests : IDisposable
UpdatedAt = DateTime.UtcNow.AddDays(-1),
};
}
private string Ver(Guid id) =>
ProposalSystem.Application.Common.RowVersionCodec.Encode(_db.Proposals.Find(id)?.Version ?? 1);
}

View file

@ -0,0 +1,67 @@
# ADR 0004 — Optimistic concurrency: SHOC wire contract on a Postgres version column
- **Status:** Accepted (2026-07-13)
- **Decision owner:** Adam Moussa
- **Scope:** `api/` proposal aggregate, `shared/api-contracts`, all clients (web, mobile, suggestions Lambda)
## Context
SHOC-alignment Phase 6 ports shoc-backend's optimistic-concurrency convention
(PRs #10/#13–#18) to the proposal aggregate. SHOC's mechanics are built on SQL
Server `rowversion` (`byte[8]`, auto-rotated, base64 on the wire) with a
double-guard: pre-check the client token against the loaded row, stamp it as
EF's original value so the UPDATE's WHERE clause re-enforces it, and on a lost
race reload and embed the winner's state in a 409. PostgreSQL has no
`rowversion`; the candidates were the `xmin` system column or an explicit
version column.
## Decision
1. **Explicit `long Version` column** on `Proposals` and `LineItems`,
`IsConcurrencyToken`, additive migration with `DEFAULT 1`, incremented by
the mutating services. Not `xmin`: the xUnit suite runs on InMemory/SQLite
where xmin doesn't exist (SHOC needed an InMemory shim for the same
reason), the handbook expects a real, reversible migration, and xmin leaks
storage internals onto the wire.
2. **SHOC's wire contract verbatim.** Tokens are opaque base64 strings
(`RowVersionCodec`: 8-byte big-endian long — same shape as SHOC's
`"AQAAAAAAAAA="` tokens). Responses carry `rowVersion`; guarded requests
carry `proposalVersion`. Missing token → 422 `ProposalVersionRequired`;
malformed → 422 `InvalidRowVersion`; conflict → **409
`{ message, currentState }`** with the reloaded `ProposalResponse`
embedded; unguarded races → 409 `{ status, message, code }` fallback.
Both envelopes are deliberately **not** ProblemDetails (SHOC parity) and
serialize with the MVC pipeline's conventions (camelCase, string enums).
3. **One aggregate, one token.** Deviation from the drafted plan, forced by
the code: bulk line-item update is delete-all-and-recreate, so per-item
tokens are meaningless. The proposal token guards proposal fields, state
transitions, and the bulk replace; every line-item mutation
(create/bulk/delete) bumps the proposal version so nothing goes stale
silently. `LineItem.Version` exists (additive, on the wire) for future
per-item mutations only.
4. **Guard scope per SHOC precedent.** Updates and state transitions demand
the token; creates and deletes don't, but still bump the aggregate version
— a lost race there surfaces as the fallback 409 instead of a silent
overwrite (this includes `VendorProposalsController`'s vendor-cost
recalc). Internal writers (suggestions Lambda) fetch-and-echo the token
with one conflict retry.
5. **Caller contract:** the 409 `currentState` reload has no ownership
filter, so guard-reaching endpoints must stay admin-gated — enforced by
`GuardedEndpointAuthorizationTests`. Extend the guard with an ownership
predicate before wiring it to any dispatcher-reachable write.
6. **Audit atomicity (same phase):** `IAuditService.Stage` adds to the shared
context; every mutation stages before its single `SaveChangesAsync`, so
the domain change and its audit row commit or fail together. Self-saving
`LogAsync` remains for standalone events only.
## Consequences
- Breaking API change for mutating clients; web, mobile, and the suggestions
Lambda ship the token pass-through in the same change set.
- Deploys: migration auto-applies at API startup under `pg_advisory_lock`;
the column is additive with a default, so the previous Lambda version keeps
working against the migrated schema. Manual RDS snapshot before deploy;
down-script is two `DropColumn`s, to be tested against a snapshot-restored
copy before any production rollback.
- ADR 0002's boundary holds: the convention and wire contract converge with
SHOC; the platform (Postgres, integer column, explicit increments) does not.

View file

@ -447,17 +447,36 @@ def post_line_items(proposal_id: str, items: list[dict], existing_items: list[di
)
try:
resp = _retry_request(
"PUT",
f"{API_BASE_URL}/api/proposals/{proposal_id}/line-items",
json={"lineItems": line_items_payload},
headers=_api_headers(),
timeout=15,
)
if resp.status_code not in (200, 201):
# Fix: CONC-L1 — the bulk PUT is version-guarded (ADR 0004): echo the
# proposal's current rowVersion as proposalVersion, and retry once with
# a fresh token if a concurrent edit wins the race (409/stale 422).
for attempt in range(2):
proposal = fetch_proposal(proposal_id)
token = (proposal or {}).get("rowVersion")
if not token:
logger.error(
"Cannot post line items: no rowVersion for proposal %s", proposal_id
)
return
resp = _retry_request(
"PUT",
f"{API_BASE_URL}/api/proposals/{proposal_id}/line-items",
json={"lineItems": line_items_payload, "proposalVersion": token},
headers=_api_headers(),
timeout=15,
)
if resp.status_code in (200, 201):
return
if resp.status_code == 409 and attempt == 0:
logger.warning(
"Concurrency conflict posting line items for %s; retrying with fresh token",
proposal_id,
)
continue
logger.error(
"Failed to post line items: %s %s", resp.status_code, resp.text
)
return
except Exception as e:
# Fix: LAM-M5 — include stack trace in error logging
logger.exception("Error posting line items: %s", e)

View file

@ -138,9 +138,14 @@ class TestProcessSuggestion:
@patch.object(suggestions_app, "_retry_request")
def test_post_line_items_preserves_non_ai_items(self, mock_request):
"""QA-C6: Existing non-AI items are preserved when new suggestions are generated."""
mock_response = MagicMock()
mock_response.status_code = 200
mock_request.return_value = mock_response
# Fix: CONC-L1 — post_line_items now fetches the proposal's rowVersion
# first and echoes it as proposalVersion on the guarded bulk PUT.
get_response = MagicMock()
get_response.status_code = 200
get_response.json.return_value = {"rowVersion": "AAAAAAAAAAE="}
put_response = MagicMock()
put_response.status_code = 200
mock_request.side_effect = [get_response, put_response]
existing_items = [
{
@ -175,11 +180,13 @@ class TestProcessSuggestion:
suggestions_app.post_line_items("abc", new_items, existing_items)
# Verify the API was called
mock_request.assert_called_once()
# GET (token fetch) + PUT (guarded bulk update)
assert mock_request.call_count == 2
call_kwargs = mock_request.call_args
payload = call_kwargs.kwargs.get("json") or call_kwargs[1].get("json")
# The guarded PUT must echo the proposal's version token
assert payload["proposalVersion"] == "AAAAAAAAAAE="
# Should have 2 items: 1 preserved Manual + 1 new AI (old AI items are replaced)
assert len(payload["lineItems"]) == 2
sources = [li["source"] for li in payload["lineItems"]]
@ -191,6 +198,37 @@ class TestProcessSuggestion:
assert payload["lineItems"][1]["source"] == "AI"
assert payload["lineItems"][1]["description"] == "New AI item"
@patch.object(suggestions_app, "_retry_request")
def test_post_line_items_retries_once_on_conflict(self, mock_request):
"""CONC-L1: a 409 conflict refetches the token and retries the PUT once."""
get_1 = MagicMock(status_code=200)
get_1.json.return_value = {"rowVersion": "AAAAAAAAAAE="}
put_conflict = MagicMock(status_code=409, text="conflict")
get_2 = MagicMock(status_code=200)
get_2.json.return_value = {"rowVersion": "AAAAAAAAAAI="}
put_ok = MagicMock(status_code=200)
mock_request.side_effect = [get_1, put_conflict, get_2, put_ok]
suggestions_app.post_line_items(
"abc",
[
{
"description": "AI item",
"quantity": 1,
"unit": "each",
"totalPrice": 100,
"pricingMode": "TotalPrice",
}
],
[],
)
assert mock_request.call_count == 4
final_payload = mock_request.call_args.kwargs.get(
"json"
) or mock_request.call_args[1].get("json")
assert final_payload["proposalVersion"] == "AAAAAAAAAAI="
@patch.object(suggestions_app, "post_line_items")
@patch.object(suggestions_app, "generate_line_items")
@patch.object(suggestions_app, "retrieve_similar")

View file

@ -1,4 +1,5 @@
import apiClient from './client';
import type { ProposalDetail } from './proposals';
export interface DashboardStats {
pendingCount: number;
@ -11,6 +12,8 @@ export interface UpdateProposalRequest {
refinedScope?: string;
notes?: string;
assignedAdminId?: string;
/** Expected proposal rowVersion — REQUIRED by the server (422 ProposalVersionRequired when missing). */
proposalVersion?: string;
}
export interface AuditEntry {
@ -30,23 +33,47 @@ export const adminApi = {
return res.data;
},
// Fix: CONC-L2 — guarded mutations (optimistic concurrency): every write
// below requires the proposal's rowVersion token and returns the fresh
// proposal state — the server rotates the token on each guarded write, so
// callers chain from the RESPONSE's rowVersion, never the one they started
// with.
updateProposal: async (
id: string,
data: UpdateProposalRequest,
): Promise<void> => {
await apiClient.put(`/proposals/${id}`, data);
): Promise<ProposalDetail> => {
const res = await apiClient.put(`/proposals/${id}`, data);
return res.data;
},
approveProposal: async (id: string): Promise<void> => {
await apiClient.post(`/proposals/${id}/approve`);
approveProposal: async (
id: string,
proposalVersion?: string,
): Promise<ProposalDetail> => {
const res = await apiClient.post(`/proposals/${id}/approve`, {
proposalVersion,
});
return res.data;
},
sendProposal: async (id: string): Promise<void> => {
await apiClient.post(`/proposals/${id}/send`);
sendProposal: async (
id: string,
proposalVersion?: string,
): Promise<ProposalDetail> => {
const res = await apiClient.post(`/proposals/${id}/send`, {
proposalVersion,
});
return res.data;
},
reviseProposal: async (id: string): Promise<void> => {
await apiClient.post(`/proposals/${id}/revise`);
reviseProposal: async (
id: string,
proposalVersion?: string,
): Promise<ProposalDetail> => {
const res = await apiClient.post(`/proposals/${id}/revise`, {
proposalVersion,
});
return res.data;
},
getAudit: async (id: string): Promise<AuditEntry[]> => {

View file

@ -2,6 +2,19 @@ import axios from 'axios';
import Config from '../../config';
import { tokenStorage } from '../storage';
// Fix: CONC-L2 — carries HTTP status + response body so screens can branch on
// optimistic-concurrency conflicts (409 { message, currentState }).
export class ApiError extends Error {
constructor(
message: string,
public status: number,
public data?: unknown,
) {
super(message);
this.name = 'ApiError';
}
}
type RefreshFn = () => Promise<void>;
let _refreshTokens: RefreshFn | null = null;
@ -108,7 +121,7 @@ apiClient.interceptors.response.use(
const message =
data?.detail || data?.title || data?.message || 'An error occurred';
return Promise.reject(new Error(message));
return Promise.reject(new ApiError(message, status, data));
}
if (error.request) {

View file

@ -14,6 +14,8 @@ export interface LineItem {
source: LineItemSource;
createdAt: string;
updatedAt: string;
/** Opaque optimistic-concurrency token (base64); echo back untouched. */
rowVersion: string;
}
export interface UpdateLineItemEntry {
@ -34,12 +36,16 @@ export const lineItemsApi = {
return res.data;
},
// Fix: CONC-L2 — the bulk replace is guarded by the PROPOSAL rowVersion
// token; the server rotates it on success, so refetch the proposal after.
bulkUpdate: async (
proposalId: string,
lineItems: UpdateLineItemEntry[],
proposalVersion?: string,
): Promise<LineItem[]> => {
const res = await apiClient.put(`/proposals/${proposalId}/line-items`, {
lineItems,
proposalVersion,
});
return res.data;
},

View file

@ -13,6 +13,8 @@ export interface ProposalListItem {
submittedAt: string;
submittedByName: string | null;
assignedAdminName: string | null;
/** Opaque optimistic-concurrency token (base64); echo back untouched. */
rowVersion: string;
}
export interface ProposalDetail {
@ -40,6 +42,8 @@ export interface ProposalDetail {
parentProposalId: string | null;
createdAt: string;
updatedAt: string;
/** Opaque optimistic-concurrency token (base64); echo back untouched. */
rowVersion: string;
}
export interface PagedResponse<T> {

View file

@ -16,7 +16,9 @@ import {
} from 'react-native-paper';
import { useRoute, useNavigation } from '@react-navigation/native';
import { useMutation, useQueryClient } from '@tanstack/react-query';
import { ApiError } from '../../lib/api/client';
import { lineItemsApi, type UpdateLineItemEntry } from '../../lib/api/lineItems';
import { proposalsApi, type ProposalDetail } from '../../lib/api/proposals';
import { QUERY_KEYS } from '../../constants/queryKeys';
import type { NativeStackScreenProps } from '@react-navigation/native-stack';
import type { ProposalsStackParamList } from '../../navigation/types';
@ -98,16 +100,39 @@ export function LineItemEditScreen() {
{ ...entry, sortOrder: existing.length },
];
return lineItemsApi.bulkUpdate(proposalId, updated);
// Fix: CONC-L2 — the bulk replace is guarded by the proposal's
// rowVersion token; use the one the workspace already loaded, or fetch
// a fresh one if the cache is empty.
const cachedProposal = qc.getQueryData<ProposalDetail>([
QUERY_KEYS.proposal,
proposalId,
]);
const proposalVersion =
cachedProposal?.rowVersion ??
(await proposalsApi.getById(proposalId)).rowVersion;
return lineItemsApi.bulkUpdate(proposalId, updated, proposalVersion);
},
onSuccess: () => {
qc.invalidateQueries({
queryKey: [QUERY_KEYS.proposalLineItems, proposalId],
});
// Fix: CONC-L2 — the server rotated the proposal token; refetch it.
qc.invalidateQueries({ queryKey: [QUERY_KEYS.proposal, proposalId] });
setSnackbar('Line item saved');
setTimeout(() => navigation.goBack(), 800);
},
onError: (err: Error) => Alert.alert('Error', err.message),
onError: (err: Error) => {
Alert.alert('Error', err.message);
// Fix: CONC-L2 — on a 409 concurrency conflict, refetch so the
// workspace picks up the fresh state + rotated token.
if (err instanceof ApiError && err.status === 409) {
qc.invalidateQueries({ queryKey: [QUERY_KEYS.proposal, proposalId] });
qc.invalidateQueries({
queryKey: [QUERY_KEYS.proposalLineItems, proposalId],
});
}
},
});
const handleAutoCalc = () => {

View file

@ -24,6 +24,7 @@ import ReactNativeHapticFeedback from 'react-native-haptic-feedback';
import { StatusChip } from '../../components/StatusChip';
import { PriorityChip } from '../../components/PriorityChip';
import { LineItemRow } from '../../components/LineItemRow';
import { ApiError } from '../../lib/api/client';
import { proposalsApi } from '../../lib/api/proposals';
import { lineItemsApi } from '../../lib/api/lineItems';
import { adminApi } from '../../lib/api/admin';
@ -67,31 +68,45 @@ export function ProposalWorkspaceScreen() {
});
};
// Fix: CONC-L2 — on a 409 concurrency conflict, surface the server message
// and refetch so the workspace picks up the fresh state + rotated token.
const handleGuardedError = (err: Error) => {
Alert.alert('Error', err.message);
if (err instanceof ApiError && err.status === 409) {
invalidate();
}
};
const approveMutation = useMutation({
mutationFn: () => adminApi.approveProposal(proposalId),
onSuccess: () => {
mutationFn: () =>
adminApi.approveProposal(proposalId, proposal?.rowVersion),
onSuccess: (updated) => {
ReactNativeHapticFeedback.trigger('notificationSuccess');
setSnackbar('Proposal approved');
qc.setQueryData([QUERY_KEYS.proposal, proposalId], updated);
invalidate();
},
onError: (err: Error) => Alert.alert('Error', err.message),
onError: handleGuardedError,
});
const sendMutation = useMutation({
mutationFn: () => adminApi.sendProposal(proposalId),
onSuccess: () => {
mutationFn: () => adminApi.sendProposal(proposalId, proposal?.rowVersion),
onSuccess: (updated) => {
setSnackbar('Marked as sent');
qc.setQueryData([QUERY_KEYS.proposal, proposalId], updated);
invalidate();
},
onError: (err: Error) => Alert.alert('Error', err.message),
onError: handleGuardedError,
});
const regenerateMutation = useMutation({
mutationFn: async () => {
if (scopeInput.trim()) {
await adminApi.updateProposal(proposalId, {
const updated = await adminApi.updateProposal(proposalId, {
refinedScope: scopeInput.trim(),
proposalVersion: proposal?.rowVersion,
});
qc.setQueryData([QUERY_KEYS.proposal, proposalId], updated);
}
await adminApi.generateSuggestions(proposalId);
},
@ -100,7 +115,7 @@ export function ProposalWorkspaceScreen() {
setSnackbar('AI suggestions regenerating...');
setTimeout(invalidate, 5000);
},
onError: (err: Error) => Alert.alert('Error', err.message),
onError: handleGuardedError,
});
const handleDownloadPdf = async () => {

View file

@ -34,6 +34,8 @@ export interface ProposalListItem {
submittedAt: string;
submittedByName: string | null;
assignedAdminName: string | null;
/** Opaque optimistic-concurrency token (base64); echo back untouched. */
rowVersion: string;
}
export interface ProposalDetail {
@ -62,6 +64,8 @@ export interface ProposalDetail {
parentProposalId: string | null;
createdAt: string;
updatedAt: string;
/** Opaque optimistic-concurrency token (base64); echo back untouched. */
rowVersion: string;
}
export interface CreateProposalRequest {
@ -81,6 +85,16 @@ export interface UpdateProposalRequest {
poNumber?: string;
workOrderNumber?: string;
assignedAdminId?: string;
/** Expected proposal rowVersion — REQUIRED by the server (422 ProposalVersionRequired when missing). */
proposalVersion?: string;
}
/** Body for the state-transition endpoints (approve, return-to-review, send,
* revise): the expected proposal rowVersion. The server rejects a missing
* token with 422 {code:"ProposalVersionRequired"} and a malformed one with
* 422 {code:"InvalidRowVersion"}. */
export interface ProposalVersionRequest {
proposalVersion?: string;
}
export interface ProposalFilters {
@ -116,6 +130,8 @@ export interface LineItem {
source: LineItemSource;
createdAt: string;
updatedAt: string;
/** Opaque optimistic-concurrency token (base64); echo back untouched. */
rowVersion: string;
}
export interface CreateLineItemRequest {
@ -143,6 +159,8 @@ export interface UpdateLineItemEntry {
export interface BulkUpdateLineItemsRequest {
lineItems: UpdateLineItemEntry[];
/** Expected proposal rowVersion — the bulk replace is guarded by the PROPOSAL token. */
proposalVersion?: string;
}
// ── Customers (CustomerDtos.cs) ───────────────────────────────────────────
@ -285,3 +303,13 @@ export interface ApiProblem {
detail: string;
code: ApiProblemCode;
}
// Optimistic-concurrency conflict envelope (SHOC ADR 0004 — NOT ProblemDetails):
// guarded writes that lose a race return 409 with the reloaded currentState so
// clients can refresh immediately. The unguarded-race safety net returns
// 409 { status: "Conflict", message, code: 409 } instead — same `message`
// field, no currentState.
export interface ConcurrencyConflict<T> {
message: string;
currentState: T | null;
}

View file

@ -14,6 +14,8 @@ import type {
ProposalDetail,
CreateProposalRequest,
UpdateProposalRequest,
ProposalVersionRequest,
ConcurrencyConflict,
LineItem,
CreateLineItemRequest,
UpdateLineItemEntry,
@ -56,6 +58,7 @@ export const proposalListItemSchema = z.object({
submittedAt: z.string(),
submittedByName: z.string().nullable(),
assignedAdminName: z.string().nullable(),
rowVersion: z.string(),
}) satisfies z.ZodType<ProposalListItem>;
export const proposalDetailSchema = z.object({
@ -84,6 +87,7 @@ export const proposalDetailSchema = z.object({
parentProposalId: z.string().nullable(),
createdAt: z.string(),
updatedAt: z.string(),
rowVersion: z.string(),
}) satisfies z.ZodType<ProposalDetail>;
export const createProposalRequestSchema = z.object({
@ -103,8 +107,13 @@ export const updateProposalRequestSchema = z.object({
poNumber: z.string().optional(),
workOrderNumber: z.string().optional(),
assignedAdminId: z.string().optional(),
proposalVersion: z.string().optional(),
}) satisfies z.ZodType<UpdateProposalRequest>;
export const proposalVersionRequestSchema = z.object({
proposalVersion: z.string().optional(),
}) satisfies z.ZodType<ProposalVersionRequest>;
export const proposalStatsSchema = z.object({
totalCount: z.number(),
inReviewCount: z.number(),
@ -126,6 +135,7 @@ export const lineItemSchema = z.object({
source: lineItemSourceSchema,
createdAt: z.string(),
updatedAt: z.string(),
rowVersion: z.string(),
}) satisfies z.ZodType<LineItem>;
export const createLineItemRequestSchema = z.object({
@ -153,6 +163,7 @@ export const updateLineItemEntrySchema = z.object({
export const bulkUpdateLineItemsRequestSchema = z.object({
lineItems: z.array(updateLineItemEntrySchema),
proposalVersion: z.string().optional(),
}) satisfies z.ZodType<BulkUpdateLineItemsRequest>;
// ── Customers ─────────────────────────────────────────────────────────────
@ -274,3 +285,11 @@ export const apiProblemSchema = z.object({
detail: z.string(),
code: z.string(),
}) satisfies z.ZodType<ApiProblem>;
// 409 concurrency-conflict envelope. The proposal aggregate is the only
// guarded resource today, so the concrete schema is coupled to ProposalDetail;
// build others via the same pattern when new aggregates gain guards.
export const proposalConcurrencyConflictSchema = z.object({
message: z.string(),
currentState: proposalDetailSchema.nullable(),
}) satisfies z.ZodType<ConcurrencyConflict<ProposalDetail>>;

View file

@ -38,6 +38,7 @@ const proposals = [
submittedAt: '2026-07-01T12:00:00Z',
submittedByName: 'Adam Moussa',
assignedAdminName: null,
rowVersion: 'AAAAAAAAAAE=',
},
{
id: 'p-2',
@ -51,6 +52,7 @@ const proposals = [
submittedAt: '2026-07-05T09:30:00Z',
submittedByName: 'Adam Moussa',
assignedAdminName: 'Sarah Chen',
rowVersion: 'AAAAAAAAAAI=',
},
];

View file

@ -1,14 +1,20 @@
// Admin domain use-case hooks: state transitions must cross-domain
// invalidate the proposals/lineItems keys (domain README rule 3). The domain
// api modules are mocked — no axios traffic.
// invalidate the proposals/lineItems keys (domain README rule 3), thread the
// optimistic-concurrency token from the cached detail into guarded mutations,
// and recover from 409 conflicts by refreshing from the server's currentState.
// The domain api modules are mocked — no axios traffic.
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { renderHook, waitFor } from '@testing-library/react';
import { toast } from 'react-toastify';
import { createQueryHarness } from './hookTestUtils';
import { useApproveProposal } from '../admin/use-cases';
import { useApproveProposal, useSaveProposalWorkspace } from '../admin/use-cases';
import { proposalsKeys } from '../proposals/use-cases';
import { lineItemsKeys } from '../lineItems/use-cases';
import { adminApi } from '../admin/api';
import { lineItemsApi } from '../lineItems/api';
import { proposalsApi } from '../proposals/api';
import { ConflictError } from '../../lib/api/errors';
import type { ProposalDetail } from '../proposals/types';
vi.mock('../admin/api', () => ({
adminApi: {
@ -26,8 +32,8 @@ vi.mock('../admin/api', () => ({
},
}));
// useSaveProposalWorkspace pulls lineItemsApi directly; mock it so no test
// path can reach axios.
// useSaveProposalWorkspace pulls lineItemsApi and proposalsApi directly;
// mock both so no test path can reach axios.
vi.mock('../lineItems/api', () => ({
lineItemsApi: {
getAll: vi.fn(),
@ -37,27 +43,51 @@ vi.mock('../lineItems/api', () => ({
},
}));
vi.mock('../proposals/api', () => ({
proposalsApi: {
create: vi.fn(),
getAll: vi.fn(),
getById: vi.fn(),
uploadAttachment: vi.fn(),
confirmUpload: vi.fn(),
getStats: vi.fn(),
getVendors: vi.fn(),
},
}));
vi.mock('react-toastify', () => ({
toast: { success: vi.fn(), error: vi.fn(), warning: vi.fn(), info: vi.fn() },
}));
const PROPOSAL_ID = 'p-7';
const detail = (rowVersion: string): ProposalDetail =>
({ id: PROPOSAL_ID, rowVersion }) as unknown as ProposalDetail;
beforeEach(() => {
vi.clearAllMocks();
});
describe('useApproveProposal', () => {
it('invalidates the proposal detail and its line items keys and toasts on success', async () => {
vi.mocked(adminApi.approveProposal).mockResolvedValue(undefined);
it('sends the cached rowVersion token, invalidates the proposal detail and its line items keys, and toasts on success', async () => {
vi.mocked(adminApi.approveProposal).mockResolvedValue(detail('AAAAAAAAAAM='));
const { queryClient, wrapper } = createQueryHarness();
// The workspace always fetches the detail before actions are possible —
// the guarded mutation reads its token from this cache at mutate time.
queryClient.setQueryData(proposalsKeys.detail(PROPOSAL_ID), detail('AAAAAAAAAAI='));
const invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries');
const { result } = renderHook(() => useApproveProposal(PROPOSAL_ID), { wrapper });
result.current.mutate();
await waitFor(() => expect(result.current.isSuccess).toBe(true));
expect(adminApi.approveProposal).toHaveBeenCalledWith(PROPOSAL_ID);
expect(adminApi.approveProposal).toHaveBeenCalledWith(PROPOSAL_ID, 'AAAAAAAAAAI=');
// The response carries the rotated token — it must land in the detail
// cache immediately so a follow-on guarded action doesn't race the
// invalidation refetch.
expect(queryClient.getQueryData(proposalsKeys.detail(PROPOSAL_ID))).toEqual(
detail('AAAAAAAAAAM='),
);
// Approval changes proposal status AND locks/reprices line items — both
// domains' keys must be refetched (cross-domain invalidation, rule 3).
expect(invalidateSpy).toHaveBeenCalledWith({
@ -94,4 +124,78 @@ describe('useApproveProposal', () => {
expect(invalidateSpy).not.toHaveBeenCalled();
expect(toast.success).not.toHaveBeenCalled();
});
it('on 409 writes the server currentState into the detail cache, refreshes views, and toasts the conflict', async () => {
const currentState = detail('AAAAAAAAAAo=');
vi.mocked(adminApi.approveProposal).mockRejectedValue(
new ConflictError('Proposal was modified by another user.', currentState),
);
const { queryClient, wrapper } = createQueryHarness();
queryClient.setQueryData(proposalsKeys.detail(PROPOSAL_ID), detail('AAAAAAAAAAI='));
const invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries');
const { result } = renderHook(() => useApproveProposal(PROPOSAL_ID), { wrapper });
result.current.mutate();
await waitFor(() => expect(result.current.isError).toBe(true));
// Refresh-and-retry semantics: the stale detail is replaced by the
// server's reloaded state (fresh token included)...
expect(queryClient.getQueryData(proposalsKeys.detail(PROPOSAL_ID))).toEqual(currentState);
// ...every proposal view is refetched (stale-queue invariant holds even
// on the failure path)...
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: proposalsKeys.detail(PROPOSAL_ID) });
expect(invalidateSpy).toHaveBeenCalledWith({
queryKey: lineItemsKeys.byProposal(PROPOSAL_ID),
});
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: proposalsKeys.lists() });
// ...and the user gets the conflict message, not the generic failure toast.
expect(toast.warning).toHaveBeenCalledWith('Proposal was modified by another user.');
expect(toast.error).not.toHaveBeenCalled();
});
});
describe('useSaveProposalWorkspace', () => {
it('rotates the token across the guarded pair and refetches the detail for the fresh token', async () => {
vi.mocked(adminApi.updateProposal).mockResolvedValue(detail('AAAAAAAAAAM='));
vi.mocked(lineItemsApi.bulkUpdate).mockResolvedValue([]);
vi.mocked(proposalsApi.getById).mockResolvedValue(detail('AAAAAAAAAAQ='));
const { queryClient, wrapper } = createQueryHarness();
queryClient.setQueryData(proposalsKeys.detail(PROPOSAL_ID), detail('AAAAAAAAAAI='));
const { result } = renderHook(() => useSaveProposalWorkspace(PROPOSAL_ID), { wrapper });
result.current.mutate({ refinedScope: 'refined', lineItems: [] });
await waitFor(() => expect(result.current.isSuccess).toBe(true));
// PUT sends the token the workspace loaded with...
expect(adminApi.updateProposal).toHaveBeenCalledWith(PROPOSAL_ID, {
refinedScope: 'refined',
proposalVersion: 'AAAAAAAAAAI=',
});
// ...the bulk replace sends the token the PUT rotated to...
expect(lineItemsApi.bulkUpdate).toHaveBeenCalledWith(PROPOSAL_ID, [], 'AAAAAAAAAAM=');
// ...and the detail cache ends on the post-bulk token so approve-after-save
// never sends a stale version.
expect(queryClient.getQueryData(proposalsKeys.detail(PROPOSAL_ID))).toEqual(
detail('AAAAAAAAAAQ='),
);
expect(toast.success).toHaveBeenCalledWith('Changes saved');
});
it('on 409 refreshes from currentState and toasts the conflict instead of the save-failed toast', async () => {
const currentState = detail('AAAAAAAAAAo=');
vi.mocked(adminApi.updateProposal).mockRejectedValue(
new ConflictError('Proposal was modified by another user.', currentState),
);
const { queryClient, wrapper } = createQueryHarness();
queryClient.setQueryData(proposalsKeys.detail(PROPOSAL_ID), detail('AAAAAAAAAAI='));
const { result } = renderHook(() => useSaveProposalWorkspace(PROPOSAL_ID), { wrapper });
result.current.mutate({ refinedScope: 'refined', lineItems: [] });
await waitFor(() => expect(result.current.isError).toBe(true));
expect(lineItemsApi.bulkUpdate).not.toHaveBeenCalled();
expect(queryClient.getQueryData(proposalsKeys.detail(PROPOSAL_ID))).toEqual(currentState);
expect(toast.warning).toHaveBeenCalledWith('Proposal was modified by another user.');
expect(toast.error).not.toHaveBeenCalled();
});
});

View file

@ -11,7 +11,9 @@ import { useSaveProposalWorkspace } from '../admin/use-cases';
import { proposalsKeys } from '../proposals/use-cases';
import { adminApi } from '../admin/api';
import { lineItemsApi } from '../lineItems/api';
import { proposalsApi } from '../proposals/api';
import type { UpdateLineItemEntry } from '../lineItems/types';
import type { ProposalDetail } from '../proposals/types';
vi.mock('../admin/api', () => ({
adminApi: {
@ -38,6 +40,19 @@ vi.mock('../lineItems/api', () => ({
},
}));
// The save flow ends with a detail refetch (post-bulk token rotation).
vi.mock('../proposals/api', () => ({
proposalsApi: {
create: vi.fn(),
getAll: vi.fn(),
getById: vi.fn(),
uploadAttachment: vi.fn(),
confirmUpload: vi.fn(),
getStats: vi.fn(),
getVendors: vi.fn(),
},
}));
vi.mock('react-toastify', () => ({
toast: { success: vi.fn(), error: vi.fn(), warning: vi.fn(), info: vi.fn() },
}));
@ -47,6 +62,9 @@ const entries = [
{ description: 'Labor', quantity: 2, unitPrice: 150 },
] as unknown as UpdateLineItemEntry[];
const detail = (rowVersion: string): ProposalDetail =>
({ id: PROPOSAL_ID, rowVersion }) as unknown as ProposalDetail;
beforeEach(() => {
vi.clearAllMocks();
});
@ -60,17 +78,24 @@ describe('lineItemsKeys', () => {
describe('useSaveProposalWorkspace', () => {
it('persists scope + entries and invalidates line items, detail, and list/stats views', async () => {
vi.mocked(adminApi.updateProposal).mockResolvedValue(undefined);
vi.mocked(adminApi.updateProposal).mockResolvedValue(detail('v2'));
vi.mocked(lineItemsApi.bulkUpdate).mockResolvedValue([]);
vi.mocked(proposalsApi.getById).mockResolvedValue(detail('v3'));
const { queryClient, wrapper } = createQueryHarness();
queryClient.setQueryData(proposalsKeys.detail(PROPOSAL_ID), detail('v1'));
const invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries');
const { result } = renderHook(() => useSaveProposalWorkspace(PROPOSAL_ID), { wrapper });
result.current.mutate({ refinedScope: 'refined', lineItems: entries });
await waitFor(() => expect(result.current.isSuccess).toBe(true));
expect(adminApi.updateProposal).toHaveBeenCalledWith(PROPOSAL_ID, { refinedScope: 'refined' });
expect(lineItemsApi.bulkUpdate).toHaveBeenCalledWith(PROPOSAL_ID, entries);
// Token rotation: the PUT sends the cached token, the bulk replace sends
// the PUT response's rotated token.
expect(adminApi.updateProposal).toHaveBeenCalledWith(PROPOSAL_ID, {
refinedScope: 'refined',
proposalVersion: 'v1',
});
expect(lineItemsApi.bulkUpdate).toHaveBeenCalledWith(PROPOSAL_ID, entries, 'v2');
expect(invalidateSpy).toHaveBeenCalledWith({
queryKey: lineItemsKeys.byProposal(PROPOSAL_ID),
});
@ -85,7 +110,7 @@ describe('useSaveProposalWorkspace', () => {
});
it('toasts a save failure and skips invalidation', async () => {
vi.mocked(adminApi.updateProposal).mockRejectedValue(new Error('409 conflict'));
vi.mocked(adminApi.updateProposal).mockRejectedValue(new Error('validation failed'));
const { queryClient, wrapper } = createQueryHarness();
const invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries');
@ -93,7 +118,7 @@ describe('useSaveProposalWorkspace', () => {
result.current.mutate({ refinedScope: 'refined', lineItems: entries });
await waitFor(() => expect(result.current.isError).toBe(true));
expect(toast.error).toHaveBeenCalledWith('Save failed: 409 conflict');
expect(toast.error).toHaveBeenCalledWith('Save failed: validation failed');
expect(lineItemsApi.bulkUpdate).not.toHaveBeenCalled();
expect(invalidateSpy).not.toHaveBeenCalled();
});

View file

@ -9,24 +9,34 @@ export const adminApi = {
return res.data;
},
updateProposal: async (id: string, data: UpdateProposalRequest): Promise<void> => {
await apiClient.put(`/proposals/${id}`, data);
// Guarded mutations (optimistic concurrency): every write below requires the
// proposal's rowVersion token and returns the fresh proposal state — the
// server rotates the token on each guarded write, so callers chain from the
// RESPONSE's rowVersion, never the one they started with.
updateProposal: async (id: string, data: UpdateProposalRequest): Promise<ProposalDetail> => {
const res = await apiClient.put(`/proposals/${id}`, data);
return res.data;
},
approveProposal: async (id: string): Promise<void> => {
await apiClient.post(`/proposals/${id}/approve`);
approveProposal: async (id: string, proposalVersion?: string): Promise<ProposalDetail> => {
const res = await apiClient.post(`/proposals/${id}/approve`, { proposalVersion });
return res.data;
},
returnToReview: async (id: string): Promise<void> => {
await apiClient.post(`/proposals/${id}/return-to-review`);
returnToReview: async (id: string, proposalVersion?: string): Promise<ProposalDetail> => {
const res = await apiClient.post(`/proposals/${id}/return-to-review`, { proposalVersion });
return res.data;
},
sendProposal: async (id: string): Promise<void> => {
await apiClient.post(`/proposals/${id}/send`);
sendProposal: async (id: string, proposalVersion?: string): Promise<ProposalDetail> => {
const res = await apiClient.post(`/proposals/${id}/send`, { proposalVersion });
return res.data;
},
reviseProposal: async (id: string): Promise<void> => {
await apiClient.post(`/proposals/${id}/revise`);
/** Returns the NEW revision (different id), not the revised original. */
reviseProposal: async (id: string, proposalVersion?: string): Promise<ProposalDetail> => {
const res = await apiClient.post(`/proposals/${id}/revise`, { proposalVersion });
return res.data;
},
getHistory: async (id: string): Promise<ProposalDetail[]> => {

View file

@ -8,9 +8,12 @@ import { useMutation, useQuery, useQueryClient, type QueryClient } from '@tansta
import { toast } from 'react-toastify';
import { adminApi } from './api';
import { lineItemsApi } from '../lineItems/api';
import { proposalsApi } from '../proposals/api';
import { proposalsKeys } from '../proposals/use-cases';
import { lineItemsKeys } from '../lineItems/use-cases';
import { ConflictError } from '../../lib/api/errors';
import type { UpdateLineItemEntry } from '../lineItems/types';
import type { ProposalDetail } from '../proposals/types';
export const adminKeys = {
all: ['admin'] as const,
@ -33,6 +36,41 @@ function invalidateProposalViews(queryClient: QueryClient, proposalId: string) {
queryClient.invalidateQueries({ queryKey: adminKeys.dashboard() });
}
/**
* The optimistic-concurrency token for a guarded mutation, read from the
* cached proposal detail at mutate time (the workspace always fetches the
* detail before any action is possible). Read lazily inside mutationFn —
* never captured at render — so a save-then-approve chain sees the token the
* save wrote back, not the one the page rendered with.
*/
function cachedProposalVersion(queryClient: QueryClient, proposalId: string): string | undefined {
return queryClient.getQueryData<ProposalDetail>(proposalsKeys.detail(proposalId))?.rowVersion;
}
/**
* 409 recovery (refresh-and-retry semantics): write the server's reloaded
* currentState into the detail cache (fresh token immediately available),
* refetch every proposal view, and tell the user their view was stale.
* Returns true when the error was a concurrency conflict — callers skip
* their generic failure toast in that case.
*/
function handleConcurrencyConflict(
queryClient: QueryClient,
proposalId: string,
error: Error,
): boolean {
if (!(error instanceof ConflictError)) return false;
// Only seed the cache when the embedded state is actually this proposal —
// a mismatched or partial envelope falls through to invalidation, and the
// refetch restores truth.
if (error.currentState && error.currentState.id === proposalId) {
queryClient.setQueryData(proposalsKeys.detail(proposalId), error.currentState);
}
invalidateProposalViews(queryClient, proposalId);
toast.warning(error.message);
return true;
}
/** Admin dashboard KPIs (AdminDashboard). */
export function useAdminDashboard() {
return useQuery({
@ -63,14 +101,36 @@ export function useSaveProposalWorkspace(proposalId: string) {
const queryClient = useQueryClient();
return useMutation({
mutationFn: async ({ refinedScope, lineItems }: SaveWorkspaceVariables) => {
await adminApi.updateProposal(proposalId, { refinedScope });
await lineItemsApi.bulkUpdate(proposalId, lineItems);
// Guarded pair: the PUT rotates the proposal token, so the bulk replace
// must send the PUT response's token, not the one the save started with.
const updated = await adminApi.updateProposal(proposalId, {
refinedScope,
proposalVersion: cachedProposalVersion(queryClient, proposalId),
});
await lineItemsApi.bulkUpdate(proposalId, lineItems, updated.rowVersion);
// The bulk replace rotates the token AGAIN but returns only line items —
// refetch the detail so a chained guarded action (approve-after-save)
// holds the current token instead of racing the invalidation refetch.
// Both writes are already committed here: if only this trailing GET
// fails, the save must still report success, with invalidation
// recovering the token instead (CONC-L4).
try {
return await proposalsApi.getById(proposalId);
} catch {
return null;
}
},
onSuccess: () => {
onSuccess: (fresh) => {
if (fresh) {
queryClient.setQueryData(proposalsKeys.detail(proposalId), fresh);
} else {
queryClient.removeQueries({ queryKey: proposalsKeys.detail(proposalId) });
}
invalidateProposalViews(queryClient, proposalId);
toast.success('Changes saved');
},
onError: (error: Error) => {
if (handleConcurrencyConflict(queryClient, proposalId, error)) return;
toast.error(`Save failed: ${error.message}`);
},
});
@ -79,12 +139,15 @@ export function useSaveProposalWorkspace(proposalId: string) {
export function useApproveProposal(proposalId: string) {
const queryClient = useQueryClient();
return useMutation({
mutationFn: () => adminApi.approveProposal(proposalId),
onSuccess: () => {
mutationFn: () =>
adminApi.approveProposal(proposalId, cachedProposalVersion(queryClient, proposalId)),
onSuccess: (proposal) => {
queryClient.setQueryData(proposalsKeys.detail(proposal.id), proposal);
invalidateProposalViews(queryClient, proposalId);
toast.success('Proposal approved');
},
onError: (error: Error) => {
if (handleConcurrencyConflict(queryClient, proposalId, error)) return;
toast.error(`Approval failed: ${error.message}`);
},
});
@ -93,14 +156,17 @@ export function useApproveProposal(proposalId: string) {
export function useSendProposal(proposalId: string) {
const queryClient = useQueryClient();
return useMutation({
mutationFn: () => adminApi.sendProposal(proposalId),
onSuccess: () => {
mutationFn: () =>
adminApi.sendProposal(proposalId, cachedProposalVersion(queryClient, proposalId)),
onSuccess: (proposal) => {
queryClient.setQueryData(proposalsKeys.detail(proposal.id), proposal);
invalidateProposalViews(queryClient, proposalId);
toast.success('Proposal marked as sent');
},
// Fix: WEB-H5 — mutation must surface failures to the user (relocated
// from AdminWorkspace during the domain-layer refactor)
onError: (error: Error) => {
if (handleConcurrencyConflict(queryClient, proposalId, error)) return;
toast.error(`Send failed: ${error.message}`);
},
});
@ -109,14 +175,19 @@ export function useSendProposal(proposalId: string) {
export function useReviseProposal(proposalId: string) {
const queryClient = useQueryClient();
return useMutation({
mutationFn: () => adminApi.reviseProposal(proposalId),
onSuccess: () => {
mutationFn: () =>
adminApi.reviseProposal(proposalId, cachedProposalVersion(queryClient, proposalId)),
onSuccess: (revision) => {
// The response is the NEW revision (different id) — seed its detail
// cache; the invalidation below refreshes the original's views.
queryClient.setQueryData(proposalsKeys.detail(revision.id), revision);
invalidateProposalViews(queryClient, proposalId);
toast.success('Revision created');
},
// Fix: WEB-H6 — mutation must surface failures to the user (relocated
// from AdminWorkspace during the domain-layer refactor)
onError: (error: Error) => {
if (handleConcurrencyConflict(queryClient, proposalId, error)) return;
toast.error(`Revision failed: ${error.message}`);
},
});
@ -125,12 +196,15 @@ export function useReviseProposal(proposalId: string) {
export function useReturnToReview(proposalId: string) {
const queryClient = useQueryClient();
return useMutation({
mutationFn: () => adminApi.returnToReview(proposalId),
onSuccess: () => {
mutationFn: () =>
adminApi.returnToReview(proposalId, cachedProposalVersion(queryClient, proposalId)),
onSuccess: (proposal) => {
queryClient.setQueryData(proposalsKeys.detail(proposal.id), proposal);
invalidateProposalViews(queryClient, proposalId);
toast.success('Proposal returned to review');
},
onError: (error: Error) => {
if (handleConcurrencyConflict(queryClient, proposalId, error)) return;
toast.error(`Return to review failed: ${error.message}`);
},
});

View file

@ -13,8 +13,18 @@ export const lineItemsApi = {
return res.data;
},
bulkUpdate: async (proposalId: string, lineItems: UpdateLineItemEntry[]): Promise<LineItem[]> => {
const res = await apiClient.put(`/proposals/${proposalId}/line-items`, { lineItems });
/** Guarded by the PROPOSAL token: the wholesale replace conflicts with any
* concurrent change to the aggregate, so the server requires the proposal's
* current rowVersion. Create/delete stay unguarded (single-item ops). */
bulkUpdate: async (
proposalId: string,
lineItems: UpdateLineItemEntry[],
proposalVersion?: string,
): Promise<LineItem[]> => {
const res = await apiClient.put(`/proposals/${proposalId}/line-items`, {
lineItems,
proposalVersion,
});
return res.data;
},

View file

@ -7,11 +7,13 @@
* - Response interceptor clears the stored session and redirects on 401
* - Response interceptor returns friendly error message on 403
* - Response interceptor returns friendly error message on 404
* - Response interceptor throws ConflictError (with currentState) on 409
* - Response interceptor extracts server error detail from response body
* - Response interceptor handles network errors (no response)
*/
import { describe, it, expect, vi, beforeAll, beforeEach } from 'vitest';
import type { InternalAxiosRequestConfig } from 'axios';
import { ConflictError } from '../errors';
// vi.hoisted returns values accessible in both the hoisted mock scope and test scope.
const { interceptors } = vi.hoisted(() => {
@ -209,6 +211,113 @@ describe('API client interceptors', () => {
await expect(promise).rejects.toThrow('The requested resource was not found.');
});
it('409 with the guarded conflict envelope rejects with ConflictError carrying currentState', async () => {
// Must be schema-valid: the interceptor validates the envelope with
// proposalConcurrencyConflictSchema before trusting currentState.
const currentState = {
id: 'p-1',
proposalNumber: 'P-2026-001',
workOrderNumber: 'WO-1',
poNumber: null,
customerName: 'Cust',
customerAddress: 'Addr',
scopeOfWork: 'Scope',
refinedScope: null,
serviceCategory: 'HVAC',
priority: 'Standard',
status: 'Approved',
totalBidAmount: 100,
vendorTotalCost: null,
notes: '',
submittedById: 'u-1',
submittedByName: 'Submitter',
submittedAt: '2026-07-01T00:00:00Z',
assignedAdminId: null,
approvedById: null,
approvedAt: null,
sentAt: null,
currentRevision: 1,
parentProposalId: null,
createdAt: '2026-07-01T00:00:00Z',
updatedAt: '2026-07-01T00:00:00Z',
rowVersion: 'AAAAAAAAAAM=',
};
const error = {
response: {
status: 409,
data: { message: 'Proposal was modified by another user.', currentState },
},
request: {},
};
const promise = interceptors.responseRejected(error);
await expect(promise).rejects.toMatchObject({
name: 'ConflictError',
message: 'Proposal was modified by another user.',
currentState,
});
await expect(promise).rejects.toBeInstanceOf(ConflictError);
});
it('409 with a malformed currentState degrades to a state-less ConflictError', async () => {
const error = {
response: {
status: 409,
data: {
message: 'Proposal was modified by another user.',
currentState: { id: 'p-1', rowVersion: 12345, unexpected: 'shape' },
},
},
request: {},
};
const promise = interceptors.responseRejected(error);
await expect(promise).rejects.toMatchObject({
name: 'ConflictError',
message: 'Proposal was modified by another user.',
currentState: null,
});
});
it('409 with the unguarded fallback envelope rejects with ConflictError and null currentState', async () => {
const error = {
response: {
status: 409,
data: {
status: 'Conflict',
message: 'The record was modified by another user. Refresh and retry.',
code: 409,
},
},
request: {},
};
const promise = interceptors.responseRejected(error);
await expect(promise).rejects.toBeInstanceOf(ConflictError);
await expect(promise).rejects.toMatchObject({
message: 'The record was modified by another user. Refresh and retry.',
currentState: null,
});
});
it('409 with an empty body falls back to a generic conflict message', async () => {
const error = {
response: { status: 409, data: {} },
request: {},
};
const promise = interceptors.responseRejected(error);
await expect(promise).rejects.toBeInstanceOf(ConflictError);
await expect(promise).rejects.toMatchObject({
message: 'This record was modified by another user. Refresh and retry.',
currentState: null,
});
});
it('extracts detail message from server error response', async () => {
const error = {
response: {

View file

@ -1,6 +1,8 @@
import axios from 'axios';
import { API_URL, STORAGE_KEY_TOKEN } from '../../constants';
import { AUTH_SESSION_CLEARED_EVENT, clearAuth } from '../auth/authStorage';
import { proposalConcurrencyConflictSchema } from '@proposal-system/api-contracts/schemas';
import { ConflictError } from './errors';
const apiClient = axios.create({
baseURL: API_URL,
@ -51,6 +53,23 @@ apiClient.interceptors.response.use(
return Promise.reject(new Error('The requested resource was not found.'));
}
if (status === 409) {
// Optimistic-concurrency conflict (SHOC ADR 0004): the guarded path
// carries { message, currentState }; the unguarded fallback carries
// { status, message, code } with no state. Validate the envelope before
// trusting it — a malformed currentState must never seed typed caches,
// so parse failures degrade to a state-less conflict (refetch recovers).
const parsed = proposalConcurrencyConflictSchema.safeParse(data);
return Promise.reject(
new ConflictError(
parsed.success
? parsed.data.message
: data?.message || 'This record was modified by another user. Refresh and retry.',
parsed.success ? parsed.data.currentState : null,
),
);
}
const message = data?.detail || data?.title || data?.message || 'An error occurred';
return Promise.reject(new Error(message));
}

19
web/src/lib/api/errors.ts Normal file
View file

@ -0,0 +1,19 @@
// Typed transport errors thrown by the response interceptor in ./client.
import type { ConcurrencyConflict, ProposalDetail } from '@proposal-system/api-contracts';
/**
* HTTP 409 — optimistic-concurrency conflict (SHOC ADR 0004 envelope).
* Carries the server's reloaded `currentState` so callers can refresh their
* caches without a round trip. The proposal aggregate is the only guarded
* resource, so the state is typed to ProposalDetail; the unguarded-race
* fallback envelope has no state (`currentState` is null).
*/
export class ConflictError extends Error implements ConcurrencyConflict<ProposalDetail> {
readonly currentState: ProposalDetail | null;
constructor(message: string, currentState: ProposalDetail | null = null) {
super(message);
this.name = 'ConflictError';
this.currentState = currentState;
}
}