Feat: helper function checks if user is staff or superuser #2653
Conversation
add more tests for helper funcs
github-actions
bot
added
tests
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||
benefits/core/admin/users.py
Outdated
| @@ -54,6 +54,10 @@ def is_staff_member(user): | |||
| return staff_group.user_set.contains(user) | |||
|
|
|||
|
|
|||
| def is_staff_or_superuser(user): | |||
There was a problem hiding this comment.
I suggest we name this is_staff_member_or_superuser so we don't start getting confused between Django's is_staff vs. our concept of being "staff" -- the former simply means the user can log in to the admin, and the latter means that the user is in the "Cal-ITP" group.
I know it makes the name a little longer, but this distinction is important.
There was a problem hiding this comment.
I totally agree with @angela-tran, good catch!
There was a problem hiding this comment.
This method would be a great place to have some comments to explain the concept @angela-tran pointed out above. Could be full-on docstrings format or just comments https://realpython.com/documenting-python-code/#docstring-types
There was a problem hiding this comment.
Makes complete sense! Renamed it in 548ac86 as a new commit to make it easier to review the change.
There was a problem hiding this comment.
Thanks for the suggestion @machikoyasuda! Added some docstrings in 3eb9599
is_staff_member_or_superuser for added clarity between Django's staff (users that can log in to the admin) and Benefits' staff (users that are in the Cal-ITP group).
between Django's 'staff' and Benefits' 'staff' group
Part of #2201
Following our approach to breaking up Refactor: Admin permissions for Cal-ITP Staff into smaller PRs, this PR adds the
is_staff_or_superuserhelper function to check if user is staff or superuser and adds more tests for the helper functions inbenefits/core/admin/users.py.