-
Notifications
You must be signed in to change notification settings - Fork 22
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
add admin permission, move from is_staff to permission system #126
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
from django.db import migrations | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
('home', '0010_initial_data'), | ||
] | ||
|
||
operations = [ | ||
migrations.AlterModelOptions( | ||
name='user', | ||
options={'permissions': [('admin', 'Can take any planetterp admin actions')]}, | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,7 +45,7 @@ def grade_to_element(self, grade): | |
|
||
def render(self, value: dict): | ||
review = value.pop("review") | ||
is_staff = value.pop("is_staff") | ||
is_planetterp_admin = value.pop("is_planetterp_admin") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What would be involved in having this permission have the user's EDIT: I realize that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as you said, the issue here is the admin panel. is_staff is also a very stiff permission and would not allow us to split things into multiple permissions in the future if we wanted. |
||
|
||
column_html = "" | ||
if review.professor.slug: | ||
|
@@ -80,7 +80,7 @@ def render(self, value: dict): | |
|
||
# wrap long usernames to avoid increasing the information column width | ||
column_html += '<span style="white-space: normal; word-break: break-all;">' | ||
if is_staff and review.user: | ||
if is_planetterp_admin and review.user: | ||
if review.anonymous: | ||
column_html += ''' | ||
<span class="noselect" data-toggle="tooltip" data-placement="right" title="This reviewer posted anonymously"> | ||
|
@@ -110,12 +110,12 @@ def render(self, value: dict): | |
if review.created_at.date() >= date(2020, 3, 10) and review.created_at.date() <= date(2021, 8, 30): | ||
column_html += ' <i class="fas fa-head-side-mask" data-toggle="tooltip" data-placement="right" title="This review was submitted while most classes were online during the COVID-19 pandemic. It may not be indicative of a regular semester."></i>' | ||
|
||
if not review.user or (review.anonymous and not is_staff): | ||
if not review.user or (review.anonymous and not is_planetterp_admin): | ||
username = "Anonymous" | ||
else: | ||
username = review.user.username | ||
|
||
if is_staff: | ||
if is_planetterp_admin: | ||
column_html += ''' | ||
<br/ > | ||
<span> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,7 @@ | |
from django.views import View | ||
from django.http import JsonResponse | ||
from django.template.context_processors import csrf | ||
from django.contrib.auth.mixins import UserPassesTestMixin | ||
from django.contrib.auth.mixins import PermissionRequiredMixin | ||
from django.urls import reverse | ||
from django.utils.safestring import mark_safe | ||
|
||
|
@@ -21,10 +21,8 @@ | |
from home.utils import send_email, _ttl_cache | ||
from planetterp import config | ||
|
||
class Admin(UserPassesTestMixin, View): | ||
|
||
def test_func(self): | ||
return self.request.user.is_staff | ||
class Admin(PermissionRequiredMixin, View): | ||
permission_required = "home.admin" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thoughts on making an enum to store the string value? I assume the string won't change, but I think it's more pleasant to deal with an enum than with a string. Also, this would give us more flexibility in naming/updating it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this doesn't work as well as you might hope because there's no easy way to use this enum value in templates. Currently django populates a global
I don't even know how this would work with a custom enum. In addition, if we did want to reference I'm not sure the added complexity is worthwhile here. |
||
|
||
def get(self, request): | ||
reviews = ( | ||
|
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.
"moderator" is more fitting here
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.
I've gone with
mod
, asmoderator
was quite long.