Skip to content

Added button to get more info related to inheritEnv setting when prompt is dispalyed #7742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 3, 2019
Merged

Added button to get more info related to inheritEnv setting when prompt is dispalyed #7742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
11a9a10
Added button to get more info related to inheritEnv setting when prom…
Oct 2, 2019
cf80d13
Use aka.ms URL
Oct 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.nls.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions src/client/common/utils/localize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
11 changes: 7 additions & 4 deletions src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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
Expand All @@ -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) {
Expand All @@ -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://aka.ms/AA66i8f');
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/client/telemetry/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
73 changes: 63 additions & 10 deletions src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -24,6 +24,7 @@ suite('Conda Inherit Env Prompt', async () => {
let workspaceService: TypeMoq.IMock<IWorkspaceService>;
let appShell: TypeMoq.IMock<IApplicationShell>;
let interpreterService: TypeMoq.IMock<IInterpreterService>;
let browserService: TypeMoq.IMock<IBrowserService>;
let persistentStateFactory: IPersistentStateFactory;
let notificationPromptEnabled: TypeMoq.IMock<IPersistentState<any>>;
let condaInheritEnvPrompt: CondaInheritEnvPrompt;
Expand All @@ -37,12 +38,13 @@ suite('Conda Inherit Env Prompt', async () => {
setup(() => {
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
browserService = TypeMoq.Mock.ofType<IBrowserService>();
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
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<WorkspaceConfiguration>();
interpreterService
.setup(is => is.getActiveInterpreter(resource))
Expand Down Expand Up @@ -191,6 +193,7 @@ suite('Conda Inherit Env Prompt', async () => {
setup(() => {
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
browserService = TypeMoq.Mock.ofType<IBrowserService>();
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
persistentStateFactory = mock(PersistentStateFactory);
});
Expand All @@ -203,7 +206,7 @@ suite('Conda Inherit Env Prompt', async () => {
const initializeInBackgroundDeferred = createDeferred<void>();
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);
Expand All @@ -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);
});
Expand All @@ -232,6 +235,7 @@ suite('Conda Inherit Env Prompt', async () => {
setup(() => {
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
browserService = TypeMoq.Mock.ofType<IBrowserService>();
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
persistentStateFactory = mock(PersistentStateFactory);
});
Expand All @@ -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);
Expand All @@ -256,23 +260,24 @@ 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);
});
});

suite('Method promptAndUpdate()', () => {
const prompts = [InteractiveShiftEnterBanner.bannerLabelYes(), InteractiveShiftEnterBanner.bannerLabelNo()];
const prompts = [InteractiveShiftEnterBanner.bannerLabelYes(), InteractiveShiftEnterBanner.bannerLabelNo(), Common.moreInfo()];
setup(() => {
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
persistentStateFactory = mock(PersistentStateFactory);
browserService = TypeMoq.Mock.ofType<IBrowserService>();
notificationPromptEnabled = TypeMoq.Mock.ofType<IPersistentState<any>>();
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 () => {
Expand Down Expand Up @@ -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://aka.ms/AA66i8f'))
.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<WorkspaceConfiguration>();
Expand All @@ -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://aka.ms/AA66i8f'))
.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<WorkspaceConfiguration>();
Expand All @@ -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://aka.ms/AA66i8f'))
.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<WorkspaceConfiguration>();
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://aka.ms/AA66i8f'))
.returns(() => undefined)
.verifiable(TypeMoq.Times.once());
await condaInheritEnvPrompt.promptAndUpdate();
verify(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).once();
verifyAll();
workspaceConfig.verifyAll();
notificationPromptEnabled.verifyAll();
browserService.verifyAll();
});
});
});