You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by hu...@apache.org on 2026/06/16 22:54:59 UTC
(superset) 01/12: refactor(charts): extract query-context filter merge helper
This is an automated email from the ASF dual-hosted git repository.
hughhhh pushed a commit to branch hughhhh/dashboard-export-spec-review
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 5421799b68f3e0a55680b983f33a8925967fe665
Author: Hugh A Miles II <hu...@gmail.com>
AuthorDate: Sat Jun 13 09:52:46 2026 -0700
refactor(charts): extract query-context filter merge helper
Extract the extra_form_data merge block from the chart data GET endpoint
into apply_extra_form_data_to_query_context_json() in dashboard_filter_context,
so it can be reused by the upcoming dashboard Excel export task. No behavior
change; adds unit tests for the extracted helper.
Co-Authored-By: Claude Opus 4.8 <no...@anthropic.com>
---
superset/charts/data/api.py | 43 ++++----------------
superset/charts/data/dashboard_filter_context.py | 47 ++++++++++++++++++++++
.../charts/test_dashboard_filter_context.py | 47 ++++++++++++++++++++++
3 files changed, 101 insertions(+), 36 deletions(-)
diff --git a/superset/charts/data/api.py b/superset/charts/data/api.py
index d6ddd595981..d4ccfd2d775 100644
--- a/superset/charts/data/api.py
+++ b/superset/charts/data/api.py
@@ -32,6 +32,7 @@ from superset.async_events.async_query_manager import AsyncQueryTokenException
from superset.charts.api import ChartRestApi
from superset.charts.client_processing import apply_client_processing
from superset.charts.data.dashboard_filter_context import (
+ apply_extra_form_data_to_query_context_json,
DashboardFilterContext,
get_dashboard_filter_context,
)
@@ -50,11 +51,7 @@ from superset.commands.chart.exceptions import (
)
from superset.common.chart_data import ChartDataResultFormat, ChartDataResultType
from superset.connectors.sqla.models import BaseDatasource
-from superset.constants import (
- CACHE_DISABLED_TIMEOUT,
- EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS,
- EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS,
-)
+from superset.constants import CACHE_DISABLED_TIMEOUT
from superset.daos.exceptions import DatasourceNotFound
from superset.exceptions import QueryObjectValidationError, SupersetSecurityException
from superset.extensions import event_logger
@@ -204,37 +201,11 @@ class ChartDataRestApi(ChartRestApi):
except SupersetSecurityException:
return self.response_403()
- if dashboard_filter_context.extra_form_data:
- efd = dashboard_filter_context.extra_form_data
- extra_filters = efd.get("filters", [])
-
- for query in json_body.get("queries", []):
- if extra_filters:
- existing = query.get("filters") or []
- query["filters"] = existing + [
- {**f, "isExtra": True} for f in extra_filters
- ]
-
- extras = query.get("extras") or {}
- for key in EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS:
- if key in efd:
- extras[key] = efd[key]
- if extras:
- query["extras"] = extras
-
- for (
- src_key,
- target_key,
- ) in EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS.items():
- if src_key in efd:
- query[target_key] = efd[src_key]
-
- query["extra_form_data"] = efd
-
- # We need to apply the form data to the global context as jinja
- # templating pulls form data from the request globally, so this
- # fallback ensures it has the filters and extra_form_data applied
- # when used in get_sqla_query which constructs the final query.
+ # Merge the scoped native filters and extra_form_data into the saved
+ # query context so get_sqla_query constructs the final, filtered query.
+ apply_extra_form_data_to_query_context_json(
+ json_body, dashboard_filter_context.extra_form_data
+ )
# Jinja macros like metric() resolve dataset context from g.form_data
# when not given an explicit dataset_id. For GET requests there is no
diff --git a/superset/charts/data/dashboard_filter_context.py b/superset/charts/data/dashboard_filter_context.py
index 50b6ef4244e..cab9bbc45a1 100644
--- a/superset/charts/data/dashboard_filter_context.py
+++ b/superset/charts/data/dashboard_filter_context.py
@@ -314,3 +314,50 @@ def get_dashboard_filter_context(
)
return context
+
+
+def apply_extra_form_data_to_query_context_json(
+ json_body: dict[str, Any],
+ extra_form_data: dict[str, Any],
+) -> None:
+ """
+ Merge a dashboard filter context's ``extra_form_data`` into a raw query
+ context JSON body, mutating ``json_body`` in place.
+
+ For each query it appends scoped native filters (flagged ``isExtra``),
+ overrides ``extras`` keys (e.g. ``relative_start``) and regular mappings
+ (e.g. ``time_range``), and exposes the full ``extra_form_data`` so Jinja
+ templating can resolve it in ``get_sqla_query``. Shared by the chart data
+ API (GET) and the dashboard Excel export task.
+
+ :param json_body: The query context JSON body (mutated in place)
+ :param extra_form_data: The merged extra_form_data to apply
+ """
+ if not extra_form_data:
+ return
+
+ efd = extra_form_data
+ extra_filters = efd.get("filters", [])
+
+ for query in json_body.get("queries", []):
+ if extra_filters:
+ existing = query.get("filters") or []
+ query["filters"] = existing + [
+ {**f, "isExtra": True} for f in extra_filters
+ ]
+
+ extras = query.get("extras") or {}
+ for key in EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS:
+ if key in efd:
+ extras[key] = efd[key]
+ if extras:
+ query["extras"] = extras
+
+ for (
+ src_key,
+ target_key,
+ ) in EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS.items():
+ if src_key in efd:
+ query[target_key] = efd[src_key]
+
+ query["extra_form_data"] = efd
diff --git a/tests/unit_tests/charts/test_dashboard_filter_context.py b/tests/unit_tests/charts/test_dashboard_filter_context.py
index bfc2097f725..b91aaaba467 100644
--- a/tests/unit_tests/charts/test_dashboard_filter_context.py
+++ b/tests/unit_tests/charts/test_dashboard_filter_context.py
@@ -16,6 +16,7 @@
# under the License.
from __future__ import annotations
+from copy import deepcopy
from typing import Any
from unittest.mock import MagicMock, patch
@@ -27,6 +28,7 @@ from superset.charts.data.dashboard_filter_context import (
_is_filter_in_scope_for_chart,
_merge_extra_form_data,
_validate_chart_on_dashboard,
+ apply_extra_form_data_to_query_context_json,
DashboardFilterContext,
DashboardFilterStatus,
get_dashboard_filter_context,
@@ -277,6 +279,51 @@ def test_merge_extra_form_data_merges_custom_form_data_dicts() -> None:
assert merged["custom_form_data"] == {"groupby": ["col1"], "foo": "bar"}
+# --- apply_extra_form_data_to_query_context_json ---
+
+
+def test_apply_extra_form_data_noop_when_empty() -> None:
+ """An empty extra_form_data leaves the body untouched."""
+ original = {"queries": [{"filters": [{"col": "a", "op": "IN", "val": ["x"]}]}]}
+ json_body = deepcopy(original)
+ apply_extra_form_data_to_query_context_json(json_body, {})
+ assert json_body == original
+
+
+def test_apply_extra_form_data_appends_filters_with_is_extra() -> None:
+ json_body = {"queries": [{"filters": [{"col": "a", "op": "IN", "val": ["x"]}]}]}
+ efd = {"filters": [{"col": "region", "op": "IN", "val": ["US"]}]}
+ apply_extra_form_data_to_query_context_json(json_body, efd)
+ filters = json_body["queries"][0]["filters"]
+ assert len(filters) == 2
+ assert filters[0] == {"col": "a", "op": "IN", "val": ["x"]}
+ assert filters[1] == {"col": "region", "op": "IN", "val": ["US"], "isExtra": True}
+ assert json_body["queries"][0]["extra_form_data"] == efd
+
+
+def test_apply_extra_form_data_overrides_regular_mapping() -> None:
+ """time_range in extra_form_data overrides the query's time_range."""
+ json_body = {"queries": [{"time_range": "last week"}]}
+ apply_extra_form_data_to_query_context_json(json_body, {"time_range": "last month"})
+ assert json_body["queries"][0]["time_range"] == "last month"
+
+
+def test_apply_extra_form_data_overrides_extras_keys() -> None:
+ """relative_start/relative_end land in the query's extras dict."""
+ json_body = {"queries": [{}]}
+ apply_extra_form_data_to_query_context_json(
+ json_body, {"relative_start": "2020-01-01"}
+ )
+ assert json_body["queries"][0]["extras"]["relative_start"] == "2020-01-01"
+
+
+def test_apply_extra_form_data_handles_missing_queries() -> None:
+ """A body without a queries key is tolerated (no-op)."""
+ json_body: dict[str, Any] = {}
+ apply_extra_form_data_to_query_context_json(json_body, {"time_range": "last month"})
+ assert json_body == {}
+
+
# --- _get_filter_target_column ---