Skip to content

Commit 6b7d8d1

Browse files
authored
Ensure survey notification respects telemetry.disableFeedback setting (#24903)
Fixes #24904 Follow up to microsoft/vscode#243276 This is to ensure that we only show the survey feedback if VS Code's `telemetry.disableFeedback` setting isn't enabled
1 parent f12d5bc commit 6b7d8d1

File tree

2 files changed

+47
-2
lines changed

2 files changed

+47
-2
lines changed

src/client/activation/extensionSurvey.ts

+14-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import { inject, injectable } from 'inversify';
77
import * as querystring from 'querystring';
88
import { env, UIKind } from 'vscode';
9-
import { IApplicationEnvironment, IApplicationShell } from '../common/application/types';
9+
import { IApplicationEnvironment, IApplicationShell, IWorkspaceService } from '../common/application/types';
1010
import { ShowExtensionSurveyPrompt } from '../common/experiments/groups';
1111
import '../common/extensions';
1212
import { IPlatformService } from '../common/platform/types';
@@ -37,6 +37,7 @@ export class ExtensionSurveyPrompt implements IExtensionSingleActivationService
3737
@inject(IExperimentService) private experiments: IExperimentService,
3838
@inject(IApplicationEnvironment) private appEnvironment: IApplicationEnvironment,
3939
@inject(IPlatformService) private platformService: IPlatformService,
40+
@inject(IWorkspaceService) private readonly workspace: IWorkspaceService,
4041
private sampleSizePerOneHundredUsers: number = 10,
4142
private waitTimeToShowSurvey: number = WAIT_TIME_TO_SHOW_SURVEY,
4243
) {}
@@ -57,6 +58,18 @@ export class ExtensionSurveyPrompt implements IExtensionSingleActivationService
5758
if (env.uiKind === UIKind?.Web) {
5859
return false;
5960
}
61+
62+
let feedbackDisabled = false;
63+
64+
const telemetryConfig = this.workspace.getConfiguration('telemetry');
65+
if (telemetryConfig) {
66+
feedbackDisabled = telemetryConfig.get<boolean>('disableFeedback', false);
67+
}
68+
69+
if (feedbackDisabled) {
70+
return false;
71+
}
72+
6073
const doNotShowSurveyAgain = this.persistentState.createGlobalPersistentState(
6174
extensionSurveyStateKeys.doNotShowAgain,
6275
false,

src/test/activation/extensionSurvey.unit.test.ts

+33-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import * as sinon from 'sinon';
88
import { anything, instance, mock, verify, when } from 'ts-mockito';
99
import * as TypeMoq from 'typemoq';
1010
import { ExtensionSurveyPrompt, extensionSurveyStateKeys } from '../../client/activation/extensionSurvey';
11-
import { IApplicationEnvironment, IApplicationShell } from '../../client/common/application/types';
11+
import { IApplicationEnvironment, IApplicationShell, IWorkspaceService } from '../../client/common/application/types';
1212
import { ShowExtensionSurveyPrompt } from '../../client/common/experiments/groups';
1313
import { PersistentStateFactory } from '../../client/common/persistentState';
1414
import { IPlatformService } from '../../client/common/platform/types';
@@ -23,6 +23,7 @@ import { createDeferred } from '../../client/common/utils/async';
2323
import { Common, ExtensionSurveyBanner } from '../../client/common/utils/localize';
2424
import { OSType } from '../../client/common/utils/platform';
2525
import { sleep } from '../core';
26+
import { WorkspaceConfiguration } from 'vscode';
2627

2728
suite('Extension survey prompt - shouldShowBanner()', () => {
2829
let appShell: TypeMoq.IMock<IApplicationShell>;
@@ -35,6 +36,8 @@ suite('Extension survey prompt - shouldShowBanner()', () => {
3536
let disableSurveyForTime: TypeMoq.IMock<IPersistentState<any>>;
3637
let doNotShowAgain: TypeMoq.IMock<IPersistentState<any>>;
3738
let extensionSurveyPrompt: ExtensionSurveyPrompt;
39+
let workspaceService: TypeMoq.IMock<IWorkspaceService>;
40+
3841
setup(() => {
3942
experiments = TypeMoq.Mock.ofType<IExperimentService>();
4043
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
@@ -45,6 +48,7 @@ suite('Extension survey prompt - shouldShowBanner()', () => {
4548
doNotShowAgain = TypeMoq.Mock.ofType<IPersistentState<any>>();
4649
platformService = TypeMoq.Mock.ofType<IPlatformService>();
4750
appEnvironment = TypeMoq.Mock.ofType<IApplicationEnvironment>();
51+
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
4852
when(
4953
persistentStateFactory.createGlobalPersistentState(
5054
extensionSurveyStateKeys.disableSurveyForTime,
@@ -63,6 +67,7 @@ suite('Extension survey prompt - shouldShowBanner()', () => {
6367
experiments.object,
6468
appEnvironment.object,
6569
platformService.object,
70+
workspaceService.object,
6671
10,
6772
);
6873
});
@@ -122,6 +127,23 @@ suite('Extension survey prompt - shouldShowBanner()', () => {
122127
}
123128
random.verifyAll();
124129
});
130+
test('Returns false if telemetry.disableFeedback is enabled', async () => {
131+
disableSurveyForTime.setup((d) => d.value).returns(() => false);
132+
doNotShowAgain.setup((d) => d.value).returns(() => false);
133+
134+
const telemetryConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
135+
workspaceService.setup((w) => w.getConfiguration('telemetry')).returns(() => telemetryConfig.object);
136+
telemetryConfig
137+
.setup((t) => t.get(TypeMoq.It.isValue('disableFeedback'), TypeMoq.It.isValue(false)))
138+
.returns(() => true);
139+
140+
const result = extensionSurveyPrompt.shouldShowBanner();
141+
142+
expect(result).to.equal(false, 'Banner should not be shown when telemetry.disableFeedback is true');
143+
workspaceService.verify((w) => w.getConfiguration('telemetry'), TypeMoq.Times.once());
144+
telemetryConfig.verify((t) => t.get('disableFeedback', false), TypeMoq.Times.once());
145+
});
146+
125147
test('Returns true if user is in the random sampling', async () => {
126148
disableSurveyForTime.setup((d) => d.value).returns(() => false);
127149
doNotShowAgain.setup((d) => d.value).returns(() => false);
@@ -142,6 +164,7 @@ suite('Extension survey prompt - shouldShowBanner()', () => {
142164
experiments.object,
143165
appEnvironment.object,
144166
platformService.object,
167+
workspaceService.object,
145168
100,
146169
);
147170
disableSurveyForTime.setup((d) => d.value).returns(() => false);
@@ -162,6 +185,7 @@ suite('Extension survey prompt - shouldShowBanner()', () => {
162185
experiments.object,
163186
appEnvironment.object,
164187
platformService.object,
188+
workspaceService.object,
165189
0,
166190
);
167191
disableSurveyForTime.setup((d) => d.value).returns(() => false);
@@ -186,6 +210,7 @@ suite('Extension survey prompt - showSurvey()', () => {
186210
let platformService: TypeMoq.IMock<IPlatformService>;
187211
let appEnvironment: TypeMoq.IMock<IApplicationEnvironment>;
188212
let extensionSurveyPrompt: ExtensionSurveyPrompt;
213+
let workspaceService: TypeMoq.IMock<IWorkspaceService>;
189214
setup(() => {
190215
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
191216
browserService = TypeMoq.Mock.ofType<IBrowserService>();
@@ -195,6 +220,7 @@ suite('Extension survey prompt - showSurvey()', () => {
195220
doNotShowAgain = TypeMoq.Mock.ofType<IPersistentState<any>>();
196221
platformService = TypeMoq.Mock.ofType<IPlatformService>();
197222
appEnvironment = TypeMoq.Mock.ofType<IApplicationEnvironment>();
223+
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
198224
when(
199225
persistentStateFactory.createGlobalPersistentState(
200226
extensionSurveyStateKeys.disableSurveyForTime,
@@ -214,6 +240,7 @@ suite('Extension survey prompt - showSurvey()', () => {
214240
experiments.object,
215241
appEnvironment.object,
216242
platformService.object,
243+
workspaceService.object,
217244
10,
218245
);
219246
});
@@ -406,6 +433,7 @@ suite('Extension survey prompt - activate()', () => {
406433
let extensionSurveyPrompt: ExtensionSurveyPrompt;
407434
let platformService: TypeMoq.IMock<IPlatformService>;
408435
let appEnvironment: TypeMoq.IMock<IApplicationEnvironment>;
436+
let workspaceService: TypeMoq.IMock<IWorkspaceService>;
409437
setup(() => {
410438
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
411439
browserService = TypeMoq.Mock.ofType<IBrowserService>();
@@ -414,6 +442,7 @@ suite('Extension survey prompt - activate()', () => {
414442
experiments = TypeMoq.Mock.ofType<IExperimentService>();
415443
platformService = TypeMoq.Mock.ofType<IPlatformService>();
416444
appEnvironment = TypeMoq.Mock.ofType<IApplicationEnvironment>();
445+
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
417446
});
418447

419448
teardown(() => {
@@ -431,6 +460,7 @@ suite('Extension survey prompt - activate()', () => {
431460
experiments.object,
432461
appEnvironment.object,
433462
platformService.object,
463+
workspaceService.object,
434464
10,
435465
);
436466
experiments
@@ -460,6 +490,7 @@ suite('Extension survey prompt - activate()', () => {
460490
experiments.object,
461491
appEnvironment.object,
462492
platformService.object,
493+
workspaceService.object,
463494
10,
464495
50,
465496
);
@@ -494,6 +525,7 @@ suite('Extension survey prompt - activate()', () => {
494525
experiments.object,
495526
appEnvironment.object,
496527
platformService.object,
528+
workspaceService.object,
497529
10,
498530
50,
499531
);

0 commit comments

Comments
 (0)