feat: indicate which changed files have MR discussions

src/__mocks__/vscode.js

@@ -2,8 +2,10 @@ const { Uri } = require('../test_utils/uri');
 const { EventEmitter } = require('../test_utils/event_emitter');
 
 module.exports = {
-  TreeItem: function TreeItem(label, collapsibleState) {
-    return { label, collapsibleState };
+  TreeItem: function TreeItem(labelOrUri, collapsibleState) {
+    return typeof labelOrUri === 'string'
+      ? { label: labelOrUri, collapsibleState }
+      : { resourceUri: labelOrUri, collapsibleState };
   },
   ThemeIcon: function ThemeIcon(id) {
     return { id };

src/constants.ts

@@ -9,3 +9,6 @@ export const MODIFIED = 'modified';
 export const DO_NOT_SHOW_VERSION_WARNING = 'DO_NOT_SHOW_VERSION_WARNING';
 // NOTE: This needs to _always_ be a 3 digits
 export const MINIMUM_VERSION = '13.5.0';
+
+export const CHANGE_TYPE_QUERY_KEY = 'changeType';
+export const HAS_COMMENTS_QUERY_KEY = 'hasComments';

src/data_providers/items/changed_file_item.test.ts

 import { PROGRAMMATIC_COMMANDS } from '../../command_names';
+import { CHANGE_TYPE_QUERY_KEY, HAS_COMMENTS_QUERY_KEY } from '../../constants';
 import { diffFile, mr, mrVersion } from '../../test_utils/entities';
 import { ChangedFileItem } from './changed_file_item';
 
@@ -8,9 +9,34 @@ describe('ChangedFileItem', () => {
       'should not show diff for %s',
       extension => {
         const changedImageFile = { ...diffFile, new_path: `file${extension}` };
-        const item = new ChangedFileItem(mr, mrVersion, changedImageFile, '/repository/fsPath');
+        const item = new ChangedFileItem(mr, mrVersion, changedImageFile, '/repo', () => false);
+
         expect(item.command?.command).toBe(PROGRAMMATIC_COMMANDS.NO_IMAGE_REVIEW);
       },
     );
+
+    it('should indicate change type', () => {
+      const changedImageFile = { ...diffFile, new_path: `file.jpg` };
+      const item = new ChangedFileItem(mr, mrVersion, changedImageFile, '/repo', () => false);
+
+      expect(item.resourceUri?.query).toContain(`${CHANGE_TYPE_QUERY_KEY}=`);
+    });
+  });
+
+  describe('captures whether there are comments on the changes', () => {
+    let areThereChanges: boolean;
+
+    const createItem = () =>
+      new ChangedFileItem(mr, mrVersion, diffFile, '/repository/fsPath', () => areThereChanges);
+
+    it('indicates there are comments', () => {
+      areThereChanges = true;
+      expect(createItem().resourceUri?.query).toMatch(`${HAS_COMMENTS_QUERY_KEY}=true`);
+    });
+
+    it('indicates there are no comments', () => {
+      areThereChanges = false;
+      expect(createItem().resourceUri?.query).toMatch(`${HAS_COMMENTS_QUERY_KEY}=false`);
+    });
   });
 });

src/data_providers/items/changed_file_item.ts

-import { TreeItem, Uri } from 'vscode';
+import * as vscode from 'vscode';
 import { posix as path } from 'path';
 import { toReviewUri, ReviewParams } from '../../review/review_uri';
 import { PROGRAMMATIC_COMMANDS, VS_COMMANDS } from '../../command_names';
-import { ADDED, DELETED, RENAMED, MODIFIED } from '../../constants';
+import {
+  ADDED,
+  DELETED,
+  RENAMED,
+  MODIFIED,
+  CHANGE_TYPE_QUERY_KEY,
+  HAS_COMMENTS_QUERY_KEY,
+} from '../../constants';
 
 export type ChangeType = typeof ADDED | typeof DELETED | typeof RENAMED | typeof MODIFIED;
+export type HasCommentsFn = (reviewUri: vscode.Uri) => boolean;
 
 const getChangeType = (file: RestDiffFile): ChangeType => {
   if (file.new_file) return ADDED;
@@ -28,29 +36,56 @@ const imageExtensions = [
 const looksLikeImage = (filePath: string) =>
   imageExtensions.includes(path.extname(filePath).toLowerCase());
 
-export class ChangedFileItem extends TreeItem {
-  mr: RestMr;
-
-  mrVersion: RestMrVersion;
-
-  repositoryPath: string;
-
-  file: RestDiffFile;
+const getBaseAndHeadUri = (
+  mr: RestMr,
+  mrVersion: RestMrVersion,
+  file: RestDiffFile,
+  repositoryPath: string,
+) => {
+  const commonParams: ReviewParams = {
+    repositoryRoot: repositoryPath,
+    projectId: mr.project_id,
+    mrId: mr.id,
+  };
+  const emptyFileUri = toReviewUri(commonParams);
+  const baseFileUri = file.new_file
+    ? emptyFileUri
+    : toReviewUri({
+        ...commonParams,
+        path: file.old_path,
+        commit: mrVersion.base_commit_sha,
+      });
+  const headFileUri = file.deleted_file
+    ? emptyFileUri
+    : toReviewUri({
+        ...commonParams,
+        path: file.new_path,
+        commit: mrVersion.head_commit_sha,
+      });
+  return { baseFileUri, headFileUri };
+};
 
-  constructor(mr: RestMr, mrVersion: RestMrVersion, file: RestDiffFile, repositoryPath: string) {
-    const changeType = getChangeType(file);
-    const query = new URLSearchParams([['changeType', changeType]]).toString();
-    super(Uri.file(file.new_path).with({ query }));
+export class ChangedFileItem extends vscode.TreeItem {
+  constructor(
+    mr: RestMr,
+    mrVersion: RestMrVersion,
+    file: RestDiffFile,
+    repositoryPath: string,
+    hasComment: HasCommentsFn,
+  ) {
+    super(vscode.Uri.file(file.new_path));
     this.description = path
       .dirname(`/${file.new_path}`)
       .split('/')
       .slice(1)
       .join('/');
-    this.mr = mr;
-    this.mrVersion = mrVersion;
-    this.repositoryPath = repositoryPath;
-    this.file = file;
-
+    const { baseFileUri, headFileUri } = getBaseAndHeadUri(mr, mrVersion, file, repositoryPath);
+    const hasComments = hasComment(baseFileUri) || hasComment(headFileUri);
+    const query = new URLSearchParams([
+      [CHANGE_TYPE_QUERY_KEY, getChangeType(file)],
+      [HAS_COMMENTS_QUERY_KEY, String(hasComments)],
+    ]).toString();
+    this.resourceUri = this.resourceUri?.with({ query });
     if (looksLikeImage(file.old_path) || looksLikeImage(file.new_path)) {
       this.command = {
         title: 'Images are not supported',
@@ -58,27 +93,6 @@ export class ChangedFileItem extends TreeItem {
       };
       return;
     }
-    const commonParams: ReviewParams = {
-      repositoryRoot: repositoryPath,
-      projectId: mr.project_id,
-      mrId: mr.id,
-    };
-    const emptyFileUri = toReviewUri(commonParams);
-    const baseFileUri = file.new_file
-      ? emptyFileUri
-      : toReviewUri({
-          ...commonParams,
-          path: file.old_path,
-          commit: mrVersion.base_commit_sha,
-        });
-    const headFileUri = file.deleted_file
-      ? emptyFileUri
-      : toReviewUri({
-          ...commonParams,
-          path: file.new_path,
-          commit: mrVersion.head_commit_sha,
-        });
-
     this.command = {
       title: 'Show changes',
       command: VS_COMMANDS.DIFF,

src/data_providers/items/mr_item_model.test.ts

@@ -6,10 +6,12 @@ import {
   noteOnDiffTextSnippet,
   multipleNotes,
 } from '../../../test/integration/fixtures/graphql/discussions.js';
+import * as mrVersion from '../../../test/integration/fixtures/rest/mr_version.json';
 import { CommentingRangeProvider } from '../../review/commenting_range_provider';
 import { createWrappedRepository } from '../../test_utils/create_wrapped_repository';
 import { fromReviewUri } from '../../review/review_uri';
 import { WrappedRepository } from '../../git/wrapped_repository';
+import { CHANGE_TYPE_QUERY_KEY, HAS_COMMENTS_QUERY_KEY } from '../../constants';
 
 const createCommentControllerMock = vscode.comments.createCommentController as jest.Mock;
 
@@ -94,6 +96,14 @@ describe('MrItemModel', () => {
     expect(path).toBe(discussionPosition.oldPath);
   });
 
+  it('should return changed file items as children', async () => {
+    gitLabService.getMrDiff = jest.fn().mockResolvedValue(mrVersion);
+    const [overview, changedItem] = await item.getChildren();
+    expect(changedItem.resourceUri?.path).toBe('.deleted.yml');
+    expect(changedItem.resourceUri?.query).toMatch(`${CHANGE_TYPE_QUERY_KEY}=deleted`);
+    expect(changedItem.resourceUri?.query).toMatch(`${HAS_COMMENTS_QUERY_KEY}=false`);
+  });
+
   describe('commenting range', () => {
     it('should not add a commenting range provider if user does not have permission to comment', async () => {
       canUserCommentOnMr = false;
@@ -111,17 +121,6 @@ describe('MrItemModel', () => {
       expect(commentController.commentingRangeProvider).toBeInstanceOf(CommentingRangeProvider);
     });
 
-    // this test ensures that we add comment controller to disposables before calling API.
-    it('comment controller can be disposed regardless of API failures', async () => {
-      gitLabService.getDiscussions = () => Promise.reject(new Error());
-
-      await item.getChildren();
-
-      expect(commentController.dispose).not.toHaveBeenCalled();
-      item.dispose();
-      expect(commentController.dispose).toHaveBeenCalled();
-    });
-
     it('when we create comment controller for the same MR, we dispose the previously created controller', async () => {
       await item.getChildren();
 

src/data_providers/items/mr_item_model.ts

@@ -62,17 +62,23 @@ export class MrItemModel extends ItemModel {
     return item;
   }
 
-  async getChildren(): Promise<vscode.TreeItem[]> {
-    const overview = new vscode.TreeItem('Overview');
-    overview.iconPath = new vscode.ThemeIcon('note');
-    overview.command = {
+  private get overviewItem() {
+    const result = new vscode.TreeItem('Overview');
+    result.iconPath = new vscode.ThemeIcon('note');
+    result.command = {
       command: PROGRAMMATIC_COMMANDS.SHOW_RICH_CONTENT,
       arguments: [this.mr, this.repository.rootFsPath],
       title: 'Show MR Overview',
     };
-    const { mrVersion } = await this.repository.reloadMr(this.mr);
+    return result;
+  }
+
+  private async getMrDiscussions(): Promise<GqlTextDiffDiscussion[]> {
     try {
-      await this.initializeMrDiscussions(mrVersion);
+      const discussions = await this.repository.getGitLabService().getDiscussions({
+        issuable: this.mr,
+      });
+      return discussions.filter(isTextDiffDiscussion);
     } catch (e) {
       handleError(
         new UserFriendlyError(
@@ -83,14 +89,31 @@ export class MrItemModel extends ItemModel {
         ),
       );
     }
+    return [];
+  }
+
+  async getChildren(): Promise<vscode.TreeItem[]> {
+    const { mrVersion } = await this.repository.reloadMr(this.mr);
+    const discussions = await this.getMrDiscussions();
+
+    await this.addAllCommentsToVsCode(mrVersion, discussions);
 
+    const allUrisWithComments = discussions.map(d =>
+      uriForDiscussion(this.repository, this.mr, d).toString(),
+    );
     const changedFiles = mrVersion.diffs.map(
-      d => new ChangedFileItem(this.mr, mrVersion, d, this.repository.rootFsPath),
+      diff =>
+        new ChangedFileItem(this.mr, mrVersion, diff, this.repository.rootFsPath, uri =>
+          allUrisWithComments.includes(uri.toString()),
+        ),
     );
-    return [overview, ...changedFiles];
+    return [this.overviewItem, ...changedFiles];
   }
 
-  private async initializeMrDiscussions(mrVersion: RestMrVersion): Promise<void> {
+  private async addAllCommentsToVsCode(
+    mrVersion: RestMrVersion,
+    discussions: GqlTextDiffDiscussion[],
+  ): Promise<void> {
     const gitlabService = this.repository.getGitLabService();
     const userCanComment = await gitlabService.canUserCommentOnMr(this.mr);
 
@@ -101,11 +124,7 @@ export class MrItemModel extends ItemModel {
     );
     this.setDisposableChildren([commentController]);
 
-    const discussions = await gitlabService.getDiscussions({
-      issuable: this.mr,
-    });
-    const discussionsOnDiff = discussions.filter(isTextDiffDiscussion);
-    discussionsOnDiff.forEach(discussion => {
+    discussions.forEach(discussion => {
       const { position } = firstNoteFrom(discussion);
       const vsThread = commentController.createCommentThread(
         uriForDiscussion(this.repository, this.mr, discussion),

src/extension.js

@@ -27,7 +27,8 @@ const {
   submitEdit,
   createComment,
 } = require('./commands/mr_discussion_commands');
-const { fileDecorationProvider } = require('./review/file_decoration_provider');
+const { hasCommentsDecorationProvider } = require('./review/has_comments_decoration_provider');
+const { changeTypeDecorationProvider } = require('./review/change_type_decoration_provider');
 const { checkVersion } = require('./utils/check_version');
 const { checkoutMrBranch } = require('./commands/checkout_mr_branch');
 
@@ -126,7 +127,8 @@ const activate = context => {
   registerCiCompletion(context);
   gitExtensionWrapper.init();
   context.subscriptions.push(gitExtensionWrapper);
-  vscode.window.registerFileDecorationProvider(fileDecorationProvider);
+  vscode.window.registerFileDecorationProvider(hasCommentsDecorationProvider);
+  vscode.window.registerFileDecorationProvider(changeTypeDecorationProvider);
 
   checkVersion(gitExtensionWrapper, context);
 };

src/review/file_decoration_provider.test.ts → src/review/change_type_decoration_provider.test.ts

 import * as vscode from 'vscode';
-import { fileDecorationProvider, decorations } from './file_decoration_provider';
-import { ADDED, DELETED, RENAMED, MODIFIED } from '../constants';
+import { changeTypeDecorationProvider, decorations } from './change_type_decoration_provider';
+import { ADDED, DELETED, RENAMED, MODIFIED, CHANGE_TYPE_QUERY_KEY } from '../constants';
 
 describe('FileDecoratorProvider', () => {
   it.each`
@@ -10,9 +10,9 @@ describe('FileDecoratorProvider', () => {
     ${RENAMED}  | ${decorations[RENAMED]}
     ${MODIFIED} | ${decorations[MODIFIED]}
   `('Correctly maps changeType to decorator', ({ changeType, decoration }) => {
-    const uri: vscode.Uri = vscode.Uri.file(`./test?changeType=${changeType}`);
+    const uri: vscode.Uri = vscode.Uri.file(`./test?${CHANGE_TYPE_QUERY_KEY}=${changeType}`);
     const { token } = new vscode.CancellationTokenSource();
-    const returnValue = fileDecorationProvider.provideFileDecoration(uri, token);
+    const returnValue = changeTypeDecorationProvider.provideFileDecoration(uri, token);
 
     expect(returnValue).toEqual(decoration);
   });

src/review/file_decoration_provider.ts → src/review/change_type_decoration_provider.ts

 import * as vscode from 'vscode';
-import { ADDED, DELETED, RENAMED, MODIFIED } from '../constants';
+import { ADDED, DELETED, RENAMED, MODIFIED, CHANGE_TYPE_QUERY_KEY } from '../constants';
 
 export const decorations: Record<string, vscode.FileDecoration | undefined> = {
   [ADDED]: {
@@ -19,11 +19,11 @@ export const decorations: Record<string, vscode.FileDecoration | undefined> = {
   },
 };
 
-export const fileDecorationProvider: vscode.FileDecorationProvider = {
+export const changeTypeDecorationProvider: vscode.FileDecorationProvider = {
   provideFileDecoration: uri => {
     if (uri.scheme === 'file') {
       const params = new URLSearchParams(uri.query);
-      const changeType = params.get('changeType');
+      const changeType = params.get(CHANGE_TYPE_QUERY_KEY);
       if (changeType) {
         return decorations[changeType];
       }

src/review/has_comments_decoration_provider.test.ts

+import * as vscode from 'vscode';
+import { HAS_COMMENTS_QUERY_KEY } from '../constants';
+import { hasCommentsDecorationProvider } from './has_comments_decoration_provider';
+
+describe('FileDecoratorProvider', () => {
+  it.each`
+    urlQuery                              | decoration
+    ${`?${HAS_COMMENTS_QUERY_KEY}=true`}  | ${'💬'}
+    ${`?${HAS_COMMENTS_QUERY_KEY}=false`} | ${undefined}
+    ${''}                                 | ${undefined}
+  `('Correctly maps hasComments query to decorator', async ({ urlQuery, decoration }) => {
+    const uri: vscode.Uri = vscode.Uri.file(`./test${urlQuery}`);
+    const { token } = new vscode.CancellationTokenSource();
+    const returnValue = await hasCommentsDecorationProvider.provideFileDecoration(uri, token);
+
+    expect(returnValue?.badge).toEqual(decoration);
+  });
+});

src/review/has_comments_decoration_provider.ts

+import * as vscode from 'vscode';
+import { HAS_COMMENTS_QUERY_KEY } from '../constants';
+
+export const hasCommentsDecorationProvider: vscode.FileDecorationProvider = {
+  provideFileDecoration: uri => {
+    if (uri.scheme !== 'file') {
+      return undefined;
+    }
+    const params = new URLSearchParams(uri.query);
+    const hasComments = params.get(HAS_COMMENTS_QUERY_KEY) === 'true';
+    if (hasComments) {
+      return { badge: '💬' };
+    }
+    return undefined;
+  },
+};

src/test_utils/uri.ts

@@ -54,7 +54,7 @@ export class Uri implements vscode.Uri {
   }
 
   toJSON(): string {
-    return JSON.stringify(this);
+    return JSON.stringify({ ...this });
   }
 
   static parse(stringUri: string): Uri {