MR review: interacting with existing comments - POC
Problem to solve
The #243 (closed) proof of concept work didn't validate the workflows for creating and updating comments on the MR diff. The VS Code API for creating/editing comments is not documented and so we have to result to try to copy a reference implementation like GitHub pull request extension.
This issue is to create POC that validates the implementation of interacting with existing comments (responding, resolving, deleting, possibly reactions). Creating new comments and reviews is going to be out of scope for this POC.
Proposal
- Investigate VS Code API's for triggering actions on comments (edit, (un)resolve, delete, report abuse?)
- Implement POC for editing comments.
- Investigate VS Code API's for access control (i.e. only show users actions that are available to them (e.g. only the author can edit the comment).
- validate that the GraphQL queries and mutations are in place
- if time allows, investigate how to implement reactions (is the VS Code reaction API compatible with GitLab?)
Further details
- Explanation of how the current MR review POC works is here: #243 (closed)
POC Result
TL;DR: This POC implemented editing and resolving comments, validating the presence of all APIs (both GitLab and VS Code). The only feature that we are not able to implement straight away is emoji reactions.
All code is available in the existing-comments-poc
branch.
✅ Editing comments
GitLab
We'll use adminNote
permission attribute (see Appendix 2) to decide whether the user can edit a note. GitLab GraphQL API provides an updateNote
mutation that will change the note body.
VS Code
We will add a pencil-shaped icon to the comment title (see Appendix 1). Clicking this icon will change the note mode to vscode.CommentMode.Editing
. We add two comment context action (see Appendix 1) Submit
and Cancel
. Submit will trigger the GraphQL mutation, cancel reverts any changes to the comment text.
We also add the canAdmin
permission attribute on the comment context so the VS Code can decide whether to show the edit button or not.
⚠ Caveyats
- Comment drafts: VS Code is not telling us when the comment editing is in progress. We only get notified when the user triggers one of the context actions (submit/cancel). If users close the editor or refresh the MR during editing, they'll lose the draft and we can't do anything about it.
✔ Deleting comments
I've validated the API, but haven't implemented this feature.
Very similar to editing comments without the need to switch comment into editing mode.
We will show a warning message before removing the comment:
const shouldDelete = await vscode.window.showWarningMessage('Delete comment?', { modal: true }, 'Delete');
✅ Resolving/Unresolving discussions
GitLab
We'll use resolvable
permission attribute (see Appendix 2) to decide whether we can let user resolve/unresolve a discussion. GitLab GraphQL API provides a discussionToggleResolve
mutation that will set the resolved
attribute.
VS Code
We will add a check
icon to the thread title (see Appendix 1). Clicking this icon will resolve the thread and it will set the thread context to resolved
. VS Code recognises the context change and it will show close
icon instead.
⚠ Caveyats
- The
cancel
andcheck
icons might not accurately show the user what is going to happen. At least they have a tooltip that shows on hover
❌ Emoji reactions
Implementing reactions is going to be facing issues on both the GitLab and the VS Code side.
❌ GitLab
We don't have a GraphQL endpoint to retrieve reactions for a note. Issue: gitlab#320963 (closed)
We do have an endpoint to add or remove a reaction:
mutation{
awardEmojiAdd(input:{awardableId: "gid://gitlab/DiffNote/447164415", name: "tada"}){
errors
}
}
⚠ VS Code
The VS Code extension API is very much GitHub oriented. It is made to support half a dozen predefined reactions, not a full emoji selection as we have at GitLab. You can see that the emoji picker doesn't scale above maybe 10 emojis. Also, each emoji needs to have a PNG icon in the project.
Implementation notes
- add a
commentHandler
commentController.reactionHandler = async ( comment: vscode.Comment, reaction: vscode.CommentReaction, ): Promise<void> => {};
- Each comment can have
reactions
. If they havecount === 0
, they don't show at the bottom of the commentreactions: [ { label: '✅', iconPath: '$(check)', count: 1, authorHasReacted: true, }, ],
- Each reaction has to have a PNG icon otherwise, it shows up as
✔ Bonus: Open the comment in a browser
Since we are most likely not going to be adding the "Report Abuse" button and also because it might be difficult to introduce reactions from the get-go, I suggested we include globe
button in the title of each note. When the user clicks this button, VS Code will open the comment permalink in the browser.
The POC haven't fully implemented this feature, but all the necessary information (MR URL and note ID) is available.
Appendix 1: Context menu on comment threads and comments
Extensions can contribute menus. The important menus for the MR Review comments are:
-
The comment thread title menu bar -
comments/commentThread/title
-
The comment thread context menu -
comments/commentThread/context
Appendix 2: Access control
on each note
Field | Type | Description |
---|---|---|
adminNote | Boolean! | Indicates the user can perform admin_note on this resource |
awardEmoji | Boolean! | Indicates the user can perform award_emoji on this resource |
createNote | Boolean! | Indicates the user can perform create_note on this resource |
readNote | Boolean! | Indicates the user can perform read_note on this resource |
repositionNote | Boolean! | Indicates the user can perform reposition_note on this resource |
resolveNote | Boolean! | Indicates the user can perform resolve_note on this resource |
on merge request
We need the createNote
value to know whether the user can respond to a discussion.
mergeRequest(iid: "3") {
id
webUrl
userPermissions{
createNote
}
}
on a discussion
A discussion contains a boolean field resolvable
indicating whether the discussion can be resolved or not.