Skip to content

Commit 5a96051

Browse files
alexwizpkibanamachine
alexwizp
and
kibanamachine
committed
[Lens] fix do not submit invalid query in filtered metric (elastic#107542)
* [Lens] Do not submit invalid query in filtered metric Closes: elastic#95611 * fix CI * fix PR comments * fix PR comments * fix PR comment * move closePopover to useCallback * add filter validation to utils/isColumnInvalid Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
1 parent f448fcd commit 5a96051

File tree

4 files changed

+105
-51
lines changed

4 files changed

+105
-51
lines changed

‎x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx

+15-8
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import { ReactWrapper, ShallowWrapper } from 'enzyme';
99
import 'jest-canvas-mock';
10-
import React, { ChangeEvent, MouseEvent, ReactElement } from 'react';
10+
import React, { ChangeEvent, MouseEvent } from 'react';
1111
import { act } from 'react-dom/test-utils';
1212
import {
1313
EuiComboBox,
@@ -16,7 +16,6 @@ import {
1616
EuiRange,
1717
EuiSelect,
1818
EuiButtonIcon,
19-
EuiPopover,
2019
} from '@elastic/eui';
2120
import { DataPublicPluginStart } from '../../../../../../src/plugins/data/public';
2221
import {
@@ -33,7 +32,7 @@ import { documentField } from '../document_field';
3332
import { OperationMetadata } from '../../types';
3433
import { DateHistogramIndexPatternColumn } from '../operations/definitions/date_histogram';
3534
import { getFieldByNameFactory } from '../pure_helpers';
36-
import { Filtering } from './filtering';
35+
import { Filtering, setFilter } from './filtering';
3736
import { TimeShift } from './time_shift';
3837
import { DimensionEditor } from './dimension_editor';
3938
import { AdvancedOptions } from './advanced_options';
@@ -1541,9 +1540,13 @@ describe('IndexPatternDimensionEditorPanel', () => {
15411540
{...getProps({ filter: { language: 'kuery', query: 'a: b' } })}
15421541
/>
15431542
);
1543+
15441544
expect(
1545-
(wrapper.find(Filtering).find(EuiPopover).prop('children') as ReactElement).props.value
1546-
).toEqual({ language: 'kuery', query: 'a: b' });
1545+
wrapper
1546+
.find(Filtering)
1547+
.find('button[data-test-subj="indexPattern-filters-existingFilterTrigger"]')
1548+
.text()
1549+
).toBe(`a: b`);
15471550
});
15481551

15491552
it('should allow to set filter initially', () => {
@@ -1609,11 +1612,15 @@ describe('IndexPatternDimensionEditorPanel', () => {
16091612
const props = getProps({
16101613
filter: { language: 'kuery', query: 'a: b' },
16111614
});
1615+
16121616
wrapper = mount(<IndexPatternDimensionEditorComponent {...props} />);
1613-
(wrapper.find(Filtering).find(EuiPopover).prop('children') as ReactElement).props.onChange({
1614-
language: 'kuery',
1615-
query: 'c: d',
1617+
1618+
act(() => {
1619+
const { updateLayer, columnId, layer } = wrapper.find(Filtering).props();
1620+
1621+
updateLayer(setFilter(columnId, layer, { language: 'kuery', query: 'c: d' }));
16161622
});
1623+
16171624
expect(setState.mock.calls[0]).toEqual([expect.any(Function), { isDimensionComplete: true }]);
16181625
expect(setState.mock.calls[0][0](props.state)).toEqual({
16191626
...props.state,

‎x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/filtering.tsx

+64-27
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,28 @@
44
* 2.0; you may not use this file except in compliance with the Elastic License
55
* 2.0.
66
*/
7-
8-
import { EuiButtonIcon, EuiLink, EuiPanel, EuiPopover } from '@elastic/eui';
9-
import { EuiFormRow, EuiFlexItem, EuiFlexGroup } from '@elastic/eui';
7+
import React, { useState, useEffect, useCallback } from 'react';
108
import { i18n } from '@kbn/i18n';
11-
import React, { useState } from 'react';
12-
import { Query } from 'src/plugins/data/public';
9+
import { isEqual } from 'lodash';
10+
import {
11+
EuiButtonIcon,
12+
EuiLink,
13+
EuiPanel,
14+
EuiPopover,
15+
EuiFormRow,
16+
EuiFlexItem,
17+
EuiFlexGroup,
18+
EuiPopoverProps,
19+
} from '@elastic/eui';
20+
import type { Query } from 'src/plugins/data/public';
1321
import { IndexPatternColumn, operationDefinitionMap } from '../operations';
14-
import { isQueryValid } from '../operations/definitions/filters';
22+
import { validateQuery } from '../operations/definitions/filters';
1523
import { QueryInput } from '../query_input';
16-
import { IndexPattern, IndexPatternLayer } from '../types';
24+
import type { IndexPattern, IndexPatternLayer } from '../types';
25+
26+
const filterByLabel = i18n.translate('xpack.lens.indexPattern.filterBy.label', {
27+
defaultMessage: 'Filter by',
28+
});
1729

1830
// to do: get the language from uiSettings
1931
export const defaultFilter: Query = {
@@ -49,29 +61,49 @@ export function Filtering({
4961
updateLayer: (newLayer: IndexPatternLayer) => void;
5062
isInitiallyOpen: boolean;
5163
}) {
64+
const inputFilter = selectedColumn.filter;
65+
const [queryInput, setQueryInput] = useState(inputFilter ?? defaultFilter);
5266
const [filterPopoverOpen, setFilterPopoverOpen] = useState(isInitiallyOpen);
67+
68+
useEffect(() => {
69+
const { isValid } = validateQuery(queryInput, indexPattern);
70+
71+
if (isValid && !isEqual(inputFilter, queryInput)) {
72+
updateLayer(setFilter(columnId, layer, queryInput));
73+
}
74+
}, [columnId, layer, queryInput, indexPattern, updateLayer, inputFilter]);
75+
76+
const onClosePopup: EuiPopoverProps['closePopover'] = useCallback(() => {
77+
setFilterPopoverOpen(false);
78+
if (inputFilter) {
79+
setQueryInput(inputFilter);
80+
}
81+
}, [inputFilter]);
82+
5383
const selectedOperation = operationDefinitionMap[selectedColumn.operationType];
54-
if (!selectedOperation.filterable || !selectedColumn.filter) {
84+
85+
if (!selectedOperation.filterable || !inputFilter) {
5586
return null;
5687
}
5788

58-
const isInvalid = !isQueryValid(selectedColumn.filter, indexPattern);
89+
const { isValid: isInputFilterValid } = validateQuery(inputFilter, indexPattern);
90+
const { isValid: isQueryInputValid, error: queryInputError } = validateQuery(
91+
queryInput,
92+
indexPattern
93+
);
5994

6095
return (
6196
<EuiFormRow
6297
display="columnCompressed"
98+
label={filterByLabel}
6399
fullWidth
64-
label={i18n.translate('xpack.lens.indexPattern.filterBy.label', {
65-
defaultMessage: 'Filter by',
66-
})}
100+
isInvalid={!isInputFilterValid}
67101
>
68102
<EuiFlexGroup gutterSize="s" alignItems="center">
69103
<EuiFlexItem>
70104
<EuiPopover
71105
isOpen={filterPopoverOpen}
72-
closePopover={() => {
73-
setFilterPopoverOpen(false);
74-
}}
106+
closePopover={onClosePopup}
75107
anchorClassName="eui-fullWidth"
76108
panelClassName="lnsIndexPatternDimensionEditor__filtersEditor"
77109
button={
@@ -85,12 +117,12 @@ export function Filtering({
85117
onClick={() => {
86118
setFilterPopoverOpen(!filterPopoverOpen);
87119
}}
88-
color={isInvalid ? 'danger' : 'text'}
120+
color={isInputFilterValid ? 'text' : 'danger'}
89121
title={i18n.translate('xpack.lens.indexPattern.filterBy.clickToEdit', {
90122
defaultMessage: 'Click to edit',
91123
})}
92124
>
93-
{selectedColumn.filter.query ||
125+
{inputFilter.query ||
94126
i18n.translate('xpack.lens.indexPattern.filterBy.emptyFilterQuery', {
95127
defaultMessage: '(empty)',
96128
})}
@@ -113,16 +145,21 @@ export function Filtering({
113145
</EuiPanel>
114146
}
115147
>
116-
<QueryInput
117-
indexPatternTitle={indexPattern.title}
118-
data-test-subj="indexPattern-filter-by-input"
119-
value={selectedColumn.filter || defaultFilter}
120-
onChange={(newQuery) => {
121-
updateLayer(setFilter(columnId, layer, newQuery));
122-
}}
123-
isInvalid={false}
124-
onSubmit={() => {}}
125-
/>
148+
<EuiFormRow
149+
label={filterByLabel}
150+
isInvalid={!isQueryInputValid}
151+
error={queryInputError}
152+
fullWidth={true}
153+
>
154+
<QueryInput
155+
indexPatternTitle={indexPattern.title}
156+
data-test-subj="indexPattern-filter-by-input"
157+
value={queryInput}
158+
onChange={setQueryInput}
159+
isInvalid={!isQueryInputValid}
160+
onSubmit={() => {}}
161+
/>
162+
</EuiFormRow>
126163
</EuiPopover>
127164
</EuiFlexItem>
128165
</EuiFlexGroup>

‎x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx

+18-15
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,17 @@
66
*/
77

88
import './filters.scss';
9-
109
import React, { MouseEventHandler, useState } from 'react';
10+
import { fromKueryExpression, luceneStringToDsl, toElasticsearchQuery } from '@kbn/es-query';
1111
import { omit } from 'lodash';
1212
import { i18n } from '@kbn/i18n';
1313
import { EuiFormRow, EuiLink, htmlIdGenerator } from '@elastic/eui';
1414
import { updateColumnParam } from '../../layer_helpers';
15-
import { OperationDefinition } from '../index';
16-
import { BaseIndexPatternColumn } from '../column_types';
15+
import type { OperationDefinition } from '../index';
16+
import type { BaseIndexPatternColumn } from '../column_types';
1717
import { FilterPopover } from './filter_popover';
18-
import { IndexPattern } from '../../../types';
19-
import {
20-
AggFunctionsMapping,
21-
Query,
22-
esKuery,
23-
esQuery,
24-
} from '../../../../../../../../src/plugins/data/public';
18+
import type { IndexPattern } from '../../../types';
19+
import type { AggFunctionsMapping, Query } from '../../../../../../../../src/plugins/data/public';
2520
import { queryFilterToAst } from '../../../../../../../../src/plugins/data/common';
2621
import { buildExpressionFunction } from '../../../../../../../../src/plugins/expressions/public';
2722
import { NewBucketButton, DragDropBuckets, DraggableBucketContainer } from '../shared_components';
@@ -58,19 +53,27 @@ const defaultFilter: Filter = {
5853
label: '',
5954
};
6055

61-
export const isQueryValid = (input: Query, indexPattern: IndexPattern) => {
56+
export const validateQuery = (input: Query, indexPattern: IndexPattern) => {
57+
let isValid = true;
58+
let error: string | undefined;
59+
6260
try {
6361
if (input.language === 'kuery') {
64-
esKuery.toElasticsearchQuery(esKuery.fromKueryExpression(input.query), indexPattern);
62+
toElasticsearchQuery(fromKueryExpression(input.query), indexPattern);
6563
} else {
66-
esQuery.luceneStringToDsl(input.query);
64+
luceneStringToDsl(input.query);
6765
}
68-
return true;
6966
} catch (e) {
70-
return false;
67+
isValid = false;
68+
error = e.message;
7169
}
70+
71+
return { isValid, error };
7272
};
7373

74+
export const isQueryValid = (input: Query, indexPattern: IndexPattern) =>
75+
validateQuery(input, indexPattern).isValid;
76+
7477
export interface FiltersIndexPatternColumn extends BaseIndexPatternColumn {
7578
operationType: typeof OPERATION_NAME;
7679
params: {

‎x-pack/plugins/lens/public/indexpattern_datasource/utils.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import type {
1515
import { operationDefinitionMap, IndexPatternColumn } from './operations';
1616

1717
import { getInvalidFieldMessage } from './operations/definitions/helpers';
18+
import { isQueryValid } from './operations/definitions/filters';
1819

1920
/**
2021
* Normalizes the specified operation type. (e.g. document operations
@@ -68,7 +69,13 @@ export function isColumnInvalid(
6869
operationDefinitionMap
6970
);
7071

71-
return (operationErrorMessages && operationErrorMessages.length > 0) || referencesHaveErrors;
72+
const filterHasError = column.filter ? !isQueryValid(column.filter, indexPattern) : false;
73+
74+
return (
75+
(operationErrorMessages && operationErrorMessages.length > 0) ||
76+
referencesHaveErrors ||
77+
filterHasError
78+
);
7279
}
7380

7481
function getReferencesErrors(

0 commit comments

Comments
 (0)