From a2bcb06fab6ca07f55d096e7c24675da24f3d348 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 10 Feb 2020 09:34:41 +0100 Subject: [PATCH 01/53] Remove AppState --- .../discover/np_ready/angular/discover.js | 134 ++++++++++++------ .../np_ready/angular/discover_state.ts | 49 +++++++ .../angular/doc_table/actions/columns.ts | 29 ++-- .../np_ready/angular/doc_table/doc_table.ts | 17 --- .../components/field_chooser/field_chooser.js | 8 +- 5 files changed, 148 insertions(+), 89 deletions(-) create mode 100644 src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index 69f69d449354c..c5b775217ca5b 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -25,6 +25,7 @@ import moment from 'moment'; import dateMath from '@elastic/datemath'; import { i18n } from '@kbn/i18n'; import '../components/field_chooser/field_chooser'; +import { getAppState as getAppStateNew } from './discover_state'; import { RequestAdapter } from '../../../../../../../plugins/inspector/public'; import { @@ -53,7 +54,6 @@ import { intervalOptions, migrateLegacyQuery, unhashUrl, - stateMonitorFactory, subscribeWithScope, tabifyAggResponse, getAngularModule, @@ -119,7 +119,7 @@ app.config($routeProvider => { template: indexTemplate, reloadOnSearch: false, resolve: { - savedObjects: function(redirectWhenMissing, $route, kbnUrl, Promise, $rootScope, State) { + savedObjects: function(redirectWhenMissing, $route, kbnUrl, Promise, $rootScope) { const indexPatterns = getServices().indexPatterns; const savedSearchId = $route.current.params.id; return ensureDefaultIndexPattern(core, getServices().data, $rootScope, kbnUrl).then(() => { @@ -134,18 +134,16 @@ app.config($routeProvider => { * * @type {State} */ - const state = new State('_a', {}); - const id = getIndexPatternId( - state.index, - indexPatternList, - uiSettings.get('defaultIndex') - ); - state.destroy(); + const { initialState, start, stop } = getAppStateNew({}); + start(); + const { index } = initialState; + const id = getIndexPatternId(index, indexPatternList, uiSettings.get('defaultIndex')); + stop(); return Promise.props({ list: indexPatternList, loaded: indexPatterns.get(id), - stateVal: state.index, - stateValFound: !!state.index && id === state.index, + stateVal: index, + stateValFound: !!index && id === index, }); }), savedSearch: getServices() @@ -197,7 +195,57 @@ function discoverController( getAppState, globalState ) { - const filterStateManager = new FilterStateManager(globalState, getAppState, filterManager); + // the actual courier.SearchSource + // the saved savedSearch + const savedSearch = $route.current.locals.savedObjects.savedSearch; + $scope.searchSource = savedSearch.searchSource; + $scope.indexPattern = resolveIndexPatternLoading(); + const { stateContainer, start, stop } = getAppStateNew(getStateDefaults()); + stateContainer.subscribe(newState => { + if (!_.isEqual(newState, $scope.state)) { + $scope.state = { ...newState }; + $scope.$digest(); + } + }); + + const $state = { ...stateContainer.getState() }; + $scope.state = $state; + const syncState = partial => { + const newState = { ...$state, ...partial }; + if (!_.isEqual(stateContainer.getState(), newState)) { + stateContainer.set(newState); + } + }; + $scope.$watch('state.sort', sort => { + syncState({ sort }); + }); + $scope.$watch('state.index', (index, prevIndex) => { + syncState({ index }); + if (prevIndex == null || prevIndex === index) return; + $route.reload(); + }); + + $scope.$watchCollection('state.columns', function(val) { + if (!_.isEqual(val, stateContainer.get().columns)) { + stateContainer.set({ ...$state, ...{ columns: val } }); + } + }); + start(); + const filterStateManager = new FilterStateManager( + globalState, + () => { + return { + set filters(_filters) { + $scope.state.filters = _filters; + stateContainer.set({ ...$state, ...{ filters: _filters } }); + }, + get filters() { + return $scope.state.filters; + }, + }; + }, + filterManager + ); const inspectorAdapters = { requests: new RequestAdapter(), @@ -229,15 +277,13 @@ function discoverController( return interval.val !== 'custom'; }; - // the saved savedSearch - const savedSearch = $route.current.locals.savedObjects.savedSearch; - let abortController; $scope.$on('$destroy', () => { if (abortController) abortController.abort(); savedSearch.destroy(); subscriptions.unsubscribe(); filterStateManager.destroy(); + stop(); }); const $appStatus = ($scope.appStatus = this.appStatus = { @@ -384,10 +430,6 @@ function discoverController( $scope.topNavMenu = getTopNavLinks(); - // the actual courier.SearchSource - $scope.searchSource = savedSearch.searchSource; - $scope.indexPattern = resolveIndexPatternLoading(); - $scope.searchSource .setField('index', $scope.indexPattern) .setField('highlightAll', true) @@ -430,9 +472,6 @@ function discoverController( ]); } - let stateMonitor; - - const $state = ($scope.state = new AppState(getStateDefaults())); const $fetchObservable = new Subject(); $scope.screenTitle = savedSearch.title; @@ -506,9 +545,6 @@ function discoverController( indexPatternId: searchSource.getField('index').id, }; }; - - $scope.uiState = $state.makeStateful('uiState'); - function getStateDefaults() { return { query: ($scope.savedQuery && $scope.savedQuery.attributes.query) || @@ -549,10 +585,6 @@ function discoverController( }); }; - $scope.$watchCollection('state.columns', function() { - $state.save(); - }); - $scope.opts = { // number of records to fetch, then paginate through sampleSize: config.get('discover:sampleSize'), @@ -573,11 +605,11 @@ function discoverController( }; const init = _.once(function() { - stateMonitor = stateMonitorFactory.create($state, getStateDefaults()); - stateMonitor.onChange(status => { - $appStatus.dirty = status.dirty || !savedSearch.id; - }); - $scope.$on('$destroy', () => stateMonitor.destroy()); + //stateMonitor = stateMonitorFactory.create($state, getStateDefaults()); + // stateMonitor.onChange(status => { + // $appStatus.dirty = status.dirty || !savedSearch.id; + // }); + // $scope.$on('$destroy', () => stateMonitor.destroy()); $scope.updateDataSource().then(function() { const searchBarChanges = merge( @@ -616,16 +648,16 @@ function discoverController( subscribeWithScope($scope, filterManager.getUpdates$(), { next: () => { $scope.updateDataSource().then(function() { - $state.save(); + //$state.save(); }); }, }) ); // update data source when hitting forward/back and the query changes - $scope.$listen($state, 'fetch_with_changes', function(diff) { - if (diff.indexOf('query') >= 0) $fetchObservable.next(); - }); + //$scope.$listen($state, 'fetch_with_changes', function(diff) { + // if (diff.indexOf('query') >= 0) $fetchObservable.next(); + //}); $scope.$watch('opts.timefield', function(timefield) { $scope.enableTimeRangeSelector = !!timefield; @@ -709,7 +741,7 @@ function discoverController( } init.complete = true; - $state.replace(); + //$state.replace(); if (shouldSearchOnPageLoad()) { $fetchObservable.next(); @@ -726,7 +758,7 @@ function discoverController( try { const id = await savedSearch.save(saveOptions); $scope.$evalAsync(() => { - stateMonitor.setInitialState($state.toJSON()); + //stateMonitor.setInitialState($state.toJSON()); if (id) { toastNotifications.addSuccess({ title: i18n.translate('kbn.discover.notifications.savedSearchTitle', { @@ -778,7 +810,7 @@ function discoverController( .updateDataSource() .then(setupVisualization) .then(function() { - $state.save(); + //$state.save(); $scope.fetchStatus = fetchStatuses.LOADING; logInspectorRequest(); return $scope.searchSource.fetch({ @@ -807,6 +839,7 @@ function discoverController( $scope.updateQuery = function({ query }) { $state.query = query; + syncState({ query }); $fetchObservable.next(); }; @@ -911,18 +944,27 @@ function discoverController( return filterManager.addFilters(newFilters); }; + function getColumns(columns) { + if (columns.length > 1 && columns.indexOf('_source') !== -1) { + return columns.filter(col => col !== '_source'); + } else if (columns.length !== 0) { + return columns; + } + return ['_source']; + } + $scope.addColumn = function addColumn(columnName) { $scope.indexPattern.popularizeField(columnName, 1); - columnActions.addColumn($scope.state.columns, columnName); + $scope.state.columns = getColumns(columnActions.addColumn($scope.state.columns, columnName)); }; $scope.removeColumn = function removeColumn(columnName) { - $scope.indexPattern.popularizeField(columnName, 1); - columnActions.removeColumn($scope.state.columns, columnName); + // $scope.indexPattern.popularizeField(columnName, 1); + $scope.state.columns = getColumns(columnActions.removeColumn($scope.state.columns, columnName)); }; $scope.moveColumn = function moveColumn(columnName, newIndex) { - columnActions.moveColumn($scope.state.columns, columnName, newIndex); + $scope.state.columns = columnActions.moveColumn($scope.state.columns, columnName, newIndex); }; $scope.scrollToTop = function() { @@ -946,7 +988,7 @@ function discoverController( } else { delete $state.savedQuery; } - $state.save(); + syncState({ savedQuery: $state.savedQuery }); }; async function setupVisualization() { diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts new file mode 100644 index 0000000000000..e36994162a6dd --- /dev/null +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -0,0 +1,49 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import _ from 'lodash'; +import { createStateContainer } from '../../../../../../../plugins/kibana_utils/public'; +import { createKbnUrlStateStorage } from '../../../../../../../plugins/kibana_utils/public'; +import { syncState } from '../../../../../../../plugins/kibana_utils/public'; + +interface DiscoverAppState { + columns: string[][]; + filters: any; + index: string; + interval: string; + query: any; + sort: [string, string]; +} + +export function getAppState(defaultState: DiscoverAppState) { + const stateStorage = createKbnUrlStateStorage(); + const initialStateFromUrl = stateStorage.get('_a'); + const initialState = { + ...defaultState, + ...(_.cloneDeep(initialStateFromUrl) as DiscoverAppState), + } as DiscoverAppState; + + const stateContainer = createStateContainer(initialState) as any; + + const { start, stop } = syncState({ + storageKey: '_a', + stateContainer, + stateStorage, + }); + return { stateContainer, initialState, start, stop }; +} diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/actions/columns.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/actions/columns.ts index ec4fe8025a7c7..8a0c0bbb78ee7 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/actions/columns.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/actions/columns.ts @@ -19,33 +19,24 @@ export function addColumn(columns: string[], columnName: string) { if (columns.includes(columnName)) { - return; + return columns; } - - columns.push(columnName); + return [...columns, columnName]; } export function removeColumn(columns: string[], columnName: string) { if (!columns.includes(columnName)) { - return; + return columns; } - - columns.splice(columns.indexOf(columnName), 1); + return columns.filter(col => col !== columnName); } export function moveColumn(columns: string[], columnName: string, newIndex: number) { - if (newIndex < 0) { - return; - } - - if (newIndex >= columns.length) { - return; + if (newIndex < 0 || newIndex >= columns.length || !columns.includes(columnName)) { + return columns; } - - if (!columns.includes(columnName)) { - return; - } - - columns.splice(columns.indexOf(columnName), 1); // remove at old index - columns.splice(newIndex, 0, columnName); // insert before new index + const modifiedColumns = [...columns]; + modifiedColumns.splice(modifiedColumns.indexOf(columnName), 1); // remove at old index + modifiedColumns.splice(newIndex, 0, columnName); // insert before new index + return modifiedColumns; } diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts index 3329ffc7cd102..0d199e22a02fc 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts @@ -83,23 +83,6 @@ export function createDocTableDirective( $scope.limit += 50; }; - // This exists to fix the problem of an empty initial column list not playing nice with watchCollection. - $scope.$watch('columns', function(columns: string[]) { - if (columns.length !== 0) return; - - const $state = getAppState(); - $scope.columns.push('_source'); - if ($state) $state.replace(); - }); - - $scope.$watchCollection('columns', function(columns: string[], oldColumns: string[]) { - if (oldColumns.length === 1 && oldColumns[0] === '_source' && $scope.columns.length > 1) { - _.pull($scope.columns, '_source'); - } - - if ($scope.columns.length === 0) $scope.columns.push('_source'); - }); - $scope.$watch('hits', (hits: any) => { if (!hits) return; diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/components/field_chooser/field_chooser.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/components/field_chooser/field_chooser.js index 47b3ec6b07e8e..dee13ed790b4d 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/components/field_chooser/field_chooser.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/components/field_chooser/field_chooser.js @@ -26,7 +26,7 @@ import './discover_index_pattern_directive'; import { FieldList } from '../../../../../../../../plugins/data/public'; import fieldChooserTemplate from './field_chooser.html'; -export function createFieldChooserDirective($location, config, $route) { +export function createFieldChooserDirective($location, config) { return { restrict: 'E', scope: { @@ -51,14 +51,8 @@ export function createFieldChooserDirective($location, config, $route) { $scope.indexPatternList = _.sortBy($scope.indexPatternList, o => o.get('title')); $scope.setIndexPattern = function(id) { $scope.state.index = id; - $scope.state.save(); }; - $scope.$watch('state.index', function(id, previousId) { - if (previousId == null || previousId === id) return; - $route.reload(); - }); - const filter = ($scope.filter = { props: ['type', 'aggregatable', 'searchable', 'missing', 'name'], defaults: { From 30c03746d4611d52827b854e0e226ed8c382aa5a Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 10 Feb 2020 17:56:58 +0100 Subject: [PATCH 02/53] Fix sort --- .../kibana/public/discover/np_ready/angular/discover.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index c5b775217ca5b..6a9b856024a47 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -204,7 +204,6 @@ function discoverController( stateContainer.subscribe(newState => { if (!_.isEqual(newState, $scope.state)) { $scope.state = { ...newState }; - $scope.$digest(); } }); @@ -637,9 +636,11 @@ function discoverController( // get the current sort from searchSource as array of arrays const currentSort = getSortArray($scope.searchSource.getField('sort'), $scope.indexPattern); - // if the searchSource doesn't know, tell it so - if (!angular.equals(sort, currentSort)) $fetchObservable.next(); + if (!angular.equals(sort, currentSort)) { + syncState({ sort }); + $fetchObservable.next(); + } }); // update data source when filters update From 5081d16054b7a5b608ba6b167a36797971a17ce5 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Wed, 12 Feb 2020 06:36:45 +0100 Subject: [PATCH 03/53] Replace GlobalState --- .../discover/np_ready/angular/discover.js | 140 +++++++++--------- .../np_ready/angular/discover_state.ts | 108 +++++++++++--- 2 files changed, 153 insertions(+), 95 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index 6a9b856024a47..09f816e759f21 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -25,7 +25,7 @@ import moment from 'moment'; import dateMath from '@elastic/datemath'; import { i18n } from '@kbn/i18n'; import '../components/field_chooser/field_chooser'; -import { getAppState as getAppStateNew } from './discover_state'; +import { getState } from './discover_state'; import { RequestAdapter } from '../../../../../../../plugins/inspector/public'; import { @@ -58,7 +58,6 @@ import { tabifyAggResponse, getAngularModule, ensureDefaultIndexPattern, - registerTimefilterWithGlobalStateFactory, } from '../../kibana_services'; import { Vis } from '../../../../../visualizations/public'; @@ -79,7 +78,6 @@ import { indexPatterns as indexPatternsUtils, } from '../../../../../../../plugins/data/public'; import { getIndexPatternId } from '../helpers/get_index_pattern_id'; -import { FilterStateManager } from '../../../../../data/public'; const fetchStatuses = { UNINITIALIZED: 'uninitialized', @@ -88,9 +86,6 @@ const fetchStatuses = { }; const app = getAngularModule(); -app.run((globalState, $rootScope) => { - registerTimefilterWithGlobalStateFactory(timefilter, globalState, $rootScope); -}); app.config($routeProvider => { const defaults = { @@ -134,9 +129,9 @@ app.config($routeProvider => { * * @type {State} */ - const { initialState, start, stop } = getAppStateNew({}); + const { appStateContainer, start, stop } = getState({}); start(); - const { index } = initialState; + const { index } = appStateContainer.getState(); const id = getIndexPatternId(index, indexPatternList, uiSettings.get('defaultIndex')); stop(); return Promise.props({ @@ -186,72 +181,92 @@ function discoverController( $scope, $timeout, $window, - AppState, Promise, config, kbnUrl, localStorage, - uiCapabilities, - getAppState, - globalState + uiCapabilities ) { // the actual courier.SearchSource // the saved savedSearch + const subscriptions = new Subscription(); const savedSearch = $route.current.locals.savedObjects.savedSearch; $scope.searchSource = savedSearch.searchSource; $scope.indexPattern = resolveIndexPatternLoading(); - const { stateContainer, start, stop } = getAppStateNew(getStateDefaults()); - stateContainer.subscribe(newState => { + const $appStatus = ($scope.appStatus = { + dirty: !savedSearch.id, + }); + const onChangeAppState = dirty => { + $appStatus.dirty = dirty || !savedSearch.id; + }; + + const { + appStateContainer, + globalStateContainer, + start, + stop, + syncAppState, + syncGlobalState, + getAppFilters, + getGlobalFilters, + } = getState(getStateDefaults(), false, onChangeAppState); + + //stateMonitor = stateMonitorFactory.create($state, getStateDefaults()); + //stateMonitor.onChange(status => { + // $appStatus.dirty = status.dirty || !savedSearch.id; + //}); + appStateContainer.subscribe(newState => { if (!_.isEqual(newState, $scope.state)) { $scope.state = { ...newState }; } }); - const $state = { ...stateContainer.getState() }; - $scope.state = $state; - const syncState = partial => { - const newState = { ...$state, ...partial }; - if (!_.isEqual(stateContainer.getState(), newState)) { - stateContainer.set(newState); + globalStateContainer.subscribe(newState => { + if (newState.time && newState.time.from !== timefilter.from) { + timefilter.setTime(newState.time); } - }; - $scope.$watch('state.sort', sort => { - syncState({ sort }); }); + + const $state = _.cloneDeep(appStateContainer.getState()); + $scope.state = $state; + $scope.$watch('state.index', (index, prevIndex) => { - syncState({ index }); + syncAppState({ index }); if (prevIndex == null || prevIndex === index) return; $route.reload(); }); - $scope.$watchCollection('state.columns', function(val) { - if (!_.isEqual(val, stateContainer.get().columns)) { - stateContainer.set({ ...$state, ...{ columns: val } }); + $scope.$watchCollection('state.columns', val => { + if (!_.isEqual(val, appStateContainer.get().columns)) { + syncAppState({ columns: val }); } }); - start(); - const filterStateManager = new FilterStateManager( - globalState, - () => { - return { - set filters(_filters) { - $scope.state.filters = _filters; - stateContainer.set({ ...$state, ...{ filters: _filters } }); - }, - get filters() { - return $scope.state.filters; - }, - }; - }, - filterManager + // update data source when filters update + + subscriptions.add( + subscribeWithScope($scope, filterManager.getUpdates$(), { + next: () => { + const appFiltersState = getAppFilters(); + const appFilters = filterManager.getAppFilters(); + if (!_.isEqual(appFiltersState, appFilters)) { + syncAppState({ filters: appFilters }); + } + const globalFiltersState = getGlobalFilters(); + const globalFilters = filterManager.getGlobalFilters(); + if (!_.isEqual(globalFilters, globalFiltersState)) { + syncGlobalState({ filters: globalFilters }); + } + $scope.updateDataSource(); + }, + }) ); + start(); + const inspectorAdapters = { requests: new RequestAdapter(), }; - const subscriptions = new Subscription(); - $scope.timefilterUpdateHandler = ranges => { timefilter.setTime({ from: moment(ranges.from).toISOString(), @@ -281,14 +296,9 @@ function discoverController( if (abortController) abortController.abort(); savedSearch.destroy(); subscriptions.unsubscribe(); - filterStateManager.destroy(); stop(); }); - const $appStatus = ($scope.appStatus = this.appStatus = { - dirty: !savedSearch.id, - }); - const getTopNavLinks = () => { const newSearch = { id: 'new', @@ -604,12 +614,6 @@ function discoverController( }; const init = _.once(function() { - //stateMonitor = stateMonitorFactory.create($state, getStateDefaults()); - // stateMonitor.onChange(status => { - // $appStatus.dirty = status.dirty || !savedSearch.id; - // }); - // $scope.$on('$destroy', () => stateMonitor.destroy()); - $scope.updateDataSource().then(function() { const searchBarChanges = merge( timefilter.getAutoRefreshFetch$(), @@ -638,23 +642,11 @@ function discoverController( const currentSort = getSortArray($scope.searchSource.getField('sort'), $scope.indexPattern); // if the searchSource doesn't know, tell it so if (!angular.equals(sort, currentSort)) { - syncState({ sort }); + syncAppState({ sort }); $fetchObservable.next(); } }); - // update data source when filters update - - subscriptions.add( - subscribeWithScope($scope, filterManager.getUpdates$(), { - next: () => { - $scope.updateDataSource().then(function() { - //$state.save(); - }); - }, - }) - ); - // update data source when hitting forward/back and the query changes //$scope.$listen($state, 'fetch_with_changes', function(diff) { // if (diff.indexOf('query') >= 0) $fetchObservable.next(); @@ -840,7 +832,7 @@ function discoverController( $scope.updateQuery = function({ query }) { $state.query = query; - syncState({ query }); + syncAppState({ query }); $fetchObservable.next(); }; @@ -902,6 +894,12 @@ function discoverController( from: dateMath.parse(timefilter.getTime().from), to: dateMath.parse(timefilter.getTime().to, { roundUp: true }), }; + syncGlobalState({ + time: { + from: timefilter.getTime().from, + to: timefilter.getTime().to, + }, + }); }; $scope.toMoment = function(datetime) { @@ -916,7 +914,7 @@ function discoverController( kbnUrl.change('/discover'); }; - $scope.updateDataSource = Promise.method(function updateDataSource() { + $scope.updateDataSource = Promise.method(() => { const { indexPattern, searchSource } = $scope; searchSource .setField('size', $scope.opts.sampleSize) @@ -989,7 +987,7 @@ function discoverController( } else { delete $state.savedQuery; } - syncState({ savedQuery: $state.savedQuery }); + syncAppState({ savedQuery: $state.savedQuery }); }; async function setupVisualization() { diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index e36994162a6dd..4e1e47dcecbd1 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -17,33 +17,93 @@ * under the License. */ import _ from 'lodash'; -import { createStateContainer } from '../../../../../../../plugins/kibana_utils/public'; -import { createKbnUrlStateStorage } from '../../../../../../../plugins/kibana_utils/public'; -import { syncState } from '../../../../../../../plugins/kibana_utils/public'; - -interface DiscoverAppState { - columns: string[][]; - filters: any; - index: string; - interval: string; - query: any; - sort: [string, string]; +import { + createStateContainer, + createKbnUrlStateStorage, + syncStates, +} from '../../../../../../../plugins/kibana_utils/public'; +import { Filter } from '../../../../../../../plugins/data/common/es_query/filters'; + +interface AppState { + columns?: string[]; + filters?: Filter[]; + index?: string; + interval?: string; + query?: any; + sort?: string[]; +} + +interface GlobalState { + filters?: Filter[]; + time?: { from: string; to: string }; } -export function getAppState(defaultState: DiscoverAppState) { - const stateStorage = createKbnUrlStateStorage(); - const initialStateFromUrl = stateStorage.get('_a'); - const initialState = { - ...defaultState, - ...(_.cloneDeep(initialStateFromUrl) as DiscoverAppState), - } as DiscoverAppState; +/** + * Builds and returns appState and globalState containers and helper functions + * Used to sync URL with UI state + */ +export function getState( + initialAppState: AppState, + storeInSessionStorage: boolean, + onChangeAppStatus: (dirty: boolean) => void +) { + const stateStorage = createKbnUrlStateStorage({ + useHash: storeInSessionStorage, + }); - const stateContainer = createStateContainer(initialState) as any; + const globalStateInitial = stateStorage.get('_g') as GlobalState; + const globalStateContainer = createStateContainer(globalStateInitial); - const { start, stop } = syncState({ - storageKey: '_a', - stateContainer, - stateStorage, + const appStateFromUrl = stateStorage.get('_a') as AppState; + const appStateContainer = createStateContainer({ + ...initialAppState, + ...appStateFromUrl, }); - return { stateContainer, initialState, start, stop }; + + const { start, stop } = syncStates([ + { + storageKey: '_a', + stateContainer: appStateContainer, + stateStorage, + }, + { + storageKey: '_g', + stateContainer: globalStateContainer, + stateStorage, + }, + ]); + + return { + globalStateContainer, + appStateContainer, + start, + stop, + syncGlobalState: (newPartial: GlobalState) => { + const oldState = globalStateContainer.getState(); + const newState = { ...oldState, ...newPartial }; + if (!_.isEqual(oldState, newState)) { + globalStateContainer.set(newState); + } + }, + syncAppState: (newPartial: AppState) => { + const oldState = appStateContainer.getState(); + const newState = { ...oldState, ...newPartial }; + if (!_.isEqual(oldState, newState)) { + onChangeAppStatus(true); + appStateContainer.set(newState); + } + }, + getGlobalFilters: () => getFilters(globalStateContainer.getState()), + getAppFilters: () => getFilters(appStateContainer.getState()), + }; } + +/** + * Helper function to return array of filter object of a given state + */ +const getFilters = (state: AppState | GlobalState): Filter[] => { + if (!state || !Array.isArray(state.filters)) { + return []; + } + return state.filters; +}; From a18455a8f31965c576ce167eb494167b00529f1c Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Wed, 12 Feb 2020 11:51:23 +0100 Subject: [PATCH 04/53] Fix refreshInterval, cleanup $appStatus + stateMonitor --- .../discover/np_ready/angular/discover.js | 22 ++++++++++--------- .../np_ready/angular/discover_state.ts | 16 +++++++++----- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index 09f816e759f21..6d5273d0ed157 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -193,9 +193,9 @@ function discoverController( const savedSearch = $route.current.locals.savedObjects.savedSearch; $scope.searchSource = savedSearch.searchSource; $scope.indexPattern = resolveIndexPatternLoading(); - const $appStatus = ($scope.appStatus = { + const $appStatus = { dirty: !savedSearch.id, - }); + }; const onChangeAppState = dirty => { $appStatus.dirty = dirty || !savedSearch.id; }; @@ -209,12 +209,9 @@ function discoverController( syncGlobalState, getAppFilters, getGlobalFilters, + setInitialAppState, } = getState(getStateDefaults(), false, onChangeAppState); - //stateMonitor = stateMonitorFactory.create($state, getStateDefaults()); - //stateMonitor.onChange(status => { - // $appStatus.dirty = status.dirty || !savedSearch.id; - //}); appStateContainer.subscribe(newState => { if (!_.isEqual(newState, $scope.state)) { $scope.state = { ...newState }; @@ -222,8 +219,12 @@ function discoverController( }); globalStateContainer.subscribe(newState => { - if (newState.time && newState.time.from !== timefilter.from) { - timefilter.setTime(newState.time); + const { time, refreshInterval } = newState; + if (time && time.from !== timefilter.from) { + timefilter.setTime(time); + } + if (refreshInterval && !_.isEqual(refreshInterval, timefilter.getRefreshInterval())) { + timefilter.setRefreshInterval(refreshInterval); } }); @@ -354,7 +355,7 @@ function discoverController( onSave={onSave} onClose={() => {}} title={savedSearch.title} - showCopyOnSave={savedSearch.id ? true : false} + showCopyOnSave={!!savedSearch.id} objectType="search" description={i18n.translate('kbn.discover.localMenu.saveSaveSearchDescription', { defaultMessage: @@ -751,7 +752,7 @@ function discoverController( try { const id = await savedSearch.save(saveOptions); $scope.$evalAsync(() => { - //stateMonitor.setInitialState($state.toJSON()); + setInitialAppState($state); if (id) { toastNotifications.addSuccess({ title: i18n.translate('kbn.discover.notifications.savedSearchTitle', { @@ -899,6 +900,7 @@ function discoverController( from: timefilter.getTime().from, to: timefilter.getTime().to, }, + refreshInterval: timefilter.getRefreshInterval(), }); }; diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index 4e1e47dcecbd1..366cd8c769379 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -43,7 +43,7 @@ interface GlobalState { * Used to sync URL with UI state */ export function getState( - initialAppState: AppState, + defaultAppState: AppState, storeInSessionStorage: boolean, onChangeAppStatus: (dirty: boolean) => void ) { @@ -55,10 +55,11 @@ export function getState( const globalStateContainer = createStateContainer(globalStateInitial); const appStateFromUrl = stateStorage.get('_a') as AppState; - const appStateContainer = createStateContainer({ - ...initialAppState, + let initialAppState = { + ...defaultAppState, ...appStateFromUrl, - }); + }; + const appStateContainer = createStateContainer(initialAppState); const { start, stop } = syncStates([ { @@ -89,12 +90,17 @@ export function getState( const oldState = appStateContainer.getState(); const newState = { ...oldState, ...newPartial }; if (!_.isEqual(oldState, newState)) { - onChangeAppStatus(true); appStateContainer.set(newState); } + if (!_.isEqual(initialAppState, newState)) { + onChangeAppStatus(true); + } }, getGlobalFilters: () => getFilters(globalStateContainer.getState()), getAppFilters: () => getFilters(appStateContainer.getState()), + setInitialAppState: (newState: AppState) => { + initialAppState = newState; + }, }; } From 968a91b48f48870fecf89fe5b9ac7fa5e37fbfc4 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Wed, 12 Feb 2020 12:25:01 +0100 Subject: [PATCH 05/53] Add hash history --- .../kibana/public/discover/np_ready/angular/discover_state.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index 366cd8c769379..b201e01c8aacf 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -17,6 +17,7 @@ * under the License. */ import _ from 'lodash'; +import { createHashHistory } from 'history'; import { createStateContainer, createKbnUrlStateStorage, @@ -49,6 +50,7 @@ export function getState( ) { const stateStorage = createKbnUrlStateStorage({ useHash: storeInSessionStorage, + history: createHashHistory(), }); const globalStateInitial = stateStorage.get('_g') as GlobalState; From bff416acd78f151992f2211efcc0c80af0f860b6 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Wed, 12 Feb 2020 14:52:49 +0100 Subject: [PATCH 06/53] Fix sorting --- .../kibana/public/discover/np_ready/angular/discover.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index 6d5273d0ed157..1758054b1d605 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -45,7 +45,6 @@ import '../components/fetch_error'; import { getPainlessError } from './get_painless_error'; import { discoverResponseHandler } from './response_handler'; import { - angular, buildVislibDimensions, getRequestInspectorStats, getResponseInspectorStats, @@ -642,7 +641,7 @@ function discoverController( // get the current sort from searchSource as array of arrays const currentSort = getSortArray($scope.searchSource.getField('sort'), $scope.indexPattern); // if the searchSource doesn't know, tell it so - if (!angular.equals(sort, currentSort)) { + if (!_.isEqual(sort, currentSort)) { syncAppState({ sort }); $fetchObservable.next(); } @@ -930,6 +929,7 @@ function discoverController( $scope.setSortOrder = function setSortOrder(sortPair) { $scope.state.sort = sortPair; + $state.sort = sortPair; }; // TODO: On array fields, negating does not negate the combination, rather all terms From 73343b379a55f7a3bf484fb2d9debd4fedb00b27 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Thu, 13 Feb 2020 08:42:11 +0100 Subject: [PATCH 07/53] Fix search embeddable column update handling --- .../np_ready/embeddable/search_embeddable.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/embeddable/search_embeddable.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/embeddable/search_embeddable.ts index 3f877520b5bf9..662112b9211d7 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/embeddable/search_embeddable.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/embeddable/search_embeddable.ts @@ -215,24 +215,24 @@ export class SearchEmbeddable extends Embeddable return; } indexPattern.popularizeField(columnName, 1); - columnActions.addColumn(searchScope.columns, columnName); - this.updateInput({ columns: searchScope.columns }); + const columns = columnActions.addColumn(searchScope.columns, columnName); + this.updateInput({ columns }); }; searchScope.removeColumn = (columnName: string) => { if (!searchScope.columns) { return; } - columnActions.removeColumn(searchScope.columns, columnName); - this.updateInput({ columns: searchScope.columns }); + const columns = columnActions.removeColumn(searchScope.columns, columnName); + this.updateInput({ columns }); }; searchScope.moveColumn = (columnName, newIndex: number) => { if (!searchScope.columns) { return; } - columnActions.moveColumn(searchScope.columns, columnName, newIndex); - this.updateInput({ columns: searchScope.columns }); + const columns = columnActions.moveColumn(searchScope.columns, columnName, newIndex); + this.updateInput({ columns }); }; searchScope.filter = async (field, value, operator) => { From 45732209d545c172a8f54e9d4537a153161807db Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Thu, 13 Feb 2020 13:23:23 +0100 Subject: [PATCH 08/53] Fix functional test --- test/functional/apps/discover/_discover.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/apps/discover/_discover.js b/test/functional/apps/discover/_discover.js index 432e83891aa92..bf10ea15e4730 100644 --- a/test/functional/apps/discover/_discover.js +++ b/test/functional/apps/discover/_discover.js @@ -190,6 +190,7 @@ export default function({ getService, getPageObjects }) { await kibanaServer.uiSettings.replace({ 'dateFormat:tz': 'America/Phoenix' }); await browser.refresh(); await PageObjects.header.awaitKibanaChrome(); + await queryBar.setQuery(''); await PageObjects.timePicker.setDefaultAbsoluteRange(); log.debug( From c6ac8b96b7b7e21e84999ce6c4f29ba4fc0e3375 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Thu, 13 Feb 2020 16:26:36 +0100 Subject: [PATCH 09/53] Add interval syncing to url --- .../kibana/public/discover/np_ready/angular/discover.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index 97748f30e2501..436bc9ecb7c18 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -658,6 +658,7 @@ function discoverController( $scope.$watch('state.interval', function(newInterval, oldInterval) { if (newInterval !== oldInterval) { + syncAppState({ interval: newInterval }); $fetchObservable.next(); } }); From 29362179ff3cf9b6d89233837046c5646a2311a6 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Thu, 13 Feb 2020 16:28:26 +0100 Subject: [PATCH 10/53] Sync initial app state to URL --- .../public/discover/np_ready/angular/discover_state.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index b201e01c8aacf..d95a9787df895 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -61,6 +61,12 @@ export function getState( ...defaultAppState, ...appStateFromUrl, }; + + // make sure url ('_a') matches initial state + if (!_.isEqual(initialAppState, appStateFromUrl)) { + stateStorage.set('_a', initialAppState, { replace: true }); + } + const appStateContainer = createStateContainer(initialAppState); const { start, stop } = syncStates([ From 551fce23913df374c6954ccb6f1feeb3bfe06165 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Sat, 15 Feb 2020 22:26:33 +0100 Subject: [PATCH 11/53] Improve code --- .../discover/np_ready/angular/discover.js | 46 +++++++++++++------ .../np_ready/angular/discover_state.ts | 40 ++++++++++++++-- 2 files changed, 69 insertions(+), 17 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index 436bc9ecb7c18..220ec0642b160 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -17,7 +17,7 @@ * under the License. */ -import _ from 'lodash'; +import _, { cloneDeep } from 'lodash'; import React from 'react'; import { Subscription, Subject, merge } from 'rxjs'; import { debounceTime } from 'rxjs/operators'; @@ -25,7 +25,7 @@ import moment from 'moment'; import dateMath from '@elastic/datemath'; import { i18n } from '@kbn/i18n'; import '../components/field_chooser/field_chooser'; -import { getState } from './discover_state'; +import { getState, isEqualState, isEqualFilters, splitState } from './discover_state'; import { RequestAdapter } from '../../../../../../../plugins/inspector/public'; import { @@ -211,13 +211,27 @@ function discoverController( setInitialAppState, } = getState(getStateDefaults(), false, onChangeAppState); + const $state = _.cloneDeep(appStateContainer.getState()); + $scope.state = $state; + + filterManager.setFilters(cloneDeep([...getGlobalFilters(), ...getAppFilters()])); + appStateContainer.subscribe(newState => { - if (!_.isEqual(newState, $scope.state)) { + const { filters: newStateFilters, state: newStatePartial } = splitState(newState); + const { state: oldStatePartial } = splitState($scope.state); + + if (!isEqualState(newStatePartial, oldStatePartial)) { $scope.state = { ...newState }; + $scope.$digest(); + } + if (!isEqualFilters(newStateFilters, filterManager.getAppFilters())) { + filterManager.setAppFilters(Array.isArray(newStateFilters) ? cloneDeep(newStateFilters) : []); } }); globalStateContainer.subscribe(newState => { + const { filters: newStateFilters } = splitState(newState); + const { time, refreshInterval } = newState; if (time && time.from !== timefilter.from) { timefilter.setTime(time); @@ -225,11 +239,13 @@ function discoverController( if (refreshInterval && !_.isEqual(refreshInterval, timefilter.getRefreshInterval())) { timefilter.setRefreshInterval(refreshInterval); } + if (!isEqualFilters(newStateFilters, filterManager.getGlobalFilters())) { + filterManager.setGlobalFilters( + Array.isArray(newStateFilters) ? cloneDeep(newStateFilters) : [] + ); + } }); - const $state = _.cloneDeep(appStateContainer.getState()); - $scope.state = $state; - $scope.$watch('state.index', (index, prevIndex) => { syncAppState({ index }); if (prevIndex == null || prevIndex === index) return; @@ -242,20 +258,24 @@ function discoverController( } }); // update data source when filters update - subscriptions.add( subscribeWithScope($scope, filterManager.getUpdates$(), { next: () => { - const appFiltersState = getAppFilters(); - const appFilters = filterManager.getAppFilters(); - if (!_.isEqual(appFiltersState, appFilters)) { - syncAppState({ filters: appFilters }); - } const globalFiltersState = getGlobalFilters(); const globalFilters = filterManager.getGlobalFilters(); - if (!_.isEqual(globalFilters, globalFiltersState)) { + if (!isEqualFilters(globalFilters, globalFiltersState)) { syncGlobalState({ filters: globalFilters }); } + const appFiltersState = getAppFilters(); + const appFilters = filterManager.getAppFilters(); + if (!isEqualFilters(appFiltersState, appFilters)) { + syncAppState({ + filters: appFilters.map(filter => { + delete filter.meta.value; + return filter; + }), + }); + } $scope.updateDataSource(); }, }) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index d95a9787df895..a2beb9527fd09 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -23,7 +23,7 @@ import { createKbnUrlStateStorage, syncStates, } from '../../../../../../../plugins/kibana_utils/public'; -import { Filter } from '../../../../../../../plugins/data/common/es_query/filters'; +import { esFilters, Filter } from '../../../../../../../plugins/data/public'; interface AppState { columns?: string[]; @@ -90,17 +90,17 @@ export function getState( syncGlobalState: (newPartial: GlobalState) => { const oldState = globalStateContainer.getState(); const newState = { ...oldState, ...newPartial }; - if (!_.isEqual(oldState, newState)) { + if (!isEqualState(oldState, newState)) { globalStateContainer.set(newState); } }, syncAppState: (newPartial: AppState) => { const oldState = appStateContainer.getState(); const newState = { ...oldState, ...newPartial }; - if (!_.isEqual(oldState, newState)) { + if (!isEqualState(oldState, newState)) { appStateContainer.set(newState); } - if (!_.isEqual(initialAppState, newState)) { + if (!isEqualState(initialAppState, newState)) { onChangeAppStatus(true); } }, @@ -112,6 +112,38 @@ export function getState( }; } +/** + * Helper function to compare 2 different filter states + */ +export function isEqualFilters(filtersA: Filter[], filtersB: Filter[]) { + if (!filtersA && !filtersB) { + return true; + } else if (!filtersA || !filtersB) { + return false; + } + return esFilters.compareFilters(filtersA, filtersB, esFilters.COMPARE_ALL_OPTIONS); +} + +export function splitState(state: AppState | GlobalState = {}) { + const { filters = [], ...statePartial } = state; + return { filters, state: statePartial }; +} + +/** + * Helper function to compare 2 different state, is needed since comparing filters + * works differently + */ +export function isEqualState(stateA: AppState | GlobalState, stateB: AppState | GlobalState) { + if (!stateA && !stateB) { + return true; + } else if (!stateA || !stateB) { + return false; + } + const { filters: stateAFilters = [], ...stateAPartial } = stateA; + const { filters: stateBFilters = [], ...stateBPartial } = stateB; + return _.isEqual(stateAPartial, stateBPartial) && isEqualFilters(stateAFilters, stateBFilters); +} + /** * Helper function to return array of filter object of a given state */ From 956882d0a615a0bdb9e72cdedcd90144ea382af4 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 17 Feb 2020 06:38:24 +0100 Subject: [PATCH 12/53] Improve code --- .../discover/np_ready/angular/discover.js | 23 ++---- .../np_ready/angular/discover_state.ts | 79 ++++++++++++------- 2 files changed, 59 insertions(+), 43 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index 220ec0642b160..5222518a4cc30 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -192,12 +192,6 @@ function discoverController( const savedSearch = $route.current.locals.savedObjects.savedSearch; $scope.searchSource = savedSearch.searchSource; $scope.indexPattern = resolveIndexPatternLoading(); - const $appStatus = { - dirty: !savedSearch.id, - }; - const onChangeAppState = dirty => { - $appStatus.dirty = dirty || !savedSearch.id; - }; const { appStateContainer, @@ -208,8 +202,12 @@ function discoverController( syncGlobalState, getAppFilters, getGlobalFilters, - setInitialAppState, - } = getState(getStateDefaults(), false, onChangeAppState); + resetInitialAppState, + isAppStateDirty, + } = getState({ + defaultAppState: getStateDefaults(), + storeInSessionStorage: false, + }); const $state = _.cloneDeep(appStateContainer.getState()); $scope.state = $state; @@ -427,7 +425,7 @@ function discoverController( ...sharingData, title: savedSearch.title, }, - isDirty: $appStatus.dirty, + isDirty: !savedSearch.id || isAppStateDirty(), }); }, }; @@ -667,11 +665,6 @@ function discoverController( } }); - // update data source when hitting forward/back and the query changes - //$scope.$listen($state, 'fetch_with_changes', function(diff) { - // if (diff.indexOf('query') >= 0) $fetchObservable.next(); - //}); - $scope.$watch('opts.timefield', function(timefield) { $scope.enableTimeRangeSelector = !!timefield; }); @@ -772,7 +765,7 @@ function discoverController( try { const id = await savedSearch.save(saveOptions); $scope.$evalAsync(() => { - setInitialAppState($state); + resetInitialAppState(); if (id) { toastNotifications.addSuccess({ title: i18n.translate('kbn.discover.notifications.savedSearchTitle', { diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index a2beb9527fd09..ce5943dbea42d 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -22,6 +22,7 @@ import { createStateContainer, createKbnUrlStateStorage, syncStates, + ReduxLikeStateContainer, } from '../../../../../../../plugins/kibana_utils/public'; import { esFilters, Filter } from '../../../../../../../plugins/data/public'; @@ -39,18 +40,25 @@ interface GlobalState { time?: { from: string; to: string }; } +interface GetStateArgs { + defaultAppState?: AppState; + storeInSessionStorage?: boolean; + onChangeAppStatus?: (dirty: boolean) => void; + hashHistory?: any; +} + /** * Builds and returns appState and globalState containers and helper functions * Used to sync URL with UI state */ -export function getState( - defaultAppState: AppState, - storeInSessionStorage: boolean, - onChangeAppStatus: (dirty: boolean) => void -) { +export async function getState({ + defaultAppState = {}, + storeInSessionStorage = false, + hashHistory, +}: GetStateArgs) { const stateStorage = createKbnUrlStateStorage({ useHash: storeInSessionStorage, - history: createHashHistory(), + history: hashHistory ? hashHistory : createHashHistory(), }); const globalStateInitial = stateStorage.get('_g') as GlobalState; @@ -64,7 +72,7 @@ export function getState( // make sure url ('_a') matches initial state if (!_.isEqual(initialAppState, appStateFromUrl)) { - stateStorage.set('_a', initialAppState, { replace: true }); + await stateStorage.set('_a', initialAppState, { replace: true }); } const appStateContainer = createStateContainer(initialAppState); @@ -72,12 +80,20 @@ export function getState( const { start, stop } = syncStates([ { storageKey: '_a', - stateContainer: appStateContainer, + stateContainer: { + ...appStateContainer, + // handle null value when url switch doesn't contain state info + ...{ set: value => value && appStateContainer.set(value) }, + }, stateStorage, }, { storageKey: '_g', - stateContainer: globalStateContainer, + stateContainer: { + ...globalStateContainer, + // handle null value when url switch doesn't contain state info + ...{ set: value => value && globalStateContainer.set(value) }, + }, stateStorage, }, ]); @@ -87,31 +103,33 @@ export function getState( appStateContainer, start, stop, - syncGlobalState: (newPartial: GlobalState) => { - const oldState = globalStateContainer.getState(); - const newState = { ...oldState, ...newPartial }; - if (!isEqualState(oldState, newState)) { - globalStateContainer.set(newState); - } - }, - syncAppState: (newPartial: AppState) => { - const oldState = appStateContainer.getState(); - const newState = { ...oldState, ...newPartial }; - if (!isEqualState(oldState, newState)) { - appStateContainer.set(newState); - } - if (!isEqualState(initialAppState, newState)) { - onChangeAppStatus(true); - } - }, + syncGlobalState: (newPartial: GlobalState) => setState(globalStateContainer, newPartial), + syncAppState: (newPartial: AppState) => setState(appStateContainer, newPartial), getGlobalFilters: () => getFilters(globalStateContainer.getState()), getAppFilters: () => getFilters(appStateContainer.getState()), - setInitialAppState: (newState: AppState) => { - initialAppState = newState; + resetInitialAppState: () => { + initialAppState = appStateContainer.getState(); }, + flush: () => stateStorage.flush(), + isAppStateDirty: () => !isEqualState(initialAppState, appStateContainer.getState()), }; } +/** + * Helper function to merge a given new state with the existing state and to set the given state + * container + */ +export function setState( + stateContainer: ReduxLikeStateContainer, + newState: AppState | GlobalState +) { + const oldState = stateContainer.getState(); + const mergedState = { ...oldState, ...newState }; + if (!isEqualState(oldState, mergedState)) { + stateContainer.set(newState); + } +} + /** * Helper function to compare 2 different filter states */ @@ -124,6 +142,11 @@ export function isEqualFilters(filtersA: Filter[], filtersB: Filter[]) { return esFilters.compareFilters(filtersA, filtersB, esFilters.COMPARE_ALL_OPTIONS); } +/** + * helper function to extract filters of the given state + * returns a state object without filters and an array of filters + */ + export function splitState(state: AppState | GlobalState = {}) { const { filters = [], ...statePartial } = state; return { filters, state: statePartial }; From 7ebf01efa03205679d669942d9a20207c6332ead Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 17 Feb 2020 06:39:41 +0100 Subject: [PATCH 13/53] Add jest tests --- .../np_ready/angular/discover_state.test.ts | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts new file mode 100644 index 0000000000000..70af6a81ee8fb --- /dev/null +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts @@ -0,0 +1,98 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { getState } from './discover_state'; +import { createBrowserHistory, History } from 'history'; + +let history: History; +let state: any; +const getCurrentUrl = () => history.createHref(history.location); + +describe('Test discover state', () => { + beforeEach(async () => { + history = createBrowserHistory(); + history.push('/'); + state = await getState({ + defaultAppState: { index: 'test' }, + hashHistory: history, + }); + state.start(); + }); + afterEach(() => { + state.stop(); + }); + test('syncGlobalState', async () => { + state.syncGlobalState({ time: { from: 'a', to: 'b' } }); + state.flush(); + expect(getCurrentUrl()).toMatchInlineSnapshot(`"/#?_a=(index:test)&_g=(time:(from:a,to:b))"`); + }); + test('syncAppState', async () => { + state.syncAppState({ index: 'modified' }); + state.flush(); + expect(getCurrentUrl()).toMatchInlineSnapshot(`"/#?_a=(index:modified)"`); + }); + test('set appState and globalState', async () => { + state.syncAppState({ index: 'modified' }); + state.syncGlobalState({ time: { from: 'a', to: 'b' } }); + state.flush(); + expect(getCurrentUrl()).toMatchInlineSnapshot( + `"/#?_a=(index:modified)&_g=(time:(from:a,to:b))"` + ); + }); + test('URL change is propagated to appState and globalState', async () => { + history.push('/#?_a=(index:modified)&_g=(time:(from:a,to:b))'); + expect(state.globalStateContainer.getState()).toMatchInlineSnapshot(` + Object { + "time": Object { + "from": "a", + "to": "b", + }, + } + `); + expect(state.appStateContainer.getState()).toMatchInlineSnapshot(` + Object { + "index": "modified", + } + `); + }); + test('URL navigation to url without _g and _a', async () => { + await history.push('/#?_g=(time:(from:a,to:b))'); + await history.push('/'); + expect(state.globalStateContainer.getState()).toMatchInlineSnapshot(` + Object { + "time": Object { + "from": "a", + "to": "b", + }, + } + `); + expect(state.appStateContainer.getState()).toMatchInlineSnapshot(` + Object { + "index": "test", + } + `); + }); + + test('isAppStateDirty', async () => { + state.syncAppState({ index: 'modified' }); + expect(state.isAppStateDirty()).toBeTruthy(); + state.resetInitialAppState(); + expect(state.isAppStateDirty()).toBeFalsy(); + }); +}); From 66493e1bd1b286ad8aa1447d4b3556ad39c04421 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 17 Feb 2020 18:24:23 +0100 Subject: [PATCH 14/53] Fix broken Discover bootstrap (due to an async/await function) --- .../public/discover/np_ready/angular/discover.js | 15 ++++++++------- .../np_ready/angular/discover_state.test.ts | 3 ++- .../discover/np_ready/angular/discover_state.ts | 15 ++++++++------- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index a11750f44a495..69f2de902322c 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -128,11 +128,10 @@ app.config($routeProvider => { * * @type {State} */ - const { appStateContainer, start, stop } = getState({}); - start(); + const { appStateContainer, replaceUrlState } = getState({}); const { index } = appStateContainer.getState(); + replaceUrlState(); const id = getIndexPatternId(index, indexPatternList, uiSettings.get('defaultIndex')); - stop(); return Promise.props({ list: indexPatternList, loaded: indexPatterns.get(id), @@ -245,8 +244,8 @@ function discoverController( }); $scope.$watch('state.index', (index, prevIndex) => { - syncAppState({ index }); if (prevIndex == null || prevIndex === index) return; + syncAppState({ index }); $route.reload(); }); @@ -845,9 +844,11 @@ function discoverController( }; $scope.updateQuery = function({ query }) { - $state.query = query; - syncAppState({ query }); - $fetchObservable.next(); + if (!_.isEqual(query, $state.query)) { + $state.query = query; + syncAppState({ query }); + $fetchObservable.next(); + } }; function onResults(resp) { diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts index 70af6a81ee8fb..78e072eef5653 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts @@ -28,11 +28,12 @@ describe('Test discover state', () => { beforeEach(async () => { history = createBrowserHistory(); history.push('/'); - state = await getState({ + state = getState({ defaultAppState: { index: 'test' }, hashHistory: history, }); state.start(); + await state.replaceUrlState(); }); afterEach(() => { state.stop(); diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index ce5943dbea42d..f5352fc00b86c 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -51,7 +51,7 @@ interface GetStateArgs { * Builds and returns appState and globalState containers and helper functions * Used to sync URL with UI state */ -export async function getState({ +export function getState({ defaultAppState = {}, storeInSessionStorage = false, hashHistory, @@ -70,11 +70,6 @@ export async function getState({ ...appStateFromUrl, }; - // make sure url ('_a') matches initial state - if (!_.isEqual(initialAppState, appStateFromUrl)) { - await stateStorage.set('_a', initialAppState, { replace: true }); - } - const appStateContainer = createStateContainer(initialAppState); const { start, stop } = syncStates([ @@ -112,6 +107,12 @@ export async function getState({ }, flush: () => stateStorage.flush(), isAppStateDirty: () => !isEqualState(initialAppState, appStateContainer.getState()), + replaceUrlState: async () => { + // make sure url ('_a') matches initial state + if (!_.isEqual(initialAppState, appStateFromUrl)) { + await stateStorage.set('_a', initialAppState, { replace: true }); + } + }, }; } @@ -126,7 +127,7 @@ export function setState( const oldState = stateContainer.getState(); const mergedState = { ...oldState, ...newState }; if (!isEqualState(oldState, mergedState)) { - stateContainer.set(newState); + stateContainer.set(mergedState); } } From 89264258df6983520bdde03342ae8511ebd8eb72 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Tue, 18 Feb 2020 11:56:09 +0100 Subject: [PATCH 15/53] Fix testsuite, broken by filter cleanup --- .../discover/np_ready/angular/discover.js | 32 +++++++++++-------- .../np_ready/angular/discover_state.ts | 7 ++-- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index 69f2de902322c..5a870d76868d9 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -128,9 +128,8 @@ app.config($routeProvider => { * * @type {State} */ - const { appStateContainer, replaceUrlState } = getState({}); + const { appStateContainer } = getState({}); const { index } = appStateContainer.getState(); - replaceUrlState(); const id = getIndexPatternId(index, indexPatternList, uiSettings.get('defaultIndex')); return Promise.props({ list: indexPatternList, @@ -195,13 +194,14 @@ function discoverController( const { appStateContainer, globalStateContainer, - start, - stop, + start: startStateSync, + stop: stopStateSync, syncAppState, syncGlobalState, getAppFilters, getGlobalFilters, resetInitialAppState, + replaceUrlState, isAppStateDirty, } = getState({ defaultAppState: getStateDefaults(), @@ -213,7 +213,7 @@ function discoverController( filterManager.setFilters(cloneDeep([...getGlobalFilters(), ...getAppFilters()])); - appStateContainer.subscribe(newState => { + const appStateUnsubscribe = appStateContainer.subscribe(newState => { const { filters: newStateFilters, state: newStatePartial } = splitState(newState); const { state: oldStatePartial } = splitState($scope.state); @@ -226,7 +226,7 @@ function discoverController( } }); - globalStateContainer.subscribe(newState => { + const globalStateUnsubscribe = globalStateContainer.subscribe(newState => { const { filters: newStateFilters } = splitState(newState); const { time, refreshInterval } = newState; @@ -268,18 +268,19 @@ function discoverController( if (!isEqualFilters(appFiltersState, appFilters)) { syncAppState({ filters: appFilters.map(filter => { - delete filter.meta.value; + if (typeof filter.meta.value === 'function') { + delete filter.meta.value; + } return filter; }), }); } + $scope.state.filters = appFilters; $scope.updateDataSource(); }, }) ); - start(); - const inspectorAdapters = { requests: new RequestAdapter(), }; @@ -313,7 +314,9 @@ function discoverController( if (abortController) abortController.abort(); savedSearch.destroy(); subscriptions.unsubscribe(); - stop(); + appStateUnsubscribe(); + globalStateUnsubscribe(); + stopStateSync(); }); const getTopNavLinks = () => { @@ -630,8 +633,8 @@ function discoverController( ); }; - const init = _.once(function() { - $scope.updateDataSource().then(function() { + const init = _.once(() => { + $scope.updateDataSource().then(async () => { const searchBarChanges = merge( timefilter.getAutoRefreshFetch$(), timefilter.getFetch$(), @@ -747,7 +750,8 @@ function discoverController( } init.complete = true; - //$state.replace(); + await replaceUrlState(); + startStateSync(); if (shouldSearchOnPageLoad()) { $fetchObservable.next(); @@ -780,7 +784,7 @@ function discoverController( kbnUrl.change('/discover/{{id}}', { id: savedSearch.id }); } else { // Update defaults so that "reload saved query" functions correctly - $state.setDefaults(getStateDefaults()); + syncAppState(getStateDefaults()); docTitle.change(savedSearch.lastSavedTitle); } } diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index f5352fc00b86c..0c874e6a073fb 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -108,10 +108,8 @@ export function getState({ flush: () => stateStorage.flush(), isAppStateDirty: () => !isEqualState(initialAppState, appStateContainer.getState()), replaceUrlState: async () => { - // make sure url ('_a') matches initial state - if (!_.isEqual(initialAppState, appStateFromUrl)) { - await stateStorage.set('_a', initialAppState, { replace: true }); - } + await stateStorage.set('_a', appStateContainer.getState(), { replace: true }); + await stateStorage.set('_g', globalStateContainer.getState(), { replace: true }); }, }; } @@ -147,7 +145,6 @@ export function isEqualFilters(filtersA: Filter[], filtersB: Filter[]) { * helper function to extract filters of the given state * returns a state object without filters and an array of filters */ - export function splitState(state: AppState | GlobalState = {}) { const { filters = [], ...statePartial } = state; return { filters, state: statePartial }; From 472e480be9277b794b108f374f0c585a0e02cf33 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Tue, 18 Feb 2020 14:33:56 +0100 Subject: [PATCH 16/53] Remove Filter cleanup --- .../kibana/public/discover/np_ready/angular/discover.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index 5a870d76868d9..5557720aaa68d 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -266,14 +266,7 @@ function discoverController( const appFiltersState = getAppFilters(); const appFilters = filterManager.getAppFilters(); if (!isEqualFilters(appFiltersState, appFilters)) { - syncAppState({ - filters: appFilters.map(filter => { - if (typeof filter.meta.value === 'function') { - delete filter.meta.value; - } - return filter; - }), - }); + syncAppState({ filters: appFilters }); } $scope.state.filters = appFilters; $scope.updateDataSource(); From 5167b3168f87d3c8d15790773ba4891bbb482b61 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Tue, 18 Feb 2020 16:44:10 +0100 Subject: [PATCH 17/53] Cleanup code --- .../public/discover/get_inner_angular.ts | 67 +++---------------- .../discover/np_ready/angular/discover.js | 3 +- .../np_ready/angular/discover_state.ts | 9 ++- .../np_ready/angular/doc_table/doc_table.ts | 2 - .../public/discover/np_ready/application.ts | 2 - 5 files changed, 18 insertions(+), 65 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts b/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts index 373395c86636c..6fbb2672a741b 100644 --- a/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts +++ b/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts @@ -23,19 +23,11 @@ import angular from 'angular'; import { EuiIcon } from '@elastic/eui'; // @ts-ignore -import { StateProvider } from 'ui/state_management/state'; -// @ts-ignore import { EventsProvider } from 'ui/events'; import { PersistedState } from 'ui/persisted_state'; import { i18nDirective, i18nFilter, I18nProvider } from '@kbn/i18n/angular'; import { CoreStart, LegacyCoreStart, IUiSettingsClient } from 'kibana/public'; // @ts-ignore -import { AppStateProvider } from 'ui/state_management/app_state'; -// @ts-ignore -import { GlobalStateProvider } from 'ui/state_management/global_state'; -// @ts-ignore -import { StateManagementConfigProvider } from 'ui/state_management/config_provider'; -// @ts-ignore import { KbnUrlProvider, RedirectWhenMissingProvider } from 'ui/url'; // @ts-ignore import { createTopNavDirective, createTopNavHelper } from 'ui/kbn_top_nav/kbn_top_nav'; @@ -121,8 +113,6 @@ export function initializeInnerAngularModule( createLocalKbnUrlModule(); createLocalPersistedStateModule(); createLocalTopNavModule(navigation); - createLocalGlobalStateModule(); - createLocalAppStateModule(); createLocalStorageModule(); createElasticSearchModule(data); createPagerFactoryModule(); @@ -160,8 +150,6 @@ export function initializeInnerAngularModule( 'discoverPrivate', 'discoverPersistedState', 'discoverTopNav', - 'discoverGlobalState', - 'discoverAppState', 'discoverLocalStorageProvider', 'discoverEs', 'discoverDocTable', @@ -184,19 +172,6 @@ export function initializeInnerAngularModule( .service('debounce', ['$timeout', DebounceProviderTimeout]); } -export function createLocalGlobalStateModule() { - angular - .module('discoverGlobalState', [ - 'discoverPrivate', - 'discoverConfig', - 'discoverKbnUrl', - 'discoverPromise', - ]) - .service('globalState', function(Private: IPrivate) { - return Private(GlobalStateProvider); - }); -} - function createLocalPersistedStateModule() { angular .module('discoverPersistedState', ['discoverPrivate', 'discoverPromise']) @@ -218,18 +193,15 @@ function createLocalKbnUrlModule() { } function createLocalConfigModule(uiSettings: IUiSettingsClient) { - angular - .module('discoverConfig', ['discoverPrivate']) - .provider('stateManagementConfig', StateManagementConfigProvider) - .provider('config', () => { - return { - $get: () => ({ - get: (value: string) => { - return uiSettings ? uiSettings.get(value) : undefined; - }, - }), - }; - }); + angular.module('discoverConfig', ['discoverPrivate']).provider('config', () => { + return { + $get: () => ({ + get: (value: string) => { + return uiSettings ? uiSettings.get(value) : undefined; + }, + }), + }; + }); } function createLocalPromiseModule() { @@ -255,26 +227,6 @@ function createLocalI18nModule() { .directive('i18nId', i18nDirective); } -function createLocalAppStateModule() { - angular - .module('discoverAppState', [ - 'discoverGlobalState', - 'discoverPrivate', - 'discoverConfig', - 'discoverKbnUrl', - 'discoverPromise', - ]) - .service('AppState', function(Private: IPrivate) { - return Private(AppStateProvider); - }) - .service('getAppState', function(Private: any) { - return Private(AppStateProvider).getAppState; - }) - .service('State', function(Private: any) { - return Private(StateProvider); - }); -} - function createLocalStorageModule() { angular .module('discoverLocalStorageProvider', ['discoverPrivate']) @@ -306,7 +258,6 @@ function createDocTableModule() { .module('discoverDocTable', [ 'discoverKbnUrl', 'discoverConfig', - 'discoverAppState', 'discoverPagerFactory', 'react', ]) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index 5557720aaa68d..ebc481b38e5c5 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -743,8 +743,8 @@ function discoverController( } init.complete = true; + await replaceUrlState(); - startStateSync(); if (shouldSearchOnPageLoad()) { $fetchObservable.next(); @@ -1129,4 +1129,5 @@ function discoverController( addHelpMenuToAppChrome(chrome); init(); + startStateSync(); } diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index 0c874e6a073fb..1fa6b9a54a273 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -108,8 +108,13 @@ export function getState({ flush: () => stateStorage.flush(), isAppStateDirty: () => !isEqualState(initialAppState, appStateContainer.getState()), replaceUrlState: async () => { - await stateStorage.set('_a', appStateContainer.getState(), { replace: true }); - await stateStorage.set('_g', globalStateContainer.getState(), { replace: true }); + if (appStateContainer.getState()) { + await stateStorage.set('_a', appStateContainer.getState(), { replace: true }); + } + + if (globalStateContainer.getState()) { + await stateStorage.set('_g', globalStateContainer.getState(), { replace: true }); + } }, }; } diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts index 0d199e22a02fc..22bfdc6a66a3a 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts @@ -17,7 +17,6 @@ * under the License. */ -import _ from 'lodash'; import { IUiSettingsClient } from 'kibana/public'; import html from './doc_table.html'; import './infinite_scroll'; @@ -35,7 +34,6 @@ interface LazyScope extends ng.IScope { export function createDocTableDirective( config: IUiSettingsClient, - getAppState: any, pagerFactory: any, $filter: any ) { diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/application.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/application.ts index 83f4a5962e3cd..7a4819bb0f2d1 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/application.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/application.ts @@ -36,8 +36,6 @@ function mountDiscoverApp(moduleName: string, element: HTMLElement) { // bootstrap angular into detached element and attach it later to // make angular-within-angular possible const $injector = angular.bootstrap(mountpoint, [moduleName]); - // initialize global state handler - $injector.get('globalState'); element.appendChild(mountpoint); return $injector; } From fe0e580686e74e3cd621e1c8af0bc6a759072fbc Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Wed, 19 Feb 2020 10:25:40 +0100 Subject: [PATCH 18/53] Code improvements --- .../discover/np_ready/angular/discover.js | 28 ++++++------------- .../angular/doc_table/actions/columns.ts | 18 ++++++++++-- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index ebc481b38e5c5..830e1da5041d3 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -207,9 +207,11 @@ function discoverController( defaultAppState: getStateDefaults(), storeInSessionStorage: false, }); - - const $state = _.cloneDeep(appStateContainer.getState()); - $scope.state = $state; + if (appStateContainer.getState().index !== $scope.indexPattern.id) { + //used index pattern is different than the given by url/state which is invalid + syncAppState({ index: $scope.indexPattern.id }); + } + const $state = ($scope.state = { ...appStateContainer.getState() }); filterManager.setFilters(cloneDeep([...getGlobalFilters(), ...getAppFilters()])); @@ -743,9 +745,6 @@ function discoverController( } init.complete = true; - - await replaceUrlState(); - if (shouldSearchOnPageLoad()) { $fetchObservable.next(); } @@ -813,7 +812,6 @@ function discoverController( .updateDataSource() .then(setupVisualization) .then(function() { - //$state.save(); $scope.fetchStatus = fetchStatuses.LOADING; logInspectorRequest(); return $scope.searchSource.fetch({ @@ -957,23 +955,14 @@ function discoverController( return filterManager.addFilters(newFilters); }; - function getColumns(columns) { - if (columns.length > 1 && columns.indexOf('_source') !== -1) { - return columns.filter(col => col !== '_source'); - } else if (columns.length !== 0) { - return columns; - } - return ['_source']; - } - $scope.addColumn = function addColumn(columnName) { $scope.indexPattern.popularizeField(columnName, 1); - $scope.state.columns = getColumns(columnActions.addColumn($scope.state.columns, columnName)); + $scope.state.columns = columnActions.addColumn($scope.state.columns, columnName); }; $scope.removeColumn = function removeColumn(columnName) { - // $scope.indexPattern.popularizeField(columnName, 1); - $scope.state.columns = getColumns(columnActions.removeColumn($scope.state.columns, columnName)); + $scope.indexPattern.popularizeField(columnName, 1); + $scope.state.columns = columnActions.removeColumn($scope.state.columns, columnName); }; $scope.moveColumn = function moveColumn(columnName, newIndex) { @@ -1129,5 +1118,6 @@ function discoverController( addHelpMenuToAppChrome(chrome); init(); + replaceUrlState(); startStateSync(); } diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/actions/columns.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/actions/columns.ts index 8a0c0bbb78ee7..cec1a097da5bf 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/actions/columns.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/actions/columns.ts @@ -17,18 +17,32 @@ * under the License. */ +/** + * Helper function to provide a fallback to a single _source column if the given array of columns + * is empty, and removes _source if there are more than 1 columns given + * @param columns + */ +function buildColumns(columns: string[]) { + if (columns.length > 1 && columns.indexOf('_source') !== -1) { + return columns.filter(col => col !== '_source'); + } else if (columns.length !== 0) { + return columns; + } + return ['_source']; +} + export function addColumn(columns: string[], columnName: string) { if (columns.includes(columnName)) { return columns; } - return [...columns, columnName]; + return buildColumns([...columns, columnName]); } export function removeColumn(columns: string[], columnName: string) { if (!columns.includes(columnName)) { return columns; } - return columns.filter(col => col !== columnName); + return buildColumns(columns.filter(col => col !== columnName)); } export function moveColumn(columns: string[], columnName: string, newIndex: number) { From 5726b184d3f17b055cf8f8d7a18a1fdb66c18e64 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Wed, 19 Feb 2020 12:32:35 +0100 Subject: [PATCH 19/53] Refactoring of wordings --- .../discover/np_ready/angular/discover.js | 30 ++--- .../np_ready/angular/discover_state.test.ts | 36 ++--- .../np_ready/angular/discover_state.ts | 125 +++++++++++++++--- 3 files changed, 139 insertions(+), 52 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index 830e1da5041d3..a94b549d48e1d 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -194,10 +194,10 @@ function discoverController( const { appStateContainer, globalStateContainer, - start: startStateSync, - stop: stopStateSync, - syncAppState, - syncGlobalState, + startSync: startStateSync, + stopSync: stopStateSync, + setAppState, + setGlobalState, getAppFilters, getGlobalFilters, resetInitialAppState, @@ -209,7 +209,7 @@ function discoverController( }); if (appStateContainer.getState().index !== $scope.indexPattern.id) { //used index pattern is different than the given by url/state which is invalid - syncAppState({ index: $scope.indexPattern.id }); + setAppState({ index: $scope.indexPattern.id }); } const $state = ($scope.state = { ...appStateContainer.getState() }); @@ -247,13 +247,13 @@ function discoverController( $scope.$watch('state.index', (index, prevIndex) => { if (prevIndex == null || prevIndex === index) return; - syncAppState({ index }); + setAppState({ index }); $route.reload(); }); $scope.$watchCollection('state.columns', val => { if (!_.isEqual(val, appStateContainer.get().columns)) { - syncAppState({ columns: val }); + setAppState({ columns: val }); } }); // update data source when filters update @@ -263,12 +263,12 @@ function discoverController( const globalFiltersState = getGlobalFilters(); const globalFilters = filterManager.getGlobalFilters(); if (!isEqualFilters(globalFilters, globalFiltersState)) { - syncGlobalState({ filters: globalFilters }); + setGlobalState({ filters: globalFilters }); } const appFiltersState = getAppFilters(); const appFilters = filterManager.getAppFilters(); if (!isEqualFilters(appFiltersState, appFilters)) { - syncAppState({ filters: appFilters }); + setAppState({ filters: appFilters }); } $scope.state.filters = appFilters; $scope.updateDataSource(); @@ -657,7 +657,7 @@ function discoverController( const currentSort = getSortArray($scope.searchSource.getField('sort'), $scope.indexPattern); // if the searchSource doesn't know, tell it so if (!_.isEqual(sort, currentSort)) { - syncAppState({ sort }); + setAppState({ sort }); $fetchObservable.next(); } }); @@ -668,7 +668,7 @@ function discoverController( $scope.$watch('state.interval', function(newInterval, oldInterval) { if (newInterval !== oldInterval) { - syncAppState({ interval: newInterval }); + setAppState({ interval: newInterval }); $fetchObservable.next(); } }); @@ -776,7 +776,7 @@ function discoverController( kbnUrl.change('/discover/{{id}}', { id: savedSearch.id }); } else { // Update defaults so that "reload saved query" functions correctly - syncAppState(getStateDefaults()); + setAppState(getStateDefaults()); docTitle.change(savedSearch.lastSavedTitle); } } @@ -841,7 +841,7 @@ function discoverController( $scope.updateQuery = function({ query }) { if (!_.isEqual(query, $state.query)) { $state.query = query; - syncAppState({ query }); + setAppState({ query }); $fetchObservable.next(); } }; @@ -904,7 +904,7 @@ function discoverController( from: dateMath.parse(timefilter.getTime().from), to: dateMath.parse(timefilter.getTime().to, { roundUp: true }), }; - syncGlobalState({ + setGlobalState({ time: { from: timefilter.getTime().from, to: timefilter.getTime().to, @@ -990,7 +990,7 @@ function discoverController( } else { delete $state.savedQuery; } - syncAppState({ savedQuery: $state.savedQuery }); + setAppState({ savedQuery: $state.savedQuery }); }; async function setupVisualization() { diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts index 78e072eef5653..d0956438d73ad 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts @@ -17,11 +17,11 @@ * under the License. */ -import { getState } from './discover_state'; +import { getState, GetStateReturn } from './discover_state'; import { createBrowserHistory, History } from 'history'; let history: History; -let state: any; +let state: GetStateReturn; const getCurrentUrl = () => history.createHref(history.location); describe('Test discover state', () => { @@ -32,31 +32,31 @@ describe('Test discover state', () => { defaultAppState: { index: 'test' }, hashHistory: history, }); - state.start(); + state.startSync(); await state.replaceUrlState(); }); afterEach(() => { - state.stop(); + state.stopSync(); }); - test('syncGlobalState', async () => { - state.syncGlobalState({ time: { from: 'a', to: 'b' } }); - state.flush(); + test('setting global state and sycing to URL', async () => { + state.setGlobalState({ time: { from: 'a', to: 'b' } }); + state.flushToUrl(); expect(getCurrentUrl()).toMatchInlineSnapshot(`"/#?_a=(index:test)&_g=(time:(from:a,to:b))"`); }); - test('syncAppState', async () => { - state.syncAppState({ index: 'modified' }); - state.flush(); + test('setting app state and syncing to URL', async () => { + state.setAppState({ index: 'modified' }); + state.flushToUrl(); expect(getCurrentUrl()).toMatchInlineSnapshot(`"/#?_a=(index:modified)"`); }); - test('set appState and globalState', async () => { - state.syncAppState({ index: 'modified' }); - state.syncGlobalState({ time: { from: 'a', to: 'b' } }); - state.flush(); + test('setting appState and globalState and syncing to URL', async () => { + state.setAppState({ index: 'modified' }); + state.setGlobalState({ time: { from: 'a', to: 'b' } }); + state.flushToUrl(); expect(getCurrentUrl()).toMatchInlineSnapshot( `"/#?_a=(index:modified)&_g=(time:(from:a,to:b))"` ); }); - test('URL change is propagated to appState and globalState', async () => { + test('changing URL to be propagated to appState and globalState', async () => { history.push('/#?_a=(index:modified)&_g=(time:(from:a,to:b))'); expect(state.globalStateContainer.getState()).toMatchInlineSnapshot(` Object { @@ -72,7 +72,7 @@ describe('Test discover state', () => { } `); }); - test('URL navigation to url without _g and _a', async () => { + test('URL navigation to url without _g and _a, state should not change', async () => { await history.push('/#?_g=(time:(from:a,to:b))'); await history.push('/'); expect(state.globalStateContainer.getState()).toMatchInlineSnapshot(` @@ -90,8 +90,8 @@ describe('Test discover state', () => { `); }); - test('isAppStateDirty', async () => { - state.syncAppState({ index: 'modified' }); + test('isAppStateDirty to find out whether the current state has changed', async () => { + state.setAppState({ index: 'modified' }); expect(state.isAppStateDirty()).toBeTruthy(); state.resetInitialAppState(); expect(state.isAppStateDirty()).toBeFalsy(); diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index 1fa6b9a54a273..0cb87381dae1c 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -17,36 +17,121 @@ * under the License. */ import _ from 'lodash'; -import { createHashHistory } from 'history'; +import { createHashHistory, History } from 'history'; import { createStateContainer, createKbnUrlStateStorage, syncStates, ReduxLikeStateContainer, } from '../../../../../../../plugins/kibana_utils/public'; -import { esFilters, Filter } from '../../../../../../../plugins/data/public'; +import { esFilters, Filter, Query } from '../../../../../../../plugins/data/public'; interface AppState { + /** + * Columns displayed in the table + */ columns?: string[]; + /** + * Array of applied filters + */ filters?: Filter[]; + /** + * id of the used index pattern + */ index?: string; + /** + * Used interval of the histogram + */ interval?: string; - query?: any; - sort?: string[]; + /** + * Lucence or KQL query + */ + query?: Query; + /** + * Array of the used sorting [[field,direction],...] + */ + sort?: string[][]; } interface GlobalState { + /** + * Array of applied filters + */ filters?: Filter[]; + /** + * Time filter + */ time?: { from: string; to: string }; } -interface GetStateArgs { +interface GetStateParams { + /** + * Default state used for merging with with URL state to get the initial state + */ defaultAppState?: AppState; + /** + * Determins the use of long vs. short/hashed urls + */ storeInSessionStorage?: boolean; - onChangeAppStatus?: (dirty: boolean) => void; - hashHistory?: any; + /** + * Browser history used for testing + */ + hashHistory?: History; } +export interface GetStateReturn { + /** + * Global state, the _g part of the URL + */ + globalStateContainer: ReduxLikeStateContainer; + /** + * App state, the _a part of the URL + */ + appStateContainer: ReduxLikeStateContainer; + /** + * Start sync between state and URL + */ + startSync: () => void; + /** + * Stop sync between state and URL + */ + stopSync: () => void; + /** + * Set app state to with a partial new app state + */ + setAppState: (newState: Partial) => void; + /** + * Set global state to with a partial new app state + */ + setGlobalState: (newState: Partial) => void; + /** + * Get global filters + */ + getGlobalFilters: () => Filter[]; + /** + * Get global filters + */ + getAppFilters: () => Filter[]; + /** + * Sync state to URL, used for testing + */ + flushToUrl: () => void; + /** + * Reset initial state to the current app state + */ + resetInitialAppState: () => void; + /** + * Returns whether the current app state is different to the initial state + */ + isAppStateDirty: () => void; + /** + * Replace the current URL with the current state without adding another browser history entry + */ + replaceUrlState: () => Promise; +} +const GLOBAL_STATE_URL_KEY = '_g'; +const APP_STATE_URL_KEY = '_a'; + /** * Builds and returns appState and globalState containers and helper functions * Used to sync URL with UI state @@ -55,16 +140,16 @@ export function getState({ defaultAppState = {}, storeInSessionStorage = false, hashHistory, -}: GetStateArgs) { +}: GetStateParams): GetStateReturn { const stateStorage = createKbnUrlStateStorage({ useHash: storeInSessionStorage, history: hashHistory ? hashHistory : createHashHistory(), }); - const globalStateInitial = stateStorage.get('_g') as GlobalState; + const globalStateInitial = stateStorage.get(GLOBAL_STATE_URL_KEY) as GlobalState; const globalStateContainer = createStateContainer(globalStateInitial); - const appStateFromUrl = stateStorage.get('_a') as AppState; + const appStateFromUrl = stateStorage.get(APP_STATE_URL_KEY) as AppState; let initialAppState = { ...defaultAppState, ...appStateFromUrl, @@ -74,7 +159,7 @@ export function getState({ const { start, stop } = syncStates([ { - storageKey: '_a', + storageKey: APP_STATE_URL_KEY, stateContainer: { ...appStateContainer, // handle null value when url switch doesn't contain state info @@ -83,7 +168,7 @@ export function getState({ stateStorage, }, { - storageKey: '_g', + storageKey: GLOBAL_STATE_URL_KEY, stateContainer: { ...globalStateContainer, // handle null value when url switch doesn't contain state info @@ -96,24 +181,26 @@ export function getState({ return { globalStateContainer, appStateContainer, - start, - stop, - syncGlobalState: (newPartial: GlobalState) => setState(globalStateContainer, newPartial), - syncAppState: (newPartial: AppState) => setState(appStateContainer, newPartial), + startSync: start, + stopSync: stop, + setGlobalState: (newPartial: GlobalState) => setState(globalStateContainer, newPartial), + setAppState: (newPartial: AppState) => setState(appStateContainer, newPartial), getGlobalFilters: () => getFilters(globalStateContainer.getState()), getAppFilters: () => getFilters(appStateContainer.getState()), resetInitialAppState: () => { initialAppState = appStateContainer.getState(); }, - flush: () => stateStorage.flush(), + flushToUrl: () => stateStorage.flush(), isAppStateDirty: () => !isEqualState(initialAppState, appStateContainer.getState()), replaceUrlState: async () => { if (appStateContainer.getState()) { - await stateStorage.set('_a', appStateContainer.getState(), { replace: true }); + await stateStorage.set(APP_STATE_URL_KEY, appStateContainer.getState(), { replace: true }); } if (globalStateContainer.getState()) { - await stateStorage.set('_g', globalStateContainer.getState(), { replace: true }); + await stateStorage.set(GLOBAL_STATE_URL_KEY, globalStateContainer.getState(), { + replace: true, + }); } }, }; From ac31008e40f5c524c20892e904f8992e0fcd889c Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Wed, 19 Feb 2020 20:47:10 +0100 Subject: [PATCH 20/53] Add missing angular state library, that was removed in the last commit --- .../public/discover/get_inner_angular.ts | 23 +++++++++------ .../discover/np_ready/angular/discover.js | 4 +-- .../np_ready/angular/discover_state.test.ts | 9 +++--- .../np_ready/angular/discover_state.ts | 28 ++++++++++--------- 4 files changed, 34 insertions(+), 30 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts b/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts index 6fbb2672a741b..d03652f1ebe2f 100644 --- a/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts +++ b/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts @@ -28,6 +28,8 @@ import { PersistedState } from 'ui/persisted_state'; import { i18nDirective, i18nFilter, I18nProvider } from '@kbn/i18n/angular'; import { CoreStart, LegacyCoreStart, IUiSettingsClient } from 'kibana/public'; // @ts-ignore +import { StateManagementConfigProvider } from 'ui/state_management/config_provider'; +// @ts-ignore import { KbnUrlProvider, RedirectWhenMissingProvider } from 'ui/url'; // @ts-ignore import { createTopNavDirective, createTopNavHelper } from 'ui/kbn_top_nav/kbn_top_nav'; @@ -193,15 +195,18 @@ function createLocalKbnUrlModule() { } function createLocalConfigModule(uiSettings: IUiSettingsClient) { - angular.module('discoverConfig', ['discoverPrivate']).provider('config', () => { - return { - $get: () => ({ - get: (value: string) => { - return uiSettings ? uiSettings.get(value) : undefined; - }, - }), - }; - }); + angular + .module('discoverConfig', ['discoverPrivate']) + .provider('stateManagementConfig', StateManagementConfigProvider) + .provider('config', () => { + return { + $get: () => ({ + get: (value: string) => { + return uiSettings ? uiSettings.get(value) : undefined; + }, + }), + }; + }); } function createLocalPromiseModule() { diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index a94b549d48e1d..bdfcf72481ea1 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -194,7 +194,6 @@ function discoverController( const { appStateContainer, globalStateContainer, - startSync: startStateSync, stopSync: stopStateSync, setAppState, setGlobalState, @@ -1118,6 +1117,5 @@ function discoverController( addHelpMenuToAppChrome(chrome); init(); - replaceUrlState(); - startStateSync(); + replaceUrlState(true); } diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts index d0956438d73ad..39e5d528bbee2 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts @@ -32,8 +32,7 @@ describe('Test discover state', () => { defaultAppState: { index: 'test' }, hashHistory: history, }); - state.startSync(); - await state.replaceUrlState(); + await state.replaceUrlState(true); }); afterEach(() => { state.stopSync(); @@ -41,19 +40,19 @@ describe('Test discover state', () => { test('setting global state and sycing to URL', async () => { state.setGlobalState({ time: { from: 'a', to: 'b' } }); state.flushToUrl(); - expect(getCurrentUrl()).toMatchInlineSnapshot(`"/#?_a=(index:test)&_g=(time:(from:a,to:b))"`); + expect(getCurrentUrl()).toMatchInlineSnapshot(`"/#?_g=(time:(from:a,to:b))&_a=(index:test)"`); }); test('setting app state and syncing to URL', async () => { state.setAppState({ index: 'modified' }); state.flushToUrl(); - expect(getCurrentUrl()).toMatchInlineSnapshot(`"/#?_a=(index:modified)"`); + expect(getCurrentUrl()).toMatchInlineSnapshot(`"/#?_g=()&_a=(index:modified)"`); }); test('setting appState and globalState and syncing to URL', async () => { state.setAppState({ index: 'modified' }); state.setGlobalState({ time: { from: 'a', to: 'b' } }); state.flushToUrl(); expect(getCurrentUrl()).toMatchInlineSnapshot( - `"/#?_a=(index:modified)&_g=(time:(from:a,to:b))"` + `"/#?_g=(time:(from:a,to:b))&_a=(index:modified)"` ); }); test('changing URL to be propagated to appState and globalState', async () => { diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index 0cb87381dae1c..3c3e67deef0fe 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -127,7 +127,7 @@ export interface GetStateReturn { /** * Replace the current URL with the current state without adding another browser history entry */ - replaceUrlState: () => Promise; + replaceUrlState: (startSync: boolean) => Promise; } const GLOBAL_STATE_URL_KEY = '_g'; const APP_STATE_URL_KEY = '_a'; @@ -147,7 +147,7 @@ export function getState({ }); const globalStateInitial = stateStorage.get(GLOBAL_STATE_URL_KEY) as GlobalState; - const globalStateContainer = createStateContainer(globalStateInitial); + const globalStateContainer = createStateContainer(globalStateInitial || {}); const appStateFromUrl = stateStorage.get(APP_STATE_URL_KEY) as AppState; let initialAppState = { @@ -159,20 +159,20 @@ export function getState({ const { start, stop } = syncStates([ { - storageKey: APP_STATE_URL_KEY, + storageKey: GLOBAL_STATE_URL_KEY, stateContainer: { - ...appStateContainer, + ...globalStateContainer, // handle null value when url switch doesn't contain state info - ...{ set: value => value && appStateContainer.set(value) }, + ...{ set: value => value && globalStateContainer.set(value) }, }, stateStorage, }, { - storageKey: GLOBAL_STATE_URL_KEY, + storageKey: APP_STATE_URL_KEY, stateContainer: { - ...globalStateContainer, + ...appStateContainer, // handle null value when url switch doesn't contain state info - ...{ set: value => value && globalStateContainer.set(value) }, + ...{ set: value => value && appStateContainer.set(value) }, }, stateStorage, }, @@ -192,16 +192,18 @@ export function getState({ }, flushToUrl: () => stateStorage.flush(), isAppStateDirty: () => !isEqualState(initialAppState, appStateContainer.getState()), - replaceUrlState: async () => { - if (appStateContainer.getState()) { - await stateStorage.set(APP_STATE_URL_KEY, appStateContainer.getState(), { replace: true }); - } - + replaceUrlState: async (startSync = true) => { if (globalStateContainer.getState()) { await stateStorage.set(GLOBAL_STATE_URL_KEY, globalStateContainer.getState(), { replace: true, }); } + if (appStateContainer.getState()) { + await stateStorage.set(APP_STATE_URL_KEY, appStateContainer.getState(), { replace: true }); + } + if (startSync) { + start(); + } }, }; } From 86789333dc578bf44a400ef05fe80d814a788183 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Thu, 20 Feb 2020 12:34:59 +0100 Subject: [PATCH 21/53] Migrate GlobalState handling to use helper functions --- .../discover/np_ready/angular/discover.js | 67 +++++-------------- .../np_ready/angular/discover_state.test.ts | 40 ++--------- .../np_ready/angular/discover_state.ts | 55 ++++----------- .../functional/apps/discover/_shared_links.js | 4 +- 4 files changed, 39 insertions(+), 127 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index bdfcf72481ea1..3c4da50af8f81 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -17,23 +17,21 @@ * under the License. */ -import _, { cloneDeep } from 'lodash'; +import _ from 'lodash'; import React from 'react'; import { Subscription, Subject, merge } from 'rxjs'; -import { debounceTime } from 'rxjs/operators'; +import { debounceTime, map } from 'rxjs/operators'; import moment from 'moment'; import dateMath from '@elastic/datemath'; import { i18n } from '@kbn/i18n'; import '../components/field_chooser/field_chooser'; -import { getState, isEqualState, isEqualFilters, splitState } from './discover_state'; +import { getState, isEqualState, splitState } from './discover_state'; import { RequestAdapter } from '../../../../../../../plugins/inspector/public'; import { SavedObjectSaveModal, showSaveModal, } from '../../../../../../../plugins/saved_objects/public'; -// doc table -import './doc_table'; import { getSortArray } from './doc_table/lib/get_sort'; import { getSortForSearchSource } from './doc_table/lib/get_sort_for_search_source'; import * as columnActions from './doc_table/actions/columns'; @@ -62,6 +60,7 @@ import { const { core, chrome, + data, docTitle, filterManager, share, @@ -75,6 +74,8 @@ import { getRootBreadcrumbs, getSavedSearchBreadcrumbs } from '../helpers/breadc import { esFilters, indexPatterns as indexPatternsUtils, + syncQuery, + syncAppFilters, } from '../../../../../../../plugins/data/public'; import { getIndexPatternId } from '../helpers/get_index_pattern_id'; @@ -193,15 +194,13 @@ function discoverController( const { appStateContainer, - globalStateContainer, stopSync: stopStateSync, setAppState, - setGlobalState, getAppFilters, - getGlobalFilters, resetInitialAppState, replaceUrlState, isAppStateDirty, + kbnUrlStateStorage, } = getState({ defaultAppState: getStateDefaults(), storeInSessionStorage: false, @@ -212,36 +211,22 @@ function discoverController( } const $state = ($scope.state = { ...appStateContainer.getState() }); - filterManager.setFilters(cloneDeep([...getGlobalFilters(), ...getAppFilters()])); + const stopSyncingAppFilters = syncAppFilters(filterManager, { + set: filters => setAppState({ filters: filters }), + get: () => getAppFilters(), + state$: appStateContainer.state$.pipe(map(state => state.filters)), + }); + + const { stop: stopSyncingGlobalStateWithUrl } = syncQuery(data.query, kbnUrlStateStorage); const appStateUnsubscribe = appStateContainer.subscribe(newState => { - const { filters: newStateFilters, state: newStatePartial } = splitState(newState); + const { state: newStatePartial } = splitState(newState); const { state: oldStatePartial } = splitState($scope.state); if (!isEqualState(newStatePartial, oldStatePartial)) { $scope.state = { ...newState }; $scope.$digest(); } - if (!isEqualFilters(newStateFilters, filterManager.getAppFilters())) { - filterManager.setAppFilters(Array.isArray(newStateFilters) ? cloneDeep(newStateFilters) : []); - } - }); - - const globalStateUnsubscribe = globalStateContainer.subscribe(newState => { - const { filters: newStateFilters } = splitState(newState); - - const { time, refreshInterval } = newState; - if (time && time.from !== timefilter.from) { - timefilter.setTime(time); - } - if (refreshInterval && !_.isEqual(refreshInterval, timefilter.getRefreshInterval())) { - timefilter.setRefreshInterval(refreshInterval); - } - if (!isEqualFilters(newStateFilters, filterManager.getGlobalFilters())) { - filterManager.setGlobalFilters( - Array.isArray(newStateFilters) ? cloneDeep(newStateFilters) : [] - ); - } }); $scope.$watch('state.index', (index, prevIndex) => { @@ -259,17 +244,7 @@ function discoverController( subscriptions.add( subscribeWithScope($scope, filterManager.getUpdates$(), { next: () => { - const globalFiltersState = getGlobalFilters(); - const globalFilters = filterManager.getGlobalFilters(); - if (!isEqualFilters(globalFilters, globalFiltersState)) { - setGlobalState({ filters: globalFilters }); - } - const appFiltersState = getAppFilters(); - const appFilters = filterManager.getAppFilters(); - if (!isEqualFilters(appFiltersState, appFilters)) { - setAppState({ filters: appFilters }); - } - $scope.state.filters = appFilters; + $scope.state.filters = filterManager.getAppFilters(); $scope.updateDataSource(); }, }) @@ -309,8 +284,9 @@ function discoverController( savedSearch.destroy(); subscriptions.unsubscribe(); appStateUnsubscribe(); - globalStateUnsubscribe(); + stopSyncingAppFilters(); stopStateSync(); + stopSyncingGlobalStateWithUrl(); }); const getTopNavLinks = () => { @@ -903,13 +879,6 @@ function discoverController( from: dateMath.parse(timefilter.getTime().from), to: dateMath.parse(timefilter.getTime().to, { roundUp: true }), }; - setGlobalState({ - time: { - from: timefilter.getTime().from, - to: timefilter.getTime().to, - }, - refreshInterval: timefilter.getRefreshInterval(), - }); }; $scope.toMoment = function(datetime) { diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts index 39e5d528bbee2..f2502e243b5d7 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts @@ -37,34 +37,14 @@ describe('Test discover state', () => { afterEach(() => { state.stopSync(); }); - test('setting global state and sycing to URL', async () => { - state.setGlobalState({ time: { from: 'a', to: 'b' } }); - state.flushToUrl(); - expect(getCurrentUrl()).toMatchInlineSnapshot(`"/#?_g=(time:(from:a,to:b))&_a=(index:test)"`); - }); test('setting app state and syncing to URL', async () => { state.setAppState({ index: 'modified' }); state.flushToUrl(); - expect(getCurrentUrl()).toMatchInlineSnapshot(`"/#?_g=()&_a=(index:modified)"`); + expect(getCurrentUrl()).toMatchInlineSnapshot(`"/#?_a=(index:modified)"`); }); - test('setting appState and globalState and syncing to URL', async () => { - state.setAppState({ index: 'modified' }); - state.setGlobalState({ time: { from: 'a', to: 'b' } }); - state.flushToUrl(); - expect(getCurrentUrl()).toMatchInlineSnapshot( - `"/#?_g=(time:(from:a,to:b))&_a=(index:modified)"` - ); - }); - test('changing URL to be propagated to appState and globalState', async () => { - history.push('/#?_a=(index:modified)&_g=(time:(from:a,to:b))'); - expect(state.globalStateContainer.getState()).toMatchInlineSnapshot(` - Object { - "time": Object { - "from": "a", - "to": "b", - }, - } - `); + + test('changing URL to be propagated to appState', async () => { + history.push('/#?_a=(index:modified)'); expect(state.appStateContainer.getState()).toMatchInlineSnapshot(` Object { "index": "modified", @@ -72,19 +52,11 @@ describe('Test discover state', () => { `); }); test('URL navigation to url without _g and _a, state should not change', async () => { - await history.push('/#?_g=(time:(from:a,to:b))'); + history.push('/#?_a=(index:modified)'); await history.push('/'); - expect(state.globalStateContainer.getState()).toMatchInlineSnapshot(` - Object { - "time": Object { - "from": "a", - "to": "b", - }, - } - `); expect(state.appStateContainer.getState()).toMatchInlineSnapshot(` Object { - "index": "test", + "index": "modified", } `); }); diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index 3c3e67deef0fe..3553fef718347 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -21,8 +21,9 @@ import { createHashHistory, History } from 'history'; import { createStateContainer, createKbnUrlStateStorage, - syncStates, + syncState, ReduxLikeStateContainer, + IKbnUrlStateStorage, } from '../../../../../../../plugins/kibana_utils/public'; import { esFilters, Filter, Query } from '../../../../../../../plugins/data/public'; @@ -81,9 +82,9 @@ interface GetStateParams { export interface GetStateReturn { /** - * Global state, the _g part of the URL + * kbnUrlStateStorage */ - globalStateContainer: ReduxLikeStateContainer; + kbnUrlStateStorage: IKbnUrlStateStorage; /** * App state, the _a part of the URL */ @@ -100,14 +101,6 @@ export interface GetStateReturn { * Set app state to with a partial new app state */ setAppState: (newState: Partial) => void; - /** - * Set global state to with a partial new app state - */ - setGlobalState: (newState: Partial) => void; - /** - * Get global filters - */ - getGlobalFilters: () => Filter[]; /** * Get global filters */ @@ -129,7 +122,6 @@ export interface GetStateReturn { */ replaceUrlState: (startSync: boolean) => Promise; } -const GLOBAL_STATE_URL_KEY = '_g'; const APP_STATE_URL_KEY = '_a'; /** @@ -146,9 +138,6 @@ export function getState({ history: hashHistory ? hashHistory : createHashHistory(), }); - const globalStateInitial = stateStorage.get(GLOBAL_STATE_URL_KEY) as GlobalState; - const globalStateContainer = createStateContainer(globalStateInitial || {}); - const appStateFromUrl = stateStorage.get(APP_STATE_URL_KEY) as AppState; let initialAppState = { ...defaultAppState, @@ -157,35 +146,22 @@ export function getState({ const appStateContainer = createStateContainer(initialAppState); - const { start, stop } = syncStates([ - { - storageKey: GLOBAL_STATE_URL_KEY, - stateContainer: { - ...globalStateContainer, - // handle null value when url switch doesn't contain state info - ...{ set: value => value && globalStateContainer.set(value) }, - }, - stateStorage, - }, - { - storageKey: APP_STATE_URL_KEY, - stateContainer: { - ...appStateContainer, - // handle null value when url switch doesn't contain state info - ...{ set: value => value && appStateContainer.set(value) }, - }, - stateStorage, + const { start, stop } = syncState({ + storageKey: APP_STATE_URL_KEY, + stateContainer: { + ...appStateContainer, + // handle null value when url switch doesn't contain state info + ...{ set: value => value && appStateContainer.set(value) }, }, - ]); + stateStorage, + }); return { - globalStateContainer, + kbnUrlStateStorage: stateStorage, appStateContainer, startSync: start, stopSync: stop, - setGlobalState: (newPartial: GlobalState) => setState(globalStateContainer, newPartial), setAppState: (newPartial: AppState) => setState(appStateContainer, newPartial), - getGlobalFilters: () => getFilters(globalStateContainer.getState()), getAppFilters: () => getFilters(appStateContainer.getState()), resetInitialAppState: () => { initialAppState = appStateContainer.getState(); @@ -193,11 +169,6 @@ export function getState({ flushToUrl: () => stateStorage.flush(), isAppStateDirty: () => !isEqualState(initialAppState, appStateContainer.getState()), replaceUrlState: async (startSync = true) => { - if (globalStateContainer.getState()) { - await stateStorage.set(GLOBAL_STATE_URL_KEY, globalStateContainer.getState(), { - replace: true, - }); - } if (appStateContainer.getState()) { await stateStorage.set(APP_STATE_URL_KEY, appStateContainer.getState(), { replace: true }); } diff --git a/test/functional/apps/discover/_shared_links.js b/test/functional/apps/discover/_shared_links.js index d7160a39679b7..25eac2a8df97c 100644 --- a/test/functional/apps/discover/_shared_links.js +++ b/test/functional/apps/discover/_shared_links.js @@ -84,7 +84,7 @@ export default function({ getService, getPageObjects }) { const expectedUrl = baseUrl + '/app/kibana?_t=1453775307251#' + - '/discover?_g=(refreshInterval:(pause:!t,value:0),time' + + '/discover?_g=(filters:!(),refreshInterval:(pause:!t,value:0),time' + ":(from:'2015-09-19T06:31:44.000Z',to:'2015-09" + "-23T18:31:44.000Z'))&_a=(columns:!(_source),index:'logstash-" + "*',interval:auto,query:(language:kuery,query:'')" + @@ -110,7 +110,7 @@ export default function({ getService, getPageObjects }) { baseUrl + '/app/kibana#' + '/discover/ab12e3c0-f231-11e6-9486-733b1ac9221a' + - '?_g=(refreshInterval%3A(pause%3A!t%2Cvalue%3A0)' + + '?_g=(filters:!(),refreshInterval%3A(pause%3A!t%2Cvalue%3A0)' + "%2Ctime%3A(from%3A'2015-09-19T06%3A31%3A44.000Z'%2C" + "to%3A'2015-09-23T18%3A31%3A44.000Z'))"; await PageObjects.discover.loadSavedSearch('A Saved Search'); From 870a0ab2e8db6c596460726b2837c32edbbe2b0d Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Thu, 20 Feb 2020 15:44:28 +0100 Subject: [PATCH 22/53] Fix test --- test/functional/apps/discover/_shared_links.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/apps/discover/_shared_links.js b/test/functional/apps/discover/_shared_links.js index 25eac2a8df97c..ebcded47bee5d 100644 --- a/test/functional/apps/discover/_shared_links.js +++ b/test/functional/apps/discover/_shared_links.js @@ -110,7 +110,7 @@ export default function({ getService, getPageObjects }) { baseUrl + '/app/kibana#' + '/discover/ab12e3c0-f231-11e6-9486-733b1ac9221a' + - '?_g=(filters:!(),refreshInterval%3A(pause%3A!t%2Cvalue%3A0)' + + '?_g=(filters%3A!(),refreshInterval%3A(pause%3A!t%2Cvalue%3A0)' + "%2Ctime%3A(from%3A'2015-09-19T06%3A31%3A44.000Z'%2C" + "to%3A'2015-09-23T18%3A31%3A44.000Z'))"; await PageObjects.discover.loadSavedSearch('A Saved Search'); From 90b46fb233de9686b0295f8e933cdc7566cfec51 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Thu, 20 Feb 2020 17:51:33 +0100 Subject: [PATCH 23/53] Adapt test, fix global filters for context --- .../np_ready/angular/doc_table/components/table_row.ts | 9 ++++++++- test/functional/apps/discover/_shared_links.js | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/components/table_row.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/components/table_row.ts index 8df035d098469..de32d8364017e 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/components/table_row.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/components/table_row.ts @@ -33,6 +33,7 @@ import { dispatchRenderComplete } from '../../../../../../../../../plugins/kiban import cellTemplateHtml from '../components/table_row/cell.html'; import truncateByHeightTemplateHtml from '../components/table_row/truncate_by_height.html'; import { esFilters } from '../../../../../../../../../plugins/data/public'; +import { getServices } from '../../../../kibana_services'; // guesstimate at the minimum number of chars wide cells in the table should be const MIN_LINE_LENGTH = 20; @@ -116,12 +117,18 @@ export function createTableRowDirective( anchorId: $scope.row._id, indexPattern: $scope.indexPattern.id, }); + const globalFilters: any = getServices().filterManager.getGlobalFilters(); + const appFilters: any = getServices().filterManager.getAppFilters(); const hash = $httpParamSerializer({ + _g: rison.encode({ + filters: globalFilters || [], + }), _a: rison.encode({ columns: $scope.columns, - filters: ($scope.filters || []).map(esFilters.disableFilter), + filters: (appFilters || []).map(esFilters.disableFilter), }), }); + return `${path}?${hash}`; }; diff --git a/test/functional/apps/discover/_shared_links.js b/test/functional/apps/discover/_shared_links.js index ebcded47bee5d..48630348b7967 100644 --- a/test/functional/apps/discover/_shared_links.js +++ b/test/functional/apps/discover/_shared_links.js @@ -110,7 +110,7 @@ export default function({ getService, getPageObjects }) { baseUrl + '/app/kibana#' + '/discover/ab12e3c0-f231-11e6-9486-733b1ac9221a' + - '?_g=(filters%3A!(),refreshInterval%3A(pause%3A!t%2Cvalue%3A0)' + + '?_g=(filters%3A!()%2CrefreshInterval%3A(pause%3A!t%2Cvalue%3A0)' + "%2Ctime%3A(from%3A'2015-09-19T06%3A31%3A44.000Z'%2C" + "to%3A'2015-09-23T18%3A31%3A44.000Z'))"; await PageObjects.discover.loadSavedSearch('A Saved Search'); From eb2ad4c862e90c5c476023ee0f0c7b74346d1c5e Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Fri, 21 Feb 2020 08:50:55 +0100 Subject: [PATCH 24/53] Fix url back navigation test --- test/functional/apps/home/_navigation.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/functional/apps/home/_navigation.ts b/test/functional/apps/home/_navigation.ts index 6fd631baa27d7..efc0dad394464 100644 --- a/test/functional/apps/home/_navigation.ts +++ b/test/functional/apps/home/_navigation.ts @@ -52,7 +52,7 @@ export default function({ getService, getPageObjects }: FtrProviderContext) { describe('Kibana browser back navigation should work', function describeIndexTests() { before(async () => { - await esArchiver.loadIfNeeded('makelogs'); + await esArchiver.loadIfNeeded('logstash_functional'); if (browser.isInternetExplorer) { await kibanaServer.uiSettings.replace({ 'state:storeInSessionStorage': false }); } @@ -87,9 +87,7 @@ export default function({ getService, getPageObjects }: FtrProviderContext) { // Navigating back from time settings await browser.goBack(); // undo time settings - await browser.goBack(); // undo automatically set config, should it be in the history stack? (separate issue!) currUrl = await browser.getCurrentUrl(); - // Discover view also keeps adds some default arguments into the _a URL parameter, so we can only check that the url starts the same. expect(currUrl.startsWith(discoverUrl)).to.be(true); // Navigate back home From d427b88c3284bc9915be22532b825a65efacb780 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 24 Feb 2020 09:32:19 +0100 Subject: [PATCH 25/53] Refactoring to remove use-default-behaviors="true" to topnav --- .../kibana/public/discover/kibana_services.ts | 2 - .../discover/np_ready/angular/discover.html | 21 +- .../discover/np_ready/angular/discover.js | 179 ++++++++++++------ .../np_ready/angular/discover_state.test.ts | 11 +- .../np_ready/angular/discover_state.ts | 55 +++--- 5 files changed, 177 insertions(+), 91 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/kibana_services.ts b/src/legacy/core_plugins/kibana/public/discover/kibana_services.ts index 91b5c7f13dc95..b9bff8f3e7fa5 100644 --- a/src/legacy/core_plugins/kibana/public/discover/kibana_services.ts +++ b/src/legacy/core_plugins/kibana/public/discover/kibana_services.ts @@ -61,7 +61,6 @@ export { timezoneProvider } from 'ui/vis/lib/timezone'; export { tabifyAggResponse } from '../../../data/public'; export { unhashUrl } from '../../../../../plugins/kibana_utils/public'; export { - migrateLegacyQuery, ensureDefaultIndexPattern, formatMsg, formatStack, @@ -81,7 +80,6 @@ export { SortDirection, } from '../../../../../plugins/data/public'; export { ElasticSearchHit } from './np_ready/doc_views/doc_views_types'; -export { registerTimefilterWithGlobalStateFactory } from 'ui/timefilter/setup_router'; export { getFormat } from 'ui/visualize/loader/pipeline_helpers/utilities'; // @ts-ignore export { buildPointSeriesData } from 'ui/agg_response/point_series/point_series'; diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.html b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.html index 3fd3c5b5b7633..db41834d69716 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.html +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.html @@ -7,17 +7,28 @@

{{screenTitle}}

config="topNavMenu" screen-title="screenTitle" show-search-bar="true" + show-filter-bar="true" + show-save-query="showSaveQuery" show-date-picker="enableTimeRangeSelector" index-patterns="[indexPattern]" - query="state.query" + + filters="filters" + on-filters-updated="onFiltersUpdated" + on-query-submit="updateQuery" - show-save-query="showSaveQuery" - saved-query-id="state.savedQuery" - on-saved-query-id-change="updateSavedQueryId" + saved-query="savedQuery" + date-range-from="timeRangeObj.from" + date-range-to="timeRangeObj.to" + is-refresh-paused="refreshInterval.pause" + refresh-interval="refreshInterval.value" + on-refresh-change="onRefreshChange" + + on-saved="onSavedQuerySaved" + on-saved-query-updated="onSavedQueryUpdated" + on-clear-saved-query="onSavedQueryCleared" - use-default-behaviors="true" > diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index 3c4da50af8f81..495a792147a3c 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -24,8 +24,7 @@ import { debounceTime, map } from 'rxjs/operators'; import moment from 'moment'; import dateMath from '@elastic/datemath'; import { i18n } from '@kbn/i18n'; -import '../components/field_chooser/field_chooser'; -import { getState, isEqualState, splitState } from './discover_state'; +import { getState, splitState } from './discover_state'; import { RequestAdapter } from '../../../../../../../plugins/inspector/public'; import { @@ -49,7 +48,6 @@ import { getServices, hasSearchStategyForIndexPattern, intervalOptions, - migrateLegacyQuery, unhashUrl, subscribeWithScope, tabifyAggResponse, @@ -118,7 +116,10 @@ app.config($routeProvider => { const indexPatterns = getServices().indexPatterns; const savedSearchId = $route.current.params.id; return ensureDefaultIndexPattern(core, getServices().data, $rootScope, kbnUrl).then(() => { + const { appStateContainer } = getState({}); + const { index, savedQuery } = appStateContainer.getState(); return Promise.props({ + savedQuery: savedQuery ? data.query.savedQueries.getSavedQuery(savedQuery) : undefined, ip: indexPatterns.getCache().then(indexPatternList => { /** * In making the indexPattern modifiable it was placed in appState. Unfortunately, @@ -129,8 +130,6 @@ app.config($routeProvider => { * * @type {State} */ - const { appStateContainer } = getState({}); - const { index } = appStateContainer.getState(); const id = getIndexPatternId(index, indexPatternList, uiSettings.get('defaultIndex')); return Promise.props({ list: indexPatternList, @@ -185,12 +184,20 @@ function discoverController( localStorage, uiCapabilities ) { - // the actual courier.SearchSource - // the saved savedSearch const subscriptions = new Subscription(); + const $fetchObservable = new Subject(); + let inspectorRequest; const savedSearch = $route.current.locals.savedObjects.savedSearch; + if ($route.current.locals.savedObjects.savedQuery) { + $scope.savedQuery = $route.current.locals.savedObjects.savedQuery; + } + $scope.searchSource = savedSearch.searchSource; $scope.indexPattern = resolveIndexPatternLoading(); + $scope.timeRangeObj = { + ..._.cloneDeep(timefilter.getTime()), + }; + $scope.refreshInterval = timefilter.getRefreshInterval(); const { appStateContainer, @@ -209,23 +216,59 @@ function discoverController( //used index pattern is different than the given by url/state which is invalid setAppState({ index: $scope.indexPattern.id }); } - const $state = ($scope.state = { ...appStateContainer.getState() }); + $scope.state = { ...appStateContainer.getState() }; + + const updateStateFromSavedQuery = savedQuery => { + setAppState({ + savedQuery: savedQuery.id, + query: savedQuery.attributes.query, + filters: savedQuery.attributes.filters || [], + }); + + if (savedQuery.attributes.timefilter) { + timefilter.setTime({ + from: savedQuery.attributes.timefilter.from, + to: savedQuery.attributes.timefilter.to, + }); + if (savedQuery.attributes.timefilter.refreshInterval) { + timefilter.setRefreshInterval(savedQuery.attributes.timefilter.refreshInterval); + } + } + }; const stopSyncingAppFilters = syncAppFilters(filterManager, { set: filters => setAppState({ filters: filters }), get: () => getAppFilters(), state$: appStateContainer.state$.pipe(map(state => state.filters)), }); - + // syncs `_g` portion of url with query services const { stop: stopSyncingGlobalStateWithUrl } = syncQuery(data.query, kbnUrlStateStorage); - const appStateUnsubscribe = appStateContainer.subscribe(newState => { + $scope.filters = filterManager.getFilters(); + $scope.onFiltersUpdated = filters => filterManager.setFilters(filters); + + const appStateUnsubscribe = appStateContainer.subscribe(async newState => { const { state: newStatePartial } = splitState(newState); const { state: oldStatePartial } = splitState($scope.state); - if (!isEqualState(newStatePartial, oldStatePartial)) { - $scope.state = { ...newState }; - $scope.$digest(); + if (!_.isEqual(newStatePartial, oldStatePartial)) { + $scope.$evalAsync(async () => { + $scope.state = { ...newState }; + + if (oldStatePartial.savedQuery !== newStatePartial.savedQuery) { + if (newStatePartial.savedQuery) { + $scope.savedQuery = await data.query.savedQueries.getSavedQuery( + newStatePartial.savedQuery + ); + updateStateFromSavedQuery($scope.savedQuery); + } else { + delete $scope.savedQuery; + } + $fetchObservable.next(); + } else { + $scope.$digest(); + } + }); } }); @@ -235,6 +278,15 @@ function discoverController( $route.reload(); }); + $scope.onRefreshChange = ({ isPaused, refreshInterval }) => { + const newInterval = { + pause: isPaused, + value: refreshInterval, + }; + timefilter.setRefreshInterval(newInterval); + $scope.refreshInterval = newInterval; + }; + $scope.$watchCollection('state.columns', val => { if (!_.isEqual(val, appStateContainer.get().columns)) { setAppState({ columns: val }); @@ -244,6 +296,7 @@ function discoverController( subscriptions.add( subscribeWithScope($scope, filterManager.getUpdates$(), { next: () => { + $scope.filters = filterManager.getFilters(); $scope.state.filters = filterManager.getAppFilters(); $scope.updateDataSource(); }, @@ -262,7 +315,6 @@ function discoverController( }); }; $scope.intervalOptions = intervalOptions; - $scope.showInterval = false; $scope.minimumVisibleRows = 50; $scope.fetchStatus = fetchStatuses.UNINITIALIZED; $scope.showSaveQuery = uiCapabilities.discover.saveQuery; @@ -471,8 +523,6 @@ function discoverController( ]); } - const $fetchObservable = new Subject(); - $scope.screenTitle = savedSearch.title; const getFieldCounts = async () => { @@ -492,8 +542,7 @@ function discoverController( }); }; - const getSharingDataFields = async () => { - const selectedFields = $state.columns; + const getSharingDataFields = async (selectedFields, timeFieldName, hideTimeColumn) => { if (selectedFields.length === 1 && selectedFields[0] === '_source') { const fieldCounts = await getFieldCounts(); return { @@ -502,8 +551,6 @@ function discoverController( }; } - const timeFieldName = $scope.indexPattern.timeFieldName; - const hideTimeColumn = config.get('doc_table:hideTimeColumn'); const fields = timeFieldName && !hideTimeColumn ? [timeFieldName, ...selectedFields] : selectedFields; return { @@ -515,12 +562,16 @@ function discoverController( this.getSharingData = async () => { const searchSource = $scope.searchSource.createCopy(); - const { searchFields, selectFields } = await getSharingDataFields(); + const { searchFields, selectFields } = await getSharingDataFields( + $scope.state.columns, + $scope.indexPattern.timeFieldName, + config.get('doc_table:hideTimeColumn') + ); searchSource.setField('fields', searchFields); searchSource.setField( 'sort', getSortForSearchSource( - $state.sort, + $scope.state.sort, $scope.indexPattern, config.get('discover:sort:defaultOrder') ) @@ -563,8 +614,8 @@ function discoverController( }; } - $state.index = $scope.indexPattern.id; - $state.sort = getSortArray($state.sort, $scope.indexPattern); + $scope.state.index = $scope.indexPattern.id; + $scope.state.sort = getSortArray($scope.state.sort, $scope.indexPattern); $scope.getBucketIntervalToolTipText = () => { return i18n.translate('kbn.discover.bucketIntervalTooltip', { @@ -624,7 +675,13 @@ function discoverController( }, }) ); - + subscriptions.add( + subscribeWithScope($scope, timefilter.getRefreshIntervalUpdate$(), { + next: () => { + $scope.refreshInterval = timefilter.getRefreshInterval(); + }, + }) + ); $scope.$watchCollection('state.sort', function(sort) { if (!sort) return; @@ -659,15 +716,6 @@ function discoverController( } }); - $scope.$watch('state.query', (newQuery, oldQuery) => { - if (!_.isEqual(newQuery, oldQuery)) { - const query = migrateLegacyQuery(newQuery); - if (!_.isEqual(query, newQuery)) { - $scope.updateQuery({ query }); - } - } - }); - $scope.$watchMulti( ['rows', 'fetchStatus'], (function updateResultState() { @@ -813,16 +861,21 @@ function discoverController( }); }; - $scope.updateQuery = function({ query }) { - if (!_.isEqual(query, $state.query)) { - $state.query = query; + $scope.updateQuery = function({ query, dateRange }) { + if (dateRange) { + timefilter.setTime(dateRange); + $scope.timeRangeObj = dateRange; + } + + if (!_.isEqual(query, $scope.state.query)) { + $scope.state.query = query; setAppState({ query }); - $fetchObservable.next(); } + $fetchObservable.next(); }; function onResults(resp) { - logInspectorResponse(resp); + inspectorRequest.stats(getResponseInspectorStats($scope.searchSource, resp)).ok({ json: resp }); if ($scope.opts.timefield) { const tabifiedData = tabifyAggResponse($scope.vis.aggs, resp); @@ -853,8 +906,6 @@ function discoverController( $scope.fetchStatus = fetchStatuses.COMPLETE; } - let inspectorRequest; - function logInspectorRequest() { inspectorAdapters.requests.reset(); const title = i18n.translate('kbn.discover.inspectorRequestDataTitle', { @@ -870,11 +921,11 @@ function discoverController( }); } - function logInspectorResponse(resp) { - inspectorRequest.stats(getResponseInspectorStats($scope.searchSource, resp)).ok({ json: resp }); - } - $scope.updateTime = function() { + $scope.timeRangeObj = { + ..._.cloneDeep(timefilter.getTime()), + }; + //this is the timerange for the histogram, should be refactored $scope.timeRange = { from: dateMath.parse(timefilter.getTime().from), to: dateMath.parse(timefilter.getTime().to, { roundUp: true }), @@ -899,15 +950,18 @@ function discoverController( .setField('size', $scope.opts.sampleSize) .setField( 'sort', - getSortForSearchSource($state.sort, indexPattern, config.get('discover:sort:defaultOrder')) + getSortForSearchSource( + $scope.state.sort, + indexPattern, + config.get('discover:sort:defaultOrder') + ) ) - .setField('query', !$state.query ? null : $state.query) + .setField('query', !$scope.state.query ? null : $scope.state.query) .setField('filter', filterManager.getFilters()); }); $scope.setSortOrder = function setSortOrder(sortPair) { $scope.state.sort = sortPair; - $state.sort = sortPair; }; // TODO: On array fields, negating does not negate the combination, rather all terms @@ -952,13 +1006,26 @@ function discoverController( $scope.minimumVisibleRows = $scope.hits; }; - $scope.updateSavedQueryId = newSavedQueryId => { - if (newSavedQueryId) { - $state.savedQuery = newSavedQueryId; - } else { - delete $state.savedQuery; - } - setAppState({ savedQuery: $state.savedQuery }); + $scope.onSavedQuerySaved = savedQuery => { + $scope.savedQuery = savedQuery; + $scope.state.savedQuery = savedQuery.id; + updateStateFromSavedQuery(savedQuery); + $fetchObservable.next(); + }; + + $scope.onSavedQueryUpdated = savedQuery => { + $scope.savedQuery = { ...savedQuery }; + $scope.state.savedQuery = savedQuery.id; + updateStateFromSavedQuery(savedQuery); + $fetchObservable.next(); + }; + + $scope.onSavedQueryCleared = () => { + $scope.savedQuery = undefined; + const query = { ...$scope.state.query, ...{ query: '', filters: [] } }; + $scope.state.query = query; + setAppState({ query }); + $fetchObservable.next(); }; async function setupVisualization() { @@ -975,7 +1042,7 @@ function discoverController( schema: 'segment', params: { field: $scope.opts.timefield, - interval: $state.interval, + interval: $scope.state.interval, timeRange: timefilter.getTime(), }, }, diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts index f2502e243b5d7..e077bfa32fe85 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts @@ -51,7 +51,7 @@ describe('Test discover state', () => { } `); }); - test('URL navigation to url without _g and _a, state should not change', async () => { + test('URL navigation to url without _a, state should not change', async () => { history.push('/#?_a=(index:modified)'); await history.push('/'); expect(state.appStateContainer.getState()).toMatchInlineSnapshot(` @@ -61,10 +61,17 @@ describe('Test discover state', () => { `); }); - test('isAppStateDirty to find out whether the current state has changed', async () => { + test('isAppStateDirty returns whether the current state has changed', async () => { state.setAppState({ index: 'modified' }); expect(state.isAppStateDirty()).toBeTruthy(); state.resetInitialAppState(); expect(state.isAppStateDirty()).toBeFalsy(); }); + + test('getPreviousAppState returns the state before the current', async () => { + state.setAppState({ index: 'first' }); + const stateA = state.appStateContainer.getState(); + state.setAppState({ index: 'second' }); + expect(state.getPreviousAppState()).toEqual(stateA); + }); }); diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index 3553fef718347..39d9fd20df799 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -26,6 +26,7 @@ import { IKbnUrlStateStorage, } from '../../../../../../../plugins/kibana_utils/public'; import { esFilters, Filter, Query } from '../../../../../../../plugins/data/public'; +import { migrateLegacyQuery } from '../../../../../../../plugins/kibana_legacy/public'; interface AppState { /** @@ -54,17 +55,6 @@ interface AppState { sort?: string[][]; } -interface GlobalState { - /** - * Array of applied filters - */ - filters?: Filter[]; - /** - * Time filter - */ - time?: { from: string; to: string }; -} - interface GetStateParams { /** * Default state used for merging with with URL state to get the initial state @@ -113,6 +103,10 @@ export interface GetStateReturn { * Reset initial state to the current app state */ resetInitialAppState: () => void; + /** + * Return the Appstate before the current app state, useful for diffing changes + */ + getPreviousAppState: () => AppState; /** * Returns whether the current app state is different to the initial state */ @@ -143,29 +137,38 @@ export function getState({ ...defaultAppState, ...appStateFromUrl, }; - + let previousAppState: AppState; const appStateContainer = createStateContainer(initialAppState); + const appStateContainerModified = { + ...appStateContainer, + ...{ + set: (value: AppState | null) => { + if (value) { + previousAppState = appStateContainer.getState(); + appStateContainer.set(value); + } + }, + }, + }; + const { start, stop } = syncState({ storageKey: APP_STATE_URL_KEY, - stateContainer: { - ...appStateContainer, - // handle null value when url switch doesn't contain state info - ...{ set: value => value && appStateContainer.set(value) }, - }, + stateContainer: appStateContainerModified, stateStorage, }); return { kbnUrlStateStorage: stateStorage, - appStateContainer, + appStateContainer: appStateContainerModified, startSync: start, stopSync: stop, - setAppState: (newPartial: AppState) => setState(appStateContainer, newPartial), + setAppState: (newPartial: AppState) => setState(appStateContainerModified, newPartial), getAppFilters: () => getFilters(appStateContainer.getState()), resetInitialAppState: () => { initialAppState = appStateContainer.getState(); }, + getPreviousAppState: () => previousAppState, flushToUrl: () => stateStorage.flush(), isAppStateDirty: () => !isEqualState(initialAppState, appStateContainer.getState()), replaceUrlState: async (startSync = true) => { @@ -183,13 +186,13 @@ export function getState({ * Helper function to merge a given new state with the existing state and to set the given state * container */ -export function setState( - stateContainer: ReduxLikeStateContainer, - newState: AppState | GlobalState -) { +export function setState(stateContainer: ReduxLikeStateContainer, newState: AppState) { const oldState = stateContainer.getState(); const mergedState = { ...oldState, ...newState }; if (!isEqualState(oldState, mergedState)) { + if (mergedState.query) { + mergedState.query = migrateLegacyQuery(mergedState.query); + } stateContainer.set(mergedState); } } @@ -210,7 +213,7 @@ export function isEqualFilters(filtersA: Filter[], filtersB: Filter[]) { * helper function to extract filters of the given state * returns a state object without filters and an array of filters */ -export function splitState(state: AppState | GlobalState = {}) { +export function splitState(state: AppState = {}) { const { filters = [], ...statePartial } = state; return { filters, state: statePartial }; } @@ -219,7 +222,7 @@ export function splitState(state: AppState | GlobalState = {}) { * Helper function to compare 2 different state, is needed since comparing filters * works differently */ -export function isEqualState(stateA: AppState | GlobalState, stateB: AppState | GlobalState) { +export function isEqualState(stateA: AppState, stateB: AppState) { if (!stateA && !stateB) { return true; } else if (!stateA || !stateB) { @@ -233,7 +236,7 @@ export function isEqualState(stateA: AppState | GlobalState, stateB: AppState | /** * Helper function to return array of filter object of a given state */ -const getFilters = (state: AppState | GlobalState): Filter[] => { +const getFilters = (state: AppState): Filter[] => { if (!state || !Array.isArray(state.filters)) { return []; } From 9e5f2d5d613f101778264782a4e4fbe251451886 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 24 Feb 2020 15:07:57 +0100 Subject: [PATCH 26/53] Remove filters where not needed --- .../discover/np_ready/angular/doc_table/components/table_row.ts | 1 - .../public/discover/np_ready/angular/doc_table/doc_table.html | 2 -- .../public/discover/np_ready/angular/doc_table/doc_table.ts | 1 - 3 files changed, 4 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/components/table_row.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/components/table_row.ts index de32d8364017e..7a090d6b7820c 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/components/table_row.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/components/table_row.ts @@ -56,7 +56,6 @@ export function createTableRowDirective( scope: { columns: '=', filter: '=', - filters: '=?', indexPattern: '=', row: '=kbnTableRow', onAddColumn: '=?', diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.html b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.html index 61bb5cbf39cbe..3ce43426caf44 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.html +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.html @@ -43,7 +43,6 @@ sorting="sorting" index-pattern="indexPattern" filter="filter" - filters="filters" class="kbnDocTable__row" on-add-column="onAddColumn" on-remove-column="onRemoveColumn" @@ -93,7 +92,6 @@ sorting="sorting" index-pattern="indexPattern" filter="filter" - filters="filters" class="kbnDocTable__row" ng-class="{'kbnDocTable__row--highlight': row['$$_isAnchor']}" data-test-subj="docTableRow{{ row['$$_isAnchor'] ? ' docTableAnchorRow' : ''}}" diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts index 22bfdc6a66a3a..6df9aedfe3f63 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts @@ -49,7 +49,6 @@ export function createDocTableDirective( isLoading: '=?', infiniteScroll: '=?', filter: '=?', - filters: '=?', minimumVisibleRows: '=?', onAddColumn: '=?', onChangeSortOrder: '=?', From 146763b48a36ff43ceaf918974f4af13eccc462d Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 24 Feb 2020 15:15:47 +0100 Subject: [PATCH 27/53] Remove unnecessary imports --- .../public/discover/np_ready/angular/doc_table/doc_table.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts index 6df9aedfe3f63..0ca8286c17081 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/doc_table.ts @@ -19,12 +19,7 @@ import { IUiSettingsClient } from 'kibana/public'; import html from './doc_table.html'; -import './infinite_scroll'; -import './components/table_header'; -import './components/table_row'; import { dispatchRenderComplete } from '../../../../../../../../plugins/kibana_utils/public'; -import './components/pager'; -import './lib/pager'; // @ts-ignore import { getLimitedSearchResultsMessage } from './doc_table_strings'; From 4ff82abed103815532157e6b7bfff0b5322a11fe Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 24 Feb 2020 17:59:04 +0100 Subject: [PATCH 28/53] Fix missing fetch when using back button and query string changes --- .../kibana/public/discover/np_ready/angular/discover.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index 495a792147a3c..ff020f23e7a80 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -208,6 +208,7 @@ function discoverController( replaceUrlState, isAppStateDirty, kbnUrlStateStorage, + getPreviousAppState, } = getState({ defaultAppState: getStateDefaults(), storeInSessionStorage: false, @@ -250,11 +251,16 @@ function discoverController( const appStateUnsubscribe = appStateContainer.subscribe(async newState => { const { state: newStatePartial } = splitState(newState); const { state: oldStatePartial } = splitState($scope.state); + let fetchData = false; if (!_.isEqual(newStatePartial, oldStatePartial)) { $scope.$evalAsync(async () => { $scope.state = { ...newState }; + if (!_.isEqual(newStatePartial.query, getPreviousAppState().query)) { + fetchData = true; + } + if (oldStatePartial.savedQuery !== newStatePartial.savedQuery) { if (newStatePartial.savedQuery) { $scope.savedQuery = await data.query.savedQueries.getSavedQuery( @@ -264,6 +270,9 @@ function discoverController( } else { delete $scope.savedQuery; } + fetchData = true; + } + if (fetchData) { $fetchObservable.next(); } else { $scope.$digest(); From 06f125a8f1e4e71fb19c7ebc1595757d208311cb Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Tue, 25 Feb 2020 09:54:40 +0100 Subject: [PATCH 29/53] Cleanup and improve code --- .../discover/np_ready/angular/discover.js | 56 ++++++++----------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index ff020f23e7a80..90f0589b5f0c1 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -251,28 +251,33 @@ function discoverController( const appStateUnsubscribe = appStateContainer.subscribe(async newState => { const { state: newStatePartial } = splitState(newState); const { state: oldStatePartial } = splitState($scope.state); - let fetchData = false; if (!_.isEqual(newStatePartial, oldStatePartial)) { $scope.$evalAsync(async () => { $scope.state = { ...newState }; - if (!_.isEqual(newStatePartial.query, getPreviousAppState().query)) { - fetchData = true; - } + // detect changes that should trigger fetching of new data + const queryChanged = !_.isEqual(newStatePartial.query, getPreviousAppState().query); - if (oldStatePartial.savedQuery !== newStatePartial.savedQuery) { - if (newStatePartial.savedQuery) { + const currentSort = getSortArray($scope.searchSource.getField('sort'), $scope.indexPattern); + const sortChanged = !_.isEqual(currentSort, newStatePartial.sort); + const savedQueryChanged = oldStatePartial.savedQuery !== newStatePartial.savedQuery; + + if (savedQueryChanged) { + if ( + newStatePartial.savedQuery && + (!$scope.savedQuery || $scope.savedQuery.id !== newStatePartial.savedQuery) + ) { + // savedQuery was only changed in url which means it has to be fetched $scope.savedQuery = await data.query.savedQueries.getSavedQuery( newStatePartial.savedQuery ); updateStateFromSavedQuery($scope.savedQuery); - } else { - delete $scope.savedQuery; + } else if ($scope.savedQuery && typeof newStatePartial.savedQuery === 'undefined') { + $scope.savedQuery = undefined; } - fetchData = true; } - if (fetchData) { + if (queryChanged || sortChanged || savedQueryChanged) { $fetchObservable.next(); } else { $scope.$digest(); @@ -691,17 +696,6 @@ function discoverController( }, }) ); - $scope.$watchCollection('state.sort', function(sort) { - if (!sort) return; - - // get the current sort from searchSource as array of arrays - const currentSort = getSortArray($scope.searchSource.getField('sort'), $scope.indexPattern); - // if the searchSource doesn't know, tell it so - if (!_.isEqual(sort, currentSort)) { - setAppState({ sort }); - $fetchObservable.next(); - } - }); $scope.$watch('opts.timefield', function(timefield) { $scope.enableTimeRangeSelector = !!timefield; @@ -969,8 +963,8 @@ function discoverController( .setField('filter', filterManager.getFilters()); }); - $scope.setSortOrder = function setSortOrder(sortPair) { - $scope.state.sort = sortPair; + $scope.setSortOrder = function setSortOrder(sort) { + setAppState({ sort }); }; // TODO: On array fields, negating does not negate the combination, rather all terms @@ -1017,24 +1011,22 @@ function discoverController( $scope.onSavedQuerySaved = savedQuery => { $scope.savedQuery = savedQuery; - $scope.state.savedQuery = savedQuery.id; updateStateFromSavedQuery(savedQuery); - $fetchObservable.next(); }; $scope.onSavedQueryUpdated = savedQuery => { $scope.savedQuery = { ...savedQuery }; - $scope.state.savedQuery = savedQuery.id; updateStateFromSavedQuery(savedQuery); - $fetchObservable.next(); }; $scope.onSavedQueryCleared = () => { - $scope.savedQuery = undefined; - const query = { ...$scope.state.query, ...{ query: '', filters: [] } }; - $scope.state.query = query; - setAppState({ query }); - $fetchObservable.next(); + //reset filters and query string, remove savedQuery from state + const state = { + ...appStateContainer.getState(), + ...{ query: { ...$scope.state.query, ...{ query: '' } }, filters: [] }, + }; + delete state.savedQuery; + appStateContainer.set(state); }; async function setupVisualization() { From be925c3e3cc188911b4cd4034ed5dfbdcc798387 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Tue, 25 Feb 2020 14:02:03 +0100 Subject: [PATCH 30/53] Address PR comments --- .../discover/np_ready/angular/discover.js | 98 +++++++++---------- .../np_ready/angular/discover_state.ts | 12 +-- 2 files changed, 49 insertions(+), 61 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index ff020f23e7a80..02ee66a0e472f 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -115,7 +115,7 @@ app.config($routeProvider => { savedObjects: function(redirectWhenMissing, $route, kbnUrl, Promise, $rootScope) { const indexPatterns = getServices().indexPatterns; const savedSearchId = $route.current.params.id; - return ensureDefaultIndexPattern(core, getServices().data, $rootScope, kbnUrl).then(() => { + return ensureDefaultIndexPattern(core, data, $rootScope, kbnUrl).then(() => { const { appStateContainer } = getState({}); const { index, savedQuery } = appStateContainer.getState(); return Promise.props({ @@ -194,9 +194,7 @@ function discoverController( $scope.searchSource = savedSearch.searchSource; $scope.indexPattern = resolveIndexPatternLoading(); - $scope.timeRangeObj = { - ..._.cloneDeep(timefilter.getTime()), - }; + $scope.timeRangeObj = timefilter.getTime(); $scope.refreshInterval = timefilter.getRefreshInterval(); const { @@ -211,7 +209,7 @@ function discoverController( getPreviousAppState, } = getState({ defaultAppState: getStateDefaults(), - storeInSessionStorage: false, + storeInSessionStorage: config.get('state:storeInSessionStorage'), }); if (appStateContainer.getState().index !== $scope.indexPattern.id) { //used index pattern is different than the given by url/state which is invalid @@ -238,7 +236,7 @@ function discoverController( }; const stopSyncingAppFilters = syncAppFilters(filterManager, { - set: filters => setAppState({ filters: filters }), + set: filters => setAppState({ filters }), get: () => getAppFilters(), state$: appStateContainer.state$.pipe(map(state => state.filters)), }); @@ -251,28 +249,33 @@ function discoverController( const appStateUnsubscribe = appStateContainer.subscribe(async newState => { const { state: newStatePartial } = splitState(newState); const { state: oldStatePartial } = splitState($scope.state); - let fetchData = false; if (!_.isEqual(newStatePartial, oldStatePartial)) { $scope.$evalAsync(async () => { $scope.state = { ...newState }; - if (!_.isEqual(newStatePartial.query, getPreviousAppState().query)) { - fetchData = true; - } + // detect changes that should trigger fetching of new data + const queryChanged = !_.isEqual(newStatePartial.query, getPreviousAppState().query); - if (oldStatePartial.savedQuery !== newStatePartial.savedQuery) { - if (newStatePartial.savedQuery) { + const currentSort = getSortArray($scope.searchSource.getField('sort'), $scope.indexPattern); + const sortChanged = !_.isEqual(currentSort, newStatePartial.sort); + const savedQueryChanged = oldStatePartial.savedQuery !== newStatePartial.savedQuery; + + if (savedQueryChanged) { + if ( + newStatePartial.savedQuery && + (!$scope.savedQuery || $scope.savedQuery.id !== newStatePartial.savedQuery) + ) { + // savedQuery was only changed in url which means it has to be fetched $scope.savedQuery = await data.query.savedQueries.getSavedQuery( newStatePartial.savedQuery ); updateStateFromSavedQuery($scope.savedQuery); - } else { - delete $scope.savedQuery; + } else if ($scope.savedQuery && typeof newStatePartial.savedQuery === 'undefined') { + $scope.savedQuery = undefined; } - fetchData = true; } - if (fetchData) { + if (queryChanged || sortChanged || savedQueryChanged) { $fetchObservable.next(); } else { $scope.$digest(); @@ -604,14 +607,20 @@ function discoverController( indexPatternId: searchSource.getField('index').id, }; }; + + function getDefaultQuery() { + return { + query: '', + language: localStorage.get('kibana.userQueryLanguage') || config.get('search:queryLanguage'), + }; + } + function getStateDefaults() { return { - query: ($scope.savedQuery && $scope.savedQuery.attributes.query) || - $scope.searchSource.getField('query') || { - query: '', - language: - localStorage.get('kibana.userQueryLanguage') || config.get('search:queryLanguage'), - }, + query: + ($scope.savedQuery && $scope.savedQuery.attributes.query) || + $scope.searchSource.getField('query') || + getDefaultQuery(), sort: getSortArray(savedSearch.sort, $scope.indexPattern), columns: savedSearch.columns.length > 0 ? savedSearch.columns : config.get('defaultColumns').slice(), @@ -691,17 +700,6 @@ function discoverController( }, }) ); - $scope.$watchCollection('state.sort', function(sort) { - if (!sort) return; - - // get the current sort from searchSource as array of arrays - const currentSort = getSortArray($scope.searchSource.getField('sort'), $scope.indexPattern); - // if the searchSource doesn't know, tell it so - if (!_.isEqual(sort, currentSort)) { - setAppState({ sort }); - $fetchObservable.next(); - } - }); $scope.$watch('opts.timefield', function(timefield) { $scope.enableTimeRangeSelector = !!timefield; @@ -931,9 +929,7 @@ function discoverController( } $scope.updateTime = function() { - $scope.timeRangeObj = { - ..._.cloneDeep(timefilter.getTime()), - }; + $scope.timeRangeObj = timefilter.getTime(); //this is the timerange for the histogram, should be refactored $scope.timeRange = { from: dateMath.parse(timefilter.getTime().from), @@ -965,12 +961,12 @@ function discoverController( config.get('discover:sort:defaultOrder') ) ) - .setField('query', !$scope.state.query ? null : $scope.state.query) + .setField('query', $scope.state.query || null) .setField('filter', filterManager.getFilters()); }); - $scope.setSortOrder = function setSortOrder(sortPair) { - $scope.state.sort = sortPair; + $scope.setSortOrder = function setSortOrder(sort) { + setAppState({ sort }); }; // TODO: On array fields, negating does not negate the combination, rather all terms @@ -1015,26 +1011,20 @@ function discoverController( $scope.minimumVisibleRows = $scope.hits; }; - $scope.onSavedQuerySaved = savedQuery => { + $scope.onSavedQueryUpdated = $scope.onSavedQuerySaved = savedQuery => { $scope.savedQuery = savedQuery; - $scope.state.savedQuery = savedQuery.id; updateStateFromSavedQuery(savedQuery); - $fetchObservable.next(); - }; - - $scope.onSavedQueryUpdated = savedQuery => { - $scope.savedQuery = { ...savedQuery }; - $scope.state.savedQuery = savedQuery.id; - updateStateFromSavedQuery(savedQuery); - $fetchObservable.next(); }; $scope.onSavedQueryCleared = () => { - $scope.savedQuery = undefined; - const query = { ...$scope.state.query, ...{ query: '', filters: [] } }; - $scope.state.query = query; - setAppState({ query }); - $fetchObservable.next(); + //reset filters and query string, remove savedQuery from state + const state = { + ...appStateContainer.getState(), + query: getDefaultQuery(), + filters: [], + }; + delete state.savedQuery; + appStateContainer.set(state); }; async function setupVisualization() { diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index 39d9fd20df799..5de174d1ab1e1 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -142,13 +142,11 @@ export function getState({ const appStateContainerModified = { ...appStateContainer, - ...{ - set: (value: AppState | null) => { - if (value) { - previousAppState = appStateContainer.getState(); - appStateContainer.set(value); - } - }, + set: (value: AppState | null) => { + if (value) { + previousAppState = appStateContainer.getState(); + appStateContainer.set(value); + } }, }; From 06e0e4339734af54f01bb153cbf83eca2e581b8b Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 2 Mar 2020 08:41:23 +0100 Subject: [PATCH 31/53] Use syncQueryStateWithUrl & stopSyncingQueryAppStateWithStateContainer --- .../discover/np_ready/angular/discover.html | 2 +- .../discover/np_ready/angular/discover.js | 149 +++++++----------- .../np_ready/angular/discover_state.ts | 8 + .../query/state_sync/sync_state_with_url.ts | 5 +- .../ui/search_bar/lib/use_saved_query.ts | 1 + .../functional/apps/discover/_shared_links.js | 2 +- 6 files changed, 69 insertions(+), 98 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.html b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.html index 418156582ae92..8c057f2697954 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.html +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.html @@ -12,7 +12,7 @@

{{screenTitle}}

show-date-picker="enableTimeRangeSelector" show-save-query="showSaveQuery" index-patterns="[indexPattern]" - query="query" + query="state.query" on-query-submit="updateQuery" saved-query-id="state.savedQuery" on-saved-query-id-change="updateSavedQueryId" diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index 635e8429ca107..68112c5e4dcd5 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -60,6 +60,7 @@ const { chrome, data, docTitle, + indexPatterns, filterManager, share, timefilter, @@ -113,13 +114,11 @@ app.config($routeProvider => { reloadOnSearch: false, resolve: { savedObjects: function(redirectWhenMissing, $route, kbnUrl, Promise, $rootScope) { - const indexPatterns = getServices().indexPatterns; const savedSearchId = $route.current.params.id; return ensureDefaultIndexPattern(core, data, $rootScope, kbnUrl).then(() => { const { appStateContainer } = getState({}); - const { index, savedQuery } = appStateContainer.getState(); + const { index } = appStateContainer.getState(); return Promise.props({ - savedQuery: savedQuery ? data.query.savedQueries.getSavedQuery(savedQuery) : undefined, ip: indexPatterns.getCache().then(indexPatternList => { /** * In making the indexPattern modifiable it was placed in appState. Unfortunately, @@ -188,20 +187,21 @@ function discoverController( const $fetchObservable = new Subject(); let inspectorRequest; const savedSearch = $route.current.locals.savedObjects.savedSearch; - if ($route.current.locals.savedObjects.savedQuery) { - $scope.savedQuery = $route.current.locals.savedObjects.savedQuery; - } - $scope.searchSource = savedSearch.searchSource; $scope.indexPattern = resolveIndexPatternLoading(); $scope.timeRangeObj = timefilter.getTime(); $scope.refreshInterval = timefilter.getRefreshInterval(); + const getTimeField = () => { + return indexPatternsUtils.isDefault($scope.indexPattern) + ? $scope.indexPattern.timeFieldName + : undefined; + }; + const { appStateContainer, stopSync: stopStateSync, setAppState, - resetInitialAppState, replaceUrlState, isAppStateDirty, kbnUrlStateStorage, @@ -215,73 +215,50 @@ function discoverController( setAppState({ index: $scope.indexPattern.id }); } $scope.state = { ...appStateContainer.getState() }; - $scope.query = $scope.state.query; - - const updateStateFromSavedQuery = savedQuery => { - setAppState({ - savedQuery: savedQuery.id, - query: savedQuery.attributes.query, - filters: savedQuery.attributes.filters || [], - }); - if (savedQuery.attributes.timefilter) { - timefilter.setTime({ - from: savedQuery.attributes.timefilter.from, - to: savedQuery.attributes.timefilter.to, - }); - if (savedQuery.attributes.timefilter.refreshInterval) { - timefilter.setRefreshInterval(savedQuery.attributes.timefilter.refreshInterval); - } - } - }; + // syncs `_g` portion of url with query services + const { stop: stopSyncingGlobalStateWithUrl } = syncQueryStateWithUrl( + data.query, + kbnUrlStateStorage + ); const stopSyncingQueryAppStateWithStateContainer = connectToQueryState( data.query, appStateContainer, { filters: esFilters.FilterStateStore.APP_STATE } ); - // syncs `_g` portion of url with query services - const { stopSyncingGlobalStateWithUrl } = syncQueryStateWithUrl(data.query, kbnUrlStateStorage); $scope.filters = filterManager.getFilters(); - $scope.onFiltersUpdated = filters => filterManager.setFilters(filters); const appStateUnsubscribe = appStateContainer.subscribe(async newState => { const { state: newStatePartial } = splitState(newState); - const { state: oldStatePartial } = splitState($scope.state); + const { state: oldStatePartial } = splitState(getPreviousAppState()); if (!_.isEqual(newStatePartial, oldStatePartial)) { $scope.$evalAsync(async () => { $scope.state = { ...newState }; // detect changes that should trigger fetching of new data - const queryChanged = !_.isEqual(newStatePartial.query, getPreviousAppState().query); - - const currentSort = getSortArray($scope.searchSource.getField('sort'), $scope.indexPattern); - const sortChanged = !_.isEqual(currentSort, newStatePartial.sort); - const savedQueryChanged = oldStatePartial.savedQuery !== newStatePartial.savedQuery; - - if (savedQueryChanged) { - if ( - newStatePartial.savedQuery && - (!$scope.savedQuery || $scope.savedQuery.id !== newStatePartial.savedQuery) - ) { - // savedQuery was only changed in url which means it has to be fetched - $scope.savedQuery = await data.query.savedQueries.getSavedQuery( - newStatePartial.savedQuery - ); - updateStateFromSavedQuery($scope.savedQuery); - } else if ($scope.savedQuery && typeof newStatePartial.savedQuery === 'undefined') { - $scope.savedQuery = undefined; + const queryChanged = !_.isEqual(newStatePartial.query, oldStatePartial.query); + const sortChanged = !_.isEqual(oldStatePartial.sort, newStatePartial.sort); + const intervalChanged = !_.isEqual(newStatePartial.interval, oldStatePartial.interval); + const indexPatternChanged = !_.isEqual(newStatePartial.index, oldStatePartial.index); + if (indexPatternChanged) { + $scope.indexPattern = await indexPatterns.get(newStatePartial.index); + $scope.opts.timefield = getTimeField(); + $scope.enableTimeRangeSelector = !!$scope.opts.timefield; + /** + const sort = getSortArray(newStatePartial.sort, $scope.indexPattern); + console.log(sort, newStatePartial.sort); + if (!_.isEqual(sort, newStatePartial.sort)) { + return replaceUrlAppState(sort); } + **/ + $scope.vis = undefined; } - if (queryChanged || sortChanged) { - if (queryChanged) { - $scope.query = newStatePartial.query; - } + + if (queryChanged || sortChanged || intervalChanged || indexPatternChanged) { $fetchObservable.next(); - } else { - $scope.$digest(); } }); } @@ -293,20 +270,6 @@ function discoverController( $route.reload(); }); - $scope.onRefreshChange = ({ isPaused, refreshInterval }) => { - const newInterval = { - pause: isPaused, - value: refreshInterval, - }; - timefilter.setRefreshInterval(newInterval); - $scope.refreshInterval = newInterval; - }; - - $scope.$watchCollection('state.columns', val => { - if (!_.isEqual(val, appStateContainer.get().columns)) { - setAppState({ columns: val }); - } - }); // update data source when filters update subscriptions.add( subscribeWithScope($scope, filterManager.getUpdates$(), { @@ -620,18 +583,13 @@ function discoverController( function getStateDefaults() { return { - query: - ($scope.savedQuery && $scope.savedQuery.attributes.query) || - $scope.searchSource.getField('query') || - getDefaultQuery(), + query: $scope.searchSource.getField('query') || getDefaultQuery(), sort: getSortArray(savedSearch.sort, $scope.indexPattern), columns: savedSearch.columns.length > 0 ? savedSearch.columns : config.get('defaultColumns').slice(), index: $scope.indexPattern.id, interval: 'auto', - filters: - ($scope.savedQuery && $scope.savedQuery.attributes.filters) || - _.cloneDeep($scope.searchSource.getOwnField('filter')), + filters: _.cloneDeep($scope.searchSource.getOwnField('filter')), }; } @@ -659,9 +617,7 @@ function discoverController( $scope.opts = { // number of records to fetch, then paginate through sampleSize: config.get('discover:sampleSize'), - timefield: indexPatternsUtils.isDefault($scope.indexPattern) - ? $scope.indexPattern.timeFieldName - : undefined, + timefield: getTimeField(), savedSearch: savedSearch, indexPatternList: $route.current.locals.savedObjects.ip.list, }; @@ -705,20 +661,15 @@ function discoverController( }) ); - $scope.$watch('opts.timefield', function(timefield) { - $scope.enableTimeRangeSelector = !!timefield; - }); - $scope.$watch('state.interval', function(newInterval, oldInterval) { if (newInterval !== oldInterval) { setAppState({ interval: newInterval }); - $fetchObservable.next(); } }); $scope.$watch('vis.aggs', function() { // no timefield, no vis, nothing to update - if (!$scope.opts.timefield) return; + if (!getTimeField() || !$scope.vis) return; const buckets = $scope.vis.getAggConfig().bySchemaGroup('buckets'); @@ -773,7 +724,7 @@ function discoverController( })() ); - if ($scope.opts.timefield) { + if (getTimeField()) { setupVisualization(); $scope.updateTime(); } @@ -794,7 +745,6 @@ function discoverController( try { const id = await savedSearch.save(saveOptions); $scope.$evalAsync(() => { - resetInitialAppState(); if (id) { toastNotifications.addSuccess({ title: i18n.translate('kbn.discover.notifications.savedSearchTitle', { @@ -872,10 +822,12 @@ function discoverController( }); }; - $scope.updateQuery = function({ query }) { - if (!_.isEqual(query, $scope.query)) { - setAppState({ query }); + $scope.updateQuery = function({ query, dateRange }) { + if (dateRange) { + timefilter.setTime(dateRange); } + setAppState({ query }); + $fetchObservable.next(); }; $scope.updateSavedQueryId = newSavedQueryId => { @@ -896,7 +848,7 @@ function discoverController( function onResults(resp) { inspectorRequest.stats(getResponseInspectorStats($scope.searchSource, resp)).ok({ json: resp }); - if ($scope.opts.timefield) { + if (getTimeField()) { const tabifiedData = tabifyAggResponse($scope.vis.aggs, resp); $scope.searchSource.rawResponse = resp; Promise.resolve( @@ -961,9 +913,10 @@ function discoverController( kbnUrl.change('/discover'); }; - $scope.updateDataSource = Promise.method(() => { + $scope.updateDataSource = () => { const { indexPattern, searchSource } = $scope; searchSource + .setField('index', $scope.indexPattern) .setField('size', $scope.opts.sampleSize) .setField( 'sort', @@ -975,7 +928,8 @@ function discoverController( ) .setField('query', $scope.state.query || null) .setField('filter', filterManager.getFilters()); - }); + return Promise.resolve(); + }; $scope.setSortOrder = function setSortOrder(sort) { setAppState({ sort }); @@ -996,16 +950,19 @@ function discoverController( $scope.addColumn = function addColumn(columnName) { $scope.indexPattern.popularizeField(columnName, 1); - $scope.state.columns = columnActions.addColumn($scope.state.columns, columnName); + const columns = columnActions.addColumn($scope.state.columns, columnName); + setAppState({ columns }); }; $scope.removeColumn = function removeColumn(columnName) { $scope.indexPattern.popularizeField(columnName, 1); - $scope.state.columns = columnActions.removeColumn($scope.state.columns, columnName); + const columns = columnActions.removeColumn($scope.state.columns, columnName); + setAppState({ columns }); }; $scope.moveColumn = function moveColumn(columnName, newIndex) { - $scope.state.columns = columnActions.moveColumn($scope.state.columns, columnName, newIndex); + const columns = columnActions.moveColumn($scope.state.columns, columnName, newIndex); + setAppState({ columns }); }; $scope.scrollToTop = function() { @@ -1071,10 +1028,12 @@ function discoverController( visSavedObject.vis = $scope.vis; $scope.searchSource.onRequestStart((searchSource, options) => { + if (!$scope.vis) return; return $scope.vis.getAggConfig().onSearchRequestStart(searchSource, options); }); $scope.searchSource.setField('aggs', function() { + if (!$scope.vis) return; return $scope.vis.getAggConfig().toDsl(); }); } diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index 5de174d1ab1e1..7abef87707e7b 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -91,6 +91,10 @@ export interface GetStateReturn { * Set app state to with a partial new app state */ setAppState: (newState: Partial) => void; + /** + * Set state in Url using history.replace + */ + replaceUrlAppState: (newState: Partial) => Promise; /** * Get global filters */ @@ -162,6 +166,10 @@ export function getState({ startSync: start, stopSync: stop, setAppState: (newPartial: AppState) => setState(appStateContainerModified, newPartial), + replaceUrlAppState: async (newPartial: AppState) => { + const state = { ...appStateContainer.getState(), newPartial }; + await stateStorage.set(APP_STATE_URL_KEY, state, { replace: true }); + }, getAppFilters: () => getFilters(appStateContainer.getState()), resetInitialAppState: () => { initialAppState = appStateContainer.getState(); diff --git a/src/plugins/data/public/query/state_sync/sync_state_with_url.ts b/src/plugins/data/public/query/state_sync/sync_state_with_url.ts index cd7058b9f8f1c..94db717a6ca4c 100644 --- a/src/plugins/data/public/query/state_sync/sync_state_with_url.ts +++ b/src/plugins/data/public/query/state_sync/sync_state_with_url.ts @@ -85,7 +85,10 @@ export const syncQueryStateWithUrl = ( stateContainer: { ...globalQueryStateContainer, set: state => { - globalQueryStateContainer.set(state || defaultState); + if (typeof state !== 'object' || state === null) { + return; + } + globalQueryStateContainer.set(state); }, }, storageKey: GLOBAL_STATE_STORAGE_KEY, diff --git a/src/plugins/data/public/ui/search_bar/lib/use_saved_query.ts b/src/plugins/data/public/ui/search_bar/lib/use_saved_query.ts index fdeeaab1dff06..817e890b7b42b 100644 --- a/src/plugins/data/public/ui/search_bar/lib/use_saved_query.ts +++ b/src/plugins/data/public/ui/search_bar/lib/use_saved_query.ts @@ -71,6 +71,7 @@ export const useSavedQuery = (props: UseSavedQueriesProps): UseSavedQueriesRetur }; if (props.savedQueryId) fetchSavedQuery(props.savedQueryId); + else setSavedQuery(undefined); }, [ defaultLanguage, props.notifications.toasts, diff --git a/test/functional/apps/discover/_shared_links.js b/test/functional/apps/discover/_shared_links.js index 48630348b7967..75b1490149708 100644 --- a/test/functional/apps/discover/_shared_links.js +++ b/test/functional/apps/discover/_shared_links.js @@ -86,7 +86,7 @@ export default function({ getService, getPageObjects }) { '/app/kibana?_t=1453775307251#' + '/discover?_g=(filters:!(),refreshInterval:(pause:!t,value:0),time' + ":(from:'2015-09-19T06:31:44.000Z',to:'2015-09" + - "-23T18:31:44.000Z'))&_a=(columns:!(_source),index:'logstash-" + + "-23T18:31:44.000Z'))&_a=(columns:!(_source),filters:!(),index:'logstash-" + "*',interval:auto,query:(language:kuery,query:'')" + ',sort:!())'; const actualUrl = await PageObjects.share.getSharedUrl(); From 3c1e892ee385410e5dbacef7ac8c88ae5ca6526d Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 2 Mar 2020 11:06:11 +0100 Subject: [PATCH 32/53] Fix persisted state problem --- .../kibana/public/discover/get_inner_angular.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts b/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts index e3f9dcc49e9d3..fdb5d0ad75e32 100644 --- a/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts +++ b/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts @@ -22,9 +22,6 @@ // They can stay even after NP cutover import angular from 'angular'; import { EuiIcon } from '@elastic/eui'; -// @ts-ignore -import { EventsProvider } from 'ui/events'; -import { PersistedState } from 'ui/persisted_state'; import { i18nDirective, i18nFilter, I18nProvider } from '@kbn/i18n/angular'; import { CoreStart, LegacyCoreStart, IUiSettingsClient } from 'kibana/public'; // @ts-ignore @@ -73,6 +70,7 @@ import { createTopNavDirective, createTopNavHelper, } from '../../../../../plugins/kibana_legacy/public'; +import { PersistedState } from '../../../../../plugins/visualizations/public'; /** * returns the main inner angular module, it contains all the parts of Angular Discover @@ -177,11 +175,10 @@ export function initializeInnerAngularModule( function createLocalPersistedStateModule() { angular .module('discoverPersistedState', ['discoverPrivate', 'discoverPromise']) - .factory('PersistedState', (Private: IPrivate) => { - const Events = Private(EventsProvider); + .factory('PersistedState', () => { return class AngularPersistedState extends PersistedState { constructor(value: any, path: any) { - super(value, path, Events); + super(value, path); } }; }); From fac1e4b5837a324f3d7c6effc1c7e6e97f0ffe6e Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Mon, 2 Mar 2020 16:05:09 +0300 Subject: [PATCH 33/53] Remove discoverPersistedState --- .../public/discover/get_inner_angular.ts | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts b/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts index fdb5d0ad75e32..76d475c4f7f96 100644 --- a/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts +++ b/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts @@ -70,7 +70,6 @@ import { createTopNavDirective, createTopNavHelper, } from '../../../../../plugins/kibana_legacy/public'; -import { PersistedState } from '../../../../../plugins/visualizations/public'; /** * returns the main inner angular module, it contains all the parts of Angular Discover @@ -111,7 +110,6 @@ export function initializeInnerAngularModule( createLocalPromiseModule(); createLocalConfigModule(core.uiSettings); createLocalKbnUrlModule(); - createLocalPersistedStateModule(); createLocalTopNavModule(navigation); createLocalStorageModule(); createElasticSearchModule(data); @@ -130,7 +128,7 @@ export function initializeInnerAngularModule( 'discoverPrivate', 'discoverDocTable', 'discoverPagerFactory', - 'discoverPersistedState', + 'discoverPromise', ]) .config(watchMultiDecorator) .directive('icon', reactDirective => reactDirective(EuiIcon)) @@ -148,7 +146,7 @@ export function initializeInnerAngularModule( 'discoverConfig', 'discoverI18n', 'discoverPrivate', - 'discoverPersistedState', + 'discoverPromise', 'discoverTopNav', 'discoverLocalStorageProvider', 'discoverEs', @@ -172,18 +170,6 @@ export function initializeInnerAngularModule( .service('debounce', ['$timeout', DebounceProviderTimeout]); } -function createLocalPersistedStateModule() { - angular - .module('discoverPersistedState', ['discoverPrivate', 'discoverPromise']) - .factory('PersistedState', () => { - return class AngularPersistedState extends PersistedState { - constructor(value: any, path: any) { - super(value, path); - } - }; - }); -} - function createLocalKbnUrlModule() { angular .module('discoverKbnUrl', ['discoverPrivate', 'ngRoute']) From 73885ac8124a4612d2dc57bc633cef3a162d64d1 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 2 Mar 2020 15:28:33 +0100 Subject: [PATCH 34/53] Allow indexPattern switch without $route.reload() --- .../discover/np_ready/angular/discover.js | 18 ++++++++---------- .../np_ready/angular/discover_state.ts | 2 +- .../field_chooser/discover_index_pattern.tsx | 14 +++++++++----- .../field_chooser/field_chooser.html | 4 ++-- .../components/field_chooser/field_chooser.js | 7 ------- 5 files changed, 20 insertions(+), 25 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js index 68112c5e4dcd5..fa6b503c347bd 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js @@ -203,6 +203,7 @@ function discoverController( stopSync: stopStateSync, setAppState, replaceUrlState, + replaceUrlAppState, isAppStateDirty, kbnUrlStateStorage, getPreviousAppState, @@ -247,13 +248,12 @@ function discoverController( $scope.indexPattern = await indexPatterns.get(newStatePartial.index); $scope.opts.timefield = getTimeField(); $scope.enableTimeRangeSelector = !!$scope.opts.timefield; - /** + const sort = getSortArray(newStatePartial.sort, $scope.indexPattern); - console.log(sort, newStatePartial.sort); - if (!_.isEqual(sort, newStatePartial.sort)) { - return replaceUrlAppState(sort); + if (newStatePartial.sort && !_.isEqual(sort, newStatePartial.sort)) { + return await replaceUrlAppState(sort); } - **/ + // is needed to rerender the histogram $scope.vis = undefined; } @@ -264,11 +264,9 @@ function discoverController( } }); - $scope.$watch('state.index', (index, prevIndex) => { - if (prevIndex == null || prevIndex === index) return; - setAppState({ index }); - $route.reload(); - }); + $scope.setIndexPattern = id => { + setAppState({ index: id }); + }; // update data source when filters update subscriptions.add( diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts index 7abef87707e7b..440028d8238b9 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts @@ -166,7 +166,7 @@ export function getState({ startSync: start, stopSync: stop, setAppState: (newPartial: AppState) => setState(appStateContainerModified, newPartial), - replaceUrlAppState: async (newPartial: AppState) => { + replaceUrlAppState: async (newPartial: AppState = {}) => { const state = { ...appStateContainer.getState(), newPartial }; await stateStorage.set(APP_STATE_URL_KEY, state, { replace: true }); }, diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/components/field_chooser/discover_index_pattern.tsx b/src/legacy/core_plugins/kibana/public/discover/np_ready/components/field_chooser/discover_index_pattern.tsx index cca523ee2c1bd..6d90f8eac9c6c 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/components/field_chooser/discover_index_pattern.tsx +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/components/field_chooser/discover_index_pattern.tsx @@ -16,9 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useState } from 'react'; +import React, { useState, useEffect } from 'react'; import { SavedObject } from 'kibana/server'; -import { IndexPatternAttributes } from 'src/plugins/data/public'; +import { IndexPattern, IndexPatternAttributes } from 'src/plugins/data/public'; import { I18nProvider } from '@kbn/i18n/react'; import { IndexPatternRef } from './types'; @@ -31,7 +31,7 @@ export interface DiscoverIndexPatternProps { /** * currently selected index pattern, due to angular issues it's undefined at first rendering */ - selectedIndexPattern: SavedObject; + selectedIndexPattern: IndexPattern; /** * triggered when user selects a new index pattern */ @@ -50,12 +50,16 @@ export function DiscoverIndexPattern({ id: entity.id, title: entity.attributes!.title, })); - const { id: selectedId, attributes } = selectedIndexPattern || {}; + const { id: selectedId, title: selectedTitle } = selectedIndexPattern || {}; const [selected, setSelected] = useState({ id: selectedId, - title: attributes?.title || '', + title: selectedTitle || '', }); + useEffect(() => { + const { id, title } = selectedIndexPattern; + setSelected({ id, title }); + }, [selectedIndexPattern]); if (!selectedId) { return null; } diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/components/field_chooser/field_chooser.html b/src/legacy/core_plugins/kibana/public/discover/np_ready/components/field_chooser/field_chooser.html index 1587c2af79752..fd63c26aa2bb3 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/components/field_chooser/field_chooser.html +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/components/field_chooser/field_chooser.html @@ -1,7 +1,7 @@