Skip to content

Commit

Permalink
chore: Add explicit ON DELETE CASCADE for dashboard_slices
Browse files Browse the repository at this point in the history
  • Loading branch information
john-bodley committed Aug 9, 2023
1 parent f6c3f0c commit dc8dae0
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 14 deletions.
1 change: 1 addition & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 0 additions & 3 deletions superset/daos/chart.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 0 additions & 1 deletion superset/daos/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
4 changes: 2 additions & 2 deletions superset/models/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
)

Expand Down
1 change: 0 additions & 1 deletion tests/integration_tests/charts/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
3 changes: 0 additions & 3 deletions tests/integration_tests/dashboard_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 0 additions & 2 deletions tests/integration_tests/tagging_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit dc8dae0

Please sign in to comment.