github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-24 09:48:58 -04:00
Code Issues Projects Releases Packages Wiki Activity

[Security Solution][Notes] - add delete note to flyout notes tab (#186843)

Browse source
This commit is contained in:
Philippe Oberti 2024-06-25 12:37:20 -05:00 • committed by GitHub
parent 7ce69fc52c
commit d5b0ddda62
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 354 additions and 42 deletions
Download patch file Download diff file Expand all files Collapse all files

2
x-pack/plugins/security_solution/public/common/mock/global_state.ts
View file

@ -521,10 +521,12 @@ export const mockGlobalState: State = {
status: {
fetchNotesByDocumentId: ReqStatus.Idle,
createNote: ReqStatus.Idle,
deleteNote: ReqStatus.Idle,
},
error: {
fetchNotesByDocumentId: null,
createNote: null,
deleteNote: null,
},
},
};

31
x-pack/plugins/security_solution/public/flyout/document_details/left/components/add_note.test.tsx
View file

@ -52,9 +52,16 @@ describe('AddNote', () => {
});
it('should render the add note button in loading state while creating a new note', () => {
const state = { ...mockGlobalState };
state.notes.status.createNote = ReqStatus.Loading;
const store = createMockStore(state);
const store = createMockStore({
...mockGlobalState,
notes: {
...mockGlobalState.notes,
status: {
...mockGlobalState.notes.status,
createNote: ReqStatus.Loading,
},
},
});
const { container } = render(
<TestProviders store={store}>
@ -66,10 +73,20 @@ describe('AddNote', () => {
});
it('should render error toast if create a note fails', () => {
const state = { ...mockGlobalState };
state.notes.status.createNote = ReqStatus.Failed;
state.notes.error.createNote = { type: 'http', status: 500 };
const store = createMockStore(state);
const store = createMockStore({
...mockGlobalState,
notes: {
...mockGlobalState.notes,
status: {
...mockGlobalState.notes.status,
createNote: ReqStatus.Failed,
},
error: {
...mockGlobalState.notes.error,
createNote: { type: 'http', status: 500 },
},
},
});
render(
<TestProviders store={store}>

139
x-pack/plugins/security_solution/public/flyout/document_details/left/components/notes_list.test.tsx
View file

@ -7,9 +7,14 @@
import { render } from '@testing-library/react';
import React from 'react';
import { ADD_NOTE_LOADING_TEST_ID, NOTES_COMMENT_TEST_ID, NOTES_LOADING_TEST_ID } from './test_ids';
import {
ADD_NOTE_LOADING_TEST_ID,
DELETE_NOTE_BUTTON_TEST_ID,
NOTES_COMMENT_TEST_ID,
NOTES_LOADING_TEST_ID,
} from './test_ids';
import { createMockStore, mockGlobalState, TestProviders } from '../../../../common/mock';
import { FETCH_NOTES_ERROR, NO_NOTES, NotesList } from './notes_list';
import { DELETE_NOTE_ERROR, FETCH_NOTES_ERROR, NO_NOTES, NotesList } from './notes_list';
import { ReqStatus } from '../../../../notes/store/notes.slice';
const mockAddError = jest.fn();
@ -19,6 +24,15 @@ jest.mock('../../../../common/hooks/use_app_toasts', () => ({
}),
}));
const mockDispatch = jest.fn();
jest.mock('react-redux', () => {
const original = jest.requireActual('react-redux');
return {
...original,
useDispatch: () => mockDispatch,
};
});
const renderNotesList = () =>
render(
<TestProviders>
@ -31,12 +45,20 @@ describe('NotesList', () => {
const { getByTestId, getByText } = renderNotesList();
expect(getByTestId(`${NOTES_COMMENT_TEST_ID}-0`)).toBeInTheDocument();
expect(getByText('note-1')).toBeInTheDocument();
expect(getByTestId(`${DELETE_NOTE_BUTTON_TEST_ID}-0`)).toBeInTheDocument();
});
it('should render loading spinner if notes are being fetched', () => {
const state = { ...mockGlobalState };
state.notes.status.fetchNotesByDocumentId = ReqStatus.Loading;
const store = createMockStore(state);
const store = createMockStore({
...mockGlobalState,
notes: {
...mockGlobalState.notes,
status: {
...mockGlobalState.notes.status,
fetchNotesByDocumentId: ReqStatus.Loading,
},
},
});
const { getByTestId } = render(
<TestProviders store={store}>
@ -48,9 +70,16 @@ describe('NotesList', () => {
});
it('should render no data message if no notes are present', () => {
const state = { ...mockGlobalState };
state.notes.status.fetchNotesByDocumentId = ReqStatus.Succeeded;
const store = createMockStore(state);
const store = createMockStore({
...mockGlobalState,
notes: {
...mockGlobalState.notes,
status: {
...mockGlobalState.notes.status,
fetchNotesByDocumentId: ReqStatus.Succeeded,
},
},
});
const { getByText } = render(
<TestProviders store={store}>
@ -62,10 +91,20 @@ describe('NotesList', () => {
});
it('should render error toast if fetching notes fails', () => {
const state = { ...mockGlobalState };
state.notes.status.fetchNotesByDocumentId = ReqStatus.Failed;
state.notes.error.fetchNotesByDocumentId = { type: 'http', status: 500 };
const store = createMockStore(state);
const store = createMockStore({
...mockGlobalState,
notes: {
...mockGlobalState.notes,
status: {
...mockGlobalState.notes.status,
fetchNotesByDocumentId: ReqStatus.Failed,
},
error: {
...mockGlobalState.notes.error,
fetchNotesByDocumentId: { type: 'http', status: 500 },
},
},
});
render(
<TestProviders store={store}>
@ -78,10 +117,17 @@ describe('NotesList', () => {
});
});
it('should render create loading when user create a new note', () => {
const state = { ...mockGlobalState };
state.notes.status.createNote = ReqStatus.Loading;
const store = createMockStore(state);
it('should render create loading when user creates a new note', () => {
const store = createMockStore({
...mockGlobalState,
notes: {
...mockGlobalState.notes,
status: {
...mockGlobalState.notes.status,
createNote: ReqStatus.Loading,
},
},
});
const { getByTestId } = render(
<TestProviders store={store}>
@ -91,4 +137,65 @@ describe('NotesList', () => {
expect(getByTestId(ADD_NOTE_LOADING_TEST_ID)).toBeInTheDocument();
});
it('should dispatch delete action when user deletes a new note', () => {
const { getByTestId } = renderNotesList();
const deleteIcon = getByTestId(`${DELETE_NOTE_BUTTON_TEST_ID}-0`);
expect(deleteIcon).toBeInTheDocument();
expect(deleteIcon).not.toHaveAttribute('disabled');
deleteIcon.click();
expect(mockDispatch).toHaveBeenCalled();
});
it('should have delete icons disabled and show spinner if a new note is being deleted', () => {
const store = createMockStore({
...mockGlobalState,
notes: {
...mockGlobalState.notes,
status: {
...mockGlobalState.notes.status,
deleteNote: ReqStatus.Loading,
},
},
});
const { getByTestId } = render(
<TestProviders store={store}>
<NotesList eventId={'event-id'} />
</TestProviders>
);
expect(getByTestId(`${DELETE_NOTE_BUTTON_TEST_ID}-0`)).toHaveAttribute('disabled');
});
it('should render error toast if deleting a note fails', () => {
const store = createMockStore({
...mockGlobalState,
notes: {
...mockGlobalState.notes,
status: {
...mockGlobalState.notes.status,
deleteNote: ReqStatus.Failed,
},
error: {
...mockGlobalState.notes.error,
deleteNote: { type: 'http', status: 500 },
},
},
});
render(
<TestProviders store={store}>
<NotesList eventId={'event-id'} />
</TestProviders>
);
expect(mockAddError).toHaveBeenCalledWith(null, {
title: DELETE_NOTE_ERROR,
});
});
});

65
x-pack/plugins/security_solution/public/flyout/document_details/left/components/notes_list.tsx
View file

@ -5,17 +5,31 @@
* 2.0.
*/
import React, { memo, useEffect } from 'react';
import { EuiComment, EuiCommentList, EuiLoadingElastic, EuiMarkdownFormat } from '@elastic/eui';
import { useSelector } from 'react-redux';
import React, { memo, useCallback, useEffect, useState } from 'react';
import {
EuiButtonIcon,
EuiComment,
EuiCommentList,
EuiLoadingElastic,
EuiMarkdownFormat,
} from '@elastic/eui';
import { useDispatch, useSelector } from 'react-redux';
import { FormattedRelative } from '@kbn/i18n-react';
import { i18n } from '@kbn/i18n';
import { ADD_NOTE_LOADING_TEST_ID, NOTES_COMMENT_TEST_ID, NOTES_LOADING_TEST_ID } from './test_ids';
import {
ADD_NOTE_LOADING_TEST_ID,
DELETE_NOTE_BUTTON_TEST_ID,
NOTES_COMMENT_TEST_ID,
NOTES_LOADING_TEST_ID,
} from './test_ids';
import type { State } from '../../../../common/store';
import type { Note } from '../../../../../common/api/timeline';
import {
deleteNote,
ReqStatus,
selectCreateNoteStatus,
selectDeleteNoteError,
selectDeleteNoteStatus,
selectFetchNotesByDocumentIdError,
selectFetchNotesByDocumentIdStatus,
selectNotesByDocumentId,
@ -34,6 +48,15 @@ export const FETCH_NOTES_ERROR = i18n.translate(
export const NO_NOTES = i18n.translate('xpack.securitySolution.notes.noNotesLabel', {
defaultMessage: 'No notes have been created for this document',
});
export const DELETE_NOTE = i18n.translate('xpack.securitySolution.notes.deleteNoteLabel', {
defaultMessage: 'Delete note',
});
export const DELETE_NOTE_ERROR = i18n.translate(
'xpack.securitySolution.notes.deleteNoteErrorLabel',
{
defaultMessage: 'Error deleting note',
}
);
export interface NotesListProps {
/**
@ -45,9 +68,11 @@ export interface NotesListProps {
/**
* Renders a list of notes for the document.
* If a note belongs to a timeline, a timeline icon will be shown the top right corner.
* Also, a delete icon is shown in the top right corner to delete a note.
* When a note is being created, the component renders a loading spinner when the new note is about to be added.
*/
export const NotesList = memo(({ eventId }: NotesListProps) => {
const dispatch = useDispatch();
const { addError: addErrorToast } = useAppToasts();
const fetchStatus = useSelector((state: State) => selectFetchNotesByDocumentIdStatus(state));
@ -56,6 +81,18 @@ export const NotesList = memo(({ eventId }: NotesListProps) => {
const createStatus = useSelector((state: State) => selectCreateNoteStatus(state));
const deleteStatus = useSelector((state: State) => selectDeleteNoteStatus(state));
const deleteError = useSelector((state: State) => selectDeleteNoteError(state));
const [deletingNoteId, setDeletingNoteId] = useState('');
const deleteNoteFc = useCallback(
(noteId: string) => {
setDeletingNoteId(noteId);
dispatch(deleteNote({ id: noteId }));
},
[dispatch]
);
useEffect(() => {
if (fetchStatus === ReqStatus.Failed && fetchError) {
addErrorToast(null, {
@ -64,6 +101,14 @@ export const NotesList = memo(({ eventId }: NotesListProps) => {
}
}, [addErrorToast, fetchError, fetchStatus]);
useEffect(() => {
if (deleteStatus === ReqStatus.Failed && deleteError) {
addErrorToast(null, {
title: DELETE_NOTE_ERROR,
});
}
}, [addErrorToast, deleteError, deleteStatus]);
if (fetchStatus === ReqStatus.Loading) {
return <EuiLoadingElastic data-test-subj={NOTES_LOADING_TEST_ID} size="xxl" />;
}
@ -81,6 +126,18 @@ export const NotesList = memo(({ eventId }: NotesListProps) => {
username={note.createdBy}
timestamp={<>{note.created && <FormattedRelative value={new Date(note.created)} />}</>}
event={ADDED_A_NOTE}
actions={
<EuiButtonIcon
data-test-subj={`${DELETE_NOTE_BUTTON_TEST_ID}-${index}`}
title={DELETE_NOTE}
aria-label={DELETE_NOTE}
color="text"
iconType="trash"
onClick={() => deleteNoteFc(note.noteId)}
disabled={deletingNoteId !== note.noteId && deleteStatus === ReqStatus.Loading}
isLoading={deletingNoteId === note.noteId && deleteStatus === ReqStatus.Loading}
/>
}
>
<EuiMarkdownFormat textSize="s">{note.note || ''}</EuiMarkdownFormat>
</EuiComment>

1
x-pack/plugins/security_solution/public/flyout/document_details/left/components/test_ids.ts
View file

@ -97,3 +97,4 @@ export const NOTES_COMMENT_TEST_ID = `${PREFIX}NotesComment` as const;
export const ADD_NOTE_LOADING_TEST_ID = `${PREFIX}AddNotesLoading` as const;
export const ADD_NOTE_MARKDOWN_TEST_ID = `${PREFIX}AddNotesMarkdown` as const;
export const ADD_NOTE_BUTTON_TEST_ID = `${PREFIX}AddNotesButton` as const;
export const DELETE_NOTE_BUTTON_TEST_ID = `${PREFIX}DeleteNotesButton` as const;

11
x-pack/plugins/security_solution/public/notes/api/api.ts
View file

@ -54,3 +54,14 @@ export const generateNoteMock = (documentId: string) => ({
updated: new Date().getTime(),
updatedBy: 'elastic',
});
/**
* Deletes a note
*/
export const deleteNote = async (noteId: string) => {
const response = await KibanaServices.get().http.delete<{ data: unknown }>(NOTE_URL, {
body: JSON.stringify({ noteId }),
version: '2023-10-31',
});
return response;
};

118
x-pack/plugins/security_solution/public/notes/store/notes.slice.test.ts
View file

@ -7,6 +7,7 @@
import {
createNote,
deleteNote,
fetchNotesByDocumentId,
initialNotesState,
notesReducer,
@ -14,6 +15,8 @@ import {
selectAllNotes,
selectCreateNoteError,
selectCreateNoteStatus,
selectDeleteNoteError,
selectDeleteNoteStatus,
selectFetchNotesByDocumentIdError,
selectFetchNotesByDocumentIdStatus,
selectNoteById,
@ -31,8 +34,12 @@ const initialNonEmptyState = {
[mockNote.noteId]: mockNote,
},
ids: [mockNote.noteId],
status: { fetchNotesByDocumentId: ReqStatus.Idle, createNote: ReqStatus.Idle },
error: { fetchNotesByDocumentId: null, createNote: null },
status: {
fetchNotesByDocumentId: ReqStatus.Idle,
createNote: ReqStatus.Idle,
deleteNote: ReqStatus.Idle,
},
error: { fetchNotesByDocumentId: null, createNote: null, deleteNote: null },
};
describe('notesSlice', () => {
@ -41,13 +48,17 @@ describe('notesSlice', () => {
expect(notesReducer(initalEmptyState, { type: 'unknown' })).toEqual({
entities: {},
ids: [],
status: { fetchNotesByDocumentId: ReqStatus.Idle, createNote: ReqStatus.Idle },
error: { fetchNotesByDocumentId: null, createNote: null },
status: {
fetchNotesByDocumentId: ReqStatus.Idle,
createNote: ReqStatus.Idle,
deleteNote: ReqStatus.Idle,
},
error: { fetchNotesByDocumentId: null, createNote: null, deleteNote: null },
});
});
describe('fetchNotesByDocumentId', () => {
it('should set correct state when fetching notes by document id', () => {
it('should set correct status state when fetching notes by document id', () => {
const action = { type: fetchNotesByDocumentId.pending.type };
expect(notesReducer(initalEmptyState, action)).toEqual({
@ -56,8 +67,9 @@ describe('notesSlice', () => {
status: {
fetchNotesByDocumentId: ReqStatus.Loading,
createNote: ReqStatus.Idle,
deleteNote: ReqStatus.Idle,
},
error: { fetchNotesByDocumentId: null, createNote: null },
error: { fetchNotesByDocumentId: null, createNote: null, deleteNote: null },
});
});
@ -80,8 +92,9 @@ describe('notesSlice', () => {
status: {
fetchNotesByDocumentId: ReqStatus.Succeeded,
createNote: ReqStatus.Idle,
deleteNote: ReqStatus.Idle,
},
error: { fetchNotesByDocumentId: null, createNote: null },
error: { fetchNotesByDocumentId: null, createNote: null, deleteNote: null },
});
});
@ -105,12 +118,13 @@ describe('notesSlice', () => {
status: {
fetchNotesByDocumentId: ReqStatus.Succeeded,
createNote: ReqStatus.Idle,
deleteNote: ReqStatus.Idle,
},
error: { fetchNotesByDocumentId: null, createNote: null },
error: { fetchNotesByDocumentId: null, createNote: null, deleteNote: null },
});
});
it('should set correct state when error on fetch notes by document id', () => {
it('should set correct error state when failing to fetch notes by document id', () => {
const action = { type: fetchNotesByDocumentId.rejected.type, error: 'error' };
expect(notesReducer(initalEmptyState, action)).toEqual({
@ -119,14 +133,19 @@ describe('notesSlice', () => {
status: {
fetchNotesByDocumentId: ReqStatus.Failed,
createNote: ReqStatus.Idle,
deleteNote: ReqStatus.Idle,
},
error: {
fetchNotesByDocumentId: 'error',
createNote: null,
deleteNote: null,
},
error: { fetchNotesByDocumentId: 'error', createNote: null },
});
});
});
describe('createNote', () => {
it('should set correct state when creating a note by document id', () => {
it('should set correct status state when creating a note by document id', () => {
const action = { type: createNote.pending.type };
expect(notesReducer(initalEmptyState, action)).toEqual({
@ -135,8 +154,9 @@ describe('notesSlice', () => {
status: {
fetchNotesByDocumentId: ReqStatus.Idle,
createNote: ReqStatus.Loading,
deleteNote: ReqStatus.Idle,
},
error: { fetchNotesByDocumentId: null, createNote: null },
error: { fetchNotesByDocumentId: null, createNote: null, deleteNote: null },
});
});
@ -159,12 +179,13 @@ describe('notesSlice', () => {
status: {
fetchNotesByDocumentId: ReqStatus.Idle,
createNote: ReqStatus.Succeeded,
deleteNote: ReqStatus.Idle,
},
error: { fetchNotesByDocumentId: null, createNote: null },
error: { fetchNotesByDocumentId: null, createNote: null, deleteNote: null },
});
});
it('should set correct state when error on create a note by document id', () => {
it('should set correct error state when failing to create a note by document id', () => {
const action = { type: createNote.rejected.type, error: 'error' };
expect(notesReducer(initalEmptyState, action)).toEqual({
@ -173,8 +194,67 @@ describe('notesSlice', () => {
status: {
fetchNotesByDocumentId: ReqStatus.Idle,
createNote: ReqStatus.Failed,
deleteNote: ReqStatus.Idle,
},
error: {
fetchNotesByDocumentId: null,
createNote: 'error',
deleteNote: null,
},
});
});
});
describe('deleteNote', () => {
it('should set correct status state when deleting a note', () => {
const action = { type: deleteNote.pending.type };
expect(notesReducer(initalEmptyState, action)).toEqual({
entities: {},
ids: [],
status: {
fetchNotesByDocumentId: ReqStatus.Idle,
createNote: ReqStatus.Idle,
deleteNote: ReqStatus.Loading,
},
error: { fetchNotesByDocumentId: null, createNote: null, deleteNote: null },
});
});
it('should set correct state when success on deleting a note', () => {
const action = {
type: deleteNote.fulfilled.type,
payload: mockNote.noteId,
};
expect(notesReducer(initialNonEmptyState, action)).toEqual({
entities: {},
ids: [],
status: {
fetchNotesByDocumentId: ReqStatus.Idle,
createNote: ReqStatus.Idle,
deleteNote: ReqStatus.Succeeded,
},
error: { fetchNotesByDocumentId: null, createNote: null, deleteNote: null },
});
});
it('should set correct state when failing to create a note by document id', () => {
const action = { type: deleteNote.rejected.type, error: 'error' };
expect(notesReducer(initalEmptyState, action)).toEqual({
entities: {},
ids: [],
status: {
fetchNotesByDocumentId: ReqStatus.Idle,
createNote: ReqStatus.Idle,
deleteNote: ReqStatus.Failed,
},
error: {
fetchNotesByDocumentId: null,
createNote: null,
deleteNote: 'error',
},
error: { fetchNotesByDocumentId: null, createNote: 'error' },
});
});
});
@ -218,6 +298,14 @@ describe('notesSlice', () => {
expect(selectCreateNoteError(mockGlobalState)).toEqual(null);
});
it('should return delete note status', () => {
expect(selectDeleteNoteStatus(mockGlobalState)).toEqual(ReqStatus.Idle);
});
it('should return delete note error', () => {
expect(selectDeleteNoteError(mockGlobalState)).toEqual(null);
});
it('should return all notes for an existing document id', () => {
expect(selectNotesByDocumentId(mockGlobalState, '1')).toEqual([mockNote]);
});

29
x-pack/plugins/security_solution/public/notes/store/notes.slice.ts
View file

@ -11,6 +11,7 @@ import { createSelector } from 'reselect';
import type { State } from '../../common/store';
import {
createNote as createNoteApi,
deleteNote as deleteNoteApi,
fetchNotesByDocumentId as fetchNotesByDocumentIdApi,
} from '../api/api';
import type { NormalizedEntities, NormalizedEntity } from './normalize';
@ -33,10 +34,12 @@ export interface NotesState extends EntityState<Note> {
status: {
fetchNotesByDocumentId: ReqStatus;
createNote: ReqStatus;
deleteNote: ReqStatus;
};
error: {
fetchNotesByDocumentId: SerializedError | HttpError | null;
createNote: SerializedError | HttpError | null;
deleteNote: SerializedError | HttpError | null;
};
}
@ -48,10 +51,12 @@ export const initialNotesState: NotesState = notesAdapter.getInitialState({
status: {
fetchNotesByDocumentId: ReqStatus.Idle,
createNote: ReqStatus.Idle,
deleteNote: ReqStatus.Idle,
},
error: {
fetchNotesByDocumentId: null,
createNote: null,
deleteNote: null,
},
});
@ -74,6 +79,15 @@ export const createNote = createAsyncThunk<NormalizedEntity<Note>, { note: BareN
}
);
export const deleteNote = createAsyncThunk<string, { id: string }, {}>(
'notes/deleteNote',
async (args) => {
const { id } = args;
await deleteNoteApi(id);
return id;
}
);
const notesSlice = createSlice({
name: 'notes',
initialState: initialNotesState,
@ -101,6 +115,17 @@ const notesSlice = createSlice({
.addCase(createNote.rejected, (state, action) => {
state.status.createNote = ReqStatus.Failed;
state.error.createNote = action.payload ?? action.error;
})
.addCase(deleteNote.pending, (state) => {
state.status.deleteNote = ReqStatus.Loading;
})
.addCase(deleteNote.fulfilled, (state, action) => {
notesAdapter.removeOne(state, action.payload);
state.status.deleteNote = ReqStatus.Succeeded;
})
.addCase(deleteNote.rejected, (state, action) => {
state.status.deleteNote = ReqStatus.Failed;
state.error.deleteNote = action.payload ?? action.error;
});
},
});
@ -123,6 +148,10 @@ export const selectCreateNoteStatus = (state: State) => state.notes.status.creat
export const selectCreateNoteError = (state: State) => state.notes.error.createNote;
export const selectDeleteNoteStatus = (state: State) => state.notes.status.deleteNote;
export const selectDeleteNoteError = (state: State) => state.notes.error.deleteNote;
export const selectNotesByDocumentId = createSelector(
[selectAllNotes, (state, documentId) => documentId],
(notes, documentId) => notes.filter((note) => note.eventId === documentId)