Skip to content
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

Removing groups changes users current group #19376

Merged
merged 1 commit into from
Oct 8, 2019

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Oct 7, 2019

When removing a group that is a user's current_group_id, change it.

The {before,after}_destroy hooks work differently in rails 5.2

The user-group association is now removed before the cleanup.
So using this association to lookup users will always look at no users, since those records are deleted.

Removing this dependency then just focuses on users who have a stale current_group.
Which is the basic premise of this task

so now, user's default_group are properly updated.

Added the third test case around groups to be super sure this is working

When removing a group that is a user's current_group_id, change it.

The {before,after}_destroy hooks work differently in rails 5.2

The user-group association is now removed before the cleanup.
So using this association to lookup users will always look at
no users.

Removing this dependency focuses on users who have a stale current_group.
Which is the basic premise of this task
@kbrock
Copy link
Member Author

kbrock commented Oct 8, 2019

strange tests failure - feels like it should have been failing in the past.

the update is to delete the user from the widget test - because that user is not significant in the scheme of seeing when a group is removed.

@miq-bot
Copy link
Member

miq-bot commented Oct 8, 2019

Checked commit kbrock@981a439 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 🍰

@jrafanie jrafanie self-assigned this Oct 8, 2019
Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jrafanie jrafanie merged commit 87fa4b7 into ManageIQ:master Oct 8, 2019
@jrafanie jrafanie added this to the Sprint 122 Ending Oct 14, 2019 milestone Oct 8, 2019
@kbrock kbrock deleted the rails_52_groups branch October 9, 2019 02:58
@chessbyte chessbyte mentioned this pull request Mar 31, 2020
38 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants