[Unified search] Fixes the wrong operator updates (#148802)

## Summary

Closes https://github.com/elastic/kibana/issues/148785

- Fixes the bug reported on the issue (not released) and another bug
which happens when you do the opposite (from is between to is -
screenshot2)
- Not allows to add a boolean filter is value is not selected
- Default to zero when a user tries to add a phrase filter on a number
field with no value (this is how it worked on 8.6)


![bug](https://user-images.githubusercontent.com/17003240/212303868-db237f61-8ce3-423d-99b3-268496b81198.gif)

<img width="972" alt="image"
src="https://user-images.githubusercontent.com/17003240/212306049-50edde9b-e3a5-4825-a93d-65cb64ee6300.png">


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
This commit is contained in:
Stratoula Kalafateli 2023-01-16 10:10:42 +02:00 committed by GitHub
parent fd7001df9d
commit f129f7fe19
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 277 additions and 12 deletions

View file

@ -0,0 +1,257 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import { buildQueryFilter, Filter, FilterStateStore } from '../build_filters';
import { updateFilter } from './update_filter';
describe('updateFilter', () => {
test('should return the correct filter with type undefined if the given operator is undefined', () => {
const filter: Filter = {
$state: { store: FilterStateStore.GLOBAL_STATE },
...buildQueryFilter({ query_string: { query: 'apache' } }, 'index1', ''),
};
const updatedFilter = updateFilter(filter, 'test-field');
expect(updatedFilter).toStrictEqual({
$state: { store: 'globalState' },
meta: {
alias: '',
field: 'test-field',
index: 'index1',
key: 'test-field',
params: { query: undefined },
type: undefined,
value: undefined,
},
query: undefined,
});
});
test('should return the correct filter if the operator type is exists', () => {
const filter: Filter = {
$state: { store: FilterStateStore.GLOBAL_STATE },
...buildQueryFilter({ query_string: { query: 'apache' } }, 'index1', ''),
};
const operator = {
message: 'exists',
type: 'exists',
negate: false,
};
const updatedFilter = updateFilter(filter, 'test-field', operator);
expect(updatedFilter).toStrictEqual({
$state: { store: 'globalState' },
meta: {
alias: '',
index: 'index1',
params: undefined,
negate: false,
type: 'exists',
value: 'exists',
},
query: { exists: { field: filter.meta.key } },
});
});
test('should return the correct filter if the operator type is exists even if params are given', () => {
const filter: Filter = {
$state: { store: FilterStateStore.GLOBAL_STATE },
...buildQueryFilter({ query_string: { query: 'apache' } }, 'index1', ''),
};
const operator = {
message: 'exists',
type: 'exists',
negate: false,
};
const updatedFilter = updateFilter(filter, 'test-field', operator, [100]);
expect(updatedFilter).toStrictEqual({
$state: { store: 'globalState' },
meta: {
alias: '',
index: 'index1',
params: undefined,
negate: false,
type: 'exists',
value: 'exists',
},
query: { exists: { field: filter.meta.key } },
});
});
test('should return the correct filter for the is operator', () => {
const filter: Filter = {
$state: { store: FilterStateStore.GLOBAL_STATE },
...buildQueryFilter({ query_string: { query: 'apache' } }, 'index1', '', {
key: 'test-field',
}),
};
const operator = {
message: 'is',
type: 'phrase',
negate: true,
};
const updatedFilter = updateFilter(filter, 'test-field', operator, 10);
expect(updatedFilter).toStrictEqual({
$state: { store: 'globalState' },
meta: {
alias: '',
index: 'index1',
params: { query: 10 },
key: 'test-field',
negate: true,
type: 'phrase',
value: undefined,
},
query: { match_phrase: { 'test-field': 10 } },
});
});
test('should return the correct filter for the is operator for number field and no params given', () => {
const filter: Filter = {
$state: { store: FilterStateStore.GLOBAL_STATE },
...buildQueryFilter({ query_string: { query: 'apache' } }, 'index1', '', {
key: 'test-field',
}),
};
const operator = {
message: 'is',
type: 'phrase',
negate: true,
};
const updatedFilter = updateFilter(filter, 'test-field', operator, undefined, 'number');
expect(updatedFilter).toStrictEqual({
$state: { store: 'globalState' },
meta: {
alias: '',
index: 'index1',
params: { query: 0 },
key: 'test-field',
negate: true,
type: 'phrase',
value: undefined,
},
query: { match_phrase: { 'test-field': 0 } },
});
});
test('should return the correct filter for the range operator without params', () => {
const filter: Filter = {
$state: { store: FilterStateStore.GLOBAL_STATE },
...buildQueryFilter({ query_string: { query: 'apache' } }, 'index1', '', {
key: 'test-field',
}),
};
const operator = {
message: 'is between',
type: 'range',
negate: false,
};
const updatedFilter = updateFilter(filter, 'test-field', operator);
expect(updatedFilter).toStrictEqual({
$state: { store: 'globalState' },
meta: {
alias: '',
index: 'index1',
params: {
gte: undefined,
lt: undefined,
},
key: 'test-field',
negate: false,
type: 'range',
},
query: {
range: {
'test-field': {
gte: undefined,
lt: undefined,
},
},
},
});
});
test('should return the correct filter for the range operator with params', () => {
const filter: Filter = {
$state: { store: FilterStateStore.GLOBAL_STATE },
...buildQueryFilter({ query_string: { query: 'apache' } }, 'index1', '', {
key: 'test-field',
}),
};
const operator = {
message: 'is between',
type: 'range',
negate: false,
};
const updatedFilter = updateFilter(filter, 'test-field', operator, {
from: 10,
to: 20,
query: 200,
});
expect(updatedFilter).toStrictEqual({
$state: { store: 'globalState' },
meta: {
alias: '',
index: 'index1',
params: {
gte: 10,
lt: 20,
},
key: 'test-field',
negate: false,
type: 'range',
},
query: {
range: {
'test-field': {
gte: 10,
lt: 20,
},
},
},
});
});
test('should return the correct filter for is one of operator', () => {
const filter: Filter = {
$state: { store: FilterStateStore.GLOBAL_STATE },
...buildQueryFilter({ query_string: { query: 'apache' } }, 'index1', '', {
key: 'test-field',
}),
};
const operator = {
message: 'is one of',
type: 'phrases',
negate: true,
};
const updatedFilter = updateFilter(filter, 'test-field', operator, ['value1', 'value2']);
expect(updatedFilter).toStrictEqual({
$state: { store: 'globalState' },
meta: {
alias: '',
index: 'index1',
params: ['value1', 'value2'],
key: 'test-field',
negate: true,
type: 'phrases',
},
query: {
bool: {
minimum_should_match: 1,
...filter!.query?.should,
should: [
{
match_phrase: { 'test-field': 'value1' },
},
{
match_phrase: { 'test-field': 'value2' },
},
],
},
},
});
});
});

View file

@ -8,7 +8,7 @@
import { identity, pickBy } from 'lodash';
import type { Filter, FilterMeta } from '..';
import type { Filter, FilterMeta, RangeFilterParams } from '..';
type FilterOperator = Pick<FilterMeta, 'type' | 'negate'>;
@ -16,7 +16,8 @@ export const updateFilter = (
filter: Filter,
field?: string,
operator?: FilterOperator,
params?: Filter['meta']['params']
params?: Filter['meta']['params'],
fieldType?: string
) => {
if (!field || !operator) {
return updateField(filter, field);
@ -32,7 +33,7 @@ export const updateFilter = (
return updateWithIsOneOfOperator(filter, operator, params);
}
return updateWithIsOperator(filter, operator, params);
return updateWithIsOperator(filter, operator, params, fieldType);
};
function updateField(filter: Filter, field?: string) {
@ -68,33 +69,37 @@ function updateWithExistsOperator(filter: Filter, operator?: FilterOperator) {
function updateWithIsOperator(
filter: Filter,
operator?: FilterOperator,
params?: Filter['meta']['params']
params?: Filter['meta']['params'],
fieldType?: string
) {
const safeParams = fieldType === 'number' && !params ? 0 : params;
return {
...filter,
meta: {
...filter.meta,
negate: operator?.negate,
type: operator?.type,
params: { ...filter.meta.params, query: params },
params: { ...filter.meta.params, query: safeParams },
value: undefined,
},
query: { match_phrase: { [filter.meta.key!]: params ?? '' } },
query: { match_phrase: { [filter.meta.key!]: safeParams ?? '' } },
};
}
function updateWithRangeOperator(
filter: Filter,
operator: FilterOperator,
rawParams: Array<Filter['meta']['params']>,
rawParams: RangeFilterParams,
field: string
) {
const params = {
...filter.meta.params,
const rangeParams = {
...pickBy(rawParams, identity),
};
params.gte = params.from;
params.lt = params.to;
const params = {
gte: rangeParams?.from,
lt: rangeParams?.to,
};
const updatedFilter = {
...filter,

View file

@ -52,6 +52,8 @@ export function validateParams(params: any, field: DataViewField) {
return isSemverValid(params);
}
return true;
case 'boolean':
return typeof params === 'boolean';
default:
return true;
}

View file

@ -183,7 +183,8 @@ export const updateFilters = (
getFilterByPath(newFilters, dest.path),
field?.name,
operator,
params
params,
field?.type
);
const pathInArray = getPathInArray(dest.path);
const { targetArray } = getContainerMetaByPath(newFilters, pathInArray);