This is an automated email from the ASF dual-hosted git repository. erikrit pushed a commit to branch revert-8298-query-overwrite in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
commit ef526fd4ad6453af092d34af7fe4b3b2d3b280b2 Author: Erik Ritter <erik.rit...@airbnb.com> AuthorDate: Fri Oct 4 10:24:56 2019 -0700 Revert "Allow overwriting a SQLLab query that has previously been saved (#8298)" This reverts commit fbbc5f0577454f24cac9e5fd2d01f9f390f18048. --- .gitignore | 1 - superset/assets/package.json | 2 +- .../spec/javascripts/sqllab/SaveQuery_spec.jsx | 33 ++------------- superset/assets/src/SqlLab/actions/sqlLab.js | 45 +++------------------ .../assets/src/SqlLab/components/SaveQuery.jsx | 47 +++++++--------------- .../assets/src/SqlLab/components/SqlEditor.jsx | 7 ++-- superset/assets/src/SqlLab/reducers/sqlLab.js | 7 ---- .../SqlLab/utils/reduxStateToLocalStorageHelper.js | 1 - superset/views/core.py | 2 +- superset/views/sql_lab.py | 10 +---- 10 files changed, 31 insertions(+), 124 deletions(-) diff --git a/.gitignore b/.gitignore index 30770ff..ab5126a 100644 --- a/.gitignore +++ b/.gitignore @@ -24,7 +24,6 @@ .coverage .DS_Store .eggs -.envrc .idea .mypy_cache .python-version diff --git a/superset/assets/package.json b/superset/assets/package.json index d275e86..657b8c6 100644 --- a/superset/assets/package.json +++ b/superset/assets/package.json @@ -79,9 +79,9 @@ "@superset-ui/legacy-preset-chart-deckgl": "^0.1.0", "@superset-ui/legacy-preset-chart-nvd3": "^0.11.0", "@superset-ui/number-format": "^0.12.1", + "@superset-ui/query": "^0.12.2", "@superset-ui/plugin-chart-table": "^0.11.0", "@superset-ui/preset-chart-xy": "^0.11.0", - "@superset-ui/query": "^0.12.2", "@superset-ui/time-format": "^0.12.1", "@superset-ui/translation": "^0.12.0", "@types/react-json-tree": "^0.6.11", diff --git a/superset/assets/spec/javascripts/sqllab/SaveQuery_spec.jsx b/superset/assets/spec/javascripts/sqllab/SaveQuery_spec.jsx index 46c215c..d4a2f46 100644 --- a/superset/assets/spec/javascripts/sqllab/SaveQuery_spec.jsx +++ b/superset/assets/spec/javascripts/sqllab/SaveQuery_spec.jsx @@ -19,18 +19,14 @@ import React from 'react'; import { FormControl } from 'react-bootstrap'; import { shallow } from 'enzyme'; -import * as sinon from 'sinon'; import SaveQuery from '../../../src/SqlLab/components/SaveQuery'; import ModalTrigger from '../../../src/components/ModalTrigger'; -import Button from '../../../src/components/Button'; describe('SavedQuery', () => { const mockedProps = { - query: { - dbId: 1, - schema: 'main', - sql: 'SELECT * FROM t', - }, + dbId: 1, + schema: 'main', + sql: 'SELECT * FROM t', defaultLabel: 'untitled', animation: false, }; @@ -58,27 +54,4 @@ describe('SavedQuery', () => { const modal = shallow(wrapper.instance().renderModalBody()); expect(modal.find(FormControl)).toHaveLength(2); }); - it('has a save button if this is a new query', () => { - const saveSpy = sinon.spy(); - const wrapper = shallow(<SaveQuery {...mockedProps} onSave={saveSpy} />); - const modal = shallow(wrapper.instance().renderModalBody()); - expect(modal.find(Button)).toHaveLength(2); - modal.find(Button).at(0).simulate('click'); - expect(saveSpy.calledOnce).toBe(true); - }); - it('has an update button if this is an existing query', () => { - const updateSpy = sinon.spy(); - const props = { - ...mockedProps, - query: { - ...mockedProps.query, - remoteId: '42', - }, - }; - const wrapper = shallow(<SaveQuery {...props} onUpdate={updateSpy} />); - const modal = shallow(wrapper.instance().renderModalBody()); - expect(modal.find(Button)).toHaveLength(3); - modal.find(Button).at(0).simulate('click'); - expect(updateSpy.calledOnce).toBe(true); - }); }); diff --git a/superset/assets/src/SqlLab/actions/sqlLab.js b/superset/assets/src/SqlLab/actions/sqlLab.js index f4fcc8e..5f2baa7 100644 --- a/superset/assets/src/SqlLab/actions/sqlLab.js +++ b/superset/assets/src/SqlLab/actions/sqlLab.js @@ -20,8 +20,6 @@ import shortid from 'shortid'; import JSONbig from 'json-bigint'; import { t } from '@superset-ui/translation'; import { SupersetClient } from '@superset-ui/connection'; -import invert from 'lodash/invert'; -import mapKeys from 'lodash/mapKeys'; import { now } from '../../modules/dates'; import { @@ -34,7 +32,6 @@ import COMMON_ERR_MESSAGES from '../../utils/errorMessages'; export const RESET_STATE = 'RESET_STATE'; export const ADD_QUERY_EDITOR = 'ADD_QUERY_EDITOR'; -export const UPDATE_QUERY_EDITOR = 'UPDATE_QUERY_EDITOR'; export const CLONE_QUERY_TO_NEW_TAB = 'CLONE_QUERY_TO_NEW_TAB'; export const REMOVE_QUERY_EDITOR = 'REMOVE_QUERY_EDITOR'; export const MERGE_TABLE = 'MERGE_TABLE'; @@ -85,24 +82,6 @@ export const addInfoToast = addInfoToastAction; export const addSuccessToast = addSuccessToastAction; export const addDangerToast = addDangerToastAction; -// a map of SavedQuery field names to the different names used client-side, -// because for now making the names consistent is too complicated -// so it might as well only happen in one place -const queryClientMapping = { - id: 'remoteId', - db_id: 'dbId', - client_id: 'id', - label: 'title', -}; -const queryServerMapping = invert(queryClientMapping); - -// uses a mapping like those above to convert object key names to another style -const fieldConverter = mapping => obj => - mapKeys(obj, (value, key) => key in mapping ? mapping[key] : key); - -const convertQueryToServer = fieldConverter(queryServerMapping); -const convertQueryToClient = fieldConverter(queryClientMapping); - export function resetState() { return { type: RESET_STATE }; } @@ -126,29 +105,13 @@ export function saveQuery(query) { return dispatch => SupersetClient.post({ endpoint: '/savedqueryviewapi/api/create', - postPayload: convertQueryToServer(query), + postPayload: query, stringify: false, }) .then(() => dispatch(addSuccessToast(t('Your query was saved')))) .catch(() => dispatch(addDangerToast(t('Your query could not be saved')))); } -export function updateQueryEditor(alterations) { - return { type: UPDATE_QUERY_EDITOR, alterations }; -} - -export function updateSavedQuery(query) { - return dispatch => - SupersetClient.put({ - endpoint: `/savedqueryviewapi/api/update/${query.remoteId}`, - postPayload: convertQueryToServer(query), - stringify: false, - }) - .then(() => dispatch(addSuccessToast(t('Your query was updated')))) - .catch(() => dispatch(addDangerToast(t('Your query could not be updated')))) - .then(() => dispatch(updateQueryEditor(query))); -} - export function scheduleQuery(query) { return dispatch => SupersetClient.post({ @@ -541,9 +504,13 @@ export function popSavedQuery(saveQueryId) { return function (dispatch) { return SupersetClient.get({ endpoint: `/savedqueryviewapi/api/get/${saveQueryId}` }) .then(({ json }) => { + const { result } = json; const queryEditorProps = { - ...convertQueryToClient(json.result), + title: result.label, + dbId: result.db_id ? parseInt(result.db_id, 10) : null, + schema: result.schema, autorun: false, + sql: result.sql, }; return dispatch(addQueryEditor(queryEditorProps)); }) diff --git a/superset/assets/src/SqlLab/components/SaveQuery.jsx b/superset/assets/src/SqlLab/components/SaveQuery.jsx index 14650d0..1b3c502 100644 --- a/superset/assets/src/SqlLab/components/SaveQuery.jsx +++ b/superset/assets/src/SqlLab/components/SaveQuery.jsx @@ -25,11 +25,12 @@ import Button from '../../components/Button'; import ModalTrigger from '../../components/ModalTrigger'; const propTypes = { - query: PropTypes.object, defaultLabel: PropTypes.string, + sql: PropTypes.string, + schema: PropTypes.string, + dbId: PropTypes.number, animation: PropTypes.bool, onSave: PropTypes.func, - onUpdate: PropTypes.func, saveQueryWarning: PropTypes.string, }; const defaultProps = { @@ -49,21 +50,23 @@ class SaveQuery extends React.PureComponent { }; this.toggleSave = this.toggleSave.bind(this); this.onSave = this.onSave.bind(this); - this.onUpdate = this.onUpdate.bind(this); this.onCancel = this.onCancel.bind(this); this.onLabelChange = this.onLabelChange.bind(this); this.onDescriptionChange = this.onDescriptionChange.bind(this); } onSave() { - this.props.onSave(this.queryPayload()); - this.close(); - } - onUpdate() { - this.props.onUpdate(this.queryPayload()); - this.close(); + const query = { + label: this.state.label, + description: this.state.description, + db_id: this.props.dbId, + schema: this.props.schema, + sql: this.props.sql, + }; + this.props.onSave(query); + this.saveModal.close(); } onCancel() { - this.close(); + this.saveModal.close(); } onLabelChange(e) { this.setState({ label: e.target.value }); @@ -71,21 +74,10 @@ class SaveQuery extends React.PureComponent { onDescriptionChange(e) { this.setState({ description: e.target.value }); } - queryPayload() { - return { - ...this.props.query, - title: this.state.label, - description: this.state.description, - }; - } - close() { - if (this.saveModal) this.saveModal.close(); - } toggleSave(e) { this.setState({ target: e.target, showSave: !this.state.showSave }); } renderModalBody() { - const isSaved = !!this.props.query.remoteId; return ( <FormGroup bsSize="small"> <Row> @@ -132,21 +124,12 @@ class SaveQuery extends React.PureComponent { )} <Row> <Col md={12}> - {isSaved && ( - <Button - bsStyle="primary" - onClick={this.onUpdate} - className="m-r-3" - > - {t('Update')} - </Button> - )} <Button - bsStyle={isSaved ? undefined : 'primary'} + bsStyle="primary" onClick={this.onSave} className="m-r-3" > - {isSaved ? t('Save New') : t('Save')} + {t('Save')} </Button> <Button onClick={this.onCancel} className="cancelQuery"> {t('Cancel')} diff --git a/superset/assets/src/SqlLab/components/SqlEditor.jsx b/superset/assets/src/SqlLab/components/SqlEditor.jsx index 001d91d..9ab0ce9 100644 --- a/superset/assets/src/SqlLab/components/SqlEditor.jsx +++ b/superset/assets/src/SqlLab/components/SqlEditor.jsx @@ -432,11 +432,12 @@ class SqlEditor extends React.PureComponent { } <span className="m-r-5"> <SaveQuery - query={qe} - defaultLabel={qe.description == null ? qe.title : qe.description} + defaultLabel={qe.title} + sql={qe.sql} className="m-r-5" onSave={this.props.actions.saveQuery} - onUpdate={this.props.actions.updateSavedQuery} + schema={qe.schema} + dbId={qe.dbId} saveQueryWarning={this.props.saveQueryWarning} /> </span> diff --git a/superset/assets/src/SqlLab/reducers/sqlLab.js b/superset/assets/src/SqlLab/reducers/sqlLab.js index aafc753..72bfc29 100644 --- a/superset/assets/src/SqlLab/reducers/sqlLab.js +++ b/superset/assets/src/SqlLab/reducers/sqlLab.js @@ -39,18 +39,11 @@ export default function sqlLabReducer(state = {}, action) { const newState = Object.assign({}, state, { tabHistory }); return addToArr(newState, 'queryEditors', action.queryEditor); }, - [actions.UPDATE_QUERY_EDITOR]() { - const id = action.alterations.remoteId; - const existing = state.queryEditors.find(qe => qe.remoteId === id); - if (existing == null) return state; - return alterInArr(state, 'queryEditors', existing, action.alterations, 'remoteId'); - }, [actions.CLONE_QUERY_TO_NEW_TAB]() { const progenitor = state.queryEditors.find( qe => qe.id === state.tabHistory[state.tabHistory.length - 1], ); const qe = { - remoteId: progenitor.remoteId, id: shortid.generate(), title: t('Copy of %s', progenitor.title), dbId: action.query.dbId ? action.query.dbId : null, diff --git a/superset/assets/src/SqlLab/utils/reduxStateToLocalStorageHelper.js b/superset/assets/src/SqlLab/utils/reduxStateToLocalStorageHelper.js index d0a8f5f..aef0867 100644 --- a/superset/assets/src/SqlLab/utils/reduxStateToLocalStorageHelper.js +++ b/superset/assets/src/SqlLab/utils/reduxStateToLocalStorageHelper.js @@ -19,7 +19,6 @@ import { LOCALSTORAGE_MAX_QUERY_AGE_MS } from '../constants'; const PERSISTENT_QUERY_EDITOR_KEYS = new Set([ - 'remoteId', 'autorun', 'dbId', 'height', diff --git a/superset/views/core.py b/superset/views/core.py index d5bad8b..07e4b2c 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -2678,7 +2678,7 @@ class Superset(BaseSupersetView): @expose("/sql_json/", methods=["POST"]) @event_logger.log_this def sql_json(self): - """Runs arbitrary sql and returns data as json""" + """Runs arbitrary sql and returns and json""" # Collect Values database_id: int = request.json.get("database_id") schema: str = request.json.get("schema") diff --git a/superset/views/sql_lab.py b/superset/views/sql_lab.py index 06339cd..1332bfa 100644 --- a/superset/views/sql_lab.py +++ b/superset/views/sql_lab.py @@ -156,15 +156,7 @@ class SavedQueryViewApi(SavedQueryView): "sql", "extra_json", ] - show_columns = [ - "id", - "label", - "db_id", - "schema", - "description", - "sql", - "extra_json", - ] + show_columns = ["label", "db_id", "schema", "description", "sql", "extra_json"] add_columns = show_columns edit_columns = add_columns