codeant-ai-for-open-source[bot] commented on code in PR #40399:
URL: https://github.com/apache/superset/pull/40399#discussion_r3422470752
##########
tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py:
##########
@@ -575,6 +686,166 @@ async def
test_generate_dashboard_empty_string_title_preserved(
created = mock_dashboard_cls.return_value
assert created.dashboard_title == ""
+ @patch("superset.models.dashboard.Dashboard")
+ @patch("superset.daos.dashboard.DashboardDAO.find_by_id")
+ @patch("superset.db.session")
+ @pytest.mark.asyncio
+ async def test_generate_dashboard_position_json_override(
+ self,
+ mock_db_session,
+ mock_find_by_id,
+ mock_dashboard_cls,
+ mcp_server,
+ ) -> None:
+ """An explicit ``position_json`` replaces the auto-generated layout
+ in full — the tool serializes the caller's dict verbatim into the
+ dashboard's ``position_json`` column rather than calling the
+ layout-builder helper."""
+ from superset.utils import json
+
+ charts = [_mock_chart(id=1, slice_name="Sales")]
+ mock_dashboard = _mock_dashboard(id=70, title="Custom Layout
Dashboard")
+ _setup_generate_dashboard_mocks(
+ mock_db_session,
+ mock_find_by_id,
+ mock_dashboard_cls,
+ charts,
+ mock_dashboard,
+ )
+
+ custom_layout = {
+ "ROOT_ID": {"type": "ROOT", "children": ["GRID_ID"]},
+ "GRID_ID": {"type": "GRID", "children": ["ROW-custom"]},
+ "ROW-custom": {
+ "type": "ROW",
+ "children": ["CHART-1"],
+ "meta": {"background": "BACKGROUND_TRANSPARENT"},
+ },
+ "CHART-1": {
+ "type": "CHART",
+ "children": [],
+ "meta": {"chartId": 1, "width": 12, "height": 100},
+ },
+ }
+ request = {
+ "chart_ids": [1],
+ "dashboard_title": "Custom Layout Dashboard",
+ "position_json": custom_layout,
+ }
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "generate_dashboard", {"request": request}
+ )
+
+ assert result.structured_content["error"] is None
+ created = mock_dashboard_cls.return_value
+ # position_json on the model is a JSON string; round-trip it
+ # and verify the caller's layout — not the auto-generated 2-col
+ # grid — was written.
+ stored = json.loads(created.position_json)
+ assert stored == custom_layout
+ # The auto-generated layout's HEADER/ROW ids wouldn't match
+ # `ROW-custom`; this sanity-check guards against regressions
+ # where the override silently merges with the default.
+ assert "ROW-custom" in stored
+
+ @patch("superset.models.dashboard.Dashboard")
+ @patch("superset.daos.dashboard.DashboardDAO.find_by_id")
+ @patch("superset.db.session")
+ @pytest.mark.asyncio
+ async def test_generate_dashboard_json_metadata_overrides_shallow_merged(
+ self,
+ mock_db_session,
+ mock_find_by_id,
+ mock_dashboard_cls,
+ mcp_server,
+ ) -> None:
+ """``json_metadata_overrides`` is shallow-merged on top of the
+ defaults: caller-supplied keys win, defaults for keys the caller
+ didn't touch survive."""
+ from superset.utils import json
+
+ charts = [_mock_chart(id=1, slice_name="Sales")]
+ mock_dashboard = _mock_dashboard(id=71, title="Themed Dashboard")
+ _setup_generate_dashboard_mocks(
+ mock_db_session,
+ mock_find_by_id,
+ mock_dashboard_cls,
+ charts,
+ mock_dashboard,
+ )
+
+ overrides = {
+ "label_colors": {"Electronics": "#4C78A8"},
+ "cross_filters_enabled": True,
+ }
+ request = {
+ "chart_ids": [1],
+ "dashboard_title": "Themed Dashboard",
+ "json_metadata_overrides": overrides,
+ }
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "generate_dashboard", {"request": request}
+ )
+
+ assert result.structured_content["error"] is None
+ created = mock_dashboard_cls.return_value
+ merged = json.loads(created.json_metadata)
+ # Caller overrides land verbatim
+ assert merged["label_colors"] == {"Electronics": "#4C78A8"}
+ assert merged["cross_filters_enabled"] is True
+ # Defaults for keys the caller did NOT supply must still be
+ # present — this is what makes it a *shallow merge* rather
+ # than a full replace.
+ assert "timed_refresh_immune_slices" in merged
+ assert "refresh_frequency" in merged
+
+ @patch("superset.models.dashboard.Dashboard")
+ @patch("superset.daos.dashboard.DashboardDAO.find_by_id")
+ @patch("superset.db.session")
+ @pytest.mark.asyncio
+ async def test_generate_dashboard_slug_and_css_applied(
+ self,
+ mock_db_session,
+ mock_find_by_id,
+ mock_dashboard_cls,
+ mcp_server,
+ ) -> None:
Review Comment:
**Suggestion:** Provide concrete type hints for this method's parameters so
the new test is fully annotated, not only the return value. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The method is newly added and its fixture/mock parameters lack type hints.
Because the custom rule requires new Python code to be fully typed, this
is a real violation and the suggestion is verified.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=331ceccba75a4b34b1e23f2dc030a3bb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=331ceccba75a4b34b1e23f2dc030a3bb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py
**Line:** 810:816
**Comment:**
*Custom Rule: Provide concrete type hints for this method's parameters
so the new test is fully annotated, not only the return value.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40399&comment_hash=29bf660d2245bdaa33b7f9485b630cfdbc9727188ff7fea8739f5fd9b7466953&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40399&comment_hash=29bf660d2245bdaa33b7f9485b630cfdbc9727188ff7fea8739f5fd9b7466953&reaction=dislike'>👎</a>
--
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]
