Skip to content

Commit 6a77bec

Browse files
ycterceroyctercero
authored
[Security Solution][Exceptions] - Fixes exceptions builder nested deletion issue and adds unit tests (#74250)
### 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
1 parent 36f25e6 commit 6a77bec

File tree

18 files changed

+1260
-172
lines changed

18 files changed

+1260
-172
lines changed

‎x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import * as i18n from './translations';
3131
import { TimelineNonEcsData, Ecs } from '../../../../graphql/types';
3232
import { useAppToasts } from '../../../hooks/use_app_toasts';
3333
import { useKibana } from '../../../lib/kibana';
34-
import { ExceptionBuilder } from '../builder';
34+
import { ExceptionBuilderComponent } from '../builder';
3535
import { Loader } from '../../loader';
3636
import { useAddOrUpdateException } from '../use_add_exception';
3737
import { useSignalIndex } from '../../../../detections/containers/detection_engine/alerts/use_signal_index';
@@ -317,7 +317,7 @@ export const AddExceptionModal = memo(function AddExceptionModal({
317317
<ModalBodySection className="builder-section">
318318
<EuiText>{i18n.EXCEPTION_BUILDER_INFO}</EuiText>
319319
<EuiSpacer />
320-
<ExceptionBuilder
320+
<ExceptionBuilderComponent
321321
exceptionListItems={initialExceptionItems}
322322
listType={exceptionListType}
323323
listId={ruleExceptionList.list_id}

‎x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.test.tsx‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import React from 'react';
8+
import { ThemeProvider } from 'styled-components';
9+
import { mount } from 'enzyme';
10+
import euiLightVars from '@elastic/eui/dist/eui_theme_light.json';
11+
12+
import { BuilderAndBadgeComponent } from './and_badge';
13+
14+
describe('BuilderAndBadgeComponent', () => {
15+
test('it renders exceptionItemEntryFirstRowAndBadge for very first exception item in builder', () => {
16+
const wrapper = mount(
17+
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
18+
<BuilderAndBadgeComponent entriesLength={2} exceptionItemIndex={0} />
19+
</ThemeProvider>
20+
);
21+
22+
expect(
23+
wrapper.find('[data-test-subj="exceptionItemEntryFirstRowAndBadge"]').exists()
24+
).toBeTruthy();
25+
});
26+
27+
test('it renders exceptionItemEntryInvisibleAndBadge if "entriesLength" is 1 or less', () => {
28+
const wrapper = mount(
29+
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
30+
<BuilderAndBadgeComponent entriesLength={1} exceptionItemIndex={0} />
31+
</ThemeProvider>
32+
);
33+
34+
expect(
35+
wrapper.find('[data-test-subj="exceptionItemEntryInvisibleAndBadge"]').exists()
36+
).toBeTruthy();
37+
});
38+
39+
test('it renders regular "and" badge if exception item is not the first one and includes more than one entry', () => {
40+
const wrapper = mount(
41+
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
42+
<BuilderAndBadgeComponent entriesLength={2} exceptionItemIndex={1} />
43+
</ThemeProvider>
44+
);
45+
46+
expect(wrapper.find('[data-test-subj="exceptionItemEntryAndBadge"]').exists()).toBeTruthy();
47+
});
48+
});

‎x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.tsx‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import React from 'react';
8+
import { EuiFlexItem } from '@elastic/eui';
9+
import styled from 'styled-components';
10+
11+
import { AndOrBadge } from '../../and_or_badge';
12+
13+
const MyInvisibleAndBadge = styled(EuiFlexItem)`
14+
visibility: hidden;
15+
`;
16+
17+
const MyFirstRowContainer = styled(EuiFlexItem)`
18+
padding-top: 20px;
19+
`;
20+
21+
interface BuilderAndBadgeProps {
22+
entriesLength: number;
23+
exceptionItemIndex: number;
24+
}
25+
26+
export const BuilderAndBadgeComponent = React.memo<BuilderAndBadgeProps>(
27+
({ entriesLength, exceptionItemIndex }) => {
28+
const badge = <AndOrBadge includeAntennas type="and" />;
29+
30+
if (entriesLength > 1 && exceptionItemIndex === 0) {
31+
return (
32+
<MyFirstRowContainer grow={false} data-test-subj="exceptionItemEntryFirstRowAndBadge">
33+
{badge}
34+
</MyFirstRowContainer>
35+
);
36+
} else if (entriesLength <= 1) {
37+
return (
38+
<MyInvisibleAndBadge grow={false} data-test-subj="exceptionItemEntryInvisibleAndBadge">
39+
{badge}
40+
</MyInvisibleAndBadge>
41+
);
42+
} else {
43+
return (
44+
<EuiFlexItem grow={false} data-test-subj="exceptionItemEntryAndBadge">
45+
{badge}
46+
</EuiFlexItem>
47+
);
48+
}
49+
}
50+
);
51+
52+
BuilderAndBadgeComponent.displayName = 'BuilderAndBadge';

‎x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_delete_button.test.tsx‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { mount } from 'enzyme';
8+
import React from 'react';
9+
10+
import { getExceptionListItemSchemaMock } from '../../../../../../lists/common/schemas/response/exception_list_item_schema.mock';
11+
import { getEntryMatchMock } from '../../../../../../lists/common/schemas/types/entry_match.mock';
12+
13+
import { BuilderEntryDeleteButtonComponent } from './entry_delete_button';
14+
15+
describe('BuilderEntryDeleteButtonComponent', () => {
16+
test('it renders firstRowBuilderDeleteButton for very first entry in builder', () => {
17+
const wrapper = mount(
18+
<BuilderEntryDeleteButtonComponent
19+
entryIndex={0}
20+
exceptionItemIndex={0}
21+
nestedParentIndex={null}
22+
isOnlyItem={false}
23+
entries={getExceptionListItemSchemaMock().entries}
24+
onDelete={jest.fn()}
25+
/>
26+
);
27+
28+
expect(wrapper.find('[data-test-subj="firstRowBuilderDeleteButton"] button')).toHaveLength(1);
29+
});
30+
31+
test('it does not render firstRowBuilderDeleteButton if entryIndex is not 0', () => {
32+
const wrapper = mount(
33+
<BuilderEntryDeleteButtonComponent
34+
entryIndex={1}
35+
exceptionItemIndex={0}
36+
nestedParentIndex={null}
37+
isOnlyItem={false}
38+
entries={getExceptionListItemSchemaMock().entries}
39+
onDelete={jest.fn()}
40+
/>
41+
);
42+
43+
expect(wrapper.find('[data-test-subj="firstRowBuilderDeleteButton"]')).toHaveLength(0);
44+
expect(wrapper.find('[data-test-subj="builderDeleteButton"] button')).toHaveLength(1);
45+
});
46+
47+
test('it does not render firstRowBuilderDeleteButton if exceptionItemIndex is not 0', () => {
48+
const wrapper = mount(
49+
<BuilderEntryDeleteButtonComponent
50+
entryIndex={0}
51+
exceptionItemIndex={1}
52+
nestedParentIndex={null}
53+
isOnlyItem={false}
54+
entries={getExceptionListItemSchemaMock().entries}
55+
onDelete={jest.fn()}
56+
/>
57+
);
58+
59+
expect(wrapper.find('[data-test-subj="firstRowBuilderDeleteButton"]')).toHaveLength(0);
60+
expect(wrapper.find('[data-test-subj="builderDeleteButton"] button')).toHaveLength(1);
61+
});
62+
63+
test('it does not render firstRowBuilderDeleteButton if nestedParentIndex is not null', () => {
64+
const wrapper = mount(
65+
<BuilderEntryDeleteButtonComponent
66+
entryIndex={0}
67+
exceptionItemIndex={0}
68+
nestedParentIndex={0}
69+
isOnlyItem={false}
70+
entries={getExceptionListItemSchemaMock().entries}
71+
onDelete={jest.fn()}
72+
/>
73+
);
74+
75+
expect(wrapper.find('[data-test-subj="firstRowBuilderDeleteButton"]')).toHaveLength(0);
76+
expect(wrapper.find('[data-test-subj="builderDeleteButton"] button')).toHaveLength(1);
77+
});
78+
79+
test('it invokes "onDelete" when button is clicked', () => {
80+
const onDelete = jest.fn();
81+
82+
const wrapper = mount(
83+
<BuilderEntryDeleteButtonComponent
84+
entryIndex={0}
85+
exceptionItemIndex={1}
86+
nestedParentIndex={null}
87+
isOnlyItem={false}
88+
entries={getExceptionListItemSchemaMock().entries}
89+
onDelete={onDelete}
90+
/>
91+
);
92+
93+
wrapper.find('[data-test-subj="builderDeleteButton"] button').simulate('click');
94+
95+
expect(onDelete).toHaveBeenCalledTimes(1);
96+
expect(onDelete).toHaveBeenCalledWith(0, null);
97+
});
98+
99+
test('it disables button if it is the only entry left and no field has been selected', () => {
100+
const exceptionItem = {
101+
...getExceptionListItemSchemaMock(),
102+
entries: [{ ...getEntryMatchMock(), field: '' }],
103+
};
104+
const wrapper = mount(
105+
<BuilderEntryDeleteButtonComponent
106+
entryIndex={0}
107+
exceptionItemIndex={0}
108+
nestedParentIndex={0}
109+
isOnlyItem
110+
entries={exceptionItem.entries}
111+
onDelete={jest.fn()}
112+
/>
113+
);
114+
115+
const button = wrapper.find('[data-test-subj="builderDeleteButton"] button').at(0);
116+
117+
expect(button.prop('disabled')).toBeTruthy();
118+
});
119+
120+
test('it does not disable button if it is the only entry left and field has been selected', () => {
121+
const wrapper = mount(
122+
<BuilderEntryDeleteButtonComponent
123+
entryIndex={1}
124+
exceptionItemIndex={0}
125+
nestedParentIndex={null}
126+
isOnlyItem
127+
entries={getExceptionListItemSchemaMock().entries}
128+
onDelete={jest.fn()}
129+
/>
130+
);
131+
132+
const button = wrapper.find('[data-test-subj="builderDeleteButton"] button').at(0);
133+
134+
expect(button.prop('disabled')).toBeFalsy();
135+
});
136+
});

‎x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_delete_button.tsx‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import React, { useCallback } from 'react';
8+
import { EuiButtonIcon, EuiFlexItem } from '@elastic/eui';
9+
import styled from 'styled-components';
10+
11+
import { BuilderEntry } from '../types';
12+
13+
const MyFirstRowContainer = styled(EuiFlexItem)`
14+
padding-top: 20px;
15+
`;
16+
17+
interface BuilderEntryDeleteButtonProps {
18+
entries: BuilderEntry[];
19+
isOnlyItem: boolean;
20+
entryIndex: number;
21+
exceptionItemIndex: number;
22+
nestedParentIndex: number | null;
23+
onDelete: (item: number, parent: number | null) => void;
24+
}
25+
26+
export const BuilderEntryDeleteButtonComponent = React.memo<BuilderEntryDeleteButtonProps>(
27+
({ entries, nestedParentIndex, isOnlyItem, entryIndex, exceptionItemIndex, onDelete }) => {
28+
const isDisabled: boolean =
29+
isOnlyItem &&
30+
entries.length === 1 &&
31+
exceptionItemIndex === 0 &&
32+
(entries[0].field == null || entries[0].field === '');
33+
34+
const handleDelete = useCallback((): void => {
35+
onDelete(entryIndex, nestedParentIndex);
36+
}, [onDelete, entryIndex, nestedParentIndex]);
37+
38+
const button = (
39+
<EuiButtonIcon
40+
color="danger"
41+
iconType="trash"
42+
onClick={handleDelete}
43+
isDisabled={isDisabled}
44+
aria-label="entryDeleteButton"
45+
className="exceptionItemEntryDeleteButton"
46+
data-test-subj="builderItemEntryDeleteButton"
47+
/>
48+
);
49+
50+
if (entryIndex === 0 && exceptionItemIndex === 0 && nestedParentIndex == null) {
51+
// This logic was added to work around it including the field
52+
// labels in centering the delete icon for the first row
53+
return (
54+
<MyFirstRowContainer grow={false} data-test-subj="firstRowBuilderDeleteButton">
55+
{button}
56+
</MyFirstRowContainer>
57+
);
58+
} else {
59+
return (
60+
<EuiFlexItem grow={false} data-test-subj="builderDeleteButton">
61+
{button}
62+
</EuiFlexItem>
63+
);
64+
}
65+
}
66+
);
67+
68+
BuilderEntryDeleteButtonComponent.displayName = 'BuilderEntryDeleteButton';

0 commit comments

Comments
 (0)