From 393d08b10c056927f8fb6a28c554e246cbf3e310 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Tue, 4 Aug 2020 09:23:56 -0400 Subject: [PATCH 1/3] cleaned up exceptions builder and added tests --- .../exceptions/add_exception_modal/index.tsx | 4 +- .../exceptions/builder/and_badge.test.tsx | 68 +++ .../exceptions/builder/and_badge.tsx | 55 +++ .../builder_button_options.stories.tsx | 108 ----- .../builder/entry_delete_button.test.tsx | 136 ++++++ .../builder/entry_delete_button.tsx | 68 +++ ...ntry_item.test.tsx => entry_item.test.tsx} | 17 +- ...{builder_entry_item.tsx => entry_item.tsx} | 3 - ..._item.test.tsx => exception_item.test.tsx} | 55 +-- ..._exception_item.tsx => exception_item.tsx} | 99 +---- .../exceptions/builder/helpers.test.tsx | 6 +- .../components/exceptions/builder/helpers.tsx | 19 +- .../exceptions/builder/index.test.tsx | 418 ++++++++++++++++++ .../components/exceptions/builder/index.tsx | 15 +- ...ptions.test.tsx => logic_buttons.test.tsx} | 22 +- ...r_button_options.tsx => logic_buttons.tsx} | 4 +- .../exceptions/edit_exception_modal/index.tsx | 4 +- .../public/graphql/introspection.json | 18 - .../security_solution/public/graphql/types.ts | 5 - .../security_solution/server/graphql/types.ts | 5 - 20 files changed, 829 insertions(+), 300 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.test.tsx create mode 100644 x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.tsx delete mode 100644 x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_button_options.stories.tsx create mode 100644 x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_delete_button.test.tsx create mode 100644 x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_delete_button.tsx rename x-pack/plugins/security_solution/public/common/components/exceptions/builder/{builder_entry_item.test.tsx => entry_item.test.tsx} (97%) rename x-pack/plugins/security_solution/public/common/components/exceptions/builder/{builder_entry_item.tsx => entry_item.tsx} (99%) rename x-pack/plugins/security_solution/public/common/components/exceptions/builder/{builder_exception_item.test.tsx => exception_item.test.tsx} (84%) rename x-pack/plugins/security_solution/public/common/components/exceptions/builder/{builder_exception_item.tsx => exception_item.tsx} (57%) create mode 100644 x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.test.tsx rename x-pack/plugins/security_solution/public/common/components/exceptions/builder/{builder_button_options.test.tsx => logic_buttons.test.tsx} (94%) rename x-pack/plugins/security_solution/public/common/components/exceptions/builder/{builder_button_options.tsx => logic_buttons.tsx} (94%) 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 e6eaa4947e404..7526c52d16fde 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 @@ -31,7 +31,7 @@ import * as i18n from './translations'; import { TimelineNonEcsData, Ecs } from '../../../../graphql/types'; import { useAppToasts } from '../../../hooks/use_app_toasts'; import { useKibana } from '../../../lib/kibana'; -import { ExceptionBuilder } from '../builder'; +import { ExceptionBuilderComponent } from '../builder'; import { Loader } from '../../loader'; import { useAddOrUpdateException } from '../use_add_exception'; import { useSignalIndex } from '../../../../detections/containers/detection_engine/alerts/use_signal_index'; @@ -317,7 +317,7 @@ export const AddExceptionModal = memo(function AddExceptionModal({ {i18n.EXCEPTION_BUILDER_INFO} - { + test('it renders exceptionItemEntryFirstRowAndBadge for very first exception item in builder', () => { + const wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + expect( + wrapper.find('[data-test-subj="exceptionItemEntryFirstRowAndBadge"]').exists() + ).toBeTruthy(); + }); + + test('it renders exceptionItemEntryInvisibleAndBadge if "andLogicIncluded" and "entriesLength" is 1 or less', () => { + const wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + expect( + wrapper.find('[data-test-subj="exceptionItemEntryInvisibleAndBadge"]').exists() + ).toBeTruthy(); + }); + + test('it renders regular "and" badge if "entriesLength" is greater than 1, "andLogicIncluded" is true, and "exceptionItemIndex" is not 0', () => { + const wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + expect(wrapper.find('[data-test-subj="exceptionItemEntryAndBadge"]').exists()).toBeTruthy(); + }); + + test('it does not render a badge if "andLogicIncluded" is false', () => { + const wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + expect(wrapper.find('[data-test-subj="exceptionItemEntryAndBadge"]').exists()).toBeFalsy(); + expect( + wrapper.find('[data-test-subj="exceptionItemEntryInvisibleAndBadge"]').exists() + ).toBeFalsy(); + expect( + wrapper.find('[data-test-subj="exceptionItemEntryFirstRowAndBadge"]').exists() + ).toBeFalsy(); + }); +}); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.tsx new file mode 100644 index 0000000000000..ef9c68e9d988e --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.tsx @@ -0,0 +1,55 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * 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 from 'react'; +import { EuiFlexItem } from '@elastic/eui'; +import styled from 'styled-components'; + +import { AndOrBadge } from '../../and_or_badge'; + +const MyInvisibleAndBadge = styled(EuiFlexItem)` + visibility: hidden; +`; + +const MyFirstRowContainer = styled(EuiFlexItem)` + padding-top: 20px; +`; + +interface BuilderAndBadgeProps { + entriesLength: number; + exceptionItemIndex: number; + andLogicIncluded: boolean; +} + +export const BuilderAndBadgeComponent = React.memo( + ({ entriesLength, exceptionItemIndex, andLogicIncluded }) => { + const badge = ; + + if (andLogicIncluded && entriesLength > 1 && exceptionItemIndex === 0) { + return ( + + {badge} + + ); + } else if (andLogicIncluded && entriesLength <= 1) { + return ( + + {badge} + + ); + } else if (andLogicIncluded && entriesLength > 1) { + return ( + + {badge} + + ); + } else { + return <>; + } + } +); + +BuilderAndBadgeComponent.displayName = 'BuilderAndBadge'; diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_button_options.stories.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_button_options.stories.tsx deleted file mode 100644 index 5ca2d2b86a527..0000000000000 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_button_options.stories.tsx +++ /dev/null @@ -1,108 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -import { storiesOf, addDecorator } from '@storybook/react'; -import { action } from '@storybook/addon-actions'; -import React from 'react'; -import { ThemeProvider } from 'styled-components'; -import euiLightVars from '@elastic/eui/dist/eui_theme_light.json'; - -import { BuilderButtonOptions } from './builder_button_options'; - -addDecorator((storyFn) => ( - ({ eui: euiLightVars, darkMode: false })}>{storyFn()} -)); - -storiesOf('Components|Exceptions|BuilderButtonOptions', module) - .add('and/or buttons', () => { - return ( - - ); - }) - .add('nested button - isNested false', () => { - return ( - - ); - }) - .add('nested button - isNested true', () => { - return ( - - ); - }) - .add('and disabled', () => { - return ( - - ); - }) - .add('or disabled', () => { - return ( - - ); - }) - .add('nested disabled', () => { - return ( - - ); - }); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_delete_button.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_delete_button.test.tsx new file mode 100644 index 0000000000000..b766a0536d237 --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_delete_button.test.tsx @@ -0,0 +1,136 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { mount } from 'enzyme'; +import React from 'react'; + +import { getExceptionListItemSchemaMock } from '../../../../../../lists/common/schemas/response/exception_list_item_schema.mock'; +import { getEntryMatchMock } from '../../../../../../lists/common/schemas/types/entry_match.mock'; + +import { BuilderEntryDeleteButtonComponent } from './entry_delete_button'; + +describe('BuilderEntryDeleteButtonComponent', () => { + test('it renders firstRowBuilderDeleteButton for very first entry in builder', () => { + const wrapper = mount( + + ); + + expect(wrapper.find('[data-test-subj="firstRowBuilderDeleteButton"] button')).toHaveLength(1); + }); + + test('it does not render firstRowBuilderDeleteButton if entryIndex is not 0', () => { + const wrapper = mount( + + ); + + expect(wrapper.find('[data-test-subj="firstRowBuilderDeleteButton"]')).toHaveLength(0); + expect(wrapper.find('[data-test-subj="builderDeleteButton"] button')).toHaveLength(1); + }); + + test('it does not render firstRowBuilderDeleteButton if exceptionItemIndex is not 0', () => { + const wrapper = mount( + + ); + + expect(wrapper.find('[data-test-subj="firstRowBuilderDeleteButton"]')).toHaveLength(0); + expect(wrapper.find('[data-test-subj="builderDeleteButton"] button')).toHaveLength(1); + }); + + test('it does not render firstRowBuilderDeleteButton if nestedParentIndex is not null', () => { + const wrapper = mount( + + ); + + expect(wrapper.find('[data-test-subj="firstRowBuilderDeleteButton"]')).toHaveLength(0); + expect(wrapper.find('[data-test-subj="builderDeleteButton"] button')).toHaveLength(1); + }); + + test('it invokes "onDelete" when button is clicked', () => { + const onDelete = jest.fn(); + + const wrapper = mount( + + ); + + wrapper.find('[data-test-subj="builderDeleteButton"] button').simulate('click'); + + expect(onDelete).toHaveBeenCalledTimes(1); + expect(onDelete).toHaveBeenCalledWith(0, null); + }); + + test('it disables button if it is the only entry left and no field has been selected', () => { + const exceptionItem = { + ...getExceptionListItemSchemaMock(), + entries: [{ ...getEntryMatchMock(), field: '' }], + }; + const wrapper = mount( + + ); + + const button = wrapper.find('[data-test-subj="builderDeleteButton"] button').at(0); + + expect(button.prop('disabled')).toBeTruthy(); + }); + + test('it does not disable button if it is the only entry left and field has been selected', () => { + const wrapper = mount( + + ); + + const button = wrapper.find('[data-test-subj="builderDeleteButton"] button').at(0); + + expect(button.prop('disabled')).toBeFalsy(); + }); +}); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_delete_button.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_delete_button.tsx new file mode 100644 index 0000000000000..e63f95064cba0 --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_delete_button.tsx @@ -0,0 +1,68 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * 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 } from 'react'; +import { EuiButtonIcon, EuiFlexItem } from '@elastic/eui'; +import styled from 'styled-components'; + +import { BuilderEntry } from '../types'; + +const MyFirstRowContainer = styled(EuiFlexItem)` + padding-top: 20px; +`; + +interface BuilderEntryDeleteButtonProps { + entries: BuilderEntry[]; + isOnlyItem: boolean; + entryIndex: number; + exceptionItemIndex: number; + nestedParentIndex: number | null; + onDelete: (item: number, parent: number | null) => void; +} + +export const BuilderEntryDeleteButtonComponent = React.memo( + ({ entries, nestedParentIndex, isOnlyItem, entryIndex, exceptionItemIndex, onDelete }) => { + const isDisabled: boolean = + isOnlyItem && + entries.length === 1 && + exceptionItemIndex === 0 && + (entries[0].field == null || entries[0].field === ''); + + const handleDelete = useCallback((): void => { + onDelete(entryIndex, nestedParentIndex); + }, [onDelete, entryIndex, nestedParentIndex]); + + const button = ( + + ); + + if (entryIndex === 0 && exceptionItemIndex === 0 && nestedParentIndex == null) { + // This logic was added to work around it including the field + // labels in centering the delete icon for the first row + return ( + + {button} + + ); + } else { + return ( + + {button} + + ); + } + } +); + +BuilderEntryDeleteButtonComponent.displayName = 'BuilderEntryDeleteButton'; diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_entry_item.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.test.tsx similarity index 97% rename from x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_entry_item.test.tsx rename to x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.test.tsx index 0f54ec29cc540..2a116c4cd8acf 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_entry_item.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.test.tsx @@ -8,7 +8,7 @@ import { mount } from 'enzyme'; import React from 'react'; import { EuiComboBox, EuiComboBoxOptionOption } from '@elastic/eui'; -import { BuilderEntryItem } from './builder_entry_item'; +import { BuilderEntryItem } from './entry_item'; import { isOperator, isNotOperator, @@ -64,7 +64,6 @@ describe('BuilderEntryItem', () => { }} showLabel={true} listType="detection" - addNested={false} onChange={jest.fn()} /> ); @@ -91,7 +90,6 @@ describe('BuilderEntryItem', () => { }} showLabel={false} listType="detection" - addNested={false} onChange={jest.fn()} /> ); @@ -122,7 +120,6 @@ describe('BuilderEntryItem', () => { }} showLabel={false} listType="detection" - addNested={false} onChange={jest.fn()} /> ); @@ -155,7 +152,6 @@ describe('BuilderEntryItem', () => { }} showLabel={false} listType="detection" - addNested={false} onChange={jest.fn()} /> ); @@ -188,7 +184,6 @@ describe('BuilderEntryItem', () => { }} showLabel={false} listType="detection" - addNested={false} onChange={jest.fn()} /> ); @@ -221,7 +216,6 @@ describe('BuilderEntryItem', () => { }} showLabel={true} listType="detection" - addNested={false} onChange={jest.fn()} /> ); @@ -254,7 +248,6 @@ describe('BuilderEntryItem', () => { }} showLabel={true} listType="detection" - addNested={false} onChange={jest.fn()} /> ); @@ -287,7 +280,6 @@ describe('BuilderEntryItem', () => { }} showLabel={false} listType="detection" - addNested={false} onChange={jest.fn()} /> ); @@ -323,7 +315,6 @@ describe('BuilderEntryItem', () => { }} showLabel={false} listType="detection" - addNested={false} onChange={jest.fn()} /> ); @@ -377,7 +368,6 @@ describe('BuilderEntryItem', () => { }} showLabel={false} listType="detection" - addNested={false} onChange={jest.fn()} /> ); @@ -416,7 +406,6 @@ describe('BuilderEntryItem', () => { }} showLabel={false} listType="detection" - addNested={false} onChange={mockOnChange} /> ); @@ -451,7 +440,6 @@ describe('BuilderEntryItem', () => { }} showLabel={false} listType="detection" - addNested={false} onChange={mockOnChange} /> ); @@ -486,7 +474,6 @@ describe('BuilderEntryItem', () => { }} showLabel={false} listType="detection" - addNested={false} onChange={mockOnChange} /> ); @@ -521,7 +508,6 @@ describe('BuilderEntryItem', () => { }} showLabel={false} listType="detection" - addNested={false} onChange={mockOnChange} /> ); @@ -556,7 +542,6 @@ describe('BuilderEntryItem', () => { }} showLabel={false} listType="detection" - addNested={false} onChange={mockOnChange} /> ); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_entry_item.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.tsx similarity index 99% rename from x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_entry_item.tsx rename to x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.tsx index 3883a2fad2cf2..3044f6d01b745 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_entry_item.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.tsx @@ -34,7 +34,6 @@ interface EntryItemProps { indexPattern: IIndexPattern; showLabel: boolean; listType: ExceptionListType; - addNested: boolean; onChange: (arg: BuilderEntry, i: number) => void; onlyShowListOperators?: boolean; } @@ -43,7 +42,6 @@ export const BuilderEntryItem: React.FC = ({ entry, indexPattern, listType, - addNested, showLabel, onChange, onlyShowListOperators = false, @@ -51,7 +49,6 @@ export const BuilderEntryItem: React.FC = ({ const handleFieldChange = useCallback( ([newField]: IFieldType[]): void => { const { updatedEntry, index } = getEntryOnFieldChange(entry, newField); - onChange(updatedEntry, index); }, [onChange, entry] diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_exception_item.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.test.tsx similarity index 84% rename from x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_exception_item.test.tsx rename to x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.test.tsx index 7624ce147abd9..e90639a2c0285 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_exception_item.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.test.tsx @@ -15,11 +15,11 @@ import { getExceptionListItemSchemaMock } from '../../../../../../lists/common/s import { getEntryMatchMock } from '../../../../../../lists/common/schemas/types/entry_match.mock'; import { getEntryMatchAnyMock } from '../../../../../../lists/common/schemas/types/entry_match_any.mock'; -import { ExceptionListItemComponent } from './builder_exception_item'; +import { BuilderExceptionListItemComponent } from './exception_item'; jest.mock('../../../../common/lib/kibana'); -describe('ExceptionListItemComponent', () => { +describe('BuilderExceptionListItemComponent', () => { const getValueSuggestionsMock = jest.fn().mockResolvedValue(['value 1', 'value 2']); beforeAll(() => { @@ -46,7 +46,7 @@ describe('ExceptionListItemComponent', () => { }; const wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> - { andLogicIncluded={true} isOnlyItem={false} listType="detection" - addNested={false} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> @@ -71,11 +70,11 @@ describe('ExceptionListItemComponent', () => { }); test('it renders "and" badge when more than one exception item entry exists and it is not the first exception item', () => { - const exceptionItem = { ...getExceptionListItemSchemaMock() }; + const exceptionItem = getExceptionListItemSchemaMock(); exceptionItem.entries = [getEntryMatchMock(), getEntryMatchMock()]; const wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> - { andLogicIncluded={true} isOnlyItem={false} listType="detection" - addNested={false} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> @@ -98,11 +96,11 @@ describe('ExceptionListItemComponent', () => { }); test('it renders indented "and" badge when "andLogicIncluded" is "true" and only one entry exists', () => { - const exceptionItem = { ...getExceptionListItemSchemaMock() }; + const exceptionItem = getExceptionListItemSchemaMock(); exceptionItem.entries = [getEntryMatchMock()]; const wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> - { andLogicIncluded={true} isOnlyItem={false} listType="detection" - addNested={false} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> @@ -127,11 +124,11 @@ describe('ExceptionListItemComponent', () => { }); test('it renders no "and" badge when "andLogicIncluded" is "false"', () => { - const exceptionItem = { ...getExceptionListItemSchemaMock() }; + const exceptionItem = getExceptionListItemSchemaMock(); exceptionItem.entries = [getEntryMatchMock()]; const wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> - { andLogicIncluded={false} isOnlyItem={false} listType="detection" - addNested={false} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> @@ -167,7 +163,7 @@ describe('ExceptionListItemComponent', () => { entries: [{ ...getEntryMatchMock(), field: '' }], }; const wrapper = mount( - { andLogicIncluded={false} isOnlyItem={true} listType="detection" - addNested={false} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> ); expect( - wrapper.find('[data-test-subj="exceptionItemEntryDeleteButton"] button').props().disabled + wrapper.find('[data-test-subj="builderItemEntryDeleteButton"] button').props().disabled ).toBeTruthy(); }); test('it does not render delete button disabled when it is not the only entry left in builder', () => { - const exceptionItem = { ...getExceptionListItemSchemaMock() }; + const exceptionItem = getExceptionListItemSchemaMock(); exceptionItem.entries = [getEntryMatchMock()]; const wrapper = mount( - { andLogicIncluded={false} isOnlyItem={false} listType="detection" - addNested={false} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> ); expect( - wrapper.find('[data-test-subj="exceptionItemEntryDeleteButton"] button').props().disabled + wrapper.find('[data-test-subj="builderItemEntryDeleteButton"] button').props().disabled ).toBeFalsy(); }); test('it does not render delete button disabled when "exceptionItemIndex" is not "0"', () => { - const exceptionItem = { ...getExceptionListItemSchemaMock() }; + const exceptionItem = getExceptionListItemSchemaMock(); exceptionItem.entries = [getEntryMatchMock()]; const wrapper = mount( - { // this to be true, but done for testing purposes isOnlyItem={true} listType="detection" - addNested={false} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> ); expect( - wrapper.find('[data-test-subj="exceptionItemEntryDeleteButton"] button').props().disabled + wrapper.find('[data-test-subj="builderItemEntryDeleteButton"] button').props().disabled ).toBeFalsy(); }); test('it does not render delete button disabled when more than one entry exists', () => { - const exceptionItem = { ...getExceptionListItemSchemaMock() }; + const exceptionItem = getExceptionListItemSchemaMock(); exceptionItem.entries = [getEntryMatchMock(), getEntryMatchMock()]; const wrapper = mount( - { andLogicIncluded={false} isOnlyItem={true} listType="detection" - addNested={false} onDeleteExceptionItem={jest.fn()} onChangeExceptionItem={jest.fn()} /> ); expect( - wrapper.find('[data-test-subj="exceptionItemEntryDeleteButton"] button').at(0).props() + wrapper.find('[data-test-subj="builderItemEntryDeleteButton"] button').at(0).props() .disabled ).toBeFalsy(); }); test('it invokes "onChangeExceptionItem" when delete button clicked', () => { const mockOnDeleteExceptionItem = jest.fn(); - const exceptionItem = { ...getExceptionListItemSchemaMock() }; + const exceptionItem = getExceptionListItemSchemaMock(); exceptionItem.entries = [getEntryMatchMock(), getEntryMatchAnyMock()]; const wrapper = mount( - { andLogicIncluded={false} isOnlyItem={true} listType="detection" - addNested={false} onDeleteExceptionItem={mockOnDeleteExceptionItem} onChangeExceptionItem={jest.fn()} /> ); wrapper - .find('[data-test-subj="exceptionItemEntryDeleteButton"] button') + .find('[data-test-subj="builderItemEntryDeleteButton"] button') .at(0) .simulate('click'); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_exception_item.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.tsx similarity index 57% rename from x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_exception_item.tsx rename to x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.tsx index 50a6150833796..aa4f74febc5a2 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/builder_exception_item.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.tsx @@ -5,23 +5,16 @@ */ import React, { useMemo, useCallback } from 'react'; -import { EuiButtonIcon, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; +import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; import styled from 'styled-components'; import { IIndexPattern } from '../../../../../../../../src/plugins/data/common'; -import { AndOrBadge } from '../../and_or_badge'; -import { BuilderEntryItem } from './builder_entry_item'; import { getFormattedBuilderEntries, getUpdatedEntriesOnDelete } from './helpers'; import { FormattedBuilderEntry, ExceptionsBuilderExceptionItem, BuilderEntry } from '../types'; import { ExceptionListType } from '../../../../../public/lists_plugin_deps'; - -const MyInvisibleAndBadge = styled(EuiFlexItem)` - visibility: hidden; -`; - -const MyFirstRowContainer = styled(EuiFlexItem)` - padding-top: 20px; -`; +import { BuilderEntryItem } from './entry_item'; +import { BuilderEntryDeleteButtonComponent } from './entry_delete_button'; +import { BuilderAndBadgeComponent } from './and_badge'; const MyBeautifulLine = styled(EuiFlexItem)` &:after { @@ -33,7 +26,7 @@ const MyBeautifulLine = styled(EuiFlexItem)` } `; -interface ExceptionListItemProps { +interface BuilderExceptionListItemProps { exceptionItem: ExceptionsBuilderExceptionItem; exceptionId: string; exceptionItemIndex: number; @@ -41,13 +34,12 @@ interface ExceptionListItemProps { andLogicIncluded: boolean; isOnlyItem: boolean; listType: ExceptionListType; - addNested: boolean; onDeleteExceptionItem: (item: ExceptionsBuilderExceptionItem, index: number) => void; onChangeExceptionItem: (item: ExceptionsBuilderExceptionItem, index: number) => void; onlyShowListOperators?: boolean; } -export const ExceptionListItemComponent = React.memo( +export const BuilderExceptionListItemComponent = React.memo( ({ exceptionItem, exceptionId, @@ -55,7 +47,6 @@ export const ExceptionListItemComponent = React.memo( indexPattern, isOnlyItem, listType, - addNested, andLogicIncluded, onDeleteExceptionItem, onChangeExceptionItem, @@ -81,8 +72,8 @@ export const ExceptionListItemComponent = React.memo( (entryIndex: number, parentIndex: number | null): void => { const updatedExceptionItem = getUpdatedEntriesOnDelete( exceptionItem, - parentIndex ? parentIndex : entryIndex, - parentIndex ? entryIndex : null + entryIndex, + parentIndex ); onDeleteExceptionItem(updatedExceptionItem, exceptionItemIndex); @@ -98,63 +89,14 @@ export const ExceptionListItemComponent = React.memo( [exceptionItem.entries, indexPattern] ); - const getAndBadge = useCallback((): JSX.Element => { - const badge = ; - - if (andLogicIncluded && exceptionItem.entries.length > 1 && exceptionItemIndex === 0) { - return ( - - {badge} - - ); - } else if (andLogicIncluded && exceptionItem.entries.length <= 1) { - return ( - - {badge} - - ); - } else if (andLogicIncluded && exceptionItem.entries.length > 1) { - return ( - - {badge} - - ); - } else { - return <>; - } - }, [exceptionItem.entries.length, exceptionItemIndex, andLogicIncluded]); - - const getDeleteButton = useCallback( - (entryIndex: number, parentIndex: number | null): JSX.Element => { - const button = ( - handleDeleteEntry(entryIndex, parentIndex)} - isDisabled={ - isOnlyItem && - exceptionItem.entries.length === 1 && - exceptionItemIndex === 0 && - (exceptionItem.entries[0].field == null || exceptionItem.entries[0].field === '') - } - aria-label="entryDeleteButton" - className="exceptionItemEntryDeleteButton" - data-test-subj="exceptionItemEntryDeleteButton" - /> - ); - if (entryIndex === 0 && exceptionItemIndex === 0 && parentIndex == null) { - return {button}; - } else { - return {button}; - } - }, - [exceptionItemIndex, exceptionItem.entries, handleDeleteEntry, isOnlyItem] - ); - return ( - {getAndBadge()} + {entries.map((item, index) => ( @@ -166,7 +108,6 @@ export const ExceptionListItemComponent = React.memo( entry={item} indexPattern={indexPattern} listType={listType} - addNested={addNested} showLabel={ exceptionItemIndex === 0 && index === 0 && item.nested !== 'child' } @@ -174,10 +115,14 @@ export const ExceptionListItemComponent = React.memo( onlyShowListOperators={onlyShowListOperators} /> - {getDeleteButton( - item.entryIndex, - item.parent != null ? item.parent.parentIndex : null - )} + ))} @@ -189,4 +134,4 @@ export const ExceptionListItemComponent = React.memo( } ); -ExceptionListItemComponent.displayName = 'ExceptionListItem'; +BuilderExceptionListItemComponent.displayName = 'BuilderExceptionListItem'; 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 224c99756eb5c..a3c5d09a0fb64 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 @@ -654,7 +654,7 @@ describe('Exception builder helpers', () => { expect(output).toEqual(expected); }); - test('it removes entry corresponding to "nestedEntryIndex"', () => { + test('it removes nested entry of "entryIndex" with corresponding parent index', () => { const payloadItem: ExceptionsBuilderExceptionItem = { ...getExceptionListItemSchemaMock(), entries: [ @@ -664,10 +664,10 @@ describe('Exception builder helpers', () => { }, ], }; - const output = getUpdatedEntriesOnDelete(payloadItem, 0, 1); + const output = getUpdatedEntriesOnDelete(payloadItem, 0, 0); const expected: ExceptionsBuilderExceptionItem = { ...getExceptionListItemSchemaMock(), - entries: [{ ...getEntryNestedMock(), entries: [{ ...getEntryExistsMock() }] }], + entries: [{ ...getEntryNestedMock(), entries: [{ ...getEntryMatchAnyMock() }] }], }; 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 8585f58504e31..85e95c2698f02 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 @@ -248,22 +248,23 @@ export const getFormattedBuilderEntries = ( export const getUpdatedEntriesOnDelete = ( exceptionItem: ExceptionsBuilderExceptionItem, entryIndex: number, - nestedEntryIndex: number | null + nestedParentIndex: number | null ): ExceptionsBuilderExceptionItem => { - const itemOfInterest: BuilderEntry = exceptionItem.entries[entryIndex]; + const itemOfInterest: BuilderEntry = + exceptionItem.entries[nestedParentIndex != null ? nestedParentIndex : entryIndex]; - if (nestedEntryIndex != null && itemOfInterest.type === OperatorTypeEnum.NESTED) { + if (nestedParentIndex != null && itemOfInterest.type === OperatorTypeEnum.NESTED) { const updatedEntryEntries: Array = [ - ...itemOfInterest.entries.slice(0, nestedEntryIndex), - ...itemOfInterest.entries.slice(nestedEntryIndex + 1), + ...itemOfInterest.entries.slice(0, entryIndex), + ...itemOfInterest.entries.slice(entryIndex + 1), ]; if (updatedEntryEntries.length === 0) { return { ...exceptionItem, entries: [ - ...exceptionItem.entries.slice(0, entryIndex), - ...exceptionItem.entries.slice(entryIndex + 1), + ...exceptionItem.entries.slice(0, nestedParentIndex), + ...exceptionItem.entries.slice(nestedParentIndex + 1), ], }; } else { @@ -277,9 +278,9 @@ export const getUpdatedEntriesOnDelete = ( return { ...exceptionItem, entries: [ - ...exceptionItem.entries.slice(0, entryIndex), + ...exceptionItem.entries.slice(0, nestedParentIndex), updatedItemOfInterest, - ...exceptionItem.entries.slice(entryIndex + 1), + ...exceptionItem.entries.slice(nestedParentIndex + 1), ], }; } 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 new file mode 100644 index 0000000000000..458c8635bcce3 --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.test.tsx @@ -0,0 +1,418 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * 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 from 'react'; +import { ThemeProvider } from 'styled-components'; +import { mount } from 'enzyme'; +import euiLightVars from '@elastic/eui/dist/eui_theme_light.json'; +import { wait as waitFor } from '@testing-library/react'; + +import { + fields, + getField, +} from '../../../../../../../../src/plugins/data/common/index_patterns/fields/fields.mocks.ts'; +import { getExceptionListItemSchemaMock } from '../../../../../../lists/common/schemas/response/exception_list_item_schema.mock'; +import { getEntryMatchAnyMock } from '../../../../../../lists/common/schemas/types/entry_match_any.mock'; + +import { useKibana } from '../../../../common/lib/kibana'; +import { getEmptyValue } from '../../empty_value'; + +import { ExceptionBuilderComponent } from './'; + +jest.mock('../../../../common/lib/kibana'); + +describe('ExceptionBuilderComponent', () => { + const getValueSuggestionsMock = jest.fn().mockResolvedValue(['value 1', 'value 2']); + + beforeAll(() => { + (useKibana as jest.Mock).mockReturnValue({ + services: { + data: { + autocomplete: { + getValueSuggestions: getValueSuggestionsMock, + }, + }, + }, + }); + }); + + afterEach(() => { + getValueSuggestionsMock.mockClear(); + }); + + test('it displays empty entry if no "exceptionListItems" are passed in', () => { + const 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...' + ); + }); + + test('it displays "exceptionListItems" that are passed in', async () => { + const 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( + 'is one of' + ); + expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatchAny"]').text()).toEqual( + 'some ip' + ); + }); + + test('it displays "or", "and" and "add nested button" enabled', () => { + const wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + expect( + wrapper.find('[data-test-subj="exceptionsAndButton"] button').prop('disabled') + ).toBeFalsy(); + expect( + wrapper.find('[data-test-subj="exceptionsOrButton"] button').prop('disabled') + ).toBeFalsy(); + expect( + wrapper.find('[data-test-subj="exceptionsNestedButton"] button').prop('disabled') + ).toBeFalsy(); + }); + + test('it adds an entry when "and" clicked', async () => { + const wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + expect(wrapper.find('EuiFlexGroup[data-test-subj="exceptionItemEntryContainer"]')).toHaveLength( + 1 + ); + + wrapper.find('[data-test-subj="exceptionsAndButton"] button').simulate('click'); + + await waitFor(() => { + expect( + wrapper.find('EuiFlexGroup[data-test-subj="exceptionItemEntryContainer"]') + ).toHaveLength(2); + expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').at(0).text()).toEqual( + 'Search' + ); + expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').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="exceptionBuilderEntryField"]').at(1).text()).toEqual( + 'Search' + ); + expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').at(1).text()).toEqual( + 'is' + ); + expect( + wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatch"]').at(1).text() + ).toEqual('Search field value...'); + }); + }); + + test('it adds an exception item when "or" clicked', async () => { + const wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + expect(wrapper.find('EuiFlexGroup[data-test-subj="exceptionEntriesContainer"]')).toHaveLength( + 1 + ); + + wrapper.find('[data-test-subj="exceptionsOrButton"] button').simulate('click'); + + await waitFor(() => { + expect(wrapper.find('EuiFlexGroup[data-test-subj="exceptionEntriesContainer"]')).toHaveLength( + 2 + ); + + const item1 = wrapper.find('EuiFlexGroup[data-test-subj="exceptionEntriesContainer"]').at(0); + const item2 = wrapper.find('EuiFlexGroup[data-test-subj="exceptionEntriesContainer"]').at(1); + + expect(item1.find('[data-test-subj="exceptionBuilderEntryField"]').at(0).text()).toEqual( + 'Search' + ); + expect(item1.find('[data-test-subj="exceptionBuilderEntryOperator"]').at(0).text()).toEqual( + 'is' + ); + expect(item1.find('[data-test-subj="exceptionBuilderEntryFieldMatch"]').at(0).text()).toEqual( + 'Search field value...' + ); + + expect(item2.find('[data-test-subj="exceptionBuilderEntryField"]').at(0).text()).toEqual( + 'Search' + ); + expect(item2.find('[data-test-subj="exceptionBuilderEntryOperator"]').at(0).text()).toEqual( + 'is' + ); + expect(item2.find('[data-test-subj="exceptionBuilderEntryFieldMatch"]').at(0).text()).toEqual( + 'Search field value...' + ); + }); + }); + + test('it displays empty entry if user deletes last remaining entry', () => { + const 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( + 'is one of' + ); + expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatchAny"]').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...' + ); + }); + + test('it displays "and" badge if at least one exception item includes more than one entry', () => { + const wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + expect( + wrapper.find('[data-test-subj="exceptionItemEntryFirstRowAndBadge"]').exists() + ).toBeFalsy(); + + wrapper.find('[data-test-subj="exceptionsAndButton"] button').simulate('click'); + + expect( + wrapper.find('[data-test-subj="exceptionItemEntryFirstRowAndBadge"]').exists() + ).toBeTruthy(); + }); + + test('it does not display "and" badge if none of the exception items include more than one entry', () => { + const wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + wrapper.find('[data-test-subj="exceptionsOrButton"] button').simulate('click'); + + expect( + wrapper.find('[data-test-subj="exceptionItemEntryFirstRowAndBadge"]').exists() + ).toBeFalsy(); + + wrapper.find('[data-test-subj="exceptionsOrButton"] button').simulate('click'); + + expect( + wrapper.find('[data-test-subj="exceptionItemEntryFirstRowAndBadge"]').exists() + ).toBeFalsy(); + }); + + describe('nested entry', () => { + test('it adds a nested entry when "add nested entry" clicked', async () => { + const wrapper = mount( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + wrapper.find('[data-test-subj="exceptionsNestedButton"] button').simulate('click'); + + await waitFor(() => { + const entry2 = wrapper + .find('EuiFlexGroup[data-test-subj="exceptionItemEntryContainer"]') + .at(1); + 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="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 b82607a541aaa..165f3314c2f15 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 @@ -7,7 +7,7 @@ import React, { useCallback, useEffect, useReducer } from 'react'; import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; import styled from 'styled-components'; -import { ExceptionListItemComponent } from './builder_exception_item'; +import { BuilderExceptionListItemComponent } from './exception_item'; import { IIndexPattern } from '../../../../../../../../src/plugins/data/common'; import { ExceptionListItemSchema, @@ -20,7 +20,7 @@ import { entriesNested, } from '../../../../../public/lists_plugin_deps'; import { AndOrBadge } from '../../and_or_badge'; -import { BuilderButtonOptions } from './builder_button_options'; +import { BuilderLogicButtons } from './logic_buttons'; import { getNewExceptionItem, filterExceptionItems } from '../helpers'; import { ExceptionsBuilderExceptionItem, CreateExceptionListItemBuilderSchema } from '../types'; import { State, exceptionsBuilderReducer } from './reducer'; @@ -72,7 +72,7 @@ interface ExceptionBuilderProps { onChange: (arg: OnChangeProps) => void; } -export const ExceptionBuilder = ({ +export const ExceptionBuilderComponent = ({ exceptionListItems, listType, listId, @@ -310,6 +310,8 @@ export const ExceptionBuilder = ({ onChange({ exceptionItems: filterExceptionItems(exceptions), exceptionsToDelete }); }, [onChange, exceptionsToDelete, exceptions]); + // Defaults builder to never be sans entry, instead + // always falls back to an empty entry if user deletes all useEffect(() => { if ( exceptions.length === 0 || @@ -351,13 +353,12 @@ export const ExceptionBuilder = ({ ))} - )} - { +describe('BuilderLogicButtons', () => { test('it renders "and" and "or" buttons', () => { const wrapper = mount( - { const onOrClicked = jest.fn(); const wrapper = mount( - { const onAndClicked = jest.fn(); const wrapper = mount( - { const onAddClickWhenNested = jest.fn(); const wrapper = mount( - { test('it disables "and" button if "isAndDisabled" is true', () => { const wrapper = mount( - { test('it disables "or" button if "isOrDisabled" is "true"', () => { const wrapper = mount( - { test('it disables "add nested" button if "isNestedDisabled" is "true"', () => { const wrapper = mount( - { const onNestedClicked = jest.fn(); const wrapper = mount( - { const onAndClicked = jest.fn(); const wrapper = mount( - void; } -export const BuilderButtonOptions: React.FC = ({ +export const BuilderLogicButtons: React.FC = ({ isOrDisabled = false, isAndDisabled = false, showNestedButton = false, 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 6109b85f2da5a..e1352ac38dc49 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 @@ -31,7 +31,7 @@ import { import * as i18n from './translations'; import { useKibana } from '../../../lib/kibana'; import { useAppToasts } from '../../../hooks/use_app_toasts'; -import { ExceptionBuilder } from '../builder'; +import { ExceptionBuilderComponent } from '../builder'; import { useAddOrUpdateException } from '../use_add_exception'; import { AddExceptionComments } from '../add_exception_comments'; import { @@ -232,7 +232,7 @@ export const EditExceptionModal = memo(function EditExceptionModal({ {i18n.EXCEPTION_BUILDER_INFO} - Date: Tue, 4 Aug 2020 10:28:24 -0400 Subject: [PATCH 2/3] updated stories --- .../builder/logic_buttons.stories.tsx | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 x-pack/plugins/security_solution/public/common/components/exceptions/builder/logic_buttons.stories.tsx diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/logic_buttons.stories.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/logic_buttons.stories.tsx new file mode 100644 index 0000000000000..e4508fa2a9a1d --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/logic_buttons.stories.tsx @@ -0,0 +1,109 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { storiesOf, addDecorator } from '@storybook/react'; +import { action } from '@storybook/addon-actions'; +import React from 'react'; +import { ThemeProvider } from 'styled-components'; +import euiLightVars from '@elastic/eui/dist/eui_theme_light.json'; + +import { BuilderLogicButtons } from './logic_buttons'; + +addDecorator((storyFn) => ( + ({ eui: euiLightVars, darkMode: false })}>{storyFn()} +)); + +storiesOf('Exceptions|BuilderLogicButtons', module) + .add('and/or buttons', () => { + return ( + + ); + }) + .add('nested button - isNested false', () => { + return ( + + ); + }) + .add('nested button - isNested true', () => { + return ( + + ); + }) + .add('and disabled', () => { + return ( + + ); + }) + .add('or disabled', () => { + return ( + + ); + }) + .add('nested disabled', () => { + return ( + + ); + }); From 13edaa6e7ba39b456c3fcc0fc20d67168a5d6a19 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Tue, 4 Aug 2020 17:48:52 -0400 Subject: [PATCH 3/3] applied PR feedback --- .../exceptions/builder/and_badge.test.tsx | 30 +- .../exceptions/builder/and_badge.tsx | 11 +- .../exceptions/builder/exception_item.tsx | 11 +- .../components/exceptions/builder/helpers.tsx | 3 +- .../exceptions/builder/index.test.tsx | 6 +- .../exceptions/builder/reducer.test.ts | 441 ++++++++++++++++++ 6 files changed, 462 insertions(+), 40 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/common/components/exceptions/builder/reducer.test.ts diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.test.tsx index a207d06dca743..7b602f065fa61 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.test.tsx @@ -15,7 +15,7 @@ describe('BuilderAndBadgeComponent', () => { test('it renders exceptionItemEntryFirstRowAndBadge for very first exception item in builder', () => { const wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> - + ); @@ -24,10 +24,10 @@ describe('BuilderAndBadgeComponent', () => { ).toBeTruthy(); }); - test('it renders exceptionItemEntryInvisibleAndBadge if "andLogicIncluded" and "entriesLength" is 1 or less', () => { + test('it renders exceptionItemEntryInvisibleAndBadge if "entriesLength" is 1 or less', () => { const wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> - + ); @@ -36,33 +36,13 @@ describe('BuilderAndBadgeComponent', () => { ).toBeTruthy(); }); - test('it renders regular "and" badge if "entriesLength" is greater than 1, "andLogicIncluded" is true, and "exceptionItemIndex" is not 0', () => { + test('it renders regular "and" badge if exception item is not the first one and includes more than one entry', () => { const wrapper = mount( ({ eui: euiLightVars, darkMode: false })}> - + ); expect(wrapper.find('[data-test-subj="exceptionItemEntryAndBadge"]').exists()).toBeTruthy(); }); - - test('it does not render a badge if "andLogicIncluded" is false', () => { - const wrapper = mount( - ({ eui: euiLightVars, darkMode: false })}> - - - ); - - expect(wrapper.find('[data-test-subj="exceptionItemEntryAndBadge"]').exists()).toBeFalsy(); - expect( - wrapper.find('[data-test-subj="exceptionItemEntryInvisibleAndBadge"]').exists() - ).toBeFalsy(); - expect( - wrapper.find('[data-test-subj="exceptionItemEntryFirstRowAndBadge"]').exists() - ).toBeFalsy(); - }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.tsx index ef9c68e9d988e..3ce2f704b3643 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.tsx @@ -21,33 +21,30 @@ const MyFirstRowContainer = styled(EuiFlexItem)` interface BuilderAndBadgeProps { entriesLength: number; exceptionItemIndex: number; - andLogicIncluded: boolean; } export const BuilderAndBadgeComponent = React.memo( - ({ entriesLength, exceptionItemIndex, andLogicIncluded }) => { + ({ entriesLength, exceptionItemIndex }) => { const badge = ; - if (andLogicIncluded && entriesLength > 1 && exceptionItemIndex === 0) { + if (entriesLength > 1 && exceptionItemIndex === 0) { return ( {badge} ); - } else if (andLogicIncluded && entriesLength <= 1) { + } else if (entriesLength <= 1) { return ( {badge} ); - } else if (andLogicIncluded && entriesLength > 1) { + } else { return ( {badge} ); - } else { - return <>; } } ); 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 aa4f74febc5a2..cd8b66acd223a 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 @@ -92,11 +92,12 @@ export const BuilderExceptionListItemComponent = React.memo - + {andLogicIncluded && ( + + )} {entries.map((item, index) => ( 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 85e95c2698f02..f6b703b7e622e 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 @@ -250,8 +250,7 @@ export const getUpdatedEntriesOnDelete = ( entryIndex: number, nestedParentIndex: number | null ): ExceptionsBuilderExceptionItem => { - const itemOfInterest: BuilderEntry = - exceptionItem.entries[nestedParentIndex != null ? nestedParentIndex : entryIndex]; + const itemOfInterest: BuilderEntry = exceptionItem.entries[nestedParentIndex ?? entryIndex]; if (nestedParentIndex != null && itemOfInterest.type === OperatorTypeEnum.NESTED) { const updatedEntryEntries: Array = [ 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 458c8635bcce3..3fa0e59f9acb0 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 @@ -27,7 +27,7 @@ jest.mock('../../../../common/lib/kibana'); describe('ExceptionBuilderComponent', () => { const getValueSuggestionsMock = jest.fn().mockResolvedValue(['value 1', 'value 2']); - beforeAll(() => { + beforeEach(() => { (useKibana as jest.Mock).mockReturnValue({ services: { data: { @@ -113,6 +113,8 @@ describe('ExceptionBuilderComponent', () => { expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatchAny"]').text()).toEqual( 'some ip' ); + + wrapper.unmount(); }); test('it displays "or", "and" and "add nested button" enabled', () => { @@ -304,6 +306,8 @@ describe('ExceptionBuilderComponent', () => { expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatch"]').text()).toEqual( 'Search field value...' ); + + wrapper.unmount(); }); test('it displays "and" badge if at least one exception item includes more than one entry', () => { 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 new file mode 100644 index 0000000000000..ee5bd1329f35b --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/reducer.test.ts @@ -0,0 +1,441 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { getExceptionListItemSchemaMock } from '../../../../../../lists/common/schemas/response/exception_list_item_schema.mock'; +import { getEntryMatchMock } from '../../../../../../lists/common/schemas/types/entry_match.mock'; +import { getEntryNestedMock } from '../../../../../../lists/common/schemas/types/entry_nested.mock'; +import { getEntryListMock } from '../../../../../../lists/common/schemas/types/entry_list.mock'; + +import { ExceptionsBuilderExceptionItem } from '../types'; +import { Action, State, exceptionsBuilderReducer } from './reducer'; +import { getDefaultEmptyEntry } from './helpers'; + +const initialState: State = { + disableAnd: false, + disableNested: false, + disableOr: false, + andLogicIncluded: false, + addNested: false, + exceptions: [], + exceptionsToDelete: [], +}; + +describe('exceptionsBuilderReducer', () => { + let reducer: (state: State, action: Action) => State; + + beforeEach(() => { + reducer = exceptionsBuilderReducer(); + }); + + describe('#setExceptions', () => { + test('should return "andLogicIncluded" ', () => { + const update = reducer(initialState, { + type: 'setExceptions', + exceptions: [], + }); + + expect(update).toEqual({ + disableAnd: false, + disableNested: false, + disableOr: false, + andLogicIncluded: false, + addNested: false, + exceptions: [], + exceptionsToDelete: [], + }); + }); + + test('should set "andLogicIncluded" to true if any of the exceptions include entries with length greater than 1 ', () => { + const exceptions: ExceptionsBuilderExceptionItem[] = [ + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock(), getEntryMatchMock()], + }, + ]; + const { andLogicIncluded } = reducer(initialState, { + type: 'setExceptions', + exceptions, + }); + + expect(andLogicIncluded).toBeTruthy(); + }); + + test('should set "andLogicIncluded" to false if any of the exceptions include entries with length greater than 1 ', () => { + const exceptions: ExceptionsBuilderExceptionItem[] = [ + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock()], + }, + ]; + const { andLogicIncluded } = reducer(initialState, { + type: 'setExceptions', + exceptions, + }); + + expect(andLogicIncluded).toBeFalsy(); + }); + + test('should set "addNested" to true if last exception entry is type nested', () => { + const exceptions: ExceptionsBuilderExceptionItem[] = [ + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock()], + }, + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock(), getEntryNestedMock()], + }, + ]; + const { addNested } = reducer(initialState, { + type: 'setExceptions', + exceptions, + }); + + expect(addNested).toBeTruthy(); + }); + + test('should set "addNested" to false if last exception item entry is not type nested', () => { + const exceptions: ExceptionsBuilderExceptionItem[] = [ + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock(), getEntryNestedMock()], + }, + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock()], + }, + ]; + const { addNested } = reducer(initialState, { + type: 'setExceptions', + exceptions, + }); + + expect(addNested).toBeFalsy(); + }); + + test('should set "disableOr" to true if last exception entry is type nested', () => { + const exceptions: ExceptionsBuilderExceptionItem[] = [ + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock()], + }, + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock(), getEntryNestedMock()], + }, + ]; + const { disableOr } = reducer(initialState, { + type: 'setExceptions', + exceptions, + }); + + expect(disableOr).toBeTruthy(); + }); + + test('should set "disableOr" to false if last exception item entry is not type nested', () => { + const exceptions: ExceptionsBuilderExceptionItem[] = [ + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock(), getEntryNestedMock()], + }, + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock()], + }, + ]; + const { disableOr } = reducer(initialState, { + type: 'setExceptions', + exceptions, + }); + + expect(disableOr).toBeFalsy(); + }); + + test('should set "disableNested" to true if an exception item includes an entry of type list', () => { + const exceptions: ExceptionsBuilderExceptionItem[] = [ + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryListMock()], + }, + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock(), getEntryNestedMock()], + }, + ]; + const { disableNested } = reducer(initialState, { + type: 'setExceptions', + exceptions, + }); + + expect(disableNested).toBeTruthy(); + }); + + test('should set "disableNested" to false if an exception item does not include an entry of type list', () => { + const exceptions: ExceptionsBuilderExceptionItem[] = [ + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock(), getEntryNestedMock()], + }, + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock()], + }, + ]; + const { disableNested } = reducer(initialState, { + type: 'setExceptions', + exceptions, + }); + + expect(disableNested).toBeFalsy(); + }); + + // What does that even mean?! :) Just checking if a user has selected + // to add a nested entry but has not yet selected the nested field + test('should set "disableAnd" to true if last exception item is a nested entry with no entries itself', () => { + const exceptions: ExceptionsBuilderExceptionItem[] = [ + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryListMock()], + }, + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock(), { ...getEntryNestedMock(), entries: [] }], + }, + ]; + const { disableAnd } = reducer(initialState, { + type: 'setExceptions', + exceptions, + }); + + expect(disableAnd).toBeTruthy(); + }); + + test('should set "disableAnd" to false if last exception item is a nested entry with no entries itself', () => { + const exceptions: ExceptionsBuilderExceptionItem[] = [ + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock(), getEntryNestedMock()], + }, + { + ...getExceptionListItemSchemaMock(), + entries: [getEntryMatchMock()], + }, + ]; + const { disableAnd } = reducer(initialState, { + type: 'setExceptions', + exceptions, + }); + + expect(disableAnd).toBeFalsy(); + }); + }); + + describe('#setDefault', () => { + test('should restore initial state and add default empty entry to item" ', () => { + const update = reducer( + { + disableAnd: false, + disableNested: false, + disableOr: false, + andLogicIncluded: true, + addNested: false, + exceptions: [getExceptionListItemSchemaMock()], + exceptionsToDelete: [], + }, + { + type: 'setDefault', + initialState, + lastException: { + ...getExceptionListItemSchemaMock(), + entries: [], + }, + } + ); + + expect(update).toEqual({ + ...initialState, + exceptions: [ + { + ...getExceptionListItemSchemaMock(), + entries: [getDefaultEmptyEntry()], + }, + ], + }); + }); + }); + + describe('#setExceptionsToDelete', () => { + test('should add passed in exception item to "exceptionsToDelete"', () => { + const exceptions: ExceptionsBuilderExceptionItem[] = [ + { + ...getExceptionListItemSchemaMock(), + id: '1', + entries: [getEntryListMock()], + }, + { + ...getExceptionListItemSchemaMock(), + id: '2', + entries: [getEntryMatchMock(), { ...getEntryNestedMock(), entries: [] }], + }, + ]; + const { exceptionsToDelete } = reducer( + { + disableAnd: false, + disableNested: false, + disableOr: false, + andLogicIncluded: true, + addNested: false, + exceptions, + exceptionsToDelete: [], + }, + { + type: 'setExceptionsToDelete', + exceptions: [ + { + ...getExceptionListItemSchemaMock(), + id: '1', + entries: [getEntryListMock()], + }, + ], + } + ); + + expect(exceptionsToDelete).toEqual([ + { + ...getExceptionListItemSchemaMock(), + id: '1', + entries: [getEntryListMock()], + }, + ]); + }); + }); + + describe('#setDisableAnd', () => { + test('should set "disableAnd" to false if "action.shouldDisable" is false', () => { + const { disableAnd } = reducer( + { + disableAnd: true, + disableNested: false, + disableOr: false, + andLogicIncluded: true, + addNested: false, + exceptions: [getExceptionListItemSchemaMock()], + exceptionsToDelete: [], + }, + { + type: 'setDisableAnd', + shouldDisable: false, + } + ); + + expect(disableAnd).toBeFalsy(); + }); + + test('should set "disableAnd" to true if "action.shouldDisable" is true', () => { + const { disableAnd } = reducer( + { + disableAnd: false, + disableNested: false, + disableOr: false, + andLogicIncluded: true, + addNested: false, + exceptions: [getExceptionListItemSchemaMock()], + exceptionsToDelete: [], + }, + { + type: 'setDisableAnd', + shouldDisable: true, + } + ); + + expect(disableAnd).toBeTruthy(); + }); + }); + + describe('#setDisableOr', () => { + test('should set "disableOr" to false if "action.shouldDisable" is false', () => { + const { disableOr } = reducer( + { + disableAnd: false, + disableNested: false, + disableOr: true, + andLogicIncluded: true, + addNested: false, + exceptions: [getExceptionListItemSchemaMock()], + exceptionsToDelete: [], + }, + { + type: 'setDisableOr', + shouldDisable: false, + } + ); + + expect(disableOr).toBeFalsy(); + }); + + test('should set "disableOr" to true if "action.shouldDisable" is true', () => { + const { disableOr } = reducer( + { + disableAnd: false, + disableNested: false, + disableOr: false, + andLogicIncluded: true, + addNested: false, + exceptions: [getExceptionListItemSchemaMock()], + exceptionsToDelete: [], + }, + { + type: 'setDisableOr', + shouldDisable: true, + } + ); + + expect(disableOr).toBeTruthy(); + }); + }); + + describe('#setAddNested', () => { + test('should set "addNested" to false if "action.addNested" is false', () => { + const { addNested } = reducer( + { + disableAnd: false, + disableNested: true, + disableOr: false, + andLogicIncluded: true, + addNested: true, + exceptions: [getExceptionListItemSchemaMock()], + exceptionsToDelete: [], + }, + { + type: 'setAddNested', + addNested: false, + } + ); + + expect(addNested).toBeFalsy(); + }); + + test('should set "disableOr" to true if "action.addNested" is true', () => { + const { addNested } = reducer( + { + disableAnd: false, + disableNested: false, + disableOr: false, + andLogicIncluded: true, + addNested: false, + exceptions: [getExceptionListItemSchemaMock()], + exceptionsToDelete: [], + }, + { + type: 'setAddNested', + addNested: true, + } + ); + + expect(addNested).toBeTruthy(); + }); + }); +});