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.

Sign up for GitHub

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

Jump to bottom

Add check for nil group to system_context_requester #19309

Merged
merged 1 commit into from
Sep 19, 2019
Merged

Add check for nil group to system_context_requester #19309

merged 1 commit into from
Sep 19, 2019

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Sep 18, 2019 •
edited
Loading

System retirement breaks currently if a user's groups get deleted.

https://bugzilla.redhat.com/show_bug.cgi?id=1750990

given the situation we're in here...

@miq-bot miq-bot added the wip label Sep 18, 2019
@d-m-u
Copy link
Contributor Author

d-m-u commented Sep 18, 2019

@miq-bot assign @tinaafitz
@miq-bot add_label bug

@miq-bot miq-bot added the bug label Sep 18, 2019
@d-m-u
Copy link
Contributor Author

d-m-u commented Sep 18, 2019

@miq-bot add_label retirement

@d-m-u d-m-u force-pushed the removing_dependency_on_user_group_from_retirement_mixin branch from c05d12c to ae9ef98 Compare September 18, 2019 15:37
@d-m-u
Copy link
Contributor Author

d-m-u commented Sep 18, 2019

@lfu please review

@d-m-u
Copy link
Contributor Author

d-m-u commented Sep 18, 2019

@tinaafitz I have five days to get this in but I'm not sure what else this PR needs. Please advise.

@d-m-u
Copy link
Contributor Author

d-m-u commented Sep 18, 2019

@miq-bot add_label hammer/yes

@d-m-u
Copy link
Contributor Author

d-m-u commented Sep 19, 2019

Ooookay, now four days to get this in, two of which are weekend days: @tinaafitz please take a look?

@d-m-u d-m-u force-pushed the removing_dependency_on_user_group_from_retirement_mixin branch from ae9ef98 to 584271a Compare September 19, 2019 11:32
@miq-bot
Copy link
Member

miq-bot commented Sep 19, 2019

Checked commit d-m-u@584271a with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

@d-m-u d-m-u changed the title [WIP] Add check for nil group to system_context_requester Add check for nil group to system_context_requester Sep 19, 2019
@d-m-u
Copy link
Contributor Author

d-m-u commented Sep 19, 2019

@miq-bot remove_label wip

@miq-bot miq-bot removed the wip label Sep 19, 2019
@tinaafitz
Copy link
Member

@gmcculloug Please review.

@d-m-u
Copy link
Contributor Author

d-m-u commented Sep 19, 2019

@miq-bot add_label changelog/yes
or whatever

@gmcculloug gmcculloug merged commit 957a270 into ManageIQ:master Sep 19, 2019
@gmcculloug gmcculloug added this to the Sprint 121 Ending Sep 30, 2019 milestone Sep 19, 2019
d-m-u added a commit to d-m-u/manageiq that referenced this pull request Sep 20, 2019
@d-m-u
Users without groups should use admin retirement
53339c3 
Also yes I changed it to be more pretty but it does the same

Backport of ManageIQ#19309

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1754043
@d-m-u d-m-u mentioned this pull request Sep 20, 2019
Merged
d-m-u added a commit to d-m-u/manageiq that referenced this pull request Sep 20, 2019
@d-m-u
Users without groups should use admin retirement
3529179 
Backport of ManageIQ#19309

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1754043
d-m-u added a commit to d-m-u/manageiq that referenced this pull request Sep 20, 2019
@d-m-u
Users without groups should use admin retirement
77e6479 
Backport of ManageIQ#19309

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1754043
d-m-u added a commit to d-m-u/manageiq that referenced this pull request Sep 20, 2019
@d-m-u
Users without groups should use admin retirement
cf9ee20 
Backport of ManageIQ#19309

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1754043
d-m-u added a commit to d-m-u/manageiq that referenced this pull request Sep 20, 2019
@d-m-u
Users without groups should use admin retirement
e6ecf26 
Backport of ManageIQ#19309

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1754043
d-m-u added a commit to d-m-u/manageiq that referenced this pull request Sep 20, 2019
@d-m-u
Users without groups should use admin retirement
a392f02 
Backport of ManageIQ#19309

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1754043
d-m-u added a commit to d-m-u/manageiq that referenced this pull request Sep 23, 2019
@d-m-u
Users without groups should use admin retirement
6dfff4d 
Backport of ManageIQ#19309

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1754043
d-m-u added a commit to d-m-u/manageiq that referenced this pull request Sep 23, 2019
@d-m-u
Users without groups should use admin retirement
e53ed26 
Backport of ManageIQ#19309

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1754043
d-m-u added a commit to d-m-u/manageiq that referenced this pull request Sep 23, 2019
@d-m-u
Users without groups should use admin retirement
9a505fc 
Backport of ManageIQ#19309

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1754043
d-m-u added a commit to d-m-u/manageiq that referenced this pull request Sep 23, 2019
@d-m-u
Users without groups should use admin retirement
df73dc8 
Backport of ManageIQ#19309

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1754043
@simaishi
Copy link
Contributor

Backported to hammer via #19319

@d-m-u d-m-u deleted the removing_dependency_on_user_group_from_retirement_mixin branch September 26, 2019 10:35
simaishi pushed a commit that referenced this pull request Nov 1, 2019
@gmcculloug @simaishi
Merge pull request #19309 from d-m-u/removing_dependency_on_user_grou…
6254a4b 
…p_from_retirement_mixin

Add check for nil group to system_context_requester

(cherry picked from commit 957a270)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1767896
@simaishi
Copy link
Contributor

simaishi commented Nov 1, 2019

Ivanchuk backport details:

$ git log -1
commit 6254a4b14d64286239b3e2a612b13a38cfeee13e
Author: Greg McCullough <[email protected]>
Date:   Thu Sep 19 17:40:48 2019 -0400

    Merge pull request #19309 from d-m-u/removing_dependency_on_user_group_from_retirement_mixin
    
    Add check for nil group to system_context_requester
    
    (cherry picked from commit 957a270a4567ab9bb0cd11ac27d5a0a6e16b9b6b)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1767896

@simaishi
Copy link
Contributor

simaishi commented Nov 1, 2019

@d-m-u please take a look at Travis failure in ivanchuk:

  1) VM Retirement Management #retirement_check with user lacking group uses user as requester
     Failure/Error: updated_count = existing_objects.update_all(:retirement_requester => requester.userid)
     NoMethodError:
       undefined method `userid' for nil:NilClass
     # ./app/models/mixins/retirement_mixin.rb:33:in `set_retirement_requester'
     # ./app/models/mixins/retirement_mixin.rb:16:in `make_retire_request'
     # ./app/models/mixins/retirement_mixin.rb:138:in `retirement_check'
     # ./spec/models/vm/retirement_management_spec.rb:35:in `block (4 levels) in <top (required)>'

https://travis-ci.org/ManageIQ/manageiq/jobs/606067149

@d-m-u d-m-u mentioned this pull request Nov 4, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkanoor mkanoor mkanoor approved these changes

@lfu lfu lfu approved these changes

@tinaafitz tinaafitz tinaafitz approved these changes

Assignees

@tinaafitz tinaafitz

Labels
bug changelog/yes hammer/backported ivanchuk/backported lifecycle/retirement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@d-m-u @miq-bot @tinaafitz @simaishi @lfu @mkanoor @gmcculloug