[SIP-4] replace dashboard ajax calls with `SupersetClient` (#5854)

* [core] replace dashboard ajax calls with SupersetClient

* [core] fix SupersetClient dashboard tests

* [dashboard][superset-client] don't error by parsing save dashboard response as json

superset/assets/spec/javascripts/dashboard/components/DashboardBuilder_spec.jsx

@@ -1,6 +1,7 @@
 import { Provider } from 'react-redux';
 import React from 'react';
 import { shallow, mount } from 'enzyme';
+import sinon from 'sinon';
 
 import ParentSize from '@vx/responsive/build/components/ParentSize';
 import { Sticky, StickyContainer } from 'react-sticky';
@@ -11,6 +12,8 @@ import DashboardBuilder from '../../../../src/dashboard/components/DashboardBuil
 import DashboardComponent from '../../../../src/dashboard/containers/DashboardComponent';
 import DashboardHeader from '../../../../src/dashboard/containers/DashboardHeader';
 import DashboardGrid from '../../../../src/dashboard/containers/DashboardGrid';
+import * as dashboardStateActions from '../../../../src/dashboard/actions/dashboardState';
+
 import WithDragDropContext from '../helpers/WithDragDropContext';
 import {
   dashboardLayout as undoableDashboardLayout,
@@ -23,6 +26,19 @@ const dashboardLayout = undoableDashboardLayout.present;
 const layoutWithTabs = undoableDashboardLayoutWithTabs.present;
 
 describe('DashboardBuilder', () => {
+  let favStarStub;
+
+  beforeAll(() => {
+    // this is invoked on mount, so we stub it instead of making a request
+    favStarStub = sinon
+      .stub(dashboardStateActions, 'fetchFaveStar')
+      .returns({ type: 'mock-action' });
+  });
+
+  afterAll(() => {
+    favStarStub.restore();
+  });
+
   const props = {
     dashboardLayout,
     deleteTopLevelTabs() {},

superset/assets/spec/javascripts/dashboard/reducers/sliceEntities_spec.js

@@ -23,7 +23,7 @@ describe('sliceEntities reducer', () => {
   it('should set slices', () => {
     const result = sliceEntitiesReducer(
       { slices: { a: {} } },
-      { type: SET_ALL_SLICES, slices: { 1: {}, 2: {} } },
+      { type: SET_ALL_SLICES, payload: { slices: { 1: {}, 2: {} } } },
     );
 
     expect(result.slices).toEqual({
@@ -39,10 +39,10 @@ describe('sliceEntities reducer', () => {
       {},
       {
         type: FETCH_ALL_SLICES_FAILED,
-        error: { responseJSON: { message: 'errorrr' } },
+        payload: { error: 'failed' },
       },
     );
     expect(result.isLoading).toBe(false);
-    expect(result.errorMessage.indexOf('errorrr')).toBeGreaterThan(-1);
+    expect(result.errorMessage.indexOf('failed')).toBeGreaterThan(-1);
   });
 });

superset/assets/src/dashboard/actions/dashboardState.js

@@ -1,6 +1,6 @@
 /* eslint camelcase: 0 */
-import $ from 'jquery';
 import { ActionCreators as UndoActionCreators } from 'redux-undo';
+import { SupersetClient } from '@superset-ui/core';
 
 import { addChart, removeChart, refreshChart } from '../../chart/chartAction';
 import { chart as initChart } from '../../chart/chartReducer';
@@ -14,7 +14,6 @@ import {
 } from '../../logger';
 import { SAVE_TYPE_OVERWRITE } from '../util/constants';
 import { t } from '../../locales';
-
 import {
   addSuccessToast,
   addWarningToast,
@@ -57,12 +56,21 @@ export function toggleFaveStar(isStarred) {
 export const FETCH_FAVE_STAR = 'FETCH_FAVE_STAR';
 export function fetchFaveStar(id) {
   return function fetchFaveStarThunk(dispatch) {
-    const url = `${FAVESTAR_BASE_URL}/${id}/count`;
+    return SupersetClient.get({
-    return $.get(url).done(data => {
+      endpoint: `${FAVESTAR_BASE_URL}/${id}/count`,
-      if (data.count > 0) {
+    })
-        dispatch(toggleFaveStar(true));
+      .then(({ json }) => {
-      }
+        if (json.count > 0) dispatch(toggleFaveStar(true));
-    });
+      })
+      .catch(() =>
+        dispatch(
+          addDangerToast(
+            t(
+              'There was an issue fetching the favorite status of this dashboard.',
+            ),
+          ),
+        ),
+      );
   };
 }
 
@@ -70,9 +78,17 @@ export const SAVE_FAVE_STAR = 'SAVE_FAVE_STAR';
 export function saveFaveStar(id, isStarred) {
   return function saveFaveStarThunk(dispatch) {
     const urlSuffix = isStarred ? 'unselect' : 'select';
-    const url = `${FAVESTAR_BASE_URL}/${id}/${urlSuffix}/`;
+    return SupersetClient.get({
-    $.get(url);
+      endpoint: `${FAVESTAR_BASE_URL}/${id}/${urlSuffix}/`,
-    dispatch(toggleFaveStar(!isStarred));
+    })
+      .then(() => {
+        dispatch(toggleFaveStar(!isStarred));
+      })
+      .catch(() =>
+        dispatch(
+          addDangerToast(t('There was an issue favoriting this dashboard.')),
+        ),
+      );
   };
 }
 
@@ -111,28 +127,30 @@ export function saveDashboardRequestSuccess() {
 
 export function saveDashboardRequest(data, id, saveType) {
   const path = saveType === SAVE_TYPE_OVERWRITE ? 'save_dash' : 'copy_dash';
-  const url = `/superset/${path}/${id}/`;
+
   return dispatch =>
-    $.ajax({
+    SupersetClient.post({
-      type: 'POST',
+      endpoint: `/superset/${path}/${id}/`,
-      url,
+      postPayload: { data },
-      data: {
+      parseMethod: null,
-        data: JSON.stringify(data),
+    })
-      },
+      .then(response =>
-      success: () => {
+        Promise.all([
-        dispatch(saveDashboardRequestSuccess());
+          Promise.resolve(response),
-        dispatch(addSuccessToast(t('This dashboard was saved successfully.')));
+          dispatch(saveDashboardRequestSuccess()),
-      },
+          dispatch(
-      error: error => {
+            addSuccessToast(t('This dashboard was saved successfully.')),
-        const errorMsg = getAjaxErrorMsg(error);
+          ),
+        ]),
+      )
+      .catch(error =>
         dispatch(
           addDangerToast(
             `${t('Sorry, there was an error saving this dashboard: ')}
-          ${errorMsg}`,
+          ${getAjaxErrorMsg(error) || error}`,
           ),
-        );
+        ),
-      },
+      );
-    });
 }
 
 export function fetchCharts(chartList = [], force = false, interval = 0) {

superset/assets/src/dashboard/actions/datasources.js

@@ -1,4 +1,5 @@
-import $ from 'jquery';
+import { SupersetClient } from '@superset-ui/core';
+import { getAjaxErrorMsg } from '../../modules/utils';
 
 export const SET_DATASOURCE = 'SET_DATASOURCE';
 export function setDatasource(datasource, key) {
@@ -24,13 +25,12 @@ export function fetchDatasourceMetadata(key) {
       return dispatch(setDatasource(datasource, key));
     }
 
-    const url = `/superset/fetch_datasource_metadata?datasourceKey=${key}`;
+    return SupersetClient.get({
-    return $.ajax({
+      endpoint: `/superset/fetch_datasource_metadata?datasourceKey=${key}`,
-      type: 'GET',
+    })
-      url,
+      .then(data => dispatch(data, key))
-      success: data => dispatch(setDatasource(data, key)),
+      .catch(error =>
-      error: error =>
+        dispatch(fetchDatasourceFailed(getAjaxErrorMsg(error), key)),
-        dispatch(fetchDatasourceFailed(error.responseJSON.error, key)),
+      );
-    });
   };
 }

superset/assets/src/dashboard/actions/sliceEntities.js

@@ -1,11 +1,13 @@
 /* eslint camelcase: 0 */
-import $ from 'jquery';
+import { SupersetClient } from '@superset-ui/core';
 
+import { addDangerToast } from '../../messageToasts/actions';
+import { t } from '../../locales';
 import { getDatasourceParameter } from '../../modules/utils';
 
 export const SET_ALL_SLICES = 'SET_ALL_SLICES';
 export function setAllSlices(slices) {
-  return { type: SET_ALL_SLICES, slices };
+  return { type: SET_ALL_SLICES, payload: { slices } };
 }
 
 export const FETCH_ALL_SLICES_STARTED = 'FETCH_ALL_SLICES_STARTED';
@@ -15,7 +17,7 @@ export function fetchAllSlicesStarted() {
 
 export const FETCH_ALL_SLICES_FAILED = 'FETCH_ALL_SLICES_FAILED';
 export function fetchAllSlicesFailed(error) {
-  return { type: FETCH_ALL_SLICES_FAILED, error };
+  return { type: FETCH_ALL_SLICES_FAILED, payload: { error } };
 }
 
 export function fetchAllSlices(userId) {
@@ -24,13 +26,12 @@ export function fetchAllSlices(userId) {
     if (sliceEntities.lastUpdated === 0) {
       dispatch(fetchAllSlicesStarted());
 
-      const uri = `/sliceaddview/api/read?_flt_0_created_by=${userId}`;
+      return SupersetClient.get({
-      return $.ajax({
+        endpoint: `/sliceaddview/api/read?_flt_0_created_by=${userId}`,
-        url: uri,
+      })
-        type: 'GET',
+        .then(({ json }) => {
-        success: response => {
           const slices = {};
-          response.result.forEach(slice => {
+          json.result.forEach(slice => {
             let form_data = JSON.parse(slice.params);
             let datasource = form_data.datasource;
             if (!datasource) {
@@ -60,10 +61,26 @@ export function fetchAllSlices(userId) {
               };
             }
           });
+
           return dispatch(setAllSlices(slices));
-        },
+        })
-        error: error => dispatch(fetchAllSlicesFailed(error)),
+        .catch(error =>
-      });
+          Promise.all([
+            dispatch(
+              fetchAllSlicesFailed(
+                error.error ||
+                  error.statusText ||
+                  t('Could not fetch all saved charts'),
+              ),
+            ),
+            dispatch(
+              addDangerToast(
+                t('Sorry there was an error fetching saved charts: ') +
+                  error.error || error.statusText,
+              ),
+            ),
+          ]),
+        );
     }
 
     return dispatch(setAllSlices(sliceEntities.slices));

superset/assets/src/dashboard/components/SaveModal.jsx

@@ -93,9 +93,14 @@ class SaveModal extends React.PureComponent {
         t('You must pick a name for the new dashboard'),
       );
     } else {
-      this.onSave(data, dashboardId, saveType).done(resp => {
+      this.onSave(data, dashboardId, saveType).then(([resp]) => {
-        if (saveType === SAVE_TYPE_NEWDASHBOARD) {
+        if (
-          window.location = `/superset/dashboard/${resp.id}/`;
+          saveType === SAVE_TYPE_NEWDASHBOARD &&
+          resp &&
+          resp.json &&
+          resp.json.id
+        ) {
+          window.location = `/superset/dashboard/${resp.json.id}/`;
         }
       });
       this.modal.close();

superset/assets/src/dashboard/components/SliceAdder.jsx

@@ -226,8 +226,6 @@ class SliceAdder extends React.Component {
 
         {this.props.isLoading && <Loading />}
 
-        {this.props.errorMessage && <div>{this.props.errorMessage}</div>}
-
         {!this.props.isLoading &&
           this.state.filteredSlices.length > 0 && (
             <List
@@ -243,6 +241,10 @@ class SliceAdder extends React.Component {
             />
           )}
 
+        {this.props.errorMessage && (
+          <div className="error-message">{this.props.errorMessage}</div>
+        )}
+
         {/* Drag preview is just a single fixed-position element */}
         <AddSliceDragPreview slices={this.state.filteredSlices} />
       </div>

superset/assets/src/dashboard/components/gridComponents/Chart.jsx

@@ -233,7 +233,7 @@ class Chart extends React.Component {
             latestQueryFormData={chart.latestQueryFormData}
             lastRendered={chart.lastRendered}
             queryResponse={chart.queryResponse}
-            queryRequest={chart.queryRequest}
+            queryController={chart.queryController}
             triggerQuery={chart.triggerQuery}
           />
         </div>

superset/assets/src/dashboard/reducers/sliceEntities.js

@@ -3,6 +3,7 @@ import {
   FETCH_ALL_SLICES_STARTED,
   SET_ALL_SLICES,
 } from '../actions/sliceEntities';
+
 import { t } from '../../locales';
 
 export const initSliceEntities = {
@@ -27,22 +28,17 @@ export default function sliceEntitiesReducer(
       return {
         ...state,
         isLoading: false,
-        slices: { ...state.slices, ...action.slices }, // append more slices
+        slices: { ...state.slices, ...action.payload.slices },
         lastUpdated: new Date().getTime(),
       };
     },
     [FETCH_ALL_SLICES_FAILED]() {
-      const respJSON = action.error.responseJSON;
-      const errorMessage =
-        t('Sorry, there was an error fetching slices: ') +
-        (respJSON && respJSON.message)
-          ? respJSON.message
-          : action.error.responseText;
       return {
         ...state,
         isLoading: false,
-        errorMessage,
         lastUpdated: new Date().getTime(),
+        errorMessage:
+          action.payload.error || t('Could not fetch all saved charts'),
       };
     },
   };

superset/assets/src/dashboard/stylesheets/builder-sidepane.less

@@ -127,6 +127,13 @@
   }
 
   .slice-adder-container {
+    position: relative;
+    min-height: 200px; /* for loader positioning */
+
+    .error-message {
+      padding: 16px;
+    }
+
     .controls {
       display: flex;
       padding: 16px;

superset/assets/src/dashboard/util/propShapes.jsx

@@ -27,7 +27,7 @@ export const chartPropShape = PropTypes.shape({
   chartUpdateEndTime: PropTypes.number,
   chartUpdateStartTime: PropTypes.number,
   latestQueryFormData: PropTypes.object,
-  queryRequest: PropTypes.object,
+  queryController: PropTypes.shape({ abort: PropTypes.func }),
   queryResponse: PropTypes.object,
   triggerQuery: PropTypes.bool,
   lastRendered: PropTypes.number,