From 781a20423a373408f847834100a15e1f9b15a276 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Fri, 7 Jul 2023 09:44:33 -0300 Subject: [PATCH] fix: Incorrect dependency between filters related feature flags (#24608) --- .../DashboardBuilder/DashboardBuilder.tsx | 8 +++-- .../FilterBar/FilterBar.test.tsx | 6 ++++ .../FilterControls/FilterControls.tsx | 31 ++++++++++--------- .../nativeFilters/FilterBar/Header/index.tsx | 11 +++++-- .../nativeFilters/FilterBar/Horizontal.tsx | 12 +++++-- .../FilterBar/HorizontalFilterBar.test.tsx | 7 ++++- .../nativeFilters/FilterBar/Vertical.tsx | 18 +++++++++-- .../dashboard/containers/DashboardPage.tsx | 7 ++++- 8 files changed, 73 insertions(+), 27 deletions(-) diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index ea0354334aaa3..3af76c5c1cc13 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -525,9 +525,11 @@ const DashboardBuilder: FC = () => { threshold: [1], }); - const filterSetEnabled = isFeatureEnabled( - FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET, - ); + // Filter sets depend on native filters + const filterSetEnabled = + isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET) && + isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS); + const showFilterBar = (crossFiltersEnabled || nativeFiltersEnabled) && !editMode; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx index d6428e0f002da..a38843ec46665 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx @@ -76,6 +76,12 @@ const getModalTestId = testWithId(FILTERS_CONFIG_MODAL_TEST_ID, true); const FILTER_NAME = 'Time filter 1'; const FILTER_SET_NAME = 'New filter set'; +// @ts-ignore +global.featureFlags = { + [FeatureFlag.DASHBOARD_NATIVE_FILTERS]: true, + [FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET]: true, +}; + const addFilterFlow = async () => { // open filter config modal userEvent.click(screen.getByTestId(getTestId('collapsable'))); 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 629ec75cb000c..c74d1b08a967e 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx @@ -212,20 +212,23 @@ const FilterControls: FC = ({ selectedCrossFilters.at(-1), ), })); - const nativeFiltersInScope = filtersInScope.map((filter, index) => ({ - id: filter.id, - element: ( -
- {renderer(filter, index)} -
- ), - })); - return [...crossFilters, ...nativeFiltersInScope]; + if (isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS)) { + const nativeFiltersInScope = filtersInScope.map((filter, index) => ({ + id: filter.id, + element: ( +
+ {renderer(filter, index)} +
+ ), + })); + return [...crossFilters, ...nativeFiltersInScope]; + } + return [...crossFilters]; }, [filtersInScope, renderer, rendererCrossFilter, selectedCrossFilters]); const renderHorizontalContent = () => ( diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx index 7cd4877d11e2c..d839cd4d39404 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx @@ -17,7 +17,14 @@ * under the License. */ /* eslint-disable no-param-reassign */ -import { css, styled, t, useTheme } from '@superset-ui/core'; +import { + FeatureFlag, + css, + isFeatureEnabled, + styled, + t, + useTheme, +} from '@superset-ui/core'; import React, { FC, useMemo } from 'react'; import Icons from 'src/components/Icons'; import Button from 'src/components/Button'; @@ -117,7 +124,7 @@ const Header: FC = ({ toggleFiltersBar }) => { - {canEdit && ( + {canEdit && isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) && ( = ({ : []; const hasFilters = filterValues.length > 0 || selectedCrossFilters.length > 0; + const actionsElement = useMemo( + () => + isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) ? actions : null, + [actions], + ); + return ( @@ -137,7 +143,7 @@ const HorizontalFilterBar: React.FC = ({ ) : ( <> - {canEdit && ( + {canEdit && isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) && ( = ({ onFilterSelectionChange={onSelectionChange} /> )} - {actions} + {actionsElement} )} 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 c0b2b7c199403..efad04a0388d7 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/HorizontalFilterBar.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/HorizontalFilterBar.test.tsx @@ -17,7 +17,7 @@ * under the License. */ -import { NativeFilterType } from '@superset-ui/core'; +import { FeatureFlag, NativeFilterType } from '@superset-ui/core'; import React from 'react'; import { render, screen, waitFor } from 'spec/helpers/testing-library'; import HorizontalBar from './Horizontal'; @@ -32,6 +32,11 @@ const defaultProps = { onSelectionChange: jest.fn(), }; +// @ts-ignore +global.featureFlags = { + [FeatureFlag.DASHBOARD_NATIVE_FILTERS]: true, +}; + const renderWrapper = (overrideProps?: Record) => waitFor(() => render(, { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx index 07b327e3b47d2..d9212f4058ce9 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx @@ -287,6 +287,17 @@ const VerticalFilterBar: React.FC = ({ [], ); + const actionsElement = useMemo( + () => + isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) ? actions : null, + [actions], + ); + + // Filter sets depend on native filters + const filterSetEnabled = + isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET) && + isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS); + return ( = ({
- ) : isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET) ? ( + ) : filterSetEnabled ? ( <> {crossFilters} {filterSetsTabs} @@ -324,11 +335,12 @@ const VerticalFilterBar: React.FC = ({
<> {crossFilters} - {filterControls} + {isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) && + filterControls}
)} - {actions} + {actionsElement}
diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index 90790872a2cea..aef0fb3b6e7c2 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -167,6 +167,11 @@ export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { const readyToRender = Boolean(dashboard && charts); const { dashboard_title, css, metadata, id = 0 } = dashboard || {}; + // Filter sets depend on native filters + const filterSetEnabled = + isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET) && + isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS); + useEffect(() => { // mark tab id as redundant when user closes browser tab - a new id will be // generated next time user opens a dashboard and the old one won't be reused @@ -216,7 +221,7 @@ export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { if (readyToRender) { if (!isDashboardHydrated.current) { isDashboardHydrated.current = true; - if (isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET)) { + if (filterSetEnabled) { // only initialize filterset once dispatch(getFilterSets(id)); }