feat: Add confirmation modal for unsaved changes (#33809)

superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx

@@ -23,6 +23,7 @@ import {
   screen,
   userEvent,
   waitFor,
+  within,
 } from 'spec/helpers/testing-library';
 import fetchMock from 'fetch-mock';
 import * as chartAction from 'src/components/Chart/chartAction';
@@ -30,6 +31,7 @@ import * as saveModalActions from 'src/explore/actions/saveModalActions';
 import * as downloadAsImage from 'src/utils/downloadAsImage';
 import * as exploreUtils from 'src/explore/exploreUtils';
 import { FeatureFlag, VizType } from '@superset-ui/core';
+import { useUnsavedChangesPrompt } from 'src/hooks/useUnsavedChangesPrompt';
 import ExploreHeader from '.';
 
 const chartEndpoint = 'glob:*api/v1/chart/*';
@@ -40,6 +42,10 @@ window.featureFlags = {
   [FeatureFlag.EmbeddableCharts]: true,
 };
 
+jest.mock('src/hooks/useUnsavedChangesPrompt', () => ({
+  useUnsavedChangesPrompt: jest.fn(),
+}));
+
 const createProps = (additionalProps = {}) => ({
   chart: {
     id: 1,
@@ -134,6 +140,18 @@ fetchMock.post(
 describe('ExploreChartHeader', () => {
   jest.setTimeout(15000); // ✅ Applies to all tests in this suite
 
+  beforeEach(() => {
+    jest.clearAllMocks();
+
+    (useUnsavedChangesPrompt as jest.Mock).mockReturnValue({
+      showModal: false,
+      setShowModal: jest.fn(),
+      handleConfirmNavigation: jest.fn(),
+      handleSaveAndCloseModal: jest.fn(),
+      triggerManualSave: jest.fn(),
+    });
+  });
+
   test('Cancelling changes to the properties should reset previous properties', async () => {
     const props = createProps();
     render(<ExploreHeader {...props} />, { useRedux: true });
@@ -201,35 +219,173 @@ describe('ExploreChartHeader', () => {
   });
 
   test('Save chart', async () => {
-    const setSaveChartModalVisibility = jest.spyOn(
+    const setSaveChartModalVisibilitySpy = jest.spyOn(
       saveModalActions,
       'setSaveChartModalVisibility',
     );
+
+    const setSaveChartModalVisibilityMock =
+      setSaveChartModalVisibilitySpy as jest.Mock;
+
+    const triggerManualSave = jest.fn(() => {
+      setSaveChartModalVisibilityMock(true);
+    });
+
+    (useUnsavedChangesPrompt as jest.Mock).mockReturnValue({
+      showModal: false,
+      setShowModal: jest.fn(),
+      handleConfirmNavigation: jest.fn(),
+      handleSaveAndCloseModal: jest.fn(),
+      triggerManualSave,
+    });
+
     const props = createProps();
     render(<ExploreHeader {...props} />, { useRedux: true });
-    expect(await screen.findByText('Save')).toBeInTheDocument();
-    userEvent.click(screen.getByText('Save'));
-    expect(setSaveChartModalVisibility).toHaveBeenCalledWith(true);
-    setSaveChartModalVisibility.mockClear();
+
+    const saveButton: HTMLElement = await screen.findByRole('button', {
+      name: /save/i,
+    });
+
+    userEvent.click(saveButton);
+
+    expect(triggerManualSave).toHaveBeenCalled();
+    expect(setSaveChartModalVisibilityMock).toHaveBeenCalledWith(true);
+
+    setSaveChartModalVisibilityMock.mockClear();
   });
 
   test('Save disabled', async () => {
-    const setSaveChartModalVisibility = jest.spyOn(
-      saveModalActions,
-      'setSaveChartModalVisibility',
-    );
+    const triggerManualSave = jest.fn();
+
+    (useUnsavedChangesPrompt as jest.Mock).mockReturnValue({
+      showModal: false,
+      setShowModal: jest.fn(),
+      handleConfirmNavigation: jest.fn(),
+      handleSaveAndCloseModal: jest.fn(),
+      triggerManualSave,
+    });
+
     const props = createProps();
     render(<ExploreHeader {...props} saveDisabled />, { useRedux: true });
-    expect(await screen.findByText('Save')).toBeInTheDocument();
-    userEvent.click(screen.getByText('Save'));
-    expect(setSaveChartModalVisibility).not.toHaveBeenCalled();
-    setSaveChartModalVisibility.mockClear();
+
+    const saveButton: HTMLElement = await screen.findByRole('button', {
+      name: /save/i,
+    });
+
+    expect(saveButton).toBeDisabled();
+
+    userEvent.click(saveButton);
+
+    expect(triggerManualSave).not.toHaveBeenCalled();
+  });
+
+  test('should render UnsavedChangesModal when showModal is true', async () => {
+    const props = createProps();
+
+    (useUnsavedChangesPrompt as jest.Mock).mockReturnValue({
+      showModal: true,
+      setShowModal: jest.fn(),
+      handleConfirmNavigation: jest.fn(),
+      handleSaveAndCloseModal: jest.fn(),
+      triggerManualSave: jest.fn(),
+    });
+
+    render(<ExploreHeader {...props} />, { useRedux: true });
+
+    expect(await screen.findByRole('dialog')).toBeInTheDocument();
+    expect(
+      await screen.findByText('Save changes to your chart?'),
+    ).toBeInTheDocument();
+    expect(
+      await screen.findByText("If you don't save, changes will be lost."),
+    ).toBeInTheDocument();
+  });
+
+  test('should call handleSaveAndCloseModal when clicking Save in UnsavedChangesModal', async () => {
+    const handleSaveAndCloseModal = jest.fn();
+
+    (useUnsavedChangesPrompt as jest.Mock).mockReturnValue({
+      showModal: true,
+      setShowModal: jest.fn(),
+      handleConfirmNavigation: jest.fn(),
+      handleSaveAndCloseModal,
+      triggerManualSave: jest.fn(),
+    });
+
+    const props = createProps();
+    render(<ExploreHeader {...props} />, { useRedux: true });
+
+    const modal: HTMLElement = await screen.findByRole('dialog');
+    const saveButton: HTMLElement = within(modal).getByRole('button', {
+      name: /save/i,
+    });
+
+    userEvent.click(saveButton);
+
+    expect(handleSaveAndCloseModal).toHaveBeenCalled();
+  });
+
+  test('should call handleConfirmNavigation when clicking Discard in UnsavedChangesModal', async () => {
+    const handleConfirmNavigation = jest.fn();
+
+    (useUnsavedChangesPrompt as jest.Mock).mockReturnValue({
+      showModal: true,
+      setShowModal: jest.fn(),
+      handleConfirmNavigation,
+      handleSaveAndCloseModal: jest.fn(),
+      triggerManualSave: jest.fn(),
+    });
+
+    const props = createProps();
+    render(<ExploreHeader {...props} />, { useRedux: true });
+
+    const modal: HTMLElement = await screen.findByRole('dialog');
+    const discardButton: HTMLElement = within(modal).getByRole('button', {
+      name: /discard/i,
+    });
+
+    userEvent.click(discardButton);
+
+    expect(handleConfirmNavigation).toHaveBeenCalled();
+  });
+
+  test('should call setShowModal(false) when clicking close button in UnsavedChangesModal', async () => {
+    const setShowModal = jest.fn();
+
+    (useUnsavedChangesPrompt as jest.Mock).mockReturnValue({
+      showModal: true,
+      setShowModal,
+      handleConfirmNavigation: jest.fn(),
+      handleSaveAndCloseModal: jest.fn(),
+      triggerManualSave: jest.fn(),
+    });
+
+    const props = createProps();
+    render(<ExploreHeader {...props} />, { useRedux: true });
+
+    const closeButton: HTMLElement = await screen.findByRole('button', {
+      name: /close/i,
+    });
+
+    userEvent.click(closeButton);
+
+    expect(setShowModal).toHaveBeenCalledWith(false);
   });
 });
 
 describe('Additional actions tests', () => {
   jest.setTimeout(15000); // ✅ Applies to all tests in this suite
 
+  beforeEach(() => {
+    (useUnsavedChangesPrompt as jest.Mock).mockReturnValue({
+      showModal: false,
+      setShowModal: jest.fn(),
+      handleConfirmNavigation: jest.fn(),
+      handleSaveAndCloseModal: jest.fn(),
+      triggerManualSave: jest.fn(),
+    });
+  });
+
   test('Should render a button', async () => {
     const props = createProps();
     render(<ExploreHeader {...props} />, { useRedux: true });
@@ -356,6 +512,14 @@ describe('Additional actions tests', () => {
     beforeEach(() => {
       spyDownloadAsImage = sinon.spy(downloadAsImage, 'default');
       spyExportChart = sinon.spy(exploreUtils, 'exportChart');
+
+      (useUnsavedChangesPrompt as jest.Mock).mockReturnValue({
+        showModal: false,
+        setShowModal: jest.fn(),
+        handleConfirmNavigation: jest.fn(),
+        handleSaveAndCloseModal: jest.fn(),
+        triggerManualSave: jest.fn(),
+      });
     });
 
     afterEach(async () => {

superset-frontend/src/explore/components/ExploreChartHeader/index.jsx

@@ -16,11 +16,16 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { useCallback, useEffect, useState } from 'react';
+import { useCallback, useEffect, useMemo, useState } from 'react';
 import { useHistory } from 'react-router-dom';
 import { useDispatch } from 'react-redux';
 import PropTypes from 'prop-types';
-import { Tooltip, Button, DeleteModal } from '@superset-ui/core/components';
+import {
+  Tooltip,
+  Button,
+  DeleteModal,
+  UnsavedChangesModal,
+} from '@superset-ui/core/components';
 import { AlteredSliceTag } from 'src/components';
 import { css, logging, SupersetClient, t } from '@superset-ui/core';
 import { chartPropShape } from 'src/dashboard/util/propShapes';
@@ -32,6 +37,8 @@ import { setSaveChartModalVisibility } from 'src/explore/actions/saveModalAction
 import { applyColors, resetColors } from 'src/utils/colorScheme';
 import ReportModal from 'src/features/reports/ReportModal';
 import { deleteActiveReport } from 'src/features/reports/ReportModal/actions';
+import { useUnsavedChangesPrompt } from 'src/hooks/useUnsavedChangesPrompt';
+import { getChartFormDiffs } from 'src/utils/getChartFormDiffs';
 import { useExploreAdditionalActionsMenu } from '../useExploreAdditionalActionsMenu';
 import { useExploreMetadataBar } from './useExploreMetadataBar';
 
@@ -50,6 +57,7 @@ const propTypes = {
   timeout: PropTypes.number,
   chart: chartPropShape,
   saveDisabled: PropTypes.bool,
+  isSaveModalVisible: PropTypes.bool,
 };
 
 const saveButtonStyles = theme => css`
@@ -83,13 +91,16 @@ export const ExploreChartHeader = ({
   sliceName,
   saveDisabled,
   metadata,
+  isSaveModalVisible,
 }) => {
   const dispatch = useDispatch();
   const { latestQueryFormData, sliceFormData } = chart;
   const [isPropertiesModalOpen, setIsPropertiesModalOpen] = useState(false);
   const [isReportModalOpen, setIsReportModalOpen] = useState(false);
   const [currentReportDeleting, setCurrentReportDeleting] = useState(null);
-  const updateCategoricalNamespace = async () => {
+  const [shouldForceCloseModal, setShouldForceCloseModal] = useState(false);
+
+  const updateCategoricalNamespace = useCallback(async () => {
     const { dashboards } = metadata || {};
     const dashboard =
       dashboardId && dashboards && dashboards.find(d => d.id === dashboardId);
@@ -117,11 +128,11 @@ export const ExploreChartHeader = ({
         logging.info(t('Unable to retrieve dashboard colors'));
       }
     }
-  };
+  }, [dashboardColorScheme, dashboardId, metadata]);
 
   useEffect(() => {
     updateCategoricalNamespace();
-  }, []);
+  }, [updateCategoricalNamespace]);
 
   const openPropertiesModal = () => {
     setIsPropertiesModalOpen(true);
@@ -139,10 +150,6 @@ export const ExploreChartHeader = ({
     setIsReportModalOpen(false);
   };
 
-  const showModal = useCallback(() => {
-    dispatch(setSaveChartModalVisibility(true));
-  }, [dispatch]);
-
   const updateSlice = useCallback(
     slice => {
       dispatch(sliceUpdated(slice));
@@ -179,8 +186,53 @@ export const ExploreChartHeader = ({
     );
 
   const metadataBar = useExploreMetadataBar(metadata, slice);
-
   const oldSliceName = slice?.slice_name;
+
+  const originalFormData = useMemo(() => {
+    if (!sliceFormData) return {};
+    return {
+      ...sliceFormData,
+      chartTitle: oldSliceName,
+    };
+  }, [sliceFormData, oldSliceName]);
+
+  const currentFormData = useMemo(
+    () => ({ ...formData, chartTitle: sliceName }),
+    [formData, sliceName],
+  );
+
+  const formDiffs = useMemo(
+    () => getChartFormDiffs(originalFormData, currentFormData),
+    [originalFormData, currentFormData],
+  );
+
+  const {
+    showModal: showUnsavedChangesModal,
+    setShowModal: setShowUnsavedChangesModal,
+    handleConfirmNavigation,
+    handleSaveAndCloseModal,
+    triggerManualSave,
+  } = useUnsavedChangesPrompt({
+    hasUnsavedChanges: Object.keys(formDiffs).length > 0,
+    onSave: () => dispatch(setSaveChartModalVisibility(true)),
+    isSaveModalVisible,
+    manualSaveOnUnsavedChanges: true,
+  });
+
+  const showModal = useCallback(() => {
+    triggerManualSave();
+  }, [triggerManualSave]);
+
+  useEffect(() => {
+    if (!isSaveModalVisible) setShouldForceCloseModal(true);
+  }, [isSaveModalVisible, setShowUnsavedChangesModal]);
+
+  useEffect(() => {
+    if (!showUnsavedChangesModal && shouldForceCloseModal) {
+      setShouldForceCloseModal(false);
+    }
+  }, [showUnsavedChangesModal, shouldForceCloseModal]);
+
   return (
     <>
       <PageHeaderWithActions
@@ -212,11 +264,9 @@ export const ExploreChartHeader = ({
             {sliceFormData ? (
               <AlteredSliceTag
                 className="altered"
-                origFormData={{
-                  ...sliceFormData,
-                  chartTitle: oldSliceName,
-                }}
-                currentFormData={{ ...formData, chartTitle: sliceName }}
+                diffs={formDiffs}
+                origFormData={originalFormData}
+                currentFormData={currentFormData}
               />
             ) : null}
             {metadataBar}
@@ -286,6 +336,15 @@ export const ExploreChartHeader = ({
           title={t('Delete Report?')}
         />
       )}
+
+      <UnsavedChangesModal
+        title={t('Save changes to your chart?')}
+        body={t("If you don't save, changes will be lost.")}
+        showModal={showUnsavedChangesModal}
+        onHide={() => setShowUnsavedChangesModal(false)}
+        onConfirmNavigation={handleConfirmNavigation}
+        handleSave={handleSaveAndCloseModal}
+      />
     </>
   );
 };