feat(mcp): add remove_chart_from_dashboard tool#40958
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #40958 +/- ##
==========================================
- Coverage 64.50% 64.43% -0.07%
==========================================
Files 2666 2667 +1
Lines 146864 147073 +209
Branches 33910 33939 +29
==========================================
+ Hits 94728 94760 +32
- Misses 50421 50596 +175
- Partials 1715 1717 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code Review Agent Run #19fe4aActionable Suggestions - 0Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
8f2823f to
1ae5e61
Compare
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
- _find_chart_keys now matches chartId as both int and str to handle imported/hand-edited layouts where chartId may be stored as a string - validate position_json parses to a dict before passing to layout helpers, preventing AttributeError on malformed-but-valid JSON (e.g. "[]") - test: add return type annotation to mock_auth fixture; add docstrings to _chart_node and _call_remove test helpers Addresses CodeAnt AI review findings on apache#40958.
Code Review Agent Run #db9f95Actionable Suggestions - 0Additional Suggestions - 4
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
- _find_chart_keys now matches chartId as both int and str to handle imported/hand-edited layouts where chartId may be stored as a string - validate position_json parses to a dict before passing to layout helpers, preventing AttributeError on malformed-but-valid JSON (e.g. "[]") - test: add return type annotation to mock_auth fixture; add docstrings to _chart_node and _call_remove test helpers Addresses CodeAnt AI review findings on apache#40958.
af9ac74 to
4b889c5
Compare
|
Addressed in 9fe80bb — the 4 additional suggestions from Bito run #db9f95:
|
- _find_chart_keys now matches chartId as both int and str to handle imported/hand-edited layouts where chartId may be stored as a string - validate position_json parses to a dict before passing to layout helpers, preventing AttributeError on malformed-but-valid JSON (e.g. "[]") - test: add return type annotation to mock_auth fixture; add docstrings to _chart_node and _call_remove test helpers Addresses CodeAnt AI review findings on apache#40958.
9fe80bb to
283fb13
Compare
|
Addressed review comment (CodeAnt, comment 3482429213): Fixed the The The test now asserts |
Code Review Agent Run #1f6456Actionable Suggestions - 0Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
fe7cdc4 to
520750a
Compare
- _find_chart_keys now matches chartId as both int and str to handle imported/hand-edited layouts where chartId may be stored as a string - validate position_json parses to a dict before passing to layout helpers, preventing AttributeError on malformed-but-valid JSON (e.g. "[]") - test: add return type annotation to mock_auth fixture; add docstrings to _chart_node and _call_remove test helpers Addresses CodeAnt AI review findings on apache#40958.
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #88aeb7Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
d17e951 to
d33ee6c
Compare
- _find_chart_keys now matches chartId as both int and str to handle imported/hand-edited layouts where chartId may be stored as a string - validate position_json parses to a dict before passing to layout helpers, preventing AttributeError on malformed-but-valid JSON (e.g. "[]") - test: add return type annotation to mock_auth fixture; add docstrings to _chart_node and _call_remove test helpers Addresses CodeAnt AI review findings on apache#40958.
- _find_chart_keys now matches chartId as both int and str to handle imported/hand-edited layouts where chartId may be stored as a string - validate position_json parses to a dict before passing to layout helpers, preventing AttributeError on malformed-but-valid JSON (e.g. "[]") - test: add return type annotation to mock_auth fixture; add docstrings to _chart_node and _call_remove test helpers Addresses CodeAnt AI review findings on apache#40958.
- 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
…n_metadata When remove_chart_from_dashboard cleaned stale references from json_metadata and routed the updated blob through UpdateDashboardCommand, DashboardDAO.set_dash_metadata's positions branch ran and overwrote dashboard.slices from layout data — silently dropping charts that were in the slices relationship but absent from position_json. Instead, write the cleaned metadata directly to updated_dashboard.json_metadata and commit, bypassing the positions branch entirely. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- _clean_json_metadata now also removes the chart's entry from default_filters (stored as a JSON-encoded string inside json_metadata), matching the pruning done by DashboardDAO.set_dash_metadata when the positions branch runs. - The secondary json_metadata commit (written directly to avoid the set_dash_metadata positions-branch slices overwrite) is now wrapped in its own try/except: if it fails, the chart removal is still reported as successful and the error is logged as a warning, since the chart has already been detached from layout and slices. - Update docstrings and module-level description to mention default_filters. - Add test_clean_json_metadata_cleans_default_filters unit test. - Extend test_json_metadata_cleanup to cover default_filters cleanup.
Moves the json_metadata read from before UpdateDashboardCommand.run() to after the re-fetch so cleanup is applied to the latest persisted state. The old approach read a pre-command snapshot and wrote it back after the re-fetch, silently overwriting any concurrent edits to dashboard metadata. Updates test_json_metadata_cleanup to place json_metadata on the re-fetched dashboard mock (updated_dashboard), reflecting the new read path.
…ard malformed layout - Extract _clean_filter_scopes and _clean_default_filters helpers to bring _clean_json_metadata McCabe complexity from 14 to below the ruff C901 threshold of 10. - Fix bug in _clean_default_filters: when default_filters is already a dict (not a JSON-encoded string), re-serialize it back to a JSON string after removing the chart key so downstream json.loads calls don't receive a dict. - Return an error response instead of silently using an empty layout when position_json cannot be parsed; this prevents accidentally wiping dashboard layouts on malformed data. - Add test_clean_json_metadata_reserializes_dict_default_filters and test_malformed_position_json_returns_error covering the two bug fixes.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…=True Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
da20872 to
451eaab
Compare
|
Bito Automatic Review Skipped – PR Already Merged |
2 participants
SUMMARY
Adds a new MCP tool,
remove_chart_from_dashboard, the missing inverse ofadd_chart_to_existing_dashboard. Without it, agents that add a wrong chart to a dashboard have no recovery path.The tool:
position_json(a chart can appear under multiple tabs), and prunes ROW/COLUMN containers left empty by the removal so no orphaned wrapper nodes remain. TAB/TABS/GRID/ROOT containers are intentionally never pruned.slicesrelationship (ORM objects with the removed chart excluded) toUpdateDashboardCommand, since updatingposition_jsonalone does not syncdashboard.slices.json_metadata:expanded_slices,timed_refresh_immune_slices, andfilter_scopes(both filter-chart keys and per-columnimmunelists). When metadata changed, the new layout is included under"positions"soDashboardDAO.set_dash_metadatatakes its legacy branch that preserves and re-scopesfilter_scopesinstead of dropping them.permission_denied=Trueso the LLM informs the user instead of attempting a workaround), and chart not present in the dashboard.ToolAnnotations(readOnlyHint=False, destructiveHint=True)andtags=["mutate"]/Dashboard-writeRBAC, mirroringadd_chart_to_existing_dashboard(same authorization, command write, and post-write eager re-fetch patterns).The chart itself is not deleted — it remains available to other dashboards.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (MCP service tool, no UI changes)
TESTING INSTRUCTIONS
pytest tests/unit_tests/mcp_service/dashboard/tool/test_remove_chart_from_dashboard.pyadd_chart_to_existing_dashboardwith adashboard_id/chart_id, thenremove_chart_from_dashboardwith the same arguments.json_metadatano longer references the chart.Tests cover: dashboard not found, permission denied, chart not in dashboard, simple grid removal with empty-row pruning, chart inside a COLUMN (sibling kept; lone chart prunes COLUMN+ROW), chart under multiple tabs (all occurrences removed, tabs kept),
json_metadatacleanup, chart attached to slices but absent from layout, and malformed-metadata robustness.ADDITIONAL INFORMATION