Skip to content

Commit

Permalink
feat(native_filter_migration): add transition mode (#16992)
Browse files Browse the repository at this point in the history
* feat: [Migrate filter_box to filter component] add transition mode

* rebase and fix comments

* rebase and fix commnent -- patch 2
  • Loading branch information
Grace Guo committed Nov 9, 2021
1 parent 6b1de57 commit 7d22c9c
Show file tree
Hide file tree
Showing 27 changed files with 1,132 additions and 78 deletions.
23 changes: 20 additions & 3 deletions superset-frontend/src/chart/Chart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ const propTypes = {
vizType: PropTypes.string.isRequired,
triggerRender: PropTypes.bool,
isFiltersInitialized: PropTypes.bool,
isDeactivatedViz: PropTypes.bool,
// state
chartAlert: PropTypes.string,
chartStatus: PropTypes.string,
Expand Down Expand Up @@ -82,6 +83,7 @@ const defaultProps = {
triggerRender: false,
dashboardId: null,
chartStackTrace: null,
isDeactivatedViz: false,
};

const Styles = styled.div`
Expand Down Expand Up @@ -114,13 +116,25 @@ class Chart extends React.PureComponent {
}

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

componentDidUpdate() {
if (this.props.triggerQuery) {
// 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;
}
this.runQuery();
}
}
Expand Down Expand Up @@ -221,6 +235,8 @@ class Chart extends React.PureComponent {
onQuery,
refreshOverlayVisible,
queriesResponse = [],
isDeactivatedViz = false,
width,
} = this.props;

const isLoading = chartStatus === 'loading';
Expand Down Expand Up @@ -250,6 +266,7 @@ class Chart extends React.PureComponent {
className="chart-container"
data-test="chart-container"
height={height}
width={width}
>
<div
className={`slice_container ${isFaded ? ' faded' : ''}`}
Expand All @@ -266,7 +283,7 @@ class Chart extends React.PureComponent {
</RefreshOverlayWrapper>
)}

{isLoading && <Loading />}
{isLoading && !isDeactivatedViz && <Loading />}
</Styles>
</ErrorBoundary>
);
Expand Down
16 changes: 12 additions & 4 deletions superset-frontend/src/common/hooks/apiResources/apiResources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,18 @@ export function useTransformedResource<IN, OUT>(
// While incomplete, there is no result - no need to transform.
return resource;
}
return {
...resource,
result: transformFn(resource.result),
};
try {
return {
...resource,
result: transformFn(resource.result),
};
} catch (e) {
return {
status: ResourceStatus.ERROR,
result: null,
error: e,
};
}
}, [resource, transformFn]);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ export const useDashboard = (idOrSlug: string | number) =>
useApiV1Resource<Dashboard>(`/api/v1/dashboard/${idOrSlug}`),
dashboard => ({
...dashboard,
metadata: dashboard.json_metadata && JSON.parse(dashboard.json_metadata),
metadata:
(dashboard.json_metadata && JSON.parse(dashboard.json_metadata)) || {},
position_data:
dashboard.position_json && JSON.parse(dashboard.position_json),
}),
Expand Down
4 changes: 4 additions & 0 deletions superset-frontend/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ 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
72 changes: 47 additions & 25 deletions superset-frontend/src/dashboard/actions/hydrate.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,21 @@ 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 { FeatureFlag, isFeatureEnabled } from '../../featureFlags';
import extractUrlParams from '../util/extractUrlParams';
import getNativeFilterConfig from '../util/filterboxMigrationHelper';

export const HYDRATE_DASHBOARD = 'HYDRATE_DASHBOARD';

export const hydrateDashboard = (dashboardData, chartData) => (
dispatch,
getState,
) => {
export const hydrateDashboard = (
dashboardData,
chartData,
filterboxMigrationState = FILTER_BOX_MIGRATION_STATES.NOOP,
) => (dispatch, getState) => {
const { user, common } = getState();
let { metadata } = dashboardData;

const { metadata } = dashboardData;
const regularUrlParams = extractUrlParams('regular');
const reservedUrlParams = extractUrlParams('reserved');
const editMode = reservedUrlParams.edit === 'true';
Expand Down Expand Up @@ -227,19 +231,25 @@ export const hydrateDashboard = (dashboardData, chartData) => (
const componentId = chartIdToLayoutId[key];
const directPathToFilter = (layout[componentId].parents || []).slice();
directPathToFilter.push(componentId);
dashboardFilters[key] = {
...dashboardFilter,
chartId: key,
componentId,
datasourceId: slice.form_data.datasource,
filterName: slice.slice_name,
directPathToFilter,
columns,
labels,
scopes: scopesByChartId,
isInstantFilter: !!slice.form_data.instant_filtering,
isDateFilter: Object.keys(columns).includes(TIME_RANGE),
};
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),
};
}
}

// sync layout names with current slice names in case a slice was edited
Expand Down Expand Up @@ -278,17 +288,28 @@ export const hydrateDashboard = (dashboardData, chartData) => (
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(
chartData,
filterScopes,
preselectFilters,
);
metadata.native_filter_configuration = filterConfig;
metadata.show_native_filters = true;
}
const nativeFilters = getInitialNativeFilterState({
filterConfig: metadata?.native_filter_configuration || [],
filterConfig,
});

if (!metadata) {
metadata = {};
}

metadata.show_native_filters =
dashboardData?.metadata?.show_native_filters ??
isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS);
(isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) &&
[
FILTER_BOX_MIGRATION_STATES.CONVERTED,
FILTER_BOX_MIGRATION_STATES.REVIEWING,
FILTER_BOX_MIGRATION_STATES.NOOP,
].includes(filterboxMigrationState));

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 @@ -379,6 +400,7 @@ export const hydrateDashboard = (dashboardData, chartData) => (
lastModifiedTime: dashboardData.changed_on,
isRefreshing: false,
activeTabs: [],
filterboxMigrationState,
},
dashboardLayout,
},
Expand Down
9 changes: 7 additions & 2 deletions superset-frontend/src/dashboard/actions/sliceEntities.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function fetchAllSlicesFailed(error) {
}

const FETCH_SLICES_PAGE_SIZE = 200;
export function fetchAllSlices(userId) {
export function fetchAllSlices(userId, excludeFilterBox = false) {
return (dispatch, getState) => {
const { sliceEntities } = getState();
if (sliceEntities.lastUpdated === 0) {
Expand Down Expand Up @@ -71,7 +71,12 @@ export function fetchAllSlices(userId) {
})
.then(({ json }) => {
const slices = {};
json.result.forEach(slice => {
let { result } = json;
// disable add filter_box viz to dashboard
if (excludeFilterBox) {
result = result.filter(slice => slice.viz_type !== 'filter_box');
}
result.forEach(slice => {
let form_data = JSON.parse(slice.params);
form_data = {
...form_data,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@
*/
import { useSelector } from 'react-redux';
import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
import { useCallback, useEffect, useState } from 'react';
import { useCallback, useEffect, useState, useContext } 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,
Expand All @@ -30,6 +31,7 @@ import { Filter } from '../nativeFilters/types';

// eslint-disable-next-line import/prefer-default-export
export const useNativeFilters = () => {
const filterboxMigrationState = useContext(MigrationContext);
const [isInitialized, setIsInitialized] = useState(false);
const [dashboardFiltersOpen, setDashboardFiltersOpen] = useState(
getUrlParam(URL_PARAMS.showFilters) ?? true,
Expand Down Expand Up @@ -74,12 +76,14 @@ export const useNativeFilters = () => {
useEffect(() => {
if (
filterValues.length === 0 &&
dashboardFiltersOpen &&
nativeFiltersEnabled
nativeFiltersEnabled &&
['CONVERTED', 'REVIEWING', 'NOOP'].includes(filterboxMigrationState)
) {
toggleDashboardFiltersOpen(false);
} else {
toggleDashboardFiltersOpen(true);
}
}, [filterValues.length]);
}, [filterValues.length, filterboxMigrationState]);

useEffect(() => {
if (showDashboard) {
Expand Down
100 changes: 100 additions & 0 deletions superset-frontend/src/dashboard/components/FilterBoxMigrationModal.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import React, { FunctionComponent } from 'react';
import { styled, t } from '@superset-ui/core';

import Modal from 'src/components/Modal';
import Button from 'src/components/Button';

const StyledFilterBoxMigrationModal = styled(Modal)`
.modal-content {
height: 900px;
display: flex;
flex-direction: column;
align-items: stretch;
}
.modal-header {
flex: 0 1 auto;
}
.modal-body {
flex: 1 1 auto;
overflow: auto;
}
.modal-footer {
flex: 0 1 auto;
}
.ant-modal-body {
overflow: auto;
}
`;

interface FilterBoxMigrationModalProps {
onHide: () => void;
onClickReview: () => void;
onClickSnooze: () => void;
show: boolean;
hideFooter: boolean;
}

const FilterBoxMigrationModal: FunctionComponent<FilterBoxMigrationModalProps> = ({
onClickReview,
onClickSnooze,
onHide,
show,
hideFooter = false,
}) => (
<StyledFilterBoxMigrationModal
show={show}
onHide={onHide}
title={t('Ready to review filters in this dashboard?')}
hideFooter={hideFooter}
footer={
<>
<Button buttonSize="small" onClick={onClickSnooze}>
{t('Remind me in 24 hours')}
</Button>
<Button buttonSize="small" onClick={onHide}>
{t('Cancel')}
</Button>
<Button
buttonSize="small"
buttonStyle="primary"
onClick={onClickReview}
>
{t('Start Review')}
</Button>
</>
}
responsive
>
<div>
{t(
'filter_box will be deprecated ' +
'in a future version of Superset. ' +
'Please replace filter_box by dashboard filter components.',
)}
</div>
</StyledFilterBoxMigrationModal>
);

export default FilterBoxMigrationModal;

0 comments on commit 7d22c9c

Please sign in to comment.