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 ---