From 044f768b5afbd3e98b9c2bb933143d3f801126d8 Mon Sep 17 00:00:00 2001 From: Ian Huff Date: Tue, 6 Nov 2018 15:45:44 -0800 Subject: [PATCH] Add data science survey banner (#3224) * Add data science survey banner * fix hygiene that didn't show up on local run * don't await on survey call --- package.nls.json | 5 +- src/client/activation/serviceRegistry.ts | 4 +- src/client/common/types.ts | 1 + src/client/common/utils/localize.ts | 6 + .../datascience/dataScienceSurveyBanner.ts | 124 ++++++++++++++++++ src/client/datascience/datascience.ts | 26 +++- .../datascienceSurveyBanner.unit.test.ts | 111 ++++++++++++++++ 7 files changed, 269 insertions(+), 8 deletions(-) create mode 100644 src/client/datascience/dataScienceSurveyBanner.ts create mode 100644 src/test/datascience/datascienceSurveyBanner.unit.test.ts diff --git a/package.nls.json b/package.nls.json index 9a6a44ca667b..cd97e8a608ee 100644 --- a/package.nls.json +++ b/package.nls.json @@ -83,5 +83,8 @@ "Interpreters.LoadingInterpreters": "Loading Python Interpreters", "DataScience.restartKernelMessage" : "Do you want to restart the Jupter kernel? All variables will be lost.", "DataScience.restartKernelMessageYes" : "Restart", - "DataScience.restartKernelMessageNo" : "Cancel" + "DataScience.restartKernelMessageNo" : "Cancel", + "DataScienceSurveyBanner.bannerMessage": "Can you please take 2 minutes to tell us how the Python Data Science features are working for you?", + "DataScienceSurveyBanner.bannerLabelYes": "Yes, take survey now", + "DataScienceSurveyBanner.bannerLabelNo": "No, thanks" } diff --git a/src/client/activation/serviceRegistry.ts b/src/client/activation/serviceRegistry.ts index 2ea04b4ec4ed..fceb6ffd09b4 100644 --- a/src/client/activation/serviceRegistry.ts +++ b/src/client/activation/serviceRegistry.ts @@ -4,7 +4,8 @@ 'use strict'; import { INugetRepository } from '../common/nuget/types'; -import { BANNER_NAME_LS_SURVEY, BANNER_NAME_PROPOSE_LS, IPythonExtensionBanner } from '../common/types'; +import { BANNER_NAME_DS_SURVEY, BANNER_NAME_LS_SURVEY, BANNER_NAME_PROPOSE_LS, IPythonExtensionBanner } from '../common/types'; +import { DataScienceSurveyBanner } from '../datascience/dataScienceSurveyBanner'; import { IServiceManager } from '../ioc/types'; import { LanguageServerSurveyBanner } from '../languageServices/languageServerSurveyBanner'; import { ProposeLanguageServerBanner } from '../languageServices/proposeLanguageServerBanner'; @@ -23,6 +24,7 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.add(IExtensionActivator, LanguageServerExtensionActivator, ExtensionActivators.DotNet); serviceManager.addSingleton(IPythonExtensionBanner, LanguageServerSurveyBanner, BANNER_NAME_LS_SURVEY); serviceManager.addSingleton(IPythonExtensionBanner, ProposeLanguageServerBanner, BANNER_NAME_PROPOSE_LS); + serviceManager.addSingleton(IPythonExtensionBanner, DataScienceSurveyBanner, BANNER_NAME_DS_SURVEY); serviceManager.addSingleton(ILanguageServerFolderService, LanguageServerFolderService); serviceManager.addSingleton(ILanguageServerPackageService, LanguageServerPackageService); serviceManager.addSingleton(INugetRepository, StableLanguageServerPackageRepository, LanguageServerDownloadChannel.stable); diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 3a40f48674d9..6c3968b4f6a3 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -325,6 +325,7 @@ export interface IPythonExtensionBanner { } export const BANNER_NAME_LS_SURVEY: string = 'LSSurveyBanner'; export const BANNER_NAME_PROPOSE_LS: string = 'ProposeLS'; +export const BANNER_NAME_DS_SURVEY: string = 'DSSurveyBanner'; export type DeprecatedSettingAndValue = { setting: string; diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index ac921b8716d0..c5feb1d7367b 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -19,6 +19,12 @@ export namespace Interpreters { export const refreshing = localize('Interpreters.RefreshingInterpreters', 'Refreshing Python Interpreters'); } +export namespace DataScienceSurveyBanner { + export const bannerMessage = localize('DataScienceSurveyBanner.bannerMessage', 'Can you please take 2 minutes to tell us how the Python Data Science features are working for you?'); + export const bannerLabelYes = localize('DataScienceSurveyBanner.bannerLabelYes', 'Yes, take survey now'); + export const bannerLabelNo = localize('DataScienceSurveyBanner.bannerLabelNo', 'No, thanks'); +} + export namespace DataScience { export const historyTitle = localize('DataScience.historyTitle', 'Python Interactive'); export const badWebPanelFormatString = localize('DataScience.badWebPanelFormatString', '

{0} is not a valid file name

'); diff --git a/src/client/datascience/dataScienceSurveyBanner.ts b/src/client/datascience/dataScienceSurveyBanner.ts new file mode 100644 index 000000000000..2ccca9ae9b49 --- /dev/null +++ b/src/client/datascience/dataScienceSurveyBanner.ts @@ -0,0 +1,124 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { inject, injectable } from 'inversify'; +import { IApplicationShell } from '../common/application/types'; +import '../common/extensions'; +import { + IBrowserService, IPersistentStateFactory, + IPythonExtensionBanner +} from '../common/types'; +import * as localize from '../common/utils/localize'; + +export enum DSSurveyStateKeys { + ShowBanner = 'ShowDSSurveyBanner', + ShowAttemptCounter = 'DSSurveyShowAttempt' +} + +enum DSSurveyLabelIndex { + Yes, + No +} + +@injectable() +export class DataScienceSurveyBanner implements IPythonExtensionBanner { + private disabledInCurrentSession: boolean = false; + private isInitialized: boolean = false; + private bannerMessage: string = localize.DataScienceSurveyBanner.bannerMessage(); + private bannerLabels: string[] = [localize.DataScienceSurveyBanner.bannerLabelYes(), localize.DataScienceSurveyBanner.bannerLabelNo()]; + private readonly commandThreshold: number; + private readonly surveyLink: string; + + constructor( + @inject(IApplicationShell) private appShell: IApplicationShell, + @inject(IPersistentStateFactory) private persistentState: IPersistentStateFactory, + @inject(IBrowserService) private browserService: IBrowserService, + commandThreshold: number = 500, + surveyLink: string = 'https://aka.ms/pyaisurvey') { + this.commandThreshold = commandThreshold; + this.surveyLink = surveyLink; + this.initialize(); + } + + public initialize(): void { + if (this.isInitialized) { + return; + } + this.isInitialized = true; + } + + public get optionLabels(): string[] { + return this.bannerLabels; + } + + public get shownCount(): Promise { + return this.getPythonDSCommandCounter(); + } + + public get enabled(): boolean { + return this.persistentState.createGlobalPersistentState(DSSurveyStateKeys.ShowBanner, true).value; + } + + public async showBanner(): Promise { + if (!this.enabled || this.disabledInCurrentSession) { + return; + } + + const launchCounter: number = await this.incrementPythonDataScienceCommandCounter(); + const show = await this.shouldShowBanner(launchCounter); + if (!show) { + return; + } + + const response = await this.appShell.showInformationMessage(this.bannerMessage, ...this.bannerLabels); + switch (response) { + case this.bannerLabels[DSSurveyLabelIndex.Yes]: + { + await this.launchSurvey(); + await this.disable(); + break; + } + case this.bannerLabels[DSSurveyLabelIndex.No]: { + await this.disable(); + break; + } + default: { + // Disable for the current session. + this.disabledInCurrentSession = true; + } + } + } + + public async shouldShowBanner(launchCounter?: number): Promise { + if (!this.enabled || this.disabledInCurrentSession) { + return false; + } + + if (!launchCounter) { + launchCounter = await this.getPythonDSCommandCounter(); + } + + return launchCounter >= this.commandThreshold; + } + + public async disable(): Promise { + await this.persistentState.createGlobalPersistentState(DSSurveyStateKeys.ShowBanner, false).updateValue(false); + } + + public async launchSurvey(): Promise { + this.browserService.launch(this.surveyLink); + } + + private async getPythonDSCommandCounter(): Promise { + const state = this.persistentState.createGlobalPersistentState(DSSurveyStateKeys.ShowAttemptCounter, 0); + return state.value; + } + + private async incrementPythonDataScienceCommandCounter(): Promise { + const state = this.persistentState.createGlobalPersistentState(DSSurveyStateKeys.ShowAttemptCounter, 0); + await state.updateValue(state.value + 1); + return state.value; + } +} diff --git a/src/client/datascience/datascience.ts b/src/client/datascience/datascience.ts index 62e26c535bca..14af6f634d44 100644 --- a/src/client/datascience/datascience.ts +++ b/src/client/datascience/datascience.ts @@ -7,9 +7,9 @@ import { inject, injectable } from 'inversify'; import * as vscode from 'vscode'; import { ICommandManager } from '../common/application/types'; import { PythonSettings } from '../common/configSettings'; -import { PYTHON } from '../common/constants'; +import { isTestExecution, PYTHON } from '../common/constants'; import { ContextKey } from '../common/contextKey'; -import { IConfigurationService, IDisposableRegistry, IExtensionContext } from '../common/types'; +import { BANNER_NAME_DS_SURVEY, IConfigurationService, IDisposableRegistry, IExtensionContext, IPythonExtensionBanner } from '../common/types'; import { IServiceContainer } from '../ioc/types'; import { Commands, EditorContexts } from './constants'; import { ICodeWatcher, IDataScience, IDataScienceCodeLensProvider, IDataScienceCommandListener } from './types'; @@ -22,6 +22,7 @@ export class DataScience implements IDataScience { private readonly dataScienceCodeLensProvider: IDataScienceCodeLensProvider; private readonly commandListeners: IDataScienceCommandListener[]; private readonly configuration: IConfigurationService; + private readonly dataScienceSurveyBanner: IPythonExtensionBanner; constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { this.commandManager = this.serviceContainer.get(ICommandManager); @@ -30,6 +31,7 @@ export class DataScience implements IDataScience { this.dataScienceCodeLensProvider = this.serviceContainer.get(IDataScienceCodeLensProvider); this.commandListeners = this.serviceContainer.getAll(IDataScienceCommandListener); this.configuration = this.serviceContainer.get(IConfigurationService); + this.dataScienceSurveyBanner = this.serviceContainer.get(IPythonExtensionBanner, BANNER_NAME_DS_SURVEY); } public async activate(): Promise { @@ -54,7 +56,10 @@ export class DataScience implements IDataScience { } } - public runAllCells(codeWatcher: ICodeWatcher): Promise { + public async runAllCells(codeWatcher: ICodeWatcher): Promise { + if (!isTestExecution()) { + this.dataScienceSurveyBanner.showBanner().ignoreErrors(); + } let activeCodeWatcher: ICodeWatcher | undefined = codeWatcher; if (!activeCodeWatcher) { activeCodeWatcher = this.getCurrentCodeWatcher(); @@ -66,7 +71,10 @@ export class DataScience implements IDataScience { } } - public runCell(codeWatcher: ICodeWatcher, range: vscode.Range): Promise { + public async runCell(codeWatcher: ICodeWatcher, range: vscode.Range): Promise { + if (!isTestExecution()) { + this.dataScienceSurveyBanner.showBanner().ignoreErrors(); + } if (codeWatcher) { return codeWatcher.runCell(range); } else { @@ -74,7 +82,10 @@ export class DataScience implements IDataScience { } } - public runCurrentCell(): Promise { + public async runCurrentCell(): Promise { + if (!isTestExecution()) { + this.dataScienceSurveyBanner.showBanner().ignoreErrors(); + } const activeCodeWatcher = this.getCurrentCodeWatcher(); if (activeCodeWatcher) { return activeCodeWatcher.runCurrentCell(); @@ -83,7 +94,10 @@ export class DataScience implements IDataScience { } } - public runCurrentCellAndAdvance(): Promise { + public async runCurrentCellAndAdvance(): Promise { + if (!isTestExecution()) { + this.dataScienceSurveyBanner.showBanner().ignoreErrors(); + } const activeCodeWatcher = this.getCurrentCodeWatcher(); if (activeCodeWatcher) { return activeCodeWatcher.runCurrentCellAndAdvance(); diff --git a/src/test/datascience/datascienceSurveyBanner.unit.test.ts b/src/test/datascience/datascienceSurveyBanner.unit.test.ts new file mode 100644 index 000000000000..78d850ae6aed --- /dev/null +++ b/src/test/datascience/datascienceSurveyBanner.unit.test.ts @@ -0,0 +1,111 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +// tslint:disable:no-any max-func-body-length + +import { expect } from 'chai'; +import * as typemoq from 'typemoq'; +import { IApplicationShell } from '../../client/common/application/types'; +import { IBrowserService, IPersistentState, IPersistentStateFactory } from '../../client/common/types'; +import { DataScienceSurveyBanner, DSSurveyStateKeys } from '../../client/datascience/dataScienceSurveyBanner'; + +suite('Data Science Survey Banner', () => { + let appShell: typemoq.IMock; + let browser: typemoq.IMock; + const targetUri: string = 'https://microsoft.com'; + + const message = 'Can you please take 2 minutes to tell us how the Python Data Science features are working for you?'; + const yes = 'Yes, take survey now'; + const no = 'No, thanks'; + + setup(() => { + appShell = typemoq.Mock.ofType(); + browser = typemoq.Mock.ofType(); + }); + test('Data science banner should be enabled after we hit our command execution count', async () => { + const enabledValue: boolean = true; + const attemptCounter: number = 1000; + const testBanner: DataScienceSurveyBanner = preparePopup(attemptCounter, enabledValue, 0, appShell.object, browser.object, targetUri); + const expectedUri: string = targetUri; + let receivedUri: string = ''; + browser.setup(b => b.launch( + typemoq.It.is((a: string) => { + receivedUri = a; + return a === expectedUri; + })) + ).verifiable(typemoq.Times.once()); + await testBanner.launchSurvey(); + // This is technically not necessary, but it gives + // better output than the .verifyAll messages do. + expect(receivedUri).is.equal(expectedUri, 'Uri given to launch mock is incorrect.'); + + // verify that the calls expected were indeed made. + browser.verifyAll(); + browser.reset(); + }); + test('Do not show data science banner when it is disabled', () => { + appShell.setup(a => a.showInformationMessage(typemoq.It.isValue(message), + typemoq.It.isValue(yes), + typemoq.It.isValue(no))) + .verifiable(typemoq.Times.never()); + const enabledValue: boolean = false; + const attemptCounter: number = 0; + const testBanner: DataScienceSurveyBanner = preparePopup(attemptCounter, enabledValue, 0, appShell.object, browser.object, targetUri); + testBanner.showBanner().ignoreErrors(); + }); + test('Do not show data science banner if we have not hit our command count', () => { + appShell.setup(a => a.showInformationMessage(typemoq.It.isValue(message), + typemoq.It.isValue(yes), + typemoq.It.isValue(no))) + .verifiable(typemoq.Times.never()); + const enabledValue: boolean = true; + const attemptCounter: number = 100; + const testBanner: DataScienceSurveyBanner = preparePopup(attemptCounter, enabledValue, 1000, appShell.object, browser.object, targetUri); + testBanner.showBanner().ignoreErrors(); + }); +}); + +function preparePopup( + commandCounter: number, + enabledValue: boolean, + commandThreshold: number, + appShell: IApplicationShell, + browser: IBrowserService, + targetUri: string +): DataScienceSurveyBanner { + const myfactory: typemoq.IMock = typemoq.Mock.ofType(); + const enabledValState: typemoq.IMock> = typemoq.Mock.ofType>(); + const attemptCountState: typemoq.IMock> = typemoq.Mock.ofType>(); + enabledValState.setup(a => a.updateValue(typemoq.It.isValue(true))).returns(() => { + enabledValue = true; + return Promise.resolve(); + }); + enabledValState.setup(a => a.updateValue(typemoq.It.isValue(false))).returns(() => { + enabledValue = false; + return Promise.resolve(); + }); + + attemptCountState.setup(a => a.updateValue(typemoq.It.isAnyNumber())).returns(() => { + commandCounter += 1; + return Promise.resolve(); + }); + + enabledValState.setup(a => a.value).returns(() => enabledValue); + attemptCountState.setup(a => a.value).returns(() => commandCounter); + + myfactory.setup(a => a.createGlobalPersistentState(typemoq.It.isValue(DSSurveyStateKeys.ShowBanner), + typemoq.It.isValue(true))).returns(() => { + return enabledValState.object; + }); + myfactory.setup(a => a.createGlobalPersistentState(typemoq.It.isValue(DSSurveyStateKeys.ShowBanner), + typemoq.It.isValue(false))).returns(() => { + return enabledValState.object; + }); + myfactory.setup(a => a.createGlobalPersistentState(typemoq.It.isValue(DSSurveyStateKeys.ShowAttemptCounter), + typemoq.It.isAnyNumber())).returns(() => { + return attemptCountState.object; + }); + return new DataScienceSurveyBanner(appShell, myfactory.object, browser, commandThreshold, targetUri); +}