3c56c0926a
DEBT-026 (high impact): ProposeForReviewHandler + POST /approvals/{id}/propose
wires ApprovalWorkflowPolicy.CanProposeForReview, which previously had no
Handler/Endpoint calling it. Before this, a proposal created via POST
/approvals could never reach Approved/Active through the running application
- the maker-checker gate was not completable end-to-end via HTTP.
DEBT-025 (medium impact): GetApprovalByIdEndpoint (GET /approvals/{id}) +
ApprovalWorkflowSql.GetEvidenceForProposalAsync make evidence attached during
approval (PBO/DSR/OOS artifact links) readable via HTTP instead of only by
querying model_operations.approval_evidence directly.
Both discovered while resolving DEBT-017 earlier the same session. 4 new
tests added. dotnet build -c Release clean. Not verified against a live
database (no SSH tunnel open in this environment) - see
TECH_DEBT_REGISTER.md and WBS_PROGRESS_TRACKER.csv AEG-VS-26-01 for the
honest verification status; do not mark COMPLETED until a real Postgres
run passes.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
304 lines
14 KiB
C#
304 lines
14 KiB
C#
namespace KArtSell.Integration.Tests.ApprovalWorkflow;
|
|
|
|
using System;
|
|
using System.Collections.Generic;
|
|
using System.Threading;
|
|
using System.Threading.Tasks;
|
|
using Dapper;
|
|
using KArtSell.BuildingBlocks.Time;
|
|
using KArtSell.Modules.ModelOperations.Domain.ApprovalWorkflow;
|
|
using KArtSell.Modules.ModelOperations.Features.ApprovalWorkflow;
|
|
using Npgsql;
|
|
using Xunit;
|
|
|
|
/// <summary>
|
|
/// Integration coverage for the CANONICAL maker-checker approval slice
|
|
/// (<c>KArtSell.Modules.ModelOperations.Features.ApprovalWorkflow</c> — the one wired into
|
|
/// <c>Program.cs</c> and reachable over HTTP). Exercises Handler + Sql + a real PostgreSQL
|
|
/// database, per CLAUDE.md's Vertical Slice testing rules (no mocks for this layer).
|
|
///
|
|
/// Replaces the old <c>KArtSell.Modules.ModelOperations.ApprovalWorkflow</c> namespace's test
|
|
/// coverage, deleted as part of resolving TECH_DEBT_REGISTER.md DEBT-017: that implementation
|
|
/// was dead code (all 4 endpoints <c>[DontRegister]</c>'d) even though it had 20/20 passing
|
|
/// tests, while this one — the implementation actually reachable at runtime — had none.
|
|
///
|
|
/// Requires KARTSELL_POSTGRES (or appsettings.Development.json ConnectionStrings:Postgres) to
|
|
/// point at a reachable PostgreSQL instance with migrations 0035/0036 applied. See
|
|
/// TestDatabaseConnection.GetConnectionString().
|
|
/// </summary>
|
|
[Collection("Database")]
|
|
public class ApprovalWorkflowTests : IAsyncLifetime
|
|
{
|
|
private readonly string _connectionString;
|
|
private readonly ApprovalWorkflowSql _sql;
|
|
private readonly IClock _clock = new SystemClock();
|
|
|
|
public ApprovalWorkflowTests()
|
|
{
|
|
_connectionString = TestDatabaseConnection.GetConnectionString();
|
|
_sql = new ApprovalWorkflowSql(_connectionString);
|
|
}
|
|
|
|
public async Task InitializeAsync()
|
|
{
|
|
await using var conn = new NpgsqlConnection(_connectionString);
|
|
await conn.OpenAsync();
|
|
await conn.ExecuteAsync("SELECT 1");
|
|
}
|
|
|
|
public Task DisposeAsync() => Task.CompletedTask;
|
|
|
|
private async Task<Guid> SeedModelAsync()
|
|
{
|
|
var modelId = Guid.NewGuid();
|
|
await using var conn = new NpgsqlConnection(_connectionString);
|
|
await conn.OpenAsync();
|
|
await conn.ExecuteAsync(
|
|
"INSERT INTO model_operations.models (id, ticker, correlation_id) VALUES (@Id, @Ticker, @CorrelationId)",
|
|
new { Id = modelId, Ticker = "TEST", CorrelationId = Guid.NewGuid() });
|
|
return modelId;
|
|
}
|
|
|
|
[Fact]
|
|
public async Task CreateProposal_WithMakerRole_InsertsDraftProposal_AndEffectiveAtRoundTrips()
|
|
{
|
|
// Arrange: DateOnly binding is the specific regression this test guards against — the
|
|
// previously-deleted ApprovalSql.cs could not INSERT a DateOnly parameter through Dapper
|
|
// at all (fixed in commit 2ccf74c). Use a date with no "today" coincidence risk.
|
|
var modelId = await SeedModelAsync();
|
|
var handler = new CreateApprovalProposalHandler(_sql, _clock);
|
|
var effectiveAt = new DateOnly(2028, 2, 29); // leap day: exercises DateOnly edge case too
|
|
|
|
// Act
|
|
var proposalId = await handler.Handle(
|
|
"maker@company.com", "Maker", modelId, effectiveAt, "Model passed OOS testing", Guid.NewGuid());
|
|
|
|
// Assert
|
|
var proposal = await _sql.GetProposalAsync(proposalId);
|
|
Assert.NotNull(proposal);
|
|
Assert.Equal(ApprovalStatus.Draft, proposal!.Status);
|
|
Assert.Equal(modelId, proposal.ModelId);
|
|
Assert.Equal("maker@company.com", proposal.CreatedBy);
|
|
Assert.Equal(effectiveAt, proposal.EffectiveAt);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task CreateProposal_WithNonMakerRole_ThrowsUnauthorized()
|
|
{
|
|
// Arrange
|
|
var modelId = await SeedModelAsync();
|
|
var handler = new CreateApprovalProposalHandler(_sql, _clock);
|
|
|
|
// Act & Assert
|
|
await Assert.ThrowsAsync<UnauthorizedAccessException>(() =>
|
|
handler.Handle("viewer@company.com", "Viewer", modelId, DateOnly.FromDateTime(DateTime.UtcNow), "no role", Guid.NewGuid()));
|
|
}
|
|
|
|
[Fact]
|
|
public async Task ProposeForReview_ByCreatingMaker_TransitionsDraftToProposed()
|
|
{
|
|
// Arrange: this handler didn't exist until DEBT-026 was fixed — before that, a proposal
|
|
// created via POST /approvals could never reach Proposed/Approved/Active through the
|
|
// running application at all (only test code bypassing the handler via
|
|
// _sql.UpdateProposalStatusAsync directly could move it, as the other tests in this file
|
|
// still do to set up their own preconditions).
|
|
var modelId = await SeedModelAsync();
|
|
var createHandler = new CreateApprovalProposalHandler(_sql, _clock);
|
|
var proposalId = await createHandler.Handle(
|
|
"maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)),
|
|
"Ready to send for review", Guid.NewGuid());
|
|
|
|
var proposeHandler = new ProposeForReviewHandler(_sql, _clock);
|
|
|
|
// Act
|
|
await proposeHandler.Handle(proposalId, "maker@company.com", Guid.NewGuid());
|
|
|
|
// Assert
|
|
var proposal = await _sql.GetProposalAsync(proposalId);
|
|
Assert.NotNull(proposal);
|
|
Assert.Equal(ApprovalStatus.Proposed, proposal!.Status);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task ProposeForReview_ByDifferentUserThanCreator_ThrowsUnauthorized()
|
|
{
|
|
// Arrange
|
|
var modelId = await SeedModelAsync();
|
|
var createHandler = new CreateApprovalProposalHandler(_sql, _clock);
|
|
var proposalId = await createHandler.Handle(
|
|
"maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)),
|
|
"Only the creator may propose it", Guid.NewGuid());
|
|
|
|
var proposeHandler = new ProposeForReviewHandler(_sql, _clock);
|
|
|
|
// Act & Assert
|
|
await Assert.ThrowsAsync<UnauthorizedAccessException>(() =>
|
|
proposeHandler.Handle(proposalId, "someone-else@company.com", Guid.NewGuid()));
|
|
}
|
|
|
|
[Fact]
|
|
public async Task GetEvidenceForProposalAsync_ReturnsEvidenceAttachedDuringApproval()
|
|
{
|
|
// Arrange: this query didn't exist until DEBT-025 was fixed — evidence attached during
|
|
// approval (the whole point of this slice per CLAUDE.md's "Activation gating") was
|
|
// otherwise only readable by querying model_operations.approval_evidence directly.
|
|
var modelId = await SeedModelAsync();
|
|
var createHandler = new CreateApprovalProposalHandler(_sql, _clock);
|
|
var proposalId = await createHandler.Handle(
|
|
"maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)),
|
|
"Needs evidence readback", Guid.NewGuid());
|
|
await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Proposed);
|
|
|
|
var approveHandler = new ApproveApprovalHandler(_sql, _clock);
|
|
var evidence = new List<(string Type, string Url, string? Comment)>
|
|
{
|
|
("PBO_SCORE", "s3://evidence/pbo-0.95.json", "Verified"),
|
|
};
|
|
await approveHandler.Handle(proposalId, "checker@company.com", "Checker", "Approved", evidence, Guid.NewGuid());
|
|
|
|
// Act
|
|
var retrieved = await _sql.GetEvidenceForProposalAsync(proposalId);
|
|
|
|
// Assert
|
|
Assert.Single(retrieved);
|
|
Assert.Equal("PBO_SCORE", retrieved[0].EvidenceType);
|
|
Assert.Equal("s3://evidence/pbo-0.95.json", retrieved[0].EvidenceUrl);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task Approve_WithDifferentChecker_TransitionsToApproved_AndAttachesEvidence()
|
|
{
|
|
// Arrange
|
|
var modelId = await SeedModelAsync();
|
|
var createHandler = new CreateApprovalProposalHandler(_sql, _clock);
|
|
var proposalId = await createHandler.Handle(
|
|
"maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)),
|
|
"Model passed OOS testing; PBO score 0.95", Guid.NewGuid());
|
|
await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Proposed);
|
|
|
|
var approveHandler = new ApproveApprovalHandler(_sql, _clock);
|
|
var evidence = new List<(string Type, string Url, string? Comment)>
|
|
{
|
|
("PBO_SCORE", "s3://evidence/pbo-0.95.json", "Verified"),
|
|
("OOS_RETURN", "s3://evidence/oos-returns.csv", "Acceptable"),
|
|
};
|
|
|
|
// Act
|
|
await approveHandler.Handle(proposalId, "checker@company.com", "Checker", "PBO verified, OOS metrics acceptable", evidence, Guid.NewGuid());
|
|
|
|
// Assert
|
|
var proposal = await _sql.GetProposalAsync(proposalId);
|
|
Assert.NotNull(proposal);
|
|
Assert.Equal(ApprovalStatus.Approved, proposal!.Status);
|
|
Assert.Equal("checker@company.com", proposal.ApprovedBy);
|
|
Assert.NotNull(proposal.ApprovedAt);
|
|
|
|
await using var conn = new NpgsqlConnection(_connectionString);
|
|
await conn.OpenAsync();
|
|
var evidenceCount = await conn.QuerySingleAsync<int>(
|
|
"SELECT COUNT(*) FROM model_operations.approval_evidence WHERE approval_proposal_id = @ProposalId",
|
|
new { ProposalId = proposalId });
|
|
Assert.Equal(2, evidenceCount);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task Approve_BySameMakerAsChecker_ThrowsUnauthorized_SeparationOfDuties()
|
|
{
|
|
// Arrange
|
|
var modelId = await SeedModelAsync();
|
|
var createHandler = new CreateApprovalProposalHandler(_sql, _clock);
|
|
var proposalId = await createHandler.Handle(
|
|
"maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)),
|
|
"Self-approval attempt", Guid.NewGuid());
|
|
await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Proposed);
|
|
|
|
var approveHandler = new ApproveApprovalHandler(_sql, _clock);
|
|
|
|
// Act & Assert: same user, even with Checker role, cannot approve their own proposal
|
|
await Assert.ThrowsAsync<UnauthorizedAccessException>(() =>
|
|
approveHandler.Handle(proposalId, "maker@company.com", "Checker", "self-approved", [], Guid.NewGuid()));
|
|
}
|
|
|
|
[Fact]
|
|
public async Task Approve_WithNonCheckerRole_ThrowsUnauthorized()
|
|
{
|
|
// Arrange
|
|
var modelId = await SeedModelAsync();
|
|
var createHandler = new CreateApprovalProposalHandler(_sql, _clock);
|
|
var proposalId = await createHandler.Handle(
|
|
"maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)),
|
|
"Needs a Checker role, not just a different user", Guid.NewGuid());
|
|
await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Proposed);
|
|
|
|
var approveHandler = new ApproveApprovalHandler(_sql, _clock);
|
|
|
|
// Act & Assert: different user, but wrong role
|
|
await Assert.ThrowsAsync<UnauthorizedAccessException>(() =>
|
|
approveHandler.Handle(proposalId, "other-maker@company.com", "Maker", "wrong role", [], Guid.NewGuid()));
|
|
}
|
|
|
|
[Fact]
|
|
public async Task Activate_BySreAfterApproval_TransitionsToActive()
|
|
{
|
|
// Arrange
|
|
var modelId = await SeedModelAsync();
|
|
var createHandler = new CreateApprovalProposalHandler(_sql, _clock);
|
|
var proposalId = await createHandler.Handle(
|
|
"maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)),
|
|
"Ready for activation", Guid.NewGuid());
|
|
await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Proposed);
|
|
await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Approved, "checker@company.com", "Approved");
|
|
|
|
var activateHandler = new ActivateModelHandler(_sql, _clock);
|
|
|
|
// Act
|
|
await activateHandler.Handle(proposalId, "sre@company.com", "SRE", Guid.NewGuid());
|
|
|
|
// Assert
|
|
var proposal = await _sql.GetProposalAsync(proposalId);
|
|
Assert.NotNull(proposal);
|
|
Assert.Equal(ApprovalStatus.Active, proposal!.Status);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task Activate_WithNonSreRole_ThrowsUnauthorized()
|
|
{
|
|
// Arrange
|
|
var modelId = await SeedModelAsync();
|
|
var createHandler = new CreateApprovalProposalHandler(_sql, _clock);
|
|
var proposalId = await createHandler.Handle(
|
|
"maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)),
|
|
"Should not activate without SRE", Guid.NewGuid());
|
|
await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Proposed);
|
|
await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Approved, "checker@company.com", "Approved");
|
|
|
|
var activateHandler = new ActivateModelHandler(_sql, _clock);
|
|
|
|
// Act & Assert
|
|
await Assert.ThrowsAsync<UnauthorizedAccessException>(() =>
|
|
activateHandler.Handle(proposalId, "maker@company.com", "Maker", Guid.NewGuid()));
|
|
}
|
|
|
|
[Fact]
|
|
public async Task ListProposalsAsync_FiltersByStatusAndModelId()
|
|
{
|
|
// Arrange
|
|
var modelId = await SeedModelAsync();
|
|
var createHandler = new CreateApprovalProposalHandler(_sql, _clock);
|
|
var draftId = await createHandler.Handle(
|
|
"maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)),
|
|
"Stays in Draft", Guid.NewGuid());
|
|
var proposedId = await createHandler.Handle(
|
|
"maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)),
|
|
"Moves to Proposed", Guid.NewGuid());
|
|
await _sql.UpdateProposalStatusAsync(proposedId, ApprovalStatus.Proposed);
|
|
|
|
// Act
|
|
var proposedOnly = await _sql.ListProposalsAsync(ApprovalStatus.Proposed, modelId);
|
|
|
|
// Assert
|
|
Assert.Contains(proposedOnly, p => p.Id == proposedId);
|
|
Assert.DoesNotContain(proposedOnly, p => p.Id == draftId);
|
|
}
|
|
}
|