From b5c29f5c0c9dc60cc8cfff13a048c757bc1e9c1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Kopyci=C5=84ski?= Date: Sat, 1 Aug 2020 01:17:16 +0200 Subject: [PATCH] [Security Solution] Fix timeline pin event callback (#73981) (#74017) * [Security Solution] Fix timeline pin event callback * - added tests * - restored the original disabled button behavior Co-authored-by: Andrew Goldstein Co-authored-by: Andrew Goldstein --- .../components/timeline/body/helpers.test.ts | 69 +++++++++++++++++++ .../components/timeline/body/helpers.ts | 14 ++-- .../components/timeline/pin/index.test.tsx | 69 ++++++++++++++++++- .../components/timeline/pin/index.tsx | 2 +- 4 files changed, 148 insertions(+), 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 index c8adaa891610a..f4dc691f3d059 100644 --- 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 index 6a5e25632c29b..73b5a58ef7b65 100644 --- 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 index 657976e2f4787..2ca27ded86c9d 100644 --- 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( + + ); + + 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( + + ); + + 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( + + ); + + 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( + + ); + + 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 index 30fe8ae0ca1f6..27780c7754d00 100644 --- 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( iconSize={iconSize} iconType={getPinIcon(pinned)} onClick={onClick} - isDisabled={isTemplate} + isDisabled={isTemplate || !allowUnpinning} /> ); }