From 05947ade7680b868d540f6c3458b2833ef1450f7 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Tue, 18 Feb 2020 17:05:32 -0700 Subject: [PATCH] [SIEM] [Detection Engine] Fixes Signals Table bulk selection issues (#56825) ## Summary This PR fixes regressions around the bulk selection action, including: * Incorrect total when opening/closing signals * Selection total persisting after opening/closing signals * `Select All` checkbox remaining selected after opening/closing signals * Bulk action not being enabled after opening/closing via single action while others are selected ##### Incorrect total / persisted selection total
Before

![selection_persisted_wrong_count](https://user-images.githubusercontent.com/2946766/73814700-0d7faf80-47a1-11ea-9ec1-c6cb6a3d30c3.gif)

After

![selection_persisted_wrong_count_fix](https://user-images.githubusercontent.com/2946766/73814704-107aa000-47a1-11ea-9eb9-4f56d3d3f8c2.gif)

##### Bulk action not being enabled
Before

![selection_disabled_bulk_action](https://user-images.githubusercontent.com/2946766/73814695-09ec2880-47a1-11ea-8a37-ae35a19979ab.gif)

After

![selection_disabled_bulk_action_fixed](https://user-images.githubusercontent.com/2946766/73814701-0f497300-47a1-11ea-8c73-3a131bda59f6.gif)

### Checklist Delete any items that are not applicable to this PR. - [ ] ~Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)~ - [ ] ~[Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~ - [ ] ~[Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios~ - [ ] ~This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~ - [ ] ~This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)~ - [ ] ~This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)~ ### For maintainers - [ ] ~This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~ --- .../events_viewer/events_viewer.tsx | 1 - .../siem/public/containers/query_template.tsx | 30 +++++++++++++++++-- .../siem/public/containers/timeline/index.tsx | 29 +++++++++++++++--- .../components/signals/index.tsx | 2 +- .../signals/signals_utility_bar/index.tsx | 1 + .../siem/public/store/timeline/helpers.ts | 11 +++++++ 6 files changed, 65 insertions(+), 9 deletions(-) diff --git a/x-pack/legacy/plugins/siem/public/components/events_viewer/events_viewer.tsx b/x-pack/legacy/plugins/siem/public/components/events_viewer/events_viewer.tsx index b05529f9a497f9..8e2c5bdd12d683 100644 --- a/x-pack/legacy/plugins/siem/public/components/events_viewer/events_viewer.tsx +++ b/x-pack/legacy/plugins/siem/public/components/events_viewer/events_viewer.tsx @@ -157,7 +157,6 @@ const EventsViewerComponent: React.FC = ({ totalCountMinusDeleted ) ?? i18n.UNIT(totalCountMinusDeleted)}`; - // TODO: Reset eventDeletedIds/eventLoadingIds on refresh/loadmore (getUpdatedAt) return ( <> = FetchMoreQueryOptions & +export type FetchMoreOptionsArgs = FetchMoreQueryOptions & FetchMoreOptions; // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -40,6 +40,19 @@ export class QueryTemplate< tiebreaker?: string ) => FetchMoreOptionsArgs; + private refetch!: (variables?: TVariables) => Promise>; + + private executeBeforeFetchMore!: ({ id }: { id?: string }) => void; + + private executeBeforeRefetch!: ({ id }: { id?: string }) => void; + + public setExecuteBeforeFetchMore = (val: ({ id }: { id?: string }) => void) => { + this.executeBeforeFetchMore = val; + }; + public setExecuteBeforeRefetch = (val: ({ id }: { id?: string }) => void) => { + this.executeBeforeRefetch = val; + }; + public setFetchMore = ( val: (fetchMoreOptions: FetchMoreOptionsArgs) => PromiseApolloQueryResult ) => { @@ -52,6 +65,17 @@ export class QueryTemplate< this.fetchMoreOptions = val; }; - public wrappedLoadMore = (newCursor: string, tiebreaker?: string) => - this.fetchMore(this.fetchMoreOptions(newCursor, tiebreaker)); + public setRefetch = (val: (variables?: TVariables) => Promise>) => { + this.refetch = val; + }; + + public wrappedLoadMore = (newCursor: string, tiebreaker?: string) => { + this.executeBeforeFetchMore({ id: this.props.id }); + return this.fetchMore(this.fetchMoreOptions(newCursor, tiebreaker)); + }; + + public wrappedRefetch = (variables?: TVariables) => { + this.executeBeforeRefetch({ id: this.props.id }); + return this.refetch(variables); + }; } diff --git a/x-pack/legacy/plugins/siem/public/containers/timeline/index.tsx b/x-pack/legacy/plugins/siem/public/containers/timeline/index.tsx index 68d87ef565fb73..ccd8babd41e68a 100644 --- a/x-pack/legacy/plugins/siem/public/containers/timeline/index.tsx +++ b/x-pack/legacy/plugins/siem/public/containers/timeline/index.tsx @@ -8,7 +8,7 @@ import { getOr } from 'lodash/fp'; import memoizeOne from 'memoize-one'; import React from 'react'; import { Query } from 'react-apollo'; -import { compose } from 'redux'; +import { compose, Dispatch } from 'redux'; import { connect, ConnectedProps } from 'react-redux'; import { IIndexPattern } from '../../../../../../../src/plugins/data/common/index_patterns'; @@ -26,6 +26,8 @@ import { createFilter } from '../helpers'; import { QueryTemplate, QueryTemplateProps } from '../query_template'; import { EventType } from '../../store/timeline/model'; import { timelineQuery } from './index.gql_query'; +import { timelineActions } from '../../store/timeline'; +import { SIGNALS_PAGE_TIMELINE_ID } from '../../pages/detection_engine/components/signals'; export interface TimelineArgs { events: TimelineItem[]; @@ -39,6 +41,10 @@ export interface TimelineArgs { getUpdatedAt: () => number; } +export interface CustomReduxProps { + clearSignalsState: ({ id }: { id?: string }) => void; +} + export interface OwnProps extends QueryTemplateProps { children?: (args: TimelineArgs) => React.ReactNode; eventType?: EventType; @@ -50,7 +56,7 @@ export interface OwnProps extends QueryTemplateProps { fields: string[]; } -type TimelineQueryProps = OwnProps & PropsFromRedux & WithKibanaProps; +type TimelineQueryProps = OwnProps & PropsFromRedux & WithKibanaProps & CustomReduxProps; class TimelineQueryComponent extends QueryTemplate< TimelineQueryProps, @@ -68,6 +74,7 @@ class TimelineQueryComponent extends QueryTemplate< public render() { const { children, + clearSignalsState, eventType = 'raw', id, indexPattern, @@ -97,6 +104,7 @@ class TimelineQueryComponent extends QueryTemplate< defaultIndex, inspect: isInspected, }; + return ( query={timelineQuery} @@ -105,6 +113,10 @@ class TimelineQueryComponent extends QueryTemplate< variables={variables} > {({ data, loading, fetchMore, refetch }) => { + this.setRefetch(refetch); + this.setExecuteBeforeRefetch(clearSignalsState); + this.setExecuteBeforeFetchMore(clearSignalsState); + const timelineEdges = getOr([], 'source.Timeline.edges', data); this.setFetchMore(fetchMore); this.setFetchMoreOptions((newCursor: string, tiebreaker?: string) => ({ @@ -138,7 +150,7 @@ class TimelineQueryComponent extends QueryTemplate< return children!({ id, inspect: getOr(null, 'source.Timeline.inspect', data), - refetch, + refetch: this.wrappedRefetch, loading, totalCount: getOr(0, 'source.Timeline.totalCount', data), pageInfo: getOr({}, 'source.Timeline.pageInfo', data), @@ -168,7 +180,16 @@ const makeMapStateToProps = () => { return mapStateToProps; }; -const connector = connect(makeMapStateToProps); +const mapDispatchToProps = (dispatch: Dispatch) => ({ + clearSignalsState: ({ id }: { id?: string }) => { + if (id != null && id === SIGNALS_PAGE_TIMELINE_ID) { + dispatch(timelineActions.clearEventsLoading({ id })); + dispatch(timelineActions.clearEventsDeleted({ id })); + } + }, +}); + +const connector = connect(makeMapStateToProps, mapDispatchToProps); type PropsFromRedux = ConnectedProps; diff --git a/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals/index.tsx b/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals/index.tsx index 7cd26ac0cc41bc..75f19218d9b387 100644 --- a/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals/index.tsx +++ b/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals/index.tsx @@ -47,7 +47,7 @@ import { } from './types'; import { dispatchUpdateTimeline } from '../../../../components/open_timeline/helpers'; -const SIGNALS_PAGE_TIMELINE_ID = 'signals-page'; +export const SIGNALS_PAGE_TIMELINE_ID = 'signals-page'; interface OwnProps { canUserCRUD: boolean; diff --git a/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals/signals_utility_bar/index.tsx b/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals/signals_utility_bar/index.tsx index 13d77385c53d42..86772eb0e155de 100644 --- a/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals/signals_utility_bar/index.tsx +++ b/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals/signals_utility_bar/index.tsx @@ -114,6 +114,7 @@ const SignalsUtilityBarComponent: React.FC = ({ export const SignalsUtilityBar = React.memo( SignalsUtilityBarComponent, (prevProps, nextProps) => + prevProps.areEventsLoading === nextProps.areEventsLoading && prevProps.selectedEventIds === nextProps.selectedEventIds && prevProps.totalCount === nextProps.totalCount && prevProps.showClearSelection === nextProps.showClearSelection diff --git a/x-pack/legacy/plugins/siem/public/store/timeline/helpers.ts b/x-pack/legacy/plugins/siem/public/store/timeline/helpers.ts index f56bbc34ce165a..fa70c1b04608da 100644 --- a/x-pack/legacy/plugins/siem/public/store/timeline/helpers.ts +++ b/x-pack/legacy/plugins/siem/public/store/timeline/helpers.ts @@ -1161,11 +1161,22 @@ export const setDeletedTimelineEvents = ({ ? union(timeline.deletedEventIds, eventIds) : timeline.deletedEventIds.filter(currentEventId => !eventIds.includes(currentEventId)); + const selectedEventIds = Object.fromEntries( + Object.entries(timeline.selectedEventIds).filter( + ([selectedEventId]) => !deletedEventIds.includes(selectedEventId) + ) + ); + + const isSelectAllChecked = + Object.keys(selectedEventIds).length > 0 ? timeline.isSelectAllChecked : false; + return { ...timelineById, [id]: { ...timeline, deletedEventIds, + selectedEventIds, + isSelectAllChecked, }, }; };