From 962fd4cca39c89b81bd4624219763eb63e9b68a6 Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Tue, 28 Jan 2025 09:39:30 -0700 Subject: [PATCH] chore: Removing DASHBOARD_CROSS_FILTERS flag and all that comes with it. (#31794) Co-authored-by: Kamil Gabryjelski --- RESOURCES/FEATURE_FLAGS.md | 1 - UPDATING.md | 1 + .../src/utils/featureFlags.ts | 2 - .../ChartContextMenu/ChartContextMenu.tsx | 4 +- .../ChartContextMenu/useContextMenu.test.tsx | 1 - .../src/components/Chart/ChartRenderer.jsx | 7 +- .../src/dashboard/actions/dashboardState.js | 26 +++--- .../dashboard/actions/dashboardState.test.js | 1 + .../src/dashboard/actions/hydrate.js | 18 ++-- .../src/dashboard/components/Dashboard.jsx | 7 +- .../dashboard/components/Dashboard.test.jsx | 1 + .../DashboardBuilder/DashboardBuilder.tsx | 6 +- .../OverwriteConfirmModal.test.tsx | 4 +- .../components/SliceHeaderControls/index.tsx | 1 - .../FilterBarSettings.test.tsx | 70 +++++----------- .../FilterBar/FilterBarSettings/index.tsx | 11 +-- .../FilterControls/FilterControls.tsx | 22 ++--- .../nativeFilters/FilterBar/Horizontal.tsx | 29 ++----- .../FilterBar/HorizontalFilterBar.test.tsx | 3 + .../nativeFilters/FilterBar/Vertical.tsx | 12 +-- .../components/nativeFilters/selectors.ts | 82 +++++++++---------- .../components/nativeFilters/utils.test.ts | 73 +++-------------- .../components/nativeFilters/utils.ts | 5 +- .../src/dashboard/util/crossFilters.test.ts | 35 +------- .../src/dashboard/util/crossFilters.ts | 9 +- superset-frontend/src/dataMask/reducer.ts | 20 ++--- .../databases/DatabaseModal/index.test.tsx | 11 +-- superset/config.py | 1 - 28 files changed, 141 insertions(+), 322 deletions(-) diff --git a/RESOURCES/FEATURE_FLAGS.md b/RESOURCES/FEATURE_FLAGS.md index 714971378ecb7..3855729f364f5 100644 --- a/RESOURCES/FEATURE_FLAGS.md +++ b/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 diff --git a/UPDATING.md b/UPDATING.md index 65ec06ac79ab4..65161ac1e8721 100644 --- a/UPDATING.md +++ b/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//_upload and /api/v1/database/_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". diff --git a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts index 34799d3c52324..216a4ba58138c 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts +++ b/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', diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx index 9b3752152ea25..d619dae4897a1 100644 --- a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx +++ b/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) diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx index c06857d556dea..e1db166269b5c 100644 --- a/superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx +++ b/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, }; diff --git a/superset-frontend/src/components/Chart/ChartRenderer.jsx b/superset-frontend/src/components/Chart/ChartRenderer.jsx index dd456b0db43c3..25fc045d21054 100644 --- a/superset-frontend/src/components/Chart/ChartRenderer.jsx +++ b/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, }; diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index 4ade6f33278fb..33111c75d508c 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/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 diff --git a/superset-frontend/src/dashboard/actions/dashboardState.test.js b/superset-frontend/src/dashboard/actions/dashboardState.test.js index e6da8b04650ff..9df0ac9cba44f 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.test.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.test.js @@ -61,6 +61,7 @@ describe('dashboardState actions', () => { present: mockDashboardData.positions, future: {}, }, + charts: {}, }; const newDashboardData = mockDashboardData; diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js index 8dd36a8063231..55754b1c271ea 100644 --- a/superset-frontend/src/dashboard/actions/hydrate.js +++ b/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); diff --git a/superset-frontend/src/dashboard/components/Dashboard.jsx b/superset-frontend/src/dashboard/components/Dashboard.jsx index 48e70c8fd468a..762e120390858 100644 --- a/superset-frontend/src/dashboard/components/Dashboard.jsx +++ b/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; diff --git a/superset-frontend/src/dashboard/components/Dashboard.test.jsx b/superset-frontend/src/dashboard/components/Dashboard.test.jsx index e3421ee04057d..60652210adb73 100644 --- a/superset-frontend/src/dashboard/components/Dashboard.test.jsx +++ b/superset-frontend/src/dashboard/components/Dashboard.test.jsx @@ -61,6 +61,7 @@ describe('Dashboard', () => { userId: dashboardInfo.userId, impressionId: 'id', loadStats: {}, + chartConfiguration: {}, }; const ChildrenComponent = () =>
Test
; diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index cdd59b459a091..e18583ad78b22 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -396,9 +396,6 @@ const DashboardBuilder = () => { const fullSizeChartId = useSelector( state => state.dashboardState.fullSizeChartId, ); - const crossFiltersEnabled = isFeatureEnabled( - FeatureFlag.DashboardCrossFilters, - ); const filterBarOrientation = useSelector( ({ 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 + diff --git a/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.test.tsx b/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.test.tsx index 5d93fd6900b04..645a859bea594 100644 --- a/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.test.tsx +++ b/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( ( ({ dashboardInfo }) => dashboardInfo.dash_edit_perm, ) && - isFeatureEnabled(FeatureFlag.DashboardCrossFilters) && getChartMetadataRegistry() .get(props.slice.viz_type) ?.behaviors?.includes(Behavior.InteractiveChart); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/FilterBarSettings.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/FilterBarSettings.test.tsx index 2c6196962ec5a..b073fe655e995 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/FilterBarSettings.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/FilterBarSettings.test.tsx @@ -72,6 +72,10 @@ const setup = (dashboardInfoOverride: Partial = {}) => }), ); +beforeEach(() => { + fetchMock.restore(); +}); + test('Dropdown trigger renders with FF HORIZONTAL_FILTER_BAR on', async () => { // @ts-ignore global.featureFlags = { @@ -104,33 +108,7 @@ 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(); @@ -138,11 +116,6 @@ test('Popover shows cross-filtering option on by default', async () => { }); 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,18 +203,21 @@ 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 () => { @@ -250,7 +225,6 @@ test('On failed request, restore previous selection', async () => { 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(); }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/index.tsx index 9afc05b40c55f..404d73799959d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/index.tsx @@ -83,13 +83,9 @@ const FilterBarSettings = () => { ); const [selectedFilterBarOrientation, setSelectedFilterBarOrientation] = useState(filterBarOrientation); - const isCrossFiltersFeatureEnabled = isFeatureEnabled( - FeatureFlag.DashboardCrossFilters, - ); - const shouldEnableCrossFilters = - isCrossFiltersEnabled && isCrossFiltersFeatureEnabled; + const [crossFiltersEnabled, setCrossFiltersEnabled] = useState( - shouldEnableCrossFilters, + isCrossFiltersEnabled, ); const canEdit = useSelector( ({ dashboardInfo }) => dashboardInfo.dash_edit_perm, @@ -188,7 +184,7 @@ const FilterBarSettings = () => { divider: canSetHorizontalFilterBar, }); } - if (isCrossFiltersFeatureEnabled && canEdit) { + if (canEdit) { items.push({ key: CROSS_FILTERS_MENU_KEY, label: crossFiltersMenuItem, @@ -222,7 +218,6 @@ const FilterBarSettings = () => { crossFiltersMenuItem, dashboardId, filterValues, - isCrossFiltersFeatureEnabled, ]); if (!menuItems.length) { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx index d959026792a65..18126b2f99838 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx @@ -62,15 +62,12 @@ import crossFiltersSelector from '../CrossFilters/selectors'; import CrossFilter from '../CrossFilters/CrossFilter'; import { useFilterOutlined } from '../useFilterOutlined'; import { useChartsVerboseMaps } from '../utils'; -import { CrossFilterIndicator } from '../../selectors'; type FilterControlsProps = { dataMaskSelected: DataMaskStateWithId; onFilterSelectionChange: (filter: Filter, dataMask: DataMask) => void; }; -const EMPTY_ARRAY: CrossFilterIndicator[] = []; - const FilterControls: FC = ({ dataMaskSelected, onFilterSelectionChange, @@ -94,20 +91,15 @@ const FilterControls: FC = ({ const chartLayoutItems = useChartLayoutItems(); const verboseMaps = useChartsVerboseMaps(); - const isCrossFiltersEnabled = isFeatureEnabled( - FeatureFlag.DashboardCrossFilters, - ); const selectedCrossFilters = useMemo( () => - isCrossFiltersEnabled - ? crossFiltersSelector({ - dataMask, - chartIds, - chartLayoutItems, - verboseMaps, - }) - : EMPTY_ARRAY, - [chartIds, chartLayoutItems, dataMask, isCrossFiltersEnabled, verboseMaps], + crossFiltersSelector({ + dataMask, + chartIds, + chartLayoutItems, + verboseMaps, + }), + [chartIds, chartLayoutItems, dataMask, verboseMaps], ); const { filterControlFactory, filtersWithValues } = useFilterControlFactory( dataMaskSelected, diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx index 0e5b82aed2651..13761afc4a930 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx @@ -18,13 +18,7 @@ */ import { FC, memo, useMemo } from 'react'; -import { - DataMaskStateWithId, - FeatureFlag, - isFeatureEnabled, - styled, - t, -} from '@superset-ui/core'; +import { DataMaskStateWithId, styled, t } from '@superset-ui/core'; import Loading from 'src/components/Loading'; import { RootState } from 'src/dashboard/types'; import { useChartLayoutItems } from 'src/dashboard/util/useChartLayoutItems'; @@ -35,7 +29,6 @@ import { useChartsVerboseMaps, getFilterBarTestId } from './utils'; import { HorizontalBarProps } from './types'; import FilterBarSettings from './FilterBarSettings'; import crossFiltersSelector from './CrossFilters/selectors'; -import { CrossFilterIndicator } from '../selectors'; const HorizontalBar = styled.div` ${({ theme }) => ` @@ -72,7 +65,6 @@ const FilterBarEmptyStateContainer = styled.div` `} `; -const EMPTY_ARRAY: CrossFilterIndicator[] = []; const HorizontalFilterBar: FC = ({ actions, dataMaskSelected, @@ -85,22 +77,17 @@ const HorizontalFilterBar: FC = ({ ); const chartIds = useChartIds(); const chartLayoutItems = useChartLayoutItems(); - const isCrossFiltersEnabled = isFeatureEnabled( - FeatureFlag.DashboardCrossFilters, - ); const verboseMaps = useChartsVerboseMaps(); const selectedCrossFilters = useMemo( () => - isCrossFiltersEnabled - ? crossFiltersSelector({ - dataMask, - chartIds, - chartLayoutItems, - verboseMaps, - }) - : EMPTY_ARRAY, - [chartIds, chartLayoutItems, dataMask, isCrossFiltersEnabled, verboseMaps], + crossFiltersSelector({ + dataMask, + chartIds, + chartLayoutItems, + verboseMaps, + }), + [chartIds, chartLayoutItems, dataMask, verboseMaps], ); const hasFilters = filterValues.length > 0 || selectedCrossFilters.length > 0; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/HorizontalFilterBar.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/HorizontalFilterBar.test.tsx index 3201ffb1a547d..cef4b2ba46a52 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/HorizontalFilterBar.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/HorizontalFilterBar.test.tsx @@ -35,6 +35,9 @@ const renderWrapper = (overrideProps?: Record) => render(, { useRedux: true, initialState: { + dashboardState: { + sliceIds: [], + }, dashboardInfo: { dash_edit_perm: true, }, diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx index d6960ad37027f..686320c81cbb9 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx @@ -30,7 +30,7 @@ import { FC, } from 'react'; import cx from 'classnames'; -import { FeatureFlag, isFeatureEnabled, styled, t } from '@superset-ui/core'; +import { styled, t } from '@superset-ui/core'; import Icons from 'src/components/Icons'; import Loading from 'src/components/Loading'; import { EmptyState } from 'src/components/EmptyState'; @@ -191,14 +191,6 @@ const VerticalFilterBar: FC = ({ [canEdit, dataMaskSelected, filterValues.length, onSelectionChange], ); - const crossFilters = useMemo( - () => - isFeatureEnabled(FeatureFlag.DashboardCrossFilters) ? ( - - ) : null, - [], - ); - return ( = ({ ) : (
<> - {crossFilters} + {filterControls}
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts b/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts index 232ff57de2ffe..0539a4317a77e 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts @@ -21,11 +21,9 @@ import { DataMaskStateWithId, DataMaskType, ensureIsArray, - FeatureFlag, Filters, FilterState, getColumnLabel, - isFeatureEnabled, NativeFilterType, NO_TIME_RANGE, QueryFormColumn, @@ -289,39 +287,37 @@ export const selectChartCrossFilters = ( filterEmitter = false, ): Indicator[] | CrossFilterIndicator[] => { let crossFilterIndicators: any = []; - if (isFeatureEnabled(FeatureFlag.DashboardCrossFilters)) { - crossFilterIndicators = Object.values(chartConfiguration) - .filter(chartConfig => { - const inScope = - chartConfig.crossFilters?.chartsInScope?.includes(chartId); - if (!filterEmitter && inScope) { - return true; - } - if (filterEmitter && !inScope) { - return true; - } - return false; - }) - .map(chartConfig => { - const filterIndicator = getCrossFilterIndicator( - Number(chartConfig.id), - dataMask[chartConfig.id], - chartLayoutItems, - ); - const filterStatus = getStatus({ - label: filterIndicator.value, - column: filterIndicator.column - ? getColumnLabel(filterIndicator.column) - : undefined, - type: DataMaskType.CrossFilters, - appliedColumns, - rejectedColumns, - }); + crossFilterIndicators = Object.values(chartConfiguration) + .filter(chartConfig => { + const inScope = + chartConfig.crossFilters?.chartsInScope?.includes(chartId); + if (!filterEmitter && inScope) { + return true; + } + if (filterEmitter && !inScope) { + return true; + } + return false; + }) + .map(chartConfig => { + const filterIndicator = getCrossFilterIndicator( + Number(chartConfig.id), + dataMask[chartConfig.id], + chartLayoutItems, + ); + const filterStatus = getStatus({ + label: filterIndicator.value, + column: filterIndicator.column + ? getColumnLabel(filterIndicator.column) + : undefined, + type: DataMaskType.CrossFilters, + appliedColumns, + rejectedColumns, + }); - return { ...filterIndicator, status: filterStatus }; - }) - .filter(filter => filter.status === IndicatorStatus.CrossFilterApplied); - } + return { ...filterIndicator, status: filterStatus }; + }) + .filter(filter => filter.status === IndicatorStatus.CrossFilterApplied); return crossFilterIndicators; }; @@ -379,16 +375,14 @@ export const selectNativeIndicatorsForChart = ( }); let crossFilterIndicators: any = []; - if (isFeatureEnabled(FeatureFlag.DashboardCrossFilters)) { - crossFilterIndicators = selectChartCrossFilters( - dataMask, - chartId, - chartLayoutItems, - chartConfiguration, - appliedColumns, - rejectedColumns, - ); - } + crossFilterIndicators = selectChartCrossFilters( + dataMask, + chartId, + chartLayoutItems, + chartConfiguration, + appliedColumns, + rejectedColumns, + ); const indicators = crossFilterIndicators.concat(nativeFilterIndicators); cachedNativeIndicatorsForChart[chartId] = indicators; cachedNativeFilterDataForChart[chartId] = { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts index a5611e394d3b1..17abcf993ac57 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts @@ -16,75 +16,28 @@ * specific language governing permissions and limitations * under the License. */ -import { Behavior, FeatureFlag, isFeatureEnabled } from '@superset-ui/core'; +import { Behavior } from '@superset-ui/core'; import { DashboardLayout } from 'src/dashboard/types'; import { CHART_TYPE } from 'src/dashboard/util/componentTypes'; import { nativeFilterGate, findTabsWithChartsInScope } from './utils'; -jest.mock('@superset-ui/core', () => ({ - ...jest.requireActual('@superset-ui/core'), - isFeatureEnabled: jest.fn(), -})); - -const mockedIsFeatureEnabled = isFeatureEnabled as jest.Mock; - describe('nativeFilterGate', () => { - describe('with all feature flags disabled', () => { - beforeAll(() => { - mockedIsFeatureEnabled.mockImplementation(() => false); - }); - - afterAll(() => { - mockedIsFeatureEnabled.mockRestore(); - }); - - it('should return true for regular chart', () => { - expect(nativeFilterGate([])).toEqual(true); - }); - - it('should return true for cross filter chart', () => { - expect(nativeFilterGate([Behavior.InteractiveChart])).toEqual(true); - }); - - it('should return false for native filter chart with cross filter support', () => { - expect( - nativeFilterGate([Behavior.NativeFilter, Behavior.InteractiveChart]), - ).toEqual(false); - }); - - it('should return false for native filter behavior', () => { - expect(nativeFilterGate([Behavior.NativeFilter])).toEqual(false); - }); + it('should return true for regular chart', () => { + expect(nativeFilterGate([])).toEqual(true); }); - describe('with cross filters and experimental feature flag enabled', () => { - beforeAll(() => { - mockedIsFeatureEnabled.mockImplementation((featureFlag: FeatureFlag) => - [FeatureFlag.DashboardCrossFilters].includes(featureFlag), - ); - }); - - afterAll(() => { - mockedIsFeatureEnabled.mockRestore(); - }); - - it('should return true for regular chart', () => { - expect(nativeFilterGate([])).toEqual(true); - }); - - it('should return true for cross filter chart', () => { - expect(nativeFilterGate([Behavior.InteractiveChart])).toEqual(true); - }); + it('should return true for cross filter chart', () => { + expect(nativeFilterGate([Behavior.InteractiveChart])).toEqual(true); + }); - it('should return true for native filter chart with cross filter support', () => { - expect( - nativeFilterGate([Behavior.NativeFilter, Behavior.InteractiveChart]), - ).toEqual(true); - }); + it('should return true for native filter chart with cross filter support', () => { + expect( + nativeFilterGate([Behavior.NativeFilter, Behavior.InteractiveChart]), + ).toEqual(true); + }); - it('should return false for native filter behavior', () => { - expect(nativeFilterGate([Behavior.NativeFilter])).toEqual(false); - }); + it('should return false for native filter behavior', () => { + expect(nativeFilterGate([Behavior.NativeFilter])).toEqual(false); }); }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts index 734e0ce91fe9d..ef577b354b40a 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts @@ -23,8 +23,6 @@ import { EXTRA_FORM_DATA_APPEND_KEYS, EXTRA_FORM_DATA_OVERRIDE_KEYS, ExtraFormData, - isFeatureEnabled, - FeatureFlag, Filter, getChartMetadataRegistry, QueryFormData, @@ -150,8 +148,7 @@ export function getExtraFormData( export function nativeFilterGate(behaviors: Behavior[]): boolean { return ( !behaviors.includes(Behavior.NativeFilter) || - (isFeatureEnabled(FeatureFlag.DashboardCrossFilters) && - behaviors.includes(Behavior.InteractiveChart)) + behaviors.includes(Behavior.InteractiveChart) ); } diff --git a/superset-frontend/src/dashboard/util/crossFilters.test.ts b/superset-frontend/src/dashboard/util/crossFilters.test.ts index 3323a11efea31..8523719ef4e4e 100644 --- a/superset-frontend/src/dashboard/util/crossFilters.test.ts +++ b/superset-frontend/src/dashboard/util/crossFilters.test.ts @@ -16,12 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { - Behavior, - FeatureFlag, - getChartMetadataRegistry, - VizType, -} from '@superset-ui/core'; +import { Behavior, getChartMetadataRegistry, VizType } from '@superset-ui/core'; import { getCrossFiltersConfiguration } from './crossFilters'; import { DEFAULT_CROSS_FILTER_SCOPING } from '../constants'; @@ -152,11 +147,6 @@ afterEach(() => { }); test('Generate correct cross filters configuration without initial configuration', () => { - // @ts-ignore - global.featureFlags = { - [FeatureFlag.DashboardCrossFilters]: true, - }; - // @ts-ignore expect(getCrossFiltersConfiguration(DASHBOARD_LAYOUT, {}, CHARTS)).toEqual({ chartConfiguration: { @@ -186,11 +176,6 @@ test('Generate correct cross filters configuration without initial configuration }); test('Generate correct cross filters configuration with initial configuration', () => { - // @ts-ignore - global.featureFlags = { - [FeatureFlag.DashboardCrossFilters]: true, - }; - expect( getCrossFiltersConfiguration( DASHBOARD_LAYOUT, @@ -227,25 +212,7 @@ test('Generate correct cross filters configuration with initial configuration', }); }); -test('Return undefined if DASHBOARD_CROSS_FILTERS feature flag is disabled', () => { - // @ts-ignore - global.featureFlags = { - [FeatureFlag.DashboardCrossFilters]: false, - }; - expect( - getCrossFiltersConfiguration( - DASHBOARD_LAYOUT, - CHART_CONFIG_METADATA, - CHARTS, - ), - ).toEqual(undefined); -}); - test('Recalculate charts in global filter scope when charts change', () => { - // @ts-ignore - global.featureFlags = { - [FeatureFlag.DashboardCrossFilters]: true, - }; expect( getCrossFiltersConfiguration( { diff --git a/superset-frontend/src/dashboard/util/crossFilters.ts b/superset-frontend/src/dashboard/util/crossFilters.ts index 123435a430a1f..04d3e368fc0c9 100644 --- a/superset-frontend/src/dashboard/util/crossFilters.ts +++ b/superset-frontend/src/dashboard/util/crossFilters.ts @@ -19,10 +19,8 @@ import { cloneDeep } from 'lodash'; import { Behavior, - FeatureFlag, getChartMetadataRegistry, isDefined, - isFeatureEnabled, } from '@superset-ui/core'; import { getChartIdsInFilterScope } from './getChartIdsInFilterScope'; import { @@ -38,8 +36,7 @@ import { CHART_TYPE } from './componentTypes'; export const isCrossFiltersEnabled = ( metadataCrossFiltersEnabled: boolean | undefined, ): boolean => - isFeatureEnabled(FeatureFlag.DashboardCrossFilters) && - (metadataCrossFiltersEnabled === undefined || metadataCrossFiltersEnabled); + metadataCrossFiltersEnabled === undefined || metadataCrossFiltersEnabled; export const getCrossFiltersConfiguration = ( dashboardLayout: DashboardLayout, @@ -49,10 +46,6 @@ export const getCrossFiltersConfiguration = ( >, charts: ChartsState, ) => { - if (!isFeatureEnabled(FeatureFlag.DashboardCrossFilters)) { - return undefined; - } - const chartLayoutItems = Object.values(dashboardLayout).filter( item => item?.type === CHART_TYPE, ); diff --git a/superset-frontend/src/dataMask/reducer.ts b/superset-frontend/src/dataMask/reducer.ts index b79f17fb1dbec..6d240b498953e 100644 --- a/superset-frontend/src/dataMask/reducer.ts +++ b/superset-frontend/src/dataMask/reducer.ts @@ -24,8 +24,6 @@ import { DataMask, DataMaskStateWithId, DataMaskWithId, - isFeatureEnabled, - FeatureFlag, Filter, FilterConfiguration, Filters, @@ -148,16 +146,14 @@ const dataMaskReducer = produce( // TODO: update hydrate to .ts // @ts-ignore case HYDRATE_DASHBOARD: - if (isFeatureEnabled(FeatureFlag.DashboardCrossFilters)) { - Object.keys( - // @ts-ignore - action.data.dashboardInfo?.metadata?.chart_configuration, - ).forEach(id => { - cleanState[id] = { - ...getInitialDataMask(id), // take initial data - }; - }); - } + Object.keys( + // @ts-ignore + action.data.dashboardInfo?.metadata?.chart_configuration, + ).forEach(id => { + cleanState[id] = { + ...getInitialDataMask(id), // take initial data + }; + }); fillNativeFilters( // @ts-ignore action.data.dashboardInfo?.metadata?.native_filter_configuration ?? diff --git a/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx b/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx index fdb81145cbf67..b6b3e208bd493 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx @@ -314,15 +314,12 @@ const databaseFixture: DatabaseObject = { }; describe('DatabaseModal', () => { - const renderAndWait = async () => { - const mounted = act(async () => { + const renderAndWait = async () => + waitFor(() => render(, { useRedux: true, - }); - }); - - return mounted; - }; + }), + ); beforeEach(async () => { await renderAndWait(); diff --git a/superset/config.py b/superset/config.py index 7c1436bb50502..7f6b24f68425c 100644 --- a/superset/config.py +++ b/superset/config.py @@ -487,7 +487,6 @@ class D3TimeFormat(TypedDict, total=False): "LISTVIEWS_DEFAULT_CARD_VIEW": False, # When True, this escapes HTML (rather than rendering it) in Markdown components "ESCAPE_MARKDOWN_HTML": False, - "DASHBOARD_CROSS_FILTERS": True, # deprecated "DASHBOARD_VIRTUALIZATION": True, # This feature flag is stil in beta and is not recommended for production use. "GLOBAL_ASYNC_QUERIES": False,