diff --git a/superset-frontend/src/constants.ts b/superset-frontend/src/constants.ts index c72077a6f1075..a41450895c8cb 100644 --- a/superset-frontend/src/constants.ts +++ b/superset-frontend/src/constants.ts @@ -99,6 +99,10 @@ export const URL_PARAMS = { name: 'show_database_modal', type: 'boolean', }, + saveAction: { + name: 'save_action', + type: 'string', + }, } as const; export const RESERVED_CHART_URL_PARAMS: string[] = [ diff --git a/superset-frontend/src/explore/ExplorePage.tsx b/superset-frontend/src/explore/ExplorePage.tsx index 8ae31cb88335a..d8aefda1345bc 100644 --- a/superset-frontend/src/explore/ExplorePage.tsx +++ b/superset-frontend/src/explore/ExplorePage.tsx @@ -16,50 +16,58 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useRef, useState } from 'react'; import { useDispatch } from 'react-redux'; +import { useLocation } from 'react-router-dom'; import { makeApi, t } from '@superset-ui/core'; import Loading from 'src/components/Loading'; +import { addDangerToast } from 'src/components/MessageToasts/actions'; +import { isNullish } from 'src/utils/common'; +import { getUrlParam } from 'src/utils/urlUtils'; +import { URL_PARAMS } from 'src/constants'; import { getParsedExploreURLParams } from './exploreUtils/getParsedExploreURLParams'; import { hydrateExplore } from './actions/hydrateExplore'; import ExploreViewContainer from './components/ExploreViewContainer'; import { ExploreResponsePayload } from './types'; import { fallbackExploreInitialData } from './fixtures'; -import { addDangerToast } from '../components/MessageToasts/actions'; -import { isNullish } from '../utils/common'; const loadErrorMessage = t('Failed to load chart data.'); -const fetchExploreData = () => { - const exploreUrlParams = getParsedExploreURLParams(); - return makeApi<{}, ExploreResponsePayload>({ +const fetchExploreData = (exploreUrlParams: URLSearchParams) => + makeApi<{}, ExploreResponsePayload>({ method: 'GET', endpoint: 'api/v1/explore/', })(exploreUrlParams); -}; const ExplorePage = () => { const [isLoaded, setIsLoaded] = useState(false); + const isExploreInitialized = useRef(false); const dispatch = useDispatch(); + const location = useLocation(); useEffect(() => { - fetchExploreData() - .then(({ result }) => { - if (isNullish(result.dataset?.id) && isNullish(result.dataset?.uid)) { + const exploreUrlParams = getParsedExploreURLParams(location); + const isSaveAction = !!getUrlParam(URL_PARAMS.saveAction); + if (!isExploreInitialized.current || isSaveAction) { + fetchExploreData(exploreUrlParams) + .then(({ result }) => { + if (isNullish(result.dataset?.id) && isNullish(result.dataset?.uid)) { + dispatch(hydrateExplore(fallbackExploreInitialData)); + dispatch(addDangerToast(loadErrorMessage)); + } else { + dispatch(hydrateExplore(result)); + } + }) + .catch(() => { dispatch(hydrateExplore(fallbackExploreInitialData)); dispatch(addDangerToast(loadErrorMessage)); - } else { - dispatch(hydrateExplore(result)); - } - }) - .catch(() => { - dispatch(hydrateExplore(fallbackExploreInitialData)); - dispatch(addDangerToast(loadErrorMessage)); - }) - .finally(() => { - setIsLoaded(true); - }); - }, [dispatch]); + }) + .finally(() => { + setIsLoaded(true); + isExploreInitialized.current = true; + }); + } + }, [dispatch, location]); if (!isLoaded) { return ; diff --git a/superset-frontend/src/explore/actions/hydrateExplore.ts b/superset-frontend/src/explore/actions/hydrateExplore.ts index ec49c0cdd5b05..24de502c474e0 100644 --- a/superset-frontend/src/explore/actions/hydrateExplore.ts +++ b/superset-frontend/src/explore/actions/hydrateExplore.ts @@ -89,6 +89,7 @@ export const hydrateExplore = controlsTransferred: [], standalone: getUrlParam(URL_PARAMS.standalone), force: getUrlParam(URL_PARAMS.force), + sliceDashboards: initialFormData.dashboards, }; // apply initial mapStateToProps for all controls, must execute AFTER diff --git a/superset-frontend/src/explore/actions/saveModalActions.js b/superset-frontend/src/explore/actions/saveModalActions.js index 91abb04e180d9..af7f4bc549296 100644 --- a/superset-frontend/src/explore/actions/saveModalActions.js +++ b/superset-frontend/src/explore/actions/saveModalActions.js @@ -16,8 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import { SupersetClient } from '@superset-ui/core'; -import { buildV1ChartDataPayload, getExploreUrl } from '../exploreUtils'; +import { SupersetClient, t } from '@superset-ui/core'; +import { addSuccessToast } from 'src/components/MessageToasts/actions'; +import { buildV1ChartDataPayload } from '../exploreUtils'; export const FETCH_DASHBOARDS_SUCCEEDED = 'FETCH_DASHBOARDS_SUCCEEDED'; export function fetchDashboardsSucceeded(choices) { @@ -60,36 +61,140 @@ export function removeSaveModalAlert() { return { type: REMOVE_SAVE_MODAL_ALERT }; } -export function saveSlice(formData, requestParams) { - return dispatch => { - let url = getExploreUrl({ - formData, - endpointType: 'base', - force: false, - curUrl: null, - requestParams, +export const getSlicePayload = ( + sliceName, + formDataWithNativeFilters, + owners, +) => { + const formData = { + ...formDataWithNativeFilters, + adhoc_filters: formDataWithNativeFilters.adhoc_filters?.filter( + f => !f.isExtra, + ), + }; + + const [datasourceId, datasourceType] = formData.datasource.split('__'); + const payload = { + params: JSON.stringify(formData), + slice_name: sliceName, + viz_type: formData.viz_type, + datasource_id: parseInt(datasourceId, 10), + datasource_type: datasourceType, + dashboards: formData.dashboards, + owners, + query_context: JSON.stringify( + buildV1ChartDataPayload({ + formData, + force: false, + resultFormat: 'json', + resultType: 'full', + setDataMask: null, + ownState: null, + }), + ), + }; + return payload; +}; + +const addToasts = (isNewSlice, sliceName, addedToDashboard) => { + const toasts = []; + if (isNewSlice) { + toasts.push(addSuccessToast(t('Chart [%s] has been saved', sliceName))); + } else { + toasts.push( + addSuccessToast(t('Chart [%s] has been overwritten', sliceName)), + ); + } + + if (addedToDashboard) { + if (addedToDashboard.new) { + toasts.push( + addSuccessToast( + t( + 'Dashboard [%s] just got created and chart [%s] was added to it', + addedToDashboard.title, + sliceName, + ), + ), + ); + } else { + toasts.push( + addSuccessToast( + t( + 'Chart [%s] was added to dashboard [%s]', + sliceName, + addedToDashboard.title, + ), + ), + ); + } + } + + return toasts; +}; + +// Update existing slice +export const updateSlice = + ({ slice_id: sliceId, owners }, sliceName, formData, addedToDashboard) => + async dispatch => { + try { + const response = await SupersetClient.put({ + endpoint: `/api/v1/chart/${sliceId}`, + jsonPayload: getSlicePayload(sliceName, formData, owners), + }); + + dispatch(saveSliceSuccess()); + addToasts(false, sliceName, addedToDashboard).map(dispatch); + return response.json; + } catch (error) { + dispatch(saveSliceFailed()); + throw error; + } + }; + +// Create new slice +export const createSlice = + (sliceName, formData, addedToDashboard) => async dispatch => { + try { + const response = await SupersetClient.post({ + endpoint: `/api/v1/chart/`, + jsonPayload: getSlicePayload(sliceName, formData), + }); + + dispatch(saveSliceSuccess()); + addToasts(true, sliceName, addedToDashboard).map(dispatch); + return response.json; + } catch (error) { + dispatch(saveSliceFailed()); + throw error; + } + }; + +// Create new dashboard +export const createDashboard = dashboardName => async dispatch => { + try { + const response = await SupersetClient.post({ + endpoint: `/api/v1/dashboard/`, + jsonPayload: { dashboard_title: dashboardName }, }); - // TODO: This will be removed in the next PR that will change the logic to save a slice - url = url.replace('/explore', '/superset/explore'); + return response.json; + } catch (error) { + dispatch(saveSliceFailed()); + throw error; + } +}; - // Save the query context so we can re-generate the data from Python - // for alerts and reports - const queryContext = buildV1ChartDataPayload({ - formData, - force: false, - resultFormat: 'json', - resultType: 'full', +// Get existing dashboard from ID +export const getDashboard = dashboardId => async dispatch => { + try { + const response = await SupersetClient.get({ + endpoint: `/api/v1/dashboard/${dashboardId}`, }); - return SupersetClient.post({ - url, - postPayload: { form_data: formData, query_context: queryContext }, - }) - .then(response => { - dispatch(saveSliceSuccess(response.json)); - return response.json; - }) - .catch(() => dispatch(saveSliceFailed())); - }; -} + return response.json; + } catch (error) { + dispatch(saveSliceFailed()); + throw error; + } +}; diff --git a/superset-frontend/src/explore/actions/saveModalActions.test.js b/superset-frontend/src/explore/actions/saveModalActions.test.js new file mode 100644 index 0000000000000..1d97c3f45e90c --- /dev/null +++ b/superset-frontend/src/explore/actions/saveModalActions.test.js @@ -0,0 +1,292 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import sinon from 'sinon'; +import fetchMock from 'fetch-mock'; +import { ADD_TOAST } from 'src/components/MessageToasts/actions'; +import { + createDashboard, + createSlice, + fetchDashboards, + FETCH_DASHBOARDS_FAILED, + FETCH_DASHBOARDS_SUCCEEDED, + getDashboard, + SAVE_SLICE_FAILED, + SAVE_SLICE_SUCCESS, + updateSlice, +} 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'; +const datasourceId = 11; +const datasourceType = 'sample_datasource_type'; +const dashboards = [12, 13]; +const queryContext = { sampleKey: 'sampleValue' }; +const formData = { + viz_type: vizType, + datasource: `${datasourceId}__${datasourceType}`, + dashboards, +}; + +const sliceResponsePayload = { + id: 10, +}; + +const sampleError = new Error('sampleError'); + +jest.mock('../exploreUtils', () => ({ + buildV1ChartDataPayload: jest.fn(() => queryContext), +})); + +/** + * Tests updateSlice action + */ + +const updateSliceEndpoint = `glob:*/api/v1/chart/${sliceId}`; +test('updateSlice handles success', async () => { + fetchMock.reset(); + fetchMock.put(updateSliceEndpoint, sliceResponsePayload); + const dispatch = sinon.spy(); + const slice = await updateSlice( + { slice_id: sliceId }, + sliceName, + formData, + )(dispatch); + + expect(fetchMock.calls(updateSliceEndpoint)).toHaveLength(1); + expect(dispatch.callCount).toBe(2); + expect(dispatch.getCall(0).args[0].type).toBe(SAVE_SLICE_SUCCESS); + expect(dispatch.getCall(1).args[0].type).toBe(ADD_TOAST); + expect(dispatch.getCall(1).args[0].payload.toastType).toBe('SUCCESS_TOAST'); + expect(dispatch.getCall(1).args[0].payload.text).toBe( + 'Chart [New chart] has been overwritten', + ); + + expect(slice).toEqual(sliceResponsePayload); +}); + +test('updateSlice handles failure', async () => { + fetchMock.reset(); + fetchMock.put(updateSliceEndpoint, { throws: sampleError }); + const dispatch = sinon.spy(); + let caughtError; + try { + await updateSlice({ slice_id: sliceId }, sliceName, formData)(dispatch); + } catch (error) { + caughtError = error; + } + + expect(caughtError).toEqual(sampleError); + expect(fetchMock.calls(updateSliceEndpoint)).toHaveLength(4); + expect(dispatch.callCount).toBe(1); + expect(dispatch.getCall(0).args[0].type).toBe(SAVE_SLICE_FAILED); +}); + +/** + * Tests createSlice action + */ + +const createSliceEndpoint = `glob:*/api/v1/chart/`; +test('createSlice handles success', async () => { + fetchMock.reset(); + fetchMock.post(createSliceEndpoint, sliceResponsePayload); + const dispatch = sinon.spy(); + const slice = await createSlice(sliceName, formData)(dispatch); + expect(fetchMock.calls(createSliceEndpoint)).toHaveLength(1); + expect(dispatch.callCount).toBe(2); + expect(dispatch.getCall(0).args[0].type).toBe(SAVE_SLICE_SUCCESS); + expect(dispatch.getCall(1).args[0].type).toBe(ADD_TOAST); + expect(dispatch.getCall(1).args[0].payload.toastType).toBe('SUCCESS_TOAST'); + expect(dispatch.getCall(1).args[0].payload.text).toBe( + 'Chart [New chart] has been saved', + ); + + expect(slice).toEqual(sliceResponsePayload); +}); + +test('createSlice handles failure', async () => { + fetchMock.reset(); + fetchMock.post(createSliceEndpoint, { throws: sampleError }); + const dispatch = sinon.spy(); + let caughtError; + try { + await createSlice(sliceName, formData)(dispatch); + } catch (error) { + caughtError = error; + } + + expect(caughtError).toEqual(sampleError); + expect(fetchMock.calls(createSliceEndpoint)).toHaveLength(4); + expect(dispatch.callCount).toBe(1); + expect(dispatch.getCall(0).args[0].type).toBe(SAVE_SLICE_FAILED); +}); + +const dashboardId = 14; +const dashboardName = 'New dashboard'; +const dashboardResponsePayload = { + id: 14, +}; + +/** + * Tests createDashboard action + */ + +const createDashboardEndpoint = `glob:*/api/v1/dashboard/`; +test('createDashboard handles success', async () => { + fetchMock.reset(); + fetchMock.post(createDashboardEndpoint, dashboardResponsePayload); + const dispatch = sinon.spy(); + const dashboard = await createDashboard(dashboardName)(dispatch); + expect(fetchMock.calls(createDashboardEndpoint)).toHaveLength(1); + expect(dispatch.callCount).toBe(0); + expect(dashboard).toEqual(dashboardResponsePayload); +}); + +test('createDashboard handles failure', async () => { + fetchMock.reset(); + fetchMock.post(createDashboardEndpoint, { throws: sampleError }); + const dispatch = sinon.spy(); + let caughtError; + try { + await createDashboard(dashboardName)(dispatch); + } catch (error) { + caughtError = error; + } + + expect(caughtError).toEqual(sampleError); + expect(fetchMock.calls(createDashboardEndpoint)).toHaveLength(4); + expect(dispatch.callCount).toBe(1); + 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); + const dispatch = sinon.spy(); + const slice = await updateSlice({ slice_id: sliceId }, sliceName, formData, { + new: true, + title: dashboardName, + })(dispatch); + + expect(fetchMock.calls(updateSliceEndpoint)).toHaveLength(1); + expect(dispatch.callCount).toBe(3); + expect(dispatch.getCall(0).args[0].type).toBe(SAVE_SLICE_SUCCESS); + expect(dispatch.getCall(1).args[0].type).toBe(ADD_TOAST); + expect(dispatch.getCall(1).args[0].payload.toastType).toBe('SUCCESS_TOAST'); + expect(dispatch.getCall(1).args[0].payload.text).toBe( + 'Chart [New chart] has been overwritten', + ); + expect(dispatch.getCall(2).args[0].type).toBe(ADD_TOAST); + expect(dispatch.getCall(2).args[0].payload.toastType).toBe('SUCCESS_TOAST'); + expect(dispatch.getCall(2).args[0].payload.text).toBe( + 'Dashboard [New dashboard] just got created and chart [New chart] was added to it', + ); + + expect(slice).toEqual(sliceResponsePayload); +}); + +test('updateSlice with add to existing dashboard handles success', async () => { + fetchMock.reset(); + fetchMock.put(updateSliceEndpoint, sliceResponsePayload); + const dispatch = sinon.spy(); + const slice = await updateSlice({ slice_id: sliceId }, sliceName, formData, { + new: false, + title: dashboardName, + })(dispatch); + + expect(fetchMock.calls(updateSliceEndpoint)).toHaveLength(1); + expect(dispatch.callCount).toBe(3); + expect(dispatch.getCall(0).args[0].type).toBe(SAVE_SLICE_SUCCESS); + expect(dispatch.getCall(1).args[0].type).toBe(ADD_TOAST); + expect(dispatch.getCall(1).args[0].payload.toastType).toBe('SUCCESS_TOAST'); + expect(dispatch.getCall(1).args[0].payload.text).toBe( + 'Chart [New chart] has been overwritten', + ); + expect(dispatch.getCall(2).args[0].type).toBe(ADD_TOAST); + expect(dispatch.getCall(2).args[0].payload.toastType).toBe('SUCCESS_TOAST'); + expect(dispatch.getCall(2).args[0].payload.text).toBe( + 'Chart [New chart] was added to dashboard [New dashboard]', + ); + + expect(slice).toEqual(sliceResponsePayload); +}); diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx index cc374c1cf9b96..c3e610c29fbec 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx +++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx @@ -464,6 +464,14 @@ function ExploreViewContainer(props) { return false; }, [lastQueriedControls, props.controls]); + const saveAction = getUrlParam(URL_PARAMS.saveAction); + useChangeEffect(saveAction, () => { + if (['saveas', 'overwrite'].includes(saveAction)) { + onQuery(); + addHistory({ isReplace: true }); + } + }); + useEffect(() => { if (props.ownState !== undefined) { onQuery(); @@ -586,6 +594,7 @@ function ExploreViewContainer(props) { form_data={props.form_data} sliceName={props.sliceName} dashboardId={props.dashboardId} + sliceDashboards={props.exploreState.sliceDashboards ?? []} /> )} { - const middlewares = [thunk]; - const mockStore = configureStore(middlewares); - const initialState = { - chart: {}, - saveModal: { - dashboards: [], - }, - explore: { - datasource: {}, - slice: { - slice_id: 1, - slice_name: 'title', - owners: [1], - }, - alert: null, - }, - user: { - userId: 1, - }, - }; - const store = mockStore(initialState); - - const defaultProps = { - onHide: () => ({}), - actions: bindActionCreators(saveModalActions, arg => { - if (typeof arg === 'function') { - return arg(jest.fn); - } - return arg; - }), - form_data: { datasource: '107__table', url_params: { foo: 'bar' } }, - }; - const mockEvent = { - target: { - value: 'mock event target', +import { BrowserRouter } from 'react-router-dom'; + +const middlewares = [thunk]; +const mockStore = configureStore(middlewares); +const initialState = { + chart: {}, + saveModal: { + dashboards: [], + }, + explore: { + datasource: {}, + slice: { + slice_id: 1, + slice_name: 'title', + owners: [1], }, - value: 10, - }; - - const mockDashboardData = { - pks: ['id'], - result: [{ id: 'id', dashboard_title: 'dashboard title' }], - }; - - const saveEndpoint = `glob:*/dashboardasync/api/read?_flt_0_owners=${1}`; - - beforeAll(() => fetchMock.get(saveEndpoint, mockDashboardData)); - - afterAll(() => fetchMock.restore()); - - const getWrapper = () => - shallow() - .dive() - .dive(); - - it('renders a Modal with the right set of components', () => { - const wrapper = getWrapper(); - expect(wrapper.find(StyledModal)).toExist(); - expect(wrapper.find(Radio)).toHaveLength(2); - - const footerWrapper = shallow(wrapper.find(StyledModal).props().footer); - - expect(footerWrapper.find(Button)).toHaveLength(3); - }); - - it('renders the right footer buttons when an existing dashboard', () => { - const wrapper = getWrapper(); - const footerWrapper = shallow(wrapper.find(StyledModal).props().footer); - const saveAndGoDash = footerWrapper - .find('#btn_modal_save_goto_dash') - .getElement(); - const save = footerWrapper.find('#btn_modal_save').getElement(); - expect(save.props.children).toBe('Save'); - expect(saveAndGoDash.props.children).toBe('Save & go to dashboard'); - }); - - it('renders the right footer buttons when a new dashboard', () => { - const wrapper = getWrapper(); - wrapper.setState({ - saveToDashboardId: null, - newDashboardName: 'Test new dashboard', - }); - const footerWrapper = shallow(wrapper.find(StyledModal).props().footer); - const saveAndGoDash = footerWrapper - .find('#btn_modal_save_goto_dash') - .getElement(); - const save = footerWrapper.find('#btn_modal_save').getElement(); - expect(save.props.children).toBe('Save to new dashboard'); - expect(saveAndGoDash.props.children).toBe('Save & go to new dashboard'); - }); - - it('overwrite radio button is disabled for new slice', () => { - const wrapper = getWrapper(); - wrapper.setProps({ slice: null }); - expect(wrapper.find('#overwrite-radio').prop('disabled')).toBe(true); - }); - - it('disable overwrite option for non-owner', () => { - const wrapperForNonOwner = getWrapper(); - wrapperForNonOwner.setProps({ userId: 2 }); - const overwriteRadio = wrapperForNonOwner.find('#overwrite-radio'); - expect(overwriteRadio).toHaveLength(1); - expect(overwriteRadio.prop('disabled')).toBe(true); - }); - - it('saves a new slice', () => { - const wrapperForNewSlice = getWrapper(); - wrapperForNewSlice.setProps({ can_overwrite: false }); - wrapperForNewSlice.instance().changeAction('saveas'); - const saveasRadio = wrapperForNewSlice.find('#saveas-radio'); - saveasRadio.simulate('click'); - expect(wrapperForNewSlice.state().action).toBe('saveas'); - }); - - it('overwrite a slice', () => { - const wrapperForOverwrite = getWrapper(); - const overwriteRadio = wrapperForOverwrite.find('#overwrite-radio'); - overwriteRadio.simulate('click'); - expect(wrapperForOverwrite.state().action).toBe('overwrite'); - }); - - it('componentDidMount', () => { - sinon.spy(defaultProps.actions, 'fetchDashboards'); - mount( - - - , - ); - expect(defaultProps.actions.fetchDashboards.calledOnce).toBe(true); - - defaultProps.actions.fetchDashboards.restore(); - }); - - it('onChange', () => { - const wrapper = getWrapper(); - const dashboardId = mockEvent.value; - - wrapper.instance().onSliceNameChange(mockEvent); - expect(wrapper.state().newSliceName).toBe(mockEvent.target.value); - - wrapper.instance().onDashboardSelectChange(dashboardId); - expect(wrapper.state().saveToDashboardId).toBe(dashboardId); - }); - - describe('saveOrOverwrite', () => { - beforeEach(() => { - sinon.stub(exploreUtils, 'getExploreUrl').callsFake(() => 'mockURL'); - - sinon.stub(defaultProps.actions, 'saveSlice').callsFake(() => - Promise.resolve({ - dashboard_url: 'http://localhost/mock_dashboard/', - slice: { slice_url: '/mock_slice/' }, - }), - ); - }); - - afterEach(() => { - exploreUtils.getExploreUrl.restore(); - defaultProps.actions.saveSlice.restore(); - }); - - it('should save slice without url_params in form_data', () => { - const wrapper = getWrapper(); - wrapper.instance().saveOrOverwrite(true); - const { args } = defaultProps.actions.saveSlice.getCall(0); - expect(args[0]).toEqual({ datasource: '107__table' }); - }); - - it('existing dashboard', () => { - const wrapper = getWrapper(); - const saveToDashboardId = 100; - - wrapper.setState({ saveToDashboardId }); - wrapper.instance().saveOrOverwrite(true); - const { args } = defaultProps.actions.saveSlice.getCall(0); - expect(args[1].save_to_dashboard_id).toBe(saveToDashboardId); - }); - - it('new dashboard', () => { - const wrapper = getWrapper(); - const newDashboardName = 'new dashboard name'; - - wrapper.setState({ newDashboardName }); - wrapper.instance().saveOrOverwrite(true); - const { args } = defaultProps.actions.saveSlice.getCall(0); - expect(args[1].new_dashboard_name).toBe(newDashboardName); - }); - - describe('should always reload or redirect', () => { - const originalLocation = window.location; - delete window.location; - window.location = { assign: jest.fn() }; - const stub = sinon.stub(window.location, 'assign'); - - afterAll(() => { - delete window.location; - window.location = originalLocation; - }); - - let wrapper; - - beforeEach(() => { - stub.resetHistory(); - wrapper = getWrapper(); - }); - - it('Save & go to dashboard', () => - new Promise(done => { - wrapper.instance().saveOrOverwrite(true); - defaultProps.actions.saveSlice().then(() => { - expect(window.location.assign.callCount).toEqual(1); - expect(window.location.assign.getCall(0).args[0]).toEqual( - 'http://localhost/mock_dashboard/?foo=bar', - ); - done(); - }); - })); + alert: null, + }, + user: { + userId: 1, + }, +}; + +const store = mockStore(initialState); + +const defaultProps = { + onHide: () => ({}), + actions: bindActionCreators(saveModalActions, arg => { + if (typeof arg === 'function') { + return arg(jest.fn); + } + return arg; + }), + form_data: { datasource: '107__table', url_params: { foo: 'bar' } }, +}; + +const mockEvent = { + target: { + value: 'mock event target', + }, + value: 10, +}; + +const mockDashboardData = { + pks: ['id'], + result: [{ id: 'id', dashboard_title: 'dashboard title' }], +}; + +const fetchDashboardsEndpoint = `glob:*/dashboardasync/api/read?_flt_0_owners=${1}`; + +beforeAll(() => fetchMock.get(fetchDashboardsEndpoint, mockDashboardData)); + +afterAll(() => fetchMock.restore()); + +const getWrapper = () => + shallow( + + + , + ) + .dive() + .dive() + .dive() + .dive() + .dive() + .dive() + .dive(); + +test('renders a Modal with the right set of components', () => { + const wrapper = getWrapper(); + expect(wrapper.find(StyledModal)).toExist(); + expect(wrapper.find(Radio)).toHaveLength(2); + + const footerWrapper = shallow(wrapper.find(StyledModal).props().footer); + + expect(footerWrapper.find(Button)).toHaveLength(3); +}); - it('saveas new slice', () => - new Promise(done => { - wrapper.setState({ - action: 'saveas', - newSliceName: 'new slice name', - }); - wrapper.instance().saveOrOverwrite(false); - defaultProps.actions.saveSlice().then(() => { - expect(window.location.assign.callCount).toEqual(1); - expect(window.location.assign.getCall(0).args[0]).toEqual( - '/mock_slice/?foo=bar', - ); - done(); - }); - })); +test('renders the right footer buttons when existing dashboard selected', () => { + const wrapper = getWrapper(); + const footerWrapper = shallow(wrapper.find(StyledModal).props().footer); + const saveAndGoDash = footerWrapper + .find('#btn_modal_save_goto_dash') + .getElement(); + const save = footerWrapper.find('#btn_modal_save').getElement(); + expect(save.props.children).toBe('Save'); + expect(saveAndGoDash.props.children).toBe('Save & go to dashboard'); +}); - it('overwrite original slice', () => - new Promise(done => { - wrapper.setState({ action: 'overwrite' }); - wrapper.instance().saveOrOverwrite(false); - defaultProps.actions.saveSlice().then(() => { - expect(window.location.assign.callCount).toEqual(1); - expect(window.location.assign.getCall(0).args[0]).toEqual( - '/mock_slice/?foo=bar', - ); - done(); - }); - })); - }); +test('renders the right footer buttons when new dashboard selected', () => { + const wrapper = getWrapper(); + wrapper.setState({ + saveToDashboardId: null, + newDashboardName: 'Test new dashboard', }); + const footerWrapper = shallow(wrapper.find(StyledModal).props().footer); + const saveAndGoDash = footerWrapper + .find('#btn_modal_save_goto_dash') + .getElement(); + const save = footerWrapper.find('#btn_modal_save').getElement(); + expect(save.props.children).toBe('Save to new dashboard'); + expect(saveAndGoDash.props.children).toBe('Save & go to new dashboard'); +}); - describe('fetchDashboards', () => { - let dispatch; - let actionThunk; - const userID = 1; - - beforeEach(() => { - fetchMock.resetHistory(); - dispatch = sinon.spy(); - }); - - const makeRequest = () => { - actionThunk = saveModalActions.fetchDashboards(userID); - return actionThunk(dispatch); - }; +test('disables overwrite option for new slice', () => { + const wrapper = getWrapper(); + wrapper.setProps({ slice: null }); + expect(wrapper.find('#overwrite-radio').prop('disabled')).toBe(true); +}); - it('makes the fetch request', () => - makeRequest().then(() => { - expect(fetchMock.calls(saveEndpoint)).toHaveLength(1); +test('disables overwrite option for non-owner', () => { + const wrapperForNonOwner = getWrapper(); + wrapperForNonOwner.setProps({ userId: 2 }); + const overwriteRadio = wrapperForNonOwner.find('#overwrite-radio'); + expect(overwriteRadio).toHaveLength(1); + expect(overwriteRadio.prop('disabled')).toBe(true); +}); - return Promise.resolve(); - })); +test('sets action when saving as new slice', () => { + const wrapperForNewSlice = getWrapper(); + wrapperForNewSlice.setProps({ can_overwrite: false }); + wrapperForNewSlice.instance().changeAction('saveas'); + const saveasRadio = wrapperForNewSlice.find('#saveas-radio'); + saveasRadio.simulate('click'); + expect(wrapperForNewSlice.state().action).toBe('saveas'); +}); - it('calls correct actions on success', () => - makeRequest().then(() => { - expect(dispatch.callCount).toBe(1); - expect(dispatch.getCall(0).args[0].type).toBe( - saveModalActions.FETCH_DASHBOARDS_SUCCEEDED, - ); +test('sets action when overwriting slice', () => { + const wrapperForOverwrite = getWrapper(); + const overwriteRadio = wrapperForOverwrite.find('#overwrite-radio'); + overwriteRadio.simulate('click'); + expect(wrapperForOverwrite.state().action).toBe('overwrite'); +}); - return Promise.resolve(); - })); +test('fetches dashboards on component mount', () => { + sinon.spy(defaultProps.actions, 'fetchDashboards'); + mount( + + + , + ); + expect(defaultProps.actions.fetchDashboards.calledOnce).toBe(true); - it('calls correct actions on error', () => { - fetchMock.get( - saveEndpoint, - { throws: 'error' }, - { overwriteRoutes: true }, - ); + defaultProps.actions.fetchDashboards.restore(); +}); - return makeRequest().then(() => { - expect(dispatch.callCount).toBe(1); - expect(dispatch.getCall(0).args[0].type).toBe( - saveModalActions.FETCH_DASHBOARDS_FAILED, - ); +test('updates slice name and selected dashboard', () => { + const wrapper = getWrapper(); + const dashboardId = mockEvent.value; - fetchMock.get(saveEndpoint, mockDashboardData, { - overwriteRoutes: true, - }); + wrapper.instance().onSliceNameChange(mockEvent); + expect(wrapper.state().newSliceName).toBe(mockEvent.target.value); - return Promise.resolve(); - }); - }); - }); + wrapper.instance().onDashboardSelectChange(dashboardId); + expect(wrapper.state().saveToDashboardId).toBe(dashboardId); +}); - it('removeAlert', () => { - sinon.spy(defaultProps.actions, 'removeSaveModalAlert'); - const wrapper = getWrapper(); - wrapper.setProps({ alert: 'old alert' }); +test('removes alert', () => { + sinon.spy(defaultProps.actions, 'removeSaveModalAlert'); + const wrapper = getWrapper(); + wrapper.setProps({ alert: 'old alert' }); - wrapper.instance().removeAlert(); - expect(defaultProps.actions.removeSaveModalAlert.callCount).toBe(1); - expect(wrapper.state().alert).toBeNull(); - defaultProps.actions.removeSaveModalAlert.restore(); - }); + wrapper.instance().removeAlert(); + expect(defaultProps.actions.removeSaveModalAlert.callCount).toBe(1); + expect(wrapper.state().alert).toBeNull(); + defaultProps.actions.removeSaveModalAlert.restore(); }); diff --git a/superset-frontend/src/explore/components/SaveModal.tsx b/superset-frontend/src/explore/components/SaveModal.tsx index 9eb0d7b97ed26..02050eaecde51 100644 --- a/superset-frontend/src/explore/components/SaveModal.tsx +++ b/superset-frontend/src/explore/components/SaveModal.tsx @@ -21,7 +21,7 @@ import React from 'react'; import { Input } from 'src/components/Input'; import { Form, FormItem } from 'src/components/Form'; import Alert from 'src/components/Alert'; -import { JsonObject, t, styled } from '@superset-ui/core'; +import { t, styled } from '@superset-ui/core'; import ReactMarkdown from 'react-markdown'; import Modal from 'src/components/Modal'; import { Radio } from 'src/components/Radio'; @@ -29,12 +29,13 @@ import Button from 'src/components/Button'; import { Select } from 'src/components'; import { SelectValue } from 'antd/lib/select'; import { connect } from 'react-redux'; +import { withRouter, RouteComponentProps } from 'react-router-dom'; // Session storage key for recent dashboard const SK_DASHBOARD_ID = 'save_chart_recent_dashboard'; const SELECT_PLACEHOLDER = t('**Select** a dashboard OR **create** a new one'); -type SaveModalProps = { +interface SaveModalProps extends RouteComponentProps { onHide: () => void; actions: Record; form_data?: Record; @@ -45,7 +46,8 @@ type SaveModalProps = { slice?: Record; datasource?: Record; dashboardId: '' | number | null; -}; + sliceDashboards: number[]; +} type ActionType = 'overwrite' | 'saveas'; @@ -131,39 +133,94 @@ class SaveModal extends React.Component { saveOrOverwrite(gotodash: boolean) { this.setState({ alert: null }); this.props.actions.removeSaveModalAlert(); - const sliceParams: Record = {}; + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { url_params, ...formData } = this.props.form_data || {}; + + let promise = Promise.resolve(); - if (this.props.slice && this.props.slice.slice_id) { - sliceParams.slice_id = this.props.slice.slice_id; + // Create or retrieve dashboard + type DashboardGetResponse = { + id: number; + url: string; + dashboard_title: string; + }; + + let dashboard: DashboardGetResponse | null = null; + if (this.state.newDashboardName || this.state.saveToDashboardId) { + let saveToDashboardId = this.state.saveToDashboardId || null; + if (!this.state.saveToDashboardId) { + promise = promise + .then(() => + this.props.actions.createDashboard(this.state.newDashboardName), + ) + .then((response: { id: number }) => { + saveToDashboardId = response.id; + }); + } + + promise = promise + .then(() => this.props.actions.getDashboard(saveToDashboardId)) + .then((response: { result: DashboardGetResponse }) => { + dashboard = response.result; + const dashboards = new Set(this.props.sliceDashboards); + dashboards.add(dashboard.id); + formData.dashboards = Array.from(dashboards); + }); + } + + // Update or create slice + if (this.state.action === 'overwrite') { + promise = promise.then(() => + this.props.actions.updateSlice( + this.props.slice, + this.state.newSliceName, + formData, + dashboard + ? { + title: dashboard.dashboard_title, + new: !this.state.saveToDashboardId, + } + : null, + ), + ); + } else { + promise = promise.then(() => + this.props.actions.createSlice( + this.state.newSliceName, + formData, + dashboard + ? { + title: dashboard.dashboard_title, + new: !this.state.saveToDashboardId, + } + : null, + ), + ); } - if (sliceParams.action === 'saveas') { - if (this.state.newSliceName === '') { - this.setState({ alert: t('Please enter a chart name') }); + + promise.then(((value: { id: number }) => { + // Update recent dashboard + if (dashboard) { + sessionStorage.setItem(SK_DASHBOARD_ID, `${dashboard.id}`); + } else { + sessionStorage.removeItem(SK_DASHBOARD_ID); + } + + // Go to new dashboard url + if (gotodash && dashboard) { + this.props.history.push(dashboard.url); return; } - } - sliceParams.action = this.state.action; - sliceParams.slice_name = this.state.newSliceName; - sliceParams.save_to_dashboard_id = this.state.saveToDashboardId; - sliceParams.new_dashboard_name = this.state.newDashboardName; - const { url_params, ...formData } = this.props.form_data || {}; - this.props.actions - .saveSlice(formData, sliceParams) - .then((data: JsonObject) => { - if (data.dashboard_id === null) { - sessionStorage.removeItem(SK_DASHBOARD_ID); - } else { - sessionStorage.setItem(SK_DASHBOARD_ID, data.dashboard_id); - } - // Go to new slice url or dashboard url - let url = gotodash ? data.dashboard_url : data.slice.slice_url; - if (url_params) { - const prefix = url.includes('?') ? '&' : '?'; - url = `${url}${prefix}${new URLSearchParams(url_params).toString()}`; - } - window.location.assign(url); - }); + const searchParams = new URLSearchParams(this.props.location.search); + searchParams.set('save_action', this.state.action); + searchParams.delete('form_data_key'); + if (this.state.action === 'saveas') { + searchParams.set('slice_id', value.id.toString()); + } + this.props.history.replace(`/explore/?${searchParams.toString()}`); + }) as (value: any) => void); + this.props.onHide(); } @@ -296,11 +353,19 @@ class SaveModal extends React.Component { } } +interface StateProps { + datasource: any; + slice: any; + userId: any; + dashboards: any; + alert: any; +} + function mapStateToProps({ explore, saveModal, user, -}: Record): Partial { +}: Record): StateProps { return { datasource: explore.datasource, slice: explore.slice, @@ -310,4 +375,4 @@ function mapStateToProps({ }; } -export default connect(mapStateToProps, () => ({}))(SaveModal); +export default withRouter(connect(mapStateToProps, () => ({}))(SaveModal)); diff --git a/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.ts b/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.ts index 042b8f7f88766..1e5007875a189 100644 --- a/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.ts +++ b/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.ts @@ -17,6 +17,11 @@ * under the License. */ +export interface Location { + search: string; + pathname: string; +} + // mapping { url_param: v1_explore_request_param } const EXPLORE_URL_SEARCH_PARAMS = { form_data: { @@ -70,8 +75,8 @@ const EXPLORE_URL_PATH_PARAMS = { // search params can be placed in form_data object // we need to "flatten" the search params to use them with /v1/explore endpoint -const getParsedExploreURLSearchParams = () => { - const urlSearchParams = new URLSearchParams(window.location.search); +const getParsedExploreURLSearchParams = (search: string) => { + const urlSearchParams = new URLSearchParams(search); return Object.keys(EXPLORE_URL_SEARCH_PARAMS).reduce((acc, currentParam) => { const paramValue = urlSearchParams.get(currentParam); if (paramValue === null) { @@ -96,21 +101,23 @@ const getParsedExploreURLSearchParams = () => { }; // path params need to be transformed to search params to use them with /v1/explore endpoint -const getParsedExploreURLPathParams = () => +const getParsedExploreURLPathParams = (pathname: string) => Object.keys(EXPLORE_URL_PATH_PARAMS).reduce((acc, currentParam) => { const re = new RegExp(`/(${currentParam})/(\\w+)`); - const pathGroups = window.location.pathname.match(re); + const pathGroups = pathname.match(re); if (pathGroups && pathGroups[2]) { return { ...acc, [EXPLORE_URL_PATH_PARAMS[currentParam]]: pathGroups[2] }; } return acc; }, {}); -export const getParsedExploreURLParams = () => +export const getParsedExploreURLParams = ( + location: Location = window.location, +) => new URLSearchParams( Object.entries({ - ...getParsedExploreURLSearchParams(), - ...getParsedExploreURLPathParams(), + ...getParsedExploreURLSearchParams(location.search), + ...getParsedExploreURLPathParams(location.pathname), }) .map(entry => entry.join('=')) .join('&'),