john-bodley closed pull request #5525: [ad-hoc filters] Fixing issue with 
legacy filters
URL: https://github.com/apache/incubator-superset/pull/5525
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/superset/migrations/versions/bddc498dd179_adhoc_filters.py 
b/superset/migrations/versions/bddc498dd179_adhoc_filters.py
index 55503f0071..c22e0595b1 100644
--- a/superset/migrations/versions/bddc498dd179_adhoc_filters.py
+++ b/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
diff --git a/superset/utils.py b/superset/utils.py
index 04885a6bb0..c0c0740bd9 100644
--- a/superset/utils.py
+++ b/superset/utils.py
@@ -713,17 +713,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 = {
             '__time_range': 'time_range',
             '__time_col': 'granularity_sqla',
@@ -734,11 +759,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 and existing['val'] 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']):
@@ -757,16 +791,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']
 
@@ -921,6 +955,25 @@ def since_until_to_time_range(form_data):
     form_data['time_range'] = ' : '.join((since, until))
 
 
+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
@@ -928,7 +981,7 @@ def split_adhoc_filters_into_base_filters(fd):
     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 = []
diff --git a/superset/views/core.py b/superset/views/core.py
index ef3ca4d548..a49f37e4b5 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -1281,8 +1281,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':
diff --git a/superset/viz.py b/superset/viz.py
index 59acb9f7fe..770d7eaff0 100644
--- a/superset/viz.py
+++ b/superset/viz.py
@@ -246,10 +246,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')
diff --git a/tests/utils_tests.py b/tests/utils_tests.py
index e76a5e5bb8..e7b70f6eed 100644
--- a/tests/utils_tests.py
+++ b/tests/utils_tests.py
@@ -15,6 +15,7 @@
 from superset.exceptions import SupersetException
 from superset.utils import (
     base_json_conv,
+    convert_legacy_filters_into_adhoc,
     datetime_f,
     get_since_until,
     json_int_dttm_ser,
@@ -51,6 +52,26 @@ def mock_parse_human_datetime(s):
         return datetime(2018, 12, 31, 23, 59, 59)
 
 
+def mock_to_adhoc(filt, expressionType='SIMPLE', clause='where'):
+    result = {
+        'clause': clause.upper(),
+        'expressionType': expressionType,
+    }
+
+    if expressionType == 'SIMPLE':
+        result.update({
+            'comparator': filt['val'],
+            'operator': filt['op'],
+            'subject': filt['col'],
+        })
+    elif expressionType == 'SQL':
+        result.update({
+            'sqlExpression': filt[clause],
+        })
+
+    return result
+
+
 class UtilsTestCase(unittest.TestCase):
     def test_json_int_dttm_ser(self):
         dttm = datetime(2020, 1, 1)
@@ -93,6 +114,7 @@ def test_zlib_compression(self):
         got_str = zlib_decompress_to_string(blob)
         self.assertEquals(json_str, got_str)
 
+    @patch('superset.utils.to_adhoc', mock_to_adhoc)
     def test_merge_extra_filters(self):
         # does nothing if no extra filters
         form_data = {'A': 1, 'B': 2, 'c': 'test'}
@@ -101,7 +123,7 @@ def test_merge_extra_filters(self):
         self.assertEquals(form_data, expected)
         # empty extra_filters
         form_data = {'A': 1, 'B': 2, 'c': 'test', 'extra_filters': []}
-        expected = {'A': 1, 'B': 2, 'c': 'test', 'filters': []}
+        expected = {'A': 1, 'B': 2, 'c': 'test', 'adhoc_filters': []}
         merge_extra_filters(form_data)
         self.assertEquals(form_data, expected)
         # copy over extra filters into empty filters
@@ -109,22 +131,67 @@ def test_merge_extra_filters(self):
             {'col': 'a', 'op': 'in', 'val': 'someval'},
             {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
         ]}
-        expected = {'filters': [
-            {'col': 'a', 'op': 'in', 'val': 'someval'},
-            {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
-        ]}
+        expected = {
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'comparator': 'someval',
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': 'a',
+                },
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['c1', 'c2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': '==',
+                    'subject': 'B',
+                },
+            ],
+        }
         merge_extra_filters(form_data)
         self.assertEquals(form_data, expected)
         # adds extra filters to existing filters
-        form_data = {'extra_filters': [
-            {'col': 'a', 'op': 'in', 'val': 'someval'},
-            {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
-        ], 'filters': [{'col': 'D', 'op': '!=', 'val': ['G1', 'g2']}]}
-        expected = {'filters': [
-            {'col': 'D', 'op': '!=', 'val': ['G1', 'g2']},
-            {'col': 'a', 'op': 'in', 'val': 'someval'},
-            {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
-        ]}
+        form_data = {
+            'extra_filters': [
+                {'col': 'a', 'op': 'in', 'val': 'someval'},
+                {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+            ],
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['G1', 'g2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': '!=',
+                    'subject': 'D',
+                },
+            ],
+        }
+        expected = {
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['G1', 'g2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': '!=',
+                    'subject': 'D',
+                },
+                {
+                    'clause': 'WHERE',
+                    'comparator': 'someval',
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': 'a',
+                },
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['c1', 'c2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': '==',
+                    'subject': 'B',
+                },
+            ],
+        }
         merge_extra_filters(form_data)
         self.assertEquals(form_data, expected)
         # adds extra filters to existing filters and sets time options
@@ -137,7 +204,15 @@ def test_merge_extra_filters(self):
             {'col': '__granularity', 'op': 'in', 'val': '90 seconds'},
         ]}
         expected = {
-            'filters': [{'col': 'A', 'op': 'like', 'val': 'hello'}],
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'comparator': 'hello',
+                    'expressionType': 'SIMPLE',
+                    'operator': 'like',
+                    'subject': 'A',
+                },
+            ],
             'time_range': '1 year ago :',
             'granularity_sqla': 'birth_year',
             'time_grain_sqla': 'years',
@@ -147,66 +222,140 @@ def test_merge_extra_filters(self):
         merge_extra_filters(form_data)
         self.assertEquals(form_data, expected)
 
+    @patch('superset.utils.to_adhoc', mock_to_adhoc)
     def test_merge_extra_filters_ignores_empty_filters(self):
         form_data = {'extra_filters': [
             {'col': 'a', 'op': 'in', 'val': ''},
             {'col': 'B', 'op': '==', 'val': []},
         ]}
-        expected = {'filters': []}
+        expected = {'adhoc_filters': []}
         merge_extra_filters(form_data)
         self.assertEquals(form_data, expected)
 
+    @patch('superset.utils.to_adhoc', mock_to_adhoc)
     def test_merge_extra_filters_ignores_nones(self):
         form_data = {
-            'filters': [
-                {'col': None, 'op': 'in', 'val': ''},
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'comparator': '',
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': None,
+                },
             ],
             'extra_filters': [
                 {'col': 'B', 'op': '==', 'val': []},
             ],
         }
         expected = {
-            'filters': [
-                {'col': None, 'op': 'in', 'val': ''},
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'comparator': '',
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': None,
+                },
             ],
         }
         merge_extra_filters(form_data)
         self.assertEquals(form_data, expected)
 
+    @patch('superset.utils.to_adhoc', mock_to_adhoc)
     def test_merge_extra_filters_ignores_equal_filters(self):
         form_data = {
             'extra_filters': [
                 {'col': 'a', 'op': 'in', 'val': 'someval'},
                 {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
             ],
-            'filters': [
-                {'col': 'a', 'op': 'in', 'val': 'someval'},
-                {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'comparator': 'someval',
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': 'a',
+                },
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['c1', 'c2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': '==',
+                    'subject': 'B',
+                },
+            ],
+        }
+        expected = {
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'comparator': 'someval',
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': 'a',
+                },
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['c1', 'c2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': '==',
+                    'subject': 'B',
+                },
             ],
         }
-        expected = {'filters': [
-            {'col': 'a', 'op': 'in', 'val': 'someval'},
-            {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
-        ]}
         merge_extra_filters(form_data)
         self.assertEquals(form_data, expected)
 
+    @patch('superset.utils.to_adhoc', mock_to_adhoc)
     def test_merge_extra_filters_merges_different_val_types(self):
         form_data = {
             'extra_filters': [
                 {'col': 'a', 'op': 'in', 'val': ['g1', 'g2']},
                 {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
             ],
-            'filters': [
-                {'col': 'a', 'op': 'in', 'val': 'someval'},
-                {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'comparator': 'someval',
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': 'a',
+                },
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['c1', 'c2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': '==',
+                    'subject': 'B',
+                },
+            ],
+        }
+        expected = {
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'comparator': 'someval',
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': 'a',
+                },
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['c1', 'c2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': '==',
+                    'subject': 'B',
+                },
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['g1', 'g2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': 'a',
+                },
             ],
         }
-        expected = {'filters': [
-            {'col': 'a', 'op': 'in', 'val': 'someval'},
-            {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
-            {'col': 'a', 'op': 'in', 'val': ['g1', 'g2']},
-        ]}
         merge_extra_filters(form_data)
         self.assertEquals(form_data, expected)
         form_data = {
@@ -214,36 +363,107 @@ def 
test_merge_extra_filters_merges_different_val_types(self):
                 {'col': 'a', 'op': 'in', 'val': 'someval'},
                 {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
             ],
-            'filters': [
-                {'col': 'a', 'op': 'in', 'val': ['g1', 'g2']},
-                {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['g1', 'g2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': 'a',
+                },
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['c1', 'c2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': '==',
+                    'subject': 'B',
+                },
+            ],
+        }
+        expected = {
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['g1', 'g2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': 'a',
+                },
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['c1', 'c2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': '==',
+                    'subject': 'B',
+                },
+                {
+                    'clause': 'WHERE',
+                    'comparator': 'someval',
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': 'a',
+                },
             ],
         }
-        expected = {'filters': [
-            {'col': 'a', 'op': 'in', 'val': ['g1', 'g2']},
-            {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
-            {'col': 'a', 'op': 'in', 'val': 'someval'},
-        ]}
         merge_extra_filters(form_data)
         self.assertEquals(form_data, expected)
 
+    @patch('superset.utils.to_adhoc', mock_to_adhoc)
     def test_merge_extra_filters_adds_unequal_lists(self):
         form_data = {
             'extra_filters': [
                 {'col': 'a', 'op': 'in', 'val': ['g1', 'g2', 'g3']},
                 {'col': 'B', 'op': '==', 'val': ['c1', 'c2', 'c3']},
             ],
-            'filters': [
-                {'col': 'a', 'op': 'in', 'val': ['g1', 'g2']},
-                {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['g1', 'g2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': 'a',
+                },
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['c1', 'c2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': '==',
+                    'subject': 'B',
+                },
+            ],
+        }
+        expected = {
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['g1', 'g2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': 'a',
+                },
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['c1', 'c2'],
+                    'expressionType': 'SIMPLE',
+                    'operator': '==',
+                    'subject': 'B',
+                },
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['g1', 'g2', 'g3'],
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': 'a',
+                },
+                {
+                    'clause': 'WHERE',
+                    'comparator': ['c1', 'c2', 'c3'],
+                    'expressionType': 'SIMPLE',
+                    'operator': '==',
+                    'subject': 'B',
+                },
             ],
         }
-        expected = {'filters': [
-            {'col': 'a', 'op': 'in', 'val': ['g1', 'g2']},
-            {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
-            {'col': 'a', 'op': 'in', 'val': ['g1', 'g2', 'g3']},
-            {'col': 'B', 'op': '==', 'val': ['c1', 'c2', 'c3']},
-        ]}
         merge_extra_filters(form_data)
         self.assertEquals(form_data, expected)
 
@@ -408,3 +628,88 @@ def test_get_since_until(self):
         result = get_since_until(form_data)
         expected = datetime(2016, 11, 2), datetime(2016, 11, 8)
         self.assertEqual(result, expected)
+
+    @patch('superset.utils.to_adhoc', mock_to_adhoc)
+    def test_convert_legacy_filters_into_adhoc_where(self):
+        form_data = {
+            'where': 'a = 1',
+        }
+        expected = {
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'expressionType': 'SQL',
+                    'sqlExpression': 'a = 1',
+                },
+            ],
+        }
+        convert_legacy_filters_into_adhoc(form_data)
+        self.assertEquals(form_data, expected)
+
+    @patch('superset.utils.to_adhoc', mock_to_adhoc)
+    def test_convert_legacy_filters_into_adhoc_filters(self):
+        form_data = {
+            'filters': [{'col': 'a', 'op': 'in', 'val': 'someval'}],
+        }
+        expected = {
+            'adhoc_filters': [
+                {
+                    'clause': 'WHERE',
+                    'comparator': 'someval',
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': 'a',
+                },
+            ],
+        }
+        convert_legacy_filters_into_adhoc(form_data)
+        self.assertEquals(form_data, expected)
+
+    @patch('superset.utils.to_adhoc', mock_to_adhoc)
+    def test_convert_legacy_filters_into_adhoc_having(self):
+        form_data = {
+            'having': 'COUNT(1) = 1',
+        }
+        expected = {
+            'adhoc_filters': [
+                {
+                    'clause': 'HAVING',
+                    'expressionType': 'SQL',
+                    'sqlExpression': 'COUNT(1) = 1',
+                },
+            ],
+        }
+        convert_legacy_filters_into_adhoc(form_data)
+        self.assertEquals(form_data, expected)
+
+    @patch('superset.utils.to_adhoc', mock_to_adhoc)
+    def test_convert_legacy_filters_into_adhoc_having_filters(self):
+        form_data = {
+            'having_filters': [{'col': 'COUNT(1)', 'op': '==', 'val': 1}],
+        }
+        expected = {
+            'adhoc_filters': [
+                {
+                    'clause': 'HAVING',
+                    'comparator': 1,
+                    'expressionType': 'SIMPLE',
+                    'operator': '==',
+                    'subject': 'COUNT(1)',
+                },
+            ],
+        }
+        convert_legacy_filters_into_adhoc(form_data)
+        self.assertEquals(form_data, expected)
+
+    @patch('superset.utils.to_adhoc', mock_to_adhoc)
+    def test_convert_legacy_filters_into_adhoc_existing(self):
+        form_data = {
+            'adhoc_filters': [],
+            'filters': [{'col': 'a', 'op': 'in', 'val': 'someval'}],
+            'having': 'COUNT(1) = 1',
+            'having_filters': [{'col': 'COUNT(1)', 'op': '==', 'val': 1}],
+            'where': 'a = 1',
+        }
+        expected = {'adhoc_filters': []}
+        convert_legacy_filters_into_adhoc(form_data)
+        self.assertEquals(form_data, expected)
diff --git a/tests/viz_tests.py b/tests/viz_tests.py
index 95a69fcac4..61539b6681 100644
--- a/tests/viz_tests.py
+++ b/tests/viz_tests.py
@@ -264,33 +264,6 @@ def test_adhoc_filters_overwrite_legacy_filters(self):
         self.assertEqual('(value3 in (\'North America\'))', 
query_obj['extras']['where'])
         self.assertEqual('', query_obj['extras']['having'])
 
-    def test_legacy_filters_still_appear_without_adhoc_filters(self):
-        form_data = {
-            'metrics': [{
-                'expressionType': 'SIMPLE',
-                'aggregate': 'SUM',
-                'label': 'SUM(value1)',
-                'column': {'column_name': 'value1', 'type': 'DOUBLE'},
-            }],
-            'having': 'SUM(value1) > 5',
-            'where': 'value3 in (\'North America\')',
-            'filters': [{'col': 'value2', 'val': '100', 'op': '>'}],
-            'having_filters': [{'op': '<', 'val': '10', 'col': 'SUM(value1)'}],
-        }
-        datasource = Mock()
-        test_viz = viz.TableViz(datasource, form_data)
-        query_obj = test_viz.query_obj()
-        self.assertEqual(
-            [{'col': 'value2', 'val': '100', 'op': '>'}],
-            query_obj['filter'],
-        )
-        self.assertEqual(
-            [{'op': '<', 'val': '10', 'col': 'SUM(value1)'}],
-            query_obj['extras']['having_druid'],
-        )
-        self.assertEqual('value3 in (\'North America\')', 
query_obj['extras']['where'])
-        self.assertEqual('SUM(value1) > 5', query_obj['extras']['having'])
-
     @patch('superset.viz.BaseViz.query_obj')
     def test_query_obj_merges_percent_metrics(self, super_query_obj):
         datasource = Mock()


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to