codeant-ai-for-open-source[bot] commented on code in PR #45038:
URL: https://github.com/apache/superset/pull/45038#discussion_r4201123889
##########
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:
**Suggestion:** `scope.selectedLayers` encodes chart IDs in its keys, but
this only remaps `excluded`. After duplication, layer-scoped filters still
target the original chart IDs.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Incomplete
implementation`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=607105ac5dca4282a46d9f5c8a089906&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=607105ac5dca4282a46d9f5c8a089906&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/daos/dashboard.py
**Line:** 568:577
**Comment:**
*Incomplete Implementation: `scope.selectedLayers` encodes chart IDs in
its keys, but this only remaps `excluded`. After duplication, layer-scoped
filters still target the original chart IDs.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45038&comment_hash=5274f8bf12c4aa1c6f4074071f42b996dc973fba81284fe2133679df61eb8fa4&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45038&comment_hash=5274f8bf12c4aa1c6f4074071f42b996dc973fba81284fe2133679df61eb8fa4&reaction=dislike'>๐</a>
##########
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:
**Suggestion:** `chart_customization_config` also stores scope references,
but this remaps only native filters. Customization exclusions and cached
targets can therefore still point to original charts after duplication.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Incomplete
implementation`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b41675985bdb4043ba7950da44d46d70&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b41675985bdb4043ba7950da44d46d70&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/daos/dashboard.py
**Line:** 612:614
**Comment:**
*Incomplete Implementation: `chart_customization_config` also stores
scope references, but this remaps only native filters. Customization exclusions
and cached targets can therefore still point to original charts after
duplication.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45038&comment_hash=4e0cb4ffa3fbdce66ec11fca7abcc337da1cc661e89385a201caaded88020359&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45038&comment_hash=4e0cb4ffa3fbdce66ec11fca7abcc337da1cc661e89385a201caaded88020359&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]
