refactor: move comment therad initialization to MrItemModel

src/data_providers/items/mr_item_model.test.ts

 import * as vscode from 'vscode';
 import { MrItemModel } from './mr_item_model';
-import { mr } from '../../test_utils/entities';
+import { mr, repository } from '../../test_utils/entities';
 import {
   discussionOnDiff,
   noteOnDiffTextSnippet,
@@ -8,6 +8,7 @@ import {
 } from '../../../test/integration/fixtures/graphql/discussions.js';
 import { CommentingRangeProvider } from '../../review/commenting_range_provider';
 import { createWrappedRepository } from '../../test_utils/create_wrapped_repository';
+import { fromReviewUri } from '../../review/review_uri';
 
 const createCommentControllerMock = vscode.comments.createCommentController as jest.Mock;
 
@@ -50,8 +51,7 @@ describe('MrItemModel', () => {
       'gitlab-mr-gitlab-org/gitlab!2000',
       'Issuable Title',
     );
-    const [uri, range] = createCommentThreadMock.mock.calls[0];
-    expect(uri.path).toBe('src/webview/src/components/LabelNoteOld.vue');
+    const [_, range] = createCommentThreadMock.mock.calls[0];
     expect(range.start.line).toBe(47);
     expect(commentThread.comments.length).toBe(1);
     const firstComment = commentThread.comments[0];
@@ -60,6 +60,20 @@ describe('MrItemModel', () => {
     expect(firstComment.body).toMatch(noteOnDiffTextSnippet);
   });
 
+  it('should associate the thread with the correct URI', async () => {
+    await item.getChildren();
+
+    const [uri] = createCommentThreadMock.mock.calls[0];
+    const { mrId, projectId, repositoryRoot, commit, path } = fromReviewUri(uri);
+    expect(mrId).toBe(mr.id);
+    expect(projectId).toBe(mr.project_id);
+    expect(repositoryRoot).toBe(repository.rootFsPath);
+
+    const discussionPosition = discussionOnDiff.notes.nodes[0].position;
+    expect(commit).toBe(discussionPosition.diffRefs.baseSha);
+    expect(path).toBe(discussionPosition.oldPath);
+  });
+
   describe('commenting range', () => {
     it('should not add a commenting range provider if user does not have permission to comment', async () => {
       canUserCommentOnMr = false;

src/data_providers/items/mr_item_model.ts

 import * as vscode from 'vscode';
+import * as assert from 'assert';
 import { PROGRAMMATIC_COMMANDS } from '../../command_names';
 import { ChangedFileItem } from './changed_file_item';
 import { ItemModel } from './item_model';
@@ -9,12 +10,40 @@ import { GitLabCommentThread } from '../../review/gitlab_comment_thread';
 import { CommentingRangeProvider } from '../../review/commenting_range_provider';
 import { WrappedRepository } from '../../git/wrapped_repository';
 import { commentControllerProvider } from '../../review/comment_controller_provider';
+import { GqlTextDiffNote } from '../../gitlab/graphql/shared';
+import { toReviewUri } from '../../review/review_uri';
+import {
+  commentRangeFromPosition,
+  commitFromPosition,
+  pathFromPosition,
+} from '../../review/gql_position_parser';
 
 const isTextDiffDiscussion = (discussion: GqlDiscussion): discussion is GqlTextDiffDiscussion => {
   const firstNote = discussion.notes.nodes[0];
   return firstNote?.position?.positionType === 'text';
 };
 
+const firstNoteFrom = (discussion: GqlTextDiffDiscussion): GqlTextDiffNote => {
+  const note = discussion.notes.nodes[0];
+  assert(note, 'discussion should contain at least one note');
+  return note;
+};
+
+const uriForDiscussion = (
+  repository: WrappedRepository,
+  mr: RestMr,
+  discussion: GqlTextDiffDiscussion,
+): vscode.Uri => {
+  const { position } = firstNoteFrom(discussion);
+  return toReviewUri({
+    path: pathFromPosition(position),
+    commit: commitFromPosition(position),
+    repositoryRoot: repository.rootFsPath,
+    projectId: mr.project_id,
+    mrId: mr.id,
+  });
+};
+
 export class MrItemModel extends ItemModel {
   constructor(readonly mr: RestMr, readonly repository: WrappedRepository) {
     super();
@@ -76,13 +105,20 @@ export class MrItemModel extends ItemModel {
     });
     const discussionsOnDiff = discussions.filter(isTextDiffDiscussion);
     discussionsOnDiff.forEach(discussion => {
-      return GitLabCommentThread.createThread({
-        commentController,
-        repositoryRoot: this.repository.rootFsPath,
-        mr: this.mr,
+      const { position } = firstNoteFrom(discussion);
+      const vsThread = commentController.createCommentThread(
+        uriForDiscussion(this.repository, this.mr, discussion),
+        commentRangeFromPosition(position),
+        // the comments need to know about the thread, so we first
+        // create empty thread to be able to create comments
+        [],
+      );
+      return new GitLabCommentThread(
+        vsThread,
         discussion,
-        gitlabService,
-      });
+        this.repository.getGitLabService(),
+        this.mr,
+      );
     });
   }
 }

src/review/gitlab_comment_thread.test.ts

@@ -6,12 +6,9 @@ import {
 } from '../../test/integration/fixtures/graphql/discussions.js';
 import { GitLabComment } from './gitlab_comment';
 import { GitLabNewService } from '../gitlab/gitlab_new_service';
-import { mr, project } from '../test_utils/entities';
-import { GqlTextDiffNote, GqlTextPosition } from '../gitlab/graphql/shared';
+import { mr } from '../test_utils/entities';
+import { GqlTextDiffNote } from '../gitlab/graphql/shared';
 import { GqlTextDiffDiscussion } from '../gitlab/graphql/get_discussions';
-import { fromReviewUri } from './review_uri';
-
-const TEST_ROOT = '/repositoryRoot';
 
 describe('GitLabCommentThread', () => {
   let gitlabCommentThread: GitLabCommentThread;
@@ -44,21 +41,13 @@ describe('GitLabCommentThread', () => {
   };
 
   const createGitLabCommentThread = (discussion: GqlTextDiffDiscussion) => {
-    const fakeCommentController: vscode.CommentController = {
-      id: 'id',
-      label: 'label',
-      dispose: () => undefined,
-      createCommentThread: (uri, range, comments) => {
-        vsCommentThread = {
-          uri,
-          range,
-          comments,
-          collapsibleState: vscode.CommentThreadCollapsibleState.Collapsed,
-          canReply: true,
-          dispose: jest.fn(),
-        };
-        return vsCommentThread;
-      },
+    vsCommentThread = {
+      uri: {} as any,
+      range: {} as any,
+      comments: {} as any,
+      collapsibleState: vscode.CommentThreadCollapsibleState.Collapsed,
+      canReply: true,
+      dispose: jest.fn(),
     };
     gitlabService = ({
       setResolved: jest.fn(),
@@ -66,13 +55,7 @@ describe('GitLabCommentThread', () => {
       updateNoteBody: jest.fn(),
       createNote: jest.fn(),
     } as unknown) as GitLabNewService;
-    gitlabCommentThread = GitLabCommentThread.createThread({
-      commentController: fakeCommentController,
-      repositoryRoot: TEST_ROOT,
-      mr,
-      discussion,
-      gitlabService,
-    });
+    gitlabCommentThread = new GitLabCommentThread(vsCommentThread, discussion, gitlabService, mr);
   };
 
   beforeEach(() => {
@@ -83,64 +66,6 @@ describe('GitLabCommentThread', () => {
     expect(vsCommentThread.collapsibleState).toBe(vscode.CommentThreadCollapsibleState.Expanded);
   });
 
-  it('takes position from the first note', () => {
-    expect(vsCommentThread.range.start.line).toBe(noteOnDiff.position.oldLine - 1); // vs code numbers lines from 0
-  });
-
-  describe('new thread URI', () => {
-    it('populates basic uri parameters', () => {
-      const { projectId, repositoryRoot, mrId } = fromReviewUri(vsCommentThread.uri);
-
-      expect(projectId).toBe(mr.project_id);
-      expect(mrId).toBe(mr.id);
-      expect(repositoryRoot).toBe(TEST_ROOT);
-    });
-
-    it('generates uri for discussion on old diff version', () => {
-      createGitLabCommentThread(
-        createGqlTextDiffDiscussion({
-          ...noteOnDiff,
-          position: {
-            ...noteOnDiff.position,
-            oldLine: 10, // comment is on the old version of the diff
-            newLine: null,
-            oldPath: 'old/path.js',
-          } as GqlTextPosition,
-        }),
-      );
-
-      const { commit, path } = fromReviewUri(vsCommentThread.uri);
-
-      expect(commit).toBe(noteOnDiff.position.diffRefs.baseSha);
-      expect(path).toBe('old/path.js');
-    });
-
-    it('generates uri for discussion on new diff version', () => {
-      createGitLabCommentThread(
-        createGqlTextDiffDiscussion({
-          ...noteOnDiff,
-          position: {
-            ...noteOnDiff.position,
-            oldLine: null,
-            newLine: 10, // comment is on the new version of the diff
-            newPath: 'new/path.js',
-          } as GqlTextPosition,
-        }),
-      );
-
-      const { commit, path } = fromReviewUri(vsCommentThread.uri);
-
-      expect(commit).toBe(noteOnDiff.position.diffRefs.headSha);
-      expect(path).toBe('new/path.js');
-    });
-  });
-
-  it('generates uri for the discussion', () => {
-    // TODO: improve the uri tests once we merge https://gitlab.com/gitlab-org/gitlab-vscode-extension/-/merge_requests/192
-    const { baseSha } = noteOnDiff.position.diffRefs; // baseSha points to the commit that the MR started from (old lines)
-    expect(vsCommentThread.uri.query).toMatch(baseSha);
-  });
-
   describe('allowing replies to the thread', () => {
     const createNoteAndSetCreatePermissions = (createNote: boolean): GqlTextDiffNote => ({
       ...(noteOnDiff as GqlTextDiffNote),

src/review/gitlab_comment_thread.ts

@@ -2,10 +2,8 @@ import * as vscode from 'vscode';
 import * as assert from 'assert';
 import { GitLabNewService } from '../gitlab/gitlab_new_service';
 import { GitLabComment } from './gitlab_comment';
-import { toReviewUri } from './review_uri';
 import { GqlTextDiffDiscussion } from '../gitlab/graphql/get_discussions';
-import { GqlNote, GqlTextDiffNote, GqlTextPosition } from '../gitlab/graphql/shared';
-import { commitFromPosition, pathFromPosition } from './gql_position_parser';
+import { GqlNote, GqlTextDiffNote } from '../gitlab/graphql/shared';
 
 const firstNoteFrom = (discussion: GqlTextDiffDiscussion): GqlTextDiffNote => {
   const note = discussion.notes.nodes[0];
@@ -17,20 +15,6 @@ const isDiffNote = (note: GqlNote): note is GqlTextDiffNote => {
   return Boolean(note.position && note.position.positionType === 'text');
 };
 
-const commentRangeFromPosition = (position: GqlTextPosition): vscode.Range => {
-  const glLine = position.oldLine ?? position.newLine;
-  const vsPosition = new vscode.Position(glLine - 1, 0); // VS Code numbers lines starting with 0, GitLab starts with 1
-  return new vscode.Range(vsPosition, vsPosition);
-};
-
-interface CreateThreadOptions {
-  commentController: vscode.CommentController;
-  repositoryRoot: string;
-  mr: RestMr;
-  discussion: GqlTextDiffDiscussion;
-  gitlabService: GitLabNewService;
-}
-
 export class GitLabCommentThread {
   private resolved: boolean;
 
@@ -124,28 +108,4 @@ export class GitLabCommentThread {
       this.vsThread.contextValue = this.resolved ? 'resolved' : 'unresolved';
     }
   }
-
-  static createThread({
-    commentController,
-    repositoryRoot,
-    mr,
-    discussion,
-    gitlabService,
-  }: CreateThreadOptions): GitLabCommentThread {
-    const { position } = firstNoteFrom(discussion);
-    const vsThread = commentController.createCommentThread(
-      toReviewUri({
-        path: pathFromPosition(position),
-        commit: commitFromPosition(position),
-        repositoryRoot,
-        projectId: mr.project_id,
-        mrId: mr.id,
-      }),
-      commentRangeFromPosition(position),
-      // the comments need to know about the thread, so we first
-      // create empty thread to be able to create comments
-      [],
-    );
-    return new GitLabCommentThread(vsThread, discussion, gitlabService, mr);
-  }
 }

src/review/gql_position_parser.test.ts

-import { commitFromPosition, pathFromPosition } from './gql_position_parser';
+import * as vscode from 'vscode';
+import {
+  commentRangeFromPosition,
+  commitFromPosition,
+  pathFromPosition,
+} from './gql_position_parser';
 import { noteOnDiff } from '../../test/integration/fixtures/graphql/discussions.js';
 import { GqlTextPosition } from '../gitlab/graphql/shared';
 
@@ -6,7 +11,7 @@ const { position } = noteOnDiff;
 
 const oldPosition = {
   ...position,
-  oldLine: 1,
+  oldLine: 5,
   newLine: null,
   oldPath: 'oldPath.js',
   diffRefs: {
@@ -18,7 +23,7 @@ const oldPosition = {
 const newPosition = {
   ...position,
   oldLine: null,
-  newLine: 1,
+  newLine: 20,
   newPath: 'newPath.js',
   diffRefs: {
     ...position.diffRefs,
@@ -43,3 +48,14 @@ describe('commitFromPosition', () => {
     expect(commitFromPosition(newPosition)).toBe('1234');
   });
 });
+
+describe('commentRangeFromPosition', () => {
+  it('returns range with old line', () => {
+    const line = new vscode.Position(4, 0);
+    expect(commentRangeFromPosition(oldPosition)).toEqual(new vscode.Range(line, line));
+  });
+  it('returns headSha for new position', () => {
+    const line = new vscode.Position(19, 0);
+    expect(commentRangeFromPosition(newPosition)).toEqual(new vscode.Range(line, line));
+  });
+});

src/review/gql_position_parser.ts

+import * as vscode from 'vscode';
 import { GqlTextPosition } from '../gitlab/graphql/shared';
 
 const isOld = (position: GqlTextPosition) => position.oldLine !== null;
@@ -9,3 +10,9 @@ export const pathFromPosition = (position: GqlTextPosition): string => {
 export const commitFromPosition = (position: GqlTextPosition): string => {
   return isOld(position) ? position.diffRefs.baseSha : position.diffRefs.headSha;
 };
+
+export const commentRangeFromPosition = (position: GqlTextPosition): vscode.Range => {
+  const glLine = position.oldLine ?? position.newLine;
+  const vsPosition = new vscode.Position(glLine - 1, 0); // VS Code numbers lines starting with 0, GitLab starts with 1
+  return new vscode.Range(vsPosition, vsPosition);
+};