Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.x] No reload on changes to disabled filters in dashboard (#41144) #42162

Merged
merged 1 commit into from
Jul 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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