From 2dac01901a923ee0508ef8004a45848b83199830 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Fri, 20 Mar 2020 23:31:27 -0700 Subject: [PATCH 1/2] feat: [explore] don't save filters inherited from a dashboard When navigating to explore from a dashboard context, the current dashboard filter(s) are passed along to explore so that the context is kept. So say you're filtering on "country=Romania", in your dashboard and pivot to explore, that filter is still there and keep on exploring. Now a common issue is that you'll want to make some tweak to your chart that are unrelated to the filter, say toggling the legend off for instance, and then save it. Now you back to your dashboard and even though you started with an "all countries" dashboard, with a global filter on country, now that one chart is stuck on "Romania". Typically you notice this when filtering on something else, say "Italy" and then that one chart now has two mutually exclusive filters, and show "No data". Now, the fix is to flag the filter as "extra" (that's the not-so-good internal name we use for these inherited filters) and make it clear that that specific filter is special and won't be saved when saving the chart. --- superset-frontend/src/explore/AdhocFilter.js | 1 + .../explore/components/AdhocFilterOption.jsx | 40 +++++++++++++------ superset/utils/core.py | 2 + superset/views/core.py | 10 +++++ 4 files changed, 40 insertions(+), 13 deletions(-) diff --git a/superset-frontend/src/explore/AdhocFilter.js b/superset-frontend/src/explore/AdhocFilter.js index c11d4fdef1d27..7520cec9bcd53 100644 --- a/superset-frontend/src/explore/AdhocFilter.js +++ b/superset-frontend/src/explore/AdhocFilter.js @@ -79,6 +79,7 @@ export default class AdhocFilter { this.operator = null; this.comparator = null; } + this.isExtra = !!adhocFilter.isExtra; this.fromFormData = !!adhocFilter.filterOptionName; this.filterOptionName = diff --git a/superset-frontend/src/explore/components/AdhocFilterOption.jsx b/superset-frontend/src/explore/components/AdhocFilterOption.jsx index 0c9ce81c1c20a..aef295875ffca 100644 --- a/superset-frontend/src/explore/components/AdhocFilterOption.jsx +++ b/superset-frontend/src/explore/components/AdhocFilterOption.jsx @@ -24,6 +24,8 @@ import AdhocFilterEditPopover from './AdhocFilterEditPopover'; import AdhocFilter from '../AdhocFilter'; import columnType from '../propTypes/columnType'; import adhocMetricType from '../propTypes/adhocMetricType'; +import InfoTooltipWithTrigger from '../../components/InfoTooltipWithTrigger'; +import { t } from '@superset-ui/translation'; const propTypes = { adhocFilter: PropTypes.instanceOf(AdhocFilter).isRequired, @@ -80,7 +82,6 @@ export default class AdhocFilterOption extends React.PureComponent { datasource={this.props.datasource} /> ); - return ( - +
+ {adhocFilter.isExtra && + + } + +
); } diff --git a/superset/utils/core.py b/superset/utils/core.py index 23d6d4ef9256d..1fc74d24d4964 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -805,6 +805,7 @@ def to_adhoc(filt, expressionType="SIMPLE", clause="where"): "clause": clause.upper(), "expressionType": expressionType, "filterOptionName": str(uuid.uuid4()), + "isExtra": filt.get("isExtra"), } if expressionType == "SIMPLE": @@ -860,6 +861,7 @@ def get_filter_key(f): existing_filters[get_filter_key(existing)] = existing["comparator"] for filtr in form_data["extra_filters"]: + filtr["isExtra"] = True # Pull out time filters/options and merge into form data if date_options.get(filtr["col"]): if filtr.get("val"): diff --git a/superset/views/core.py b/superset/views/core.py index 447461ef3f740..114e25d83d5ed 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -946,6 +946,12 @@ def filter(self, datasource_type, datasource_id, column): ) return json_success(payload) + @staticmethod + def remove_extra_filters(filters): + """Extra filters are ones inherited from the dashboard's temporary context + Those should not be saved when saving the chart""" + return [f for f in filters if not f.get("isExtra")] + def save_or_overwrite_slice( self, args, @@ -967,6 +973,10 @@ def save_or_overwrite_slice( form_data.pop("slice_id") # don't save old slice_id slc = Slice(owners=[g.user] if g.user else []) + form_data["adhoc_filters"] = self.remove_extra_filters( + form_data["adhoc_filters"] + ) + slc.params = json.dumps(form_data, indent=2, sort_keys=True) slc.datasource_name = datasource_name slc.viz_type = form_data["viz_type"] From 935a46b9328063493abd515f82dd679d7738d51e Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Sun, 22 Mar 2020 16:51:04 -0700 Subject: [PATCH 2/2] fix build --- .../javascripts/explore/AdhocFilter_spec.js | 1 + .../explore/components/AdhocFilterOption.jsx | 6 +++--- superset/examples/random_time_series.py | 4 ++-- superset/utils/core.py | 2 +- superset/views/core.py | 2 +- tests/core_tests.py | 19 ++++++++++++++----- tests/viz_tests.py | 4 ++++ 7 files changed, 26 insertions(+), 12 deletions(-) diff --git a/superset-frontend/spec/javascripts/explore/AdhocFilter_spec.js b/superset-frontend/spec/javascripts/explore/AdhocFilter_spec.js index a559bd7099018..dd4818128e363 100644 --- a/superset-frontend/spec/javascripts/explore/AdhocFilter_spec.js +++ b/superset-frontend/spec/javascripts/explore/AdhocFilter_spec.js @@ -40,6 +40,7 @@ describe('AdhocFilter', () => { filterOptionName: adhocFilter.filterOptionName, sqlExpression: null, fromFormData: false, + isExtra: false, }); }); diff --git a/superset-frontend/src/explore/components/AdhocFilterOption.jsx b/superset-frontend/src/explore/components/AdhocFilterOption.jsx index aef295875ffca..10979c24ffad1 100644 --- a/superset-frontend/src/explore/components/AdhocFilterOption.jsx +++ b/superset-frontend/src/explore/components/AdhocFilterOption.jsx @@ -19,13 +19,13 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Label, OverlayTrigger } from 'react-bootstrap'; +import { t } from '@superset-ui/translation'; import AdhocFilterEditPopover from './AdhocFilterEditPopover'; import AdhocFilter from '../AdhocFilter'; import columnType from '../propTypes/columnType'; import adhocMetricType from '../propTypes/adhocMetricType'; import InfoTooltipWithTrigger from '../../components/InfoTooltipWithTrigger'; -import { t } from '@superset-ui/translation'; const propTypes = { adhocFilter: PropTypes.instanceOf(AdhocFilter).isRequired, @@ -95,7 +95,7 @@ export default class AdhocFilterOption extends React.PureComponent { onExited={this.onOverlayExited} >
- {adhocFilter.isExtra && + {adhocFilter.isExtra && ( - } + )}