fix(dashboard): invalid active tab state (#33106)

superset-frontend/src/dashboard/actions/dashboardState.js

@@ -657,8 +657,61 @@ export function setDirectPathToChild(path) {
 }
 
 export const SET_ACTIVE_TAB = 'SET_ACTIVE_TAB';
+
+function findTabsToRestore(tabId, prevTabId, dashboardState, dashboardLayout) {
+  const { activeTabs: prevActiveTabs, inactiveTabs: prevInactiveTabs } =
+    dashboardState;
+  const { present: currentLayout } = dashboardLayout;
+  const restoredTabs = [];
+  const queue = [tabId];
+  const visited = new Set();
+  while (queue.length > 0) {
+    const seek = queue.shift();
+    if (!visited.has(seek)) {
+      visited.add(seek);
+      const found =
+        prevInactiveTabs?.filter(inactiveTabId =>
+          currentLayout[inactiveTabId]?.parents
+            .filter(id => id.startsWith('TAB-'))
+            .slice(-1)
+            .includes(seek),
+        ) ?? [];
+      restoredTabs.push(...found);
+      queue.push(...found);
+    }
+  }
+  const activeTabs = restoredTabs ? [tabId].concat(restoredTabs) : [tabId];
+  const tabChanged = Boolean(prevTabId) && tabId !== prevTabId;
+  const inactiveTabs = tabChanged
+    ? prevActiveTabs.filter(
+        activeTabId =>
+          activeTabId !== prevTabId &&
+          currentLayout[activeTabId]?.parents.includes(prevTabId),
+      )
+    : [];
+  return {
+    activeTabs,
+    inactiveTabs,
+  };
+}
+
 export function setActiveTab(tabId, prevTabId) {
-  return { type: SET_ACTIVE_TAB, tabId, prevTabId };
+  return (dispatch, getState) => {
+    const { dashboardLayout, dashboardState } = getState();
+    const { activeTabs, inactiveTabs } = findTabsToRestore(
+      tabId,
+      prevTabId,
+      dashboardState,
+      dashboardLayout,
+    );
+
+    return dispatch({
+      type: SET_ACTIVE_TAB,
+      activeTabs,
+      prevTabId,
+      inactiveTabs,
+    });
+  };
 }
 
 // Even though SET_ACTIVE_TABS is not being called from Superset's codebase,

superset-frontend/src/dashboard/reducers/dashboardState.js

@@ -209,12 +209,17 @@ export default function dashboardStateReducer(state = {}, action) {
       };
     },
     [SET_ACTIVE_TAB]() {
-      const newActiveTabs = new Set(state.activeTabs);
-      newActiveTabs.delete(action.prevTabId);
-      newActiveTabs.add(action.tabId);
+      const newActiveTabs = new Set(state.activeTabs).difference(
+        new Set(action.inactiveTabs.concat(action.prevTabId)),
+      );
+      const newInactiveTabs = new Set(state.inactiveTabs)
+        .difference(new Set(action.activeTabs))
+        .union(new Set(action.inactiveTabs));
+
       return {
         ...state,
-        activeTabs: Array.from(newActiveTabs),
+        inactiveTabs: Array.from(newInactiveTabs),
+        activeTabs: Array.from(newActiveTabs.union(new Set(action.activeTabs))),
       };
     },
     [SET_ACTIVE_TABS]() {

superset-frontend/src/dashboard/reducers/dashboardState.test.ts

@@ -16,24 +16,112 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-
+import configureMockStore from 'redux-mock-store';
+import thunk from 'redux-thunk';
 import dashboardStateReducer from './dashboardState';
 import { setActiveTab, setActiveTabs } from '../actions/dashboardState';
 
+const middlewares = [thunk];
+const mockStore = configureMockStore(middlewares);
+
 describe('DashboardState reducer', () => {
-  it('SET_ACTIVE_TAB', () => {
-    expect(
-      dashboardStateReducer({ activeTabs: [] }, setActiveTab('tab1')),
-    ).toEqual({ activeTabs: ['tab1'] });
-    expect(
-      dashboardStateReducer({ activeTabs: ['tab1'] }, setActiveTab('tab1')),
-    ).toEqual({ activeTabs: ['tab1'] });
-    expect(
-      dashboardStateReducer(
-        { activeTabs: ['tab1'] },
-        setActiveTab('tab2', 'tab1'),
-      ),
-    ).toEqual({ activeTabs: ['tab2'] });
+  describe('SET_ACTIVE_TAB', () => {
+    it('switches a single tab', () => {
+      const store = mockStore({
+        dashboardState: { activeTabs: [] },
+        dashboardLayout: { present: { tab1: { parents: [] } } },
+      });
+      const request = setActiveTab('tab1');
+      const thunkAction = request(store.dispatch, store.getState);
+
+      expect(dashboardStateReducer({ activeTabs: [] }, thunkAction)).toEqual({
+        activeTabs: ['tab1'],
+        inactiveTabs: [],
+      });
+
+      const request2 = setActiveTab('tab2', 'tab1');
+      const thunkAction2 = request2(store.dispatch, store.getState);
+      expect(
+        dashboardStateReducer({ activeTabs: ['tab1'] }, thunkAction2),
+      ).toEqual({ activeTabs: ['tab2'], inactiveTabs: [] });
+    });
+
+    it('switches a multi-depth tab', () => {
+      const initState = { activeTabs: ['TAB-1', 'TAB-A', 'TAB-__a'] };
+      const store = mockStore({
+        dashboardState: initState,
+        dashboardLayout: {
+          present: {
+            'TAB-1': { parents: [] },
+            'TAB-2': { parents: [] },
+            'TAB-A': { parents: ['TAB-1', 'TABS-1'] },
+            'TAB-B': { parents: ['TAB-1', 'TABS-1'] },
+            'TAB-__a': { parents: ['TAB-1', 'TABS-1', 'TAB-A', 'TABS-A'] },
+            'TAB-__b': { parents: ['TAB-1', 'TABS-1', 'TAB-B', 'TABS-B'] },
+          },
+        },
+      });
+      let request = setActiveTab('TAB-B', 'TAB-A');
+      let thunkAction = request(store.dispatch, store.getState);
+      let result = dashboardStateReducer(
+        { activeTabs: ['TAB-1', 'TAB-A', 'TAB-__a'] },
+        thunkAction,
+      );
+      expect(result).toEqual({
+        activeTabs: expect.arrayContaining(['TAB-1', 'TAB-B']),
+        inactiveTabs: ['TAB-__a'],
+      });
+      request = setActiveTab('TAB-2', 'TAB-1');
+      thunkAction = request(store.dispatch, () => ({
+        ...(store.getState() ?? {}),
+        dashboardState: result,
+      }));
+      result = dashboardStateReducer(result, thunkAction);
+      expect(result).toEqual({
+        activeTabs: ['TAB-2'],
+        inactiveTabs: expect.arrayContaining(['TAB-B', 'TAB-__a']),
+      });
+      request = setActiveTab('TAB-1', 'TAB-2');
+      thunkAction = request(store.dispatch, () => ({
+        ...(store.getState() ?? {}),
+        dashboardState: result,
+      }));
+      result = dashboardStateReducer(result, thunkAction);
+      expect(result).toEqual({
+        activeTabs: expect.arrayContaining(['TAB-1', 'TAB-B']),
+        inactiveTabs: ['TAB-__a'],
+      });
+      request = setActiveTab('TAB-A', 'TAB-B');
+      thunkAction = request(store.dispatch, () => ({
+        ...(store.getState() ?? {}),
+        dashboardState: result,
+      }));
+      result = dashboardStateReducer(result, thunkAction);
+      expect(result).toEqual({
+        activeTabs: expect.arrayContaining(['TAB-1', 'TAB-A', 'TAB-__a']),
+        inactiveTabs: [],
+      });
+      request = setActiveTab('TAB-2', 'TAB-1');
+      thunkAction = request(store.dispatch, () => ({
+        ...(store.getState() ?? {}),
+        dashboardState: result,
+      }));
+      result = dashboardStateReducer(result, thunkAction);
+      expect(result).toEqual({
+        activeTabs: expect.arrayContaining(['TAB-2']),
+        inactiveTabs: ['TAB-A', 'TAB-__a'],
+      });
+      request = setActiveTab('TAB-1', 'TAB-2');
+      thunkAction = request(store.dispatch, () => ({
+        ...(store.getState() ?? {}),
+        dashboardState: result,
+      }));
+      result = dashboardStateReducer(result, thunkAction);
+      expect(result).toEqual({
+        activeTabs: expect.arrayContaining(['TAB-1', 'TAB-A', 'TAB-__a']),
+        inactiveTabs: [],
+      });
+    });
   });
   it('SET_ACTIVE_TABS', () => {
     expect(