Skip to content

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

poc-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');

delete-confirmation

Resolving/Unresolving discussions

poc-resolve-thread

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 and check icons might not accurately show the user what is going to happen. At least they have a tooltip that shows on hover Screenshot_2021-02-09_at_17.52.00

List of all supported icions

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.

700face67d864dd78fb55f15934f39cc

Implementation notes
  1. add a commentHandler
    commentController.reactionHandler = async (
      comment: vscode.Comment,
      reaction: vscode.CommentReaction,
    ): Promise<void> => {};
  2. Each comment can have reactions. If they have count === 0, they don't show at the bottom of the comment
    reactions: [
      {
    	label: '',
    	iconPath: '$(check)',
    	count: 1,
    	authorHasReacted: true,
      },
    ],
  3. Each reaction has to have a PNG icon otherwise, it shows up as e8eecbc1d7624b87bf6f8ec3d951ca69

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.

poc-comment-on-the-web

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 Extension_Development_Host_-git_service_ts___173_—_gitlab-vscode-extension-TEST.png

  • The comment thread context menu - comments/commentThread/context Extension_Development_Host_-git_service_ts___173_—_gitlab-vscode-extension-TEST.png

  • The comment title menu bar - comments/comment/title comments/comment/title

  • The comment context menu - comments/comment/context Extension_Development_Host_-git_service_ts___173_—_gitlab-vscode-extension-TEST.png

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.

Edited by Tomas Vik