Add a 'More Information' option in telemetry popup

This commit moves the URL in the text of the telemetry pop up to a
separate button. Clicking on the button will automatically open the
link in the browser.

Also, adds unit tests for the open dialog helper functions.

extensions/ql-vscode/CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## [UNRELEASED]
 
+- Add a _More Information_ button in the telemetry popup. [#742](https://github.com/github/vscode-codeql/pull/742)
+
 ## 1.4.1 - 29 January 2021
 
 - Reword the telemetry modal dialog box. [#738](https://github.com/github/vscode-codeql/pull/738)

extensions/ql-vscode/src/helpers.ts

@@ -4,8 +4,10 @@ import * as yaml from 'js-yaml';
 import * as path from 'path';
 import {
   ExtensionContext,
+  Uri,
   window as Window,
-  workspace
+  workspace,
+  env
 } from 'vscode';
 import { CodeQLCliServer } from './cli';
 import { logger } from './logging';
@@ -100,6 +102,38 @@ export async function showBinaryChoiceDialog(message: string, modal = true): Pro
   return chosenItem?.title === yesItem.title;
 }
 
+/**
+ * Opens a modal dialog for the user to make a yes/no choice.
+ *
+ * @param message The message to show.
+ * @param modal If true (the default), show a modal dialog box, otherwise dialog is non-modal and can
+ *        be closed even if the user does not make a choice.
+ *
+ * @return
+ *  `true` if the user clicks 'Yes',
+ *  `false` if the user clicks 'No' or cancels the dialog,
+ *  `undefined` if the dialog is closed without the user making a choice.
+ */
+export async function showBinaryChoiceWithUrlDialog(message: string, url: string, modal = true): Promise<boolean | undefined> {
+  const urlItem = { title: 'More Information', isCloseAffordance: false };
+  const yesItem = { title: 'Yes', isCloseAffordance: false };
+  const noItem = { title: 'No', isCloseAffordance: true };
+  let chosenItem;
+
+  // Keep the dialog open as long as the user is clicking the 'more information' option.
+  do {
+    chosenItem = await Window.showInformationMessage(message, { modal }, urlItem, yesItem, noItem);
+    if (chosenItem === urlItem) {
+      await env.openExternal(Uri.parse(url, true));
+    }
+  } while (chosenItem === urlItem);
+
+  if (!chosenItem) {
+    return undefined;
+  }
+  return chosenItem?.title === yesItem.title;
+}
+
 /**
  * Show an information message with a customisable action.
  * @param message The message to show.

extensions/ql-vscode/src/telemetry.ts

@@ -4,7 +4,7 @@ import { ConfigListener, CANARY_FEATURES, ENABLE_TELEMETRY, GLOBAL_ENABLE_TELEME
 import * as appInsights from 'applicationinsights';
 import { logger } from './logging';
 import { UserCancellationException } from './commandRunner';
-import { showBinaryChoiceDialog } from './helpers';
+import { showBinaryChoiceWithUrlDialog } from './helpers';
 
 // Key is injected at build time through the APP_INSIGHTS_KEY environment variable.
 const key = 'REPLACE-APP-INSIGHTS-KEY';
@@ -164,10 +164,11 @@ export class TelemetryListener extends ConfigListener {
       let result = undefined;
       if (GLOBAL_ENABLE_TELEMETRY.getValue()) {
         // Extension won't start until this completes.
-        result = await showBinaryChoiceDialog(
-          'Does the CodeQL Extension by GitHub have your permission to collect usage data and metrics to help us improve CodeQL for VSCode?\n\nFor details of what we collect and how we use it, see https://github.com/github/vscode-codeql/blob/main/extensions/ql-vscode/TELEMETRY.md.',
+        result = await showBinaryChoiceWithUrlDialog(
+          'Does the CodeQL Extension by GitHub have your permission to collect usage data and metrics to help us improve CodeQL for VSCode?',
+          'https://github.com/github/vscode-codeql/blob/main/extensions/ql-vscode/TELEMETRY.md',
           // We make this dialog modal for now.
-          // Note that  non-modal dialogs allow for markdown in their text, but modal dialogs do not.
+          // Note that non-modal dialogs allow for markdown in their text, but modal dialogs do not.
           // If we do decide to keep this dialog as modal, then this implementation can change and
           // we no longer need to call Promise.race. Before committing this PR, we need to make
           // this decision.

extensions/ql-vscode/src/vscode-tests/no-workspace/helpers.test.ts

@@ -1,14 +1,16 @@
 import { expect } from 'chai';
 import 'mocha';
-import { ExtensionContext, Memento } from 'vscode';
+import { ExtensionContext, Memento, window } from 'vscode';
 import * as yaml from 'js-yaml';
 import * as tmp from 'tmp';
 import * as path from 'path';
 import * as fs from 'fs-extra';
 import * as sinon from 'sinon';
 
-import { getInitialQueryContents, InvocationRateLimiter, isLikelyDbLanguageFolder } from '../../helpers';
+import { getInitialQueryContents, InvocationRateLimiter, isLikelyDbLanguageFolder, showBinaryChoiceDialog, showBinaryChoiceWithUrlDialog, showInformationMessageWithAction } from '../../helpers';
 import { reportStreamProgress } from '../../commandRunner';
+import Sinon = require('sinon');
+import { fail } from 'assert';
 
 describe('helpers', () => {
   let sandbox: sinon.SinonSandbox;
@@ -225,4 +227,81 @@ describe('helpers', () => {
       message: 'My prefix (Size unknown)',
     });
   });
+
+  describe('open dialog', () => {
+    let showInformationMessageSpy: Sinon.SinonStub;
+    beforeEach(() => {
+      showInformationMessageSpy = sandbox.stub(window, 'showInformationMessage');
+    });
+
+    it('should show a binary choice dialog and return `yes`', (done) => {
+      // pretend user clicks on the url twice and then 'yes'
+      showInformationMessageSpy.onCall(0).resolvesArg(2);
+      const res = showBinaryChoiceDialog('xxx');
+      res.then((val) => {
+        expect(val).to.eq(true);
+        done();
+      });
+      res.catch(e => fail(e));
+    });
+
+    it('should show a binary choice dialog and return `no`', (done) => {
+      // pretend user clicks on the url twice and then 'yes'
+      showInformationMessageSpy.onCall(0).resolvesArg(3);
+      const res = showBinaryChoiceDialog('xxx');
+      res.then((val) => {
+        expect(val).to.eq(false);
+        done();
+      });
+      res.catch(e => fail(e));
+    });
+
+    it('should show an info dialog and confirm the action', (done) => {
+      // pretend user clicks on the url twice and then 'yes'
+      showInformationMessageSpy.onCall(0).resolvesArg(1);
+      const res = showInformationMessageWithAction('xxx', 'yyy');
+      res.then((val) => {
+        expect(val).to.eq(true);
+        done();
+      });
+      res.catch(e => fail(e));
+    });
+
+    it('should show an info dialog and not confirm the', (done) => {
+      // pretend user clicks on the url twice and then 'yes'
+      showInformationMessageSpy.onCall(0).resolves(undefined);
+      const res = showInformationMessageWithAction('xxx', 'yyy');
+      res.then((val) => {
+        expect(val).to.eq(false);
+        done();
+      });
+      res.catch(e => fail(e));
+    });
+
+    it('should show a binary choice dialog with a url and return `yes`', (done) => {
+      // pretend user clicks on the url twice and then 'yes'
+      showInformationMessageSpy.onCall(0).resolvesArg(2);
+      showInformationMessageSpy.onCall(1).resolvesArg(2);
+      showInformationMessageSpy.onCall(0).resolvesArg(3);
+      const res = showBinaryChoiceWithUrlDialog('xxx', 'invalid:url');
+      res.then((val) => {
+        expect(val).to.eq(true);
+        done();
+      });
+      res.catch(e => fail(e));
+    });
+
+    it('should show a binary choice dialog with a url and return `no`', (done) => {
+      // pretend user clicks on the url twice and then 'yes'
+      showInformationMessageSpy.onCall(0).resolvesArg(2);
+      showInformationMessageSpy.onCall(1).resolvesArg(2);
+      showInformationMessageSpy.onCall(0).resolvesArg(4);
+      const res = showBinaryChoiceWithUrlDialog('xxx', 'invalid:url');
+      res.then((val) => {
+        expect(val).to.eq(false);
+        done();
+      });
+      res.catch(e => fail(e));
+    });
+  });
 });

extensions/ql-vscode/src/vscode-tests/no-workspace/telemetry.test.ts

@@ -15,7 +15,7 @@ const expect = chai.expect;
 
 const sandbox = sinon.createSandbox();
 
-describe('telemetry reporting', function() {
+describe.only('telemetry reporting', function() {
   // setting preferences can trigger lots of background activity
   // so need to bump up the timeout of this test.
   this.timeout(10000);
@@ -249,7 +249,7 @@ describe('telemetry reporting', function() {
   });
 
   it('should request permission if popup has never been seen before', async () => {
-    sandbox.stub(window, 'showInformationMessage').resolvesArg(2 /* "yes" item */);
+    sandbox.stub(window, 'showInformationMessage').resolvesArg(3 /* "yes" item */);
     await ctx.globalState.update('telemetry-request-viewed', false);
     await enableTelemetry('codeQL.telemetry', false);
 
@@ -262,7 +262,7 @@ describe('telemetry reporting', function() {
   });
 
   it('should prevent telemetry if permission is denied', async () => {
-    sandbox.stub(window, 'showInformationMessage').resolvesArg(3 /* "no" item */);
+    sandbox.stub(window, 'showInformationMessage').resolvesArg(4 /* "no" item */);
     await ctx.globalState.update('telemetry-request-viewed', false);
     await enableTelemetry('codeQL.telemetry', true);