Skip to content

Commit 3eb7b23

Browse files
kimadelinekimadeline
authored
Update "Tip" notification for new users to either show the existing tip, a link to a feedback survey or nothing (microsoft#13554)
* Update vscode-tas-client * Add experiment group enum * Add method to retrieve experiment values * Implementation + tests * News file * Update wording of the news entry * Add telemetry * More tests * No opting-in and out of this one * Don't fetch value if opted out, add tests * Address comments
1 parent 98204f8 commit 3eb7b23

11 files changed

Lines changed: 202 additions & 28 deletions

File tree

‎news/1 Enhancements/13535.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Update "Tip" notification for new users to either show the existing tip, a link to a feedback survey or nothing.

‎package-lock.json‎

Lines changed: 7 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3523,7 +3523,7 @@
35233523
"vscode-languageclient": "^7.0.0-next.8",
35243524
"vscode-languageserver": "^7.0.0-next.6",
35253525
"vscode-languageserver-protocol": "^3.16.0-next.6",
3526-
"vscode-tas-client": "^0.0.864",
3526+
"vscode-tas-client": "^0.1.4",
35273527
"vsls": "^0.3.1291",
35283528
"winreg": "^1.2.4",
35293529
"winston": "^3.2.1",

‎src/client/common/experiments/groups.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,3 +91,10 @@ export enum RemoveKernelToolbarInInteractiveWindow {
9191
export enum TryPylance {
9292
experiment = 'tryPylance'
9393
}
94+
95+
// Experiment for the content of the tip being displayed on first extension launch:
96+
// interpreter selection tip, feedback survey or nothing.
97+
export enum SurveyAndInterpreterTipNotification {
98+
tipExperiment = 'pythonTipPromptWording',
99+
surveyExperiment = 'pythonMailingListPromptWording'
100+
}

‎src/client/common/experiments/service.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,14 @@ export class ExperimentService implements IExperimentService {
104104
return this.experimentationService.isCachedFlightEnabled(experiment);
105105
}
106106

107+
public async getExperimentValue<T extends boolean | number | string>(experiment: string): Promise<T | undefined> {
108+
if (!this.experimentationService || this._optOutFrom.includes('All') || this._optOutFrom.includes(experiment)) {
109+
return;
110+
}
111+
112+
return this.experimentationService.getTreatmentVariableAsync('vscode', experiment);
113+
}
114+
107115
private logExperiments() {
108116
const experiments = this.globalState.get<{ features: string[] }>(EXP_MEMENTO_KEY, { features: [] });
109117

‎src/client/common/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,7 @@ export interface IExperimentsManager {
637637
export const IExperimentService = Symbol('IExperimentService');
638638
export interface IExperimentService {
639639
inExperiment(experimentName: string): Promise<boolean>;
640+
getExperimentValue<T extends boolean | number | string>(experimentName: string): Promise<T | undefined>;
640641
}
641642

642643
export type InterpreterConfigurationScope = { uri: Resource; configTarget: ConfigurationTarget };

‎src/client/interpreter/display/interpreterSelectionTip.ts‎

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,31 +6,72 @@
66
import { inject, injectable } from 'inversify';
77
import { IExtensionSingleActivationService } from '../../activation/types';
88
import { IApplicationShell } from '../../common/application/types';
9-
import { IPersistentState, IPersistentStateFactory } from '../../common/types';
9+
import { SurveyAndInterpreterTipNotification } from '../../common/experiments/groups';
10+
import { IBrowserService, IExperimentService, IPersistentState, IPersistentStateFactory } from '../../common/types';
1011
import { swallowExceptions } from '../../common/utils/decorators';
11-
import { Common, Interpreters } from '../../common/utils/localize';
12+
import { Common } from '../../common/utils/localize';
13+
import { sendTelemetryEvent } from '../../telemetry';
14+
import { EventName } from '../../telemetry/constants';
15+
16+
enum NotificationType {
17+
Tip,
18+
Survey,
19+
NoPrompt
20+
}
1221

1322
@injectable()
1423
export class InterpreterSelectionTip implements IExtensionSingleActivationService {
1524
private readonly storage: IPersistentState<boolean>;
25+
private notificationType: NotificationType;
26+
private notificationContent: string | undefined;
27+
1628
constructor(
1729
@inject(IApplicationShell) private readonly shell: IApplicationShell,
18-
@inject(IPersistentStateFactory) private readonly factory: IPersistentStateFactory
30+
@inject(IPersistentStateFactory) private readonly factory: IPersistentStateFactory,
31+
@inject(IExperimentService) private readonly experiments: IExperimentService,
32+
@inject(IBrowserService) private browserService: IBrowserService
1933
) {
2034
this.storage = this.factory.createGlobalPersistentState('InterpreterSelectionTip', false);
35+
this.notificationType = NotificationType.NoPrompt;
2136
}
37+
2238
public async activate(): Promise<void> {
2339
if (this.storage.value) {
2440
return;
2541
}
42+
43+
if (await this.experiments.inExperiment(SurveyAndInterpreterTipNotification.surveyExperiment)) {
44+
this.notificationType = NotificationType.Survey;
45+
this.notificationContent = await this.experiments.getExperimentValue(
46+
SurveyAndInterpreterTipNotification.surveyExperiment
47+
);
48+
} else if (await this.experiments.inExperiment(SurveyAndInterpreterTipNotification.tipExperiment)) {
49+
this.notificationType = NotificationType.Tip;
50+
this.notificationContent = await this.experiments.getExperimentValue(
51+
SurveyAndInterpreterTipNotification.tipExperiment
52+
);
53+
}
54+
2655
this.showTip().ignoreErrors();
2756
}
2857
@swallowExceptions('Failed to display tip')
2958
private async showTip() {
30-
const selection = await this.shell.showInformationMessage(Interpreters.selectInterpreterTip(), Common.gotIt());
31-
if (selection !== Common.gotIt()) {
32-
return;
59+
if (this.notificationType === NotificationType.Tip) {
60+
await this.shell.showInformationMessage(this.notificationContent!, Common.gotIt());
61+
sendTelemetryEvent(EventName.ACTIVATION_TIP_PROMPT, undefined);
62+
} else if (this.notificationType === NotificationType.Survey) {
63+
const selection = await this.shell.showInformationMessage(
64+
this.notificationContent!,
65+
Common.bannerLabelYes(),
66+
Common.bannerLabelNo()
67+
);
68+
69+
if (selection === Common.bannerLabelYes()) {
70+
sendTelemetryEvent(EventName.ACTIVATION_SURVEY_PROMPT, undefined);
71+
this.browserService.launch('https://aka.ms/mailingListSurvey');
72+
}
3373
}
74+
3475
await this.storage.updateValue(true);
3576
}
3677
}

‎src/client/telemetry/constants.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ export enum EventName {
7474
PLAY_BUTTON_ICON_DISABLED = 'PLAY_BUTTON_ICON.DISABLED',
7575
PYTHON_WEB_APP_RELOAD = 'PYTHON_WEB_APP.RELOAD',
7676
EXTENSION_SURVEY_PROMPT = 'EXTENSION_SURVEY_PROMPT',
77+
ACTIVATION_TIP_PROMPT = 'ACTIVATION_TIP_PROMPT',
78+
ACTIVATION_SURVEY_PROMPT = 'ACTIVATION_SURVEY_PROMPT',
7779

7880
PYTHON_LANGUAGE_SERVER_CURRENT_SELECTION = 'PYTHON_LANGUAGE_SERVER_CURRENT_SELECTION',
7981
PYTHON_LANGUAGE_SERVER_LIST_BLOB_STORE_PACKAGES = 'PYTHON_LANGUAGE_SERVER.LIST_BLOB_PACKAGES',

‎src/client/telemetry/index.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,6 +1398,14 @@ export interface IEventNamePropertyMapping {
13981398
*/
13991399
selection: 'Yes' | 'Maybe later' | 'Do not show again' | undefined;
14001400
};
1401+
/**
1402+
* Telemetry event sent when the Python interpreter tip is shown on activation for new users.
1403+
*/
1404+
[EventName.ACTIVATION_TIP_PROMPT]: never | undefined;
1405+
/**
1406+
* Telemetry event sent when the feedback survey prompt is shown on activation for new users, and they click on the survey link.
1407+
*/
1408+
[EventName.ACTIVATION_SURVEY_PROMPT]: never | undefined;
14011409
/**
14021410
* Telemetry event sent when 'Extract Method' command is invoked
14031411
*/

‎src/test/common/experiments/service.unit.test.ts‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,4 +296,79 @@ suite('Experimentation service', () => {
296296
sinon.assert.notCalled(isCachedFlightEnabledStub);
297297
});
298298
});
299+
300+
suite('Experiment value retrieval', () => {
301+
const experiment = 'Test Experiment - experiment';
302+
let getTreatmentVariableAsyncStub: sinon.SinonStub;
303+
304+
setup(() => {
305+
getTreatmentVariableAsyncStub = sinon.stub().returns(Promise.resolve('value'));
306+
sinon.stub(tasClient, 'getExperimentationService').returns({
307+
getTreatmentVariableAsync: getTreatmentVariableAsyncStub
308+
// tslint:disable-next-line: no-any
309+
} as any);
310+
311+
configureApplicationEnvironment('stable', extensionVersion);
312+
});
313+
314+
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 () => {
315+
configureSettings(true, [], []);
316+
317+
const experimentService = new ExperimentService(
318+
instance(configurationService),
319+
instance(appEnvironment),
320+
globalMemento,
321+
outputChannel
322+
);
323+
const result = await experimentService.getExperimentValue(experiment);
324+
325+
assert.equal(result, 'value');
326+
sinon.assert.calledOnce(getTreatmentVariableAsyncStub);
327+
});
328+
329+
test('If the experiment setting is disabled, getExperimentValue should return undefined', async () => {
330+
configureSettings(false, [], []);
331+
332+
const experimentService = new ExperimentService(
333+
instance(configurationService),
334+
instance(appEnvironment),
335+
globalMemento,
336+
outputChannel
337+
);
338+
const result = await experimentService.getExperimentValue(experiment);
339+
340+
assert.isUndefined(result);
341+
sinon.assert.notCalled(getTreatmentVariableAsyncStub);
342+
});
343+
344+
test('If the opt-out setting contains "All", getExperimentValue should return undefined', async () => {
345+
configureSettings(true, [], ['All']);
346+
347+
const experimentService = new ExperimentService(
348+
instance(configurationService),
349+
instance(appEnvironment),
350+
globalMemento,
351+
outputChannel
352+
);
353+
const result = await experimentService.getExperimentValue(experiment);
354+
355+
assert.isUndefined(result);
356+
sinon.assert.notCalled(getTreatmentVariableAsyncStub);
357+
});
358+
359+
test('If the opt-out setting contains the experiment name, igetExperimentValue should return undefined', async () => {
360+
configureSettings(true, [], [experiment]);
361+
362+
const experimentService = new ExperimentService(
363+
instance(configurationService),
364+
instance(appEnvironment),
365+
globalMemento,
366+
outputChannel
367+
);
368+
const result = await experimentService.getExperimentValue(experiment);
369+
370+
assert.isUndefined(result);
371+
sinon.assert.notCalled(getTreatmentVariableAsyncStub);
372+
});
373+
});
299374
});

0 commit comments

Comments
 (0)