Skip to content
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

Ensure survey notification respects telemetry.disableFeedback setting #24903

Merged
merged 1 commit into from
Mar 14, 2025
Merged

Ensure survey notification respects telemetry.disableFeedback setting #24903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 14 additions & 1 deletion src/client/activation/extensionSurvey.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { inject, injectable } from 'inversify';
import * as querystring from 'querystring';
import { env, UIKind } from 'vscode';
import { IApplicationEnvironment, IApplicationShell } from '../common/application/types';
import { IApplicationEnvironment, IApplicationShell, IWorkspaceService } from '../common/application/types';
import { ShowExtensionSurveyPrompt } from '../common/experiments/groups';
import '../common/extensions';
import { IPlatformService } from '../common/platform/types';
Expand Down Expand Up @@ -37,6 +37,7 @@ export class ExtensionSurveyPrompt implements IExtensionSingleActivationService
@inject(IExperimentService) private experiments: IExperimentService,
@inject(IApplicationEnvironment) private appEnvironment: IApplicationEnvironment,
@inject(IPlatformService) private platformService: IPlatformService,
@inject(IWorkspaceService) private readonly workspace: IWorkspaceService,
private sampleSizePerOneHundredUsers: number = 10,
private waitTimeToShowSurvey: number = WAIT_TIME_TO_SHOW_SURVEY,
) {}
Expand All @@ -57,6 +58,18 @@ export class ExtensionSurveyPrompt implements IExtensionSingleActivationService
if (env.uiKind === UIKind?.Web) {
return false;
}

let feedbackDisabled = false;

const telemetryConfig = this.workspace.getConfiguration('telemetry');
if (telemetryConfig) {
feedbackDisabled = telemetryConfig.get<boolean>('disableFeedback', false);
}

if (feedbackDisabled) {
return false;
}

const doNotShowSurveyAgain = this.persistentState.createGlobalPersistentState(
extensionSurveyStateKeys.doNotShowAgain,
false,
Expand Down
34 changes: 33 additions & 1 deletion src/test/activation/extensionSurvey.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import * as sinon from 'sinon';
import { anything, instance, mock, verify, when } from 'ts-mockito';
import * as TypeMoq from 'typemoq';
import { ExtensionSurveyPrompt, extensionSurveyStateKeys } from '../../client/activation/extensionSurvey';
import { IApplicationEnvironment, IApplicationShell } from '../../client/common/application/types';
import { IApplicationEnvironment, IApplicationShell, IWorkspaceService } from '../../client/common/application/types';
import { ShowExtensionSurveyPrompt } from '../../client/common/experiments/groups';
import { PersistentStateFactory } from '../../client/common/persistentState';
import { IPlatformService } from '../../client/common/platform/types';
Expand All @@ -23,6 +23,7 @@ import { createDeferred } from '../../client/common/utils/async';
import { Common, ExtensionSurveyBanner } from '../../client/common/utils/localize';
import { OSType } from '../../client/common/utils/platform';
import { sleep } from '../core';
import { WorkspaceConfiguration } from 'vscode';

suite('Extension survey prompt - shouldShowBanner()', () => {
let appShell: TypeMoq.IMock<IApplicationShell>;
Expand All @@ -35,6 +36,8 @@ suite('Extension survey prompt - shouldShowBanner()', () => {
let disableSurveyForTime: TypeMoq.IMock<IPersistentState<any>>;
let doNotShowAgain: TypeMoq.IMock<IPersistentState<any>>;
let extensionSurveyPrompt: ExtensionSurveyPrompt;
let workspaceService: TypeMoq.IMock<IWorkspaceService>;

setup(() => {
experiments = TypeMoq.Mock.ofType<IExperimentService>();
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
Expand All @@ -45,6 +48,7 @@ suite('Extension survey prompt - shouldShowBanner()', () => {
doNotShowAgain = TypeMoq.Mock.ofType<IPersistentState<any>>();
platformService = TypeMoq.Mock.ofType<IPlatformService>();
appEnvironment = TypeMoq.Mock.ofType<IApplicationEnvironment>();
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
when(
persistentStateFactory.createGlobalPersistentState(
extensionSurveyStateKeys.disableSurveyForTime,
Expand All @@ -63,6 +67,7 @@ suite('Extension survey prompt - shouldShowBanner()', () => {
experiments.object,
appEnvironment.object,
platformService.object,
workspaceService.object,
10,
);
});
Expand Down Expand Up @@ -122,6 +127,23 @@ suite('Extension survey prompt - shouldShowBanner()', () => {
}
random.verifyAll();
});
test('Returns false if telemetry.disableFeedback is enabled', async () => {
disableSurveyForTime.setup((d) => d.value).returns(() => false);
doNotShowAgain.setup((d) => d.value).returns(() => false);

const telemetryConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
workspaceService.setup((w) => w.getConfiguration('telemetry')).returns(() => telemetryConfig.object);
telemetryConfig
.setup((t) => t.get(TypeMoq.It.isValue('disableFeedback'), TypeMoq.It.isValue(false)))
.returns(() => true);

const result = extensionSurveyPrompt.shouldShowBanner();

expect(result).to.equal(false, 'Banner should not be shown when telemetry.disableFeedback is true');
workspaceService.verify((w) => w.getConfiguration('telemetry'), TypeMoq.Times.once());
telemetryConfig.verify((t) => t.get('disableFeedback', false), TypeMoq.Times.once());
});

test('Returns true if user is in the random sampling', async () => {
disableSurveyForTime.setup((d) => d.value).returns(() => false);
doNotShowAgain.setup((d) => d.value).returns(() => false);
Expand All @@ -142,6 +164,7 @@ suite('Extension survey prompt - shouldShowBanner()', () => {
experiments.object,
appEnvironment.object,
platformService.object,
workspaceService.object,
100,
);
disableSurveyForTime.setup((d) => d.value).returns(() => false);
Expand All @@ -162,6 +185,7 @@ suite('Extension survey prompt - shouldShowBanner()', () => {
experiments.object,
appEnvironment.object,
platformService.object,
workspaceService.object,
0,
);
disableSurveyForTime.setup((d) => d.value).returns(() => false);
Expand All @@ -186,6 +210,7 @@ suite('Extension survey prompt - showSurvey()', () => {
let platformService: TypeMoq.IMock<IPlatformService>;
let appEnvironment: TypeMoq.IMock<IApplicationEnvironment>;
let extensionSurveyPrompt: ExtensionSurveyPrompt;
let workspaceService: TypeMoq.IMock<IWorkspaceService>;
setup(() => {
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
browserService = TypeMoq.Mock.ofType<IBrowserService>();
Expand All @@ -195,6 +220,7 @@ suite('Extension survey prompt - showSurvey()', () => {
doNotShowAgain = TypeMoq.Mock.ofType<IPersistentState<any>>();
platformService = TypeMoq.Mock.ofType<IPlatformService>();
appEnvironment = TypeMoq.Mock.ofType<IApplicationEnvironment>();
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
when(
persistentStateFactory.createGlobalPersistentState(
extensionSurveyStateKeys.disableSurveyForTime,
Expand All @@ -214,6 +240,7 @@ suite('Extension survey prompt - showSurvey()', () => {
experiments.object,
appEnvironment.object,
platformService.object,
workspaceService.object,
10,
);
});
Expand Down Expand Up @@ -406,6 +433,7 @@ suite('Extension survey prompt - activate()', () => {
let extensionSurveyPrompt: ExtensionSurveyPrompt;
let platformService: TypeMoq.IMock<IPlatformService>;
let appEnvironment: TypeMoq.IMock<IApplicationEnvironment>;
let workspaceService: TypeMoq.IMock<IWorkspaceService>;
setup(() => {
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
browserService = TypeMoq.Mock.ofType<IBrowserService>();
Expand All @@ -414,6 +442,7 @@ suite('Extension survey prompt - activate()', () => {
experiments = TypeMoq.Mock.ofType<IExperimentService>();
platformService = TypeMoq.Mock.ofType<IPlatformService>();
appEnvironment = TypeMoq.Mock.ofType<IApplicationEnvironment>();
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
});

teardown(() => {
Expand All @@ -431,6 +460,7 @@ suite('Extension survey prompt - activate()', () => {
experiments.object,
appEnvironment.object,
platformService.object,
workspaceService.object,
10,
);
experiments
Expand Down Expand Up @@ -460,6 +490,7 @@ suite('Extension survey prompt - activate()', () => {
experiments.object,
appEnvironment.object,
platformService.object,
workspaceService.object,
10,
50,
);
Expand Down Expand Up @@ -494,6 +525,7 @@ suite('Extension survey prompt - activate()', () => {
experiments.object,
appEnvironment.object,
platformService.object,
workspaceService.object,
10,
50,
);
Expand Down
Loading