Add dashboard page to grant and revoke permissions. #14615
Conversation
ashercodeorg
force-pushed
the
revokePrivileges
branch
from
f900bd4
to
ca1911b
Compare
|
|
||
| = submit_tag 'Revoke All Permissions' | ||
|
|
||
| -# TODO(asher). |
There was a problem hiding this comment.
Yep. This is intentionally being left, as I'm not sure whether this functionality is needed.
|
|
||
| # Grants the indicated permission to the indicated user. Expects params[:email] and | ||
| # params[:permission] to be populated. | ||
| def grant_permission |
There was a problem hiding this comment.
It would be worth extracting the core functionality in this action to a method on the User model.
There was a problem hiding this comment.
Most of that functionality is already there I'd say, via user.permission = permission. And the User model is bloated enough that I don't think see a gain from adding logic unlikely to be used elsewhere (the logic surrounding permission being admin or a UserPermission).
| redirect_to :permissions_form | ||
| end | ||
|
|
||
| def revoke_all_permissions |
There was a problem hiding this comment.
Same as above — prefer User.revoke_all_permissions_for_email params[:email]. This would let us put the tests on the model instead of the controller.
There was a problem hiding this comment.
Here I totally agree with you. Done.
There was a problem hiding this comment.
Note that, though I've added tests to the model, I'm being lazy and testing for effect in controller (not updating tests) rather than confirming user.revoke_all_privileges is being called.
ashercodeorg
force-pushed
the
revokePrivileges
branch
from
73137cd
to
6a6bd62
Compare
|
PTAL. |
| hashed_email = User.hash_email params[:email] | ||
| # Though in theory a hashed email specifies a unique account, in practice it may not. As this is | ||
| # security related, we therefore iterate rather than use find_by_hashed_email. | ||
| User.with_deleted.where(hashed_email: hashed_email).each(&:revoke_all_permissions) |
After confirming this meets our support needs, the comparable pegasus page for granting permissions (code.org/private/privileges) will be removed (via #14636).