fix(dashboard-import): remap chartsInScope on import (#26338)#40140
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #40140 +/- ##
==========================================
- Coverage 64.18% 64.18% -0.01%
==========================================
Files 2591 2591
Lines 138446 138455 +9
Branches 32116 32118 +2
==========================================
+ Hits 88868 88870 +2
- Misses 48047 48053 +6
- Partials 1531 1532 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #c144e5
Actionable Suggestions - 1
-
tests/unit_tests/dashboards/commands/importers/v1/utils_test.py - 1
- Missing chartsInScope Remapping · Line 126-216
Review Details
-
Files reviewed - 1 · Commit Range:
cdab809..cdab809- tests/unit_tests/dashboards/commands/importers/v1/utils_test.py
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
rusackas
changed the title
There was a problem hiding this comment.
Code Review Agent Run #5da3f2
Actionable Suggestions - 1
-
superset/commands/dashboard/importers/v1/utils.py - 1
- Semantic Duplication in chartsInScope Handling · Line 154-158
Review Details
-
Files reviewed - 1 · Commit Range:
cdab809..31a7ebe- superset/commands/dashboard/importers/v1/utils.py
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
Per bito review on PR #40140: the native-filter and cross-filter blocks were running identical chartsInScope remap logic. Pull the shared shape into a single private helper so the two callsites can't drift. No behavior change. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code Review Agent Run #ad0d84Actionable 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 |
rusackas
requested review from
betodealmeida,
dpgaspar,
hainenber and
mistercrunch
aminghadersohi
left a comment
aminghadersohi
left a comment
There was a problem hiding this comment.
The fix is correct and well-tested.
chartsInScope on native filters and cross-filter configs holds source-environment integer chart IDs after export. update_id_refs was already remapping every other ID-bearing field (scope.excluded, position chartIds, filter_scopes keys, etc.) but skipping chartsInScope, leaving stale source-env IDs that broke filtersInScope/filtersOutScope computation on the imported dashboard.
The _remap_charts_in_scope helper cleanly DRYs the remap across both surfaces (native filters and cross-filter configs), and drop-on-unresolvable matches the scope.excluded convention.
One NIT: the docstring in test_update_id_refs_remaps_charts_in_scope says the export converts chartsInScope to UUIDs (citing export_example.py:325), but the production ExportDashboardsCommand does not — it leaves integer IDs. The test fixture and the id_map usage are correct; only the comment is misleading.
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Closes #26338 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
`chartsInScope` is a denormalized cache of the charts a filter currently applies to. `update_id_refs` was remapping the rest of the scope contract (`scope.excluded`, position chartIds, etc.) but leaving chartsInScope untouched, so the imported dashboard's filtersInScope / filtersOutScope computation operated on stale source-environment IDs — and filters ended up applied to the wrong charts (or none at all). Adds two remap passes mirroring the existing `scope.excluded` handling: - One on each `native_filter_configuration` entry's `chartsInScope`. - One on each `chart_configuration[*].crossFilters.chartsInScope` (the cross-filter version). Drop unresolvable IDs rather than passing them through, matching the existing convention for `scope.excluded`. Companion regression tests added in this PR's earlier commit cover both cases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Per bito review on PR #40140: the native-filter and cross-filter blocks were running identical chartsInScope remap logic. Pull the shared shape into a single private helper so the two callsites can't drift. No behavior change. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Picks up the third call site flagged by @sadpandajoe — the per-chart and native-filter chartsInScope branches were both routed through _remap_charts_in_scope, but global_chart_configuration.chartsInScope (which sits next to global_chart_configuration.scope.excluded) was missed and would keep stale source-env IDs after import. Adds a companion test test_update_id_refs_remaps_global_chart_configuration_charts_in_scope that builds the same source-env shape used in the other two regression tests and asserts unresolved IDs drop while resolved IDs map across. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rusackas
force-pushed
the
tdd/issue-26338-chartsinscope-import
branch
from
994834c to
e2ba073
Compare
|
Bito Automatic Review Skipped – PR Already Merged |
4 participants
SUMMARY
Closes #26338.
chartsInScopeis a denormalized cache of the charts a native filter (or cross-filter) currently applies to.update_id_refswas remapping the rest of the scope contract (scope.excluded, position chart IDs, native filter dataset IDs, etc.) but leavingchartsInScopeuntouched. The imported dashboard'sfiltersInScope/filtersOutScopecomputation then operated on stale source-environment IDs — filters ended up applied to the wrong charts (or none at all), exactly as #26338 reports.This PR:
Adds a
chartsInScoperemap pass insuperset/commands/dashboard/importers/v1/utils.pyfor both:native_filter_configurationentry'schartsInScopechart_configuration[*].crossFilters.chartsInScope(the cross-filter version)Mirrors the existing
scope.excludedhandling. Drops unresolvable IDs rather than passing them through, matching the established convention.Locks in regression tests (added in the PR's earlier commit):
test_update_id_refs_remaps_charts_in_scope— native filter casetest_update_id_refs_remaps_cross_filter_charts_in_scope— cross-filter caseBEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend ID-remapping change. CI on the test commit (before this fix) showed the bug verbatim:
CI on this fix commit should green.
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION
chartsInScopereferences are not fixed during a Dashboard import #26338🤖 Generated with Claude Code