Skip to content

Commit

Permalink
[Security Solution] [Alerts Table] [RAC] Prevent duplicate items bein…
Browse files Browse the repository at this point in the history
…g added by bulk actions select all (elastic#182007)

## Summary

Related issue: elastic#181972

When the useBulkActions hook in the alerts table is used with the
optionally defined at registration time hook useBulkActionsConfig, and
that hook returns a stable array, the hook calls a function that mutates
this array directly, which causes duplicate items to appear in the alert
context menu. This is due to the useBulkActions hook using the bulk
actions state via context, and so runs every time a user selects a
row/all rows. Doing this via filter might not be the most performant
way, but in order to have everything referentially stable, I think the
arguments to useBulkActions/useBulkUntrackActions should be changed a
bit, but that can be a discussion for another time. The test case added
below will currently fail on main/8.14.

Before:

![bulk_actions_with_dupe](https://github.com/elastic/kibana/assets/56408403/7f730bb9-fcb2-4a8e-93f8-3e08523e3a34)


After:

![bulk_actions_no_dupe](https://github.com/elastic/kibana/assets/56408403/01f76c6b-59fb-459f-8950-1fc1fe8dfe12)


### 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
  • Loading branch information
kqualters-elastic committed Apr 30, 2024
1 parent e01d800 commit 8a1d295
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { useBulkActions, useBulkAddToCaseActions, useBulkUntrackActions } from '
import { AppMockRenderer, createAppMockRenderer } from '../../test_utils';
import { createCasesServiceMock } from '../index.mock';
import { AlertsTableQueryContext } from '../contexts/alerts_table_context';
import { BulkActionsVerbs } from '../../../../types';

jest.mock('./apis/bulk_get_cases');
jest.mock('../../../../common/lib/kibana');
Expand Down Expand Up @@ -422,5 +423,45 @@ describe('bulk action hooks', () => {
]
`);
});

it('should not append duplicate items on rerender', async () => {
const onClick = () => {};
const items = [
{
label: 'test',
key: 'test',
'data-test-subj': 'test',
disableOnQuery: true,
disabledLabel: 'test',
onClick,
},
];
const customBulkActionConfig = [
{
id: 0,
items,
},
];
const useBulkActionsConfig = () => customBulkActionConfig;
const { result, rerender } = renderHook(
() => useBulkActions({ alerts: [], query: {}, casesConfig, refresh, useBulkActionsConfig }),
{
wrapper: appMockRender.AppWrapper,
}
);
const initialBulkActions = result.current.bulkActions[0].items
? [...result.current.bulkActions[0].items]
: [];
result.current.updateBulkActionsState({ action: BulkActionsVerbs.selectCurrentPage });
rerender();
result.current.updateBulkActionsState({ action: BulkActionsVerbs.clear });
rerender();
result.current.updateBulkActionsState({ action: BulkActionsVerbs.selectCurrentPage });
rerender();
result.current.updateBulkActionsState({ action: BulkActionsVerbs.selectAll });
rerender();
const newBulkActions = result.current.bulkActions[0].items;
expect(initialBulkActions).toEqual(newBulkActions);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
BulkActionsPanelConfig,
BulkActionsState,
BulkActionsVerbs,
BulkActionsReducerAction,
UseBulkActionsRegistry,
} from '../../../../types';
import {
Expand Down Expand Up @@ -50,6 +51,7 @@ export interface UseBulkActions {
bulkActions: BulkActionsPanelConfig[];
setIsBulkActionsLoading: (isLoading: boolean) => void;
clearSelection: () => void;
updateBulkActionsState: React.Dispatch<BulkActionsReducerAction>;
}

type UseBulkAddToCaseActionsProps = Pick<BulkActionsProps, 'casesConfig' | 'refresh'> &
Expand Down Expand Up @@ -90,7 +92,9 @@ const addItemsToInitialPanel = ({
}) => {
if (panels.length > 0) {
if (panels[0].items) {
panels[0].items.push(...items);
panels[0].items = [...panels[0].items, ...items].filter(
(item, index, self) => index === self.findIndex((newItem) => newItem.key === item.key)
);
}
return panels;
} else {
Expand Down Expand Up @@ -205,6 +209,33 @@ export const useBulkUntrackActions = ({
const hasUptimePermission = application?.capabilities.uptime?.show;
const hasSloPermission = application?.capabilities.slo?.show;
const hasObservabilityPermission = application?.capabilities.observability?.show;
const onClick = useCallback(
async (alerts?: TimelineItem[]) => {
if (!alerts) return;
const alertUuids = alerts.map((alert) => alert._id);
const indices = alerts.map((alert) => alert._index ?? '');
try {
setIsBulkActionsLoading(true);
if (isAllSelected) {
await untrackAlertsByQuery({ query, featureIds });
} else {
await untrackAlerts({ indices, alertUuids });
}
onSuccess();
} finally {
setIsBulkActionsLoading(false);
}
},
[
query,
featureIds,
isAllSelected,
onSuccess,
setIsBulkActionsLoading,
untrackAlerts,
untrackAlertsByQuery,
]
);

return useMemo(() => {
// Check if at least one Observability feature is enabled
Expand All @@ -225,39 +256,18 @@ export const useBulkUntrackActions = ({
disableOnQuery: false,
disabledLabel: MARK_AS_UNTRACKED,
'data-test-subj': 'mark-as-untracked',
onClick: async (alerts?: TimelineItem[]) => {
if (!alerts) return;
const alertUuids = alerts.map((alert) => alert._id);
const indices = alerts.map((alert) => alert._index ?? '');
try {
setIsBulkActionsLoading(true);
if (isAllSelected) {
await untrackAlertsByQuery({ query, featureIds });
} else {
await untrackAlerts({ indices, alertUuids });
}
onSuccess();
} finally {
setIsBulkActionsLoading(false);
}
},
onClick,
},
];
}, [
onSuccess,
setIsBulkActionsLoading,
untrackAlerts,
application?.capabilities,
hasApmPermission,
hasInfrastructurePermission,
hasLogsPermission,
hasUptimePermission,
hasSloPermission,
hasObservabilityPermission,
featureIds,
query,
isAllSelected,
untrackAlertsByQuery,
onClick,
]);
};

Expand Down

0 comments on commit 8a1d295

Please sign in to comment.