-
Notifications
You must be signed in to change notification settings - Fork 61
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
Fix role update issue #46 #47
Conversation
app/controllers/refinery/authentication/devise/admin/users_controller.rb
Show resolved
Hide resolved
@@ -63,7 +63,7 @@ def create_successful | |||
# if the user is a superuser and can assign roles according to this site's | |||
# settings then the roles are set with the POST data. | |||
if user_can_assign_roles? | |||
@user.roles = @selected_role_names.map { |r| Refinery::Authentication::Devise::Role[r.downcase] } | |||
@user.roles = @selected_role_names.map(&:underscore) |
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.
this was supposed to turn the array into roles
@user.roles = @selected_role_names.map(&:underscore) | |
@user.roles = @selected_role_names.map { |r| Refinery::Authentication::Devise::Role[r.underscore] } |
end | ||
|
||
store_user_memento | ||
|
||
@user.roles = @selected_role_names.map { |r| Refinery::Authentication::Devise::Role[r.downcase] } | ||
@user.roles = @selected_role_names.map(&:underscore) |
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.
this was supposed to turn the array into roles
@user.roles = @selected_role_names.map(&:underscore) | |
@user.roles = @selected_role_names.map { |r| Refinery::Authentication::Devise::Role[r.underscore] } |
@dreamsoftech ^ I reviewed this for you 😄 |
@parndt Pushed changes |
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.
@bricesanchez looks okay to me; what do you think?
For some reason, CI is seriously unhappy with it. |
@parndt It seems like it needs to rebase this branch against master. |
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.
Thanks for this bugfix!
I will be ok to merge it if CI goes green
thank you! |
From #46: