sadpandajoe commented on code in PR #45038:
URL: https://github.com/apache/superset/pull/45038#discussion_r4210827556
##########
superset/daos/dashboard.py:
##########
@@ -531,6 +535,109 @@ def favorited_ids(dashboards: list[Dashboard]) ->
list[FavStar]:
.all()
]
+ @staticmethod
+ def _remap_filter_scope(
+ container: dict[str, Any] | Any,
+ old_to_new_slice_ids: dict[int, int],
+ ) -> None:
+ """Remap scope.excluded and chartsInScope of a filter container.
+
+ This method updates in-place the chart ID references stored inside
+ a filter configuration container. Both native filters and cross-filter
+ scopes store denormalized lists of chart IDs in chartsInScope and
+ scope.excluded. When duplicate_slices is requested during dashboard
+ copy, these identifiers must point to the newly cloned slice IDs.
+
+ Non-dictionary elements, visual dividers (type DIVIDER or IDs starting
+ with NATIVE_FILTER_DIVIDER), and non-list attributes are skipped
safely.
+
+ :param container: Dictionary holding filter scope or cross-filter
+ configuration.
+ :param old_to_new_slice_ids: Mapping from original chart ID to
+ duplicated chart ID.
+ """
+ if not isinstance(container, dict):
+ return
+
+ # Skip divider entities which represent visual section dividers in
filter bar
+ if container.get("type") == "DIVIDER" or str(
+ container.get("id", "")
+ ).startswith(("NATIVE_FILTER_DIVIDER", "DIVIDER")):
+ return
+
+ scope = container.get("scope")
+ if isinstance(scope, dict) and isinstance(scope.get("excluded"), list):
+ remapped_excluded: list[int] = []
+ for cid in scope["excluded"]:
+ try:
+ int_id = int(cid)
+ remapped_excluded.append(old_to_new_slice_ids.get(int_id,
int_id))
+ except (ValueError, TypeError):
+ remapped_excluded.append(cid)
+ scope["excluded"] = remapped_excluded
Review Comment:
Agreed—a filter scoped to a single layer keeps `selectedLayers` entries like
`chart-<old_id>-layer-0`, so on the copied dashboard the layer scope is looked
up under the original chart ID and the cloned chart gets no layer-specific
scope, so the filter can hit every layer of the copy. Should the chart-ID part
of each `selectedLayers` entry be remapped (keeping the layer index) alongside
`excluded` and `chartsInScope`?
##########
superset/daos/dashboard.py:
##########
@@ -531,6 +535,109 @@ def favorited_ids(dashboards: list[Dashboard]) ->
list[FavStar]:
.all()
]
+ @staticmethod
+ def _remap_filter_scope(
+ container: dict[str, Any] | Any,
+ old_to_new_slice_ids: dict[int, int],
+ ) -> None:
+ """Remap scope.excluded and chartsInScope of a filter container.
+
+ This method updates in-place the chart ID references stored inside
+ a filter configuration container. Both native filters and cross-filter
+ scopes store denormalized lists of chart IDs in chartsInScope and
+ scope.excluded. When duplicate_slices is requested during dashboard
+ copy, these identifiers must point to the newly cloned slice IDs.
+
+ Non-dictionary elements, visual dividers (type DIVIDER or IDs starting
+ with NATIVE_FILTER_DIVIDER), and non-list attributes are skipped
safely.
+
+ :param container: Dictionary holding filter scope or cross-filter
+ configuration.
+ :param old_to_new_slice_ids: Mapping from original chart ID to
+ duplicated chart ID.
+ """
+ if not isinstance(container, dict):
+ return
+
+ # Skip divider entities which represent visual section dividers in
filter bar
+ if container.get("type") == "DIVIDER" or str(
+ container.get("id", "")
+ ).startswith(("NATIVE_FILTER_DIVIDER", "DIVIDER")):
+ return
+
+ scope = container.get("scope")
+ if isinstance(scope, dict) and isinstance(scope.get("excluded"), list):
+ remapped_excluded: list[int] = []
+ for cid in scope["excluded"]:
+ try:
+ int_id = int(cid)
+ remapped_excluded.append(old_to_new_slice_ids.get(int_id,
int_id))
+ except (ValueError, TypeError):
+ remapped_excluded.append(cid)
+ scope["excluded"] = remapped_excluded
+
+ if isinstance(container.get("chartsInScope"), list):
+ remapped_in_scope: list[int] = []
+ for cid in container["chartsInScope"]:
+ try:
+ int_id = int(cid)
+ remapped_in_scope.append(old_to_new_slice_ids.get(int_id,
int_id))
+ except (ValueError, TypeError):
+ remapped_in_scope.append(cid)
+ container["chartsInScope"] = remapped_in_scope
+
+ @classmethod
+ def _remap_filter_scopes(
+ cls,
+ metadata: dict[str, Any],
+ old_to_new_slice_ids: dict[int, int],
+ ) -> None:
+ """Remap filter scopes and cross-filter references in dashboard
metadata.
+
+ Mutates metadata in-place to redirect slice ID references across:
+ 1. native_filter_configuration: list of native filter definitions.
+ 2. global_chart_configuration: dashboard-wide cross-filter scoping.
+ 3. chart_configuration: per-chart cross-filter scopes, keys, and chart
IDs.
+
+ This ensures that after duplicating dashboard charts, all filter
+ scopes remain bound to the new chart copies instead of the originals.
+
+ :param metadata: Deserialized dashboard json_metadata dictionary.
+ :param old_to_new_slice_ids: Mapping from original chart ID to
+ duplicated chart ID.
+ """
+ if not isinstance(metadata, dict) or not old_to_new_slice_ids:
+ return
+
+ if isinstance(metadata.get("native_filter_configuration"), list):
+ for native_filter in metadata["native_filter_configuration"]:
+ cls._remap_filter_scope(native_filter, old_to_new_slice_ids)
Review Comment:
Agreed—`chart_customization_config` uses the same
`scope.excluded`/`chartsInScope` shape and is skipped by
`_remap_filter_scopes`. A Dynamic Group By control that excludes chart B still
lists B's original ID after Save As, so scope recomputation includes the copied
B and the control changes it unexpectedly. Should the customization entries go
through `_remap_filter_scope` as well?
--
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]
