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.

Sign up for GitHub

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] Filtering on a search cell first inclusive then exclusive gives bad results - #39802 (#41754) #41857

Merged
merged 1 commit into from
Jul 24, 2019
Merged

[7.x] Filtering on a search cell first inclusive then exclusive gives bad results - #39802 (#41754) #41857

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 18 additions & 23 deletions src/legacy/core_plugins/data/public/filter/filter_manager/filter_manager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,8 +398,8 @@ describe('filter_manager', () => {
});

test('should fire the update and fetch events', async function() {
const updateStub = sinon.stub();
const fetchStub = sinon.stub();
const updateStub = jest.fn();
const fetchStub = jest.fn();

filterManager.getUpdates$().subscribe({
next: updateStub,
Expand All @@ -416,8 +416,8 @@ describe('filter_manager', () => {
expect(globalStateStub.save.callCount).toBe(1);

// this time, events should be emitted
expect(fetchStub.called);
expect(updateStub.called);
expect(fetchStub).toBeCalledTimes(1);
expect(updateStub).toBeCalledTimes(1);
});
});

Expand Down Expand Up @@ -595,8 +595,8 @@ describe('filter_manager', () => {
});

test('should fire the update and fetch events', async function() {
const updateStub = sinon.stub();
const fetchStub = sinon.stub();
const updateStub = jest.fn();
const fetchStub = jest.fn();

await filterManager.addFilters(readyFilters, false);

Expand All @@ -611,8 +611,8 @@ describe('filter_manager', () => {
filterManager.removeFilter(readyFilters[0]);

// this time, events should be emitted
expect(fetchStub.called);
expect(updateStub.called);
expect(fetchStub).toBeCalledTimes(1);
expect(updateStub).toBeCalledTimes(1);
});

test('should remove matching filters', async function() {
Expand Down Expand Up @@ -664,19 +664,12 @@ describe('filter_manager', () => {
});

describe('invert', () => {
test('invert to disabled', async () => {
const f1 = getFilter(FilterStateStore.GLOBAL_STATE, false, false, 'age', 34);
filterManager.invertFilter(f1);
expect(f1.meta.negate).toBe(true);
filterManager.invertFilter(f1);
expect(f1.meta.negate).toBe(false);
});

test('should fire the update and fetch events', function() {
const updateStub = sinon.stub();
const fetchStub = sinon.stub();
test('should fire the update and fetch events', async function() {
await filterManager.addFilters(readyFilters);
expect(filterManager.getFilters()).toHaveLength(3);

filterManager.addFilters(readyFilters);
const updateStub = jest.fn();
const fetchStub = jest.fn();
filterManager.getUpdates$().subscribe({
next: updateStub,
});
Expand All @@ -685,9 +678,11 @@ describe('filter_manager', () => {
next: fetchStub,
});

filterManager.invertFilter(readyFilters[1]);
expect(fetchStub.called);
expect(updateStub.called);
readyFilters[1].meta.negate = !readyFilters[1].meta.negate;
await filterManager.addFilters(readyFilters[1]);
expect(filterManager.getFilters()).toHaveLength(3);
expect(fetchStub).toBeCalledTimes(1);
expect(updateStub).toBeCalledTimes(1);
});
});

Expand Down
12 changes: 6 additions & 6 deletions src/legacy/core_plugins/data/public/filter/filter_manager/filter_manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export class FilterManager {
/* Getters */

public getFilters() {
return this.filters;
return _.cloneDeep(this.filters);
}

public getAppFilters() {
Expand All @@ -119,7 +119,7 @@ export class FilterManager {
}

public getPartitionedFilters(): PartitionedFilters {
return FilterManager.partitionFilters(this.filters);
return FilterManager.partitionFilters(this.getFilters());
}

public getUpdates$() {
Expand All @@ -137,6 +137,10 @@ export class FilterManager {
filters = [filters];
}

if (filters.length === 0) {
return;
}

const { uiSettings } = npSetup.core;
if (pinFilterStatus === undefined) {
pinFilterStatus = uiSettings.get('filters:pinnedByDefault');
Expand Down Expand Up @@ -178,10 +182,6 @@ export class FilterManager {
}
}

public invertFilter(filter: Filter) {
filter.meta.negate = !filter.meta.negate;
}

public async removeAll() {
await this.setFilters([]);
}
Expand Down
6 changes: 5 additions & 1 deletion src/legacy/core_plugins/data/public/filter/filter_manager/lib/map_phrase.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ function getParams(filter, indexPattern) {
// for example a user might manually edit the url or the index pattern's ID might change due to
// external factors e.g. a reindex. We only need the index in order to grab the field formatter, so we fallback
// on displaying the raw value if the index or field is invalid.
const value = (indexPattern && indexPattern.fields.byName[key]) ? indexPattern.fields.byName[key].format.convert(query) : query;
const value = (
indexPattern &&
indexPattern.fields &&
indexPattern.fields.byName[key]
) ? indexPattern.fields.byName[key].format.convert(query) : query;
return { type, key, value, params };
}

Expand Down
1 change: 0 additions & 1 deletion ...legacy/core_plugins/kibana/public/context/query_parameters/__tests__/action_add_filter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ describe('context app', function () {
function createQueryFilterStub() {
return {
addFilters: sinon.stub(),
invertFilter: sinon.stub(),
getAppFilters: sinon.stub(),
};
}
21 changes: 14 additions & 7 deletions src/legacy/ui/public/filter_manager/__tests__/filter_generator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import expect from '@kbn/expect';
import ngMock from 'ng_mock';
import { getFilterGenerator } from '..';
import { FilterBarQueryFilterProvider } from '../../filter_manager/query_filter';
import { uniqFilters } from '../../../../core_plugins/data/public/filter/filter_manager/lib/uniq_filters';
import { getPhraseScript } from '@kbn/es-query';
let queryFilter;
let filterGen;
Expand Down Expand Up @@ -63,10 +64,7 @@ describe('Filter Manager', function () {
sinon.stub(queryFilter, 'getAppFilters').callsFake(() => appState.filters);
sinon.stub(queryFilter, 'addFilters').callsFake((filters) => {
if (!Array.isArray(filters)) filters = [filters];
appState.filters = appState.filters.concat(filters);
});
sinon.stub(queryFilter, 'invertFilter').callsFake((filter) => {
filter.meta.negate = !filter.meta.negate;
appState.filters = uniqFilters(appState.filters.concat(filters));
});
}));

Expand Down Expand Up @@ -116,7 +114,10 @@ describe('Filter Manager', function () {

// NOTE: negating exists filters also forces disabled to false
filterGen.add('myField', 1, '-', 'myIndex');
checkAddFilters(0, null, 1);
checkAddFilters(1, [{
meta: { index: 'myIndex', negate: true, disabled: false },
query: { match: { myField: { query: 1, type: 'phrase' } } }
}], 1);
expect(appState.filters).to.have.length(1);

filterGen.add('_exists_', 'myField', '+', 'myIndex');
Expand All @@ -127,7 +128,10 @@ describe('Filter Manager', function () {
expect(appState.filters).to.have.length(2);

filterGen.add('_exists_', 'myField', '-', 'myIndex');
checkAddFilters(0, null, 3);
checkAddFilters(1, [{
meta: { index: 'myIndex', negate: true, disabled: false },
exists: { field: 'myField' }
}], 3);
expect(appState.filters).to.have.length(2);

const scriptedField = { name: 'scriptedField', scripted: true, script: 1, lang: 'painless' };
Expand All @@ -139,7 +143,10 @@ describe('Filter Manager', function () {
expect(appState.filters).to.have.length(3);

filterGen.add(scriptedField, 1, '-', 'myIndex');
checkAddFilters(0, null, 5);
checkAddFilters(1, [{
meta: { index: 'myIndex', negate: true, disabled: false, field: 'scriptedField' },
script: getPhraseScript(scriptedField, 1)
}], 5);
expect(appState.filters).to.have.length(3);
});

Expand Down
3 changes: 2 additions & 1 deletion src/legacy/ui/public/filter_manager/filter_generator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,9 @@ export function getFilterGenerator(queryFilter) {
if (existing) {
existing.meta.disabled = false;
if (existing.meta.negate !== negate) {
queryFilter.invertFilter(existing);
existing.meta.negate = !existing.meta.negate;
}
newFilters.push(existing);
return;
}

Expand Down
1 change: 0 additions & 1 deletion src/legacy/ui/public/filter_manager/query_filter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export function FilterBarQueryFilterProvider(getAppState, globalState) {
queryFilter.getAppFilters = filterManager.getAppFilters.bind(filterManager);
queryFilter.getGlobalFilters = filterManager.getGlobalFilters.bind(filterManager);
queryFilter.removeFilter = filterManager.removeFilter.bind(filterManager);
queryFilter.invertFilter = filterManager.invertFilter.bind(filterManager);
queryFilter.addFilters = filterManager.addFilters.bind(filterManager);
queryFilter.setFilters = filterManager.setFilters.bind(filterManager);
queryFilter.addFiltersAndChangeTimeFilter = filterManager.addFiltersAndChangeTimeFilter.bind(filterManager);
Expand Down