fix(dashboard): replace chartsInScope references at import time#38171
fix(dashboard): replace chartsInScope references at import time#38171
Conversation
|
Related Documentation Checked 0 published document(s) in 2 knowledge base(s). No updates required. How did I do? Any feedback? |
Code Review Agent Run #6f38e8Actionable 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 |
Sequence DiagramShows how the import process now remaps chartsInScope references (native filters, global config, and per-chart cross-filters) using the new chart ID map so imported dashboards reference the newly created charts. sequenceDiagram
participant Importer
participant Utils
participant IDMap
participant Storage
Importer->>Utils: update_id_refs(config, id_map)
Utils->>IDMap: resolve chart UUIDs -> id_map
Utils->>Utils: Remap native_filter_configuration.chartsInScope using id_map
Utils->>Utils: update_cross_filter_scoping -> remap global_chart_configuration.chartsInScope and chart_configuration[*].crossFilters.chartsInScope
Utils-->>Importer: return fixed config (all chartsInScope updated)
Importer->>Storage: create/import dashboard and charts with updated references
Generated by CodeAnt AI |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| for old_id_str, chart_config in metadata["chart_configuration"].items(): | ||
| try: | ||
| old_id_int = int(old_id_str) |
There was a problem hiding this comment.
Suggestion: The new update_cross_filter_scoping implementation assumes that metadata["chart_configuration"] is keyed by numeric chart IDs (strings convertible to int), but export_example.remap_chart_configuration writes example dashboards with chart_configuration keyed by chart UUIDs; as a result, per-chart cross-filter configurations from example exports are silently skipped on import, and user-defined cross-filter scoping is lost. To fix this, when a key cannot be parsed as an int, also look it up as a UUID using the position data (via build_uuid_to_id_map) so both numeric-ID and UUID-keyed configurations are correctly remapped. [logic error]
Severity Level: Major ⚠️
- ⚠️ Example dashboards lose per-chart cross-filter configurations on import.
- ⚠️ Cross-filter scoping from export_example not preserved in metadata.
- ⚠️ Users of example bundles see cross-filtering disabled or incorrect.| for old_id_str, chart_config in metadata["chart_configuration"].items(): | |
| try: | |
| old_id_int = int(old_id_str) | |
| # Some exporters (e.g. export_example) key chart_configuration by chart UUID | |
| # instead of numeric chart id. Build a UUID -> old_id map from the position | |
| # data so we can handle both formats. | |
| old_ids = build_uuid_to_id_map(fixed.get("position", {})) | |
| for old_id_str, chart_config in metadata["chart_configuration"].items(): | |
| old_id_int = None | |
| try: | |
| old_id_int = int(old_id_str) | |
| except (TypeError, ValueError): | |
| # Fallback: treat key as a chart UUID and resolve via position | |
| old_id_int = old_ids.get(old_id_str) | |
| if old_id_int is None: |
Steps of Reproduction ✅
1. Export a dashboard with cross-filters using the example exporter: call
`ExportExampleCommand.run()` in `superset/commands/dashboard/export_example.py:487-672`,
which internally calls `export_dashboard_yaml()` at `export_example.py:391-459`.
2. In `export_dashboard_yaml()`, observe that `chart_configuration` is remapped by
`remap_chart_configuration()` at `export_example.py:347-375`, producing metadata where
`metadata["chart_configuration"]` is keyed by chart UUID strings and each entry's `"id"`
and `"crossFilters"]["chartsInScope"]` also contain chart UUIDs.
3. Import the generated `dashboard.yaml` via the v1 dashboards importer (e.g. through the
dashboards API, which constructs an `ImportDashboardsCommand` at
`superset/dashboards/api.py:1940`); the v1 importer wires `update_id_refs()` into the
import pipeline at `superset/commands/dashboard/importers/v1/__init__.py:174` (`config =
update_id_refs(config, chart_ids, dataset_info)`).
4. Inside `update_id_refs()` (`superset/commands/dashboard/importers/v1/utils.py:60-151`),
the function calls `update_cross_filter_scoping(fixed, id_map)` at `utils.py:150`; in
`update_cross_filter_scoping()` (`utils.py:172-205`), the loop at `utils.py:188-192`
attempts `int(old_id_str)` for each `metadata["chart_configuration"]` key, but since keys
are UUID strings from the example export, `int()` raises `ValueError`, causing the
`except` block at `utils.py:191-192` to `continue` and skip each entry, resulting in
`new_chart_configuration` remaining empty and `metadata["chart_configuration"]` being set
to `{}` at `utils.py:205`, thereby dropping all per-chart cross-filter configuration on
the imported dashboard.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/commands/dashboard/importers/v1/utils.py
**Line:** 188:190
**Comment:**
*Logic Error: The new `update_cross_filter_scoping` implementation assumes that `metadata["chart_configuration"]` is keyed by numeric chart IDs (strings convertible to `int`), but `export_example.remap_chart_configuration` writes example dashboards with `chart_configuration` keyed by chart UUIDs; as a result, per-chart cross-filter configurations from example exports are silently skipped on import, and user-defined cross-filter scoping is lost. To fix this, when a key cannot be parsed as an int, also look it up as a UUID using the position data (via `build_uuid_to_id_map`) so both numeric-ID and UUID-keyed configurations are correctly remapped.
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.
2 participants
SUMMARY
Originally authored by @rdubois-kh in #26389 - This PR adopts and rebases the original contribution.
This PR fixes issue #26338:
chartsInScopereferences are not fixed during a Dashboard import.When a dashboard is imported using the
ImportDashboardsCommand(v1), the chart ID references need to be updated to use the IDs of the newly created charts in the target instance. The original code was missing updates forchartsInScopereferences in several places:chartsInScopeinnative_filter_configurationentrieschartsInScopeinglobal_chart_configurationchartsInScopeinchart_configuration[id].crossFiltersThis resulted in broken filter scoping after dashboard imports, where filters would reference old chart IDs that don't exist in the target instance.
Changes made:
chartsInScoperemapping inupdate_id_refs()for native filterschartsInScoperemapping inupdate_cross_filter_scoping()for global and per-chart configurationsupdate_cross_filter_scoping()to extract helper functions_remap_chart_ids()and_update_cross_filter_scope()to reduce complexityFixes #26338
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A - Backend-only change
TESTING INSTRUCTIONS
chartsInScopein the imported dashboard'sjson_metadatacontains the correct new chart IDsOr run the unit tests:
ADDITIONAL INFORMATION
chartsInScopereferences are not fixed during a Dashboard import #26338Attribution: This PR adopts the work from #26389 by @rdubois-kh. The original PR needed a rebase due to conflicts with recent changes to the cross-filter scoping code.