From 65e5dfd1de1abc2b6b1c37dc10d205206fc40d86 Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Thu, 21 Jan 2021 16:36:57 +0200 Subject: [PATCH 1/8] feat: add cross filters --- .../src/chart/ChartContainer.jsx | 2 ++ superset-frontend/src/chart/ChartRenderer.jsx | 2 ++ .../components/nativeFilters/utils.ts | 21 ++++++++++++++++++- .../src/dashboard/containers/Chart.jsx | 1 + .../charts/getFormDataWithExtraFilters.ts | 6 ++++-- .../components/controls/VizTypeControl.jsx | 13 ++++++++++-- superset-frontend/src/featureFlags.ts | 1 + superset/config.py | 1 + 8 files changed, 42 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/chart/ChartContainer.jsx b/superset-frontend/src/chart/ChartContainer.jsx index f509281f3dc0b..ecc4a2d8a8fc0 100644 --- a/superset-frontend/src/chart/ChartContainer.jsx +++ b/superset-frontend/src/chart/ChartContainer.jsx @@ -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, diff --git a/superset-frontend/src/chart/ChartRenderer.jsx b/superset-frontend/src/chart/ChartRenderer.jsx index 0c21a3a5c0653..381c85f7f6be0 100644 --- a/superset-frontend/src/chart/ChartRenderer.jsx +++ b/superset-frontend/src/chart/ChartRenderer.jsx @@ -42,6 +42,7 @@ const propTypes = { refreshOverlayVisible: PropTypes.bool, // dashboard callbacks addFilter: PropTypes.func, + setExtraFormData: PropTypes.func, onFilterMenuOpen: PropTypes.func, onFilterMenuClose: PropTypes.func, }; @@ -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), }; } diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts index 7f89aee46a7de..872b761e8616e 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts @@ -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, @@ -33,6 +37,7 @@ import { TreeItem, } from './types'; import { DASHBOARD_ROOT_ID } from '../../util/constants'; +import { FeatureFlag, isFeatureEnabled } from '../../../featureFlags'; export const useForceUpdate = () => { const [, updateState] = React.useState({}); @@ -196,6 +201,7 @@ export function mergeExtraFormData( export function getExtraFormData( nativeFilters: NativeFiltersState, + charts: Charts, ): ExtraFormData { let extraFormData: ExtraFormData = {}; Object.keys(nativeFilters.filters).forEach(key => { @@ -203,6 +209,19 @@ export function getExtraFormData( 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; + if (isNativeFilter) { + const filterState = nativeFilters.filtersState[key] || {}; + const { extraFormData: newExtra = {} } = filterState; + extraFormData = mergeExtraFormData(extraFormData, newExtra); + } + }); + } return extraFormData; } diff --git a/superset-frontend/src/dashboard/containers/Chart.jsx b/superset-frontend/src/dashboard/containers/Chart.jsx index abd784a5c70f4..58dcca9e5456f 100644 --- a/superset-frontend/src/dashboard/containers/Chart.jsx +++ b/superset-frontend/src/dashboard/containers/Chart.jsx @@ -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, diff --git a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts index 446e924a3f4bb..d64eb4ca94c5d 100644 --- a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts +++ b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts @@ -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'; @@ -33,6 +33,7 @@ const cachedFormdataByChart = {}; interface GetFormDataWithExtraFiltersArguments { chart: ChartQueryPayload; + charts: Charts; filters: DataRecordFilters; colorScheme?: string; colorNamespace?: string; @@ -45,6 +46,7 @@ interface GetFormDataWithExtraFiltersArguments { // filters param only contains those applicable to this chart. export default function getFormDataWithExtraFilters({ chart, + charts, filters, colorScheme, colorNamespace, @@ -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; diff --git a/superset-frontend/src/explore/components/controls/VizTypeControl.jsx b/superset-frontend/src/explore/components/controls/VizTypeControl.jsx index c24c0c321b424..acef5e76ab5c0 100644 --- a/superset-frontend/src/explore/components/controls/VizTypeControl.jsx +++ b/superset-frontend/src/explore/components/controls/VizTypeControl.jsx @@ -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, @@ -166,7 +167,11 @@ 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) || + !registry.get(type).isNativeFilter, + ) .map(type => ({ key: type, value: registry.get(type), @@ -174,7 +179,11 @@ const VizTypeControl = props => { .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)); diff --git a/superset-frontend/src/featureFlags.ts b/superset-frontend/src/featureFlags.ts index 52e157c62997a..c8b8acc20c893 100644 --- a/superset-frontend/src/featureFlags.ts +++ b/superset-frontend/src/featureFlags.ts @@ -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', diff --git a/superset/config.py b/superset/config.py index 43155366c5ece..24664f3baa0e5 100644 --- a/superset/config.py +++ b/superset/config.py @@ -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 From 5d5d9e1d21094a1a37377915bc3dd1708f46ace1 Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Sun, 24 Jan 2021 09:33:22 +0200 Subject: [PATCH 2/8] refactor: fix CR notes --- .../util/getFormDataWithExtraFilters_spec.ts | 27 +++++++++++-------- .../components/nativeFilters/utils.ts | 13 ++++----- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts b/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts index d90bf8114e03b..2267e015649c4 100644 --- a/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts +++ b/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts @@ -20,19 +20,24 @@ import getFormDataWithExtraFilters from 'src/dashboard/util/charts/getFormDataWi describe('getFormDataWithExtraFilters', () => { const chartId = 8675309; + const mockChart = { + id: chartId, + formData: { + viz_type: 'filter_select', + filters: [ + { + col: 'country_name', + op: 'IN', + val: ['United States'], + }, + ], + }, + }; const mockArgs = { - chart: { - id: chartId, - formData: { - filters: [ - { - col: 'country_name', - op: 'IN', - val: ['United States'], - }, - ], - }, + charts: { + [chartId]: mockChart, }, + chart: mockChart, filters: { region: ['Spain'], color: ['pink', 'purple'], diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts index 872b761e8616e..e8a881811a3e5 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts @@ -29,6 +29,7 @@ import { } from 'src/dashboard/util/componentTypes'; import { FormInstance } from 'antd/lib/form'; import React from 'react'; +import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import { CascadeFilter, Filter, @@ -37,7 +38,6 @@ import { TreeItem, } from './types'; import { DASHBOARD_ROOT_ID } from '../../util/constants'; -import { FeatureFlag, isFeatureEnabled } from '../../../featureFlags'; export const useForceUpdate = () => { const [, updateState] = React.useState({}); @@ -199,6 +199,11 @@ export function mergeExtraFormData( }; } +export function isNativeFilter(vizType: string) { + // @ts-ignore need export from superset-ui `ItemWithValue` + return getChartMetadataRegistry().items[vizType]?.value.isNativeFilter; +} + export function getExtraFormData( nativeFilters: NativeFiltersState, charts: Charts, @@ -211,11 +216,7 @@ export function getExtraFormData( }); 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; - if (isNativeFilter) { + if (isNativeFilter(chart?.formData?.viz_type)) { const filterState = nativeFilters.filtersState[key] || {}; const { extraFormData: newExtra = {} } = filterState; extraFormData = mergeExtraFormData(extraFormData, newExtra); From e4ec4096c249565ed0e45e6633e08ebd47d87e33 Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Sun, 24 Jan 2021 09:42:45 +0200 Subject: [PATCH 3/8] lint: fix lint --- superset-frontend/src/chart/ChartRenderer.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/chart/ChartRenderer.jsx b/superset-frontend/src/chart/ChartRenderer.jsx index 381c85f7f6be0..0bbe8335158a0 100644 --- a/superset-frontend/src/chart/ChartRenderer.jsx +++ b/superset-frontend/src/chart/ChartRenderer.jsx @@ -74,7 +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), + setExtraFormData: extraFormData => this.props.actions?.setExtraFormData(this.props.chartId, extraFormData), }; } From cadcb2daaacfc45707b6cf7de31658f2150a754b Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Sun, 24 Jan 2021 09:46:47 +0200 Subject: [PATCH 4/8] lint: fix lint --- superset-frontend/src/chart/ChartRenderer.jsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/chart/ChartRenderer.jsx b/superset-frontend/src/chart/ChartRenderer.jsx index 0bbe8335158a0..eafe4b40c7be3 100644 --- a/superset-frontend/src/chart/ChartRenderer.jsx +++ b/superset-frontend/src/chart/ChartRenderer.jsx @@ -74,7 +74,8 @@ 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), + setExtraFormData: extraFormData => + this.props.actions?.setExtraFormData(this.props.chartId, extraFormData), }; } From a477026c3432ba23371397f35b2decfb2da994a1 Mon Sep 17 00:00:00 2001 From: amitmiran137 Date: Tue, 9 Feb 2021 22:48:16 +0200 Subject: [PATCH 5/8] chore: pre-commit --- requirements/development.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/development.txt b/requirements/development.txt index f0eb4938a3889..c27babf48583c 100644 --- a/requirements/development.txt +++ b/requirements/development.txt @@ -20,7 +20,7 @@ jdcal==1.4.1 # via openpyxl jmespath==0.10.0 # via boto3, botocore jsonlines==1.2.0 # via tabulator linear-tsv==1.1.0 # via tabulator -mysqlclient==1.4.2.post1 # via -r requirements/development.in +#mysqlclient==1.4.2.post1 # via -r requirements/development.in openpyxl==3.0.5 # via tabulator pillow==7.2.0 # via -r requirements/development.in psycopg2-binary==2.8.5 # via -r requirements/development.in From a5d9def2b8330969fbe1cb8da532aabd4614a259 Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Wed, 10 Feb 2021 08:40:08 +0200 Subject: [PATCH 6/8] refactor: under chage --- requirements/development.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/development.txt b/requirements/development.txt index c27babf48583c..f0eb4938a3889 100644 --- a/requirements/development.txt +++ b/requirements/development.txt @@ -20,7 +20,7 @@ jdcal==1.4.1 # via openpyxl jmespath==0.10.0 # via boto3, botocore jsonlines==1.2.0 # via tabulator linear-tsv==1.1.0 # via tabulator -#mysqlclient==1.4.2.post1 # via -r requirements/development.in +mysqlclient==1.4.2.post1 # via -r requirements/development.in openpyxl==3.0.5 # via tabulator pillow==7.2.0 # via -r requirements/development.in psycopg2-binary==2.8.5 # via -r requirements/development.in From 64768caea13ceb6cf83909603c6e4825f2fae91c Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Thu, 11 Feb 2021 14:57:36 +0200 Subject: [PATCH 7/8] refactor: move to behaviors --- .../src/dashboard/components/nativeFilters/utils.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts index 3d72051fc2c58..d4417b2f032a1 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts @@ -21,6 +21,7 @@ import { QueryFormData, getChartMetadataRegistry, QueryObject, + Behavior, } from '@superset-ui/core'; import { Charts } from 'src/dashboard/types'; import { RefObject } from 'react'; @@ -100,9 +101,11 @@ export function mergeExtraFormData( }; } -export function isNativeFilter(vizType: string) { +export function isCrossFilter(vizType: string) { // @ts-ignore need export from superset-ui `ItemWithValue` - return getChartMetadataRegistry().items[vizType]?.value.isNativeFilter; + return getChartMetadataRegistry().items[vizType]?.value.behaviors?.includes( + Behavior.CROSS_FILTER, + ); } export function getExtraFormData( @@ -117,7 +120,7 @@ export function getExtraFormData( }); if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) { Object.entries(charts).forEach(([key, chart]) => { - if (isNativeFilter(chart?.formData?.viz_type)) { + if (isCrossFilter(chart?.formData?.viz_type)) { const filterState = nativeFilters.filtersState[key] || {}; const { extraFormData: newExtra = {} } = filterState; extraFormData = mergeExtraFormData(extraFormData, newExtra); From d931e063e404a1e8b4d7d038a1bef83406db8f22 Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Thu, 11 Feb 2021 15:06:09 +0200 Subject: [PATCH 8/8] lint: fix lint --- .../explore/components/controls/VizTypeControl.jsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/explore/components/controls/VizTypeControl.jsx b/superset-frontend/src/explore/components/controls/VizTypeControl.jsx index 4dc67faa7f73a..085fdd8da64df 100644 --- a/superset-frontend/src/explore/components/controls/VizTypeControl.jsx +++ b/superset-frontend/src/explore/components/controls/VizTypeControl.jsx @@ -24,7 +24,6 @@ import { useDynamicPluginContext } from 'src/components/DynamicPlugins'; import { Tooltip } from 'src/common/components/Tooltip'; import Modal from 'src/common/components/Modal'; import Label from 'src/components/Label'; - import ControlHeader from '../ControlHeader'; import './VizTypeControl.less'; import { FeatureFlag, isFeatureEnabled } from '../../../featureFlags'; @@ -169,7 +168,11 @@ const VizTypeControl = props => { const filteredTypes = DEFAULT_ORDER.filter(type => registry.has(type)) .filter(type => { const behaviors = registry.get(type)?.behaviors || []; - return isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) && behaviors.includes(Behavior.CROSS_FILTER) || !behaviors.length; + return ( + (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) && + behaviors.includes(Behavior.CROSS_FILTER)) || + !behaviors.length + ); }) .map(type => ({ key: type, @@ -180,7 +183,11 @@ const VizTypeControl = props => { .entries() .filter(entry => { const behaviors = entry.value?.behaviors || []; - return isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) && behaviors.includes(Behavior.CROSS_FILTER) || !behaviors.length; + return ( + (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) && + behaviors.includes(Behavior.CROSS_FILTER)) || + !behaviors.length + ); }) .filter(({ key }) => !typesWithDefaultOrder.has(key)), )