diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/utils.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/utils.ts index 0b8776b06c1..44322e1c42c 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/utils.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/utils.ts @@ -163,10 +163,6 @@ export function interceptDatasets() { cy.intercept('GET', `/api/v1/dashboard/*/datasets`).as('getDatasets'); } -export function interceptDashboardasync() { - cy.intercept('GET', `/dashboardasync/api/read*`).as('getDashboardasync'); -} - export function interceptFilterState() { cy.intercept('POST', `/api/v1/dashboard/*/filter_state*`).as( 'postFilterState', diff --git a/superset-frontend/cypress-base/cypress/integration/explore/utils.ts b/superset-frontend/cypress-base/cypress/integration/explore/utils.ts index 15e7dcba1b6..04cf1f18199 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/utils.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/utils.ts @@ -17,10 +17,7 @@ * under the License. */ -import { - interceptGet as interceptDashboardGet, - interceptDashboardasync, -} from '../dashboard/utils'; +import { interceptGet as interceptDashboardGet } from '../dashboard/utils'; export function interceptFiltering() { cy.intercept('GET', `/api/v1/chart/?q=*`).as('filtering'); @@ -61,12 +58,10 @@ export function setFilter(filter: string, option: string) { export function saveChartToDashboard(dashboardName: string) { interceptDashboardGet(); - interceptDashboardasync(); interceptUpdate(); interceptExploreGet(); cy.getBySel('query-save-button').click(); - cy.wait('@getDashboardasync'); cy.getBySelLike('chart-modal').should('be.visible'); cy.get( '[data-test="save-chart-modal-select-dashboard-form"] [aria-label="Select a dashboard"]', diff --git a/superset-frontend/src/explore/actions/saveModalActions.js b/superset-frontend/src/explore/actions/saveModalActions.js index 1c3c3b765f7..9bd51613915 100644 --- a/superset-frontend/src/explore/actions/saveModalActions.js +++ b/superset-frontend/src/explore/actions/saveModalActions.js @@ -40,29 +40,6 @@ export function setSaveChartModalVisibility(isVisible) { return { type: SET_SAVE_CHART_MODAL_VISIBILITY, isVisible }; } -export function fetchDashboards(userId) { - return function fetchDashboardsThunk(dispatch) { - return SupersetClient.get({ - endpoint: `/dashboardasync/api/read?_flt_0_owners=${userId}`, - }) - .then(({ json }) => { - const choices = json.pks.map((id, index) => ({ - value: id, - label: (json.result[index] || {}).dashboard_title, - })); - choices.sort((a, b) => - a.label.localeCompare(b.label, { - sensitivity: 'base', - numeric: true, - }), - ); - - return dispatch(fetchDashboardsSucceeded(choices)); - }) - .catch(() => dispatch(fetchDashboardsFailed(userId))); - }; -} - export const SAVE_SLICE_FAILED = 'SAVE_SLICE_FAILED'; export function saveSliceFailed() { return { type: SAVE_SLICE_FAILED }; @@ -241,20 +218,6 @@ export const createDashboard = dashboardName => async dispatch => { } }; -// Get existing dashboard from ID -export const getDashboard = dashboardId => async dispatch => { - try { - const response = await SupersetClient.get({ - endpoint: `/api/v1/dashboard/${dashboardId}`, - }); - - return response.json; - } catch (error) { - dispatch(saveSliceFailed()); - throw error; - } -}; - // Get dashboards the slice is added to export const getSliceDashboards = slice => async dispatch => { try { diff --git a/superset-frontend/src/explore/actions/saveModalActions.test.js b/superset-frontend/src/explore/actions/saveModalActions.test.js index f89729f5ff9..1662ead63e5 100644 --- a/superset-frontend/src/explore/actions/saveModalActions.test.js +++ b/superset-frontend/src/explore/actions/saveModalActions.test.js @@ -23,10 +23,6 @@ import { ADD_TOAST } from 'src/components/MessageToasts/actions'; import { createDashboard, createSlice, - fetchDashboards, - FETCH_DASHBOARDS_FAILED, - FETCH_DASHBOARDS_SUCCEEDED, - getDashboard, getSliceDashboards, SAVE_SLICE_FAILED, SAVE_SLICE_SUCCESS, @@ -34,37 +30,6 @@ import { getSlicePayload, } from './saveModalActions'; -/** - * Tests fetchDashboards action - */ - -const userId = 1; -const fetchDashboardsEndpoint = `glob:*/dashboardasync/api/read?_flt_0_owners=${1}`; -const mockDashboardData = { - pks: ['id'], - result: [{ id: 'id', dashboard_title: 'dashboard title' }], -}; - -test('fetchDashboards handles success', async () => { - fetchMock.reset(); - fetchMock.get(fetchDashboardsEndpoint, mockDashboardData); - const dispatch = sinon.spy(); - await fetchDashboards(userId)(dispatch); - expect(fetchMock.calls(fetchDashboardsEndpoint)).toHaveLength(1); - expect(dispatch.callCount).toBe(1); - expect(dispatch.getCall(0).args[0].type).toBe(FETCH_DASHBOARDS_SUCCEEDED); -}); - -test('fetchDashboards handles failure', async () => { - fetchMock.reset(); - fetchMock.get(fetchDashboardsEndpoint, { throws: 'error' }); - const dispatch = sinon.spy(); - await fetchDashboards(userId)(dispatch); - expect(fetchMock.calls(fetchDashboardsEndpoint)).toHaveLength(4); // 3 retries - expect(dispatch.callCount).toBe(1); - expect(dispatch.getCall(0).args[0].type).toBe(FETCH_DASHBOARDS_FAILED); -}); - const sliceId = 10; const sliceName = 'New chart'; const vizType = 'sample_viz_type'; @@ -176,7 +141,6 @@ test('createSlice handles failure', async () => { expect(dispatch.getCall(0).args[0].type).toBe(SAVE_SLICE_FAILED); }); -const dashboardId = 14; const dashboardName = 'New dashboard'; const dashboardResponsePayload = { id: 14, @@ -214,38 +178,6 @@ test('createDashboard handles failure', async () => { expect(dispatch.getCall(0).args[0].type).toBe(SAVE_SLICE_FAILED); }); -/** - * Tests getDashboard action - */ - -const getDashboardEndpoint = `glob:*/api/v1/dashboard/${dashboardId}`; -test('getDashboard handles success', async () => { - fetchMock.reset(); - fetchMock.get(getDashboardEndpoint, dashboardResponsePayload); - const dispatch = sinon.spy(); - const dashboard = await getDashboard(dashboardId)(dispatch); - expect(fetchMock.calls(getDashboardEndpoint)).toHaveLength(1); - expect(dispatch.callCount).toBe(0); - expect(dashboard).toEqual(dashboardResponsePayload); -}); - -test('getDashboard handles failure', async () => { - fetchMock.reset(); - fetchMock.get(getDashboardEndpoint, { throws: sampleError }); - const dispatch = sinon.spy(); - let caughtError; - try { - await getDashboard(dashboardId)(dispatch); - } catch (error) { - caughtError = error; - } - - expect(caughtError).toEqual(sampleError); - expect(fetchMock.calls(getDashboardEndpoint)).toHaveLength(4); - expect(dispatch.callCount).toBe(1); - expect(dispatch.getCall(0).args[0].type).toBe(SAVE_SLICE_FAILED); -}); - test('updateSlice with add to new dashboard handles success', async () => { fetchMock.reset(); fetchMock.put(updateSliceEndpoint, sliceResponsePayload); diff --git a/superset-frontend/src/explore/components/SaveModal.test.jsx b/superset-frontend/src/explore/components/SaveModal.test.jsx index 15bfc64e758..74d1c1199c7 100644 --- a/superset-frontend/src/explore/components/SaveModal.test.jsx +++ b/superset-frontend/src/explore/components/SaveModal.test.jsx @@ -20,10 +20,8 @@ import React from 'react'; import configureStore from 'redux-mock-store'; import thunk from 'redux-thunk'; import { bindActionCreators } from 'redux'; -import { Provider } from 'react-redux'; import { shallow } from 'enzyme'; -import { styledMount as mount } from 'spec/helpers/theming'; import { Radio } from 'src/components/Radio'; import Button from 'src/components/Button'; import sinon from 'sinon'; @@ -39,6 +37,7 @@ const initialState = { chart: {}, saveModal: { dashboards: [], + isVisible: true, }, explore: { datasource: {}, @@ -57,6 +56,7 @@ const initialState = { const initialStore = mockStore(initialState); const defaultProps = { + addDangerToast: jest.fn(), onHide: () => ({}), actions: bindActionCreators(saveModalActions, arg => { if (typeof arg === 'function') { @@ -83,6 +83,7 @@ const queryStore = mockStore({ chart: {}, saveModal: { dashboards: [], + isVisible: true, }, explore: { datasource: { name: 'test', type: 'query' }, @@ -144,8 +145,7 @@ test('renders the right footer buttons when existing dashboard selected', () => test('renders the right footer buttons when new dashboard selected', () => { const wrapper = getWrapper(); wrapper.setState({ - saveToDashboardId: null, - newDashboardName: 'Test new dashboard', + dashboard: { label: 'Test new dashboard', value: 'Test new dashboard' }, }); const footerWrapper = shallow(wrapper.find(StyledModal).props().footer); const saveAndGoDash = footerWrapper @@ -186,18 +186,6 @@ test('sets action when overwriting slice', () => { expect(wrapperForOverwrite.state().action).toBe('overwrite'); }); -test('fetches dashboards on component mount', () => { - sinon.spy(defaultProps.actions, 'fetchDashboards'); - mount( - - - , - ); - expect(defaultProps.actions.fetchDashboards.calledOnce).toBe(true); - - defaultProps.actions.fetchDashboards.restore(); -}); - test('updates slice name and selected dashboard', () => { const wrapper = getWrapper(); const dashboardId = mockEvent.value; @@ -205,8 +193,8 @@ test('updates slice name and selected dashboard', () => { wrapper.instance().onSliceNameChange(mockEvent); expect(wrapper.state().newSliceName).toBe(mockEvent.target.value); - wrapper.instance().onDashboardSelectChange(dashboardId); - expect(wrapper.state().saveToDashboardId).toBe(dashboardId); + wrapper.instance().onDashboardChange({ value: dashboardId }); + expect(wrapper.state().dashboard.value).toBe(dashboardId); }); test('removes alert', () => { diff --git a/superset-frontend/src/explore/components/SaveModal.tsx b/superset-frontend/src/explore/components/SaveModal.tsx index 9e63f10b614..849baa1417b 100644 --- a/superset-frontend/src/explore/components/SaveModal.tsx +++ b/superset-frontend/src/explore/components/SaveModal.tsx @@ -19,17 +19,18 @@ /* eslint camelcase: 0 */ import React from 'react'; import { Dispatch } from 'redux'; -import { SelectValue } from 'antd/lib/select'; +import { isFeatureEnabled } from 'src/featureFlags'; +import rison from 'rison'; import { connect } from 'react-redux'; import { withRouter, RouteComponentProps } from 'react-router-dom'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import { css, DatasourceType, - ensureIsArray, FeatureFlag, isDefined, styled, + SupersetClient, t, } from '@superset-ui/core'; import { Input } from 'src/components/Input'; @@ -38,11 +39,10 @@ import Alert from 'src/components/Alert'; import Modal from 'src/components/Modal'; import { Radio } from 'src/components/Radio'; import Button from 'src/components/Button'; -import { Select } from 'src/components'; +import { AsyncSelect } from 'src/components'; import Loading from 'src/components/Loading'; import { setSaveChartModalVisibility } from 'src/explore/actions/saveModalActions'; import { SaveActionType } from 'src/explore/types'; -import { isFeatureEnabled } from 'src/featureFlags'; // Session storage key for recent dashboard const SK_DASHBOARD_ID = 'save_chart_recent_dashboard'; @@ -52,7 +52,6 @@ interface SaveModalProps extends RouteComponentProps { actions: Record; form_data?: Record; userId: number; - dashboards: Array; alert?: string; sliceName?: string; slice?: Record; @@ -63,15 +62,14 @@ interface SaveModalProps extends RouteComponentProps { } type SaveModalState = { - saveToDashboardId: number | string | null; newSliceName?: string; - newDashboardName?: string; datasetName: string; alert: string | null; action: SaveActionType; isLoading: boolean; saveStatus?: string | null; vizType?: string; + dashboard?: { label: string; value: string | number }; }; export const StyledModal = styled(Modal)` @@ -89,15 +87,15 @@ class SaveModal extends React.Component { constructor(props: SaveModalProps) { super(props); this.state = { - saveToDashboardId: null, newSliceName: props.sliceName, datasetName: props.datasource?.name, alert: null, action: this.canOverwriteSlice() ? 'overwrite' : 'saveas', isLoading: false, vizType: props.form_data?.viz_type, + dashboard: undefined, }; - this.onDashboardSelectChange = this.onDashboardSelectChange.bind(this); + this.onDashboardChange = this.onDashboardChange.bind(this); this.onSliceNameChange = this.onSliceNameChange.bind(this); this.changeAction = this.changeAction.bind(this); this.saveOrOverwrite = this.saveOrOverwrite.bind(this); @@ -107,7 +105,8 @@ class SaveModal extends React.Component { } isNewDashboard(): boolean { - return !!(!this.state.saveToDashboardId && this.state.newDashboardName); + const { dashboard } = this.state; + return typeof dashboard?.value === 'string'; } canOverwriteSlice(): boolean { @@ -117,30 +116,26 @@ class SaveModal extends React.Component { ); } - componentDidMount() { - this.props.actions.fetchDashboards(this.props.userId).then(() => { - if (ensureIsArray(this.props.dashboards).length === 0) { - return; - } - const dashboardIds = this.props.dashboards?.map( - dashboard => dashboard.value, - ); + async componentDidMount() { + let { dashboardId } = this.props; + if (!dashboardId) { const lastDashboard = sessionStorage.getItem(SK_DASHBOARD_ID); - let recentDashboard = lastDashboard && parseInt(lastDashboard, 10); - - if (this.props.dashboardId) { - recentDashboard = this.props.dashboardId; + dashboardId = lastDashboard && parseInt(lastDashboard, 10); + } + if (dashboardId) { + try { + const result = await this.loadDashboard(dashboardId); + if (result) { + this.setState({ + dashboard: { label: result.dashboard_title, value: result.id }, + }); + } + } catch (error) { + this.props.actions.addDangerToast( + t('An error occurred while loading dashboard information.'), + ); } - - if ( - recentDashboard !== null && - dashboardIds.indexOf(recentDashboard) !== -1 - ) { - this.setState({ - saveToDashboardId: recentDashboard, - }); - } - }); + } } handleDatasetNameChange = (e: React.FormEvent) => { @@ -152,11 +147,8 @@ class SaveModal extends React.Component { this.setState({ newSliceName: event.target.value }); } - onDashboardSelectChange(selected: SelectValue) { - const newDashboardName = selected ? String(selected) : undefined; - const saveToDashboardId = - selected && typeof selected === 'number' ? selected : null; - this.setState({ saveToDashboardId, newDashboardName }); + onDashboardChange(dashboard: { label: string; value: string | number }) { + this.setState({ dashboard }); } changeAction(action: SaveActionType) { @@ -206,19 +198,22 @@ class SaveModal extends React.Component { delete formData.url_params; let dashboard: DashboardGetResponse | null = null; - if (this.state.newDashboardName || this.state.saveToDashboardId) { - let saveToDashboardId = this.state.saveToDashboardId || null; - if (!this.state.saveToDashboardId) { + if (this.state.dashboard) { + let validId = this.state.dashboard.value; + if (this.isNewDashboard()) { const response = await this.props.actions.createDashboard( - this.state.newDashboardName, + this.state.dashboard.label, ); - saveToDashboardId = response.id; + validId = response.id; + } + + try { + dashboard = await this.loadDashboard(validId as number); + } catch (error) { + this.props.actions.saveSliceFailed(); + return; } - const response = await this.props.actions.getDashboard( - saveToDashboardId, - ); - dashboard = response.result; if (isDefined(dashboard) && isDefined(dashboard?.id)) { sliceDashboards = sliceDashboards.includes(dashboard.id) ? sliceDashboards @@ -240,7 +235,7 @@ class SaveModal extends React.Component { dashboard ? { title: dashboard.dashboard_title, - new: !this.state.saveToDashboardId, + new: this.isNewDashboard(), } : null, ); @@ -251,7 +246,7 @@ class SaveModal extends React.Component { dashboard ? { title: dashboard.dashboard_title, - new: !this.state.saveToDashboardId, + new: this.isNewDashboard(), } : null, ); @@ -284,95 +279,132 @@ class SaveModal extends React.Component { } } - renderSaveChartModal = () => { - const dashboardSelectValue = - this.state.saveToDashboardId || this.state.newDashboardName; - - return ( -
- {(this.state.alert || this.props.alert) && ( - - )} - - this.changeAction('overwrite')} - data-test="save-overwrite-radio" - > - {t('Save (Overwrite)')} - - this.changeAction('saveas')} - > - {t('Save as...')} - - -
- - - - {this.props.datasource?.type === 'query' && ( - - - - - )} - {!( - isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) && - this.state.vizType === 'filter_box' - ) && ( - - + + {this.props.datasource?.type === 'query' && ( + + + + + )} + {!( + isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) && + this.state.vizType === 'filter_box' + ) && ( + + + {t('Select')} + {t(' a dashboard OR ')} + {t('create')} + {t(' a new one')} + + } + /> + + )} + + ); + renderFooter = () => (