fix: remove extras from all adhoc_filters controls (#21450)

Co-authored-by: Ville Brofeldt <ville.brofeldt@apple.com>

superset-frontend/src/explore/actions/saveModalActions.js

@@ -20,6 +20,8 @@ import { SupersetClient, t } from '@superset-ui/core';
 import { addSuccessToast } from 'src/components/MessageToasts/actions';
 import { buildV1ChartDataPayload } from '../exploreUtils';
 
+const ADHOC_FILTER_REGEX = /^adhoc_filters/;
+
 export const FETCH_DASHBOARDS_SUCCEEDED = 'FETCH_DASHBOARDS_SUCCEEDED';
 export function fetchDashboardsSucceeded(choices) {
   return { type: FETCH_DASHBOARDS_SUCCEEDED, choices };
@@ -66,11 +68,16 @@ export const getSlicePayload = (
   formDataWithNativeFilters,
   owners,
 ) => {
+  const adhocFilters = Object.entries(formDataWithNativeFilters).reduce(
+    (acc, [key, value]) =>
+      ADHOC_FILTER_REGEX.test(key)
+        ? { ...acc, [key]: value?.filter(f => !f.isExtra) }
+        : acc,
+    {},
+  );
   const formData = {
     ...formDataWithNativeFilters,
-    adhoc_filters: formDataWithNativeFilters.adhoc_filters?.filter(
-      f => !f.isExtra,
-    ),
+    ...adhocFilters,
   };
 
   const [datasourceId, datasourceType] = formData.datasource.split('__');

superset/utils/core.py

@@ -136,6 +136,8 @@
 
 InputType = TypeVar("InputType")
 
+ADHOC_FILTERS_REGEX = re.compile("^adhoc_filters")
+
 
 class LenientEnum(Enum):
     """Enums with a `get` method that convert a enum value to `Enum` if it is a
@@ -1937,3 +1939,16 @@ def create_zip(files: Dict[str, Any]) -> BytesIO:
                 fp.write(contents)
     buf.seek(0)
     return buf
+
+
+def remove_extra_adhoc_filters(form_data: Dict[str, Any]) -> None:
+    """
+    Remove filters from slice data that originate from a filter box or native filter
+    """
+    adhoc_filters = {
+        key: value for key, value in form_data.items() if ADHOC_FILTERS_REGEX.match(key)
+    }
+    for key, value in adhoc_filters.items():
+        form_data[key] = [
+            filter_ for filter_ in value or [] if not filter_.get("isExtra")
+        ]

superset/views/core.py

@@ -987,14 +987,8 @@ def filter(  # pylint: disable=no-self-use
         return json_success(payload)
 
     @staticmethod
-    def remove_extra_filters(filters: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
-        """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(
         # pylint: disable=too-many-arguments,too-many-locals
-        self,
         slc: Optional[Slice],
         slice_add_perm: bool,
         slice_overwrite_perm: bool,
@@ -1014,9 +1008,7 @@ 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") or []
-        )
+        utils.remove_extra_adhoc_filters(form_data)
 
         assert slc
         slc.params = json.dumps(form_data, indent=2, sort_keys=True)
@@ -1564,7 +1556,7 @@ def recent_activity(  # pylint: disable=too-many-locals
     @has_access_api
     @event_logger.log_this
     @expose("/available_domains/", methods=["GET"])
-    def available_domains(self) -> FlaskResponse:  # pylint: disable=no-self-use
+    def available_domains(self) -> FlaskResponse:
         """
         Returns the list of available Superset Webserver domains (if any)
         defined in config. This enables charts embedded in other apps to

tests/unit_tests/utils/test_core.py

@@ -0,0 +1,86 @@
+# -*- coding: utf-8 -*-
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import Any, Dict
+
+import pytest
+
+from superset.utils.core import QueryObjectFilterClause, remove_extra_adhoc_filters
+
+ADHOC_FILTER: QueryObjectFilterClause = {
+    "col": "foo",
+    "op": "==",
+    "val": "bar",
+}
+
+EXTRA_FILTER: QueryObjectFilterClause = {
+    "col": "foo",
+    "op": "==",
+    "val": "bar",
+    "isExtra": True,
+}
+
+
+@pytest.mark.parametrize(
+    "original,expected",
+    [
+        ({"foo": "bar"}, {"foo": "bar"}),
+        (
+            {"foo": "bar", "adhoc_filters": [ADHOC_FILTER]},
+            {"foo": "bar", "adhoc_filters": [ADHOC_FILTER]},
+        ),
+        (
+            {"foo": "bar", "adhoc_filters": [EXTRA_FILTER]},
+            {"foo": "bar", "adhoc_filters": []},
+        ),
+        (
+            {
+                "foo": "bar",
+                "adhoc_filters": [ADHOC_FILTER, EXTRA_FILTER],
+            },
+            {"foo": "bar", "adhoc_filters": [ADHOC_FILTER]},
+        ),
+        (
+            {
+                "foo": "bar",
+                "adhoc_filters_b": [ADHOC_FILTER, EXTRA_FILTER],
+            },
+            {"foo": "bar", "adhoc_filters_b": [ADHOC_FILTER]},
+        ),
+        (
+            {
+                "foo": "bar",
+                "custom_adhoc_filters": [
+                    ADHOC_FILTER,
+                    EXTRA_FILTER,
+                ],
+            },
+            {
+                "foo": "bar",
+                "custom_adhoc_filters": [
+                    ADHOC_FILTER,
+                    EXTRA_FILTER,
+                ],
+            },
+        ),
+    ],
+)
+def test_remove_extra_adhoc_filters(
+    original: Dict[str, Any], expected: Dict[str, Any]
+) -> None:
+    remove_extra_adhoc_filters(original)
+    assert expected == original