michael-s-molina commented on code in PR #25088:
URL: https://github.com/apache/superset/pull/25088#discussion_r1306034680
##########
superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js:
##########
@@ -96,3 +98,86 @@ export function clearQueryEditors(queryEditors) {
),
);
}
+
+const CLEAR_ENTITY_HELPERS_MAP = {
+ tables: emptyTablePersistData,
+ queries: emptyQueryResults,
+ queryEditors: clearQueryEditors,
+ unsavedQueryEditor: qe => clearQueryEditors([qe])[0],
+};
+
+const sqlLabPersistStateConfig = {
+ paths: ['sqlLab'],
+ config: {
+ slicer: paths => state => {
+ const subset = {};
+ paths.forEach(path => {
+ // this line is used to remove old data from browser localStorage.
+ // we used to persist all redux state into localStorage, but
+ // it caused configurations passed from server-side got override.
+ // see PR 6257 for details
+ delete state[path].common; // eslint-disable-line no-param-reassign
+ if (path === 'sqlLab') {
+ subset[path] = Object.fromEntries(
+ Object.entries(state[path]).map(([key, value]) => [
+ key,
+ CLEAR_ENTITY_HELPERS_MAP[key]?.(value) ?? value,
+ ]),
+ );
+ }
+ });
+
+ const data = JSON.stringify(subset);
+ // 2 digit precision
+ const currentSize =
+ Math.round(((data.length * BYTES_PER_CHAR) / KB_STORAGE) * 100) / 100;
+ if (state.localStorageUsageInKilobytes !== currentSize) {
+ state.localStorageUsageInKilobytes = currentSize; //
eslint-disable-line no-param-reassign
+ }
+
+ return subset;
+ },
+ merge: (initialState, persistedState = {}) => {
+ const result = {
+ ...initialState,
+ ...persistedState,
+ sqlLab: {
+ ...(persistedState?.sqlLab || {}),
+ // Overwrite initialState over persistedState for sqlLab
+ // since a logic in getInitialState overrides the value from
persistedState
+ ...initialState.sqlLab,
+ },
+ };
+ return result;
+ },
+ },
+};
+
+export const persistSqlLabStateEnhander = persistState(
Review Comment:
Could you move the enhancer-related code to its own file? Maybe
`src/SqlLab/middleware/persistSqlLabStateEnhancer.ts`.
##########
superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js:
##########
@@ -96,3 +98,86 @@ export function clearQueryEditors(queryEditors) {
),
);
}
+
+const CLEAR_ENTITY_HELPERS_MAP = {
+ tables: emptyTablePersistData,
+ queries: emptyQueryResults,
+ queryEditors: clearQueryEditors,
+ unsavedQueryEditor: qe => clearQueryEditors([qe])[0],
+};
+
+const sqlLabPersistStateConfig = {
+ paths: ['sqlLab'],
+ config: {
+ slicer: paths => state => {
+ const subset = {};
+ paths.forEach(path => {
+ // this line is used to remove old data from browser localStorage.
+ // we used to persist all redux state into localStorage, but
+ // it caused configurations passed from server-side got override.
+ // see PR 6257 for details
+ delete state[path].common; // eslint-disable-line no-param-reassign
+ if (path === 'sqlLab') {
+ subset[path] = Object.fromEntries(
+ Object.entries(state[path]).map(([key, value]) => [
+ key,
+ CLEAR_ENTITY_HELPERS_MAP[key]?.(value) ?? value,
+ ]),
+ );
+ }
+ });
+
+ const data = JSON.stringify(subset);
+ // 2 digit precision
+ const currentSize =
+ Math.round(((data.length * BYTES_PER_CHAR) / KB_STORAGE) * 100) / 100;
+ if (state.localStorageUsageInKilobytes !== currentSize) {
+ state.localStorageUsageInKilobytes = currentSize; //
eslint-disable-line no-param-reassign
+ }
+
+ return subset;
+ },
+ merge: (initialState, persistedState = {}) => {
+ const result = {
+ ...initialState,
+ ...persistedState,
+ sqlLab: {
+ ...(persistedState?.sqlLab || {}),
+ // Overwrite initialState over persistedState for sqlLab
+ // since a logic in getInitialState overrides the value from
persistedState
+ ...initialState.sqlLab,
+ },
+ };
+ return result;
+ },
+ },
+};
+
+export const persistSqlLabStateEnhander = persistState(
+ sqlLabPersistStateConfig.paths,
+ sqlLabPersistStateConfig.config,
+);
+
+export function rehydratePersistedState(dispatch, persistedState) {
Review Comment:
```suggestion
export function rehydratePersistedState(dispatch, state) {
```
##########
superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js:
##########
@@ -96,3 +98,86 @@ export function clearQueryEditors(queryEditors) {
),
);
}
+
+const CLEAR_ENTITY_HELPERS_MAP = {
+ tables: emptyTablePersistData,
+ queries: emptyQueryResults,
+ queryEditors: clearQueryEditors,
+ unsavedQueryEditor: qe => clearQueryEditors([qe])[0],
+};
+
+const sqlLabPersistStateConfig = {
+ paths: ['sqlLab'],
+ config: {
+ slicer: paths => state => {
+ const subset = {};
+ paths.forEach(path => {
+ // this line is used to remove old data from browser localStorage.
+ // we used to persist all redux state into localStorage, but
+ // it caused configurations passed from server-side got override.
+ // see PR 6257 for details
+ delete state[path].common; // eslint-disable-line no-param-reassign
+ if (path === 'sqlLab') {
+ subset[path] = Object.fromEntries(
+ Object.entries(state[path]).map(([key, value]) => [
+ key,
+ CLEAR_ENTITY_HELPERS_MAP[key]?.(value) ?? value,
+ ]),
+ );
+ }
+ });
+
+ const data = JSON.stringify(subset);
+ // 2 digit precision
+ const currentSize =
+ Math.round(((data.length * BYTES_PER_CHAR) / KB_STORAGE) * 100) / 100;
+ if (state.localStorageUsageInKilobytes !== currentSize) {
+ state.localStorageUsageInKilobytes = currentSize; //
eslint-disable-line no-param-reassign
+ }
+
+ return subset;
+ },
+ merge: (initialState, persistedState = {}) => {
+ const result = {
+ ...initialState,
+ ...persistedState,
+ sqlLab: {
+ ...(persistedState?.sqlLab || {}),
+ // Overwrite initialState over persistedState for sqlLab
+ // since a logic in getInitialState overrides the value from
persistedState
+ ...initialState.sqlLab,
+ },
+ };
+ return result;
+ },
+ },
+};
+
+export const persistSqlLabStateEnhander = persistState(
+ sqlLabPersistStateConfig.paths,
+ sqlLabPersistStateConfig.config,
+);
+
+export function rehydratePersistedState(dispatch, persistedState) {
+ // Rehydrate server side persisted table metadata
+ persistedState.sqlLab.tables.forEach(
Review Comment:
```suggestion
state.sqlLab.tables.forEach(
```
##########
superset-frontend/src/views/store.ts:
##########
@@ -113,6 +122,7 @@ const CombinedDatasourceReducers = (
};
const reducers = {
+ ...sqlLabReducers,
Review Comment:
`sqlLabReducers` is including some reducers that already exist in this file
such as `messageToasts` and `common`. Can you import the reducers directly?
Like:
`import sqlLabReducer from 'src/SqlLab/reducers/sqlLab';`
This will allow you to delete:
`src/SqlLab/reducers/index.js`
`src/SqlLab/reducers/common.js`
It seems `src/SqlLab/reducers/localStorageUsage.js` does nothing. Can we
delete it?
##########
superset-frontend/src/SqlLab/actions/sqlLab.js:
##########
@@ -136,8 +139,23 @@ export function getUpToDateQuery(rootState, queryEditor,
key) {
};
}
-export function resetState() {
- return { type: RESET_STATE };
+export function resetState(data) {
+ return (dispatch, getState) => {
+ const { common, user, config } = getState();
+ const initialState = getInitialState({
Review Comment:
Why do we send all parameters (`common`, `user`, `config`, etc) when
resetting but we don't send them when initializing (`App`, `store`)?
##########
superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js:
##########
@@ -96,3 +98,86 @@ export function clearQueryEditors(queryEditors) {
),
);
}
+
+const CLEAR_ENTITY_HELPERS_MAP = {
+ tables: emptyTablePersistData,
+ queries: emptyQueryResults,
+ queryEditors: clearQueryEditors,
+ unsavedQueryEditor: qe => clearQueryEditors([qe])[0],
+};
+
+const sqlLabPersistStateConfig = {
+ paths: ['sqlLab'],
+ config: {
+ slicer: paths => state => {
+ const subset = {};
+ paths.forEach(path => {
+ // this line is used to remove old data from browser localStorage.
+ // we used to persist all redux state into localStorage, but
+ // it caused configurations passed from server-side got override.
+ // see PR 6257 for details
+ delete state[path].common; // eslint-disable-line no-param-reassign
+ if (path === 'sqlLab') {
+ subset[path] = Object.fromEntries(
+ Object.entries(state[path]).map(([key, value]) => [
+ key,
+ CLEAR_ENTITY_HELPERS_MAP[key]?.(value) ?? value,
+ ]),
+ );
+ }
+ });
+
+ const data = JSON.stringify(subset);
+ // 2 digit precision
+ const currentSize =
+ Math.round(((data.length * BYTES_PER_CHAR) / KB_STORAGE) * 100) / 100;
+ if (state.localStorageUsageInKilobytes !== currentSize) {
+ state.localStorageUsageInKilobytes = currentSize; //
eslint-disable-line no-param-reassign
+ }
+
+ return subset;
+ },
+ merge: (initialState, persistedState = {}) => {
+ const result = {
+ ...initialState,
+ ...persistedState,
+ sqlLab: {
+ ...(persistedState?.sqlLab || {}),
+ // Overwrite initialState over persistedState for sqlLab
+ // since a logic in getInitialState overrides the value from
persistedState
+ ...initialState.sqlLab,
+ },
+ };
+ return result;
+ },
+ },
+};
+
+export const persistSqlLabStateEnhander = persistState(
Review Comment:
```suggestion
export const persistSqlLabStateEnhancer = persistState(
```
##########
superset-frontend/src/views/store.ts:
##########
@@ -156,9 +166,12 @@ export function setupStore({
},
middleware: getMiddleware,
devTools: process.env.WEBPACK_MODE === 'development' && !disableDebugger,
+ ...(!isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) && {
+ enhancers: [persistSqlLabStateEnhander as StoreEnhancer],
Review Comment:
```suggestion
enhancers: [persistSqlLabStateEnhancer as StoreEnhancer],
```
##########
superset-frontend/src/views/store.ts:
##########
@@ -34,6 +38,11 @@ import logger from 'src/middleware/loggerMiddleware';
import saveModal from 'src/explore/reducers/saveModalReducer';
import explore from 'src/explore/reducers/exploreReducer';
import exploreDatasources from 'src/explore/reducers/datasourcesReducer';
+import { FeatureFlag, isFeatureEnabled } from '@superset-ui/core';
+
+import { persistSqlLabStateEnhander } from
'src/SqlLab/utils/reduxStateToLocalStorageHelper';
Review Comment:
```suggestion
import { persistSqlLabStateEnhancer } from
'src/SqlLab/utils/reduxStateToLocalStorageHelper';
```
##########
superset-frontend/src/SqlLab/App.jsx:
##########
@@ -30,16 +29,13 @@ import { GlobalStyles } from 'src/GlobalStyles';
import { setupStore, userReducer } from 'src/views/store';
import setupExtensions from 'src/setup/setupExtensions';
import getBootstrapData from 'src/utils/getBootstrapData';
-import { tableApiUtil } from 'src/hooks/apiResources/tables';
import getInitialState from './reducers/getInitialState';
import { reducers } from './reducers/index';
import App from './components/App';
import {
- emptyTablePersistData,
- emptyQueryResults,
- clearQueryEditors,
+ persistSqlLabStateEnhander,
Review Comment:
```suggestion
persistSqlLabStateEnhancer,
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
