Skip to content

Commit

Permalink
[8.14] [Security Solution] [Alerts Table] [RAC] Prevent duplicate ite…
Browse files Browse the repository at this point in the history
…ms being added by bulk actions select all (#182007) (#182109)

# Backport

This will backport the following commits from `main` to `8.14`:
- [[Security Solution] [Alerts Table] [RAC] Prevent duplicate items
being added by bulk actions select all
(#182007)](#182007)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Kevin
Qualters","email":"56408403+kqualters-elastic@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-04-30T12:11:43Z","message":"[Security
Solution] [Alerts Table] [RAC] Prevent duplicate items being added by
bulk actions select all (#182007)\n\n## Summary\r\n\r\nRelated issue:
#181972\r\n\r\nWhen the useBulkActions hook in the alerts table is used
with the\r\noptionally defined at registration time hook
useBulkActionsConfig, and\r\nthat hook returns a stable array, the hook
calls a function that mutates\r\nthis array directly, which causes
duplicate items to appear in the alert\r\ncontext menu. This is due to
the useBulkActions hook using the bulk\r\nactions state via context, and
so runs every time a user selects a\r\nrow/all rows. Doing this via
filter might not be the most performant\r\nway, but in order to have
everything referentially stable, I think the\r\narguments to
useBulkActions/useBulkUntrackActions should be changed a\r\nbit, but
that can be a discussion for another time. The test case added\r\nbelow
will currently fail on
main/8.14.\r\n\r\nBefore:\r\n\r\n![bulk_actions_with_dupe](https://github.com/elastic/kibana/assets/56408403/7f730bb9-fcb2-4a8e-93f8-3e08523e3a34)\r\n\r\n\r\nAfter:\r\n\r\n![bulk_actions_no_dupe](https://github.com/elastic/kibana/assets/56408403/01f76c6b-59fb-459f-8950-1fc1fe8dfe12)\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"8a1d2950fa16f8cb50a21cb793640a290b54032c","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:ResponseOps","Team:Threat
Hunting:Investigations","v8.14.0","v8.15.0"],"title":"[Security
Solution] [Alerts Table] [RAC] Prevent duplicate items being added by
bulk actions select
all","number":182007,"url":"#182007
Solution] [Alerts Table] [RAC] Prevent duplicate items being added by
bulk actions select all (#182007)\n\n## Summary\r\n\r\nRelated issue:
#181972\r\n\r\nWhen the useBulkActions hook in the alerts table is used
with the\r\noptionally defined at registration time hook
useBulkActionsConfig, and\r\nthat hook returns a stable array, the hook
calls a function that mutates\r\nthis array directly, which causes
duplicate items to appear in the alert\r\ncontext menu. This is due to
the useBulkActions hook using the bulk\r\nactions state via context, and
so runs every time a user selects a\r\nrow/all rows. Doing this via
filter might not be the most performant\r\nway, but in order to have
everything referentially stable, I think the\r\narguments to
useBulkActions/useBulkUntrackActions should be changed a\r\nbit, but
that can be a discussion for another time. The test case added\r\nbelow
will currently fail on
main/8.14.\r\n\r\nBefore:\r\n\r\n![bulk_actions_with_dupe](https://github.com/elastic/kibana/assets/56408403/7f730bb9-fcb2-4a8e-93f8-3e08523e3a34)\r\n\r\n\r\nAfter:\r\n\r\n![bulk_actions_no_dupe](https://github.com/elastic/kibana/assets/56408403/01f76c6b-59fb-459f-8950-1fc1fe8dfe12)\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"8a1d2950fa16f8cb50a21cb793640a290b54032c"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"8.14","label":"v8.14.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"#182007
Solution] [Alerts Table] [RAC] Prevent duplicate items being added by
bulk actions select all (#182007)\n\n## Summary\r\n\r\nRelated issue:
#181972\r\n\r\nWhen the useBulkActions hook in the alerts table is used
with the\r\noptionally defined at registration time hook
useBulkActionsConfig, and\r\nthat hook returns a stable array, the hook
calls a function that mutates\r\nthis array directly, which causes
duplicate items to appear in the alert\r\ncontext menu. This is due to
the useBulkActions hook using the bulk\r\nactions state via context, and
so runs every time a user selects a\r\nrow/all rows. Doing this via
filter might not be the most performant\r\nway, but in order to have
everything referentially stable, I think the\r\narguments to
useBulkActions/useBulkUntrackActions should be changed a\r\nbit, but
that can be a discussion for another time. The test case added\r\nbelow
will currently fail on
main/8.14.\r\n\r\nBefore:\r\n\r\n![bulk_actions_with_dupe](https://github.com/elastic/kibana/assets/56408403/7f730bb9-fcb2-4a8e-93f8-3e08523e3a34)\r\n\r\n\r\nAfter:\r\n\r\n![bulk_actions_no_dupe](https://github.com/elastic/kibana/assets/56408403/01f76c6b-59fb-459f-8950-1fc1fe8dfe12)\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"8a1d2950fa16f8cb50a21cb793640a290b54032c"}}]}]
BACKPORT-->

Co-authored-by: Kevin Qualters <56408403+kqualters-elastic@users.noreply.github.com>
  • Loading branch information
kibanamachine and kqualters-elastic committed Apr 30, 2024
1 parent 5cefbfa commit 1fc22a2
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 1fc22a2

Please sign in to comment.