You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ma...@apache.org on 2020/03/24 06:05:13 UTC
[incubator-superset] branch master updated: feat: [explore] don't
save filters inherited from a dashboard (#9340)
This is an automated email from the ASF dual-hosted git repository.
maximebeauchemin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 98a71be feat: [explore] don't save filters inherited from a dashboard (#9340)
98a71be is described below
commit 98a71be80bf679224dd0191b6c5562447fa4ee5d
Author: Maxime Beauchemin <ma...@gmail.com>
AuthorDate: Mon Mar 23 23:05:00 2020 -0700
feat: [explore] don't save filters inherited from a dashboard (#9340)
* 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.
* fix build
---
.../spec/javascripts/explore/AdhocFilter_spec.js | 1 +
superset-frontend/src/explore/AdhocFilter.js | 1 +
.../src/explore/components/AdhocFilterOption.jsx | 40 +++++++++++++++-------
superset/examples/random_time_series.py | 4 +--
superset/utils/core.py | 2 ++
superset/views/core.py | 10 ++++++
tests/core_tests.py | 19 +++++++---
tests/viz_tests.py | 4 +++
8 files changed, 61 insertions(+), 20 deletions(-)
diff --git a/superset-frontend/spec/javascripts/explore/AdhocFilter_spec.js b/superset-frontend/spec/javascripts/explore/AdhocFilter_spec.js
index a559bd7..dd48181 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/AdhocFilter.js b/superset-frontend/src/explore/AdhocFilter.js
index c11d4fd..7520cec 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 0c9ce81..10979c2 100644
--- a/superset-frontend/src/explore/components/AdhocFilterOption.jsx
+++ b/superset-frontend/src/explore/components/AdhocFilterOption.jsx
@@ -19,11 +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';
const propTypes = {
adhocFilter: PropTypes.instanceOf(AdhocFilter).isRequired,
@@ -80,7 +82,6 @@ export default class AdhocFilterOption extends React.PureComponent {
datasource={this.props.datasource}
/>
);
-
return (
<OverlayTrigger
ref="overlay"
@@ -93,18 +94,31 @@ export default class AdhocFilterOption extends React.PureComponent {
onEntered={this.onOverlayEntered}
onExited={this.onOverlayExited}
>
- <Label className="adhoc-filter-option">
- <div onMouseDownCapture={this.onMouseDown}>
- <span className="m-r-5 option-label">
- {adhocFilter.getDefaultLabel()}
- <i
- className={`glyphicon glyphicon-triangle-${
- this.state.overlayShown ? 'left' : 'right'
- } adhoc-label-arrow`}
- />
- </span>
- </div>
- </Label>
+ <div>
+ {adhocFilter.isExtra && (
+ <InfoTooltipWithTrigger
+ icon="exclamation-triangle"
+ placement="top"
+ className="m-r-5 text-muted"
+ tooltip={t(`
+ This filter was inherited from the dashboard's context.
+ It won't be saved when saving the chart.
+ `)}
+ />
+ )}
+ <Label className="adhoc-filter-option">
+ <div onMouseDownCapture={this.onMouseDown}>
+ <span className="m-r-5 option-label">
+ {adhocFilter.getDefaultLabel()}
+ <i
+ className={`glyphicon glyphicon-triangle-${
+ this.state.overlayShown ? 'left' : 'right'
+ } adhoc-label-arrow`}
+ />
+ </span>
+ </div>
+ </Label>
+ </div>
</OverlayTrigger>
);
}
diff --git a/superset/examples/random_time_series.py b/superset/examples/random_time_series.py
index 151d04c..7ce865c 100644
--- a/superset/examples/random_time_series.py
+++ b/superset/examples/random_time_series.py
@@ -60,8 +60,8 @@ def load_random_time_series_data(only_metadata=False, force=False):
slice_data = {
"granularity_sqla": "day",
"row_limit": config["ROW_LIMIT"],
- "since": "1 year ago",
- "until": "now",
+ "since": "2019-01-01",
+ "until": "2019-02-01",
"metric": "count",
"viz_type": "cal_heatmap",
"domain_granularity": "month",
diff --git a/superset/utils/core.py b/superset/utils/core.py
index 23d6d4e..e23b469 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": True if filt.get("isExtra") is True else False,
}
if expressionType == "SIMPLE":
@@ -860,6 +861,7 @@ def merge_extra_filters(form_data: dict):
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 447461e..eb8efb1 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -946,6 +946,12 @@ class Superset(BaseSupersetView):
)
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 @@ class Superset(BaseSupersetView):
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.get("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"]
diff --git a/tests/core_tests.py b/tests/core_tests.py
index 8b4ce69..8d5641b 100644
--- a/tests/core_tests.py
+++ b/tests/core_tests.py
@@ -288,9 +288,10 @@ class CoreTests(SupersetTestCase):
self.login(username="admin")
slice_name = f"Energy Sankey"
slice_id = self.get_slice(slice_name, db.session).id
- copy_name = f"Test Sankey Save_{random.random()}"
+ copy_name_prefix = "Test Sankey"
+ copy_name = f"{copy_name_prefix}[save]{random.random()}"
tbl_id = self.table_ids.get("energy_usage")
- new_slice_name = f"Test Sankey Overwrite_{random.random()}"
+ new_slice_name = f"{copy_name_prefix}[overwrite]{random.random()}"
url = (
"/superset/explore/table/{}/?slice_name={}&"
@@ -298,8 +299,9 @@ class CoreTests(SupersetTestCase):
)
form_data = {
+ "adhoc_filters": [],
"viz_type": "sankey",
- "groupby": "target",
+ "groupby": ["target"],
"metric": "sum__value",
"row_limit": 5000,
"slice_id": slice_id,
@@ -319,8 +321,9 @@ class CoreTests(SupersetTestCase):
self.assertEqual(slc.viz.form_data, form_data)
form_data = {
+ "adhoc_filters": [],
"viz_type": "sankey",
- "groupby": "source",
+ "groupby": ["source"],
"metric": "sum__value",
"row_limit": 5000,
"slice_id": new_slice_id,
@@ -338,7 +341,13 @@ class CoreTests(SupersetTestCase):
self.assertEqual(slc.viz.form_data, form_data)
# Cleanup
- db.session.delete(slc)
+ slices = (
+ db.session.query(Slice)
+ .filter(Slice.slice_name.like(copy_name_prefix + "%"))
+ .all()
+ )
+ for slc in slices:
+ db.session.delete(slc)
db.session.commit()
def test_filter_endpoint(self):
diff --git a/tests/viz_tests.py b/tests/viz_tests.py
index 80da72e..ab10f49 100644
--- a/tests/viz_tests.py
+++ b/tests/viz_tests.py
@@ -1081,6 +1081,7 @@ class BaseDeckGLVizTestCase(SupersetTestCase):
"comparator": "",
"operator": "IS NOT NULL",
"subject": "lat",
+ "isExtra": False,
},
{
"clause": "WHERE",
@@ -1089,6 +1090,7 @@ class BaseDeckGLVizTestCase(SupersetTestCase):
"comparator": "",
"operator": "IS NOT NULL",
"subject": "lon",
+ "isExtra": False,
},
],
"delimited_key": [
@@ -1099,6 +1101,7 @@ class BaseDeckGLVizTestCase(SupersetTestCase):
"comparator": "",
"operator": "IS NOT NULL",
"subject": "lonlat",
+ "isExtra": False,
}
],
"geohash_key": [
@@ -1109,6 +1112,7 @@ class BaseDeckGLVizTestCase(SupersetTestCase):
"comparator": "",
"operator": "IS NOT NULL",
"subject": "geo",
+ "isExtra": False,
}
],
}