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.

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

[Query] Remove es query dependency on format.convert #103174

Merged
merged 6 commits into from
Jun 27, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 3 additions & 1 deletion src/plugins/data/common/es_query/filters/meta_filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* Side Public License, v 1.
*/

import { ConvertFn } from './types';

export enum FilterStateStore {
APP_STATE = 'appState',
GLOBAL_STATE = 'globalState',
Expand Down Expand Up @@ -35,7 +37,7 @@ export type FilterMeta = {
type?: string;
key?: string;
params?: any;
value?: string;
value?: string | ConvertFn;
};

// eslint-disable-next-line
Expand Down
7 changes: 1 addition & 6 deletions src/plugins/data/common/es_query/filters/phrases_filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,6 @@ export const buildPhrasesFilter = (
const type = FILTERS.PHRASES;
const key = field.name;

const format = (f: IFieldType, value: any) =>
f && f.format && f.format.convert ? f.format.convert(value) : value;

const value = params.map((v: any) => format(field, v)).join(', ');

let should;
if (field.scripted) {
should = params.map((v: any) => ({
Expand All @@ -60,7 +55,7 @@ export const buildPhrasesFilter = (
}

return {
meta: { index, type, key, value, params },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually weren't using the field formatter correctly here, showing the non-formatted values on the filter.
I assume this is a regression from the new index patterns changes.

meta: { index, type, key, params },
query: {
bool: {
should,
Expand Down
5 changes: 1 addition & 4 deletions src/plugins/data/common/es_query/filters/range_filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,7 @@ export const getRangeFilterField = (filter: RangeFilter) => {
};

const formatValue = (field: IFieldType, params: any[]) =>
map(params, (val: any, key: string) => get(operators, key) + format(field, val)).join(' ');

const format = (field: IFieldType, value: any) =>
field && field.format && field.format.convert ? field.format.convert(value) : value;
map(params, (val: any, key: string) => get(operators, key) + val).join(' ');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value wan't used, as map_range already builds it's own label.


// Creates a filter where the value for the given field is in the given range
// params should be an object containing `lt`, `lte`, `gt`, and/or `gte`
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/data/common/es_query/filters/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,5 @@ export enum FILTERS {
GEO_POLYGON = 'geo_polygon',
SPATIAL_FILTER = 'spatial_filter',
}

export type ConvertFn = (value: any) => string;
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* 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 { stubIndexPattern, phraseFilter } from 'src/plugins/data/common/stubs';
import { getDisplayValueFromFilter } from './get_display_value';

describe('getDisplayValueFromFilter', () => {
it('returns the value if string', () => {
phraseFilter.meta.value = 'abc';
const displayValue = getDisplayValueFromFilter(phraseFilter, [stubIndexPattern]);
expect(displayValue).toBe('abc');
});

it('returns the value if undefined', () => {
phraseFilter.meta.value = undefined;
const displayValue = getDisplayValueFromFilter(phraseFilter, [stubIndexPattern]);
expect(displayValue).toBe('');
});

it('calls the value function if proivided', () => {
phraseFilter.meta.value = jest.fn((x) => {
return 'abc';
});
const displayValue = getDisplayValueFromFilter(phraseFilter, [stubIndexPattern]);
expect(displayValue).toBe('abc');
expect(phraseFilter.meta.value).toHaveBeenCalledWith(undefined);
});

it('calls the value function if proivided, with formatter', () => {
stubIndexPattern.getFormatterForField = jest.fn().mockReturnValue('banana');
phraseFilter.meta.value = jest.fn((x) => {
return x + 'abc';
});
const displayValue = getDisplayValueFromFilter(phraseFilter, [stubIndexPattern]);
expect(stubIndexPattern.getFormatterForField).toHaveBeenCalledTimes(1);
expect(phraseFilter.meta.value).toHaveBeenCalledWith('banana');
expect(displayValue).toBe('bananaabc');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@ function getValueFormatter(indexPattern?: IIndexPattern, key?: string) {
}

export function getDisplayValueFromFilter(filter: Filter, indexPatterns: IIndexPattern[]): string {
if (typeof filter.meta.value === 'function') {
const { key, value } = filter.meta;
if (typeof value === 'function') {
const indexPattern = getIndexPatternFromFilter(filter, indexPatterns);
const valueFormatter: any = getValueFormatter(indexPattern, filter.meta.key);
return (filter.meta.value as any)(valueFormatter);
const valueFormatter = getValueFormatter(indexPattern, key);
return value(valueFormatter);
} else {
return filter.meta.value || '';
return value || '';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,29 @@
* Side Public License, v 1.
*/

import { Filter, isPhrasesFilter } from '../../../../../common';
import { Filter, FilterValueFormatter, isPhrasesFilter } from '../../../../../common';

const getFormattedValueFn = (params: any) => {
return (formatter?: FilterValueFormatter) => {
return params
.map((v: any) => {
return formatter ? formatter.convert(v) : v;
})
.join(', ');
};
};

export const mapPhrases = (filter: Filter) => {
if (!isPhrasesFilter(filter)) {
throw filter;
}

const { type, key, value, params } = filter.meta;
const { type, key, params } = filter.meta;

return { type, key, value, params };
return {
type,
key,
value: getFormattedValueFn(params),
params,
};
};