From 1aae341bc64d4ae3b65c9ab1e0530d9cd4b134f3 Mon Sep 17 00:00:00 2001 From: Jim Unger Date: Thu, 1 Sep 2016 09:43:48 -0500 Subject: [PATCH 1/8] adds state_monitor Former-commit-id: f592318cde7b36aca2eb7a5510e6084eaff23b35 --- .../__tests__/state_monitor.js | 214 ++++++++++++++++++ .../public/state_management/state_monitor.js | 85 +++++++ 2 files changed, 299 insertions(+) create mode 100644 src/ui/public/state_management/__tests__/state_monitor.js create mode 100644 src/ui/public/state_management/state_monitor.js diff --git a/src/ui/public/state_management/__tests__/state_monitor.js b/src/ui/public/state_management/__tests__/state_monitor.js new file mode 100644 index 000000000000..232aef49bde2 --- /dev/null +++ b/src/ui/public/state_management/__tests__/state_monitor.js @@ -0,0 +1,214 @@ +import expect from 'expect.js'; +import sinon from 'sinon'; +import { cloneDeep } from 'lodash'; +import stateMonitor from 'ui/state_management/state_monitor'; +import SimpleEmitter from 'ui/utils/SimpleEmitter'; + +describe('stateMonitor', function () { + const noop = () => {}; + const eventTypes = [ + 'save_with_changes', + 'reset_with_changes', + 'fetch_with_changes', + ]; + + let mockState; + let stateJSON; + + function setState(mockState, obj, emit = true) { + mockState.toJSON = () => cloneDeep(obj); + stateJSON = cloneDeep(obj); + if (emit) mockState.emit(eventTypes[0]); + } + + function createMockState(state = {}) { + const mockState = new SimpleEmitter(); + setState(mockState, state, false); + return mockState; + } + + beforeEach(() => { + mockState = createMockState({}); + }); + + it('should have a create method', function () { + expect(stateMonitor).to.have.property('create'); + expect(stateMonitor.create).to.be.a('function'); + }); + + describe('factory creation', function () { + it('should not call onChange with only the state', function () { + const monitor = stateMonitor.create(mockState); + const changeStub = sinon.stub(); + monitor.onChange(changeStub); + sinon.assert.notCalled(changeStub); + }); + + it('should not call onChange with matching defaultState', function () { + const monitor = stateMonitor.create(mockState, {}); + const changeStub = sinon.stub(); + monitor.onChange(changeStub); + sinon.assert.notCalled(changeStub); + }); + + it('should call onChange with differing defaultState', function () { + const monitor = stateMonitor.create(mockState, { test: true }); + const changeStub = sinon.stub(); + monitor.onChange(changeStub); + sinon.assert.calledOnce(changeStub); + }); + }); + + describe('instance', function () { + let monitor; + + beforeEach(() => { + monitor = stateMonitor.create(mockState); + }); + + describe('onChange', function () { + it('should throw if not given a handler function', function () { + const fn = () => monitor.onChange('not a function'); + expect(fn).to.throwException(/must be a function/); + }); + + eventTypes.forEach((eventType) => { + describe(`when ${eventType} is emitted`, function () { + let handlerFn; + + beforeEach(() => { + handlerFn = sinon.stub(); + monitor.onChange(handlerFn); + sinon.assert.notCalled(handlerFn); + }); + + it('should get called', function () { + mockState.emit(eventType); + sinon.assert.calledOnce(handlerFn); + }); + + it('should be given the state status', function () { + mockState.emit(eventType); + const args = handlerFn.firstCall.args; + expect(args[0]).to.be.an('object'); + }); + + it('should be given the event type', function () { + mockState.emit(eventType); + const args = handlerFn.firstCall.args; + expect(args[1]).to.equal(eventType); + }); + + it('should be given the changed keys', function () { + const keys = ['one', 'two', 'three']; + mockState.emit(eventType, keys); + const args = handlerFn.firstCall.args; + expect(args[2]).to.equal(keys); + }); + }); + }); + }); + + describe('ignoreProps', function () { + it('should not set status to dirty when ignored properties change', function () { + let status; + const mockState = createMockState({ messages: { world: 'hello', foo: 'bar' } }); + const monitor = stateMonitor.create(mockState); + const changeStub = sinon.stub(); + monitor.ignoreProps('messages.world'); + monitor.onChange(changeStub); + sinon.assert.notCalled(changeStub); + + // update the ignored state prop + setState(mockState, { messages: { world: 'howdy', foo: 'bar' } }); + sinon.assert.calledOnce(changeStub); + status = changeStub.firstCall.args[0]; + expect(status).to.have.property('clean', true); + expect(status).to.have.property('dirty', false); + + // update a prop that is not ignored + setState(mockState, { messages: { world: 'howdy', foo: 'baz' } }); + sinon.assert.calledTwice(changeStub); + status = changeStub.secondCall.args[0]; + expect(status).to.have.property('clean', false); + expect(status).to.have.property('dirty', true); + }); + }); + + describe('status object', function () { + let handlerFn; + + beforeEach(() => { + handlerFn = sinon.stub(); + monitor.onChange(handlerFn); + }); + + it('should be clean by default', function () { + mockState.emit(eventTypes[0]); + const status = handlerFn.firstCall.args[0]; + expect(status).to.have.property('clean', true); + expect(status).to.have.property('dirty', false); + }); + + it('should be dirty when state changes', function () { + setState(mockState, { message: 'i am dirty now' }); + const status = handlerFn.firstCall.args[0]; + expect(status).to.have.property('clean', false); + expect(status).to.have.property('dirty', true); + }); + + it('should be clean when state is reset', function () { + const defaultState = { message: 'i am the original state' }; + const handlerFn = sinon.stub(); + + let status; + + // initial state and monitor setup + const mockState = createMockState(defaultState); + const monitor = stateMonitor.create(mockState); + monitor.onChange(handlerFn); + sinon.assert.notCalled(handlerFn); + + // change the state and emit an event + setState(mockState, { message: 'i am dirty now' }); + sinon.assert.calledOnce(handlerFn); + status = handlerFn.firstCall.args[0]; + expect(status).to.have.property('clean', false); + expect(status).to.have.property('dirty', true); + + // reset the state and emit an event + setState(mockState, defaultState); + sinon.assert.calledTwice(handlerFn); + status = handlerFn.secondCall.args[0]; + expect(status).to.have.property('clean', true); + expect(status).to.have.property('dirty', false); + }); + }); + + describe('destroy', function () { + let stateSpy; + let cleanMethod; + + beforeEach(() => { + stateSpy = sinon.spy(mockState, 'off'); + sinon.assert.notCalled(stateSpy); + }); + + it('should remove the listeners', function () { + monitor.onChange(noop); + monitor.destroy(); + sinon.assert.callCount(stateSpy, eventTypes.length); + eventTypes.forEach((eventType) => { + sinon.assert.calledWith(stateSpy, eventType); + }); + }); + + it('should stop the instance from being used any more', function () { + monitor.onChange(noop); + monitor.destroy(); + const fn = () => monitor.onChange(noop); + expect(fn).to.throwException(/has been destroyed/); + }); + }); + }); +}); diff --git a/src/ui/public/state_management/state_monitor.js b/src/ui/public/state_management/state_monitor.js new file mode 100644 index 000000000000..f13b8d1eef71 --- /dev/null +++ b/src/ui/public/state_management/state_monitor.js @@ -0,0 +1,85 @@ +import { cloneDeep, isEqual, set } from 'lodash'; + +export default { + create: (state, defaultState) => stateMonitor(state, defaultState) +}; + +function stateMonitor(state, defaultState) { + let destroyed = false; + let ignoredProps = []; + let changeHandlers = []; + + const originalState = cloneDeep(defaultState) || cloneDeep(state.toJSON()); + + function filterState(state) { + ignoredProps.forEach(path => { + set(state, path, true); + }); + return state; + } + + function getStatus() { + const currentState = filterState(state.toJSON()); + const isClean = isEqual(currentState, originalState); + + return { + clean: isClean, + dirty: !isClean, + }; + } + + function dispatchChange(type = null, keys = []) { + const status = getStatus(); + changeHandlers.forEach(changeHandler => { + changeHandler(status, type, keys); + }); + } + + function dispatchFetch(keys) { + dispatchChange('fetch_with_changes', keys); + }; + + function dispatchSave(keys) { + dispatchChange('save_with_changes', keys); + }; + + function dispatchReset(keys) { + dispatchChange('reset_with_changes', keys); + }; + + return { + ignoreProps(props) { + ignoredProps = ignoredProps.concat(props); + filterState(originalState); + return this; + }, + + onChange(callback) { + if (destroyed) throw new Error('Monitor has been destroyed'); + if (typeof callback !== 'function') throw new Error('onChange handler must be a function'); + + changeHandlers.push(callback); + + // Listen for state events. + state.on('fetch_with_changes', dispatchFetch); + state.on('save_with_changes', dispatchSave); + state.on('reset_with_changes', dispatchReset); + + // if the state is already dirty, fire the change handler immediately + const status = getStatus(); + if (status.dirty) { + dispatchChange(); + } + + return this; + }, + + destroy() { + destroyed = true; + changeHandlers = undefined; + state.off('fetch_with_changes', dispatchFetch); + state.off('save_with_changes', dispatchSave); + state.off('reset_with_changes', dispatchReset); + } + }; +} From 384350e2897b2fd9a7e5b94363800a71ef7db81f Mon Sep 17 00:00:00 2001 From: Jim Unger Date: Thu, 1 Sep 2016 13:56:17 -0500 Subject: [PATCH 2/8] Adds state_monitor to discover Former-commit-id: 450294b3fcddc9188ec82f9f7ef297071cd9f849 --- .../public/discover/controllers/discover.js | 20 +++++++++++++++++-- .../kibana/public/discover/index.html | 2 +- .../__tests__/state_monitor.js | 2 +- .../public/state_management/state_monitor.js | 2 +- 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/core_plugins/kibana/public/discover/controllers/discover.js b/src/core_plugins/kibana/public/discover/controllers/discover.js index 053e13766eaf..21428e134184 100644 --- a/src/core_plugins/kibana/public/discover/controllers/discover.js +++ b/src/core_plugins/kibana/public/discover/controllers/discover.js @@ -23,6 +23,7 @@ import PluginsKibanaDiscoverHitSortFnProvider from 'plugins/kibana/discover/_hit import FilterBarQueryFilterProvider from 'ui/filter_bar/query_filter'; import FilterManagerProvider from 'ui/filter_manager'; import AggTypesBucketsIntervalOptionsProvider from 'ui/agg_types/buckets/_interval_options'; +import stateMonitor from 'ui/state_management/state_monitor'; import uiRoutes from 'ui/routes'; import uiModules from 'ui/modules'; import indexTemplate from 'plugins/kibana/discover/index.html'; @@ -74,7 +75,15 @@ uiRoutes } }); -app.controller('discover', function ($scope, config, courier, $route, $window, Notifier, +app + .directive('discoverApp', function () { + return { + controllerAs: 'discoverApp', + controller: discoverController, + }; + }); + +function discoverController($scope, config, courier, $route, $window, Notifier, AppState, timefilter, Promise, Private, kbnUrl, highlightTags) { const Vis = Private(VisProvider); @@ -131,6 +140,7 @@ app.controller('discover', function ($scope, config, courier, $route, $window, N docTitle.change(savedSearch.title); } + const $appStatus = $scope.appStatus = this.appStatus = {}; const $state = $scope.state = new AppState(getStateDefaults()); $scope.uiState = $state.makeStateful('uiState'); @@ -173,6 +183,12 @@ app.controller('discover', function ($scope, config, courier, $route, $window, N $scope.failuresShown = showTotal; }; + const monitor = stateMonitor.create($state, getStateDefaults()); + monitor.onChange((status) => { + $appStatus.dirty = status.dirty; + }); + $scope.$on('$destroy', () => monitor.destroy()); + $scope.updateDataSource() .then(function () { $scope.$listen(timefilter, 'fetch', function () { @@ -566,4 +582,4 @@ app.controller('discover', function ($scope, config, courier, $route, $window, N } init(); -}); +}; diff --git a/src/core_plugins/kibana/public/discover/index.html b/src/core_plugins/kibana/public/discover/index.html index ba66dabafa15..817324e4e040 100644 --- a/src/core_plugins/kibana/public/discover/index.html +++ b/src/core_plugins/kibana/public/discover/index.html @@ -1,4 +1,4 @@ -
+
diff --git a/src/ui/public/state_management/__tests__/state_monitor.js b/src/ui/public/state_management/__tests__/state_monitor.js index 232aef49bde2..96e443496895 100644 --- a/src/ui/public/state_management/__tests__/state_monitor.js +++ b/src/ui/public/state_management/__tests__/state_monitor.js @@ -2,7 +2,7 @@ import expect from 'expect.js'; import sinon from 'sinon'; import { cloneDeep } from 'lodash'; import stateMonitor from 'ui/state_management/state_monitor'; -import SimpleEmitter from 'ui/utils/SimpleEmitter'; +import SimpleEmitter from 'ui/utils/simple_emitter'; describe('stateMonitor', function () { const noop = () => {}; diff --git a/src/ui/public/state_management/state_monitor.js b/src/ui/public/state_management/state_monitor.js index f13b8d1eef71..307badc10a7c 100644 --- a/src/ui/public/state_management/state_monitor.js +++ b/src/ui/public/state_management/state_monitor.js @@ -19,7 +19,7 @@ function stateMonitor(state, defaultState) { } function getStatus() { - const currentState = filterState(state.toJSON()); + const currentState = filterState(cloneDeep(state.toJSON())); const isClean = isEqual(currentState, originalState); return { From 65130c37dc7d636ba3783f55ab4198baab751402 Mon Sep 17 00:00:00 2001 From: Jim Unger Date: Thu, 1 Sep 2016 15:13:47 -0500 Subject: [PATCH 3/8] Adds state monitor to dashboard app Former-commit-id: 7dbc147f83f9c6857bc7640e2153ff8d755cb87f --- src/core_plugins/kibana/public/dashboard/index.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/core_plugins/kibana/public/dashboard/index.js b/src/core_plugins/kibana/public/dashboard/index.js index 5016556de0a5..9ce6ace82220 100644 --- a/src/core_plugins/kibana/public/dashboard/index.js +++ b/src/core_plugins/kibana/public/dashboard/index.js @@ -13,6 +13,7 @@ import 'plugins/kibana/dashboard/services/saved_dashboards'; import 'plugins/kibana/dashboard/styles/main.less'; import FilterBarQueryFilterProvider from 'ui/filter_bar/query_filter'; import DocTitleProvider from 'ui/doc_title'; +import stateMonitor from 'ui/state_management/state_monitor'; import uiRoutes from 'ui/routes'; import uiModules from 'ui/modules'; import indexTemplate from 'plugins/kibana/dashboard/index.html'; @@ -54,6 +55,7 @@ uiRoutes app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, kbnUrl) { return { + controllerAs: 'dashboardApp', controller: function ($scope, $rootScope, $route, $routeParams, $location, Private, getAppState) { const queryFilter = Private(FilterBarQueryFilterProvider); @@ -94,6 +96,7 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, const $state = $scope.state = new AppState(stateDefaults); const $uiState = $scope.uiState = $state.makeStateful('uiState'); + const $appStatus = $scope.appStatus = this.appStatus = {}; $scope.$watchCollection('state.options', function (newVal, oldVal) { if (!angular.equals(newVal, oldVal)) $state.save(); @@ -143,6 +146,14 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, } initPanelIndices(); + + // watch for state changes and update the appStatus.dirty value + const monitor = stateMonitor.create($state, stateDefaults); + monitor.onChange((status) => { + $appStatus.dirty = status.dirty; + }); + $scope.$on('$destroy', () => monitor.destroy()); + $scope.$emit('application.load'); } From 3ee3bf0f6e3440fef6fdbf3da8c50f519169c569 Mon Sep 17 00:00:00 2001 From: Jim Unger Date: Thu, 1 Sep 2016 15:14:44 -0500 Subject: [PATCH 4/8] add state manager to visualize app Former-commit-id: da391f392c93e92d86079f6daeec707ccae0bddb --- .../public/visualize/editor/editor.html | 3 +- .../kibana/public/visualize/editor/editor.js | 39 +++++++++++++------ 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/core_plugins/kibana/public/visualize/editor/editor.html b/src/core_plugins/kibana/public/visualize/editor/editor.html index 58b0b08f5e73..581827e89e05 100644 --- a/src/core_plugins/kibana/public/visualize/editor/editor.html +++ b/src/core_plugins/kibana/public/visualize/editor/editor.html @@ -1,4 +1,4 @@ -
+
@@ -91,5 +91,4 @@
-
diff --git a/src/core_plugins/kibana/public/visualize/editor/editor.js b/src/core_plugins/kibana/public/visualize/editor/editor.js index 8eca1caf8882..9bcd066a1993 100644 --- a/src/core_plugins/kibana/public/visualize/editor/editor.js +++ b/src/core_plugins/kibana/public/visualize/editor/editor.js @@ -12,6 +12,7 @@ import DocTitleProvider from 'ui/doc_title'; import UtilsBrushEventProvider from 'ui/utils/brush_event'; import FilterBarQueryFilterProvider from 'ui/filter_bar/query_filter'; import FilterBarFilterBarClickHandlerProvider from 'ui/filter_bar/filter_bar_click_handler'; +import stateMonitor from 'ui/state_management/state_monitor'; import uiRoutes from 'ui/routes'; import uiModules from 'ui/modules'; import editorTemplate from 'plugins/kibana/visualize/editor/editor.html'; @@ -55,8 +56,14 @@ uiModules 'kibana/notify', 'kibana/courier' ]) -.controller('VisEditor', function ($scope, $route, timefilter, AppState, $location, kbnUrl, $timeout, courier, Private, Promise) { +.directive('visualizeApp', function () { + return { + controllerAs: 'visualizeApp', + controller: VisEditor, + }; +}); +function VisEditor($scope, $route, timefilter, AppState, $location, kbnUrl, $timeout, courier, Private, Promise) { const docTitle = Private(DocTitleProvider); const brushEvent = Private(UtilsBrushEventProvider); const queryFilter = Private(FilterBarQueryFilterProvider); @@ -66,6 +73,8 @@ uiModules location: 'Visualization Editor' }); + const $appStatus = this.appStatus = {}; + const savedVis = $route.current.locals.savedVis; const vis = savedVis.vis; @@ -104,16 +113,16 @@ uiModules docTitle.change(savedVis.title); } - let $state = $scope.$state = (function initState() { - const savedVisState = vis.getState(); - const stateDefaults = { - uiState: savedVis.uiStateJSON ? JSON.parse(savedVis.uiStateJSON) : {}, - linked: !!savedVis.savedSearchId, - query: searchSource.getOwn('query') || {query_string: {query: '*'}}, - filters: searchSource.getOwn('filter') || [], - vis: savedVisState - }; + const savedVisState = vis.getState(); + const stateDefaults = { + uiState: savedVis.uiStateJSON ? JSON.parse(savedVis.uiStateJSON) : {}, + linked: !!savedVis.savedSearchId, + query: searchSource.getOwn('query') || {query_string: {query: '*'}}, + filters: searchSource.getOwn('filter') || [], + vis: savedVisState + }; + let $state = $scope.$state = (function initState() { $state = new AppState(stateDefaults); if (!angular.equals($state.vis, savedVisState)) { @@ -138,10 +147,18 @@ uiModules $scope.editableVis = editableVis; $scope.state = $state; $scope.uiState = $state.makeStateful('uiState'); + $scope.appStatus = $appStatus; + vis.setUiState($scope.uiState); $scope.timefilter = timefilter; $scope.opts = _.pick($scope, 'doSave', 'savedVis', 'shareData', 'timefilter'); + const monitor = stateMonitor.create($state, stateDefaults); + monitor.ignoreProps([ 'vis.listeners' ]).onChange((status) => { + $appStatus.dirty = status.dirty; + }); + $scope.$on('$destroy', () => monitor.destroy()); + editableVis.listeners.click = vis.listeners.click = filterBarClickHandler($state); editableVis.listeners.brush = vis.listeners.brush = brushEvent; @@ -316,4 +333,4 @@ uiModules } init(); -}); +}; From 6602b1b5e2d195dcf11943948b5f20922cfb70ba Mon Sep 17 00:00:00 2001 From: Jim Unger Date: Tue, 6 Sep 2016 12:38:27 -0500 Subject: [PATCH 5/8] Applies changes from PR #8082 Former-commit-id: adf0f48b3547fffc8ebc540bcd20fc107d88aa20 --- .../kibana/public/dashboard/index.js | 10 +++-- .../public/discover/controllers/discover.js | 10 +++-- .../kibana/public/visualize/editor/editor.js | 10 +++-- ...te_monitor.js => state_monitor_factory.js} | 43 ++++++++++++++++++- ...te_monitor.js => state_monitor_factory.js} | 40 ++++++++++++----- 5 files changed, 88 insertions(+), 25 deletions(-) rename src/ui/public/state_management/__tests__/{state_monitor.js => state_monitor_factory.js} (82%) rename src/ui/public/state_management/{state_monitor.js => state_monitor_factory.js} (57%) diff --git a/src/core_plugins/kibana/public/dashboard/index.js b/src/core_plugins/kibana/public/dashboard/index.js index 9ce6ace82220..d843a3bb3d11 100644 --- a/src/core_plugins/kibana/public/dashboard/index.js +++ b/src/core_plugins/kibana/public/dashboard/index.js @@ -13,7 +13,7 @@ import 'plugins/kibana/dashboard/services/saved_dashboards'; import 'plugins/kibana/dashboard/styles/main.less'; import FilterBarQueryFilterProvider from 'ui/filter_bar/query_filter'; import DocTitleProvider from 'ui/doc_title'; -import stateMonitor from 'ui/state_management/state_monitor'; +import stateMonitorFactory from 'ui/state_management/state_monitor_factory'; import uiRoutes from 'ui/routes'; import uiModules from 'ui/modules'; import indexTemplate from 'plugins/kibana/dashboard/index.html'; @@ -94,6 +94,7 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, filters: _.reject(dash.searchSource.getOwn('filter'), matchQueryFilter), }; + let stateMonitor; const $state = $scope.state = new AppState(stateDefaults); const $uiState = $scope.uiState = $state.makeStateful('uiState'); const $appStatus = $scope.appStatus = this.appStatus = {}; @@ -148,11 +149,11 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, initPanelIndices(); // watch for state changes and update the appStatus.dirty value - const monitor = stateMonitor.create($state, stateDefaults); - monitor.onChange((status) => { + stateMonitor = stateMonitorFactory.create($state, stateDefaults); + stateMonitor.onChange((status) => { $appStatus.dirty = status.dirty; }); - $scope.$on('$destroy', () => monitor.destroy()); + $scope.$on('$destroy', () => stateMonitor.destroy()); $scope.$emit('application.load'); } @@ -227,6 +228,7 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, dash.save() .then(function (id) { + stateMonitor.setInitialState($state.toJSON()); $scope.kbnTopNav.close('save'); if (id) { notify.info('Saved Dashboard as "' + dash.title + '"'); diff --git a/src/core_plugins/kibana/public/discover/controllers/discover.js b/src/core_plugins/kibana/public/discover/controllers/discover.js index 21428e134184..a49348722c37 100644 --- a/src/core_plugins/kibana/public/discover/controllers/discover.js +++ b/src/core_plugins/kibana/public/discover/controllers/discover.js @@ -23,7 +23,7 @@ import PluginsKibanaDiscoverHitSortFnProvider from 'plugins/kibana/discover/_hit import FilterBarQueryFilterProvider from 'ui/filter_bar/query_filter'; import FilterManagerProvider from 'ui/filter_manager'; import AggTypesBucketsIntervalOptionsProvider from 'ui/agg_types/buckets/_interval_options'; -import stateMonitor from 'ui/state_management/state_monitor'; +import stateMonitorFactory from 'ui/state_management/state_monitor_factory'; import uiRoutes from 'ui/routes'; import uiModules from 'ui/modules'; import indexTemplate from 'plugins/kibana/discover/index.html'; @@ -140,6 +140,7 @@ function discoverController($scope, config, courier, $route, $window, Notifier, docTitle.change(savedSearch.title); } + let stateMonitor; const $appStatus = $scope.appStatus = this.appStatus = {}; const $state = $scope.state = new AppState(getStateDefaults()); $scope.uiState = $state.makeStateful('uiState'); @@ -183,11 +184,11 @@ function discoverController($scope, config, courier, $route, $window, Notifier, $scope.failuresShown = showTotal; }; - const monitor = stateMonitor.create($state, getStateDefaults()); - monitor.onChange((status) => { + stateMonitor = stateMonitorFactory.create($state, getStateDefaults()); + stateMonitor.onChange((status) => { $appStatus.dirty = status.dirty; }); - $scope.$on('$destroy', () => monitor.destroy()); + $scope.$on('$destroy', () => stateMonitor.destroy()); $scope.updateDataSource() .then(function () { @@ -314,6 +315,7 @@ function discoverController($scope, config, courier, $route, $window, Notifier, return savedSearch.save() .then(function (id) { + stateMonitor.setInitialState($state.toJSON()); $scope.kbnTopNav.close('save'); if (id) { diff --git a/src/core_plugins/kibana/public/visualize/editor/editor.js b/src/core_plugins/kibana/public/visualize/editor/editor.js index 9bcd066a1993..a1b5cbf2a01b 100644 --- a/src/core_plugins/kibana/public/visualize/editor/editor.js +++ b/src/core_plugins/kibana/public/visualize/editor/editor.js @@ -12,7 +12,7 @@ import DocTitleProvider from 'ui/doc_title'; import UtilsBrushEventProvider from 'ui/utils/brush_event'; import FilterBarQueryFilterProvider from 'ui/filter_bar/query_filter'; import FilterBarFilterBarClickHandlerProvider from 'ui/filter_bar/filter_bar_click_handler'; -import stateMonitor from 'ui/state_management/state_monitor'; +import stateMonitorFactory from 'ui/state_management/state_monitor_factory'; import uiRoutes from 'ui/routes'; import uiModules from 'ui/modules'; import editorTemplate from 'plugins/kibana/visualize/editor/editor.html'; @@ -73,6 +73,7 @@ function VisEditor($scope, $route, timefilter, AppState, $location, kbnUrl, $tim location: 'Visualization Editor' }); + let stateMonitor; const $appStatus = this.appStatus = {}; const savedVis = $route.current.locals.savedVis; @@ -153,11 +154,11 @@ function VisEditor($scope, $route, timefilter, AppState, $location, kbnUrl, $tim $scope.timefilter = timefilter; $scope.opts = _.pick($scope, 'doSave', 'savedVis', 'shareData', 'timefilter'); - const monitor = stateMonitor.create($state, stateDefaults); - monitor.ignoreProps([ 'vis.listeners' ]).onChange((status) => { + stateMonitor = stateMonitorFactory.create($state, stateDefaults); + stateMonitor.ignoreProps([ 'vis.listeners' ]).onChange((status) => { $appStatus.dirty = status.dirty; }); - $scope.$on('$destroy', () => monitor.destroy()); + $scope.$on('$destroy', () => stateMonitor.destroy()); editableVis.listeners.click = vis.listeners.click = filterBarClickHandler($state); editableVis.listeners.brush = vis.listeners.brush = brushEvent; @@ -265,6 +266,7 @@ function VisEditor($scope, $route, timefilter, AppState, $location, kbnUrl, $tim savedVis.save() .then(function (id) { + stateMonitor.setInitialState($state.toJSON()); $scope.kbnTopNav.close('save'); if (id) { diff --git a/src/ui/public/state_management/__tests__/state_monitor.js b/src/ui/public/state_management/__tests__/state_monitor_factory.js similarity index 82% rename from src/ui/public/state_management/__tests__/state_monitor.js rename to src/ui/public/state_management/__tests__/state_monitor_factory.js index 96e443496895..94b187377924 100644 --- a/src/ui/public/state_management/__tests__/state_monitor.js +++ b/src/ui/public/state_management/__tests__/state_monitor_factory.js @@ -1,10 +1,10 @@ import expect from 'expect.js'; import sinon from 'sinon'; import { cloneDeep } from 'lodash'; -import stateMonitor from 'ui/state_management/state_monitor'; +import stateMonitor from 'ui/state_management/state_monitor_factory'; import SimpleEmitter from 'ui/utils/simple_emitter'; -describe('stateMonitor', function () { +describe('stateMonitorFactory', function () { const noop = () => {}; const eventTypes = [ 'save_with_changes', @@ -135,6 +135,45 @@ describe('stateMonitor', function () { }); }); + describe('setInitialState', function () { + let changeStub; + + beforeEach(() => { + changeStub = sinon.stub(); + monitor.onChange(changeStub); + sinon.assert.notCalled(changeStub); + }); + + it('should throw if no state is provided', function () { + const fn = () => monitor.setInitialState(); + expect(fn).to.throwException(/must be an object/); + }); + + it('should throw if given the wrong type', function () { + const fn = () => monitor.setInitialState([]); + expect(fn).to.throwException(/must be an object/); + }); + + it('should trigger the onChange handler', function () { + monitor.setInitialState({ new: 'state' }); + sinon.assert.calledOnce(changeStub); + }); + + it('should change the status with differing state', function () { + monitor.setInitialState({ new: 'state' }); + sinon.assert.calledOnce(changeStub); + + const status = changeStub.firstCall.args[0]; + expect(status).to.have.property('clean', false); + expect(status).to.have.property('dirty', true); + }); + + it('should not trigger the onChange handler without state change', function () { + monitor.setInitialState(cloneDeep(mockState.toJSON())); + sinon.assert.notCalled(changeStub); + }); + }); + describe('status object', function () { let handlerFn; diff --git a/src/ui/public/state_management/state_monitor.js b/src/ui/public/state_management/state_monitor_factory.js similarity index 57% rename from src/ui/public/state_management/state_monitor.js rename to src/ui/public/state_management/state_monitor_factory.js index 307badc10a7c..e37aeda2824e 100644 --- a/src/ui/public/state_management/state_monitor.js +++ b/src/ui/public/state_management/state_monitor_factory.js @@ -1,17 +1,23 @@ -import { cloneDeep, isEqual, set } from 'lodash'; +import { cloneDeep, isEqual, set, isPlainObject } from 'lodash'; export default { - create: (state, defaultState) => stateMonitor(state, defaultState) + create: (state, customInitialState) => stateMonitor(state, customInitialState) }; -function stateMonitor(state, defaultState) { +function stateMonitor(state, customInitialState) { let destroyed = false; let ignoredProps = []; let changeHandlers = []; + let initialState; - const originalState = cloneDeep(defaultState) || cloneDeep(state.toJSON()); + setInitialState(customInitialState); - function filterState(state) { + function setInitialState(customInitialState) { + // state.toJSON returns a reference, clone so we can mutate it safely + initialState = cloneDeep(customInitialState) || cloneDeep(state.toJSON()); + } + + function removeIgnoredProps(state) { ignoredProps.forEach(path => { set(state, path, true); }); @@ -19,8 +25,8 @@ function stateMonitor(state, defaultState) { } function getStatus() { - const currentState = filterState(cloneDeep(state.toJSON())); - const isClean = isEqual(currentState, originalState); + const currentState = removeIgnoredProps(cloneDeep(state.toJSON())); + const isClean = isEqual(currentState, initialState); return { clean: isClean, @@ -48,9 +54,23 @@ function stateMonitor(state, defaultState) { }; return { + setInitialState(customInitialState) { + if (!isPlainObject(customInitialState)) throw new TypeError('The default state must be an object'); + + // check the current status + const previousStatus = getStatus(); + + // update the initialState and apply ignoredProps + setInitialState(customInitialState); + removeIgnoredProps(initialState); + + // fire the change handler if the status has changed + if (!isEqual(previousStatus, getStatus())) dispatchChange(); + }, + ignoreProps(props) { ignoredProps = ignoredProps.concat(props); - filterState(originalState); + removeIgnoredProps(initialState); return this; }, @@ -67,9 +87,7 @@ function stateMonitor(state, defaultState) { // if the state is already dirty, fire the change handler immediately const status = getStatus(); - if (status.dirty) { - dispatchChange(); - } + if (status.dirty) dispatchChange(); return this; }, From c43d3b453a498af4c1f1697a8f0194c3eb05624e Mon Sep 17 00:00:00 2001 From: Jim Unger Date: Wed, 7 Sep 2016 11:20:28 -0500 Subject: [PATCH 6/8] added comment and fixed indent issue Former-commit-id: 258a12e6a5dbf59409026b0cb0f4e24a3a395054 --- .../kibana/public/discover/controllers/discover.js | 13 ++++++------- .../state_management/state_monitor_factory.js | 1 + 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/core_plugins/kibana/public/discover/controllers/discover.js b/src/core_plugins/kibana/public/discover/controllers/discover.js index a49348722c37..62ebb265493f 100644 --- a/src/core_plugins/kibana/public/discover/controllers/discover.js +++ b/src/core_plugins/kibana/public/discover/controllers/discover.js @@ -75,13 +75,12 @@ uiRoutes } }); -app - .directive('discoverApp', function () { - return { - controllerAs: 'discoverApp', - controller: discoverController, - }; - }); +app.directive('discoverApp', function () { + return { + controllerAs: 'discoverApp', + controller: discoverController, + }; +}); function discoverController($scope, config, courier, $route, $window, Notifier, AppState, timefilter, Promise, Private, kbnUrl, highlightTags) { diff --git a/src/ui/public/state_management/state_monitor_factory.js b/src/ui/public/state_management/state_monitor_factory.js index e37aeda2824e..6fe7d89f0858 100644 --- a/src/ui/public/state_management/state_monitor_factory.js +++ b/src/ui/public/state_management/state_monitor_factory.js @@ -25,6 +25,7 @@ function stateMonitor(state, customInitialState) { } function getStatus() { + // state.toJSON returns a reference, clone so we can mutate it safely const currentState = removeIgnoredProps(cloneDeep(state.toJSON())); const isClean = isEqual(currentState, initialState); From 86889c2a8ba540af62f20f198ef4d6c07e9e6476 Mon Sep 17 00:00:00 2001 From: Jim Unger Date: Wed, 7 Sep 2016 11:25:38 -0500 Subject: [PATCH 7/8] Changed root node type of discover, dashboard, and visualization apps Former-commit-id: 860b7d7413e38684a87d34e2b1e4f59f9b0412d6 --- src/core_plugins/kibana/public/dashboard/index.html | 4 ++-- src/core_plugins/kibana/public/discover/index.html | 4 ++-- src/core_plugins/kibana/public/visualize/editor/editor.html | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/core_plugins/kibana/public/dashboard/index.html b/src/core_plugins/kibana/public/dashboard/index.html index 8bc0f088aecf..bbcb5812cd06 100644 --- a/src/core_plugins/kibana/public/dashboard/index.html +++ b/src/core_plugins/kibana/public/dashboard/index.html @@ -1,4 +1,4 @@ -
+
@@ -46,4 +46,4 @@

Ready to get started?

-
+ diff --git a/src/core_plugins/kibana/public/discover/index.html b/src/core_plugins/kibana/public/discover/index.html index 817324e4e040..3a89231617bf 100644 --- a/src/core_plugins/kibana/public/discover/index.html +++ b/src/core_plugins/kibana/public/discover/index.html @@ -1,4 +1,4 @@ -
+
@@ -126,4 +126,4 @@

Searching

-
+ diff --git a/src/core_plugins/kibana/public/visualize/editor/editor.html b/src/core_plugins/kibana/public/visualize/editor/editor.html index 581827e89e05..c43967427a8b 100644 --- a/src/core_plugins/kibana/public/visualize/editor/editor.html +++ b/src/core_plugins/kibana/public/visualize/editor/editor.html @@ -1,4 +1,4 @@ -
+
@@ -91,4 +91,4 @@
-
+ From 2a8e1f7b55e8e41b953aaa6c4a056b2ed4f88721 Mon Sep 17 00:00:00 2001 From: Jim Unger Date: Wed, 7 Sep 2016 13:58:14 -0500 Subject: [PATCH 8/8] Adds restrict=E property to the directive definitions of dashboard, discover, visualize Former-commit-id: 4e279c03f06877fd4ff34aef7905354dcb1b8bf2 --- src/core_plugins/kibana/public/dashboard/index.js | 1 + .../kibana/public/discover/controllers/discover.js | 3 ++- src/core_plugins/kibana/public/visualize/editor/editor.js | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core_plugins/kibana/public/dashboard/index.js b/src/core_plugins/kibana/public/dashboard/index.js index d843a3bb3d11..e928e00c3c16 100644 --- a/src/core_plugins/kibana/public/dashboard/index.js +++ b/src/core_plugins/kibana/public/dashboard/index.js @@ -55,6 +55,7 @@ uiRoutes app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, kbnUrl) { return { + restrict: 'E', controllerAs: 'dashboardApp', controller: function ($scope, $rootScope, $route, $routeParams, $location, Private, getAppState) { diff --git a/src/core_plugins/kibana/public/discover/controllers/discover.js b/src/core_plugins/kibana/public/discover/controllers/discover.js index 62ebb265493f..19acb15ea009 100644 --- a/src/core_plugins/kibana/public/discover/controllers/discover.js +++ b/src/core_plugins/kibana/public/discover/controllers/discover.js @@ -77,8 +77,9 @@ uiRoutes app.directive('discoverApp', function () { return { + restrict: 'E', controllerAs: 'discoverApp', - controller: discoverController, + controller: discoverController }; }); diff --git a/src/core_plugins/kibana/public/visualize/editor/editor.js b/src/core_plugins/kibana/public/visualize/editor/editor.js index a1b5cbf2a01b..548aebe4684b 100644 --- a/src/core_plugins/kibana/public/visualize/editor/editor.js +++ b/src/core_plugins/kibana/public/visualize/editor/editor.js @@ -58,6 +58,7 @@ uiModules ]) .directive('visualizeApp', function () { return { + restrict: 'E', controllerAs: 'visualizeApp', controller: VisEditor, };