Keep volatile information out of stored search source #91517
Conversation
flash1293
added
Feature:Discover
|
Pinging @elastic/kibana-app (Team:KibanaApp) |
| @@ -169,10 +169,10 @@ function discoverController($route, $scope, Promise) { | |||
| let inspectorRequest; | |||
| let isChangingIndexPattern = false; | |||
| const savedSearch = $route.current.locals.savedObjects.savedSearch; | |||
| $scope.searchSource = savedSearch.searchSource; | |||
| $scope.persistentSearchSource = savedSearch.searchSource; | |||
There was a problem hiding this comment.
It seems to me this is not used anywhere outside the current scope? Could we already not store that on $scope and make it a local variable instead?
There was a problem hiding this comment.
Good point, fixed
| @@ -16,7 +16,8 @@ import { DiscoverServices } from '../../build_services'; | |||
| * Helper function to update the given searchSource before fetching/sharing/persisting | |||
| */ | |||
| export function updateSearchSource( | |||
| searchSource: ISearchSource, | |||
| persistentSearchSource: ISearchSource, | |||
| volatileSearchSource: ISearchSource | undefined, | |||
There was a problem hiding this comment.
🎨 Would also allow us not to need to pass undefined explicitly to this method above.
| volatileSearchSource: ISearchSource | undefined, | |
| volatileSearchSource?: ISearchSource, |
There was a problem hiding this comment.
That's not possible because the third argument is mandatory. I refactored it to just take a single object with all parameters instead.
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
|
|
||
| // searchSource which applies time range | ||
| const timeRangeSearchSource = savedSearch.searchSource.create(); | ||
| const volatileSearchSource = savedSearch.searchSource.create(); |
There was a problem hiding this comment.
Should these changes be applied to embeddable as well?
There was a problem hiding this comment.
As mentioned in the description:
The saved search embeddable is still polluting the original search source, but it doesn't matter here because it's never saved.
We can structure it the same way there, but it's not related to the issue of polluted saved objects because the embeddable search source is never saved. To keep the scope small I would like to do these things in 7.13
There was a problem hiding this comment.
Sorry, missed this line in the description
Fixes #91459
Separates the search source in discover into two separate sources - the persistent search source tied to the saved object containing stuff which should be saved (filters, query) and the volatile search source which is a child of the persistent one containing all other things (size, highlight, sorting, fields, ...)
CSV export gets passed the volatile search source so it's operating on the very same state (minus some specific cleanup)
There is one weird thing but I guess we can open a separate issue for this - when creating a reporting job from discover, index pattern runtime fields work correctly, because the
runtime_mappingpart of the dsl object becomes part of the job params. On dashboards, this part of the dsl object is not used (kibana/x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.tsx
Line 104 in 4584a8b
The saved search embeddable is still polluting the original search source, but it doesn't matter here because it's never saved.