From 4d4d98ec48f8fc0ee3d08a0c656ca14c2530ec0e Mon Sep 17 00:00:00 2001 From: Liza Katz Date: Mon, 1 Jul 2019 15:57:57 +0300 Subject: [PATCH] Query Filter \ Filter Manager: de-angularize and move to data plugin (#37311) * 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 * Update src/legacy/core_plugins/data/public/filter/filter_manager/filter_manager.ts Co-Authored-By: Stacey Gammon * Code review minor fixes --- .../filter_manager/filter_manager.test.ts | 20 ++++++++- .../filter/filter_manager/filter_manager.ts | 42 +++++++++---------- .../filter_manager/filter_state_manager.ts | 2 +- 3 files changed, 39 insertions(+), 25 deletions(-) diff --git a/src/legacy/core_plugins/data/public/filter/filter_manager/filter_manager.test.ts b/src/legacy/core_plugins/data/public/filter/filter_manager/filter_manager.test.ts index 5df1e286fa289..3e264760a148b 100644 --- a/src/legacy/core_plugins/data/public/filter/filter_manager/filter_manager.test.ts +++ b/src/legacy/core_plugins/data/public/filter/filter_manager/filter_manager.test.ts @@ -70,7 +70,7 @@ describe('filter_manager', () => { let updateListener: sinon.SinonSpy; let filterManager: FilterManager; - let indexPatterns: any; + let indexPatterns: StubIndexPatterns; let readyFilters: Filter[]; beforeEach(() => { @@ -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); diff --git a/src/legacy/core_plugins/data/public/filter/filter_manager/filter_manager.ts b/src/legacy/core_plugins/data/public/filter/filter_manager/filter_manager.ts index de765c0200365..dfa7bc9c216bc 100644 --- a/src/legacy/core_plugins/data/public/filter/filter_manager/filter_manager.ts +++ b/src/legacy/core_plugins/data/public/filter/filter_manager/filter_manager.ts @@ -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'; @@ -42,20 +42,13 @@ import { IndexPatterns } from '../../index_patterns'; export class FilterManager { private indexPatterns: IndexPatterns; private filters: Filter[] = []; - private updated$: Subject = new Subject(); - private fetch$: Subject = new Subject(); - private updateSubscription$: Subscription | undefined; + private updated$: Subject = new Subject(); + private fetch$: Subject = 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; @@ -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 { @@ -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 => { @@ -106,6 +96,8 @@ export class FilterManager { } ); + const filtersUpdated = !_.isEqual(this.filters, newFilters); + this.filters = newFilters; if (filtersUpdated) { this.updated$.next(); @@ -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) { diff --git a/src/legacy/core_plugins/data/public/filter/filter_manager/filter_state_manager.ts b/src/legacy/core_plugins/data/public/filter/filter_manager/filter_state_manager.ts index d9c5af9a6c52f..4fec67df919f4 100644 --- a/src/legacy/core_plugins/data/public/filter/filter_manager/filter_state_manager.ts +++ b/src/legacy/core_plugins/data/public/filter/filter_manager/filter_state_manager.ts @@ -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 {