-
Notifications
You must be signed in to change notification settings - Fork 221
[API View] Add bulk comment operations support #12944
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[API View] Add bulk comment operations support #12944
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request adds bulk comment operations to APIView, enabling users to efficiently manage multiple related comments through a unified dialog interface. The implementation supports three dispositions (Keep Open, Resolve, Delete), allows bulk severity updates, and enables applying votes across multiple comments.
Key changes:
- Introduced a
commentsBatchOperationAPI endpoint to replace the previousresolveBatchComments, supporting multiple dispositions and severity updates - Created reusable
CommentSeverityHelperutility class to centralize severity-related logic and eliminate code duplication - Added
CommentSeverityComponentfor consistent severity display and editing across the application
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/dotnet/APIView/ClientSPA/src/app/_services/comments/comments.service.ts | Updated service method to support disposition and severity parameters in batch operations |
| src/dotnet/APIView/ClientSPA/src/app/_modules/shared/review-page-layout.module.ts | Added CommentSeverityComponent to module declarations and exports |
| src/dotnet/APIView/ClientSPA/src/app/_helpers/comment-severity.helper.ts | New utility class centralizing severity normalization, labeling, and styling logic |
| src/dotnet/APIView/ClientSPA/src/app/_components/shared/related-comments-dialog/related-comments-dialog.component.ts | Enhanced dialog to support disposition selection, severity management, and dynamic placeholder text |
| src/dotnet/APIView/ClientSPA/src/app/_components/shared/related-comments-dialog/related-comments-dialog.component.scss | Added styling for bulk actions section, severity dropdowns, and disposition menus |
| src/dotnet/APIView/ClientSPA/src/app/_components/shared/related-comments-dialog/related-comments-dialog.component.html | Updated UI to include disposition dropdown, severity selector, and reorganized action controls |
| src/dotnet/APIView/ClientSPA/src/app/_components/shared/comment-thread/comment-thread.component.ts | Refactored to use CommentSeverityHelper, updated batch operation handling to support new dispositions |
| src/dotnet/APIView/ClientSPA/src/app/_components/shared/comment-severity/comment-severity.component.ts | New component providing reusable severity display with click-to-edit or always-visible dropdown modes |
| src/dotnet/APIView/ClientSPA/src/app/_components/shared/comment-severity/comment-severity.component.scss | Styling for severity badges, dropdowns, and dark mode support |
| src/dotnet/APIView/ClientSPA/src/app/_components/shared/comment-severity/comment-severity.component.html | Template supporting both editable and read-only severity display modes |
| src/dotnet/APIView/ClientSPA/src/app/_components/code-panel/code-panel.component.ts | Enhanced comment update handling to support severity changes via CommentTextUpdate action |
| src/dotnet/APIView/APIViewWeb/Managers/Interfaces/ICommentsManager.cs | Renamed interface method from ResolveBatchConversationAsync to CommentsBatchOperationAsync |
| src/dotnet/APIView/APIViewWeb/Managers/CommentsManager.cs | Implemented batch operation logic supporting multiple dispositions, severity updates, and reply creation |
| src/dotnet/APIView/APIViewWeb/LeanModels/ResolveBatchConversationRequest.cs | Added ConversationDisposition enum and Severity/Disposition fields to request model |
| src/dotnet/APIView/APIViewWeb/LeanControllers/CommentsController.cs | Renamed endpoint from resolveBatchComments to commentsBatchOperation |
| src/dotnet/APIView/APIViewUnitTests/CommentsManagerTests.cs | Added comprehensive tests for new batch operation functionality including KeepOpen, Delete, and severity update scenarios |
Comments suppressed due to low confidence (2)
src/dotnet/APIView/APIViewWeb/Managers/CommentsManager.cs:227
- Missing SignalR notification: The
UpdateCommentSeverityAsyncmethod updates the comment severity but doesn't send a SignalR notification to inform other connected clients about the change. This is inconsistent with similar methods likeUpdateCommentAsync(lines 190-200) which do send notifications.
This could result in severity changes not being reflected in real-time for other users viewing the same review.
Consider adding a SignalR notification after line 226:
await _commentsRepository.UpsertCommentAsync(comment);
await _signalRHubContext.Clients.All.SendAsync("ReceiveCommentUpdates",
new CommentUpdatesDto()
{
CommentThreadUpdateAction = CommentThreadUpdateAction.CommentTextUpdate,
CommentId = comment.Id,
ReviewId = comment.ReviewId,
ElementId = comment.ElementId,
NodeId = comment.ElementId,
Severity = comment.Severity,
});
return comment; public async Task<CommentItemModel> UpdateCommentSeverityAsync(ClaimsPrincipal user, CommentItemModel comment, CommentSeverity? severity)
{
await AssertOwnerAsync(user, comment);
comment.ChangeHistory.Add(
new CommentChangeHistoryModel()
{
ChangeAction = CommentChangeAction.Edited,
ChangedBy = user.GetGitHubLogin(),
ChangedOn = DateTime.Now,
});
comment.LastEditedOn = DateTime.Now;
comment.Severity = severity;
await _commentsRepository.UpsertCommentAsync(comment);
return comment;
src/dotnet/APIView/APIViewUnitTests/CommentsManagerTests.cs:524
- Missing test case: There should be a test that verifies behavior when multiple comments with the same
ElementIdare selected for batch resolution. This would help identify the performance issue whereResolveConversationis called multiple times for the same element.
Suggested test case:
[Fact]
public async Task CommentsBatchOperationAsync_MultipleCommentsOnSameElement_ResolvesOnce()
{
// Setup two comments on the same element
var comment1 = new CommentItemModel
{
Id = "comment1",
ElementId = "element1",
// ...
};
var comment2 = new CommentItemModel
{
Id = "comment2",
ElementId = "element1", // Same element
// ...
};
commentsRepoMock.Setup(r => r.GetCommentsAsync("review1", "element1"))
.ReturnsAsync(new List<CommentItemModel> { comment1, comment2 });
var request = new ResolveBatchConversationRequest
{
CommentIds = new List<string> { "comment1", "comment2" },
Disposition = ConversationDisposition.Resolve
};
await manager.CommentsBatchOperationAsync(user, "review1", request);
// Verify GetCommentsAsync for element1 is called only once, not twice
commentsRepoMock.Verify(r => r.GetCommentsAsync("review1", "element1"), Times.Once);
} [Fact]
public async Task ResolveBatchConversationAsync_MultipleComments_ProcessesAllInOrder()
{
CommentsManager manager = CreateManager(out Mock<ICosmosCommentsRepository> commentsRepoMock, out _);
ClaimsPrincipal user = CreateUser("test-user");
List<CommentItemModel> comments = new()
{
new() { ReviewId = "review1", Id = "comment1", ElementId = "element1", Upvotes = new List<string>(), Downvotes = new List<string>(), Severity = CommentSeverity.ShouldFix},
new() { ReviewId = "review1", Id = "comment2", ElementId = "element2", Upvotes = new List<string>(), Downvotes = new List<string>(), Severity = CommentSeverity.ShouldFix },
new() { ReviewId = "review1", Id = "comment3", ElementId = "element3", Upvotes = new List<string>(), Downvotes = new List<string>(), Severity = CommentSeverity.ShouldFix }
};
foreach (var comment in comments)
{
commentsRepoMock.Setup(r => r.GetCommentAsync("review1", comment.Id)).ReturnsAsync(comment);
commentsRepoMock.Setup(r => r.GetCommentsAsync("review1", comment.ElementId))
.ReturnsAsync(new List<CommentItemModel> { comment });
}
ResolveBatchConversationRequest request = new()
{
CommentIds = new List<string> { "comment1", "comment2", "comment3" },
Vote = FeedbackVote.Up,
Severity = CommentSeverity.ShouldFix,
Disposition = ConversationDisposition.Resolve
};
await manager.CommentsBatchOperationAsync(user, "review1", request);
foreach (var comment in comments)
{
Assert.Single(comment.Upvotes);
Assert.Contains("test-user", comment.Upvotes);
Assert.True(comment.IsResolved);
}
commentsRepoMock.Verify(r => r.UpsertCommentAsync(It.IsAny<CommentItemModel>()), Times.Exactly(6)); // 3 votes + 3 resolves
}
...PIView/ClientSPA/src/app/_components/shared/comment-severity/comment-severity.component.html
Show resolved
Hide resolved
src/dotnet/APIView/APIViewWeb/LeanModels/ResolveBatchConversationRequest.cs
Show resolved
Hide resolved
src/dotnet/APIView/APIViewWeb/LeanModels/ResolveBatchConversationRequest.cs
Outdated
Show resolved
Hide resolved
...PA/src/app/_components/shared/related-comments-dialog/related-comments-dialog.component.html
Show resolved
Hide resolved
tjprescott
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @AlitzelMendez!
closes: #12874
Summary
Adds bulk comment operations to APIView, allowing users to perform actions on multiple related comments simultaneously through a unified dialog interface.
Changes
Features
Demo
bulk_resolve_bulk_operation.mp4
Validated in different resolutions / screen sizes
I made some adjustments to fit better on different resolutions unfortunately we can't provide the exact behavoir on all of them, couple of examples


