From 2e570e3d6f0eec5a85a7f02557c7ae9d882f1866 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Tue, 16 May 2017 12:10:34 -0700 Subject: [PATCH 01/17] Consolide chrome visibility toggle code into single module --- .../getting_started/getting_started.js | 7 ------- .../public/lib/add_setup_work.js | 20 ++++++++++++++++++- .../public/getting_started/opt_out_helpers.js | 2 -- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/core_plugins/getting_started/public/components/getting_started/getting_started.js b/src/core_plugins/getting_started/public/components/getting_started/getting_started.js index 1ec9c774d445f..14de2353076aa 100644 --- a/src/core_plugins/getting_started/public/components/getting_started/getting_started.js +++ b/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); diff --git a/src/core_plugins/getting_started/public/lib/add_setup_work.js b/src/core_plugins/getting_started/public/lib/add_setup_work.js index 144d37fa7c891..7d65a93d0c353 100644 --- a/src/core_plugins/getting_started/public/lib/add_setup_work.js +++ b/src/core_plugins/getting_started/public/lib/add_setup_work.js @@ -10,6 +10,10 @@ import { CREATE_INDEX_PATTERN_ROUTE } from './constants'; +// Start out with the chrome not being shown to prevent a flicker by +// hiding it later +uiChrome.setVisible(false); + uiRoutes .addSetupWork(function gettingStartedGateCheck(Private, $injector) { const getIds = Private(IndexPatternsGetIdsProvider); @@ -26,6 +30,12 @@ uiRoutes if (indexPatternsExist) { + // TODO: extract out into fucntion + + // If index patterns exist, we're not going to show the user the Getting Starte 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(); @@ -54,6 +64,12 @@ uiRoutes // If the user has explicitly opted out of the Getting Started page if (hasOptedOutOfGettingStarted()) { + // TODO: extract out into fucntion + + // 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) { @@ -70,9 +86,11 @@ uiRoutes throw WAIT_FOR_URL_CHANGE_TOKEN; } + // At this point we want to show the user the Getting Started page. So show the chrome. + uiChrome.setVisible(false); + // 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; } diff --git a/src/ui/public/getting_started/opt_out_helpers.js b/src/ui/public/getting_started/opt_out_helpers.js index e6cd8320f0ffe..d46b233799d34 100644 --- a/src/ui/public/getting_started/opt_out_helpers.js +++ b/src/ui/public/getting_started/opt_out_helpers.js @@ -1,4 +1,3 @@ -import uiChrome from 'ui/chrome'; import { GETTING_STARTED_OPT_OUT_FLAG } from './constants'; export function hasOptedOutOfGettingStarted() { @@ -7,5 +6,4 @@ export function hasOptedOutOfGettingStarted() { export function optOutOfGettingStarted() { window.localStorage.setItem(GETTING_STARTED_OPT_OUT_FLAG, true); - uiChrome.setVisible(true); } \ No newline at end of file From 12d9dbc47ef6757067ff38c6b2a761e2f6386097 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Tue, 16 May 2017 12:19:05 -0700 Subject: [PATCH 02/17] Extracting code into helper functions for better readability --- .../public/lib/add_setup_work.js | 95 ++++++++++--------- 1 file changed, 49 insertions(+), 46 deletions(-) diff --git a/src/core_plugins/getting_started/public/lib/add_setup_work.js b/src/core_plugins/getting_started/public/lib/add_setup_work.js index 7d65a93d0c353..30447b2b8c7b9 100644 --- a/src/core_plugins/getting_started/public/lib/add_setup_work.js +++ b/src/core_plugins/getting_started/public/lib/add_setup_work.js @@ -22,70 +22,73 @@ uiRoutes const $route = $injector.get('$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; - const isOnGettingStartedPage = get(currentRoute, 'originalPath') === GETTING_STARTED_ROUTE; if (indexPatternsExist) { + return handleExistingIndexPatternsScenario(indexPatterns); + } - // TODO: extract out into fucntion + if (hasOptedOutOfGettingStarted()) { + return handleGettingStartedOptedOutScenario(); + } - // If index patterns exist, we're not going to show the user the Getting Starte page. - // So we can show the chrome again at this point. - uiChrome.setVisible(true); + return showGettingStartedPage(); + }); - // The user need not see the Getting Started page, so opt them out of it - optOutOfGettingStarted(); + function handleExistingIndexPatternsScenario(indexPatterns) { + // If index patterns exist, we're not going to show the user the Getting Starte 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; - } + // The user need not see the Getting Started page, so opt them out of it + optOutOfGettingStarted(); - // Otherwise, check if we have a default index pattern - let defaultIndexPattern = config.get('defaultIndex'); + // 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; + } - // 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); - } + // Otherwise, check if we have a default index pattern + let defaultIndexPattern = config.get('defaultIndex'); - // At this point, we have a default index pattern and are all set! - return; + // 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, no index patterns exist. + // At this point, we have a default index pattern and are all set! + return; + } - // If the user has explicitly opted out of the Getting Started page - if (hasOptedOutOfGettingStarted()) { + function handleGettingStartedOptedOutScenario() { + // 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); - // TODO: extract out into fucntion - - // 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; + // 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() { // At this point we want to show the user the Getting Started page. So show the chrome. uiChrome.setVisible(false); @@ -94,5 +97,5 @@ uiRoutes kbnUrl.change(GETTING_STARTED_ROUTE); throw WAIT_FOR_URL_CHANGE_TOKEN; } - }); + } }); From f3b6b65e53cfb92051647af1a5d8a95715c2b707 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Tue, 16 May 2017 12:23:51 -0700 Subject: [PATCH 03/17] Removing redundant setting --- src/core_plugins/getting_started/public/lib/add_setup_work.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/core_plugins/getting_started/public/lib/add_setup_work.js b/src/core_plugins/getting_started/public/lib/add_setup_work.js index 30447b2b8c7b9..dc5c3e3027f48 100644 --- a/src/core_plugins/getting_started/public/lib/add_setup_work.js +++ b/src/core_plugins/getting_started/public/lib/add_setup_work.js @@ -89,9 +89,6 @@ uiRoutes } function showGettingStartedPage() { - // At this point we want to show the user the Getting Started page. So show the chrome. - uiChrome.setVisible(false); - // Redirect the user to the Getting Started page (unless they are on it already) if (!isOnGettingStartedPage) { kbnUrl.change(GETTING_STARTED_ROUTE); From 6a419950f05a37fdf1e2ee962287c90baf7dda78 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Tue, 16 May 2017 13:13:12 -0700 Subject: [PATCH 04/17] Extracting functions to higher level in scope so they are not defined multiple times --- .../public/lib/add_setup_work.js | 121 +++++++++--------- 1 file changed, 61 insertions(+), 60 deletions(-) diff --git a/src/core_plugins/getting_started/public/lib/add_setup_work.js b/src/core_plugins/getting_started/public/lib/add_setup_work.js index dc5c3e3027f48..a670903352a3c 100644 --- a/src/core_plugins/getting_started/public/lib/add_setup_work.js +++ b/src/core_plugins/getting_started/public/lib/add_setup_work.js @@ -10,6 +10,64 @@ import { CREATE_INDEX_PATTERN_ROUTE } from './constants'; +function handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config) { + // If index patterns exist, we're not going to show the user the Getting Starte 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; + } +} + // Start out with the chrome not being shown to prevent a flicker by // hiding it later uiChrome.setVisible(false); @@ -29,70 +87,13 @@ uiRoutes const indexPatternsExist = Array.isArray(indexPatterns) && indexPatterns.length > 0; if (indexPatternsExist) { - return handleExistingIndexPatternsScenario(indexPatterns); + return handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config); } if (hasOptedOutOfGettingStarted()) { - return handleGettingStartedOptedOutScenario(); + return handleGettingStartedOptedOutScenario(currentRoute, kbnUrl); } - return showGettingStartedPage(); + return showGettingStartedPage(kbnUrl, isOnGettingStartedPage); }); - - function handleExistingIndexPatternsScenario(indexPatterns) { - // If index patterns exist, we're not going to show the user the Getting Starte 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() { - // 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() { - // 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; - } - } }); From cf3c6c84ab2d3428eadd7cd759283f5b47dc5e1c Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Wed, 17 May 2017 06:10:32 -0700 Subject: [PATCH 05/17] Fixing typo in comment --- src/core_plugins/getting_started/public/lib/add_setup_work.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core_plugins/getting_started/public/lib/add_setup_work.js b/src/core_plugins/getting_started/public/lib/add_setup_work.js index a670903352a3c..901f0e704e8b9 100644 --- a/src/core_plugins/getting_started/public/lib/add_setup_work.js +++ b/src/core_plugins/getting_started/public/lib/add_setup_work.js @@ -11,7 +11,7 @@ import { } from './constants'; function handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config) { - // If index patterns exist, we're not going to show the user the Getting Starte page. + // 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); From 1c89de6496ee516d0feba72ee12a050164dadc9e Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Wed, 17 May 2017 06:29:29 -0700 Subject: [PATCH 06/17] Starting to add unit tests --- .../public/lib/add_setup_work.js | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/src/core_plugins/getting_started/public/lib/add_setup_work.js b/src/core_plugins/getting_started/public/lib/add_setup_work.js index 901f0e704e8b9..666503eed9711 100644 --- a/src/core_plugins/getting_started/public/lib/add_setup_work.js +++ b/src/core_plugins/getting_started/public/lib/add_setup_work.js @@ -10,7 +10,7 @@ import { CREATE_INDEX_PATTERN_ROUTE } from './constants'; -function handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config) { +export 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); @@ -38,7 +38,7 @@ function handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config return; } -function handleGettingStartedOptedOutScenario(currentRoute, kbnUrl) { +export 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); @@ -60,7 +60,7 @@ function handleGettingStartedOptedOutScenario(currentRoute, kbnUrl) { throw WAIT_FOR_URL_CHANGE_TOKEN; } -function showGettingStartedPage(kbnUrl, isOnGettingStartedPage) { +export 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); @@ -68,32 +68,32 @@ function showGettingStartedPage(kbnUrl, isOnGettingStartedPage) { } } -// Start out with the chrome not being shown to prevent a flicker by -// hiding it later -uiChrome.setVisible(false); +export function gettingStartedGateCheck(Private, $injector) { + const getIds = Private(IndexPatternsGetIdsProvider); + const kbnUrl = Private(KbnUrlProvider); + const config = $injector.get('config'); + const $route = $injector.get('$route'); -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'); + const isOnGettingStartedPage = get(currentRoute, 'originalPath') === GETTING_STARTED_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; - return getIds() - .then(indexPatterns => { - const indexPatternsExist = Array.isArray(indexPatterns) && indexPatterns.length > 0; + if (indexPatternsExist) { + return handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config); + } - if (indexPatternsExist) { - return handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config); - } + if (hasOptedOutOfGettingStarted()) { + return handleGettingStartedOptedOutScenario(currentRoute, kbnUrl); + } - if (hasOptedOutOfGettingStarted()) { - return handleGettingStartedOptedOutScenario(currentRoute, kbnUrl); - } - - return showGettingStartedPage(kbnUrl, isOnGettingStartedPage); - }); + 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(gettingStartedGateCheck); \ No newline at end of file From 70a0a636f66c3073fde5ac1aa726ba05c615c407 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Wed, 17 May 2017 06:33:42 -0700 Subject: [PATCH 07/17] Actually adding the tests this time --- .../public/lib/__tests__/add_setup_work.js | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js diff --git a/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js b/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js new file mode 100644 index 0000000000000..b772d9cf57110 --- /dev/null +++ b/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js @@ -0,0 +1,44 @@ +import expect from 'expect.js'; +import sinon from 'sinon'; + +import { + showGettingStartedPage +} from '../add_setup_work'; +import { GETTING_STARTED_ROUTE } from '../constants'; + +describe('Getting Started page', () => { + describe('add_setup_work', () => { + describe('showGettingStartedPage', () => { + let spyKbnUrl; + let isOnGettingstartedPage; + + beforeEach(() => { + spyKbnUrl = { + change: sinon.spy() + }; + }); + + describe('user is not already on Getting Started page', () => { + beforeEach(() => { + isOnGettingstartedPage = false; + }); + + it ('redirects the user to the Getting Started page', () => { + showGettingStartedPage(spyKbnUrl, isOnGettingstartedPage); + expect(spyKbnUrl.change.calledWith(GETTING_STARTED_ROUTE)).to.be(true); + }); + }); + + describe('user is already on Getting Started page', () => { + beforeEach(() => { + isOnGettingstartedPage = true; + }); + + it ('redirects the user to the Getting Started page', () => { + showGettingStartedPage(spyKbnUrl, isOnGettingstartedPage); + expect(spyKbnUrl.change.called).to.be(false); + }); + }); + }); + }); +}); \ No newline at end of file From 4bdc458926ed732d3507ecbfaca876cf8f19a72c Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Wed, 17 May 2017 07:59:13 -0700 Subject: [PATCH 08/17] Adding unit tests for handleGettingStartedOptedOutScenario function --- .../public/lib/__tests__/add_setup_work.js | 57 ++++++++++++++++--- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js b/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js index b772d9cf57110..263201e0e2d43 100644 --- a/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js +++ b/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js @@ -1,23 +1,28 @@ import expect from 'expect.js'; import sinon from 'sinon'; +import uiChrome from 'ui/chrome'; import { - showGettingStartedPage + showGettingStartedPage, + handleGettingStartedOptedOutScenario } from '../add_setup_work'; -import { GETTING_STARTED_ROUTE } from '../constants'; +import { + GETTING_STARTED_ROUTE, + CREATE_INDEX_PATTERN_ROUTE +} from '../constants'; describe('Getting Started page', () => { describe('add_setup_work', () => { + let spyKbnUrl; + beforeEach(() => { + spyKbnUrl = { + change: sinon.spy() + }; + }); + describe('showGettingStartedPage', () => { - let spyKbnUrl; let isOnGettingstartedPage; - beforeEach(() => { - spyKbnUrl = { - change: sinon.spy() - }; - }); - describe('user is not already on Getting Started page', () => { beforeEach(() => { isOnGettingstartedPage = false; @@ -40,5 +45,39 @@ describe('Getting Started page', () => { }); }); }); + + describe('handleGettingStartedOptedOutScenario', () => { + let currentRoute; + beforeEach(() => { + currentRoute = {}; + }); + + it ('sets the chrome to visible', () => { + handleGettingStartedOptedOutScenario(currentRoute, spyKbnUrl); + expect(uiChrome.getVisible()).to.be(true); + }); + + describe('current route does not require a default index pattern', () => { + beforeEach(() => { + currentRoute.requireDefaultIndex = false; + }); + + it ('returns without redirecting the user', () => { + handleGettingStartedOptedOutScenario(currentRoute, spyKbnUrl); + expect(spyKbnUrl.change.called).to.be(false); + }); + }); + + describe('current route requires a default index pattern', () => { + beforeEach(() => { + currentRoute.requireDefaultIndex = true; + }); + + it ('redirects the user to the Create Index Pattern page', () => { + handleGettingStartedOptedOutScenario(currentRoute, spyKbnUrl); + expect(spyKbnUrl.change.calledWith(CREATE_INDEX_PATTERN_ROUTE)).to.be(true); + }); + }); + }); }); }); \ No newline at end of file From 19f0bea97f7ac81c9f80598606283122d1493949 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Wed, 17 May 2017 07:59:28 -0700 Subject: [PATCH 09/17] Binding this to window to prevent Uncaught TypeError: Illegal invocation --- src/ui/public/notify/notifier.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/public/notify/notifier.js b/src/ui/public/notify/notifier.js index 5395e44089111..1b937bdf3cb70 100644 --- a/src/ui/public/notify/notifier.js +++ b/src/ui/public/notify/notifier.js @@ -230,7 +230,7 @@ Notifier.config = { errorLifetime: 300000, warningLifetime: 10000, infoLifetime: 5000, - setInterval: window.setInterval, + setInterval: window.setInterval.bind(window), clearInterval: window.clearInterval }; From d458195c928f37adf857351786f2ab382e20a215 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Wed, 17 May 2017 08:27:54 -0700 Subject: [PATCH 10/17] Adding unit tests for handleExistingIndexPatternsScenario function --- .../public/lib/__tests__/add_setup_work.js | 109 +++++++++++++++--- .../public/getting_started/opt_out_helpers.js | 2 +- 2 files changed, 94 insertions(+), 17 deletions(-) diff --git a/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js b/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js index 263201e0e2d43..e7ad2267c3fa6 100644 --- a/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js +++ b/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js @@ -1,23 +1,29 @@ import expect from 'expect.js'; import sinon from 'sinon'; import uiChrome from 'ui/chrome'; +import { Notifier } from 'ui/notify/notifier'; import { showGettingStartedPage, - handleGettingStartedOptedOutScenario + handleGettingStartedOptedOutScenario, + handleExistingIndexPatternsScenario } from '../add_setup_work'; import { GETTING_STARTED_ROUTE, CREATE_INDEX_PATTERN_ROUTE } from '../constants'; +import { hasOptedOutOfGettingStarted } from 'ui/getting_started/opt_out_helpers'; describe('Getting Started page', () => { describe('add_setup_work', () => { - let spyKbnUrl; + let kbnUrl; + let currentRoute; + beforeEach(() => { - spyKbnUrl = { + kbnUrl = { change: sinon.spy() }; + currentRoute = {}; }); describe('showGettingStartedPage', () => { @@ -29,8 +35,8 @@ describe('Getting Started page', () => { }); it ('redirects the user to the Getting Started page', () => { - showGettingStartedPage(spyKbnUrl, isOnGettingstartedPage); - expect(spyKbnUrl.change.calledWith(GETTING_STARTED_ROUTE)).to.be(true); + showGettingStartedPage(kbnUrl, isOnGettingstartedPage); + expect(kbnUrl.change.calledWith(GETTING_STARTED_ROUTE)).to.be(true); }); }); @@ -40,31 +46,77 @@ describe('Getting Started page', () => { }); it ('redirects the user to the Getting Started page', () => { - showGettingStartedPage(spyKbnUrl, isOnGettingstartedPage); - expect(spyKbnUrl.change.called).to.be(false); + showGettingStartedPage(kbnUrl, isOnGettingstartedPage); + expect(kbnUrl.change.called).to.be(false); }); }); }); describe('handleGettingStartedOptedOutScenario', () => { - let currentRoute; + it ('sets the chrome to visible', () => { + handleGettingStartedOptedOutScenario(currentRoute, kbnUrl); + expect(uiChrome.getVisible()).to.be(true); + }); + + describe('current route does not require a default index pattern', () => { + beforeEach(() => { + currentRoute.requireDefaultIndex = false; + }); + + it ('returns without redirecting the user', () => { + handleGettingStartedOptedOutScenario(currentRoute, kbnUrl); + expect(kbnUrl.change.called).to.be(false); + }); + }); + + describe('current route requires a default index pattern', () => { + beforeEach(() => { + currentRoute.requireDefaultIndex = true; + }); + + afterEach(() => { + // Clear out any notifications + Notifier.prototype._notifs.length = 0; + }); + + it ('redirects the user to the Create Index Pattern page', () => { + handleGettingStartedOptedOutScenario(currentRoute, kbnUrl); + expect(kbnUrl.change.calledWith(CREATE_INDEX_PATTERN_ROUTE)).to.be(true); + }); + }); + }); + + describe('handleExistingIndexPatternsScenario', () => { + + let indexPatterns; + let config; + beforeEach(() => { - currentRoute = {}; + config = { + get: sinon.stub(), + set: sinon.spy() + }; }); it ('sets the chrome to visible', () => { - handleGettingStartedOptedOutScenario(currentRoute, spyKbnUrl); + handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config); expect(uiChrome.getVisible()).to.be(true); }); + it ('opts the user out of the Getting Started page', () => { + handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config); + expect(hasOptedOutOfGettingStarted()).to.be(true); + }); + describe('current route does not require a default index pattern', () => { beforeEach(() => { currentRoute.requireDefaultIndex = false; }); - it ('returns without redirecting the user', () => { - handleGettingStartedOptedOutScenario(currentRoute, spyKbnUrl); - expect(spyKbnUrl.change.called).to.be(false); + it ('returns without performing any default index pattern checks', () => { + handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config); + expect(config.get.called).to.be(false); + expect(config.set.called).to.be(false); }); }); @@ -73,9 +125,34 @@ describe('Getting Started page', () => { currentRoute.requireDefaultIndex = true; }); - it ('redirects the user to the Create Index Pattern page', () => { - handleGettingStartedOptedOutScenario(currentRoute, spyKbnUrl); - expect(spyKbnUrl.change.calledWith(CREATE_INDEX_PATTERN_ROUTE)).to.be(true); + describe('default index pattern exists', () => { + beforeEach(() => { + config.get + .withArgs('defaultIndex') + .returns('an-index-pattern'); + }); + + it ('returns without setting a default index pattern', () => { + handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config); + expect(config.set.called).to.be(false); + }); + }); + + describe('default index pattern does not exist', () => { + beforeEach(() => { + indexPatterns = [ + 'logstash-*', + 'cars' + ]; + config.get + .withArgs('defaultIndex') + .returns(undefined); + }); + + it ('sets the first index pattern as the default index pattern', () => { + handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config); + expect(config.set.calledWith('defaultIndex', indexPatterns[0])).to.be(true); + }); }); }); }); diff --git a/src/ui/public/getting_started/opt_out_helpers.js b/src/ui/public/getting_started/opt_out_helpers.js index d46b233799d34..dc3b17062e487 100644 --- a/src/ui/public/getting_started/opt_out_helpers.js +++ b/src/ui/public/getting_started/opt_out_helpers.js @@ -1,7 +1,7 @@ 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() { From 0350d561ce70cd7dce44a4451da90da214bed5fd Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Wed, 17 May 2017 16:07:03 -0700 Subject: [PATCH 11/17] Add helper function to aid unit testing --- src/ui/public/getting_started/opt_out_helpers.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/ui/public/getting_started/opt_out_helpers.js b/src/ui/public/getting_started/opt_out_helpers.js index dc3b17062e487..9e07c34a42066 100644 --- a/src/ui/public/getting_started/opt_out_helpers.js +++ b/src/ui/public/getting_started/opt_out_helpers.js @@ -6,4 +6,11 @@ export function hasOptedOutOfGettingStarted() { export function optOutOfGettingStarted() { window.localStorage.setItem(GETTING_STARTED_OPT_OUT_FLAG, true); +} + +/** + * This function is intended for unit testing + */ +export function undoOptOutOfGettingStarted() { + window.localStorage.removeItem(GETTING_STARTED_OPT_OUT_FLAG); } \ No newline at end of file From e54fa40b0a66db0fc4788b130ba790fca71d0a88 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Wed, 17 May 2017 16:08:01 -0700 Subject: [PATCH 12/17] Refactor unit tests --- .../public/lib/__tests__/add_setup_work.js | 231 +++++++++--------- .../public/lib/add_setup_work.js | 30 ++- 2 files changed, 141 insertions(+), 120 deletions(-) diff --git a/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js b/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js index e7ad2267c3fa6..716ed75b0e0c9 100644 --- a/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js +++ b/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js @@ -1,157 +1,168 @@ 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 { - showGettingStartedPage, - handleGettingStartedOptedOutScenario, - handleExistingIndexPatternsScenario -} from '../add_setup_work'; +import { gettingStartedGateCheck } from '../add_setup_work'; import { GETTING_STARTED_ROUTE, CREATE_INDEX_PATTERN_ROUTE } from '../constants'; -import { hasOptedOutOfGettingStarted } from 'ui/getting_started/opt_out_helpers'; +import { + hasOptedOutOfGettingStarted, + undoOptOutOfGettingStarted +} from 'ui/getting_started/opt_out_helpers'; describe('Getting Started page', () => { describe('add_setup_work', () => { - let kbnUrl; - let currentRoute; - - beforeEach(() => { - kbnUrl = { - change: sinon.spy() - }; - currentRoute = {}; - }); - - describe('showGettingStartedPage', () => { - let isOnGettingstartedPage; + describe('gettingStartedGateCheck', () => { - describe('user is not already on Getting Started page', () => { - beforeEach(() => { - isOnGettingstartedPage = false; - }); + let getIds; + let kbnUrl; + let config; + let $route; - it ('redirects the user to the Getting Started page', () => { - showGettingStartedPage(kbnUrl, isOnGettingstartedPage); - expect(kbnUrl.change.calledWith(GETTING_STARTED_ROUTE)).to.be(true); - }); + beforeEach(() => { + kbnUrl = { + change: sinon.spy() + }; + $route = {}; + set($route, 'current.$$route', {}); }); - describe('user is already on Getting Started page', () => { + describe('if index patterns exist', () => { beforeEach(() => { - isOnGettingstartedPage = true; + config = { + get: sinon.stub(), + set: sinon.spy() + }; + getIds = sinon.stub() + .returns(Promise.resolve([ 'logstash-*', 'cars' ])); }); - it ('redirects the user to the Getting Started page', () => { - showGettingStartedPage(kbnUrl, isOnGettingstartedPage); - expect(kbnUrl.change.called).to.be(false); + it('sets the chrome to visible', async () => { + await gettingStartedGateCheck(getIds, kbnUrl, config, $route); + expect(uiChrome.getVisible()).to.be(true); }); - }); - }); - - describe('handleGettingStartedOptedOutScenario', () => { - it ('sets the chrome to visible', () => { - handleGettingStartedOptedOutScenario(currentRoute, kbnUrl); - expect(uiChrome.getVisible()).to.be(true); - }); - describe('current route does not require a default index pattern', () => { - beforeEach(() => { - currentRoute.requireDefaultIndex = false; + it('opts the user out of the Getting Started page', async () => { + await gettingStartedGateCheck(getIds, kbnUrl, config, $route); + expect(hasOptedOutOfGettingStarted()).to.be(true); }); - it ('returns without redirecting the user', () => { - handleGettingStartedOptedOutScenario(currentRoute, kbnUrl); - expect(kbnUrl.change.called).to.be(false); - }); - }); + describe('if the current route does not require a default index pattern', () => { + beforeEach(() => { + $route.current.$$route.requireDefaultIndex = false; + }); - describe('current route requires a default index pattern', () => { - beforeEach(() => { - currentRoute.requireDefaultIndex = true; + 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); + }); }); - afterEach(() => { - // Clear out any notifications - Notifier.prototype._notifs.length = 0; - }); + describe('if the current route requires a default index pattern', () => { + beforeEach(() => { + set($route, 'current.$$route.requireDefaultIndex', true); + }); - it ('redirects the user to the Create Index Pattern page', () => { - handleGettingStartedOptedOutScenario(currentRoute, kbnUrl); - expect(kbnUrl.change.calledWith(CREATE_INDEX_PATTERN_ROUTE)).to.be(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('handleExistingIndexPatternsScenario', () => { - let indexPatterns; - let config; + describe('if no index patterns exist', () => { + beforeEach(() => { + getIds = sinon.stub() + .returns(Promise.resolve([])); + }); - beforeEach(() => { - config = { - get: sinon.stub(), - set: sinon.spy() - }; - }); + 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); + }); - it ('sets the chrome to visible', () => { - handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config); - 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 ('opts the user out of the Getting Started page', () => { - handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config); - expect(hasOptedOutOfGettingStarted()).to.be(true); - }); + it('returns without redirecting the user', async () => { + await gettingStartedGateCheck(getIds, kbnUrl, config, $route); + expect(kbnUrl.change.called).to.be(false); + }); + }); - describe('current route does not require a default index pattern', () => { - beforeEach(() => { - currentRoute.requireDefaultIndex = false; - }); + describe('if the current route requires a default index pattern', () => { + beforeEach(() => { + $route.current.$$route.requireDefaultIndex = true; + }); - it ('returns without performing any default index pattern checks', () => { - handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config); - expect(config.get.called).to.be(false); - expect(config.set.called).to.be(false); - }); - }); + afterEach(() => { + // Clear out any notifications + Notifier.prototype._notifs.length = 0; + }); - describe('current route requires a default index pattern', () => { - beforeEach(() => { - currentRoute.requireDefaultIndex = true; + it('redirects the user to the Create Index Pattern page', async () => { + await gettingStartedGateCheck(getIds, kbnUrl, config, $route); + expect(kbnUrl.change.calledWith(CREATE_INDEX_PATTERN_ROUTE)).to.be(true); + }); + }); }); - describe('default index pattern exists', () => { + describe('if the user has not opted out of the Getting Started page', () => { beforeEach(() => { - config.get - .withArgs('defaultIndex') - .returns('an-index-pattern'); + undoOptOutOfGettingStarted(); + getIds = sinon.stub() + .returns(Promise.resolve([])); }); - it ('returns without setting a default index pattern', () => { - handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config); - expect(config.set.called).to.be(false); - }); - }); + describe('if the user is not already on Getting Started page', () => { + beforeEach(() => { + $route.current.$$route.originalPath = 'discover'; + }); - describe('default index pattern does not exist', () => { - beforeEach(() => { - indexPatterns = [ - 'logstash-*', - 'cars' - ]; - config.get - .withArgs('defaultIndex') - .returns(undefined); + it('redirects the user to the Getting Started page', async () => { + await gettingStartedGateCheck(getIds, kbnUrl, config, $route); + expect(kbnUrl.change.calledWith(GETTING_STARTED_ROUTE)).to.be(true); + }); }); - it ('sets the first index pattern as the default index pattern', () => { - handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config); - expect(config.set.calledWith('defaultIndex', indexPatterns[0])).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); + }); }); }); }); diff --git a/src/core_plugins/getting_started/public/lib/add_setup_work.js b/src/core_plugins/getting_started/public/lib/add_setup_work.js index 666503eed9711..41fd61ff735ea 100644 --- a/src/core_plugins/getting_started/public/lib/add_setup_work.js +++ b/src/core_plugins/getting_started/public/lib/add_setup_work.js @@ -10,7 +10,7 @@ import { CREATE_INDEX_PATTERN_ROUTE } from './constants'; -export function handleExistingIndexPatternsScenario(indexPatterns, currentRoute, config) { +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); @@ -38,7 +38,7 @@ export function handleExistingIndexPatternsScenario(indexPatterns, currentRoute, return; } -export function handleGettingStartedOptedOutScenario(currentRoute, kbnUrl) { +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); @@ -60,7 +60,7 @@ export function handleGettingStartedOptedOutScenario(currentRoute, kbnUrl) { throw WAIT_FOR_URL_CHANGE_TOKEN; } -export function showGettingStartedPage(kbnUrl, isOnGettingStartedPage) { +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); @@ -68,12 +68,10 @@ export function showGettingStartedPage(kbnUrl, isOnGettingStartedPage) { } } -export function gettingStartedGateCheck(Private, $injector) { - const getIds = Private(IndexPatternsGetIdsProvider); - const kbnUrl = Private(KbnUrlProvider); - const config = $injector.get('config'); - const $route = $injector.get('$route'); - +/* + * 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; @@ -93,7 +91,19 @@ export function gettingStartedGateCheck(Private, $injector) { }); } +/** + * This function sets up Angular-injected dependencies and immediately calls + * gettingStartedGateCheck, which does the actual work. + */ +function _gettingStartedGateCheck(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); +} + // Start out with the chrome not being shown to prevent a flicker by // hiding it later uiChrome.setVisible(false); -uiRoutes.addSetupWork(gettingStartedGateCheck); \ No newline at end of file +uiRoutes.addSetupWork(_gettingStartedGateCheck); \ No newline at end of file From 65148b312df7c3a992ac08bb02b367c38cbdf27e Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Wed, 17 May 2017 17:14:17 -0700 Subject: [PATCH 13/17] Inlining function to prevent naming awkwardness --- .../getting_started/public/lib/add_setup_work.js | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/core_plugins/getting_started/public/lib/add_setup_work.js b/src/core_plugins/getting_started/public/lib/add_setup_work.js index 41fd61ff735ea..ef6330e8370f9 100644 --- a/src/core_plugins/getting_started/public/lib/add_setup_work.js +++ b/src/core_plugins/getting_started/public/lib/add_setup_work.js @@ -91,19 +91,13 @@ export function gettingStartedGateCheck(getIds, kbnUrl, config, $route) { }); } -/** - * This function sets up Angular-injected dependencies and immediately calls - * gettingStartedGateCheck, which does the actual work. - */ -function _gettingStartedGateCheck(Private, $injector) { +// 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); -} - -// Start out with the chrome not being shown to prevent a flicker by -// hiding it later -uiChrome.setVisible(false); -uiRoutes.addSetupWork(_gettingStartedGateCheck); \ No newline at end of file +}); \ No newline at end of file From 965bd0b0e8aedea77636452249e03af07e0cd524 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Wed, 17 May 2017 17:14:50 -0700 Subject: [PATCH 14/17] Binding this to window to remove ambiguity --- src/ui/public/notify/notifier.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/public/notify/notifier.js b/src/ui/public/notify/notifier.js index 1b937bdf3cb70..0382c6cefeb2f 100644 --- a/src/ui/public/notify/notifier.js +++ b/src/ui/public/notify/notifier.js @@ -231,7 +231,7 @@ Notifier.config = { warningLifetime: 10000, infoLifetime: 5000, setInterval: window.setInterval.bind(window), - clearInterval: window.clearInterval + clearInterval: window.clearInterval.bind(window) }; Notifier.applyConfig = function (config) { From f39599d5f227feddec1eae53637681db6e4b4ca9 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 19 May 2017 04:47:54 -0700 Subject: [PATCH 15/17] Adding missing import --- src/core_plugins/getting_started/public/lib/add_setup_work.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core_plugins/getting_started/public/lib/add_setup_work.js b/src/core_plugins/getting_started/public/lib/add_setup_work.js index ef6330e8370f9..c524572fc4e8d 100644 --- a/src/core_plugins/getting_started/public/lib/add_setup_work.js +++ b/src/core_plugins/getting_started/public/lib/add_setup_work.js @@ -4,6 +4,7 @@ import uiChrome from 'ui/chrome'; import { Notifier } from 'ui/notify/notifier'; import { IndexPatternsGetIdsProvider } from 'ui/index_patterns/_get_ids'; import { hasOptedOutOfGettingStarted, optOutOfGettingStarted } from 'ui/getting_started/opt_out_helpers'; +import { KbnUrlProvider } from 'ui/url'; import { GETTING_STARTED_ROUTE, From fbe8c607562970c583ec18d73b5d9ba2f907b4be Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 19 May 2017 05:08:44 -0700 Subject: [PATCH 16/17] Fixing import --- src/core_plugins/getting_started/public/lib/add_setup_work.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core_plugins/getting_started/public/lib/add_setup_work.js b/src/core_plugins/getting_started/public/lib/add_setup_work.js index c524572fc4e8d..3c9bf39cbcfde 100644 --- a/src/core_plugins/getting_started/public/lib/add_setup_work.js +++ b/src/core_plugins/getting_started/public/lib/add_setup_work.js @@ -3,8 +3,8 @@ 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 { KbnUrlProvider } from 'ui/url'; import { GETTING_STARTED_ROUTE, From 3b418b7d345fec606ed16043bf359c494ef8baa5 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Mon, 22 May 2017 08:40:31 -0700 Subject: [PATCH 17/17] Catching expected error --- .../public/lib/__tests__/add_setup_work.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js b/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js index 716ed75b0e0c9..bf526140db112 100644 --- a/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js +++ b/src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js @@ -3,7 +3,7 @@ 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, @@ -130,7 +130,11 @@ describe('Getting Started page', () => { }); it('redirects the user to the Create Index Pattern page', async () => { - await gettingStartedGateCheck(getIds, kbnUrl, config, $route); + 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); }); }); @@ -149,7 +153,11 @@ describe('Getting Started page', () => { }); it('redirects the user to the Getting Started page', async () => { - await gettingStartedGateCheck(getIds, kbnUrl, config, $route); + 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); }); });