From 399f6e3ddc8bb21fd7b39cdf850510b2692fbe12 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Tue, 10 Jan 2023 16:48:51 +0100 Subject: [PATCH] feat(dashboard): Display a loading spinner while dashboard is being saved (#22588) --- .../src/dashboard/actions/dashboardState.js | 14 ++++++++++++++ .../src/dashboard/actions/dashboardState.test.js | 8 +++++--- superset-frontend/src/dashboard/actions/hydrate.js | 1 + .../DashboardBuilder/DashboardBuilder.test.tsx | 14 ++++++++++++++ .../DashboardBuilder/DashboardBuilder.tsx | 12 ++++++++++++ .../src/dashboard/reducers/dashboardState.js | 14 ++++++++++++++ superset-frontend/src/dashboard/types.ts | 1 + 7 files changed, 61 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index 9bbe618c88da7..518dd7b5dc31f 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.js @@ -203,9 +203,20 @@ export function setOverrideConfirm(overwriteConfirmMetadata) { }; } +export const SAVE_DASHBOARD_STARTED = 'SAVE_DASHBOARD_STARTED'; +export function saveDashboardStarted() { + return { type: SAVE_DASHBOARD_STARTED }; +} + +export const SAVE_DASHBOARD_FINISHED = 'SAVE_DASHBOARD_FINISHED'; +export function saveDashboardFinished() { + return { type: SAVE_DASHBOARD_FINISHED }; +} + export function saveDashboardRequest(data, id, saveType) { return (dispatch, getState) => { dispatch({ type: UPDATE_COMPONENTS_PARENTS_LIST }); + dispatch(saveDashboardStarted()); const { dashboardFilters, dashboardLayout } = getState(); const layout = dashboardLayout.present; @@ -291,6 +302,7 @@ export function saveDashboardRequest(data, id, saveType) { const chartConfiguration = handleChartConfiguration(); dispatch(setChartConfiguration(chartConfiguration)); } + dispatch(saveDashboardFinished()); dispatch(addSuccessToast(t('This dashboard was saved successfully.'))); return response; }; @@ -322,6 +334,7 @@ export function saveDashboardRequest(data, id, saveType) { if (lastModifiedTime) { dispatch(saveDashboardRequestSuccess(lastModifiedTime)); } + dispatch(saveDashboardFinished()); // redirect to the new slug or id window.history.pushState( { event: 'dashboard_properties_changed' }, @@ -347,6 +360,7 @@ export function saveDashboardRequest(data, id, saveType) { if (typeof message === 'string' && message === 'Forbidden') { errorText = t('You do not have permission to edit this dashboard'); } + dispatch(saveDashboardFinished()); dispatch(addDangerToast(errorText)); }; diff --git a/superset-frontend/src/dashboard/actions/dashboardState.test.js b/superset-frontend/src/dashboard/actions/dashboardState.test.js index 25aa54ed6638a..00b358bc43972 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.test.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.test.js @@ -22,6 +22,7 @@ import { waitFor } from '@testing-library/react'; import { removeSliceFromDashboard, + SAVE_DASHBOARD_STARTED, saveDashboardRequest, SET_OVERRIDE_CONFIRM, } from 'src/dashboard/actions/dashboardState'; @@ -104,10 +105,11 @@ describe('dashboardState actions', () => { }); const thunk = saveDashboardRequest(newDashboardData, 1, 'save_dash'); thunk(dispatch, getState); - expect(dispatch.callCount).toBe(1); + expect(dispatch.callCount).toBe(2); expect(dispatch.getCall(0).args[0].type).toBe( UPDATE_COMPONENTS_PARENTS_LIST, ); + expect(dispatch.getCall(1).args[0].type).toBe(SAVE_DASHBOARD_STARTED); }); it('should post dashboard data with updated redux state', () => { @@ -162,10 +164,10 @@ describe('dashboardState actions', () => { expect(getStub.callCount).toBe(1); expect(postStub.callCount).toBe(0); await waitFor(() => - expect(dispatch.getCall(1).args[0].type).toBe(SET_OVERRIDE_CONFIRM), + expect(dispatch.getCall(2).args[0].type).toBe(SET_OVERRIDE_CONFIRM), ); expect( - dispatch.getCall(1).args[0].overwriteConfirmMetadata.dashboardId, + dispatch.getCall(2).args[0].overwriteConfirmMetadata.dashboardId, ).toBe(id); }); diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js index 917492bd7b209..ed359a8cee10d 100644 --- a/superset-frontend/src/dashboard/actions/hydrate.js +++ b/superset-frontend/src/dashboard/actions/hydrate.js @@ -454,6 +454,7 @@ export const hydrateDashboard = editMode: canEdit && editMode, isPublished: dashboard.published, hasUnsavedChanges: false, + dashboardIsSaving: false, maxUndoHistoryExceeded: false, lastModifiedTime: dashboard.changed_on, isRefreshing: false, diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx index 65969e2ca6d7f..b5f249b89602e 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx @@ -249,6 +249,20 @@ describe('DashboardBuilder', () => { (setDirectPathToChild as jest.Mock).mockReset(); }); + it('should not display a loading spinner when saving is not in progress', () => { + const { queryByAltText } = setup(); + + expect(queryByAltText('Loading...')).not.toBeInTheDocument(); + }); + + it('should display a loading spinner when saving is in progress', async () => { + const { findByAltText } = setup({ + dashboardState: { dashboardIsSaving: true }, + }); + + expect(await findByAltText('Loading...')).toBeVisible(); + }); + describe('when nativeFiltersEnabled', () => { beforeEach(() => { (isFeatureEnabled as jest.Mock).mockImplementation( diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index d2ce6a2f1a3ed..ab14c4d93205c 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -246,6 +246,9 @@ const DashboardBuilder: FC = () => { const canEdit = useSelector( ({ dashboardInfo }) => dashboardInfo.dash_edit_perm, ); + const dashboardIsSaving = useSelector( + ({ dashboardState }) => dashboardState.dashboardIsSaving, + ); const nativeFilters = useSelector((state: RootState) => state.nativeFilters); const focusedFilterId = nativeFilters?.focusedFilterId; const fullSizeChartId = useSelector( @@ -533,6 +536,15 @@ const DashboardBuilder: FC = () => { + {dashboardIsSaving && ( + + )} ); }; diff --git a/superset-frontend/src/dashboard/reducers/dashboardState.js b/superset-frontend/src/dashboard/reducers/dashboardState.js index 1c339001d17d6..5d81cd8ac11f0 100644 --- a/superset-frontend/src/dashboard/reducers/dashboardState.js +++ b/superset-frontend/src/dashboard/reducers/dashboardState.js @@ -43,6 +43,8 @@ import { ON_FILTERS_REFRESH_SUCCESS, SET_DATASETS_STATUS, SET_OVERRIDE_CONFIRM, + SAVE_DASHBOARD_STARTED, + SAVE_DASHBOARD_FINISHED, } from '../actions/dashboardState'; import { HYDRATE_DASHBOARD } from '../actions/hydrate'; @@ -111,6 +113,18 @@ export default function dashboardStateReducer(state = {}, action) { [ON_CHANGE]() { return { ...state, hasUnsavedChanges: true }; }, + [SAVE_DASHBOARD_STARTED]() { + return { + ...state, + dashboardIsSaving: true, + }; + }, + [SAVE_DASHBOARD_FINISHED]() { + return { + ...state, + dashboardIsSaving: false, + }; + }, [ON_SAVE]() { return { ...state, diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts index 1bdd1c14a1c7a..e24356be61910 100644 --- a/superset-frontend/src/dashboard/types.ts +++ b/superset-frontend/src/dashboard/types.ts @@ -70,6 +70,7 @@ export type DashboardState = { isRefreshing: boolean; isFiltersRefreshing: boolean; hasUnsavedChanges: boolean; + dashboardIsSaving: boolean; colorScheme: string; sliceIds: number[]; directPathLastUpdated: number;