Move callouts to detection engine and common#233850
Merged
PhilippeOberti merged 7 commits intoelastic:mainfrom Sep 8, 2025
Merged
Move callouts to detection engine and common#233850PhilippeOberti merged 7 commits intoelastic:mainfrom
PhilippeOberti merged 7 commits intoelastic:mainfrom
Conversation
1d13734 to
4ba712d
Compare
4 tasks
02a1e08 to
6f42226
Compare
Contributor
|
Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations) |
Contributor
Author
Files by Code Ownerelastic/security-detection-engine
elastic/security-detection-rule-management
elastic/security-entity-analytics
elastic/security-solution
elastic/security-threat-hunting
elastic/security-threat-hunting-investigations
|
machadoum
approved these changes
Sep 5, 2025
Member
machadoum
left a comment
There was a problem hiding this comment.
Entity Analytics code LGTM!
NicholasPeretti
approved these changes
Sep 5, 2025
jkelas
reviewed
Sep 8, 2025
Contributor
jkelas
left a comment
There was a problem hiding this comment.
@PhilippeOberti this LGTM, but I left 2 comments / questions. Please take a look.
Contributor
There was a problem hiding this comment.
The exported functions from this file are only used for detection_rules. I think that in that case this file should rather reside in detection_engine/rule_management. Would you agree?
Contributor
Author
There was a problem hiding this comment.
I was not sure, but yes that makes sense. Will make the changes in a few minutes :)
Contributor
There was a problem hiding this comment.
I am sorry @PhilippeOberti but I think I overlooked something and mislead you with my previous comment. I can see that the 'transforms' is also used in rule_creation_ui. Which means that the previous place, the common folder, was correct. I apologize for the confusion.
Contributor
Author
There was a problem hiding this comment.
All good, I was suspecting but thought there was a good reason for moving it up... 😆
Reverted in 408b60e
Contributor
There was a problem hiding this comment.
I think this file should rather be placed in 'common' as this is used not only for rules, but also for alerts and exceptions.
Contributor
Author
There was a problem hiding this comment.
Yup, I'll take another look in a few minutes, thanks!
Contributor
Author
There was a problem hiding this comment.
Done in 1db8c71.
Sorry it took a bit longer than expect... I noticed that there was another missing_privileges folder under public/common but not under public/common/components. I reached out to who created created it and it was a mistake. The 2 folders are now merged, I added some description to the components and the missing unit tests :)
Contributor
There was a problem hiding this comment.
It looks like a great improvement @PhilippeOberti .
However I noticed that you also moved the use_missing_privileges to the common/components/missing_priviledges. I think this hook had its place inside a folder named hooks, and it should remain so, thus it should be in common/hooks. Would you agree?
Contributor
Author
There was a problem hiding this comment.
I thought about it but didn't like it that much. I'm not opposed to it though, so I will make the change in an hour or so!
Contributor
Author
vitaliidm
approved these changes
Sep 8, 2025
Contributor
vitaliidm
left a comment
There was a problem hiding this comment.
detection engine area LGTM
01ac00b to
1db8c71
Compare
jkelas
approved these changes
Sep 8, 2025
Contributor
jkelas
left a comment
There was a problem hiding this comment.
LGTM. My comments were addressed, I am approving
65ff6a4 to
a81b716
Compare
Contributor
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
|
eleonoramicozzi
pushed a commit
to eleonoramicozzi/kibana
that referenced
this pull request
Sep 10, 2025
## Summary This PR is a follow up of the big [alerts page refactor one](elastic#222457). Please let me know if some of the changes do not make sense! 😄 ### Context @maximpn had rightfully [pointed out](elastic#222457 (comment)) that some files should be moved into different folder and the CODEOWNERS file should be updated accordingly. I originally had done the change in the previous PR, but this increased the scope and added new codeowners that needed to approve, at a time where the PR was ready to be merged. ### Changes This PR only moves files around and updated the related imports. No other code changes are introduced, and therefore no UI or logic is changed. _Edit: some files were originally moved to the `detection_engine` folder, but I was rightfully pointed out that some should belong to the `common` folder. When doing the move, I realized we had other similar components. I merged the folders together and added missing unit tests to the other files._ ### 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 - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Contributor
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
KodeRad
pushed a commit
to KodeRad/kibana
that referenced
this pull request
Sep 15, 2025
## Summary This PR is a follow up of the big [alerts page refactor one](elastic#222457). Please let me know if some of the changes do not make sense! 😄 ### Context @maximpn had rightfully [pointed out](elastic#222457 (comment)) that some files should be moved into different folder and the CODEOWNERS file should be updated accordingly. I originally had done the change in the previous PR, but this increased the scope and added new codeowners that needed to approve, at a time where the PR was ready to be merged. ### Changes This PR only moves files around and updated the related imports. No other code changes are introduced, and therefore no UI or logic is changed. _Edit: some files were originally moved to the `detection_engine` folder, but I was rightfully pointed out that some should belong to the `common` folder. When doing the move, I realized we had other similar components. I merged the folders together and added missing unit tests to the other files._ ### 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 - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
CAWilson94
pushed a commit
to CAWilson94/kibana
that referenced
this pull request
Sep 24, 2025
## Summary This PR is a follow up of the big [alerts page refactor one](elastic#222457). Please let me know if some of the changes do not make sense! 😄 ### Context @maximpn had rightfully [pointed out](elastic#222457 (comment)) that some files should be moved into different folder and the CODEOWNERS file should be updated accordingly. I originally had done the change in the previous PR, but this increased the scope and added new codeowners that needed to approve, at a time where the PR was ready to be merged. ### Changes This PR only moves files around and updated the related imports. No other code changes are introduced, and therefore no UI or logic is changed. _Edit: some files were originally moved to the `detection_engine` folder, but I was rightfully pointed out that some should belong to the `common` folder. When doing the move, I realized we had other similar components. I merged the folders together and added missing unit tests to the other files._ ### 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 - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
niros1
pushed a commit
that referenced
this pull request
Sep 30, 2025
## Summary This PR is a follow up of the big [alerts page refactor one](#222457). Please let me know if some of the changes do not make sense! 😄 ### Context @maximpn had rightfully [pointed out](#222457 (comment)) that some files should be moved into different folder and the CODEOWNERS file should be updated accordingly. I originally had done the change in the previous PR, but this increased the scope and added new codeowners that needed to approve, at a time where the PR was ready to be merged. ### Changes This PR only moves files around and updated the related imports. No other code changes are introduced, and therefore no UI or logic is changed. _Edit: some files were originally moved to the `detection_engine` folder, but I was rightfully pointed out that some should belong to the `common` folder. When doing the move, I realized we had other similar components. I merged the folders together and added missing unit tests to the other files._ ### 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 - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR is a follow up of the big alerts page refactor one.
Please let me know if some of the changes do not make sense! 😄
Context
@maximpn had rightfully pointed out that some files should be moved into different folder and the CODEOWNERS file should be updated accordingly. I originally had done the change in the previous PR, but this increased the scope and added new codeowners that needed to approve, at a time where the PR was ready to be merged.
Changes
This PR only moves files around and updated the related imports. No other code changes are introduced, and therefore no UI or logic is changed.
Edit: some files were originally moved to the
detection_enginefolder, but I was rightfully pointed out that some should belong to thecommonfolder. When doing the move, I realized we had other similar components. I merged the folders together and added missing unit tests to the other files.Checklist
release_note:*label is applied per the guidelinesbackport:*labels.