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.

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

[Security Solution] [Timeline] delete notes #154834

Merged
merged 19 commits into from Apr 25, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
dfb4376
WIP first pass at deleting timeline notes
kqualters-elastic Apr 12, 2023
77d6719
Merge remote-tracking branch 'upstream/main' into timeline-delete-note
kqualters-elastic Apr 12, 2023
fc37db9
Update
kqualters-elastic Apr 12, 2023
676a946
Merge remote-tracking branch 'upstream/main' into timeline-delete-note
kqualters-elastic Apr 24, 2023
860d466
Add cypress test, delete unused interface
kqualters-elastic Apr 24, 2023
92ca33f
Add unit/cypress test, i18n
kqualters-elastic Apr 24, 2023
34fd50e
Merge remote-tracking branch 'upstream/main' into timeline-delete-note
kqualters-elastic Apr 24, 2023
0ac946d
Remove unused mock
kqualters-elastic Apr 24, 2023
cde1374
Click most recently created note only
kqualters-elastic Apr 24, 2023
431972a
Merge remote-tracking branch 'upstream/main' into timeline-delete-note
kqualters-elastic Apr 24, 2023
2c1d486
Fix failing tests
kqualters-elastic Apr 25, 2023
0fc47ad
Merge remote-tracking branch 'upstream/main' into timeline-delete-note
kqualters-elastic Apr 25, 2023
d259563
Disable deleting more than 1 note at a time
kqualters-elastic Apr 25, 2023
cc286b6
Merge remote-tracking branch 'upstream/main' into timeline-delete-note
kqualters-elastic Apr 25, 2023
64ff37c
Replace isLoading with isFetching
kqualters-elastic Apr 25, 2023
4eb223e
Merge remote-tracking branch 'upstream/main' into timeline-delete-note
kqualters-elastic Apr 25, 2023
25b86a5
Use useMutation instead of useQuery
kqualters-elastic Apr 25, 2023
703e0a9
Merge remote-tracking branch 'upstream/main' into timeline-delete-note
kqualters-elastic Apr 25, 2023
ca3e419
Merge remote-tracking branch 'upstream/main' into timeline-delete-note
kqualters-elastic Apr 25, 2023
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
Expand Up @@ -8,13 +8,16 @@
import { getTimelineNonValidQuery } from '../../objects/timeline';

import {
DELETE_NOTE,
NOTES_AUTHOR,
NOTES_CODE_BLOCK,
NOTE_DESCRIPTION,
NOTES_LINK,
NOTES_TEXT,
NOTES_TEXT_AREA,
MARKDOWN_INVESTIGATE_BUTTON,
} from '../../screens/timeline';
import { MODAL_CONFIRMATION_BTN } from '../../screens/alerts_detection_rules';
import { createTimeline } from '../../tasks/api_calls/timelines';

import { cleanKibana } from '../../tasks/common';
Expand Down Expand Up @@ -97,4 +100,12 @@ describe('Timeline notes tab', () => {
);
cy.get(MARKDOWN_INVESTIGATE_BUTTON).should('exist');
});

it('should be able to delete a note', () => {
const deleteNoteContent = 'delete me';
addNotesToTimeline(deleteNoteContent);
cy.get(DELETE_NOTE).last().click();
cy.get(MODAL_CONFIRMATION_BTN).click();
cy.get(NOTE_DESCRIPTION).last().should('not.have.text', deleteNoteContent);
});
});
Expand Up @@ -92,6 +92,8 @@ export const NOTES_AUTHOR = '.euiCommentEvent__headerUsername';

export const NOTES_LINK = '[data-test-subj="markdown-link"]';

export const DELETE_NOTE = '[data-test-subj="delete-note"]';

export const MARKDOWN_INVESTIGATE_BUTTON =
'[data-test-subj="insight-investigate-in-timeline-button"]';

Expand Down
Expand Up @@ -15,6 +15,8 @@ export const updateNote = actionCreator<{ note: Note }>('UPDATE_NOTE');

export const addNotes = actionCreator<{ notes: Note[] }>('ADD_NOTE');

export const deleteNote = actionCreator<{ id: string }>('DELETE_NOTE');

export const addError = actionCreator<{ id: string; title: string; message: string[] }>(
'ADD_ERRORS'
);
Expand Down
Expand Up @@ -9,7 +9,7 @@ import { reducerWithInitialState } from 'typescript-fsa-reducers';

import type { Note } from '../../lib/note';

import { addError, addErrorHash, addNotes, removeError, updateNote } from './actions';
import { addError, addErrorHash, addNotes, removeError, updateNote, deleteNote } from './actions';
import type { AppModel, NotesById } from './model';
import { allowedExperimentalValues } from '../../../../common/experimental_features';

Expand All @@ -36,6 +36,14 @@ export const appReducer = reducerWithInitialState(initialAppState)
...state,
notesById: notes.reduce<NotesById>((acc, note: Note) => ({ ...acc, [note.id]: note }), {}),
}))
.case(deleteNote, (state, { id }) => ({
...state,
notesById: Object.fromEntries(
Object.entries(state.notesById).filter(([_, note]) => {
return note.id !== id && note.saveObjectId !== id;
})
),
}))
.case(updateNote, (state, { note }) => ({
...state,
notesById: updateNotesById({ note, notesById: state.notesById }),
Expand Down
Expand Up @@ -8,13 +8,15 @@
import { cloneDeep } from 'lodash/fp';
import moment from 'moment';
import { mountWithIntl } from '@kbn/test-jest-helpers';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import React from 'react';
import '../../../../common/mock/formatted_relative';
import { useDeepEqualSelector } from '../../../../common/hooks/use_selector';
import { mockTimelineResults } from '../../../../common/mock/timeline_results';
import type { OpenTimelineResult, TimelineResultNote } from '../types';
import { NotePreviews } from '.';

jest.mock('../../../../common/lib/kibana');
jest.mock('../../../../common/hooks/use_selector');

jest.mock('react-redux', () => {
Expand All @@ -30,19 +32,31 @@ describe('NotePreviews', () => {
let note1updated: number;
let note2updated: number;
let note3updated: number;
let queryClient: QueryClient;

beforeEach(() => {
mockResults = cloneDeep(mockTimelineResults);
note1updated = moment('2019-03-24T04:12:33.000Z').valueOf();
note2updated = moment(note1updated).add(1, 'minute').valueOf();
note3updated = moment(note2updated).add(1, 'minute').valueOf();
(useDeepEqualSelector as jest.Mock).mockReset();
queryClient = new QueryClient({
defaultOptions: {
queries: {
retry: false,
},
},
});
});

test('it renders a note preview for each note when isModal is false', () => {
const hasNotes: OpenTimelineResult[] = [{ ...mockResults[0] }];

const wrapper = mountWithIntl(<NotePreviews notes={hasNotes[0].notes} />);
const wrapper = mountWithIntl(
<QueryClientProvider client={queryClient}>
<NotePreviews notes={hasNotes[0].notes} />
</QueryClientProvider>
);

hasNotes[0].notes?.forEach(({ savedObjectId }) => {
expect(wrapper.find(`[data-test-subj="note-preview-${savedObjectId}"]`).exists()).toBe(true);
Expand All @@ -52,7 +66,11 @@ describe('NotePreviews', () => {
test('it renders a note preview for each note when isModal is true', () => {
const hasNotes: OpenTimelineResult[] = [{ ...mockResults[0] }];

const wrapper = mountWithIntl(<NotePreviews notes={hasNotes[0].notes} />);
const wrapper = mountWithIntl(
<QueryClientProvider client={queryClient}>
<NotePreviews notes={hasNotes[0].notes} />
</QueryClientProvider>
);

hasNotes[0].notes?.forEach(({ savedObjectId }) => {
expect(wrapper.find(`[data-test-subj="note-preview-${savedObjectId}"]`).exists()).toBe(true);
Expand Down Expand Up @@ -81,7 +99,11 @@ describe('NotePreviews', () => {
},
];

const wrapper = mountWithIntl(<NotePreviews notes={nonUniqueNotes} />);
const wrapper = mountWithIntl(
<QueryClientProvider client={queryClient}>
<NotePreviews notes={nonUniqueNotes} />
</QueryClientProvider>
);

expect(wrapper.find('div.euiCommentEvent__headerUsername').at(1).text()).toEqual('bob');
});
Expand All @@ -108,7 +130,11 @@ describe('NotePreviews', () => {
},
];

const wrapper = mountWithIntl(<NotePreviews notes={nonUniqueNotes} />);
const wrapper = mountWithIntl(
<QueryClientProvider client={queryClient}>
<NotePreviews notes={nonUniqueNotes} />
</QueryClientProvider>
);

expect(wrapper.find('div.euiCommentEvent__headerUsername').at(2).text()).toEqual('bob');
});
Expand All @@ -134,7 +160,11 @@ describe('NotePreviews', () => {
},
];

const wrapper = mountWithIntl(<NotePreviews notes={nonUniqueNotes} />);
const wrapper = mountWithIntl(
<QueryClientProvider client={queryClient}>
<NotePreviews notes={nonUniqueNotes} />{' '}
</QueryClientProvider>
);

expect(wrapper.find('div.euiCommentEvent__headerUsername').at(2).text()).toEqual('bob');
});
Expand All @@ -144,7 +174,9 @@ describe('NotePreviews', () => {
(useDeepEqualSelector as jest.Mock).mockReturnValue(timeline);

const wrapper = mountWithIntl(
<NotePreviews notes={[]} showTimelineDescription timelineId="test-timeline-id" />
<QueryClientProvider client={queryClient}>
<NotePreviews notes={[]} showTimelineDescription timelineId="test-timeline-id" />
</QueryClientProvider>
);

expect(wrapper.find('[data-test-subj="note-preview-description"]').first().text()).toContain(
Expand All @@ -156,8 +188,59 @@ describe('NotePreviews', () => {
const timeline = mockTimelineResults[0];
(useDeepEqualSelector as jest.Mock).mockReturnValue({ ...timeline, description: undefined });

const wrapper = mountWithIntl(<NotePreviews notes={[]} />);
const wrapper = mountWithIntl(
<QueryClientProvider client={queryClient}>
<NotePreviews notes={[]} />
</QueryClientProvider>
);

expect(wrapper.find('[data-test-subj="note-preview-description"]').exists()).toBe(false);
});

test('it should disable the delete note button if the savedObjectId is falsy', () => {
const timeline = mockTimelineResults[0];
(useDeepEqualSelector as jest.Mock).mockReturnValue(timeline);

const wrapper = mountWithIntl(
<QueryClientProvider client={queryClient}>
<NotePreviews
notes={[
{
note: 'disabled delete',
updated: note2updated,
updatedBy: 'alice',
},
]}
showTimelineDescription
timelineId="test-timeline-id"
/>
</QueryClientProvider>
);

expect(wrapper.find('[data-test-subj="delete-note"] button').prop('disabled')).toBeTruthy();
});

test('it should enable the delete button if the savedObjectId exists', () => {
const timeline = mockTimelineResults[0];
(useDeepEqualSelector as jest.Mock).mockReturnValue(timeline);

const wrapper = mountWithIntl(
<QueryClientProvider client={queryClient}>
<NotePreviews
notes={[
{
note: 'enabled delete',
savedObjectId: 'test-id',
updated: note2updated,
updatedBy: 'alice',
},
]}
showTimelineDescription
timelineId="test-timeline-id"
/>
</QueryClientProvider>
);

expect(wrapper.find('[data-test-subj="delete-note"] button').prop('disabled')).toBeFalsy();
});
});