Skip to content

Commit

Permalink
Revert "feat(native_filter_migration): add transition mode (#16992)"
Browse files Browse the repository at this point in the history
This reverts commit 7d22c9c.
  • Loading branch information
john-bodley committed Feb 23, 2023
1 parent 95eb8d7 commit 606461f
Show file tree
Hide file tree
Showing 23 changed files with 60 additions and 933 deletions.
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

0 comments on commit 606461f

Please sign in to comment.