fix(dashboard): Dashboard import commands not correctly replacing charts

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

@@ -19,7 +19,7 @@
 
 from marshmallow import Schema
 from sqlalchemy.orm import Session  # noqa: F401
-from sqlalchemy.sql import select
+from sqlalchemy.sql import delete, insert
 
 from superset import db
 from superset.charts.schemas import ImportV1ChartSchema
@@ -120,32 +120,32 @@
                 charts.append(chart)
                 chart_ids[str(chart.uuid)] = chart.id
 
-        # 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()
-
         # import dashboards
         dashboards: list[Dashboard] = []
-        dashboard_chart_ids: list[tuple[int, int]] = []
         for file_name, config in configs.items():
             if file_name.startswith("dashboards/"):
                 config = update_id_refs(config, chart_ids, dataset_info)
                 dashboard = import_dashboard(config, overwrite=overwrite)
                 dashboards.append(dashboard)
+
+                # set ref in the dashboard_slices table
+                dashboard_chart_ids: list[dict[str, int]] = []
                 for uuid in find_chart_uuids(config["position"]):
                     if uuid not in chart_ids:
                         break
                     chart_id = chart_ids[uuid]
-                    if (dashboard.id, chart_id) not in existing_relationships:
-                        dashboard_chart_ids.append((dashboard.id, chart_id))
-
-        # 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)
+                    dashboard_chart_id = {
+                        "dashboard_id": dashboard.id,
+                        "slice_id": chart_id,
+                    }
+                    dashboard_chart_ids.append(dashboard_chart_id)
+
+                db.session.execute(
+                    delete(dashboard_slices).where(
+                        dashboard_slices.c.dashboard_id == dashboard.id
+                    )
+                )
+                db.session.execute(insert(dashboard_slices).values(dashboard_chart_ids))
 
         # Migrate any filter-box charts to native dashboard filters.
         for dashboard in dashboards:

tests/integration_tests/dashboards/commands_tests.py

@@ -51,7 +51,9 @@
 from tests.integration_tests.base_tests import SupersetTestCase
 from tests.integration_tests.fixtures.importexport import (
     chart_config,
+    chart_config_2,
     dashboard_config,
+    dashboard_config_2,
     dashboard_export,
     dashboard_metadata_config,
     database_config,
@@ -742,6 +744,67 @@ def test_import_v1_dashboard_validation(self):
             }
         }
 
+    @patch("superset.security.manager.g")
+    @patch("superset.commands.database.importers.v1.utils.add_permissions")
+    def test_import_v1_dashboard_overwrite_removes_deleted_charts(
+        self, mock_add_permissions, mock_g
+    ):
+        """Test that existing dashboards are updated without old charts."""
+        mock_g.user = security_manager.find_user("admin")
+
+        num_dashboards = db.session.query(Dashboard).count()
+
+        contents = {
+            "metadata.yaml": yaml.safe_dump(dashboard_metadata_config),
+            "databases/imported_database.yaml": yaml.safe_dump(database_config),
+            "datasets/imported_dataset.yaml": yaml.safe_dump(dataset_config),
+            "charts/imported_chart.yaml": yaml.safe_dump(chart_config),
+            "dashboards/imported_dashboard.yaml": yaml.safe_dump(dashboard_config),
+        }
+        command = v1.ImportDashboardsCommand(contents, overwrite=True)
+        command.run()
+
+        dashboard = (
+            db.session.query(Dashboard).filter_by(uuid=dashboard_config["uuid"]).one()
+        )
+
+        old_chart = dashboard.slices[0]
+        assert str(old_chart.uuid) == chart_config["uuid"]
+
+        overwrite_contents = {
+            "metadata.yaml": yaml.safe_dump(dashboard_metadata_config),
+            "databases/imported_database.yaml": yaml.safe_dump(database_config),
+            "datasets/imported_dataset.yaml": yaml.safe_dump(dataset_config),
+            "charts/imported_chart.yaml": yaml.safe_dump(chart_config_2),
+            "dashboards/imported_dashboard.yaml": yaml.safe_dump(dashboard_config_2),
+        }
+        overwrite_command = v1.ImportDashboardsCommand(
+            overwrite_contents, overwrite=True
+        )
+        overwrite_command.run()
+
+        new_num_dashboards = db.session.query(Dashboard).count()
+        assert new_num_dashboards == num_dashboards + 1
+
+        dashboard = (
+            db.session.query(Dashboard).filter_by(uuid=dashboard_config["uuid"]).one()
+        )
+
+        assert len(dashboard.slices) == 1
+
+        new_chart = dashboard.slices[0]
+        assert str(new_chart.uuid) == chart_config_2["uuid"]
+
+        dataset = old_chart.table
+        database = dataset.database
+
+        db.session.delete(dashboard)
+        db.session.delete(old_chart)
+        db.session.delete(new_chart)
+        db.session.delete(dataset)
+        db.session.delete(database)
+        db.session.commit()
+
 
 class TestCopyDashboardCommand(SupersetTestCase):
     @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")

tests/integration_tests/fixtures/importexport.py

@@ -681,6 +681,96 @@
     },
     "version": "1.0.0",
 }
+
+chart_config_2: dict[str, Any] = {
+    "slice_name": "Deck Path 2",
+    "viz_type": "deck_path",
+    "params": {
+        "color_picker": {"a": 1, "b": 135, "g": 122, "r": 0},
+        "datasource": "12__table",
+        "js_columns": ["color"],
+        "js_data_mutator": r"data => data.map(d => ({\n    ...d,\n    color: colors.hexToRGB(d.extraProps.color)\n}));",  # noqa: E501
+        "js_onclick_href": "",
+        "js_tooltip": "",
+        "line_column": "path_json",
+        "line_type": "json",
+        "line_width": 100,
+        "mapbox_style": "mapbox://styles/mapbox/light-v9",
+        "reverse_long_lat": False,
+        "row_limit": 5000,
+        "slice_id": 43,
+        "time_grain_sqla": None,
+        "time_range": " : ",
+        "viewport": {
+            "altitude": 1.5,
+            "bearing": 0,
+            "height": 1094,
+            "latitude": 37.73671752604488,
+            "longitude": -122.18885402582598,
+            "maxLatitude": 85.05113,
+            "maxPitch": 10,
+            "maxZoom": 40,
+            "minLatitude": -85.05113,
+            "minPitch": 0,
+            "minZoom": 0,
+            "pitch": 0,
+            "width": 669,
+            "zoom": 9.51847667620428,
+        },
+        "viz_type": "deck_path",
+    },
+    "query_context": '{"datasource":{"id":12,"type":"table"},"force":false,"queries":[{"time_range":" : ","filters":[],"extras":{"time_grain_sqla":null,"having":"","where":""},"applied_time_extras":{},"columns":[],"metrics":[],"annotation_layers":[],"row_limit":5000,"timeseries_limit":0,"order_desc":true,"url_params":{},"custom_params":{},"custom_form_data":{}}],"result_format":"json","result_type":"full"}',  # noqa: E501
+    "cache_timeout": None,
+    "uuid": "82bb160d-a859-44ce-86cc-9a92de113514",
+    "version": "1.0.0",
+    "dataset_uuid": "10808100-158b-42c4-842e-f32b99d88dfb",
+}
+
+dashboard_config_2: dict[str, Any] = {
+    "dashboard_title": "Test dash",
+    "description": None,
+    "css": "",
+    "slug": None,
+    "uuid": "c4b28c4e-a1fe-4cf8-a5ac-d6f11d6fdd51",
+    "position": {
+        "CHART-PsLqYTEu9ZS": {
+            "children": [],
+            "id": "CHART-PsLqYTEu9ZS",
+            "meta": {
+                "chartId": 283,
+                "height": 50,
+                "sliceName": "Games",
+                "uuid": "82bb160d-a859-44ce-86cc-9a92de113514",
+                "width": 4,
+            },
+            "parents": ["ROOT_ID", "GRID_ID", "ROW-dP_CHaK2q"],
+            "type": "CHART",
+        },
+        "DASHBOARD_VERSION_KEY": "v2",
+        "GRID_ID": {
+            "children": ["ROW-dP_CHaK2q"],
+            "id": "GRID_ID",
+            "parents": ["ROOT_ID"],
+            "type": "GRID",
+        },
+        "HEADER_ID": {
+            "id": "HEADER_ID",
+            "meta": {"text": "Test dash"},
+            "type": "HEADER",
+        },
+        "ROOT_ID": {"children": ["GRID_ID"], "id": "ROOT_ID", "type": "ROOT"},
+        "ROW-dP_CHaK2q": {
+            "children": ["CHART-PsLqYTEu9ZS"],
+            "id": "ROW-dP_CHaK2q",
+            "meta": {"0": "ROOT_ID", "background": "BACKGROUND_TRANSPARENT"},
+            "parents": ["ROOT_ID", "GRID_ID"],
+            "type": "ROW",
+        },
+    },
+    "metadata": {},
+    "version": "1.0.0",
+}
+
 saved_queries_config = {
     "schema": "public",
     "label": "Test Saved Query",