This is an automated email from the ASF dual-hosted git repository. rusackas pushed a commit to branch adopt-pr-26389-fix-charts-in-scope in repository https://gitbox.apache.org/repos/asf/superset.git
commit 4dc4acb7de73c92dc900214d2812f9e1ab536406 Author: Rémy Dubois <[email protected]> AuthorDate: Tue Jan 2 09:22:11 2024 +0100 fix(26338): replace chartsInScope references at import time --- superset/commands/dashboard/importers/v1/utils.py | 93 +++++++++------- .../dashboards/commands/importers/v1/utils_test.py | 120 +++++++++++++++++++++ 2 files changed, 174 insertions(+), 39 deletions(-) diff --git a/superset/commands/dashboard/importers/v1/utils.py b/superset/commands/dashboard/importers/v1/utils.py index c506db72cb6..27df911db00 100644 --- a/superset/commands/dashboard/importers/v1/utils.py +++ b/superset/commands/dashboard/importers/v1/utils.py @@ -139,55 +139,70 @@ def update_id_refs( # pylint: disable=too-many-locals # noqa: C901 native_filter["scope"]["excluded"] = [ id_map[old_id] for old_id in scope_excluded if old_id in id_map ] + + # fix chartsInScope references (from PR #26389) + charts_in_scope = native_filter.get("chartsInScope", []) + if charts_in_scope: + native_filter["chartsInScope"] = [ + id_map[old_id] for old_id in charts_in_scope if old_id in id_map + ] + fixed = update_cross_filter_scoping(fixed, id_map) return fixed +def _remap_chart_ids(id_list: list[int], id_map: dict[int, int]) -> list[int]: + """Remap old chart IDs to new IDs, dropping any that aren't in the map.""" + return [id_map[old_id] for old_id in id_list if old_id in id_map] + + +def _update_cross_filter_scope( + cross_filter_config: dict[str, Any], id_map: dict[int, int] +) -> None: + """Update scope.excluded and chartsInScope in a cross-filter configuration.""" + scope = cross_filter_config.get("scope", {}) + if isinstance(scope, dict): + if excluded := scope.get("excluded", []): + scope["excluded"] = _remap_chart_ids(excluded, id_map) + + if charts_in_scope := cross_filter_config.get("chartsInScope", []): + cross_filter_config["chartsInScope"] = _remap_chart_ids(charts_in_scope, id_map) + + def update_cross_filter_scoping( config: dict[str, Any], id_map: dict[int, int] ) -> dict[str, Any]: - # fix cross filter references + """Fix cross filter references by remapping chart IDs.""" fixed = config.copy() + metadata = fixed.get("metadata", {}) - cross_filter_global_config = fixed.get("metadata", {}).get( - "global_chart_configuration", {} - ) - scope_excluded = cross_filter_global_config.get("scope", {}).get("excluded", []) - if scope_excluded: - cross_filter_global_config["scope"]["excluded"] = [ - id_map[old_id] for old_id in scope_excluded if old_id in id_map - ] + # Update global_chart_configuration + global_config = metadata.get("global_chart_configuration", {}) + _update_cross_filter_scope(global_config, id_map) - if "chart_configuration" in (metadata := fixed.get("metadata", {})): - # 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 - - new_id = id_map.get(old_id_int) - if new_id is None: - continue - - if isinstance(chart_config, dict): - chart_config["id"] = new_id - - # Update cross filter scope excluded ids - scope = chart_config.get("crossFilters", {}).get("scope", {}) - if isinstance(scope, dict): - 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 - ] - - new_chart_configuration[str(new_id)] = chart_config - - metadata["chart_configuration"] = new_chart_configuration + # Update chart_configuration entries + if "chart_configuration" not in metadata: + return fixed + + 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 + + new_id = id_map.get(old_id_int) + if new_id is None: + continue + + if isinstance(chart_config, dict): + chart_config["id"] = new_id + if cross_filters := chart_config.get("crossFilters", {}): + _update_cross_filter_scope(cross_filters, id_map) + + new_chart_configuration[str(new_id)] = chart_config + + metadata["chart_configuration"] = new_chart_configuration return fixed diff --git a/tests/unit_tests/dashboards/commands/importers/v1/utils_test.py b/tests/unit_tests/dashboards/commands/importers/v1/utils_test.py index 72bf490a5b0..d5a6498cf38 100644 --- a/tests/unit_tests/dashboards/commands/importers/v1/utils_test.py +++ b/tests/unit_tests/dashboards/commands/importers/v1/utils_test.py @@ -123,6 +123,53 @@ def test_update_native_filter_config_scope_excluded(): } +def test_update_native_filter_charts_in_scope(): + """ + Test that chartsInScope references in native filters are updated during import. + + This is a fix for issue #26338 - chartsInScope references were not being + updated to use new chart IDs during dashboard import. + """ + from superset.commands.dashboard.importers.v1.utils import update_id_refs + + config = { + "position": { + "CHART1": { + "id": "CHART1", + "meta": {"chartId": 101, "uuid": "uuid1"}, + "type": "CHART", + }, + "CHART2": { + "id": "CHART2", + "meta": {"chartId": 102, "uuid": "uuid2"}, + "type": "CHART", + }, + }, + "metadata": { + "native_filter_configuration": [{"chartsInScope": [101, 102, 103]}], + }, + } + chart_ids = {"uuid1": 1, "uuid2": 2} + dataset_info: dict[str, dict[str, Any]] = {} # not used + + fixed = update_id_refs(config, chart_ids, dataset_info) + assert fixed == { + "position": { + "CHART1": { + "id": "CHART1", + "meta": {"chartId": 1, "uuid": "uuid1"}, + "type": "CHART", + }, + "CHART2": { + "id": "CHART2", + "meta": {"chartId": 2, "uuid": "uuid2"}, + "type": "CHART", + }, + }, + "metadata": {"native_filter_configuration": [{"chartsInScope": [1, 2]}]}, + } + + def test_update_id_refs_cross_filter_chart_configuration_key_and_excluded_mapping(): from superset.commands.dashboard.importers.v1.utils import update_id_refs @@ -199,3 +246,76 @@ def test_update_id_refs_cross_filter_handles_string_excluded(): fixed = update_id_refs(config, chart_ids, dataset_info) # Should not raise and should remap key assert "1" in fixed["metadata"]["chart_configuration"] + + +def test_update_id_refs_cross_filter_charts_in_scope(): + """ + Test that chartsInScope references in cross-filter configurations are updated. + + This is a fix for issue #26338 - chartsInScope references in chart_configuration + and global_chart_configuration were not being updated during dashboard import. + """ + from superset.commands.dashboard.importers.v1.utils import update_id_refs + + config: dict[str, Any] = { + "position": { + "CHART1": { + "id": "CHART1", + "meta": {"chartId": 101, "uuid": "uuid1"}, + "type": "CHART", + }, + "CHART2": { + "id": "CHART2", + "meta": {"chartId": 102, "uuid": "uuid2"}, + "type": "CHART", + }, + "CHART3": { + "id": "CHART3", + "meta": {"chartId": 103, "uuid": "uuid3"}, + "type": "CHART", + }, + }, + "metadata": { + "chart_configuration": { + "101": { + "id": 101, + "crossFilters": { + "chartsInScope": [102, 103], + "scope": {"excluded": [101]}, + }, + }, + "102": { + "id": 102, + "crossFilters": { + "chartsInScope": [101, 103, 999], # 999 should be dropped + "scope": {"excluded": []}, + }, + }, + }, + "global_chart_configuration": { + "chartsInScope": [101, 102, 103, 999], # 999 should be dropped + "scope": {"excluded": [103]}, + }, + }, + } + + chart_ids = {"uuid1": 1, "uuid2": 2, "uuid3": 3} + dataset_info: dict[str, dict[str, Any]] = {} + + fixed = update_id_refs(config, chart_ids, dataset_info) + + metadata = fixed["metadata"] + + # Check chart_configuration chartsInScope is updated + assert metadata["chart_configuration"]["1"]["crossFilters"]["chartsInScope"] == [ + 2, + 3, + ] + assert metadata["chart_configuration"]["2"]["crossFilters"]["chartsInScope"] == [ + 1, + 3, + ] + + # Check global_chart_configuration chartsInScope is updated + assert metadata["global_chart_configuration"]["chartsInScope"] == [1, 2, 3] + assert metadata["global_chart_configuration"]["scope"]["excluded"] == [3]
