diff --git a/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_exists.tsx b/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_exists.tsx index f2161e376eab5..4d38a80465bac 100644 --- a/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_exists.tsx +++ b/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_exists.tsx @@ -4,24 +4,28 @@ * you may not use this file except in compliance with the Elastic License. */ import React from 'react'; -import { EuiComboBox } from '@elastic/eui'; +import { EuiFormRow, EuiComboBox } from '@elastic/eui'; interface AutocompleteFieldExistsProps { placeholder: string; + rowLabel?: string; } export const AutocompleteFieldExistsComponent: React.FC = ({ placeholder, + rowLabel, }): JSX.Element => ( - + + + ); AutocompleteFieldExistsComponent.displayName = 'AutocompleteFieldExists'; diff --git a/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_lists.tsx b/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_lists.tsx index 4349e70594ecb..28a73801c8c0f 100644 --- a/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_lists.tsx +++ b/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_lists.tsx @@ -4,12 +4,13 @@ * you may not use this file except in compliance with the Elastic License. */ import React, { useState, useEffect, useCallback, useMemo } from 'react'; -import { EuiComboBoxOptionOption, EuiComboBox } from '@elastic/eui'; +import { EuiFormRow, EuiComboBoxOptionOption, EuiComboBox } from '@elastic/eui'; import { IFieldType } from '../../../../../../../src/plugins/data/common'; import { useFindLists, ListSchema } from '../../../lists_plugin_deps'; import { useKibana } from '../../../common/lib/kibana'; -import { getGenericComboBoxProps, paramIsValid } from './helpers'; +import { getGenericComboBoxProps } from './helpers'; +import * as i18n from './translations'; interface AutocompleteFieldListsProps { placeholder: string; @@ -19,11 +20,13 @@ interface AutocompleteFieldListsProps { isDisabled: boolean; isClearable: boolean; isRequired?: boolean; + rowLabel?: string; onChange: (arg: ListSchema) => void; } export const AutocompleteFieldListsComponent: React.FC = ({ placeholder, + rowLabel, selectedField, selectedValue, isLoading = false, @@ -32,7 +35,7 @@ export const AutocompleteFieldListsComponent: React.FC { - const [touched, setIsTouched] = useState(false); + const [error, setError] = useState(undefined); const { http } = useKibana().services; const [lists, setLists] = useState([]); const { loading, result, start } = useFindLists(); @@ -75,7 +78,9 @@ export const AutocompleteFieldListsComponent: React.FC setIsTouched(true), [setIsTouched]); + const setIsTouchedValue = useCallback((): void => { + setError(selectedValue == null ? i18n.FIELD_REQUIRED_ERR : undefined); + }, [selectedValue]); useEffect(() => { if (result != null) { @@ -93,30 +98,27 @@ export const AutocompleteFieldListsComponent: React.FC paramIsValid(selectedValue, selectedField, isRequired, touched), - [selectedField, selectedValue, isRequired, touched] - ); - const isLoadingState = useMemo((): boolean => isLoading || loading, [isLoading, loading]); return ( - + + + ); }; diff --git a/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match.test.tsx b/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match.test.tsx index 94040ccb639be..077489fa83241 100644 --- a/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match.test.tsx @@ -5,9 +5,10 @@ */ import React from 'react'; import { ThemeProvider } from 'styled-components'; -import { mount } from 'enzyme'; +import { mount, ReactWrapper } from 'enzyme'; import euiLightVars from '@elastic/eui/dist/eui_theme_light.json'; -import { EuiComboBox, EuiComboBoxOptionOption } from '@elastic/eui'; +import { EuiSuperSelect, EuiComboBox, EuiComboBoxOptionOption } from '@elastic/eui'; +import { act } from '@testing-library/react'; import { fields, @@ -15,22 +16,58 @@ import { } from '../../../../../../../src/plugins/data/common/index_patterns/fields/fields.mocks'; import { AutocompleteFieldMatchComponent } from './field_value_match'; import { useFieldValueAutocomplete } from './hooks/use_field_value_autocomplete'; + jest.mock('./hooks/use_field_value_autocomplete'); describe('AutocompleteFieldMatchComponent', () => { + let wrapper: ReactWrapper; + const getValueSuggestionsMock = jest .fn() - .mockResolvedValue([false, ['value 3', 'value 4'], jest.fn()]); + .mockResolvedValue([false, true, ['value 3', 'value 4'], jest.fn()]); - beforeAll(() => { + beforeEach(() => { (useFieldValueAutocomplete as jest.Mock).mockReturnValue([ false, + true, ['value 1', 'value 2'], getValueSuggestionsMock, ]); }); + + afterEach(() => { + jest.clearAllMocks(); + wrapper.unmount(); + }); + + test('it renders row label if one passed in', () => { + wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + expect( + wrapper.find('[data-test-subj="valuesAutocompleteMatchLabel"] label').at(0).text() + ).toEqual('Row Label'); + }); + test('it renders disabled if "isDisabled" is true', () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { }} isLoading={false} isClearable={false} - isDisabled={true} + isDisabled onChange={jest.fn()} + onError={jest.fn()} /> ); expect( - wrapper - .find(`[data-test-subj="valuesAutocompleteComboBox matchComboxBox"] input`) - .prop('disabled') + wrapper.find('[data-test-subj="valuesAutocompleteMatch"] input').prop('disabled') ).toBeTruthy(); }); test('it renders loading if "isLoading" is true', () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { title: 'logstash-*', fields, }} - isLoading={true} + isLoading isClearable={false} isDisabled={false} onChange={jest.fn()} + onError={jest.fn()} /> ); - wrapper - .find(`[data-test-subj="valuesAutocompleteComboBox matchComboxBox"] button`) - .at(0) - .simulate('click'); + wrapper.find('[data-test-subj="valuesAutocompleteMatch"] button').at(0).simulate('click'); expect( wrapper - .find( - `EuiComboBoxOptionsList[data-test-subj="valuesAutocompleteComboBox matchComboxBox-optionsList"]` - ) + .find('EuiComboBoxOptionsList[data-test-subj="valuesAutocompleteMatch-optionsList"]') .prop('isLoading') ).toBeTruthy(); }); test('it allows user to clear values if "isClearable" is true', () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { isClearable={true} isDisabled={false} onChange={jest.fn()} + onError={jest.fn()} /> ); expect( wrapper - .find(`[data-test-subj="comboBoxInput"]`) + .find('[data-test-subj="comboBoxInput"]') .hasClass('euiComboBox__inputWrap-isClearable') ).toBeTruthy(); }); test('it correctly displays selected value', () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { isClearable={false} isDisabled={false} onChange={jest.fn()} + onError={jest.fn()} /> ); expect( - wrapper - .find(`[data-test-subj="valuesAutocompleteComboBox matchComboxBox"] EuiComboBoxPill`) - .at(0) - .text() + wrapper.find('[data-test-subj="valuesAutocompleteMatch"] EuiComboBoxPill').at(0).text() ).toEqual('126.45.211.34'); }); test('it invokes "onChange" when new value created', async () => { const mockOnChange = jest.fn(); - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { isClearable={false} isDisabled={false} onChange={mockOnChange} + onError={jest.fn()} /> ); @@ -173,7 +205,7 @@ describe('AutocompleteFieldMatchComponent', () => { test('it invokes "onChange" when new value selected', async () => { const mockOnChange = jest.fn(); - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { isClearable={false} isDisabled={false} onChange={mockOnChange} + onError={jest.fn()} /> ); @@ -199,9 +232,8 @@ describe('AutocompleteFieldMatchComponent', () => { expect(mockOnChange).toHaveBeenCalledWith('value 1'); }); - test('it invokes updateSuggestions when new value searched', async () => { - const mockOnChange = jest.fn(); - const wrapper = mount( + test('it refreshes autocomplete with search query when new value searched', () => { + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { isLoading={false} isClearable={false} isDisabled={false} - onChange={mockOnChange} + onChange={jest.fn()} + onError={jest.fn()} /> ); + act(() => { + ((wrapper.find(EuiComboBox).props() as unknown) as { + onSearchChange: (a: string) => void; + }).onSearchChange('value 1'); + }); - ((wrapper.find(EuiComboBox).props() as unknown) as { - onSearchChange: (a: string) => void; - }).onSearchChange('value 1'); - - expect(getValueSuggestionsMock).toHaveBeenCalledWith({ - fieldSelected: getField('machine.os.raw'), - patterns: { + expect(useFieldValueAutocomplete).toHaveBeenCalledWith({ + selectedField: getField('machine.os.raw'), + operatorType: 'match', + query: 'value 1', + fieldValue: '', + indexPattern: { id: '1234', title: 'logstash-*', fields, }, - value: 'value 1', + }); + }); + + describe('boolean type', () => { + const valueSuggestionsMock = jest.fn().mockResolvedValue([false, false, [], jest.fn()]); + + beforeEach(() => { + (useFieldValueAutocomplete as jest.Mock).mockReturnValue([ + false, + false, + [], + valueSuggestionsMock, + ]); + }); + + test('it displays only two options - "true" or "false"', () => { + wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + expect( + wrapper.find('[data-test-subj="valuesAutocompleteMatchBoolean"]').exists() + ).toBeTruthy(); + expect( + wrapper.find('[data-test-subj="valuesAutocompleteMatchBoolean"]').at(0).prop('options') + ).toEqual([ + { + inputDisplay: 'true', + value: 'true', + }, + { + inputDisplay: 'false', + value: 'false', + }, + ]); + }); + + test('it invokes "onChange" with "true" when selected', () => { + const mockOnChange = jest.fn(); + wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + ((wrapper.find(EuiSuperSelect).props() as unknown) as { + onChange: (a: string) => void; + }).onChange('true'); + + expect(mockOnChange).toHaveBeenCalledWith('true'); + }); + + test('it invokes "onChange" with "false" when selected', () => { + const mockOnChange = jest.fn(); + wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + ((wrapper.find(EuiSuperSelect).props() as unknown) as { + onChange: (a: string) => void; + }).onChange('false'); + + expect(mockOnChange).toHaveBeenCalledWith('false'); + }); + }); + + describe('number type', () => { + const valueSuggestionsMock = jest.fn().mockResolvedValue([false, false, [], jest.fn()]); + + beforeEach(() => { + (useFieldValueAutocomplete as jest.Mock).mockReturnValue([ + false, + false, + [], + valueSuggestionsMock, + ]); + }); + + test('it number input when field type is number', () => { + wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + expect( + wrapper.find('[data-test-subj="valueAutocompleteFieldMatchNumber"]').exists() + ).toBeTruthy(); + }); + + test('it invokes "onChange" with numeric value when inputted', () => { + const mockOnChange = jest.fn(); + wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + wrapper + .find('[data-test-subj="valueAutocompleteFieldMatchNumber"] input') + .at(0) + .simulate('change', { target: { value: '8' } }); + + expect(mockOnChange).toHaveBeenCalledWith('8'); }); }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match.tsx b/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match.tsx index dfb3761bb3497..2b729a17b04e1 100644 --- a/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match.tsx +++ b/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match.tsx @@ -3,8 +3,14 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import React, { useCallback, useMemo, useState } from 'react'; -import { EuiComboBoxOptionOption, EuiComboBox } from '@elastic/eui'; +import React, { useCallback, useMemo, useState, useEffect } from 'react'; +import { + EuiSuperSelect, + EuiFormRow, + EuiFieldNumber, + EuiComboBoxOptionOption, + EuiComboBox, +} from '@elastic/eui'; import { uniq } from 'lodash'; import { IFieldType, IIndexPattern } from '../../../../../../../src/plugins/data/common'; @@ -24,11 +30,14 @@ interface AutocompleteFieldMatchProps { isClearable: boolean; isRequired?: boolean; fieldInputWidth?: number; + rowLabel?: string; onChange: (arg: string) => void; + onError?: (arg: boolean) => void; } export const AutocompleteFieldMatchComponent: React.FC = ({ placeholder, + rowLabel, selectedField, selectedValue, indexPattern, @@ -38,24 +47,45 @@ export const AutocompleteFieldMatchComponent: React.FC { + const [searchQuery, setSearchQuery] = useState(''); const [touched, setIsTouched] = useState(false); - const [isLoadingSuggestions, suggestions, updateSuggestions] = useFieldValueAutocomplete({ + const [error, setError] = useState(undefined); + const [isLoadingSuggestions, isSuggestingValues, suggestions] = useFieldValueAutocomplete({ selectedField, operatorType: OperatorTypeEnum.MATCH, fieldValue: selectedValue, + query: searchQuery, indexPattern, }); const getLabel = useCallback((option: string): string => option, []); const optionsMemo = useMemo((): string[] => { const valueAsStr = String(selectedValue); - return selectedValue ? uniq([valueAsStr, ...suggestions]) : suggestions; + return selectedValue != null && selectedValue.trim() !== '' + ? uniq([valueAsStr, ...suggestions]) + : suggestions; }, [suggestions, selectedValue]); const selectedOptionsMemo = useMemo((): string[] => { const valueAsStr = String(selectedValue); return selectedValue ? [valueAsStr] : []; }, [selectedValue]); + const handleError = useCallback( + (err: string | undefined): void => { + setError((existingErr): string | undefined => { + const oldErr = existingErr != null; + const newErr = err != null; + if (oldErr !== newErr && onError != null) { + onError(newErr); + } + + return err; + }); + }, + [setError, onError] + ); + const { comboOptions, labels, selectedComboOptions } = useMemo( (): GetGenericComboBoxPropsReturn => getGenericComboBoxProps({ @@ -66,59 +96,182 @@ export const AutocompleteFieldMatchComponent: React.FC { - const [newValue] = newOptions.map(({ label }) => optionsMemo[labels.indexOf(label)]); - onChange(newValue ?? ''); - }; + const handleValuesChange = useCallback( + (newOptions: EuiComboBoxOptionOption[]): void => { + const [newValue] = newOptions.map(({ label }) => optionsMemo[labels.indexOf(label)]); + handleError(undefined); + onChange(newValue ?? ''); + }, + [handleError, labels, onChange, optionsMemo] + ); - const onSearchChange = (searchVal: string): void => { - if (updateSuggestions != null) { - updateSuggestions({ - fieldSelected: selectedField, - value: searchVal, - patterns: indexPattern, - }); - } - }; + const handleSearchChange = useCallback( + (searchVal: string): void => { + if (searchVal !== '' && selectedField != null) { + const err = paramIsValid(searchVal, selectedField, isRequired, touched); + handleError(err); - const isValid = useMemo( - (): boolean => paramIsValid(selectedValue, selectedField, isRequired, touched), - [selectedField, selectedValue, isRequired, touched] + setSearchQuery(searchVal); + } + }, + [handleError, isRequired, selectedField, touched] ); - const setIsTouchedValue = useCallback((): void => setIsTouched(true), [setIsTouched]); + const handleCreateOption = useCallback( + (option: string): boolean | undefined => { + const err = paramIsValid(option, selectedField, isRequired, touched); + handleError(err); - const inputPlaceholder = useMemo( - (): string => (isLoading || isLoadingSuggestions ? i18n.LOADING : placeholder), - [isLoading, isLoadingSuggestions, placeholder] + if (err != null) { + // Explicitly reject the user's input + return false; + } else { + onChange(option); + } + }, + [isRequired, onChange, selectedField, touched, handleError] ); + const handleNonComboBoxInputChange = (event: React.ChangeEvent): void => { + const newValue = event.target.value; + onChange(newValue); + }; + + const handleBooleanInputChange = (newOption: string): void => { + onChange(newOption); + }; + + const setIsTouchedValue = useCallback((): void => { + setIsTouched(true); + + const err = paramIsValid(selectedValue, selectedField, isRequired, true); + handleError(err); + }, [setIsTouched, handleError, selectedValue, selectedField, isRequired]); + + const inputPlaceholder = useMemo((): string => { + if (isLoading || isLoadingSuggestions) { + return i18n.LOADING; + } else if (selectedField == null) { + return i18n.SELECT_FIELD_FIRST; + } else { + return placeholder; + } + }, [isLoading, selectedField, isLoadingSuggestions, placeholder]); + const isLoadingState = useMemo((): boolean => isLoading || isLoadingSuggestions, [ isLoading, isLoadingSuggestions, ]); - return ( - - ); + useEffect((): void => { + setError(undefined); + if (onError != null) { + onError(false); + } + }, [selectedField, onError]); + + const defaultInput = useMemo((): JSX.Element => { + return ( + + + + ); + }, [ + comboOptions, + error, + fieldInputWidth, + handleCreateOption, + handleSearchChange, + handleValuesChange, + inputPlaceholder, + isClearable, + isDisabled, + isLoadingState, + rowLabel, + selectedComboOptions, + selectedField, + setIsTouchedValue, + ]); + + if (!isSuggestingValues && selectedField != null) { + switch (selectedField.type) { + case 'number': + return ( + + 0 + ? parseFloat(selectedValue) + : selectedValue ?? '' + } + onChange={handleNonComboBoxInputChange} + data-test-subj="valueAutocompleteFieldMatchNumber" + style={fieldInputWidth ? { width: `${fieldInputWidth}px` } : {}} + fullWidth + /> + + ); + case 'boolean': + return ( + + + + ); + default: + return defaultInput; + } + } else { + return defaultInput; + } }; AutocompleteFieldMatchComponent.displayName = 'AutocompleteFieldMatch'; diff --git a/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match_any.test.tsx b/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match_any.test.tsx index 4074150f76d06..217ad6580d6d4 100644 --- a/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match_any.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match_any.test.tsx @@ -5,9 +5,10 @@ */ import React from 'react'; import { ThemeProvider } from 'styled-components'; -import { mount } from 'enzyme'; +import { mount, ReactWrapper } from 'enzyme'; import euiLightVars from '@elastic/eui/dist/eui_theme_light.json'; import { EuiComboBox, EuiComboBoxOptionOption } from '@elastic/eui'; +import { act } from '@testing-library/react'; import { fields, @@ -15,24 +16,34 @@ import { } from '../../../../../../../src/plugins/data/common/index_patterns/fields/fields.mocks'; import { AutocompleteFieldMatchAnyComponent } from './field_value_match_any'; import { useFieldValueAutocomplete } from './hooks/use_field_value_autocomplete'; + jest.mock('./hooks/use_field_value_autocomplete'); describe('AutocompleteFieldMatchAnyComponent', () => { + let wrapper: ReactWrapper; const getValueSuggestionsMock = jest .fn() - .mockResolvedValue([false, ['value 3', 'value 4'], jest.fn()]); + .mockResolvedValue([false, true, ['value 3', 'value 4'], jest.fn()]); - beforeAll(() => { + beforeEach(() => { (useFieldValueAutocomplete as jest.Mock).mockReturnValue([ false, + true, ['value 1', 'value 2'], getValueSuggestionsMock, ]); }); + + afterEach(() => { + jest.clearAllMocks(); + wrapper.unmount(); + }); + test('it renders disabled if "isDisabled" is true', () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { isClearable={false} isDisabled={true} onChange={jest.fn()} + onError={jest.fn()} /> ); expect( - wrapper - .find(`[data-test-subj="valuesAutocompleteComboBox matchAnyComboxBox"] input`) - .prop('disabled') + wrapper.find(`[data-test-subj="valuesAutocompleteMatchAny"] input`).prop('disabled') ).toBeTruthy(); }); test('it renders loading if "isLoading" is true', () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { isClearable={false} isDisabled={false} onChange={jest.fn()} + onError={jest.fn()} /> ); - wrapper - .find(`[data-test-subj="valuesAutocompleteComboBox matchAnyComboxBox"] button`) - .at(0) - .simulate('click'); + wrapper.find(`[data-test-subj="valuesAutocompleteMatchAny"] button`).at(0).simulate('click'); expect( wrapper - .find( - `EuiComboBoxOptionsList[data-test-subj="valuesAutocompleteComboBox matchAnyComboxBox-optionsList"]` - ) + .find(`EuiComboBoxOptionsList[data-test-subj="valuesAutocompleteMatchAny-optionsList"]`) .prop('isLoading') ).toBeTruthy(); }); test('it allows user to clear values if "isClearable" is true', () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { isClearable={true} isDisabled={false} onChange={jest.fn()} + onError={jest.fn()} /> ); @@ -116,9 +125,10 @@ describe('AutocompleteFieldMatchAnyComponent', () => { }); test('it correctly displays selected value', () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { isClearable={false} isDisabled={false} onChange={jest.fn()} + onError={jest.fn()} /> ); expect( - wrapper - .find(`[data-test-subj="valuesAutocompleteComboBox matchAnyComboxBox"] EuiComboBoxPill`) - .at(0) - .text() + wrapper.find(`[data-test-subj="valuesAutocompleteMatchAny"] EuiComboBoxPill`).at(0).text() ).toEqual('126.45.211.34'); }); test('it invokes "onChange" when new value created', async () => { const mockOnChange = jest.fn(); - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { isClearable={false} isDisabled={false} onChange={mockOnChange} + onError={jest.fn()} /> ); @@ -173,9 +183,10 @@ describe('AutocompleteFieldMatchAnyComponent', () => { test('it invokes "onChange" when new value selected', async () => { const mockOnChange = jest.fn(); - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { isClearable={false} isDisabled={false} onChange={mockOnChange} + onError={jest.fn()} /> ); @@ -199,11 +211,11 @@ describe('AutocompleteFieldMatchAnyComponent', () => { expect(mockOnChange).toHaveBeenCalledWith(['value 1']); }); - test('it invokes updateSuggestions when new value searched', async () => { - const mockOnChange = jest.fn(); - const wrapper = mount( + test('it refreshes autocomplete with search query when new value searched', () => { + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { isLoading={false} isClearable={false} isDisabled={false} - onChange={mockOnChange} + onChange={jest.fn()} /> ); + act(() => { + ((wrapper.find(EuiComboBox).props() as unknown) as { + onSearchChange: (a: string) => void; + }).onSearchChange('value 1'); + }); - ((wrapper.find(EuiComboBox).props() as unknown) as { - onSearchChange: (a: string) => void; - }).onSearchChange('value 1'); - - expect(getValueSuggestionsMock).toHaveBeenCalledWith({ - fieldSelected: getField('machine.os.raw'), - patterns: { + expect(useFieldValueAutocomplete).toHaveBeenCalledWith({ + selectedField: getField('machine.os.raw'), + operatorType: 'match_any', + query: 'value 1', + fieldValue: [], + indexPattern: { id: '1234', title: 'logstash-*', fields, }, - value: 'value 1', }); }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match_any.tsx b/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match_any.tsx index 1952ef865e045..623f8d99137a5 100644 --- a/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match_any.tsx +++ b/x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match_any.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ import React, { useState, useCallback, useMemo } from 'react'; -import { EuiComboBoxOptionOption, EuiComboBox } from '@elastic/eui'; +import { EuiFormRow, EuiComboBoxOptionOption, EuiComboBox } from '@elastic/eui'; import { uniq } from 'lodash'; import { IFieldType, IIndexPattern } from '../../../../../../../src/plugins/data/common'; @@ -23,11 +23,14 @@ interface AutocompleteFieldMatchAnyProps { isDisabled: boolean; isClearable: boolean; isRequired?: boolean; + rowLabel?: string; onChange: (arg: string[]) => void; + onError?: (arg: boolean) => void; } export const AutocompleteFieldMatchAnyComponent: React.FC = ({ placeholder, + rowLabel, selectedField, selectedValue, indexPattern, @@ -36,12 +39,16 @@ export const AutocompleteFieldMatchAnyComponent: React.FC { + const [searchQuery, setSearchQuery] = useState(''); const [touched, setIsTouched] = useState(false); - const [isLoadingSuggestions, suggestions, updateSuggestions] = useFieldValueAutocomplete({ + const [error, setError] = useState(undefined); + const [isLoadingSuggestions, isSuggestingValues, suggestions] = useFieldValueAutocomplete({ selectedField, operatorType: OperatorTypeEnum.MATCH_ANY, fieldValue: selectedValue, + query: searchQuery, indexPattern, }); const getLabel = useCallback((option: string): string => option, []); @@ -59,32 +66,65 @@ export const AutocompleteFieldMatchAnyComponent: React.FC { - const newValues: string[] = newOptions.map(({ label }) => optionsMemo[labels.indexOf(label)]); - onChange(newValues); - }; - - const onSearchChange = (searchVal: string) => { - if (updateSuggestions != null) { - updateSuggestions({ - fieldSelected: selectedField, - value: searchVal, - patterns: indexPattern, + const handleError = useCallback( + (err: string | undefined): void => { + setError((existingErr): string | undefined => { + const oldErr = existingErr != null; + const newErr = err != null; + if (oldErr !== newErr && onError != null) { + onError(newErr); + } + + return err; }); - } - }; + }, + [setError, onError] + ); + + const handleValuesChange = useCallback( + (newOptions: EuiComboBoxOptionOption[]): void => { + const newValues: string[] = newOptions.map(({ label }) => optionsMemo[labels.indexOf(label)]); + handleError(undefined); + onChange(newValues); + }, + [handleError, labels, onChange, optionsMemo] + ); + + const handleSearchChange = useCallback( + (searchVal: string) => { + if (searchVal === '') { + handleError(undefined); + } + + if (searchVal !== '' && selectedField != null) { + const err = paramIsValid(searchVal, selectedField, isRequired, touched); + handleError(err); + + setSearchQuery(searchVal); + } + }, + [handleError, isRequired, selectedField, touched] + ); - const onCreateOption = (option: string) => onChange([...(selectedValue || []), option]); + const handleCreateOption = useCallback( + (option: string): boolean | void => { + const err = paramIsValid(option, selectedField, isRequired, touched); + handleError(err); - const isValid = useMemo((): boolean => { - const areAnyInvalid = - selectedComboOptions.filter( - ({ label }) => !paramIsValid(label, selectedField, isRequired, touched) - ).length > 0; - return !areAnyInvalid; - }, [selectedComboOptions, selectedField, isRequired, touched]); + if (err != null) { + // Explicitly reject the user's input + return false; + } else { + onChange([...(selectedValue || []), option]); + } + }, + [handleError, isRequired, onChange, selectedField, selectedValue, touched] + ); - const setIsTouchedValue = useCallback((): void => setIsTouched(true), [setIsTouched]); + const setIsTouchedValue = useCallback((): void => { + handleError(selectedComboOptions.length === 0 ? i18n.FIELD_REQUIRED_ERR : undefined); + setIsTouched(true); + }, [setIsTouched, handleError, selectedComboOptions]); const inputPlaceholder = useMemo( (): string => (isLoading || isLoadingSuggestions ? i18n.LOADING : placeholder), @@ -96,25 +136,83 @@ export const AutocompleteFieldMatchAnyComponent: React.FC - ); + const defaultInput = useMemo((): JSX.Element => { + return ( + + + + ); + }, [ + comboOptions, + error, + handleCreateOption, + handleSearchChange, + handleValuesChange, + inputPlaceholder, + isClearable, + isDisabled, + isLoadingState, + rowLabel, + selectedComboOptions, + selectedField, + setIsTouchedValue, + ]); + + if (!isSuggestingValues && selectedField != null) { + switch (selectedField.type) { + case 'number': + return ( + + + + ); + default: + return defaultInput; + } + } + + return defaultInput; }; AutocompleteFieldMatchAnyComponent.displayName = 'AutocompleteFieldMatchAny'; diff --git a/x-pack/plugins/security_solution/public/common/components/autocomplete/helpers.test.ts b/x-pack/plugins/security_solution/public/common/components/autocomplete/helpers.test.ts index 225b407e4649e..f78740f764202 100644 --- a/x-pack/plugins/security_solution/public/common/components/autocomplete/helpers.test.ts +++ b/x-pack/plugins/security_solution/public/common/components/autocomplete/helpers.test.ts @@ -7,6 +7,7 @@ import moment from 'moment'; import '../../../common/mock/match_media'; import { getField } from '../../../../../../../src/plugins/data/common/index_patterns/fields/fields.mocks'; +import * as i18n from './translations'; import { EXCEPTION_OPERATORS, isOperator, @@ -14,7 +15,7 @@ import { existsOperator, doesNotExistOperator, } from './operators'; -import { getOperators, paramIsValid, getGenericComboBoxProps } from './helpers'; +import { getOperators, checkEmptyValue, paramIsValid, getGenericComboBoxProps } from './helpers'; describe('helpers', () => { // @ts-ignore @@ -55,69 +56,120 @@ describe('helpers', () => { }); }); + describe('#checkEmptyValue', () => { + test('returns no errors if no field has been selected', () => { + const isValid = checkEmptyValue('', undefined, true, false); + + expect(isValid).toBeUndefined(); + }); + + test('returns error string if user has touched a required input and left empty', () => { + const isValid = checkEmptyValue(undefined, getField('@timestamp'), true, true); + + expect(isValid).toEqual(i18n.FIELD_REQUIRED_ERR); + }); + + test('returns no errors if required input is empty but user has not yet touched it', () => { + const isValid = checkEmptyValue(undefined, getField('@timestamp'), true, false); + + expect(isValid).toBeUndefined(); + }); + + test('returns no errors if user has touched an input that is not required and left empty', () => { + const isValid = checkEmptyValue(undefined, getField('@timestamp'), false, true); + + expect(isValid).toBeUndefined(); + }); + + test('returns no errors if user has touched an input that is not required and left empty string', () => { + const isValid = checkEmptyValue('', getField('@timestamp'), false, true); + + expect(isValid).toBeUndefined(); + }); + + test('returns null if input value is not empty string or undefined', () => { + const isValid = checkEmptyValue('hellooo', getField('@timestamp'), false, true); + + expect(isValid).toBeNull(); + }); + }); + describe('#paramIsValid', () => { - test('returns false if value is undefined and "isRequired" nad "touched" are true', () => { - const isValid = paramIsValid(undefined, getField('@timestamp'), true, true); + test('returns no errors if no field has been selected', () => { + const isValid = paramIsValid('', undefined, true, false); - expect(isValid).toBeFalsy(); + expect(isValid).toBeUndefined(); }); - test('returns true if value is undefined and "isRequired" is true but "touched" is false', () => { - const isValid = paramIsValid(undefined, getField('@timestamp'), true, false); + test('returns error string if user has touched a required input and left empty', () => { + const isValid = paramIsValid(undefined, getField('@timestamp'), true, true); - expect(isValid).toBeTruthy(); + expect(isValid).toEqual(i18n.FIELD_REQUIRED_ERR); }); - test('returns true if value is undefined and "isRequired" is false', () => { - const isValid = paramIsValid(undefined, getField('@timestamp'), false, false); + test('returns no errors if required input is empty but user has not yet touched it', () => { + const isValid = paramIsValid(undefined, getField('@timestamp'), true, false); - expect(isValid).toBeTruthy(); + expect(isValid).toBeUndefined(); }); - test('returns false if value is empty string when "isRequired" is true and "touched" is false', () => { - const isValid = paramIsValid('', getField('@timestamp'), true, false); + test('returns no errors if user has touched an input that is not required and left empty', () => { + const isValid = paramIsValid(undefined, getField('@timestamp'), false, true); - expect(isValid).toBeTruthy(); + expect(isValid).toBeUndefined(); }); - test('returns true if value is empty string and "isRequired" is false', () => { - const isValid = paramIsValid('', getField('@timestamp'), false, false); + test('returns no errors if user has touched an input that is not required and left empty string', () => { + const isValid = paramIsValid('', getField('@timestamp'), false, true); - expect(isValid).toBeTruthy(); + expect(isValid).toBeUndefined(); }); - test('returns true if type is "date" and value is valid and "isRequired" is false', () => { + test('returns no errors if field is of type date and value is valid', () => { const isValid = paramIsValid( '1994-11-05T08:15:30-05:00', getField('@timestamp'), false, - false + true ); - expect(isValid).toBeTruthy(); + expect(isValid).toBeUndefined(); }); - test('returns true if type is "date" and value is valid and "isRequired" is true', () => { - const isValid = paramIsValid( - '1994-11-05T08:15:30-05:00', - getField('@timestamp'), - true, - false - ); + test('returns errors if filed is of type date and value is not valid', () => { + const isValid = paramIsValid('1593478826', getField('@timestamp'), false, true); + + expect(isValid).toEqual(i18n.DATE_ERR); + }); + + test('returns no errors if field is of type number and value is an integer', () => { + const isValid = paramIsValid('4', getField('bytes'), true, true); + + expect(isValid).toBeUndefined(); + }); + + test('returns no errors if field is of type number and value is a float', () => { + const isValid = paramIsValid('4.3', getField('bytes'), true, true); + + expect(isValid).toBeUndefined(); + }); + + test('returns no errors if field is of type number and value is a long', () => { + const isValid = paramIsValid('-9223372036854775808', getField('bytes'), true, true); - expect(isValid).toBeTruthy(); + expect(isValid).toBeUndefined(); }); - test('returns false if type is "date" and value is not valid and "isRequired" is false', () => { - const isValid = paramIsValid('1593478826', getField('@timestamp'), false, false); + test('returns errors if field is of type number and value is "hello"', () => { + const isValid = paramIsValid('hello', getField('bytes'), true, true); - expect(isValid).toBeFalsy(); + expect(isValid).toEqual(i18n.NUMBER_ERR); }); - test('returns false if type is "date" and value is not valid and "isRequired" is true', () => { - const isValid = paramIsValid('1593478826', getField('@timestamp'), true, true); + test('returns errors if field is of type number and value is "123abc"', () => { + const isValid = paramIsValid('123abc', getField('bytes'), true, true); - expect(isValid).toBeFalsy(); + expect(isValid).toEqual(i18n.NUMBER_ERR); }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/autocomplete/helpers.ts b/x-pack/plugins/security_solution/public/common/components/autocomplete/helpers.ts index 8bbc022181475..1ad296e0299b1 100644 --- a/x-pack/plugins/security_solution/public/common/components/autocomplete/helpers.ts +++ b/x-pack/plugins/security_solution/public/common/components/autocomplete/helpers.ts @@ -17,7 +17,14 @@ import { doesNotExistOperator, } from './operators'; import { GetGenericComboBoxPropsReturn, OperatorOption } from './types'; +import * as i18n from './translations'; +/** + * Returns the appropriate operators given a field type + * + * @param field IFieldType selected field + * + */ export const getOperators = (field: IFieldType | undefined): OperatorOption[] => { if (field == null) { return [isOperator]; @@ -30,33 +37,81 @@ export const getOperators = (field: IFieldType | undefined): OperatorOption[] => } }; -export const paramIsValid = ( - params: string | undefined, +/** + * Determines if empty value is ok + * + * @param param the value being checked + * @param field the selected field + * @param isRequired whether or not an empty value is allowed + * @param touched has field been touched by user + * @returns undefined if valid, string with error message if invalid, + * null if no checks matched + */ +export const checkEmptyValue = ( + param: string | undefined, field: IFieldType | undefined, isRequired: boolean, touched: boolean -): boolean => { - if (isRequired && touched && (params == null || params === '')) { - return false; +): string | undefined | null => { + if (isRequired && touched && (param == null || param.trim() === '')) { + return i18n.FIELD_REQUIRED_ERR; + } + + if ( + field == null || + (isRequired && !touched) || + (!isRequired && (param == null || param === '')) + ) { + return undefined; } - if ((isRequired && !touched) || (!isRequired && (params == null || params === ''))) { - return true; + return null; +}; + +/** + * Very basic validation for values + * + * @param param the value being checked + * @param field the selected field + * @param isRequired whether or not an empty value is allowed + * @param touched has field been touched by user + * @returns undefined if valid, string with error message if invalid + */ +export const paramIsValid = ( + param: string | undefined, + field: IFieldType | undefined, + isRequired: boolean, + touched: boolean +): string | undefined => { + if (field == null) { + return undefined; } - const types = field != null && field.esTypes != null ? field.esTypes : []; + const emptyValueError = checkEmptyValue(param, field, isRequired, touched); + if (emptyValueError !== null) { + return emptyValueError; + } - return types.reduce((acc, type) => { - switch (type) { - case 'date': - const moment = dateMath.parse(params ?? ''); - return Boolean(moment && moment.isValid()); - default: - return acc; - } - }, true); + switch (field.type) { + case 'date': + const moment = dateMath.parse(param ?? ''); + const isDate = Boolean(moment && moment.isValid()); + return isDate ? undefined : i18n.DATE_ERR; + case 'number': + const isNum = param != null && param.trim() !== '' && !isNaN(+param); + return isNum ? undefined : i18n.NUMBER_ERR; + default: + return undefined; + } }; +/** + * Determines the options, selected values and option labels for EUI combo box + * + * @param options options user can select from + * @param selectedOptions user selection if any + * @param getLabel helper function to know which property to use for labels + */ export function getGenericComboBoxProps({ options, selectedOptions, diff --git a/x-pack/plugins/security_solution/public/common/components/autocomplete/hooks/use_field_value_autocomplete.test.ts b/x-pack/plugins/security_solution/public/common/components/autocomplete/hooks/use_field_value_autocomplete.test.ts index 82e9c21f6b839..2b6506f54b796 100644 --- a/x-pack/plugins/security_solution/public/common/components/autocomplete/hooks/use_field_value_autocomplete.test.ts +++ b/x-pack/plugins/security_solution/public/common/components/autocomplete/hooks/use_field_value_autocomplete.test.ts @@ -22,7 +22,7 @@ describe('useFieldValueAutocomplete', () => { const onErrorMock = jest.fn(); const getValueSuggestionsMock = jest.fn().mockResolvedValue(['value 1', 'value 2']); - beforeAll(() => { + beforeEach(() => { (useKibana as jest.Mock).mockReturnValue({ services: { data: { @@ -50,11 +50,12 @@ describe('useFieldValueAutocomplete', () => { operatorType: OperatorTypeEnum.MATCH, fieldValue: '', indexPattern: undefined, + query: '', }) ); await waitForNextUpdate(); - expect(result.current).toEqual([false, [], result.current[2]]); + expect(result.current).toEqual([false, true, [], result.current[3]]); }); }); @@ -69,11 +70,12 @@ describe('useFieldValueAutocomplete', () => { operatorType: OperatorTypeEnum.EXISTS, fieldValue: '', indexPattern: stubIndexPatternWithFields, + query: '', }) ); await waitForNextUpdate(); - const expectedResult: UseFieldValueAutocompleteReturn = [false, [], result.current[2]]; + const expectedResult: UseFieldValueAutocompleteReturn = [false, true, [], result.current[3]]; expect(getValueSuggestionsMock).not.toHaveBeenCalled(); expect(result.current).toEqual(expectedResult); @@ -91,11 +93,12 @@ describe('useFieldValueAutocomplete', () => { operatorType: OperatorTypeEnum.EXISTS, fieldValue: '', indexPattern: stubIndexPatternWithFields, + query: '', }) ); await waitForNextUpdate(); - const expectedResult: UseFieldValueAutocompleteReturn = [false, [], result.current[2]]; + const expectedResult: UseFieldValueAutocompleteReturn = [false, true, [], result.current[3]]; expect(getValueSuggestionsMock).not.toHaveBeenCalled(); expect(result.current).toEqual(expectedResult); @@ -113,18 +116,71 @@ describe('useFieldValueAutocomplete', () => { operatorType: OperatorTypeEnum.EXISTS, fieldValue: '', indexPattern: undefined, + query: '', }) ); await waitForNextUpdate(); - const expectedResult: UseFieldValueAutocompleteReturn = [false, [], result.current[2]]; + const expectedResult: UseFieldValueAutocompleteReturn = [false, true, [], result.current[3]]; expect(getValueSuggestionsMock).not.toHaveBeenCalled(); expect(result.current).toEqual(expectedResult); }); }); - test('returns suggestions of "true" and "false" if field type is boolean', async () => { + test('it uses full path name for nested fields to fetch suggestions', async () => { + const suggestionsMock = jest.fn().mockResolvedValue([]); + + (useKibana as jest.Mock).mockReturnValue({ + services: { + data: { + autocomplete: { + getValueSuggestions: suggestionsMock, + }, + }, + }, + }); + await act(async () => { + const signal = new AbortController().signal; + const { waitForNextUpdate } = renderHook< + UseFieldValueAutocompleteProps, + UseFieldValueAutocompleteReturn + >(() => + useFieldValueAutocomplete({ + selectedField: { ...getField('nestedField.child'), name: 'child' }, + operatorType: OperatorTypeEnum.MATCH, + fieldValue: '', + indexPattern: stubIndexPatternWithFields, + query: '', + }) + ); + // Note: initial `waitForNextUpdate` is hook initialization + await waitForNextUpdate(); + await waitForNextUpdate(); + + expect(suggestionsMock).toHaveBeenCalledWith({ + field: { ...getField('nestedField.child'), name: 'nestedField.child' }, + indexPattern: { + fields: [ + { + aggregatable: true, + esTypes: ['integer'], + filterable: true, + name: 'response', + searchable: true, + type: 'number', + }, + ], + id: '1234', + title: 'logstash-*', + }, + query: '', + signal, + }); + }); + }); + + test('returns "isSuggestingValues" of false if field type is boolean', async () => { await act(async () => { const { result, waitForNextUpdate } = renderHook< UseFieldValueAutocompleteProps, @@ -135,24 +191,59 @@ describe('useFieldValueAutocomplete', () => { operatorType: OperatorTypeEnum.MATCH, fieldValue: '', indexPattern: stubIndexPatternWithFields, + query: '', }) ); + // Note: initial `waitForNextUpdate` is hook initialization await waitForNextUpdate(); await waitForNextUpdate(); - const expectedResult: UseFieldValueAutocompleteReturn = [ - false, - ['true', 'false'], - result.current[2], - ]; + const expectedResult: UseFieldValueAutocompleteReturn = [false, false, [], result.current[3]]; expect(getValueSuggestionsMock).not.toHaveBeenCalled(); expect(result.current).toEqual(expectedResult); }); }); + test('returns "isSuggestingValues" of false to note that autocomplete service is not in use if no autocomplete suggestions available', async () => { + const suggestionsMock = jest.fn().mockResolvedValue([]); + + (useKibana as jest.Mock).mockReturnValue({ + services: { + data: { + autocomplete: { + getValueSuggestions: suggestionsMock, + }, + }, + }, + }); + await act(async () => { + const { result, waitForNextUpdate } = renderHook< + UseFieldValueAutocompleteProps, + UseFieldValueAutocompleteReturn + >(() => + useFieldValueAutocomplete({ + selectedField: getField('bytes'), + operatorType: OperatorTypeEnum.MATCH, + fieldValue: '', + indexPattern: stubIndexPatternWithFields, + query: '', + }) + ); + // Note: initial `waitForNextUpdate` is hook initialization + await waitForNextUpdate(); + await waitForNextUpdate(); + + const expectedResult: UseFieldValueAutocompleteReturn = [false, false, [], result.current[3]]; + + expect(suggestionsMock).toHaveBeenCalled(); + expect(result.current).toEqual(expectedResult); + }); + }); + test('returns suggestions', async () => { await act(async () => { + const signal = new AbortController().signal; const { result, waitForNextUpdate } = renderHook< UseFieldValueAutocompleteProps, UseFieldValueAutocompleteReturn @@ -162,22 +253,25 @@ describe('useFieldValueAutocomplete', () => { operatorType: OperatorTypeEnum.MATCH, fieldValue: '', indexPattern: stubIndexPatternWithFields, + query: '', }) ); + // Note: initial `waitForNextUpdate` is hook initialization await waitForNextUpdate(); await waitForNextUpdate(); const expectedResult: UseFieldValueAutocompleteReturn = [ false, + true, ['value 1', 'value 2'], - result.current[2], + result.current[3], ]; expect(getValueSuggestionsMock).toHaveBeenCalledWith({ field: getField('@tags'), indexPattern: stubIndexPatternWithFields, query: '', - signal: new AbortController().signal, + signal, }); expect(result.current).toEqual(expectedResult); }); @@ -194,20 +288,23 @@ describe('useFieldValueAutocomplete', () => { operatorType: OperatorTypeEnum.MATCH, fieldValue: '', indexPattern: stubIndexPatternWithFields, + query: '', }) ); + // Note: initial `waitForNextUpdate` is hook initialization await waitForNextUpdate(); await waitForNextUpdate(); - expect(result.current[2]).not.toBeNull(); + expect(result.current[3]).not.toBeNull(); // Added check for typescripts sake, if null, // would not reach below logic as test would stop above - if (result.current[2] != null) { - result.current[2]({ + if (result.current[3] != null) { + result.current[3]({ fieldSelected: getField('@tags'), value: 'hello', patterns: stubIndexPatternWithFields, + searchQuery: '', }); } @@ -215,8 +312,9 @@ describe('useFieldValueAutocomplete', () => { const expectedResult: UseFieldValueAutocompleteReturn = [ false, + true, ['value 1', 'value 2'], - result.current[2], + result.current[3], ]; expect(getValueSuggestionsMock).toHaveBeenCalledTimes(2); diff --git a/x-pack/plugins/security_solution/public/common/components/autocomplete/hooks/use_field_value_autocomplete.ts b/x-pack/plugins/security_solution/public/common/components/autocomplete/hooks/use_field_value_autocomplete.ts index a53914da93f27..085d0d09757e1 100644 --- a/x-pack/plugins/security_solution/public/common/components/autocomplete/hooks/use_field_value_autocomplete.ts +++ b/x-pack/plugins/security_solution/public/common/components/autocomplete/hooks/use_field_value_autocomplete.ts @@ -11,18 +11,22 @@ import { IFieldType, IIndexPattern } from '../../../../../../../../src/plugins/d import { useKibana } from '../../../../common/lib/kibana'; import { OperatorTypeEnum } from '../../../../lists_plugin_deps'; -type Func = (args: { +interface FuncArgs { fieldSelected: IFieldType | undefined; value: string | string[] | undefined; + searchQuery: string; patterns: IIndexPattern | undefined; -}) => void; +} + +type Func = (args: FuncArgs) => void; -export type UseFieldValueAutocompleteReturn = [boolean, string[], Func | null]; +export type UseFieldValueAutocompleteReturn = [boolean, boolean, string[], Func | null]; export interface UseFieldValueAutocompleteProps { selectedField: IFieldType | undefined; operatorType: OperatorTypeEnum; fieldValue: string | string[] | undefined; + query: string; indexPattern: IIndexPattern | undefined; } /** @@ -33,10 +37,12 @@ export const useFieldValueAutocomplete = ({ selectedField, operatorType, fieldValue, + query, indexPattern, }: UseFieldValueAutocompleteProps): UseFieldValueAutocompleteReturn => { const { services } = useKibana(); const [isLoading, setIsLoading] = useState(false); + const [isSuggestingValues, setIsSuggestingValues] = useState(true); const [suggestions, setSuggestions] = useState([]); const updateSuggestions = useRef(null); @@ -45,42 +51,39 @@ export const useFieldValueAutocomplete = ({ const abortCtrl = new AbortController(); const fetchSuggestions = debounce( - async ({ - fieldSelected, - value, - patterns, - }: { - fieldSelected: IFieldType | undefined; - value: string | string[] | undefined; - patterns: IIndexPattern | undefined; - }) => { - const inputValue: string | string[] = value ?? ''; - const userSuggestion: string = Array.isArray(inputValue) - ? inputValue[inputValue.length - 1] ?? '' - : inputValue; - + async ({ fieldSelected, value, searchQuery, patterns }: FuncArgs) => { try { if (isSubscribed) { if (fieldSelected == null || patterns == null) { return; } - setIsLoading(true); - - // Fields of type boolean should only display two options if (fieldSelected.type === 'boolean') { - setIsLoading(false); - setSuggestions(['true', 'false']); + setIsSuggestingValues(false); return; } + setIsLoading(true); + + const field = + fieldSelected.subType != null && fieldSelected.subType.nested != null + ? { + ...fieldSelected, + name: `${fieldSelected.subType.nested.path}.${fieldSelected.name}`, + } + : fieldSelected; + const newSuggestions = await services.data.autocomplete.getValueSuggestions({ indexPattern: patterns, - field: fieldSelected, - query: userSuggestion.trim(), + field, + query: searchQuery, signal: abortCtrl.signal, }); + if (newSuggestions.length === 0) { + setIsSuggestingValues(false); + } + setIsLoading(false); setSuggestions([...newSuggestions]); } @@ -98,6 +101,7 @@ export const useFieldValueAutocomplete = ({ fetchSuggestions({ fieldSelected: selectedField, value: fieldValue, + searchQuery: query, patterns: indexPattern, }); } @@ -108,7 +112,7 @@ export const useFieldValueAutocomplete = ({ isSubscribed = false; abortCtrl.abort(); }; - }, [services.data.autocomplete, selectedField, operatorType, fieldValue, indexPattern]); + }, [services.data.autocomplete, selectedField, operatorType, fieldValue, indexPattern, query]); - return [isLoading, suggestions, updateSuggestions.current]; + return [isLoading, isSuggestingValues, suggestions, updateSuggestions.current]; }; diff --git a/x-pack/plugins/security_solution/public/common/components/autocomplete/translations.ts b/x-pack/plugins/security_solution/public/common/components/autocomplete/translations.ts index 6d83086b15e6a..83950d446ce7a 100644 --- a/x-pack/plugins/security_solution/public/common/components/autocomplete/translations.ts +++ b/x-pack/plugins/security_solution/public/common/components/autocomplete/translations.ts @@ -9,3 +9,25 @@ import { i18n } from '@kbn/i18n'; export const LOADING = i18n.translate('xpack.securitySolution.autocomplete.loadingDescription', { defaultMessage: 'Loading...', }); + +export const SELECT_FIELD_FIRST = i18n.translate( + 'xpack.securitySolution.autocomplete.selectField', + { + defaultMessage: 'Please select a field first...', + } +); + +export const FIELD_REQUIRED_ERR = i18n.translate( + 'xpack.securitySolution.autocomplete.fieldRequiredError', + { + defaultMessage: 'Value cannot be empty', + } +); + +export const NUMBER_ERR = i18n.translate('xpack.securitySolution.autocomplete.invalidNumberError', { + defaultMessage: 'Not a valid number', +}); + +export const DATE_ERR = i18n.translate('xpack.securitySolution.autocomplete.invalidDateError', { + defaultMessage: 'Not a valid date', +}); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.test.tsx index 35bd5ee572160..2a778cd6585a1 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.test.tsx @@ -407,4 +407,24 @@ describe('When the add exception modal is opened', () => { }); }); }); + + test('when there are exception builder errors submit button is disabled', async () => { + const wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + const callProps = ExceptionBuilderComponent.mock.calls[0][0]; + await waitFor(() => callProps.onChange({ exceptionItems: [], errorExists: true })); + expect( + wrapper.find('button[data-test-subj="add-exception-confirm-button"]').getDOMNode() + ).toBeDisabled(); + }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx index bf483387580ce..6c5e39b3e6aad 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx @@ -106,6 +106,7 @@ export const AddExceptionModal = memo(function AddExceptionModal({ alertStatus, }: AddExceptionModalProps) { const { http } = useKibana().services; + const [errorsExist, setErrorExists] = useState(false); const [comment, setComment] = useState(''); const { rule: maybeRule } = useRuleAsync(ruleId); const [shouldCloseAlert, setShouldCloseAlert] = useState(false); @@ -158,10 +159,13 @@ export const AddExceptionModal = memo(function AddExceptionModal({ const handleBuilderOnChange = useCallback( ({ exceptionItems, + errorExists, }: { exceptionItems: Array; - }): void => { + errorExists: boolean; + }) => { setExceptionItemsToAdd(exceptionItems); + setErrorExists(errorExists); }, [setExceptionItemsToAdd] ); @@ -310,8 +314,9 @@ export const AddExceptionModal = memo(function AddExceptionModal({ const isSubmitButtonDisabled = useMemo( (): boolean => fetchOrCreateListError != null || - exceptionItemsToAdd.every((item) => item.entries.length === 0), - [fetchOrCreateListError, exceptionItemsToAdd] + exceptionItemsToAdd.every((item) => item.entries.length === 0) || + errorsExist, + [fetchOrCreateListError, exceptionItemsToAdd, errorsExist] ); const addExceptionMessage = diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.test.tsx index 59a5db2a09779..60b2f079c869b 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.test.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { mount } from 'enzyme'; +import { ReactWrapper, mount } from 'enzyme'; import React from 'react'; import { EuiComboBox, EuiComboBoxOptionOption } from '@elastic/eui'; @@ -45,8 +45,15 @@ jest.mock('../../../../lists_plugin_deps', () => { }); describe('BuilderEntryItem', () => { + let wrapper: ReactWrapper; + + afterEach(() => { + jest.clearAllMocks(); + wrapper.unmount(); + }); + test('it renders field labels if "showLabel" is "true"', () => { - const wrapper = mount( + wrapper = mount( { showLabel={true} listType="detection" onChange={jest.fn()} + setErrorsExist={jest.fn()} /> ); @@ -72,7 +80,7 @@ describe('BuilderEntryItem', () => { }); test('it renders field values correctly when operator is "isOperator"', () => { - const wrapper = mount( + wrapper = mount( { showLabel={false} listType="detection" onChange={jest.fn()} + setErrorsExist={jest.fn()} /> ); @@ -102,7 +111,7 @@ describe('BuilderEntryItem', () => { }); test('it renders field values correctly when operator is "isNotOperator"', () => { - const wrapper = mount( + wrapper = mount( { showLabel={false} listType="detection" onChange={jest.fn()} + setErrorsExist={jest.fn()} /> ); @@ -134,7 +144,7 @@ describe('BuilderEntryItem', () => { }); test('it renders field values correctly when operator is "isOneOfOperator"', () => { - const wrapper = mount( + wrapper = mount( { showLabel={false} listType="detection" onChange={jest.fn()} + setErrorsExist={jest.fn()} /> ); @@ -166,7 +177,7 @@ describe('BuilderEntryItem', () => { }); test('it renders field values correctly when operator is "isNotOneOfOperator"', () => { - const wrapper = mount( + wrapper = mount( { showLabel={false} listType="detection" onChange={jest.fn()} + setErrorsExist={jest.fn()} /> ); @@ -198,7 +210,7 @@ describe('BuilderEntryItem', () => { }); test('it renders field values correctly when operator is "isInListOperator"', () => { - const wrapper = mount( + wrapper = mount( { showLabel={true} listType="detection" onChange={jest.fn()} + setErrorsExist={jest.fn()} /> ); @@ -224,13 +237,13 @@ describe('BuilderEntryItem', () => { expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').text()).toEqual( 'is in list' ); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldList"]').text()).toEqual( - 'some name' - ); + expect( + wrapper.find('[data-test-subj="valuesAutocompleteComboBox listsComboxBox"]').at(1).text() + ).toEqual('some name'); }); test('it renders field values correctly when operator is "isNotInListOperator"', () => { - const wrapper = mount( + wrapper = mount( { showLabel={true} listType="detection" onChange={jest.fn()} + setErrorsExist={jest.fn()} /> ); @@ -256,13 +270,13 @@ describe('BuilderEntryItem', () => { expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').text()).toEqual( 'is not in list' ); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldList"]').text()).toEqual( - 'some name' - ); + expect( + wrapper.find('[data-test-subj="valuesAutocompleteComboBox listsComboxBox"]').at(1).text() + ).toEqual('some name'); }); test('it renders field values correctly when operator is "existsOperator"', () => { - const wrapper = mount( + wrapper = mount( { showLabel={false} listType="detection" onChange={jest.fn()} + setErrorsExist={jest.fn()} /> ); @@ -297,7 +312,7 @@ describe('BuilderEntryItem', () => { }); test('it renders field values correctly when operator is "doesNotExistOperator"', () => { - const wrapper = mount( + wrapper = mount( { showLabel={false} listType="detection" onChange={jest.fn()} + setErrorsExist={jest.fn()} /> ); @@ -332,7 +348,7 @@ describe('BuilderEntryItem', () => { }); test('it uses "correspondingKeywordField" if it exists', () => { - const wrapper = mount( + wrapper = mount( { showLabel={false} listType="detection" onChange={jest.fn()} + setErrorsExist={jest.fn()} /> ); @@ -388,7 +405,7 @@ describe('BuilderEntryItem', () => { test('it invokes "onChange" when new field is selected and resets operator and value fields', () => { const mockOnChange = jest.fn(); - const wrapper = mount( + wrapper = mount( { showLabel={false} listType="detection" onChange={mockOnChange} + setErrorsExist={jest.fn()} /> ); @@ -422,7 +440,7 @@ describe('BuilderEntryItem', () => { test('it invokes "onChange" when new operator is selected', () => { const mockOnChange = jest.fn(); - const wrapper = mount( + wrapper = mount( { showLabel={false} listType="detection" onChange={mockOnChange} + setErrorsExist={jest.fn()} /> ); @@ -456,7 +475,7 @@ describe('BuilderEntryItem', () => { test('it invokes "onChange" when new value field is entered for match operator', () => { const mockOnChange = jest.fn(); - const wrapper = mount( + wrapper = mount( { showLabel={false} listType="detection" onChange={mockOnChange} + setErrorsExist={jest.fn()} /> ); @@ -490,7 +510,7 @@ describe('BuilderEntryItem', () => { test('it invokes "onChange" when new value field is entered for match_any operator', () => { const mockOnChange = jest.fn(); - const wrapper = mount( + wrapper = mount( { showLabel={false} listType="detection" onChange={mockOnChange} + setErrorsExist={jest.fn()} /> ); @@ -524,7 +545,7 @@ describe('BuilderEntryItem', () => { test('it invokes "onChange" when new value field is entered for list operator', () => { const mockOnChange = jest.fn(); - const wrapper = mount( + wrapper = mount( { showLabel={false} listType="detection" onChange={mockOnChange} + setErrorsExist={jest.fn()} /> ); @@ -560,4 +582,72 @@ describe('BuilderEntryItem', () => { 0 ); }); + + test('it invokes "setErrorsExist" when user touches value input and leaves empty', () => { + const mockSetErrorExists = jest.fn(); + wrapper = mount( + + ); + + ((wrapper.find(EuiComboBox).at(2).props() as unknown) as { + onBlur: () => void; + }).onBlur(); + + expect(mockSetErrorExists).toHaveBeenCalledWith(true); + }); + + test('it invokes "setErrorsExist" when invalid value inputted for field value input', () => { + const mockSetErrorExists = jest.fn(); + wrapper = mount( + + ); + ((wrapper.find(EuiComboBox).at(2).props() as unknown) as { + onBlur: () => void; + }).onBlur(); + + // Invalid input because field type is number + ((wrapper.find(EuiComboBox).at(2).props() as unknown) as { + onSearchChange: (arg: string) => void; + }).onSearchChange('hellooo'); + + expect(mockSetErrorExists).toHaveBeenCalledWith(true); + }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.tsx index 450b48a793e4e..8f00763f91411 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.tsx @@ -40,6 +40,7 @@ interface EntryItemProps { showLabel: boolean; listType: ExceptionListType; onChange: (arg: BuilderEntry, i: number) => void; + setErrorsExist: (arg: boolean) => void; onlyShowListOperators?: boolean; } @@ -49,8 +50,16 @@ export const BuilderEntryItem: React.FC = ({ listType, showLabel, onChange, + setErrorsExist, onlyShowListOperators = false, }): JSX.Element => { + const handleError = useCallback( + (err: boolean): void => { + setErrorsExist(err); + }, + [setErrorsExist] + ); + const handleFieldChange = useCallback( ([newField]: IFieldType[]): void => { const { updatedEntry, index } = getEntryOnFieldChange(entry, newField); @@ -165,18 +174,15 @@ export const BuilderEntryItem: React.FC = ({ } }; - const getFieldValueComboBox = (type: OperatorTypeEnum): JSX.Element => { + const getFieldValueComboBox = (type: OperatorTypeEnum, isFirst: boolean): JSX.Element => { switch (type) { case OperatorTypeEnum.MATCH: const value = typeof entry.value === 'string' ? entry.value : undefined; return ( = ({ isLoading={false} isClearable={false} indexPattern={indexPattern} + onError={handleError} onChange={handleFieldMatchValueChange} isRequired data-test-subj="exceptionBuilderEntryFieldMatch" @@ -193,6 +200,7 @@ export const BuilderEntryItem: React.FC = ({ const values: string[] = Array.isArray(entry.value) ? entry.value : []; return ( = ({ isLoading={false} isClearable={false} indexPattern={indexPattern} + onError={handleError} onChange={handleFieldMatchAnyValueChange} isRequired data-test-subj="exceptionBuilderEntryFieldMatchAny" @@ -215,6 +224,7 @@ export const BuilderEntryItem: React.FC = ({ const id = typeof entry.value === 'string' ? entry.value : undefined; return ( = ({ case OperatorTypeEnum.EXISTS: return ( @@ -240,23 +251,14 @@ export const BuilderEntryItem: React.FC = ({ } }; - const renderFieldValueInput = (isFirst: boolean, entryType: OperatorTypeEnum): JSX.Element => { - if (isFirst) { - return ( - - {getFieldValueComboBox(entryType)} - - ); - } else { - return getFieldValueComboBox(entryType); - } - }; + const renderFieldValueInput = (isFirst: boolean, entryType: OperatorTypeEnum): JSX.Element => + getFieldValueComboBox(entryType, isFirst); return ( diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.test.tsx index 0f9be25e046b2..3d0b3ea20fa76 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.test.tsx @@ -58,6 +58,7 @@ describe('BuilderExceptionListItemComponent', () => { andLogicIncluded={true} isOnlyItem={false} listType="detection" + setErrorsExist={jest.fn()} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> @@ -86,6 +87,7 @@ describe('BuilderExceptionListItemComponent', () => { andLogicIncluded={true} isOnlyItem={false} listType="detection" + setErrorsExist={jest.fn()} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> @@ -112,6 +114,7 @@ describe('BuilderExceptionListItemComponent', () => { andLogicIncluded={true} isOnlyItem={false} listType="detection" + setErrorsExist={jest.fn()} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> @@ -140,6 +143,7 @@ describe('BuilderExceptionListItemComponent', () => { andLogicIncluded={false} isOnlyItem={false} listType="detection" + setErrorsExist={jest.fn()} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> @@ -175,6 +179,7 @@ describe('BuilderExceptionListItemComponent', () => { andLogicIncluded={false} isOnlyItem={true} listType="detection" + setErrorsExist={jest.fn()} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> @@ -202,6 +207,7 @@ describe('BuilderExceptionListItemComponent', () => { andLogicIncluded={false} isOnlyItem={false} listType="detection" + setErrorsExist={jest.fn()} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> @@ -230,6 +236,7 @@ describe('BuilderExceptionListItemComponent', () => { // this to be true, but done for testing purposes isOnlyItem={true} listType="detection" + setErrorsExist={jest.fn()} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> @@ -256,6 +263,7 @@ describe('BuilderExceptionListItemComponent', () => { andLogicIncluded={false} isOnlyItem={true} listType="detection" + setErrorsExist={jest.fn()} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> @@ -284,6 +292,7 @@ describe('BuilderExceptionListItemComponent', () => { andLogicIncluded={false} isOnlyItem={true} listType="detection" + setErrorsExist={jest.fn()} onDeleteExceptionItem={mockOnDeleteExceptionItem} onChangeExceptionItem={jest.fn()} /> diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.tsx index 49a159cdfe623..9d3b24c933bfc 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.tsx @@ -41,6 +41,7 @@ interface BuilderExceptionListItemProps { listType: ExceptionListType; onDeleteExceptionItem: (item: ExceptionsBuilderExceptionItem, index: number) => void; onChangeExceptionItem: (item: ExceptionsBuilderExceptionItem, index: number) => void; + setErrorsExist: (arg: boolean) => void; onlyShowListOperators?: boolean; } @@ -55,6 +56,7 @@ export const BuilderExceptionListItemComponent = React.memo { const handleEntryChange = useCallback( @@ -118,6 +120,7 @@ export const BuilderExceptionListItemComponent = React.memo diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.test.tsx index 2ee0fe88f73f7..a273c53e03b8e 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.test.tsx @@ -855,7 +855,7 @@ describe('Exception builder helpers', () => { }); describe('#getOperatorOptions', () => { - test('it returns "isOperator" if "item.nested" is "parent"', () => { + test('it returns "isOperator" when field type is nested but field itself has not yet been selected', () => { const payloadItem: FormattedBuilderEntry = getMockNestedParentBuilderEntry(); const output = getOperatorOptions(payloadItem, 'endpoint', false); const expected: OperatorOption[] = [isOperator]; @@ -883,7 +883,7 @@ describe('Exception builder helpers', () => { expect(output).toEqual(expected); }); - test('it returns "isOperator" if "listType" is "endpoint" and field type is boolean', () => { + test('it returns "isOperator" if "listType" is "endpoint" and field type is boolean', () => { const payloadItem: FormattedBuilderEntry = getMockBuilderEntry(); const output = getOperatorOptions(payloadItem, 'endpoint', true); const expected: OperatorOption[] = [isOperator]; @@ -911,10 +911,15 @@ describe('Exception builder helpers', () => { expect(output).toEqual(expected); }); - test('it returns "isOperator" and "existsOperator" if field type is boolean', () => { + test('it returns "isOperator", "isNotOperator", "doesNotExistOperator" and "existsOperator" if field type is boolean', () => { const payloadItem: FormattedBuilderEntry = getMockBuilderEntry(); const output = getOperatorOptions(payloadItem, 'detection', true); - const expected: OperatorOption[] = [isOperator, existsOperator]; + const expected: OperatorOption[] = [ + isOperator, + isNotOperator, + existsOperator, + doesNotExistOperator, + ]; expect(output).toEqual(expected); }); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx index 2fda14dfa04d7..1a22037eb8df8 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx @@ -23,6 +23,8 @@ import { isOneOfOperator, EXCEPTION_OPERATORS, EXCEPTION_OPERATORS_SANS_LISTS, + isNotOperator, + doesNotExistOperator, } from '../../autocomplete/operators'; import { OperatorOption } from '../../autocomplete/types'; import { @@ -371,7 +373,7 @@ export const getOperatorOptions = ( return isBoolean ? [isOperator, existsOperator] : [isOperator, isOneOfOperator, existsOperator]; } else { return isBoolean - ? [isOperator, existsOperator] + ? [isOperator, isNotOperator, existsOperator, doesNotExistOperator] : includeValueListOperators ? EXCEPTION_OPERATORS : EXCEPTION_OPERATORS_SANS_LISTS; diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.test.tsx index e6f57fe666780..198bf5096322a 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.test.tsx @@ -6,7 +6,7 @@ import React from 'react'; import { ThemeProvider } from 'styled-components'; -import { mount } from 'enzyme'; +import { ReactWrapper, mount } from 'enzyme'; import euiLightVars from '@elastic/eui/dist/eui_theme_light.json'; import { waitFor } from '@testing-library/react'; @@ -25,6 +25,8 @@ import { ExceptionBuilderComponent } from './'; jest.mock('../../../../common/lib/kibana'); describe('ExceptionBuilderComponent', () => { + let wrapper: ReactWrapper; + const getValueSuggestionsMock = jest.fn().mockResolvedValue(['value 1', 'value 2']); beforeEach(() => { @@ -41,10 +43,12 @@ describe('ExceptionBuilderComponent', () => { afterEach(() => { getValueSuggestionsMock.mockClear(); + jest.clearAllMocks(); + wrapper.unmount(); }); test('it displays empty entry if no "exceptionListItems" are passed in', () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { expect(wrapper.find('EuiFlexGroup[data-test-subj="exceptionItemEntryContainer"]')).toHaveLength( 1 ); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').text()).toEqual('Search'); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').text()).toEqual('is'); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatch"]').text()).toEqual( - 'Search field value...' + expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').at(0).text()).toEqual( + 'Search' + ); + expect(wrapper.find('[data-test-subj="operatorAutocompleteComboBox"]').at(0).text()).toEqual( + 'is' + ); + expect(wrapper.find('[data-test-subj="valuesAutocompleteMatch"]').at(0).text()).toEqual( + 'Please select a field first...' ); }); test('it displays "exceptionListItems" that are passed in', async () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { expect(wrapper.find('EuiFlexGroup[data-test-subj="exceptionItemEntryContainer"]')).toHaveLength( 1 ); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').text()).toEqual('ip'); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').text()).toEqual( + expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').at(0).text()).toEqual( + 'ip' + ); + expect(wrapper.find('[data-test-subj="operatorAutocompleteComboBox"]').at(0).text()).toEqual( 'is one of' ); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatchAny"]').text()).toEqual( + expect(wrapper.find('[data-test-subj="valuesAutocompleteMatchAny"]').at(0).text()).toEqual( 'some ip' ); - - wrapper.unmount(); }); test('it displays "or", "and" and "add nested button" enabled', () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { }); test('it adds an entry when "and" clicked', async () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').at(0).text()).toEqual( 'Search' ); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').at(0).text()).toEqual( + expect(wrapper.find('[data-test-subj="operatorAutocompleteComboBox"]').at(0).text()).toEqual( 'is' ); - expect( - wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatch"]').at(0).text() - ).toEqual('Search field value...'); + expect(wrapper.find('[data-test-subj="valuesAutocompleteMatch"]').at(0).text()).toEqual( + 'Please select a field first...' + ); expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').at(1).text()).toEqual( 'Search' ); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').at(1).text()).toEqual( + expect(wrapper.find('[data-test-subj="operatorAutocompleteComboBox"]').at(1).text()).toEqual( 'is' ); - expect( - wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatch"]').at(1).text() - ).toEqual('Search field value...'); + expect(wrapper.find('[data-test-subj="valuesAutocompleteMatch"]').at(1).text()).toEqual( + 'Please select a field first...' + ); }); }); test('it adds an exception item when "or" clicked', async () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { expect(item1.find('[data-test-subj="exceptionBuilderEntryField"]').at(0).text()).toEqual( 'Search' ); - expect(item1.find('[data-test-subj="exceptionBuilderEntryOperator"]').at(0).text()).toEqual( + expect(item1.find('[data-test-subj="operatorAutocompleteComboBox"]').at(0).text()).toEqual( 'is' ); - expect(item1.find('[data-test-subj="exceptionBuilderEntryFieldMatch"]').at(0).text()).toEqual( - 'Search field value...' + expect(item1.find('[data-test-subj="valuesAutocompleteMatch"]').at(0).text()).toEqual( + 'Please select a field first...' ); expect(item2.find('[data-test-subj="exceptionBuilderEntryField"]').at(0).text()).toEqual( 'Search' ); - expect(item2.find('[data-test-subj="exceptionBuilderEntryOperator"]').at(0).text()).toEqual( + expect(item2.find('[data-test-subj="operatorAutocompleteComboBox"]').at(0).text()).toEqual( 'is' ); - expect(item2.find('[data-test-subj="exceptionBuilderEntryFieldMatch"]').at(0).text()).toEqual( - 'Search field value...' + expect(item2.find('[data-test-subj="valuesAutocompleteMatch"]').at(0).text()).toEqual( + 'Please select a field first...' ); }); }); test('it displays empty entry if user deletes last remaining entry', () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { ); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').text()).toEqual('ip'); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').text()).toEqual( + expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').at(0).text()).toEqual( + 'ip' + ); + expect(wrapper.find('[data-test-subj="operatorAutocompleteComboBox"]').at(0).text()).toEqual( 'is one of' ); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatchAny"]').text()).toEqual( + expect(wrapper.find('[data-test-subj="valuesAutocompleteMatchAny"]').at(0).text()).toEqual( 'some ip' ); wrapper.find('[data-test-subj="firstRowBuilderDeleteButton"] button').simulate('click'); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').text()).toEqual('Search'); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').text()).toEqual('is'); - expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatch"]').text()).toEqual( - 'Search field value...' + expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').at(0).text()).toEqual( + 'Search' + ); + expect(wrapper.find('[data-test-subj="operatorAutocompleteComboBox"]').at(0).text()).toEqual( + 'is' + ); + expect(wrapper.find('[data-test-subj="valuesAutocompleteMatch"]').at(0).text()).toEqual( + 'Please select a field first...' ); - - wrapper.unmount(); }); test('it displays "and" badge if at least one exception item includes more than one entry', () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { }); test('it does not display "and" badge if none of the exception items include more than one entry', () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { describe('nested entry', () => { test('it adds a nested entry when "add nested entry" clicked', async () => { - const wrapper = mount( + wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> { expect(entry2.find('[data-test-subj="exceptionBuilderEntryField"]').at(0).text()).toEqual( 'Search nested field' ); - expect( - entry2.find('[data-test-subj="exceptionBuilderEntryOperator"]').at(0).text() - ).toEqual('is'); + expect(entry2.find('[data-test-subj="operatorAutocompleteComboBox"]').at(0).text()).toEqual( + 'is' + ); expect( entry2.find('[data-test-subj="exceptionBuilderEntryFieldExists"]').at(0).text() ).toEqual(getEmptyValue()); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx index 5904e0034a51c..1edee7af029e4 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx @@ -52,11 +52,13 @@ const initialState: State = { addNested: false, exceptions: [], exceptionsToDelete: [], + errorExists: 0, }; interface OnChangeProps { exceptionItems: Array; exceptionsToDelete: ExceptionListItemSchema[]; + errorExists: boolean; } interface ExceptionBuilderProps { @@ -93,6 +95,7 @@ export const ExceptionBuilderComponent = ({ disableNested, disableOr, addNested, + errorExists, }, dispatch, ] = useReducer(exceptionsBuilderReducer(), { @@ -102,6 +105,16 @@ export const ExceptionBuilderComponent = ({ disableNested: isNestedDisabled, }); + const setErrorsExist = useCallback( + (hasErrors: boolean): void => { + dispatch({ + type: 'setErrorsExist', + errorExists: hasErrors, + }); + }, + [dispatch] + ); + const setUpdateExceptions = useCallback( (items: ExceptionsBuilderExceptionItem[]): void => { dispatch({ @@ -306,8 +319,12 @@ export const ExceptionBuilderComponent = ({ // Bubble up changes to parent useEffect(() => { - onChange({ exceptionItems: filterExceptionItems(exceptions), exceptionsToDelete }); - }, [onChange, exceptionsToDelete, exceptions]); + onChange({ + exceptionItems: filterExceptionItems(exceptions), + exceptionsToDelete, + errorExists: errorExists > 0, + }); + }, [onChange, exceptionsToDelete, exceptions, errorExists]); // Defaults builder to never be sans entry, instead // always falls back to an empty entry if user deletes all @@ -364,6 +381,7 @@ export const ExceptionBuilderComponent = ({ onDeleteExceptionItem={handleDeleteExceptionItem} onChangeExceptionItem={handleExceptionItemChange} onlyShowListOperators={containsValueListEntry(exceptions)} + setErrorsExist={setErrorsExist} /> diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/reducer.test.ts b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/reducer.test.ts index ee5bd1329f35b..a21108778297a 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/reducer.test.ts +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/reducer.test.ts @@ -21,6 +21,7 @@ const initialState: State = { addNested: false, exceptions: [], exceptionsToDelete: [], + errorExists: 0, }; describe('exceptionsBuilderReducer', () => { @@ -45,6 +46,7 @@ describe('exceptionsBuilderReducer', () => { addNested: false, exceptions: [], exceptionsToDelete: [], + errorExists: 0, }); }); @@ -244,6 +246,7 @@ describe('exceptionsBuilderReducer', () => { addNested: false, exceptions: [getExceptionListItemSchemaMock()], exceptionsToDelete: [], + errorExists: 0, }, { type: 'setDefault', @@ -290,6 +293,7 @@ describe('exceptionsBuilderReducer', () => { addNested: false, exceptions, exceptionsToDelete: [], + errorExists: 0, }, { type: 'setExceptionsToDelete', @@ -324,6 +328,7 @@ describe('exceptionsBuilderReducer', () => { addNested: false, exceptions: [getExceptionListItemSchemaMock()], exceptionsToDelete: [], + errorExists: 0, }, { type: 'setDisableAnd', @@ -344,6 +349,7 @@ describe('exceptionsBuilderReducer', () => { addNested: false, exceptions: [getExceptionListItemSchemaMock()], exceptionsToDelete: [], + errorExists: 0, }, { type: 'setDisableAnd', @@ -366,6 +372,7 @@ describe('exceptionsBuilderReducer', () => { addNested: false, exceptions: [getExceptionListItemSchemaMock()], exceptionsToDelete: [], + errorExists: 0, }, { type: 'setDisableOr', @@ -386,6 +393,7 @@ describe('exceptionsBuilderReducer', () => { addNested: false, exceptions: [getExceptionListItemSchemaMock()], exceptionsToDelete: [], + errorExists: 0, }, { type: 'setDisableOr', @@ -408,6 +416,7 @@ describe('exceptionsBuilderReducer', () => { addNested: true, exceptions: [getExceptionListItemSchemaMock()], exceptionsToDelete: [], + errorExists: 0, }, { type: 'setAddNested', @@ -428,6 +437,7 @@ describe('exceptionsBuilderReducer', () => { addNested: false, exceptions: [getExceptionListItemSchemaMock()], exceptionsToDelete: [], + errorExists: 0, }, { type: 'setAddNested', @@ -438,4 +448,69 @@ describe('exceptionsBuilderReducer', () => { expect(addNested).toBeTruthy(); }); }); + + describe('#setErrorsExist', () => { + test('should increase "errorExists" by one if payload is "true"', () => { + const { errorExists } = reducer( + { + disableAnd: false, + disableNested: true, + disableOr: false, + andLogicIncluded: true, + addNested: true, + exceptions: [getExceptionListItemSchemaMock()], + exceptionsToDelete: [], + errorExists: 0, + }, + { + type: 'setErrorsExist', + errorExists: true, + } + ); + + expect(errorExists).toEqual(1); + }); + + test('should decrease "errorExists" by one if payload is "false"', () => { + const { errorExists } = reducer( + { + disableAnd: false, + disableNested: false, + disableOr: false, + andLogicIncluded: true, + addNested: false, + exceptions: [getExceptionListItemSchemaMock()], + exceptionsToDelete: [], + errorExists: 1, + }, + { + type: 'setErrorsExist', + errorExists: false, + } + ); + + expect(errorExists).toEqual(0); + }); + + test('should not decrease "errorExists" if decreasing would dip into negative numbers', () => { + const { errorExists } = reducer( + { + disableAnd: false, + disableNested: false, + disableOr: false, + andLogicIncluded: true, + addNested: false, + exceptions: [getExceptionListItemSchemaMock()], + exceptionsToDelete: [], + errorExists: 0, + }, + { + type: 'setErrorsExist', + errorExists: false, + } + ); + + expect(errorExists).toEqual(0); + }); + }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/reducer.ts b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/reducer.ts index 431e91b917bef..dfb2af63a31bc 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/reducer.ts +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/reducer.ts @@ -17,6 +17,7 @@ export interface State { addNested: boolean; exceptions: ExceptionsBuilderExceptionItem[]; exceptionsToDelete: ExceptionListItemSchema[]; + errorExists: number; } export type Action = @@ -44,6 +45,10 @@ export type Action = | { type: 'setAddNested'; addNested: boolean; + } + | { + type: 'setErrorsExist'; + errorExists: boolean; }; export const exceptionsBuilderReducer = () => (state: State, action: Action): State => { @@ -105,6 +110,15 @@ export const exceptionsBuilderReducer = () => (state: State, action: Action): St addNested: action.addNested, }; } + case 'setErrorsExist': { + const { errorExists } = state; + const errTotal = action.errorExists ? errorExists + 1 : errorExists - 1; + + return { + ...state, + errorExists: errTotal < 0 ? 0 : errTotal, + }; + } default: return state; } diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.test.tsx index aa379a3aa58b2..9d017b7f1891e 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.test.tsx @@ -332,4 +332,26 @@ describe('When the edit exception modal is opened', () => { ).toBeDisabled(); }); }); + + test('when there are exception builder errors has the add exception button disabled', async () => { + const wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + const callProps = ExceptionBuilderComponent.mock.calls[0][0]; + await waitFor(() => callProps.onChange({ exceptionItems: [], errorExists: true })); + + expect( + wrapper.find('button[data-test-subj="edit-exception-confirm-button"]').getDOMNode() + ).toBeDisabled(); + }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.tsx index 257c8e8c4d873..8842503d3f3b5 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.tsx @@ -99,6 +99,7 @@ export const EditExceptionModal = memo(function EditExceptionModal({ }: EditExceptionModalProps) { const { http } = useKibana().services; const [comment, setComment] = useState(''); + const [errorsExist, setErrorExists] = useState(false); const { rule: maybeRule } = useRuleAsync(ruleId); const [updateError, setUpdateError] = useState(null); const [hasVersionConflict, setHasVersionConflict] = useState(false); @@ -109,8 +110,11 @@ export const EditExceptionModal = memo(function EditExceptionModal({ >([]); const { addError, addSuccess } = useAppToasts(); const { loading: isSignalIndexLoading, signalIndexName } = useSignalIndex(); + const memoSignalIndexName = useMemo(() => (signalIndexName !== null ? [signalIndexName] : []), [ + signalIndexName, + ]); const [isSignalIndexPatternLoading, { indexPatterns: signalIndexPatterns }] = useFetchIndex( - signalIndexName !== null ? [signalIndexName] : [] + memoSignalIndexName ); const [isIndexPatternLoading, { indexPatterns }] = useFetchIndex(ruleIndices); @@ -188,17 +192,23 @@ export const EditExceptionModal = memo(function EditExceptionModal({ }, [shouldDisableBulkClose]); const isSubmitButtonDisabled = useMemo( - () => exceptionItemsToAdd.every((item) => item.entries.length === 0) || hasVersionConflict, - [exceptionItemsToAdd, hasVersionConflict] + () => + exceptionItemsToAdd.every((item) => item.entries.length === 0) || + hasVersionConflict || + errorsExist, + [exceptionItemsToAdd, hasVersionConflict, errorsExist] ); const handleBuilderOnChange = useCallback( ({ exceptionItems, + errorExists, }: { exceptionItems: Array; + errorExists: boolean; }) => { setExceptionItemsToAdd(exceptionItems); + setErrorExists(errorExists); }, [setExceptionItemsToAdd] );