Commit 47f244bc authored by Tomas Vik (OOO back on 2026-08-31)'s avatar Tomas Vik (OOO back on 2026-08-31) 🌴
Browse files

feat: indicate which changed files have MR discussions

parent 06888a24
Loading
Loading
Loading
Loading
+4 −2
Original line number Diff line number Diff line
@@ -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 };
+3 −0
Original line number Diff line number Diff line
@@ -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';
+27 −1
Original line number Diff line number Diff line
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`);
    });
  });
});
+46 −32
Original line number Diff line number Diff line
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,36 +36,12 @@ 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;

  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 }));
    this.description = path
      .dirname(`/${file.new_path}`)
      .split('/')
      .slice(1)
      .join('/');
    this.mr = mr;
    this.mrVersion = mrVersion;
    this.repositoryPath = repositoryPath;
    this.file = file;

    if (looksLikeImage(file.old_path) || looksLikeImage(file.new_path)) {
      this.command = {
        title: 'Images are not supported',
        command: PROGRAMMATIC_COMMANDS.NO_IMAGE_REVIEW,
      };
      return;
    }
const getBaseAndHeadUri = (
  mr: RestMr,
  mrVersion: RestMrVersion,
  file: RestDiffFile,
  repositoryPath: string,
) => {
  const commonParams: ReviewParams = {
    repositoryRoot: repositoryPath,
    projectId: mr.project_id,
@@ -78,7 +62,37 @@ export class ChangedFileItem extends TreeItem {
        path: file.new_path,
        commit: mrVersion.head_commit_sha,
      });
  return { baseFileUri, headFileUri };
};

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('/');
    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',
        command: PROGRAMMATIC_COMMANDS.NO_IMAGE_REVIEW,
      };
      return;
    }
    this.command = {
      title: 'Show changes',
      command: VS_COMMANDS.DIFF,
+10 −11
Original line number Diff line number Diff line
@@ -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();

Loading