From 467b008f36f10ac0b49593a739534525d1bc0e95 Mon Sep 17 00:00:00 2001
From: Richard Fogaca Nienkotter
 <63572350+richardfogaca@users.noreply.github.com>
Date: Thu, 13 Nov 2025 18:01:33 -0300
Subject: [PATCH] fix: save button was enabled even no changes were made to the
 dashboard (#35817)

---
 .../components/Header/Header.test.tsx         | 131 ++++++++++++++++++
 .../src/dashboard/components/Header/index.jsx |  18 ++-
 2 files changed, 143 insertions(+), 6 deletions(-)

diff --git a/superset-frontend/src/dashboard/components/Header/Header.test.tsx b/superset-frontend/src/dashboard/components/Header/Header.test.tsx
index ddf13c25ecf..7dcc0a5953a 100644
--- a/superset-frontend/src/dashboard/components/Header/Header.test.tsx
+++ b/superset-frontend/src/dashboard/components/Header/Header.test.tsx
@@ -711,3 +711,134 @@ test('should call setShowUnsavedChangesModal(false) on cancel', async () => {
 
   expect(setShowModal).toHaveBeenCalledWith(false);
 });
+
+test('should clear history and unsaved changes when entering edit mode', () => {
+  const clearDashboardHistory = jest.fn();
+
+  jest.spyOn(redux, 'bindActionCreators').mockImplementation(() => ({
+    addSuccessToast,
+    addDangerToast,
+    addWarningToast,
+    onUndo,
+    onRedo,
+    setEditMode,
+    setUnsavedChanges,
+    fetchFaveStar,
+    saveFaveStar,
+    savePublished,
+    fetchCharts,
+    updateDashboardTitle,
+    updateCss,
+    onChange,
+    onSave,
+    setMaxUndoHistoryExceeded,
+    maxUndoHistoryToast,
+    logEvent,
+    setRefreshFrequency,
+    onRefresh,
+    dashboardInfoChanged,
+    dashboardTitleChanged,
+    clearDashboardHistory,
+  }));
+
+  const canEditState = {
+    dashboardInfo: {
+      ...initialState.dashboardInfo,
+      dash_edit_perm: true,
+    },
+  };
+
+  setup(canEditState);
+
+  const editButton = screen.getByText('Edit dashboard');
+  userEvent.click(editButton);
+
+  expect(clearDashboardHistory).toHaveBeenCalledTimes(1);
+  expect(setUnsavedChanges).toHaveBeenCalledWith(false);
+});
+
+test('should mark theme change as unsaved when in edit mode', async () => {
+  const testStore = createStore(
+    {
+      ...initialState,
+      ...editableState,
+      dashboardInfo: {
+        ...editableState.dashboardInfo,
+        theme: 'LIGHT',
+      },
+    },
+    reducerIndex,
+  );
+
+  render(
+    <div className="dashboard">
+      <Header />
+    </div>,
+    {
+      useRedux: true,
+      useTheme: true,
+      store: testStore,
+    },
+  );
+
+  expect(setUnsavedChanges).not.toHaveBeenCalledWith(true);
+
+  testStore.dispatch({
+    type: 'DASHBOARD_INFO_UPDATED',
+    newInfo: {
+      theme: 'DARK',
+    },
+  });
+
+  await waitFor(() => {
+    expect(setUnsavedChanges).toHaveBeenCalledWith(true);
+  });
+});
+
+test('should not mark initial theme as unsaved change', () => {
+  setup({
+    ...editableState,
+    dashboardInfo: {
+      ...editableState.dashboardInfo,
+      theme: 'LIGHT',
+    },
+  });
+
+  expect(setUnsavedChanges).not.toHaveBeenCalledWith(true);
+});
+
+test('should sync theme ref when navigating between dashboards', async () => {
+  const testStore = createStore(
+    {
+      ...initialState,
+      dashboardInfo: {
+        ...initialState.dashboardInfo,
+        theme: 'LIGHT',
+      },
+    },
+    reducerIndex,
+  );
+
+  render(
+    <div className="dashboard">
+      <Header />
+    </div>,
+    {
+      useRedux: true,
+      useTheme: true,
+      store: testStore,
+    },
+  );
+
+  testStore.dispatch({
+    type: 'DASHBOARD_INFO_UPDATED',
+    newInfo: {
+      id: 2,
+      theme: 'DARK',
+    },
+  });
+
+  await waitFor(() => {
+    expect(setUnsavedChanges).toHaveBeenCalledTimes(0);
+  });
+});
diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx
index 17f37440b5d..16bec548f9e 100644
--- a/superset-frontend/src/dashboard/components/Header/index.jsx
+++ b/superset-frontend/src/dashboard/components/Header/index.jsx
@@ -218,6 +218,7 @@ const Header = () => {
   const refreshTimer = useRef(0);
   const ctrlYTimeout = useRef(0);
   const ctrlZTimeout = useRef(0);
+  const previousThemeRef = useRef(dashboardInfo.theme);
 
   const dashboardTitle = layout[DASHBOARD_HEADER_ID]?.meta?.text;
   const { slug } = dashboardInfo;
@@ -324,11 +325,12 @@ const Header = () => {
     startPeriodicRender(refreshFrequency * 1000);
   }, [refreshFrequency, startPeriodicRender]);
 
-  // Ensure theme changes are tracked as unsaved changes
+  // Track theme changes as unsaved changes, and sync ref when navigating between dashboards
   useEffect(() => {
-    if (editMode && dashboardInfo.theme !== undefined) {
+    if (editMode && dashboardInfo.theme !== previousThemeRef.current) {
       boundActionCreators.setUnsavedChanges(true);
     }
+    previousThemeRef.current = dashboardInfo.theme;
   }, [dashboardInfo.theme, editMode, boundActionCreators]);
 
   useEffect(() => {
@@ -559,6 +561,12 @@ const Header = () => {
     [boundActionCreators],
   );
 
+  const handleEnterEditMode = useCallback(() => {
+    toggleEditMode();
+    boundActionCreators.clearDashboardHistory?.();
+    boundActionCreators.setUnsavedChanges(false);
+  }, [toggleEditMode, boundActionCreators]);
+
   const NavExtension = extensionsRegistry.get('dashboard.nav.right');
 
   const editableTitleProps = useMemo(
@@ -710,10 +718,7 @@ const Header = () => {
             {userCanEdit && (
               <Button
                 buttonStyle="secondary"
-                onClick={() => {
-                  toggleEditMode();
-                  boundActionCreators.clearDashboardHistory?.(); // Resets the `past` as an empty array
-                }}
+                onClick={handleEnterEditMode}
                 data-test="edit-dashboard-button"
                 className="action-button"
                 css={editButtonStyle}
@@ -736,6 +741,7 @@ const Header = () => {
       emphasizeUndo,
       handleCtrlY,
       handleCtrlZ,
+      handleEnterEditMode,
       hasUnsavedChanges,
       overwriteDashboard,
       redoLength,