[Security Solution][Investigations][Tech Debt] - FilterManager usage refactor

michaelolo24 commented 2 years ago

Background:

As with most other applications in Kibana, the security_solution is making use of the FilterManager for managing the filter state associated with the KQL Search Bar. Within the security_solution though, there are two instances of the SearchBar that are maintained. One in the main security_solution application seen on the Overview, Alerts, Rule Details, Hosts, and Network pages and then the other within the Timeline flyout. Both of which are visible in the below video:

https://user-images.githubusercontent.com/17211684/152554993-fa920db6-8ca3-401a-84ce-d13eda9dae49.mov

There have been a slew of bugs either directly or indirectly associated with the filterManager such as:

https://github.com/elastic/kibana/issues/75142 https://github.com/elastic/kibana/issues/116395 https://github.com/elastic/kibana/issues/108165 https://github.com/elastic/kibana/issues/123838

The goal of this PR will be to develop a consistent pattern for managing this state alongside redux and refactoring any existing calls to this new pattern.

As a reference the tests located here: https://github.com/elastic/kibana/blob/main/src/plugins/data/public/query/filter_manager/filter_manager.test.ts may provide helpful insight into how to manage this state

Within the following files we either create new instances of a FilterManager through new FilterManager() or check for a given filterManager associated with either the main app or timeline. On top of that, there is a concept of a filterManagerBackup which is probably unnecessary with better initialization of the filterManager. A decision will need to be made about how to separate the primary filterManager from the timeline flyout one (see: TimelineId.active) and whether to store it in redux or localStorage at all, but the below files as well as the filterManager slice of the timeline state should be refactored to adhere to a more deterministic pattern.

x-pack/plugins/security_solution/public/common/components/hover_actions/use_hover_action_items.tsx
x-pack/plugins/security_solution/public/detections/components/alerts_table/timeline_actions/use_investigate_in_timeline.tsx
x-pack/plugins/security_solution/public/timelines/components/timeline/query_tab_content/index.tsx
x-pack/plugins/security_solution/public/detections/components/rules/description_step/index.tsx
`x-pack/plugins/security_solution/public/detections/components/rules/query_bar/index.tsx

Tasks:

### Tasks
- [ ] Investigate updating the core data service to allow for multiple instances of `FilterManager`. This will involve managing storing multiple instances in url state. (https://github.com/elastic/kibana/blob/main/src/plugins/data/public/query/filter_manager/filter_manager.ts)
- [ ] https://github.com/elastic/kibana/issues/178238

Acceptance Criteria

[ ] Existing functionality prior to code removal should remain
[ ] Cypress tests
- [ ] Filter in and filter out work as expected
- [ ] Filters are retained across pages
- [ ] Main App Filter does not conflict/interfere with Timeline Filter
- [ ] Filters are retained on refresh
- [ ] Filters are retained when opened in a new window
- [ ] Filters are not retained when crossing app boundaries
[ ] Untested unit tests paths should have unit tests added

kqualters-elastic commented 2 years ago

started looking at this here https://github.com/elastic/kibana/pull/119129

elasticmachine commented 2 years ago

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

kqualters-elastic commented 2 years ago

This is probably a bigger issue than I realized, as each timeline in state as an instance of this massive object in state and can also behave unexpectedly https://redux.js.org/style-guide/#do-not-put-non-serializable-values-in-state-or-actions

michaelolo24 commented 7 months ago

@lgestc can this ticket be closed or is there additional work being tracked?

elastic / kibana