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

chore: revert "feat(native_filter_migration): add transition mode (#16992)" #23144

Merged
merged 1 commit into from
Feb 23, 2023
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 @@ -56,7 +56,6 @@ These features are **finished** but currently being tested. They are usable, but
- DASHBOARD_FILTERS_EXPERIMENTAL
- DASHBOARD_NATIVE_FILTERS
- DYNAMIC_PLUGINS: [(docs)](https://superset.apache.org/docs/installation/running-on-kubernetes)
- ENABLE_FILTER_BOX_MIGRATION
- ENABLE_JAVASCRIPT_CONTROLS
- GENERIC_CHART_AXES
- GLOBAL_ASYNC_QUERIES [(docs)](https://github.com/apache/superset/blob/master/CONTRIBUTING.md#async-chart-queries)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ export enum FeatureFlag {
ENABLE_ADVANCED_DATA_TYPES = 'ENABLE_ADVANCED_DATA_TYPES',
ENABLE_DND_WITH_CLICK_UX = 'ENABLE_DND_WITH_CLICK_UX',
ENABLE_EXPLORE_DRAG_AND_DROP = 'ENABLE_EXPLORE_DRAG_AND_DROP',
ENABLE_FILTER_BOX_MIGRATION = 'ENABLE_FILTER_BOX_MIGRATION',
ENABLE_JAVASCRIPT_CONTROLS = 'ENABLE_JAVASCRIPT_CONTROLS',
ENABLE_TEMPLATE_PROCESSING = 'ENABLE_TEMPLATE_PROCESSING',
ENABLE_TEMPLATE_REMOVE_FILTERS = 'ENABLE_TEMPLATE_REMOVE_FILTERS',
Expand Down
21 changes: 3 additions & 18 deletions superset-frontend/src/components/Chart/Chart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ const propTypes = {
triggerRender: PropTypes.bool,
force: PropTypes.bool,
isFiltersInitialized: PropTypes.bool,
isDeactivatedViz: PropTypes.bool,
// state
chartAlert: PropTypes.string,
chartStatus: PropTypes.string,
Expand Down Expand Up @@ -94,7 +93,6 @@ const defaultProps = {
triggerRender: false,
dashboardId: null,
chartStackTrace: null,
isDeactivatedViz: false,
force: false,
isInView: true,
};
Expand Down Expand Up @@ -140,25 +138,13 @@ class Chart extends React.PureComponent {
}

componentDidMount() {
// during migration, hold chart queries before user choose review or cancel
if (
this.props.triggerQuery &&
this.props.filterboxMigrationState !== 'UNDECIDED'
) {
if (this.props.triggerQuery) {
this.runQuery();
}
}

componentDidUpdate() {
// during migration, hold chart queries before user choose review or cancel
if (
this.props.triggerQuery &&
this.props.filterboxMigrationState !== 'UNDECIDED'
) {
// if the chart is deactivated (filter_box), only load once
if (this.props.isDeactivatedViz && this.props.queriesResponse) {
return;
}
if (this.props.triggerQuery) {
this.runQuery();
}
}
Expand Down Expand Up @@ -261,7 +247,6 @@ class Chart extends React.PureComponent {
errorMessage,
chartIsStale,
queriesResponse = [],
isDeactivatedViz = false,
width,
} = this.props;

Expand Down Expand Up @@ -332,7 +317,7 @@ class Chart extends React.PureComponent {
<Loading />
)}
</div>
{isLoading && !isDeactivatedViz && <Loading />}
{isLoading && <Loading />}
</Styles>
</ErrorBoundary>
);
Expand Down
4 changes: 0 additions & 4 deletions superset-frontend/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ export const BOOL_TRUE_DISPLAY = 'True';
export const BOOL_FALSE_DISPLAY = 'False';

export const URL_PARAMS = {
migrationState: {
name: 'migration_state',
type: 'string',
},
standalone: {
name: 'standalone',
type: 'number',
Expand Down
67 changes: 17 additions & 50 deletions superset-frontend/src/dashboard/actions/hydrate.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,9 @@ import newComponentFactory from 'src/dashboard/util/newComponentFactory';
import { TIME_RANGE } from 'src/visualizations/FilterBox/FilterBox';
import { URL_PARAMS } from 'src/constants';
import { getUrlParam } from 'src/utils/urlUtils';
import { FILTER_BOX_MIGRATION_STATES } from 'src/explore/constants';
import { ResourceStatus } from 'src/hooks/apiResources/apiResources';
import { FeatureFlag, isFeatureEnabled } from '../../featureFlags';
import extractUrlParams from '../util/extractUrlParams';
import getNativeFilterConfig from '../util/filterboxMigrationHelper';
import { updateColorSchema } from './dashboardInfo';
import { getChartIdsInFilterScope } from '../util/getChartIdsInFilterScope';
import updateComponentParentsList from '../util/updateComponentParentsList';
Expand All @@ -63,14 +61,7 @@ import { FilterBarOrientation } from '../types';
export const HYDRATE_DASHBOARD = 'HYDRATE_DASHBOARD';

export const hydrateDashboard =
({
history,
dashboard,
charts,
filterboxMigrationState = FILTER_BOX_MIGRATION_STATES.NOOP,
dataMask,
activeTabs,
}) =>
({ history, dashboard, charts, dataMask, activeTabs }) =>
(dispatch, getState) => {
const { user, common, dashboardState } = getState();
const { metadata, position_data: positionData } = dashboard;
Expand Down Expand Up @@ -232,25 +223,18 @@ export const hydrateDashboard =
const componentId = chartIdToLayoutId[key];
const directPathToFilter = (layout[componentId].parents || []).slice();
directPathToFilter.push(componentId);
if (
[
FILTER_BOX_MIGRATION_STATES.NOOP,
FILTER_BOX_MIGRATION_STATES.SNOOZED,
].includes(filterboxMigrationState)
) {
dashboardFilters[key] = {
...dashboardFilter,
chartId: key,
componentId,
datasourceId: slice.form_data.datasource,
filterName: slice.slice_name,
directPathToFilter,
columns,
labels,
scopes: scopesByChartId,
isDateFilter: Object.keys(columns).includes(TIME_RANGE),
};
}
dashboardFilters[key] = {
...dashboardFilter,
chartId: key,
componentId,
datasourceId: slice.form_data.datasource,
filterName: slice.slice_name,
directPathToFilter,
columns,
labels,
scopes: scopesByChartId,
isDateFilter: Object.keys(columns).includes(TIME_RANGE),
};
}

// sync layout names with current slice names in case a slice was edited
Expand Down Expand Up @@ -319,28 +303,12 @@ export const hydrateDashboard =
directPathToChild.push(directLinkComponentId);
}

// should convert filter_box to filter component?
let filterConfig = metadata?.native_filter_configuration || [];
if (filterboxMigrationState === FILTER_BOX_MIGRATION_STATES.REVIEWING) {
filterConfig = getNativeFilterConfig(
charts,
filterScopes,
preselectFilters,
);
metadata.native_filter_configuration = filterConfig;
metadata.show_native_filters = true;
}
const nativeFilters = getInitialNativeFilterState({
filterConfig,
filterConfig: metadata?.native_filter_configuration || [],
});
metadata.show_native_filters =
dashboard?.metadata?.show_native_filters ??
(isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) &&
[
FILTER_BOX_MIGRATION_STATES.CONVERTED,
FILTER_BOX_MIGRATION_STATES.REVIEWING,
FILTER_BOX_MIGRATION_STATES.NOOP,
].includes(filterboxMigrationState));
metadata.show_native_filters = isFeatureEnabled(
FeatureFlag.DASHBOARD_NATIVE_FILTERS,
);

if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) {
// If user just added cross filter to dashboard it's not saving it scope on server,
Expand Down Expand Up @@ -465,7 +433,6 @@ export const hydrateDashboard =
isRefreshing: false,
isFiltersRefreshing: false,
activeTabs: activeTabs || dashboardState?.activeTabs || [],
filterboxMigrationState,
datasetsStatus: ResourceStatus.LOADING,
},
dashboardLayout,
Expand Down
32 changes: 6 additions & 26 deletions superset-frontend/src/dashboard/actions/sliceEntities.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ export function fetchAllSlicesFailed(error) {

export function fetchSlices(
userId,
excludeFilterBox,
dispatch,
filter_value,
sortColumn = 'changed_on',
Expand Down Expand Up @@ -84,11 +83,7 @@ export function fetchSlices(
})}`,
})
.then(({ json }) => {
let { result } = json;
// disable add filter_box viz to dashboard
if (excludeFilterBox) {
result = result.filter(slice => slice.viz_type !== 'filter_box');
}
const { result } = json;
result.forEach(slice => {
let form_data = JSON.parse(slice.params);
form_data = {
Expand Down Expand Up @@ -135,46 +130,31 @@ export function fetchSlices(
);
}

export function fetchAllSlices(userId, excludeFilterBox = false) {
export function fetchAllSlices(userId) {
return (dispatch, getState) => {
const { sliceEntities } = getState();
if (sliceEntities.lastUpdated === 0) {
dispatch(fetchAllSlicesStarted());
return fetchSlices(userId, excludeFilterBox, dispatch, undefined);
return fetchSlices(userId, dispatch, undefined);
}

return dispatch(setAllSlices(sliceEntities.slices));
};
}

export function fetchSortedSlices(
userId,
excludeFilterBox = false,
order_column,
) {
export function fetchSortedSlices(userId, order_column) {
return dispatch => {
dispatch(fetchAllSlicesStarted());
return fetchSlices(
userId,
excludeFilterBox,
dispatch,
undefined,
order_column,
);
return fetchSlices(userId, dispatch, undefined, order_column);
};
}

export function fetchFilteredSlices(
userId,
excludeFilterBox = false,
filter_value,
) {
export function fetchFilteredSlices(userId, filter_value) {
return (dispatch, getState) => {
dispatch(fetchAllSlicesStarted());
const { sliceEntities } = getState();
return fetchSlices(
userId,
excludeFilterBox,
dispatch,
filter_value,
undefined,
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/src/dashboard/actions/sliceEntities.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('slice entity actions', () => {
describe('fetchFilteredSlices', () => {
it('should dispatch an fetchAllSlicesStarted action', async () => {
const { dispatch, getState } = setup();
const thunk1 = fetchFilteredSlices('userId', false, 'filter_value');
const thunk1 = fetchFilteredSlices('userId', 'filter_value');
await thunk1(dispatch, getState);
expect(dispatch.getCall(0).args[0]).toEqual({
type: FETCH_ALL_SLICES_STARTED,
Expand All @@ -82,7 +82,7 @@ describe('slice entity actions', () => {
sliceEntities: { slices: {}, lastUpdated: 1 },
});

const thunk1 = fetchAllSlices('userId', false, 'filter_value');
const thunk1 = fetchAllSlices('userId', 'filter_value');
await thunk1(dispatch, getState);

expect(spy.get.callCount).toBe(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,17 @@
*/
import { useSelector } from 'react-redux';
import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
import { useCallback, useEffect, useState, useContext } from 'react';
import { useCallback, useEffect, useState } from 'react';
import { URL_PARAMS } from 'src/constants';
import { getUrlParam } from 'src/utils/urlUtils';
import { RootState } from 'src/dashboard/types';
import { MigrationContext } from 'src/dashboard/containers/DashboardPage';
import {
useFilters,
useNativeFiltersDataMask,
} from '../nativeFilters/FilterBar/state';

// eslint-disable-next-line import/prefer-default-export
export const useNativeFilters = () => {
const filterboxMigrationState = useContext(MigrationContext);
const [isInitialized, setIsInitialized] = useState(false);
const showNativeFilters = useSelector<RootState, boolean>(
state =>
Expand Down Expand Up @@ -78,15 +76,13 @@ export const useNativeFilters = () => {
useEffect(() => {
if (
expandFilters === false ||
(filterValues.length === 0 &&
nativeFiltersEnabled &&
['CONVERTED', 'REVIEWING', 'NOOP'].includes(filterboxMigrationState))
(filterValues.length === 0 && nativeFiltersEnabled)
) {
toggleDashboardFiltersOpen(false);
} else {
toggleDashboardFiltersOpen(true);
}
}, [filterValues.length, filterboxMigrationState]);
}, [filterValues.length]);

useEffect(() => {
if (showDashboard) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import downloadAsImage from 'src/utils/downloadAsImage';
import getDashboardUrl from 'src/dashboard/util/getDashboardUrl';
import { getActiveFilters } from 'src/dashboard/util/activeDashboardFilters';
import { getUrlParam } from 'src/utils/urlUtils';
import { FILTER_BOX_MIGRATION_STATES } from 'src/explore/constants';
import { LOG_ACTIONS_DASHBOARD_DOWNLOAD_AS_IMAGE } from 'src/logger/LogUtils';

const propTypes = {
Expand Down Expand Up @@ -70,19 +69,13 @@ const propTypes = {
refreshLimit: PropTypes.number,
refreshWarning: PropTypes.string,
lastModifiedTime: PropTypes.number.isRequired,
filterboxMigrationState: PropTypes.oneOf(
Object.keys(FILTER_BOX_MIGRATION_STATES).map(
key => FILTER_BOX_MIGRATION_STATES[key],
),
),
};

const defaultProps = {
colorNamespace: undefined,
colorScheme: undefined,
refreshLimit: 0,
refreshWarning: null,
filterboxMigrationState: FILTER_BOX_MIGRATION_STATES.NOOP,
};

const MENU_KEYS = {
Expand Down Expand Up @@ -230,7 +223,6 @@ class HeaderActionsDropdown extends React.PureComponent {
lastModifiedTime,
addSuccessToast,
addDangerToast,
filterboxMigrationState,
setIsDropdownVisible,
isDropdownVisible,
...rest
Expand Down Expand Up @@ -378,15 +370,14 @@ class HeaderActionsDropdown extends React.PureComponent {
</Menu>
)
) : null}
{editMode &&
filterboxMigrationState !== FILTER_BOX_MIGRATION_STATES.CONVERTED && (
<Menu.Item key={MENU_KEYS.SET_FILTER_MAPPING}>
<FilterScopeModal
className="m-r-5"
triggerNode={t('Set filter mapping')}
/>
</Menu.Item>
)}
{editMode && (
<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
Loading