-
Notifications
You must be signed in to change notification settings - Fork 35
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
Introduce a UserAccess model #1242
Conversation
@@ -0,0 +1,82 @@ | |||
class UserAccess < Struct.new(:user) | |||
class NotAuthorizedError < StandardError; end |
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 will come in handy later.
|
||
def resources_for(resource_model, action) | ||
return resource_model.all if user.power_user? | ||
return resource_model.none unless ACTION_TO_LEVEL.fetch(action).include?(user.access_level.to_sym) |
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 clause has been moved up, it used to be like this before. It makes more sense this way since we're just pulling accesses off a concrete User
.
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.
Makes sense to me. Shipit
…le-server into permissions-ui * 'permissions-ui' of https://github.com/simpledotorg/simple-server: Remove unused packages Introduce a UserAccess model (#1242) Various reports fixes (#1240)
Story card: https://app.clubhouse.io/simpledotorg/story/128/support-for-adding-and-removing-permissions-to-an-access-level
Because
The User class is lifting a lot of access-related weight, move it out to a more dedicated place.
I first pulled it out into a concern, but it didn't deserve one.
Then I pulled it out into a module/mixin, but mixins are a very poor abstraction that don't mean anything, esp not a great place to put concrete domain logic.
So I eventually pulled up everything from
User
andAccess
up into a concrete class that defines theAccess
behavior betweenUser
andAccess
and just use the Delegator pattern to forward methods fromUser
toUserAccess
for convenience.This also makes the behavior easier to test and doesn't spill over the specs in 2 places.
This addresses
UserAccess
User
toUserAccess
Access
andUser
permissions-related specs toUserAccess