diff --git a/UPDATING.md b/UPDATING.md index 22648f140a527..a6c826a3844dd 100644 --- a/UPDATING.md +++ b/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. diff --git a/superset/daos/chart.py b/superset/daos/chart.py index c8dc216a4d2f2..a99e80da405e2 100644 --- a/superset/daos/chart.py +++ b/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( diff --git a/superset/daos/dashboard.py b/superset/daos/dashboard.py index f01d61044882b..425d640e7742d 100644 --- a/superset/daos/dashboard.py +++ b/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 diff --git a/superset/migrations/versions/2023-08-09_14-17_8ace289026f3_add_on_delete_cascade_for_dashboard_slices.py b/superset/migrations/versions/2023-08-09_14-17_8ace289026f3_add_on_delete_cascade_for_dashboard_slices.py new file mode 100644 index 0000000000000..caac489bd1b1a --- /dev/null +++ b/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) diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py index 0fecf15a55180..719a6df8e4b10 100644 --- a/superset/models/dashboard.py +++ b/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"), ) diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py index c5e88426fad80..ae64eba8071ab 100644 --- a/tests/integration_tests/charts/api_tests.py +++ b/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) diff --git a/tests/integration_tests/dashboard_tests.py b/tests/integration_tests/dashboard_tests.py index a8cbd97aa2f02..fef4edd6cc4d1 100644 --- a/tests/integration_tests/dashboard_tests.py +++ b/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() diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py index cb6095c0e7f4b..5f50bf4b50e11 100644 --- a/tests/integration_tests/security/guest_token_security_tests.py +++ b/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() diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index 72ba577d9ffc0..4ecfd1049f31e 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/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