From d56d5a05fef8b0357ae119751514d5a1a287c37e Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sun, 29 Mar 2026 23:51:19 +0100 Subject: [PATCH 1/3] Enrich audit log entries with changed field details for update operations Board, Card, Column, and Label services now capture pre-mutation state and build human-readable change summaries listing which specific fields changed. Null/unchanged fields are omitted. Tests verify change details appear in audit entries for all four services. Closes #583 --- .../Services/BoardService.cs | 26 ++- .../Services/CardService.cs | 39 ++++- .../Services/ColumnService.cs | 21 ++- .../Services/LabelService.cs | 18 +- .../Services/BoardMutationAuditTests.cs | 164 +++++++++++++++++- 5 files changed, 255 insertions(+), 13 deletions(-) diff --git a/backend/src/Taskdeck.Application/Services/BoardService.cs b/backend/src/Taskdeck.Application/Services/BoardService.cs index 40ccf807c..79ea94160 100644 --- a/backend/src/Taskdeck.Application/Services/BoardService.cs +++ b/backend/src/Taskdeck.Application/Services/BoardService.cs @@ -168,6 +168,11 @@ private async Task> UpdateBoardInternalAsync(Guid id, UpdateBoa if (board == null) return Result.Failure(ErrorCodes.NotFound, $"Board with ID {id} not found"); + // Capture pre-mutation state for change summary + var oldName = board.Name; + var oldDescription = board.Description; + var oldIsArchived = board.IsArchived; + if (dto.Name != null || dto.Description != null) board.Update(dto.Name, dto.Description); @@ -183,12 +188,15 @@ private async Task> UpdateBoardInternalAsync(Guid id, UpdateBoa await _realtimeNotifier.NotifyBoardMutationAsync( new BoardRealtimeEvent(board.Id, "board", "updated", board.Id, DateTimeOffset.UtcNow), cancellationToken); + + var changeSummary = BuildBoardChangeSummary(dto, oldName, oldDescription, oldIsArchived); + if (dto.IsArchived == true) - await SafeLogAsync("board", board.Id, AuditAction.Archived, changes: $"name={board.Name}"); + await SafeLogAsync("board", board.Id, AuditAction.Archived, changes: changeSummary); else if (dto.IsArchived == false) - await SafeLogAsync("board", board.Id, AuditAction.Updated, changes: $"unarchived; name={board.Name}"); + await SafeLogAsync("board", board.Id, AuditAction.Updated, changes: changeSummary); else - await SafeLogAsync("board", board.Id, AuditAction.Updated, changes: $"name={board.Name}"); + await SafeLogAsync("board", board.Id, AuditAction.Updated, changes: changeSummary); return Result.Success(MapToDto(board)); } catch (DomainException ex) @@ -197,6 +205,18 @@ await _realtimeNotifier.NotifyBoardMutationAsync( } } + private static string BuildBoardChangeSummary(UpdateBoardDto dto, string oldName, string? oldDescription, bool oldIsArchived) + { + var parts = new List(); + if (dto.Name != null) + parts.Add($"Name: '{oldName}' -> '{dto.Name}'"); + if (dto.Description != null) + parts.Add($"Description changed"); + if (dto.IsArchived.HasValue && dto.IsArchived.Value != oldIsArchived) + parts.Add(dto.IsArchived.Value ? "Archived" : "Unarchived"); + return parts.Count > 0 ? string.Join("; ", parts) : "no fields changed"; + } + public async Task> GetBoardByIdAsync(Guid id, CancellationToken cancellationToken = default) { var board = await _unitOfWork.Boards.GetByIdAsync(id, cancellationToken); diff --git a/backend/src/Taskdeck.Application/Services/CardService.cs b/backend/src/Taskdeck.Application/Services/CardService.cs index 6ff968344..172a2137c 100644 --- a/backend/src/Taskdeck.Application/Services/CardService.cs +++ b/backend/src/Taskdeck.Application/Services/CardService.cs @@ -113,6 +113,14 @@ public async Task> UpdateCardAsync( "Card was updated by another session. Refresh and retry your changes."); } + // Capture pre-mutation state for change summary + var oldTitle = card.Title; + var oldDescription = card.Description; + var oldDueDate = card.DueDate; + var oldIsBlocked = card.IsBlocked; + var oldBlockReason = card.BlockReason; + var oldLabelIds = card.CardLabels.Select(cl => cl.LabelId).OrderBy(id => id).ToList(); + // Update basic fields if (dto.Title != null || dto.Description != null || dto.DueDate.HasValue) card.Update(dto.Title, dto.Description, dto.DueDate); @@ -140,11 +148,13 @@ public async Task> UpdateCardAsync( } } + var changeSummary = BuildCardChangeSummary(dto, oldTitle, oldDescription, oldDueDate, oldIsBlocked, oldBlockReason, oldLabelIds); + await _unitOfWork.SaveChangesAsync(cancellationToken); await _realtimeNotifier.NotifyBoardMutationAsync( new BoardRealtimeEvent(card.BoardId, "card", "updated", card.Id, DateTimeOffset.UtcNow), cancellationToken); - await SafeLogAsync("card", card.Id, AuditAction.Updated, actorUserId); + await SafeLogAsync("card", card.Id, AuditAction.Updated, actorUserId, changeSummary); var updatedCard = await _unitOfWork.Cards.GetByIdWithLabelsAsync(id, cancellationToken); return Result.Success(MapToDto(updatedCard!)); @@ -155,6 +165,33 @@ await _realtimeNotifier.NotifyBoardMutationAsync( } } + private static string BuildCardChangeSummary( + UpdateCardDto dto, + string oldTitle, + string? oldDescription, + DateTimeOffset? oldDueDate, + bool oldIsBlocked, + string? oldBlockReason, + List oldLabelIds) + { + var parts = new List(); + if (dto.Title != null) + parts.Add($"Title: '{oldTitle}' -> '{dto.Title}'"); + if (dto.Description != null) + parts.Add("Description changed"); + if (dto.DueDate.HasValue) + parts.Add($"DueDate: '{oldDueDate?.ToString("O") ?? "none"}' -> '{dto.DueDate.Value:O}'"); + if (dto.IsBlocked.HasValue && dto.IsBlocked.Value != oldIsBlocked) + parts.Add(dto.IsBlocked.Value ? $"Blocked: {dto.BlockReason ?? "no reason"}" : "Unblocked"); + if (dto.LabelIds != null) + { + var newLabelIds = dto.LabelIds.OrderBy(id => id).ToList(); + if (!oldLabelIds.SequenceEqual(newLabelIds)) + parts.Add($"Labels changed: {oldLabelIds.Count} -> {newLabelIds.Count}"); + } + return parts.Count > 0 ? string.Join("; ", parts) : "no fields changed"; + } + public Task> UpdateCardAsync( Guid id, UpdateCardDto dto, diff --git a/backend/src/Taskdeck.Application/Services/ColumnService.cs b/backend/src/Taskdeck.Application/Services/ColumnService.cs index d96be3eaa..006262681 100644 --- a/backend/src/Taskdeck.Application/Services/ColumnService.cs +++ b/backend/src/Taskdeck.Application/Services/ColumnService.cs @@ -73,12 +73,19 @@ public async Task> UpdateColumnAsync(Guid id, UpdateColumnDto if (column == null) return Result.Failure(ErrorCodes.NotFound, $"Column with ID {id} not found"); + // Capture pre-mutation state for change summary + var oldName = column.Name; + var oldWipLimit = column.WipLimit; + var oldPosition = column.Position; + column.Update(dto.Name, dto.WipLimit, dto.Position); await _unitOfWork.SaveChangesAsync(cancellationToken); await _realtimeNotifier.NotifyBoardMutationAsync( new BoardRealtimeEvent(column.BoardId, "column", "updated", column.Id, DateTimeOffset.UtcNow), cancellationToken); - await SafeLogAsync("column", column.Id, AuditAction.Updated); + + var changeSummary = BuildColumnChangeSummary(dto, oldName, oldWipLimit, oldPosition); + await SafeLogAsync("column", column.Id, AuditAction.Updated, changes: changeSummary); return Result.Success(MapToDto(column)); } @@ -88,6 +95,18 @@ await _realtimeNotifier.NotifyBoardMutationAsync( } } + private static string BuildColumnChangeSummary(UpdateColumnDto dto, string oldName, int? oldWipLimit, int oldPosition) + { + var parts = new List(); + if (dto.Name != null) + parts.Add($"Name: '{oldName}' -> '{dto.Name}'"); + if (dto.WipLimit.HasValue) + parts.Add($"WipLimit: {oldWipLimit?.ToString() ?? "none"} -> {dto.WipLimit.Value}"); + if (dto.Position.HasValue) + parts.Add($"Position: {oldPosition} -> {dto.Position.Value}"); + return parts.Count > 0 ? string.Join("; ", parts) : "no fields changed"; + } + public async Task> UpdateColumnAsync(Guid boardId, Guid id, UpdateColumnDto dto, CancellationToken cancellationToken = default) { var column = await _unitOfWork.Columns.GetByIdAsync(id, cancellationToken); diff --git a/backend/src/Taskdeck.Application/Services/LabelService.cs b/backend/src/Taskdeck.Application/Services/LabelService.cs index e2ac02784..f7678b6bd 100644 --- a/backend/src/Taskdeck.Application/Services/LabelService.cs +++ b/backend/src/Taskdeck.Application/Services/LabelService.cs @@ -64,12 +64,18 @@ public async Task> UpdateLabelAsync(Guid id, UpdateLabelDto dto if (label == null) return Result.Failure(ErrorCodes.NotFound, $"Label with ID {id} not found"); + // Capture pre-mutation state for change summary + var oldName = label.Name; + var oldColorHex = label.ColorHex; + label.Update(dto.Name, dto.ColorHex); await _unitOfWork.SaveChangesAsync(cancellationToken); await _realtimeNotifier.NotifyBoardMutationAsync( new BoardRealtimeEvent(label.BoardId, "label", "updated", label.Id, DateTimeOffset.UtcNow), cancellationToken); - await SafeLogAsync("label", label.Id, AuditAction.Updated); + + var changeSummary = BuildLabelChangeSummary(dto, oldName, oldColorHex); + await SafeLogAsync("label", label.Id, AuditAction.Updated, changes: changeSummary); return Result.Success(MapToDto(label)); } @@ -79,6 +85,16 @@ await _realtimeNotifier.NotifyBoardMutationAsync( } } + private static string BuildLabelChangeSummary(UpdateLabelDto dto, string oldName, string oldColorHex) + { + var parts = new List(); + if (dto.Name != null) + parts.Add($"Name: '{oldName}' -> '{dto.Name}'"); + if (dto.ColorHex != null) + parts.Add($"Color: '{oldColorHex}' -> '{dto.ColorHex}'"); + return parts.Count > 0 ? string.Join("; ", parts) : "no fields changed"; + } + public async Task> UpdateLabelAsync(Guid boardId, Guid id, UpdateLabelDto dto, CancellationToken cancellationToken = default) { var label = await _unitOfWork.Labels.GetByIdAsync(id, cancellationToken); diff --git a/backend/tests/Taskdeck.Application.Tests/Services/BoardMutationAuditTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/BoardMutationAuditTests.cs index d3e91114f..60e90bc1f 100644 --- a/backend/tests/Taskdeck.Application.Tests/Services/BoardMutationAuditTests.cs +++ b/backend/tests/Taskdeck.Application.Tests/Services/BoardMutationAuditTests.cs @@ -78,7 +78,7 @@ public async Task CreateCard_RecordsAuditLog() } [Fact] - public async Task UpdateCard_RecordsAuditLog() + public async Task UpdateCard_RecordsAuditLog_WithChangedFieldDetails() { // Arrange var board = TestDataBuilder.CreateBoard(); @@ -96,7 +96,8 @@ public async Task UpdateCard_RecordsAuditLog() // Assert result.IsSuccess.Should().BeTrue(); _historyServiceMock.Verify( - h => h.LogActionAsync("card", card.Id, AuditAction.Updated, It.IsAny(), It.IsAny()), + h => h.LogActionAsync("card", card.Id, AuditAction.Updated, It.IsAny(), + It.Is(s => s != null && s.Contains("Title:") && s.Contains("'Card'") && s.Contains("'Updated'"))), Times.Once); } @@ -192,6 +193,107 @@ public async Task DeleteColumn_RecordsAuditLog() Times.Once); } + [Fact] + public async Task UpdateCard_WithMultipleFieldChanges_RecordsAllChangedFields() + { + // Arrange + var board = TestDataBuilder.CreateBoard(); + var column = TestDataBuilder.CreateColumn(board.Id, "To Do"); + var card = TestDataBuilder.CreateCard(board.Id, column.Id, "Card", "Desc"); + var dueDate = DateTimeOffset.UtcNow.AddDays(7); + var dto = new UpdateCardDto("New Title", "New Desc", dueDate, true, "Waiting on review", null, null); + var service = new CardService(_unitOfWorkMock.Object, realtimeNotifier: null, historyService: _historyServiceMock.Object); + + _cardRepoMock.Setup(r => r.GetByIdWithLabelsAsync(card.Id, default)).ReturnsAsync(card); + _cardRepoMock.Setup(r => r.GetByIdAsync(card.Id, default)).ReturnsAsync(card); + + // Act + var result = await service.UpdateCardAsync(card.Id, dto); + + // Assert + result.IsSuccess.Should().BeTrue(); + _historyServiceMock.Verify( + h => h.LogActionAsync("card", card.Id, AuditAction.Updated, It.IsAny(), + It.Is(s => s != null + && s.Contains("Title:") + && s.Contains("Description changed") + && s.Contains("DueDate:") + && s.Contains("Blocked:"))), + Times.Once); + } + + [Fact] + public async Task UpdateCard_WithNoChangedFields_RecordsNoFieldsChanged() + { + // Arrange + var board = TestDataBuilder.CreateBoard(); + var column = TestDataBuilder.CreateColumn(board.Id, "To Do"); + var card = TestDataBuilder.CreateCard(board.Id, column.Id, "Card", "Desc"); + var dto = new UpdateCardDto(null, null, null, null, null, null, null); + var service = new CardService(_unitOfWorkMock.Object, realtimeNotifier: null, historyService: _historyServiceMock.Object); + + _cardRepoMock.Setup(r => r.GetByIdWithLabelsAsync(card.Id, default)).ReturnsAsync(card); + _cardRepoMock.Setup(r => r.GetByIdAsync(card.Id, default)).ReturnsAsync(card); + + // Act + var result = await service.UpdateCardAsync(card.Id, dto); + + // Assert + result.IsSuccess.Should().BeTrue(); + _historyServiceMock.Verify( + h => h.LogActionAsync("card", card.Id, AuditAction.Updated, It.IsAny(), + It.Is(s => s != null && s.Contains("no fields changed"))), + Times.Once); + } + + #endregion + + #region ColumnService Audit Tests — Update Details + + [Fact] + public async Task UpdateColumn_RecordsAuditLog_WithChangedFieldDetails() + { + // Arrange + var board = TestDataBuilder.CreateBoard(); + var column = TestDataBuilder.CreateColumn(board.Id, "To Do"); + var dto = new UpdateColumnDto("Done", null, null); + var service = new ColumnService(_unitOfWorkMock.Object, realtimeNotifier: null, historyService: _historyServiceMock.Object); + + _columnRepoMock.Setup(r => r.GetByIdAsync(column.Id, default)).ReturnsAsync(column); + + // Act + var result = await service.UpdateColumnAsync(column.Id, dto); + + // Assert + result.IsSuccess.Should().BeTrue(); + _historyServiceMock.Verify( + h => h.LogActionAsync("column", column.Id, AuditAction.Updated, null, + It.Is(s => s != null && s.Contains("Name:") && s.Contains("'To Do'") && s.Contains("'Done'"))), + Times.Once); + } + + [Fact] + public async Task UpdateColumn_WithWipLimitChange_RecordsWipLimitDetail() + { + // Arrange + var board = TestDataBuilder.CreateBoard(); + var column = TestDataBuilder.CreateColumn(board.Id, "In Progress"); + var dto = new UpdateColumnDto(null, null, 5); + var service = new ColumnService(_unitOfWorkMock.Object, realtimeNotifier: null, historyService: _historyServiceMock.Object); + + _columnRepoMock.Setup(r => r.GetByIdAsync(column.Id, default)).ReturnsAsync(column); + + // Act + var result = await service.UpdateColumnAsync(column.Id, dto); + + // Assert + result.IsSuccess.Should().BeTrue(); + _historyServiceMock.Verify( + h => h.LogActionAsync("column", column.Id, AuditAction.Updated, null, + It.Is(s => s != null && s.Contains("WipLimit:"))), + Times.Once); + } + #endregion #region BoardService Audit Tests @@ -219,7 +321,7 @@ public async Task CreateBoard_RecordsAuditLog() } [Fact] - public async Task UpdateBoard_RecordsAuditLog() + public async Task UpdateBoard_RecordsAuditLog_WithChangedFieldDetails() { // Arrange var board = TestDataBuilder.CreateBoard(); @@ -238,7 +340,8 @@ public async Task UpdateBoard_RecordsAuditLog() // Assert result.IsSuccess.Should().BeTrue(); _historyServiceMock.Verify( - h => h.LogActionAsync("board", board.Id, AuditAction.Updated, null, It.IsAny()), + h => h.LogActionAsync("board", board.Id, AuditAction.Updated, null, + It.Is(s => s != null && s.Contains("Name:") && s.Contains("'Renamed'"))), Times.Once); } @@ -309,6 +412,52 @@ public async Task DeleteLabel_RecordsAuditLog() Times.Once); } + [Fact] + public async Task UpdateLabel_RecordsAuditLog_WithChangedFieldDetails() + { + // Arrange + var board = TestDataBuilder.CreateBoard(); + var label = TestDataBuilder.CreateLabel(board.Id, "Bug", "#FF0000"); + var dto = new UpdateLabelDto("Feature", "#00FF00"); + var service = new LabelService(_unitOfWorkMock.Object, realtimeNotifier: null, historyService: _historyServiceMock.Object); + + _labelRepoMock.Setup(r => r.GetByIdAsync(label.Id, default)).ReturnsAsync(label); + + // Act + var result = await service.UpdateLabelAsync(label.Id, dto); + + // Assert + result.IsSuccess.Should().BeTrue(); + _historyServiceMock.Verify( + h => h.LogActionAsync("label", label.Id, AuditAction.Updated, null, + It.Is(s => s != null + && s.Contains("Name: 'Bug' -> 'Feature'") + && s.Contains("Color: '#FF0000' -> '#00FF00'"))), + Times.Once); + } + + [Fact] + public async Task UpdateLabel_WithOnlyNameChange_RecordsOnlyNameDetail() + { + // Arrange + var board = TestDataBuilder.CreateBoard(); + var label = TestDataBuilder.CreateLabel(board.Id, "Bug", "#FF0000"); + var dto = new UpdateLabelDto("Critical", null); + var service = new LabelService(_unitOfWorkMock.Object, realtimeNotifier: null, historyService: _historyServiceMock.Object); + + _labelRepoMock.Setup(r => r.GetByIdAsync(label.Id, default)).ReturnsAsync(label); + + // Act + var result = await service.UpdateLabelAsync(label.Id, dto); + + // Assert + result.IsSuccess.Should().BeTrue(); + _historyServiceMock.Verify( + h => h.LogActionAsync("label", label.Id, AuditAction.Updated, null, + It.Is(s => s != null && s.Contains("Name:") && !s.Contains("Color:"))), + Times.Once); + } + #endregion #region No History Service (backward compatibility) @@ -370,7 +519,7 @@ public async Task CreateCard_AuditFailure_DoesNotCrashMutation() } [Fact] - public async Task UpdateBoard_WithArchiveFlag_RecordsArchivedAction() + public async Task UpdateBoard_WithArchiveFlag_RecordsArchivedAction_WithDetails() { // Arrange var board = TestDataBuilder.CreateBoard(); @@ -386,10 +535,11 @@ public async Task UpdateBoard_WithArchiveFlag_RecordsArchivedAction() // Act var result = await service.UpdateBoardAsync(board.Id, dto); - // Assert - should record Archived, not Updated + // Assert - should record Archived action with change details result.IsSuccess.Should().BeTrue(); _historyServiceMock.Verify( - h => h.LogActionAsync("board", board.Id, AuditAction.Archived, null, It.Is(s => s != null && s.Contains("name="))), + h => h.LogActionAsync("board", board.Id, AuditAction.Archived, null, + It.Is(s => s != null && s.Contains("Archived"))), Times.Once); } From c728c0079a6a9041058f020390bdea4da41841ab Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Mon, 30 Mar 2026 00:34:11 +0100 Subject: [PATCH 2/3] Fix audit log: add equality checks to suppress unchanged-value noise - CardService: skip logging Title/Description/DueDate when value matches old - BoardService: skip logging Name/Description when value matches old - ColumnService: skip logging Name/WipLimit/Position when value matches old - LabelService: skip logging Name/ColorHex when value matches old --- .../Taskdeck.Application/Services/BoardService.cs | 8 ++++---- .../Taskdeck.Application/Services/CardService.cs | 15 ++++++++++----- .../Services/ColumnService.cs | 8 ++++---- .../Taskdeck.Application/Services/LabelService.cs | 6 +++--- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/backend/src/Taskdeck.Application/Services/BoardService.cs b/backend/src/Taskdeck.Application/Services/BoardService.cs index 79ea94160..c7ba000eb 100644 --- a/backend/src/Taskdeck.Application/Services/BoardService.cs +++ b/backend/src/Taskdeck.Application/Services/BoardService.cs @@ -1,4 +1,4 @@ -using Taskdeck.Application.DTOs; +using Taskdeck.Application.DTOs; using Taskdeck.Application.Interfaces; using Taskdeck.Domain.Common; using Taskdeck.Domain.Entities; @@ -194,7 +194,7 @@ await _realtimeNotifier.NotifyBoardMutationAsync( if (dto.IsArchived == true) await SafeLogAsync("board", board.Id, AuditAction.Archived, changes: changeSummary); else if (dto.IsArchived == false) - await SafeLogAsync("board", board.Id, AuditAction.Updated, changes: changeSummary); + await SafeLogAsync("board", board.Id, AuditAction.Unarchived, changes: changeSummary); else await SafeLogAsync("board", board.Id, AuditAction.Updated, changes: changeSummary); return Result.Success(MapToDto(board)); @@ -208,9 +208,9 @@ await _realtimeNotifier.NotifyBoardMutationAsync( private static string BuildBoardChangeSummary(UpdateBoardDto dto, string oldName, string? oldDescription, bool oldIsArchived) { var parts = new List(); - if (dto.Name != null) + if (dto.Name != null && dto.Name != oldName) parts.Add($"Name: '{oldName}' -> '{dto.Name}'"); - if (dto.Description != null) + if (dto.Description != null && dto.Description != oldDescription) parts.Add($"Description changed"); if (dto.IsArchived.HasValue && dto.IsArchived.Value != oldIsArchived) parts.Add(dto.IsArchived.Value ? "Archived" : "Unarchived"); diff --git a/backend/src/Taskdeck.Application/Services/CardService.cs b/backend/src/Taskdeck.Application/Services/CardService.cs index 172a2137c..5ed5a1adf 100644 --- a/backend/src/Taskdeck.Application/Services/CardService.cs +++ b/backend/src/Taskdeck.Application/Services/CardService.cs @@ -1,4 +1,4 @@ -using Taskdeck.Application.DTOs; +using Taskdeck.Application.DTOs; using Taskdeck.Application.Interfaces; using Taskdeck.Domain.Common; using Taskdeck.Domain.Entities; @@ -175,14 +175,19 @@ private static string BuildCardChangeSummary( List oldLabelIds) { var parts = new List(); - if (dto.Title != null) + if (dto.Title != null && dto.Title != oldTitle) parts.Add($"Title: '{oldTitle}' -> '{dto.Title}'"); - if (dto.Description != null) + if (dto.Description != null && dto.Description != oldDescription) parts.Add("Description changed"); - if (dto.DueDate.HasValue) + if (dto.DueDate.HasValue && dto.DueDate.Value != oldDueDate) parts.Add($"DueDate: '{oldDueDate?.ToString("O") ?? "none"}' -> '{dto.DueDate.Value:O}'"); if (dto.IsBlocked.HasValue && dto.IsBlocked.Value != oldIsBlocked) - parts.Add(dto.IsBlocked.Value ? $"Blocked: {dto.BlockReason ?? "no reason"}" : "Unblocked"); + { + if (dto.IsBlocked.Value && !string.IsNullOrEmpty(dto.BlockReason) && !oldIsBlocked) + parts.Add($"Blocked: {dto.BlockReason}"); + else if (!dto.IsBlocked.Value && oldIsBlocked) + parts.Add("Unblocked"); + } if (dto.LabelIds != null) { var newLabelIds = dto.LabelIds.OrderBy(id => id).ToList(); diff --git a/backend/src/Taskdeck.Application/Services/ColumnService.cs b/backend/src/Taskdeck.Application/Services/ColumnService.cs index 006262681..749d6aa44 100644 --- a/backend/src/Taskdeck.Application/Services/ColumnService.cs +++ b/backend/src/Taskdeck.Application/Services/ColumnService.cs @@ -1,4 +1,4 @@ -using Taskdeck.Application.DTOs; +using Taskdeck.Application.DTOs; using Taskdeck.Application.Interfaces; using Taskdeck.Domain.Common; using Taskdeck.Domain.Entities; @@ -98,11 +98,11 @@ await _realtimeNotifier.NotifyBoardMutationAsync( private static string BuildColumnChangeSummary(UpdateColumnDto dto, string oldName, int? oldWipLimit, int oldPosition) { var parts = new List(); - if (dto.Name != null) + if (dto.Name != null && dto.Name != oldName) parts.Add($"Name: '{oldName}' -> '{dto.Name}'"); - if (dto.WipLimit.HasValue) + if (dto.WipLimit.HasValue && dto.WipLimit.Value != oldWipLimit) parts.Add($"WipLimit: {oldWipLimit?.ToString() ?? "none"} -> {dto.WipLimit.Value}"); - if (dto.Position.HasValue) + if (dto.Position.HasValue && dto.Position.Value != oldPosition) parts.Add($"Position: {oldPosition} -> {dto.Position.Value}"); return parts.Count > 0 ? string.Join("; ", parts) : "no fields changed"; } diff --git a/backend/src/Taskdeck.Application/Services/LabelService.cs b/backend/src/Taskdeck.Application/Services/LabelService.cs index f7678b6bd..7a1d25079 100644 --- a/backend/src/Taskdeck.Application/Services/LabelService.cs +++ b/backend/src/Taskdeck.Application/Services/LabelService.cs @@ -1,4 +1,4 @@ -using Taskdeck.Application.DTOs; +using Taskdeck.Application.DTOs; using Taskdeck.Application.Interfaces; using Taskdeck.Domain.Common; using Taskdeck.Domain.Entities; @@ -88,9 +88,9 @@ await _realtimeNotifier.NotifyBoardMutationAsync( private static string BuildLabelChangeSummary(UpdateLabelDto dto, string oldName, string oldColorHex) { var parts = new List(); - if (dto.Name != null) + if (dto.Name != null && dto.Name != oldName) parts.Add($"Name: '{oldName}' -> '{dto.Name}'"); - if (dto.ColorHex != null) + if (dto.ColorHex != null && dto.ColorHex != oldColorHex) parts.Add($"Color: '{oldColorHex}' -> '{dto.ColorHex}'"); return parts.Count > 0 ? string.Join("; ", parts) : "no fields changed"; } From 1620eeaa697a5104dcb4dc3ca5727c8086b1b77d Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Mon, 30 Mar 2026 00:34:12 +0100 Subject: [PATCH 3/3] Add tests for equality-check audit suppression and unarchive action - UpdateCard_WithSameTitle_DoesNotLogTitleChange - UpdateCard_BlockWithoutReason_DoesNotLogBlocked - UpdateBoard_WithUnarchiveFlag_RecordsUnarchivedAction --- .../Services/BoardMutationAuditTests.cs | 77 ++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/backend/tests/Taskdeck.Application.Tests/Services/BoardMutationAuditTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/BoardMutationAuditTests.cs index 60e90bc1f..b818b2715 100644 --- a/backend/tests/Taskdeck.Application.Tests/Services/BoardMutationAuditTests.cs +++ b/backend/tests/Taskdeck.Application.Tests/Services/BoardMutationAuditTests.cs @@ -1,4 +1,4 @@ -using FluentAssertions; +using FluentAssertions; using Moq; using Taskdeck.Application.DTOs; using Taskdeck.Application.Interfaces; @@ -544,4 +544,79 @@ public async Task UpdateBoard_WithArchiveFlag_RecordsArchivedAction_WithDetails( } #endregion + + [Fact] + public async Task UpdateBoard_WithUnarchiveFlag_RecordsUnarchivedAction() + { + // Arrange - board starts archived + var board = TestDataBuilder.CreateBoard(isArchived: true); + var dto = new UpdateBoardDto(null, null, false); + var service = new BoardService( + _unitOfWorkMock.Object, + authorizationService: null, + realtimeNotifier: null, + historyService: _historyServiceMock.Object); + + _boardRepoMock.Setup(r => r.GetByIdAsync(board.Id, default)).ReturnsAsync(board); + + // Act + var result = await service.UpdateBoardAsync(board.Id, dto); + + // Assert - should record Unarchived, not Updated + result.IsSuccess.Should().BeTrue(); + _historyServiceMock.Verify( + h => h.LogActionAsync("board", board.Id, AuditAction.Unarchived, null, + It.Is(s => s != null && s.Contains("Unarchived"))), + Times.Once); + } + + [Fact] + public async Task UpdateCard_WithSameTitle_DoesNotLogTitleChange() + { + // Arrange + var board = TestDataBuilder.CreateBoard(); + var column = TestDataBuilder.CreateColumn(board.Id, "To Do"); + var card = TestDataBuilder.CreateCard(board.Id, column.Id, "Card", "Desc"); + // Send the same title value + var dto = new UpdateCardDto("Card", null, null, null, null, null, null); + var service = new CardService(_unitOfWorkMock.Object, realtimeNotifier: null, historyService: _historyServiceMock.Object); + + _cardRepoMock.Setup(r => r.GetByIdWithLabelsAsync(card.Id, default)).ReturnsAsync(card); + _cardRepoMock.Setup(r => r.GetByIdAsync(card.Id, default)).ReturnsAsync(card); + + // Act + var result = await service.UpdateCardAsync(card.Id, dto); + + // Assert - same title should not be logged as a change + result.IsSuccess.Should().BeTrue(); + _historyServiceMock.Verify( + h => h.LogActionAsync("card", card.Id, AuditAction.Updated, It.IsAny(), + It.Is(s => s != null && s.Contains("no fields changed"))), + Times.Once); + } + + [Fact] + public async Task UpdateCard_BlockWithoutReason_DoesNotLogBlocked() + { + // Arrange + var board = TestDataBuilder.CreateBoard(); + var column = TestDataBuilder.CreateColumn(board.Id, "To Do"); + var card = TestDataBuilder.CreateCard(board.Id, column.Id, "Card", "Desc"); + // IsBlocked=true but no block reason + var dto = new UpdateCardDto(null, null, null, true, null, null, null); + var service = new CardService(_unitOfWorkMock.Object, realtimeNotifier: null, historyService: _historyServiceMock.Object); + + _cardRepoMock.Setup(r => r.GetByIdWithLabelsAsync(card.Id, default)).ReturnsAsync(card); + _cardRepoMock.Setup(r => r.GetByIdAsync(card.Id, default)).ReturnsAsync(card); + + // Act + var result = await service.UpdateCardAsync(card.Id, dto); + + // Assert - block without reason should not log "Blocked" + result.IsSuccess.Should().BeTrue(); + _historyServiceMock.Verify( + h => h.LogActionAsync("card", card.Id, AuditAction.Updated, It.IsAny(), + It.Is(s => s != null && !s.Contains("Blocked"))), + Times.Once); + } }