mirror of
https://github.com/elastic/kibana.git
synced 2025-04-25 02:09:32 -04:00
[Security Solution] [Alerts Table] [RAC] Prevent duplicate items being added by bulk actions select all (#182007)
## Summary Related issue: #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:  After:  ### 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
This commit is contained in:
parent
e01d80066c
commit
8a1d2950fa
2 changed files with 75 additions and 24 deletions
|
@ -10,6 +10,7 @@ import { useBulkActions, useBulkAddToCaseActions, useBulkUntrackActions } from '
|
||||||
import { AppMockRenderer, createAppMockRenderer } from '../../test_utils';
|
import { AppMockRenderer, createAppMockRenderer } from '../../test_utils';
|
||||||
import { createCasesServiceMock } from '../index.mock';
|
import { createCasesServiceMock } from '../index.mock';
|
||||||
import { AlertsTableQueryContext } from '../contexts/alerts_table_context';
|
import { AlertsTableQueryContext } from '../contexts/alerts_table_context';
|
||||||
|
import { BulkActionsVerbs } from '../../../../types';
|
||||||
|
|
||||||
jest.mock('./apis/bulk_get_cases');
|
jest.mock('./apis/bulk_get_cases');
|
||||||
jest.mock('../../../../common/lib/kibana');
|
jest.mock('../../../../common/lib/kibana');
|
||||||
|
@ -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);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
@ -16,6 +16,7 @@ import {
|
||||||
BulkActionsPanelConfig,
|
BulkActionsPanelConfig,
|
||||||
BulkActionsState,
|
BulkActionsState,
|
||||||
BulkActionsVerbs,
|
BulkActionsVerbs,
|
||||||
|
BulkActionsReducerAction,
|
||||||
UseBulkActionsRegistry,
|
UseBulkActionsRegistry,
|
||||||
} from '../../../../types';
|
} from '../../../../types';
|
||||||
import {
|
import {
|
||||||
|
@ -50,6 +51,7 @@ export interface UseBulkActions {
|
||||||
bulkActions: BulkActionsPanelConfig[];
|
bulkActions: BulkActionsPanelConfig[];
|
||||||
setIsBulkActionsLoading: (isLoading: boolean) => void;
|
setIsBulkActionsLoading: (isLoading: boolean) => void;
|
||||||
clearSelection: () => void;
|
clearSelection: () => void;
|
||||||
|
updateBulkActionsState: React.Dispatch<BulkActionsReducerAction>;
|
||||||
}
|
}
|
||||||
|
|
||||||
type UseBulkAddToCaseActionsProps = Pick<BulkActionsProps, 'casesConfig' | 'refresh'> &
|
type UseBulkAddToCaseActionsProps = Pick<BulkActionsProps, 'casesConfig' | 'refresh'> &
|
||||||
|
@ -90,7 +92,9 @@ const addItemsToInitialPanel = ({
|
||||||
}) => {
|
}) => {
|
||||||
if (panels.length > 0) {
|
if (panels.length > 0) {
|
||||||
if (panels[0].items) {
|
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;
|
return panels;
|
||||||
} else {
|
} else {
|
||||||
|
@ -205,6 +209,33 @@ export const useBulkUntrackActions = ({
|
||||||
const hasUptimePermission = application?.capabilities.uptime?.show;
|
const hasUptimePermission = application?.capabilities.uptime?.show;
|
||||||
const hasSloPermission = application?.capabilities.slo?.show;
|
const hasSloPermission = application?.capabilities.slo?.show;
|
||||||
const hasObservabilityPermission = application?.capabilities.observability?.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(() => {
|
return useMemo(() => {
|
||||||
// Check if at least one Observability feature is enabled
|
// Check if at least one Observability feature is enabled
|
||||||
|
@ -225,28 +256,10 @@ export const useBulkUntrackActions = ({
|
||||||
disableOnQuery: false,
|
disableOnQuery: false,
|
||||||
disabledLabel: MARK_AS_UNTRACKED,
|
disabledLabel: MARK_AS_UNTRACKED,
|
||||||
'data-test-subj': 'mark-as-untracked',
|
'data-test-subj': 'mark-as-untracked',
|
||||||
onClick: async (alerts?: TimelineItem[]) => {
|
onClick,
|
||||||
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);
|
|
||||||
}
|
|
||||||
},
|
|
||||||
},
|
},
|
||||||
];
|
];
|
||||||
}, [
|
}, [
|
||||||
onSuccess,
|
|
||||||
setIsBulkActionsLoading,
|
|
||||||
untrackAlerts,
|
|
||||||
application?.capabilities,
|
application?.capabilities,
|
||||||
hasApmPermission,
|
hasApmPermission,
|
||||||
hasInfrastructurePermission,
|
hasInfrastructurePermission,
|
||||||
|
@ -254,10 +267,7 @@ export const useBulkUntrackActions = ({
|
||||||
hasUptimePermission,
|
hasUptimePermission,
|
||||||
hasSloPermission,
|
hasSloPermission,
|
||||||
hasObservabilityPermission,
|
hasObservabilityPermission,
|
||||||
featureIds,
|
onClick,
|
||||||
query,
|
|
||||||
isAllSelected,
|
|
||||||
untrackAlertsByQuery,
|
|
||||||
]);
|
]);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue