Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sql lab localStorage config #6257

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion superset/assets/spec/javascripts/sqllab/App_spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import sinon from 'sinon';

import App from 'src/SqlLab/components/App';
import TabbedSqlEditors from 'src/SqlLab/components/TabbedSqlEditors';
import { sqlLabReducer } from 'src/SqlLab/reducers';
import sqlLabReducer from 'src/SqlLab/reducers/index';

describe('SqlLab App', () => {
const middlewares = [thunk];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import fetchMock from 'fetch-mock';

import shortid from 'shortid';
import { queries, queryWithBadColumns } from './fixtures';
import { sqlLabReducer } from '../../../src/SqlLab/reducers';
import * as actions from '../../../src/SqlLab/actions';
import sqlLabReducer from '../../../src/SqlLab/reducers/index';
import * as actions from '../../../src/SqlLab/actions/sqlLab';
import ExploreResultsButton from '../../../src/SqlLab/components/ExploreResultsButton';
import * as exploreUtils from '../../../src/explore/exploreUtils';
import Button from '../../../src/components/Button';
Expand All @@ -23,9 +23,9 @@ describe('ExploreResultsButton', () => {
const initialState = {
sqlLab: {
...sqlLabReducer(undefined, {}),
common: {
conf: { SUPERSET_WEBSERVER_TIMEOUT: 45 },
},
},
common: {
conf: { SUPERSET_WEBSERVER_TIMEOUT: 45 },
},
};
const store = mockStore(initialState);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
import sinon from 'sinon';
import fetchMock from 'fetch-mock';

import * as actions from '../../../src/SqlLab/actions';
import { query } from './fixtures';
import * as actions from '../../../../src/SqlLab/actions/sqlLab';
import { query } from '../fixtures';

describe('async actions', () => {
let dispatch;
Expand Down
2 changes: 1 addition & 1 deletion superset/assets/spec/javascripts/sqllab/fixtures.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import sinon from 'sinon';
import * as actions from '../../../src/SqlLab/actions';
import * as actions from '../../../src/SqlLab/actions/sqlLab';

export const mockedActions = sinon.stub(Object.assign({}, actions));

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as r from '../../../src/SqlLab/reducers';
import * as actions from '../../../src/SqlLab/actions';
import { table, initialState as mockState } from './fixtures';
import sqlLabReducer from '../../../../src/SqlLab/reducers/sqlLab';
import * as actions from '../../../../src/SqlLab/actions/sqlLab';
import { table, initialState as mockState } from '../fixtures';

const initialState = mockState.sqlLab;

Expand All @@ -12,7 +12,7 @@ describe('sqlLabReducer', () => {
queries: { [testQuery.id]: testQuery },
};
beforeEach(() => {
newState = r.sqlLabReducer(newState, actions.cloneQueryToNewTab(testQuery));
newState = sqlLabReducer(newState, actions.cloneQueryToNewTab(testQuery));
});

it('should have at most one more tab', () => {
Expand All @@ -39,52 +39,52 @@ describe('sqlLabReducer', () => {
newState = { ...initialState };
defaultQueryEditor = newState.queryEditors[0];
qe = Object.assign({}, defaultQueryEditor);
newState = r.sqlLabReducer(newState, actions.addQueryEditor(qe));
newState = sqlLabReducer(newState, actions.addQueryEditor(qe));
qe = newState.queryEditors[newState.queryEditors.length - 1];
});
it('should add a query editor', () => {
expect(newState.queryEditors).toHaveLength(2);
});
it('should remove a query editor', () => {
expect(newState.queryEditors).toHaveLength(2);
newState = r.sqlLabReducer(newState, actions.removeQueryEditor(qe));
newState = sqlLabReducer(newState, actions.removeQueryEditor(qe));
expect(newState.queryEditors).toHaveLength(1);
});
it('should set q query editor active', () => {
newState = r.sqlLabReducer(newState, actions.addQueryEditor(qe));
newState = r.sqlLabReducer(newState, actions.setActiveQueryEditor(defaultQueryEditor));
newState = sqlLabReducer(newState, actions.addQueryEditor(qe));
newState = sqlLabReducer(newState, actions.setActiveQueryEditor(defaultQueryEditor));
expect(newState.tabHistory[newState.tabHistory.length - 1]).toBe(defaultQueryEditor.id);
});
it('should not fail while setting DB', () => {
const dbId = 9;
newState = r.sqlLabReducer(newState, actions.queryEditorSetDb(qe, dbId));
newState = sqlLabReducer(newState, actions.queryEditorSetDb(qe, dbId));
expect(newState.queryEditors[1].dbId).toBe(dbId);
});
it('should not fail while setting schema', () => {
const schema = 'foo';
newState = r.sqlLabReducer(newState, actions.queryEditorSetSchema(qe, schema));
newState = sqlLabReducer(newState, actions.queryEditorSetSchema(qe, schema));
expect(newState.queryEditors[1].schema).toBe(schema);
});
it('should not fail while setting autorun ', () => {
newState = r.sqlLabReducer(newState, actions.queryEditorSetAutorun(qe, false));
newState = sqlLabReducer(newState, actions.queryEditorSetAutorun(qe, false));
expect(newState.queryEditors[1].autorun).toBe(false);
newState = r.sqlLabReducer(newState, actions.queryEditorSetAutorun(qe, true));
newState = sqlLabReducer(newState, actions.queryEditorSetAutorun(qe, true));
expect(newState.queryEditors[1].autorun).toBe(true);
});
it('should not fail while setting title', () => {
const title = 'a new title';
newState = r.sqlLabReducer(newState, actions.queryEditorSetTitle(qe, title));
newState = sqlLabReducer(newState, actions.queryEditorSetTitle(qe, title));
expect(newState.queryEditors[1].title).toBe(title);
});
it('should not fail while setting Sql', () => {
const sql = 'SELECT nothing from dev_null';
newState = r.sqlLabReducer(newState, actions.queryEditorSetSql(qe, sql));
newState = sqlLabReducer(newState, actions.queryEditorSetSql(qe, sql));
expect(newState.queryEditors[1].sql).toBe(sql);
});
it('should set selectedText', () => {
const selectedText = 'TEST';
expect(newState.queryEditors[0].selectedText).toBeNull();
newState = r.sqlLabReducer(
newState = sqlLabReducer(
newState, actions.queryEditorSetSelectedText(newState.queryEditors[0], 'TEST'));
expect(newState.queryEditors[0].selectedText).toBe(selectedText);
});
Expand All @@ -94,7 +94,7 @@ describe('sqlLabReducer', () => {
let newTable;
beforeEach(() => {
newTable = Object.assign({}, table);
newState = r.sqlLabReducer(initialState, actions.mergeTable(newTable));
newState = sqlLabReducer(initialState, actions.mergeTable(newTable));
newTable = newState.tables[0];
});
it('should add a table', () => {
Expand All @@ -104,18 +104,18 @@ describe('sqlLabReducer', () => {
it('should merge the table attributes', () => {
// Merging the extra attribute
newTable.extra = true;
newState = r.sqlLabReducer(newState, actions.mergeTable(newTable));
newState = sqlLabReducer(newState, actions.mergeTable(newTable));
expect(newState.tables).toHaveLength(1);
expect(newState.tables[0].extra).toBe(true);
});
it('should expand and collapse a table', () => {
newState = r.sqlLabReducer(newState, actions.collapseTable(newTable));
newState = sqlLabReducer(newState, actions.collapseTable(newTable));
expect(newState.tables[0].expanded).toBe(false);
newState = r.sqlLabReducer(newState, actions.expandTable(newTable));
newState = sqlLabReducer(newState, actions.expandTable(newTable));
expect(newState.tables[0].expanded).toBe(true);
});
it('should remove a table', () => {
newState = r.sqlLabReducer(newState, actions.removeTable(newTable));
newState = sqlLabReducer(newState, actions.removeTable(newTable));
expect(newState.tables).toHaveLength(0);
});
});
Expand All @@ -128,22 +128,22 @@ describe('sqlLabReducer', () => {
newQuery = { ...query };
});
it('should start a query', () => {
newState = r.sqlLabReducer(newState, actions.startQuery(newQuery));
newState = sqlLabReducer(newState, actions.startQuery(newQuery));
expect(Object.keys(newState.queries)).toHaveLength(1);
});
it('should stop the query', () => {
newState = r.sqlLabReducer(newState, actions.startQuery(newQuery));
newState = r.sqlLabReducer(newState, actions.stopQuery(newQuery));
newState = sqlLabReducer(newState, actions.startQuery(newQuery));
newState = sqlLabReducer(newState, actions.stopQuery(newQuery));
const q = newState.queries[Object.keys(newState.queries)[0]];
expect(q.state).toBe('stopped');
});
it('should remove a query', () => {
newState = r.sqlLabReducer(newState, actions.startQuery(newQuery));
newState = r.sqlLabReducer(newState, actions.removeQuery(newQuery));
newState = sqlLabReducer(newState, actions.startQuery(newQuery));
newState = sqlLabReducer(newState, actions.removeQuery(newQuery));
expect(Object.keys(newState.queries)).toHaveLength(0);
});
it('should refresh queries when polling returns empty', () => {
newState = r.sqlLabReducer(newState, actions.refreshQueries({}));
newState = sqlLabReducer(newState, actions.refreshQueries({}));
});
});
});
27 changes: 22 additions & 5 deletions superset/assets/src/SqlLab/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import thunkMiddleware from 'redux-thunk';
import { hot } from 'react-hot-loader';

import { initFeatureFlags } from 'src/featureFlags';
import getInitialState from './getInitialState';
import rootReducer from './reducers';
import getInitialState from './reducers/getInitialState';
import rootReducer from './reducers/index';
import { initEnhancer } from '../reduxUtils';
import App from './components/App';
import setupApp from '../setup/setupApp';
Expand All @@ -20,14 +20,31 @@ setupApp();
const appContainer = document.getElementById('app');
const bootstrapData = JSON.parse(appContainer.getAttribute('data-bootstrap'));
initFeatureFlags(bootstrapData.common.feature_flags);
const state = getInitialState(bootstrapData);
const initialState = getInitialState(bootstrapData);
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a temporary solution for migrating over existing localStorage, and eventually it will be dead code. Do you foresee this happening for any other field currently under sqlLab if we decide to change the schema?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. this line is to fix the screwed localStorage.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a comment explaining what is going on here so people that don't have context can understand it?

also, what's the plan for removing this once it becomes dead code? can we safely delete it in a month? 2 months?

Copy link
Author

@graceguo-supercat graceguo-supercat Nov 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a good question..we don't have many options on browser's localStorage if user didn't open sql lab. but if user didn't open sql lab for 2 months he probably didn't critically depend on localStorage.

this PR will help @jeffreythewang's feature: #4941. So I am thinking after a few new features in Sql Lab (a month long is also good), we can remove this cleanup line.

subset[path] = state[path];
});
return subset;
},
},
};

const store = createStore(
rootReducer,
state,
initialState,
compose(
applyMiddleware(thunkMiddleware),
initEnhancer(),
initEnhancer(true, sqlLabPersistStateConfig),
),
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ import JSONbig from 'json-bigint';
import { t } from '@superset-ui/translation';
import { SupersetClient } from '@superset-ui/connection';

import { now } from '../modules/dates';
import { now } from '../../modules/dates';
import {
addSuccessToast as addSuccessToastAction,
addDangerToast as addDangerToastAction,
addInfoToast as addInfoToastAction,
} from '../messageToasts/actions';
import getClientErrorObject from '../utils/getClientErrorObject';
import COMMON_ERR_MESSAGES from '../utils/errorMessages';
} from '../../messageToasts/actions/index';
import getClientErrorObject from '../../utils/getClientErrorObject';
import COMMON_ERR_MESSAGES from '../../utils/errorMessages';

export const RESET_STATE = 'RESET_STATE';
export const ADD_QUERY_EDITOR = 'ADD_QUERY_EDITOR';
Expand Down
2 changes: 1 addition & 1 deletion superset/assets/src/SqlLab/components/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import TabbedSqlEditors from './TabbedSqlEditors';
import QueryAutoRefresh from './QueryAutoRefresh';
import QuerySearch from './QuerySearch';
import ToastPresenter from '../../messageToasts/containers/ToastPresenter';
import * as Actions from '../actions';
import * as Actions from '../actions/sqlLab';

class App extends React.PureComponent {
constructor(props) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { t } from '@superset-ui/translation';

import shortid from 'shortid';
import { exportChart } from '../../explore/exploreUtils';
import * as actions from '../actions';
import * as actions from '../actions/sqlLab';
import InfoTooltipWithTrigger from '../../components/InfoTooltipWithTrigger';
import Button from '../../components/Button';

Expand Down Expand Up @@ -184,10 +184,10 @@ class ExploreResultsButton extends React.PureComponent {
ExploreResultsButton.propTypes = propTypes;
ExploreResultsButton.defaultProps = defaultProps;

function mapStateToProps({ sqlLab }) {
function mapStateToProps({ sqlLab, common }) {
return {
errorMessage: sqlLab.errorMessage,
timeout: sqlLab.common ? sqlLab.common.conf.SUPERSET_WEBSERVER_TIMEOUT : null,
timeout: common.conf ? common.conf.SUPERSET_WEBSERVER_TIMEOUT : null,
};
}

Expand Down
2 changes: 1 addition & 1 deletion superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';
import { SupersetClient } from '@superset-ui/connection';

import * as Actions from '../actions';
import * as Actions from '../actions/sqlLab';

const QUERY_UPDATE_FREQ = 2000;
const QUERY_UPDATE_BUFFER_MS = 5000;
Expand Down
2 changes: 1 addition & 1 deletion superset/assets/src/SqlLab/components/SouthPane.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { connect } from 'react-redux';
import { bindActionCreators } from 'redux';
import { t } from '@superset-ui/translation';

import * as Actions from '../actions';
import * as Actions from '../actions/sqlLab';
import QueryHistory from './QueryHistory';
import ResultSet from './ResultSet';
import { STATUS_OPTIONS, STATE_BSSTYLE_MAP } from '../constants';
Expand Down
2 changes: 1 addition & 1 deletion superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { bindActionCreators } from 'redux';
import URI from 'urijs';
import { t } from '@superset-ui/translation';

import * as Actions from '../actions';
import * as Actions from '../actions/sqlLab';
import SqlEditor from './SqlEditor';
import { areArraysShallowEqual } from '../../reduxUtils';
import TabStatusIcon from './TabStatusIcon';
Expand Down
3 changes: 3 additions & 0 deletions superset/assets/src/SqlLab/reducers/common.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function commonReducer(state = {}) {
return state;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import shortid from 'shortid';
import { t } from '@superset-ui/translation';
import getToastsFromPyFlashMessages from '../messageToasts/utils/getToastsFromPyFlashMessages';
import getToastsFromPyFlashMessages from '../../messageToasts/utils/getToastsFromPyFlashMessages';

export default function getInitialState({ defaultDbId, ...restBootstrapData }) {
const defaultQueryEditor = {
Expand All @@ -24,10 +24,13 @@ export default function getInitialState({ defaultDbId, ...restBootstrapData }) {
tabHistory: [defaultQueryEditor.id],
tables: [],
queriesLastUpdate: Date.now(),
...restBootstrapData,
},
messageToasts: getToastsFromPyFlashMessages(
(restBootstrapData.common || {}).flash_messages || [],
),
common: {
flash_messages: restBootstrapData.common.flash_messages,
conf: restBootstrapData.common.conf,
},
};
}
11 changes: 11 additions & 0 deletions superset/assets/src/SqlLab/reducers/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { combineReducers } from 'redux';

import sqlLab from './sqlLab';
import messageToasts from '../../messageToasts/reducers/index';
import common from './common';

export default combineReducers({
sqlLab,
messageToasts,
common,
});
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
import { combineReducers } from 'redux';
import shortid from 'shortid';
import { t } from '@superset-ui/translation';

import messageToasts from '../messageToasts/reducers';
import getInitialState from './getInitialState';
import * as actions from './actions';
import { now } from '../modules/dates';
import * as actions from '../actions/sqlLab';
import { now } from '../../modules/dates';
import {
addToObject,
alterInObject,
alterInArr,
removeFromArr,
getFromArr,
addToArr,
} from '../reduxUtils';
} from '../../reduxUtils';

export const sqlLabReducer = function (state = {}, action) {
export default function sqlLabReducer(state = {}, action) {
const actionHandlers = {
[actions.ADD_QUERY_EDITOR]() {
const tabHistory = state.tabHistory.slice();
Expand Down Expand Up @@ -276,9 +274,4 @@ export const sqlLabReducer = function (state = {}, action) {
return actionHandlers[action.type]();
}
return state;
};

export default combineReducers({
sqlLab: sqlLabReducer,
messageToasts,
});
}
Loading