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

Ability - case insensitive check of admin_role #316

Merged
merged 1 commit into from
Jun 14, 2017

Conversation

ouranos
Copy link
Contributor

@ouranos ouranos commented Jun 9, 2017

No description provided.

@ouranos ouranos added this to the v3.3 milestone Jun 9, 2017
@ouranos ouranos requested a review from x4d3 June 9, 2017 07:00
@ouranos ouranos force-pushed the feature/ability-admin-case branch from 963cfa3 to 748027c Compare June 9, 2017 07:13
@@ -141,7 +141,7 @@ def impac_abilities(user)

# Abilities for admin user
def admin_abilities(user)
if user.admin_role == 'admin'
if user.admin_role.try(:downcase) == 'admin'
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer 'admin'.casecmp(user.admin_role)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah didn't know this one.

Interestingly, it seems better to use downcase + == in ruby 2.4:

In anycase, the role is not unicode so it should be safe to use 'admin'.casecmp(user.admin_role).zero?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, let me know if good and I'll squash the commits once the specs are green

@ouranos ouranos self-assigned this Jun 13, 2017
@ouranos ouranos force-pushed the feature/ability-admin-case branch from 4aace74 to 8f2638f Compare June 14, 2017 00:42
@ouranos ouranos merged commit 7f7e00a into maestrano:master Jun 14, 2017
@ouranos ouranos deleted the feature/ability-admin-case branch June 14, 2017 00:58
aluqueGH pushed a commit to aluqueGH/mno-enterprise that referenced this pull request Jul 10, 2018
Remove time from subscriptions start and end dates
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.

2 participants