Skip to content

fix(26338): replace chartsInScope references at import time #26389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
rdubois-kh wants to merge 3 commits into
base: master
Choose a base branch
from
+141 −9
Draft

fix(26338): replace chartsInScope references at import time #26389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c6732c4
fix(26338): replace chartsInScope references at import time
rdubois-kh Jan 2, 2024
8075dfd
Merge branch 'master' of github.com:apache/superset into rdubois/fix/…
rdubois-kh Apr 25, 2024
1fab897
make the charts references replacement more exhaustive and complete
rdubois-kh Apr 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 40 additions & 5 deletions superset/commands/dashboard/importers/v1/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,24 @@ def build_uuid_to_id_map(position: dict[str, Any]) -> dict[str, int]:
}


def __fix_charts_references_in_scope(
configuration: dict[str, Any], charts_map: dict[int, int]
) -> dict[str, Any]:
Comment on lines +60 to +62
Copy link

@korbit-ai korbit-ai bot Mar 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect use of double underscore prefix category Readability

Tell me more
What is the issue?

Function name uses double underscores which in Python implies name mangling, but this is not a class method where that would be appropriate.

Why this matters

Double underscores can make the function harder to import/reference and suggest private class method behavior when this is just a module-level utility function.

Suggested change ∙ Feature Preview
def _fix_charts_references_in_scope(
    configuration: dict[str, Any], charts_map: dict[int, int]
) -> dict[str, Any]:
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

configuration_scope = configuration.get("scope", {})
if isinstance(configuration_scope, dict): # means is not "global"
if scope_excluded := configuration_scope.get("excluded", []):
configuration_scope["excluded"] = [
charts_map[old_id] for old_id in scope_excluded if old_id in charts_map
]

if charts_in_scope := configuration.get("chartsInScope", []):
configuration["chartsInScope"] = [
charts_map[old_id] for old_id in charts_in_scope if old_id in charts_map
]

return configuration


def update_id_refs( # pylint: disable=too-many-locals
config: dict[str, Any],
chart_ids: dict[str, int],
Expand Down Expand Up @@ -134,11 +152,28 @@ def update_id_refs( # pylint: disable=too-many-locals
if dataset_uuid:
target["datasetId"] = dataset_info[dataset_uuid]["datasource_id"]

scope_excluded = native_filter.get("scope", {}).get("excluded", [])
if scope_excluded:
native_filter["scope"]["excluded"] = [
id_map[old_id] for old_id in scope_excluded if old_id in id_map
]
__fix_charts_references_in_scope(native_filter, id_map)

# fix global chart configuration
global_chart_configuration = fixed.get("metadata", {}).get(
"global_chart_configuration", {}
)
__fix_charts_references_in_scope(global_chart_configuration, id_map)

# fix chart configuration
chart_configuration = fixed.get("metadata", {}).get("chart_configuration", {})
for (
chart_configuration_key,
chart_configuration_value,
) in chart_configuration.copy().items():
if new_chart_configuration_key := id_map.get(int(chart_configuration_key)):
chart_configuration_value["id"] = new_chart_configuration_key
__fix_charts_references_in_scope(
chart_configuration_value["crossFilters"], id_map
)
chart_configuration[str(new_chart_configuration_key)] = (
chart_configuration.pop(chart_configuration_key)
)

return fixed

Expand Down
105 changes: 101 additions & 4 deletions tests/unit_tests/dashboards/commands/importers/v1/utils_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def test_update_id_refs_immune_missing( # pylint: disable=invalid-name
}


def test_update_native_filter_config_scope_excluded():
def test_update_charts_reference():
from superset.commands.dashboard.importers.v1.utils import update_id_refs

config = {
Expand All @@ -97,12 +97,61 @@ def test_update_native_filter_config_scope_excluded():
"meta": {"chartId": 102, "uuid": "uuid2"},
"type": "CHART",
},
"CHART3": {
"id": "CHART3",
"meta": {"chartId": 103, "uuid": "uuid3"},
"type": "CHART",
},
"CHART4": {
"id": "CHART4",
"meta": {"chartId": 104, "uuid": "uuid4"},
"type": "CHART",
},
},
"metadata": {
"native_filter_configuration": [{"scope": {"excluded": [101, 102, 103]}}],
"chart_configuration": {
"101": {
"id": 101,
"crossFilters": {
"chartsInScope": [102, 103],
"scope": {"excluded": [101, 110]},
},
},
"102": {
"id": 102,
"crossFilters": {
"chartsInScope": [],
"scope": {"excluded": [101, 102, 103, 110]},
},
},
"103": {
"id": 103,
"crossFilters": {
"chartsInScope": [110],
"scope": {"excluded": [101, 102, 103]},
},
},
"104": {
"id": 104,
"crossFilters": {
"chartsInScope": [110],
"scope": "global",
},
},
},
"global_chart_configuration": {
"chartsInScope": [101, 103, 110],
"scope": {"excluded": [102, 110]},
},
"native_filter_configuration": [
{
"chartsInScope": [101, 102, 103, 110],
"scope": {"excluded": [101, 102, 103, 110]},
}
],
},
}
chart_ids = {"uuid1": 1, "uuid2": 2}
chart_ids = {"uuid1": 1, "uuid2": 2, "uuid3": 3, "uuid4": 4}
dataset_info: dict[str, dict[str, Any]] = {} # not used

fixed = update_id_refs(config, chart_ids, dataset_info)
Expand All @@ -118,6 +167,54 @@ def test_update_native_filter_config_scope_excluded():
"meta": {"chartId": 2, "uuid": "uuid2"},
"type": "CHART",
},
"CHART3": {
"id": "CHART3",
"meta": {"chartId": 3, "uuid": "uuid3"},
"type": "CHART",
},
"CHART4": {
"id": "CHART4",
"meta": {"chartId": 4, "uuid": "uuid4"},
"type": "CHART",
},
},
"metadata": {
"chart_configuration": {
"1": {
"id": 1,
"crossFilters": {
"chartsInScope": [2, 3],
"scope": {"excluded": [1]},
},
},
"2": {
"id": 2,
"crossFilters": {
"chartsInScope": [],
"scope": {"excluded": [1, 2, 3]},
},
},
"3": {
"id": 3,
"crossFilters": {
"chartsInScope": [],
"scope": {"excluded": [1, 2, 3]},
},
},
"4": {
"id": 4,
"crossFilters": {
"chartsInScope": [],
"scope": "global",
},
},
},
"global_chart_configuration": {
"chartsInScope": [1, 3],
"scope": {"excluded": [2]},
},
"native_filter_configuration": [
{"chartsInScope": [1, 2, 3], "scope": {"excluded": [1, 2, 3]}}
],
},
"metadata": {"native_filter_configuration": [{"scope": {"excluded": [1, 2]}}]},
}
Loading