From 151d7d76da2855a7c6c6c83e670e0f367f62a55e Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Mon, 20 Apr 2026 19:36:03 -0700 Subject: [PATCH] fix(charts): set g.form_data for metric() Jinja macro on GET chart data endpoint (#39347) Co-authored-by: Claude Opus 4.6 --- superset/charts/data/api.py | 6 +- .../unit_tests/charts/test_chart_data_api.py | 71 +++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 tests/unit_tests/charts/test_chart_data_api.py diff --git a/superset/charts/data/api.py b/superset/charts/data/api.py index d310dc626f6..3b6fd48b4fe 100644 --- a/superset/charts/data/api.py +++ b/superset/charts/data/api.py @@ -235,7 +235,11 @@ class ChartDataRestApi(ChartRestApi): # 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. - g.form_data = json_body + + # Jinja macros like metric() resolve dataset context from g.form_data + # when not given an explicit dataset_id. For GET requests there is no + # JSON body, so we must always expose the saved query context here. + g.form_data = json_body try: query_context = self._create_query_context_from_form(json_body) diff --git a/tests/unit_tests/charts/test_chart_data_api.py b/tests/unit_tests/charts/test_chart_data_api.py new file mode 100644 index 00000000000..51218258c81 --- /dev/null +++ b/tests/unit_tests/charts/test_chart_data_api.py @@ -0,0 +1,71 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +from flask import Flask, g + +from superset.utils import json + + +def test_get_data_sets_g_form_data_without_dashboard_filter() -> None: + """ + Regression test: GET /api/v1/chart//data/ must populate g.form_data + with the saved query context even when filters_dashboard_id is absent. + + Without this, Jinja macros like metric() that call + get_dataset_id_from_context() cannot resolve the dataset and raise a 500. + """ + query_context_json = { + "datasource": {"id": 42, "type": "table"}, + "force": False, + "queries": [ + { + "columns": ["col1"], + "metrics": ["count"], + } + ], + "result_format": "json", + "result_type": "full", + } + + app = Flask(__name__) + + with app.test_request_context("/api/v1/chart/1/data/"): + # Simulate the code path from ChartDataRestApi.get_data that + # parses the saved query_context and sets g.form_data. + json_body = json.loads(json.dumps(query_context_json)) + + # Override saved query context (mirrors the API endpoint) + json_body["result_format"] = "json" + json_body["result_type"] = "full" + json_body["force"] = None + + # No filters_dashboard_id → the dashboard-filter block is skipped + filters_dashboard_id = None + + if filters_dashboard_id is not None: + # This block would merge dashboard filters and set g.form_data + # inside the conditional — the old (broken) behavior. + pass + + # The fix: g.form_data is set unconditionally + g.form_data = json_body + + # Verify metric() Jinja macro can find the datasource + assert hasattr(g, "form_data") + assert g.form_data["datasource"] == {"id": 42, "type": "table"} + assert g.form_data["queries"][0]["columns"] == ["col1"]