Skip to content

Commit

Permalink
No reload on changes to disabled filters in dashboard (#41144)
Browse files Browse the repository at this point in the history
  • Loading branch information
flash1293 authored Jul 29, 2019
1 parent c5e57e8 commit 1258ca6
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 } },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -94,6 +96,10 @@ export class SearchEmbeddable extends Embeddable<SearchInput, SearchOutput>
public readonly type = SEARCH_EMBEDDABLE_TYPE;
private filterGen: FilterManager;

private prevTimeRange?: TimeRange;
private prevFilters?: Filter[];
private prevQuery?: Query;

constructor(
{
$rootScope,
Expand Down Expand Up @@ -259,10 +265,20 @@ export class SearchEmbeddable extends Embeddable<SearchInput, SearchOutput>
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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <T extends {}>(o: T): Array<keyof T> => Object.keys(o) as Array<keyof T>;
Expand Down Expand Up @@ -76,6 +76,7 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut
private query?: Query;
private title?: string;
private filters?: Filter[];
private visCustomizations: VisualizeInput['vis'];
private subscription: Subscription;
public readonly type = VISUALIZE_EMBEDDABLE_TYPE;

Expand Down Expand Up @@ -112,7 +113,6 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut
this.uiState.on('change', this.uiStateChangeHandler);

this.subscription = Rx.merge(this.getOutput$(), this.getInput$()).subscribe(() => {
this.reload();
this.handleChanges();
});
}
Expand All @@ -138,14 +138,17 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut
// pass anything from here to the handler.update method
const visCustomizations = this.input.vis;
if (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);
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();
}
Expand All @@ -157,19 +160,19 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut
const updatedParams: VisualizeUpdateParams = {};

// Check if timerange has changed
if (this.input.timeRange !== this.timeRange) {
if (!_.isEqual(this.input.timeRange, this.timeRange)) {
this.timeRange = _.cloneDeep(this.input.timeRange);
updatedParams.timeRange = this.timeRange;
}

// Check if filters has changed
if (this.input.filters !== this.filters) {
if (!onlyDisabledFiltersChanged(this.input.filters, this.filters)) {
updatedParams.filters = this.input.filters;
this.filters = this.input.filters;
}

// Check if query has changed
if (this.input.query !== this.query) {
if (!_.isEqual(this.input.query, this.query)) {
updatedParams.query = this.input.query;
this.query = this.input.query;
}
Expand All @@ -183,6 +186,7 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut

if (this.handler && !_.isEmpty(updatedParams)) {
this.handler.update(updatedParams);
this.handler.reload();
}
}

Expand Down Expand Up @@ -211,8 +215,8 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut
// Append visualization to container instead of replacing its content
append: true,
timeRange: _.cloneDeep(this.input.timeRange),
query: this.input.query,
filters: this.input.filters,
query: this.query,
filters: this.filters,
cssClass: `panel-content panel-content--fullWidth`,
dataAttrs,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
Embeddable,
executeTriggerActions
} from '../../../../../../src/legacy/core_plugins/embeddable_api/public/index';
import { onlyDisabledFiltersChanged } from '../../../../../../src/legacy/core_plugins/data/public';
import { I18nContext } from 'ui/i18n';

import { GisMap } from '../connected_components/gis_map';
Expand Down Expand Up @@ -65,7 +66,7 @@ export class MapEmbeddable extends Embeddable {
onContainerStateChanged(containerState) {
if (!_.isEqual(containerState.timeRange, this._prevTimeRange) ||
!_.isEqual(containerState.query, this._prevQuery) ||
!_.isEqual(containerState.filters, this._prevFilters)) {
!onlyDisabledFiltersChanged(containerState.filters, this._prevFilters)) {
this._dispatchSetQuery(containerState);
}

Expand Down

0 comments on commit 1258ca6

Please sign in to comment.