[Security Solution] Fix timeline pin event callback (#73981)

* [Security Solution] Fix timeline pin event callback * - added tests * - restored the original disabled button behavior Co-authored-by: Andrew Goldstein <andrew.goldstein@elastic.co>
elastic · Jul 31, 2020 · 5aca964 · 5aca964
1 parent 2c71a3f
commit 5aca964
Show file tree

Hide file tree

Showing 4 changed files with 148 additions and 6 deletions.
diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/helpers.test.ts b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/helpers.test.ts
@@ -9,6 +9,7 @@ import { Ecs } from '../../../../graphql/types';
 import {
   eventHasNotes,
   eventIsPinned,
+  getPinOnClick,
   getPinTooltip,
   stringifyEvent,
   isInvestigateInResolverActionEnabled,
@@ -298,4 +299,72 @@ describe('helpers', () => {
       expect(isInvestigateInResolverActionEnabled(data)).toBeFalsy();
     });
   });
+
+  describe('getPinOnClick', () => {
+    const eventId = 'abcd';
+
+    test('it invokes `onPinEvent` with the expected eventId when the event is NOT pinned, and allowUnpinning is true', () => {
+      const isEventPinned = false; // the event is NOT pinned
+      const allowUnpinning = true;
+      const onPinEvent = jest.fn();
+
+      getPinOnClick({
+        allowUnpinning,
+        eventId,
+        onPinEvent,
+        onUnPinEvent: jest.fn(),
+        isEventPinned,
+      });
+
+      expect(onPinEvent).toBeCalledWith(eventId);
+    });
+
+    test('it does NOT invoke `onPinEvent` when the event is NOT pinned, and allowUnpinning is false', () => {
+      const isEventPinned = false; // the event is NOT pinned
+      const allowUnpinning = false;
+      const onPinEvent = jest.fn();
+
+      getPinOnClick({
+        allowUnpinning,
+        eventId,
+        onPinEvent,
+        onUnPinEvent: jest.fn(),
+        isEventPinned,
+      });
+
+      expect(onPinEvent).not.toBeCalled();
+    });
+
+    test('it invokes `onUnPinEvent` with the expected eventId when the event is pinned, and allowUnpinning is true', () => {
+      const isEventPinned = true; // the event is pinned
+      const allowUnpinning = true;
+      const onUnPinEvent = jest.fn();
+
+      getPinOnClick({
+        allowUnpinning,
+        eventId,
+        onPinEvent: jest.fn(),
+        onUnPinEvent,
+        isEventPinned,
+      });
+
+      expect(onUnPinEvent).toBeCalledWith(eventId);
+    });
+
+    test('it does NOT invoke `onUnPinEvent` when the event is pinned, and allowUnpinning is false', () => {
+      const isEventPinned = true; // the event is pinned
+      const allowUnpinning = false;
+      const onUnPinEvent = jest.fn();
+
+      getPinOnClick({
+        allowUnpinning,
+        eventId,
+        onPinEvent: jest.fn(),
+        onUnPinEvent,
+        isEventPinned,
+      });
+
+      expect(onUnPinEvent).not.toBeCalled();
+    });
+  });
 });
diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/helpers.ts b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/helpers.ts
@@ -3,7 +3,8 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-import { get, isEmpty, noop } from 'lodash/fp';
+
+import { get, isEmpty } from 'lodash/fp';
 import { Dispatch } from 'redux';
 
 import { Ecs, TimelineItem, TimelineNonEcsData } from '../../../../graphql/types';
@@ -65,11 +66,16 @@ export const getPinOnClick = ({
   onPinEvent,
   onUnPinEvent,
   isEventPinned,
-}: GetPinOnClickParams): (() => void) => {
+}: GetPinOnClickParams) => {
   if (!allowUnpinning) {
-    return noop;
+    return;
+  }
+
+  if (isEventPinned) {
+    onUnPinEvent(eventId);
+  } else {
+    onPinEvent(eventId);
   }
-  return isEventPinned ? () => onUnPinEvent(eventId) : () => onPinEvent(eventId);
 };
 
 /**

diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/pin/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/pin/index.test.tsx
@@ -4,7 +4,16 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { getPinIcon } from './';
+import { mount } from 'enzyme';
+import React from 'react';
+
+import { TimelineType } from '../../../../../common/types/timeline';
+
+import { getPinIcon, Pin } from './';
+
+interface ButtonIcon {
+  isDisabled: boolean;
+}
 
 describe('pin', () => {
   describe('getPinRotation', () => {
@@ -16,4 +25,62 @@ describe('pin', () => {
       expect(getPinIcon(false)).toEqual('pin');
     });
   });
+
+  describe('disabled button behavior', () => {
+    test('the button is enabled when allowUnpinning is true, and timelineType is NOT `template` (the default)', () => {
+      const allowUnpinning = true;
+      const wrapper = mount(
+        <Pin allowUnpinning={allowUnpinning} onClick={jest.fn()} pinned={false} />
+      );
+
+      expect(
+        (wrapper.find('[data-test-subj="pin"]').first().props() as ButtonIcon).isDisabled
+      ).toBe(false);
+    });
+
+    test('the button is disabled when allowUnpinning is false, and timelineType is NOT `template` (the default)', () => {
+      const allowUnpinning = false;
+      const wrapper = mount(
+        <Pin allowUnpinning={allowUnpinning} onClick={jest.fn()} pinned={false} />
+      );
+
+      expect(
+        (wrapper.find('[data-test-subj="pin"]').first().props() as ButtonIcon).isDisabled
+      ).toBe(true);
+    });
+
+    test('the button is disabled when allowUnpinning is true, and timelineType is `template`', () => {
+      const allowUnpinning = true;
+      const timelineType = TimelineType.template;
+      const wrapper = mount(
+        <Pin
+          allowUnpinning={allowUnpinning}
+          onClick={jest.fn()}
+          pinned={false}
+          timelineType={timelineType}
+        />
+      );
+
+      expect(
+        (wrapper.find('[data-test-subj="pin"]').first().props() as ButtonIcon).isDisabled
+      ).toBe(true);
+    });
+
+    test('the button is disabled when allowUnpinning is false, and timelineType is `template`', () => {
+      const allowUnpinning = false;
+      const timelineType = TimelineType.template;
+      const wrapper = mount(
+        <Pin
+          allowUnpinning={allowUnpinning}
+          onClick={jest.fn()}
+          pinned={false}
+          timelineType={timelineType}
+        />
+      );
+
+      expect(
+        (wrapper.find('[data-test-subj="pin"]').first().props() as ButtonIcon).isDisabled
+      ).toBe(true);
+    });
+  });
 });
diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/pin/index.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/pin/index.tsx
@@ -34,7 +34,7 @@ export const Pin = React.memo<Props>(
         iconSize={iconSize}
         iconType={getPinIcon(pinned)}
         onClick={onClick}
-        isDisabled={isTemplate}
+        isDisabled={isTemplate || !allowUnpinning}
       />
     );
   }