-
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
[Security Solution] remove isDraggable props + various related cleanups #207959
Conversation
6ec8642
to
15a5642
Compare
15a5642
to
71b7f16
Compare
c1d469a
to
c7d6b5e
Compare
Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations) |
Files by Code Ownerelastic/kibana-localization
elastic/kibana-operations
elastic/security-detection-engine
elastic/security-engineering-productivity
elastic/security-entity-analytics
elastic/security-generative-ai
elastic/security-solution
elastic/security-threat-hunting-explore
elastic/security-threat-hunting-investigations
|
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.
elastic/security-generative-ai
changes 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.
packages/kbn-babel-preset/styled_components_files.js
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.
Desk tested relevant areas in Entity Analytics for current isDraggable usage—can confirm it’s not in use. Great work, thanks! 🎉
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.
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.
Threat Hunting Explore changes 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.
Awesome clean up! Investigations changes LGTM, just had a question about a cypress test
@@ -60,10 +59,10 @@ describe('Alerts cell actions', { tags: ['@ess', '@serverless'] }, () => { | |||
.first() | |||
.invoke('text') | |||
.then((severityVal) => { | |||
filterForAlertProperty(ALERT_TABLE_SEVERITY_VALUES, 0); | |||
filterForAlertProperty(ALERT_TABLE_SEVERITY_VALUES, 1); |
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.
why did the number change?
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.
because I changed the way the dom element was queried. With the simplification, it removed some extra unused dom elements which had a data-test-subj
that was used. To keep things simple and not introduce new code, I used another existing data-test-subj
, but that one is on multiple elements on the page (in the KPI chart first then in the alerts table).
It is not the most elegant way to solve this, but it was the only way I found without modifying application's code.
c7d6b5e
to
e0d861f
Compare
💛 Build succeeded, but was flaky
Failed CI Steps
Test Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
History
|
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/13188628402 |
💔 All backports failed
Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…ps (elastic#207959) ## Summary This PR removes the `isDraggable` prop throughout Security Solution. Unless I'm mistaken, this property isn't necessary anymore, as we do not use those draggable elements anymore. From what I could see, we had its value set to `false` everywhere. This lead to a lot of files impacted, but most of them have only a couple of lines changed. In some files though, removing the `isDraggable` prop allowed to remove more code than became obsolete. **No UI changes should have been introduced in this PR!** ### What this PR does - removes `isDraggable` everywhere - performs the extra small cleanup when obvious - updates all corresponding unit e2e and tests ### What this PR does - rename files or component names to limit the already extensive impact of the code change (cherry picked from commit ebb31d2) # Conflicts: # x-pack/solutions/security/plugins/security_solution/public/detections/components/alerts_kpis/severity_level_panel/columns.tsx # x-pack/solutions/security/plugins/security_solution/public/one_discover/cell_renderers/cell_renderer.test.tsx # x-pack/solutions/security/plugins/security_solution/public/one_discover/cell_renderers/cell_renderers.tsx # x-pack/solutions/security/plugins/security_solution/public/timelines/components/netflow/__snapshots__/index.test.tsx.snap # x-pack/solutions/security/plugins/security_solution/public/timelines/components/timeline/body/renderers/netflow/__snapshots__/netflow_row_renderer.test.tsx.snap # x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/alerts_cell_actions.cy.ts
…ps (elastic#207959) ## Summary This PR removes the `isDraggable` prop throughout Security Solution. Unless I'm mistaken, this property isn't necessary anymore, as we do not use those draggable elements anymore. From what I could see, we had its value set to `false` everywhere. This lead to a lot of files impacted, but most of them have only a couple of lines changed. In some files though, removing the `isDraggable` prop allowed to remove more code than became obsolete. **No UI changes should have been introduced in this PR!** ### What this PR does - removes `isDraggable` everywhere - performs the extra small cleanup when obvious - updates all corresponding unit e2e and tests ### What this PR does - rename files or component names to limit the already extensive impact of the code change
…ps (elastic#207959) ## Summary This PR removes the `isDraggable` prop throughout Security Solution. Unless I'm mistaken, this property isn't necessary anymore, as we do not use those draggable elements anymore. From what I could see, we had its value set to `false` everywhere. This lead to a lot of files impacted, but most of them have only a couple of lines changed. In some files though, removing the `isDraggable` prop allowed to remove more code than became obsolete. **No UI changes should have been introduced in this PR!** ### What this PR does - removes `isDraggable` everywhere - performs the extra small cleanup when obvious - updates all corresponding unit e2e and tests ### What this PR does - rename files or component names to limit the already extensive impact of the code change
…ps (elastic#207959) ## Summary This PR removes the `isDraggable` prop throughout Security Solution. Unless I'm mistaken, this property isn't necessary anymore, as we do not use those draggable elements anymore. From what I could see, we had its value set to `false` everywhere. This lead to a lot of files impacted, but most of them have only a couple of lines changed. In some files though, removing the `isDraggable` prop allowed to remove more code than became obsolete. **No UI changes should have been introduced in this PR!** ### What this PR does - removes `isDraggable` everywhere - performs the extra small cleanup when obvious - updates all corresponding unit e2e and tests ### What this PR does - rename files or component names to limit the already extensive impact of the code change (cherry picked from commit ebb31d2) # Conflicts: # x-pack/solutions/security/plugins/security_solution/public/detections/components/alerts_kpis/severity_level_panel/columns.tsx # x-pack/solutions/security/plugins/security_solution/public/one_discover/cell_renderers/cell_renderer.test.tsx # x-pack/solutions/security/plugins/security_solution/public/one_discover/cell_renderers/cell_renderers.tsx # x-pack/solutions/security/plugins/security_solution/public/timelines/components/netflow/__snapshots__/index.test.tsx.snap # x-pack/solutions/security/plugins/security_solution/public/timelines/components/timeline/body/renderers/netflow/__snapshots__/netflow_row_renderer.test.tsx.snap # x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/alerts_cell_actions.cy.ts
…cleanups (#207959) (#210116) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution] remove isDraggable props + various related cleanups (#207959)](#207959) <!--- Backport version: 9.6.4 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Philippe Oberti","email":"philippe.oberti@elastic.co"},"sourceCommit":{"committedDate":"2025-02-06T21:56:09Z","message":"[Security Solution] remove isDraggable props + various related cleanups (#207959)\n\n## Summary\r\n\r\nThis PR removes the `isDraggable` prop throughout Security Solution.\r\nUnless I'm mistaken, this property isn't necessary anymore, as we do not\r\nuse those draggable elements anymore. From what I could see, we had its\r\nvalue set to `false` everywhere.\r\n\r\nThis lead to a lot of files impacted, but most of them have only a\r\ncouple of lines changed. In some files though, removing the\r\n`isDraggable` prop allowed to remove more code than became obsolete.\r\n\r\n**No UI changes should have been introduced in this PR!**\r\n\r\n### What this PR does\r\n\r\n- removes `isDraggable` everywhere\r\n- performs the extra small cleanup when obvious\r\n- updates all corresponding unit e2e and tests\r\n\r\n### What this PR does\r\n\r\n- rename files or component names to limit the already extensive impact\r\nof the code change","sha":"ebb31d249f5d940fac6188230c03b8435ddb26ca","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Threat Hunting:Investigations","backport:version","v9.1.0","v8.19.0"],"title":"[Security Solution] remove isDraggable props + various related cleanups","number":207959,"url":"https://github.com/elastic/kibana/pull/207959","mergeCommit":{"message":"[Security Solution] remove isDraggable props + various related cleanups (#207959)\n\n## Summary\r\n\r\nThis PR removes the `isDraggable` prop throughout Security Solution.\r\nUnless I'm mistaken, this property isn't necessary anymore, as we do not\r\nuse those draggable elements anymore. From what I could see, we had its\r\nvalue set to `false` everywhere.\r\n\r\nThis lead to a lot of files impacted, but most of them have only a\r\ncouple of lines changed. In some files though, removing the\r\n`isDraggable` prop allowed to remove more code than became obsolete.\r\n\r\n**No UI changes should have been introduced in this PR!**\r\n\r\n### What this PR does\r\n\r\n- removes `isDraggable` everywhere\r\n- performs the extra small cleanup when obvious\r\n- updates all corresponding unit e2e and tests\r\n\r\n### What this PR does\r\n\r\n- rename files or component names to limit the already extensive impact\r\nof the code change","sha":"ebb31d249f5d940fac6188230c03b8435ddb26ca"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/207959","number":207959,"mergeCommit":{"message":"[Security Solution] remove isDraggable props + various related cleanups (#207959)\n\n## Summary\r\n\r\nThis PR removes the `isDraggable` prop throughout Security Solution.\r\nUnless I'm mistaken, this property isn't necessary anymore, as we do not\r\nuse those draggable elements anymore. From what I could see, we had its\r\nvalue set to `false` everywhere.\r\n\r\nThis lead to a lot of files impacted, but most of them have only a\r\ncouple of lines changed. In some files though, removing the\r\n`isDraggable` prop allowed to remove more code than became obsolete.\r\n\r\n**No UI changes should have been introduced in this PR!**\r\n\r\n### What this PR does\r\n\r\n- removes `isDraggable` everywhere\r\n- performs the extra small cleanup when obvious\r\n- updates all corresponding unit e2e and tests\r\n\r\n### What this PR does\r\n\r\n- rename files or component names to limit the already extensive impact\r\nof the code change","sha":"ebb31d249f5d940fac6188230c03b8435ddb26ca"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
Summary
This PR removes the
isDraggable
prop throughout Security Solution. Unless I'm mistaken, this property isn't necessary anymore, as we do not use those draggable elements anymore. From what I could see, we had its value set tofalse
everywhere.This lead to a lot of files impacted, but most of them have only a couple of lines changed. In some files though, removing the
isDraggable
prop allowed to remove more code than became obsolete.No UI changes should have been introduced in this PR!
What this PR does
isDraggable
everywhereWhat this PR does