From 8f492c0faf74732730f95452e48abaf96d83f7ba Mon Sep 17 00:00:00 2001 From: Arthur Bassi Date: Fri, 24 Jul 2026 18:28:44 -0300 Subject: [PATCH] feat(work-orders): allow comment edit and resolve author audit display names (#24) * feat(work-orders): enrich board search overdue filters and 0-based paging * fix(work-orders): align stacked services with CI build * fix(tests): pass userDataService in comment service unit test * fix(work-orders): use dedicated overdue query flag Stop treating WorkOrderType.Other as an overdue sentinel. Board and advanced search now accept overdue=true while types=Other filters real Other rows; combining both uses OR. * feat(work-orders): allow comment edit and resolve author audit display names Add PATCH comment for author/Admin, return authorName, and resolve AssignTo audit values to user display names. * fix(work-orders): enforce author-only comment edits per SH-122 Remove the undocumented Admin override so only the original comment author can edit, matching the ticket acceptance criteria. --------- Co-authored-by: Arthur Bassi Co-authored-by: Alexandre Brandizzi --- .../WorkOrderRouteContractTests.cs | 9 +- .../Controllers/WorkOrderDetailController.cs | 28 ++ .../Implementation/WorkOrderCommentService.cs | 5 +- .../Interfaces/IWorkOrderCommentService.cs | 3 +- .../WorkOrderPhase6Tests.cs | 243 ++++++++++++++++++ 5 files changed, 279 insertions(+), 9 deletions(-) diff --git a/Api.SeaHavenIndustries.Tests/WorkOrderRouteContractTests.cs b/Api.SeaHavenIndustries.Tests/WorkOrderRouteContractTests.cs index 7fd4260..dd1d0c5 100644 --- a/Api.SeaHavenIndustries.Tests/WorkOrderRouteContractTests.cs +++ b/Api.SeaHavenIndustries.Tests/WorkOrderRouteContractTests.cs @@ -36,10 +36,10 @@ public class WorkOrderRouteContractTests /// /// Baseline public endpoint set (verb + action-relative route) that the original single - /// WorkOrderController exposed. Every action is reachable under both api/WorkOrder and - /// api/workorders; that base-route duplication is collapsed here, so this is the distinct - /// action-relative contract. 45 routes come from 43 actions (Editworkorder and - /// GetWorkorderById each bind two routes). + /// WorkOrderController exposed, plus the author-only board-comment edit endpoint (SH-122). + /// Every action is reachable under both api/WorkOrder and api/workorders; that base-route + /// duplication is collapsed here, so this is the distinct action-relative contract. 46 routes + /// come from 44 actions (Editworkorder and GetWorkorderById each bind two routes). /// private static readonly HashSet ExpectedWorkOrderEndpoints = new(StringComparer.Ordinal) { @@ -67,6 +67,7 @@ public class WorkOrderRouteContractTests "GET {id:int}/detail", "GET {id:int}/media", "PATCH {id:int}/board", + "PATCH {id:int}/comments/{commentId:int}", "POST AddChecklistItem", "POST AddComment", "POST AddCommentJson", diff --git a/Api.SeaHavenIndustries/Controllers/WorkOrderDetailController.cs b/Api.SeaHavenIndustries/Controllers/WorkOrderDetailController.cs index 7e3b657..3701db9 100644 --- a/Api.SeaHavenIndustries/Controllers/WorkOrderDetailController.cs +++ b/Api.SeaHavenIndustries/Controllers/WorkOrderDetailController.cs @@ -71,5 +71,33 @@ namespace Api.SeaHavenIndustries.Controllers return UnprocessableEntity(new WorkOrderBoardValidationErrorDto { Code = ex.Code, Message = ex.Message }); } } + + [HttpPatch("{id:int}/comments/{commentId:int}")] + public async Task UpdateBoardComment( + int id, + int commentId, + [FromBody] WorkOrderCommentCreateDto request) + { + try + { + var actorId = User.FindFirstValue(ClaimTypes.NameIdentifier); + var comment = await _workOrderCommentService.UpdateCommentAsync( + id, commentId, request, actorId); + return Ok(comment); + } + catch (WorkOrderBoardValidationException ex) when (ex.Code == "NotFound") + { + return NotFound(new WorkOrderBoardValidationErrorDto { Code = ex.Code, Message = ex.Message }); + } + catch (WorkOrderBoardValidationException ex) when (ex.Code == "Forbidden") + { + return StatusCode(StatusCodes.Status403Forbidden, + new WorkOrderBoardValidationErrorDto { Code = ex.Code, Message = ex.Message }); + } + catch (WorkOrderBoardValidationException ex) + { + return UnprocessableEntity(new WorkOrderBoardValidationErrorDto { Code = ex.Code, Message = ex.Message }); + } + } } } diff --git a/SeaHaven.Services/Implementation/WorkOrderCommentService.cs b/SeaHaven.Services/Implementation/WorkOrderCommentService.cs index aa17d38..e0ad136 100644 --- a/SeaHaven.Services/Implementation/WorkOrderCommentService.cs +++ b/SeaHaven.Services/Implementation/WorkOrderCommentService.cs @@ -65,8 +65,7 @@ namespace SeaHaven.Services.Implementation int workOrderId, int commentId, WorkOrderCommentCreateDto request, - string? actorId, - bool isAdmin) + string? actorId) { var workOrder = await _detailData.GetWorkOrderForMediaAsync(workOrderId); if (workOrder == null) @@ -89,7 +88,7 @@ namespace SeaHaven.Services.Implementation var isAuthor = !string.IsNullOrWhiteSpace(actorId) && string.Equals(comment.UserId, actorId, StringComparison.Ordinal); - if (!isAuthor && !isAdmin) + if (!isAuthor) throw new WorkOrderBoardValidationException("Forbidden", "You are not allowed to edit this comment."); comment.Commenttext = request.Text.Trim(); diff --git a/SeaHaven.Services/Interfaces/IWorkOrderCommentService.cs b/SeaHaven.Services/Interfaces/IWorkOrderCommentService.cs index 5e6a4ea..3c5d27c 100644 --- a/SeaHaven.Services/Interfaces/IWorkOrderCommentService.cs +++ b/SeaHaven.Services/Interfaces/IWorkOrderCommentService.cs @@ -11,7 +11,6 @@ namespace SeaHaven.Services.Interfaces int workOrderId, int commentId, WorkOrderCommentCreateDto request, - string? actorId, - bool isAdmin); + string? actorId); } } diff --git a/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs b/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs index 198f18f..e30e651 100644 --- a/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs @@ -133,6 +133,13 @@ public class WorkOrderDetailServiceTests NewValue = "1", CreatedAt = DateTime.UtcNow }); + context.Users.Add(new ApplicationUser + { + Id = "user-1", + UserName = "alice", + FirstName = "Alice", + LastName = "Dispatcher" + }); context.Comments.Add(new Comments { WorkerOrderId = 1, @@ -152,11 +159,75 @@ public class WorkOrderDetailServiceTests Assert.Equal("HVAC PM Completion", detail.Completion.Template!.Name); Assert.Single(detail.Comments); Assert.Equal("user-1", detail.Comments[0].AuthorId); + Assert.Equal("Alice Dispatcher", detail.Comments[0].AuthorName); Assert.Equal("Called vendor", detail.Comments[0].Text); Assert.Single(detail.Audit); Assert.Equal("system", detail.Audit[0].Type); Assert.Equal("WeekRolled", detail.Audit[0].Action); } + + [Fact] + public async Task GetAudit_ResolvesAssignToUserIdsToDisplayNames() + { + var (context, service) = CreateSut(); + const string oldUserId = "a1000001-0001-4000-8000-000000000002"; + const string newUserId = "b5e356d5-d926-4f92-8673-ee13cddeff0f"; + + context.Users.AddRange( + new ApplicationUser + { + Id = oldUserId, + UserName = "alice", + FirstName = "Alice", + LastName = "Dispatcher" + }, + new ApplicationUser + { + Id = newUserId, + UserName = "bob", + FirstName = "Bob", + LastName = "Tech" + }); + context.workOrders.Add(new WorkOrder + { + Id = 1, + InternalWONumber = "00000000001", + LifecycleStatus = LifecycleStatus.Pending, + RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 } + }); + context.WorkOrderAuditLogs.Add(new WorkOrderAuditLog + { + WorkOrderId = 1, + UserId = "actor-1", + EventType = "Manual", + Action = "AssignmentChanged", + FieldName = "AssignTo", + OldValue = oldUserId, + NewValue = newUserId, + CreatedAt = DateTime.UtcNow + }); + context.WorkOrderAuditLogs.Add(new WorkOrderAuditLog + { + WorkOrderId = 1, + UserId = "actor-1", + EventType = "Manual", + Action = "AssignmentChanged", + FieldName = "AssignTo", + OldValue = newUserId, + NewValue = "", + CreatedAt = DateTime.UtcNow.AddMinutes(1) + }); + await context.SaveChangesAsync(); + + var audit = await service.GetAuditAsync(1); + + Assert.NotNull(audit); + Assert.Equal(2, audit!.Count); + var unassigned = audit.Single(a => a.NewValue == "Unassigned"); + Assert.Equal("Bob Tech", unassigned.OldValue); + var reassigned = audit.Single(a => a.NewValue == "Bob Tech"); + Assert.Equal("Alice Dispatcher", reassigned.OldValue); + } } public class WorkOrderCompletionServiceTests @@ -309,6 +380,13 @@ public class WorkOrderCommentServiceTests .Options; var context = new ApplicationDbContext(options); context.workOrders.Add(new WorkOrder { Id = 1, LifecycleStatus = LifecycleStatus.Scheduled }); + context.Users.Add(new ApplicationUser + { + Id = "user-abc", + UserName = "bob", + FirstName = "Bob", + LastName = "Tech" + }); await context.SaveChangesAsync(); var detailData = new WorkOrderDetailDataService(context); @@ -319,9 +397,174 @@ public class WorkOrderCommentServiceTests var result = await service.AddCommentAsync(1, new WorkOrderCommentCreateDto { Text = "Note" }, "user-abc"); Assert.Equal("user-abc", result.AuthorId); + Assert.Equal("Bob Tech", result.AuthorName); Assert.Equal("Note", result.Text); Assert.False(string.IsNullOrWhiteSpace(result.Time)); } + + [Fact] + public async Task GetComments_FallsBackToCommenterWhenUserIdMissing() + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + var context = new ApplicationDbContext(options); + context.workOrders.Add(new WorkOrder { Id = 1, LifecycleStatus = LifecycleStatus.Scheduled }); + context.Comments.Add(new Comments + { + WorkerOrderId = 1, + Commenter = "Legacy Sync Author", + Commenttext = "Synced note", + CreatedDate = DateTime.UtcNow + }); + await context.SaveChangesAsync(); + + var detailData = new WorkOrderDetailDataService(context); + var commentData = new CommentDataService(context); + var userData = new UserDataService(context); + var service = new WorkOrderCommentService(detailData, commentData, userData); + + var comments = await service.GetCommentsAsync(1); + + Assert.NotNull(comments); + Assert.Single(comments!); + Assert.Null(comments[0].AuthorId); + Assert.Equal("Legacy Sync Author", comments[0].AuthorName); + Assert.Equal("Synced note", comments[0].Text); + } + + private static async Task<(ApplicationDbContext Context, WorkOrderCommentService Service, Comments Comment)> SeedEditableCommentAsync( + LifecycleStatus status = LifecycleStatus.Scheduled, + string authorId = "user-author", + string commentType = "General", + string recordType = "WorkOrder", + int workOrderId = 1) + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + var context = new ApplicationDbContext(options); + context.workOrders.Add(new WorkOrder { Id = workOrderId, LifecycleStatus = status }); + context.Users.Add(new ApplicationUser + { + Id = authorId, + UserName = "author", + FirstName = "Alice", + LastName = "Dispatcher" + }); + var comment = new Comments + { + WorkerOrderId = workOrderId, + UserId = authorId, + Commenttext = "Original", + CommentType = commentType, + RecordType = recordType, + CreatedDate = DateTime.UtcNow + }; + context.Comments.Add(comment); + await context.SaveChangesAsync(); + + var service = new WorkOrderCommentService( + new WorkOrderDetailDataService(context), + new CommentDataService(context), + new UserDataService(context)); + return (context, service, comment); + } + + [Fact] + public async Task UpdateComment_Author_UpdatesText() + { + var (_, service, comment) = await SeedEditableCommentAsync(); + + var result = await service.UpdateCommentAsync( + 1, + comment.Id, + new WorkOrderCommentCreateDto { Text = " Updated note " }, + "user-author"); + + Assert.Equal("Updated note", result.Text); + Assert.Equal("user-author", result.AuthorId); + Assert.Equal("Alice Dispatcher", result.AuthorName); + } + + [Fact] + public async Task UpdateComment_AdminNonAuthor_ThrowsForbidden() + { + var (_, service, comment) = await SeedEditableCommentAsync(); + + var ex = await Assert.ThrowsAsync(() => + service.UpdateCommentAsync( + 1, + comment.Id, + new WorkOrderCommentCreateDto { Text = "Admin edit" }, + "admin-user")); + + Assert.Equal("Forbidden", ex.Code); + } + + [Fact] + public async Task UpdateComment_NonAuthor_ThrowsForbidden() + { + var (_, service, comment) = await SeedEditableCommentAsync(); + + var ex = await Assert.ThrowsAsync(() => + service.UpdateCommentAsync( + 1, + comment.Id, + new WorkOrderCommentCreateDto { Text = "Nope" }, + "other-user")); + + Assert.Equal("Forbidden", ex.Code); + } + + [Fact] + public async Task UpdateComment_LegacyVendorComment_ThrowsNotEditable() + { + var (_, service, comment) = await SeedEditableCommentAsync( + commentType: "vendor", + recordType: "WorkOrder"); + + var ex = await Assert.ThrowsAsync(() => + service.UpdateCommentAsync( + 1, + comment.Id, + new WorkOrderCommentCreateDto { Text = "Nope" }, + "user-author")); + + Assert.Equal("NotEditable", ex.Code); + } + + [Fact] + public async Task UpdateComment_ReadOnlyWorkOrder_Throws() + { + var (_, service, comment) = await SeedEditableCommentAsync(LifecycleStatus.Canceled); + + var ex = await Assert.ThrowsAsync(() => + service.UpdateCommentAsync( + 1, + comment.Id, + new WorkOrderCommentCreateDto { Text = "Nope" }, + "user-author")); + + Assert.Equal("ReadOnly", ex.Code); + } + + [Fact] + public async Task UpdateComment_WrongWorkOrder_ThrowsNotFound() + { + var (context, service, comment) = await SeedEditableCommentAsync(workOrderId: 1); + context.workOrders.Add(new WorkOrder { Id = 2, LifecycleStatus = LifecycleStatus.Scheduled }); + await context.SaveChangesAsync(); + + var ex = await Assert.ThrowsAsync(() => + service.UpdateCommentAsync( + 2, + comment.Id, + new WorkOrderCommentCreateDto { Text = "Nope" }, + "user-author")); + + Assert.Equal("NotFound", ex.Code); + } } public class WorkOrderMediaServiceTests