Add a 'More Information' option in telemetry popup

extensions/ql-vscode/CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - Add a status bar item for the CodeQL CLI to show the current version. [#741](https://github.com/github/vscode-codeql/pull/741)
 - Fix version constraint for flagging CLI support of non-destructive updates [#744](https://github.com/github/vscode-codeql/pull/744)
+- Add a _More Information_ button in the telemetry popup. [#742](https://github.com/github/vscode-codeql/pull/742)
 
 ## 1.4.1 - 29 January 2021
 

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,41 @@ 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): 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.
+  // To prevent an infinite loop, if the user clicks 'more information' 5 times, close the dialog and return cancelled
+  let count = 0;
+  do {
+    chosenItem = await Window.showInformationMessage(message, { modal: true }, urlItem, yesItem, noItem);
+    if (chosenItem === urlItem) {
+      await env.openExternal(Uri.parse(url, true));
+    }
+    count++;
+  } while (chosenItem === urlItem && count < 5);
+
+  if (!chosenItem || chosenItem.title === urlItem.title) {
+    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,14 +164,9 @@ 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.',
-          // We make this dialog modal for now.
-          // 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.
-          true
+        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'
         );
       }
       if (result !== undefined) {

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,91 @@ 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 chooses 'yes'
+      showInformationMessageSpy.onCall(0).resolvesArg(2);
+      const res = showBinaryChoiceDialog('xxx');
+      res.then((val) => {
+        expect(val).to.eq(true);
+        done();
+      }).catch(e => fail(e));
+    });
+
+    it('should show a binary choice dialog and return `no`', (done) => {
+      // pretend user chooses 'no'
+      showInformationMessageSpy.onCall(0).resolvesArg(3);
+      const res = showBinaryChoiceDialog('xxx');
+      res.then((val) => {
+        expect(val).to.eq(false);
+        done();
+      }).catch(e => fail(e));
+    });
+
+    it('should show an info dialog and confirm the action', (done) => {
+      // pretend user chooses to run action
+      showInformationMessageSpy.onCall(0).resolvesArg(1);
+      const res = showInformationMessageWithAction('xxx', 'yyy');
+      res.then((val) => {
+        expect(val).to.eq(true);
+        done();
+      }).catch(e => fail(e));
+    });
+
+    it('should show an action dialog and avoid choosing the action', (done) => {
+      // pretend user does not choose to run action
+      showInformationMessageSpy.onCall(0).resolves(undefined);
+      const res = showInformationMessageWithAction('xxx', 'yyy');
+      res.then((val) => {
+        expect(val).to.eq(false);
+        done();
+      }).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 clicks 'yes'
+      showInformationMessageSpy.onCall(0).resolvesArg(2);
+      showInformationMessageSpy.onCall(1).resolvesArg(2);
+      showInformationMessageSpy.onCall(2).resolvesArg(3);
+      const res = showBinaryChoiceWithUrlDialog('xxx', 'invalid:url');
+      res.then((val) => {
+        expect(val).to.eq(true);
+        done();
+      }).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 clicks 'no'
+      showInformationMessageSpy.onCall(0).resolvesArg(2);
+      showInformationMessageSpy.onCall(1).resolvesArg(2);
+      showInformationMessageSpy.onCall(2).resolvesArg(4);
+      const res = showBinaryChoiceWithUrlDialog('xxx', 'invalid:url');
+      res.then((val) => {
+        expect(val).to.eq(false);
+        done();
+      }).catch(e => fail(e));
+    });
+
+    it('should show a binary choice dialog and exit after clcking `more info` 5 times', (done) => {
+      // pretend user clicks on the url twice and then clicks 'no'
+      showInformationMessageSpy.onCall(0).resolvesArg(2);
+      showInformationMessageSpy.onCall(1).resolvesArg(2);
+      showInformationMessageSpy.onCall(2).resolvesArg(2);
+      showInformationMessageSpy.onCall(3).resolvesArg(2);
+      showInformationMessageSpy.onCall(4).resolvesArg(2);
+      const res = showBinaryChoiceWithUrlDialog('xxx', 'invalid:url');
+      res.then((val) => {
+        // No choie was made
+        expect(val).to.eq(undefined);
+        expect(showInformationMessageSpy.getCalls().length).to.eq(5);
+        done();
+      }).catch(e => fail(e));
+    });
+  });
 });

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

@@ -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);