[Discover] Show "Unsaved changes" badge on time filter changes in case time range is stored along with saved search #178659
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
| @@ -375,6 +375,14 @@ export function getDiscoverStateContainer({ | |||
| * state containers initializing and subscribing to changes triggering e.g. data fetching | |||
| */ | |||
| const initializeAndSync = () => { | |||
| const timefilerUnsubscribe = services.timefilter.getTimeUpdate$().subscribe(() => { | |||
There was a problem hiding this comment.
ℹ️ This needs to be the first thing that's wired up because the next line is pulling the current state from the URL which might change the time filter and thus needs to re-check whether the saved search has changed
| @@ -17,6 +17,8 @@ export const savedSearchMock = { | |||
| id: 'the-saved-search-id', | |||
| title: 'A saved search', | |||
| searchSource: createSearchSourceMock({ index: dataViewMock }), | |||
| columns: ['default_column'], | |||
There was a problem hiding this comment.
ℹ️ Making the mocked saved search more realistic, as these are filled in automatically when discover loads the saved search
|
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
|
/ci |
| // This needs to be the first thing that's wired up because initAndSync is pulling the current state from the URL which | ||
| // might change the time filter and thus needs to re-check whether the saved search has changed. | ||
| const timefilerUnsubscribe = services.timefilter.getTimeUpdate$().subscribe(() => { | ||
| savedSearchContainer.update({ |
There was a problem hiding this comment.
This method contains a lot of logic. What if we apply only timeRange and refreshInterval changes here instead of the whole update?
There was a problem hiding this comment.
Made an update, is this what you had in mind?
There was a problem hiding this comment.
Yes, thanks! I think we also need to update refreshInterval if it got changed.
There was a problem hiding this comment.
You're right, could you take another look?
| if (savedSearch.timeRestore) { | ||
| savedSearch.timeRange = services.timefilter.getTime(); | ||
| } |
There was a problem hiding this comment.
| if (savedSearch.timeRestore) { | |
| savedSearch.timeRange = services.timefilter.getTime(); | |
| } |
It should already work based on the code at the end of the function. Doesn't it?
There was a problem hiding this comment.
You're right, missed that, thanks!
|
/ci |
src/plugins/discover/public/application/main/utils/update_saved_search.test.ts
Outdated
Show resolved
Hide resolved
…d_search.test.ts Co-authored-by: Julia Rechkunova <julia.rechkunova@gmail.com>
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
Closes #172288
There's two parts to this: