Skip to content

Commit

Permalink
[8.2] [Security Solution] Remove sourcerer browser fields hover actio…
Browse files Browse the repository at this point in the history
…ns to help performance (#131363) (#132261)

* [Security Solution] Remove sourcerer browser fields hover actions to help performance (#131363)

* Batch setState calls to make sure all state updates are applied evenly

* Remove sourcerer hook from useHoverActions and pass needed fields as props

* Update snapshots, remove ReactDOM batching

* Make row renderers aggregatable where appropriate

* add pagination to details table

* Fix hover actions on host/network details

* Remove unneeded props

* fix table pagination tests

* update test

* Show top n for authentications and threat indicator match rules

* Fix anomaly score, entity, influence, and user id show top N

* Pass props on wrapper and not data provider

* Add missing row renderer draggables to use top N props

* Update snapshots

* Pr feedback

Co-authored-by: Michael Olorunnisola <michael.olorunnisola@elastic.co>
Co-authored-by: Robert Austin <robert.austin@elastic.co>
(cherry picked from commit f3c1ad7)

# Conflicts:
#	x-pack/plugins/security_solution/public/common/components/authentication/helpers.tsx
#	x-pack/plugins/security_solution/public/common/lib/cell_actions/expanded_cell_value_actions.tsx

* meant to delete this

* fix some lint rules i hope

Co-authored-by: Kevin Qualters <56408403+kqualters-elastic@users.noreply.github.com>
  • Loading branch information
Robert Austin and kqualters-elastic authored May 16, 2022
1 parent 915bf7b commit 6fdb90c
Show file tree
Hide file tree
Showing 89 changed files with 593 additions and 107 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ interface Props {
hideTopN?: boolean;
isDraggable?: boolean;
render: RenderFunctionProp;
isAggregatable?: boolean;
fieldType?: string;
timelineId?: string;
truncate?: boolean;
onFilterAdded?: () => void;
Expand Down Expand Up @@ -131,6 +133,8 @@ const DraggableOnWrapperComponent: React.FC<Props> = ({
hideTopN = false,
onFilterAdded,
render,
fieldType = '',
isAggregatable = false,
timelineId,
truncate,
}) => {
Expand All @@ -154,6 +158,8 @@ const DraggableOnWrapperComponent: React.FC<Props> = ({
hideTopN,
onFilterAdded,
render,
fieldType,
isAggregatable,
timelineId,
truncate,
});
Expand Down Expand Up @@ -313,6 +319,8 @@ const DraggableWrapperComponent: React.FC<Props> = ({
isDraggable = false,
onFilterAdded,
render,
isAggregatable = false,
fieldType = '',
timelineId,
truncate,
}) => {
Expand All @@ -327,6 +335,8 @@ const DraggableWrapperComponent: React.FC<Props> = ({
dataProvider,
hideTopN,
isDraggable,
isAggregatable,
fieldType,
onFilterAdded,
render,
timelineId,
Expand Down Expand Up @@ -372,6 +382,8 @@ const DraggableWrapperComponent: React.FC<Props> = ({
dataProvider={dataProvider}
hideTopN={hideTopN}
onFilterAdded={onFilterAdded}
fieldType={fieldType}
isAggregatable={isAggregatable}
render={render}
timelineId={timelineId}
truncate={truncate}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,8 @@ describe('helpers', () => {
test('it returns true for an aggregatable field that is an allowed type', () => {
expect(
allowTopN({
browserField: aggregatableAllowedType,
fieldType: 'keyword',
isAggregatable: true,
fieldName: aggregatableAllowedType.name,
hideTopN: false,
})
Expand All @@ -663,7 +664,8 @@ describe('helpers', () => {
test('it returns true for a allowlisted non-BrowserField', () => {
expect(
allowTopN({
browserField: undefined,
fieldType: 'not right',
isAggregatable: false,
fieldName: 'kibana.alert.rule.name',
hideTopN: false,
})
Expand All @@ -678,7 +680,8 @@ describe('helpers', () => {

expect(
allowTopN({
browserField: nonAggregatableAllowedType,
fieldType: 'keyword',
isAggregatable: false,
fieldName: nonAggregatableAllowedType.name,
hideTopN: false,
})
Expand All @@ -693,31 +696,21 @@ describe('helpers', () => {

expect(
allowTopN({
browserField: aggregatableNotAllowedType,
fieldType: 'text',
isAggregatable: false,
fieldName: aggregatableNotAllowedType.name,
hideTopN: false,
})
).toBe(false);
});

test('it returns false if the BrowserField is missing the aggregatable property', () => {
const missingAggregatable = omit('aggregatable', aggregatableAllowedType);

expect(
allowTopN({
browserField: missingAggregatable,
fieldName: missingAggregatable.name,
hideTopN: false,
})
).toBe(false);
});

test('it returns false if the BrowserField is missing the type property', () => {
const missingType = omit('type', aggregatableAllowedType);

expect(
allowTopN({
browserField: missingType,
fieldType: 'not real',
isAggregatable: false,
fieldName: missingType.name,
hideTopN: false,
})
Expand All @@ -727,7 +720,8 @@ describe('helpers', () => {
test('it returns false for a non-allowlisted field when a BrowserField is not provided', () => {
expect(
allowTopN({
browserField: undefined,
fieldType: 'string',
isAggregatable: false,
fieldName: 'non-allowlisted',
hideTopN: false,
})
Expand All @@ -737,7 +731,8 @@ describe('helpers', () => {
test('it returns false when hideTopN is true', () => {
expect(
allowTopN({
browserField: aggregatableAllowedType,
fieldType: 'keyword',
isAggregatable: true,
fieldName: aggregatableAllowedType.name,
hideTopN: true, // <-- the Top N action shall not be shown for this (otherwise valid) field
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { Dispatch } from 'redux';
import { ActionCreator } from 'typescript-fsa';
import { getProviderIdFromDraggable } from '@kbn/securitysolution-t-grid';

import { BrowserField } from '../../containers/source';
import { dragAndDropActions } from '../../store/actions';
import { IdToDataProvider } from '../../store/drag_and_drop/model';
import { addContentToTimeline } from '../../../timelines/components/timeline/data_providers/helpers';
Expand Down Expand Up @@ -90,16 +89,16 @@ export const addProviderToTimeline = ({
};

export const allowTopN = ({
browserField,
isAggregatable,
fieldType,
fieldName,
hideTopN,
}: {
browserField: Partial<BrowserField> | undefined;
fieldName: string;
isAggregatable: boolean;
fieldType: string;
hideTopN: boolean;
}): boolean => {
const isAggregatable = browserField?.aggregatable ?? false;
const fieldType = browserField?.type ?? '';
const isAllowedType = [
'boolean',
'geo-point',
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export interface DefaultDraggableType {
hideTopN?: boolean;
id: string;
isDraggable?: boolean;
fieldType?: string;
isAggregatable?: boolean;
field: string;
value?: string | number | null;
name?: string | null;
Expand Down Expand Up @@ -102,6 +104,8 @@ export const DefaultDraggable = React.memo<DefaultDraggableType>(
id,
isDraggable = true,
field,
fieldType = '',
isAggregatable = false,
value,
name,
children,
Expand Down Expand Up @@ -151,6 +155,8 @@ export const DefaultDraggable = React.memo<DefaultDraggableType>(
return (
<DraggableWrapper
dataProvider={dataProviderProp}
fieldType={fieldType}
isAggregatable={isAggregatable}
hideTopN={hideTopN}
isDraggable={isDraggable}
render={renderCallback}
Expand Down Expand Up @@ -198,6 +204,8 @@ const DraggableBadgeComponent: React.FC<BadgeDraggableType> = ({
value,
iconType,
isDraggable,
isAggregatable,
fieldType,
name,
color = 'hollow',
children,
Expand All @@ -208,6 +216,8 @@ const DraggableBadgeComponent: React.FC<BadgeDraggableType> = ({
<DefaultDraggable
id={`draggable-badge-default-draggable-${contextId}-${eventId}-${field}-${value}`}
isDraggable={isDraggable}
isAggregatable={isAggregatable}
fieldType={fieldType}
field={field}
name={name}
value={value}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ const StyledEuiInMemoryTable = styled(EuiInMemoryTable as any)`
flex: 1;
overflow: auto;
overflow-x: hidden;
&::-webkit-scrollbar {
height: ${({ theme }) => theme.eui.euiScrollBar};
width: ${({ theme }) => theme.eui.euiScrollBar};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ export const ActionCell: React.FC<Props> = React.memo(
values,
});

const { aggregatable, type } = fieldFromBrowserField || { aggregatable: false, type: '' };

const [showTopN, setShowTopN] = useState<boolean>(false);
const { timelineId: timelineIdFind } = useContext(TimelineContext);
const [hoverActionsOwnFocus] = useState<boolean>(false);
Expand All @@ -74,6 +76,8 @@ export const ActionCell: React.FC<Props> = React.memo(
dataProvider={actionCellConfig?.dataProvider}
enableOverflowButton={true}
field={data.field}
isAggregatable={aggregatable}
fieldType={type}
hideAddToTimeline={hideAddToTimeline}
isObjectArray={data.isObjectArray}
onFilterAdded={onFilterAdded}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export const FieldValueCell = React.memo(
fieldFormat={data.format}
fieldName={data.field}
fieldType={data.type}
isAggregatable={fieldFromBrowserField.aggregatable}
isDraggable={isDraggable}
isObjectArray={data.isObjectArray}
value={value}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ interface Props {
draggableId?: DraggableId;
enableOverflowButton?: boolean;
field: string;
fieldType: string;
isAggregatable: boolean;
goGetTimelineId?: (args: boolean) => void;
hideAddToTimeline?: boolean;
hideTopN?: boolean;
Expand Down Expand Up @@ -136,6 +138,8 @@ export const HoverActions: React.FC<Props> = React.memo(
enableOverflowButton = false,
applyWidthAndPadding = true,
field,
fieldType,
isAggregatable,
goGetTimelineId,
isObjectArray,
hideAddToTimeline = false,
Expand Down Expand Up @@ -219,6 +223,8 @@ export const HoverActions: React.FC<Props> = React.memo(
draggableId,
enableOverflowButton: enableOverflowButton && !isCaseView,
field,
fieldType,
isAggregatable,
handleHoverActionClicked,
hideAddToTimeline,
hideTopN,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,9 @@ import { DraggableId } from 'react-beautiful-dnd';
import { isEmpty } from 'lodash';

import { useKibana } from '../../lib/kibana';
import { getAllFieldsByName } from '../../containers/source';
import { allowTopN } from '../drag_and_drop/helpers';
import { useDeepEqualSelector } from '../../hooks/use_selector';
import { ColumnHeaderOptions, DataProvider, TimelineId } from '../../../../common/types/timeline';
import { SourcererScopeName } from '../../store/sourcerer/model';
import { useSourcererDataView } from '../../containers/sourcerer';
import { timelineSelectors } from '../../../timelines/store/timeline';
import { ShowTopNButton } from './actions/show_top_n';
import { FilterManager } from '../../../../../../../src/plugins/data/public';
Expand All @@ -29,6 +26,8 @@ export interface UseHoverActionItemsProps {
draggableId?: DraggableId;
enableOverflowButton?: boolean;
field: string;
fieldType: string;
isAggregatable: boolean;
handleHoverActionClicked: () => void;
hideAddToTimeline: boolean;
hideTopN: boolean;
Expand Down Expand Up @@ -59,6 +58,8 @@ export const useHoverActionItems = ({
draggableId,
enableOverflowButton,
field,
fieldType,
isAggregatable,
handleHoverActionClicked,
hideTopN,
hideAddToTimeline,
Expand Down Expand Up @@ -103,23 +104,6 @@ export const useHoverActionItems = ({
[uiSettings, timelineId, activeFilterManager, filterManagerBackup]
);

// Regarding data from useManageTimeline:
// * `indexToAdd`, which enables the alerts index to be appended to
// the `indexPattern` returned by `useWithSource`, may only be populated when
// this component is rendered in the context of the active timeline. This
// behavior enables the 'All events' view by appending the alerts index
// to the index pattern.
const activeScope: SourcererScopeName =
timelineId === TimelineId.active
? SourcererScopeName.timeline
: timelineId != null &&
[TimelineId.detectionsPage, TimelineId.detectionsRulesDetailsPage].includes(
timelineId as TimelineId
)
? SourcererScopeName.detections
: SourcererScopeName.default;
const { browserFields } = useSourcererDataView(activeScope);

/*
* In the case of `DisableOverflowButton`, we show filters only when topN is NOT opened. As after topN button is clicked, the chart panel replace current hover actions in the hover actions' popover, so we have to hide all the actions.
* in the case of `EnableOverflowButton`, we only need to hide all the items in the overflow popover as the chart's panel opens in the overflow popover, so non-overflowed actions are not affected.
Expand Down Expand Up @@ -222,7 +206,8 @@ export const useHoverActionItems = ({
</div>
) : null,
allowTopN({
browserField: getAllFieldsByName(browserFields)[field],
fieldType,
isAggregatable,
fieldName: field,
hideTopN,
})
Expand All @@ -246,13 +231,14 @@ export const useHoverActionItems = ({
return item != null;
}),
[
browserFields,
dataProvider,
dataType,
defaultFocusedButtonRef,
draggableId,
enableOverflowButton,
field,
fieldType,
isAggregatable,
filterManager,
getAddToTimelineButton,
getColumnToggleButton,
Expand Down
Loading

0 comments on commit 6fdb90c

Please sign in to comment.