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

feat(cross-filters): add cross filters #12662

Merged
merged 13 commits into from
Feb 12, 2021
Merged
2 changes: 2 additions & 0 deletions superset-frontend/src/chart/ChartContainer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ import { bindActionCreators } from 'redux';
import * as actions from './chartAction';
import { logEvent } from '../logger/actions';
import Chart from './Chart';
import { setExtraFormData } from '../dashboard/actions/nativeFilters';

function mapDispatchToProps(dispatch) {
return {
actions: bindActionCreators(
{
...actions,
setExtraFormData,
logEvent,
},
dispatch,
Expand Down
2 changes: 2 additions & 0 deletions superset-frontend/src/chart/ChartRenderer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const propTypes = {
refreshOverlayVisible: PropTypes.bool,
// dashboard callbacks
addFilter: PropTypes.func,
setExtraFormData: PropTypes.func,
onFilterMenuOpen: PropTypes.func,
onFilterMenuClose: PropTypes.func,
};
Expand Down Expand Up @@ -73,6 +74,7 @@ class ChartRenderer extends React.Component {
setControlValue: this.handleSetControlValue,
onFilterMenuOpen: this.props.onFilterMenuOpen,
onFilterMenuClose: this.props.onFilterMenuClose,
setExtraFormData: (extraFormData) => this.props.actions?.setExtraFormData(this.props.chartId, extraFormData),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@
* specific language governing permissions and limitations
* under the License.
*/
import { ExtraFormData, QueryObject } from '@superset-ui/core';
import {
ExtraFormData,
getChartMetadataRegistry,
QueryObject,
} from '@superset-ui/core';
import { Charts, Layout, LayoutItem } from 'src/dashboard/types';
import {
CHART_TYPE,
Expand All @@ -33,6 +37,7 @@ import {
TreeItem,
} from './types';
import { DASHBOARD_ROOT_ID } from '../../util/constants';
import { FeatureFlag, isFeatureEnabled } from '../../../featureFlags';
simcha90 marked this conversation as resolved.
Show resolved Hide resolved

export const useForceUpdate = () => {
const [, updateState] = React.useState({});
Expand Down Expand Up @@ -196,13 +201,27 @@ export function mergeExtraFormData(

export function getExtraFormData(
nativeFilters: NativeFiltersState,
charts: Charts,
): ExtraFormData {
let extraFormData: ExtraFormData = {};
Object.keys(nativeFilters.filters).forEach(key => {
const filterState = nativeFilters.filtersState[key] || {};
const { extraFormData: newExtra = {} } = filterState;
extraFormData = mergeExtraFormData(extraFormData, newExtra);
});
if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) {
Object.entries(charts).forEach(([key, chart]) => {
const { isNativeFilter } = getChartMetadataRegistry().items[
chart?.formData?.viz_type
// @ts-ignore need export from superset-ui `ItemWithValue`
].value;
simcha90 marked this conversation as resolved.
Show resolved Hide resolved
if (isNativeFilter) {
const filterState = nativeFilters.filtersState[key] || {};
const { extraFormData: newExtra = {} } = filterState;
extraFormData = mergeExtraFormData(extraFormData, newExtra);
}
});
}
return extraFormData;
}

Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/dashboard/containers/Chart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ function mapStateToProps(
// note: this method caches filters if possible to prevent render cascades
const formData = getFormDataWithExtraFilters({
chart,
charts: chartQueries,
filters: getAppliedFilterValues(id),
colorScheme,
colorNamespace,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
CategoricalColorNamespace,
DataRecordFilters,
} from '@superset-ui/core';
import { ChartQueryPayload } from 'src/dashboard/types';
import { ChartQueryPayload, Charts } from 'src/dashboard/types';
import { NativeFiltersState } from 'src/dashboard/components/nativeFilters/types';
import { getExtraFormData } from 'src/dashboard/components/nativeFilters/utils';
import getEffectiveExtraFilters from './getEffectiveExtraFilters';
Expand All @@ -33,6 +33,7 @@ const cachedFormdataByChart = {};

interface GetFormDataWithExtraFiltersArguments {
chart: ChartQueryPayload;
charts: Charts;
filters: DataRecordFilters;
colorScheme?: string;
colorNamespace?: string;
Expand All @@ -45,6 +46,7 @@ interface GetFormDataWithExtraFiltersArguments {
// filters param only contains those applicable to this chart.
export default function getFormDataWithExtraFilters({
chart,
charts,
filters,
colorScheme,
colorNamespace,
Expand Down Expand Up @@ -73,7 +75,7 @@ export default function getFormDataWithExtraFilters({
...(colorScheme && { color_scheme: colorScheme }),
label_colors: labelColors,
extra_filters: getEffectiveExtraFilters(filters),
extra_form_data: getExtraFormData(nativeFilters),
extra_form_data: getExtraFormData(nativeFilters, charts),
};
cachedFiltersByChart[sliceId] = filters;
cachedFormdataByChart[sliceId] = formData;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import Label from 'src/components/Label';

import ControlHeader from '../ControlHeader';
import './VizTypeControl.less';
import { FeatureFlag, isFeatureEnabled } from '../../../featureFlags';

const propTypes = {
description: PropTypes.string,
Expand Down Expand Up @@ -166,15 +167,23 @@ const VizTypeControl = props => {
const filterString = filter.toLowerCase();

const filteredTypes = DEFAULT_ORDER.filter(type => registry.has(type))
.filter(type => !registry.get(type).isNativeFilter)
.filter(
type =>
isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) ||
Copy link
Contributor

@agatapst agatapst Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@simchanielsen could you explain why we need this feature flag enabled here (and below)?

Copy link
Contributor Author

@simcha90 simcha90 Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agatapst DASHBOARD_NATIVE_FILTERS feature flag filter out native filter charts from the view that responsible for creation of charts, because if they will be created as charts they will not can filter other charts, but with new feature of cross-filter functionality they will can to do it, so no need any more filter out them from this view. Because of that FeatureFlag.DASHBOARD_CROSS_FILTERS just cancel DASHBOARD_NATIVE_FILTERS flag

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, makes sense 🙂

!registry.get(type).isNativeFilter,
)
.map(type => ({
key: type,
value: registry.get(type),
}))
.concat(
registry
.entries()
.filter(entry => !entry.value.isNativeFilter)
.filter(
entry =>
isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) ||
!entry.value.isNativeFilter,
)
.filter(({ key }) => !typesWithDefaultOrder.has(key)),
)
.filter(entry => entry.value.name.toLowerCase().includes(filterString));
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/featureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export enum FeatureFlag {
DISPLAY_MARKDOWN_HTML = 'DISPLAY_MARKDOWN_HTML',
ESCAPE_MARKDOWN_HTML = 'ESCAPE_MARKDOWN_HTML',
DASHBOARD_NATIVE_FILTERS = 'DASHBOARD_NATIVE_FILTERS',
DASHBOARD_CROSS_FILTERS = 'DASHBOARD_CROSS_FILTERS',
VERSIONED_EXPORT = 'VERSIONED_EXPORT',
GLOBAL_ASYNC_QUERIES = 'GLOBAL_ASYNC_QUERIES',
ENABLE_TEMPLATE_PROCESSING = 'ENABLE_TEMPLATE_PROCESSING',
Expand Down
1 change: 1 addition & 0 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ def _try_json_readsha( # pylint: disable=unused-argument
# When True, this escapes HTML (rather than rendering it) in Markdown components
"ESCAPE_MARKDOWN_HTML": False,
"DASHBOARD_NATIVE_FILTERS": False,
"DASHBOARD_CROSS_FILTERS": False,
"GLOBAL_ASYNC_QUERIES": False,
"VERSIONED_EXPORT": False,
# Note that: RowLevelSecurityFilter is only given by default to the Admin role
Expand Down