[ad-hoc filters] Fixing issue with legacy filters (#5525)

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)
-
+            utils.convert_legacy_filters_into_adhoc(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]
-
             slc.params = json.dumps(params, sort_keys=True)
         except Exception:
             pass

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
+    # extra_filters are temporary/contextual filters (using the legacy constructs)
-    # to the slice definition. We use those for dynamic interactive
+    # that are external to the slice definition. We use those for dynamic
-    # filters like the ones emitted by the "Filter Box" visualization
+    # 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:
+        if (
-            form_data['filters'] = []
+            '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']:
+        for existing in form_data['adhoc_filters']:
-            if existing['col'] is not None and existing['val'] is not None:
+            if (
-                existing_filters[get_filter_key(existing)] = existing['val']
+                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 merge_extra_filters(form_data):
                                 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 = []

superset/views/core.py

@@ -1281,8 +1281,9 @@ class Superset(BaseSupersetView):
         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

@@ -246,10 +246,9 @@ class BaseViz(object):
         # 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')

tests/utils_tests.py

@@ -15,6 +15,7 @@ import numpy
 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 @@ class UtilsTestCase(unittest.TestCase):
         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 @@ class UtilsTestCase(unittest.TestCase):
         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 @@ class UtilsTestCase(unittest.TestCase):
             {'col': 'a', 'op': 'in', 'val': 'someval'},
             {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
         ]}
-        expected = {'filters': [
+        expected = {
-            {'col': 'a', 'op': 'in', 'val': 'someval'},
+            'adhoc_filters': [
-            {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+                {
-        ]}
+                    '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': [
+        form_data = {
-            {'col': 'a', 'op': 'in', 'val': 'someval'},
+            'extra_filters': [
-            {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+                {'col': 'a', 'op': 'in', 'val': 'someval'},
-        ], 'filters': [{'col': 'D', 'op': '!=', 'val': ['G1', 'g2']}]}
+                {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
-        expected = {'filters': [
+            ],
-            {'col': 'D', 'op': '!=', 'val': ['G1', 'g2']},
+            'adhoc_filters': [
-            {'col': 'a', 'op': 'in', 'val': 'someval'},
+                {
-            {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+                    '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 @@ class UtilsTestCase(unittest.TestCase):
             {'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 @@ class UtilsTestCase(unittest.TestCase):
         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': [
+            'adhoc_filters': [
-                {'col': None, 'op': 'in', 'val': ''},
+                {
+                    'clause': 'WHERE',
+                    'comparator': '',
+                    'expressionType': 'SIMPLE',
+                    'operator': 'in',
+                    'subject': None,
+                },
             ],
             'extra_filters': [
                 {'col': 'B', 'op': '==', 'val': []},
             ],
         }
         expected = {
-            'filters': [
+            'adhoc_filters': [
-                {'col': None, 'op': 'in', 'val': ''},
+                {
+                    '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': [
+            'adhoc_filters': [
-                {'col': 'a', 'op': 'in', 'val': 'someval'},
+                {
-                {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+                    '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': [
+            'adhoc_filters': [
-                {'col': 'a', 'op': 'in', 'val': 'someval'},
+                {
-                {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+                    '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 @@ class UtilsTestCase(unittest.TestCase):
                 {'col': 'a', 'op': 'in', 'val': 'someval'},
                 {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
             ],
-            'filters': [
+            'adhoc_filters': [
-                {'col': 'a', 'op': 'in', 'val': ['g1', 'g2']},
+                {
-                {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+                    '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': [
+            'adhoc_filters': [
-                {'col': 'a', 'op': 'in', 'val': ['g1', 'g2']},
+                {
-                {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+                    '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 @@ class UtilsTestCase(unittest.TestCase):
         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)

tests/viz_tests.py

@@ -264,33 +264,6 @@ class TableVizTestCase(unittest.TestCase):
         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()