fix: Incorrect dependency between filters related feature flags (#24608)

apache · Jul 7, 2023 · 781a204 · 781a204
1 parent 5725627
commit 781a204
Show file tree

Hide file tree

Showing 8 changed files with 73 additions and 27 deletions.
diff --git 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<DashboardBuilderProps> = () => {
     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
@@ -76,6 +76,12 @@ const getModalTestId = testWithId<string>(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/...ontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx b/...ontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx
@@ -212,20 +212,23 @@ const FilterControls: FC<FilterControlsProps> = ({
         selectedCrossFilters.at(-1),
       ),
     }));
-    const nativeFiltersInScope = filtersInScope.map((filter, index) => ({
-      id: filter.id,
-      element: (
-        <div
-          className="filter-item-wrapper"
-          css={css`
-            flex-shrink: 0;
-          `}
-        >
-          {renderer(filter, index)}
-        </div>
-      ),
-    }));
-    return [...crossFilters, ...nativeFiltersInScope];
+    if (isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS)) {
+      const nativeFiltersInScope = filtersInScope.map((filter, index) => ({
+        id: filter.id,
+        element: (
+          <div
+            className="filter-item-wrapper"
+            css={css`
+              flex-shrink: 0;
+            `}
+          >
+            {renderer(filter, index)}
+          </div>
+        ),
+      }));
+      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
@@ -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<HeaderProps> = ({ toggleFiltersBar }) => {
           <Icons.Expand iconColor={theme.colors.grayscale.base} />
         </HeaderButton>
       </TitleArea>
-      {canEdit && (
+      {canEdit && isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) && (
         <AddFiltersButtonContainer>
           <FilterConfigurationLink
             dashboardId={dashboardId}

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx
@@ -17,7 +17,7 @@
  * under the License.
  */
 
-import React from 'react';
+import React, { useMemo } from 'react';
 import {
   DataMaskStateWithId,
   FeatureFlag,
@@ -129,6 +129,12 @@ const HorizontalFilterBar: React.FC<HorizontalBarProps> = ({
     : [];
   const hasFilters = filterValues.length > 0 || selectedCrossFilters.length > 0;
 
+  const actionsElement = useMemo(
+    () =>
+      isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) ? actions : null,
+    [actions],
+  );
+
   return (
     <HorizontalBar {...getFilterBarTestId()}>
       <HorizontalBarContent>
@@ -137,7 +143,7 @@ const HorizontalFilterBar: React.FC<HorizontalBarProps> = ({
         ) : (
           <>
             <FilterBarSettings />
-            {canEdit && (
+            {canEdit && isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) && (
               <FiltersLinkContainer hasFilters={hasFilters}>
                 <FilterConfigurationLink
                   dashboardId={dashboardId}
@@ -158,7 +164,7 @@ const HorizontalFilterBar: React.FC<HorizontalBarProps> = ({
                 onFilterSelectionChange={onSelectionChange}
               />
             )}
-            {actions}
+            {actionsElement}
           </>
         )}
       </HorizontalBarContent>

diff --git a/...et-frontend/src/dashboard/components/nativeFilters/FilterBar/HorizontalFilterBar.test.tsx b/...et-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<string, any>) =>
   waitFor(() =>
     render(<HorizontalBar {...defaultProps} {...overrideProps} />, {

diff --git 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<VerticalBarProps> = ({
     [],
   );
 
+  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 (
     <FilterBarScrollContext.Provider value={isScrolling}>
       <BarWrapper
@@ -315,7 +326,7 @@ const VerticalFilterBar: React.FC<VerticalBarProps> = ({
             <div css={{ height }}>
               <Loading />
             </div>
-          ) : isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET) ? (
+          ) : filterSetEnabled ? (
             <>
               {crossFilters}
               {filterSetsTabs}
@@ -324,11 +335,12 @@ const VerticalFilterBar: React.FC<VerticalBarProps> = ({
             <div css={tabPaneStyle} onScroll={onScroll}>
               <>
                 {crossFilters}
-                {filterControls}
+                {isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) &&
+                  filterControls}
               </>
             </div>
           )}
-          {actions}
+          {actionsElement}
         </Bar>
       </BarWrapper>
     </FilterBarScrollContext.Provider>

diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx
@@ -167,6 +167,11 @@ export const DashboardPage: FC<PageProps> = ({ 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<PageProps> = ({ 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));
           }