codeant-ai-for-open-source[bot] commented on code in PR #43338:
URL: https://github.com/apache/superset/pull/43338#discussion_r3816157653


##########
tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py:
##########
@@ -1360,6 +1360,161 @@ async def 
test_json_format_also_eager_loads_metrics(self, mcp_server, mock_auth)
             assert _extract_metrics_load_path(query_options[0]) == ["table", 
"metrics"]
 
 
+class TestSavedChartExtraFormDataFilters:
+    """Regression tests: extra_form_data filters passed alongside a saved
+    chart identifier must reach the executed query, not just the cached
+    form_data / unsaved-chart path already covered elsewhere.
+
+    A chart with a saved query_context is the common case (any chart that
+    has been opened and saved through Explore), so this is the primary path
+    exercised when a caller passes extra_form_data with a chart identifier.
+    """
+
+    def _chart(self) -> SimpleNamespace:
+        from superset.utils import json as utils_json
+
+        return SimpleNamespace(
+            id=9,
+            slice_name="Sales",
+            viz_type="table",
+            datasource_id=1,
+            datasource_type="table",
+            query_context=utils_json.dumps(
+                {
+                    "datasource": {"id": 1, "type": "table"},
+                    "queries": [
+                        {
+                            "columns": ["country"],
+                            "metrics": ["count"],
+                            "filters": [],
+                            "row_limit": 100,
+                        }
+                    ],
+                    "result_format": "json",
+                    "result_type": "full",
+                }
+            ),
+            params=None,
+        )
+
+    async def _run(self, extra_form_data: dict[str, Any], mcp_server: Any) -> 
Any:
+        from unittest.mock import patch
+
+        from fastmcp import Client
+
+        module = importlib.import_module(
+            "superset.mcp_service.chart.tool.get_chart_data"
+        )
+
+        captured: dict[str, Any] = {}
+
+        def fake_load(self: Any, data: dict[str, Any]) -> Any:
+            captured["loaded_query_context_json"] = data
+            return SimpleNamespace(queries=[SimpleNamespace(filter=[])])
+
+        class _Command:
+            def __init__(self, query_context: Any) -> None: ...
+            def validate(self) -> None: ...
+            def run(self) -> dict[str, Any]:
+                return {
+                    "queries": [
+                        {
+                            "data": [{"country": "USA"}],
+                            "colnames": ["country"],
+                            "rowcount": 1,
+                        }
+                    ]
+                }

Review Comment:
   ✅ **Customized review instruction saved!**
   
   **Instruction:**
   > In this unit test module, do not require real schema loading or query 
execution for tests specifically verifying that extra_form_data is merged into 
the raw query dictionary; keep those dependencies stubbed and reserve 
end-to-end coverage for integration tests.
   
   **Applied to:**
     - `tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py`
   
   ---
   💡 *To manage or update this instruction, visit: [CodeAnt AI 
Settings](https://app.codeant.ai/org/settings/learnings)*



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to