-
Notifications
You must be signed in to change notification settings - Fork 14k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Add explicit ON DELETE CASCADE for dashboard_slices #24938
chore: Add explicit ON DELETE CASCADE for dashboard_slices #24938
Conversation
dc79d04
to
b3fc170
Compare
Codecov Report
@@ Coverage Diff @@
## master #24938 +/- ##
==========================================
- Coverage 69.00% 69.00% -0.01%
==========================================
Files 1906 1906
Lines 74147 74143 -4
Branches 8209 8209
==========================================
- Hits 51166 51162 -4
Misses 20858 20858
Partials 2123 2123
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
b3fc170
to
b7bb0dc
Compare
@@ -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 = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My default this is an empty list.
|
||
hidden_dash = Dashboard() | ||
hidden_dash.dashboard_title = "Not My Dashboard" | ||
hidden_dash.slug = not_my_dash_slug | ||
hidden_dash.slices = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See previous.
@@ -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 = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See previous.
@@ -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 = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See previous.
b7bb0dc
to
ed7f067
Compare
ed7f067
to
dc8dae0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for the improvement @john-bodley!
SUMMARY
Somewhat akin to #24628, this PR ensures explicit ON DELETE CASCADE operations are set for the
dashboard_slices
table which means these relationships don't need to be explicitly deleted for non-SQLAlchemy ORM operations.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
CI.
ADDITIONAL INFORMATION