Skip to content

Commit

Permalink
Query Filter \ Filter Manager: de-angularize and move to data plugin (#…
Browse files Browse the repository at this point in the history
…37311) (#40012)

* Embeddable and Visualize to use type definitions from kbn-es-query package

* moved state watching to new filter manager and removed query filter dependency on rootScope

* Deleted unused pinFilter

* Extracted merge fitlers function

* Changed mappedFilters name

* Move state management to new filter management as well. query filter is now just a wrapper.

* Code works correctly with new setup of query filter

* improved typing

* Moved mapping into new filter manager

* removed promise dependency from query filter

* Extracted filter state manager from query filter

* Moved addFiltersAndChangeTimeFilter logic into new filter manager

* Fixed addFiltersAndChangeTimeFilter implementation

* Moved all logic to new filter manager - query filter is just a wrapper now!

* Connected query_filter observable management to new filter manager as well

* Usage example in discover

* filter state management tests + fire less events from filter state manager

* Tests for filter manager and filter state manager

* Moved new filter manager to data plugin

* Got rid of FilterManagerProvider and turned it into a getFilterGenerator function that works with the new FilterManager.
Need to merge this code with FilterManager.

* Added tests that make sure that filter manager listens to filter state manager

* fix typo

* Fixed action add filter test

* Fixed increasePredecessorCount test

* Fixed increaseSuccessorCount test

* Fixed setPredecessorCount test

* Fixed setQueryParameters test

* Fixed setSuccessorCount test

* Fixed doc table filter actions test

* Fixed filter generator tests

* rename argument

* Moved push filters into vis filters (only place where its used)

* Filter type fix, test fix

* Removed irrelevant filter tests (for deleted methods)
    Added state to filters

* Fixed most of get filters errors by sleeping (TODO!)

* Added destroy methods
Improved merge logic
Improved remove filter tests to use add filter (to avoid having for filter state manager to catch up)

* Fixed discover functional tests
Added default empty filter state in filter generator
Some more browser test adjustments

* Allow filter $state to be empty

* Fixed types

* Code review with @splager
- subscribeWithScope
- query filter to return angular promises
- remove unneeded Promise.resolve

* Separate class\type defenitions from plugin instance setup in shim plugin definition
This helps avoiding circular dependency issues that were obsereved in filter-manager branch (due to code starting to use the data plugin).

* typescript fun

* Fixed merge

* updated some get filter tests to work with add filter, and therefore moved them to add_filters.js

* Remove shouldFetch and add comment explaning the observable.

* Minor code style fixes

* Code review fixes and changes

* Moved Karma tests to jest

* Fixed merge

* Fixed some type errors

* Improved watchFilterState logic

* Removed addFiltersAndChangeTimeFilter test for now, as I'm having TS difficulties

* filters can also be null

* Further watch state fine tuning

* Get data instance inside provider to avoid circular deps

* mock chrome

* Removed change to range filter

* Deleted unnecessary injects

* dedup setFilters as well

* deleted unused subscription

* Added tests for setFilters de-dupe

* Update src/legacy/core_plugins/data/public/filter/filter_manager/filter_state_manager.ts

Co-Authored-By: Stacey Gammon <gammon@elastic.co>

* Update src/legacy/core_plugins/data/public/filter/filter_manager/filter_manager.ts

Co-Authored-By: Stacey Gammon <gammon@elastic.co>

* Code review minor fixes
  • Loading branch information
Liza Katz authored Jul 1, 2019
1 parent 196fa49 commit 00baee5
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe('filter_manager', () => {
let updateListener: sinon.SinonSpy<any[], any>;

let filterManager: FilterManager;
let indexPatterns: any;
let indexPatterns: StubIndexPatterns;
let readyFilters: Filter[];

beforeEach(() => {
Expand Down Expand Up @@ -440,6 +440,24 @@ describe('filter_manager', () => {
expect(globalStateStub.filters.length).toBe(3);
});

test('should de-dupe globalStateStub filters being set', async () => {
const f1 = getFilter(FilterStateStore.GLOBAL_STATE, false, false, 'age', 34);
const f2 = _.cloneDeep(f1);
await filterManager.setFilters([f1, f2]);
expect(filterManager.getAppFilters()).toHaveLength(0);
expect(filterManager.getGlobalFilters()).toHaveLength(1);
expect(filterManager.getFilters()).toHaveLength(1);
});

test('should de-dupe appStateStub filters being set', async () => {
const f1 = getFilter(FilterStateStore.APP_STATE, false, false, 'age', 34);
const f2 = _.cloneDeep(f1);
await filterManager.setFilters([f1, f2]);
expect(filterManager.getAppFilters()).toHaveLength(1);
expect(filterManager.getGlobalFilters()).toHaveLength(0);
expect(filterManager.getFilters()).toHaveLength(1);
});

test('should mutate global filters on appStateStub filter changes', async function() {
const idx = 1;
await filterManager.addFilters(readyFilters, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import { Filter, isFilterPinned, FilterStateStore } from '@kbn/es-query';

import _ from 'lodash';
import { Subject, Subscription } from 'rxjs';
import { Subject } from 'rxjs';

import { npSetup } from 'ui/new_platform';

Expand All @@ -42,20 +42,13 @@ import { IndexPatterns } from '../../index_patterns';
export class FilterManager {
private indexPatterns: IndexPatterns;
private filters: Filter[] = [];
private updated$: Subject<any> = new Subject();
private fetch$: Subject<any> = new Subject();
private updateSubscription$: Subscription | undefined;
private updated$: Subject<void> = new Subject();
private fetch$: Subject<void> = new Subject();

constructor(indexPatterns: IndexPatterns) {
this.indexPatterns = indexPatterns;
}

destroy() {
if (this.updateSubscription$) {
this.updateSubscription$.unsubscribe();
}
}

private mergeIncomingFilters(partitionedFilters: PartitionedFilters): Filter[] {
const globalFilters = partitionedFilters.globalFilters;
const appFilters = partitionedFilters.appFilters;
Expand All @@ -74,11 +67,11 @@ export class FilterManager {
appFilters.splice(i, 1);
});

return uniqFilters(appFilters.reverse().concat(globalFilters.reverse())).reverse();
return FilterManager.mergeFilters(appFilters, globalFilters);
}

private filtersUpdated(newFilters: Filter[]): boolean {
return !_.isEqual(this.filters, newFilters);
private static mergeFilters(appFilters: Filter[], globalFilters: Filter[]): Filter[] {
return uniqFilters(appFilters.reverse().concat(globalFilters.reverse())).reverse();
}

private static partitionFilters(filters: Filter[]): PartitionedFilters {
Expand All @@ -90,9 +83,6 @@ export class FilterManager {
}

private handleStateUpdate(newFilters: Filter[]) {
// This is where the angular update magic \ syncing diget happens
const filtersUpdated = this.filtersUpdated(newFilters);

// global filters should always be first
newFilters.sort(
(a: Filter, b: Filter): number => {
Expand All @@ -106,6 +96,8 @@ export class FilterManager {
}
);

const filtersUpdated = !_.isEqual(this.filters, newFilters);

this.filters = newFilters;
if (filtersUpdated) {
this.updated$.next();
Expand Down Expand Up @@ -155,24 +147,28 @@ export class FilterManager {
pinFilterStatus = uiSettings.get('filters:pinnedByDefault');
}

// set the store of all filters
// TODO: is this necessary?
// Set the store of all filters. For now.
// In the future, all filters should come in with filter state store already set.
const store = pinFilterStatus ? FilterStateStore.GLOBAL_STATE : FilterStateStore.APP_STATE;
FilterManager.setFiltersStore(filters, store);

const mappedFilters = await mapAndFlattenFilters(this.indexPatterns, filters);

// This is where we add new filters to the correct place (app \ global)
const newPartitionedFilters = FilterManager.partitionFilters(mappedFilters);
const partitionFilters = this.getPartitionedFilters();
partitionFilters.appFilters.push(...newPartitionedFilters.appFilters);
partitionFilters.globalFilters.push(...newPartitionedFilters.globalFilters);
const currentFilters = this.getPartitionedFilters();
currentFilters.appFilters.push(...newPartitionedFilters.appFilters);
currentFilters.globalFilters.push(...newPartitionedFilters.globalFilters);

const newFilters = this.mergeIncomingFilters(partitionFilters);
const newFilters = this.mergeIncomingFilters(currentFilters);
this.handleStateUpdate(newFilters);
}

public async setFilters(newFilters: Filter[]) {
const mappedFilters = await mapAndFlattenFilters(this.indexPatterns, newFilters);
this.handleStateUpdate(mappedFilters);
const newPartitionedFilters = FilterManager.partitionFilters(mappedFilters);
const mergedFilters = this.mergeIncomingFilters(newPartitionedFilters);
this.handleStateUpdate(mergedFilters);
}

public removeFilter(filter: Filter) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { FilterManager } from './filter_manager';

/**
* FilterStateManager is responsible for watching for filter changes
* and synching with FilterManager, as well as syncing FilterManager changes
* and syncing with FilterManager, as well as syncing FilterManager changes
* back to the URL.
**/
export class FilterStateManager {
Expand Down

0 comments on commit 00baee5

Please sign in to comment.