applied PR feedback

elastic · Aug 4, 2020 · 13edaa6 · 13edaa6
1 parent 3fda1e2
commit 13edaa6
Show file tree

Hide file tree

Showing 6 changed files with 462 additions and 40 deletions.
diff --git a/.../plugins/security_solution/public/common/components/exceptions/builder/and_badge.test.tsx b/.../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(
       <ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
-        <BuilderAndBadgeComponent entriesLength={2} exceptionItemIndex={0} andLogicIncluded />
+        <BuilderAndBadgeComponent entriesLength={2} exceptionItemIndex={0} />
       </ThemeProvider>
     );
 
@@ -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(
       <ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
-        <BuilderAndBadgeComponent entriesLength={1} exceptionItemIndex={0} andLogicIncluded />
+        <BuilderAndBadgeComponent entriesLength={1} exceptionItemIndex={0} />
       </ThemeProvider>
     );
 
@@ -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(
       <ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
-        <BuilderAndBadgeComponent entriesLength={2} exceptionItemIndex={1} andLogicIncluded />
+        <BuilderAndBadgeComponent entriesLength={2} exceptionItemIndex={1} />
       </ThemeProvider>
     );
 
     expect(wrapper.find('[data-test-subj="exceptionItemEntryAndBadge"]').exists()).toBeTruthy();
   });
-
-  test('it does not render a badge if "andLogicIncluded" is false', () => {
-    const wrapper = mount(
-      <ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
-        <BuilderAndBadgeComponent
-          entriesLength={2}
-          exceptionItemIndex={1}
-          andLogicIncluded={false}
-        />
-      </ThemeProvider>
-    );
-
-    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
@@ -21,33 +21,30 @@ const MyFirstRowContainer = styled(EuiFlexItem)`
 interface BuilderAndBadgeProps {
   entriesLength: number;
   exceptionItemIndex: number;
-  andLogicIncluded: boolean;
 }
 
 export const BuilderAndBadgeComponent = React.memo<BuilderAndBadgeProps>(
-  ({ entriesLength, exceptionItemIndex, andLogicIncluded }) => {
+  ({ entriesLength, exceptionItemIndex }) => {
     const badge = <AndOrBadge includeAntennas type="and" />;
 
-    if (andLogicIncluded && entriesLength > 1 && exceptionItemIndex === 0) {
+    if (entriesLength > 1 && exceptionItemIndex === 0) {
       return (
         <MyFirstRowContainer grow={false} data-test-subj="exceptionItemEntryFirstRowAndBadge">
           {badge}
         </MyFirstRowContainer>
       );
-    } else if (andLogicIncluded && entriesLength <= 1) {
+    } else if (entriesLength <= 1) {
       return (
         <MyInvisibleAndBadge grow={false} data-test-subj="exceptionItemEntryInvisibleAndBadge">
           {badge}
         </MyInvisibleAndBadge>
       );
-    } else if (andLogicIncluded && entriesLength > 1) {
+    } else {
       return (
         <EuiFlexItem grow={false} data-test-subj="exceptionItemEntryAndBadge">
           {badge}
         </EuiFlexItem>
       );
-    } else {
-      return <></>;
     }
   }
 );

diff --git a/.../plugins/security_solution/public/common/components/exceptions/builder/exception_item.tsx b/.../plugins/security_solution/public/common/components/exceptions/builder/exception_item.tsx
@@ -92,11 +92,12 @@ export const BuilderExceptionListItemComponent = React.memo<BuilderExceptionList
     return (
       <EuiFlexItem>
         <EuiFlexGroup gutterSize="s" data-test-subj="exceptionEntriesContainer">
-          <BuilderAndBadgeComponent
-            entriesLength={exceptionItem.entries.length}
-            andLogicIncluded={andLogicIncluded}
-            exceptionItemIndex={exceptionItemIndex}
-          />
+          {andLogicIncluded && (
+            <BuilderAndBadgeComponent
+              entriesLength={exceptionItem.entries.length}
+              exceptionItemIndex={exceptionItemIndex}
+            />
+          )}
           <EuiFlexItem grow={6}>
             <EuiFlexGroup gutterSize="s" direction="column">
               {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
@@ -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<EmptyEntry | EntryMatch | EntryMatchAny | EntryExists> = [

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
@@ -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', () => {