Skip to content

Commit

Permalink
[Security Solution] [Fix] Alert Page Controls do not recover from inv…
Browse files Browse the repository at this point in the history
…alid query. (elastic#156542)

## Summary

This PR handles elastic#156016 .

Previously, if user supplied an invalid kql or lucene query, Alert Page
controls will go in error state and not recover until user reloaded the
page or navigated away and back to the Alert Page.

This PR prevents Alert Page Controls going in that error state.

| Before | After |
|--|--|
| <video
src="https://user-images.githubusercontent.com/7485038/235931286-6da23567-4ae8-454a-92b8-a595a20f5655.mov"
/> | <video
src="https://user-images.githubusercontent.com/7485038/235930584-485df881-d22c-44f3-9d53-f673820eb673.mov"
/> |

### Checklist

Delete any items that are not applicable to this PR.

- [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

(cherry picked from commit ff65ca4)
  • Loading branch information
logeekal committed May 3, 2023
1 parent cc3b077 commit 9c867bd
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,19 @@ const getStoreWithCustomState = (newState: typeof state = state) => {
return createStore(newState, SUB_PLUGINS_REDUCER, kibanaObservable, storage);
};

const TestComponent: FC<ComponentProps<typeof TestProviders>> = (props) => (
const TestComponent: FC<
ComponentProps<typeof TestProviders> & {
filterGroupProps?: Partial<ComponentProps<typeof FilterGroup>>;
}
> = (props) => (
<TestProviders store={getStoreWithCustomState()} {...props}>
<FilterGroup
initialControls={DEFAULT_DETECTION_PAGE_FILTERS}
dataViewId="security-solution-default"
chainingSystem="HIERARCHICAL"
onFilterChange={onFilterChangeMock}
onInit={onInitMock}
{...props.filterGroupProps}
/>
</TestProviders>
);
Expand Down Expand Up @@ -522,6 +527,36 @@ describe(' Filter Group Component ', () => {
expect(screen.queryByTestId(TEST_IDS.SAVE_CHANGE_POPOVER)).toBeVisible();
});
});
it('should update controlGroup with new filters and queries when valid query is supplied', async () => {
const validQuery = { query: { language: 'kuery', query: '' } };
// pass an invalid query
render(<TestComponent filterGroupProps={validQuery} />);

await waitFor(() => {
expect(controlGroupMock.updateInput).toHaveBeenNthCalledWith(
1,
expect.objectContaining({
filters: undefined,
query: validQuery.query,
})
);
});
});

it('should not update controlGroup with new filters and queries when invalid query is supplied', async () => {
const invalidQuery = { query: { language: 'kuery', query: '\\' } };
// pass an invalid query
render(<TestComponent filterGroupProps={invalidQuery} />);

await waitFor(() => {
expect(controlGroupMock.updateInput).toHaveBeenCalledWith(
expect.objectContaining({
filters: [],
query: undefined,
})
);
});
});
});

describe('Filter Changed Banner', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import { FilterGroupContext } from './filter_group_context';
import { NUM_OF_CONTROLS } from './config';
import { TEST_IDS } from './constants';
import { URL_PARAM_ARRAY_EXCEPTION_MSG } from './translations';
import { convertToBuildEsQuery } from '../../lib/kuery';

const FilterWrapper = styled.div.attrs((props) => ({
className: props.className,
Expand Down Expand Up @@ -149,14 +150,41 @@ const FilterGroupComponent = (props: PropsWithChildren<FilterGroupProps>) => {
return cleanup;
}, []);

const { filters: validatedFilters, query: validatedQuery } = useMemo(() => {
const [_, kqlError] = convertToBuildEsQuery({
config: {},
queries: query ? [query] : [],
filters: filters ?? [],
indexPattern: { fields: [], title: '' },
});

// we only need to handle kqlError because control group can handle Lucene error
if (kqlError) {
/*
* Based on the behaviour from other components,
* ignore all filters and queries if there is some error
* in the input filters and queries
*
* */
return {
filters: [],
query: undefined,
};
}
return {
filters,
query,
};
}, [filters, query]);

useEffect(() => {
controlGroup?.updateInput({
filters: validatedFilters,
query: validatedQuery,
timeRange,
filters,
query,
chainingSystem,
});
}, [timeRange, filters, query, chainingSystem, controlGroup]);
}, [timeRange, chainingSystem, controlGroup, validatedQuery, validatedFilters]);

const handleInputUpdates = useCallback(
(newInput: ControlGroupInput) => {
Expand All @@ -171,7 +199,7 @@ const FilterGroupComponent = (props: PropsWithChildren<FilterGroupProps>) => {
[setControlGroupInputUpdates, getStoredControlInput, isViewMode, setHasPendingChanges]
);

const handleFilterUpdates = useCallback(
const handleOutputFilterUpdates = useCallback(
({ filters: newFilters }: ControlGroupOutput) => {
if (isEqual(currentFiltersRef.current, newFilters)) return;
if (onFilterChange) onFilterChange(newFilters ?? []);
Expand All @@ -181,8 +209,8 @@ const FilterGroupComponent = (props: PropsWithChildren<FilterGroupProps>) => {
);

const debouncedFilterUpdates = useMemo(
() => debounce(handleFilterUpdates, 500),
[handleFilterUpdates]
() => debounce(handleOutputFilterUpdates, 500),
[handleOutputFilterUpdates]
);

useEffect(() => {
Expand Down

0 comments on commit 9c867bd

Please sign in to comment.