Skip to content

Commit 9fe80bb

Browse files
fix(mcp): address review feedback on remove_chart_from_dashboard
- Fix ruff E501: shorten _call_remove docstring to ≤88 chars - Add remove_chart_from_dashboard to FieldPermissionsMiddleware TOOL_OBJECT_TYPE_MAP (was missing, unlike add_chart_to_existing_dashboard) - Add remove_chart_from_dashboard to tool-search-optimization.md mutate tools list for documentation consistency - Add created_on/changed_on to the re-fetch-fallback DashboardInfo path so its shape matches the successful re-fetch path - Add removed_layout_keys=[] and permission_denied=False to the exception handler response for a consistent schema across all code paths
1 parent 4b889c5 commit 9fe80bb

4 files changed

Lines changed: 7 additions & 2 deletions

File tree

superset/mcp_service/dashboard/tool/remove_chart_from_dashboard.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,8 @@ def remove_chart_from_dashboard( # noqa: C901 — complexity is structural (lay
366366
id=updated_dashboard.id,
367367
dashboard_title=updated_dashboard.dashboard_title,
368368
published=updated_dashboard.published,
369+
created_on=updated_dashboard.created_on,
370+
changed_on=updated_dashboard.changed_on,
369371
chart_count=len(remaining_slices),
370372
url=dashboard_url,
371373
),
@@ -439,5 +441,7 @@ def remove_chart_from_dashboard( # noqa: C901 — complexity is structural (lay
439441
return RemoveChartFromDashboardResponse(
440442
dashboard=None,
441443
dashboard_url=None,
444+
removed_layout_keys=[],
445+
permission_denied=False,
442446
error=f"Failed to remove chart from dashboard: {str(e)}",
443447
)

superset/mcp_service/docs/tool-search-optimization.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ Superset MCP tools are categorized with tags to help clients configure optimal l
3434
| `core` | Essential discovery and health tools | `health_check`, `get_instance_info`, `list_charts`, `list_dashboards`, `list_datasets` | Always load |
3535
| `discovery` | Detailed resource information and schema | `get_chart_info`, `get_dashboard_info`, `get_dataset_info`, `get_schema` | Can defer |
3636
| `data` | Data retrieval and previews | `get_chart_preview`, `get_chart_data` | Defer |
37-
| `mutate` | Create/modify resources | `generate_chart`, `update_chart`, `update_chart_preview`, `generate_dashboard`, `add_chart_to_existing_dashboard`, `execute_sql` | Defer |
37+
| `mutate` | Create/modify resources | `generate_chart`, `update_chart`, `update_chart_preview`, `generate_dashboard`, `add_chart_to_existing_dashboard`, `remove_chart_from_dashboard`, `execute_sql` | Defer |
3838
| `explore` | URL generation for exploration | `generate_explore_link`, `open_sql_lab_with_context` | Defer |
3939

4040
## Client Configuration

superset/mcp_service/middleware.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,6 +1056,7 @@ class FieldPermissionsMiddleware(Middleware):
10561056
"get_dashboard_info": "dashboard",
10571057
"generate_dashboard": "dashboard",
10581058
"add_chart_to_existing_dashboard": "dashboard",
1059+
"remove_chart_from_dashboard": "dashboard",
10591060
}
10601061

10611062
async def on_call_tool(

tests/unit_tests/mcp_service/dashboard/tool/test_remove_chart_from_dashboard.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ def _tabbed_layout() -> dict[str, Any]:
256256
async def _call_remove(
257257
mcp_server: object, dashboard_id: int = 1, chart_id: int = 10
258258
) -> dict[str, Any]:
259-
"""Call remove_chart_from_dashboard via the MCP client and return structured content."""
259+
"""Call remove_chart_from_dashboard via MCP client; return structured content."""
260260
async with Client(mcp_server) as client:
261261
result = await client.call_tool(
262262
"remove_chart_from_dashboard",

0 commit comments

Comments
 (0)