diff --git a/api/src/ProposalSystem.Api/Controllers/AdminController.cs b/api/src/ProposalSystem.Api/Controllers/AdminController.cs index 2b226fc..466fbd5 100644 --- a/api/src/ProposalSystem.Api/Controllers/AdminController.cs +++ b/api/src/ProposalSystem.Api/Controllers/AdminController.cs @@ -22,20 +22,25 @@ public class AdminController : ControllerBase [HttpGet("dashboard")] public async Task> GetDashboard(CancellationToken ct) { + // Fix: API-M7 — AsNoTracking on read-only dashboard queries var pendingCount = await _db.Proposals + .AsNoTracking() .CountAsync(p => p.Status == ProposalStatus.InReview, ct); var weekStart = DateTime.UtcNow.AddDays(-7); var approvedThisWeek = await _db.Proposals + .AsNoTracking() .CountAsync(p => p.ApprovedAt >= weekStart, ct); var approvedCount = await _db.Proposals + .AsNoTracking() .CountAsync(p => p.ApprovedAt.HasValue, ct); double avgTurnaround = 0; if (approvedCount > 0) { var recentApproved = await _db.Proposals + .AsNoTracking() .Where(p => p.ApprovedAt.HasValue) .OrderByDescending(p => p.ApprovedAt) .Take(200) @@ -44,7 +49,7 @@ public class AdminController : ControllerBase avgTurnaround = recentApproved.Average(p => (p.ApprovedAt!.Value - p.SubmittedAt).TotalHours); } - var totalProposals = await _db.Proposals.CountAsync(ct); + var totalProposals = await _db.Proposals.AsNoTracking().CountAsync(ct); return Ok(new DashboardResponse(pendingCount, approvedThisWeek, avgTurnaround, totalProposals)); } diff --git a/api/src/ProposalSystem.Api/Controllers/AuthController.cs b/api/src/ProposalSystem.Api/Controllers/AuthController.cs index 0c16e87..6f48aa4 100644 --- a/api/src/ProposalSystem.Api/Controllers/AuthController.cs +++ b/api/src/ProposalSystem.Api/Controllers/AuthController.cs @@ -20,12 +20,14 @@ public class AuthController : ControllerBase private readonly ProposalDbContext _db; private readonly IHttpClientFactory _httpClientFactory; private readonly IConfiguration _config; + private readonly ILogger _logger; - public AuthController(ProposalDbContext db, IHttpClientFactory httpClientFactory, IConfiguration config) + public AuthController(ProposalDbContext db, IHttpClientFactory httpClientFactory, IConfiguration config, ILogger logger) { _db = db; _httpClientFactory = httpClientFactory; _config = config; + _logger = logger; } [HttpPost("callback")] @@ -45,8 +47,9 @@ public class AuthController : ControllerBase var domain = _config["Auth:CognitoDomain"]; var clientId = _config["Auth:ClientId"]; + // Fix: API-M10 — return generic error to avoid leaking internal auth configuration details if (string.IsNullOrEmpty(domain) || string.IsNullOrEmpty(clientId)) - return StatusCode(500, new { message = "Auth not configured" }); + return StatusCode(500, new { message = "Authentication service unavailable" }); var tokenResponse = await ExchangeCodeAsync(domain, clientId, request.Code, request.RedirectUri, ct); if (tokenResponse == null) @@ -55,8 +58,9 @@ public class AuthController : ControllerBase var handler = new JwtSecurityTokenHandler(); var authority = _config["Auth:Authority"]; + // Fix: API-M10 — return generic error to avoid leaking internal auth configuration details if (string.IsNullOrEmpty(authority)) - return StatusCode(500, new { message = "Token validation is not configured" }); + return StatusCode(500, new { message = "Authentication service unavailable" }); var configManager = new ConfigurationManager( $"{authority}/.well-known/openid-configuration", @@ -112,7 +116,14 @@ public class AuthController : ControllerBase var changed = false; if (user.Email != email) { user.Email = email; changed = true; } if (user.DisplayName != name) { user.DisplayName = name; changed = true; } - if (user.Role != role) { user.Role = role; changed = true; } + if (user.Role != role) + { + // Fix: API-M13 — log previous role on Cognito-synced role changes + _logger.LogInformation("User {Email} role changed from {OldRole} to {NewRole} via Cognito sync", + user.Email, user.Role, role); + user.Role = role; + changed = true; + } if (changed) { user.UpdatedAt = DateTime.UtcNow; @@ -137,8 +148,9 @@ public class AuthController : ControllerBase return NotFound(); var signingKey = _config["Auth:DevSigningKey"]; + // Fix: API-M10 — return generic error to avoid leaking dev auth configuration details if (string.IsNullOrEmpty(signingKey)) - return StatusCode(500, new { message = "Dev signing key not configured" }); + return StatusCode(500, new { message = "Authentication service unavailable" }); if (string.IsNullOrWhiteSpace(request.Email)) return BadRequest(new { message = "Email is required" }); diff --git a/api/src/ProposalSystem.Api/Controllers/FilesController.cs b/api/src/ProposalSystem.Api/Controllers/FilesController.cs index 1456f7e..2aba4db 100644 --- a/api/src/ProposalSystem.Api/Controllers/FilesController.cs +++ b/api/src/ProposalSystem.Api/Controllers/FilesController.cs @@ -3,6 +3,7 @@ using System.Text.Json; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; using ProposalSystem.Application.DTOs; using ProposalSystem.Application.Interfaces; using ProposalSystem.Domain.Entities; @@ -20,19 +21,22 @@ public class FilesController : ControllerBase private readonly IJobPublisher _jobPublisher; private readonly IAuditService _audit; private readonly IConfiguration _config; + private readonly ILogger _logger; public FilesController( ProposalDbContext db, IS3Service s3, IJobPublisher jobPublisher, IAuditService audit, - IConfiguration config) + IConfiguration config, + ILogger logger) { _db = db; _s3 = s3; _jobPublisher = jobPublisher; _audit = audit; _config = config; + _logger = logger; } // Fix: API-M6 — max file size for presigned upload URLs (25 MB) @@ -160,7 +164,12 @@ public class FilesController : ControllerBase await process.WaitForExitAsync(ct); if (process.ExitCode != 0) - return StatusCode(500, new { message = "PDF generation failed", detail = stderr }); + { + // Fix: API-M9 — log stderr instead of returning it to the client + _logger.LogError("Dev PDF generation failed for proposal {ProposalId} (exit code {ExitCode}): {Stderr}", + proposalId, process.ExitCode, stderr); + return StatusCode(500, new { message = "PDF generation failed" }); + } var result = JsonSerializer.Deserialize(stdout.Trim()); var filePath = result.GetProperty("path").GetString()!; @@ -197,7 +206,9 @@ public class FilesController : ControllerBase Guid proposalId, CancellationToken ct) { + // Fix: API-M7 — AsNoTracking on read-only query var pdfs = await _db.GeneratedPdfs + .AsNoTracking() .Where(p => p.ProposalId == proposalId) .OrderByDescending(p => p.Revision) .Select(p => new PdfVersionResponse(p.Revision, p.GeneratedAt)) @@ -244,7 +255,9 @@ public class FilesController : ControllerBase Guid proposalId, CancellationToken ct) { + // Fix: API-M7 — AsNoTracking on read-only query var entities = await _db.VendorProposals + .AsNoTracking() .Where(v => v.ProposalId == proposalId) .OrderByDescending(v => v.UploadedAt) .ToListAsync(ct); diff --git a/api/src/ProposalSystem.Api/Controllers/GeneratedPdfsController.cs b/api/src/ProposalSystem.Api/Controllers/GeneratedPdfsController.cs index 96f135b..cca658d 100644 --- a/api/src/ProposalSystem.Api/Controllers/GeneratedPdfsController.cs +++ b/api/src/ProposalSystem.Api/Controllers/GeneratedPdfsController.cs @@ -1,6 +1,7 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; +using ProposalSystem.Application.DTOs; using ProposalSystem.Application.Interfaces; using ProposalSystem.Domain.Entities; using ProposalSystem.Infrastructure.Data; @@ -59,5 +60,3 @@ public class GeneratedPdfsController : ControllerBase return proposal.SubmittedById == _currentUser.UserId; } } - -public record CreateGeneratedPdfRequest(Guid ProposalId, string S3Key); diff --git a/api/src/ProposalSystem.Api/Controllers/UsersController.cs b/api/src/ProposalSystem.Api/Controllers/UsersController.cs index 042f092..a2a5b91 100644 --- a/api/src/ProposalSystem.Api/Controllers/UsersController.cs +++ b/api/src/ProposalSystem.Api/Controllers/UsersController.cs @@ -27,7 +27,9 @@ public class UsersController : ControllerBase [HttpGet("me")] public async Task> GetMe(CancellationToken ct) { + // Fix: API-M7 — AsNoTracking on read-only query var user = await _db.Users + .AsNoTracking() .FirstOrDefaultAsync(u => u.Id == _currentUser.UserId, ct); if (user == null) return NotFound(); @@ -39,7 +41,9 @@ public class UsersController : ControllerBase [Authorize(Roles = "sysadmins")] public async Task>> GetAll(CancellationToken ct) { + // Fix: API-M7 — AsNoTracking on read-only query var users = await _db.Users + .AsNoTracking() .OrderBy(u => u.DisplayName) .Select(u => new UserResponse(u.Id, u.Email, u.DisplayName, u.Role, u.IsActive, u.CreatedAt)) .ToListAsync(ct); @@ -54,11 +58,18 @@ public class UsersController : ControllerBase var user = await _db.Users.FindAsync(new object[] { id }, ct); if (user == null) return NotFound(); + // Fix: API-M13 — log previous role alongside new role + var previousRole = user.Role; user.Role = request.Role; user.UpdatedAt = DateTime.UtcNow; await _db.SaveChangesAsync(ct); - await _audit.LogAsync(AuditAction.UpdateRole, null, $"User {user.Email} role changed to {request.Role}", ct); + var auditDetails = System.Text.Json.JsonSerializer.Serialize(new + { + email = user.Email, + role = new { old = previousRole.ToString(), @new = request.Role.ToString() } + }); + await _audit.LogAsync(AuditAction.UpdateRole, null, auditDetails, ct); return NoContent(); } diff --git a/api/src/ProposalSystem.Api/Controllers/VendorProposalsController.cs b/api/src/ProposalSystem.Api/Controllers/VendorProposalsController.cs index 9b3b455..619ac5f 100644 --- a/api/src/ProposalSystem.Api/Controllers/VendorProposalsController.cs +++ b/api/src/ProposalSystem.Api/Controllers/VendorProposalsController.cs @@ -1,6 +1,7 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; +using ProposalSystem.Application.DTOs; using ProposalSystem.Domain.Entities; using ProposalSystem.Infrastructure.Data; @@ -63,12 +64,3 @@ public class VendorProposalsController : ControllerBase return NoContent(); } } - -public record UpdateVendorProposalRequest( - string? VendorName, - string? ExtractedData, - decimal? TotalVendorCost, - string? ProcessingStatus -); - -public record UpdateStatusRequest(string ProcessingStatus); diff --git a/api/src/ProposalSystem.Api/Program.cs b/api/src/ProposalSystem.Api/Program.cs index f8999e3..535099f 100644 --- a/api/src/ProposalSystem.Api/Program.cs +++ b/api/src/ProposalSystem.Api/Program.cs @@ -107,6 +107,7 @@ else if (devMode) } else { + // Fix: API-M2 — fail loud on missing auth config; app must not silently run unauthenticated throw new InvalidOperationException( "Authentication is not configured. Set Auth:Authority for Cognito or Auth:DevMode=true (Development only)."); } diff --git a/api/src/ProposalSystem.Application/DTOs/FileDtos.cs b/api/src/ProposalSystem.Application/DTOs/FileDtos.cs index 89ff5df..5bd6ded 100644 --- a/api/src/ProposalSystem.Application/DTOs/FileDtos.cs +++ b/api/src/ProposalSystem.Application/DTOs/FileDtos.cs @@ -25,3 +25,15 @@ public record VendorProposalResponse( string ProcessingStatus, object? ExtractedData ); + +// Fix: API-M5 — moved request DTOs here from controllers so validators can reference them +public record UpdateVendorProposalRequest( + string? VendorName, + string? ExtractedData, + decimal? TotalVendorCost, + string? ProcessingStatus +); + +public record UpdateStatusRequest(string ProcessingStatus); + +public record CreateGeneratedPdfRequest(Guid ProposalId, string S3Key); diff --git a/api/src/ProposalSystem.Application/Validators/CreateGeneratedPdfValidator.cs b/api/src/ProposalSystem.Application/Validators/CreateGeneratedPdfValidator.cs new file mode 100644 index 0000000..7c5ecfa --- /dev/null +++ b/api/src/ProposalSystem.Application/Validators/CreateGeneratedPdfValidator.cs @@ -0,0 +1,18 @@ +using FluentValidation; +using ProposalSystem.Application.DTOs; + +namespace ProposalSystem.Application.Validators; + +// Fix: API-M5 — add FluentValidation for GeneratedPdf DTO +public class CreateGeneratedPdfValidator : AbstractValidator +{ + public CreateGeneratedPdfValidator() + { + RuleFor(x => x.ProposalId) + .NotEmpty().WithMessage("Proposal ID is required"); + + RuleFor(x => x.S3Key) + .NotEmpty().WithMessage("S3 key is required") + .MaximumLength(1000); + } +} diff --git a/api/src/ProposalSystem.Application/Validators/CreateSimilarReferenceValidator.cs b/api/src/ProposalSystem.Application/Validators/CreateSimilarReferenceValidator.cs new file mode 100644 index 0000000..43e99bb --- /dev/null +++ b/api/src/ProposalSystem.Application/Validators/CreateSimilarReferenceValidator.cs @@ -0,0 +1,19 @@ +using FluentValidation; +using ProposalSystem.Application.DTOs; + +namespace ProposalSystem.Application.Validators; + +// Fix: API-M5 — add FluentValidation for SimilarReference DTO +public class CreateSimilarReferenceValidator : AbstractValidator +{ + public CreateSimilarReferenceValidator() + { + RuleFor(x => x.ReferencedLibraryItemId) + .NotEmpty().WithMessage("Referenced library item ID is required") + .MaximumLength(500); + + RuleFor(x => x.SimilarityScore) + .InclusiveBetween(0f, 1f) + .WithMessage("Similarity score must be between 0 and 1"); + } +} diff --git a/api/src/ProposalSystem.Application/Validators/UpdateVendorProposalValidator.cs b/api/src/ProposalSystem.Application/Validators/UpdateVendorProposalValidator.cs new file mode 100644 index 0000000..8e38198 --- /dev/null +++ b/api/src/ProposalSystem.Application/Validators/UpdateVendorProposalValidator.cs @@ -0,0 +1,41 @@ +using FluentValidation; +using ProposalSystem.Application.DTOs; +using ProposalSystem.Domain.Entities; + +namespace ProposalSystem.Application.Validators; + +// Fix: API-M5 — add FluentValidation for VendorProposal DTOs +public class UpdateVendorProposalValidator : AbstractValidator +{ + public UpdateVendorProposalValidator() + { + RuleFor(x => x.VendorName) + .MaximumLength(200) + .When(x => x.VendorName != null); + + RuleFor(x => x.ExtractedData) + .MaximumLength(100000) + .When(x => x.ExtractedData != null); + + RuleFor(x => x.TotalVendorCost) + .GreaterThanOrEqualTo(0) + .When(x => x.TotalVendorCost.HasValue) + .WithMessage("Total vendor cost cannot be negative"); + + RuleFor(x => x.ProcessingStatus) + .Must(s => Enum.TryParse(s, out _)) + .When(x => x.ProcessingStatus != null) + .WithMessage("Invalid processing status"); + } +} + +public class UpdateStatusRequestValidator : AbstractValidator +{ + public UpdateStatusRequestValidator() + { + RuleFor(x => x.ProcessingStatus) + .NotEmpty().WithMessage("Processing status is required") + .Must(s => Enum.TryParse(s, out _)) + .WithMessage("Invalid processing status"); + } +} diff --git a/api/src/ProposalSystem.Infrastructure/Services/AuditService.cs b/api/src/ProposalSystem.Infrastructure/Services/AuditService.cs index 355fea8..b00dc02 100644 --- a/api/src/ProposalSystem.Infrastructure/Services/AuditService.cs +++ b/api/src/ProposalSystem.Infrastructure/Services/AuditService.cs @@ -18,9 +18,18 @@ public class AuditService : IAuditService public async Task LogAsync(AuditAction action, Guid? proposalId, string? details = null, CancellationToken ct = default) { - var jsonDetails = details != null - ? JsonSerializer.Serialize(new { message = details }) - : null; + // 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; + // otherwise, wrap plain text in a JSON envelope for consistency. + string? jsonDetails = null; + if (details != null) + { + var trimmed = details.TrimStart(); + if (trimmed.StartsWith('{') || trimmed.StartsWith('[')) + jsonDetails = details; + else + jsonDetails = JsonSerializer.Serialize(new { message = details }); + } var entry = new AuditLog { diff --git a/api/src/ProposalSystem.Infrastructure/Services/LineItemService.cs b/api/src/ProposalSystem.Infrastructure/Services/LineItemService.cs index 15bcb7d..b2f4a9d 100644 --- a/api/src/ProposalSystem.Infrastructure/Services/LineItemService.cs +++ b/api/src/ProposalSystem.Infrastructure/Services/LineItemService.cs @@ -1,3 +1,4 @@ +using System.Text.Json; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; using ProposalSystem.Application.DTOs; @@ -7,6 +8,7 @@ using ProposalSystem.Infrastructure.Data; namespace ProposalSystem.Infrastructure.Services; +// Fix: API-H6 — add structured logging for line item operations public class LineItemService : ILineItemService { private readonly ProposalDbContext _db; @@ -22,7 +24,9 @@ public class LineItemService : ILineItemService public async Task> GetByProposalIdAsync(Guid proposalId, CancellationToken ct = default) { + // Fix: API-M7 — AsNoTracking on read-only query return await _db.LineItems + .AsNoTracking() .Where(li => li.ProposalId == proposalId) .OrderBy(li => li.SortOrder) .Select(li => MapToResponse(li)) @@ -35,7 +39,10 @@ public class LineItemService : ILineItemService ?? throw new KeyNotFoundException($"Proposal {proposalId} not found"); if (proposal.Status == ProposalStatus.Approved || proposal.Status == ProposalStatus.Sent) + { + _logger.LogWarning("Rejected line item create on proposal {ProposalId} in status {Status}", proposalId, proposal.Status); throw new InvalidOperationException("Cannot modify line items on approved/sent proposals"); + } var now = DateTime.UtcNow; var lineItem = new LineItem @@ -57,13 +64,20 @@ public class LineItemService : ILineItemService _db.LineItems.Add(lineItem); await _db.SaveChangesAsync(ct); + _logger.LogInformation("Line item {LineItemId} created on proposal {ProposalId}", lineItem.Id, proposalId); + try { - await _audit.LogAsync(AuditAction.EditLineItem, proposalId, $"Added: {request.Description}", ct); + // 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) { - // Fix: API-M11 — log audit failures instead of silently swallowing _logger.LogError(ex, "Failed to write audit log for line item creation on proposal {ProposalId}", proposalId); } @@ -76,9 +90,11 @@ public class LineItemService : ILineItemService ?? throw new KeyNotFoundException($"Proposal {proposalId} not found"); if (proposal.Status == ProposalStatus.Approved || proposal.Status == ProposalStatus.Sent) + { + _logger.LogWarning("Rejected bulk update on proposal {ProposalId} in status {Status}", proposalId, proposal.Status); throw new InvalidOperationException("Cannot modify line items on approved/sent proposals"); + } - // Fix: API-M8 — wrap delete-all/insert-all in explicit transaction await using var transaction = await _db.Database.BeginTransactionAsync(ct); try { @@ -115,7 +131,13 @@ public class LineItemService : ILineItemService try { - await _audit.LogAsync(AuditAction.EditLineItem, proposalId, $"Bulk update: {newItems.Count} items", ct); + // 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) { @@ -140,12 +162,23 @@ public class LineItemService : ILineItemService var proposal = await _db.Proposals.FindAsync(new object[] { proposalId }, ct)!; if (proposal!.Status == ProposalStatus.Approved || proposal.Status == ProposalStatus.Sent) + { + _logger.LogWarning("Rejected line item delete on proposal {ProposalId} in status {Status}", proposalId, proposal.Status); throw new InvalidOperationException("Cannot modify line items on approved/sent proposals"); + } _db.LineItems.Remove(lineItem); await _db.SaveChangesAsync(ct); - await _audit.LogAsync(AuditAction.EditLineItem, proposalId, $"Removed: {lineItem.Description}", ct); + _logger.LogInformation("Line item {LineItemId} deleted from proposal {ProposalId}", lineItemId, proposalId); + + // Fix: API-M12 — capture deleted line item details in audit trail + var deleteAuditDetails = JsonSerializer.Serialize(new + { + 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); } private static LineItemResponse MapToResponse(LineItem li) => new( diff --git a/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs b/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs index bf33f52..4d5ed07 100644 --- a/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs +++ b/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs @@ -1,3 +1,4 @@ +using System.Text.Json; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; using ProposalSystem.Application.DTOs; @@ -7,6 +8,7 @@ using ProposalSystem.Infrastructure.Data; namespace ProposalSystem.Infrastructure.Services; +// Fix: API-H6 — add structured logging for state transitions and errors public class ProposalService : IProposalService { private readonly ProposalDbContext _db; @@ -44,6 +46,7 @@ public class ProposalService : IProposalService Id = Guid.NewGuid(), ProposalNumber = proposalNumber, WorkOrderNumber = request.WorkOrderNumber, + PoNumber = request.PoNumber, CustomerName = request.CustomerName, CustomerAddress = request.CustomerAddress, ScopeOfWork = request.ScopeOfWork, @@ -61,13 +64,15 @@ public class ProposalService : IProposalService 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) { - // Fix: API-M11 — log audit failures instead of silently swallowing _logger.LogError(ex, "Failed to write audit log for proposal submission {ProposalId}", proposal.Id); } @@ -77,7 +82,6 @@ public class ProposalService : IProposalService } catch (Exception ex) { - // Fix: API-M11 — log job publish failures instead of silently swallowing _logger.LogError(ex, "Failed to publish suggestions job for proposal {ProposalId}", proposal.Id); } @@ -86,7 +90,9 @@ public class ProposalService : IProposalService public async Task GetByIdAsync(Guid id, CancellationToken ct = default) { + // Fix: API-M7 — AsNoTracking on read-only query var proposal = await _db.Proposals + .AsNoTracking() .Include(p => p.SubmittedBy) .Include(p => p.AssignedAdmin) .Include(p => p.ApprovedBy) @@ -103,7 +109,9 @@ public class ProposalService : IProposalService var page = Math.Max(1, filter.Page); var pageSize = Math.Clamp(filter.PageSize, 1, 100); + // Fix: API-M7 — AsNoTracking on read-only list query var query = _db.Proposals + .AsNoTracking() .Include(p => p.SubmittedBy) .Include(p => p.AssignedAdmin) .AsQueryable(); @@ -171,23 +179,44 @@ public class ProposalService : IProposalService .FirstOrDefaultAsync(p => p.Id == id, ct) ?? throw new KeyNotFoundException($"Proposal {id} not found"); + // Fix: API-M12 — capture before/after values for audit trail + var changes = new Dictionary(); + if (request.RefinedScope != null) + { + changes["refinedScope"] = new { old = proposal.RefinedScope, @new = request.RefinedScope }; proposal.RefinedScope = request.RefinedScope; + } if (request.Notes != null) + { + changes["notes"] = new { old = proposal.Notes, @new = request.Notes }; proposal.Notes = request.Notes; + } + + if (request.PoNumber != null) + { + changes["poNumber"] = new { old = proposal.PoNumber, @new = request.PoNumber }; + proposal.PoNumber = request.PoNumber; + } + + if (request.WorkOrderNumber != null) + { + changes["workOrderNumber"] = new { old = proposal.WorkOrderNumber, @new = request.WorkOrderNumber }; + proposal.WorkOrderNumber = request.WorkOrderNumber; + } if (request.AssignedAdminId.HasValue) + { + changes["assignedAdminId"] = new { old = proposal.AssignedAdminId, @new = request.AssignedAdminId.Value }; proposal.AssignedAdminId = request.AssignedAdminId.Value; - - if (request.Status.HasValue && request.Status.Value == ProposalStatus.InReview - && (proposal.Status == ProposalStatus.Draft || proposal.Status == ProposalStatus.Revised)) - proposal.Status = request.Status.Value; + } proposal.UpdatedAt = DateTime.UtcNow; await _db.SaveChangesAsync(ct); - await _audit.LogAsync(AuditAction.Edit, id, null, ct); + var auditDetails = changes.Count > 0 ? JsonSerializer.Serialize(changes) : null; + await _audit.LogAsync(AuditAction.Edit, id, auditDetails, ct); return MapToResponse(proposal); } @@ -202,14 +231,26 @@ public class ProposalService : IProposalService ?? throw new KeyNotFoundException($"Proposal {id} not found"); if (proposal.Status == ProposalStatus.Approved) + { + _logger.LogInformation("Proposal {ProposalId} already approved, returning idempotent response", id); return MapToResponse(proposal); + } - if (proposal.Status != ProposalStatus.InReview) + if (proposal.Status != ProposalStatus.InReview && proposal.Status != ProposalStatus.Revised) + { + _logger.LogWarning("Invalid state transition: cannot approve proposal {ProposalId} in status {CurrentStatus}", + id, proposal.Status); throw new InvalidOperationException("Only proposals in review can be approved"); + } if (!proposal.LineItems.Any() || proposal.LineItems.All(li => li.TotalPrice <= 0)) + { + _logger.LogWarning("Cannot approve proposal {ProposalId}: no priced line items", id); throw new InvalidOperationException("Cannot approve proposal without priced line items"); + } + // Fix: API-M12 — capture before/after status for audit trail + var previousStatus = proposal.Status; proposal.Status = ProposalStatus.Approved; proposal.ApprovedById = _currentUser.UserId; proposal.ApprovedAt = DateTime.UtcNow; @@ -217,7 +258,39 @@ public class ProposalService : IProposalService proposal.UpdatedAt = DateTime.UtcNow; await _db.SaveChangesAsync(ct); - await _audit.LogAsync(AuditAction.Approve, id, null, ct); + var approveAuditDetails = JsonSerializer.Serialize(new { status = new { old = previousStatus.ToString(), @new = ProposalStatus.Approved.ToString() } }); + await _audit.LogAsync(AuditAction.Approve, id, approveAuditDetails, ct); + + _logger.LogInformation("Proposal {ProposalId} approved by user {UserId}, total bid {TotalBidAmount}", + id, _currentUser.UserId, proposal.TotalBidAmount); + + return MapToResponse(proposal); + } + + public async Task ReturnToReviewAsync(Guid id, CancellationToken ct = default) + { + var proposal = await _db.Proposals + .Include(p => p.SubmittedBy) + .Include(p => p.ApprovedBy) + .FirstOrDefaultAsync(p => p.Id == id, ct) + ?? throw new KeyNotFoundException($"Proposal {id} not found"); + + if (proposal.Status == ProposalStatus.InReview) + return MapToResponse(proposal); + + if (proposal.Status != ProposalStatus.Approved) + throw new InvalidOperationException("Only approved proposals can be returned to review"); + + // Fix: API-M12 — capture before/after status for audit trail + var previousStatus = proposal.Status; + proposal.Status = ProposalStatus.InReview; + proposal.ApprovedById = null; + 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); return MapToResponse(proposal); } @@ -231,17 +304,29 @@ public class ProposalService : IProposalService ?? throw new KeyNotFoundException($"Proposal {id} not found"); if (proposal.Status == ProposalStatus.Sent) + { + _logger.LogInformation("Proposal {ProposalId} already sent, returning idempotent response", id); return MapToResponse(proposal); + } if (proposal.Status != ProposalStatus.Approved) + { + _logger.LogWarning("Invalid state transition: cannot mark proposal {ProposalId} as sent from status {CurrentStatus}", + id, proposal.Status); throw new InvalidOperationException("Only approved proposals can be marked as sent"); + } + // 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); - await _audit.LogAsync(AuditAction.MarkSent, id, null, ct); + var sentAuditDetails = JsonSerializer.Serialize(new { status = new { old = previousStatus.ToString(), @new = ProposalStatus.Sent.ToString() } }); + await _audit.LogAsync(AuditAction.MarkSent, id, sentAuditDetails, ct); + + _logger.LogInformation("Proposal {ProposalId} marked as sent by user {UserId}", id, _currentUser.UserId); await _jobPublisher.PublishAsync("library-ingest", new { proposalId = id }, ct); @@ -260,17 +345,26 @@ public class ProposalService : IProposalService var existingRevision = await _db.Proposals .FirstOrDefaultAsync(p => p.ParentProposalId == proposal.Id, ct); if (existingRevision != null) + { + _logger.LogInformation("Proposal {ProposalId} already revised, returning existing revision {RevisionId}", + id, existingRevision.Id); return MapToResponse(existingRevision); + } } if (proposal.Status != ProposalStatus.Sent) + { + _logger.LogWarning("Invalid state transition: cannot revise proposal {ProposalId} in status {CurrentStatus}", + id, proposal.Status); throw new InvalidOperationException("Only sent proposals can be revised"); + } var revision = new Proposal { Id = Guid.NewGuid(), ProposalNumber = $"{proposal.ProposalNumber}-R{proposal.CurrentRevision + 1}", WorkOrderNumber = proposal.WorkOrderNumber, + PoNumber = proposal.PoNumber, CustomerName = proposal.CustomerName, CustomerAddress = proposal.CustomerAddress, ScopeOfWork = proposal.ScopeOfWork, @@ -308,13 +402,23 @@ public class ProposalService : IProposalService }); } + // Fix: API-M12 — capture before/after status for audit trail + var previousReviseStatus = proposal.Status; proposal.Status = ProposalStatus.Revised; proposal.UpdatedAt = DateTime.UtcNow; _db.Proposals.Add(revision); await _db.SaveChangesAsync(ct); - await _audit.LogAsync(AuditAction.CreateRevision, revision.Id, $"Revised from {proposal.Id}", 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); + + _logger.LogInformation("Proposal {ProposalId} revised to {RevisionId} (revision {RevisionNumber}) by user {UserId}", + id, revision.Id, revision.CurrentRevision, _currentUser.UserId); return MapToResponse(revision); } @@ -326,7 +430,9 @@ public class ProposalService : IProposalService var rootId = proposal.ParentProposalId ?? proposal.Id; + // Fix: API-M7 — AsNoTracking on read-only revision history query var revisions = await _db.Proposals + .AsNoTracking() .Where(p => p.Id == rootId || p.ParentProposalId == rootId) .OrderBy(p => p.CurrentRevision) .ToListAsync(ct); @@ -336,7 +442,9 @@ public class ProposalService : IProposalService public async Task> GetAuditTrailAsync(Guid id, CancellationToken ct = default) { + // Fix: API-M7 — AsNoTracking on read-only audit trail query return await _db.AuditLogs + .AsNoTracking() .Include(a => a.User) .Where(a => a.ProposalId == id) .OrderByDescending(a => a.Timestamp) @@ -355,10 +463,13 @@ public class ProposalService : IProposalService public async Task GetStatsAsync(CancellationToken ct = default) { - var userId = _currentUser.UserId; + // Fix: API-M7 — AsNoTracking on read-only stats query + var query = _db.Proposals.AsNoTracking().AsQueryable(); - var counts = await _db.Proposals - .Where(p => p.SubmittedById == userId) + if (_currentUser.Role == UserRole.Dispatcher) + query = query.Where(p => p.SubmittedById == _currentUser.UserId); + + var counts = await query .GroupBy(_ => 1) .Select(g => new { @@ -378,6 +489,7 @@ public class ProposalService : IProposalService p.Id, p.ProposalNumber, p.WorkOrderNumber, + p.PoNumber, p.CustomerName, p.CustomerAddress, p.ScopeOfWork,