From 11a9a1079c8abdebef6c67401a0eaa48094fb87b Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 2 Oct 2019 14:28:38 -0700 Subject: [PATCH 1/2] Added button to get more info related to inheritEnv setting when prompt is displayed --- package.nls.json | 1 + src/client/common/utils/localize.ts | 1 + .../virtualEnvs/condaInheritEnvPrompt.ts | 11 ++- src/client/telemetry/index.ts | 3 +- .../condaInheritEnvPrompt.unit.test.ts | 73 ++++++++++++++++--- 5 files changed, 74 insertions(+), 15 deletions(-) diff --git a/package.nls.json b/package.nls.json index 1c70d070fa5c..ed684a6544db 100644 --- a/package.nls.json +++ b/package.nls.json @@ -125,6 +125,7 @@ "Logging.CurrentWorkingDirectory": "cwd:", "Common.doNotShowAgain": "Do not show again", "Common.reload": "Reload", + "Common.moreInfo": "More Info", "OutputChannelNames.languageServer": "Python Language Server", "OutputChannelNames.python": "Python", "OutputChannelNames.pythonTest": "Python Test Log", diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 883d86a28241..7137b4c97e07 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -33,6 +33,7 @@ export namespace Common { export const notNow = localize('Common.notNow', 'Not now'); export const doNotShowAgain = localize('Common.doNotShowAgain', 'Do not show again'); export const reload = localize('Common.reload', 'Reload'); + export const moreInfo = localize('Common.moreInfo', 'More Info'); } export namespace LanguageService { diff --git a/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts b/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts index 0dfec6eb5780..579820c0628e 100644 --- a/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts +++ b/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts @@ -6,8 +6,8 @@ import { ConfigurationTarget, Uri } from 'vscode'; import { IExtensionActivationService } from '../../activation/types'; import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; import { traceDecorators, traceError } from '../../common/logger'; -import { IPersistentStateFactory } from '../../common/types'; -import { InteractiveShiftEnterBanner, Interpreters } from '../../common/utils/localize'; +import { IBrowserService, IPersistentStateFactory } from '../../common/types'; +import { Common, InteractiveShiftEnterBanner, Interpreters } from '../../common/utils/localize'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { IInterpreterService, InterpreterType } from '../contracts'; @@ -19,6 +19,7 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService { constructor( @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, + @inject(IBrowserService) private browserService: IBrowserService, @inject(IApplicationShell) private readonly appShell: IApplicationShell, @inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory, @optional() public hasPromptBeenShownInCurrentSession: boolean = false @@ -43,8 +44,8 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService { if (!notificationPromptEnabled.value) { return; } - const prompts = [InteractiveShiftEnterBanner.bannerLabelYes(), InteractiveShiftEnterBanner.bannerLabelNo()]; - const telemetrySelections: ['Yes', 'No'] = ['Yes', 'No']; + const prompts = [InteractiveShiftEnterBanner.bannerLabelYes(), InteractiveShiftEnterBanner.bannerLabelNo(), Common.moreInfo()]; + const telemetrySelections: ['Yes', 'No', 'More Info'] = ['Yes', 'No', 'More Info']; const selection = await this.appShell.showInformationMessage(Interpreters.condaInheritEnvMessage(), ...prompts); sendTelemetryEvent(EventName.CONDA_INHERIT_ENV_PROMPT, undefined, { selection: selection ? telemetrySelections[prompts.indexOf(selection)] : undefined }); if (!selection) { @@ -54,6 +55,8 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService { await this.workspaceService.getConfiguration('terminal').update('integrated.inheritEnv', false, ConfigurationTarget.Global); } else if (selection === prompts[1]) { await notificationPromptEnabled.updateValue(false); + } else if (selection === prompts[2]) { + this.browserService.launch('https://code.visualstudio.com/updates/v1_36#_launch-terminals-with-clean-environments'); } } diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 80b0956bb734..9c48e49a4b7b 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -939,8 +939,9 @@ export interface IEventNamePropertyMapping { /** * `Yes` When 'Yes' option is selected * `No` When 'No' option is selected + * `More info` When 'More Info' option is selected */ - selection: 'Yes' | 'No' | undefined; + selection: 'Yes' | 'No' | 'More Info' | undefined; }; /** * Telemetry event sent with details when user clicks a button in the virtual environment prompt. diff --git a/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts index 8939ee595c57..23fb04296a89 100644 --- a/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts +++ b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts @@ -10,9 +10,9 @@ import * as TypeMoq from 'typemoq'; import { ConfigurationTarget, Uri, WorkspaceConfiguration } from 'vscode'; import { IApplicationShell, IWorkspaceService } from '../../../client/common/application/types'; import { PersistentStateFactory } from '../../../client/common/persistentState'; -import { IPersistentState, IPersistentStateFactory } from '../../../client/common/types'; +import { IBrowserService, IPersistentState, IPersistentStateFactory } from '../../../client/common/types'; import { createDeferred, createDeferredFromPromise, sleep } from '../../../client/common/utils/async'; -import { InteractiveShiftEnterBanner, Interpreters } from '../../../client/common/utils/localize'; +import { Common, InteractiveShiftEnterBanner, Interpreters } from '../../../client/common/utils/localize'; import { IInterpreterService, InterpreterType } from '../../../client/interpreter/contracts'; import { CondaInheritEnvPrompt, condaInheritEnvPromptKey } from '../../../client/interpreter/virtualEnvs/condaInheritEnvPrompt'; @@ -24,6 +24,7 @@ suite('Conda Inherit Env Prompt', async () => { let workspaceService: TypeMoq.IMock; let appShell: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; + let browserService: TypeMoq.IMock; let persistentStateFactory: IPersistentStateFactory; let notificationPromptEnabled: TypeMoq.IMock>; let condaInheritEnvPrompt: CondaInheritEnvPrompt; @@ -37,12 +38,13 @@ suite('Conda Inherit Env Prompt', async () => { setup(() => { workspaceService = TypeMoq.Mock.ofType(); appShell = TypeMoq.Mock.ofType(); + browserService = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); persistentStateFactory = mock(PersistentStateFactory); - condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); + condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, browserService.object, appShell.object, instance(persistentStateFactory)); }); test('Returns false if prompt has already been shown in the current session', async () => { - condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory), true); + condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, browserService.object, appShell.object, instance(persistentStateFactory), true); const workspaceConfig = TypeMoq.Mock.ofType(); interpreterService .setup(is => is.getActiveInterpreter(resource)) @@ -191,6 +193,7 @@ suite('Conda Inherit Env Prompt', async () => { setup(() => { workspaceService = TypeMoq.Mock.ofType(); appShell = TypeMoq.Mock.ofType(); + browserService = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); persistentStateFactory = mock(PersistentStateFactory); }); @@ -203,7 +206,7 @@ suite('Conda Inherit Env Prompt', async () => { const initializeInBackgroundDeferred = createDeferred(); initializeInBackground = sinon.stub(CondaInheritEnvPrompt.prototype, 'initializeInBackground'); initializeInBackground.callsFake(() => initializeInBackgroundDeferred.promise); - condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); + condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, browserService.object, appShell.object, instance(persistentStateFactory)); const promise = condaInheritEnvPrompt.activate(resource); const deferred = createDeferredFromPromise(promise); @@ -220,7 +223,7 @@ suite('Conda Inherit Env Prompt', async () => { test('Ignores errors raised by initializeInBackground()', async () => { initializeInBackground = sinon.stub(CondaInheritEnvPrompt.prototype, 'initializeInBackground'); initializeInBackground.rejects(new Error('Kaboom')); - condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); + condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, browserService.object, appShell.object, instance(persistentStateFactory)); await condaInheritEnvPrompt.activate(resource); assert.ok(initializeInBackground.calledOnce); }); @@ -232,6 +235,7 @@ suite('Conda Inherit Env Prompt', async () => { setup(() => { workspaceService = TypeMoq.Mock.ofType(); appShell = TypeMoq.Mock.ofType(); + browserService = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); persistentStateFactory = mock(PersistentStateFactory); }); @@ -245,7 +249,7 @@ suite('Conda Inherit Env Prompt', async () => { shouldShowPrompt.callsFake(() => Promise.resolve(true)); promptAndUpdate = sinon.stub(CondaInheritEnvPrompt.prototype, 'promptAndUpdate'); promptAndUpdate.callsFake(() => Promise.resolve(undefined)); - condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); + condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, browserService.object, appShell.object, instance(persistentStateFactory)); await condaInheritEnvPrompt.initializeInBackground(resource); assert.ok(shouldShowPrompt.calledOnce); assert.ok(promptAndUpdate.calledOnce); @@ -256,7 +260,7 @@ suite('Conda Inherit Env Prompt', async () => { shouldShowPrompt.callsFake(() => Promise.resolve(false)); promptAndUpdate = sinon.stub(CondaInheritEnvPrompt.prototype, 'promptAndUpdate'); promptAndUpdate.callsFake(() => Promise.resolve(undefined)); - condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); + condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, browserService.object, appShell.object, instance(persistentStateFactory)); await condaInheritEnvPrompt.initializeInBackground(resource); assert.ok(shouldShowPrompt.calledOnce); assert.ok(promptAndUpdate.notCalled); @@ -264,15 +268,16 @@ suite('Conda Inherit Env Prompt', async () => { }); suite('Method promptAndUpdate()', () => { - const prompts = [InteractiveShiftEnterBanner.bannerLabelYes(), InteractiveShiftEnterBanner.bannerLabelNo()]; + const prompts = [InteractiveShiftEnterBanner.bannerLabelYes(), InteractiveShiftEnterBanner.bannerLabelNo(), Common.moreInfo()]; setup(() => { workspaceService = TypeMoq.Mock.ofType(); appShell = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); persistentStateFactory = mock(PersistentStateFactory); + browserService = TypeMoq.Mock.ofType(); notificationPromptEnabled = TypeMoq.Mock.ofType>(); when(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).thenReturn(notificationPromptEnabled.object); - condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); + condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, browserService.object, appShell.object, instance(persistentStateFactory)); }); test('Does not display prompt if it is disabled', async () => { @@ -311,11 +316,16 @@ suite('Conda Inherit Env Prompt', async () => { .setup(n => n.updateValue(false)) .returns(() => Promise.resolve(undefined)) .verifiable(TypeMoq.Times.never()); + browserService + .setup(b => b.launch('https://code.visualstudio.com/updates/v1_36#_launch-terminals-with-clean-environments')) + .returns(() => undefined) + .verifiable(TypeMoq.Times.never()); await condaInheritEnvPrompt.promptAndUpdate(); verify(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).once(); verifyAll(); workspaceConfig.verifyAll(); notificationPromptEnabled.verifyAll(); + browserService.verifyAll(); }); test('Update terminal settings if `Yes` is selected', async () => { const workspaceConfig = TypeMoq.Mock.ofType(); @@ -339,11 +349,16 @@ suite('Conda Inherit Env Prompt', async () => { .setup(n => n.updateValue(false)) .returns(() => Promise.resolve(undefined)) .verifiable(TypeMoq.Times.never()); + browserService + .setup(b => b.launch('https://code.visualstudio.com/updates/v1_36#_launch-terminals-with-clean-environments')) + .returns(() => undefined) + .verifiable(TypeMoq.Times.never()); await condaInheritEnvPrompt.promptAndUpdate(); verify(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).once(); verifyAll(); workspaceConfig.verifyAll(); notificationPromptEnabled.verifyAll(); + browserService.verifyAll(); }); test('Disable notification prompt if `No` is selected', async () => { const workspaceConfig = TypeMoq.Mock.ofType(); @@ -367,11 +382,49 @@ suite('Conda Inherit Env Prompt', async () => { .setup(n => n.updateValue(false)) .returns(() => Promise.resolve(undefined)) .verifiable(TypeMoq.Times.once()); + browserService + .setup(b => b.launch('https://code.visualstudio.com/updates/v1_36#_launch-terminals-with-clean-environments')) + .returns(() => undefined) + .verifiable(TypeMoq.Times.never()); + await condaInheritEnvPrompt.promptAndUpdate(); + verify(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).once(); + verifyAll(); + workspaceConfig.verifyAll(); + notificationPromptEnabled.verifyAll(); + browserService.verifyAll(); + }); + test('Launch browser if `More info` option is selected', async () => { + const workspaceConfig = TypeMoq.Mock.ofType(); + notificationPromptEnabled + .setup(n => n.value) + .returns(() => true) + .verifiable(TypeMoq.Times.once()); + appShell + .setup(a => a.showInformationMessage(Interpreters.condaInheritEnvMessage(), ...prompts)) + .returns(() => Promise.resolve(Common.moreInfo())) + .verifiable(TypeMoq.Times.once()); + workspaceService + .setup(ws => ws.getConfiguration('terminal')) + .returns(() => workspaceConfig.object) + .verifiable(TypeMoq.Times.never()); + workspaceConfig + .setup(wc => wc.update('integrated.inheritEnv', false, ConfigurationTarget.Global)) + .returns(() => Promise.resolve()) + .verifiable(TypeMoq.Times.never()); + notificationPromptEnabled + .setup(n => n.updateValue(false)) + .returns(() => Promise.resolve(undefined)) + .verifiable(TypeMoq.Times.never()); + browserService + .setup(b => b.launch('https://code.visualstudio.com/updates/v1_36#_launch-terminals-with-clean-environments')) + .returns(() => undefined) + .verifiable(TypeMoq.Times.once()); await condaInheritEnvPrompt.promptAndUpdate(); verify(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).once(); verifyAll(); workspaceConfig.verifyAll(); notificationPromptEnabled.verifyAll(); + browserService.verifyAll(); }); }); }); From cf80d135487f40699799460f8a3412fe0b1abdb8 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 3 Oct 2019 00:16:34 -0700 Subject: [PATCH 2/2] Use aka.ms URL --- .../interpreter/virtualEnvs/condaInheritEnvPrompt.ts | 2 +- .../virtualEnvs/condaInheritEnvPrompt.unit.test.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts b/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts index 579820c0628e..ea5dc824dd43 100644 --- a/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts +++ b/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts @@ -56,7 +56,7 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService { } else if (selection === prompts[1]) { await notificationPromptEnabled.updateValue(false); } else if (selection === prompts[2]) { - this.browserService.launch('https://code.visualstudio.com/updates/v1_36#_launch-terminals-with-clean-environments'); + this.browserService.launch('https://aka.ms/AA66i8f'); } } diff --git a/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts index 23fb04296a89..0b469ae9300b 100644 --- a/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts +++ b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts @@ -317,7 +317,7 @@ suite('Conda Inherit Env Prompt', async () => { .returns(() => Promise.resolve(undefined)) .verifiable(TypeMoq.Times.never()); browserService - .setup(b => b.launch('https://code.visualstudio.com/updates/v1_36#_launch-terminals-with-clean-environments')) + .setup(b => b.launch('https://aka.ms/AA66i8f')) .returns(() => undefined) .verifiable(TypeMoq.Times.never()); await condaInheritEnvPrompt.promptAndUpdate(); @@ -350,7 +350,7 @@ suite('Conda Inherit Env Prompt', async () => { .returns(() => Promise.resolve(undefined)) .verifiable(TypeMoq.Times.never()); browserService - .setup(b => b.launch('https://code.visualstudio.com/updates/v1_36#_launch-terminals-with-clean-environments')) + .setup(b => b.launch('https://aka.ms/AA66i8f')) .returns(() => undefined) .verifiable(TypeMoq.Times.never()); await condaInheritEnvPrompt.promptAndUpdate(); @@ -383,7 +383,7 @@ suite('Conda Inherit Env Prompt', async () => { .returns(() => Promise.resolve(undefined)) .verifiable(TypeMoq.Times.once()); browserService - .setup(b => b.launch('https://code.visualstudio.com/updates/v1_36#_launch-terminals-with-clean-environments')) + .setup(b => b.launch('https://aka.ms/AA66i8f')) .returns(() => undefined) .verifiable(TypeMoq.Times.never()); await condaInheritEnvPrompt.promptAndUpdate(); @@ -416,7 +416,7 @@ suite('Conda Inherit Env Prompt', async () => { .returns(() => Promise.resolve(undefined)) .verifiable(TypeMoq.Times.never()); browserService - .setup(b => b.launch('https://code.visualstudio.com/updates/v1_36#_launch-terminals-with-clean-environments')) + .setup(b => b.launch('https://aka.ms/AA66i8f')) .returns(() => undefined) .verifiable(TypeMoq.Times.once()); await condaInheritEnvPrompt.promptAndUpdate();