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.

Sign up for GitHub

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

Jump to bottom

[Security Solution][Notes] - move notes management page under manage section instead of timeline #194250

Merged
merged 2 commits into from
Oct 1, 2024
Merged

[Security Solution][Notes] - move notes management page under manage section instead of timeline #194250

merged 2 commits into from
Oct 1, 2024

Conversation

PhilippeOberti
Copy link
Contributor

@PhilippeOberti PhilippeOberti commented Sep 27, 2024 •
edited by kibanamachine
Loading

Summary

This PR moves the notes management page from a Notes tab within the Timeline page to a new Notes page accessible from the Manage section of Security Solution.
It also creates a new entry in the Investigations section in the new navigation.
Finally, it simplifies the code that had allowed us to use this notes management page in the timeline page as a separate tab.

Notes: no UI changes other than a page title have been performed in this PR, other than adding a title at the top of the page. Future work will change/enhance its content.

Current (page accessible via the Timeline section)

Screenshot 2024-09-27 at 4 56 46 PM

New section in Manage

Screenshot 2024-09-27 at 4 51 07 PM

New section in Investigations when using the new navigation

Screenshot 2024-09-27 at 4 52 37 PM

The layout and behavior of the content of the page stays identical (url is /administration/notes)

Screenshot 2024-09-27 at 4 53 57 PM

Checklist

TODO

  • waiting on finalization of the copy changes (can be done in a follow up PR)

#193059

@PhilippeOberti PhilippeOberti requested review from a team as code owners September 27, 2024 04:48
@elasticmachine
Copy link
Contributor

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

@PhilippeOberti PhilippeOberti marked this pull request as draft September 27, 2024 16:42
@PhilippeOberti PhilippeOberti force-pushed the note-management branch 2 times, most recently from 7b499f7 to 6a62a10 Compare September 27, 2024 21:55
@PhilippeOberti PhilippeOberti marked this pull request as ready for review September 27, 2024 22:01
@PhilippeOberti PhilippeOberti force-pushed the note-management branch 2 times, most recently from 717adc9 to d0347da Compare September 30, 2024 15:40
ashokaditya
ashokaditya reviewed Oct 1, 2024
View reviewed changes
Copy link
Member

@ashokaditya ashokaditya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this and was able to pull Notes page under endpoint administration without the feature flag (app/security/administration/notes). Is that expected? In the screenshots you can see the Management menu doesn't have Notes without the FF but has the Notes page under endpoint admin.

page

Screenshot 2024-10-01 at 16 08 07

menu

Screenshot 2024-10-01 at 16 08 13

@PhilippeOberti
Copy link
Contributor Author

I tested this and was able to pull Notes page under endpoint administration without the feature flag (app/security/administration/notes). Is that expected? In the screenshots you can see the Management menu doesn't have Notes without the FF but has the Notes page under endpoint admin.

great catch @ashokaditya, this is now fixed in this commit
Screenshot 2024-10-01 at 11 26 48 AM

ashokaditya
ashokaditya approved these changes Oct 1, 2024
View reviewed changes
Copy link
Member

@ashokaditya ashokaditya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Thanks for the fix.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 5931 5934 +3

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 +5.8KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
securitySolution 88.2KB 88.2KB +32.0B
securitySolutionEss 11.6KB 11.6KB -21.0B
securitySolutionServerless 21.9KB 21.9KB -21.0B
total -10.0B
Unknown metric groups

async chunk count

id before after diff
securitySolution 95 94 -1

History

  • 💛 Build #238116 was flaky d0347da073a702ab5433563c044701ae59048c9e
  • 💚 Build #237818 succeeded 717adc9a91fabc77e636eea87593fd95cb1618cb
  • 💔 Build #237815 failed 6a62a10b32776537be38ba323e94b04cbdb6e833
  • 💔 Build #237572 failed 6e347c96760162576e1379ce0b2e0c456d1bd8c4

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

@PhilippeOberti PhilippeOberti merged commit 39ac875 into elastic:main Oct 1, 2024
40 checks passed
@PhilippeOberti PhilippeOberti deleted the note-management branch October 1, 2024 18:30
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 1, 2024
@PhilippeOberti
[Security Solution][Notes] - move notes management page under manage …
bc57631 
…section instead of timeline (elastic#194250)

(cherry picked from commit 39ac875)
@kibanamachine kibanamachine mentioned this pull request Oct 1, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 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

kibanamachine added a commit that referenced this pull request Oct 1, 2024
@kibanamachine @PhilippeOberti
[8.x] [Security Solution][Notes] - move notes management page under m…
cef7801 
…anage section instead of timeline (#194250) (#194628)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution][Notes] - move notes management page under manage
section instead of timeline
(#194250)](#194250)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Philippe
Oberti","email":"philippe.oberti@elastic.co"},"sourceCommit":{"committedDate":"2024-10-01T18:30:47Z","message":"[Security
Solution][Notes] - move notes management page under manage section
instead of timeline
(#194250)","sha":"39ac875b764019e2c570b175f3cc57e0c02b20ec","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["backport","v9.0.0","Team:Threat
Hunting:Investigations","v8.16.0"],"title":"[Security Solution][Notes] -
move notes management page under manage section instead of
timeline","number":194250,"url":"https://github.com/elastic/kibana/pull/194250","mergeCommit":{"message":"[Security
Solution][Notes] - move notes management page under manage section
instead of timeline
(#194250)","sha":"39ac875b764019e2c570b175f3cc57e0c02b20ec"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194250","number":194250,"mergeCommit":{"message":"[Security
Solution][Notes] - move notes management page under manage section
instead of timeline
(#194250)","sha":"39ac875b764019e2c570b175f3cc57e0c02b20ec"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Philippe Oberti <philippe.oberti@elastic.co>
@PhilippeOberti PhilippeOberti mentioned this pull request Oct 2, 2024
Merged
@PhilippeOberti PhilippeOberti mentioned this pull request Nov 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashokaditya ashokaditya ashokaditya approved these changes

@janmonschke janmonschke janmonschke approved these changes

@parkiino parkiino Awaiting requested review from parkiino parkiino is a code owner automatically assigned from elastic/security-defend-workflows

Assignees
No one assigned
Labels
backport Team:Threat Hunting:Investigations Security Solution Investigations Team v8.16.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@PhilippeOberti @elasticmachine @kibana-ci @kibanamachine @janmonschke @ashokaditya