diff --git a/src/data_providers/items/mr_item_model.test.ts b/src/data_providers/items/mr_item_model.test.ts index 7503bf40c6b6d37975e962bbcb903a55ad48fb02..2798bad6df196682a9e9545da3898f558a877b52 100644 --- a/src/data_providers/items/mr_item_model.test.ts +++ b/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); + }); + }); }); diff --git a/src/data_providers/items/mr_item_model.ts b/src/data_providers/items/mr_item_model.ts index e0580dac1a69aab75c93323c870fd768ffaa5ec9..1c60ce4a5573b20d00ac0ad66e5e6668014200d7 100644 --- a/src/data_providers/items/mr_item_model.ts +++ b/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 { + private async initializeMrDiscussions(mrVersion: RestMrVersion): Promise { 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, }); diff --git a/src/gitlab/gitlab_new_service.ts b/src/gitlab/gitlab_new_service.ts index 6b8e9a8039cc722c7dc9a10fb131fe8bc14b1e47..9b565e96c329fb1a3598b48f4e6c3d608382184e 100644 --- a/src/gitlab/gitlab_new_service.ts +++ b/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 { + 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 { try { return await this.client.request(discussionSetResolved, { diff --git a/test/integration/fixtures/graphql/mr_permissions.json b/test/integration/fixtures/graphql/mr_permissions.json new file mode 100644 index 0000000000000000000000000000000000000000..a7e166263657d2da6d4b3855cfb7a0f1ebfe545f --- /dev/null +++ b/test/integration/fixtures/graphql/mr_permissions.json @@ -0,0 +1,9 @@ +{ + "project": { + "mergeRequest": { + "userPermissions": { + "createNote": true + } + } + } +} diff --git a/test/integration/mr_review.test.js b/test/integration/mr_review.test.js index cdcb808df9dd885e000e7202d67202aa8efe6bdd..22b6c08a4f78f6e10f5a68d0e29e08d307097368 100644 --- a/test/integration/mr_review.test.js +++ b/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'); }); diff --git a/test/integration/test_infrastructure/mock_server.js b/test/integration/test_infrastructure/mock_server.js index 238d4f314a2411ee357d665c0c710be2ceb28603..d84bb10217f758f819ad1cef0d133760569f9248 100644 --- a/test/integration/test_infrastructure/mock_server.js +++ b/test/integration/test_infrastructure/mock_server.js @@ -67,7 +67,7 @@ const getServer = (handlers = []) => { ...handlers, notFoundByDefault, ); - server.listen(); + server.listen({ onUnhandledRequest: 'warn' }); return server; };