[Security Solution][Notes] - fetch notes by saved object ids #193930

PhilippeOberti · 2024-09-24T22:44:40Z

Summary

This PR changes the behavior of the Timeline Notes tab to use the new note system. Everything is behind the securitySolutionNotesEnabled feature flag for now.
The goal of this was to not modify the current (old) Notes behavior. We will keep the code for at least one release after we switch the securitySolutionNotesEnabled on, to allow customers to use the old system needed.

Here are the main changes for the server side:

modified the getNote api to support filtering by savedObjectIds. This allows us to now search all the notes for a specific timeline
added unit tests

Here are the main changes for the UI side:

move the content of the NotesTabContentComponent into an new OldNotes component. The goal was to not change that code at all to stay on the safe side, and facilitate removing all the code when the time comes.
extract a lot of the notes components from the expandable flyout to the top level notes tab, and make them reusable for the new Timeline Notes tab:
- the AddNote logic (markdown, attach to timeline and add note button)
- the OpenTimeline logic to open the timeline from the expandable flyout
- the DeleteNote logic to delete notes
- the OpenFlyout logic to open the expandable flyout from the timeline notes page
change the UI for the AttachToTimeline logic:
- added a new the callout component to replace the icon with tooltip. The callout displays a Save timeline button if the timeline is not saved, and replaces that button by a Attach to timeline checkbox once saved
- the callout and checkbox are now only shown in the timeline flyout, between the markdown and the add button
change the timeline notes tab to force the users to save timeline if they want to attach a note to it
wrote a lot of unit tests

Timeline Notes tab

For unsaved timelines

For saved timelines

Timeline flyout Notes tab

For unsaved timelines

For saved timelines

SecuritySolution flyout Notes tab

Here's an example of a flow for the timeline notes tab

unsaved.timeline.notes.tab.mov

Here's an example of a flow for the SecuritySolution flyout

unsaved.timeline.flyout.mov

Here's an example for the SecuritySolution flyout

normal.flyout.mov

How to test

Keep in mind while testing this changes the following behaviors:

no changes should be visible unless the securitySolutionNotesEnabled feature flag is on
all previously saved timelines and related notes should be visible the exact same way once the securitySolutionNotesEnabled is turned on
creating notes with the new system (securitySolutionNotesEnabled turned on) should work in the old system (after browser refresh) with the securitySolutionNotesEnabled turned off

Checklist

Unit or functional tests were updated or added to match the most common scenarios

#193098

elasticmachine · 2024-09-27T00:18:49Z

Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations)

christineweng

Code looks good to me. Some comments from desk testing:

Adding a note in notes tab does not update the count in tab

siem dev:
Add a note in notes tab in an unsaved timeline, then investigate a new alert in timeline, the note persists.
Current behavior: after investigate a new alert in timeline, a new timeline is created and the previous note is lost
Related to (2). Add a note in notes tab in an unsaved timeline, save the timeline, the note disappear from saved timeline. Any note created in an unsaved timeline attaches to the draft only

Screen.Recording.2024-09-27.at.12.53.50.PM.mov

When creating a new timeline (via Create a timeline or from templates), there is nothing in notes tab but participant is there

UI: Is the empty message kinda of redundant?

old notes

christineweng · 2024-09-27T15:25:59Z

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

+  eventId?: string;
+  /**
+   *
+   */


missing a doc here

added in the last commit

christineweng · 2024-09-30T18:35:58Z

@PhilippeOberti notice this when i was working on pinning:

Investigate an event in timeline and save the timeline
Go back to alerts table and add a note (should not have attach to timeline)

notice the timeline icon is automatically added when it is not supposed to

Screen.Recording.2024-09-30.at.1.31.22.PM.mov

PhilippeOberti · 2024-09-30T23:13:35Z

@PhilippeOberti notice this when i was working on pinning:

Investigate an event in timeline and save the timeline

Go back to alerts table and add a note (should not have attach to timeline)

notice the timeline icon is automatically added when it is not supposed to

Thanks @christineweng this should be resolved in my last commit

PhilippeOberti · 2024-10-01T01:40:41Z

Adding a note in notes tab does not update the count in tab

In the last commit I added the code to fetch notes in the tab title as well.

Add a note in notes tab in an unsaved timeline, then investigate a new alert in timeline, the note persists.

The new behavior added in the last commit should prevent a user from being able to create a note in an unsaved timeline!

Related to (2). Add a note in notes tab in an unsaved timeline, save the timeline, the note disappear from saved timeline. Any note created in an unsaved timeline attaches to the draft only

Same here, the last commit prevents a user from creating a note in an unsaved timeline.

When creating a new timeline (via Create a timeline or from templates), there is nothing in notes tab but participant is there

If you try again now, you'll see that the title Participants is shown but nothing is below it, which I think is nicer than what we have today, where there is just an empty section and we don't really know why...

UI: Is the empty message kinda of redundant?

I guess it could be, but last time I showed it to @ferenrigue I think she was OK with it. I'll bring it up again!

…ut to timeline

…taging'

kibana-ci · 2024-10-01T19:45:54Z

💚 Build Succeeded

Buildkite Build
Commit: f388665

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	5931	5940	+9

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	20.5MB	20.5MB	+37.0KB

History

💛 Build #238247 was flaky 97289f08cd3982d392cc4fe10c7a059258f8bd87
💔 Build #237570 failed 76de4efacd79030768b361c482a713598c7fe518
💔 Build #237552 failed f07a522fb504c7abd6441853f1b69549550326c0

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

christineweng

Desk tested and LGTM!🚀 The added save timeline button is much better at gating behaviors related to an unsaved timeline 👏

Non-blocking but i was finally able to recreate the error toast from timeline templates consistently:

Have a fresh unsaved timeline, create a new timeline from a template, click on notes icon, notice there is an error toast about null
I also see this happening occasionally with events in explore pages. But only the first time I click on the note icon

Screen.Recording.2024-10-01.at.3.31.39.PM.mov

PhilippeOberti · 2024-10-01T20:44:34Z

Desk tested and LGTM!🚀 The added save timeline button is much better at gating behaviors related to an unsaved timeline 👏

Non-blocking but i was finally able to recreate the error toast from timeline templates consistently:

Have a fresh unsaved timeline, create a new timeline from a template, click on notes icon, notice there is an error toast about null

I also see this happening occasionally with events in explore pages. But only the first time I click on the note icon

Thanks for the steps to reproduce. I will get on it as soon as this PR is merged. It is behind a feature flag, and merging it will unblock some other work

kibanamachine · 2024-10-01T20:44:58Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11133177317

…#193930) (cherry picked from commit ca46f78)

kibanamachine · 2024-10-01T20:49:17Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…193930) (#194643) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution][Notes] - fetch notes by saved object ids (#193930)](#193930)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Philippe Oberti <[email protected]>

PhilippeOberti force-pushed the notes-fetch-by-saved-object-ids branch 3 times, most recently from 2872578 to 83e0262 Compare September 26, 2024 23:59

PhilippeOberti added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 Team:Threat Hunting:Investigations Security Solution Investigations Team v8.16.0 labels Sep 27, 2024

PhilippeOberti marked this pull request as ready for review September 27, 2024 00:18

PhilippeOberti requested review from a team as code owners September 27, 2024 00:18

PhilippeOberti added the backport label Sep 27, 2024

PhilippeOberti force-pushed the notes-fetch-by-saved-object-ids branch from f07a522 to 6a99a34 Compare September 27, 2024 03:01

PhilippeOberti requested review from christineweng and kqualters-elastic September 27, 2024 04:48

christineweng reviewed Sep 27, 2024

View reviewed changes

christineweng mentioned this pull request Sep 30, 2024

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

Merged

1 task

PhilippeOberti force-pushed the notes-fetch-by-saved-object-ids branch from 76de4ef to 39515e8 Compare September 30, 2024 23:13

PhilippeOberti force-pushed the notes-fetch-by-saved-object-ids branch 3 times, most recently from 4c181ec to 87b73df Compare October 1, 2024 01:29

PhilippeOberti and others added 5 commits October 1, 2024 12:52

[Security Solution][Notes] - fetch notes by saved object ids

b7f786b

open-api

420b2ec

change how the callout is displayed in the flyout and add a new callo…

7c11506

…ut to timeline

[CI] Auto-commit changed files from 'make api-docs && make api-docs-s…

887dc69

…taging'

change callout to always be blue in the flyout (per UIUX feedback)

f388665

PhilippeOberti force-pushed the notes-fetch-by-saved-object-ids branch from 97289f0 to f388665 Compare October 1, 2024 17:53

christineweng approved these changes Oct 1, 2024

View reviewed changes

PhilippeOberti merged commit ca46f78 into elastic:main Oct 1, 2024
40 checks passed

PhilippeOberti deleted the notes-fetch-by-saved-object-ids branch October 1, 2024 20:44

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 1, 2024

[Security Solution][Notes] - fetch notes by saved object ids (elastic…

fa4bc94

…#193930) (cherry picked from commit ca46f78)

kibanamachine mentioned this pull request Oct 1, 2024

[8.x] [Security Solution][Notes] - fetch notes by saved object ids (#193930) #194643

Merged

PhilippeOberti mentioned this pull request Oct 2, 2024

[Security Solution][Notes] - updating colors for the save timeline callouts #194749

Merged

1 task

This was referenced Oct 11, 2024

[SecuritySolution][Notes] - make sure that timeline is saved before allowing users to save notes #195842

Merged

[Security Solution]confirmation prompt of deleting note from alert fly out #188180

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Notes] - fetch notes by saved object ids #193930

[Security Solution][Notes] - fetch notes by saved object ids #193930

PhilippeOberti commented Sep 24, 2024 •

edited by kibanamachine

Loading

elasticmachine commented Sep 27, 2024

christineweng left a comment •

edited

Loading

christineweng Sep 27, 2024

PhilippeOberti Oct 1, 2024

christineweng commented Sep 30, 2024

PhilippeOberti commented Sep 30, 2024 •

edited

Loading

PhilippeOberti commented Oct 1, 2024

kibana-ci commented Oct 1, 2024

christineweng left a comment

PhilippeOberti commented Oct 1, 2024

kibanamachine commented Oct 1, 2024

kibanamachine commented Oct 1, 2024

[Security Solution][Notes] - fetch notes by saved object ids #193930

[Security Solution][Notes] - fetch notes by saved object ids #193930

Conversation

PhilippeOberti commented Sep 24, 2024 • edited by kibanamachine Loading

Summary

Timeline Notes tab

Timeline flyout Notes tab

SecuritySolution flyout Notes tab

Here's an example of a flow for the timeline notes tab

Here's an example of a flow for the SecuritySolution flyout

Here's an example for the SecuritySolution flyout

How to test

Checklist

elasticmachine commented Sep 27, 2024

christineweng left a comment • edited Loading

Choose a reason for hiding this comment

christineweng Sep 27, 2024

Choose a reason for hiding this comment

PhilippeOberti Oct 1, 2024

Choose a reason for hiding this comment

christineweng commented Sep 30, 2024

PhilippeOberti commented Sep 30, 2024 • edited Loading

PhilippeOberti commented Oct 1, 2024

kibana-ci commented Oct 1, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

christineweng left a comment

Choose a reason for hiding this comment

PhilippeOberti commented Oct 1, 2024

kibanamachine commented Oct 1, 2024

kibanamachine commented Oct 1, 2024

💚 All backports created successfully

Questions ?

PhilippeOberti commented Sep 24, 2024 •

edited by kibanamachine

Loading

christineweng left a comment •

edited

Loading

PhilippeOberti commented Sep 30, 2024 •

edited

Loading