[Security Solution] Multi level grouping for alerts table #152862
Conversation
…ultilevel_grouping # Conflicts: # packages/kbn-securitysolution-grouping/src/components/grouping.tsx # packages/kbn-securitysolution-grouping/src/components/types.ts # packages/kbn-securitysolution-grouping/src/hooks/use_get_group_selector.tsx # packages/kbn-securitysolution-grouping/src/hooks/use_grouping.tsx # x-pack/plugins/security_solution/public/detections/components/alerts_table/alerts_grouping.tsx
| ); | ||
| const onButtonClick = useCallback(() => { | ||
| if (!isPopoverOpen && takeActionItems.length === 0) { | ||
| setTakeActionItems(getTakeActionItems(groupFilter, groupNumber)); |
There was a problem hiding this comment.
I moved take action component rendering to be on click. It was previously passed to the component in grouping.tsx variable called groupPanels. This caused multiple performance issues rendering each action for each group despite them not being shown yet, and on top of that groupPanels re-renders a lot. Some weird side effects avoided too just by moving this here.
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
| return getLevel( | ||
| level + 1, | ||
| selectedGroups[level + 1], | ||
| JSON.stringify([ |
There was a problem hiding this comment.
Why do these have to be stringified to only be parsed in a recursive next step?
There was a problem hiding this comment.
If it is passed as an object it will cause unnecessary re-rendering
| setAlertsQuery(queryGroups); | ||
| } | ||
| }, [queryGroups, selectedGroup, setAlertsQuery]); | ||
| resetAllPagination(); |
There was a problem hiding this comment.
What's the difference between this reset and the one on line 174? Is there a path for combining them so reset happens in one place?
There was a problem hiding this comment.
This triggers a reset when the selectedGroup is changed. The call on 174 resets when the global filters change. The global filters come from security solution and the selected group comes from grouping package. I will combine them in the next PR
|
Fyi, just came across this bug: #155243 not sure if we'd want to disable column toggling or toggle columns for all table levels? |
|
@elasticmachine merge upstream |
💛 Build succeeded, but was flakyFailed CI StepsMetrics [docs]Public APIs missing comments
Async chunks
Public APIs missing exports
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
…2862) ## Multi Level Grouping Resolves elastic#150516 Resolves elastic#150514 Implements multi level grouping in Alerts table and Rule details table. Supports 3 levels deep. https://user-images.githubusercontent.com/6935300/232547389-7d778f69-d96d-4bd8-8560-f5ddd9fe8060.mov ### Test plan https://docs.google.com/document/d/15oseanNzF-u-Xeoahy1IVxI4oV3wOuO8VhA886cA1U8/edit# ### To do - [Cypress](elastic#150666) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Steph Milovic <stephanie.milovic@elastic.co>
Multi Level Grouping
Resolves #150516
Resolves #150514
Implements multi level grouping in Alerts table and Rule details table. Supports 3 levels deep.
grouping.mov
Test plan
https://docs.google.com/document/d/15oseanNzF-u-Xeoahy1IVxI4oV3wOuO8VhA886cA1U8/edit#
To do