github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-23 17:28:26 -04:00
Code Issues Projects Releases Packages Wiki Activity

[Security Solution][Notes] - fix pinning behavior in document notes (#194473)

Browse source 
## Summary

Fixed some pinning behaviors in new notes system, namely:

- Pinning is greyed out only when an event has a note attached to the
**current** timeline, else pinning should work as usual (related:
https://github.com/elastic/kibana/issues/193738)
- Adding a note and attaching to current timeline automatically pins the
event
- Pinned tab and pinning capability are updated when a note attached to
current timeline is deleted

Feature flag: `securitySolutionNotesEnabled`


https://github.com/user-attachments/assets/0163d4a4-354c-4928-a337-40a93f6c7b2a


### Checklist

- [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
This commit is contained in:
christineweng 2024-10-03 19:42:26 -05:00 • committed by GitHub
parent 3499fbbc83
commit 883dfa8ae8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 155 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files

41
x-pack/plugins/security_solution/public/common/components/header_actions/actions.tsx
View file

@ -11,7 +11,10 @@ import { EuiButtonIcon, EuiToolTip } from '@elastic/eui';
import styled from 'styled-components';
import { TimelineTabs, TableId } from '@kbn/securitysolution-data-table';
import { selectNotesByDocumentId } from '../../../notes/store/notes.slice';
import {
selectNotesByDocumentId,
selectDocumentNotesBySavedObjectId,
} from '../../../notes/store/notes.slice';
import type { State } from '../../store';
import { selectTimelineById } from '../../../timelines/store/selectors';
import {
@ -70,7 +73,7 @@ const ActionsComponent: React.FC<ActionProps> = ({
}) => {
const dispatch = useDispatch();
const { timelineType } = useShallowEqualSelector((state) =>
const { timelineType, savedObjectId } = useShallowEqualSelector((state) =>
isTimelineScope(timelineId) ? selectTimelineById(state, timelineId) : timelineDefaults
);
@ -222,24 +225,34 @@ const ActionsComponent: React.FC<ActionProps> = ({
/* only applicable for new event based notes */
const documentBasedNotes = useSelector((state: State) => selectNotesByDocumentId(state, eventId));
/* only applicable notes before event based notes */
const timelineNoteIds = useMemo(
() => eventIdToNoteIds?.[eventId] ?? emptyNotes,
[eventIdToNoteIds, eventId]
const documentBasedNotesInTimeline = useSelector((state: State) =>
selectDocumentNotesBySavedObjectId(state, {
documentId: eventId,
savedObjectId: savedObjectId ?? '',
})
);
/* note ids associated with the document AND attached to the current timeline, used for pinning */
const timelineNoteIds = useMemo(() => {
if (securitySolutionNotesEnabled) {
// if timeline is unsaved, there is no notes associated to timeline yet
return savedObjectId ? documentBasedNotesInTimeline.map((note) => note.noteId) : [];
}
return eventIdToNoteIds?.[eventId] ?? emptyNotes;
}, [
eventIdToNoteIds,
eventId,
documentBasedNotesInTimeline,
savedObjectId,
securitySolutionNotesEnabled,
]);
/* note count of the document */
const notesCount = useMemo(
() => (securitySolutionNotesEnabled ? documentBasedNotes.length : timelineNoteIds.length),
[documentBasedNotes, timelineNoteIds, securitySolutionNotesEnabled]
);
const noteIds = useMemo(() => {
return securitySolutionNotesEnabled
? documentBasedNotes.map((note) => note.noteId)
: timelineNoteIds;
}, [documentBasedNotes, timelineNoteIds, securitySolutionNotesEnabled]);
return (
<ActionsContainer data-test-subj="actions-container">
<>
@ -291,7 +304,7 @@ const ActionsComponent: React.FC<ActionProps> = ({
isAlert={isAlert(eventType)}
key="pin-event"
onPinClicked={handlePinClicked}
noteIds={noteIds}
noteIds={timelineNoteIds}
eventIsPinned={isEventPinned}
timelineType={timelineType}
/>

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

@ -57,14 +57,16 @@ const mockGlobalStateWithSavedTimeline = {
[TimelineId.active]: {
...mockGlobalState.timeline.timelineById[TimelineId.test],
savedObjectId: 'savedObjectId',
pinnedEventIds: {},
},
},
},
};
const mockStore = createMockStore(mockGlobalStateWithSavedTimeline);
const renderNotesDetails = () =>
render(
<TestProviders>
<TestProviders store={mockStore}>
<DocumentDetailsContext.Provider value={panelContextValue}>
<NotesDetails />
</DocumentDetailsContext.Provider>
@ -81,16 +83,7 @@ describe('NotesDetails', () => {
});
it('should fetch notes for the document id', () => {
const mockStore = createMockStore(mockGlobalStateWithSavedTimeline);
render(
<TestProviders store={mockStore}>
<DocumentDetailsContext.Provider value={panelContextValue}>
<NotesDetails />
</DocumentDetailsContext.Provider>
</TestProviders>
);
renderNotesDetails();
expect(mockDispatch).toHaveBeenCalled();
});

15
x-pack/plugins/security_solution/public/flyout/document_details/left/components/notes_details.tsx
View file

@ -17,6 +17,7 @@ import { AddNote } from '../../../../notes/components/add_note';
import { useAppToasts } from '../../../../common/hooks/use_app_toasts';
import { NOTES_LOADING_TEST_ID } from '../../../../notes/components/test_ids';
import { NotesList } from '../../../../notes/components/notes_list';
import { pinEvent } from '../../../../timelines/store/actions';
import type { State } from '../../../../common/store';
import type { Note } from '../../../../../common/api/timeline';
import {
@ -64,6 +65,19 @@ export const NotesDetails = memo(() => {
);
const timelineSavedObjectId = useMemo(() => timeline?.savedObjectId ?? '', [timeline]);
// Automatically pin an associated event if it's attached to a timeline and it's not pinned yet
const onNoteAddInTimeline = useCallback(() => {
const isEventPinned = eventId ? timeline?.pinnedEventIds[eventId] === true : false;
if (!isEventPinned && eventId && timelineSavedObjectId) {
dispatch(
pinEvent({
id: TimelineId.active,
eventId,
})
);
}
}, [dispatch, eventId, timelineSavedObjectId, timeline.pinnedEventIds]);
const notes: Note[] = useSelector((state: State) =>
selectSortedNotesByDocumentId(state, {
documentId: eventId,
@ -111,6 +125,7 @@ export const NotesDetails = memo(() => {
<AddNote
eventId={eventId}
timelineId={isTimelineFlyout && attachToTimeline ? timelineSavedObjectId : ''}
onNoteAdd={isTimelineFlyout && attachToTimeline ? onNoteAddInTimeline : undefined}
>
{isTimelineFlyout && (
<AttachToActiveTimeline

13
x-pack/plugins/security_solution/public/notes/components/add_note.test.tsx
View file

@ -127,4 +127,17 @@ describe('AddNote', () => {
title: CREATE_NOTE_ERROR,
});
});
it('should call onNodeAdd callback when it is available', async () => {
const onNodeAdd = jest.fn();
const { getByTestId } = render(
<TestProviders>
<AddNote eventId={'event-id'} onNoteAdd={onNodeAdd} />
</TestProviders>
);
await userEvent.type(getByTestId('euiMarkdownEditorTextArea'), 'new note');
getByTestId(ADD_NOTE_BUTTON_TEST_ID).click();
expect(onNodeAdd).toHaveBeenCalled();
});
});

11
x-pack/plugins/security_solution/public/notes/components/add_note.tsx
View file

@ -61,6 +61,10 @@ export interface AddNewNoteProps {
* Children to render between the markdown and the add note button
*/
children?: React.ReactNode;
/*
* Callback to execute when a new note is added
*/
onNoteAdd?: () => void;
}
/**
@ -68,7 +72,7 @@ export interface AddNewNoteProps {
* The checkbox is automatically checked if the flyout is opened from a timeline and that timeline is saved. It is disabled if the flyout is NOT opened from a timeline.
*/
export const AddNote = memo(
({ eventId, timelineId, disableButton = false, children }: AddNewNoteProps) => {
({ eventId, timelineId, disableButton = false, children, onNoteAdd }: AddNewNoteProps) => {
const { telemetry } = useKibana().services;
const dispatch = useDispatch();
const { addError: addErrorToast } = useAppToasts();
@ -88,11 +92,14 @@ export const AddNote = memo(
},
})
);
if (onNoteAdd) {
onNoteAdd();
}
telemetry.reportAddNoteFromExpandableFlyoutClicked({
isRelatedToATimeline: timelineId != null,
});
setEditorValue('');
}, [dispatch, editorValue, eventId, telemetry, timelineId]);
}, [dispatch, editorValue, eventId, telemetry, timelineId, onNoteAdd]);
// show a toast if the create note call fails
useEffect(() => {

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

@ -27,6 +27,7 @@ import {
selectNoteById,
selectNoteIds,
selectNotesByDocumentId,
selectDocumentNotesBySavedObjectId,
selectNotesPagination,
selectNotesTablePendingDeleteIds,
selectNotesTableSearch,
@ -608,6 +609,30 @@ describe('notesSlice', () => {
expect(selectNotesByDocumentId(mockGlobalState, 'wrong-document-id')).toHaveLength(0);
});
it('should return no notes if no notes is found with specified document id and saved object id', () => {
expect(
selectDocumentNotesBySavedObjectId(mockGlobalState, {
documentId: '1',
savedObjectId: 'wrong-savedObjectId',
})
).toHaveLength(0);
expect(
selectDocumentNotesBySavedObjectId(mockGlobalState, {
documentId: 'wrong-document-id',
savedObjectId: 'some-timeline-id',
})
).toHaveLength(0);
});
it('should return all notes for an existing document id and existing saved object id', () => {
expect(
selectDocumentNotesBySavedObjectId(mockGlobalState, {
documentId: '1',
savedObjectId: 'timeline-1',
})
).toHaveLength(1);
});
it('should return all notes sorted for an existing document id', () => {
const oldestNote = {
eventId: '1', // should be a valid id based on mockTimelineData

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

@ -318,6 +318,18 @@ export const selectNotesBySavedObjectId = createSelector(
savedObjectId.length > 0 ? notes.filter((note) => note.timelineId === savedObjectId) : []
);
export const selectDocumentNotesBySavedObjectId = createSelector(
[
selectAllNotes,
(
state: State,
{ documentId, savedObjectId }: { documentId: string; savedObjectId: string }
) => ({ documentId, savedObjectId }),
],
(notes, { documentId, savedObjectId }) =>
notes.filter((note) => note.eventId === documentId && note.timelineId === savedObjectId)
);
export const selectSortedNotesByDocumentId = createSelector(
[
selectAllNotes,

8
x-pack/plugins/security_solution/public/timelines/components/timeline/body/helpers.test.ts
View file

@ -28,7 +28,7 @@ describe('helpers', () => {
eventHasNotes: true,
timelineType: TimelineTypeEnum.default,
})
).toEqual('This event cannot be unpinned because it has notes');
).toEqual('This event cannot be unpinned because it has notes in Timeline');
});
test('it indicates the alert may NOT be unpinned when `isPinned` is `true` and the alert has notes', () => {
@ -39,7 +39,7 @@ describe('helpers', () => {
eventHasNotes: true,
timelineType: TimelineTypeEnum.default,
})
).toEqual('This alert cannot be unpinned because it has notes');
).toEqual('This alert cannot be unpinned because it has notes in Timeline');
});
test('it indicates the event is pinned when `isPinned` is `true` and the event does NOT have notes', () => {
@ -72,7 +72,7 @@ describe('helpers', () => {
eventHasNotes: true,
timelineType: TimelineTypeEnum.default,
})
).toEqual('This event cannot be unpinned because it has notes');
).toEqual('This event cannot be unpinned because it has notes in Timeline');
});
test('it indicates the alert is pinned when `isPinned` is `false` and the alert has notes', () => {
@ -83,7 +83,7 @@ describe('helpers', () => {
eventHasNotes: true,
timelineType: TimelineTypeEnum.default,
})
).toEqual('This alert cannot be unpinned because it has notes');
).toEqual('This alert cannot be unpinned because it has notes in Timeline');
});
test('it indicates the event is NOT pinned when `isPinned` is `false` and the event does NOT have notes', () => {

2
x-pack/plugins/security_solution/public/timelines/components/timeline/body/translations.ts
View file

@ -23,7 +23,7 @@ export const PINNED_WITH_NOTES = (isAlert: boolean) =>
i18n.translate('xpack.securitySolution.timeline.body.pinning.pinnnedWithNotesTooltip', {
values: { isAlert },
defaultMessage:
'This {isAlert, select, true{alert} other{event}} cannot be unpinned because it has notes',
'This {isAlert, select, true{alert} other{event}} cannot be unpinned because it has notes in Timeline',
});
export const SORTED_ASCENDING = i18n.translate(

48
x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/query/query_tab_unified_components.test.tsx
View file

@ -1026,9 +1026,31 @@ describe('query tab with unified timeline', () => {
);
});
it(
'should have the pin button with correct tooltip',
'should disable pinning when event has notes attached in timeline',
async () => {
renderTestComponents();
const mockStateWithNoteInTimeline = {
...mockGlobalState,
timeline: {
...mockGlobalState.timeline,
timelineById: {
[TimelineId.test]: {
...mockGlobalState.timeline.timelineById[TimelineId.test],
savedObjectId: 'timeline-1', // match timelineId in mocked notes data
pinnedEventIds: { '1': true },
},
},
},
};
render(
<TestProviders
store={createMockStore({
...structuredClone(mockStateWithNoteInTimeline),
})}
>
<TestComponent />
</TestProviders>
);
expect(await screen.findByTestId('discoverDocTable')).toBeVisible();
@ -1041,7 +1063,7 @@ describe('query tab with unified timeline', () => {
await waitFor(() => {
expect(screen.getByTestId('timeline-action-pin-tool-tip')).toBeVisible();
expect(screen.getByTestId('timeline-action-pin-tool-tip')).toHaveTextContent(
'This event cannot be unpinned because it has notes'
'This event cannot be unpinned because it has notes in Timeline'
);
/*
* Above event is alert and not an event but `getEventType` in
@ -1054,6 +1076,26 @@ describe('query tab with unified timeline', () => {
},
SPECIAL_TEST_TIMEOUT
);
it(
'should allow pinning when event has notes but notes are not attached in current timeline',
async () => {
renderTestComponents();
expect(await screen.findByTestId('discoverDocTable')).toBeVisible();
expect(screen.getAllByTestId('pin')).toHaveLength(1);
expect(screen.getByTestId('pin')).not.toBeDisabled();
fireEvent.mouseOver(screen.getByTestId('pin'));
await waitFor(() => {
expect(screen.getByTestId('timeline-action-pin-tool-tip')).toBeVisible();
expect(screen.getByTestId('timeline-action-pin-tool-tip')).toHaveTextContent(
'Pin event'
);
});
},
SPECIAL_TEST_TIMEOUT
);
});
describe('securitySolutionNotesEnabled = false', () => {