From 28f8eb8e88eca07d9b387105cae2c6250a914654 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 30 Jul 2019 16:59:15 +0200 Subject: [PATCH] No reload on changes to disabled filters in dashboard (#41144) (#42162) --- .../filter_manager/lib/only_disabled.test.ts | 10 ++++++ .../filter_manager/lib/only_disabled.ts | 6 ++-- .../discover/embeddable/search_embeddable.ts | 24 ++++++++++--- .../embeddable/visualize_embeddable.ts | 34 +++++++++++-------- .../maps/public/embeddable/map_embeddable.js | 3 +- 5 files changed, 54 insertions(+), 23 deletions(-) diff --git a/src/legacy/core_plugins/data/public/filter/filter_manager/lib/only_disabled.test.ts b/src/legacy/core_plugins/data/public/filter/filter_manager/lib/only_disabled.test.ts index 52dd27e9fc7adb..7a3b767b97b1b0 100644 --- a/src/legacy/core_plugins/data/public/filter/filter_manager/lib/only_disabled.test.ts +++ b/src/legacy/core_plugins/data/public/filter/filter_manager/lib/only_disabled.test.ts @@ -34,6 +34,16 @@ describe('Filter Bar Directive', function() { expect(onlyDisabledFiltersChanged(newFilters, filters)).to.be(true); }); + it('should return false if there are no old filters', function() { + const newFilters = [{ meta: { disabled: false } }] as Filter[]; + expect(onlyDisabledFiltersChanged(newFilters, undefined)).to.be(false); + }); + + it('should return false if there are no new filters', function() { + const filters = [{ meta: { disabled: false } }] as Filter[]; + expect(onlyDisabledFiltersChanged(undefined, filters)).to.be(false); + }); + it('should return false if all filters are not disabled', function() { const filters = [ { meta: { disabled: false } }, diff --git a/src/legacy/core_plugins/data/public/filter/filter_manager/lib/only_disabled.ts b/src/legacy/core_plugins/data/public/filter/filter_manager/lib/only_disabled.ts index 6df761c3db09fa..24f6b6db5352b8 100644 --- a/src/legacy/core_plugins/data/public/filter/filter_manager/lib/only_disabled.ts +++ b/src/legacy/core_plugins/data/public/filter/filter_manager/lib/only_disabled.ts @@ -27,10 +27,10 @@ const isEnabled = function(filter: Filter) { * Checks to see if only disabled filters have been changed * @returns {bool} Only disabled filters */ -export function onlyDisabledFiltersChanged(newFilters: Filter[], oldFilters: Filter[]) { +export function onlyDisabledFiltersChanged(newFilters?: Filter[], oldFilters?: Filter[]) { // If it's the same - compare only enabled filters - const newEnabledFilters = _.filter(newFilters, isEnabled); - const oldEnabledFilters = _.filter(oldFilters, isEnabled); + const newEnabledFilters = _.filter(newFilters || [], isEnabled); + const oldEnabledFilters = _.filter(oldFilters || [], isEnabled); return _.isEqual(oldEnabledFilters, newEnabledFilters); } diff --git a/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts b/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts index 8591bb0ed71371..7dec893f680250 100644 --- a/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts +++ b/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts @@ -29,6 +29,8 @@ import { getTime } from 'ui/timefilter/get_time'; import { Subscription } from 'rxjs'; import * as Rx from 'rxjs'; import { Filter, FilterStateStore } from '@kbn/es-query'; +import { TimeRange } from 'ui/timefilter/time_history'; +import { Query, onlyDisabledFiltersChanged } from '../../../../data/public'; import { APPLY_FILTER_TRIGGER, Embeddable, @@ -94,6 +96,10 @@ export class SearchEmbeddable extends Embeddable public readonly type = SEARCH_EMBEDDABLE_TYPE; private filterGen: FilterManager; + private prevTimeRange?: TimeRange; + private prevFilters?: Filter[]; + private prevQuery?: Query; + constructor( { $rootScope, @@ -259,10 +265,20 @@ export class SearchEmbeddable extends Embeddable searchScope.sort = this.input.sort || this.savedSearch.sort; searchScope.sharedItemTitle = this.panelTitle; - this.filtersSearchSource.setField('filter', this.input.filters); - this.filtersSearchSource.setField('query', this.input.query); + if ( + !onlyDisabledFiltersChanged(this.input.filters, this.prevFilters) || + !_.isEqual(this.prevQuery, this.input.query) || + !_.isEqual(this.prevTimeRange, this.input.timeRange) + ) { + this.filtersSearchSource.setField('filter', this.input.filters); + this.filtersSearchSource.setField('query', this.input.query); - // Sadly this is neccessary to tell the angular component to refetch the data. - this.courier.fetch(); + // Sadly this is neccessary to tell the angular component to refetch the data. + this.courier.fetch(); + + this.prevFilters = this.input.filters; + this.prevQuery = this.input.query; + this.prevTimeRange = this.input.timeRange; + } } } diff --git a/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable.ts b/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable.ts index a59029fc36ef14..c1b9bfd42cc432 100644 --- a/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable.ts +++ b/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable.ts @@ -38,8 +38,8 @@ import { import { Subscription } from 'rxjs'; import * as Rx from 'rxjs'; import { TimeRange } from 'ui/timefilter/time_history'; -import { Query } from 'src/legacy/core_plugins/data/public'; import { Filter } from '@kbn/es-query'; +import { Query, onlyDisabledFiltersChanged } from '../../../../data/public'; import { VISUALIZE_EMBEDDABLE_TYPE } from './constants'; const getKeys = (o: T): Array => Object.keys(o) as Array; @@ -76,6 +76,7 @@ export class VisualizeEmbeddable extends Embeddable { - this.reload(); this.handleChanges(); }); } @@ -138,14 +138,17 @@ export class VisualizeEmbeddable extends Embeddable { - this.uiState.set(key, visCustomizations[key]); - }); - this.uiState.on('change', this.uiStateChangeHandler); + if (!_.isEqual(visCustomizations, this.visCustomizations)) { + this.visCustomizations = visCustomizations; + // Turn this off or the uiStateChangeHandler will fire for every modification. + this.uiState.off('change', this.uiStateChangeHandler); + this.uiState.clearAllKeys(); + this.uiState.set('vis', visCustomizations); + getKeys(visCustomizations).forEach(key => { + this.uiState.set(key, visCustomizations[key]); + }); + this.uiState.on('change', this.uiStateChangeHandler); + } } else { this.uiState.clearAllKeys(); } @@ -157,19 +160,19 @@ export class VisualizeEmbeddable extends Embeddable