New permalinks retain previous permalink activeTab state

[fisjac]

Bug description

When using permalinks to create views of a dashboard in a given tab state, generating a permalink will store the active tabs within state. When generating a new permalink, the previous state is not cleared, rather the new activeTab state is appended to the previous state resulting in strange behavior such as new nesting configurations such as those shown in the screen recording in which tabs nested under "Parent 1" are now visible underneath "Parent 2."

How to reproduce the bug

Select a dashboard that contains multiple nesting layers and at least two parent tabs ["Parent 1", "Parent 2"]
Select a child tab from "Parent 1". ["P1 - T1"] should now be selected
Generate a permalink and copy to clipboard.
Select a child from "Parent 2". ["P2 - T1"] should now be selected
Generate a permalink and copy to clipboard
use the second permalink in the url and navigate to the dashboard
dashboard state will load a combination of previously selected permalink tabs.

Screenshots/recordings

https://www.loom.com/share/60d642624a2b4f039572610723781987

Superset version

master / latest-dev

Python version

3.9

Node version

16

Browser

Chrome

Additional context

No response

Checklist

• I have searched Superset docs and Slack and didn't find a solution to my problem.
• I have searched the GitHub issue tracker and didn't find a similar bug report.
• I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.

[dosubot]

Hey @fisjac! 👋 I'm here to help you with any bugs, questions, or contributions you have for the repository. Let's tackle this together!

To resolve the issue where generating a new permalink in your dashboard appends the new activeTab state to the previous state, causing incorrect nesting configurations, you need to ensure that the activeTabs field in the DashboardPermalinkStateSchema is correctly reset or updated without retaining the previous state.

Here is an example of how you might reset or update the activeTabs field when generating a new permalink:

from marshmallow import Schema, fields

class DashboardPermalinkStateSchema(Schema):
    dataMask = fields.Dict(
        required=False,
        allow_none=True,
        metadata={"description": "Data mask used for native filter state"},
    )
    activeTabs = fields.List(
        fields.String(),
        required=False,
        allow_none=True,
        metadata={"description": "Current active dashboard tabs"},
    )
    urlParams = fields.List(
        fields.Tuple(
            (
                fields.String(
                    required=True, allow_none=True, metadata={"description": "Key"}
                ),
                fields.String(
                    required=True, allow_none=True, metadata={"description": "Value"}
                ),
            ),
            required=False,
            allow_none=True,
            metadata={"description": "URL Parameter key-value pair"},
        ),
        required=False,
        allow_none=True,
        metadata={"description": "URL Parameters"},
    )
    anchor = fields.String(
        required=False,
        allow_none=True,
        metadata={"description": "Optional anchor link added to url hash"},
    )

class DashboardPermalinkSchema(Schema):
    dashboardId = fields.String(
        required=True,
        allow_none=False,
        metadata={"description": "The id or slug of the dashboard"},
    )
    state = fields.Nested(DashboardPermalinkStateSchema())

# Example function to generate a new permalink
def generate_new_permalink(dashboard_id, new_active_tabs):
    # Create a new state with the updated activeTabs
    new_state = {
        "dataMask": None,
        "activeTabs": new_active_tabs,
        "urlParams": None,
        "anchor": None,
    }
    
    # Create the permalink data
    permalink_data = {
        "dashboardId": dashboard_id,
        "state": new_state,
    }
    
    # Serialize the data using the schema
    schema = DashboardPermalinkSchema()
    result = schema.dump(permalink_data)
    
    return result

# Example usage
new_permalink = generate_new_permalink("dashboard_123", ["tab1", "tab2"])
print(new_permalink)

Additionally, ensure that your Redux reducer handling the SET_ACTIVE_TAB and SET_ACTIVE_TABS actions is correctly updating the state without retaining the previous activeTabs state. Here is an example from the dashboardStateReducer:

import {
  SET_ACTIVE_TAB,
  SET_ACTIVE_TABS,
} from '../actions/dashboardState';

export default function dashboardStateReducer(state = {}, action) {
  const actionHandlers = {
    [SET_ACTIVE_TAB]() {
      const newActiveTabs = new Set(state.activeTabs);
      newActiveTabs.delete(action.prevTabId);
      newActiveTabs.add(action.tabId);
      return {
        ...state,
        activeTabs: Array.from(newActiveTabs),
      };
    },
    [SET_ACTIVE_TABS]() {
      return {
        ...state,
        activeTabs: action.activeTabs,
      };
    },
  };

  if (action.type in actionHandlers) {
    return actionHandlers[action.type]();
  }
  return state;
}

In this example, the SET_ACTIVE_TABS action directly sets the activeTabs state to the new value provided in the action, ensuring that the previous state is not carried over. This should resolve the issue of incorrect nesting configurations when generating a new permalink ^[1][2].

Additionally, in the DashboardPage.tsx file, ensure that the activeTabs state is properly initialized and does not carry over any previous state when hydrating the dashboard:

useEffect(() => {
  async function getDataMaskApplied() {
    const permalinkKey = getUrlParam(URL_PARAMS.permalinkKey);
    const nativeFilterKeyValue = getUrlParam(URL_PARAMS.nativeFiltersKey);
    const isOldRison = getUrlParam(URL_PARAMS.nativeFilters);

    let dataMask = nativeFilterKeyValue || {};
    let activeTabs: string[] | undefined;
    if (permalinkKey) {
      const permalinkValue = await getPermalinkValue(permalinkKey);
      if (permalinkValue) {
        ({ dataMask, activeTabs } = permalinkValue.state);
      }
    } else if (nativeFilterKeyValue) {
      dataMask = await getFilterValue(id, nativeFilterKeyValue);
    }
    if (isOldRison) {
      dataMask = isOldRison;
    }

    if (readyToRender) {
      if (!isDashboardHydrated.current) {
        isDashboardHydrated.current = true;
      }
      dispatch(
        hydrateDashboard({
          history,
          dashboard,
          charts,
          activeTabs,
          dataMask,
        }),
      );
    }
    return null;
  }
  if (id) getDataMaskApplied();
}, [readyToRender]);

This ensures that the activeTabs state is correctly initialized and does not retain any previous state when hydrating the dashboard ^[3].

Furthermore, in the Tabs.jsx component, ensure that the activeTabs state is correctly managed and updated. Here is an example from the Tabs.jsx file:

import React from 'react';
import PropTypes from 'prop-types';
import { styled, t } from '@superset-ui/core';
import { connect } from 'react-redux';
import { LineEditableTabs } from 'src/components/Tabs';
import { LOG_ACTIONS_SELECT_DASHBOARD_TAB } from 'src/logger/LogUtils';
import { AntdModal } from 'src/components';
import { Draggable } from '../dnd/DragDroppable';
import DragHandle from '../dnd/DragHandle';
import DashboardComponent from '../../containers/DashboardComponent';
import DeleteComponentButton from '../DeleteComponentButton';
import HoverMenu from '../menu/HoverMenu';
import findTabIndexByComponentId from '../../util/findTabIndexByComponentId';
import getDirectPathToTabIndex from '../../util/getDirectPathToTabIndex';
import getLeafComponentIdFromPath from '../../util/getLeafComponentIdFromPath';
import { componentShape } from '../../util/propShapes';
import { NEW_TAB_ID } from '../../util/constants';
import { RENDER_TAB, RENDER_TAB_CONTENT } from './Tab';
import { TABS_TYPE, TAB_TYPE } from '../../util/componentTypes';

const propTypes = {
  id: PropTypes.string.isRequired,
  parentId: PropTypes.string.isRequired,
  component: componentShape.isRequired,
  parentComponent: componentShape.isRequired,
  index: PropTypes.number.isRequired,
  depth: PropTypes.number.isRequired,
  renderTabContent: PropTypes.bool,
  editMode: PropTypes.bool.isRequired,
  renderHoverMenu: PropTypes.bool,
  directPathToChild: PropTypes.arrayOf(PropTypes.string),
  activeTabs: PropTypes.arrayOf(PropTypes.string),

  logEvent: PropTypes.func.isRequired,
  setActiveTab: PropTypes.func,

  availableColumnCount: PropTypes.number,
  columnWidth: PropTypes.number,
  onResizeStart: PropTypes.func,
  onResize: PropTypes.func,
  onResizeStop: PropTypes.func,

  createComponent: PropTypes.func.isRequired,
  handleComponentDrop: PropTypes.func.isRequired,
  onChangeTab: PropTypes.func.isRequired,
  deleteComponent: PropTypes.func.isRequired,
  updateComponents: PropTypes.func.isRequired,
};

const defaultProps = {
  renderTabContent: true,
  renderHoverMenu: true,
  availableColumnCount: 0,
  columnWidth: 0,
  activeTabs: [],
  directPathToChild: [],
  setActiveTab() {},
  onResizeStart() {},
  onResize() {},
  onResizeStop() {},
};

const StyledTabsContainer = styled.div`
  width: 100%;
  background-color: ${({ theme }) => theme.colors.grayscale.light5};

  .dashboard-component-tabs-content {
    min-height: ${({ theme }) => theme.gridUnit * 12}px;
    margin-top: ${({ theme }) => theme.gridUnit / 4}px;
    position: relative;
  }

  .ant-tabs {
    overflow: visible;

    .ant-tabs-nav-wrap {
      min-height: ${({ theme }) => theme.gridUnit * 12.5}px;
    }

    .ant-tabs-content-holder {
      overflow: visible;
    }
  }

  div .ant-tabs-tab-btn {
    text-transform: none;
  }
`;

export class Tabs extends React.PureComponent {
  constructor(props) {
    super(props);
    const { tabIndex, activeKey } = this.getTabInfo(props);
    this.state = {
      tabIndex,
      activeKey,
    };
    this.handleClickTab = this.handleClickTab.bind(this);
    this.handleDeleteComponent = this.handleDeleteComponent.bind(this);
    this.handleDeleteTab = this.handleDeleteTab.bind(this);
    this.handleDropOnTab = this.handleDropOnTab.bind(this);
    this.handleDrop = this.handleDrop.bind(this);
  }

  componentDidMount() {
    this.props.setActiveTab(this.state.activeKey);
  }

  componentDidUpdate(prevProps, prevState) {
    if (prevState.activeKey !== this.state.activeKey) {
      this.props.setActiveTab(this.state.activeKey, prevState.activeKey);
    }
  }

  UNSAFE_componentWillReceiveProps(nextProps) {
    const maxIndex = Math.max(0, nextProps.component.children.length - 1);
    const currTabsIds = this.props.component.children;
    const nextTabsIds = nextProps.component.children;

    if (this.state.tabIndex > maxIndex) {
      this.setState(() => ({ tabIndex: maxIndex }));
    }

    if (nextProps.dashboardId !== this.props.dashboardId) {
      const { tabIndex, activeKey } = this.getTabInfo(nextProps);
      this.setState(() => ({
        tabIndex,
        activeKey,
      }));
    }

    if (nextProps.isComponentVisible) {
      const nextFocusComponent = getLeafComponentIdFromPath(
        nextProps.directPathToChild,
      );
      const currentFocusComponent = getLeafComponentIdFromPath(
        this.props.directPathToChild,
      );

      if (
        nextFocusComponent !== currentFocusComponent ||
        (nextFocusComponent === currentFocusComponent &&
          currTabsIds !== nextTabsIds)
      ) {
        const nextTabIndex = findTabIndexByComponentId({
          currentComponent: nextProps.component,
          directPathToChild: nextProps.directPathToChild,
        });

        if (nextTabIndex > -1 && nextTabIndex !== this.state.tabIndex) {
          this.setState(() => ({
            tabIndex: nextTabIndex,
            activeKey: nextTabsIds[nextTabIndex],
          }));
        }
      }
    }
  }

  getTabInfo = props => {
    let tabIndex = Math.max(
      0,
      findTabIndexByComponentId({
        currentComponent: props.component,
        directPathToChild: props.directPathToChild,
      }),
    );
    if (tabIndex === 0 && props.activeTabs?.length) {
      props.component.children.forEach((tabId, index) => {
        if (tabIndex === 0 && props.activeTabs.includes(tabId)) {
          tabIndex = index;
        }
      });
    }
    const { children: tabIds } = props.component;
    const activeKey = tabIds[tabIndex];

    return {
      tabIndex,
      activeKey,
    };
  };

  showDeleteConfirmModal = key => {
    const { component, deleteComponent } = this.props;
    AntdModal.confirm({
      title: t('Delete dashboard tab?'),
      content: (
        <span>
          {t(
            'Deleting a tab will remove all content within it. You may still ' +
              'reverse this action with the',
          )}{' '}
          <b>{t('undo')}</b>{' '}
          {t('button (cmd + z) until you save your changes.')}
        </span>
      ),
      onOk: () => {
        deleteComponent(key, component.id);
        const tabIndex = component.children.indexOf(key);
        this.handleDeleteTab(tabIndex);
      },
      okType: 'danger',
      okText: t('DELETE'),
      cancelText: t('CANCEL'),
      icon: null,
    });
  };

  handleEdit = (event, action) => {
    const { component, createComponent } = this.props. If (action === 'add') {
      event?.stopPropagation?.();
      createComponent({
        destination: {
          id: component.id,
          type: component.type,
          index: component.children.length,
        },
        dragging: {
          id: NEW_TAB_ID,
          type: TAB_TYPE,
        },
      });
    } else if (action === 'remove') {
      this.showDeleteConfirmModal(event);
    }
  };

  handleClickTab(tabIndex) {
    const { component } = this.props;
    const { children: tabIds } = component;

    if (tabIndex !== this.state.tabIndex) {
      const pathToTabIndex = getDirectPathToTabIndex(component, tabIndex);
      const targetTabId = pathToTabIndex[pathToTabIndex.length - 1];
      this.props.logEvent(LOG_ACTIONS_SELECT_DASHBOARD_TAB, {
        target_id: targetTabId,
        index: tabIndex,
      });

      this.props.onChangeTab({ pathToTabIndex });
    }
    this.setState(() => ({ activeKey: tabIds[tabIndex] }));
  }

  handleDeleteComponent() {
    const { deleteComponent, id, parentId } = this.props;
    deleteComponent(id, parentId);
  }

  handleDeleteTab(tabIndex) {
    if (this.state.tabIndex === tabIndex) {
      this.handleClickTab(Math.max(0, tabIndex - 1));
    }
  }

  handleDropOnTab(dropResult) {
    const { component } = this.props;

    const { destination } = dropResult;
    if (destination) {
      const dropTabIndex =
        destination.id === component.id
          ? destination.index
          : component.children.indexOf(destination.id);

      if (dropTabIndex > -1) {
        setTimeout(() => {
          this.handleClickTab(dropTabIndex);
        }, 30);
      }
    }
  }

  handleDrop(dropResult) {
    if (dropResult.dragging.type !== TABS_TYPE) {
      this.props.handleComponentDrop(dropResult);
    }
  }

  render() {
    const {
      depth,
      component: tabsComponent,
      parentComponent,
      index,
      availableColumnCount,
      columnWidth,
      onResizeStart,
      onResize,
      onResizeStop,
      renderTabContent,
      renderHoverMenu,
      isComponentVisible: isCurrentTabVisible,
      editMode,
      nativeFilters,
    } = this.props;

    const { children: tabIds } = tabsComponent;
    const { tabIndex: selectedTabIndex, activeKey } = this.state;

    let tabsToHighlight;
    const highlightedFilterId =
      nativeFilters?.focusedFilterId || nativeFilters?.hoveredFilterId;
    if (highlightedFilterId) {
      tabsToHighlight = nativeFilters.filters[highlightedFilterId]?.tabsInScope;
    }
    return (
      <Draggable
        component={tabsComponent}
        parentComponent={parentComponent}
        orientation="row"
        index={index}
        depth={depth}
        onDrop={this.handleDrop}
        editMode={editMode}
      >
        {({ dragSourceRef: tabsDragSourceRef }) => (
          <StyledTabsContainer
            className="dashboard-component dashboard-component-tabs"
            data-test="dashboard-component-tabs"
          >
            {editMode && renderHoverMenu && (
              <HoverMenu innerRef={tabsDragSourceRef} position="left">
                <DragHandle position="left" />
                <DeleteComponentButton onDelete={this.handleDeleteComponent} />
              </HoverMenu>
            )}
            <LineEditableTabs
              id={tabsComponent.id}
              activeKey={activeKey}
              onChange={key => {
                this.handleClickTab(tabIds.indexOf(key));
              }}
              onEdit={this.handleEdit}
              data-test="nav-list"
              type={editMode ? 'editable-card' : 'card'}
            >
              {tabIds.map((tabId, tabIndex) => (
                <LineEditableTabs.TabPane
                  key={tabId}
                  tab={
                    <DashboardComponent
                      id={tabId}
                      parentId={tabsComponent.id}
                      depth={depth}
                      index={tabIndex}
                      renderType={RENDER_TAB}
                      availableColumnCount={availableColumnCount}
                      columnWidth={columnWidth}
                      onDropOnTab={this.handleDropOnTab}
                      onHoverTab={() => this.handleClickTab(tabIndex)}
                      isFocused={activeKey === tabId}
                      isHighlighted={
                        activeKey !== tabId && tabsToHighlight?.includes(tabId)
                      }
                    />
                  }
                >
                  {renderTabContent && (
                    <DashboardComponent
                      id={tabId}
                      parentId={tabsComponent.id}
                      depth={depth}
                      index={tabIndex}
                      renderType={RENDER_TAB_CONTENT}
                      availableColumnCount={availableColumnCount}
                      columnWidth={columnWidth}
                      onResizeStart={onResizeStart}
                      onResize={onResize}
                      onResizeStop={onResizeStop}
                      onDropOnTab={this.handleDropOnTab}
                      isComponentVisible={
                        selectedTabIndex === tabIndex && isCurrentTabVisible
                      }
                    />
                  )}
                </LineEditableTabs.TabPane>
              ))}
            </LineEditableTabs>
          </StyledTabsContainer>
        )}
      </Draggable>
    );
  }
}
Tabs.propTypes = propTypes;
Tabs.defaultProps = defaultProps;

function mapStateToProps(state) {
  return {
    nativeFilters: state.nativeFilters,
    activeTabs: state.dashboardState.activeTabs,
    directPathToChild: state.dashboardState.directPathToChild,
  };
}
export default connect(mapStateToProps)(Tabs);

This ensures that the activeTabs state is correctly managed and updated within the Tabs component, preventing the issue of incorrect nesting configurations when generating a new permalink ^[4].

Additionally, you can refer to the load_tabbed_dashboard function in the superset/examples/tabbed_dashboard.py file for an example of creating a dashboard with nested tabs. This function demonstrates how to structure the dashboard's JSON configuration to handle nested tabs correctly ^[5].

Details

About Dosu
This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

To continue the conversation, mention @dosu.