From 11480fb7224d8d13f1ddd04a3c453fb598b29760 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 15 Jul 2019 14:31:33 -0700 Subject: [PATCH 1/7] Fix duplicate fetch in Visualize --- .../kibana/public/visualize/editor/editor.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js b/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js index 6db6e2c9ad1c..0cb705f94a0e 100644 --- a/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js +++ b/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js @@ -406,7 +406,7 @@ function VisEditor( const filterUpdateSubscription = subscribeWithScope($scope, queryFilter.getUpdates$(), { next: () => { $scope.filters = queryFilter.getFilters(); - $scope.fetch(); + $scope.globalFilters = queryFilter.getGlobalFilters(); } }); @@ -415,7 +415,6 @@ function VisEditor( $state.save(); savedVis.searchSource.setField('query', $state.query); savedVis.searchSource.setField('filter', $state.filters); - $scope.globalFilters = queryFilter.getGlobalFilters(); $scope.vis.forceReload(); }; @@ -441,9 +440,16 @@ function VisEditor( } $scope.updateQueryAndFetch = function ({ query, dateRange }) { - timefilter.setTime(dateRange); - $state.query = query; - $scope.fetch(); + const isUpdate = ( + (query && !_.isEqual(query, $state.query)) || + (dateRange && !_.isEqual(dateRange, $scope.timeRange)) + ); + + if (query && !_.isEqual(query, $state.query)) $state.query = query; + if (dateRange && !_.isEqual(dateRange, $scope.timeRange)) timefilter.setTime(dateRange); + + // If nothing has changed, trigger the fetch manually, otherwise it will happen as a result of the changes + if (!isUpdate) $scope.fetch(); }; $scope.onRefreshChange = function ({ isPaused, refreshInterval }) { From a618a3e5330a130bff25cef14f2070af07e48c57 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 24 Jul 2019 15:45:07 -0700 Subject: [PATCH 2/7] Don't refetch when disabled filters change --- .../kibana/public/visualize/editor/editor.js | 8 +++++++- .../visualize/editor/visualization_editor.js | 15 +++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js b/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js index 0cb705f94a0e..d922a30ba2d2 100644 --- a/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js +++ b/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js @@ -405,8 +405,14 @@ function VisEditor( // update the searchSource when filters update const filterUpdateSubscription = subscribeWithScope($scope, queryFilter.getUpdates$(), { next: () => { - $scope.filters = queryFilter.getFilters(); + const filters = queryFilter.getFilters(); + const isFetchRequired = !_.isEqual( + (filters || []).filter(filter => !filter.meta.disabled), + ($scope.filters || []).filter(filter => !filter.meta.disabled), + ); + $scope.filters = filters; $scope.globalFilters = queryFilter.getGlobalFilters(); + if (isFetchRequired) $scope.fetch(); } }); diff --git a/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js b/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js index a2ed44df2f5b..e2902edbc14f 100644 --- a/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js +++ b/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js @@ -17,7 +17,7 @@ * under the License. */ -import { debounce } from 'lodash'; +import { debounce, isEqual } from 'lodash'; import { uiModules } from 'ui/modules'; import 'angular-sanitize'; import { VisEditorTypesRegistryProvider } from 'ui/registry/vis_editor_types'; @@ -59,9 +59,16 @@ uiModules editor.destroy(); }); - $scope.$watchGroup(['timeRange', 'filters'], debounce(() => { - $scope.renderFunction(); - }, 100)); + const debouncedRender = debounce($scope.renderFunction, 100); + $scope.$watch('timeRange', debouncedRender); + $scope.$watch('filters', filters => { + if (!isEqual( + (filters || []).filter(filter => !filter.meta.disabled), + ($scope.filters || []).filter(filter => !filter.meta.disabled), + )) { + debouncedRender(); + } + }); } }; }); From e48cbeeb674bf2f9bc251d47d52379573026db35 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 29 Jul 2019 16:35:11 -0700 Subject: [PATCH 3/7] Use query filter fetches observable --- .../kibana/public/visualize/editor/editor.js | 26 +++++++++---------- .../visualize/editor/visualization_editor.js | 13 ++-------- .../loader/embedded_visualize_handler.ts | 15 ++++++++--- 3 files changed, 25 insertions(+), 29 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js b/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js index d922a30ba2d2..191562488429 100644 --- a/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js +++ b/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js @@ -402,20 +402,6 @@ function VisEditor( $scope.$listenAndDigestAsync(timefilter, 'timeUpdate', updateTimeRange); $scope.$listenAndDigestAsync(timefilter, 'refreshIntervalUpdate', updateRefreshInterval); - // update the searchSource when filters update - const filterUpdateSubscription = subscribeWithScope($scope, queryFilter.getUpdates$(), { - next: () => { - const filters = queryFilter.getFilters(); - const isFetchRequired = !_.isEqual( - (filters || []).filter(filter => !filter.meta.disabled), - ($scope.filters || []).filter(filter => !filter.meta.disabled), - ); - $scope.filters = filters; - $scope.globalFilters = queryFilter.getGlobalFilters(); - if (isFetchRequired) $scope.fetch(); - } - }); - // update the searchSource when query updates $scope.fetch = function () { $state.save(); @@ -424,6 +410,17 @@ function VisEditor( $scope.vis.forceReload(); }; + // update the searchSource when filters update + const filterUpdateSubscription = subscribeWithScope($scope, queryFilter.getUpdates$(), { + next: () => { + $scope.filters = queryFilter.getFilters(); + $scope.globalFilters = queryFilter.getGlobalFilters(); + } + }); + const filterFetchSubscription = subscribeWithScope($scope, queryFilter.getFetches$(), { + next: $scope.fetch + }); + $scope.$on('$destroy', function () { if ($scope._handler) { $scope._handler.destroy(); @@ -431,6 +428,7 @@ function VisEditor( savedVis.destroy(); stateMonitor.destroy(); filterUpdateSubscription.unsubscribe(); + filterFetchSubscription.unsubscribe(); }); if (!$scope.chrome.getVisible()) { diff --git a/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js b/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js index e2902edbc14f..5d931053d7f7 100644 --- a/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js +++ b/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js @@ -17,7 +17,7 @@ * under the License. */ -import { debounce, isEqual } from 'lodash'; +import { debounce } from 'lodash'; import { uiModules } from 'ui/modules'; import 'angular-sanitize'; import { VisEditorTypesRegistryProvider } from 'ui/registry/vis_editor_types'; @@ -59,16 +59,7 @@ uiModules editor.destroy(); }); - const debouncedRender = debounce($scope.renderFunction, 100); - $scope.$watch('timeRange', debouncedRender); - $scope.$watch('filters', filters => { - if (!isEqual( - (filters || []).filter(filter => !filter.meta.disabled), - ($scope.filters || []).filter(filter => !filter.meta.disabled), - )) { - debouncedRender(); - } - }); + $scope.$watch('timeRange', debounce($scope.renderFunction, 100)); } }; }); diff --git a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts index 431c5ae9be6a..774ccb4d2a06 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts @@ -18,7 +18,7 @@ */ import { EventEmitter } from 'events'; -import { debounce, forEach, get } from 'lodash'; +import { debounce, forEach, get, isEqual } from 'lodash'; import * as Rx from 'rxjs'; import { share } from 'rxjs/operators'; import { i18n } from '@kbn/i18n'; @@ -38,6 +38,7 @@ import { VisFiltersProvider } from '../../vis/vis_filters'; import { PipelineDataLoader } from './pipeline_data_loader'; import { visualizationLoader } from './visualization_loader'; import { VisualizeDataLoader } from './visualize_data_loader'; +import { onlyDisabledFiltersChanged } from '../../../../core_plugins/data/public'; import { DataAdapter, RequestAdapter } from '../../inspector/adapters'; @@ -236,15 +237,21 @@ export class EmbeddedVisualizeHandler { } let fetchRequired = false; - if (params.hasOwnProperty('timeRange')) { + if ( + params.hasOwnProperty('timeRange') && + !isEqual(this.dataLoaderParams.timeRange, params.timeRange) + ) { fetchRequired = true; this.dataLoaderParams.timeRange = params.timeRange; } - if (params.hasOwnProperty('filters')) { + if ( + params.hasOwnProperty('filters') && + !onlyDisabledFiltersChanged(this.dataLoaderParams.filters, params.filters) + ) { fetchRequired = true; this.dataLoaderParams.filters = params.filters; } - if (params.hasOwnProperty('query')) { + if (params.hasOwnProperty('query') && !isEqual(this.dataLoaderParams.query, params.query)) { fetchRequired = true; this.dataLoaderParams.query = params.query; } From 5b2f087f46492dfb3545d10c8d9ce9b8ab3bc254 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Fri, 2 Aug 2019 16:36:20 -0700 Subject: [PATCH 4/7] Add back in watch on filters --- .../kibana/public/visualize/editor/visualization_editor.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js b/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js index 5d931053d7f7..3c4945d16519 100644 --- a/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js +++ b/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js @@ -59,7 +59,7 @@ uiModules editor.destroy(); }); - $scope.$watch('timeRange', debounce($scope.renderFunction, 100)); + $scope.$watchGroup(['timeRange', 'filters'], debounce($scope.renderFunction, 100)); } }; }); From 4518608b3a7e744822ec544da206d1d057267fe2 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Fri, 2 Aug 2019 16:37:18 -0700 Subject: [PATCH 5/7] Revert change completely --- .../kibana/public/visualize/editor/visualization_editor.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js b/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js index 3c4945d16519..a2ed44df2f5b 100644 --- a/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js +++ b/src/legacy/core_plugins/kibana/public/visualize/editor/visualization_editor.js @@ -59,7 +59,9 @@ uiModules editor.destroy(); }); - $scope.$watchGroup(['timeRange', 'filters'], debounce($scope.renderFunction, 100)); + $scope.$watchGroup(['timeRange', 'filters'], debounce(() => { + $scope.renderFunction(); + }, 100)); } }; }); From 13541202ec46142605387b9e6138c82fe5064ea3 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 5 Aug 2019 10:08:06 -0700 Subject: [PATCH 6/7] Update test to include non-disabled filter --- .../public/visualize/loader/embedded_visualize_handler.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.ts b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.ts index 4e41dd54855e..d6a5ede713c6 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.ts @@ -157,7 +157,7 @@ describe('EmbeddedVisualizeHandler', () => { }); it('should call dataLoader.render with updated filters', () => { - const params = { filters: [{ foo: 'bar' }] }; + const params = { filters: [{ meta: { disabled: false } }] }; handler.update(params); jest.runAllTimers(); expect(mockDataLoaderFetch).toHaveBeenCalledWith({ ...dataLoaderParams, ...params }); From cc10e25445df20ed9cdc4a411d4034e30c096c53 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 5 Aug 2019 15:01:07 -0700 Subject: [PATCH 7/7] Remove unnecessary checks --- .../core_plugins/kibana/public/visualize/editor/editor.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js b/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js index 191562488429..4c98e54168c7 100644 --- a/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js +++ b/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js @@ -449,8 +449,8 @@ function VisEditor( (dateRange && !_.isEqual(dateRange, $scope.timeRange)) ); - if (query && !_.isEqual(query, $state.query)) $state.query = query; - if (dateRange && !_.isEqual(dateRange, $scope.timeRange)) timefilter.setTime(dateRange); + $state.query = query; + timefilter.setTime(dateRange); // If nothing has changed, trigger the fetch manually, otherwise it will happen as a result of the changes if (!isUpdate) $scope.fetch();