Skip to content

Commit

Permalink
refactor: Removes the deprecated DASHBOARD_NATIVE_FILTERS feature flag (
Browse files Browse the repository at this point in the history
  • Loading branch information
michael-s-molina authored and sfirke committed Mar 22, 2024
1 parent bb6843f commit 130bce9
Show file tree
Hide file tree
Showing 22 changed files with 110 additions and 289 deletions.
1 change: 0 additions & 1 deletion RESOURCES/FEATURE_FLAGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ These features flags currently default to True and **will be removed in a future
[//]: # "PLEASE KEEP THE LIST SORTED ALPHABETICALLY"

- DASHBOARD_CROSS_FILTERS
- DASHBOARD_NATIVE_FILTERS
- ENABLE_JAVASCRIPT_CONTROLS
- GENERIC_CHART_AXES
- KV_STORE
1 change: 1 addition & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ assists people when migrating to a new version.
- [26637](https://github.com/apache/superset/issues/26637): Sets the `DRILL_BY` feature flag to `True` by default given that the feature has been tested for a while and reached a stable state.
- [26462](https://github.com/apache/superset/issues/26462): Removes the Profile feature given that it's not actively maintained and not widely used.
- [26377](https://github.com/apache/superset/pull/26377): Removes the deprecated Redirect API that supported short URLs used before the permalink feature.
- [26329](https://github.com/apache/superset/issues/26329): Removes the deprecated `DASHBOARD_NATIVE_FILTERS` feature flag. The previous value of the feature flag was `True` and now the feature is permanently enabled.

### Potential Downtime

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export enum FeatureFlag {
CONFIRM_DASHBOARD_DIFF = 'CONFIRM_DASHBOARD_DIFF',
/** @deprecated */
DASHBOARD_CROSS_FILTERS = 'DASHBOARD_CROSS_FILTERS',
DASHBOARD_NATIVE_FILTERS = 'DASHBOARD_NATIVE_FILTERS',
DASHBOARD_VIRTUALIZATION = 'DASHBOARD_VIRTUALIZATION',
DASHBOARD_RBAC = 'DASHBOARD_RBAC',
DATAPANEL_CLOSED_BY_DEFAULT = 'DATAPANEL_CLOSED_BY_DEFAULT',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,26 +21,22 @@ import {
ControlSetItem,
ControlSetRow,
} from '@superset-ui/chart-controls';
import { FeatureFlag, isFeatureEnabled, t } from '@superset-ui/core';
import { t } from '@superset-ui/core';
import { PAGE_SIZE_OPTIONS } from '../../consts';

export const serverPaginationControlSetRow: ControlSetRow =
isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) ||
isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS)
? [
{
name: 'server_pagination',
config: {
type: 'CheckboxControl',
label: t('Server pagination'),
description: t(
'Enable server side pagination of results (experimental feature)',
),
default: false,
},
},
]
: [];
export const serverPaginationControlSetRow: ControlSetRow = [
{
name: 'server_pagination',
config: {
type: 'CheckboxControl',
label: t('Server pagination'),
description: t(
'Enable server side pagination of results (experimental feature)',
),
default: false,
},
},
];

export const serverPageLengthControlSetItem: ControlSetItem = {
name: 'server_page_length',
Expand Down
31 changes: 13 additions & 18 deletions superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,9 @@ import React from 'react';
import {
ChartDataResponseResult,
ensureIsArray,
FeatureFlag,
GenericDataType,
hasGenericChartAxes,
isAdhocColumn,
isFeatureEnabled,
isPhysicalColumn,
QueryFormColumn,
QueryMode,
Expand Down Expand Up @@ -290,22 +288,19 @@ const config: ControlPanelConfig = {
},
},
],
isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) ||
isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS)
? [
{
name: 'server_pagination',
config: {
type: 'CheckboxControl',
label: t('Server pagination'),
description: t(
'Enable server side pagination of results (experimental feature)',
),
default: false,
},
},
]
: [],
[
{
name: 'server_pagination',
config: {
type: 'CheckboxControl',
label: t('Server pagination'),
description: t(
'Enable server side pagination of results (experimental feature)',
),
default: false,
},
},
],
[
{
name: 'row_limit',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import React from 'react';
import fetchMock from 'fetch-mock';
import { render } from 'spec/helpers/testing-library';
import { fireEvent, within } from '@testing-library/react';
import * as uiCore from '@superset-ui/core';
import DashboardBuilder from 'src/dashboard/components/DashboardBuilder/DashboardBuilder';
import useStoredSidebarWidth from 'src/components/ResizableSidebar/useStoredSidebarWidth';
import {
Expand Down Expand Up @@ -247,35 +246,20 @@ describe('DashboardBuilder', () => {
expect(await findByAltText('Loading...')).toBeVisible();
});

describe('when nativeFiltersEnabled', () => {
let isFeatureEnabledMock: jest.MockInstance<boolean, [string]>;
beforeAll(() => {
isFeatureEnabledMock = jest
.spyOn(uiCore, 'isFeatureEnabled')
.mockImplementation(
flag => flag === uiCore.FeatureFlag.DASHBOARD_NATIVE_FILTERS,
);
});

afterAll(() => {
isFeatureEnabledMock.mockRestore();
});

it('should set FilterBar width by useStoredSidebarWidth', () => {
const expectedValue = 200;
const setter = jest.fn();
(useStoredSidebarWidth as jest.Mock).mockImplementation(() => [
expectedValue,
setter,
]);
const { getByTestId } = setup({
dashboardInfo: {
...mockState.dashboardInfo,
dash_edit_perm: true,
},
});
const filterbar = getByTestId('dashboard-filters-panel');
expect(filterbar).toHaveStyleRule('width', `${expectedValue}px`);
it('should set FilterBar width by useStoredSidebarWidth', () => {
const expectedValue = 200;
const setter = jest.fn();
(useStoredSidebarWidth as jest.Mock).mockImplementation(() => [
expectedValue,
setter,
]);
const { getByTestId } = setup({
dashboardInfo: {
...mockState.dashboardInfo,
dash_edit_perm: true,
},
});
const filterbar = getByTestId('dashboard-filters-panel');
expect(filterbar).toHaveStyleRule('width', `${expectedValue}px`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,9 @@
import React, { FC, useCallback, useEffect, useMemo, useRef } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import {
FeatureFlag,
Filter,
Filters,
getCategoricalSchemeRegistry,
isFeatureEnabled,
SupersetClient,
useComponentDidUpdate,
} from '@superset-ui/core';
Expand Down Expand Up @@ -104,10 +102,7 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
}, [dashboardLayout, directPathToChild]);

useEffect(() => {
if (
!isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) ||
nativeFilterScopes.length === 0
) {
if (nativeFilterScopes.length === 0) {
return;
}
const scopes = nativeFilterScopes.map(filterScope => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
* under the License.
*/
import { useSelector } from 'react-redux';
import { isFeatureEnabled, FeatureFlag } from '@superset-ui/core';
import { useCallback, useEffect, useState } from 'react';
import { URL_PARAMS } from 'src/constants';
import { getUrlParam } from 'src/utils/urlUtils';
Expand All @@ -42,8 +41,7 @@ export const useNativeFilters = () => {
);

const nativeFiltersEnabled =
isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) &&
(canEdit || (!canEdit && filterValues.length !== 0));
canEdit || (!canEdit && filterValues.length !== 0);

const requiredFirstFilter = filterValues.filter(
filter => filter.requiredFirst,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,6 @@ describe('FiltersBadge', () => {
});

it('shows the indicator when filters have been applied', () => {
// @ts-ignore
global.featureFlags = {
[SupersetUI.FeatureFlag.DASHBOARD_NATIVE_FILTERS]: true,
};
const store = getMockStoreWithNativeFilters();
// start with basic dashboard state, dispatch an event to simulate query completion
store.dispatch({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,8 @@ import userEvent from '@testing-library/user-event';
import fetchMock from 'fetch-mock';
import { HeaderDropdownProps } from 'src/dashboard/components/Header/types';
import injectCustomCss from 'src/dashboard/util/injectCustomCss';
import { FeatureFlag } from '@superset-ui/core';
import * as uiCore from '@superset-ui/core';
import HeaderActionsDropdown from '.';

let isFeatureEnabledMock: jest.MockInstance<boolean, [feature: FeatureFlag]>;

const createProps = () => ({
addSuccessToast: jest.fn(),
addDangerToast: jest.fn(),
Expand Down Expand Up @@ -135,7 +131,7 @@ test('should render the menu items', async () => {

test('should render the menu items in edit mode', async () => {
setup(editModeOnProps);
expect(screen.getAllByRole('menuitem')).toHaveLength(5);
expect(screen.getAllByRole('menuitem')).toHaveLength(4);
expect(screen.getByText('Set auto-refresh interval')).toBeInTheDocument();
expect(screen.getByText('Edit properties')).toBeInTheDocument();
expect(screen.getByText('Edit CSS')).toBeInTheDocument();
Expand All @@ -150,56 +146,14 @@ test('should render the menu items in Embedded mode', async () => {
expect(screen.getByText('Set auto-refresh interval')).toBeInTheDocument();
});

describe('with native filters feature flag disabled', () => {
beforeAll(() => {
isFeatureEnabledMock = jest
.spyOn(uiCore, 'isFeatureEnabled')
.mockImplementation(
(featureFlag: FeatureFlag) =>
featureFlag !== FeatureFlag.DASHBOARD_NATIVE_FILTERS,
);
});

afterAll(() => {
// @ts-ignore
isFeatureEnabledMock.restore();
});

it('should render filter mapping in edit mode if explicit filter scopes undefined', async () => {
setup(editModeOnProps);
expect(screen.getByText('Set filter mapping')).toBeInTheDocument();
});

it('should render filter mapping in edit mode if explicit filter scopes defined', async () => {
setup(editModeOnWithFilterScopesProps);
expect(screen.getByText('Set filter mapping')).toBeInTheDocument();
});
test('should not render filter mapping in edit mode if explicit filter scopes undefined', async () => {
setup(editModeOnProps);
expect(screen.queryByText('Set filter mapping')).not.toBeInTheDocument();
});

describe('with native filters feature flag enabled', () => {
beforeAll(() => {
isFeatureEnabledMock = jest
.spyOn(uiCore, 'isFeatureEnabled')
.mockImplementation(
(featureFlag: FeatureFlag) =>
featureFlag === FeatureFlag.DASHBOARD_NATIVE_FILTERS,
);
});

afterAll(() => {
// @ts-ignore
isFeatureEnabledMock.restore();
});

it('should not render filter mapping in edit mode if explicit filter scopes undefined', async () => {
setup(editModeOnProps);
expect(screen.queryByText('Set filter mapping')).not.toBeInTheDocument();
});

it('should render filter mapping in edit mode if explicit filter scopes defined', async () => {
setup(editModeOnWithFilterScopesProps);
expect(screen.getByText('Set filter mapping')).toBeInTheDocument();
});
test('should render filter mapping in edit mode if explicit filter scopes defined', async () => {
setup(editModeOnWithFilterScopesProps);
expect(screen.getByText('Set filter mapping')).toBeInTheDocument();
});

test('should show the share actions', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import { isEmpty } from 'lodash';
import {
isFeatureEnabled,
FeatureFlag,
SupersetClient,
t,
} from '@superset-ui/core';
import { SupersetClient, t } from '@superset-ui/core';
import { Menu } from 'src/components/Menu';
import { URL_PARAMS } from 'src/constants';
import ShareMenuItems from 'src/dashboard/components/menu/ShareMenuItems';
Expand Down Expand Up @@ -361,18 +356,14 @@ class HeaderActionsDropdown extends React.PureComponent {
</Menu>
)
) : null}
{editMode &&
!(
isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) &&
isEmpty(dashboardInfo?.metadata?.filter_scopes)
) && (
<Menu.Item key={MENU_KEYS.SET_FILTER_MAPPING}>
<FilterScopeModal
className="m-r-5"
triggerNode={t('Set filter mapping')}
/>
</Menu.Item>
)}
{editMode && !isEmpty(dashboardInfo?.metadata?.filter_scopes) && (
<Menu.Item key={MENU_KEYS.SET_FILTER_MAPPING}>
<FilterScopeModal
className="m-r-5"
triggerNode={t('Set filter mapping')}
/>
</Menu.Item>
)}

<Menu.Item key={MENU_KEYS.AUTOREFRESH_MODAL}>
<RefreshIntervalModal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import userEvent from '@testing-library/user-event';
import { stateWithoutNativeFilters } from 'spec/fixtures/mockStore';
import * as mockCore from '@superset-ui/core';
import { testWithId } from 'src/utils/testUtils';
import { FeatureFlag, Preset } from '@superset-ui/core';
import { Preset } from '@superset-ui/core';
import { TimeFilterPlugin, SelectFilterPlugin } from 'src/filters/components';
import fetchMock from 'fetch-mock';
import { FilterBarOrientation } from 'src/dashboard/types';
Expand Down Expand Up @@ -73,11 +73,6 @@ const getModalTestId = testWithId<string>(FILTERS_CONFIG_MODAL_TEST_ID, true);

const FILTER_NAME = 'Time filter 1';

// @ts-ignore
global.featureFlags = {
[FeatureFlag.DASHBOARD_NATIVE_FILTERS]: true,
};

const addFilterFlow = async () => {
// open filter config modal
userEvent.click(screen.getByTestId(getTestId('collapsable')));
Expand Down Expand Up @@ -291,10 +286,6 @@ describe('FilterBar', () => {
});

it('create filter and apply it flow', async () => {
// @ts-ignore
global.featureFlags = {
[FeatureFlag.DASHBOARD_NATIVE_FILTERS]: true,
};
renderWrapper(openedBarProps, stateWithoutNativeFilters);
expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();

Expand Down
Loading

0 comments on commit 130bce9

Please sign in to comment.