codeant-ai-for-open-source[bot] commented on code in PR #38171:
URL: https://github.com/apache/superset/pull/38171#discussion_r2838582911
##########
superset/commands/dashboard/importers/v1/utils.py:
##########
@@ -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)
Review 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]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ 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.
```
</details>
```suggestion
# 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:
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
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.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38171&comment_hash=c1472a6e5b564fc51c6b9fea7d45ca0103a74d0b24e453221e5ffc1c7b33808e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38171&comment_hash=c1472a6e5b564fc51c6b9fea7d45ca0103a74d0b24e453221e5ffc1c7b33808e&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
