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

Converge request user roles to match API and UI #17849

Merged
merged 1 commit into from
Aug 16, 2018

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Aug 13, 2018

FOLLOWUP:

We introduced feature miq_request_superadmin in f6c02e8 / #17444
The feature miq_request_approval already existed.
Merging the 2 together.

This fixes an inconsistency between API and UI
The issue existed before this feature was introduced
so this works with #17444 to fix the BZ

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

We introduced feature miq_request_superadmin in f6c02e8 / ManageIQ#17444 
The feature miq_request_approval already existed.
Merging the 2 together.

This fixes an inconsistency between API and UI
The issue existed before this feature was introduced
so this works with 17444 to fix the BZ

ManageIQ#17444
https://bugzilla.redhat.com/show_bug.cgi?id=1608554
@kbrock
Copy link
Member Author

kbrock commented Aug 13, 2018

@jrafanie and @gtanzillo This pushes us more towards using *_admin_user? type methods that you both like.

I do have a followup that will remove the ambiguous .admin_user? #17850
I've already removed references to admin_user? in all repos.

@miq-bot
Copy link
Member

miq-bot commented Aug 13, 2018

Checked commit kbrock@9a63e12 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

@jrafanie
Copy link
Member

@kbrock did you try to use the new recreation environment from the bz? I'm curious if this change fixes the api not bring back requests for approvers

@kbrock
Copy link
Member Author

kbrock commented Aug 16, 2018

@jrafanie I need to backport this stuff to that environment to check it. This backport effort is non-trivial. Since we want to make this change regardless, can we just do this and then followup if it doesn't fix all the issues?

@gtanzillo gtanzillo added this to the Sprint 93 Ending Aug 27, 2018 milestone Aug 16, 2018
@gtanzillo gtanzillo merged commit 31599e6 into ManageIQ:master Aug 16, 2018
@kbrock kbrock deleted the bz_1608554 branch August 16, 2018 20:37
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.

4 participants