Skip to content

Commit 2ee0e88

Browse files
kqualters-elasticJoseLuisGJ
authored andcommitted
[Security Solution] Cleanup alerts table rendering and reference issues (elastic#213649)
## Summary This pr fixes some odd issues with getBulkActions, which is really a hook in disguise, as well as an issue with the useGetMutedAlertsQuery hook, which was/is fetching data much more often than it should, exactly why that is I'm not sure, perhaps something to do with how timeline blocks focus to the underlying DOM when it's open, and this causes the default to true refetchOnWindowFocus prop of useQuery to re-run the query, or if there's an error with the queryKey. Below are 2 GIFs comparing react performance profiles of simply opening and then closing the timeline while on the alerts page with 50 alerts in the table. Before fix: ![pre_fixes_profiler](https://github.com/user-attachments/assets/548d1ea8-6bde-460f-90da-0cead5ea76e1) 12 renders for a total of 950 ms, a large portion of which is coming from the alert table cells. After fix: ![with_fixes_profiler](https://github.com/user-attachments/assets/7119725a-fe3a-4e66-a181-4dd7b24204f0) 8 renders for a total of 380 ms, almost none of it coming from the alert table. Each of the alerts table and timeline/discover drive some of the more stateful and complex workflows in kibana on their own, and on top of that one is rendering within a flyout on top of the other, listening to the same url changes/tens of context provider wrappers changing above them in the tree/kibana services, etc, & so proper memoization is a pre-requisite for a good ux. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
1 parent ee0ef13 commit 2ee0e88

File tree

24 files changed

+370
-237
lines changed

24 files changed

+370
-237
lines changed

src/platform/packages/shared/response-ops/alerts-apis/hooks/use_get_muted_alerts_query.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,5 +54,6 @@ export const useGetMutedAlertsQuery = (
5454
}
5555
},
5656
enabled: ruleIds.length > 0 && enabled !== false,
57+
refetchOnWindowFocus: false,
5758
});
5859
};

src/platform/packages/shared/response-ops/alerts-table/components/alerts_data_grid.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export const mockDataGridProps: Partial<BaseAlertsDataGridProps> = {
7474
onSortChange: jest.fn(),
7575
onChangePageIndex: jest.fn(),
7676
onChangePageSize: jest.fn(),
77-
getBulkActions: () => [
77+
additionalBulkActions: [
7878
{
7979
id: 0,
8080
items: [

src/platform/packages/shared/response-ops/alerts-table/components/alerts_data_grid.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ export const AlertsDataGrid = typedMemo(
7979
onChangePageSize,
8080
onChangePageIndex,
8181
actionsColumnWidth = DEFAULT_ACTIONS_COLUMN_WIDTH,
82-
getBulkActions,
82+
additionalBulkActions,
8383
fieldsBrowserOptions,
8484
cellActionsOptions,
8585
pageSizeOptions = DEFAULT_PAGE_SIZE_OPTIONS,
@@ -114,7 +114,7 @@ export const AlertsDataGrid = typedMemo(
114114
query,
115115
alertsCount: alerts.length,
116116
casesConfig: casesConfiguration,
117-
getBulkActions,
117+
additionalBulkActions,
118118
refresh: refreshQueries,
119119
hideBulkActions,
120120
http,

src/platform/packages/shared/response-ops/alerts-table/hooks/use_bulk_actions.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -583,15 +583,15 @@ describe('bulk action hooks', () => {
583583
items,
584584
},
585585
];
586-
const useBulkActionsConfig = () => customBulkActionConfig;
586+
const useBulkActionsConfig = customBulkActionConfig;
587587
const { result, rerender } = renderHook(
588588
() =>
589589
useBulkActions({
590590
alertsCount: 0,
591591
query: {},
592592
casesConfig,
593593
refresh,
594-
getBulkActions: useBulkActionsConfig,
594+
additionalBulkActions: useBulkActionsConfig,
595595
http,
596596
notifications,
597597
application,

src/platform/packages/shared/response-ops/alerts-table/hooks/use_bulk_actions.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ interface BulkActionsProps {
3939
query: Pick<QueryDslQueryContainer, 'bool' | 'ids'>;
4040
alertsCount: number;
4141
casesConfig?: PublicAlertsDataGridProps['casesConfiguration'];
42-
getBulkActions?: PublicAlertsDataGridProps['getBulkActions'];
42+
additionalBulkActions?: PublicAlertsDataGridProps['additionalBulkActions'];
4343
refresh: () => void;
4444
hideBulkActions?: boolean;
4545
application: ApplicationStart;
@@ -283,14 +283,14 @@ export const useBulkUntrackActions = ({
283283
]);
284284
};
285285

286-
const EMPTY_BULK_ACTIONS_CONFIG = () => [];
286+
const EMPTY_BULK_ACTIONS_CONFIG: BulkActionsPanelConfig[] = [];
287287

288288
export function useBulkActions({
289289
alertsCount,
290290
casesConfig,
291291
query,
292292
refresh,
293-
getBulkActions = EMPTY_BULK_ACTIONS_CONFIG,
293+
additionalBulkActions = EMPTY_BULK_ACTIONS_CONFIG,
294294
ruleTypeIds,
295295
hideBulkActions,
296296
http,
@@ -301,7 +301,6 @@ export function useBulkActions({
301301
const {
302302
bulkActionsStore: [bulkActionsState, updateBulkActionsState],
303303
} = useAlertsTableContext();
304-
const configBulkActionPanels = getBulkActions(query, refresh);
305304

306305
const clearSelection = useCallback(() => {
307306
updateBulkActionsState({ action: BulkActionsVerbs.clear });
@@ -343,11 +342,11 @@ export function useBulkActions({
343342

344343
return initialItems.length
345344
? addItemsToInitialPanel({
346-
panels: configBulkActionPanels,
345+
panels: additionalBulkActions,
347346
items: initialItems,
348347
})
349-
: configBulkActionPanels;
350-
}, [configBulkActionPanels, initialItems, hideBulkActions]);
348+
: additionalBulkActions;
349+
}, [additionalBulkActions, initialItems, hideBulkActions]);
351350

352351
const isBulkActionsColumnActive = bulkActions.length !== 0;
353352

src/platform/packages/shared/response-ops/alerts-table/reducers/bulk_actions_reducer.test.tsx

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ describe('AlertsDataGrid bulk actions', () => {
7777

7878
const dataGridProps: TestAlertsDataGridProps = {
7979
...mockDataGridProps,
80-
getBulkActions: undefined,
80+
additionalBulkActions: undefined,
8181
};
8282

8383
const baseRenderContext = {
@@ -92,7 +92,7 @@ describe('AlertsDataGrid bulk actions', () => {
9292

9393
const dataGridPropsWithBulkActions: AlertsTableWithBulkActionsContextProps = {
9494
...dataGridProps,
95-
getBulkActions: () => [
95+
additionalBulkActions: [
9696
{
9797
id: 0,
9898
items: [
@@ -173,7 +173,7 @@ describe('AlertsDataGrid bulk actions', () => {
173173
);
174174
};
175175

176-
describe('when the getBulkActions option is not set', () => {
176+
describe('when the additionalBulkActions option is not set', () => {
177177
beforeEach(() => {
178178
jest.clearAllMocks();
179179
});
@@ -249,7 +249,7 @@ describe('AlertsDataGrid bulk actions', () => {
249249
rowCount: 1,
250250
rowSelection: new Map([[0, { isLoading: false }]]),
251251
},
252-
getBulkActions: () => [
252+
additionalBulkActions: [
253253
{
254254
id: 0,
255255
items: [
@@ -310,7 +310,7 @@ describe('AlertsDataGrid bulk actions', () => {
310310
});
311311
});
312312

313-
describe('when the getBulkActions option is set', () => {
313+
describe('when the additionalBulkActions option is set', () => {
314314
beforeEach(() => {
315315
jest.clearAllMocks();
316316
});
@@ -498,7 +498,7 @@ describe('AlertsDataGrid bulk actions', () => {
498498
...createDefaultBulkActionsState(),
499499
rowSelection: new Map([[1, { isLoading: false }]]),
500500
},
501-
getBulkActions: () => [
501+
additionalBulkActions: [
502502
{
503503
id: 0,
504504
items: [
@@ -560,7 +560,7 @@ describe('AlertsDataGrid bulk actions', () => {
560560
const mockOnClick = jest.fn();
561561
const props: TestAlertsDataGridProps = {
562562
...dataGridPropsWithBulkActions,
563-
getBulkActions: () => [
563+
additionalBulkActions: [
564564
{
565565
id: 0,
566566
items: [
@@ -710,7 +710,7 @@ describe('AlertsDataGrid bulk actions', () => {
710710
[1, { isLoading: false }],
711711
]),
712712
},
713-
getBulkActions: () => [
713+
additionalBulkActions: [
714714
{
715715
id: 0,
716716
items: [

src/platform/packages/shared/response-ops/alerts-table/types.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -391,13 +391,9 @@ export interface PublicAlertsDataGridProps
391391
*/
392392
hideBulkActions?: boolean;
393393
/**
394-
* A getter to customize the bulk actions menu items
395-
* based on the current alerts search query used
394+
* An array of bulk actions to be displayed in the table
396395
*/
397-
getBulkActions?: (
398-
query: Pick<QueryDslQueryContainer, 'bool' | 'ids'>,
399-
refresh: () => void
400-
) => BulkActionsPanelConfig[];
396+
additionalBulkActions?: BulkActionsPanelConfig[];
401397
/**
402398
* Width of the actions column
403399
*/

x-pack/solutions/security/packages/data-table/store/data_table/selectors.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ const selectTableById = (state: DataTableState): TableById => state.dataTable.ta
1414

1515
export const tableByIdSelector = createSelector(selectTableById, (tableById) => tableById);
1616

17+
export const createTableSelector = (tableId: string) =>
18+
createSelector(tableByIdSelector, (tableById: TableById) => tableById[tableId]);
19+
1720
const selectTable = (state: DataTableState, tableId: string): DataTableModel =>
1821
state.dataTable.tableById[tableId];
1922

x-pack/solutions/security/plugins/security_solution/public/common/components/inspect/use_inspect.tsx

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,15 @@
66
*/
77

88
import { useCallback, useMemo } from 'react';
9-
import { useDispatch } from 'react-redux';
9+
import { useDispatch, useSelector } from 'react-redux';
1010
import { InputsModelId } from '../../store/inputs/constants';
1111
import { useDeepEqualSelector } from '../../hooks/use_selector';
1212
import { inputsSelectors } from '../../store';
1313
import { inputsActions } from '../../store/actions';
14+
import { TimelineId } from '../../../../common/types/timeline';
15+
import { selectTimelineById } from '../../../timelines/store/selectors';
16+
import { timelineDefaults } from '../../../timelines/store/defaults';
17+
import type { State } from '../../store';
1418

1519
interface UseInspectModalProps {
1620
inputId?: InputsModelId.global | InputsModelId.timeline;
@@ -34,12 +38,16 @@ export const useInspect = ({
3438
const dispatch = useDispatch();
3539

3640
const getGlobalQuery = useMemo(() => inputsSelectors.globalQueryByIdSelector(), []);
37-
const getTimelineQuery = useMemo(() => inputsSelectors.timelineQueryByIdSelector(), []);
41+
const getTimelineQuery = useMemo(() => inputsSelectors.timelineQueryByIdSelectorFactory(), []);
42+
const { activeTab } = useSelector(
43+
(state: State) => selectTimelineById(state, TimelineId.active) ?? timelineDefaults
44+
);
45+
3846
const { loading, inspect, selectedInspectIndex, isInspected, searchSessionId } =
3947
useDeepEqualSelector((state) =>
4048
inputId === InputsModelId.global
4149
? getGlobalQuery(state, queryId)
42-
: getTimelineQuery(state, queryId)
50+
: getTimelineQuery(state, `${TimelineId.active}-${activeTab}`)
4351
);
4452

4553
const handleClick = useCallback(() => {

x-pack/solutions/security/plugins/security_solution/public/common/components/page/use_refetch_by_session.tsx

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,18 @@
77

88
import type { MutableRefObject } from 'react';
99
import { useCallback, useRef, useMemo } from 'react';
10-
import { useDispatch } from 'react-redux';
10+
import { useDispatch, useSelector } from 'react-redux';
1111
import type { ISessionService } from '@kbn/data-plugin/public';
1212
import { useDeepEqualSelector } from '../../hooks/use_selector';
1313
import { useKibana } from '../../lib/kibana';
1414
import { inputsSelectors } from '../../store';
1515
import { inputsActions } from '../../store/actions';
1616
import { InputsModelId } from '../../store/inputs/constants';
1717
import type { Refetch } from '../../store/inputs/model';
18-
18+
import { TimelineId } from '../../../../common/types/timeline';
19+
import { selectTimelineById } from '../../../timelines/store/selectors';
20+
import type { State } from '../../store';
21+
import { timelineDefaults } from '../../../timelines/store/defaults';
1922
interface UseRefetchByRestartingSessionProps {
2023
inputId?: InputsModelId;
2124
queryId: string;
@@ -35,11 +38,15 @@ export const useRefetchByRestartingSession = ({
3538
const session = useRef(data.search.session);
3639

3740
const getGlobalQuery = useMemo(() => inputsSelectors.globalQueryByIdSelector(), []);
38-
const getTimelineQuery = useMemo(() => inputsSelectors.timelineQueryByIdSelector(), []);
41+
const getTimelineQuery = useMemo(() => inputsSelectors.timelineQueryByIdSelectorFactory(), []);
42+
const { activeTab } = useSelector(
43+
(state: State) => selectTimelineById(state, TimelineId.active) ?? timelineDefaults
44+
);
45+
3946
const { selectedInspectIndex } = useDeepEqualSelector((state) =>
4047
inputId === InputsModelId.global
4148
? getGlobalQuery(state, queryId)
42-
: getTimelineQuery(state, queryId)
49+
: getTimelineQuery(state, `${TimelineId.active}-${activeTab}`)
4350
);
4451

4552
const refetchByRestartingSession = useCallback(() => {

0 commit comments

Comments
 (0)