-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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.
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
Feature/remove asset criticality flag #196270
Feature/remove asset criticality flag #196270
Conversation
…cality flag from explore host and users paginated table components
…oved; flag fully removed in routes
…on removed in common functions (getItems); assertAdvancedSettings check removed for asset criticality where used.
…CriticalityEnabled check in calc risk scores
…cality flag from explore host and users paginated table components
…oved; flag fully removed in routes
…on removed in common functions (getItems); assertAdvancedSettings check removed for asset criticality where used.
…CriticalityEnabled check in calc risk scores
Fix CI issues
Fix last broken api test
Another one bites the dust
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Just a minor comment
@@ -82,17 +81,9 @@ const FlyoutRiskSummaryComponent = <T extends RiskScoreEntity>({ | |||
|
|||
const xsFontSize = useEuiFontSize('xxs').fontSize; | |||
|
|||
const [isAssetCriticalityEnabled] = useUiSetting$<boolean>(ENABLE_ASSET_CRITICALITY_SETTING); | |||
const columns = useMemo(() => buildColumns(), []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since there's no dependencies on buildColumns
anymore, we can just define a variable outside of a react component. In fact, maybe it doesn't need to be a function anymore, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I see what you mean, I think you are right - can change that to a variable instead :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally, great work 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DE Code change LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Detection Engine changes LGTM! Nice to have this all cleaned up, great work 👍
@elasticmachine merge upstream |
1 similar comment
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
|
Starting backport for target branches: 8.x |
## Summary It removes the asset criticality advanced setting, which enables the feature by default for all users. Deleted settings: ![Screenshot 2024-10-15 at 14 54 48](https://github.com/user-attachments/assets/103c3f04-fd7e-45cf-ac74-93e1eef341fa) ### How to test it? * Start Kibana with security data * Inside security solution / manage, you should be able to find the Asset Criticality page ![Screenshot 2024-10-15 at 14 57 14](https://github.com/user-attachments/assets/7ddcee91-ad76-4d8f-b14a-bacc4ba31172) * You should see the asset critically section when opening an entity flyout (explore or host page) <img width="400" src="https://github.com/user-attachments/assets/3a9ee545-566c-4687-af16-f31bd93bdc20" /> * The risk score should be updated if you update an entity's asset criticality. ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [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 --------- Co-authored-by: machadoum <[email protected]> Co-authored-by: jaredburgettelastic <[email protected]> Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit 5ae7a61)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
# Backport This will backport the following commits from `main` to `8.x`: - [Feature/remove asset criticality flag (#196270)](#196270) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Charlotte Alexandra Wilson","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-16T00:29:35Z","message":"Feature/remove asset criticality flag (#196270)\n\n## Summary\r\n\r\nIt removes the asset criticality advanced setting, which enables the\r\nfeature by default for all users.\r\n\r\nDeleted settings:\r\n![Screenshot 2024-10-15 at 14 54\r\n48](https://github.com/user-attachments/assets/103c3f04-fd7e-45cf-ac74-93e1eef341fa)\r\n\r\n### How to test it?\r\n* Start Kibana with security data\r\n* Inside security solution / manage, you should be able to find the\r\nAsset Criticality page\r\n![Screenshot 2024-10-15 at 14 57\r\n14](https://github.com/user-attachments/assets/7ddcee91-ad76-4d8f-b14a-bacc4ba31172)\r\n* You should see the asset critically section when opening an entity\r\nflyout (explore or host page) <img width=\"400\"\r\nsrc=\"https://github.com/user-attachments/assets/3a9ee545-566c-4687-af16-f31bd93bdc20\"\r\n/>\r\n* The risk score should be updated if you update an entity's asset\r\ncriticality.\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: machadoum <[email protected]>\r\nCo-authored-by: jaredburgettelastic <[email protected]>\r\nCo-authored-by: Elastic Machine <[email protected]>","sha":"5ae7a61d935e3c1778ee830a5c1ee5055abf44a0","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["v9.0.0","release_note:feature","backport:prev-minor","Theme: entity_analytics","Feature:Entity Analytics","Team:Entity Analytics"],"title":"Feature/remove asset criticality flag","number":196270,"url":"https://github.com/elastic/kibana/pull/196270","mergeCommit":{"message":"Feature/remove asset criticality flag (#196270)\n\n## Summary\r\n\r\nIt removes the asset criticality advanced setting, which enables the\r\nfeature by default for all users.\r\n\r\nDeleted settings:\r\n![Screenshot 2024-10-15 at 14 54\r\n48](https://github.com/user-attachments/assets/103c3f04-fd7e-45cf-ac74-93e1eef341fa)\r\n\r\n### How to test it?\r\n* Start Kibana with security data\r\n* Inside security solution / manage, you should be able to find the\r\nAsset Criticality page\r\n![Screenshot 2024-10-15 at 14 57\r\n14](https://github.com/user-attachments/assets/7ddcee91-ad76-4d8f-b14a-bacc4ba31172)\r\n* You should see the asset critically section when opening an entity\r\nflyout (explore or host page) <img width=\"400\"\r\nsrc=\"https://github.com/user-attachments/assets/3a9ee545-566c-4687-af16-f31bd93bdc20\"\r\n/>\r\n* The risk score should be updated if you update an entity's asset\r\ncriticality.\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: machadoum <[email protected]>\r\nCo-authored-by: jaredburgettelastic <[email protected]>\r\nCo-authored-by: Elastic Machine <[email protected]>","sha":"5ae7a61d935e3c1778ee830a5c1ee5055abf44a0"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196270","number":196270,"mergeCommit":{"message":"Feature/remove asset criticality flag (#196270)\n\n## Summary\r\n\r\nIt removes the asset criticality advanced setting, which enables the\r\nfeature by default for all users.\r\n\r\nDeleted settings:\r\n![Screenshot 2024-10-15 at 14 54\r\n48](https://github.com/user-attachments/assets/103c3f04-fd7e-45cf-ac74-93e1eef341fa)\r\n\r\n### How to test it?\r\n* Start Kibana with security data\r\n* Inside security solution / manage, you should be able to find the\r\nAsset Criticality page\r\n![Screenshot 2024-10-15 at 14 57\r\n14](https://github.com/user-attachments/assets/7ddcee91-ad76-4d8f-b14a-bacc4ba31172)\r\n* You should see the asset critically section when opening an entity\r\nflyout (explore or host page) <img width=\"400\"\r\nsrc=\"https://github.com/user-attachments/assets/3a9ee545-566c-4687-af16-f31bd93bdc20\"\r\n/>\r\n* The risk score should be updated if you update an entity's asset\r\ncriticality.\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: machadoum <[email protected]>\r\nCo-authored-by: jaredburgettelastic <[email protected]>\r\nCo-authored-by: Elastic Machine <[email protected]>","sha":"5ae7a61d935e3c1778ee830a5c1ee5055abf44a0"}}]}] BACKPORT--> Co-authored-by: Charlotte Alexandra Wilson <[email protected]>
Summary
It removes the asset criticality advanced setting, which enables the feature by default for all users.
Deleted settings:
How to test it?
Checklist