[Security Solution] Replace sourcerer in analyzer#218183
[Security Solution] Replace sourcerer in analyzer#218183christineweng merged 4 commits intoelastic:mainfrom
Conversation
|
Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations) |
| /** | ||
| * Optional callback when data view picker is closed | ||
| */ | ||
| onClosePopover?: () => void; |
There was a problem hiding this comment.
Can you provide use cases background on where onClosePopover would be used?
This seems like an odd use case to have a popover open another popover and then having to pass onClosePopover to close these cascading popovers.
There was a problem hiding this comment.
Just some older design we need to support here I guess
There was a problem hiding this comment.
@nreese yeah this was added to fix a UI bug. In analyzer, the dataview picker icon persists after opening any flyout
Screen.Recording.2025-04-15.at.9.57.58.AM.mov
There was a problem hiding this comment.
Since the data view picker is the only thing displayed in the first popover, what about just displaying the change data view popover when users click the original icon button to avoid pop-overs opening other popovers?
There was a problem hiding this comment.
@nreese Thanks! I will consult the UIUX team on this.
Currently the button and the popover content are tightly coupled in change_dataview.tsx. To separate them and exposing both options will require a refactor of that file. Is this what you are suggesting?
There was a problem hiding this comment.
We actually see lots of similar behaviors throughout Kibana, for example when popover are used in flyouts... Having a onClosePopover callback isn't a new pattern (I'm not saying it is a good one 😉).
It seems that we would have to do some good amount of refactor in order to get this to work without the onClosePopover callback, which increase the risk of breaking existing functionality.
@nreese are you really opposed to adding this callback? Even if it is a temporary solution to get us moving forward? I understand temporary is often a bad word, as things tend to usually be temporary for very long times... but such a callback does seem to me like a lot to maintain right?
There was a problem hiding this comment.
are you really opposed to adding this callback? Even if it is a temporary solution to get us moving forward? I understand temporary is often a bad word, as things tend to usually be temporary for very long times... but such a callback does seem to me like a lot to maintain right?
No, I just wanted to explore all of the options and better understand the problem space. I am fine with progress over perfection.
...ity/plugins/security_solution/public/data_view_manager/components/data_view_picker/index.tsx
Show resolved
Hide resolved
src/platform/plugins/shared/unified_search/public/dataview_picker/change_dataview.tsx
Show resolved
Hide resolved
nreese
left a comment
There was a problem hiding this comment.
kibana-presentation changes LGTM
code review only
ccb7cb7 to
5a1e681
Compare
5a1e681 to
581844a
Compare
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
|
| }; | ||
|
|
||
| const getPanelItems = () => { | ||
| const items = useMemo(() => { |
## Summary Replace sourcerer in analyzer to use dataview picker when `newDataViewPickerEnabled` is on.  ### Checklist - [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 - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
## Summary Replace sourcerer in analyzer to use dataview picker when `newDataViewPickerEnabled` is on.  ### Checklist - [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 - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
## Summary Replace sourcerer in analyzer to use dataview picker when `newDataViewPickerEnabled` is on.  ### Checklist - [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 - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
… Data View Picker (#210585) (#223044) # Backport This will backport the following commits from `main` to `8.19`: - [[Security Solution][Sourcerer] Replace Sourcerer with Discover Data View Picker (#210585)](#210585) - [[Security Solution] Rename use_data_view to use_data_view_spec #216461](#216461) - [[Security Solution] Rename use full data view hook #216614](#216614) - [[Security Solution] Replace sourcerer in global header #216685](#216685) - [[Security Solution] Remove .title use in use_selected_patterns #216994](#216994) - [[Security Solution] Render default security solution data view with managed label #216961](#216961) - [[Security Solution] Replace sourcerer in analyzer #218183](#218183) - [[Security Solution] Replace use_sourcerer_data_view #216997](#216997) - [[Security Solution] Replace sourcerer in EQL tab with dataview picker #218897](#218897) - [[Security Solution][Sourcerer] replace use get scoped data view #220196](#220196) - [[Security Solution] renaming dataView to dataViewSpec and adding types for clarity #220718](#220718) - [[Security Solution][Sourcerer] Maintain url sync support #221737](#221737) - [[Security Solution][Data View Manager] Allow passing data view to query bar #220585](#220585) - [[Security Solution] Fix data view picker privilege #222122](#222122) <!--- Backport version: 10.0.0 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Luke Gmys","email":"11671118+lgestc@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-03-31T12:12:57Z","message":"[Security Solution][Sourcerer] Replace Sourcerer with Discover Data View Picker (#210585)\n\n# Unified Data View Picker: Phase 1 Implementation\nPart of https://github.com/elastic/security-team/issues/11959\n\n## What This PR Does\nThis PR represents the first step in our transition from the current\nSourcerer component to the new unified Data View Picker. Specifically,\nthis implementation:\n- Creates a new Data View Picker component\n- Implements feature flag protection for all changes\n- Handles asynchronous effects through Redux listener middleware\n- Establishes a new Redux store architecture to support ad hoc data\nviews infrastructure\n- Utilizes ad hoc data views to handle legacy patterns from series 7\n(replacing the previous upgrade data view flow)\n\nSee the readme for more info: \n```x-pack/solutions/security/plugins/security_solution/public/data_view_manager/readme.md```\n\n## What This PR Does NOT Cover\n- Does not affect screens other than Timelines\n- Does not modify the existing Sourcerer component in any way\n- Does not fully support all URL/local storage patterns\n\n## Implementation Notes\nWe've made several accommodations to support both Sourcerer and the new Data View Picker simultaneously during this transition period, including:\n- Some interfaces might look odd, especially the hooks that return the data view or patterns - this is intentional to support existing use cases\n- There are feature flag-based conditional statements throughout the code that will be removed once the transition is complete\n\n## Testing Instructions\n1. Add the following feature flag to your configuration:\n ```\n xpack.securitySolution.enableExperimental: ['newDataViewPickerEnabled']\n ```\n2. Navigate to the Timelines interface\n3. Test interactions with the new Data View Picker\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"9679f2941550856d75e00c1faadd8c9669afe917","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","backport:skip","Team: SecuritySolution","Team:Threat Hunting:Investigations","Feature:Sourcerer","9.1 candidate","v9.1.0"],"title":"[Security Solution][Sourcerer] Replace Sourcerer with Discover Data View Picker","number":210585,"url":"https://github.com/elastic/kibana/pull/210585","mergeCommit":{"message":"[Security Solution][Sourcerer] Replace Sourcerer with Discover Data View Picker (#210585)\n\n# Unified Data View Picker: Phase 1 Implementation\nPart of https://github.com/elastic/security-team/issues/11959\n\n## What This PR Does\nThis PR represents the first step in our transition from the current\nSourcerer component to the new unified Data View Picker. Specifically,\nthis implementation:\n- Creates a new Data View Picker component\n- Implements feature flag protection for all changes\n- Handles asynchronous effects through Redux listener middleware\n- Establishes a new Redux store architecture to support ad hoc data\nviews infrastructure\n- Utilizes ad hoc data views to handle legacy patterns from series 7\n(replacing the previous upgrade data view flow)\n\nSee the readme for more info: \n```x-pack/solutions/security/plugins/security_solution/public/data_view_manager/readme.md```\n\n## What This PR Does NOT Cover\n- Does not affect screens other than Timelines\n- Does not modify the existing Sourcerer component in any way\n- Does not fully support all URL/local storage patterns\n\n## Implementation Notes\nWe've made several accommodations to support both Sourcerer and the new Data View Picker simultaneously during this transition period, including:\n- Some interfaces might look odd, especially the hooks that return the data view or patterns - this is intentional to support existing use cases\n- There are feature flag-based conditional statements throughout the code that will be removed once the transition is complete\n\n## Testing Instructions\n1. Add the following feature flag to your configuration:\n ```\n xpack.securitySolution.enableExperimental: ['newDataViewPickerEnabled']\n ```\n2. Navigate to the Timelines interface\n3. Test interactions with the new Data View Picker\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"9679f2941550856d75e00c1faadd8c9669afe917"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/210585","number":210585,"mergeCommit":{"message":"[Security Solution][Sourcerer] Replace Sourcerer with Discover Data View Picker (#210585)\n\n# Unified Data View Picker: Phase 1 Implementation\nPart of https://github.com/elastic/security-team/issues/11959\n\n## What This PR Does\nThis PR represents the first step in our transition from the current\nSourcerer component to the new unified Data View Picker. Specifically,\nthis implementation:\n- Creates a new Data View Picker component\n- Implements feature flag protection for all changes\n- Handles asynchronous effects through Redux listener middleware\n- Establishes a new Redux store architecture to support ad hoc data\nviews infrastructure\n- Utilizes ad hoc data views to handle legacy patterns from series 7\n(replacing the previous upgrade data view flow)\n\nSee the readme for more info: \n```x-pack/solutions/security/plugins/security_solution/public/data_view_manager/readme.md```\n\n## What This PR Does NOT Cover\n- Does not affect screens other than Timelines\n- Does not modify the existing Sourcerer component in any way\n- Does not fully support all URL/local storage patterns\n\n## Implementation Notes\nWe've made several accommodations to support both Sourcerer and the new Data View Picker simultaneously during this transition period, including:\n- Some interfaces might look odd, especially the hooks that return the data view or patterns - this is intentional to support existing use cases\n- There are feature flag-based conditional statements throughout the code that will be removed once the transition is complete\n\n## Testing Instructions\n1. Add the following feature flag to your configuration:\n ```\n xpack.securitySolution.enableExperimental: ['newDataViewPickerEnabled']\n ```\n2. Navigate to the Timelines interface\n3. Test interactions with the new Data View Picker\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"9679f2941550856d75e00c1faadd8c9669afe917"}}]}] BACKPORT--> --------- Co-authored-by: Luke Gmys <11671118+lgestc@users.noreply.github.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Philippe Oberti <philippe.oberti@elastic.co>
Summary
Replace sourcerer in analyzer to use dataview picker when
newDataViewPickerEnabledis on.Checklist
release_note:*label is applied per the guidelines