Adding data integrity constrains for some associations to User model #18472
Conversation
yrudman
force-pushed
the
prevent-user-deleting-if-user-present-somewere
branch
2 times, most recently
from
c91e2e4
to
ecb1e9f
Compare
yrudman
changed the title
|
@gtanzillo @jrafanie could you review I am not sure which models should have this constrain |
yrudman
force-pushed
the
prevent-user-deleting-if-user-present-somewere
branch
from
1e1aa44
to
6540d00
Compare
|
@yrudman @jrafanie I think we need some further discussion on this one. It feels like this change is definitely necessary to safeguard against deleting a user and breaking references leading to the originally reported bug. But i see you point that this could prevent a user from ever getting deleted. I'm not sure if that's such a bad thing , though. If we add the ability to reassign the referenced objects to another user, it may open another can of worms with groups, roles and RBAC. Not sure. |
app/models/user.rb
Outdated
| end | ||
|
|
||
| unless present_ref.empty? | ||
| errors.add(:base, "user with id [#{id}] has references to other models: #{present_ref.join(" ")}") |
There was a problem hiding this comment.
We should probably also mention the userid or name
app/models/user.rb
Outdated
| @@ -110,6 +112,18 @@ def validate | |||
| before_validation :dummy_password_for_external_auth | |||
| before_destroy :destroy_subscribed_widget_sets | |||
|
|
|||
| def check_reference | |||
| present_ref = [] | |||
| %w(miq_requests vms miq_widgets miq_templates).each do |association| | |||
There was a problem hiding this comment.
I assume that the user will still have miq_widgets since this callback is being prepended and will run before :destroy_subscribed_widget_sets, right?
There was a problem hiding this comment.
yes, this check will be executed first
yrudman
force-pushed
the
prevent-user-deleting-if-user-present-somewere
branch
from
6540d00
to
76614c7
Compare
… currently it is possible to delete user which still referred. BZ https://bugzilla.redhat.com/show_bug.cgi?id=1677744
yrudman
force-pushed
the
prevent-user-deleting-if-user-present-somewere
branch
from
76614c7
to
3f04d3c
Compare
|
Checked commits yrudman/manageiq@2d63bc6~...0006000 with ruby 2.3.3, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 |
@jrafanie Any other thoughts here? I'm leaning toward merging this. |
|
@miq-bot add-label changelog/yes |
|
Yeah, I'm fine with this. We'll see what it breaks and change it if needed. |
Issue: It is currently possible to destroy user which still referred to from other models
Fix:
before_destroycallback cancelingdestroyif user refer to in other modelsBZ https://bugzilla.redhat.com/show_bug.cgi?id=1677744
@miq-bot add-label bug, core