[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
apache · Feb 14, 2018 · 342180b · 342180b
1 parent d2d9731
commit 342180b
Show file tree

Hide file tree

Showing 29 changed files with 478 additions and 211 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 (
       <div
@@ -95,8 +98,6 @@ class GridCell extends React.PureComponent {
         <div ref={this.getHeaderId(slice)}>
           <SliceHeader
             slice={slice}
-            exploreChartUrl={exploreChartUrl}
-            exportCSVUrl={exportCSVUrl}
             isExpanded={isExpanded}
             isCached={isCached}
             cachedDttm={cachedDttm}
@@ -106,6 +107,8 @@ class GridCell extends React.PureComponent {
             forceRefresh={forceRefresh}
             editMode={this.props.editMode}
             annotationQuery={annotationQuery}
+            exploreChart={exploreChart}
+            exportCSV={exportCSV}
           />
         </div>
         {

diff --git 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
@@ -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 {
                   </TooltipWrapper>
                 </a>
               }
-              <a
-                className={`refresh ${isCached ? 'danger' : ''}`}
-                onClick={() => (this.props.forceRefresh(slice.slice_id))}
-              >
+              <a className={`refresh ${isCached ? 'danger' : ''}`} onClick={this.forceRefresh}>
                 <TooltipWrapper
                   placement="top"
                   label="refresh"
@@ -110,7 +116,7 @@ class SliceHeader extends React.PureComponent {
                 </TooltipWrapper>
               </a>
               {slice.description &&
-              <a onClick={() => this.props.toggleExpandSlice(slice, !isExpanded)}>
+              <a onClick={this.onToggleExpandSlice}>
                 <TooltipWrapper
                   placement="top"
                   label="description"
@@ -129,7 +135,7 @@ class SliceHeader extends React.PureComponent {
                   <i className="fa fa-pencil" />
                 </TooltipWrapper>
               </a>
-              <a className="exportCSV" href={this.props.exportCSVUrl}>
+              <a className="exportCSV" onClick={this.exportCSV}>
                 <TooltipWrapper
                   placement="top"
                   label="exportCSV"
@@ -138,7 +144,7 @@ class SliceHeader extends React.PureComponent {
                   <i className="fa fa-table" />
                 </TooltipWrapper>
               </a>
-              <a className="exploreChart" href={this.props.exploreChartUrl} target="_blank">
+              <a className="exploreChart" onClick={this.exploreChart}>
                 <TooltipWrapper
                   placement="top"
                   label="exploreChart"
@@ -148,7 +154,7 @@ class SliceHeader extends React.PureComponent {
                 </TooltipWrapper>
               </a>
               {this.props.editMode &&
-                <a className="remove-chart" onClick={() => (this.props.removeSlice(slice))}>
+                <a className="remove-chart" onClick={this.removeSlice}>
                   <TooltipWrapper
                     placement="top"
                     label="close"

diff --git a/superset/assets/javascripts/explore/actions/exploreActions.js b/superset/assets/javascripts/explore/actions/exploreActions.js
@@ -132,6 +132,11 @@ export function updateExploreEndpoints(jsonUrl, csvUrl, standaloneUrl) {
   return { type: UPDATE_EXPLORE_ENDPOINTS, jsonUrl, csvUrl, standaloneUrl };
 }
 
+export const SET_EXPLORE_CONTROLS = 'UPDATE_EXPLORE_CONTROLS';
+export function setExploreControls(formData) {
+  return { type: SET_EXPLORE_CONTROLS, formData };
+}
+
 export const REMOVE_CONTROL_PANEL_ALERT = 'REMOVE_CONTROL_PANEL_ALERT';
 export function removeControlPanelAlert() {
   return { type: REMOVE_CONTROL_PANEL_ALERT };