Display survey prompt once per session (#4110)

microsoft · Dec 3, 2020 · f5a13eb · f5a13eb
1 parent 89723c7
commit f5a13eb
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 15 deletions.
diff --git a/news/2 Fixes/4077.md b/news/2 Fixes/4077.md
@@ -0,0 +1 @@
+Display survey prompt once per session.
diff --git a/src/client/datascience/dataScienceSurveyBanner.ts b/src/client/datascience/dataScienceSurveyBanner.ts
@@ -136,11 +136,12 @@ export class DataScienceSurveyBanner implements IJupyterExtensionBanner {
         }
         const executionCount: number = this.getExecutionCount();
         const notebookCount: number = this.getOpenNotebookCount();
-        const show = await this.shouldShowBanner(executionCount, notebookCount);
+        const show = this.shouldShowBanner(executionCount, notebookCount);
         if (!show) {
             return;
         }
-
+        // Disable for the current session.
+        this.disabledInCurrentSession = true;
         const response = await this.appShell.showInformationMessage(this.bannerMessage, ...this.bannerLabels);
         switch (response) {
             case this.bannerLabels[DSSurveyLabelIndex.Yes]: {
@@ -154,14 +155,11 @@ export class DataScienceSurveyBanner implements IJupyterExtensionBanner {
                 await this.disable(3);
                 break;
             }
-            default: {
-                // Disable for the current session.
-                this.disabledInCurrentSession = true;
-            }
+            default:
         }
     }
 
-    public async shouldShowBanner(executionCount: number, notebookOpenCount: number): Promise<boolean> {
+    public shouldShowBanner(executionCount: number, notebookOpenCount: number) {
         if (!this.enabled || this.disabledInCurrentSession) {
             return false;
         }

diff --git a/src/client/datascience/insidersNativeNotebookSurveyBanner.ts b/src/client/datascience/insidersNativeNotebookSurveyBanner.ts
@@ -85,13 +85,17 @@ export class InsidersNativeNotebooksSurveyBanner implements IJupyterExtensionBan
     }
 
     public async showBanner(): Promise<void> {
+        if (this.disabledInCurrentSession) {
+            return;
+        }
+        // Disable for the current session.
+        this.disabledInCurrentSession = true;
         const executionCount: number = this.getExecutionCount();
         const notebookCount: number = this.getOpenNotebookCount();
         const show = await this.shouldShowBanner(executionCount, notebookCount);
         if (!show) {
             return;
         }
-
         const response = await this.appShell.showInformationMessage(this.bannerMessage, ...this.bannerLabels);
         switch (response) {
             case this.bannerLabels[DSSurveyLabelIndex.Yes]: {
@@ -105,15 +109,12 @@ export class InsidersNativeNotebooksSurveyBanner implements IJupyterExtensionBan
                 await this.disable(3);
                 break;
             }
-            default: {
-                // Disable for the current session.
-                this.disabledInCurrentSession = true;
-            }
+            default:
         }
     }
 
     public async shouldShowBanner(executionCount: number, notebookOpenCount: number): Promise<boolean> {
-        if (!this.enabled || this.disabledInCurrentSession) {
+        if (!this.enabled) {
             return false;
         }
 

diff --git a/src/test/datascience/datascienceSurveyBanner.vscode.test.ts b/src/test/datascience/datascienceSurveyBanner.vscode.test.ts
@@ -74,14 +74,17 @@ suite('DataScience Survey Banner', () => {
             persistentStateFactory.createGlobalPersistentState(DSSurveyStateKeys.ShowBanner, anything(), anything())
         ).thenReturn(showBannerState);
 
-        bannerService = new DataScienceSurveyBanner(
+        bannerService = createBannerService();
+    });
+    function createBannerService() {
+        return new DataScienceSurveyBanner(
             instance(appShell),
             instance(persistentStateFactory),
             instance(browser),
             instance(editorProvider),
             instance(appEnv)
         );
-    });
+    }
     test('Confirm prompt is displayed & only once per session', async () => {
         when(appShell.showInformationMessage(anything(), anything(), anything())).thenResolve();
         await showBannerState.updateValue({ data: true });
@@ -106,18 +109,21 @@ suite('DataScience Survey Banner', () => {
         resetCalls(appShell);
 
         // Attempt to display again & it won't.
+        bannerService = createBannerService();
         await bannerService.showBanner();
         verify(browser.launch(anything())).never();
         verify(appShell.showInformationMessage(anything(), anything(), anything())).never();
 
         // Advance time by 1 month & still not displayed.
         clock.tick(MillisecondsInADay * 30);
+        bannerService = createBannerService();
         await bannerService.showBanner();
         verify(browser.launch(anything())).never();
         verify(appShell.showInformationMessage(anything(), anything(), anything())).never();
 
         // Advance time by 3.5 month & it will be displayed.
         clock.tick(MillisecondsInADay * 30 * 3.5);
+        bannerService = createBannerService();
         await bannerService.showBanner();
         verify(browser.launch(anything())).never();
         verify(appShell.showInformationMessage(anything(), anything(), anything())).once();
@@ -137,12 +143,14 @@ suite('DataScience Survey Banner', () => {
         resetCalls(appShell);
 
         // Attempt to display again & it won't.
+        bannerService = createBannerService();
         await bannerService.showBanner();
         verify(browser.launch(anything())).never();
         verify(appShell.showInformationMessage(anything(), anything(), anything())).never();
 
         // Advance time by 1 month & still not displayed.
         clock.tick(MillisecondsInADay * 30);
+        bannerService = createBannerService();
         await bannerService.showBanner();
         verify(browser.launch(anything())).never();
         verify(appShell.showInformationMessage(anything(), anything(), anything())).never();
@@ -152,6 +160,7 @@ suite('DataScience Survey Banner', () => {
         when(appShell.showInformationMessage(anything(), anything(), anything())).thenResolve(
             localize.DataScienceSurveyBanner.bannerLabelNo() as any
         );
+        bannerService = createBannerService();
         await bannerService.showBanner();
         verify(browser.launch(anything())).never();
         verify(appShell.showInformationMessage(anything(), anything(), anything())).once();