PR comments

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

@@ -232,26 +232,27 @@ const ActionsComponent: React.FC<ActionProps> = ({
     })
   );
 
-  /* only applicable notes before event based notes */
-  const timelineNoteIds = useMemo(
-    () => eventIdToNoteIds?.[eventId] ?? emptyNotes,
-    [eventIdToNoteIds, eventId]
-  );
+  /* 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]
   );
 
-  /* note ids specific to the current timeline, it is used to enable/disable pinning */
-  const noteIdsInTimeline = useMemo(() => {
-    if (securitySolutionNotesEnabled) {
-      // if timeline is unsaved, there is no notes associated to timeline yet
-      return savedObjectId ? documentBasedNotesInTimeline.map((note) => note.noteId) : [];
-    }
-    return timelineNoteIds;
-  }, [documentBasedNotesInTimeline, timelineNoteIds, securitySolutionNotesEnabled, savedObjectId]);
-
   return (
     <ActionsContainer data-test-subj="actions-container">
       <>
@@ -303,7 +304,7 @@ const ActionsComponent: React.FC<ActionProps> = ({
             isAlert={isAlert(eventType)}
             key="pin-event"
             onPinClicked={handlePinClicked}
-            noteIds={noteIdsInTimeline}
+            noteIds={timelineNoteIds}
             eventIsPinned={isEventPinned}
             timelineType={timelineType}
           />

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

@@ -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();
   });
 

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

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

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

@@ -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();
+  });
 });

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

@@ -20,9 +20,6 @@ import { useKibana } from '../../common/lib/kibana';
 import { ADD_NOTE_BUTTON_TEST_ID, ADD_NOTE_MARKDOWN_TEST_ID } from './test_ids';
 import { useAppToasts } from '../../common/hooks/use_app_toasts';
 import type { State } from '../../common/store';
-import { timelineSelectors } from '../../timelines/store';
-import { TimelineId } from '../../../common/types';
-import { pinEvent } from '../../timelines/store/actions';
 import {
   createNote,
   ReqStatus,
@@ -64,14 +61,18 @@ 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;
 }
 
 /**
  * Renders a markdown editor and an add button to create new notes.
  * 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();
@@ -80,9 +81,6 @@ export const AddNote = memo(
 
     const createStatus = useSelector((state: State) => selectCreateNoteStatus(state));
     const createError = useSelector((state: State) => selectCreateNoteError(state));
-    const activeTimeline = useSelector((state: State) =>
-      timelineSelectors.selectTimelineById(state, TimelineId.active)
-    );
 
     const addNote = useCallback(() => {
       dispatch(
@@ -94,22 +92,14 @@ export const AddNote = memo(
           },
         })
       );
-
-      // Automatically pin an associated event if it's attached to a timeline and it's not pinned yet
-      const isEventPinned = eventId ? activeTimeline.pinnedEventIds[eventId] === true : false;
-      if (!isEventPinned && eventId && timelineId) {
-        dispatch(
-          pinEvent({
-            id: TimelineId.active,
-            eventId,
-          })
-        );
+      if (onNoteAdd) {
+        onNoteAdd();
       }
       telemetry.reportAddNoteFromExpandableFlyoutClicked({
         isRelatedToATimeline: timelineId != null,
       });
       setEditorValue('');
-    }, [dispatch, editorValue, eventId, telemetry, timelineId, activeTimeline.pinnedEventIds]);
+    }, [dispatch, editorValue, eventId, telemetry, timelineId, onNoteAdd]);
 
     // show a toast if the create note call fails
     useEffect(() => {

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

@@ -624,6 +624,15 @@ describe('notesSlice', () => {
       ).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

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

@@ -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', () => {

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

@@ -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(

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

@@ -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', () => {