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 <[email protected]> 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():
