Previous locations remain in the location filter when changing the account

[Athulqa]

Bug description

When switching to another account, the location filter displays locations from both the previous and current accounts

How to reproduce the bug

1. Navigate to a dashboard with account and location filters.
2. Select an account and choose a few locations.
3. Switch to a different account using the account filter.
4. The previous locations remain in the location filter

Screenshots/recordings

https://github.com/apache/superset/assets/173764651/25c4b986-dff1-439b-bdfe-e28dd8f0dfe8

Superset version

master / latest-dev

Python version

3.9

Node version

16

Browser

Chrome

Additional context

No response

Checklist

• [X] I have searched Superset docs and Slack and didn't find a solution to my problem.
• [X] I have searched the GitHub issue tracker and didn't find a similar bug report.
• [X] 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[bot]]

To fix the issue where the location filter displays locations from both the previous and current accounts when switching accounts, you need to ensure that the filters are reset or cleared when an account switch occurs. This can be done by updating the state of the filters to their default values upon account change.

Here is an example of how you might implement this:

1. Reset Filters on Account Switch: Ensure that the filters are reset to their default state when an account switch is detected.

import { nativeFilters, dataMaskWith2Filters, singleNativeFiltersState, dataMaskWith1Filter } from './path/to/mockNativeFilters';

// Function to reset filters to their default state
function resetFilters() {
  Object.keys(nativeFilters.filters).forEach(filterId => {
    nativeFilters.filters[filterId].defaultDataMask.filterState.value = null;
  });
}

// Function to handle account switch
function handleAccountSwitch() {
  // Reset filters
  resetFilters();

  // Additional logic for handling account switch
  // ...
}

// Example usage: Call handleAccountSwitch when an account switch is detected
handleAccountSwitch();

2. Update Filter State: Ensure that the filter state is updated correctly in the application state management (e.g., Redux, Context API).

import { useDispatch } from 'react-redux';
import { setDataMask } from '@superset-ui/core';

const dispatch = useDispatch();

function resetFilters() {
  Object.keys(nativeFilters.filters).forEach(filterId => {
    dispatch(setDataMask({
      filterId,
      filterState: {
        value: null,
      },
    }));
  });
}

function handleAccountSwitch() {
  // Reset filters
  resetFilters();

  // Additional logic for handling account switch
  // ...
}

// Example usage: Call handleAccountSwitch when an account switch is detected
handleAccountSwitch();

By resetting the filters to their default state when an account switch occurs, you can ensure that the location filter does not display locations from both the previous and current accounts ^[1].

To continue talking to Dosu, mention @dosu.

[Athulqa]

@dosu We are not maintaining the codebase. We are using the 3.1.1 Docker image

[dosubot[bot]]

To resolve the issue of the location filter retaining locations from the previous account when switching accounts in Superset version 3.1.1 (Docker image), you need to ensure that the data masks are cleared when switching accounts. Here is a potential solution:

1. Add a new effect to listen for account changes.
2. Clear the data masks when the account changes.

You can modify the FilterBar component as follows:

import React, {
  useEffect,
  useState,
  useCallback,
  createContext,
  useRef,
} from 'react';
import { useDispatch, useSelector } from 'react-redux';
import {
  DataMaskStateWithId,
  DataMaskWithId,
  Filter,
  DataMask,
  SLOW_DEBOUNCE,
  isNativeFilter,
  usePrevious,
  styled,
} from '@superset-ui/core';
import { useHistory } from 'react-router-dom';
import { updateDataMask, clearDataMask } from 'src/dataMask/actions';
import { useImmer } from 'use-immer';
import { isEmpty, isEqual, debounce } from 'lodash';
import { getInitialDataMask } from 'src/dataMask/reducer';
import { URL_PARAMS } from 'src/constants';
import { getUrlParam } from 'src/utils/urlUtils';
import { useTabId } from 'src/hooks/useTabId';
import { logEvent } from 'src/logger/actions';
import { LOG_ACTIONS_CHANGE_DASHBOARD_FILTER } from 'src/logger/LogUtils';
import { FilterBarOrientation, RootState } from 'src/dashboard/types';
import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
import { checkIsApplyDisabled } from './utils';
import { FiltersBarProps } from './types';
import {
  useNativeFiltersDataMask,
  useFilters,
  useFilterUpdates,
  useInitialization,
} from './state';
import { createFilterKey, updateFilterKey } from './keyValue';
import ActionButtons from './ActionButtons';
import Horizontal from './Horizontal';
import Vertical from './Vertical';
import { useSelectFiltersInScope } from '../state';

const HiddenFilterBar = styled.div`
  display: none;
`;

const EXCLUDED_URL_PARAMS: string[] = [
  URL_PARAMS.nativeFilters.name,
  URL_PARAMS.permalinkKey.name,
];

const publishDataMask = debounce(
  async (
    history,
    dashboardId,
    updateKey,
    dataMaskSelected: DataMaskStateWithId,
    tabId,
  ) => {
    const { location } = history;
    const { search } = location;
    const previousParams = new URLSearchParams(search);
    const newParams = new URLSearchParams();
    let dataMaskKey: string | null;
    previousParams.forEach((value, key) => {
      if (!EXCLUDED_URL_PARAMS.includes(key)) {
        newParams.append(key, value);
      }
    });

    const nativeFiltersCacheKey = getUrlParam(URL_PARAMS.nativeFiltersKey);
    const dataMask = JSON.stringify(dataMaskSelected);
    if (
      updateKey &&
      nativeFiltersCacheKey &&
      (await updateFilterKey(
        dashboardId,
        dataMask,
        nativeFiltersCacheKey,
        tabId,
      ))
    ) {
      dataMaskKey = nativeFiltersCacheKey;
    } else {
      dataMaskKey = await createFilterKey(dashboardId, dataMask, tabId);
    }
    if (dataMaskKey) {
      newParams.set(URL_PARAMS.nativeFiltersKey.name, dataMaskKey);
    }

    if (window.location.pathname.includes('/superset/dashboard')) {
      history.location.pathname = window.location.pathname;
      history.replace({
        search: newParams.toString(),
      });
    }
  },
  SLOW_DEBOUNCE,
);

export const FilterBarScrollContext = createContext(false);

const FilterBar: React.FC<FiltersBarProps> = ({
  orientation = FilterBarOrientation.Vertical,
  verticalConfig,
  hidden = false,
}) => {
  const history = useHistory();
  const dataMaskApplied: DataMaskStateWithId = useNativeFiltersDataMask();
  const [dataMaskSelected, setDataMaskSelected] =
    useImmer<DataMaskStateWithId>(dataMaskApplied);
  const dispatch = useDispatch();
  const [updateKey, setUpdateKey] = useState(0);
  const tabId = useTabId();
  const filters = useFilters();
  const previousFilters = usePrevious(filters);
  const filterValues = Object.values(filters);
  const nativeFilterValues = filterValues.filter(isNativeFilter);
  const dashboardId = useSelector<any, number>(
    ({ dashboardInfo }) => dashboardInfo?.id,
  );
  const previousDashboardId = usePrevious(dashboardId);
  const canEdit = useSelector<RootState, boolean>(
    ({ dashboardInfo }) => dashboardInfo.dash_edit_perm,
  );
  const user: UserWithPermissionsAndRoles = useSelector<
    RootState,
    UserWithPermissionsAndRoles
  >(state => state.user);

  const [filtersInScope] = useSelectFiltersInScope(nativeFilterValues);
  const dataMaskSelectedRef = useRef(dataMaskSelected);
  dataMaskSelectedRef.current = dataMaskSelected;

  const handleFilterSelectionChange = useCallback(
    (
      filter: Pick<Filter, 'id'> & Partial<Filter>,
      dataMask: Partial<DataMask>,
    ) => {
      setDataMaskSelected(draft => {
        if (
          dataMask.filterState?.value !== undefined &&
          dataMaskSelectedRef.current[filter.id]?.filterState?.value ===
            undefined &&
          filter.requiredFirst
        ) {
          dispatch(updateDataMask(filter.id, dataMask));
        }
        draft[filter.id] = {
          ...(getInitialDataMask(filter.id) as DataMaskWithId),
          ...dataMask,
        };
      });
    },
    [dispatch, setDataMaskSelected],
  );

  useEffect(() => {
    if (previousFilters && dashboardId === previousDashboardId) {
      const updates = {};
      Object.values(filters).forEach(currentFilter => {
        const previousFilter = previousFilters?.[currentFilter.id];
        if (!previousFilter) {
          return;
        }
        const currentType = currentFilter.filterType;
        const currentTargets = currentFilter.targets;
        const currentDataMask = currentFilter.defaultDataMask;
        const previousType = previousFilter?.filterType;
        const previousTargets = previousFilter?.targets;
        const previousDataMask = previousFilter?.defaultDataMask;
        const typeChanged = currentType !== previousType;
        const targetsChanged = !isEqual(currentTargets, previousTargets);
        const dataMaskChanged = !isEqual(currentDataMask, previousDataMask);

        if (typeChanged || targetsChanged || dataMaskChanged) {
          updates[currentFilter.id] = getInitialDataMask(currentFilter.id);
        }
      });
      if (!isEmpty(updates)) {
        setDataMaskSelected(draft => ({ ...draft, ...updates }));
        Object.keys(updates).forEach(key => dispatch(clearDataMask(key)));
      }
    }
  }, [
    JSON.stringify(filters),
    JSON.stringify(previousFilters),
    previousDashboardId,
  ]);

  const dataMaskAppliedText = JSON.stringify(dataMaskApplied);

  useEffect(() => {
    setDataMaskSelected(() => dataMaskApplied);
  }, [dataMaskAppliedText, setDataMaskSelected]);

  useEffect(() => {
    if (user?.userId) {
      publishDataMask(history, dashboardId, updateKey, dataMaskApplied, tabId);
    }
  }, [dashboardId, dataMaskAppliedText, history, updateKey, tabId]);

  // New effect to handle account switching
  useEffect(() => {
    const handleAccountSwitch = () => {
      Object.keys(dataMaskSelected).forEach(filterId => {
        dispatch(clearDataMask(filterId));
      });
      setDataMaskSelected({});
    };

    // Assuming `user` object changes when account switches
    handleAccountSwitch();
  }, [user, dispatch, setDataMaskSelected]);

  const handleApply = useCallback(() => {
    dispatch(logEvent(LOG_ACTIONS_CHANGE_DASHBOARD_FILTER, {}));
    const filterIds = Object.keys(dataMaskSelected);
    setUpdateKey(1);
    filterIds.forEach(filterId => {
      if (dataMaskSelected[filterId]) {
        dispatch(updateDataMask(filterId, dataMaskSelected[filterId]));
      }
    });
  }, [dataMaskSelected, dispatch]);

  const handleClearAll = useCallback(() => {
    const clearDataMaskIds: string[] = [];
    let dispatchAllowed = false;
    filtersInScope.filter(isNativeFilter).forEach(filter => {
      const { id } = filter;
      if (dataMaskSelected[id]) {
        if (filter.controlValues?.enableEmptyFilter) {
          dispatchAllowed = false;
        }
        clearDataMaskIds.push(id);
        setDataMaskSelected(draft => {
          if (draft[id].filterState?.value !== undefined) {
            draft[id].filterState!.value = undefined;
          }
        });
      }
    });
    if (dispatchAllowed) {
      clearDataMaskIds.forEach(id => dispatch(clearDataMask(id)));
    }
  }, [dataMaskSelected, dispatch, filtersInScope, setDataMaskSelected]);

  useFilterUpdates(dataMaskSelected, setDataMaskSelected);

  const isApplyDisabled = checkIsApplyDisabled(
    dataMaskSelected,
    dataMaskApplied,
    filtersInScope.filter(isNativeFilter),
  );
  const isInitialized = useInitialization();

  const actions = (
    <ActionButtons
      filterBarOrientation={orientation}
      width={verticalConfig?.width}
      onApply={handleApply}
      onClearAll={handleClearAll}
      dataMaskSelected={dataMaskSelected}
      dataMaskApplied={dataMaskApplied}
      isApplyDisabled={isApplyDisabled}
    />
  );

  const filterBarComponent =
    orientation === FilterBarOrientation.Horizontal ? (
      <Horizontal
        actions={actions}
        canEdit={canEdit}
        dashboardId={dashboardId}
        dataMaskSelected={dataMaskSelected}
        filterValues={filterValues}
        isInitialized={isInitialized}
        onSelectionChange={handleFilterSelectionChange}
      />
    ) : verticalConfig ? (
      <Vertical
        actions={actions}
        canEdit={canEdit}
        dataMaskSelected={dataMaskSelected}
        filtersOpen={verticalConfig.filtersOpen}
        filterValues={filterValues}
        isInitialized={isInitialized}
        height={verticalConfig.height}
        offset={verticalConfig.offset}
        onSelectionChange={handleFilterSelectionChange}
        toggleFiltersBar={verticalConfig.toggleFiltersBar}
        width={verticalConfig.width}
      />
    ) : null;

  return hidden ? (
    <HiddenFilterBar>{filterBarComponent}</HiddenFilterBar>
  ) : (
    filterBarComponent
  );
};

export default React.memo(FilterBar);

This code adds a new useEffect that listens for changes in the user object, which is assumed to change when the account switches. When the account changes, it clears all data masks and resets the dataMaskSelected state ^[1].

To continue talking to Dosu, mention @dosu.

[rusackas]

Something went wrong with the video upload, so I can't see it... but perhaps this'll help:

I'm assuming you have an account column and a location column, and have a top level filter for each.

If certain accounts have certain locations associated, you might want to use a "dependent filter" as seen here:

By design, changing one filter should not nuke another filter. That would be infuriating.