Repository navigation
fix(dashboard): save an unreadable layout chart as a missing-chart tile - #45800
Draft
mikebridge wants to merge 14 commits into
Draft
mikebridge wants to merge 14 commits into
mikebridge wants to merge 14 commits into
Conversation
A dashboard save, create or copy rebuilds chart membership from the layout it is sent. Only link a chart the layout adds if the caller can read it; charts already on the dashboard (archived ones included) and, for a copy, the source dashboard's charts stay accepted, so existing saves keep working. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The update command refuses a layout chart the caller cannot read in validate(), before tab and native-filter removals deactivate reports and email their owners. The editors and viewers of the charts being checked are loaded in two queries instead of per chart. Tests cover a chart the caller reads through a granted dataset but does not edit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… charts in the layout check Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…art-ids Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…art-ids Keep both sides in set_dash_metadata: install the stored membership baseline, refuse unreadable new layout charts against the stored dashboard_slices rows, then keep archived members the layout omits. Kept archived charts are limited to stored members. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Being archived does not exempt a chart from the layout access check: an archived chart that is not already a member and that the caller cannot read is refused, and the stored membership is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Saving a dashboard layout that adds a chart the user cannot read is refused on update, create and copy. Existing members, archived ones included, and a copy's source charts are not checked.
A dashboard save, create or copy whose layout adds a chart the caller cannot read no longer fails with 422. The chart is left unlinked and its tile is saved as the same placeholder a nonexistent chart gets, so the response does not reveal that the chart exists. Existing members, archived ones included, and a copy's source charts are not checked. Copying with charts handles a tile for a chart outside the source the same way when the caller cannot read it; a readable one still refuses the copy.
A PUT that sends only position_json kept a tile for an existing chart the caller cannot read, while a nonexistent chart's tile became the missing-chart placeholder, so reading the layout back revealed which charts exist. The raw path and the copy-with-charts layout now repair a non-member chart the caller cannot read like a missing one. The copy-with-charts handling moves into that shared repair, which also covers every save route with a parity test.
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #45800 +/- ##
=======================================
Coverage 84.76% 84.76%
=======================================
Files 3072 3072
Lines 206119 206118 -1
Branches 48539 48539
=======================================
+ Hits 174718 174719 +1
+ Misses 28101 28100 -1
+ Partials 3300 3299 -1
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:
|
github-actions
Bot
added
the
requires:rebase
github-actions
Bot
removed
the
requires:rebase
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
Saving a dashboard layout that contains a tile for a chart you can't read no longer fails. The chart isn't attached to the dashboard, and its tile is saved as the same "This chart no longer exists." placeholder a deleted chart gets. This typically happens with a stale tile left after a chart was detached. Because the result is identical, a save doesn't reveal whether a chart you can't read exists.
This is stacked on #45538 and changes its behaviour for that one case. #45538 returns 422 for such a chart; this PR leaves it unlinked as a placeholder instead.
PUT(bothjson_metadata.positionsand rawposition_json),POST, and the copy endpoint, which the MCPduplicate_dashboardtool also uses.TESTING INSTRUCTIONS
pytest tests/unit_tests/dao/dashboard_test.py tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_dashboard.pypytest tests/integration_tests/dashboards/api_tests.py -k "layout_chart_access or matches_missing_chart or layout_detached_chart"ADDITIONAL INFORMATION