diff --git a/news/1 Enhancements/13535.md b/news/1 Enhancements/13535.md new file mode 100644 index 000000000000..a496f632f66b --- /dev/null +++ b/news/1 Enhancements/13535.md @@ -0,0 +1 @@ +Update "Tip" notification for new users to either show the existing tip, a link to a feedback survey or nothing. diff --git a/package-lock.json b/package-lock.json index 597fb5e4d4ce..da565200ede0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24662,9 +24662,9 @@ } }, "tas-client": { - "version": "0.0.875", - "resolved": "https://registry.npmjs.org/tas-client/-/tas-client-0.0.875.tgz", - "integrity": "sha512-Y375pAWdOAFKAs2gZHVC3SAxGp8vHNRTpl7W6rBaB8YgZbAX0h0NHUubqHtyuNwH6VF9qy2ckagsuXZP0JignQ==", + "version": "0.0.950", + "resolved": "https://registry.npmjs.org/tas-client/-/tas-client-0.0.950.tgz", + "integrity": "sha512-AvCNjvfouxJyKln+TsobOBO5KmXklL9+FlxrEPlIgaixy1TxCC2v2Vs/MflCiyHlGl+BeIStP4oAVPqo5c0pIA==", "requires": { "axios": "^0.19.0" } @@ -27355,11 +27355,11 @@ "integrity": "sha512-s/z5ZqSe7VpoXJ6JQcvwRiPPA3nG0nAcJ/HH03zoU6QaFfnkcgPK+HshC3WKPPnC2G08xA0iRB6h7kmyBB5Adg==" }, "vscode-tas-client": { - "version": "0.0.864", - "resolved": "https://registry.npmjs.org/vscode-tas-client/-/vscode-tas-client-0.0.864.tgz", - "integrity": "sha512-mRMpeTVQ8Rx3p4yF9y8AABanzbqtLRdJA99dzeQ9MdIHsSEdp0kEwxqayzDhNHDdp8vNbQkHN8zMxSvm/ZWdpg==", + "version": "0.1.4", + "resolved": "https://registry.npmjs.org/vscode-tas-client/-/vscode-tas-client-0.1.4.tgz", + "integrity": "sha512-sC+kvLUwb6ecC7+ZoxzDtvvktVUJ3jZq6mvJpfYHeLlbj4hUpNsZ79u65/mukoO8E8C7UQUCLdWdyn/evp+oNA==", "requires": { - "tas-client": "0.0.875" + "tas-client": "0.0.950" } }, "vscode-test": { diff --git a/package.json b/package.json index 6fb4feaf542c..c0c440b64a19 100644 --- a/package.json +++ b/package.json @@ -3528,7 +3528,7 @@ "vscode-languageclient": "^7.0.0-next.8", "vscode-languageserver": "^7.0.0-next.6", "vscode-languageserver-protocol": "^3.16.0-next.6", - "vscode-tas-client": "^0.0.864", + "vscode-tas-client": "^0.1.4", "vsls": "^0.3.1291", "winreg": "^1.2.4", "winston": "^3.2.1", diff --git a/src/client/common/experiments/groups.ts b/src/client/common/experiments/groups.ts index 759c40b33af2..94b9ebc40eb9 100644 --- a/src/client/common/experiments/groups.ts +++ b/src/client/common/experiments/groups.ts @@ -91,3 +91,10 @@ export enum RemoveKernelToolbarInInteractiveWindow { export enum TryPylance { experiment = 'tryPylance' } + +// Experiment for the content of the tip being displayed on first extension launch: +// interpreter selection tip, feedback survey or nothing. +export enum SurveyAndInterpreterTipNotification { + tipExperiment = 'pythonTipPromptWording', + surveyExperiment = 'pythonMailingListPromptWording' +} diff --git a/src/client/common/experiments/service.ts b/src/client/common/experiments/service.ts index 6d30c0915349..b318732dacde 100644 --- a/src/client/common/experiments/service.ts +++ b/src/client/common/experiments/service.ts @@ -104,6 +104,14 @@ export class ExperimentService implements IExperimentService { return this.experimentationService.isCachedFlightEnabled(experiment); } + public async getExperimentValue(experiment: string): Promise { + if (!this.experimentationService || this._optOutFrom.includes('All') || this._optOutFrom.includes(experiment)) { + return; + } + + return this.experimentationService.getTreatmentVariableAsync('vscode', experiment); + } + private logExperiments() { const experiments = this.globalState.get<{ features: string[] }>(EXP_MEMENTO_KEY, { features: [] }); diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 11ef8aaead62..6c071b95529f 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -637,6 +637,7 @@ export interface IExperimentsManager { export const IExperimentService = Symbol('IExperimentService'); export interface IExperimentService { inExperiment(experimentName: string): Promise; + getExperimentValue(experimentName: string): Promise; } export type InterpreterConfigurationScope = { uri: Resource; configTarget: ConfigurationTarget }; diff --git a/src/client/interpreter/display/interpreterSelectionTip.ts b/src/client/interpreter/display/interpreterSelectionTip.ts index d03191d335f6..965312b1b33d 100644 --- a/src/client/interpreter/display/interpreterSelectionTip.ts +++ b/src/client/interpreter/display/interpreterSelectionTip.ts @@ -6,31 +6,72 @@ import { inject, injectable } from 'inversify'; import { IExtensionSingleActivationService } from '../../activation/types'; import { IApplicationShell } from '../../common/application/types'; -import { IPersistentState, IPersistentStateFactory } from '../../common/types'; +import { SurveyAndInterpreterTipNotification } from '../../common/experiments/groups'; +import { IBrowserService, IExperimentService, IPersistentState, IPersistentStateFactory } from '../../common/types'; import { swallowExceptions } from '../../common/utils/decorators'; -import { Common, Interpreters } from '../../common/utils/localize'; +import { Common } from '../../common/utils/localize'; +import { sendTelemetryEvent } from '../../telemetry'; +import { EventName } from '../../telemetry/constants'; + +enum NotificationType { + Tip, + Survey, + NoPrompt +} @injectable() export class InterpreterSelectionTip implements IExtensionSingleActivationService { private readonly storage: IPersistentState; + private notificationType: NotificationType; + private notificationContent: string | undefined; + constructor( @inject(IApplicationShell) private readonly shell: IApplicationShell, - @inject(IPersistentStateFactory) private readonly factory: IPersistentStateFactory + @inject(IPersistentStateFactory) private readonly factory: IPersistentStateFactory, + @inject(IExperimentService) private readonly experiments: IExperimentService, + @inject(IBrowserService) private browserService: IBrowserService ) { this.storage = this.factory.createGlobalPersistentState('InterpreterSelectionTip', false); + this.notificationType = NotificationType.NoPrompt; } + public async activate(): Promise { if (this.storage.value) { return; } + + if (await this.experiments.inExperiment(SurveyAndInterpreterTipNotification.surveyExperiment)) { + this.notificationType = NotificationType.Survey; + this.notificationContent = await this.experiments.getExperimentValue( + SurveyAndInterpreterTipNotification.surveyExperiment + ); + } else if (await this.experiments.inExperiment(SurveyAndInterpreterTipNotification.tipExperiment)) { + this.notificationType = NotificationType.Tip; + this.notificationContent = await this.experiments.getExperimentValue( + SurveyAndInterpreterTipNotification.tipExperiment + ); + } + this.showTip().ignoreErrors(); } @swallowExceptions('Failed to display tip') private async showTip() { - const selection = await this.shell.showInformationMessage(Interpreters.selectInterpreterTip(), Common.gotIt()); - if (selection !== Common.gotIt()) { - return; + if (this.notificationType === NotificationType.Tip) { + await this.shell.showInformationMessage(this.notificationContent!, Common.gotIt()); + sendTelemetryEvent(EventName.ACTIVATION_TIP_PROMPT, undefined); + } else if (this.notificationType === NotificationType.Survey) { + const selection = await this.shell.showInformationMessage( + this.notificationContent!, + Common.bannerLabelYes(), + Common.bannerLabelNo() + ); + + if (selection === Common.bannerLabelYes()) { + sendTelemetryEvent(EventName.ACTIVATION_SURVEY_PROMPT, undefined); + this.browserService.launch('https://aka.ms/mailingListSurvey'); + } } + await this.storage.updateValue(true); } } diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index 766e2f0e2f01..03a948a47c5a 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -74,6 +74,8 @@ export enum EventName { PLAY_BUTTON_ICON_DISABLED = 'PLAY_BUTTON_ICON.DISABLED', PYTHON_WEB_APP_RELOAD = 'PYTHON_WEB_APP.RELOAD', EXTENSION_SURVEY_PROMPT = 'EXTENSION_SURVEY_PROMPT', + ACTIVATION_TIP_PROMPT = 'ACTIVATION_TIP_PROMPT', + ACTIVATION_SURVEY_PROMPT = 'ACTIVATION_SURVEY_PROMPT', PYTHON_LANGUAGE_SERVER_CURRENT_SELECTION = 'PYTHON_LANGUAGE_SERVER_CURRENT_SELECTION', PYTHON_LANGUAGE_SERVER_LIST_BLOB_STORE_PACKAGES = 'PYTHON_LANGUAGE_SERVER.LIST_BLOB_PACKAGES', diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index a777eb4afad7..52294d80c1a5 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1398,6 +1398,14 @@ export interface IEventNamePropertyMapping { */ selection: 'Yes' | 'Maybe later' | 'Do not show again' | undefined; }; + /** + * Telemetry event sent when the Python interpreter tip is shown on activation for new users. + */ + [EventName.ACTIVATION_TIP_PROMPT]: never | undefined; + /** + * Telemetry event sent when the feedback survey prompt is shown on activation for new users, and they click on the survey link. + */ + [EventName.ACTIVATION_SURVEY_PROMPT]: never | undefined; /** * Telemetry event sent when 'Extract Method' command is invoked */ diff --git a/src/test/common/experiments/service.unit.test.ts b/src/test/common/experiments/service.unit.test.ts index bdd07b278f8f..3f9f055274a3 100644 --- a/src/test/common/experiments/service.unit.test.ts +++ b/src/test/common/experiments/service.unit.test.ts @@ -296,4 +296,79 @@ suite('Experimentation service', () => { sinon.assert.notCalled(isCachedFlightEnabledStub); }); }); + + suite('Experiment value retrieval', () => { + const experiment = 'Test Experiment - experiment'; + let getTreatmentVariableAsyncStub: sinon.SinonStub; + + setup(() => { + getTreatmentVariableAsyncStub = sinon.stub().returns(Promise.resolve('value')); + sinon.stub(tasClient, 'getExperimentationService').returns({ + getTreatmentVariableAsync: getTreatmentVariableAsyncStub + // tslint:disable-next-line: no-any + } as any); + + configureApplicationEnvironment('stable', extensionVersion); + }); + + test('If the service is enabled and the opt-out array is empty,return the value from the experimentation framework for a given experiment', async () => { + configureSettings(true, [], []); + + const experimentService = new ExperimentService( + instance(configurationService), + instance(appEnvironment), + globalMemento, + outputChannel + ); + const result = await experimentService.getExperimentValue(experiment); + + assert.equal(result, 'value'); + sinon.assert.calledOnce(getTreatmentVariableAsyncStub); + }); + + test('If the experiment setting is disabled, getExperimentValue should return undefined', async () => { + configureSettings(false, [], []); + + const experimentService = new ExperimentService( + instance(configurationService), + instance(appEnvironment), + globalMemento, + outputChannel + ); + const result = await experimentService.getExperimentValue(experiment); + + assert.isUndefined(result); + sinon.assert.notCalled(getTreatmentVariableAsyncStub); + }); + + test('If the opt-out setting contains "All", getExperimentValue should return undefined', async () => { + configureSettings(true, [], ['All']); + + const experimentService = new ExperimentService( + instance(configurationService), + instance(appEnvironment), + globalMemento, + outputChannel + ); + const result = await experimentService.getExperimentValue(experiment); + + assert.isUndefined(result); + sinon.assert.notCalled(getTreatmentVariableAsyncStub); + }); + + test('If the opt-out setting contains the experiment name, igetExperimentValue should return undefined', async () => { + configureSettings(true, [], [experiment]); + + const experimentService = new ExperimentService( + instance(configurationService), + instance(appEnvironment), + globalMemento, + outputChannel + ); + const result = await experimentService.getExperimentValue(experiment); + + assert.isUndefined(result); + sinon.assert.notCalled(getTreatmentVariableAsyncStub); + }); + }); }); diff --git a/src/test/interpreters/display/interpreterSelectionTip.unit.test.ts b/src/test/interpreters/display/interpreterSelectionTip.unit.test.ts index f411e12f7220..6eb8b046f00a 100644 --- a/src/test/interpreters/display/interpreterSelectionTip.unit.test.ts +++ b/src/test/interpreters/display/interpreterSelectionTip.unit.test.ts @@ -6,50 +6,81 @@ import { anything, instance, mock, verify, when } from 'ts-mockito'; import { ApplicationShell } from '../../../client/common/application/applicationShell'; import { IApplicationShell } from '../../../client/common/application/types'; +import { SurveyAndInterpreterTipNotification } from '../../../client/common/experiments/groups'; +import { ExperimentService } from '../../../client/common/experiments/service'; +import { BrowserService } from '../../../client/common/net/browser'; import { PersistentState, PersistentStateFactory } from '../../../client/common/persistentState'; -import { IPersistentState } from '../../../client/common/types'; -import { Common, Interpreters } from '../../../client/common/utils/localize'; +import { IBrowserService, IExperimentService, IPersistentState } from '../../../client/common/types'; +import { Common } from '../../../client/common/utils/localize'; import { InterpreterSelectionTip } from '../../../client/interpreter/display/interpreterSelectionTip'; -// tslint:disable:no-any suite('Interpreters - Interpreter Selection Tip', () => { let selectionTip: InterpreterSelectionTip; let appShell: IApplicationShell; let storage: IPersistentState; + let experimentService: IExperimentService; + let browserService: IBrowserService; setup(() => { const factory = mock(PersistentStateFactory); storage = mock(PersistentState); appShell = mock(ApplicationShell); + experimentService = mock(ExperimentService); + browserService = mock(BrowserService); when(factory.createGlobalPersistentState('InterpreterSelectionTip', false)).thenReturn(instance(storage)); - selectionTip = new InterpreterSelectionTip(instance(appShell), instance(factory)); + selectionTip = new InterpreterSelectionTip( + instance(appShell), + instance(factory), + instance(experimentService), + instance(browserService) + ); }); - test('Do not show tip', async () => { + test('Do not show notification if already shown', async () => { when(storage.value).thenReturn(true); await selectionTip.activate(); verify(appShell.showInformationMessage(anything(), anything())).never(); }); - test('Show tip and do not track it', async () => { + test('Do not show notification if in neither experiments', async () => { when(storage.value).thenReturn(false); - when(appShell.showInformationMessage(Interpreters.selectInterpreterTip(), Common.gotIt())).thenResolve(); + when(experimentService.inExperiment(anything())).thenResolve(false); await selectionTip.activate(); - verify(appShell.showInformationMessage(Interpreters.selectInterpreterTip(), Common.gotIt())).once(); - verify(storage.updateValue(true)).never(); + verify(appShell.showInformationMessage(anything(), anything())).never(); + verify(storage.updateValue(true)).once(); }); - test('Show tip and track it', async () => { + test('Show tip if in tip experiment', async () => { when(storage.value).thenReturn(false); - when(appShell.showInformationMessage(Interpreters.selectInterpreterTip(), Common.gotIt())).thenResolve( - Common.gotIt() as any - ); + when(experimentService.inExperiment(SurveyAndInterpreterTipNotification.tipExperiment)).thenResolve(true); + when(experimentService.inExperiment(SurveyAndInterpreterTipNotification.surveyExperiment)).thenResolve(false); + + await selectionTip.activate(); + + verify(appShell.showInformationMessage(anything(), Common.gotIt())).once(); + verify(storage.updateValue(true)).once(); + }); + test('Show survey link if in survey experiment', async () => { + when(experimentService.inExperiment(SurveyAndInterpreterTipNotification.tipExperiment)).thenResolve(false); + when(experimentService.inExperiment(SurveyAndInterpreterTipNotification.surveyExperiment)).thenResolve(true); await selectionTip.activate(); - verify(appShell.showInformationMessage(Interpreters.selectInterpreterTip(), Common.gotIt())).once(); + verify(appShell.showInformationMessage(anything(), Common.bannerLabelYes(), Common.bannerLabelNo())).once(); verify(storage.updateValue(true)).once(); }); + test('Open survey link if in survey experiment and "Yes" is selected', async () => { + when(experimentService.inExperiment(SurveyAndInterpreterTipNotification.tipExperiment)).thenResolve(false); + when(experimentService.inExperiment(SurveyAndInterpreterTipNotification.surveyExperiment)).thenResolve(true); + when(appShell.showInformationMessage(anything(), Common.bannerLabelYes(), Common.bannerLabelNo())).thenResolve( + // tslint:disable-next-line: no-any + Common.bannerLabelYes() as any + ); + + await selectionTip.activate(); + + verify(browserService.launch(anything())).once(); + }); });