ui: move latestQuery, latestFormattedQuery from redux to local state
#88510
Conversation
|
This change is |
There was a problem hiding this comment.
The idea was not only to save between tab switch, but also if you select a period where the data is not available, we want to at least show the query. With your change, doesn't the state change and the query gets cleanup? Can you test this scenario (go to the details with a period that has info, then select a new period where there is no data on it)?
Reviewable status:
complete! 0 of 0 LGTMs obtained
|
Yes, that behaviour is still preserved here, we're doing the exact same thing but in the component state instead of redux since this info isn't needed anywhere else in the app. The state is only updated if we receive a different, non-empty query. Included a loom! |
There was a problem hiding this comment.
You need to cleanup pkg/ui/workspaces/cluster-ui/src/store/statementDetails/statementDetails.sagas.spec.ts:670:11 to remove the references for latestQuery you remove from SQLDetailsStatsReducerState
Reviewable status:
Previously, the fields `latestQuery` and `latestFormattedQuery` representing the latest non-empty query string for a statement viewed from the detaisl page was being stored in redux. The purpose of these fields was to preserve the query when changing tabs in the stmt details page. Saving this in the redux store was unnecessary and so this commit moves these fields to the stmt details local state. Release note: None
xinhaoz
force-pushed
the
remove-redux-latestquery
branch
from
a29af4a to
27299e0
Compare
There was a problem hiding this comment.
Reviewed 7 of 7 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status:
|
bors r+ |
|
Build succeeded: |
Previously, the fields
latestQueryandlatestFormattedQueryrepresenting the latest non-empty query string for a statement viewed from the detaisl page was being stored in redux. The purpose of these fields was to preserve the query when changing tabs in the stmt details page. Saving this in the redux store was unnecessary and so this commit moves these fields to the stmt details local state.Release note: None
https://www.loom.com/share/a28d412fb83a429391210935982404de