Skip to content

fix(dashboard): replace chartsInScope references at import time #38171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rusackas wants to merge 1 commit into
base: master
Choose a base branch
from
+174 −39
Open

fix(dashboard): replace chartsInScope references at import time #38171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 54 additions & 39 deletions superset/commands/dashboard/importers/v1/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,55 +139,70 @@ def update_id_refs( # pylint: disable=too-many-locals # noqa: C901
native_filter["scope"]["excluded"] = [
id_map[old_id] for old_id in scope_excluded if old_id in id_map
]

# fix chartsInScope references (from PR #26389)
charts_in_scope = native_filter.get("chartsInScope", [])
if charts_in_scope:
native_filter["chartsInScope"] = [
id_map[old_id] for old_id in charts_in_scope if old_id in id_map
]

fixed = update_cross_filter_scoping(fixed, id_map)
return fixed


def _remap_chart_ids(id_list: list[int], id_map: dict[int, int]) -> list[int]:
"""Remap old chart IDs to new IDs, dropping any that aren't in the map."""
return [id_map[old_id] for old_id in id_list if old_id in id_map]


def _update_cross_filter_scope(
cross_filter_config: dict[str, Any], id_map: dict[int, int]
) -> None:
"""Update scope.excluded and chartsInScope in a cross-filter configuration."""
scope = cross_filter_config.get("scope", {})
if isinstance(scope, dict):
if excluded := scope.get("excluded", []):
scope["excluded"] = _remap_chart_ids(excluded, id_map)

if charts_in_scope := cross_filter_config.get("chartsInScope", []):
cross_filter_config["chartsInScope"] = _remap_chart_ids(charts_in_scope, id_map)


def update_cross_filter_scoping(
config: dict[str, Any], id_map: dict[int, int]
) -> dict[str, Any]:
# fix cross filter references
"""Fix cross filter references by remapping chart IDs."""
fixed = config.copy()
metadata = fixed.get("metadata", {})

cross_filter_global_config = fixed.get("metadata", {}).get(
"global_chart_configuration", {}
)
scope_excluded = cross_filter_global_config.get("scope", {}).get("excluded", [])
if scope_excluded:
cross_filter_global_config["scope"]["excluded"] = [
id_map[old_id] for old_id in scope_excluded if old_id in id_map
]
# Update global_chart_configuration
global_config = metadata.get("global_chart_configuration", {})
_update_cross_filter_scope(global_config, id_map)

if "chart_configuration" in (metadata := fixed.get("metadata", {})):
# Build remapped configuration in a single pass for clarity/readability.
new_chart_configuration: dict[str, Any] = {}
for old_id_str, chart_config in metadata["chart_configuration"].items():
try:
old_id_int = int(old_id_str)
except (TypeError, ValueError):
continue

new_id = id_map.get(old_id_int)
if new_id is None:
continue

if isinstance(chart_config, dict):
chart_config["id"] = new_id

# Update cross filter scope excluded ids
scope = chart_config.get("crossFilters", {}).get("scope", {})
if isinstance(scope, dict):
excluded_scope = scope.get("excluded", [])
if excluded_scope:
chart_config["crossFilters"]["scope"]["excluded"] = [
id_map[old_id]
for old_id in excluded_scope
if old_id in id_map
]

new_chart_configuration[str(new_id)] = chart_config

metadata["chart_configuration"] = new_chart_configuration
# Update chart_configuration entries
if "chart_configuration" not in metadata:
return fixed

new_chart_configuration: dict[str, Any] = {}
for old_id_str, chart_config in metadata["chart_configuration"].items():
try:
old_id_int = int(old_id_str)
Comment on lines +188 to +190
Copy link
Contributor

@codeant-ai-for-open-source codeant-ai-for-open-source bot Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Suggested change
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.
👍 | 👎

except (TypeError, ValueError):
continue

new_id = id_map.get(old_id_int)
if new_id is None:
continue

if isinstance(chart_config, dict):
chart_config["id"] = new_id
if cross_filters := chart_config.get("crossFilters", {}):
_update_cross_filter_scope(cross_filters, id_map)

new_chart_configuration[str(new_id)] = chart_config

metadata["chart_configuration"] = new_chart_configuration
return fixed


Expand Down
120 changes: 120 additions & 0 deletions tests/unit_tests/dashboards/commands/importers/v1/utils_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,53 @@ def test_update_native_filter_config_scope_excluded():
}


def test_update_native_filter_charts_in_scope():
"""
Test that chartsInScope references in native filters are updated during import.

This is a fix for issue #26338 - chartsInScope references were not being
updated to use new chart IDs during dashboard import.
"""
from superset.commands.dashboard.importers.v1.utils import update_id_refs

config = {
"position": {
"CHART1": {
"id": "CHART1",
"meta": {"chartId": 101, "uuid": "uuid1"},
"type": "CHART",
},
"CHART2": {
"id": "CHART2",
"meta": {"chartId": 102, "uuid": "uuid2"},
"type": "CHART",
},
},
"metadata": {
"native_filter_configuration": [{"chartsInScope": [101, 102, 103]}],
},
}
chart_ids = {"uuid1": 1, "uuid2": 2}
dataset_info: dict[str, dict[str, Any]] = {} # not used

fixed = update_id_refs(config, chart_ids, dataset_info)
assert fixed == {
"position": {
"CHART1": {
"id": "CHART1",
"meta": {"chartId": 1, "uuid": "uuid1"},
"type": "CHART",
},
"CHART2": {
"id": "CHART2",
"meta": {"chartId": 2, "uuid": "uuid2"},
"type": "CHART",
},
},
"metadata": {"native_filter_configuration": [{"chartsInScope": [1, 2]}]},
}


def test_update_id_refs_cross_filter_chart_configuration_key_and_excluded_mapping():
from superset.commands.dashboard.importers.v1.utils import update_id_refs

Expand Down Expand Up @@ -199,3 +246,76 @@ def test_update_id_refs_cross_filter_handles_string_excluded():
fixed = update_id_refs(config, chart_ids, dataset_info)
# Should not raise and should remap key
assert "1" in fixed["metadata"]["chart_configuration"]


def test_update_id_refs_cross_filter_charts_in_scope():
"""
Test that chartsInScope references in cross-filter configurations are updated.

This is a fix for issue #26338 - chartsInScope references in chart_configuration
and global_chart_configuration were not being updated during dashboard import.
"""
from superset.commands.dashboard.importers.v1.utils import update_id_refs

config: dict[str, Any] = {
"position": {
"CHART1": {
"id": "CHART1",
"meta": {"chartId": 101, "uuid": "uuid1"},
"type": "CHART",
},
"CHART2": {
"id": "CHART2",
"meta": {"chartId": 102, "uuid": "uuid2"},
"type": "CHART",
},
"CHART3": {
"id": "CHART3",
"meta": {"chartId": 103, "uuid": "uuid3"},
"type": "CHART",
},
},
"metadata": {
"chart_configuration": {
"101": {
"id": 101,
"crossFilters": {
"chartsInScope": [102, 103],
"scope": {"excluded": [101]},
},
},
"102": {
"id": 102,
"crossFilters": {
"chartsInScope": [101, 103, 999], # 999 should be dropped
"scope": {"excluded": []},
},
},
},
"global_chart_configuration": {
"chartsInScope": [101, 102, 103, 999], # 999 should be dropped
"scope": {"excluded": [103]},
},
},
}

chart_ids = {"uuid1": 1, "uuid2": 2, "uuid3": 3}
dataset_info: dict[str, dict[str, Any]] = {}

fixed = update_id_refs(config, chart_ids, dataset_info)

metadata = fixed["metadata"]

# Check chart_configuration chartsInScope is updated
assert metadata["chart_configuration"]["1"]["crossFilters"]["chartsInScope"] == [
2,
3,
]
assert metadata["chart_configuration"]["2"]["crossFilters"]["chartsInScope"] == [
1,
3,
]

# Check global_chart_configuration chartsInScope is updated
assert metadata["global_chart_configuration"]["chartsInScope"] == [1, 2, 3]
assert metadata["global_chart_configuration"]["scope"]["excluded"] == [3]
Loading