chore: Add explicit ON DELETE CASCADE for dashboard_slices (#24938)

UPDATING.md

@@ -24,6 +24,7 @@ assists people when migrating to a new version.
 
 ## Next
 
+- [24938](https://github.com/apache/superset/pull/24938): Augments the foreign key constraints for the `dashboard_slices` table to include an explicit CASCADE ON DELETE to ensure the relevant records are deleted when a dashboard or slice is deleted. Scheduled downtime may be advised.
 - [24657](https://github.com/apache/superset/pull/24657): Bumps the cryptography package to augment the OpenSSL security vulnerability.
 - [24628](https://github.com/apache/superset/pull/24628): Augments the foreign key constraints for the `dashboard_owner`, `report_schedule_owner`, and `slice_owner` tables to include an explicit CASCADE ON DELETE to ensure the relevant ownership records are deleted when a dataset is deleted. Scheduled downtime may be advised.
 - [24488](https://github.com/apache/superset/pull/24488): Augments the foreign key constraints for the `sql_metrics`, `sqlatable_user`, and `table_columns` tables to include an explicit CASCADE ON DELETE to ensure the relevant records are deleted when a dataset is deleted. Scheduled downtime may be advised.

superset/daos/chart.py

@@ -43,9 +43,6 @@ class ChartDAO(BaseDAO[Slice]):
     def delete(cls, items: Slice | list[Slice], commit: bool = True) -> None:
         item_ids = [item.id for item in get_iterable(items)]
         # bulk delete, first delete related data
-        for item in get_iterable(items):
-            item.dashboards = []
-            db.session.merge(item)
         # bulk delete itself
         try:
             db.session.query(Slice).filter(Slice.id.in_(item_ids)).delete(

superset/daos/dashboard.py

@@ -197,7 +197,6 @@ def delete(cls, items: Dashboard | list[Dashboard], commit: bool = True) -> None
         item_ids = [item.id for item in get_iterable(items)]
         # bulk delete, first delete related data
         for item in get_iterable(items):
-            item.slices = []
             item.embedded = []
             db.session.merge(item)
         # bulk delete itself

superset/migrations/versions/2023-08-09_14-17_8ace289026f3_add_on_delete_cascade_for_dashboard_slices.py

@@ -0,0 +1,55 @@
+# 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.
+"""add on delete cascade for dashboard slices
+
+Revision ID: 8ace289026f3
+Revises: 2e826adca42c
+Create Date: 2023-08-09 14:17:53.326191
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "8ace289026f3"
+down_revision = "2e826adca42c"
+
+
+from superset.migrations.shared.constraints import ForeignKey, redefine
+
+foreign_keys = [
+    ForeignKey(
+        table="dashboard_slices",
+        referent_table="dashboards",
+        local_cols=["dashboard_id"],
+        remote_cols=["id"],
+    ),
+    ForeignKey(
+        table="dashboard_slices",
+        referent_table="slices",
+        local_cols=["slice_id"],
+        remote_cols=["id"],
+    ),
+]
+
+
+def upgrade():
+    for foreign_key in foreign_keys:
+        redefine(foreign_key, on_delete="CASCADE")
+
+
+def downgrade():
+    for foreign_key in foreign_keys:
+        redefine(foreign_key)

superset/models/dashboard.py

@@ -105,8 +105,8 @@ def copy_dashboard(_mapper: Mapper, connection: Connection, target: Dashboard) -
     "dashboard_slices",
     metadata,
     Column("id", Integer, primary_key=True),
-    Column("dashboard_id", Integer, ForeignKey("dashboards.id")),
-    Column("slice_id", Integer, ForeignKey("slices.id")),
+    Column("dashboard_id", Integer, ForeignKey("dashboards.id", ondelete="CASCADE")),
+    Column("slice_id", Integer, ForeignKey("slices.id", ondelete="CASCADE")),
     UniqueConstraint("dashboard_id", "slice_id"),
 )
 

tests/integration_tests/charts/api_tests.py

@@ -181,7 +181,6 @@ def add_dashboard_to_chart(self):
             self.new_dashboard.dashboard_title = "New Dashboard"
             self.new_dashboard.slug = "new_slug"
             self.new_dashboard.owners = [admin]
-            self.new_dashboard.slices = []
             self.new_dashboard.published = False
             db.session.add(self.new_dashboard)
 

tests/integration_tests/dashboard_tests.py

@@ -207,12 +207,10 @@ def test_users_can_view_own_dashboard(self):
         dash.dashboard_title = "My Dashboard"
         dash.slug = my_dash_slug
         dash.owners = [user]
-        dash.slices = []
 
         hidden_dash = Dashboard()
         hidden_dash.dashboard_title = "Not My Dashboard"
         hidden_dash.slug = not_my_dash_slug
-        hidden_dash.slices = []
 
         db.session.add(dash)
         db.session.add(hidden_dash)
@@ -277,7 +275,6 @@ def test_user_can_not_view_unpublished_dash(self):
         dash.dashboard_title = "My Dashboard"
         dash.slug = slug
         dash.owners = [admin_user]
-        dash.slices = []
         dash.published = False
         db.session.add(dash)
         db.session.commit()

tests/integration_tests/security/guest_token_security_tests.py

@@ -186,8 +186,6 @@ def test_raise_for_access_dashboard_as_guest_no_rbac(self):
         # Create a draft dashboard that is not embedded
         dash = Dashboard()
         dash.dashboard_title = "My Dashboard"
-        dash.owners = []
-        dash.slices = []
         dash.published = False
         db.session.add(dash)
         db.session.commit()

tests/integration_tests/tagging_tests.py

@@ -136,7 +136,6 @@ def test_dashboard_tagging(self):
         test_dashboard = Dashboard()
         test_dashboard.dashboard_title = "test_dashboard"
         test_dashboard.slug = "test_slug"
-        test_dashboard.slices = []
         test_dashboard.published = True
 
         db.session.add(test_dashboard)
@@ -264,7 +263,6 @@ def test_tagging_system(self):
         test_dashboard = Dashboard()
         test_dashboard.dashboard_title = "test_dashboard"
         test_dashboard.slug = "test_slug"
-        test_dashboard.slices = []
         test_dashboard.published = True
 
         # Create a saved query and add it to the db