[Discover][SavedSearch] Fix default rowsPerPage for Dashboard panels#189717
[Discover][SavedSearch] Fix default rowsPerPage for Dashboard panels#189717jughosta merged 4 commits intoelastic:mainfrom
Conversation
|
/ci |
| const rowsPerPage$ = new BehaviorSubject<number | undefined>( | ||
| initialState.rowsPerPage ?? defaultRowsPerPage | ||
| ); |
There was a problem hiding this comment.
@Heenawter Is it a right way of fixing or it should be changed somewhere else?
There was a problem hiding this comment.
This definitely works!! Did a quick test and, because of how we are serializing this value, rowsPerPage will always be saved as part of the panel when it is overwritten by defaultRowsPerPage unless defaultRowsPerPage is equal to the value of the saved search rows per page. Is this desired? if not, I think you have two options:
- You leave this as-is and add a special case for
rowPerPagein your serialize so that you only add it to the serialized panel if it's not equal to the saved searchrowsPerPageor the default rows per page. (this feels like overkill to me personally) - You overwrite in the component instead, so that this value never gets saved. (this seems cleanest?)
But again, if you're fine with the current behaviour, I personally don't see anything wrong with it :)
There was a problem hiding this comment.
@Heenawter Thanks for the suggestions! I will look then into switching to (2).
|
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
|
/ci |
davismcphee
left a comment
There was a problem hiding this comment.
Code changes look good and it works well, thanks for fixing it!
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: cc @jughosta |
Heenawter
left a comment
There was a problem hiding this comment.
Code review only - implementation LGTM 👍
Summary
This PR fixes an issue with
rowsPerPageAdvanced Setting: after the refactoring it was ignored.To test:
Change
rowsPerPageon Advanced Setting page, navigate to Dashboard and add a saved search panel. It should use the configured value by default. The default value can be overwritten by custom panel settings or saved search own settings.Checklist