bito-code-review[bot] commented on code in PR #40140:
URL: https://github.com/apache/superset/pull/40140#discussion_r3248156360
##########
superset/commands/dashboard/importers/v1/utils.py:
##########
@@ -145,6 +145,18 @@ def update_id_refs( # pylint: disable=too-many-locals #
noqa: C901
id_map[old_id] for old_id in scope_excluded if old_id in id_map
]
+ # chartsInScope is a denormalized cache of the charts the filter
+ # currently applies to. It holds source-env chart IDs and must be
+ # remapped to destination IDs; otherwise the imported dashboard's
+ # filtersInScope / filtersOutScope computation operates on stale
+ # IDs and filters end up applied to the wrong charts (or none at
+ # all). Drop unresolvable IDs rather than passing them through.
+ charts_in_scope = native_filter.get("chartsInScope")
+ if isinstance(charts_in_scope, list):
+ native_filter["chartsInScope"] = [
+ id_map[old_id] for old_id in charts_in_scope if old_id in
id_map
+ ]
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Semantic Duplication in chartsInScope Handling</b></div>
<div id="fix">
The logic for remapping 'chartsInScope' IDs is duplicated between the native
filter handling (lines 154-158) and the cross filter handling (lines 231-237),
with identical code: checking if the value is a list and then
filtering/remapping IDs. This creates a maintenance risk where changes to one
copy might not be applied to the other, potentially leading to inconsistent
behavior. Extracting this into a shared helper function would improve code
maintainability and reduce divergence risk.
</div>
</div>
<small><i>Code Review Run #5da3f2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
