Prevent flicker on Getting Started page (elastic#11826)

* Consolide chrome visibility toggle code into single module

* Extracting code into helper functions for better readability

* Removing redundant setting

* Extracting functions to higher level in scope so they are not defined multiple times

* Fixing typo in comment

* Starting to add unit tests

* Actually adding the tests this time

* Adding unit tests for handleGettingStartedOptedOutScenario function

* Binding this to window to prevent Uncaught TypeError: Illegal invocation

* Adding unit tests for handleExistingIndexPatternsScenario function

* Add helper function to aid unit testing

* Refactor unit tests

* Inlining function to prevent naming awkwardness

* Binding this to window to remove ambiguity

* Adding missing import

* Fixing import

* Catching expected error

src/core_plugins/getting_started/public/components/getting_started/getting_started.js

@@ -1,5 +1,4 @@
 import { uiModules } from 'ui/modules';
-import uiChrome from 'ui/chrome';
 import 'ui/getting_started/opt_out_directive';
 import { GettingStartedRegistryProvider } from 'ui/getting_started/registry';
 import { GETTING_STARTED_REGISTRY_TYPES } from 'ui/getting_started/constants';
@@ -32,12 +31,6 @@ app.directive('gettingStarted', function ($injector) {
     controllerAs: 'gettingStarted',
     controller: class GettingStartedController {
       constructor() {
-        if (this.hasOptedOut()) {
-          uiChrome.setVisible(true);
-        } else {
-          uiChrome.setVisible(false);
-        }
-
         const registeredTopMessages = registry.byType[GETTING_STARTED_REGISTRY_TYPES.TOP_MESSAGE] || [];
         this.topMessages = registeredTopMessages.map(item => item.template);
 

src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js

@@ -0,0 +1,179 @@
+import expect from 'expect.js';
+import sinon from 'sinon';
+import { set } from 'lodash';
+import uiChrome from 'ui/chrome';
+import { Notifier } from 'ui/notify/notifier';
+import { WAIT_FOR_URL_CHANGE_TOKEN } from 'ui/routes';
+import { gettingStartedGateCheck } from '../add_setup_work';
+import {
+  GETTING_STARTED_ROUTE,
+  CREATE_INDEX_PATTERN_ROUTE
+} from '../constants';
+import {
+  hasOptedOutOfGettingStarted,
+  undoOptOutOfGettingStarted
+} from 'ui/getting_started/opt_out_helpers';
+
+describe('Getting Started page', () => {
+  describe('add_setup_work', () => {
+    describe('gettingStartedGateCheck', () => {
+
+      let getIds;
+      let kbnUrl;
+      let config;
+      let $route;
+
+      beforeEach(() => {
+        kbnUrl = {
+          change: sinon.spy()
+        };
+        $route = {};
+        set($route, 'current.$$route', {});
+      });
+
+      describe('if index patterns exist', () => {
+        beforeEach(() => {
+          config = {
+            get: sinon.stub(),
+            set: sinon.spy()
+          };
+          getIds = sinon.stub()
+            .returns(Promise.resolve([ 'logstash-*', 'cars' ]));
+        });
+
+        it('sets the chrome to visible', async () => {
+          await gettingStartedGateCheck(getIds, kbnUrl, config, $route);
+          expect(uiChrome.getVisible()).to.be(true);
+        });
+
+        it('opts the user out of the Getting Started page', async () => {
+          await gettingStartedGateCheck(getIds, kbnUrl, config, $route);
+          expect(hasOptedOutOfGettingStarted()).to.be(true);
+        });
+
+        describe('if the current route does not require a default index pattern', () => {
+          beforeEach(() => {
+            $route.current.$$route.requireDefaultIndex = false;
+          });
+
+          it('returns without performing any default index pattern checks', async () => {
+            await gettingStartedGateCheck(getIds, kbnUrl, config, $route);
+            expect(config.get.called).to.be(false);
+            expect(config.set.called).to.be(false);
+          });
+        });
+
+        describe('if the current route requires a default index pattern', () => {
+          beforeEach(() => {
+            set($route, 'current.$$route.requireDefaultIndex', true);
+          });
+
+          describe('if a default index pattern exists', () => {
+            beforeEach(() => {
+              config.get
+                .withArgs('defaultIndex')
+                .returns('an-index-pattern');
+            });
+
+            it('returns without setting a default index pattern', async () => {
+              await gettingStartedGateCheck(getIds, kbnUrl, config, $route);
+              expect(config.set.called).to.be(false);
+            });
+          });
+
+          describe('if a default index pattern does not exist', () => {
+            beforeEach(() => {
+              config.get
+                .withArgs('defaultIndex')
+                .returns(undefined);
+            });
+
+            it('sets the first index pattern as the default index pattern', async () => {
+              await gettingStartedGateCheck(getIds, kbnUrl, config, $route);
+              expect(config.set.calledWith('defaultIndex', 'logstash-*')).to.be(true);
+            });
+          });
+        });
+      });
+
+      describe('if no index patterns exist', () => {
+        beforeEach(() => {
+          getIds = sinon.stub()
+            .returns(Promise.resolve([]));
+        });
+
+        describe('if user has opted out of the Getting Started page', () => {
+          it('sets the chrome to visible', async () => {
+            await gettingStartedGateCheck(getIds, kbnUrl, config, $route);
+            expect(uiChrome.getVisible()).to.be(true);
+          });
+
+          describe('if the current route does not require a default index pattern', () => {
+            beforeEach(() => {
+              $route.current.$$route.requireDefaultIndex = false;
+            });
+
+            it('returns without redirecting the user', async () => {
+              await gettingStartedGateCheck(getIds, kbnUrl, config, $route);
+              expect(kbnUrl.change.called).to.be(false);
+            });
+          });
+
+          describe('if the current route requires a default index pattern', () => {
+            beforeEach(() => {
+              $route.current.$$route.requireDefaultIndex = true;
+            });
+
+            afterEach(() => {
+              // Clear out any notifications
+              Notifier.prototype._notifs.length = 0;
+            });
+
+            it('redirects the user to the Create Index Pattern page', async () => {
+              try {
+                await gettingStartedGateCheck(getIds, kbnUrl, config, $route);
+              } catch (e) {
+                expect(e).to.be(WAIT_FOR_URL_CHANGE_TOKEN);
+              }
+              expect(kbnUrl.change.calledWith(CREATE_INDEX_PATTERN_ROUTE)).to.be(true);
+            });
+          });
+        });
+
+        describe('if the user has not opted out of the Getting Started page', () => {
+          beforeEach(() => {
+            undoOptOutOfGettingStarted();
+            getIds = sinon.stub()
+              .returns(Promise.resolve([]));
+          });
+
+          describe('if the user is not already on Getting Started page', () => {
+            beforeEach(() => {
+              $route.current.$$route.originalPath = 'discover';
+            });
+
+            it('redirects the user to the Getting Started page', async () => {
+              try {
+                await gettingStartedGateCheck(getIds, kbnUrl, config, $route);
+              } catch (e) {
+                expect(e).to.be(WAIT_FOR_URL_CHANGE_TOKEN);
+              }
+              expect(kbnUrl.change.calledWith(GETTING_STARTED_ROUTE)).to.be(true);
+            });
+          });
+
+          describe('if the user is already on Getting Started page', () => {
+            beforeEach(() => {
+              $route.current.$$route.originalPath = GETTING_STARTED_ROUTE;
+            });
+
+            it('redirects the user to the Getting Started page', async () => {
+              await gettingStartedGateCheck(getIds, kbnUrl, config, $route);
+              expect(kbnUrl.change.called).to.be(false);
+            });
+          });
+        });
+      });
+    });
+  });
+});

src/core_plugins/getting_started/public/lib/add_setup_work.js

@@ -3,78 +3,102 @@ import uiRoutes, { WAIT_FOR_URL_CHANGE_TOKEN } from 'ui/routes';
 import uiChrome from 'ui/chrome';
 import { Notifier } from 'ui/notify/notifier';
 import { IndexPatternsGetIdsProvider } from 'ui/index_patterns/_get_ids';
+import KbnUrlProvider from 'ui/url';
 import { hasOptedOutOfGettingStarted, optOutOfGettingStarted } from 'ui/getting_started/opt_out_helpers';
 
 import {
   GETTING_STARTED_ROUTE,
   CREATE_INDEX_PATTERN_ROUTE
 } from './constants';
 
-uiRoutes
-  .addSetupWork(function gettingStartedGateCheck(Private, $injector) {
-    const getIds = Private(IndexPatternsGetIdsProvider);
-    const config = $injector.get('config');
-    const kbnUrl = $injector.get('kbnUrl');
-    const $route = $injector.get('$route');
-
-    const currentRoute = get($route, 'current.$$route');
-
-    return getIds()
-    .then(indexPatterns => {
-      const indexPatternsExist = Array.isArray(indexPatterns) && indexPatterns.length > 0;
-      const isOnGettingStartedPage = get(currentRoute, 'originalPath') === GETTING_STARTED_ROUTE;
-
-      if (indexPatternsExist) {
-
-        // The user need not see the Getting Started page, so opt them out of it
-        optOutOfGettingStarted();
-
-        // Some routes require a default index pattern to be present. If we're
-        // NOT on such a route, there's nothing more to do; send the user on their way
-        if (!currentRoute.requireDefaultIndex) {
-          return;
-        }
-
-        // Otherwise, check if we have a default index pattern
-        let defaultIndexPattern = config.get('defaultIndex');
-
-        // If we don't have an default index pattern, make the first index pattern the
-        // default one
-        if (!Boolean(defaultIndexPattern)) {
-          defaultIndexPattern = indexPatterns[0];
-          config.set('defaultIndex', defaultIndexPattern);
-        }
-
-        // At this point, we have a default index pattern and are all set!
-        return;
-      }
-
-      // At this point, no index patterns exist.
-
-      // If the user has explicitly opted out of the Getting Started page
-      if (hasOptedOutOfGettingStarted()) {
-
-        // Some routes require a default index pattern to be present. If we're
-        // NOT on such a route, there's nothing more to do; send the user on their way
-        if (!currentRoute.requireDefaultIndex) {
-          return;
-        }
-
-        // Otherwise, redirect the user to the index pattern creation page with
-        // a notification about creating an index pattern
-        const notify = new Notifier({
-          location: 'Index Patterns'
-        });
-        notify.error('Please create a new index pattern');
-        kbnUrl.change(CREATE_INDEX_PATTERN_ROUTE);
-        throw WAIT_FOR_URL_CHANGE_TOKEN;
-      }
-
-      // Redirect the user to the Getting Started page (unless they are on it already)
-      if (!isOnGettingStartedPage) {
-        uiChrome.setVisible(false);
-        kbnUrl.change(GETTING_STARTED_ROUTE);
-        throw WAIT_FOR_URL_CHANGE_TOKEN;
-      }
-    });
+function handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config) {
+  // If index patterns exist, we're not going to show the user the Getting Started page.
+  // So we can show the chrome again at this point.
+  uiChrome.setVisible(true);
+
+  // The user need not see the Getting Started page, so opt them out of it
+  optOutOfGettingStarted();
+
+  // Some routes require a default index pattern to be present. If we're
+  // NOT on such a route, there's nothing more to do; send the user on their way
+  if (!currentRoute.requireDefaultIndex) {
+    return;
+  }
+
+  // Otherwise, check if we have a default index pattern
+  let defaultIndexPattern = config.get('defaultIndex');
+
+  // If we don't have an default index pattern, make the first index pattern the
+  // default one
+  if (!Boolean(defaultIndexPattern)) {
+    defaultIndexPattern = indexPatterns[0];
+    config.set('defaultIndex', defaultIndexPattern);
+  }
+
+  // At this point, we have a default index pattern and are all set!
+  return;
+}
+
+function handleGettingStartedOptedOutScenario(currentRoute, kbnUrl) {
+  // If the user has opted out of the Getting Started page, we're not going to show them that page.
+  // So we can show the chrome again at this point.
+  uiChrome.setVisible(true);
+
+  // Some routes require a default index pattern to be present. If we're
+  // NOT on such a route, there's nothing more to do; send the user on their way
+  if (!currentRoute.requireDefaultIndex) {
+    return;
+  }
+
+  // Otherwise, redirect the user to the index pattern creation page with
+  // a notification about creating an index pattern
+  const notify = new Notifier({
+    location: 'Index Patterns'
   });
+  notify.error('Please create a new index pattern');
+
+  kbnUrl.change(CREATE_INDEX_PATTERN_ROUTE);
+  throw WAIT_FOR_URL_CHANGE_TOKEN;
+}
+
+function showGettingStartedPage(kbnUrl, isOnGettingStartedPage) {
+  // Redirect the user to the Getting Started page (unless they are on it already)
+  if (!isOnGettingStartedPage) {
+    kbnUrl.change(GETTING_STARTED_ROUTE);
+    throw WAIT_FOR_URL_CHANGE_TOKEN;
+  }
+}
+
+/*
+ * This function is exported for unit testing
+ */
+export function gettingStartedGateCheck(getIds, kbnUrl, config, $route) {
+  const currentRoute = get($route, 'current.$$route');
+  const isOnGettingStartedPage = get(currentRoute, 'originalPath') === GETTING_STARTED_ROUTE;
+
+  return getIds()
+  .then(indexPatterns => {
+    const indexPatternsExist = Array.isArray(indexPatterns) && indexPatterns.length > 0;
+
+    if (indexPatternsExist) {
+      return handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config);
+    }
+
+    if (hasOptedOutOfGettingStarted()) {
+      return handleGettingStartedOptedOutScenario(currentRoute, kbnUrl);
+    }
+
+    return showGettingStartedPage(kbnUrl, isOnGettingStartedPage);
+  });
+}
+
+// Start out with the chrome not being shown to prevent a flicker by
+// hiding it later
+uiChrome.setVisible(false);
+uiRoutes.addSetupWork((Private, $injector) => {
+  const getIds = Private(IndexPatternsGetIdsProvider);
+  const kbnUrl = Private(KbnUrlProvider);
+  const config = $injector.get('config');
+  const $route = $injector.get('$route');
+  return gettingStartedGateCheck(getIds, kbnUrl, config, $route);
+});

src/ui/public/getting_started/opt_out_helpers.js

@@ -1,11 +1,16 @@
-import uiChrome from 'ui/chrome';
 import { GETTING_STARTED_OPT_OUT_FLAG } from './constants';
 
 export function hasOptedOutOfGettingStarted() {
-  return window.localStorage.getItem(GETTING_STARTED_OPT_OUT_FLAG) || false;
+  return Boolean(window.localStorage.getItem(GETTING_STARTED_OPT_OUT_FLAG));
 }
 
 export function optOutOfGettingStarted() {
   window.localStorage.setItem(GETTING_STARTED_OPT_OUT_FLAG, true);
-  uiChrome.setVisible(true);
+}
+
+/**
+ * This function is intended for unit testing
+ */
+export function undoOptOutOfGettingStarted() {
+  window.localStorage.removeItem(GETTING_STARTED_OPT_OUT_FLAG);
 }