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

Fix role update issue #46 #47

Merged
merged 5 commits into from
Dec 12, 2023
Merged

Fix role update issue #46 #47

merged 5 commits into from
Dec 12, 2023

Conversation

dreamsoftech
Copy link
Contributor

@dreamsoftech dreamsoftech commented Oct 5, 2019

From #46:

When role titles are camel cased, it creates new role named downcased instead of updating properly.
For example, when I have role named Community Member, it creates new role named communitymember instead of updating role by community_member.

@@ -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)
Copy link
Member

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

Suggested change
@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)
Copy link
Member

@parndt parndt Oct 9, 2019

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

Suggested change
@user.roles = @selected_role_names.map(&:underscore)
@user.roles = @selected_role_names.map { |r| Refinery::Authentication::Devise::Role[r.underscore] }

@parndt
Copy link
Member

parndt commented Nov 8, 2019

@dreamsoftech ^ I reviewed this for you 😄

@dreamsoftech
Copy link
Contributor Author

@parndt Pushed changes

parndt
parndt previously approved these changes Nov 8, 2019
Copy link
Member

@parndt parndt left a 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?

@parndt
Copy link
Member

parndt commented Nov 8, 2019

For some reason, CI is seriously unhappy with it.

@dreamsoftech
Copy link
Contributor Author

@parndt It seems like it needs to rebase this branch against master.

bricesanchez
bricesanchez previously approved these changes Nov 9, 2019
Copy link
Member

@bricesanchez bricesanchez left a 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

@parndt parndt dismissed stale reviews from bricesanchez and themself via b7af987 December 12, 2023 06:23
@parndt parndt merged commit b0ec425 into refinery:master Dec 12, 2023
4 checks passed
@parndt
Copy link
Member

parndt commented Dec 12, 2023

thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants