Merge branch '264-show-diff-api' into 'main'

feat(mr review): show changed file diff (API-provided)

See merge request gitlab-org/gitlab-vscode-extension!132

src/__mocks__/vscode.js

@@ -5,4 +5,10 @@ module.exports = {
   TreeItemCollapsibleState: {
     Collapsed: 'collapsed',
   },
+  Uri: {
+    file: path => ({
+      path,
+      with: jest.fn(),
+    }),
+  },
 };

src/constants.ts

 export const GITLAB_COM_URL = 'https://gitlab.com';
+export const REVIEW_URI_SCHEME = 'gl-review';

src/data_providers/items/changed_file_item.test.ts

+import { diffFile, issuable, mrVersion, project } from '../../test_utils/entities';
+import { ChangedFileItem } from './changed_file_item';
+
+describe('ChangedFileItem', () => {
+  describe('image file', () => {
+    it.each(['.jpg', '.jpeg', '.png', '.gif', '.webp', '.tiff', '.bmp', '.avif', '.apng'])(
+      'should not show diff for %s',
+      extension => {
+        const changedImageFile = { ...diffFile, new_path: `file${extension}` };
+        const item = new ChangedFileItem(issuable, mrVersion, changedImageFile, project);
+        expect(item.command?.command).toBe('gl.noImageReview');
+      },
+    );
+  });
+});

src/data_providers/items/changed_file_item.ts

+import { TreeItem, Uri } from 'vscode';
+import { posix as path } from 'path';
+import { toReviewUri } from '../../review/review_uri';
+
+const getChangeTypeIndicator = (diff: RestDiffFile): string => {
+  if (diff.new_file) return '[added] ';
+  if (diff.deleted_file) return '[deleted] ';
+  if (diff.renamed_file) return '[renamed] ';
+  return '';
+};
+
+// Common image types https://developer.mozilla.org/en-US/docs/Web/Media/Formats/Image_types
+const imageExtensions = [
+  '.jpg',
+  '.jpeg',
+  '.png',
+  '.gif',
+  '.webp',
+  '.tiff',
+  '.bmp',
+  '.avif',
+  '.apng',
+];
+const looksLikeImage = (filePath: string) =>
+  imageExtensions.includes(path.extname(filePath).toLowerCase());
+
+export class ChangedFileItem extends TreeItem {
+  mr: RestIssuable;
+
+  mrVersion: RestMrVersion;
+
+  project: VsProject;
+
+  file: RestDiffFile;
+
+  constructor(mr: RestIssuable, mrVersion: RestMrVersion, file: RestDiffFile, project: VsProject) {
+    super(Uri.file(file.new_path));
+    // TODO add FileDecorationProvider once it is available in the 1.52 https://github.com/microsoft/vscode/issues/54938
+    this.description = `${getChangeTypeIndicator(file)}${path.dirname(`/${file.new_path}`)}`;
+    this.mr = mr;
+    this.mrVersion = mrVersion;
+    this.project = project;
+    this.file = file;
+
+    if (looksLikeImage(file.old_path) || looksLikeImage(file.new_path)) {
+      this.command = {
+        title: 'Images are not supported',
+        command: 'gl.noImageReview',
+      };
+      return;
+    }
+
+    const emptyFileUri = toReviewUri({ workspacePath: project.uri, projectId: mr.project_id });
+    const baseFileUri = file.new_file
+      ? emptyFileUri
+      : toReviewUri({
+          path: file.old_path,
+          commit: mrVersion.base_commit_sha,
+          workspacePath: project.uri,
+          projectId: mr.project_id,
+        });
+    const headFileUri = file.deleted_file
+      ? emptyFileUri
+      : toReviewUri({
+          path: file.new_path,
+          commit: mrVersion.head_commit_sha,
+          workspacePath: project.uri,
+          projectId: mr.project_id,
+        });
+
+    this.command = {
+      title: 'Show changes',
+      command: 'vscode.diff',
+      arguments: [baseFileUri, headFileUri, `${path.basename(file.new_path)} (!${mr.iid})`],
+    };
+  }
+}

src/data_providers/items/custom_query_item.test.ts

 import * as vscode from 'vscode';
-import { CustomQueryType } from '../../gitlab/custom_query_type';
+import { customQuery, project } from '../../test_utils/entities';
 
 import { CustomQueryItem } from './custom_query_item';
 
 describe('CustomQueryItem', () => {
-  const customQuery = {
-    name: 'Query name',
-    type: CustomQueryType.ISSUE,
-    maxResults: 10,
-    scope: 'all',
-    state: 'closed',
-    wip: 'no',
-    confidential: false,
-    excludeSearchIn: 'all',
-    orderBy: 'created_at',
-    sort: 'desc',
-    searchIn: 'all',
-    noItemText: 'No item',
-  };
-  const project = { label: 'Project label', uri: '/' };
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   let item;
 

src/data_providers/items/mr_item.ts

-import { TreeItem, TreeItemCollapsibleState, ThemeIcon, Uri } from 'vscode';
-import * as path from 'path';
+import { TreeItem, TreeItemCollapsibleState, ThemeIcon } from 'vscode';
 import { GitLabNewService } from '../../gitlab/gitlab_new_service';
 import { createGitService } from '../../git_service_factory';
-
-const getChangeTypeIndicator = (diff: RestDiffFile): string => {
-  if (diff.new_file) return '[added] ';
-  if (diff.deleted_file) return '[deleted] ';
-  if (diff.renamed_file) return '[renamed] ';
-  return '';
-};
+import { ChangedFileItem } from './changed_file_item';
 
 export class MrItem extends TreeItem {
   mr: RestIssuable;
@@ -37,12 +30,7 @@ export class MrItem extends TreeItem {
     const gitService = createGitService(this.project.uri);
     const instanceUrl = await gitService.fetchCurrentInstanceUrl();
     const gitlabService = new GitLabNewService(instanceUrl);
-    const diff = await gitlabService.getMrDiff(this.mr);
-    return diff.map(d => {
-      const item = new TreeItem(Uri.file(d.new_path));
-      // TODO add FileDecorationProvider once it is available in the 1.53 https://github.com/microsoft/vscode/issues/54938
-      item.description = `${getChangeTypeIndicator(d)}${path.dirname(d.new_path)}`;
-      return item;
-    });
+    const mrVersion = await gitlabService.getMrDiff(this.mr);
+    return mrVersion.diffs.map(d => new ChangedFileItem(this.mr, mrVersion, d, this.project));
   }
 }

src/extension.js

@@ -14,6 +14,8 @@ const IssuableDataProvider = require('./data_providers/issuable').DataProvider;
 const CurrentBranchDataProvider = require('./data_providers/current_branch').DataProvider;
 const { initializeLogging, handleError } = require('./log');
 const checkDeprecatedCertificateSettings = require('./check_deprecated_certificate_settings');
+const { ApiContentProvider } = require('./review/api_content_provider');
+const { REVIEW_URI_SCHEME } = require('./constants');
 
 vscode.gitLabWorkflow = {
   sidebarDataProviders: [],
@@ -65,6 +67,8 @@ const registerCommands = (context, outputChannel) => {
     'gl.refreshSidebar': sidebar.refresh,
     'gl.showRichContent': webviewController.create,
     'gl.showOutput': () => outputChannel.show(),
+    'gl.noImageReview': () =>
+      vscode.window.showInformationMessage("GitLab MR review doesn't support images yet."),
   };
 
   Object.keys(commands).forEach(cmd => {
@@ -77,7 +81,7 @@ const registerCommands = (context, outputChannel) => {
 const activate = context => {
   const outputChannel = vscode.window.createOutputChannel('GitLab Workflow');
   initializeLogging(line => outputChannel.appendLine(line));
-
+  vscode.workspace.registerTextDocumentContentProvider(REVIEW_URI_SCHEME, new ApiContentProvider());
   registerCommands(context, outputChannel);
   webviewController.addDeps(context);
   tokenService.init(context);

src/gitlab/gitlab_new_service.ts

@@ -110,7 +110,7 @@ export class GitLabNewService {
   }
 
   // This method has to use REST API till https://gitlab.com/gitlab-org/gitlab/-/issues/280803 gets done
-  async getMrDiff(mr: RestIssuable): Promise<RestDiffFile[]> {
+  async getMrDiff(mr: RestIssuable): Promise<RestMrVersion> {
     const versionsUrl = `${this.instanceUrl}/api/v4/projects/${mr.project_id}/merge_requests/${mr.iid}/versions`;
     const versionsResult = await crossFetch(versionsUrl, this.fetchOptions);
     if (!versionsResult.ok) {
@@ -123,7 +123,17 @@ export class GitLabNewService {
     if (!diffResult.ok) {
       throw new FetchError(`Fetching MR diff from ${lastVersionUrl} failed`, diffResult);
     }
-    const result = await diffResult.json();
-    return result.diffs;
+    return diffResult.json();
+  }
+
+  async getFileContent(path: string, ref: string, projectId: number): Promise<string> {
+    const pathWithoutFirstSlash = path.replace(/^\//, '');
+    const encodedPath = encodeURIComponent(pathWithoutFirstSlash);
+    const fileUrl = `${this.instanceUrl}/api/v4/projects/${projectId}/repository/files/${encodedPath}/raw?ref=${ref}`;
+    const fileResult = await crossFetch(fileUrl, this.fetchOptions);
+    if (!fileResult.ok) {
+      throw new FetchError(`Fetching file from ${fileUrl} failed`, fileResult);
+    }
+    return fileResult.text();
   }
 }

src/review/api_content_provider.ts

+import * as vscode from 'vscode';
+import { fromReviewUri } from './review_uri';
+import { GitLabNewService } from '../gitlab/gitlab_new_service';
+import { logError } from '../log';
+import { createGitService } from '../git_service_factory';
+
+export class ApiContentProvider implements vscode.TextDocumentContentProvider {
+  // eslint-disable-next-line class-methods-use-this
+  async provideTextDocumentContent(
+    uri: vscode.Uri,
+    token: vscode.CancellationToken,
+  ): Promise<string> {
+    const params = fromReviewUri(uri);
+    if (!params.path || !params.commit) return '';
+
+    const instanceUrl = await createGitService(params.workspacePath).fetchCurrentInstanceUrl();
+    const service = new GitLabNewService(instanceUrl);
+    try {
+      return service.getFileContent(params.path, params.commit, params.projectId);
+    } catch (e) {
+      logError(e);
+      throw e;
+    }
+  }
+}

src/review/review_uri.ts

+import { Uri } from 'vscode';
+import { REVIEW_URI_SCHEME } from '../constants';
+
+interface ReviewParams {
+  path?: string;
+  commit?: string;
+  workspacePath: string;
+  projectId: number;
+}
+
+export function toReviewUri({ path = '', commit, workspacePath, projectId }: ReviewParams): Uri {
+  return Uri.file(path).with({
+    scheme: REVIEW_URI_SCHEME,
+    query: JSON.stringify({ commit, workspacePath, projectId }),
+  });
+}
+
+export function fromReviewUri(uri: Uri): ReviewParams {
+  const { commit, workspacePath, projectId } = JSON.parse(uri.query);
+  return {
+    path: uri.fsPath || undefined,
+    commit,
+    workspacePath,
+    projectId,
+  };
+}

src/test_utils/entities.ts

+import { CustomQueryType } from '../gitlab/custom_query_type';
+
+export const issuable: RestIssuable = {
+  id: 1,
+  iid: 1000,
+  title: 'Issuable Title',
+  project_id: 9999,
+  web_url: 'https://gitlab.example.com/group/project/issues/1',
+};
+
+export const diffFile: RestDiffFile = {
+  old_path: 'old_file.js',
+  new_path: 'new_file.js',
+  new_file: false,
+  deleted_file: false,
+  renamed_file: true,
+};
+
+export const mrVersion: RestMrVersion = {
+  base_commit_sha: 'aaaaaaaa',
+  head_commit_sha: 'bbbbbbbb',
+  diffs: [diffFile],
+};
+
+export const project: VsProject = {
+  label: 'Project label',
+  uri: '/home/johndoe/workspace/project',
+};
+
+export const customQuery = {
+  name: 'Query name',
+  type: CustomQueryType.ISSUE,
+  maxResults: 10,
+  scope: 'all',
+  state: 'closed',
+  wip: 'no',
+  confidential: false,
+  excludeSearchIn: 'all',
+  orderBy: 'created_at',
+  sort: 'desc',
+  searchIn: 'all',
+  noItemText: 'No item',
+};

src/types.d.ts

@@ -8,9 +8,13 @@ interface RestIssuable {
   sha?: string; // only present in MR, legacy logic uses the presence to decide issuable type
 }
 
-interface RestDiffFile {
+interface RestMrVersion {
   head_commit_sha: string;
   base_commit_sha: string;
+  diffs: RestDiffFile[];
+}
+
+interface RestDiffFile {
   new_path: string;
   old_path: string;
   deleted_file: boolean;

test/integration/fixtures/rest/mr_version.json

@@ -71,8 +71,8 @@
   ],
   "diffs": [
     {
-      "old_path": ".gitlab-ci.yml",
-      "new_path": ".gitlab-ci.yml",
+      "old_path": ".deleted.yml",
+      "new_path": ".deleted.yml",
       "a_mode": "100644",
       "b_mode": "0",
       "new_file": false,
@@ -101,14 +101,34 @@
       "diff": "@@ -0,0 +1,3 @@\n+export class NewFile{\n+    private property: string;\n+}\n"
     },
     {
-      "old_path": "test.js",
-      "new_path": "test.js",
+      "old_path": "src/test.js",
+      "new_path": "src/test.js",
       "a_mode": "100644",
       "b_mode": "100644",
       "new_file": false,
       "renamed_file": false,
       "deleted_file": false,
       "diff": "@@ -13,14 +13,11 @@\n function containingFunction(){\n     function subFunction(){\n         console.log(\"Some Output\");\n+        console.log(\"Some Output1\");\n     }\n     // Issue is not present when the line after the function name and opening { is empty\n     function subFunction(){\n \n-        console.log(\"OPutput\");\n+        console.log(\"Output\");\n     }\n }\n-\n-function anotherFunction(){\n-    console.log(\"Other Output\");\n-}\n"
+    },
+    {
+      "old_path": "src/assets/insert-multi-file-snippet.gif",
+      "new_path": "src/assets/insert-multi-file-snippet.gif",
+      "a_mode": "0",
+      "b_mode": "100644",
+      "new_file": true,
+      "renamed_file": false,
+      "deleted_file": false,
+      "diff": "Binary files /dev/null and b/src/assets/insert-multi-file-snippet.gif differ\n"
+    },
+    {
+      "old_path": "Screenshot_2020-12-02_at_15.29.33.png",
+      "new_path": "Screenshot.png",
+      "a_mode": "100644",
+      "b_mode": "100644",
+      "new_file": false,
+      "renamed_file": true,
+      "deleted_file": false,
+      "diff": ""
     }
   ]
 }

test/integration/test_infrastructure/mock_server.js

@@ -24,6 +24,16 @@ const createTextEndpoint = (path, response) =>
     return res(ctx.status(200), ctx.text(response));
   });
 
+const createQueryTextEndpoint = (path, queryResponseMap) =>
+  rest.get(`${API_URL_PREFIX}${path}`, (req, res, ctx) => {
+    const response = queryResponseMap[req.url.search];
+    if (!response) {
+      console.warn(`API call ${req.url.toString()} doesn't have a query handler.`);
+      return res(ctx.status(404));
+    }
+    return res(ctx.status(200), ctx.text(response));
+  });
+
 const createPostEndpoint = (path, response) =>
   rest.post(`${API_URL_PREFIX}${path}`, (req, res, ctx) => {
     return res(ctx.status(201), ctx.json(response));
@@ -50,5 +60,6 @@ module.exports = {
   createJsonEndpoint,
   createQueryJsonEndpoint,
   createTextEndpoint,
+  createQueryTextEndpoint,
   createPostEndpoint,
 };

test/integration/tree_view.test.js

@@ -11,8 +11,10 @@ const {
   getServer,
   createQueryJsonEndpoint,
   createJsonEndpoint,
+  createQueryTextEndpoint,
 } = require('./test_infrastructure/mock_server');
 const { GITLAB_URL } = require('./test_infrastructure/constants');
+const { ApiContentProvider } = require('../../src/review/api_content_provider');
 
 describe('GitLab tree view', () => {
   let server;
@@ -91,6 +93,10 @@ describe('GitLab tree view', () => {
           { ...openIssueResponse, title: 'Custom Query Issue' },
         ],
       }),
+      createQueryTextEndpoint(`/projects/278964/repository/files/src%2Ftest.js/raw`, {
+        '?ref=1f0fa02de1f6b913d674a8be10899fb8540237a9': 'Old Version',
+        '?ref=b6d6f6fd17b52b8cf4e961218c572805e9aa7463': 'New Version',
+      }),
     ]);
     await tokenService.setToken(GITLAB_URL, 'abcd-secret');
     await vscode.workspace.getConfiguration().update('gitlab.customQueries', customQuerySettings);
@@ -143,14 +149,92 @@ describe('GitLab tree view', () => {
     const mrFiles = mrContent.slice(1);
     assert.deepStrictEqual(
       mrFiles.map(f => f.resourceUri.path),
-      ['/.gitlab-ci.yml', '/README1.md', '/new_file.ts', '/test.js'],
+      [
+        '/.deleted.yml',
+        '/README1.md',
+        '/new_file.ts',
+        '/src/test.js',
+        '/src/assets/insert-multi-file-snippet.gif',
+        '/Screenshot.png',
+      ],
     );
     assert.deepStrictEqual(
       mrFiles.map(f => f.description),
-      ['[deleted] .', '[renamed] .', '[added] .', '.'],
+      ['[deleted] /', '[renamed] /', '[added] /', '/src', '[added] /src/assets', '[renamed] /'],
     );
   });
 
+  describe('clicking on a changed file', () => {
+    let mrFiles;
+
+    const getItem = filePath => mrFiles.filter(f => f.resourceUri.path === filePath).pop();
+
+    const getDiffArgs = item => {
+      assert.strictEqual(item.command.command, 'vscode.diff');
+      return item.command.arguments;
+    };
+
+    before(async () => {
+      const mergeRequestsAssignedToMe = await openCategory('Merge requests assigned to me');
+
+      assert.strictEqual(mergeRequestsAssignedToMe.length, 1);
+      const mrItem = mergeRequestsAssignedToMe[0];
+      assert.strictEqual(mrItem.label, '!33824 · Web IDE - remove unused actions (mappings)');
+
+      const mrContent = await dataProvider.getChildren(mrItem);
+      assert.strictEqual(mrContent[0].label, 'Description');
+
+      mrFiles = mrContent.slice(1);
+    });
+
+    it('should show the correct diff title', () => {
+      const item = getItem('/README1.md');
+      const [, , diffTitle] = getDiffArgs(item);
+      assert.strictEqual(diffTitle, 'README1.md (!33824)');
+    });
+
+    it('should not show diff for images', () => {
+      const item = getItem('/Screenshot.png');
+      assert.strictEqual(item.command.command, 'gl.noImageReview');
+    });
+
+    describe('Api content provider', () => {
+      let apiContentProvider;
+
+      before(() => {
+        apiContentProvider = new ApiContentProvider();
+      });
+
+      it('should fetch base content for a diff URI', async () => {
+        const item = getItem('/src/test.js');
+        const [baseUri] = getDiffArgs(item);
+        const content = await apiContentProvider.provideTextDocumentContent(baseUri);
+        assert.strictEqual(content, 'Old Version');
+      });
+
+      it('should fetch head content for a diff URI', async () => {
+        const item = getItem('/src/test.js');
+        const [, headUri] = getDiffArgs(item);
+        const content = await apiContentProvider.provideTextDocumentContent(headUri);
+        assert.strictEqual(content, 'New Version');
+      });
+
+      it('should show empty file when asked to fetch base content for added file', async () => {
+        const item = getItem('/new_file.ts');
+        const [baseUri] = getDiffArgs(item);
+        const content = await apiContentProvider.provideTextDocumentContent(baseUri);
+        assert.strictEqual(content, '');
+      });
+
+      it('should show empty file when asked to fetch head content for deleted file', async () => {
+        const item = getItem('/.deleted.yml');
+        const [, headUri] = getDiffArgs(item);
+        const content = await apiContentProvider.provideTextDocumentContent(headUri);
+        assert.strictEqual(content, '');
+      });
+    });
+  });
+
   it('handles full custom query for MR', async () => {
     const customMergeRequests = await openCategory('Custom GitLab Query for MR');