Skip to content

Commit

Permalink
[SIEM] [Detection Engine] Fixes Signals Table bulk selection issues (#…
Browse files Browse the repository at this point in the history
…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
<details><summary>Before</summary>
<p>

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

</p>
</details>

<details><summary>After</summary>
<p>

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


##### Bulk action not being enabled
<details><summary>Before</summary>
<p>

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

<details><summary>After</summary>
<p>

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







### 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)~
  • Loading branch information
spong authored Feb 19, 2020
1 parent c47612f commit 05947ad
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ const EventsViewerComponent: React.FC<Props> = ({
totalCountMinusDeleted
) ?? i18n.UNIT(totalCountMinusDeleted)}`;

// TODO: Reset eventDeletedIds/eventLoadingIds on refresh/loadmore (getUpdatedAt)
return (
<>
<HeaderSection
Expand Down
30 changes: 27 additions & 3 deletions x-pack/legacy/plugins/siem/public/containers/query_template.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface QueryTemplateProps {
startDate?: number;
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
type FetchMoreOptionsArgs<TData, TVariables> = FetchMoreQueryOptions<any, any> &
export type FetchMoreOptionsArgs<TData, TVariables> = FetchMoreQueryOptions<any, any> &
FetchMoreOptions<TData, TVariables>;

// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand All @@ -40,6 +40,19 @@ export class QueryTemplate<
tiebreaker?: string
) => FetchMoreOptionsArgs<TData, TVariables>;

private refetch!: (variables?: TVariables) => Promise<ApolloQueryResult<TData>>;

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<TData, TVariables>) => PromiseApolloQueryResult
) => {
Expand All @@ -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<ApolloQueryResult<TData>>) => {
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);
};
}
29 changes: 25 additions & 4 deletions x-pack/legacy/plugins/siem/public/containers/timeline/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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[];
Expand All @@ -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;
Expand All @@ -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,
Expand All @@ -68,6 +74,7 @@ class TimelineQueryComponent extends QueryTemplate<
public render() {
const {
children,
clearSignalsState,
eventType = 'raw',
id,
indexPattern,
Expand Down Expand Up @@ -97,6 +104,7 @@ class TimelineQueryComponent extends QueryTemplate<
defaultIndex,
inspect: isInspected,
};

return (
<Query<GetTimelineQuery.Query, GetTimelineQuery.Variables>
query={timelineQuery}
Expand All @@ -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) => ({
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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<typeof connector>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ const SignalsUtilityBarComponent: React.FC<SignalsUtilityBarProps> = ({
export const SignalsUtilityBar = React.memo(
SignalsUtilityBarComponent,
(prevProps, nextProps) =>
prevProps.areEventsLoading === nextProps.areEventsLoading &&
prevProps.selectedEventIds === nextProps.selectedEventIds &&
prevProps.totalCount === nextProps.totalCount &&
prevProps.showClearSelection === nextProps.showClearSelection
Expand Down
11 changes: 11 additions & 0 deletions x-pack/legacy/plugins/siem/public/store/timeline/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
};
};
Expand Down

0 comments on commit 05947ad

Please sign in to comment.