chore(sqllab): Relocate user in SqlLab to root #25010
Conversation
justinpark
requested review from
eschutho,
lyndsiWilliams and
michael-s-molina
| common: { | ||
| flash_messages: string[]; | ||
| conf: JsonObject; | ||
| }; | ||
| }; | ||
|
|
||
| export type SqlLabExploreRootState = SqlLabRootState | RootState; |
There was a problem hiding this comment.
@lyndsiWilliams I removed this user selector logic since it's under the same path.
justinpark
force-pushed
the
chore--sqllab-update-user-state
branch
from
00c1c0e
to
0bf21d6
Compare
| unsavedQueryEditor, | ||
| queryCostEstimates: {}, | ||
| }, | ||
| requestedQuery, |
There was a problem hiding this comment.
Remove requestedQuery since it's no longer used by #12086
There was a problem hiding this comment.
Thanks for the PR @justinpark. I left some first-pass comments.
| @@ -105,17 +105,14 @@ const sqlLabPersistStateConfig = { | |||
| ...initialState.sqlLab, | |||
| }, | |||
| }; | |||
| // Filter out any user data that may have been persisted in an older version. | |||
There was a problem hiding this comment.
Should we also remove?
if (subset.sqlLab?.user) {
// Don't persist the user.
// User should really not be stored under the "sqlLab" field. Oh well.
delete subset.sqlLab.user;
}| return result; | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| export const store = setupStore({ | ||
| initialState, | ||
| rootReducers: reducers, | ||
| rootReducers: { ...reducers, user: userReducer }, |
There was a problem hiding this comment.
Could you add the user reducer to src/SqlLab/reducers/index.js instead?
There was a problem hiding this comment.
First I intended to do so but SqlLab/reducers/index.js will be consolidated to SPA store which is already has the userReducer (as well as it will make a circular ref issue)
I just add like this since this will be gone after SPA work.
|
I'm looking forward to this feature. Yey @justinpark! |
mistercrunch
added
🏷️ bot
SUMMARY
As a part of SQLLab SPA initiative, the SqlLab state will be consolidated into SPA redux store. In order to match the existing user state, this commit relocates the user property in SqlLab into root.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
After:
Before:
TESTING INSTRUCTIONS
Go to SQLLab and working without errors
ADDITIONAL INFORMATION