672 roles are no longer in a hierarchy #681
Conversation
app/models/user.rb
Outdated
| @@ -38,7 +38,7 @@ class User < ActiveRecord::Base | |||
| def role?(base_role) | |||
| return false unless role | |||
|
|
|||
| ROLES.index(base_role) <= ROLES.index(role.to_sym) | |||
| ROLES.index(base_role) == ROLES.index(role.to_sym) | |||
There was a problem hiding this comment.
This looks like it could be simplified to base_role.to_sym == role.to_sym. The ROLES variable could either be completely removed or instead used to verify that base_role is valid [PSEUDOCODE] unless base_role is part of ROLES throw some error.
There was a problem hiding this comment.
How do we want to deal with invalid roles? It is a getter function, so we cannot get into an invalid state with an invalid role given. With an invalid role it will always return false (which is correct), so we dont actually have a "real" error.
However, throwing an error might make it easier to find bugs
There was a problem hiding this comment.
I like the explicit (error) version I think. Shouldn't happen too much and the log would be waaaay more helpful then.
aloeser
changed the title
Conflicts: app/models/ability.rb
…op-portal into 672_refactor_role_system
app/models/ability.rb
Outdated
| @@ -4,7 +4,7 @@ class Ability | |||
| def initialize(user) | |||
| # Define abilities for the passed in user here. For example: | |||
| # | |||
| # user ||= User.new # guest user (not logged in) | |||
| # user ||= User.new # guest user [not logged in] | |||
There was a problem hiding this comment.
A bit eager with the brace removal :-D
app/models/ability.rb
Outdated
|
|
||
| # Organizers can update user roles of pupil, coach and organizer, but cannot manage admins and cannot update a role to admin | ||
| can :manage, User, role: %w(pupil coach organizer) | ||
| can :manage, User, role: %w[pupil coach organizer] |
…op-portal into 672_refactor_role_system
all roles are separate, abilities are not inherited between roles
the user has exactly one role