(superset) 01/01: fix(keys): fix filter processing

[as...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

asoare pushed a commit to branch alexandrusoare/fix/key-error-for-reports
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 06c1f4327f37e3799ccf433a22932878820e48bb
Author: alexandrusoare <al...@gmail.com>
AuthorDate: Thu Feb 26 16:51:57 2026 +0200

    fix(keys): fix filter processing
---
 superset/reports/models.py             | 24 ++++++++++---
 tests/unit_tests/reports/model_test.py | 63 ++++++++++++++++++++++++----------
 2 files changed, 64 insertions(+), 23 deletions(-)

diff --git a/superset/reports/models.py b/superset/reports/models.py
index 4fa573d6f1e..fd88b981d57 100644
--- a/superset/reports/models.py
+++ b/superset/reports/models.py
@@ -16,6 +16,7 @@
 # under the License.
 """A collection of ORM sqlalchemy models for Superset"""
 
+import logging
 from typing import Any, Optional
 
 import prison
@@ -47,6 +48,8 @@ from superset.reports.types import ReportScheduleExtra
 from superset.utils.backports import StrEnum
 from superset.utils.core import MediumText
 
+logger = logging.getLogger(__name__)
+
 metadata = Model.metadata  # pylint: disable=no-member
 
 
@@ -191,14 +194,25 @@ class ReportSchedule(AuditMixinNullable, ExtraJSONMixin, Model):
         params: dict[str, Any] = {}
         dashboard = self.extra.get("dashboard")
         if dashboard and dashboard.get("nativeFilters"):
-            for filter in dashboard.get("nativeFilters") or []:  # type: ignore
+            native_filters = dashboard.get("nativeFilters") or []
+            for native_filter in native_filters:  # type: ignore
+                native_filter_id = native_filter.get("nativeFilterId")
+                filter_type = native_filter.get("filterType")
+
+                if native_filter_id is None or filter_type is None:
+                    logger.warning(
+                        "Skipping malformed native filter missing required fields: %s",
+                        native_filter,
+                    )
+                    continue
+
                 params = {
                     **params,
                     **self._generate_native_filter(
-                        filter["nativeFilterId"],
-                        filter["filterType"],
-                        filter["columnName"],
-                        filter["filterValues"],
+                        native_filter_id,
+                        filter_type,
+                        native_filter.get("columnName", ""),
+                        native_filter.get("filterValues", []),
                     ),
                 }
         # hack(hughhh): workaround for escaping prison not handling quotes right
diff --git a/tests/unit_tests/reports/model_test.py b/tests/unit_tests/reports/model_test.py
index 19e12e14072..e052367f3ed 100644
--- a/tests/unit_tests/reports/model_test.py
+++ b/tests/unit_tests/reports/model_test.py
@@ -14,7 +14,6 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-import pytest
 
 from superset.reports.models import ReportSchedule
 
@@ -99,9 +98,10 @@ def test_report_generate_native_filter_no_values():
     }
 
 
-def test_get_native_filters_params_invalid_structure():
+def test_get_native_filters_params_missing_filter_values():
     """
-    Test the ``get_native_filters_params`` method with invalid structure.
+    Test the ``get_native_filters_params`` method with missing filterValues.
+    Should handle gracefully by using empty list as default.
     """
     report_schedule = ReportSchedule()
     report_schedule.extra = {
@@ -111,29 +111,56 @@ def test_get_native_filters_params_invalid_structure():
                     "nativeFilterId": "filter_id",
                     "columnName": "column_name",
                     "filterType": "filter_select",
-                    # Missing "filterValues" key
+                    # Missing "filterValues" key - should default to []
                 }
             ]
         }
     }
 
-    with pytest.raises(KeyError, match="'filterValues'"):
-        report_schedule.get_native_filters_params()
+    # Should not raise, should handle gracefully with empty filterValues
+    result = report_schedule.get_native_filters_params()
+    assert "filter_id" in result
+    assert "column_name" in result
 
 
-# todo(hugh): how do we want to handle this case?
-# def test_report_generate_native_filter_invalid_filter_id():
-#     """
-#     Test the ``_generate_native_filter`` method with invalid filter id.
-#     """
-#     report_schedule = ReportSchedule()
-#     native_filter_id = None
-#     column_name = "column_name"
-#     values = ["value1", "value2"]
+def test_get_native_filters_params_missing_required_fields():
+    """
+    Test the ``get_native_filters_params`` method with missing required fields.
+    Filters missing nativeFilterId or filterType should be skipped.
+    """
+    report_schedule = ReportSchedule()
+    report_schedule.extra = {
+        "dashboard": {
+            "nativeFilters": [
+                {
+                    # Missing nativeFilterId - should be skipped
+                    "filterType": "filter_select",
+                    "columnName": "column_name",
+                    "filterValues": ["value1"],
+                },
+                {
+                    # Missing filterType - should be skipped
+                    "nativeFilterId": "filter_2",
+                    "columnName": "column_name",
+                    "filterValues": ["value2"],
+                },
+                {
+                    # Valid filter - should be processed
+                    "nativeFilterId": "filter_3",
+                    "filterType": "filter_select",
+                    "columnName": "column_name",
+                    "filterValues": ["value3"],
+                },
+            ]
+        }
+    }
 
-#     assert report_schedule._generate_native_filter(
-#         native_filter_id, column_name, values
-#     ) == {}
+    result = report_schedule.get_native_filters_params()
+    # Only the valid filter should be in the result
+    assert "filter_3" in result
+    assert "filter_2" not in result
+    assert "value1" not in result
+    assert "value3" in result
 
 
 def test_report_generate_native_filter():