From 67859ba42a25b61d77e3d317391dc69b3886c5d6 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 27 Oct 2022 23:30:59 +0200 Subject: [PATCH] fix(explore): Chart save modal displays error instead of failing silently (#21920) --- superset-frontend/src/explore/ExplorePage.tsx | 159 ++++++ .../src/explore/actions/exploreActions.ts | 6 + .../explore/actions/hydrateExplore.test.ts | 212 ++++++++ .../src/explore/actions/hydrateExplore.ts | 194 ++++++++ .../src/explore/actions/saveModalActions.js | 6 + .../ExploreChartHeader.test.tsx | 49 +- .../components/ExploreChartHeader/index.jsx | 35 +- .../components/ExploreViewContainer/index.jsx | 54 +- .../src/explore/components/SaveModal.tsx | 466 ++++++++++++------ .../src/explore/reducers/exploreReducer.js | 6 + .../src/explore/reducers/saveModalReducer.js | 3 + superset-frontend/src/explore/types.ts | 10 +- 12 files changed, 1003 insertions(+), 197 deletions(-) create mode 100644 superset-frontend/src/explore/ExplorePage.tsx create mode 100644 superset-frontend/src/explore/actions/hydrateExplore.test.ts create mode 100644 superset-frontend/src/explore/actions/hydrateExplore.ts diff --git a/superset-frontend/src/explore/ExplorePage.tsx b/superset-frontend/src/explore/ExplorePage.tsx new file mode 100644 index 0000000000000..6084cee7826e3 --- /dev/null +++ b/superset-frontend/src/explore/ExplorePage.tsx @@ -0,0 +1,159 @@ +/** + * 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 React, { useEffect, useRef, useState } from 'react'; +import { useDispatch } from 'react-redux'; +import { useLocation } from 'react-router-dom'; +import { + getSharedLabelColor, + isDefined, + JsonObject, + makeApi, + SharedLabelColorSource, + t, +} from '@superset-ui/core'; +import Loading from 'src/components/Loading'; +import { addDangerToast } from 'src/components/MessageToasts/actions'; +import { getUrlParam } from 'src/utils/urlUtils'; +import { URL_PARAMS } from 'src/constants'; +import { getClientErrorObject } from 'src/utils/getClientErrorObject'; +import getFormDataWithExtraFilters from 'src/dashboard/util/charts/getFormDataWithExtraFilters'; +import { getAppliedFilterValues } from 'src/dashboard/util/activeDashboardFilters'; +import { getParsedExploreURLParams } from './exploreUtils/getParsedExploreURLParams'; +import { hydrateExplore } from './actions/hydrateExplore'; +import ExploreViewContainer from './components/ExploreViewContainer'; +import { ExploreResponsePayload, SaveActionType } from './types'; +import { fallbackExploreInitialData } from './fixtures'; +import { getItem, LocalStorageKeys } from '../utils/localStorageHelpers'; +import { getFormDataWithDashboardContext } from './controlUtils/getFormDataWithDashboardContext'; + +const isResult = (rv: JsonObject): rv is ExploreResponsePayload => + rv?.result?.form_data && + rv?.result?.dataset && + isDefined(rv?.result?.dataset?.id); + +const fetchExploreData = async (exploreUrlParams: URLSearchParams) => { + try { + const rv = await makeApi<{}, ExploreResponsePayload>({ + method: 'GET', + endpoint: 'api/v1/explore/', + })(exploreUrlParams); + if (isResult(rv)) { + return rv; + } + throw new Error(t('Failed to load chart data.')); + } catch (err) { + // todo: encapsulate the error handler + const clientError = await getClientErrorObject(err); + throw new Error( + clientError.message || + clientError.error || + t('Failed to load chart data.'), + ); + } +}; + +const getDashboardPageContext = (pageId?: string | null) => { + if (!pageId) { + return null; + } + return ( + getItem(LocalStorageKeys.dashboard__explore_context, {})[pageId] || null + ); +}; + +const getDashboardContextFormData = () => { + const dashboardPageId = getUrlParam(URL_PARAMS.dashboardPageId); + const dashboardContext = getDashboardPageContext(dashboardPageId); + if (dashboardContext) { + const sliceId = getUrlParam(URL_PARAMS.sliceId) || 0; + const { + labelColors, + sharedLabelColors, + colorScheme, + chartConfiguration, + nativeFilters, + filterBoxFilters, + dataMask, + dashboardId, + } = dashboardContext; + const dashboardContextWithFilters = getFormDataWithExtraFilters({ + chart: { id: sliceId }, + filters: getAppliedFilterValues(sliceId, filterBoxFilters), + nativeFilters, + chartConfiguration, + colorScheme, + dataMask, + labelColors, + sharedLabelColors, + sliceId, + allSliceIds: [sliceId], + extraControls: {}, + }); + Object.assign(dashboardContextWithFilters, { dashboardId }); + return dashboardContextWithFilters; + } + return null; +}; + +export default function ExplorePage() { + const [isLoaded, setIsLoaded] = useState(false); + const isExploreInitialized = useRef(false); + const dispatch = useDispatch(); + const location = useLocation(); + + useEffect(() => { + const exploreUrlParams = getParsedExploreURLParams(location); + const saveAction = getUrlParam( + URL_PARAMS.saveAction, + ) as SaveActionType | null; + const dashboardContextFormData = getDashboardContextFormData(); + if (!isExploreInitialized.current || !!saveAction) { + fetchExploreData(exploreUrlParams) + .then(({ result }) => { + const formData = dashboardContextFormData + ? getFormDataWithDashboardContext( + result.form_data, + dashboardContextFormData, + ) + : result.form_data; + dispatch( + hydrateExplore({ + ...result, + form_data: formData, + saveAction, + }), + ); + }) + .catch(err => { + dispatch(hydrateExplore(fallbackExploreInitialData)); + dispatch(addDangerToast(err.message)); + }) + .finally(() => { + setIsLoaded(true); + isExploreInitialized.current = true; + }); + } + getSharedLabelColor().source = SharedLabelColorSource.explore; + }, [dispatch, location]); + + if (!isLoaded) { + return ; + } + return ; +} diff --git a/superset-frontend/src/explore/actions/exploreActions.ts b/superset-frontend/src/explore/actions/exploreActions.ts index 2b6c0f549059d..0c721e4ba4085 100644 --- a/superset-frontend/src/explore/actions/exploreActions.ts +++ b/superset-frontend/src/explore/actions/exploreActions.ts @@ -30,6 +30,7 @@ import { toastActions, } from 'src/components/MessageToasts/actions'; import { Slice } from 'src/types/Chart'; +import { SaveActionType } from 'src/explore/types'; const FAVESTAR_BASE_URL = '/superset/favstar/slice'; @@ -117,6 +118,11 @@ export function updateChartTitle(sliceName: string) { return { type: UPDATE_CHART_TITLE, sliceName }; } +export const SET_SAVE_ACTION = 'SET_SAVE_ACTION'; +export function setSaveAction(saveAction: SaveActionType | null) { + return { type: SET_SAVE_ACTION, saveAction }; +} + export const CREATE_NEW_SLICE = 'CREATE_NEW_SLICE'; export function createNewSlice( can_add: boolean, diff --git a/superset-frontend/src/explore/actions/hydrateExplore.test.ts b/superset-frontend/src/explore/actions/hydrateExplore.test.ts new file mode 100644 index 0000000000000..353a811396d47 --- /dev/null +++ b/superset-frontend/src/explore/actions/hydrateExplore.test.ts @@ -0,0 +1,212 @@ +/** + * 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 { hydrateExplore, HYDRATE_EXPLORE } from './hydrateExplore'; +import { exploreInitialData } from '../fixtures'; + +test('creates hydrate action from initial data', () => { + const dispatch = jest.fn(); + const getState = jest.fn(() => ({ + user: {}, + charts: {}, + datasources: {}, + common: {}, + explore: {}, + })); + // ignore type check - we dont need exact explore state for this test + // @ts-ignore + hydrateExplore(exploreInitialData)(dispatch, getState); + expect(dispatch).toHaveBeenCalledWith( + expect.objectContaining({ + type: HYDRATE_EXPLORE, + data: { + charts: { + 371: { + id: 371, + chartAlert: null, + chartStatus: null, + chartStackTrace: null, + chartUpdateEndTime: null, + chartUpdateStartTime: 0, + latestQueryFormData: { + cache_timeout: undefined, + datasource: '8__table', + slice_id: 371, + url_params: undefined, + viz_type: 'table', + }, + sliceFormData: { + cache_timeout: undefined, + datasource: '8__table', + slice_id: 371, + url_params: undefined, + viz_type: 'table', + }, + queryController: null, + queriesResponse: null, + triggerQuery: false, + lastRendered: 0, + }, + }, + datasources: { + '8__table': exploreInitialData.dataset, + }, + saveModal: { + dashboards: [], + saveModalAlert: null, + isVisible: false, + }, + explore: { + can_add: false, + can_download: false, + can_overwrite: false, + isDatasourceMetaLoading: false, + isStarred: false, + triggerRender: false, + datasource: exploreInitialData.dataset, + controls: expect.any(Object), + form_data: exploreInitialData.form_data, + slice: exploreInitialData.slice, + standalone: null, + force: null, + saveAction: null, + }, + }, + }), + ); +}); + +test('creates hydrate action with existing state', () => { + const dispatch = jest.fn(); + const getState = jest.fn(() => ({ + user: {}, + charts: {}, + datasources: {}, + common: {}, + explore: { controlsTransferred: ['all_columns'] }, + })); + // ignore type check - we dont need exact explore state for this test + // @ts-ignore + hydrateExplore(exploreInitialData)(dispatch, getState); + expect(dispatch).toHaveBeenCalledWith( + expect.objectContaining({ + type: HYDRATE_EXPLORE, + data: { + charts: { + 371: { + id: 371, + chartAlert: null, + chartStatus: null, + chartStackTrace: null, + chartUpdateEndTime: null, + chartUpdateStartTime: 0, + latestQueryFormData: { + cache_timeout: undefined, + datasource: '8__table', + slice_id: 371, + url_params: undefined, + viz_type: 'table', + }, + sliceFormData: { + cache_timeout: undefined, + datasource: '8__table', + slice_id: 371, + url_params: undefined, + viz_type: 'table', + }, + queryController: null, + queriesResponse: null, + triggerQuery: false, + lastRendered: 0, + }, + }, + datasources: { + '8__table': exploreInitialData.dataset, + }, + saveModal: { + dashboards: [], + saveModalAlert: null, + isVisible: false, + }, + explore: { + can_add: false, + can_download: false, + can_overwrite: false, + isDatasourceMetaLoading: false, + isStarred: false, + triggerRender: false, + datasource: exploreInitialData.dataset, + controls: expect.any(Object), + controlsTransferred: ['all_columns'], + form_data: exploreInitialData.form_data, + slice: exploreInitialData.slice, + standalone: null, + force: null, + saveAction: null, + }, + }, + }), + ); +}); + +test('uses configured default time range if not set', () => { + const dispatch = jest.fn(); + const getState = jest.fn(() => ({ + user: {}, + charts: {}, + datasources: {}, + common: { + conf: { + DEFAULT_TIME_FILTER: 'Last year', + }, + }, + explore: {}, + })); + // @ts-ignore + hydrateExplore({ form_data: {}, slice: {}, dataset: {} })(dispatch, getState); + expect(dispatch).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + explore: expect.objectContaining({ + form_data: expect.objectContaining({ + time_range: 'Last year', + }), + }), + }), + }), + ); + const withTimeRangeSet = { + form_data: { time_range: 'Last day' }, + slice: {}, + dataset: {}, + }; + // @ts-ignore + hydrateExplore(withTimeRangeSet)(dispatch, getState); + expect(dispatch).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + explore: expect.objectContaining({ + form_data: expect.objectContaining({ + time_range: 'Last day', + }), + }), + }), + }), + ); +}); diff --git a/superset-frontend/src/explore/actions/hydrateExplore.ts b/superset-frontend/src/explore/actions/hydrateExplore.ts new file mode 100644 index 0000000000000..1f4f510b5e4e1 --- /dev/null +++ b/superset-frontend/src/explore/actions/hydrateExplore.ts @@ -0,0 +1,194 @@ +/** + * 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 { ControlStateMapping } from '@superset-ui/chart-controls'; + +import { + ChartState, + ExplorePageInitialData, + ExplorePageState, +} from 'src/explore/types'; +import { getChartKey } from 'src/explore/exploreUtils'; +import { getControlsState } from 'src/explore/store'; +import { Dispatch } from 'redux'; +import { + ensureIsArray, + getCategoricalSchemeRegistry, + getSequentialSchemeRegistry, + NO_TIME_RANGE, +} from '@superset-ui/core'; +import { + getFormDataFromControls, + applyMapStateToPropsToControl, +} from 'src/explore/controlUtils'; +import { getDatasourceUid } from 'src/utils/getDatasourceUid'; +import { getUrlParam } from 'src/utils/urlUtils'; +import { URL_PARAMS } from 'src/constants'; +import { findPermission } from 'src/utils/findPermission'; + +enum ColorSchemeType { + CATEGORICAL = 'CATEGORICAL', + SEQUENTIAL = 'SEQUENTIAL', +} + +export const HYDRATE_EXPLORE = 'HYDRATE_EXPLORE'; +export const hydrateExplore = + ({ + form_data, + slice, + dataset, + metadata, + saveAction = null, + }: ExplorePageInitialData) => + (dispatch: Dispatch, getState: () => ExplorePageState) => { + const { user, datasources, charts, sliceEntities, common, explore } = + getState(); + + const sliceId = getUrlParam(URL_PARAMS.sliceId); + const dashboardId = getUrlParam(URL_PARAMS.dashboardId); + const fallbackSlice = sliceId ? sliceEntities?.slices?.[sliceId] : null; + const initialSlice = slice ?? fallbackSlice; + const initialFormData = form_data ?? initialSlice?.form_data; + if (!initialFormData.viz_type) { + const defaultVizType = common?.conf.DEFAULT_VIZ_TYPE || 'table'; + initialFormData.viz_type = + getUrlParam(URL_PARAMS.vizType) || defaultVizType; + } + if (!initialFormData.time_range) { + initialFormData.time_range = + common?.conf?.DEFAULT_TIME_FILTER || NO_TIME_RANGE; + } + if (dashboardId) { + initialFormData.dashboardId = dashboardId; + } + const initialDatasource = dataset; + + const initialExploreState = { + form_data: initialFormData, + slice: initialSlice, + datasource: initialDatasource, + }; + const initialControls = getControlsState( + initialExploreState, + initialFormData, + ) as ControlStateMapping; + const colorSchemeKey = initialControls.color_scheme && 'color_scheme'; + const linearColorSchemeKey = + initialControls.linear_color_scheme && 'linear_color_scheme'; + // if the selected color scheme does not exist anymore + // fallbacks and selects the available default one + const verifyColorScheme = (type: ColorSchemeType) => { + const schemes = + type === 'CATEGORICAL' + ? getCategoricalSchemeRegistry() + : getSequentialSchemeRegistry(); + const key = + type === 'CATEGORICAL' ? colorSchemeKey : linearColorSchemeKey; + const registryDefaultScheme = schemes.defaultKey; + const defaultScheme = + type === 'CATEGORICAL' ? 'supersetColors' : 'superset_seq_1'; + const currentScheme = initialFormData[key]; + const colorSchemeExists = !!schemes.get(currentScheme, true); + + if (currentScheme && !colorSchemeExists) { + initialControls[key].value = registryDefaultScheme || defaultScheme; + } + }; + + if (colorSchemeKey) verifyColorScheme(ColorSchemeType.CATEGORICAL); + if (linearColorSchemeKey) verifyColorScheme(ColorSchemeType.SEQUENTIAL); + + const exploreState = { + // note this will add `form_data` to state, + // which will be manipulable by future reducers. + can_add: findPermission('can_write', 'Chart', user?.roles), + can_download: findPermission('can_csv', 'Superset', user?.roles), + can_overwrite: ensureIsArray(slice?.owners).includes( + user?.userId as number, + ), + isDatasourceMetaLoading: false, + isStarred: false, + triggerRender: false, + // duplicate datasource in exploreState - it's needed by getControlsState + datasource: initialDatasource, + // Initial control state will skip `control.mapStateToProps` + // because `bootstrapData.controls` is undefined. + controls: initialControls, + form_data: initialFormData, + slice: initialSlice, + controlsTransferred: explore.controlsTransferred, + standalone: getUrlParam(URL_PARAMS.standalone), + force: getUrlParam(URL_PARAMS.force), + metadata, + saveAction, + }; + + // apply initial mapStateToProps for all controls, must execute AFTER + // bootstrapState has initialized `controls`. Order of execution is not + // guaranteed, so controls shouldn't rely on each other's mapped state. + Object.entries(exploreState.controls).forEach(([key, controlState]) => { + exploreState.controls[key] = applyMapStateToPropsToControl( + controlState, + exploreState, + ); + }); + const sliceFormData = initialSlice + ? getFormDataFromControls(initialControls) + : null; + + const chartKey: number = getChartKey(initialExploreState); + const chart: ChartState = { + id: chartKey, + chartAlert: null, + chartStatus: null, + chartStackTrace: null, + chartUpdateEndTime: null, + chartUpdateStartTime: 0, + latestQueryFormData: getFormDataFromControls(exploreState.controls), + sliceFormData, + queryController: null, + queriesResponse: null, + triggerQuery: false, + lastRendered: 0, + }; + + return dispatch({ + type: HYDRATE_EXPLORE, + data: { + charts: { + ...charts, + [chartKey]: chart, + }, + datasources: { + ...datasources, + [getDatasourceUid(initialDatasource)]: initialDatasource, + }, + saveModal: { + dashboards: [], + saveModalAlert: null, + isVisible: false, + }, + explore: exploreState, + }, + }); + }; + +export type HydrateExplore = { + type: typeof HYDRATE_EXPLORE; + data: ExplorePageState; +}; diff --git a/superset-frontend/src/explore/actions/saveModalActions.js b/superset-frontend/src/explore/actions/saveModalActions.js index 6e87fe52b5f17..962b338828a16 100644 --- a/superset-frontend/src/explore/actions/saveModalActions.js +++ b/superset-frontend/src/explore/actions/saveModalActions.js @@ -29,6 +29,12 @@ export function fetchDashboardsFailed(userId) { return { type: FETCH_DASHBOARDS_FAILED, userId }; } +export const SET_SAVE_CHART_MODAL_VISIBILITY = + 'SET_SAVE_CHART_MODAL_VISIBILITY'; +export function setSaveChartModalVisibility(isVisible) { + return { type: SET_SAVE_CHART_MODAL_VISIBILITY, isVisible }; +} + export function fetchDashboards(userId) { return function fetchDashboardsThunk(dispatch) { return SupersetClient.get({ diff --git a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx index 540a444ab0473..1e7a2cd9b5f15 100644 --- a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx +++ b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx @@ -23,6 +23,7 @@ import { render, screen, waitFor } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import fetchMock from 'fetch-mock'; import * as chartAction from 'src/components/Chart/chartAction'; +import * as saveModalActions from 'src/explore/actions/saveModalActions'; import * as downloadAsImage from 'src/utils/downloadAsImage'; import * as exploreUtils from 'src/explore/exploreUtils'; import ExploreHeader from '.'; @@ -101,7 +102,14 @@ const createProps = () => ({ user: { userId: 1, }, - onSaveChart: jest.fn(), + metadata: { + created_on_humanized: 'a week ago', + changed_on_humanized: '2 days ago', + owners: ['John Doe'], + created_by: 'John Doe', + changed_by: 'John Doe', + dashboards: [{ id: 1, dashboard_title: 'Test' }], + }, canOverwrite: false, canDownload: false, isStarred: false, @@ -139,18 +147,49 @@ test('Cancelling changes to the properties should reset previous properties', () expect(screen.getByDisplayValue(prevChartName)).toBeInTheDocument(); }); -test('Save chart', () => { +test('renders the metadata bar when saved', async () => { + const props = createProps({ showTitlePanelItems: true }); + render(, { useRedux: true }); + expect( + await screen.findByText('Added to 1 dashboard(s)'), + ).toBeInTheDocument(); + expect(await screen.findByText('Simple description')).toBeInTheDocument(); + expect(await screen.findByText('John Doe')).toBeInTheDocument(); + expect(await screen.findByText('2 days ago')).toBeInTheDocument(); +}); + +test('does not render the metadata bar when not saved', async () => { + const props = createProps({ showTitlePanelItems: true, slice: null }); + render(, { useRedux: true }); + await waitFor(() => + expect( + screen.queryByText('Added to 1 dashboard(s)'), + ).not.toBeInTheDocument(), + ); +}); + +test('Save chart', async () => { + const setSaveChartModalVisibility = jest.spyOn( + saveModalActions, + 'setSaveChartModalVisibility', + ); const props = createProps(); render(, { useRedux: true }); userEvent.click(screen.getByText('Save')); - expect(props.onSaveChart).toHaveBeenCalled(); + expect(setSaveChartModalVisibility).toHaveBeenCalledWith(true); + setSaveChartModalVisibility.mockClear(); }); -test('Save disabled', () => { +test('Save disabled', async () => { + const setSaveChartModalVisibility = jest.spyOn( + saveModalActions, + 'setSaveChartModalVisibility', + ); const props = createProps(); render(, { useRedux: true }); userEvent.click(screen.getByText('Save')); - expect(props.onSaveChart).not.toHaveBeenCalled(); + expect(setSaveChartModalVisibility).not.toHaveBeenCalled(); + setSaveChartModalVisibility.mockClear(); }); describe('Additional actions tests', () => { diff --git a/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx b/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx index fb85882f02337..63e59d3f05bcb 100644 --- a/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx +++ b/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx @@ -16,9 +16,8 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useEffect, useState } from 'react'; -import { connect } from 'react-redux'; -import { bindActionCreators } from 'redux'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import { useDispatch } from 'react-redux'; import PropTypes from 'prop-types'; import { Tooltip } from 'src/components/Tooltip'; import { @@ -27,7 +26,6 @@ import { SupersetClient, t, } from '@superset-ui/core'; -import { toggleActive, deleteActiveReport } from 'src/reports/actions/reports'; import { chartPropShape } from 'src/dashboard/util/propShapes'; import AlteredSliceTag from 'src/components/AlteredSliceTag'; import Button from 'src/components/Button'; @@ -35,6 +33,8 @@ import Icons from 'src/components/Icons'; import PropertiesModal from 'src/explore/components/PropertiesModal'; import { sliceUpdated } from 'src/explore/actions/exploreActions'; import { PageHeaderWithActions } from 'src/components/PageHeaderWithActions'; +import MetadataBar, { MetadataType } from 'src/components/MetadataBar'; +import { setSaveChartModalVisibility } from 'src/explore/actions/saveModalActions'; import { useExploreAdditionalActionsMenu } from '../useExploreAdditionalActionsMenu'; const propTypes = { @@ -71,11 +71,10 @@ export const ExploreChartHeader = ({ canOverwrite, canDownload, isStarred, - sliceUpdated, sliceName, - onSaveChart, saveDisabled, }) => { + const dispatch = useDispatch(); const { latestQueryFormData, sliceFormData } = chart; const [isPropertiesModalOpen, setIsPropertiesModalOpen] = useState(false); @@ -132,6 +131,17 @@ export const ExploreChartHeader = ({ setIsPropertiesModalOpen(false); }; + const showModal = useCallback(() => { + dispatch(setSaveChartModalVisibility(true)); + }, [dispatch]); + + const updateSlice = useCallback( + slice => { + dispatch(sliceUpdated(slice)); + }, + [dispatch], + ); + const [menu, isDropdownVisible, setIsDropdownVisible] = useExploreAdditionalActionsMenu( latestQueryFormData, @@ -193,7 +203,7 @@ export const ExploreChartHeader = ({
+ {props.isSaveModalVisible && ( + + )} ); } @@ -693,8 +697,19 @@ function ExploreViewContainer(props) { ExploreViewContainer.propTypes = propTypes; function mapStateToProps(state) { - const { explore, charts, impressionId, dataMask, reports } = state; - const form_data = getFormDataFromControls(explore.controls); + const { + explore, + charts, + common, + impressionId, + dataMask, + reports, + user, + saveModal, + } = state; + const { controls, slice, datasource, metadata } = explore; + const form_data = getFormDataFromControls(controls); + const slice_id = form_data.slice_id ?? slice?.slice_id ?? 0; // 0 - unsaved chart form_data.extra_form_data = mergeExtraFormData( { ...form_data.extra_form_data }, { @@ -742,6 +757,9 @@ function mapStateToProps(state) { user: explore.user, exploreState: explore, reports, + metadata, + saveAction: explore.saveAction, + isSaveModalVisible: saveModal.isVisible, }; } @@ -760,4 +778,4 @@ function mapDispatchToProps(dispatch) { export default connect( mapStateToProps, mapDispatchToProps, -)(ExploreViewContainer); +)(withToasts(React.memo(ExploreViewContainer))); diff --git a/superset-frontend/src/explore/components/SaveModal.tsx b/superset-frontend/src/explore/components/SaveModal.tsx index 81e91fecd62c5..dde8d8c04f45e 100644 --- a/superset-frontend/src/explore/components/SaveModal.tsx +++ b/superset-frontend/src/explore/components/SaveModal.tsx @@ -18,24 +18,37 @@ */ /* eslint camelcase: 0 */ import React from 'react'; +import { Dispatch } from 'redux'; +import { SelectValue } from 'antd/lib/select'; +import { connect } from 'react-redux'; +import ReactMarkdown from 'react-markdown'; +import { withRouter, RouteComponentProps } from 'react-router-dom'; +import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; +import { + css, + t, + styled, + DatasourceType, + isDefined, + ensureIsArray, +} from '@superset-ui/core'; 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 ReactMarkdown from 'react-markdown'; import Modal from 'src/components/Modal'; import { Radio } from 'src/components/Radio'; import Button from 'src/components/Button'; import { Select } from 'src/components'; -import { SelectValue } from 'antd/lib/select'; -import { connect } from 'react-redux'; +import Loading from 'src/components/Loading'; +import { setSaveChartModalVisibility } from 'src/explore/actions/saveModalActions'; +import { SaveActionType } from 'src/explore/types'; // 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 = { - onHide: () => void; +interface SaveModalProps extends RouteComponentProps { + addDangerToast: (msg: string) => void; actions: Record; form_data?: Record; userId: number; @@ -45,22 +58,30 @@ type SaveModalProps = { slice?: Record; datasource?: Record; dashboardId: '' | number | null; -}; - -type ActionType = 'overwrite' | 'saveas'; + isVisible: boolean; + dispatch: Dispatch; +} type SaveModalState = { saveToDashboardId: number | string | null; newSliceName?: string; newDashboardName?: string; + datasetName: string; alert: string | null; - action: ActionType; + action: SaveActionType; + isLoading: boolean; + saveStatus?: string | null; }; export const StyledModal = styled(Modal)` .ant-modal-body { overflow: visible; } + i { + position: absolute; + top: -${({ theme }) => theme.gridUnit * 5.25}px; + left: ${({ theme }) => theme.gridUnit * 26.75}px; + } `; class SaveModal extends React.Component { @@ -69,14 +90,18 @@ class SaveModal extends React.Component { this.state = { saveToDashboardId: null, newSliceName: props.sliceName, + datasetName: props.datasource?.name, alert: null, action: this.canOverwriteSlice() ? 'overwrite' : 'saveas', + isLoading: false, }; this.onDashboardSelectChange = this.onDashboardSelectChange.bind(this); this.onSliceNameChange = this.onSliceNameChange.bind(this); this.changeAction = this.changeAction.bind(this); this.saveOrOverwrite = this.saveOrOverwrite.bind(this); this.isNewDashboard = this.isNewDashboard.bind(this); + this.removeAlert = this.removeAlert.bind(this); + this.onHide = this.onHide.bind(this); } isNewDashboard(): boolean { @@ -92,7 +117,10 @@ class SaveModal extends React.Component { componentDidMount() { this.props.actions.fetchDashboards(this.props.userId).then(() => { - const dashboardIds = this.props.dashboards.map( + if (ensureIsArray(this.props.dashboards).length === 0) { + return; + } + const dashboardIds = this.props.dashboards?.map( dashboard => dashboard.value, ); const lastDashboard = sessionStorage.getItem(SK_DASHBOARD_ID); @@ -113,6 +141,11 @@ class SaveModal extends React.Component { }); } + handleDatasetNameChange = (e: React.FormEvent) => { + // @ts-expect-error + this.setState({ datasetName: e.target.value }); + }; + onSliceNameChange(event: React.ChangeEvent) { this.setState({ newSliceName: event.target.value }); } @@ -124,48 +157,253 @@ class SaveModal extends React.Component { this.setState({ saveToDashboardId, newDashboardName }); } - changeAction(action: ActionType) { + changeAction(action: SaveActionType) { this.setState({ action }); } - saveOrOverwrite(gotodash: boolean) { - this.setState({ alert: null }); + onHide() { + this.props.dispatch(setSaveChartModalVisibility(false)); + } + + async saveOrOverwrite(gotodash: boolean) { + this.setState({ alert: null, isLoading: true }); this.props.actions.removeSaveModalAlert(); - const sliceParams: Record = {}; - if (this.props.slice && this.props.slice.slice_id) { - sliceParams.slice_id = this.props.slice.slice_id; - } - if (sliceParams.action === 'saveas') { - if (this.state.newSliceName === '') { - this.setState({ alert: t('Please enter a chart name') }); + // Create or retrieve dashboard + type DashboardGetResponse = { + id: number; + url: string; + dashboard_title: string; + }; + + try { + if (this.props.datasource?.type === DatasourceType.Query) { + const { schema, sql, database } = this.props.datasource; + const { templateParams } = this.props.datasource; + const columns = this.props.datasource?.columns || []; + + await this.props.actions.saveDataset({ + schema, + sql, + database, + templateParams, + datasourceName: this.state.datasetName, + columns, + }); + } + + // Get chart dashboards + let sliceDashboards: number[] = []; + if (this.props.slice && this.state.action === 'overwrite') { + sliceDashboards = await this.props.actions.getSliceDashboards( + this.props.slice, + ); + } + + let dashboard: DashboardGetResponse | null = null; + if (this.state.newDashboardName || this.state.saveToDashboardId) { + let saveToDashboardId = this.state.saveToDashboardId || null; + if (!this.state.saveToDashboardId) { + const response = await this.props.actions.createDashboard( + this.state.newDashboardName, + ); + saveToDashboardId = response.id; + } + + const response = await this.props.actions.getDashboard( + saveToDashboardId, + ); + dashboard = response.result; + if (isDefined(dashboard) && isDefined(dashboard?.id)) { + sliceDashboards = sliceDashboards.includes(dashboard.id) + ? sliceDashboards + : [...sliceDashboards, dashboard.id]; + const { url_params, ...formData } = this.props.form_data || {}; + this.props.actions.setFormData({ + ...formData, + dashboards: sliceDashboards, + }); + } + } + + // Update or create slice + let value: { id: number }; + if (this.state.action === 'overwrite') { + value = await this.props.actions.updateSlice( + this.props.slice, + this.state.newSliceName, + sliceDashboards, + dashboard + ? { + title: dashboard.dashboard_title, + new: !this.state.saveToDashboardId, + } + : null, + ); + } else { + value = await this.props.actions.createSlice( + this.state.newSliceName, + sliceDashboards, + dashboard + ? { + title: dashboard.dashboard_title, + new: !this.state.saveToDashboardId, + } + : null, + ); + } + + 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; } + + const searchParams = new URLSearchParams(window.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()}`); + + this.setState({ isLoading: false }); + this.onHide(); + } finally { + this.setState({ isLoading: false }); } - 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); + } + + 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' && ( + + + + + )} + + - - -