Vitor-Avila commented on code in PR #34418:
URL: https://github.com/apache/superset/pull/34418#discussion_r2280218463
##########
superset/commands/dashboard/importers/v1/utils.py:
##########
@@ -159,27 +159,38 @@ def update_cross_filter_scoping(
]
if "chart_configuration" in (metadata := fixed.get("metadata", {})):
- # in cross_filter_scopes the key is the chart ID as a string; we need
to update
- # them to be the new ID as a string:
- metadata["chart_configuration"] = {
- str(id_map[int(old_id)]): columns
- for old_id, columns in metadata["chart_configuration"].items()
- if int(old_id) in id_map
- }
- # now update scope excluded to use new IDs:
- for chart_config in metadata["chart_configuration"].values():
- if "id" in chart_config and chart_config["id"] in id_map:
- chart_config["id"] = id_map[chart_config["id"]]
- scope = chart_config.get("crossFilters", {}).get("scope", {})
+ # 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
- if not isinstance(scope, dict):
+ new_id = id_map.get(old_id_int)
+ if new_id is None:
continue
- 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
- ]
+ # Update inner id if present
+ if isinstance(chart_config, dict):
+ inner_id = chart_config.get("id")
+ if isinstance(inner_id, int) and inner_id in id_map:
+ chart_config["id"] = id_map[inner_id]
Review Comment:
Thinking out loud here, if we already found the "top/key-level ID", can't we
just do `chart_config["id"] = new_id` directly (without checking if
`chart_config` has an `id` property and that its value is in the map)?
Not sure if all `chart_configuration` values would have an ID, but I imagine
if we have a top-level and we end up adding an inner level that wouldn't break
anything right? Would just help reducing the amount of checks needed.
--
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: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
