From 1be8c0012c4a4c53caf0d7c8c65d37f13d6323c8 Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Tue, 13 Feb 2018 17:21:15 -0800 Subject: [PATCH] [Explore view] Use POST method for charting requests (#3993) * [Explore view] Use POST method for charting requests * fix per code review comments * more code review fixes * code review fix: remove duplicated calls for getting values from request * [Explore view] Use POST method for charting requests * fix per code review comments * more code review fixes * code review fix: remove duplicated calls for getting values from request --- .../SqlLab/components/VisualizeModal.jsx | 5 +- .../assets/javascripts/chart/chartAction.js | 24 +-- .../assets/javascripts/chart/chartReducer.js | 3 +- .../assets/javascripts/dashboard/actions.js | 18 +- .../dashboard/components/Dashboard.jsx | 15 ++ .../dashboard/components/GridCell.jsx | 13 +- .../dashboard/components/GridLayout.jsx | 9 +- .../dashboard/components/SliceHeader.jsx | 30 ++-- .../explore/actions/exploreActions.js | 5 + .../explore/actions/saveModalActions.js | 27 ++- .../explore/components/DisplayQueryButton.jsx | 14 +- .../explore/components/EmbedCodeButton.jsx | 5 +- .../components/ExploreActionButtons.jsx | 22 ++- .../explore/components/ExploreChartHeader.jsx | 32 ++-- .../explore/components/ExploreChartPanel.jsx | 2 + .../components/ExploreViewContainer.jsx | 56 +++++- .../explore/components/SaveModal.jsx | 4 +- .../explore/components/URLShortLinkButton.jsx | 5 +- .../javascripts/explore/exploreUtils.js | 82 +++++++-- .../explore/reducers/exploreReducer.js | 4 + .../javascripts/explore/chartActions_spec.js | 3 +- .../components/EmbedCodeButton_spec.jsx | 11 +- .../explore/components/SaveModal_spec.jsx | 17 +- .../spec/javascripts/explore/utils_spec.jsx | 160 +++++++++++++----- .../sqllab/VisualizeModal_spec.jsx | 15 +- superset/models/core.py | 10 +- superset/views/core.py | 58 ++++--- tests/core_tests.py | 26 +-- tests/druid_tests.py | 14 +- 29 files changed, 478 insertions(+), 211 deletions(-) diff --git a/superset/assets/javascripts/SqlLab/components/VisualizeModal.jsx b/superset/assets/javascripts/SqlLab/components/VisualizeModal.jsx index 23692f9aa0c7f..1c4315ff91b36 100644 --- a/superset/assets/javascripts/SqlLab/components/VisualizeModal.jsx +++ b/superset/assets/javascripts/SqlLab/components/VisualizeModal.jsx @@ -9,7 +9,7 @@ import { Alert, Button, Col, Modal } from 'react-bootstrap'; import Select from 'react-select'; import { Table } from 'reactable'; import shortid from 'shortid'; -import { getExploreUrl } from '../../explore/exploreUtils'; +import { exportChart } from '../../explore/exploreUtils'; import * as actions from '../actions'; import { VISUALIZE_VALIDATION_ERRORS } from '../constants'; import visTypes from '../../explore/stores/visTypes'; @@ -166,7 +166,8 @@ class VisualizeModal extends React.PureComponent { } notify.info(t('Creating a data source and popping a new tab')); - window.open(getExploreUrl(formData)); + // open new window for data visualization + exportChart(formData); }) .fail(() => { notify.error(this.props.errorMessage); diff --git a/superset/assets/javascripts/chart/chartAction.js b/superset/assets/javascripts/chart/chartAction.js index 089a1dd1f7f7b..5f80d9a01cf90 100644 --- a/superset/assets/javascripts/chart/chartAction.js +++ b/superset/assets/javascripts/chart/chartAction.js @@ -1,12 +1,12 @@ -import { getExploreUrl, getAnnotationJsonUrl } from '../explore/exploreUtils'; +import { getExploreUrlAndPayload, getAnnotationJsonUrl } from '../explore/exploreUtils'; import { requiresQuery, ANNOTATION_SOURCE_TYPES } from '../modules/AnnotationTypes'; import { Logger, LOG_ACTIONS_LOAD_EVENT } from '../logger'; const $ = window.$ = require('jquery'); export const CHART_UPDATE_STARTED = 'CHART_UPDATE_STARTED'; -export function chartUpdateStarted(queryRequest, key) { - return { type: CHART_UPDATE_STARTED, queryRequest, key }; +export function chartUpdateStarted(queryRequest, latestQueryFormData, key) { + return { type: CHART_UPDATE_STARTED, queryRequest, latestQueryFormData, key }; } export const CHART_UPDATE_SUCCEEDED = 'CHART_UPDATE_SUCCEEDED'; @@ -109,18 +109,22 @@ export function renderTriggered(value, key) { export const RUN_QUERY = 'RUN_QUERY'; export function runQuery(formData, force = false, timeout = 60, key) { return (dispatch) => { - const url = getExploreUrl(formData, 'json', force); - let logStart; + const { url, payload } = getExploreUrlAndPayload({ + formData, + endpointType: 'json', + force, + }); + const logStart = Logger.getTimestamp(); const queryRequest = $.ajax({ + type: 'POST', url, dataType: 'json', - timeout: timeout * 1000, - beforeSend: () => { - logStart = Logger.getTimestamp(); + data: { + form_data: JSON.stringify(payload), }, + timeout: timeout * 1000, }); - - const queryPromise = Promise.resolve(dispatch(chartUpdateStarted(queryRequest, key))) + const queryPromise = Promise.resolve(dispatch(chartUpdateStarted(queryRequest, payload, key))) .then(() => queryRequest) .then((queryResponse) => { Logger.append(LOG_ACTIONS_LOAD_EVENT, { diff --git a/superset/assets/javascripts/chart/chartReducer.js b/superset/assets/javascripts/chart/chartReducer.js index e1dfe0524d4e7..6c4c43cc9c081 100644 --- a/superset/assets/javascripts/chart/chartReducer.js +++ b/superset/assets/javascripts/chart/chartReducer.js @@ -24,7 +24,7 @@ export const chart = { chartStatus: 'loading', chartUpdateEndTime: null, chartUpdateStartTime: now(), - latestQueryFormData: null, + latestQueryFormData: {}, queryRequest: null, queryResponse: null, triggerQuery: true, @@ -47,6 +47,7 @@ export default function chartReducer(charts = {}, action) { chartUpdateEndTime: null, chartUpdateStartTime: now(), queryRequest: action.queryRequest, + latestQueryFormData: action.latestQueryFormData, }; }, [actions.CHART_UPDATE_STOPPED](state) { diff --git a/superset/assets/javascripts/dashboard/actions.js b/superset/assets/javascripts/dashboard/actions.js index 6da6b43ac773a..c7f1a6aa26e47 100644 --- a/superset/assets/javascripts/dashboard/actions.js +++ b/superset/assets/javascripts/dashboard/actions.js @@ -1,6 +1,6 @@ /* global notify */ import $ from 'jquery'; -import { getExploreUrl } from '../explore/exploreUtils'; +import { getExploreUrlAndPayload } from '../explore/exploreUtils'; export const ADD_FILTER = 'ADD_FILTER'; export function addFilter(sliceId, col, vals, merge = true, refresh = true) { @@ -59,10 +59,20 @@ export function saveSlice(slice, sliceName) { sliceParams.slice_id = slice.slice_id; sliceParams.action = 'overwrite'; sliceParams.slice_name = sliceName; - const saveUrl = getExploreUrl(slice.form_data, 'base', false, null, sliceParams); + + const { url, payload } = getExploreUrlAndPayload({ + formData: slice.form_data, + endpointType: 'base', + force: false, + curUrl: null, + requestParams: sliceParams, + }); return $.ajax({ - url: saveUrl, - type: 'GET', + url, + type: 'POST', + data: { + form_data: JSON.stringify(payload), + }, success: () => { dispatch(updateSliceName(slice, sliceName)); notify.success('This slice name was saved successfully.'); diff --git a/superset/assets/javascripts/dashboard/components/Dashboard.jsx b/superset/assets/javascripts/dashboard/components/Dashboard.jsx index bcc13778d2fe1..2a6a22799716c 100644 --- a/superset/assets/javascripts/dashboard/components/Dashboard.jsx +++ b/superset/assets/javascripts/dashboard/components/Dashboard.jsx @@ -4,6 +4,7 @@ import PropTypes from 'prop-types'; import AlertsWrapper from '../../components/AlertsWrapper'; import GridLayout from './GridLayout'; import Header from './Header'; +import { exportChart } from '../../explore/exploreUtils'; import { areObjectsEqual } from '../../reduxUtils'; import { Logger, ActionLog, LOG_ACTIONS_PAGE_LOAD, LOG_ACTIONS_LOAD_EVENT, LOG_ACTIONS_RENDER_EVENT } from '../../logger'; @@ -66,6 +67,8 @@ class Dashboard extends React.PureComponent { this.addSlicesToDashboard = this.addSlicesToDashboard.bind(this); this.fetchSlice = this.fetchSlice.bind(this); this.getFormDataExtra = this.getFormDataExtra.bind(this); + this.exploreChart = this.exploreChart.bind(this); + this.exportCSV = this.exportCSV.bind(this); this.props.actions.fetchFaveStar = this.props.actions.fetchFaveStar.bind(this); this.props.actions.saveFaveStar = this.props.actions.saveFaveStar.bind(this); this.props.actions.saveSlice = this.props.actions.saveSlice.bind(this); @@ -274,6 +277,16 @@ class Dashboard extends React.PureComponent { }); } + exploreChart(slice) { + const formData = this.getFormDataExtra(slice); + exportChart(formData); + } + + exportCSV(slice) { + const formData = this.getFormDataExtra(slice); + exportChart(formData, 'csv'); + } + // re-render chart without fetch rerenderCharts() { this.getAllSlices().forEach((slice) => { @@ -316,6 +329,8 @@ class Dashboard extends React.PureComponent { timeout={this.props.timeout} onChange={this.onChange} getFormDataExtra={this.getFormDataExtra} + exploreChart={this.exploreChart} + exportCSV={this.exportCSV} fetchSlice={this.fetchSlice} saveSlice={this.props.actions.saveSlice} removeSlice={this.props.actions.removeSlice} diff --git a/superset/assets/javascripts/dashboard/components/GridCell.jsx b/superset/assets/javascripts/dashboard/components/GridCell.jsx index 2748fccd9ae22..2ae9ea49c063b 100644 --- a/superset/assets/javascripts/dashboard/components/GridCell.jsx +++ b/superset/assets/javascripts/dashboard/components/GridCell.jsx @@ -16,8 +16,6 @@ const propTypes = { isExpanded: PropTypes.bool, widgetHeight: PropTypes.number, widgetWidth: PropTypes.number, - exploreChartUrl: PropTypes.string, - exportCSVUrl: PropTypes.string, slice: PropTypes.object, chartKey: PropTypes.string, formData: PropTypes.object, @@ -26,6 +24,8 @@ const propTypes = { removeSlice: PropTypes.func, updateSliceName: PropTypes.func, toggleExpandSlice: PropTypes.func, + exploreChart: PropTypes.func, + exportCSV: PropTypes.func, addFilter: PropTypes.func, getFilters: PropTypes.func, clearFilter: PropTypes.func, @@ -39,6 +39,8 @@ const defaultProps = { removeSlice: () => ({}), updateSliceName: () => ({}), toggleExpandSlice: () => ({}), + exploreChart: () => ({}), + exportCSV: () => ({}), addFilter: () => ({}), getFilters: () => ({}), clearFilter: () => ({}), @@ -83,9 +85,10 @@ class GridCell extends React.PureComponent { render() { const { - exploreChartUrl, exportCSVUrl, isExpanded, isLoading, isCached, cachedDttm, + isExpanded, isLoading, isCached, cachedDttm, removeSlice, updateSliceName, toggleExpandSlice, forceRefresh, chartKey, slice, datasource, formData, timeout, annotationQuery, + exploreChart, exportCSV, } = this.props; return (
{ diff --git a/superset/assets/javascripts/dashboard/components/GridLayout.jsx b/superset/assets/javascripts/dashboard/components/GridLayout.jsx index 68de7686cef5d..0361d652c5f3f 100644 --- a/superset/assets/javascripts/dashboard/components/GridLayout.jsx +++ b/superset/assets/javascripts/dashboard/components/GridLayout.jsx @@ -3,7 +3,6 @@ import PropTypes from 'prop-types'; import { Responsive, WidthProvider } from 'react-grid-layout'; import GridCell from './GridCell'; -import { getExploreUrl } from '../../explore/exploreUtils'; require('react-grid-layout/css/styles.css'); require('react-resizable/css/styles.css'); @@ -18,6 +17,8 @@ const propTypes = { timeout: PropTypes.number, onChange: PropTypes.func, getFormDataExtra: PropTypes.func, + exploreChart: PropTypes.func, + exportCSV: PropTypes.func, fetchSlice: PropTypes.func, saveSlice: PropTypes.func, removeSlice: PropTypes.func, @@ -34,6 +35,8 @@ const propTypes = { const defaultProps = { onChange: () => ({}), getFormDataExtra: () => ({}), + exploreChart: () => ({}), + exportCSV: () => ({}), fetchSlice: () => ({}), saveSlice: () => ({}), removeSlice: () => ({}), @@ -149,8 +152,8 @@ class GridLayout extends React.Component { timeout={this.props.timeout} widgetHeight={this.getWidgetHeight(slice)} widgetWidth={this.getWidgetWidth(slice)} - exploreChartUrl={getExploreUrl(this.props.getFormDataExtra(slice))} - exportCSVUrl={getExploreUrl(this.props.getFormDataExtra(slice), 'csv')} + exploreChart={this.props.exploreChart} + exportCSV={this.props.exportCSV} isExpanded={!!this.isExpanded(slice)} isLoading={currentChart.chartStatus === 'loading'} isCached={queryResponse.is_cached} diff --git a/superset/assets/javascripts/dashboard/components/SliceHeader.jsx b/superset/assets/javascripts/dashboard/components/SliceHeader.jsx index 1f4b475d8807f..8abcc86d61c72 100644 --- a/superset/assets/javascripts/dashboard/components/SliceHeader.jsx +++ b/superset/assets/javascripts/dashboard/components/SliceHeader.jsx @@ -8,16 +8,15 @@ import TooltipWrapper from '../../components/TooltipWrapper'; const propTypes = { slice: PropTypes.object.isRequired, - exploreChartUrl: PropTypes.string, - exportCSVUrl: PropTypes.string, isExpanded: PropTypes.bool, isCached: PropTypes.bool, cachedDttm: PropTypes.string, - formDataExtra: PropTypes.object, removeSlice: PropTypes.func, updateSliceName: PropTypes.func, toggleExpandSlice: PropTypes.func, forceRefresh: PropTypes.func, + exploreChart: PropTypes.func, + exportCSV: PropTypes.func, editMode: PropTypes.bool, annotationQuery: PropTypes.object, annotationError: PropTypes.object, @@ -28,6 +27,8 @@ const defaultProps = { removeSlice: () => ({}), updateSliceName: () => ({}), toggleExpandSlice: () => ({}), + exploreChart: () => ({}), + exportCSV: () => ({}), editMode: false, }; @@ -36,6 +37,11 @@ class SliceHeader extends React.PureComponent { super(props); this.onSaveTitle = this.onSaveTitle.bind(this); + this.onToggleExpandSlice = this.onToggleExpandSlice.bind(this); + this.exportCSV = this.props.exportCSV.bind(this, this.props.slice); + this.exploreChart = this.props.exploreChart.bind(this, this.props.slice); + this.forceRefresh = this.props.forceRefresh.bind(this, this.props.slice.slice_id); + this.removeSlice = this.props.removeSlice.bind(this, this.props.slice); } onSaveTitle(newTitle) { @@ -44,10 +50,13 @@ class SliceHeader extends React.PureComponent { } } + onToggleExpandSlice() { + this.props.toggleExpandSlice(this.props.slice, !this.props.isExpanded); + } + render() { const slice = this.props.slice; const isCached = this.props.isCached; - const isExpanded = !!this.props.isExpanded; const cachedWhen = moment.utc(this.props.cachedDttm).fromNow(); const refreshTooltip = isCached ? t('Served from data cached %s . Click to force refresh.', cachedWhen) : @@ -97,10 +106,7 @@ class SliceHeader extends React.PureComponent { } - (this.props.forceRefresh(slice.slice_id))} - > + {slice.description && - this.props.toggleExpandSlice(slice, !isExpanded)}> + - + - + {this.props.editMode && - (this.props.removeSlice(slice))}> + { - if (status === 'success') { +export function saveSlice(formData, requestParams) { + return (dispatch) => { + const { url, payload } = getExploreUrlAndPayload({ + formData, + endpointType: 'base', + force: false, + curUrl: null, + requestParams, + }); + return $.ajax({ + type: 'POST', + url, + data: { + form_data: JSON.stringify(payload), + }, + success: ((data) => { dispatch(saveSliceSuccess(data)); - } else { + }), + error: (() => { dispatch(saveSliceFailed()); - } + }), }); }; } diff --git a/superset/assets/javascripts/explore/components/DisplayQueryButton.jsx b/superset/assets/javascripts/explore/components/DisplayQueryButton.jsx index fe0cbdd42c86e..d098d2a9af04a 100644 --- a/superset/assets/javascripts/explore/components/DisplayQueryButton.jsx +++ b/superset/assets/javascripts/explore/components/DisplayQueryButton.jsx @@ -7,6 +7,7 @@ import sql from 'react-syntax-highlighter/dist/languages/sql'; import json from 'react-syntax-highlighter/dist/languages/json'; import github from 'react-syntax-highlighter/dist/styles/github'; import CopyToClipboard from './../../components/CopyToClipboard'; +import { getExploreUrlAndPayload } from '../exploreUtils'; import ModalTrigger from './../../components/ModalTrigger'; import Button from '../../components/Button'; @@ -23,7 +24,7 @@ const propTypes = { animation: PropTypes.bool, queryResponse: PropTypes.object, chartStatus: PropTypes.string, - queryEndpoint: PropTypes.string.isRequired, + latestQueryFormData: PropTypes.object.isRequired, }; const defaultProps = { animation: true, @@ -51,9 +52,16 @@ export default class DisplayQueryButton extends React.PureComponent { } fetchQuery() { this.setState({ isLoading: true }); + const { url, payload } = getExploreUrlAndPayload({ + formData: this.props.latestQueryFormData, + endpointType: 'query', + }); $.ajax({ - type: 'GET', - url: this.props.queryEndpoint, + type: 'POST', + url, + data: { + form_data: JSON.stringify(payload), + }, success: (data) => { this.setState({ language: data.language, diff --git a/superset/assets/javascripts/explore/components/EmbedCodeButton.jsx b/superset/assets/javascripts/explore/components/EmbedCodeButton.jsx index 01d59e1e2ad2e..a09a5335ad01f 100644 --- a/superset/assets/javascripts/explore/components/EmbedCodeButton.jsx +++ b/superset/assets/javascripts/explore/components/EmbedCodeButton.jsx @@ -2,10 +2,11 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Popover, OverlayTrigger } from 'react-bootstrap'; import CopyToClipboard from './../../components/CopyToClipboard'; +import { getExploreLongUrl } from '../exploreUtils'; import { t } from '../../locales'; const propTypes = { - slice: PropTypes.object.isRequired, + latestQueryFormData: PropTypes.object.isRequired, }; export default class EmbedCodeButton extends React.Component { @@ -29,7 +30,7 @@ export default class EmbedCodeButton extends React.Component { generateEmbedHTML() { const srcLink = ( window.location.origin + - this.props.slice.data.standalone_endpoint + + getExploreLongUrl(this.props.latestQueryFormData, 'standalone') + `&height=${this.state.height}` ); return ( diff --git a/superset/assets/javascripts/explore/components/ExploreActionButtons.jsx b/superset/assets/javascripts/explore/components/ExploreActionButtons.jsx index 57f2dfd744c45..74f9b73348ef3 100644 --- a/superset/assets/javascripts/explore/components/ExploreActionButtons.jsx +++ b/superset/assets/javascripts/explore/components/ExploreActionButtons.jsx @@ -5,29 +5,33 @@ import URLShortLinkButton from './URLShortLinkButton'; import EmbedCodeButton from './EmbedCodeButton'; import DisplayQueryButton from './DisplayQueryButton'; import { t } from '../../locales'; +import { exportChart } from '../exploreUtils'; const propTypes = { canDownload: PropTypes.oneOfType([PropTypes.string, PropTypes.bool]).isRequired, slice: PropTypes.object, - queryEndpoint: PropTypes.string.isRequired, - queryResponse: PropTypes.object, chartStatus: PropTypes.string, + latestQueryFormData: PropTypes.object, + queryResponse: PropTypes.object, }; export default function ExploreActionButtons({ - chartStatus, canDownload, slice, queryResponse, queryEndpoint }) { + canDownload, slice, chartStatus, latestQueryFormData, queryResponse }) { const exportToCSVClasses = cx('btn btn-default btn-sm', { 'disabled disabledButton': !canDownload, }); + const doExportCSV = exportChart.bind(this, latestQueryFormData, 'csv'); + const doExportChart = exportChart.bind(this, latestQueryFormData, 'json'); + if (slice) { return (
- + - +
); } return ( - + ); } diff --git a/superset/assets/javascripts/explore/components/ExploreChartHeader.jsx b/superset/assets/javascripts/explore/components/ExploreChartHeader.jsx index 0faf164df4cf9..00231df4a2069 100644 --- a/superset/assets/javascripts/explore/components/ExploreChartHeader.jsx +++ b/superset/assets/javascripts/explore/components/ExploreChartHeader.jsx @@ -9,7 +9,6 @@ import AlteredSliceTag from '../../components/AlteredSliceTag'; import FaveStar from '../../components/FaveStar'; import TooltipWrapper from '../../components/TooltipWrapper'; import Timer from '../../components/Timer'; -import { getExploreUrl } from '../exploreUtils'; import CachedLabel from '../../components/CachedLabel'; import { t } from '../../locales'; @@ -21,6 +20,7 @@ const CHART_STATUS_MAP = { const propTypes = { actions: PropTypes.object.isRequired, + addHistory: PropTypes.func, can_overwrite: PropTypes.bool.isRequired, can_download: PropTypes.bool.isRequired, isStarred: PropTypes.bool.isRequired, @@ -43,13 +43,13 @@ class ExploreChartHeader extends React.PureComponent { slice_name: newTitle, action: isNewSlice ? 'saveas' : 'overwrite', }; - const saveUrl = getExploreUrl(this.props.form_data, 'base', false, null, params); - this.props.actions.saveSlice(saveUrl) + this.props.actions.saveSlice(this.props.form_data, params) .then((data) => { if (isNewSlice) { this.props.actions.createNewSlice( data.can_add, data.can_download, data.can_overwrite, data.slice, data.form_data); + this.props.addHistory({ isReplace: true, title: `[slice] ${data.slice.slice_name}` }); } else { this.props.actions.updateChartTitle(newTitle); } @@ -68,12 +68,12 @@ class ExploreChartHeader extends React.PureComponent { render() { const formData = this.props.form_data; - const queryResponse = this.props.chart.queryResponse; - const data = { - csv_endpoint: getExploreUrl(formData, 'csv'), - json_endpoint: getExploreUrl(formData, 'json'), - standalone_endpoint: getExploreUrl(formData, 'standalone'), - }; + const { + chartStatus, + chartUpdateEndTime, + chartUpdateStartTime, + latestQueryFormData, + queryResponse } = this.props.chart; const chartSucceeded = ['success', 'rendered'].indexOf(this.props.chart.chartStatus) > 0; return (
}
diff --git a/superset/assets/javascripts/explore/components/ExploreChartPanel.jsx b/superset/assets/javascripts/explore/components/ExploreChartPanel.jsx index abb1bc0284fb0..5e4926ed26fca 100644 --- a/superset/assets/javascripts/explore/components/ExploreChartPanel.jsx +++ b/superset/assets/javascripts/explore/components/ExploreChartPanel.jsx @@ -9,6 +9,7 @@ import ExploreChartHeader from './ExploreChartHeader'; const propTypes = { actions: PropTypes.object.isRequired, + addHistory: PropTypes.func, can_overwrite: PropTypes.bool.isRequired, can_download: PropTypes.bool.isRequired, datasource: PropTypes.object, @@ -58,6 +59,7 @@ class ExploreChartPanel extends React.PureComponent { const header = ( (np.controls[key].renderTrigger && typeof this.props.controls[key] !== 'undefined' && - !areObjectsEqual(np.controls[key].value, this.props.controls[key].value)))) { + !areObjectsEqual(np.controls[key].value, this.props.controls[key].value))) + ) { this.props.actions.renderTriggered(new Date().getTime(), this.props.chart.chartKey); } } @@ -82,7 +89,8 @@ class ExploreViewContainer extends React.Component { } componentWillUnmount() { - window.removeEventListener('resize', this.handleResize.bind(this)); + window.removeEventListener('resize', this.handleResize); + window.removeEventListener('popstate', this.handlePopstate); } onQuery() { @@ -90,10 +98,7 @@ class ExploreViewContainer extends React.Component { this.props.actions.removeControlPanelAlert(); this.props.actions.triggerQuery(true, this.props.chart.chartKey); - history.pushState( - {}, - document.title, - getExploreUrl(this.props.form_data)); + this.addHistory({}); } onStop() { @@ -119,6 +124,27 @@ class ExploreViewContainer extends React.Component { } } + addHistory({ isReplace = false, title }) { + const { payload } = getExploreUrlAndPayload({ formData: this.props.form_data }); + const longUrl = getExploreLongUrl(this.props.form_data); + if (isReplace) { + history.replaceState( + payload, + title, + longUrl); + } else { + history.pushState( + payload, + title, + longUrl); + } + + // it seems some browsers don't support pushState title attribute + if (title) { + document.title = title; + } + } + handleResize() { clearTimeout(this.resizeTimer); this.resizeTimer = setTimeout(() => { @@ -126,6 +152,19 @@ class ExploreViewContainer extends React.Component { }, 250); } + handlePopstate() { + const formData = history.state; + if (formData && Object.keys(formData).length) { + this.props.actions.setExploreControls(formData); + this.props.actions.runQuery( + formData, + false, + this.props.timeout, + this.props.chart.chartKey, + ); + } + } + toggleModal() { this.setState({ showModal: !this.state.showModal }); } @@ -162,6 +201,7 @@ class ExploreViewContainer extends React.Component { width={this.state.width} height={this.state.height} {...this.props} + addHistory={this.addHistory} />); } diff --git a/superset/assets/javascripts/explore/components/SaveModal.jsx b/superset/assets/javascripts/explore/components/SaveModal.jsx index 2d716ae93bd71..6cc1a8ca2b8fc 100644 --- a/superset/assets/javascripts/explore/components/SaveModal.jsx +++ b/superset/assets/javascripts/explore/components/SaveModal.jsx @@ -5,7 +5,6 @@ import { connect } from 'react-redux'; import { Modal, Alert, Button, Radio } from 'react-bootstrap'; import Select from 'react-select'; -import { getExploreUrl } from '../exploreUtils'; import { t } from '../../locales'; const propTypes = { @@ -104,8 +103,7 @@ class SaveModal extends React.Component { } sliceParams.goto_dash = gotodash; - const saveUrl = getExploreUrl(this.props.form_data, 'base', false, null, sliceParams); - this.props.actions.saveSlice(saveUrl) + this.props.actions.saveSlice(this.props.form_data, sliceParams) .then((data) => { // Go to new slice url or dashboard url if (gotodash) { diff --git a/superset/assets/javascripts/explore/components/URLShortLinkButton.jsx b/superset/assets/javascripts/explore/components/URLShortLinkButton.jsx index ddae9a2206429..aa278b7a465ce 100644 --- a/superset/assets/javascripts/explore/components/URLShortLinkButton.jsx +++ b/superset/assets/javascripts/explore/components/URLShortLinkButton.jsx @@ -3,10 +3,11 @@ import PropTypes from 'prop-types'; import { Popover, OverlayTrigger } from 'react-bootstrap'; import CopyToClipboard from './../../components/CopyToClipboard'; import { getShortUrl } from '../../../utils/common'; +import { getExploreLongUrl } from '../exploreUtils'; import { t } from '../../locales'; const propTypes = { - slice: PropTypes.object.isRequired, + latestQueryFormData: PropTypes.object.isRequired, }; export default class URLShortLinkButton extends React.Component { @@ -24,7 +25,7 @@ export default class URLShortLinkButton extends React.Component { } getCopyUrl() { - const longUrl = window.location.pathname + window.location.search; + const longUrl = getExploreLongUrl(this.props.latestQueryFormData); getShortUrl(longUrl, this.onShortUrlSuccess.bind(this)); } diff --git a/superset/assets/javascripts/explore/exploreUtils.js b/superset/assets/javascripts/explore/exploreUtils.js index 8a01745d9a39f..309fce1e8e19c 100644 --- a/superset/assets/javascripts/explore/exploreUtils.js +++ b/superset/assets/javascripts/explore/exploreUtils.js @@ -19,31 +19,59 @@ export function getAnnotationJsonUrl(slice_id, form_data, isNative) { }).toString(); } -export function getExploreUrl(form_data, endpointType = 'base', force = false, - curUrl = null, requestParams = {}) { - if (!form_data.datasource) { +export function getURIDirectory(formData, endpointType = 'base') { + // Building the directory part of the URI + let directory = '/superset/explore/'; + if (['json', 'csv', 'query'].indexOf(endpointType) >= 0) { + directory = '/superset/explore_json/'; + } + const [datasource_id, datasource_type] = formData.datasource.split('__'); + directory += `${datasource_type}/${datasource_id}/`; + + return directory; +} + +export function getExploreLongUrl(formData, endpointType) { + if (!formData.datasource) { + return null; + } + + const uri = new URI('/'); + const directory = getURIDirectory(formData, endpointType); + const search = uri.search(true); + search.form_data = JSON.stringify(formData); + if (endpointType === 'standalone') { + search.standalone = 'true'; + } + return uri.directory(directory).search(search).toString(); +} + +export function getExploreUrlAndPayload({ + formData, + endpointType = 'base', + force = false, + curUrl = null, + requestParams = {}, +}) { + if (!formData.datasource) { return null; } // The search params from the window.location are carried through, // but can be specified with curUrl (used for unit tests to spoof // the window.location). - let uri = URI(window.location.search); + let uri = new URI([location.protocol, '//', location.host].join('')); if (curUrl) { uri = URI(URI(curUrl).search()); } - // Building the directory part of the URI - let directory = '/superset/explore/'; - if (['json', 'csv', 'query'].indexOf(endpointType) >= 0) { - directory = '/superset/explore_json/'; - } - const [datasource_id, datasource_type] = form_data.datasource.split('__'); - directory += `${datasource_type}/${datasource_id}/`; + const directory = getURIDirectory(formData, endpointType); // Building the querystring (search) part of the URI const search = uri.search(true); - search.form_data = JSON.stringify(form_data); + if (formData.slice_id) { + search.form_data = JSON.stringify({ slice_id: formData.slice_id }); + } if (force) { search.force = 'true'; } @@ -65,5 +93,33 @@ export function getExploreUrl(form_data, endpointType = 'base', force = false, }); } uri = uri.search(search).directory(directory); - return uri.toString(); + const payload = { ...formData }; + + return { + url: uri.toString(), + payload, + }; +} + +export function exportChart(formData, endpointType) { + const { url, payload } = getExploreUrlAndPayload({ formData, endpointType }); + + const exploreForm = document.createElement('form'); + exploreForm.action = url; + exploreForm.method = 'POST'; + exploreForm.target = '_blank'; + const token = document.createElement('input'); + token.type = 'hidden'; + token.name = 'csrf_token'; + token.value = (document.getElementById('csrf_token') || {}).value; + exploreForm.appendChild(token); + const data = document.createElement('input'); + data.type = 'hidden'; + data.name = 'form_data'; + data.value = JSON.stringify(payload); + exploreForm.appendChild(data); + + document.body.appendChild(exploreForm); + exploreForm.submit(); + document.body.removeChild(exploreForm); } diff --git a/superset/assets/javascripts/explore/reducers/exploreReducer.js b/superset/assets/javascripts/explore/reducers/exploreReducer.js index 7b55748800ff8..e366f285f410c 100644 --- a/superset/assets/javascripts/explore/reducers/exploreReducer.js +++ b/superset/assets/javascripts/explore/reducers/exploreReducer.js @@ -56,6 +56,10 @@ export default function exploreReducer(state = {}, action) { } return Object.assign({}, state, changes); }, + [actions.SET_EXPLORE_CONTROLS]() { + const controls = getControlsState(state, action.formData); + return Object.assign({}, state, { controls }); + }, [actions.UPDATE_CHART_TITLE]() { const updatedSlice = Object.assign({}, state.slice, { slice_name: action.slice_name }); return Object.assign({}, state, { slice: updatedSlice }); diff --git a/superset/assets/spec/javascripts/explore/chartActions_spec.js b/superset/assets/spec/javascripts/explore/chartActions_spec.js index 4caeccd3e2b90..d0f6c6b3d4f8f 100644 --- a/superset/assets/spec/javascripts/explore/chartActions_spec.js +++ b/superset/assets/spec/javascripts/explore/chartActions_spec.js @@ -13,7 +13,8 @@ describe('chart actions', () => { beforeEach(() => { dispatch = sinon.spy(); - urlStub = sinon.stub(exploreUtils, 'getExploreUrl').callsFake(() => ('mockURL')); + urlStub = sinon.stub(exploreUtils, 'getExploreUrlAndPayload') + .callsFake(() => ({ url: 'mockURL', payload: {} })); ajaxStub = sinon.stub($, 'ajax'); }); diff --git a/superset/assets/spec/javascripts/explore/components/EmbedCodeButton_spec.jsx b/superset/assets/spec/javascripts/explore/components/EmbedCodeButton_spec.jsx index 837269b9353a2..f60b254ee0276 100644 --- a/superset/assets/spec/javascripts/explore/components/EmbedCodeButton_spec.jsx +++ b/superset/assets/spec/javascripts/explore/components/EmbedCodeButton_spec.jsx @@ -3,16 +3,14 @@ import { expect } from 'chai'; import { describe, it } from 'mocha'; import { shallow, mount } from 'enzyme'; import { OverlayTrigger } from 'react-bootstrap'; +import sinon from 'sinon'; import EmbedCodeButton from '../../../../javascripts/explore/components/EmbedCodeButton'; +import * as exploreUtils from '../../../../javascripts/explore/exploreUtils'; describe('EmbedCodeButton', () => { const defaultProps = { - slice: { - data: { - standalone_endpoint: 'endpoint_url', - }, - }, + latestQueryFormData: { datasource: '107__table' }, }; it('renders', () => { @@ -25,11 +23,11 @@ describe('EmbedCodeButton', () => { }); it('returns correct embed code', () => { + const stub = sinon.stub(exploreUtils, 'getExploreLongUrl').callsFake(() => ('endpoint_url')); const wrapper = mount(); wrapper.setState({ height: '1000', width: '2000', - srcLink: 'http://localhost/endpoint_url', }); const embedHTML = ( ' { '' ); expect(wrapper.instance().generateEmbedHTML()).to.equal(embedHTML); + stub.restore(); }); }); diff --git a/superset/assets/spec/javascripts/explore/components/SaveModal_spec.jsx b/superset/assets/spec/javascripts/explore/components/SaveModal_spec.jsx index e548d21a60a0a..00d040aa1efd1 100644 --- a/superset/assets/spec/javascripts/explore/components/SaveModal_spec.jsx +++ b/superset/assets/spec/javascripts/explore/components/SaveModal_spec.jsx @@ -38,7 +38,7 @@ describe('SaveModal', () => { const defaultProps = { onHide: () => ({}), actions: saveModalActions, - form_data: {}, + form_data: { datasource: '107__table' }, }; const mockEvent = { target: { @@ -117,7 +117,7 @@ describe('SaveModal', () => { describe('saveOrOverwrite', () => { beforeEach(() => { - sinon.stub(exploreUtils, 'getExploreUrl').callsFake(() => ('mockURL')); + sinon.stub(exploreUtils, 'getExploreUrlAndPayload').callsFake(() => ({ url: 'mockURL', payload: defaultProps.form_data })); sinon.stub(saveModalActions, 'saveSlice').callsFake(() => { const d = $.Deferred(); d.resolve('done'); @@ -125,14 +125,15 @@ describe('SaveModal', () => { }); }); afterEach(() => { - exploreUtils.getExploreUrl.restore(); + exploreUtils.getExploreUrlAndPayload.restore(); saveModalActions.saveSlice.restore(); }); it('should save slice', () => { const wrapper = getWrapper(); wrapper.instance().saveOrOverwrite(true); - expect(saveModalActions.saveSlice.getCall(0).args[0]).to.equal('mockURL'); + const args = saveModalActions.saveSlice.getCall(0).args; + expect(args[0]).to.deep.equal(defaultProps.form_data); }); it('existing dashboard', () => { const wrapper = getWrapper(); @@ -144,8 +145,8 @@ describe('SaveModal', () => { wrapper.setState({ saveToDashboardId }); wrapper.instance().saveOrOverwrite(true); - const args = exploreUtils.getExploreUrl.getCall(0).args; - expect(args[4].save_to_dashboard_id).to.equal(saveToDashboardId); + const args = saveModalActions.saveSlice.getCall(0).args; + expect(args[1].save_to_dashboard_id).to.equal(saveToDashboardId); }); it('new dashboard', () => { const wrapper = getWrapper(); @@ -157,8 +158,8 @@ describe('SaveModal', () => { wrapper.setState({ newDashboardName }); wrapper.instance().saveOrOverwrite(true); - const args = exploreUtils.getExploreUrl.getCall(0).args; - expect(args[4].new_dashboard_name).to.equal(newDashboardName); + const args = saveModalActions.saveSlice.getCall(0).args; + expect(args[1].new_dashboard_name).to.equal(newDashboardName); }); }); diff --git a/superset/assets/spec/javascripts/explore/utils_spec.jsx b/superset/assets/spec/javascripts/explore/utils_spec.jsx index fc4c2b6b3f879..8d4f86837fc22 100644 --- a/superset/assets/spec/javascripts/explore/utils_spec.jsx +++ b/superset/assets/spec/javascripts/explore/utils_spec.jsx @@ -1,9 +1,10 @@ import { it, describe } from 'mocha'; import { expect } from 'chai'; import URI from 'urijs'; -import { getExploreUrl } from '../../../javascripts/explore/exploreUtils'; +import { getExploreUrlAndPayload, getExploreLongUrl } from '../../../javascripts/explore/exploreUtils'; describe('utils', () => { + const location = window.location; const formData = { datasource: '1__table', }; @@ -12,48 +13,129 @@ describe('utils', () => { expect(uri1.toString()).to.equal(uri2.toString()); } - it('getExploreUrl generates proper base url', () => { - // This assertion is to show clearly the value of location.href - // in the context of unit tests. - expect(location.href).to.equal('about:blank'); + describe('getExploreUrlAndPayload', () => { + it('generates proper base url', () => { + // This assertion is to show clearly the value of location.href + // in the context of unit tests. + expect(location.href).to.equal('about:blank'); - compareURI( - URI(getExploreUrl(formData, 'base', false, 'http://superset.com')), - URI('/superset/explore/table/1/').search({ form_data: sFormData }), - ); - }); - it('getExploreUrl generates proper json url', () => { - compareURI( - URI(getExploreUrl(formData, 'json', false, 'superset.com')), - URI('/superset/explore_json/table/1/').search({ form_data: sFormData }), - ); - }); - it('getExploreUrl generates proper json forced url', () => { - compareURI( - URI(getExploreUrl(formData, 'json', true, 'superset.com')), + const { url, payload } = getExploreUrlAndPayload({ + formData, + endpointType: 'base', + force: false, + curUrl: 'http://superset.com', + }); + compareURI( + URI(url), + URI('/superset/explore/table/1/'), + ); + expect(payload).to.deep.equals(formData); + }); + it('generates proper json url', () => { + const { url, payload } = getExploreUrlAndPayload({ + formData, + endpointType: 'json', + force: false, + curUrl: 'http://superset.com', + }); + compareURI( + URI(url), + URI('/superset/explore_json/table/1/'), + ); + expect(payload).to.deep.equals(formData); + }); + it('generates proper json forced url', () => { + const { url, payload } = getExploreUrlAndPayload({ + formData, + endpointType: 'json', + force: true, + curUrl: 'superset.com', + }); + compareURI( + URI(url), URI('/superset/explore_json/table/1/') - .search({ form_data: sFormData, force: 'true' }), - ); - }); - it('getExploreUrl generates proper csv URL', () => { - compareURI( - URI(getExploreUrl(formData, 'csv', false, 'superset.com')), + .search({ force: 'true' }), + ); + expect(payload).to.deep.equals(formData); + }); + it('generates proper csv URL', () => { + const { url, payload } = getExploreUrlAndPayload({ + formData, + endpointType: 'csv', + force: false, + curUrl: 'superset.com', + }); + compareURI( + URI(url), URI('/superset/explore_json/table/1/') - .search({ form_data: sFormData, csv: 'true' }), - ); - }); - it('getExploreUrl generates proper standalone URL', () => { - compareURI( - URI(getExploreUrl(formData, 'standalone', false, 'superset.com')), + .search({ csv: 'true' }), + ); + expect(payload).to.deep.equals(formData); + }); + it('generates proper standalone URL', () => { + const { url, payload } = getExploreUrlAndPayload({ + formData, + endpointType: 'standalone', + force: false, + curUrl: 'superset.com', + }); + compareURI( + URI(url), URI('/superset/explore/table/1/') - .search({ form_data: sFormData, standalone: 'true' }), - ); - }); - it('getExploreUrl preserves main URLs params', () => { - compareURI( - URI(getExploreUrl(formData, 'json', false, 'superset.com?foo=bar')), + .search({ standalone: 'true' }), + ); + expect(payload).to.deep.equals(formData); + }); + it('preserves main URLs params', () => { + const { url, payload } = getExploreUrlAndPayload({ + formData, + endpointType: 'json', + force: false, + curUrl: 'superset.com?foo=bar', + }); + compareURI( + URI(url), URI('/superset/explore_json/table/1/') - .search({ foo: 'bar', form_data: sFormData }), - ); + .search({ foo: 'bar' }), + ); + expect(payload).to.deep.equals(formData); + }); + it('generate proper save slice url', () => { + const { url, payload } = getExploreUrlAndPayload({ + formData, + endpointType: 'json', + force: false, + curUrl: 'superset.com?foo=bar', + }); + compareURI( + URI(url), + URI('/superset/explore_json/table/1/') + .search({ foo: 'bar' }), + ); + expect(payload).to.deep.equals(formData); + }); + it('generate proper saveas slice url', () => { + const { url, payload } = getExploreUrlAndPayload({ + formData, + endpointType: 'json', + force: false, + curUrl: 'superset.com?foo=bar', + }); + compareURI( + URI(url), + URI('/superset/explore_json/table/1/') + .search({ foo: 'bar' }), + ); + expect(payload).to.deep.equals(formData); + }); + }); + + describe('getExploreLongUrl', () => { + it('generates proper base url with form_data', () => { + compareURI( + URI(getExploreLongUrl(formData, 'base')), + URI('/superset/explore/table/1/').search({ form_data: sFormData }), + ); + }); }); }); diff --git a/superset/assets/spec/javascripts/sqllab/VisualizeModal_spec.jsx b/superset/assets/spec/javascripts/sqllab/VisualizeModal_spec.jsx index dffa8250ba03a..6c9fc5b1ed7d5 100644 --- a/superset/assets/spec/javascripts/sqllab/VisualizeModal_spec.jsx +++ b/superset/assets/spec/javascripts/sqllab/VisualizeModal_spec.jsx @@ -308,17 +308,17 @@ describe('VisualizeModal', () => { beforeEach(() => { ajaxSpy = sinon.spy($, 'ajax'); sinon.stub(JSON, 'parse').callsFake(() => ({ table_id: 107 })); - sinon.stub(exploreUtils, 'getExploreUrl').callsFake(() => ('mockURL')); + sinon.stub(exploreUtils, 'getExploreUrlAndPayload').callsFake(() => ({ url: 'mockURL', payload: { datasource: '107__table' } })); + sinon.spy(exploreUtils, 'exportChart'); sinon.stub(wrapper.instance(), 'buildVizOptions').callsFake(() => (mockOptions)); - sinon.spy(window, 'open'); datasourceSpy = sinon.stub(actions, 'createDatasource'); }); afterEach(() => { ajaxSpy.restore(); JSON.parse.restore(); - exploreUtils.getExploreUrl.restore(); + exploreUtils.getExploreUrlAndPayload.restore(); + exploreUtils.exportChart.restore(); wrapper.instance().buildVizOptions.restore(); - window.open.restore(); datasourceSpy.restore(); }); @@ -340,9 +340,8 @@ describe('VisualizeModal', () => { wrapper.setProps({ actions: { createDatasource: datasourceSpy } }); wrapper.instance().visualize(); - expect(exploreUtils.getExploreUrl.callCount).to.equal(1); - expect(exploreUtils.getExploreUrl.getCall(0).args[0].datasource).to.equal('107__table'); - expect(window.open.callCount).to.equal(1); + expect(exploreUtils.exportChart.callCount).to.equal(1); + expect(exploreUtils.exportChart.getCall(0).args[0].datasource).to.equal('107__table'); }); it('should notify error', () => { datasourceSpy.callsFake(() => { @@ -354,7 +353,7 @@ describe('VisualizeModal', () => { sinon.spy(notify, 'error'); wrapper.instance().visualize(); - expect(window.open.callCount).to.equal(0); + expect(exploreUtils.exportChart.callCount).to.equal(0); expect(notify.error.callCount).to.equal(1); }); }); diff --git a/superset/models/core.py b/superset/models/core.py index cfd6d7520373d..df45ccf533530 100644 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -209,10 +209,11 @@ def form_data(self): @property def slice_url(self): """Defines the url to access the slice""" + form_data = {'slice_id': self.id} return ( '/superset/explore/{obj.datasource_type}/' '{obj.datasource_id}/?form_data={params}'.format( - obj=self, params=parse.quote(json.dumps(self.form_data)))) + obj=self, params=parse.quote(json.dumps(form_data)))) @property def slice_id_url(self): @@ -860,9 +861,10 @@ def wrapper(*args, **kwargs): user_id = None if g.user: user_id = g.user.get_id() - d = request.args.to_dict() - post_data = request.form.to_dict() or {} - d.update(post_data) + d = request.form.to_dict() or {} + # request parameters can overwrite post body + request_params = request.args.to_dict() + d.update(request_params) d.update(kwargs) slice_id = d.get('slice_id') diff --git a/superset/views/core.py b/superset/views/core.py index 87adb6e5eb8f1..a5d689457a30f 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -718,11 +718,12 @@ def index(self, url_id): @expose('/shortner/', methods=['POST', 'GET']) def shortner(self): url = request.form.get('data') + directory = url.split('?')[0][2:] obj = models.Url(url=url) db.session.add(obj) db.session.commit() - return('http://{request.headers[Host]}/r/{obj.id}'.format( - request=request, obj=obj)) + return('http://{request.headers[Host]}/{directory}?r={obj.id}'.format( + request=request, directory=directory, obj=obj)) @expose('/msg/') def msg(self): @@ -928,16 +929,15 @@ def clean_fulfilled_requests(session): return redirect('/accessrequestsmodelview/list/') def get_form_data(self): - # get form data from url - if request.args.get('form_data'): - form_data = request.args.get('form_data') - elif request.form.get('form_data'): - # Supporting POST as well as get - form_data = request.form.get('form_data') - else: - form_data = '{}' - - d = json.loads(form_data) + d = {} + post_data = request.form.get('form_data') + request_args_data = request.args.get('form_data') + # Supporting POST + if post_data: + d.update(json.loads(post_data)) + # request params can overwrite post body + if request_args_data: + d.update(json.loads(request_args_data)) if request.args.get('viz_type'): # Converting old URLs @@ -1096,7 +1096,7 @@ def annotation_json(self, layer_id): @log_this @has_access_api - @expose('/explore_json///') + @expose('/explore_json///', methods=['GET', 'POST']) def explore_json(self, datasource_type, datasource_id): try: csv = request.args.get('csv') == 'true' @@ -1147,18 +1147,31 @@ def explorev2(self, datasource_type, datasource_id): @log_this @has_access - @expose('/explore///') + @expose('/explore///', methods=['GET', 'POST']) def explore(self, datasource_type, datasource_id): - form_data = self.get_form_data() - datasource_id = int(datasource_id) - viz_type = form_data.get('viz_type') - slice_id = form_data.get('slice_id') user_id = g.user.get_id() if g.user else None + form_data = self.get_form_data() + saved_url = None + url_id = request.args.get('r') + if url_id: + saved_url = db.session.query(models.Url).filter_by(id=url_id).first() + if saved_url: + url_str = parse.unquote_plus( + saved_url.url.split('?')[1][10:], encoding='utf-8', errors=None) + url_form_data = json.loads(url_str) + # allow form_date in request override saved url + url_form_data.update(form_data) + form_data = url_form_data + slice_id = form_data.get('slice_id') slc = None if slice_id: slc = db.session.query(models.Slice).filter_by(id=slice_id).first() + slice_form_data = slc.form_data.copy() + # allow form_data in request override slice from_data + slice_form_data.update(form_data) + form_data = slice_form_data error_redirect = '/slicemodelview/list/' datasource = ConnectorRegistry.get_datasource( @@ -1177,6 +1190,7 @@ def explore(self, datasource_type, datasource_id): 'datasource_id={datasource_id}&' ''.format(**locals())) + viz_type = form_data.get('viz_type') if not viz_type and datasource.default_endpoint: return redirect(datasource.default_endpoint) @@ -1187,6 +1201,9 @@ def explore(self, datasource_type, datasource_id): form_data['datasource'] = str(datasource_id) + '__' + datasource_type + # On explore, merge extra filters into the form data + merge_extra_filters(form_data) + # handle save or overwrite action = request.args.get('action') @@ -1210,11 +1227,6 @@ def explore(self, datasource_type, datasource_id): datasource_type, datasource.name) - form_data['datasource'] = str(datasource_id) + '__' + datasource_type - - # On explore, merge extra filters into the form data - merge_extra_filters(form_data) - standalone = request.args.get('standalone') == 'true' bootstrap_data = { 'can_add': slice_add_perm, diff --git a/tests/core_tests.py b/tests/core_tests.py index 8b4dd27ee3467..edd7ff580c2c9 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -100,10 +100,10 @@ def test_slice_json_endpoint(self): slc = self.get_slice('Girls', db.session) json_endpoint = ( - '/superset/explore_json/{}/{}?form_data={}' - .format(slc.datasource_type, slc.datasource_id, json.dumps(slc.viz.form_data)) + '/superset/explore_json/{}/{}/' + .format(slc.datasource_type, slc.datasource_id) ) - resp = self.get_resp(json_endpoint) + resp = self.get_resp(json_endpoint, {'form_data': json.dumps(slc.viz.form_data)}) assert '"Jennifer"' in resp def test_slice_csv_endpoint(self): @@ -111,10 +111,10 @@ def test_slice_csv_endpoint(self): slc = self.get_slice('Girls', db.session) csv_endpoint = ( - '/superset/explore_json/{}/{}?csv=true&form_data={}' - .format(slc.datasource_type, slc.datasource_id, json.dumps(slc.viz.form_data)) + '/superset/explore_json/{}/{}/?csv=true' + .format(slc.datasource_type, slc.datasource_id) ) - resp = self.get_resp(csv_endpoint) + resp = self.get_resp(csv_endpoint, {'form_data': json.dumps(slc.viz.form_data)}) assert 'Jennifer,' in resp def test_admin_only_permissions(self): @@ -155,7 +155,7 @@ def test_save_slice(self): url = ( '/superset/explore/table/{}/?slice_name={}&' - 'action={}&datasource_name=energy_usage&form_data={}') + 'action={}&datasource_name=energy_usage') form_data = { 'viz_type': 'sankey', @@ -170,8 +170,8 @@ def test_save_slice(self): tbl_id, copy_name, 'saveas', - json.dumps(form_data), ), + {'form_data': json.dumps(form_data)}, ) slices = db.session.query(models.Slice) \ .filter_by(slice_name=copy_name).all() @@ -191,8 +191,8 @@ def test_save_slice(self): tbl_id, new_slice_name, 'overwrite', - json.dumps(form_data), ), + {'form_data': json.dumps(form_data)}, ) slc = db.session.query(models.Slice).filter_by(id=new_slice_id).first() assert slc.slice_name == new_slice_name @@ -375,8 +375,8 @@ def test_shortner(self): 'energy_usage&datasource_id=1&datasource_type=table&' 'previous_viz_type=sankey' ) - resp = self.client.post('/r/shortner/', data=data) - assert '/r/' in resp.data.decode('utf-8') + resp = self.client.post('/r/shortner/', data=dict(data=data)) + assert '?r=' in resp.data.decode('utf-8') def test_kv(self): self.logout() @@ -780,7 +780,7 @@ def test_slice_id_is_always_logged_correctly_on_web_request(self): # superset/explore case slc = db.session.query(models.Slice).filter_by(slice_name='Girls').one() qry = db.session.query(models.Log).filter_by(slice_id=slc.id) - self.get_resp(slc.slice_url) + self.get_resp(slc.slice_url, {'form_data': json.dumps(slc.viz.form_data)}) self.assertEqual(1, qry.count()) def test_slice_id_is_always_logged_correctly_on_ajax_request(self): @@ -789,7 +789,7 @@ def test_slice_id_is_always_logged_correctly_on_ajax_request(self): slc = db.session.query(models.Slice).filter_by(slice_name='Girls').one() qry = db.session.query(models.Log).filter_by(slice_id=slc.id) slc_url = slc.slice_url.replace('explore', 'explore_json') - self.get_json_resp(slc_url) + self.get_json_resp(slc_url, {'form_data': json.dumps(slc.viz.form_data)}) self.assertEqual(1, qry.count()) def test_slice_query_endpoint(self): diff --git a/tests/druid_tests.py b/tests/druid_tests.py index c280da790a293..ee8cfba5f6f11 100644 --- a/tests/druid_tests.py +++ b/tests/druid_tests.py @@ -136,11 +136,8 @@ def test_client(self, PyDruid): 'force': 'true', } # One groupby - url = ( - '/superset/explore_json/druid/{}/?form_data={}'.format( - datasource_id, json.dumps(form_data)) - ) - resp = self.get_json_resp(url) + url = ('/superset/explore_json/druid/{}/'.format(datasource_id)) + resp = self.get_json_resp(url, {'form_data': json.dumps(form_data)}) self.assertEqual('Canada', resp['data']['records'][0]['dim1']) form_data = { @@ -156,11 +153,8 @@ def test_client(self, PyDruid): 'force': 'true', } # two groupby - url = ( - '/superset/explore_json/druid/{}/?form_data={}'.format( - datasource_id, json.dumps(form_data)) - ) - resp = self.get_json_resp(url) + url = ('/superset/explore_json/druid/{}/'.format(datasource_id)) + resp = self.get_json_resp(url, {'form_data': json.dumps(form_data)}) self.assertEqual('Canada', resp['data']['records'][0]['dim1']) def test_druid_sync_from_config(self):