fix(dashboard): enable undo/redo buttons for layout changes

superset-frontend/src/dashboard/components/Header/Header.test.tsx

@@ -18,19 +18,17 @@
  */
 import * as redux from 'redux';
 import { useUnsavedChangesPrompt } from 'src/hooks/useUnsavedChangesPrompt';
-import {
-  render,
-  screen,
-  fireEvent,
-  userEvent,
-  within,
-} from 'spec/helpers/testing-library';
+import { screen, userEvent, within, waitFor } from '@superset-ui/core/spec';
+import { ActionCreators as UndoActionCreators } from 'redux-undo';
 import fetchMock from 'fetch-mock';
 import { getExtensionsRegistry, JsonObject } from '@superset-ui/core';
 import setupExtensions from 'src/setup/setupExtensions';
 import getOwnerName from 'src/utils/getOwnerName';
+import { render, createStore } from 'spec/helpers/testing-library';
+import reducerIndex from 'spec/helpers/reducerIndex';
 import Header from '.';
 import { DASHBOARD_HEADER_ID } from '../../util/constants';
+import { UPDATE_COMPONENTS } from '../../actions/dashboardLayout';
 
 const initialState = {
   dashboardInfo: {
@@ -116,15 +114,7 @@ const undoState = {
   ...editableState,
   dashboardLayout: {
     ...initialState.dashboardLayout,
-    past: [{}],
-  },
-};
-
-const redoState = {
-  ...editableState,
-  dashboardLayout: {
-    ...initialState.dashboardLayout,
-    future: [{}],
+    past: [initialState.dashboardLayout.present],
   },
 };
 
@@ -280,19 +270,15 @@ test('should render the "Undo" action as disabled', () => {
   expect(screen.getByTestId('undo-action').parentElement).toBeDisabled();
 });
 
-test('should undo', () => {
+test('should undo when past actions exist', () => {
   setup(undoState);
   const undo = screen.getByTestId('undo-action');
-  expect(onUndo).not.toHaveBeenCalled();
-  userEvent.click(undo);
-  expect(onUndo).toHaveBeenCalledTimes(1);
-});
+  const undoButton = undo.parentElement;
 
-test('should undo with key listener', () => {
-  onUndo.mockReset();
-  setup(undoState);
+  expect(undoButton).toBeEnabled();
   expect(onUndo).not.toHaveBeenCalled();
-  fireEvent.keyDown(document.body, { key: 'z', code: 'KeyZ', ctrlKey: true });
+
+  userEvent.click(undo);
   expect(onUndo).toHaveBeenCalledTimes(1);
 });
 
@@ -301,19 +287,157 @@ test('should render the "Redo" action as disabled', () => {
   expect(screen.getByTestId('redo-action').parentElement).toBeDisabled();
 });
 
-test('should redo', () => {
-  setup(redoState);
+test('should have correct redo button structure', () => {
+  setup(editableState);
+
   const redo = screen.getByTestId('redo-action');
+  const redoButton = redo.parentElement;
+
+  expect(redoButton).toBeInTheDocument();
+  expect(redo).toBeInTheDocument();
+  expect(redoButton).toBeDisabled();
+});
+
+test('should enable undo button when past actions exist', () => {
+  setup(undoState);
+
+  const undoButton = screen.getByTestId('undo-action').parentElement;
+  const redoButton = screen.getByTestId('redo-action').parentElement;
+
+  expect(undoButton).toBeEnabled();
+  expect(redoButton).toBeDisabled();
+  expect(onUndo).not.toHaveBeenCalled();
+
+  userEvent.click(screen.getByTestId('undo-action'));
+  expect(onUndo).toHaveBeenCalledTimes(1);
+});
+
+test('should enable redo button after undo creates future history', async () => {
+  const testStore = createStore(
+    {
+      ...initialState,
+      ...editableState,
+      dashboardLayout: {
+        present: {
+          [DASHBOARD_HEADER_ID]: {
+            meta: { text: 'Original Title' },
+          },
+        },
+        past: [],
+        future: [],
+      },
+    },
+    reducerIndex,
+  );
+
+  render(
+    <div className="dashboard">
+      <Header />
+    </div>,
+    {
+      useRedux: true,
+      useTheme: true,
+      store: testStore,
+    },
+  );
+
+  testStore.dispatch({
+    type: UPDATE_COMPONENTS,
+    payload: {
+      nextComponents: {
+        [DASHBOARD_HEADER_ID]: {
+          meta: { text: 'Updated Title' },
+        },
+      },
+    },
+  });
+
+  await waitFor(() => {
+    expect(screen.getByTestId('undo-action').parentElement).toBeEnabled();
+  });
+
+  testStore.dispatch(UndoActionCreators.undo());
+
+  await waitFor(() => {
+    const redoButton = screen.getByTestId('redo-action').parentElement;
+    expect(redoButton).toBeEnabled();
+  });
+
   expect(onRedo).not.toHaveBeenCalled();
-  userEvent.click(redo);
+
+  userEvent.click(screen.getByTestId('redo-action'));
   expect(onRedo).toHaveBeenCalledTimes(1);
 });
 
-test('should redo with key listener', () => {
-  setup(redoState);
+test('should enable undo button when real actions create past history', async () => {
+  const testStore = createStore(
+    {
+      ...initialState,
+      ...editableState,
+      dashboardLayout: {
+        present: {
+          [DASHBOARD_HEADER_ID]: {
+            meta: { text: 'Original Title' },
+          },
+        },
+        past: [],
+        future: [],
+      },
+    },
+    reducerIndex,
+  );
+
+  render(
+    <div className="dashboard">
+      <Header />
+    </div>,
+    {
+      useRedux: true,
+      useTheme: true,
+      store: testStore,
+    },
+  );
+
+  const undoButton = screen.getByTestId('undo-action').parentElement;
+  expect(undoButton).toBeDisabled();
+
+  testStore.dispatch({
+    type: UPDATE_COMPONENTS,
+    payload: {
+      nextComponents: {
+        [DASHBOARD_HEADER_ID]: {
+          meta: { text: 'Updated Title' },
+        },
+      },
+    },
+  });
+
+  await waitFor(() => {
+    expect(screen.getByTestId('undo-action').parentElement).toBeEnabled();
+  });
+
+  expect(onUndo).not.toHaveBeenCalled();
+
+  userEvent.click(screen.getByTestId('undo-action'));
+  expect(onUndo).toHaveBeenCalledTimes(1);
+});
+
+test('should disable both buttons when no actions available', () => {
+  setup(editableState);
+
+  const undoButton = screen.getByTestId('undo-action').parentElement;
+  const redoButton = screen.getByTestId('redo-action').parentElement;
+
+  expect(undoButton).toBeDisabled();
+  expect(redoButton).toBeDisabled();
+  expect(onUndo).not.toHaveBeenCalled();
+  expect(onRedo).not.toHaveBeenCalled();
+
+  userEvent.click(screen.getByTestId('undo-action'));
+  userEvent.click(screen.getByTestId('redo-action'));
+
+  expect(onUndo).not.toHaveBeenCalled();
   expect(onRedo).not.toHaveBeenCalled();
-  fireEvent.keyDown(document.body, { key: 'y', code: 'KeyY', ctrlKey: true });
-  expect(onRedo).toHaveBeenCalledTimes(1);
 });
 
 test('should render the "Discard changes" button', () => {
@@ -475,7 +599,7 @@ test('should hide edit button and navbar, and show Exit fullscreen when in fulls
 test('should show Exit fullscreen when in fullscreen mode', async () => {
   setup();
 
-  fireEvent.click(screen.getByTestId('actions-trigger'));
+  userEvent.click(screen.getByTestId('actions-trigger'));
 
   expect(await screen.findByText('Exit fullscreen')).toBeInTheDocument();
 });

superset-frontend/src/dashboard/components/Header/index.jsx

@@ -533,9 +533,13 @@ const Header = () => {
         tags: updates.tags,
       });
       boundActionCreators.setUnsavedChanges(true);
-      boundActionCreators.dashboardTitleChanged(updates.title);
+
+      if (updates.title && dashboardTitle !== updates.title) {
+        boundActionCreators.updateDashboardTitle(updates.title);
+        boundActionCreators.onChange();
+      }
     },
-    [boundActionCreators],
+    [boundActionCreators, dashboardTitle],
   );
 
   const NavExtension = extensionsRegistry.get('dashboard.nav.right');
@@ -617,7 +621,9 @@ const Header = () => {
                     <StyledUndoRedoButton
                       buttonStyle="link"
                       disabled={undoLength < 1}
-                      onClick={undoLength && boundActionCreators.onUndo}
+                      onClick={
+                        undoLength > 0 ? boundActionCreators.onUndo : undefined
+                      }
                     >
                       <Icons.Undo
                         css={[
@@ -637,7 +643,9 @@ const Header = () => {
                     <StyledUndoRedoButton
                       buttonStyle="link"
                       disabled={redoLength < 1}
-                      onClick={redoLength && boundActionCreators.onRedo}
+                      onClick={
+                        redoLength > 0 ? boundActionCreators.onRedo : undefined
+                      }
                     >
                       <Icons.Redo
                         css={[

superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx

@@ -80,6 +80,11 @@ const defaultState = {
     superset_can_csv: false,
     common: { conf: { SUPERSET_WEBSERVER_TIMEOUT: 0, SQL_MAX_ROW: 666 } },
   },
+  dashboardLayout: {
+    present: {},
+    past: [],
+    future: [],
+  },
 };
 
 function setup(overrideProps, overrideState) {

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/useCrossFiltersScopingModal.test.ts

@@ -24,7 +24,15 @@ import { useCrossFiltersScopingModal } from './useCrossFiltersScopingModal';
 
 test('Renders modal after calling method open', async () => {
   const { result } = renderHook(() => useCrossFiltersScopingModal(), {
-    wrapper: createWrapper(),
+    wrapper: createWrapper({
+      initialState: {
+        dashboardLayout: {
+          present: {},
+          past: [],
+          future: [],
+        },
+      },
+    }),
   });
 
   const [openModal, Modal] = result.current;
@@ -34,6 +42,13 @@ test('Renders modal after calling method open', async () => {
 
   const { getByText } = render(result.current[1] as ReactElement, {
     useRedux: true,
+    initialState: {
+      dashboardLayout: {
+        present: {},
+        past: [],
+        future: [],
+      },
+    },
   });
 
   expect(getByText('Cross-filtering scoping')).toBeInTheDocument();

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/HorizontalFilterBar.test.tsx

@@ -41,6 +41,11 @@ const renderWrapper = (overrideProps?: Record<string, any>) =>
         dashboardInfo: {
           dash_edit_perm: true,
         },
+        dashboardLayout: {
+          present: {},
+          past: [],
+          future: [],
+        },
       },
     }),
   );

superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx

@@ -58,6 +58,11 @@ class MainPreset extends Preset {
 const defaultState = () => ({
   datasources: { ...mockDatasource },
   charts: chartQueries,
+  dashboardLayout: {
+    present: {},
+    past: [],
+    future: [],
+  },
 });
 
 const noTemporalColumnsState = () => {

superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/NativeFiltersModal.test.tsx

@@ -59,6 +59,13 @@ function setup(overridesProps?: any) {
   return render(<FiltersConfigModal {...mockedProps} {...overridesProps} />, {
     useDnd: true,
     useRedux: true,
+    initialState: {
+      dashboardLayout: {
+        present: {},
+        past: [],
+        future: [],
+      },
+    },
   });
 }