Skip to content

Commit

Permalink
[Lens] do not pass incorrect filters to the state (elastic#189292)
Browse files Browse the repository at this point in the history
## Summary

Before we would pass incorrect filters to the state and save it and then
in the renderer filter whatever we couldn't process.
That unfortunately lead to the bug in another piece of code - when
creating a link to explore underlying data in discover.
This code is aligned with how we do filtering for filtered metric - we
don't pass the filter to the state, unless it is valid.
  • Loading branch information
mbondyra authored Jul 30, 2024
1 parent b9356a8 commit 2ed5fd9
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,21 @@ import { FilterPopover } from './filter_popover';
import { LabelInput } from '../shared_components';
import { QueryStringInput } from '@kbn/unified-search-plugin/public';
import { QueryInput } from '@kbn/visualization-ui-components';
import { Query } from '@kbn/es-query';

jest.mock('.', () => ({
isQueryValid: () => true,
defaultLabel: 'label',
}));
jest.mock('.', () => ({}));

jest.mock('@kbn/visualization-ui-components', () => {
const original = jest.requireActual('@kbn/visualization-ui-components');

return {
...original,
isQueryValid: jest.fn((q: Query) => (q.query === 'bytes >= 1 and' ? false : true)),
};
});

jest.mock('@kbn/unified-search-plugin/public', () => ({
QueryStringInput: () => {
return 'QueryStringInput';
},
QueryStringInput: () => 'QueryStringInput',
}));

describe('filter popover', () => {
Expand Down Expand Up @@ -118,6 +123,16 @@ describe('filter popover', () => {
});
});

it('should not call setFilter if QueryInput value is not valid', () => {
const setFilter = jest.fn();
const instance = shallow(<FilterPopover {...defaultProps} setFilter={setFilter} />);
instance.find(QueryInput).prop('onChange')!({
query: 'bytes >= 1 and',
language: 'kuery',
});
expect(setFilter).not.toHaveBeenCalled();
});

it('should call setFilter when modifying LabelInput', () => {
const setFilter = jest.fn();
const instance = shallow(<FilterPopover {...defaultProps} setFilter={setFilter} />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import './filter_popover.scss';

import React from 'react';
import React, { useState } from 'react';
import { EuiPopover, EuiSpacer } from '@elastic/eui';
import type { Query } from '@kbn/es-query';
// Need to keep it separate to make it work Jest mocks in dimension_panel tests
Expand Down Expand Up @@ -36,9 +36,18 @@ export const FilterPopover = ({
triggerClose: () => void;
}) => {
const inputRef = React.useRef<HTMLInputElement>();
const [localFilter, setLocalFilter] = useState(() => filter);

const setFilterLabel = (label: string) => setFilter({ ...filter, label });
const setFilterQuery = (input: Query) => setFilter({ ...filter, input });
const setFilterLabel = (label: string) => {
setLocalFilter({ ...localFilter, label });
setFilter({ ...filter, label });
};
const setFilterQuery = (input: Query) => {
setLocalFilter({ ...localFilter, input });
if (isQueryValid(input, indexPattern)) {
setFilter({ ...filter, input });
}
};

const getPlaceholder = (query: Query['query']) => {
if (query === '') {
Expand All @@ -49,19 +58,23 @@ export const FilterPopover = ({
return String(query);
}
};
const closePopover = () => {
setLocalFilter({ ...localFilter, input: filter.input });
triggerClose();
};

return (
<EuiPopover
data-test-subj="indexPattern-filters-existingFilterContainer"
panelClassName="lnsIndexPatternDimensionEditor__filtersEditor"
isOpen={isOpen}
ownFocus
closePopover={triggerClose}
closePopover={closePopover}
button={button}
>
<QueryInput
isInvalid={!isQueryValid(filter.input, indexPattern)}
value={filter.input}
isInvalid={!isQueryValid(localFilter.input, indexPattern)}
value={localFilter.input}
dataView={
indexPattern.id
? { type: 'id', value: indexPattern.id }
Expand All @@ -77,11 +90,11 @@ export const FilterPopover = ({
/>
<EuiSpacer size="s" />
<LabelInput
value={filter.label || ''}
value={localFilter.label || ''}
onChange={setFilterLabel}
placeholder={getPlaceholder(filter.input.query)}
placeholder={getPlaceholder(localFilter.input.query)}
inputRef={inputRef}
onSubmit={triggerClose}
onSubmit={closePopover}
dataTestSubj="indexPattern-filters-label"
/>
</EuiPopover>
Expand Down

0 comments on commit 2ed5fd9

Please sign in to comment.