Bugfix: Fix TS to avoid assigning filters to undefined (#40249)

* Define getAppState TS correctly, to avoid filter assignment into an undefined state

* Added tests for the case where appState is undefined
This commit is contained in:
Liza Katz 2019-07-03 19:50:34 +03:00 committed by GitHub
parent 1eebc40208
commit aef6223219
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 92 additions and 47 deletions

View file

@ -58,16 +58,6 @@ describe('filter_state_manager', () => {
globalStateStub = new StubState();
const indexPatterns = new StubIndexPatterns();
filterManager = new FilterManager(indexPatterns);
// FilterStateManager is tested indirectly.
// Therefore, we don't need it's instance.
new FilterStateManager(
globalStateStub,
() => {
return appStateStub;
},
filterManager
);
});
afterEach(() => {
@ -76,51 +66,102 @@ describe('filter_state_manager', () => {
}
});
test('should update filter manager global filters', done => {
const f1 = getFilter(FilterStateStore.GLOBAL_STATE, false, false, 'age', 34);
globalStateStub.filters.push(f1);
describe('app_state_undefined', () => {
beforeEach(() => {
// FilterStateManager is tested indirectly.
// Therefore, we don't need it's instance.
new FilterStateManager(
globalStateStub,
() => {
return undefined;
},
filterManager
);
});
setTimeout(() => {
expect(filterManager.getGlobalFilters()).toHaveLength(1);
done();
}, 100);
test('should NOT watch state until both app and global state are defined', done => {
const f1 = getFilter(FilterStateStore.GLOBAL_STATE, false, false, 'age', 34);
globalStateStub.filters.push(f1);
setTimeout(() => {
expect(filterManager.getGlobalFilters()).toHaveLength(0);
done();
}, 100);
});
test('should NOT update app URL when filter manager filters are set', async () => {
appStateStub.save = sinon.stub();
globalStateStub.save = sinon.stub();
const f1 = getFilter(FilterStateStore.APP_STATE, false, false, 'age', 34);
const f2 = getFilter(FilterStateStore.GLOBAL_STATE, false, false, 'age', 34);
await filterManager.setFilters([f1, f2]);
sinon.assert.notCalled(appStateStub.save);
sinon.assert.calledOnce(globalStateStub.save);
});
});
test('should update filter manager app filters', done => {
expect(filterManager.getAppFilters()).toHaveLength(0);
describe('app_state_defined', () => {
beforeEach(() => {
// FilterStateManager is tested indirectly.
// Therefore, we don't need it's instance.
new FilterStateManager(
globalStateStub,
() => {
return appStateStub;
},
filterManager
);
});
const f1 = getFilter(FilterStateStore.APP_STATE, false, false, 'age', 34);
appStateStub.filters.push(f1);
test('should update filter manager global filters', done => {
const f1 = getFilter(FilterStateStore.GLOBAL_STATE, false, false, 'age', 34);
globalStateStub.filters.push(f1);
setTimeout(() => {
expect(filterManager.getAppFilters()).toHaveLength(1);
done();
}, 100);
});
setTimeout(() => {
expect(filterManager.getGlobalFilters()).toHaveLength(1);
done();
}, 100);
});
test('should update URL when filter manager filters are set', async () => {
appStateStub.save = sinon.stub();
globalStateStub.save = sinon.stub();
test('should update filter manager app filters', done => {
expect(filterManager.getAppFilters()).toHaveLength(0);
const f1 = getFilter(FilterStateStore.APP_STATE, false, false, 'age', 34);
const f2 = getFilter(FilterStateStore.GLOBAL_STATE, false, false, 'age', 34);
const f1 = getFilter(FilterStateStore.APP_STATE, false, false, 'age', 34);
appStateStub.filters.push(f1);
await filterManager.setFilters([f1, f2]);
setTimeout(() => {
expect(filterManager.getAppFilters()).toHaveLength(1);
done();
}, 100);
});
sinon.assert.calledOnce(appStateStub.save);
sinon.assert.calledOnce(globalStateStub.save);
});
test('should update URL when filter manager filters are set', async () => {
appStateStub.save = sinon.stub();
globalStateStub.save = sinon.stub();
test('should update URL when filter manager filters are added', async () => {
appStateStub.save = sinon.stub();
globalStateStub.save = sinon.stub();
const f1 = getFilter(FilterStateStore.APP_STATE, false, false, 'age', 34);
const f2 = getFilter(FilterStateStore.GLOBAL_STATE, false, false, 'age', 34);
const f1 = getFilter(FilterStateStore.APP_STATE, false, false, 'age', 34);
const f2 = getFilter(FilterStateStore.GLOBAL_STATE, false, false, 'age', 34);
await filterManager.setFilters([f1, f2]);
await filterManager.addFilters([f1, f2]);
sinon.assert.calledOnce(appStateStub.save);
sinon.assert.calledOnce(globalStateStub.save);
});
sinon.assert.calledOnce(appStateStub.save);
sinon.assert.calledOnce(globalStateStub.save);
test('should update URL when filter manager filters are added', async () => {
appStateStub.save = sinon.stub();
globalStateStub.save = sinon.stub();
const f1 = getFilter(FilterStateStore.APP_STATE, false, false, 'age', 34);
const f2 = getFilter(FilterStateStore.GLOBAL_STATE, false, false, 'age', 34);
await filterManager.addFilters([f1, f2]);
sinon.assert.calledOnce(appStateStub.save);
sinon.assert.calledOnce(globalStateStub.save);
});
});
});

View file

@ -23,6 +23,8 @@ import _ from 'lodash';
import { State } from 'ui/state_management/state';
import { FilterManager } from './filter_manager';
type GetAppStateFunc = () => State | undefined | null;
/**
* FilterStateManager is responsible for watching for filter changes
* and syncing with FilterManager, as well as syncing FilterManager changes
@ -31,12 +33,12 @@ import { FilterManager } from './filter_manager';
export class FilterStateManager {
filterManager: FilterManager;
globalState: State;
getAppState: () => State;
getAppState: GetAppStateFunc;
prevGlobalFilters: Filter[] | undefined;
prevAppFilters: Filter[] | undefined;
interval: NodeJS.Timeout | undefined;
constructor(globalState: State, getAppState: () => State, filterManager: FilterManager) {
constructor(globalState: State, getAppState: GetAppStateFunc, filterManager: FilterManager) {
this.getAppState = getAppState;
this.globalState = globalState;
this.filterManager = filterManager;
@ -63,7 +65,7 @@ export class FilterStateManager {
if (stateUndefined) return;
const globalFilters = this.globalState.filters || [];
const appFilters = appState.filters || [];
const appFilters = (appState && appState.filters) || [];
const globalFilterChanged = !(
this.prevGlobalFilters && _.isEqual(this.prevGlobalFilters, globalFilters)
@ -96,7 +98,9 @@ export class FilterStateManager {
// Update Angular state before saving State objects (which save it to URL)
const partitionedFilters = this.filterManager.getPartitionedFilters();
const appState = this.getAppState();
appState.filters = partitionedFilters.appFilters;
if (appState) {
appState.filters = partitionedFilters.appFilters;
}
this.globalState.filters = partitionedFilters.globalFilters;
this.saveState();
}