Dynamic dashboards: Undo redo for remove row, add tab, remove tab (#106614)

* Undo redo for remove row, add tab, remove tab * Undoing remove tab and row now adds them back in the correct place * add tests * fix lint issue
2025-06-16 14:20:45 +02:00
parent 5d54289509
commit 61430f027f
4 changed files with 315 additions and 19 deletions
@@ -0,0 +1,128 @@
+import { dashboardEditActions } from '../../edit-pane/shared';
+
+import { RowItem } from './RowItem';
+import { RowsLayoutManager } from './RowsLayoutManager';
+
+let lastUndo: (() => void) | undefined;
+let ungroupLayoutCalled = false;
+
+jest.mock('../../edit-pane/shared', () => ({
+  dashboardEditActions: {
+    addElement: jest.fn(({ perform, undo }) => {
+      perform();
+      lastUndo = undo;
+    }),
+    removeElement: jest.fn(({ perform, undo }) => {
+      perform();
+      lastUndo = undo;
+    }),
+  },
+}));
+
+jest.mock('../layouts-shared/utils', () => ({
+  ...jest.requireActual('../layouts-shared/utils'),
+  ungroupLayout: jest.fn(() => {
+    ungroupLayoutCalled = true;
+  }),
+}));
+
+describe('RowsLayoutManager', () => {
+  describe('addNewRow', () => {
+    beforeEach(() => {
+      lastUndo = undefined;
+    });
+
+    it('should add a new row with default title when no title is provided', () => {
+      const manager = new RowsLayoutManager({ rows: [] });
+      const newRow = manager.addNewRow();
+
+      expect(newRow).toBeInstanceOf(RowItem);
+      expect(newRow.state.title).toBe('New row');
+      expect(manager.state.rows).toHaveLength(1);
+      expect(manager.state.rows[0]).toBe(newRow);
+    });
+
+    it('should add a row with the provided title if it is unique', () => {
+      const manager = new RowsLayoutManager({ rows: [] });
+      const newRow = manager.addNewRow(new RowItem({ title: 'Unique Title' }));
+
+      expect(newRow.state.title).toBe('Unique Title');
+      expect(manager.state.rows).toHaveLength(1);
+      expect(manager.state.rows[0]).toBe(newRow);
+    });
+
+    it('should generate a unique title when adding a row with a duplicate title', () => {
+      const manager = new RowsLayoutManager({ rows: [] });
+      const firstRow = manager.addNewRow(new RowItem({ title: 'Test Title' }));
+      const secondRow = manager.addNewRow(new RowItem({ title: 'Test Title' }));
+
+      expect(firstRow.state.title).toBe('Test Title');
+      expect(secondRow.state.title).toBe('Test Title 1');
+      expect(manager.state.rows).toHaveLength(2);
+    });
+
+    it('should increment the number in the title for multiple duplicates', () => {
+      const manager = new RowsLayoutManager({ rows: [] });
+      const firstRow = manager.addNewRow(new RowItem({ title: 'Test Title' }));
+      const secondRow = manager.addNewRow(new RowItem({ title: 'Test Title' }));
+      const thirdRow = manager.addNewRow(new RowItem({ title: 'Test Title' }));
+
+      expect(firstRow.state.title).toBe('Test Title');
+      expect(secondRow.state.title).toBe('Test Title 1');
+      expect(thirdRow.state.title).toBe('Test Title 2');
+      expect(manager.state.rows).toHaveLength(3);
+    });
+
+    it('should handle undo action correctly', () => {
+      const manager = new RowsLayoutManager({ rows: [] });
+      manager.addNewRow(new RowItem({ title: 'Test Title' }));
+
+      expect(manager.state.rows).toHaveLength(1);
+
+      // Use the real undo function from the mock
+      expect(typeof lastUndo).toBe('function');
+      lastUndo && lastUndo();
+
+      expect(manager.state.rows).toHaveLength(0);
+    });
+  });
+
+  describe('removeRow', () => {
+    beforeEach(() => {
+      lastUndo = undefined;
+      ungroupLayoutCalled = false;
+      jest.clearAllMocks();
+    });
+
+    it('should remove a row and call removeElement', () => {
+      const manager = new RowsLayoutManager({ rows: [] });
+      const row1 = manager.addNewRow(new RowItem({ title: 'Row 1' }));
+      const row2 = manager.addNewRow(new RowItem({ title: 'Row 2' }));
+      expect(manager.state.rows).toHaveLength(2);
+      manager.removeRow(row1);
+      expect(manager.state.rows).toHaveLength(1);
+      expect(manager.state.rows[0]).toBe(row2);
+      expect(dashboardEditActions.removeElement).toHaveBeenCalled();
+    });
+
+    it('should handle undo action correctly', () => {
+      const manager = new RowsLayoutManager({ rows: [] });
+      const row1 = manager.addNewRow(new RowItem({ title: 'Row 1' }));
+      const row2 = manager.addNewRow(new RowItem({ title: 'Row 2' }));
+      manager.removeRow(row1);
+      expect(manager.state.rows).toHaveLength(1);
+      lastUndo && lastUndo();
+      expect(manager.state.rows).toHaveLength(2);
+      expect(manager.state.rows).toContain(row1);
+      expect(manager.state.rows).toContain(row2);
+    });
+
+    it('should call ungroupLayout when removing the last row', () => {
+      const manager = new RowsLayoutManager({ rows: [] });
+      const row = manager.addNewRow(new RowItem({ title: 'Only Row' }));
+      expect(manager.state.rows).toHaveLength(1);
+      manager.removeRow(row);
+      expect(ungroupLayoutCalled).toBe(true);
+    });
+  });
+});
@@ -9,11 +9,7 @@ import {
 } from '@grafana/scenes';
 import { Spec as DashboardV2Spec } from '@grafana/schema/dist/esm/schema/dashboard/v2alpha1/types.spec.gen';

-import {
-  dashboardEditActions,
-  ObjectRemovedFromCanvasEvent,
-  ObjectsReorderedOnCanvasEvent,
-} from '../../edit-pane/shared';
+import { dashboardEditActions, ObjectsReorderedOnCanvasEvent } from '../../edit-pane/shared';
 import { serializeRowsLayout } from '../../serialization/layoutSerializers/RowsLayoutSerializer';
 import { isClonedKey, joinCloneKeys } from '../../utils/clone';
 import { getDashboardSceneFor } from '../../utils/utils';
@@ -151,9 +147,18 @@ export class RowsLayoutManager extends SceneObjectBase<RowsLayoutManagerState> i
      return;
    }

-    const rows = this.state.rows.filter((r) => r !== row);
-    this.setState({ rows });
-    this.publishEvent(new ObjectRemovedFromCanvasEvent(row), true);
+    const indexOfRowToRemove = this.state.rows.findIndex((r) => r === row);
+
+    dashboardEditActions.removeElement({
+      removedObject: row,
+      source: this,
+      perform: () => this.setState({ rows: this.state.rows.filter((r) => r !== row) }),
+      undo: () => {
+        const rows = [...this.state.rows];
+        rows.splice(indexOfRowToRemove, 0, row);
+        this.setState({ rows });
+      },
+    });
  }

  public moveRow(_rowKey: string, fromIndex: number, toIndex: number) {
@@ -1,9 +1,25 @@
+import { dashboardEditActions } from '../../edit-pane/shared';
 import { RowItem } from '../layout-rows/RowItem';
 import { RowsLayoutManager } from '../layout-rows/RowsLayoutManager';

 import { TabItem } from './TabItem';
 import { TabsLayoutManager } from './TabsLayoutManager';

+let lastUndo: (() => void) | undefined;
+
+jest.mock('../../edit-pane/shared', () => ({
+  dashboardEditActions: {
+    addElement: jest.fn(({ perform, undo }) => {
+      perform();
+      lastUndo = undo;
+    }),
+    removeElement: jest.fn(({ perform, undo }) => {
+      perform();
+      lastUndo = undo;
+    }),
+  },
+}));
+
 describe('TabsLayoutManager', () => {
  describe('url sync', () => {
    it('when on top level', () => {
@@ -42,4 +58,125 @@ describe('TabsLayoutManager', () => {
      });
    });
  });
+
+  describe('addNewTab', () => {
+    beforeEach(() => {
+      lastUndo = undefined;
+    });
+
+    it('should add a new tab with default title when no title is provided', () => {
+      const manager = new TabsLayoutManager({ tabs: [] });
+      const newTab = manager.addNewTab();
+
+      expect(newTab).toBeInstanceOf(TabItem);
+      expect(newTab.state.title).toBe('New tab');
+      expect(manager.state.tabs).toHaveLength(1);
+      expect(manager.state.tabs[0]).toBe(newTab);
+    });
+
+    it('should add a tab with the provided title if it is unique', () => {
+      const manager = new TabsLayoutManager({ tabs: [] });
+      const newTab = manager.addNewTab(new TabItem({ title: 'Unique Title' }));
+
+      expect(newTab.state.title).toBe('Unique Title');
+      expect(manager.state.tabs).toHaveLength(1);
+      expect(manager.state.tabs[0]).toBe(newTab);
+    });
+
+    it('should generate a unique title when adding a tab with a duplicate title', () => {
+      const manager = new TabsLayoutManager({ tabs: [] });
+      const firstTab = manager.addNewTab(new TabItem({ title: 'Test Title' }));
+      const secondTab = manager.addNewTab(new TabItem({ title: 'Test Title' }));
+
+      expect(firstTab.state.title).toBe('Test Title');
+      expect(secondTab.state.title).toBe('Test Title 1');
+      expect(manager.state.tabs).toHaveLength(2);
+    });
+
+    it('should increment the number in the title for multiple duplicates', () => {
+      const manager = new TabsLayoutManager({ tabs: [] });
+      const firstTab = manager.addNewTab(new TabItem({ title: 'Test Title' }));
+      const secondTab = manager.addNewTab(new TabItem({ title: 'Test Title' }));
+      const thirdTab = manager.addNewTab(new TabItem({ title: 'Test Title' }));
+
+      expect(firstTab.state.title).toBe('Test Title');
+      expect(secondTab.state.title).toBe('Test Title 1');
+      expect(thirdTab.state.title).toBe('Test Title 2');
+      expect(manager.state.tabs).toHaveLength(3);
+    });
+
+    it('should handle undo action correctly', () => {
+      const manager = new TabsLayoutManager({ tabs: [] });
+      manager.addNewTab(new TabItem({ title: 'Test Title' }));
+
+      expect(manager.state.tabs).toHaveLength(1);
+
+      // Use the real undo function from the mock
+      expect(typeof lastUndo).toBe('function');
+      lastUndo && lastUndo();
+
+      expect(manager.state.tabs).toHaveLength(0);
+    });
+  });
+
+  describe('removeTab', () => {
+    beforeEach(() => {
+      lastUndo = undefined;
+    });
+
+    it('should remove a non-current tab without using removeElement', () => {
+      const manager = new TabsLayoutManager({ tabs: [] });
+      const tab1 = manager.addNewTab(new TabItem({ title: 'Tab 1' }));
+      const tab2 = manager.addNewTab(new TabItem({ title: 'Tab 2' }));
+
+      expect(manager.state.tabs).toHaveLength(2);
+      expect(manager.state.currentTabIndex).toBe(1); // tab2 is current
+
+      manager.removeTab(tab1);
+
+      expect(manager.state.tabs).toHaveLength(1);
+      expect(manager.state.tabs[0]).toBe(tab2);
+      expect(manager.state.currentTabIndex).toBe(0);
+      expect(dashboardEditActions.removeElement).not.toHaveBeenCalled();
+    });
+
+    it('should remove the current tab using removeElement', () => {
+      const manager = new TabsLayoutManager({ tabs: [] });
+      const tab1 = manager.addNewTab(new TabItem({ title: 'Tab 1' }));
+      const tab2 = manager.addNewTab(new TabItem({ title: 'Tab 2' }));
+
+      expect(manager.state.tabs).toHaveLength(2);
+      expect(manager.state.currentTabIndex).toBe(1); // tab2 is current
+
+      manager.removeTab(tab2);
+
+      expect(manager.state.tabs).toHaveLength(1);
+      expect(manager.state.tabs[0]).toBe(tab1);
+      expect(manager.state.currentTabIndex).toBe(0);
+      expect(dashboardEditActions.removeElement).toHaveBeenCalled();
+    });
+
+    it('should handle undo action correctly when removing the current tab', () => {
+      const manager = new TabsLayoutManager({ tabs: [] });
+      const tab1 = manager.addNewTab(new TabItem({ title: 'Tab 1' }));
+      const tab2 = manager.addNewTab(new TabItem({ title: 'Tab 2' }));
+
+      expect(manager.state.tabs).toHaveLength(2);
+      expect(manager.state.currentTabIndex).toBe(1); // tab2 is current
+
+      manager.removeTab(tab2);
+
+      expect(manager.state.tabs).toHaveLength(1);
+      expect(manager.state.tabs[0]).toBe(tab1);
+
+      // Use the real undo function from the mock
+      expect(typeof lastUndo).toBe('function');
+      lastUndo && lastUndo();
+
+      expect(manager.state.tabs).toHaveLength(2);
+      expect(manager.state.tabs).toContain(tab1);
+      expect(manager.state.tabs).toContain(tab2);
+      expect(manager.state.currentTabIndex).toBe(1); // tab2 should be current again
+    });
+  });
 });
@@ -9,11 +9,7 @@ import {
 } from '@grafana/scenes';
 import { Spec as DashboardV2Spec } from '@grafana/schema/dist/esm/schema/dashboard/v2alpha1/types.spec.gen';

-import {
-  NewObjectAddedToCanvasEvent,
-  ObjectRemovedFromCanvasEvent,
-  ObjectsReorderedOnCanvasEvent,
-} from '../../edit-pane/shared';
+import { dashboardEditActions, ObjectsReorderedOnCanvasEvent } from '../../edit-pane/shared';
 import { serializeTabsLayout } from '../../serialization/layoutSerializers/TabsLayoutSerializer';
 import { isClonedKey, joinCloneKeys } from '../../utils/clone';
 import { getDashboardSceneFor } from '../../utils/utils';
@@ -146,8 +142,23 @@ export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> i
      newTab.setState({ title: newTitle });
    }

-    this.setState({ tabs: [...this.state.tabs, newTab], currentTabIndex: this.state.tabs.length });
-    this.publishEvent(new NewObjectAddedToCanvasEvent(newTab), true);
+    dashboardEditActions.addElement({
+      addedObject: newTab,
+      source: this,
+      perform: () => this.setState({ tabs: [...this.state.tabs, newTab], currentTabIndex: this.state.tabs.length }),
+      undo: () => {
+        const indexOfNewTab = this.state.tabs.findIndex((t) => t === newTab);
+        this.setState({
+          tabs: this.state.tabs.filter((t) => t !== newTab),
+          // if the new tab was the current tab, set the current tab to the previous tab
+          currentTabIndex:
+            this.state.currentTabIndex === indexOfNewTab
+              ? Math.max(0, this.state.currentTabIndex - 1)
+              : this.state.currentTabIndex,
+        });
+      },
+    });
+
    return newTab;
  }

@@ -191,9 +202,25 @@ export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> i
    const currentTab = this.getCurrentTab();

    if (currentTab === tabToRemove) {
-      const nextTabIndex = this.state.currentTabIndex > 0 ? this.state.currentTabIndex - 1 : 0;
-      this.setState({ tabs: this.state.tabs.filter((t) => t !== tabToRemove), currentTabIndex: nextTabIndex });
-      this.publishEvent(new ObjectRemovedFromCanvasEvent(tabToRemove), true);
+      const currentTabIndex = this.state.currentTabIndex;
+      const indexOfTabToRemove = this.state.tabs.findIndex((t) => t === tabToRemove);
+      const nextTabIndex = currentTabIndex > 0 ? currentTabIndex - 1 : 0;
+
+      dashboardEditActions.removeElement({
+        removedObject: tabToRemove,
+        source: this,
+        perform: () =>
+          this.setState({
+            tabs: this.state.tabs.filter((t) => t !== tabToRemove),
+            currentTabIndex: currentTabIndex === indexOfTabToRemove ? nextTabIndex : currentTabIndex,
+          }),
+        undo: () => {
+          const tabs = [...this.state.tabs];
+          tabs.splice(indexOfTabToRemove, 0, tabToRemove);
+          this.setState({ tabs, currentTabIndex });
+        },
+      });
+
      return;
    }

@@ -201,7 +228,6 @@ export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> i
    const tabs = filteredTab.length === 0 ? [new TabItem()] : filteredTab;

    this.setState({ tabs, currentTabIndex: 0 });
-    this.publishEvent(new ObjectRemovedFromCanvasEvent(tabToRemove), true);
  }

  public moveTab(_tabKey: string, fromIndex: number, toIndex: number) {