Skip to content

Commit

Permalink
Get settings from defaults everywhere, update tests and perms
Browse files Browse the repository at this point in the history
  • Loading branch information
shacker committed Apr 12, 2019
1 parent 7a4984d commit 1cd9700
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 15 deletions.
4 changes: 2 additions & 2 deletions todo/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def todo_setup(django_user_model):

g1 = Group.objects.create(name="Workgroup One")
u1 = django_user_model.objects.create_user(
username="u1", password="password", email="[email protected]"
username="u1", password="password", email="[email protected]", is_staff=True
)
u1.groups.add(g1)
tlist1 = TaskList.objects.create(group=g1, name="Zip", slug="zip")
Expand All @@ -21,7 +21,7 @@ def todo_setup(django_user_model):

g2 = Group.objects.create(name="Workgroup Two")
u2 = django_user_model.objects.create_user(
username="u2", password="password", email="[email protected]"
username="u2", password="password", email="[email protected]", is_staff=True
)
u2.groups.add(g2)
tlist2 = TaskList.objects.create(group=g2, name="Zap", slug="zap")
Expand Down
12 changes: 9 additions & 3 deletions todo/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,17 @@ def test_setting_TODO_STAFF_ONLY_False(todo_setup, client, settings):
assert response.status_code == 200


def test_setting_TODO_STAFF_ONLY_True(todo_setup, client, settings):
# We use Django's user_passes_test to call `staff_check` utility function on all views.
# Just testing one view here; if it works, it works for all of them.
def test_setting_TODO_STAFF_ONLY_True(todo_setup, client, settings, django_user_model):
# We use Django's user_passes_test to call `staff_check` utility function on some views.
# Just testing one view here...
settings.TODO_STAFF_ONLY = True
url = reverse("todo:lists")

# Remove staff privileges from user u2; they should not be able to access
u2 = django_user_model.objects.get(username="u2")
u2.is_staff = False
u2.save()

client.login(username="u2", password="password")
response = client.get(url)
assert response.status_code == 302 # Redirected to login view
5 changes: 3 additions & 2 deletions todo/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from django.core import mail
from django.template.loader import render_to_string

from todo.defaults import defaults
from todo.models import Attachment, Comment, Task

log = logging.getLogger(__name__)
Expand All @@ -19,15 +20,15 @@ def staff_check(user):
https://github.com/shacker/django-todo/issues/50
"""

if hasattr(settings, "TODO_STAFF_ONLY") and settings.TODO_STAFF_ONLY:
if defaults('TODO_STAFF_ONLY'):
return user.is_staff
else:
# If unset or False, allow all logged in users
return True


def user_can_read_task(task, user):
return task.task_list.group in user.groups.all() or user.is_staff
return task.task_list.group in user.groups.all() or user.is_superuser


def todo_get_backend(task):
Expand Down
6 changes: 4 additions & 2 deletions todo/views/external_add.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from django.shortcuts import redirect, render
from django.template.loader import render_to_string

from todo.defaults import defaults
from todo.forms import AddExternalTaskForm
from todo.models import TaskList
from todo.utils import staff_check
Expand All @@ -24,6 +25,7 @@ def external_add(request) -> HttpResponse:
"""

if not settings.TODO_DEFAULT_LIST_SLUG:
# We do NOT provide a default in defaults
raise RuntimeError(
"This feature requires TODO_DEFAULT_LIST_SLUG: in settings. See documentation."
)
Expand All @@ -41,7 +43,7 @@ def external_add(request) -> HttpResponse:
task = form.save(commit=False)
task.task_list = TaskList.objects.get(slug=settings.TODO_DEFAULT_LIST_SLUG)
task.created_by = request.user
if settings.TODO_DEFAULT_ASSIGNEE:
if defaults('TODO_DEFAULT_ASSIGNEE'):
task.assigned_to = User.objects.get(username=settings.TODO_DEFAULT_ASSIGNEE)
task.save()

Expand Down Expand Up @@ -69,7 +71,7 @@ def external_add(request) -> HttpResponse:
messages.success(
request, "Your trouble ticket has been submitted. We'll get back to you soon."
)
return redirect(settings.TODO_PUBLIC_SUBMIT_REDIRECT)
return redirect(defaults("TODO_PUBLIC_SUBMIT_REDIRECT"))

else:
form = AddExternalTaskForm(initial={"priority": 999})
Expand Down
2 changes: 1 addition & 1 deletion todo/views/list_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def list_detail(request, list_id=None, list_slug=None, view_completed=False) ->
else:
# Show a specific list, ensuring permissions.
task_list = get_object_or_404(TaskList, id=list_id)
if task_list.group not in request.user.groups.all() and not request.user.is_staff:
if task_list.group not in request.user.groups.all() and not request.user.is_superuser:
raise PermissionDenied
tasks = Task.objects.filter(task_list=task_list.id)

Expand Down
16 changes: 11 additions & 5 deletions todo/views/task_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from django.shortcuts import get_object_or_404, redirect, render
from django.urls import reverse

from todo.defaults import TODO_ALLOW_FILE_ATTACHMENTS, TODO_LIMIT_FILE_ATTACHMENTS
from todo.defaults import defaults
from todo.features import HAS_TASK_MERGE
from todo.forms import AddEditTaskForm
from todo.models import Attachment, Comment, Task
Expand Down Expand Up @@ -53,7 +53,7 @@ def task_detail(request, task_id: int) -> HttpResponse:
task = get_object_or_404(Task, pk=task_id)
comment_list = Comment.objects.filter(task=task_id).order_by("-date")

# Ensure user has permission to view task. Admins can view all tasks.
# Ensure user has permission to view task. Superusers can view all tasks.
# Get the group this task belongs to, and check whether current user is a member of that group.
if not user_can_read_task(task, request.user):
raise PermissionDenied
Expand Down Expand Up @@ -120,15 +120,21 @@ class MergeForm(forms.Form):
# Handle uploaded files
if request.FILES.get("attachment_file_input"):
file = request.FILES.get("attachment_file_input")

if file.size > defaults('TODO_MAXIMUM_ATTACHMENT_SIZE'):
messages.error(request, f"File exceeds maximum attachment size.")
return redirect("todo:task_detail", task_id=task.id)

name, extension = os.path.splitext(file.name)

if extension not in TODO_LIMIT_FILE_ATTACHMENTS:
if extension not in defaults('TODO_LIMIT_FILE_ATTACHMENTS'):
messages.error(request, f"This site does not allow upload of {extension} files.")
return redirect("todo:task_detail", task_id=task.id)

Attachment.objects.create(
task=task, added_by=request.user, timestamp=datetime.datetime.now(), file=file
)
messages.success(request, f"File attached successfully")
return redirect("todo:task_detail", task_id=task.id)

context = {
Expand All @@ -137,8 +143,8 @@ class MergeForm(forms.Form):
"form": form,
"merge_form": merge_form,
"thedate": thedate,
"comment_classes": getattr(settings, "TODO_COMMENT_CLASSES", []),
"attachments_enabled": TODO_ALLOW_FILE_ATTACHMENTS,
"comment_classes": defaults("TODO_COMMENT_CLASSES"),
"attachments_enabled": defaults('TODO_ALLOW_FILE_ATTACHMENTS'),
}

return render(request, "todo/task_detail.html", context)

0 comments on commit 1cd9700

Please sign in to comment.