Skip to content

Commit

Permalink
[Security Solution][Exceptions] - Fixes exceptions builder nested del…
Browse files Browse the repository at this point in the history
…etion issue and adds unit tests (#74250) (#74321)

### Summary

- Updates logic for deleting exception item entries in the builder - found that there was a bug in deleting nested entries
- Adds more unit tests

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
yctercero and elasticmachine authored Aug 5, 2020
1 parent 4b99054 commit 4502447
Show file tree
Hide file tree
Showing 18 changed files with 1,260 additions and 172 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -317,7 +317,7 @@ export const AddExceptionModal = memo(function AddExceptionModal({
<ModalBodySection className="builder-section">
<EuiText>{i18n.EXCEPTION_BUILDER_INFO}</EuiText>
<EuiSpacer />
<ExceptionBuilder
<ExceptionBuilderComponent
exceptionListItems={initialExceptionItems}
listType={exceptionListType}
listId={ruleExceptionList.list_id}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* 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 { BuilderAndBadgeComponent } from './and_badge';

describe('BuilderAndBadgeComponent', () => {
test('it renders exceptionItemEntryFirstRowAndBadge for very first exception item in builder', () => {
const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<BuilderAndBadgeComponent entriesLength={2} exceptionItemIndex={0} />
</ThemeProvider>
);

expect(
wrapper.find('[data-test-subj="exceptionItemEntryFirstRowAndBadge"]').exists()
).toBeTruthy();
});

test('it renders exceptionItemEntryInvisibleAndBadge if "entriesLength" is 1 or less', () => {
const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<BuilderAndBadgeComponent entriesLength={1} exceptionItemIndex={0} />
</ThemeProvider>
);

expect(
wrapper.find('[data-test-subj="exceptionItemEntryInvisibleAndBadge"]').exists()
).toBeTruthy();
});

test('it renders regular "and" badge if exception item is not the first one and includes more than one entry', () => {
const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<BuilderAndBadgeComponent entriesLength={2} exceptionItemIndex={1} />
</ThemeProvider>
);

expect(wrapper.find('[data-test-subj="exceptionItemEntryAndBadge"]').exists()).toBeTruthy();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* 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;
}

export const BuilderAndBadgeComponent = React.memo<BuilderAndBadgeProps>(
({ entriesLength, exceptionItemIndex }) => {
const badge = <AndOrBadge includeAntennas type="and" />;

if (entriesLength > 1 && exceptionItemIndex === 0) {
return (
<MyFirstRowContainer grow={false} data-test-subj="exceptionItemEntryFirstRowAndBadge">
{badge}
</MyFirstRowContainer>
);
} else if (entriesLength <= 1) {
return (
<MyInvisibleAndBadge grow={false} data-test-subj="exceptionItemEntryInvisibleAndBadge">
{badge}
</MyInvisibleAndBadge>
);
} else {
return (
<EuiFlexItem grow={false} data-test-subj="exceptionItemEntryAndBadge">
{badge}
</EuiFlexItem>
);
}
}
);

BuilderAndBadgeComponent.displayName = 'BuilderAndBadge';
Original file line number Diff line number Diff line change
@@ -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(
<BuilderEntryDeleteButtonComponent
entryIndex={0}
exceptionItemIndex={0}
nestedParentIndex={null}
isOnlyItem={false}
entries={getExceptionListItemSchemaMock().entries}
onDelete={jest.fn()}
/>
);

expect(wrapper.find('[data-test-subj="firstRowBuilderDeleteButton"] button')).toHaveLength(1);
});

test('it does not render firstRowBuilderDeleteButton if entryIndex is not 0', () => {
const wrapper = mount(
<BuilderEntryDeleteButtonComponent
entryIndex={1}
exceptionItemIndex={0}
nestedParentIndex={null}
isOnlyItem={false}
entries={getExceptionListItemSchemaMock().entries}
onDelete={jest.fn()}
/>
);

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(
<BuilderEntryDeleteButtonComponent
entryIndex={0}
exceptionItemIndex={1}
nestedParentIndex={null}
isOnlyItem={false}
entries={getExceptionListItemSchemaMock().entries}
onDelete={jest.fn()}
/>
);

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(
<BuilderEntryDeleteButtonComponent
entryIndex={0}
exceptionItemIndex={0}
nestedParentIndex={0}
isOnlyItem={false}
entries={getExceptionListItemSchemaMock().entries}
onDelete={jest.fn()}
/>
);

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(
<BuilderEntryDeleteButtonComponent
entryIndex={0}
exceptionItemIndex={1}
nestedParentIndex={null}
isOnlyItem={false}
entries={getExceptionListItemSchemaMock().entries}
onDelete={onDelete}
/>
);

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(
<BuilderEntryDeleteButtonComponent
entryIndex={0}
exceptionItemIndex={0}
nestedParentIndex={0}
isOnlyItem
entries={exceptionItem.entries}
onDelete={jest.fn()}
/>
);

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(
<BuilderEntryDeleteButtonComponent
entryIndex={1}
exceptionItemIndex={0}
nestedParentIndex={null}
isOnlyItem
entries={getExceptionListItemSchemaMock().entries}
onDelete={jest.fn()}
/>
);

const button = wrapper.find('[data-test-subj="builderDeleteButton"] button').at(0);

expect(button.prop('disabled')).toBeFalsy();
});
});
Original file line number Diff line number Diff line change
@@ -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<BuilderEntryDeleteButtonProps>(
({ 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 = (
<EuiButtonIcon
color="danger"
iconType="trash"
onClick={handleDelete}
isDisabled={isDisabled}
aria-label="entryDeleteButton"
className="exceptionItemEntryDeleteButton"
data-test-subj="builderItemEntryDeleteButton"
/>
);

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 (
<MyFirstRowContainer grow={false} data-test-subj="firstRowBuilderDeleteButton">
{button}
</MyFirstRowContainer>
);
} else {
return (
<EuiFlexItem grow={false} data-test-subj="builderDeleteButton">
{button}
</EuiFlexItem>
);
}
}
);

BuilderEntryDeleteButtonComponent.displayName = 'BuilderEntryDeleteButton';
Loading

0 comments on commit 4502447

Please sign in to comment.