Skip to content

Commit 2dfc770

Browse files
authored
fix(native-filters): update TEMPORAL_RANGE filter subject when Time Column filter is applied (#36985)
1 parent 6b7b23e commit 2dfc770

File tree

3 files changed

+662
-4
lines changed

3 files changed

+662
-4
lines changed

superset/utils/core.py

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,6 +1011,39 @@ def form_data_to_adhoc(form_data: dict[str, Any], clause: str) -> AdhocFilterCla
10111011
return result
10121012

10131013

1014+
def _update_existing_temporal_filter(
1015+
temporal_filter: AdhocFilterClause,
1016+
granularity_sqla_override: str | None,
1017+
time_range: str | None,
1018+
chart_has_granularity_sqla: bool,
1019+
) -> None:
1020+
"""Update an existing temporal filter with new subject/comparator."""
1021+
if (
1022+
granularity_sqla_override is not None
1023+
and temporal_filter.get("expressionType") == "SIMPLE"
1024+
):
1025+
temporal_filter["subject"] = granularity_sqla_override
1026+
if time_range and not chart_has_granularity_sqla:
1027+
temporal_filter["comparator"] = time_range
1028+
1029+
1030+
def _create_temporal_filter(
1031+
granularity_sqla: str,
1032+
time_range: str,
1033+
) -> AdhocFilterClause:
1034+
"""Create a new TEMPORAL_RANGE adhoc filter."""
1035+
new_filter: AdhocFilterClause = {
1036+
"clause": "WHERE",
1037+
"expressionType": "SIMPLE",
1038+
"operator": FilterOperator.TEMPORAL_RANGE,
1039+
"subject": granularity_sqla,
1040+
"comparator": time_range,
1041+
"isExtra": True,
1042+
}
1043+
new_filter["filterOptionName"] = hash_from_dict(cast(dict[Any, Any], new_filter))
1044+
return new_filter
1045+
1046+
10141047
def merge_extra_form_data(form_data: dict[str, Any]) -> None: # noqa: C901
10151048
"""
10161049
Merge extra form data (appends and overrides) into the main payload
@@ -1059,10 +1092,35 @@ def merge_extra_form_data(form_data: dict[str, Any]) -> None: # noqa: C901
10591092
for fltr in append_filters
10601093
if fltr
10611094
)
1062-
if form_data.get("time_range") and not form_data.get("granularity_sqla"):
1063-
for adhoc_filter in form_data.get("adhoc_filters", []):
1064-
if adhoc_filter.get("operator") == "TEMPORAL_RANGE":
1065-
adhoc_filter["comparator"] = form_data["time_range"]
1095+
1096+
granularity_sqla_override = extra_form_data.get("granularity_sqla")
1097+
time_range = form_data.get("time_range")
1098+
chart_has_granularity_sqla = bool(form_data.get("granularity_sqla"))
1099+
1100+
temporal_filters = [
1101+
adhoc_filter
1102+
for adhoc_filter in adhoc_filters
1103+
if adhoc_filter.get("operator") == FilterOperator.TEMPORAL_RANGE
1104+
]
1105+
1106+
for temporal_filter in temporal_filters:
1107+
_update_existing_temporal_filter(
1108+
temporal_filter,
1109+
granularity_sqla_override,
1110+
time_range,
1111+
chart_has_granularity_sqla,
1112+
)
1113+
1114+
if (
1115+
not temporal_filters
1116+
and granularity_sqla_override is not None
1117+
and time_range is not None
1118+
):
1119+
new_temporal_filter = _create_temporal_filter(
1120+
granularity_sqla_override,
1121+
cast(str, time_range),
1122+
)
1123+
adhoc_filters.append(new_temporal_filter)
10661124

10671125

10681126
def merge_extra_filters(form_data: dict[str, Any]) -> None: # noqa: C901

tests/integration_tests/explore/api_tests.py

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,3 +254,95 @@ def test_get_url_params(test_client, login_as_admin, chart_id):
254254
"foo": "bar",
255255
"slice_id": str(chart_id),
256256
}
257+
258+
259+
def test_granularity_sqla_override_updates_temporal_range_filter_subject(
260+
test_client, login_as_admin, chart_id, admin_id, dataset
261+
):
262+
"""
263+
Test that extra_form_data.granularity_sqla overrides TEMPORAL_RANGE filter subject.
264+
265+
The flow is:
266+
1. Chart has TEMPORAL_RANGE adhoc filters on various columns
267+
2. Dashboard applies Time Column native filter selecting 'year' via extra_form_data
268+
3. Explore API processes form_data through merge_extra_filters/merge_extra_form_data
269+
4. All TEMPORAL_RANGE filter subjects should be updated to 'year'
270+
"""
271+
272+
form_data_with_temporal_filter = {
273+
"datasource": f"{dataset.id}__{dataset.type}",
274+
"viz_type": "country_map",
275+
"time_range": "Last week",
276+
"adhoc_filters": [
277+
{
278+
"clause": "WHERE",
279+
"comparator": "No filter",
280+
"expressionType": "SIMPLE",
281+
"operator": "TEMPORAL_RANGE",
282+
"subject": "some_other_time_col",
283+
},
284+
{
285+
"clause": "WHERE",
286+
"comparator": "foo",
287+
"expressionType": "SIMPLE",
288+
"operator": "==",
289+
"subject": "non_temporal_col",
290+
},
291+
{
292+
"clause": "WHERE",
293+
"comparator": "Last year",
294+
"expressionType": "SIMPLE",
295+
"operator": "TEMPORAL_RANGE",
296+
"subject": "another_time_col",
297+
},
298+
],
299+
"extra_form_data": {
300+
"granularity_sqla": "year",
301+
"time_range": "Last month",
302+
},
303+
}
304+
305+
test_form_data_key = f"test_granularity_override_key_{chart_id}_{dataset.id}"
306+
entry: TemporaryExploreState = {
307+
"owner": admin_id,
308+
"datasource_id": dataset.id,
309+
"datasource_type": dataset.type,
310+
"chart_id": chart_id,
311+
"form_data": json.dumps(form_data_with_temporal_filter),
312+
}
313+
cache_manager.explore_form_data_cache.set(test_form_data_key, entry)
314+
315+
try:
316+
resp = test_client.get(
317+
f"api/v1/explore/?form_data_key={test_form_data_key}"
318+
f"&datasource_id={dataset.id}&datasource_type={dataset.type}"
319+
)
320+
assert resp.status_code == 200
321+
322+
data = json.loads(resp.data.decode("utf-8"))
323+
result = data.get("result")
324+
form_data = result["form_data"]
325+
326+
adhoc_filters = form_data.get("adhoc_filters", [])
327+
temporal_range_filters = [
328+
f
329+
for f in adhoc_filters
330+
if f.get("operator") == "TEMPORAL_RANGE"
331+
and f.get("expressionType") == "SIMPLE"
332+
]
333+
334+
assert len(temporal_range_filters) == 2, "Expected two TEMPORAL_RANGE filters"
335+
for temporal_filter in temporal_range_filters:
336+
assert temporal_filter["subject"] == "year", (
337+
"Time Column native filter (granularity_sqla) should override "
338+
"TEMPORAL_RANGE filter subject for all matching filters"
339+
)
340+
341+
non_temporal_filters = [f for f in adhoc_filters if f.get("operator") == "=="]
342+
assert len(non_temporal_filters) == 1
343+
assert non_temporal_filters[0]["subject"] == "non_temporal_col"
344+
345+
assert form_data["time_range"] == "Last month"
346+
assert form_data.get("granularity") == "year"
347+
finally:
348+
cache_manager.explore_form_data_cache.delete(test_form_data_key)

0 commit comments

Comments
 (0)