From 462c58ee67b83eb72aad44651bc8f78d39e0370c Mon Sep 17 00:00:00 2001 From: Chris Williams Date: Tue, 16 Oct 2018 13:42:22 -0700 Subject: [PATCH] [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 --- .../components/DashboardBuilder_spec.jsx | 16 ++++ .../dashboard/reducers/sliceEntities_spec.js | 6 +- .../src/dashboard/actions/dashboardState.js | 74 ++++++++++++------- .../src/dashboard/actions/datasources.js | 18 ++--- .../src/dashboard/actions/sliceEntities.js | 41 +++++++--- .../src/dashboard/components/SaveModal.jsx | 11 ++- .../src/dashboard/components/SliceAdder.jsx | 6 +- .../components/gridComponents/Chart.jsx | 2 +- .../src/dashboard/reducers/sliceEntities.js | 12 +-- .../stylesheets/builder-sidepane.less | 7 ++ .../assets/src/dashboard/util/propShapes.jsx | 2 +- 11 files changed, 128 insertions(+), 67 deletions(-) diff --git a/superset/assets/spec/javascripts/dashboard/components/DashboardBuilder_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/DashboardBuilder_spec.jsx index 7fb6b38f6d..06d508e2b3 100644 --- a/superset/assets/spec/javascripts/dashboard/components/DashboardBuilder_spec.jsx +++ b/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() {}, diff --git a/superset/assets/spec/javascripts/dashboard/reducers/sliceEntities_spec.js b/superset/assets/spec/javascripts/dashboard/reducers/sliceEntities_spec.js index 34c0e7ac21..42b3a3fc1f 100644 --- a/superset/assets/spec/javascripts/dashboard/reducers/sliceEntities_spec.js +++ b/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); }); }); diff --git a/superset/assets/src/dashboard/actions/dashboardState.js b/superset/assets/src/dashboard/actions/dashboardState.js index 17f6d46da0..89c0a9aba7 100644 --- a/superset/assets/src/dashboard/actions/dashboardState.js +++ b/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 $.get(url).done(data => { - if (data.count > 0) { - dispatch(toggleFaveStar(true)); - } - }); + return SupersetClient.get({ + endpoint: `${FAVESTAR_BASE_URL}/${id}/count`, + }) + .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}/`; - $.get(url); - dispatch(toggleFaveStar(!isStarred)); + return SupersetClient.get({ + endpoint: `${FAVESTAR_BASE_URL}/${id}/${urlSuffix}/`, + }) + .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({ - type: 'POST', - url, - data: { - data: JSON.stringify(data), - }, - success: () => { - dispatch(saveDashboardRequestSuccess()); - dispatch(addSuccessToast(t('This dashboard was saved successfully.'))); - }, - error: error => { - const errorMsg = getAjaxErrorMsg(error); + SupersetClient.post({ + endpoint: `/superset/${path}/${id}/`, + postPayload: { data }, + parseMethod: null, + }) + .then(response => + Promise.all([ + Promise.resolve(response), + dispatch(saveDashboardRequestSuccess()), + dispatch( + addSuccessToast(t('This dashboard was saved successfully.')), + ), + ]), + ) + .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) { diff --git a/superset/assets/src/dashboard/actions/datasources.js b/superset/assets/src/dashboard/actions/datasources.js index d97296e9e8..eab9e99f22 100644 --- a/superset/assets/src/dashboard/actions/datasources.js +++ b/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 $.ajax({ - type: 'GET', - url, - success: data => dispatch(setDatasource(data, key)), - error: error => - dispatch(fetchDatasourceFailed(error.responseJSON.error, key)), - }); + return SupersetClient.get({ + endpoint: `/superset/fetch_datasource_metadata?datasourceKey=${key}`, + }) + .then(data => dispatch(data, key)) + .catch(error => + dispatch(fetchDatasourceFailed(getAjaxErrorMsg(error), key)), + ); }; } diff --git a/superset/assets/src/dashboard/actions/sliceEntities.js b/superset/assets/src/dashboard/actions/sliceEntities.js index b635ea05f5..33e3507c1a 100644 --- a/superset/assets/src/dashboard/actions/sliceEntities.js +++ b/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 $.ajax({ - url: uri, - type: 'GET', - success: response => { + return SupersetClient.get({ + endpoint: `/sliceaddview/api/read?_flt_0_created_by=${userId}`, + }) + .then(({ json }) => { 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)); diff --git a/superset/assets/src/dashboard/components/SaveModal.jsx b/superset/assets/src/dashboard/components/SaveModal.jsx index 8194f4662e..13863a24c2 100644 --- a/superset/assets/src/dashboard/components/SaveModal.jsx +++ b/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 => { - if (saveType === SAVE_TYPE_NEWDASHBOARD) { - window.location = `/superset/dashboard/${resp.id}/`; + this.onSave(data, dashboardId, saveType).then(([resp]) => { + if ( + saveType === SAVE_TYPE_NEWDASHBOARD && + resp && + resp.json && + resp.json.id + ) { + window.location = `/superset/dashboard/${resp.json.id}/`; } }); this.modal.close(); diff --git a/superset/assets/src/dashboard/components/SliceAdder.jsx b/superset/assets/src/dashboard/components/SliceAdder.jsx index 31ad79745c..608cfc15a3 100644 --- a/superset/assets/src/dashboard/components/SliceAdder.jsx +++ b/superset/assets/src/dashboard/components/SliceAdder.jsx @@ -226,8 +226,6 @@ class SliceAdder extends React.Component { {this.props.isLoading && } - {this.props.errorMessage &&
{this.props.errorMessage}
} - {!this.props.isLoading && this.state.filteredSlices.length > 0 && ( )} + {this.props.errorMessage && ( +
{this.props.errorMessage}
+ )} + {/* Drag preview is just a single fixed-position element */} diff --git a/superset/assets/src/dashboard/components/gridComponents/Chart.jsx b/superset/assets/src/dashboard/components/gridComponents/Chart.jsx index 7e2e6a65d5..96c92d9d4d 100644 --- a/superset/assets/src/dashboard/components/gridComponents/Chart.jsx +++ b/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} /> diff --git a/superset/assets/src/dashboard/reducers/sliceEntities.js b/superset/assets/src/dashboard/reducers/sliceEntities.js index c5e46c26ab..a96368a9ac 100644 --- a/superset/assets/src/dashboard/reducers/sliceEntities.js +++ b/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'), }; }, }; diff --git a/superset/assets/src/dashboard/stylesheets/builder-sidepane.less b/superset/assets/src/dashboard/stylesheets/builder-sidepane.less index 17c87a8f3c..08355e3bbd 100644 --- a/superset/assets/src/dashboard/stylesheets/builder-sidepane.less +++ b/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; diff --git a/superset/assets/src/dashboard/util/propShapes.jsx b/superset/assets/src/dashboard/util/propShapes.jsx index 3c06e14cf3..faedb91aee 100644 --- a/superset/assets/src/dashboard/util/propShapes.jsx +++ b/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,