fix(report): raise warning when filter type not recognized (#38676)

This commit is contained in:
Alexandru Soare
2026-03-25 01:06:44 +02:00
committed by GitHub
parent 811dcb3715
commit 37c4a36fdb
2 changed files with 183 additions and 87 deletions

View File

@@ -217,15 +217,15 @@ class ReportSchedule(AuditMixinNullable, ExtraJSONMixin, Model):
logger.warning(warning_msg)
continue
params = {
**params,
**self._generate_native_filter(
native_filter_id,
filter_type,
native_filter.get("columnName") or "",
native_filter.get("filterValues") or [],
),
}
filter_config, filter_warning = self._generate_native_filter(
native_filter_id,
filter_type,
native_filter.get("columnName") or "",
native_filter.get("filterValues") or [],
)
if filter_warning:
warnings.append(filter_warning)
params = {**params, **filter_config}
# hack(hughhh): workaround for escaping prison not handling quotes right
rison = prison.dumps(params)
rison = rison.replace("'", "%27")
@@ -237,62 +237,85 @@ class ReportSchedule(AuditMixinNullable, ExtraJSONMixin, Model):
filter_type: str,
column_name: str,
values: list[Optional[str]],
) -> dict[str, Any]:
) -> tuple[dict[str, Any], Optional[str]]:
"""
Generate a native filter configuration for the given filter type.
Returns:
A tuple of (filter_config, warning_message). If the filter type is
unrecognized, returns an empty dict and a warning message.
"""
if filter_type == "filter_time":
# For select filters, we need to use the "IN" operator
return {
native_filter_id or "": {
"id": native_filter_id or "",
"extraFormData": {"time_range": values[0]},
"filterState": {"value": values[0]},
"ownState": {},
}
}
elif filter_type == "filter_timegrain":
return {
native_filter_id or "": {
"id": native_filter_id or "",
"extraFormData": {
"time_grain_sqla": values[0], # grain
},
"filterState": {
# "label": "30 second", # grain_label
"value": values # grain
},
"ownState": {},
}
}
return (
{
native_filter_id or "": {
"id": native_filter_id or "",
"extraFormData": {"time_range": values[0]},
"filterState": {"value": values[0]},
"ownState": {},
}
},
None,
)
if filter_type == "filter_timegrain":
return (
{
native_filter_id or "": {
"id": native_filter_id or "",
"extraFormData": {
"time_grain_sqla": values[0], # grain
},
"filterState": {
# "label": "30 second", # grain_label
"value": values # grain
},
"ownState": {},
}
},
None,
)
elif filter_type == "filter_timecolumn":
return {
native_filter_id or "": {
"extraFormData": {
"granularity_sqla": values[0] # column_name
},
"filterState": {
"value": values # column_name
},
}
}
if filter_type == "filter_timecolumn":
return (
{
native_filter_id or "": {
"extraFormData": {
"granularity_sqla": values[0] # column_name
},
"filterState": {
"value": values # column_name
},
}
},
None,
)
elif filter_type == "filter_select":
return {
native_filter_id or "": {
"id": native_filter_id or "",
"extraFormData": {
"filters": [
{"col": column_name or "", "op": "IN", "val": values or []}
]
},
"filterState": {
"label": column_name or "",
"validateStatus": False,
"value": values or [],
},
"ownState": {},
}
}
elif filter_type == "filter_range":
if filter_type == "filter_select":
return (
{
native_filter_id or "": {
"id": native_filter_id or "",
"extraFormData": {
"filters": [
{
"col": column_name or "",
"op": "IN",
"val": values or [],
}
]
},
"filterState": {
"label": column_name or "",
"validateStatus": False,
"value": values or [],
},
"ownState": {},
}
},
None,
)
if filter_type == "filter_range":
# For range filters, values should be [min, max] or [value] for single value
min_val = values[0] if len(values) > 0 else None
max_val = values[1] if len(values) > 1 else None
@@ -303,25 +326,33 @@ class ReportSchedule(AuditMixinNullable, ExtraJSONMixin, Model):
if max_val is not None:
filters.append({"col": column_name or "", "op": "<=", "val": max_val})
return {
native_filter_id or "": {
"id": native_filter_id or "",
"extraFormData": {"filters": filters},
"filterState": {
"value": [min_val, max_val],
"label": f"{min_val} ≤ x ≤ {max_val}"
if min_val and max_val
else f"x ≥ {min_val}"
if min_val
else f"x ≤ {max_val}"
if max_val
else "",
},
"ownState": {},
}
}
return (
{
native_filter_id or "": {
"id": native_filter_id or "",
"extraFormData": {"filters": filters},
"filterState": {
"value": [min_val, max_val],
"label": f"{min_val} ≤ x ≤ {max_val}"
if min_val and max_val
else f"x ≥ {min_val}"
if min_val
else f"x ≤ {max_val}"
if max_val
else "",
},
"ownState": {},
}
},
None,
)
return {}
warning_msg = (
f"Skipping native filter with unrecognized filter type '{filter_type}' "
f"(filter_id: {native_filter_id})"
)
logger.warning(warning_msg)
return {}, warning_msg
class ReportRecipients(Model, AuditMixinNullable):

View File

@@ -93,9 +93,10 @@ def test_report_generate_native_filter_no_values():
filter_type = "filter_select"
values = None
assert report_schedule._generate_native_filter(
result, warning = report_schedule._generate_native_filter(
native_filter_id, filter_type, column_name, values
) == {
)
assert result == {
"filter_id": {
"id": "filter_id",
"extraFormData": {
@@ -109,6 +110,7 @@ def test_report_generate_native_filter_no_values():
"ownState": {},
}
}
assert warning is None
def test_get_native_filters_params_missing_filter_values():
@@ -215,9 +217,10 @@ def test_report_generate_native_filter():
column_name = "column_name"
values = ["value1", "value2"]
assert report_schedule._generate_native_filter(
result, warning = report_schedule._generate_native_filter(
native_filter_id, filter_type, column_name, values
) == {
)
assert result == {
"filter_id": {
"extraFormData": {
"filters": [
@@ -233,6 +236,7 @@ def test_report_generate_native_filter():
"ownState": {},
}
}
assert warning is None
def test_get_native_filters_params_empty():
@@ -269,9 +273,10 @@ def test_report_generate_native_filter_empty_values():
column_name = "column_name"
values = []
assert report_schedule._generate_native_filter(
result, warning = report_schedule._generate_native_filter(
native_filter_id, filter_type, column_name, values
) == {
)
assert result == {
"filter_id": {
"extraFormData": {
"filters": [{"col": "column_name", "op": "IN", "val": []}]
@@ -285,6 +290,7 @@ def test_report_generate_native_filter_empty_values():
"ownState": {},
}
}
assert warning is None
def test_report_generate_native_filter_no_column_name():
@@ -297,9 +303,10 @@ def test_report_generate_native_filter_no_column_name():
column_name = ""
values = ["value1", "value2"]
assert report_schedule._generate_native_filter(
result, warning = report_schedule._generate_native_filter(
native_filter_id, filter_type, column_name, values
) == {
)
assert result == {
"filter_id": {
"extraFormData": {
"filters": [{"col": "", "op": "IN", "val": ["value1", "value2"]}]
@@ -313,3 +320,61 @@ def test_report_generate_native_filter_no_column_name():
"ownState": {},
}
}
assert warning is None
def test_report_generate_native_filter_unknown_filter_type():
"""
Test the ``_generate_native_filter`` method with an unknown filter type.
Should return empty dict and a warning message.
"""
report_schedule = ReportSchedule()
native_filter_id = "filter_id"
filter_type = "filter_unknown"
column_name = "column_name"
values = ["value1", "value2"]
result, warning = report_schedule._generate_native_filter(
native_filter_id, filter_type, column_name, values
)
assert result == {}
assert warning is not None
assert "unrecognized filter type" in warning
assert "filter_unknown" in warning
assert "filter_id" in warning
def test_get_native_filters_params_unknown_filter_type():
"""
Test the ``get_native_filters_params`` method with an unknown filter type.
Should skip the filter and include a warning.
"""
report_schedule = ReportSchedule()
report_schedule.extra = {
"dashboard": {
"nativeFilters": [
{
"nativeFilterId": "filter_1",
"filterType": "filter_unknown_type",
"columnName": "column_name",
"filterValues": ["value1"],
},
{
"nativeFilterId": "filter_2",
"filterType": "filter_select",
"columnName": "column_name",
"filterValues": ["value2"],
},
]
}
}
result, warnings = report_schedule.get_native_filters_params()
# The unknown filter should be skipped, valid filter should be present
assert "filter_2" in result
assert "filter_1" not in result
assert "value2" in result
# Should have one warning for the unknown filter type
assert len(warnings) == 1
assert "unrecognized filter type" in warnings[0]
assert "filter_unknown_type" in warnings[0]