rusackas commented on code in PR #40140:
URL: https://github.com/apache/superset/pull/40140#discussion_r3245638563


##########
tests/unit_tests/dashboards/commands/importers/v1/utils_test.py:
##########
@@ -123,6 +123,99 @@ def test_update_native_filter_config_scope_excluded():
     }
 
 
+def test_update_id_refs_remaps_charts_in_scope():
+    """
+    Regression for #26338: ``chartsInScope`` on a native filter holds chart
+    IDs and must be remapped from source-env IDs to destination-env IDs
+    during import.
+
+    The export side already converts ``chartsInScope`` IDs to UUIDs (see
+    ``export_example.py:325``). The import side must symmetrically convert
+    them back to the destination environment's chart IDs. Without that
+    remap, the field carries stale source IDs into the imported dashboard
+    and breaks ``filtersInScope`` / ``filtersOutScope`` computation —
+    filters end up applied to the wrong charts (or none at all).
+    """
+    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",
+            },
+        },
+        "metadata": {
+            "native_filter_configuration": [
+                {
+                    "id": "NATIVE_FILTER-region",
+                    "scope": {"rootPath": ["ROOT_ID"], "excluded": []},
+                    # chartsInScope contains source-env chart IDs.
+                    "chartsInScope": [101, 102, 103],

Review Comment:
   Thanks — that's the right learning.



##########
tests/unit_tests/dashboards/commands/importers/v1/utils_test.py:
##########
@@ -123,6 +123,99 @@ def test_update_native_filter_config_scope_excluded():
     }
 
 
+def test_update_id_refs_remaps_charts_in_scope():
+    """
+    Regression for #26338: ``chartsInScope`` on a native filter holds chart
+    IDs and must be remapped from source-env IDs to destination-env IDs
+    during import.
+
+    The export side already converts ``chartsInScope`` IDs to UUIDs (see
+    ``export_example.py:325``). The import side must symmetrically convert
+    them back to the destination environment's chart IDs. Without that
+    remap, the field carries stale source IDs into the imported dashboard
+    and breaks ``filtersInScope`` / ``filtersOutScope`` computation —
+    filters end up applied to the wrong charts (or none at all).
+    """
+    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",
+            },
+        },
+        "metadata": {
+            "native_filter_configuration": [
+                {
+                    "id": "NATIVE_FILTER-region",
+                    "scope": {"rootPath": ["ROOT_ID"], "excluded": []},
+                    # chartsInScope contains source-env chart IDs.
+                    "chartsInScope": [101, 102, 103],
+                }
+            ],
+        },
+    }
+    chart_ids = {"uuid1": 1, "uuid2": 2}
+    dataset_info: dict[str, dict[str, Any]] = {}
+
+    fixed = update_id_refs(config, chart_ids, dataset_info)
+    filter_config = fixed["metadata"]["native_filter_configuration"][0]
+
+    # Resolved IDs are remapped; unknown IDs (103) are dropped rather than
+    # left to silently bind to whatever chart owns that integer in the
+    # destination environment.
+    assert filter_config["chartsInScope"] == [1, 2]
+
+
+def test_update_id_refs_remaps_cross_filter_charts_in_scope():
+    """
+    Companion to test_update_id_refs_remaps_charts_in_scope. Cross-filter
+    config also stores ``chartsInScope`` (under ``crossFilters`` per chart)
+    and must be remapped on import for the same reason.
+    """
+    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",
+            },
+        },
+        "metadata": {
+            "chart_configuration": {
+                "101": {
+                    "id": 101,
+                    "crossFilters": {
+                        "scope": {"rootPath": ["ROOT_ID"], "excluded": []},
+                        "chartsInScope": [101, 102, 103],
+                    },
+                },
+            },

Review Comment:
   Thanks — same convention as the sibling thread.



-- 
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]

Reply via email to