Merge branch '215-fix-double-comments' into 'main'

fix: don't double send message from issue detail See merge request gitlab-org/gitlab-vscode-extension!125

Merge branch '215-fix-double-comments' into 'main'
fix: don't double send message from issue detail See merge request gitlab-org/gitlab-vscode-extension!125
6be86190 · Tomas Vik · 544b0ff4 · b7e1ee30 · 6be86190 · 6be86190
5 changed file
--- a/.vscode/launch.json
+++ b/.vscode/launch.json
@@ -9,7 +9,10 @@
      "runtimeExecutable": "${execPath}",
      "args": ["--disable-extensions", "--extensionDevelopmentPath=${workspaceRoot}"],
      "stopOnEntry": false,
-      "preLaunchTask": "${defaultBuildTask}"
+      "preLaunchTask": "${defaultBuildTask}",
+      "env": {
+        "NODE_ENV": "development"
+      }
    },

    {

--- a/src/webview/public/dev.html
+++ b/src/webview/public/dev.html
@@ -8,6 +8,7 @@
  </head>
  <body>
    <div id="app"></div>
-    <script nonce="{{nonce}}" src="{{devScriptUri}}"></script>
+    <script nonce="{{nonce}}" src="{{vendorUri}}"></script>
+    <script nonce="{{nonce}}" src="{{appScriptUri}}"></script>
  </body>
 </html>
--- a/src/webview_controller.js
+++ b/src/webview_controller.js
 const fs = require('fs');
 const path = require('path');
+const crypto = require('crypto');
 const vscode = require('vscode');
 const gitLabService = require('./gitlab_service');

@@ -9,19 +10,6 @@ const addDeps = ctx => {
  context = ctx;
 };

-const getNonce = () => {
-  let text = '';
-  const possible = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
-
-  // Temporarily disable eslint to be able to start enforcing stricter rules
-  // eslint-disable-next-line no-plusplus
-  for (let i = 0; i < 32; i++) {
-    text += possible.charAt(Math.floor(Math.random() * possible.length));
-  }
-
-  return text;
-};
-
 const getResources = panel => {
  const paths = {
    appScriptUri: 'src/webview/dist/js/app.js',
@@ -40,17 +28,18 @@ const getResources = panel => {
 };

 const getIndexPath = () => {
-  const isDev = !fs.existsSync(path.join(context.extensionPath, 'src/webview/dist/js/app.js'));
+  const isDev = process.env.NODE_ENV === 'development';

  return isDev ? 'src/webview/public/dev.html' : 'src/webview/public/index.html';
 };

 const replaceResources = panel => {
  const { appScriptUri, vendorUri, styleUri, devScriptUri } = getResources(panel);
+  const nonce = crypto.randomBytes(20).toString('hex');

  return fs
    .readFileSync(path.join(context.extensionPath, getIndexPath()), 'UTF-8')
-    .replace(/{{nonce}}/gm, getNonce())
+    .replace(/{{nonce}}/gm, nonce)
    .replace('{{styleUri}}', styleUri)
    .replace('{{vendorUri}}', vendorUri)
    .replace('{{appScriptUri}}', appScriptUri)
@@ -66,101 +55,96 @@ const createPanel = issuable => {
  });
 };

-function sendIssuableAndDiscussions(panel, issuable, discussions, appIsReady) {
-  if (!discussions || !appIsReady) return;
-  panel.webview.postMessage({ type: 'issuableFetch', issuable, discussions });
-}
+const createMessageHandler = (panel, issuable, workspaceFolder) => async message => {
+  if (message.command === 'renderMarkdown') {
+    const alteredMarkdown = message.markdown.replace(
+      /\(\/.*(\/-)?\/merge_requests\//,
+      '(/-/merge_requests/',
+    );
+    let rendered = await gitLabService.renderMarkdown(alteredMarkdown, workspaceFolder);
+    rendered = (rendered || '')
+      .replace(/ src=".*" alt/gim, ' alt')
+      .replace(/" data-src/gim, '" src')
+      .replace(/ href="\//gim, ` href="${vscode.workspace.getConfiguration('gitlab').instanceUrl}/`)
+      .replace(/\/master\/-\/merge_requests\//gim, '/-/merge_requests/');
+
+    panel.webview.postMessage({
+      type: 'markdownRendered',
+      ref: message.ref,
+      object: message.object,
+      markdown: rendered,
+    });
+  }

-async function handleCreate(panel, issuable, workspaceFolder) {
-  let discussions = false;
-  let labelEvents = false;
-  let appIsReady = false;
-  panel.webview.onDidReceiveMessage(async message => {
-    if (message.command === 'appReady') {
-      appIsReady = true;
-      sendIssuableAndDiscussions(panel, issuable, discussions, appIsReady);
+  if (message.command === 'saveNote') {
+    const response = await gitLabService.saveNote({
+      issuable: message.issuable,
+      note: message.note,
+      noteType: message.noteType,
+    });
+
+    if (response.success !== false) {
+      const newDiscussions = await gitLabService.fetchDiscussions(issuable);
+      panel.webview.postMessage({ type: 'issuableFetch', issuable, discussions: newDiscussions });
+      panel.webview.postMessage({ type: 'noteSaved' });
+    } else {
+      panel.webview.postMessage({ type: 'noteSaved', status: false });
    }
+  }
+};

-    if (message.command === 'renderMarkdown') {
-      // Temporarily disable eslint to be able to start enforcing stricter rules
-      // eslint-disable-next-line no-param-reassign
-      message.markdown = message.markdown.replace(
-        /\(\/.*(\/-)?\/merge_requests\//,
-        '(/-/merge_requests/',
-      );
-      let rendered = await gitLabService.renderMarkdown(message.markdown, workspaceFolder);
-      rendered = (rendered || '')
-        .replace(/ src=".*" alt/gim, ' alt')
-        .replace(/" data-src/gim, '" src')
-        .replace(
-          / href="\//gim,
-          ` href="${vscode.workspace.getConfiguration('gitlab').instanceUrl}/`,
-        )
-        .replace(/\/master\/-\/merge_requests\//gim, '/-/merge_requests/');
-
-      panel.webview.postMessage({
-        type: 'markdownRendered',
-        ref: message.ref,
-        object: message.object,
-        markdown: rendered,
-      });
-    }
+async function handleChangeViewState(panel, issuable) {
+  if (!panel.active) return;

-    if (message.command === 'saveNote') {
-      const response = await gitLabService.saveNote({
-        issuable: message.issuable,
-        note: message.note,
-        noteType: message.noteType,
-      });
-
-      if (response.status !== false) {
-        const newDiscussions = await gitLabService.fetchDiscussions(issuable);
-        panel.webview.postMessage({ type: 'issuableFetch', issuable, discussions: newDiscussions });
-        panel.webview.postMessage({ type: 'noteSaved' });
-      } else {
-        panel.webview.postMessage({ type: 'noteSaved', status: false });
+  const appReadyPromise = new Promise(resolve => {
+    const sub = panel.webview.onDidReceiveMessage(async message => {
+      if (message.command === 'appReady') {
+        sub.dispose();
+        resolve();
      }
-    }
+    });
  });

-  // TODO: Call APIs in parallel
-  discussions = await gitLabService.fetchDiscussions(issuable);
-  labelEvents = await gitLabService.fetchLabelEvents(issuable);
-  discussions = discussions.concat(labelEvents);
-  discussions.sort((a, b) => {
+  const [discussions, labelEvents] = await Promise.all([
+    gitLabService.fetchDiscussions(issuable),
+    gitLabService.fetchLabelEvents(issuable),
+  ]);
+
+  const combinedEvents = discussions.concat(labelEvents);
+  combinedEvents.sort((a, b) => {
    const aCreatedAt = a.label ? a.created_at : a.notes[0].created_at;
    const bCreatedAt = b.label ? b.created_at : b.notes[0].created_at;
    return aCreatedAt < bCreatedAt ? -1 : 1;
  });
-  sendIssuableAndDiscussions(panel, issuable, discussions, appIsReady);
+  await appReadyPromise;
+  panel.webview.postMessage({ type: 'issuableFetch', issuable, discussions: combinedEvents });
 }

+const getIconPathForIssuable = issuable => {
+  const getIconUri = (shade, file) =>
+    vscode.Uri.file(path.join(context.extensionPath, 'src', 'assets', 'images', shade, file));
+  const lightIssueIcon = getIconUri('light', 'issues.svg');
+  const lightMrIcon = getIconUri('light', 'merge_requests.svg');
+  const darkIssueIcon = getIconUri('dark', 'issues.svg');
+  const darkMrIcon = getIconUri('dark', 'merge_requests.svg');
+  const isMr = issuable.squash_commit_sha !== undefined;
+  return isMr
+    ? { light: lightMrIcon, dark: darkMrIcon }
+    : { light: lightIssueIcon, dark: darkIssueIcon };
+};
+
 async function create(issuable, workspaceFolder) {
  const panel = createPanel(issuable);
  const html = replaceResources(panel);
  panel.webview.html = html;
-
-  let lightIconUri = vscode.Uri.file(
-    path.join(context.extensionPath, 'src', 'assets', 'images', 'light', 'issues.svg'),
-  );
-  let darkIconUri = vscode.Uri.file(
-    path.join(context.extensionPath, 'src', 'assets', 'images', 'dark', 'issues.svg'),
-  );
-  if (issuable.squash_commit_sha !== undefined) {
-    lightIconUri = vscode.Uri.file(
-      path.join(context.extensionPath, 'src', 'assets', 'images', 'light', 'merge_requests.svg'),
-    );
-    darkIconUri = vscode.Uri.file(
-      path.join(context.extensionPath, 'src', 'assets', 'images', 'dark', 'merge_requests.svg'),
-    );
-  }
-  panel.iconPath = { light: lightIconUri, dark: darkIconUri };
+  panel.iconPath = getIconPathForIssuable(issuable);

  panel.onDidChangeViewState(() => {
-    handleCreate(panel, issuable, workspaceFolder);
+    handleChangeViewState(panel, issuable);
  });

-  handleCreate(panel, issuable, workspaceFolder);
+  panel.webview.onDidReceiveMessage(createMessageHandler(panel, issuable, workspaceFolder));
+  return panel;
 }

 exports.addDeps = addDeps;

--- a/test/integration/test_infrastructure/mock_server.js
+++ b/test/integration/test_infrastructure/mock_server.js
@@ -24,6 +24,11 @@ const createTextEndpoint = (path, response) =>
    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));
+  });
+
 const notFoundByDefault = rest.get(/.*/, (req, res, ctx) => res(ctx.status(404)));

 const getServer = (handlers = []) => {
@@ -37,4 +42,10 @@ const getServer = (handlers = []) => {
  return server;
 };

-module.exports = { getServer, createJsonEndpoint, createQueryJsonEndpoint, createTextEndpoint };
+module.exports = {
+  getServer,
+  createJsonEndpoint,
+  createQueryJsonEndpoint,
+  createTextEndpoint,
+  createPostEndpoint,
+};
--- a/test/integration/webview.test.js
+++ b/test/integration/webview.test.js
+const assert = require('assert');
+const vscode = require('vscode');
+const sinon = require('sinon');
+const EventEmitter = require('events');
+const webviewController = require('../../src/webview_controller');
+const { tokenService } = require('../../src/services/token_service');
+const openIssueResponse = require('./fixtures/rest/open_issue.json');
+
+const {
+  getServer,
+  createJsonEndpoint,
+  createPostEndpoint,
+} = require('./test_infrastructure/mock_server');
+const { GITLAB_URL } = require('./test_infrastructure/constants');
+
+const waitForMessage = (panel, type) =>
+  new Promise(resolve => {
+    const sub = panel.webview.onDidReceiveMessage(message => {
+      if (message.type !== type) return;
+      sub.dispose();
+      resolve(message);
+    });
+  });
+
+describe('GitLab webview', () => {
+  let server;
+  let webviewPanel;
+  const sandbox = sinon.createSandbox();
+
+  before(async () => {
+    server = getServer([
+      createJsonEndpoint(
+        `/projects/${openIssueResponse.project_id}/issues/${openIssueResponse.iid}/discussions`,
+        [],
+      ),
+      createJsonEndpoint(
+        `/projects/${openIssueResponse.project_id}/issues/${openIssueResponse.iid}/resource_label_events`,
+        [],
+      ),
+      createPostEndpoint(
+        `/projects/${openIssueResponse.project_id}/issues/${openIssueResponse.iid}/notes`,
+        {},
+      ),
+    ]);
+    await tokenService.setToken(GITLAB_URL, 'abcd-secret');
+  });
+
+  /*
+  This method replaces the mechanism that the Webview panel uses for sending messages between
+  the extension and the webview. This is necessary since we can't control the webview and so
+  we need to be able to simulate events triggered by the webview and see that the extension
+  handles them well.
+  */
+  const replacePanelEventSystem = () => {
+    const { createWebviewPanel } = vscode.window;
+    sandbox.stub(vscode.window, 'createWebviewPanel').callsFake((viewType, title, column) => {
+      const panel = createWebviewPanel(viewType, title, column);
+      const eventEmitter = new EventEmitter();
+      sandbox
+        .stub(panel.webview, 'postMessage')
+        .callsFake(message => eventEmitter.emit('', message));
+      sandbox.stub(panel.webview, 'onDidReceiveMessage').callsFake(listener => {
+        eventEmitter.on('', listener);
+        return { dispose: () => {} };
+      });
+      return panel;
+    });
+  };
+
+  beforeEach(async () => {
+    server.resetHandlers();
+    replacePanelEventSystem();
+    webviewPanel = await webviewController.create(
+      openIssueResponse,
+      vscode.workspace.workspaceFolders[0],
+    );
+  });
+
+  afterEach(async () => {
+    sandbox.restore();
+  });
+
+  after(async () => {
+    server.close();
+    await tokenService.setToken(GITLAB_URL, undefined);
+  });
+
+  it('sends a message', async () => {
+    webviewPanel.webview.postMessage({
+      command: 'saveNote',
+      issuable: openIssueResponse,
+      note: 'Hello',
+      noteType: { path: 'issues' },
+    });
+    const sentMessage = await waitForMessage(webviewPanel, 'noteSaved');
+    assert.strictEqual(sentMessage.type, 'noteSaved');
+    assert.strictEqual(sentMessage.status, undefined);
+  });
+});