Skip to content

Commit

Permalink
[Cases] Remove case info from alerts when deleting an alert attachment (
Browse files Browse the repository at this point in the history
#154024)

## Summary

Users can remove alerts from a case by deleting the whole alert
attachment. This PR removes the case id from the alerts when deleting an
attachment of type alert. It does not remove the case info from all
alerts attached to a case when deleting a case. It also fixes a bug
where the success toaster will not show when deleting an attachment.

Related: #146864,
#140800

## Testing

1. Create a case and attach some alerts to the case.
2. Verify that the alerts table (in security or in o11y) shows the case
the alert is attached to. You can enable the cases column by pressing
"Fields", searching for "Cases", and then selecting the field.
3. Go to the case and find the alerts' user activity.
4. Press the `...` and press "Remove alerts(s)"
5. Go back to the alert table and verify that the case is not shown in
the Cases column for each alert.

Please check that when you remove alert(s), attachments (ml, etc), and
comments you get a success toaster with the correct text.

### Checklist

Delete any items that are not applicable to this PR.

- [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

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
  • Loading branch information
cnasikas authored Apr 1, 2023
1 parent 41fb8f3 commit 3a6f721
Show file tree
Hide file tree
Showing 22 changed files with 1,041 additions and 53 deletions.
59 changes: 59 additions & 0 deletions x-pack/plugins/cases/common/utils/attachments.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import type { CommentAttributes } from '../api';
import { CommentType } from '../api';
import {
isCommentRequestTypeExternalReference,
isCommentRequestTypePersistableState,
} from './attachments';

describe('attachments utils', () => {
describe('isCommentRequestTypeExternalReference', () => {
const externalReferenceAttachment = {
type: CommentType.externalReference as const,
} as CommentAttributes;

const commentTypeWithoutAlert = Object.values(CommentType).filter(
(type) => type !== CommentType.externalReference
);

it('returns false for type: externalReference', () => {
expect(isCommentRequestTypeExternalReference(externalReferenceAttachment)).toBe(true);
});

it.each(commentTypeWithoutAlert)('returns false for type: %s', (type) => {
const attachment = {
type,
} as CommentAttributes;

expect(isCommentRequestTypeExternalReference(attachment)).toBe(false);
});
});

describe('isCommentRequestTypePersistableState', () => {
const persistableStateAttachment = {
type: CommentType.persistableState as const,
} as CommentAttributes;

const commentTypeWithoutAlert = Object.values(CommentType).filter(
(type) => type !== CommentType.persistableState
);

it('returns false for type: persistableState', () => {
expect(isCommentRequestTypePersistableState(persistableStateAttachment)).toBe(true);
});

it.each(commentTypeWithoutAlert)('returns false for type: %s', (type) => {
const attachment = {
type,
} as CommentAttributes;

expect(isCommentRequestTypePersistableState(attachment)).toBe(false);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { ShowAlertTableLink } from './show_alert_table_link';
import { HoverableUserWithAvatarResolver } from '../../user_profiles/hoverable_user_with_avatar_resolver';
import { UserActionContentToolbar } from '../content_toolbar';
import { AlertPropertyActions } from '../property_actions/alert_property_actions';
import { DELETE_ALERTS_SUCCESS_TITLE } from './translations';

type BuilderArgs = Pick<
UserActionBuilderArgs,
Expand Down Expand Up @@ -88,7 +89,7 @@ const getSingleAlertUserAction = ({
/>
</EuiFlexItem>
<AlertPropertyActions
onDelete={() => handleDeleteComment(comment.id)}
onDelete={() => handleDeleteComment(comment.id, DELETE_ALERTS_SUCCESS_TITLE(1))}
isLoading={loadingCommentIds.includes(comment.id)}
totalAlerts={1}
/>
Expand Down Expand Up @@ -142,7 +143,9 @@ const getMultipleAlertsUserAction = ({
<ShowAlertTableLink />
</EuiFlexItem>
<AlertPropertyActions
onDelete={() => handleDeleteComment(comment.id)}
onDelete={() =>
handleDeleteComment(comment.id, DELETE_ALERTS_SUCCESS_TITLE(totalAlerts))
}
isLoading={loadingCommentIds.includes(comment.id)}
totalAlerts={totalAlerts}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,10 @@ describe('createCommentUserActionBuilder', () => {
await deleteAttachment(result, 'trash', 'Delete');

await waitFor(() => {
expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith('basic-comment-id');
expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith(
'basic-comment-id',
'Deleted comment'
);
});
});

Expand Down Expand Up @@ -319,7 +322,10 @@ describe('createCommentUserActionBuilder', () => {
await deleteAttachment(res, 'minusInCircle', 'Remove');

await waitFor(() => {
expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith('alert-comment-id');
expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith(
'alert-comment-id',
'Deleted one alert'
);
});
});

Expand Down Expand Up @@ -414,7 +420,10 @@ describe('createCommentUserActionBuilder', () => {
await deleteAttachment(res, 'minusInCircle', 'Remove');

await waitFor(() => {
expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith('alert-comment-id');
expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith(
'alert-comment-id',
'Deleted 2 alerts'
);
});
});

Expand Down Expand Up @@ -590,7 +599,8 @@ describe('createCommentUserActionBuilder', () => {

await waitFor(() => {
expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith(
'external-reference-comment-id'
'external-reference-comment-id',
'Deleted attachment'
);
});
});
Expand Down Expand Up @@ -761,7 +771,8 @@ describe('createCommentUserActionBuilder', () => {

await waitFor(() => {
expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith(
'persistable-state-comment-id'
'persistable-state-comment-id',
'Deleted attachment'
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ import type { CommentResponse } from '../../../../common/api';
import type { UserActionBuilder, UserActionBuilderArgs } from '../types';
import { UserActionTimestamp } from '../timestamp';
import type { SnakeToCamelCase } from '../../../../common/types';
import { ATTACHMENT_NOT_REGISTERED_ERROR, DEFAULT_EVENT_ATTACHMENT_TITLE } from './translations';
import {
ATTACHMENT_NOT_REGISTERED_ERROR,
DEFAULT_EVENT_ATTACHMENT_TITLE,
DELETE_REGISTERED_ATTACHMENT,
} from './translations';
import { UserActionContentToolbar } from '../content_toolbar';
import { HoverableUserWithAvatarResolver } from '../../user_profiles/hoverable_user_with_avatar_resolver';
import { RegisteredAttachmentsPropertyActions } from '../property_actions/registered_attachments_property_actions';
Expand Down Expand Up @@ -127,7 +131,7 @@ export const createRegisteredAttachmentUserActionBuilder = <
{attachmentViewObject.actions}
<RegisteredAttachmentsPropertyActions
isLoading={isLoading}
onDelete={() => handleDeleteComment(comment.id)}
onDelete={() => handleDeleteComment(comment.id, DELETE_REGISTERED_ATTACHMENT)}
/>
</UserActionContentToolbar>
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,24 @@ export const UNSAVED_DRAFT_COMMENT = i18n.translate(
defaultMessage: 'You have unsaved edits for this comment',
}
);

export const DELETE_ALERTS_SUCCESS_TITLE = (totalAlerts: number) =>
i18n.translate('xpack.cases.userActions.attachments.alerts.successToasterTitle', {
defaultMessage:
'Deleted {totalAlerts, plural, =1 {one} other {{totalAlerts}}} {totalAlerts, plural, =1 {alert} other {alerts}}',
values: { totalAlerts },
});

export const DELETE_COMMENT_SUCCESS_TITLE = i18n.translate(
'xpack.cases.userActions.attachments.comment.successToasterTitle',
{
defaultMessage: 'Deleted comment',
}
);

export const DELETE_REGISTERED_ATTACHMENT = i18n.translate(
'xpack.cases.userActions.attachments.registeredAttachment.successToasterTitle',
{
defaultMessage: 'Deleted attachment',
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export const createUserAttachmentUserActionBuilder = ({
isLoading={isLoading}
commentContent={comment.comment}
onEdit={() => handleManageMarkdownEditId(comment.id)}
onDelete={() => handleDeleteComment(comment.id)}
onDelete={() => handleDeleteComment(comment.id, i18n.DELETE_COMMENT_SUCCESS_TITLE)}
onQuote={() => handleManageQuote(comment.comment)}
/>
</UserActionContentToolbar>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export interface UserActionBuilderArgs {
handleOutlineComment: (id: string) => void;
handleManageMarkdownEditId: (id: string) => void;
handleSaveComment: ({ id, version }: { id: string; version: string }, content: string) => void;
handleDeleteComment: (id: string) => void;
handleDeleteComment: (id: string, successToasterTitle: string) => void;
handleManageQuote: (quote: string) => void;
onShowAlertDetails: (alertId: string, index: string) => void;
getRuleDetailsHref?: RuleDetailsNavigation['href'];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ export const useUserActionsHandler = (): UseUserActionsHandler => {
);

const handleDeleteComment = useCallback(
(id: string) => {
deleteComment({ caseId, commentId: id });
(id: string, successToasterTitle: string) => {
deleteComment({ caseId, commentId: id, successToasterTitle });
},
[caseId, deleteComment]
);
Expand Down
68 changes: 42 additions & 26 deletions x-pack/plugins/cases/public/containers/use_delete_comment.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,38 @@
* 2.0.
*/

import React from 'react';
import { act, renderHook } from '@testing-library/react-hooks';
import type { UseDeleteComment } from './use_delete_comment';
import { useDeleteComment } from './use_delete_comment';
import * as api from './api';
import { basicCaseId } from './mock';
import { TestProviders } from '../common/mock';
import { useRefreshCaseViewPage } from '../components/case_view/use_on_refresh_case_view_page';
import { useToasts } from '../common/lib/kibana';
import type { AppMockRenderer } from '../common/mock';
import { createAppMockRenderer } from '../common/mock';

jest.mock('../common/lib/kibana');
jest.mock('./api');
jest.mock('../components/case_view/use_on_refresh_case_view_page');

const commentId = 'ab124';

const wrapper: React.FC<string> = ({ children }) => <TestProviders>{children}</TestProviders>;
const successToasterTitle = 'Deleted';

describe('useDeleteComment', () => {
const addSuccess = jest.fn();
const addError = jest.fn();

(useToasts as jest.Mock).mockReturnValue({ addSuccess, addError });

let appMockRender: AppMockRenderer;

beforeEach(() => {
appMockRender = createAppMockRenderer();
jest.clearAllMocks();
});

it('init', async () => {
const { result } = renderHook<string, UseDeleteComment>(() => useDeleteComment(), {
wrapper,
const { result } = renderHook(() => useDeleteComment(), {
wrapper: appMockRender.AppWrapper,
});

expect(result.current).toBeTruthy();
Expand All @@ -44,17 +45,15 @@ describe('useDeleteComment', () => {
it('calls deleteComment with correct arguments - case', async () => {
const spyOnDeleteComment = jest.spyOn(api, 'deleteComment');

const { waitForNextUpdate, result } = renderHook<string, UseDeleteComment>(
() => useDeleteComment(),
{
wrapper,
}
);
const { waitForNextUpdate, result } = renderHook(() => useDeleteComment(), {
wrapper: appMockRender.AppWrapper,
});

act(() => {
result.current.mutate({
caseId: basicCaseId,
commentId,
successToasterTitle,
});
});

Expand All @@ -68,37 +67,54 @@ describe('useDeleteComment', () => {
});

it('refreshes the case page view after delete', async () => {
const { waitForNextUpdate, result } = renderHook<string, UseDeleteComment>(
() => useDeleteComment(),
{
wrapper,
}
);
const { waitForNextUpdate, result } = renderHook(() => useDeleteComment(), {
wrapper: appMockRender.AppWrapper,
});

result.current.mutate({
caseId: basicCaseId,
commentId,
successToasterTitle,
});

await waitForNextUpdate();

expect(useRefreshCaseViewPage()).toBeCalled();
});

it('shows a success toaster correctly', async () => {
const { waitForNextUpdate, result } = renderHook(() => useDeleteComment(), {
wrapper: appMockRender.AppWrapper,
});

act(() => {
result.current.mutate({
caseId: basicCaseId,
commentId,
successToasterTitle,
});
});

await waitForNextUpdate();

expect(addSuccess).toHaveBeenCalledWith({
title: 'Deleted',
className: 'eui-textBreakWord',
});
});

it('sets isError when fails to delete a case', async () => {
const spyOnDeleteComment = jest.spyOn(api, 'deleteComment');
spyOnDeleteComment.mockRejectedValue(new Error('Not possible :O'));
spyOnDeleteComment.mockRejectedValue(new Error('Error'));

const { waitForNextUpdate, result } = renderHook<string, UseDeleteComment>(
() => useDeleteComment(),
{
wrapper,
}
);
const { waitForNextUpdate, result } = renderHook(() => useDeleteComment(), {
wrapper: appMockRender.AppWrapper,
});

result.current.mutate({
caseId: basicCaseId,
commentId,
successToasterTitle,
});

await waitForNextUpdate();
Expand Down
6 changes: 4 additions & 2 deletions x-pack/plugins/cases/public/containers/use_delete_comment.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ import * as i18n from './translations';
interface MutationArgs {
caseId: string;
commentId: string;
successToasterTitle: string;
}

export const useDeleteComment = () => {
const { showErrorToast } = useCasesToast();
const { showErrorToast, showSuccessToast } = useCasesToast();
const refreshCaseViewPage = useRefreshCaseViewPage();

return useMutation(
Expand All @@ -29,7 +30,8 @@ export const useDeleteComment = () => {
},
{
mutationKey: casesMutationsKeys.deleteComment,
onSuccess: () => {
onSuccess: (_, { successToasterTitle }) => {
showSuccessToast(successToasterTitle);
refreshCaseViewPage();
},
onError: (error: ServerError) => {
Expand Down
5 changes: 5 additions & 0 deletions x-pack/plugins/cases/server/client/alerts/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,8 @@ export interface UpdateAlertCasesRequest {
alerts: AlertInfo[];
caseIds: string[];
}

export interface RemoveAlertsFromCaseRequest {
alerts: AlertInfo[];
caseId: string;
}
Loading

0 comments on commit 3a6f721

Please sign in to comment.