Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Cases] Fix bug in cases bulk action in the alerts table #160526

Merged
merged 1 commit into from Jun 26, 2023
Merged

[Cases] Fix bug in cases bulk action in the alerts table #160526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions ...cases/public/components/all_cases/selector_modal/use_cases_add_to_existing_case_modal.tsx
View file
Open in desktop
Expand Up @@ -64,6 +64,7 @@ export const useCasesAddToExistingCaseModal = (props: AddToExistingCaseModalProp
getAttachments?: ({ theCase }: { theCase?: CaseUI }) => CaseAttachmentsWithoutOwner
) => {
const attachments = getAttachments?.({ theCase }) ?? [];

// when the case is undefined in the modal
// the user clicked "create new case"
if (theCase === undefined) {
Expand Down
51 changes: 51 additions & 0 deletions ...ggers_actions_ui/public/application/sections/alerts_table/hooks/use_bulk_actions.test.tsx
View file
Open in desktop
Expand Up @@ -43,9 +43,11 @@ describe('bulk action hooks', () => {
const refresh = jest.fn();
const clearSelection = jest.fn();
const openNewCase = jest.fn();

const openExistingCase = jest.fn().mockImplementation(({ getAttachments }) => {
getAttachments({ theCase: { id: caseId } });
});

mockCaseService.helpers.canUseCases = jest.fn().mockReturnValue({ create: true, read: true });
mockCaseService.ui.getCasesContext = jest.fn().mockReturnValue(() => 'Cases context');

Expand Down Expand Up @@ -124,6 +126,55 @@ describe('bulk action hooks', () => {
expect(openExistingCase).toHaveBeenCalled();
});

it('should open the flyout from the case modal', async () => {
Copy link
Contributor

@adcoelho adcoelho Jun 26, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An e2e test for this would also be nice if there is time. Looked a bit for them but dunno if there even are any regarding bulk actions in the alerts table.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have. Do you mind if we do it on another PR? I have this issue to track it #160240.

openExistingCase.mockImplementationOnce(({ getAttachments }) => {
getAttachments({ theCase: undefined });
});

const alerts = [
{
_id: 'alert0',
_index: 'idx0',
data: [
{
field: 'kibana.alert.case_ids',
value: [caseId],
},
],
ecs: {
_id: 'alert0',
_index: 'idx0',
},
},
{
_id: 'alert1',
_index: 'idx1',
data: [
{
field: 'kibana.alert.case_ids',
value: ['test-case-2'],
},
],
ecs: {
_id: 'alert1',
_index: 'idx1',
},
},
];

const { result } = renderHook(
() => useBulkAddToCaseActions({ casesConfig, refresh, clearSelection }),
{
wrapper: appMockRender.AppWrapper,
}
);

// @ts-expect-error: cases do not need all arguments
result.current[1].onClick(alerts);

expect(mockCaseService.helpers.groupAlertsByRule).toHaveBeenCalledWith(alerts);
});

it('should remove alerts that are already attached to the case', async () => {
const { result } = renderHook(
() => useBulkAddToCaseActions({ casesConfig, refresh, clearSelection }),
Expand Down
4 changes: 4 additions & 0 deletions ...ns/triggers_actions_ui/public/application/sections/alerts_table/hooks/use_bulk_actions.ts
View file
Open in desktop
Expand Up @@ -146,6 +146,10 @@ export const useBulkAddToCaseActions = ({
onClick: (alerts?: TimelineItem[]) => {
selectCaseModal.open({
getAttachments: ({ theCase }) => {
if (theCase == null) {
return alerts ? casesService?.helpers.groupAlertsByRule(alerts) ?? [] : [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can alerts actually be missing? If this is triggered via the bulk actions it shouldn't ever, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember why but I followed the pattern used in the other action. Maybe there is a reason. I agree with you. It shouldn't be undefined.

}

return getCaseAttachments({
alerts,
caseId: theCase.id,
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/triggers_actions_ui/public/application/sections/alerts_table/types.ts
View file
Open in desktop
Expand Up @@ -48,7 +48,7 @@ type UseCasesAddToExistingCaseModal = (props?: Record<string, unknown>) => {
open: ({
getAttachments,
}: {
getAttachments: ({ theCase }: { theCase: { id: string } }) => any[];
getAttachments: ({ theCase }: { theCase?: { id: string } }) => any[];
}) => void;
close: () => void;
};
Expand Down