diff --git a/.gitignore b/.gitignore index ab5126af5d5e3..30770ff639dfb 100644 --- a/.gitignore +++ b/.gitignore @@ -24,6 +24,7 @@ .coverage .DS_Store .eggs +.envrc .idea .mypy_cache .python-version diff --git a/superset/assets/package.json b/superset/assets/package.json index 657b8c6c1f294..d275e865da70c 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 d4a2f469eb6db..46c215ceca1dc 100644 --- a/superset/assets/spec/javascripts/sqllab/SaveQuery_spec.jsx +++ b/superset/assets/spec/javascripts/sqllab/SaveQuery_spec.jsx @@ -19,14 +19,18 @@ 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 = { - dbId: 1, - schema: 'main', - sql: 'SELECT * FROM t', + query: { + dbId: 1, + schema: 'main', + sql: 'SELECT * FROM t', + }, defaultLabel: 'untitled', animation: false, }; @@ -54,4 +58,27 @@ 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(); + 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(); + 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 5f2baa71734fd..f4fcc8e16a920 100644 --- a/superset/assets/src/SqlLab/actions/sqlLab.js +++ b/superset/assets/src/SqlLab/actions/sqlLab.js @@ -20,6 +20,8 @@ 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 { @@ -32,6 +34,7 @@ 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'; @@ -82,6 +85,24 @@ 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 }; } @@ -105,13 +126,29 @@ export function saveQuery(query) { return dispatch => SupersetClient.post({ endpoint: '/savedqueryviewapi/api/create', - postPayload: query, + postPayload: convertQueryToServer(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({ @@ -504,13 +541,9 @@ export function popSavedQuery(saveQueryId) { return function (dispatch) { return SupersetClient.get({ endpoint: `/savedqueryviewapi/api/get/${saveQueryId}` }) .then(({ json }) => { - const { result } = json; const queryEditorProps = { - title: result.label, - dbId: result.db_id ? parseInt(result.db_id, 10) : null, - schema: result.schema, + ...convertQueryToClient(json.result), 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 1b3c5029a50e9..14650d048b805 100644 --- a/superset/assets/src/SqlLab/components/SaveQuery.jsx +++ b/superset/assets/src/SqlLab/components/SaveQuery.jsx @@ -25,12 +25,11 @@ 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 = { @@ -50,23 +49,21 @@ 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() { - 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(); + this.props.onSave(this.queryPayload()); + this.close(); + } + onUpdate() { + this.props.onUpdate(this.queryPayload()); + this.close(); } onCancel() { - this.saveModal.close(); + this.close(); } onLabelChange(e) { this.setState({ label: e.target.value }); @@ -74,10 +71,21 @@ 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 ( @@ -124,12 +132,21 @@ class SaveQuery extends React.PureComponent { )} + {isSaved && ( + + )}