github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-23 17:28:26 -04:00
Code Issues Projects Releases Packages Wiki Activity

Prevent flicker on Getting Started page (#11826)

Browse source 
* 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
This commit is contained in:
Shaunak Kashyap 2017-05-23 11:53:32 -07:00 • committed by GitHub
parent 66c6db03bd
commit c4b3ade27d
5 changed files with 272 additions and 71 deletions
Download patch file Download diff file Expand all files Collapse all files

7
src/core_plugins/getting_started/public/components/getting_started/getting_started.js
View file

@ -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);

179
src/core_plugins/getting_started/public/lib/__tests__/add_setup_work.js Normal file
View file

@ -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);
});
});
});
});
});
});
});

142
src/core_plugins/getting_started/public/lib/add_setup_work.js
View file

@ -3,6 +3,7 @@ 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 {
@ -10,71 +11,94 @@ import {
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');
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);
const currentRoute = get($route, 'current.$$route');
// The user need not see the Getting Started page, so opt them out of it
optOutOfGettingStarted();
return getIds()
.then(indexPatterns => {
const indexPatternsExist = Array.isArray(indexPatterns) && indexPatterns.length > 0;
const isOnGettingStartedPage = get(currentRoute, 'originalPath') === GETTING_STARTED_ROUTE;
// 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 (indexPatternsExist) {
// Otherwise, check if we have a default index pattern
let defaultIndexPattern = config.get('defaultIndex');
// The user need not see the Getting Started page, so opt them out of it
optOutOfGettingStarted();
// 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);
}
// 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;
}
// At this point, we have a default index pattern and are all set!
return;
}
// Otherwise, check if we have a default index pattern
let defaultIndexPattern = config.get('defaultIndex');
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);
// 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);
}
// 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;
}
// 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;
}
});
// 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);
});

11
src/ui/public/getting_started/opt_out_helpers.js
View file

@ -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);
}

4
src/ui/public/notify/notifier.js
View file

@ -230,8 +230,8 @@ Notifier.config = {
errorLifetime: 300000,
warningLifetime: 10000,
infoLifetime: 5000,
setInterval: window.setInterval,
clearInterval: window.clearInterval
setInterval: window.setInterval.bind(window),
clearInterval: window.clearInterval.bind(window)
};
Notifier.applyConfig = function (config) {