fix(dashboard): import with overwrite flag replaces charts instead of merging

superset/commands/dashboard/importers/v1/__init__.py

@@ -22,7 +22,7 @@
 
 from marshmallow import Schema
 from sqlalchemy.orm import Session  # noqa: F401
-from sqlalchemy.sql import select
+from sqlalchemy.sql import delete, select
 
 from superset import db
 from superset.charts.schemas import ImportV1ChartSchema
@@ -157,9 +157,14 @@ def _import(
                         )
 
         # store the existing relationship between dashboards and charts
-        existing_relationships = db.session.execute(
-            select([dashboard_slices.c.dashboard_id, dashboard_slices.c.slice_id])
-        ).fetchall()
+        # (only used when overwrite=False to avoid inserting duplicates)
+        existing_relationships: set[tuple[int, int]] = set()
+        if not overwrite:
+            existing_relationships = set(
+                db.session.execute(
+                    select(dashboard_slices.c.dashboard_id, dashboard_slices.c.slice_id)
+                ).fetchall()
+            )
 
         # import dashboards
         dashboards: list[Dashboard] = []
@@ -177,11 +182,25 @@ def _import(
                     del config["theme_uuid"]
                 dashboard = import_dashboard(config, overwrite=overwrite)
                 dashboards.append(dashboard)
+
+                # When overwriting, first delete all existing chart relationships
+                # so the dashboard is replaced rather than merged
+                if overwrite:
+                    db.session.execute(
+                        delete(dashboard_slices).where(
+                            dashboard_slices.c.dashboard_id == dashboard.id
+                        )
+                    )
+
+                # Collect chart IDs to associate with this dashboard
                 for uuid in find_chart_uuids(config["position"]):
                     if uuid not in chart_ids:
-                        break
+                        continue
                     chart_id = chart_ids[uuid]
-                    if (dashboard.id, chart_id) not in existing_relationships:
+                    if (
+                        overwrite
+                        or (dashboard.id, chart_id) not in existing_relationships
+                    ):
                         dashboard_chart_ids.append((dashboard.id, chart_id))
 
                 # Handle tags using import_tag function
@@ -197,11 +216,12 @@ def _import(
                         )
 
         # set ref in the dashboard_slices table
-        values = [
-            {"dashboard_id": dashboard_id, "slice_id": chart_id}
-            for (dashboard_id, chart_id) in dashboard_chart_ids
-        ]
-        db.session.execute(dashboard_slices.insert(), values)
+        if dashboard_chart_ids:
+            values = [
+                {"dashboard_id": dashboard_id, "slice_id": chart_id}
+                for (dashboard_id, chart_id) in dashboard_chart_ids
+            ]
+            db.session.execute(dashboard_slices.insert(), values)
 
         # Migrate any filter-box charts to native dashboard filters.
         for dashboard in dashboards:

tests/unit_tests/dashboards/commands/importers/v1/import_command_test.py

@@ -0,0 +1,214 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Tests for ImportDashboardsCommand overwrite functionality.
+
+These tests verify that when importing dashboards with overwrite=True,
+the dashboard's chart associations are replaced rather than merged.
+This addresses GitHub issue #22127.
+"""
+
+import copy
+
+from pytest_mock import MockerFixture
+from sqlalchemy.orm.session import Session
+from sqlalchemy.sql import select
+
+from tests.unit_tests.fixtures.assets_configs import (
+    charts_config_1,
+    charts_config_2,
+    dashboards_config_1,
+    dashboards_config_2,
+    databases_config,
+    datasets_config,
+)
+
+
+def test_dashboard_import_with_overwrite_replaces_charts(
+    mocker: MockerFixture, session: Session
+) -> None:
+    """
+    Test that dashboard import with overwrite=True replaces charts instead of merging.
+
+    This test verifies the fix for GitHub issue #22127:
+    1. Import a dashboard with 2 charts
+    2. Re-import the same dashboard (same UUID) with only 1 chart
+    3. The dashboard should have only 1 chart (replaced), not 3 (merged)
+    """
+    from superset import db, security_manager
+    from superset.commands.dashboard.importers.v1 import ImportDashboardsCommand
+    from superset.models.dashboard import dashboard_slices
+    from superset.models.slice import Slice
+
+    mocker.patch.object(security_manager, "can_access", return_value=True)
+
+    engine = db.session.get_bind()
+    Slice.metadata.create_all(engine)  # pylint: disable=no-member
+
+    # First import: dashboard with 2 charts (charts_config_1 has 2 charts)
+    initial_configs = {
+        **copy.deepcopy(databases_config),
+        **copy.deepcopy(datasets_config),
+        **copy.deepcopy(charts_config_1),
+        **copy.deepcopy(dashboards_config_1),
+    }
+    ImportDashboardsCommand._import(initial_configs, overwrite=True)
+
+    # Verify initial state: 2 charts associated with the dashboard
+    initial_chart_ids = db.session.scalars(select(dashboard_slices.c.slice_id)).all()
+    assert len(initial_chart_ids) == 2
+
+    # Second import: same dashboard with only 1 chart (charts_config_2 has 1 chart)
+    updated_configs = {
+        **copy.deepcopy(databases_config),
+        **copy.deepcopy(datasets_config),
+        **copy.deepcopy(charts_config_2),
+        **copy.deepcopy(dashboards_config_2),
+    }
+    ImportDashboardsCommand._import(updated_configs, overwrite=True)
+
+    # Verify final state: only 1 chart (replaced, not merged)
+    final_chart_ids = db.session.scalars(select(dashboard_slices.c.slice_id)).all()
+    assert len(final_chart_ids) == 1
+
+
+def test_dashboard_import_without_overwrite_merges_charts(
+    mocker: MockerFixture, session: Session
+) -> None:
+    """
+    Test that dashboard import without overwrite merges charts (original behavior).
+
+    When overwrite=False, new chart associations should be added to existing ones.
+    """
+    from superset import db, security_manager
+    from superset.commands.dashboard.importers.v1 import ImportDashboardsCommand
+    from superset.models.dashboard import dashboard_slices
+    from superset.models.slice import Slice
+
+    mocker.patch.object(security_manager, "can_access", return_value=True)
+
+    engine = db.session.get_bind()
+    Slice.metadata.create_all(engine)  # pylint: disable=no-member
+
+    # First import: dashboard with 1 chart
+    initial_configs = {
+        **copy.deepcopy(databases_config),
+        **copy.deepcopy(datasets_config),
+        **copy.deepcopy(charts_config_2),
+        **copy.deepcopy(dashboards_config_2),
+    }
+    ImportDashboardsCommand._import(initial_configs, overwrite=False)
+
+    # Verify initial state: 1 chart
+    initial_chart_ids = db.session.scalars(select(dashboard_slices.c.slice_id)).all()
+    assert len(initial_chart_ids) == 1
+
+    # Second import: same dashboard with 2 charts, but overwrite=False
+    updated_configs = {
+        **copy.deepcopy(databases_config),
+        **copy.deepcopy(datasets_config),
+        **copy.deepcopy(charts_config_1),
+        **copy.deepcopy(dashboards_config_1),
+    }
+    ImportDashboardsCommand._import(updated_configs, overwrite=False)
+
+    # When overwrite=False, new charts are added but existing ones remain
+    # The dashboard_config_1 has 2 charts, one of which is the same as in config_2
+    # So we should have 2 total (not 3, since the common chart is not duplicated)
+    final_chart_ids = db.session.scalars(select(dashboard_slices.c.slice_id)).all()
+    assert len(final_chart_ids) == 2
+
+
+def test_dashboard_import_with_overwrite_adds_new_charts(
+    mocker: MockerFixture, session: Session
+) -> None:
+    """
+    Test that dashboard import with overwrite=True correctly adds new charts.
+
+    This verifies that overwrite mode not only removes old charts but also
+    properly adds the new charts from the import.
+    """
+    from superset import db, security_manager
+    from superset.commands.dashboard.importers.v1 import ImportDashboardsCommand
+    from superset.models.dashboard import dashboard_slices
+    from superset.models.slice import Slice
+
+    mocker.patch.object(security_manager, "can_access", return_value=True)
+
+    engine = db.session.get_bind()
+    Slice.metadata.create_all(engine)  # pylint: disable=no-member
+
+    # First import: dashboard with 1 chart
+    initial_configs = {
+        **copy.deepcopy(databases_config),
+        **copy.deepcopy(datasets_config),
+        **copy.deepcopy(charts_config_2),
+        **copy.deepcopy(dashboards_config_2),
+    }
+    ImportDashboardsCommand._import(initial_configs, overwrite=True)
+
+    # Verify initial state: 1 chart
+    initial_chart_ids = db.session.scalars(select(dashboard_slices.c.slice_id)).all()
+    assert len(initial_chart_ids) == 1
+
+    # Second import: same dashboard with 2 charts
+    updated_configs = {
+        **copy.deepcopy(databases_config),
+        **copy.deepcopy(datasets_config),
+        **copy.deepcopy(charts_config_1),
+        **copy.deepcopy(dashboards_config_1),
+    }
+    ImportDashboardsCommand._import(updated_configs, overwrite=True)
+
+    # Verify final state: 2 charts (replaced with new set)
+    final_chart_ids = db.session.scalars(select(dashboard_slices.c.slice_id)).all()
+    assert len(final_chart_ids) == 2
+
+
+def test_dashboard_import_new_dashboard(
+    mocker: MockerFixture, session: Session
+) -> None:
+    """
+    Test that importing a new dashboard works correctly.
+    """
+    from superset import db, security_manager
+    from superset.commands.dashboard.importers.v1 import ImportDashboardsCommand
+    from superset.models.dashboard import dashboard_slices
+    from superset.models.slice import Slice
+
+    mocker.patch.object(security_manager, "can_access", return_value=True)
+
+    engine = db.session.get_bind()
+    Slice.metadata.create_all(engine)  # pylint: disable=no-member
+
+    configs = {
+        **copy.deepcopy(databases_config),
+        **copy.deepcopy(datasets_config),
+        **copy.deepcopy(charts_config_1),
+        **copy.deepcopy(dashboards_config_1),
+    }
+    expected_number_of_dashboards = len(dashboards_config_1)
+    expected_number_of_charts = len(charts_config_1)
+
+    ImportDashboardsCommand._import(configs, overwrite=True)
+    dashboard_ids = db.session.scalars(
+        select(dashboard_slices.c.dashboard_id).distinct()
+    ).all()
+    chart_ids = db.session.scalars(select(dashboard_slices.c.slice_id)).all()
+
+    assert len(chart_ids) == expected_number_of_charts
+    assert len(dashboard_ids) == expected_number_of_dashboards