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

superset-frontend/spec/javascripts/explore/AdhocFilter_spec.js

@@ -40,6 +40,7 @@ describe('AdhocFilter', () => {
       filterOptionName: adhocFilter.filterOptionName,
       sqlExpression: null,
       fromFormData: false,
+      isExtra: false,
     });
   });
 

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 =

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>
     );
   }

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",

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 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"):

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.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"]

tests/core_tests.py

@@ -288,18 +288,20 @@ def test_save_slice(self):
         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={}&"
             "action={}&datasource_name=energy_usage"
         )
 
         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 @@ def test_save_slice(self):
         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 @@ def test_save_slice(self):
         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):

tests/viz_tests.py

@@ -1081,6 +1081,7 @@ def test_filter_nulls(self, mock_uuid4):
                     "comparator": "",
                     "operator": "IS NOT NULL",
                     "subject": "lat",
+                    "isExtra": False,
                 },
                 {
                     "clause": "WHERE",
@@ -1089,6 +1090,7 @@ def test_filter_nulls(self, mock_uuid4):
                     "comparator": "",
                     "operator": "IS NOT NULL",
                     "subject": "lon",
+                    "isExtra": False,
                 },
             ],
             "delimited_key": [
@@ -1099,6 +1101,7 @@ def test_filter_nulls(self, mock_uuid4):
                     "comparator": "",
                     "operator": "IS NOT NULL",
                     "subject": "lonlat",
+                    "isExtra": False,
                 }
             ],
             "geohash_key": [
@@ -1109,6 +1112,7 @@ def test_filter_nulls(self, mock_uuid4):
                     "comparator": "",
                     "operator": "IS NOT NULL",
                     "subject": "geo",
+                    "isExtra": False,
                 }
             ],
         }