From 0b8522be502988d8b9bf64a8ea203068adf5957d Mon Sep 17 00:00:00 2001
From: Maxime Beauchemin <maximebeauchemin@gmail.com>
Date: Tue, 14 Mar 2017 12:18:33 -0700
Subject: [PATCH] [filter_box] fix time filter and inverted instantFilter
 (#2402)

* [filter_box] fix time filter and inverted instantFilter

* Added a few comments
---
 .../assets/javascripts/modules/superset.js    | 32 ++++++-------------
 superset/assets/visualizations/filter_box.jsx |  6 ++--
 superset/views/core.py                        |  6 ----
 superset/viz.py                               | 22 +++++++++----
 4 files changed, 27 insertions(+), 39 deletions(-)

diff --git a/superset/assets/javascripts/modules/superset.js b/superset/assets/javascripts/modules/superset.js
index d310dbab2a..35bdd499cb 100644
--- a/superset/assets/javascripts/modules/superset.js
+++ b/superset/assets/javascripts/modules/superset.js
@@ -63,36 +63,18 @@ const px = function () {
     const container = $(selector);
     const sliceId = data.slice_id;
     const formData = applyDefaultFormData(data.form_data);
-    const jsonEndpoint = getExploreUrl(formData, 'json');
-    const origJsonEndpoint = jsonEndpoint;
     let dttm = 0;
     const stopwatch = function () {
       dttm += 10;
       const num = dttm / 1000;
       $('#timer').text(num.toFixed(2) + ' sec');
     };
-    let qrystr = '';
     slice = {
       data,
       formData,
       container,
       containerId,
       selector,
-      querystring() {
-        const parser = document.createElement('a');
-        parser.href = jsonEndpoint;
-        if (controller.type === 'dashboard') {
-          parser.href = origJsonEndpoint;
-          let flts = controller.effectiveExtraFilters(sliceId);
-          flts = encodeURIComponent(JSON.stringify(flts));
-          qrystr = parser.search + '&extra_filters=' + flts;
-        } else if ($('#query').length === 0) {
-          qrystr = parser.search;
-        } else {
-          qrystr = '?' + $('#query').serialize();
-        }
-        return qrystr;
-      },
       getWidgetHeader() {
         return this.container.parents('div.widget').find('.chart-header');
       },
@@ -104,16 +86,20 @@ const px = function () {
         return Mustache.render(s, context);
       },
       jsonEndpoint() {
-        const parser = document.createElement('a');
-        parser.href = jsonEndpoint;
-        let endpoint = parser.pathname + this.querystring();
+        return this.endpoint('json');
+      },
+      endpoint(endpointType = 'json') {
+        const formDataExtra = Object.assign({}, formData);
+        const flts = controller.effectiveExtraFilters(sliceId);
+        if (flts) {
+          formDataExtra.extra_filters = flts;
+        }
+        let endpoint = getExploreUrl(formDataExtra, endpointType, this.force);
         if (endpoint.charAt(0) !== '/') {
           // Known issue for IE <= 11:
           // https://connect.microsoft.com/IE/feedbackdetail/view/1002846/pathname-incorrect-for-out-of-document-elements
           endpoint = '/' + endpoint;
         }
-        endpoint += '&json=true';
-        endpoint += '&force=' + this.force;
         return endpoint;
       },
       d3format(col, number) {
diff --git a/superset/assets/visualizations/filter_box.jsx b/superset/assets/visualizations/filter_box.jsx
index 9a4c76451b..34f6b7ff81 100644
--- a/superset/assets/visualizations/filter_box.jsx
+++ b/superset/assets/visualizations/filter_box.jsx
@@ -50,7 +50,7 @@ class FilterBox extends React.Component {
     const selectedValues = Object.assign({}, this.state.selectedValues);
     selectedValues[filter] = vals;
     this.setState({ selectedValues, hasChanged: true });
-    this.props.onChange(filter, vals, false, !this.props.instantFiltering);
+    this.props.onChange(filter, vals, false, this.props.instantFiltering);
   }
   render() {
     let dateFilter;
@@ -63,7 +63,7 @@ class FilterBox extends React.Component {
         }
         const options = choices.map((s) => ({ value: s, label: s }));
         return (
-          <div className="m-b-5">
+          <div className="m-b-5" key={field}>
             {field.replace('__', '')}
             <Select.Creatable
               options={options}
@@ -109,7 +109,7 @@ class FilterBox extends React.Component {
       <div>
         {dateFilter}
         {filters}
-        {this.props.instantFiltering &&
+        {!this.props.instantFiltering &&
           <Button
             bsSize="small"
             bsStyle="primary"
diff --git a/superset/views/core.py b/superset/views/core.py
index eaca53b2d4..a6c71a8276 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -913,12 +913,6 @@ class Superset(BaseSupersetView):
         if request.args.get("viz_type"):
             # Converting old URLs
             d = cast_form_data(request.args)
-
-        extra_filters = request.args.get("extra_filters")
-        filters = d.get('filters', [])
-        if extra_filters:
-            extra_filters = json.loads(extra_filters)
-            d['filters'] = filters + extra_filters
         return d
 
     def get_viz(
diff --git a/superset/viz.py b/superset/viz.py
index b880646491..02fc0791ad 100755
--- a/superset/viz.py
+++ b/superset/viz.py
@@ -136,16 +136,18 @@ class BaseViz(object):
         return df
 
     def get_extra_filters(self):
-        extra_filters = self.form_data.get('extra_filters')
-        if not extra_filters:
-            return {}
-        return json.loads(extra_filters)
+        extra_filters = self.form_data.get('extra_filters', [])
+        return {f['col']: f['val'] for f in extra_filters}
 
     def query_obj(self):
         """Building a query object"""
         form_data = self.form_data
         groupby = form_data.get("groupby") or []
         metrics = form_data.get("metrics") or ['count']
+
+        # extra_filters are temporary/contextual filters that are external
+        # to the slice definition. We use those for dynamic interactive
+        # filters like the ones emitted by the "Filter Box" visualization
         extra_filters = self.get_extra_filters()
         granularity = (
             form_data.get("granularity") or form_data.get("granularity_sqla")
@@ -154,13 +156,20 @@ class BaseViz(object):
         timeseries_limit_metric = form_data.get("timeseries_limit_metric")
         row_limit = int(
             form_data.get("row_limit") or config.get("ROW_LIMIT"))
+
+        # __form and __to are special extra_filters that target time
+        # boundaries. The rest of extra_filters are simple
+        # [column_name in list_of_values]. `__` prefix is there to avoid
+        # potential conflicts with column that would be named `from` or `to`
         since = (
             extra_filters.get('__from') or form_data.get("since", "1 year ago")
         )
+
         from_dttm = utils.parse_human_datetime(since)
         now = datetime.now()
         if from_dttm > now:
             from_dttm = now - (from_dttm - now)
+
         until = extra_filters.get('__to') or form_data.get("until", "now")
         to_dttm = utils.parse_human_datetime(until)
         if from_dttm > to_dttm:
@@ -179,15 +188,14 @@ class BaseViz(object):
         filters = form_data['filters'] if 'filters' in form_data \
                 else []
         for col, vals in self.get_extra_filters().items():
-            if not (col and vals):
+            if not (col and vals) or col.startswith('__'):
                 continue
             elif col in self.datasource.filterable_column_names:
                 # Quote values with comma to avoid conflict
-                vals = ["'{}'".format(x) if "," in x else x for x in vals]
                 filters += [{
                     'col': col,
                     'op': 'in',
-                    'val': ",".join(vals),
+                    'val': vals,
                 }]
         d = {
             'granularity': granularity,