-
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][Exceptions] - Fixes exceptions builder nested deletion issue and adds unit tests #74250
Conversation
@@ -34,7 +34,6 @@ interface EntryItemProps { | |||
indexPattern: IIndexPattern; | |||
showLabel: boolean; | |||
listType: ExceptionListType; | |||
addNested: boolean; |
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.
Wasn't being used anywhere.
Pinging @elastic/siem (Team:SIEM) |
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.
Thanks for finding and fixing this deletion bug! Pulled down the branch and verified the bug is fixed. LGTM.
Updated tags to include 7.9 as this was deemed a necessary bug fix to be backported. |
x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.tsx
Outdated
Show resolved
Hide resolved
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.
Had a few nits but this looks to be working as I expect it to. I don't have as much experience with this UI, though, so I defer to @madirey and @peluja1012 here.
x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.tsx
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/common/components/exceptions/builder/and_badge.test.tsx
Outdated
Show resolved
Hide resolved
); | ||
|
||
if (entryIndex === 0 && exceptionItemIndex === 0 && nestedParentIndex == null) { | ||
// This logic was added to work around it including the field |
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.
Could this be accomplished in CSS with e.g. a :first-of-type
selector?
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.
Yes, most likely! I've jotted down to follow up on. Thanks!
x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.test.tsx
Outdated
Show resolved
Hide resolved
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
async chunks size
History
To update your PR or re-run it, just comment with: |
…etion issue and adds unit tests (elastic#74250) ### Summary - Updates logic for deleting exception item entries in the builder - found that there was a bug in deleting nested entries - Adds more unit tests
…etion issue and adds unit tests (elastic#74250) ### Summary - Updates logic for deleting exception item entries in the builder - found that there was a bug in deleting nested entries - Adds more unit tests
* master: (74 commits) [Discover] Inline noWhiteSpace function (elastic#74331) [DOCS] Add Observability topic (elastic#73041) skip flaky suite (elastic#74327) [Security Solution][Detections] Fixes Severity Override not matching for Elastic Endpoint Security rule (elastic#74317) [Security Solution][Exceptions] - Fixes exceptions builder nested deletion issue and adds unit tests (elastic#74250) Fixed Alert details does not update page title and breadcrumb (elastic#74214) [src/dev/build] build Kibana Platform bundles from source (elastic#73591) [Reporting] Shorten asset path to help CLI FS Watcher (elastic#74185) Fix TMS not loaded in legacy maps (elastic#73570) [Security Solution] styling for notes' panel (elastic#74274) [Security Solution][Tech Debt] cleans up ts-ignore issues and some smaller linter issues (elastic#74268) Make the actions plugin support generics (elastic#71439) [Security Solution] Keep original note creator (elastic#74203) [CI] Fix xpack kibana build dir in xpack visual regression script [CI] Fix baseline_capture job by adding parallel process number back [Monitoring] Ensure setup mode works on cloud but only for alerts (elastic#73127) [Maps] Custom color ramps should show correctly on the map for mvt layers (elastic#74169) [kbn/optimizer] remove unused modules (elastic#74195) [CI] Add pipeline task queue framework and merge workers into one (elastic#71268) Using msearch for tree api endpoint (elastic#73813) ...
* master: (115 commits) [Logs UI] Correct trial period duration in anomaly splash screen (elastic#74249) [Discover] Inline noWhiteSpace function (elastic#74331) [DOCS] Add Observability topic (elastic#73041) skip flaky suite (elastic#74327) [Security Solution][Detections] Fixes Severity Override not matching for Elastic Endpoint Security rule (elastic#74317) [Security Solution][Exceptions] - Fixes exceptions builder nested deletion issue and adds unit tests (elastic#74250) Fixed Alert details does not update page title and breadcrumb (elastic#74214) [src/dev/build] build Kibana Platform bundles from source (elastic#73591) [Reporting] Shorten asset path to help CLI FS Watcher (elastic#74185) Fix TMS not loaded in legacy maps (elastic#73570) [Security Solution] styling for notes' panel (elastic#74274) [Security Solution][Tech Debt] cleans up ts-ignore issues and some smaller linter issues (elastic#74268) Make the actions plugin support generics (elastic#71439) [Security Solution] Keep original note creator (elastic#74203) [CI] Fix xpack kibana build dir in xpack visual regression script [CI] Fix baseline_capture job by adding parallel process number back [Monitoring] Ensure setup mode works on cloud but only for alerts (elastic#73127) [Maps] Custom color ramps should show correctly on the map for mvt layers (elastic#74169) [kbn/optimizer] remove unused modules (elastic#74195) [CI] Add pipeline task queue framework and merge workers into one (elastic#71268) ...
…etion issue and adds unit tests (#74250) (#74321) ### Summary - Updates logic for deleting exception item entries in the builder - found that there was a bug in deleting nested entries - Adds more unit tests Co-authored-by: Elastic Machine <[email protected]>
* master: (154 commits) [ML] Fix initial plugin's bundle size (elastic#74047) [Ingest Manager] prevent crash on unhandled rejection from setupIngestManager (elastic#74300) [Logs UI] Correct trial period duration in anomaly splash screen (elastic#74249) [Discover] Inline noWhiteSpace function (elastic#74331) [DOCS] Add Observability topic (elastic#73041) skip flaky suite (elastic#74327) [Security Solution][Detections] Fixes Severity Override not matching for Elastic Endpoint Security rule (elastic#74317) [Security Solution][Exceptions] - Fixes exceptions builder nested deletion issue and adds unit tests (elastic#74250) Fixed Alert details does not update page title and breadcrumb (elastic#74214) [src/dev/build] build Kibana Platform bundles from source (elastic#73591) [Reporting] Shorten asset path to help CLI FS Watcher (elastic#74185) Fix TMS not loaded in legacy maps (elastic#73570) [Security Solution] styling for notes' panel (elastic#74274) [Security Solution][Tech Debt] cleans up ts-ignore issues and some smaller linter issues (elastic#74268) Make the actions plugin support generics (elastic#71439) [Security Solution] Keep original note creator (elastic#74203) [CI] Fix xpack kibana build dir in xpack visual regression script [CI] Fix baseline_capture job by adding parallel process number back [Monitoring] Ensure setup mode works on cloud but only for alerts (elastic#73127) [Maps] Custom color ramps should show correctly on the map for mvt layers (elastic#74169) ...
Pinging @elastic/security-solution (Team: SecuritySolution) |
Summary
Bug
Fix
Checklist