refactor: only show commenting ranges if user can comment

src/data_providers/items/mr_item_model.test.ts

@@ -7,6 +7,7 @@ import {
   multipleNotes,
 } from '../../../test/integration/fixtures/graphql/discussions.js';
 import { createGitLabNewService } from '../../service_factory';
+import { CommentingRangeProvider } from '../../review/commenting_range_provider';
 
 jest.mock('../../service_factory');
 
@@ -16,6 +17,8 @@ const createGitLabNewServiceMock = createGitLabNewService as jest.Mock;
 describe('MrItemModel', () => {
   let item: MrItemModel;
   let commentThread: vscode.CommentThread;
+  let canUserCommentOnMr = false;
+  let commentController: any;
 
   const createCommentThreadMock = jest.fn();
 
@@ -23,12 +26,14 @@ describe('MrItemModel', () => {
     item = new MrItemModel(mr, workspace);
     commentThread = {} as vscode.CommentThread;
 
-    createCommentControllerMock.mockReturnValue({
+    commentController = {
       createCommentThread: createCommentThreadMock.mockReturnValue(commentThread),
-    });
+    };
+    createCommentControllerMock.mockReturnValue(commentController);
     createGitLabNewServiceMock.mockReturnValue({
       getDiscussions: jest.fn().mockResolvedValue([discussionOnDiff, multipleNotes]),
       getMrDiff: jest.fn().mockResolvedValue({ diffs: [] }),
+      canUserCommentOnMr: jest.fn(() => canUserCommentOnMr),
     });
   });
 
@@ -49,4 +54,22 @@ describe('MrItemModel', () => {
     expect(firstComment.mode).toBe(vscode.CommentMode.Preview);
     expect(firstComment.body).toMatch(noteOnDiffTextSnippet);
   });
+
+  describe('commenting range', () => {
+    it('should not add a commenting range provider if user does not have permission to comment', async () => {
+      canUserCommentOnMr = false;
+
+      await item.getChildren();
+
+      expect(commentController.commentingRangeProvider).toBe(undefined);
+    });
+
+    it('should add a commenting range provider if user has permission to comment', async () => {
+      canUserCommentOnMr = true;
+
+      await item.getChildren();
+
+      expect(commentController.commentingRangeProvider).toBeInstanceOf(CommentingRangeProvider);
+    });
+  });
 });

src/data_providers/items/mr_item_model.ts

@@ -42,7 +42,7 @@ export class MrItemModel extends ItemModel {
     const gitlabService = await createGitLabNewService(this.workspace.uri);
     const mrVersion = await gitlabService.getMrDiff(this.mr);
     try {
-      await this.getMrDiscussions(mrVersion);
+      await this.initializeMrDiscussions(mrVersion);
     } catch (e) {
       handleError(
         new UserFriendlyError(
@@ -60,16 +60,17 @@ export class MrItemModel extends ItemModel {
     return [overview, ...changedFiles];
   }
 
-  private async getMrDiscussions(mrVersion: RestMrVersion): Promise<void> {
+  private async initializeMrDiscussions(mrVersion: RestMrVersion): Promise<void> {
     const commentController = vscode.comments.createCommentController(
       this.mr.references.full,
       this.mr.title,
     );
-
-    commentController.commentingRangeProvider = new CommentingRangeProvider(this.mr, mrVersion);
-
     const gitlabService = await createGitLabNewService(this.workspace.uri);
 
+    if (await gitlabService.canUserCommentOnMr(this.mr)) {
+      commentController.commentingRangeProvider = new CommentingRangeProvider(this.mr, mrVersion);
+    }
+
     const discussions = await gitlabService.getDiscussions({
       issuable: this.mr,
     });

src/gitlab/gitlab_new_service.ts

@@ -308,6 +308,18 @@ const constructGetDiscussionsQuery = (isMr: boolean) => gql`
   }
 `;
 
+const getMrPermissionsQuery = gql`
+  query GetMrPermissions($projectPath: ID!, $iid: String!) {
+    project(fullPath: $projectPath) {
+      mergeRequest(iid: $iid) {
+        userPermissions {
+          createNote
+        }
+      }
+    }
+  }
+`;
+
 const discussionSetResolved = gql`
   mutation DiscussionToggleResolve($replyId: DiscussionID!, $resolved: Boolean!) {
     discussionToggleResolve(input: { id: $replyId, resolve: $resolved }) {
@@ -521,6 +533,16 @@ export class GitLabNewService {
     return discussions.nodes.map(n => this.addHostToUrl(n));
   }
 
+  async canUserCommentOnMr(issuable: RestIssuable): Promise<boolean> {
+    const projectPath = getProjectPath(issuable);
+    const result = await this.client.request(getMrPermissionsQuery, {
+      projectPath,
+      iid: String(issuable.iid),
+    });
+    assert(result?.project?.mergeRequest, `MR ${issuable.references.full} was not found.`);
+    return Boolean(result.project.mergeRequest.userPermissions?.createNote);
+  }
+
   async setResolved(replyId: string, resolved: boolean): Promise<void> {
     try {
       return await this.client.request<void>(discussionSetResolved, {

test/integration/fixtures/graphql/mr_permissions.json

+{
+  "project": {
+    "mergeRequest": {
+      "userPermissions": {
+        "createNote": true
+      }
+    }
+  }
+}

test/integration/mr_review.test.js

@@ -12,6 +12,7 @@ const versionsResponse = require('./fixtures/rest/versions.json');
 const versionResponse = require('./fixtures/rest/mr_version.json');
 const diffNote = require('./fixtures/rest/diff_note.json');
 const { projectWithMrDiscussions, noteOnDiff } = require('./fixtures/graphql/discussions');
+const mrPermissionsResponse = require('./fixtures/graphql/mr_permissions.json');
 const {
   getServer,
   createJsonEndpoint,
@@ -44,6 +45,11 @@ describe('MR Review', () => {
           return res(ctx.data(projectWithMrDiscussions));
         return res(ctx.data({ project: null }));
       }),
+      graphql.query('GetMrPermissions', (req, res, ctx) => {
+        if (req.variables.projectPath === 'gitlab-org/gitlab' && req.variables.iid === '33824')
+          return res(ctx.data(mrPermissionsResponse));
+        return res(ctx.data({ project: null }));
+      }),
     ]);
     await tokenService.setToken(GITLAB_URL, 'abcd-secret');
   });

test/integration/test_infrastructure/mock_server.js

@@ -67,7 +67,7 @@ const getServer = (handlers = []) => {
     ...handlers,
     notFoundByDefault,
   );
-  server.listen();
+  server.listen({ onUnhandledRequest: 'warn' });
   return server;
 };