Merge pull request apache#69 from john-bodley/john-bodley-cherry-picks

[cherries] Cherry picking a few bug fixes

superset/assets/src/explore/controls.jsx

@@ -1541,6 +1541,7 @@ export const controls = {
       '1 week',
       '28 days',
       '30 days',
+      '52 weeks',
       '1 year',
     ]),
     description: t('Overlay one or more timeseries from a ' +

superset/assets/src/explore/store.js

@@ -65,11 +65,17 @@ export function getControlsState(state, form_data) {
       : formData[k];
 
     // If the value is not valid anymore based on choices, clear it
-    if (control.type === 'SelectControl' && control.choices && k !== 'datasource' && formData[k]) {
+    if (
+      control.type === 'SelectControl' &&
+      !control.freeForm &&
+      control.choices &&
+      k !== 'datasource' &&
+      formData[k]
+    ) {
       const choiceValues = control.choices.map(c => c[0]);
-      if (control.multi && formData[k].length > 0 && choiceValues.indexOf(formData[k][0]) < 0) {
-        delete formData[k];
-      } else if (!control.multi && !control.freeForm && choiceValues.indexOf(formData[k]) < 0) {
+      if (control.multi && formData[k].length > 0) {
+        formData[k] = formData[k].filter(el => choiceValues.indexOf(el) > -1);
+      } else if (!control.multi && choiceValues.indexOf(formData[k]) < 0) {
         delete formData[k];
       }
     }

superset/migrations/versions/bddc498dd179_adhoc_filters.py

@@ -36,39 +36,11 @@ class Slice(Base):
 def upgrade():
     bind = op.get_bind()
     session = db.Session(bind=bind)
-    mapping = {'having': 'having_filters', 'where': 'filters'}
 
     for slc in session.query(Slice).all():
         try:
             params = json.loads(slc.params)
-
-            if not 'adhoc_filters' in params:
-                params['adhoc_filters'] = []
-
-                for clause, filters in mapping.items():
-                    if clause in params and params[clause] != '':
-                        params['adhoc_filters'].append({
-                            'clause': clause.upper(),
-                            'expressionType': 'SQL',
-                            'filterOptionName': str(uuid.uuid4()),
-                            'sqlExpression': params[clause],
-                        })
-
-                    if filters in params:
-                        for filt in params[filters]:
-                            params['adhoc_filters'].append({
-                                'clause': clause.upper(),
-                                'comparator': filt['val'],
-                                'expressionType': 'SIMPLE',
-                                'filterOptionName': str(uuid.uuid4()),
-                                'operator': filt['op'],
-                                'subject': filt['col'],
-                            })
-
-            for key in ('filters', 'having', 'having_filters', 'where'):
-                if key in params:
-                    del params[key]
-
+            utils.convert_legacy_filters_into_adhoc(params)
             slc.params = json.dumps(params, sort_keys=True)
         except Exception:
             pass

superset/models/core.py

@@ -739,8 +739,12 @@ def needs_conversion(df_series):
 
         with closing(engine.raw_connection()) as conn:
             with closing(conn.cursor()) as cursor:
-                for sql in sqls:
+                for sql in sqls[:-1]:
                     self.db_engine_spec.execute(cursor, sql)
+                    cursor.fetchall()
+
+                self.db_engine_spec.execute(cursor, sqls[-1])
+
                 df = pd.DataFrame.from_records(
                     data=list(cursor.fetchall()),
                     columns=[col_desc[0] for col_desc in cursor.description],

superset/utils.py

@@ -708,17 +708,42 @@ def get_celery_app(config):
     return _celery_app
 
 
+def to_adhoc(filt, expressionType='SIMPLE', clause='where'):
+    result = {
+        'clause': clause.upper(),
+        'expressionType': expressionType,
+        'filterOptionName': str(uuid.uuid4()),
+    }
+
+    if expressionType == 'SIMPLE':
+        result.update({
+            'comparator': filt['val'],
+            'operator': filt['op'],
+            'subject': filt['col'],
+        })
+    elif expressionType == 'SQL':
+        result.update({
+            'sqlExpression': filt[clause],
+        })
+
+    return result
+
+
 def merge_extra_filters(form_data):
-    # 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 are temporary/contextual filters (using the legacy constructs)
+    # that are external to the slice definition. We use those for dynamic
+    # interactive filters like the ones emitted by the "Filter Box" visualization.
+    # Note extra_filters only support simple filters.
     if 'extra_filters' in form_data:
         # __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`
-        if 'filters' not in form_data:
-            form_data['filters'] = []
+        if (
+            'adhoc_filters' not in form_data or
+            not isinstance(form_data['adhoc_filters'], list)
+        ):
+            form_data['adhoc_filters'] = []
         date_options = {
             '__from': 'since',
             '__to': 'until',
@@ -730,11 +755,20 @@ def merge_extra_filters(form_data):
         # Grab list of existing filters 'keyed' on the column and operator
 
         def get_filter_key(f):
-            return f['col'] + '__' + f['op']
+            if 'expressionType' in f:
+                return '{}__{}'.format(f['subject'], f['operator'])
+            else:
+                return '{}__{}'.format(f['col'], f['op'])
+
         existing_filters = {}
-        for existing in form_data['filters']:
-            if existing['col'] is not None:
-                existing_filters[get_filter_key(existing)] = existing['val']
+        for existing in form_data['adhoc_filters']:
+            if (
+                existing['expressionType'] == 'SIMPLE' and
+                existing['comparator'] is not None and
+                existing['subject'] is not None
+            ):
+                existing_filters[get_filter_key(existing)] = existing['comparator']
+
         for filtr in form_data['extra_filters']:
             # Pull out time filters/options and merge into form data
             if date_options.get(filtr['col']):
@@ -753,16 +787,16 @@ def get_filter_key(f):
                                 sorted(existing_filters[filter_key]) !=
                                 sorted(filtr['val'])
                             ):
-                                form_data['filters'] += [filtr]
+                                form_data['adhoc_filters'].append(to_adhoc(filtr))
                         else:
-                            form_data['filters'] += [filtr]
+                            form_data['adhoc_filters'].append(to_adhoc(filtr))
                     else:
                         # Do not add filter if same value already exists
                         if filtr['val'] != existing_filters[filter_key]:
-                            form_data['filters'] += [filtr]
+                            form_data['adhoc_filters'].append(to_adhoc(filtr))
                 else:
                     # Filter not found, add it
-                    form_data['filters'] += [filtr]
+                    form_data['adhoc_filters'].append(to_adhoc(filtr))
         # Remove extra filters from the form data since no longer needed
         del form_data['extra_filters']
 
@@ -843,14 +877,33 @@ def ensure_path_exists(path):
             raise
 
 
+def convert_legacy_filters_into_adhoc(fd):
+    mapping = {'having': 'having_filters', 'where': 'filters'}
+
+    if 'adhoc_filters' not in fd:
+        fd['adhoc_filters'] = []
+
+        for clause, filters in mapping.items():
+            if clause in fd and fd[clause] != '':
+                fd['adhoc_filters'].append(to_adhoc(fd, 'SQL', clause))
+
+            if filters in fd:
+                for filt in fd[filters]:
+                    fd['adhoc_filters'].append(to_adhoc(filt, 'SIMPLE', clause))
+
+    for key in ('filters', 'having', 'having_filters', 'where'):
+        if key in fd:
+            del fd[key]
+
+
 def split_adhoc_filters_into_base_filters(fd):
     """
     Mutates form data to restructure the adhoc filters in the form of the four base
     filters, `where`, `having`, `filters`, and `having_filters` which represent
     free form where sql, free form having sql, structured where clauses and structured
     having clauses.
     """
-    adhoc_filters = fd.get('adhoc_filters', None)
+    adhoc_filters = fd.get('adhoc_filters')
     if isinstance(adhoc_filters, list):
         simple_where_filters = []
         simple_having_filters = []

superset/views/core.py

@@ -1276,8 +1276,9 @@ def explore(self, datasource_type=None, datasource_id=None):
         form_data['datasource'] = str(datasource_id) + '__' + datasource_type
 
         # On explore, merge extra filters into the form data
-        utils.split_adhoc_filters_into_base_filters(form_data)
+        utils.convert_legacy_filters_into_adhoc(form_data)
         merge_extra_filters(form_data)
+        utils.split_adhoc_filters_into_base_filters(form_data)
 
         # merge request url params
         if request.method == 'GET':

superset/viz.py

@@ -238,10 +238,9 @@ def query_obj(self):
         # extras are used to query elements specific to a datasource type
         # for instance the extra where clause that applies only to Tables
 
-        utils.split_adhoc_filters_into_base_filters(form_data)
-
+        utils.convert_legacy_filters_into_adhoc(form_data)
         merge_extra_filters(form_data)
-
+        utils.split_adhoc_filters_into_base_filters(form_data)
         granularity = (
             form_data.get('granularity') or
             form_data.get('granularity_sqla')