chore: Removing DASHBOARD_CROSS_FILTERS flag and all that comes with …

…it. (#31794)

Co-authored-by: Kamil Gabryjelski <[email protected]>

RESOURCES/FEATURE_FLAGS.md

@@ -97,7 +97,6 @@ These features flags currently default to True and **will be removed in a future
 [//]: # "PLEASE KEEP THE LIST SORTED ALPHABETICALLY"
 
 - AVOID_COLORS_COLLISION
-- DASHBOARD_CROSS_FILTERS
 - DRILL_TO_DETAIL
 - ENABLE_JAVASCRIPT_CONTROLS
 - KV_STORE

UPDATING.md

@@ -28,6 +28,7 @@ assists people when migrating to a new version.
 - [31959](https://github.com/apache/superset/pull/31959) Removes the following endpoints from data uploads: /api/v1/database/<id>/<file type>_upload and /api/v1/database/<file type>_metadata, in favour of new one (Details on the PR). And simplifies permissions.
 - [31844](https://github.com/apache/superset/pull/31844) The `ALERT_REPORTS_EXECUTE_AS` and `THUMBNAILS_EXECUTE_AS` config parameters have been renamed to `ALERT_REPORTS_EXECUTORS` and `THUMBNAILS_EXECUTORS` respectively. A new config flag `CACHE_WARMUP_EXECUTORS` has also been introduced to be able to control which user is used to execute cache warmup tasks. Finally, the config flag `THUMBNAILS_SELENIUM_USER` has been removed. To use a fixed executor for async tasks, use the new `FixedExecutor` class. See the config and docs for more info on setting up different executor profiles.
 - [31894](https://github.com/apache/superset/pull/31894) Domain sharding is deprecated in favor of HTTP2. The `SUPERSET_WEBSERVER_DOMAINS` configuration will be removed in the next major version (6.0)
+- [31794](https://github.com/apache/superset/pull/31794) Removed the previously deprecated `DASHBOARD_CROSS_FILTERS` feature flag
 - [31774](https://github.com/apache/superset/pull/31774): Fixes the spelling of the `USE-ANALAGOUS-COLORS` feature flag. Please update any scripts/configuration item to use the new/corrected `USE-ANALOGOUS-COLORS` flag spelling.
 - [31582](https://github.com/apache/superset/pull/31582) Removed the legacy Area, Bar, Event Flow, Heatmap, Histogram, Line, Sankey, and Sankey Loop charts. They were all automatically migrated to their ECharts counterparts with the exception of the Event Flow and Sankey Loop charts which were removed as they were not actively maintained and not widely used. If you were using the Event Flow or Sankey Loop charts, you will need to find an alternative solution.
 - [31198](https://github.com/apache/superset/pull/31198) Disallows by default the use of the following ClickHouse functions: "version", "currentDatabase", "hostName".

superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts

@@ -30,8 +30,6 @@ export enum FeatureFlag {
   AvoidColorsCollision = 'AVOID_COLORS_COLLISION',
   ChartPluginsExperimental = 'CHART_PLUGINS_EXPERIMENTAL',
   ConfirmDashboardDiff = 'CONFIRM_DASHBOARD_DIFF',
-  /** @deprecated */
-  DashboardCrossFilters = 'DASHBOARD_CROSS_FILTERS',
   DashboardVirtualization = 'DASHBOARD_VIRTUALIZATION',
   DashboardRbac = 'DASHBOARD_RBAC',
   DatapanelClosedByDefault = 'DATAPANEL_CLOSED_BY_DEFAULT',

superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx

@@ -127,9 +127,7 @@ const ChartContextMenu = (
     canDrillBy &&
     isDisplayed(ContextMenuItem.DrillBy);
 
-  const showCrossFilters =
-    isFeatureEnabled(FeatureFlag.DashboardCrossFilters) &&
-    isDisplayed(ContextMenuItem.CrossFilter);
+  const showCrossFilters = isDisplayed(ContextMenuItem.CrossFilter);
 
   const isCrossFilteringSupportedByChart = getChartMetadataRegistry()
     .get(formData.viz_type)

superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx

@@ -29,7 +29,6 @@ const CONTEXT_MENU_TEST_ID = 'chart-context-menu';
 
 // @ts-ignore
 global.featureFlags = {
-  [FeatureFlag.DashboardCrossFilters]: true,
   [FeatureFlag.DrillToDetail]: true,
   [FeatureFlag.DrillBy]: true,
 };

superset-frontend/src/components/Chart/ChartRenderer.jsx

@@ -24,10 +24,10 @@ import {
   logging,
   Behavior,
   t,
-  isFeatureEnabled,
-  FeatureFlag,
   getChartMetadataRegistry,
   VizType,
+  isFeatureEnabled,
+  FeatureFlag,
 } from '@superset-ui/core';
 import { Logger, LOG_ACTIONS_RENDER_CHART } from 'src/logger/LogUtils';
 import { EmptyState } from 'src/components/EmptyState';
@@ -92,8 +92,7 @@ class ChartRenderer extends Component {
       showContextMenu:
         props.source === ChartSource.Dashboard &&
         !suppressContextMenu &&
-        (isFeatureEnabled(FeatureFlag.DrillToDetail) ||
-          isFeatureEnabled(FeatureFlag.DashboardCrossFilters)),
+        isFeatureEnabled(FeatureFlag.DrillToDetail),
       inContextMenu: false,
       legendState: undefined,
     };

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

@@ -353,16 +353,14 @@ export function saveDashboardRequest(data, id, saveType) {
       if (lastModifiedTime) {
         dispatch(saveDashboardRequestSuccess(lastModifiedTime));
       }
-      if (isFeatureEnabled(FeatureFlag.DashboardCrossFilters)) {
-        const { chartConfiguration, globalChartConfiguration } =
-          handleChartConfiguration();
-        dispatch(
-          saveChartConfiguration({
-            chartConfiguration,
-            globalChartConfiguration,
-          }),
-        );
-      }
+      const { chartConfiguration, globalChartConfiguration } =
+        handleChartConfiguration();
+      dispatch(
+        saveChartConfiguration({
+          chartConfiguration,
+          globalChartConfiguration,
+        }),
+      );
       dispatch(saveDashboardFinished());
       dispatch(addSuccessToast(t('This dashboard was saved successfully.')));
       return response;
@@ -435,12 +433,8 @@ export function saveDashboardRequest(data, id, saveType) {
     if (
       [SAVE_TYPE_OVERWRITE, SAVE_TYPE_OVERWRITE_CONFIRMED].includes(saveType)
     ) {
-      let chartConfiguration = {};
-      let globalChartConfiguration = {};
-      if (isFeatureEnabled(FeatureFlag.DashboardCrossFilters)) {
-        ({ chartConfiguration, globalChartConfiguration } =
-          handleChartConfiguration());
-      }
+      const { chartConfiguration, globalChartConfiguration } =
+        handleChartConfiguration();
       const updatedDashboard =
         saveType === SAVE_TYPE_OVERWRITE_CONFIRMED
           ? data

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

@@ -61,6 +61,7 @@ describe('dashboardState actions', () => {
       present: mockDashboardData.positions,
       future: {},
     },
+    charts: {},
   };
   const newDashboardData = mockDashboardData;
 

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

@@ -230,16 +230,14 @@ export const hydrateDashboard =
       filterConfig: metadata?.native_filter_configuration || [],
     });
 
-    if (isFeatureEnabled(FeatureFlag.DashboardCrossFilters)) {
-      const { chartConfiguration, globalChartConfiguration } =
-        getCrossFiltersConfiguration(
-          dashboardLayout.present,
-          metadata,
-          chartQueries,
-        );
-      metadata.chart_configuration = chartConfiguration;
-      metadata.global_chart_configuration = globalChartConfiguration;
-    }
+    const { chartConfiguration, globalChartConfiguration } =
+      getCrossFiltersConfiguration(
+        dashboardLayout.present,
+        metadata,
+        chartQueries,
+      );
+    metadata.chart_configuration = chartConfiguration;
+    metadata.global_chart_configuration = globalChartConfiguration;
 
     const { roles } = user;
     const canEdit = canUserEditDashboard(dashboard, user);

superset-frontend/src/dashboard/components/Dashboard.jsx

@@ -18,7 +18,7 @@
  */
 import { PureComponent } from 'react';
 import PropTypes from 'prop-types';
-import { isFeatureEnabled, t, FeatureFlag } from '@superset-ui/core';
+import { t } from '@superset-ui/core';
 
 import { PluginContext } from 'src/components/DynamicPlugins';
 import Loading from 'src/components/Loading';
@@ -163,10 +163,7 @@ class Dashboard extends PureComponent {
       editMode,
     } = this.props;
     const { appliedFilters, appliedOwnDataCharts } = this;
-    if (
-      isFeatureEnabled(FeatureFlag.DashboardCrossFilters) &&
-      !chartConfiguration
-    ) {
+    if (!chartConfiguration) {
       // For a first loading we need to wait for cross filters charts data loaded to get all active filters
       // for correct comparing  of filters to avoid unnecessary requests
       return;

superset-frontend/src/dashboard/components/Dashboard.test.jsx

@@ -61,6 +61,7 @@ describe('Dashboard', () => {
     userId: dashboardInfo.userId,
     impressionId: 'id',
     loadStats: {},
+    chartConfiguration: {},
   };
 
   const ChildrenComponent = () => <div>Test</div>;

superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx

@@ -396,9 +396,6 @@ const DashboardBuilder = () => {
   const fullSizeChartId = useSelector<RootState, number | null>(
     state => state.dashboardState.fullSizeChartId,
   );
-  const crossFiltersEnabled = isFeatureEnabled(
-    FeatureFlag.DashboardCrossFilters,
-  );
   const filterBarOrientation = useSelector<RootState, FilterBarOrientation>(
     ({ dashboardInfo }) =>
       isFeatureEnabled(FeatureFlag.HorizontalFilterBar)
@@ -476,8 +473,7 @@ const DashboardBuilder = () => {
     ELEMENT_ON_SCREEN_OPTIONS,
   );
 
-  const showFilterBar =
-    (crossFiltersEnabled || nativeFiltersEnabled) && !editMode;
+  const showFilterBar = !editMode;
 
   const offset =
     FILTER_BAR_HEADER_HEIGHT +

superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.test.tsx

@@ -62,8 +62,10 @@ test('requests update dashboard api when save button is clicked', async () => {
     result: overwriteConfirmMetadata.data,
   });
   const store = mockStore({
-    dashboardLayout: {},
+    dashboardLayout: { present: {} },
     dashboardFilters: {},
+    dashboardInfo: { metadata: {} },
+    charts: {},
   });
   const { findByTestId } = render(
     <OverwriteConfirmModal

superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx

@@ -177,7 +177,6 @@ const SliceHeaderControls = (
     useSelector<RootState, boolean>(
       ({ dashboardInfo }) => dashboardInfo.dash_edit_perm,
     ) &&
-    isFeatureEnabled(FeatureFlag.DashboardCrossFilters) &&
     getChartMetadataRegistry()
       .get(props.slice.viz_type)
       ?.behaviors?.includes(Behavior.InteractiveChart);

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

@@ -72,6 +72,10 @@ const setup = (dashboardInfoOverride: Partial<DashboardInfo> = {}) =>
     }),
   );
 
+beforeEach(() => {
+  fetchMock.restore();
+});
+
 test('Dropdown trigger renders with FF HORIZONTAL_FILTER_BAR on', async () => {
   // @ts-ignore
   global.featureFlags = {
@@ -104,45 +108,14 @@ test('Dropdown trigger does not render without dashboard edit permissions', asyn
   expect(screen.queryByRole('img', { name: 'gear' })).not.toBeInTheDocument();
 });
 
-test('Dropdown trigger renders with FF DASHBOARD_CROSS_FILTERS on', async () => {
-  // @ts-ignore
-  global.featureFlags = {
-    [FeatureFlag.DashboardCrossFilters]: true,
-  };
-  await setup();
-
-  expect(screen.getByRole('img', { name: 'gear' })).toBeInTheDocument();
-});
-
-test('Dropdown trigger does not render with FF DASHBOARD_CROSS_FILTERS off', async () => {
-  // @ts-ignore
-  global.featureFlags = {
-    [FeatureFlag.DashboardCrossFilters]: false,
-  };
-  await setup({
-    dash_edit_perm: false,
-  });
-
-  expect(screen.queryByRole('img', { name: 'gear' })).not.toBeInTheDocument();
-});
-
 test('Popover shows cross-filtering option on by default', async () => {
-  // @ts-ignore
-  global.featureFlags = {
-    [FeatureFlag.DashboardCrossFilters]: true,
-  };
   await setup();
   userEvent.click(screen.getByLabelText('gear'));
   expect(screen.getByText('Enable cross-filtering')).toBeInTheDocument();
   expect(screen.getByRole('checkbox')).toBeChecked();
 });
 
 test('Can enable/disable cross-filtering', async () => {
-  // @ts-ignore
-  global.featureFlags = {
-    [FeatureFlag.DashboardCrossFilters]: true,
-  };
-  fetchMock.reset();
   fetchMock.put('glob:*/api/v1/dashboard/1', {
     result: {},
   });
@@ -168,7 +141,7 @@ test('Popover opens with "Vertical" selected', async () => {
   expect(await screen.findByText('Vertical (Left)')).toBeInTheDocument();
   expect(screen.getByText('Horizontal (Top)')).toBeInTheDocument();
   expect(
-    within(screen.getAllByRole('menuitem')[2]).getByLabelText('check'),
+    within(screen.getAllByRole('menuitem')[4]).getByLabelText('check'),
   ).toBeInTheDocument();
 });
 
@@ -183,7 +156,7 @@ test('Popover opens with "Horizontal" selected', async () => {
   expect(await screen.findByText('Vertical (Left)')).toBeInTheDocument();
   expect(screen.getByText('Horizontal (Top)')).toBeInTheDocument();
   expect(
-    within(screen.getAllByRole('menuitem')[3]).getByLabelText('check'),
+    within(screen.getAllByRole('menuitem')[5]).getByLabelText('check'),
   ).toBeInTheDocument();
 });
 
@@ -192,7 +165,6 @@ test('On selection change, send request and update checked value', async () => {
   global.featureFlags = {
     [FeatureFlag.HorizontalFilterBar]: true,
   };
-  fetchMock.reset();
   fetchMock.put('glob:*/api/v1/dashboard/1', {
     result: {
       json_metadata: JSON.stringify({
@@ -209,14 +181,14 @@ test('On selection change, send request and update checked value', async () => {
   expect(await screen.findByText('Vertical (Left)')).toBeInTheDocument();
   expect(screen.getByText('Horizontal (Top)')).toBeInTheDocument();
   expect(
-    within(screen.getAllByRole('menuitem')[2]).getByLabelText('check'),
+    within(screen.getAllByRole('menuitem')[4]).getByLabelText('check'),
   ).toBeInTheDocument();
 
   userEvent.click(screen.getByText('Horizontal (Top)'));
 
   // 1st check - checkmark appears immediately after click
   expect(
-    await within(screen.getAllByRole('menuitem')[3]).findByLabelText('check'),
+    await within(screen.getAllByRole('menuitem')[5]).findByLabelText('check'),
   ).toBeInTheDocument();
   // successful query
   await waitFor(() =>
@@ -231,26 +203,28 @@ test('On selection change, send request and update checked value', async () => {
   );
   await waitFor(() => {
     const menuitems = screen.getAllByRole('menuitem');
-    expect(menuitems.length).toBeGreaterThanOrEqual(3);
+    expect(menuitems.length).toBeGreaterThanOrEqual(6);
   });
 
+  userEvent.click(screen.getByLabelText('gear'));
+  userEvent.hover(screen.getByText('Orientation of filter bar'));
+  expect(await screen.findByText('Vertical (Left)')).toBeInTheDocument();
+  expect(screen.getByText('Horizontal (Top)')).toBeInTheDocument();
+
   // 2nd check - checkmark stays after successful query
   expect(
-    await within(screen.getAllByRole('menuitem')[3]).findByLabelText('check'),
+    await within(screen.getAllByRole('menuitem')[5]).findByLabelText('check'),
   ).toBeInTheDocument();
   expect(
-    within(screen.getAllByRole('menuitem')[2]).queryByLabelText('check'),
+    within(screen.getAllByRole('menuitem')[4]).queryByLabelText('check'),
   ).not.toBeInTheDocument();
-
-  fetchMock.reset();
 });
 
 test('On failed request, restore previous selection', async () => {
   // @ts-ignore
   global.featureFlags = {
     [FeatureFlag.HorizontalFilterBar]: true,
   };
-  fetchMock.reset();
   fetchMock.put('glob:*/api/v1/dashboard/1', 400);
 
   const dangerToastSpy = jest.spyOn(mockedMessageActions, 'addDangerToast');
@@ -263,10 +237,10 @@ test('On failed request, restore previous selection', async () => {
   expect(screen.getByText('Horizontal (Top)')).toBeInTheDocument();
 
   expect(
-    within(screen.getAllByRole('menuitem')[2]).getByLabelText('check'),
+    within(screen.getAllByRole('menuitem')[4]).getByLabelText('check'),
   ).toBeInTheDocument();
   expect(
-    within(screen.getAllByRole('menuitem')[3]).queryByLabelText('check'),
+    within(screen.getAllByRole('menuitem')[5]).queryByLabelText('check'),
   ).not.toBeInTheDocument();
 
   userEvent.click(await screen.findByText('Horizontal (Top)'));
@@ -284,16 +258,14 @@ test('On failed request, restore previous selection', async () => {
 
   await waitFor(() => {
     const menuitems = screen.getAllByRole('menuitem');
-    expect(menuitems.length).toBeGreaterThanOrEqual(3);
+    expect(menuitems.length).toBeGreaterThanOrEqual(6);
   });
 
   // checkmark gets rolled back to the original selection after successful query
   expect(
-    await within(screen.getAllByRole('menuitem')[2]).findByLabelText('check'),
+    await within(screen.getAllByRole('menuitem')[4]).findByLabelText('check'),
   ).toBeInTheDocument();
   expect(
-    within(screen.getAllByRole('menuitem')[3]).queryByLabelText('check'),
+    within(screen.getAllByRole('menuitem')[5]).queryByLabelText('check'),
   ).not.toBeInTheDocument();
-
-  fetchMock.reset();
 });