From de0088742f369f8678d5831f8313a73bdd014d19 Mon Sep 17 00:00:00 2001 From: pqhuy1987 Date: Wed, 13 May 2026 09:41:14 +0700 Subject: [PATCH] =?UTF-8?q?[CLAUDE]=20PurchaseEvaluation:=20Chunk=20A=20?= =?UTF-8?q?=E2=80=94=20BE=20guard=20target=20TraLai/TuChoi=20B=E1=BA=AET?= =?UTF-8?q?=20BU=E1=BB=98C=20decision=3DReject=20+=203=20regression=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Defense-in-depth chặn FE inconsistency (gotcha #45 — Session 21 turn 3). Bug pattern: button "← Trả lại" trong PeWorkflowPanel.tsx gửi decision=Approve khi target=TraLai do `isReject` local var thiếu nhánh TraLai → BE skip Reject branch → enter APPROVE STEP → ApproveV2Async UPSERT opinion = "đã duyệt" + advance Cấp. User UAT thấy: "Trả về nhưng hệ thống vẫn duyệt". BE guard: - Service `TransitionAsync` thêm early check sau set isAdmin/isSystem - targetPhase ∈ {TraLai, TuChoi} && decision != Reject → throw ConflictException - Boundary protection cho mọi caller tương lai (API client / mobile / cron) Tests (Infra suite +3): - TransitionAsync_TargetTraLai_WithApproveDecision_Throws_AndDoesNotMutateState - TransitionAsync_TargetTuChoi_WithApproveDecision_Throws_AndDoesNotMutateState - TransitionAsync_TargetTraLai_WithRejectDecision_SetsPhaseTraLai (happy path) + NoOpNotificationService stub reusable cho future PE service tests Verify: - dotnet test SolutionErp.slnx → 84 PASS (58 Domain + 26 Infra = +3 from 81 baseline) - Build pass (0 err, 2 warn CS8602 pre-existing DocxRenderer) Pending Chunk B: FE fix PeWorkflowPanel.tsx isReject + dialog isSendBack mirror 2 app (fe-admin + fe-user) — sync với BE guard rule. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../PurchaseEvaluationWorkflowService.cs | 20 ++ ...haseEvaluationWorkflowServiceGuardTests.cs | 176 ++++++++++++++++++ 2 files changed, 196 insertions(+) create mode 100644 tests/SolutionErp.Infrastructure.Tests/Services/PurchaseEvaluationWorkflowServiceGuardTests.cs diff --git a/src/Backend/SolutionErp.Infrastructure/Services/PurchaseEvaluationWorkflowService.cs b/src/Backend/SolutionErp.Infrastructure/Services/PurchaseEvaluationWorkflowService.cs index 6ba9a5c..e03c826 100644 --- a/src/Backend/SolutionErp.Infrastructure/Services/PurchaseEvaluationWorkflowService.cs +++ b/src/Backend/SolutionErp.Infrastructure/Services/PurchaseEvaluationWorkflowService.cs @@ -47,6 +47,26 @@ public class PurchaseEvaluationWorkflowService( var isAdmin = actorRoles.Contains(AppRoles.Admin); var isSystem = actorUserId is null && decision == ApprovalDecision.AutoApprove; + // ===== GUARD: targetPhase TraLai/TuChoi BẮT BUỘC decision=Reject ===== + // Defense-in-depth chặn FE inconsistency (gotcha #45 — Session 21 turn 3): + // Bug: button "← Trả lại" trong PeWorkflowPanel.tsx gửi decision=Approve + // khi target=TraLai do `isReject` local var thiếu nhánh TraLai. BE nhận + // payload sẽ skip Reject branch → enter APPROVE STEP → ApproveV2Async + // UPSERT opinion = "đã duyệt" + advance Cấp. User UAT thấy: "Trả về + // nhưng hệ thống vẫn duyệt". + // FE fix song song trong fe-admin + fe-user (rule §3.9 mirror 2 app). + // Guard này KHÔNG xoá khi FE fix — boundary protection cho mọi caller + // tương lai (API client / mobile app / cron retry). + if ((targetPhase == PurchaseEvaluationPhase.TraLai + || targetPhase == PurchaseEvaluationPhase.TuChoi) + && decision != ApprovalDecision.Reject) + { + throw new ConflictException( + $"Transition tới {targetPhase} BẮT BUỘC decision=Reject (nhận {decision}). " + + "Báo lỗi caller — payload mismatch giữa target phase và decision " + + "(xem gotcha #45 + docs/workflow-contract.md)."); + } + // ===== REJECT BRANCH ===== if (decision == ApprovalDecision.Reject) { diff --git a/tests/SolutionErp.Infrastructure.Tests/Services/PurchaseEvaluationWorkflowServiceGuardTests.cs b/tests/SolutionErp.Infrastructure.Tests/Services/PurchaseEvaluationWorkflowServiceGuardTests.cs new file mode 100644 index 0000000..c98738f --- /dev/null +++ b/tests/SolutionErp.Infrastructure.Tests/Services/PurchaseEvaluationWorkflowServiceGuardTests.cs @@ -0,0 +1,176 @@ +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.DependencyInjection; +using SolutionErp.Application.Common.Exceptions; +using SolutionErp.Application.Notifications; +using SolutionErp.Domain.Common; +using SolutionErp.Domain.Contracts; // ApprovalDecision enum (shared HĐ/PE) +using SolutionErp.Domain.Identity; +using SolutionErp.Domain.Notifications; +using SolutionErp.Domain.PurchaseEvaluations; +using SolutionErp.Infrastructure.Services; +using SolutionErp.Infrastructure.Tests.Common; + +namespace SolutionErp.Infrastructure.Tests.Services; + +// Regression test for Session 21 turn 3 bug — gotcha #45: +// FE button "← Trả lại" trong PeWorkflowPanel gửi `decision: 1` (Approve) thay +// vì `2` (Reject) khi target = TraLai (98). Root: `isReject` local variable +// trong FE thiếu nhánh TraLai → payload mismatch giữa button label hiển thị +// "Trả lại" và decision gửi BE. +// +// Hiệu ứng cũ trước fix: BE TransitionAsync nhận decision=Approve → skip Reject +// branch (L51) → enter APPROVE STEP branch (L97) → ApproveV2Async UPSERT +// opinion đánh dấu "đã duyệt" cho NV đang nhấn nút → tiến qua Cấp tiếp theo. +// User UAT thấy: "Trả về nhưng hệ thống vẫn duyệt". +// +// Fix BE defense-in-depth: guard early throw ConflictException khi targetPhase +// ∈ {TraLai, TuChoi} mà decision != Reject — chặn FE inconsistency tại +// boundary BE thay vì depend FE đúng. +// +// FE fix song song trong fe-admin + fe-user PeWorkflowPanel.tsx (rule §3.9 +// mirror 2 app) — sync `isReject` + dialog `isSendBack` include TraLai. +public class PurchaseEvaluationWorkflowServiceGuardTests +{ + private static (PurchaseEvaluationWorkflowService svc, IdentityFixture fix, TestApplicationDbContext db) + CreateService() + { + var fix = new IdentityFixture(); + var db = fix.Services.GetRequiredService(); + var um = fix.Services.GetRequiredService>(); + var dt = new FixedDateTime(new DateTime(2026, 5, 12, 0, 0, 0, DateTimeKind.Utc)); + var notify = new NoOpNotificationService(); + var svc = new PurchaseEvaluationWorkflowService(db, dt, notify, um); + return (svc, fix, db); + } + + private static PurchaseEvaluation BuildPeInChoDuyet(string code = "PE-GUARD-001") + { + return new PurchaseEvaluation + { + Id = Guid.NewGuid(), + Type = PurchaseEvaluationType.DuyetNcc, + Phase = PurchaseEvaluationPhase.ChoDuyet, + MaPhieu = code, + TenGoiThau = "Test guard bug Trả lại", + ProjectId = Guid.NewGuid(), + DrafterUserId = Guid.NewGuid(), + CurrentApprovalLevelOrder = 1, + CurrentWorkflowStepIndex = 0, + }; + } + + [Fact] + public async Task TransitionAsync_TargetTraLai_WithApproveDecision_Throws_AndDoesNotMutateState() + { + // Arrange: phiếu PE ở ChoDuyet (typical intermediate state khi approver duyệt) + var (svc, fix, db) = CreateService(); + using (fix) + { + var pe = BuildPeInChoDuyet(); + db.PurchaseEvaluations.Add(pe); + await db.SaveChangesAsync(CancellationToken.None); + + // Act: simulate FE bug payload — button "← Trả lại" gửi decision=Approve + // thay vì Reject (gotcha #45 root cause). + var act = async () => await svc.TransitionAsync( + evaluation: pe, + targetPhase: PurchaseEvaluationPhase.TraLai, + actorUserId: Guid.NewGuid(), + actorRoles: new[] { AppRoles.CostControl }, + decision: ApprovalDecision.Approve, + comment: "test guard mismatch", + ct: CancellationToken.None); + + // Assert: BE chặn payload mismatch sớm + state phiếu KHÔNG đổi + await act.Should().ThrowAsync() + .WithMessage("*TraLai*Reject*"); + + pe.Phase.Should().Be(PurchaseEvaluationPhase.ChoDuyet, + "Guard chặn trước khi mutate phase"); + pe.CurrentApprovalLevelOrder.Should().Be(1, + "Guard chặn trước khi advance level pointer"); + } + } + + [Fact] + public async Task TransitionAsync_TargetTuChoi_WithApproveDecision_Throws_AndDoesNotMutateState() + { + // Tương tự TraLai — TuChoi cũng BẮT BUỘC decision=Reject. Defense + // double-cover invariant (FE chỉ bug TraLai branch nhưng guard nên cover + // luôn TuChoi cho consistency). + var (svc, fix, db) = CreateService(); + using (fix) + { + var pe = BuildPeInChoDuyet("PE-GUARD-002"); + db.PurchaseEvaluations.Add(pe); + await db.SaveChangesAsync(CancellationToken.None); + + var act = async () => await svc.TransitionAsync( + evaluation: pe, + targetPhase: PurchaseEvaluationPhase.TuChoi, + actorUserId: Guid.NewGuid(), + actorRoles: new[] { AppRoles.CostControl }, + decision: ApprovalDecision.Approve, + comment: "test guard tu choi", + ct: CancellationToken.None); + + await act.Should().ThrowAsync() + .WithMessage("*TuChoi*Reject*"); + + pe.Phase.Should().Be(PurchaseEvaluationPhase.ChoDuyet); + } + } + + [Fact] + public async Task TransitionAsync_TargetTraLai_WithRejectDecision_SetsPhaseTraLai() + { + // Happy path control test: decision=Reject + target=TraLai → BE đi vào + // Reject branch (L51), set Phase=TraLai, clear pointer. Verify fix + // KHÔNG break flow Trả lại đúng. + var (svc, fix, db) = CreateService(); + using (fix) + { + var pe = BuildPeInChoDuyet("PE-GUARD-003"); + db.PurchaseEvaluations.Add(pe); + await db.SaveChangesAsync(CancellationToken.None); + + await svc.TransitionAsync( + evaluation: pe, + targetPhase: PurchaseEvaluationPhase.TraLai, + actorUserId: Guid.NewGuid(), + actorRoles: new[] { AppRoles.CostControl }, + decision: ApprovalDecision.Reject, + comment: "trả lại sửa lại đi", + ct: CancellationToken.None); + + pe.Phase.Should().Be(PurchaseEvaluationPhase.TraLai, + "Reject branch set Phase=TraLai"); + pe.CurrentApprovalLevelOrder.Should().BeNull("Trả lại clear level pointer"); + pe.CurrentWorkflowStepIndex.Should().BeNull("Trả lại clear step pointer"); + pe.SlaDeadline.Should().BeNull("Trả lại clear SLA"); + } + } +} + +// Stub: not assert side effects of notify (out-of-scope cho guard test). +// Pattern reuse cho future PE service tests. +internal sealed class NoOpNotificationService : INotificationService +{ + public Task NotifyAsync( + Guid userId, + NotificationType type, + string title, + string? description = null, + string? href = null, + Guid? refId = null, + CancellationToken ct = default) => Task.CompletedTask; + + public Task NotifyManyAsync( + IEnumerable userIds, + NotificationType type, + string title, + string? description = null, + string? href = null, + Guid? refId = null, + CancellationToken ct = default) => Task.CompletedTask; +}