Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Removes the deprecated DASHBOARD_NATIVE_FILTERS feature flag #26329

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 =
michael-s-molina marked this conversation as resolved.
Show resolved Hide resolved
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
Loading