Revert "Allow overwriting a SQLLab query that has previously been sav…

…ed (apache#8298)" This reverts commit 3aa3da9.
CybercentreCanada · Oct 4, 2019 · 33c80d0 · 33c80d0
1 parent 3a2b6f9
commit 33c80d0
Show file tree

Hide file tree

Showing 10 changed files with 31 additions and 124 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,43 +50,34 @@ 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 });
   }
   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
@@ -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
@@ -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
@@ -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
@@ -2678,7 +2678,7 @@ def _sql_json_sync(
     @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
@@ -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