From ff0afe8f047bbe82e8969c89dc1e2220bdebb38a Mon Sep 17 00:00:00 2001 From: Luciana Abud <45497113+luabud@users.noreply.github.com> Date: Fri, 14 Mar 2025 02:33:23 +0000 Subject: [PATCH] Ensuring survey notification respects telemetry.disableFeedback setting --- src/client/activation/extensionSurvey.ts | 15 +++++++- .../activation/extensionSurvey.unit.test.ts | 34 ++++++++++++++++++- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/client/activation/extensionSurvey.ts b/src/client/activation/extensionSurvey.ts index c5b7c525fea8..e8df4bb850da 100644 --- a/src/client/activation/extensionSurvey.ts +++ b/src/client/activation/extensionSurvey.ts @@ -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'; @@ -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, ) {} @@ -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('disableFeedback', false); + } + + if (feedbackDisabled) { + return false; + } + const doNotShowSurveyAgain = this.persistentState.createGlobalPersistentState( extensionSurveyStateKeys.doNotShowAgain, false, diff --git a/src/test/activation/extensionSurvey.unit.test.ts b/src/test/activation/extensionSurvey.unit.test.ts index ba96b917aff3..8e191ec82810 100644 --- a/src/test/activation/extensionSurvey.unit.test.ts +++ b/src/test/activation/extensionSurvey.unit.test.ts @@ -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'; @@ -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; @@ -35,6 +36,8 @@ suite('Extension survey prompt - shouldShowBanner()', () => { let disableSurveyForTime: TypeMoq.IMock>; let doNotShowAgain: TypeMoq.IMock>; let extensionSurveyPrompt: ExtensionSurveyPrompt; + let workspaceService: TypeMoq.IMock; + setup(() => { experiments = TypeMoq.Mock.ofType(); appShell = TypeMoq.Mock.ofType(); @@ -45,6 +48,7 @@ suite('Extension survey prompt - shouldShowBanner()', () => { doNotShowAgain = TypeMoq.Mock.ofType>(); platformService = TypeMoq.Mock.ofType(); appEnvironment = TypeMoq.Mock.ofType(); + workspaceService = TypeMoq.Mock.ofType(); when( persistentStateFactory.createGlobalPersistentState( extensionSurveyStateKeys.disableSurveyForTime, @@ -63,6 +67,7 @@ suite('Extension survey prompt - shouldShowBanner()', () => { experiments.object, appEnvironment.object, platformService.object, + workspaceService.object, 10, ); }); @@ -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(); + 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); @@ -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); @@ -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); @@ -186,6 +210,7 @@ suite('Extension survey prompt - showSurvey()', () => { let platformService: TypeMoq.IMock; let appEnvironment: TypeMoq.IMock; let extensionSurveyPrompt: ExtensionSurveyPrompt; + let workspaceService: TypeMoq.IMock; setup(() => { appShell = TypeMoq.Mock.ofType(); browserService = TypeMoq.Mock.ofType(); @@ -195,6 +220,7 @@ suite('Extension survey prompt - showSurvey()', () => { doNotShowAgain = TypeMoq.Mock.ofType>(); platformService = TypeMoq.Mock.ofType(); appEnvironment = TypeMoq.Mock.ofType(); + workspaceService = TypeMoq.Mock.ofType(); when( persistentStateFactory.createGlobalPersistentState( extensionSurveyStateKeys.disableSurveyForTime, @@ -214,6 +240,7 @@ suite('Extension survey prompt - showSurvey()', () => { experiments.object, appEnvironment.object, platformService.object, + workspaceService.object, 10, ); }); @@ -406,6 +433,7 @@ suite('Extension survey prompt - activate()', () => { let extensionSurveyPrompt: ExtensionSurveyPrompt; let platformService: TypeMoq.IMock; let appEnvironment: TypeMoq.IMock; + let workspaceService: TypeMoq.IMock; setup(() => { appShell = TypeMoq.Mock.ofType(); browserService = TypeMoq.Mock.ofType(); @@ -414,6 +442,7 @@ suite('Extension survey prompt - activate()', () => { experiments = TypeMoq.Mock.ofType(); platformService = TypeMoq.Mock.ofType(); appEnvironment = TypeMoq.Mock.ofType(); + workspaceService = TypeMoq.Mock.ofType(); }); teardown(() => { @@ -431,6 +460,7 @@ suite('Extension survey prompt - activate()', () => { experiments.object, appEnvironment.object, platformService.object, + workspaceService.object, 10, ); experiments @@ -460,6 +490,7 @@ suite('Extension survey prompt - activate()', () => { experiments.object, appEnvironment.object, platformService.object, + workspaceService.object, 10, 50, ); @@ -494,6 +525,7 @@ suite('Extension survey prompt - activate()', () => { experiments.object, appEnvironment.object, platformService.object, + workspaceService.object, 10, 50, );