From 9098e3f0d48a750933d8403eb39174c34205df94 Mon Sep 17 00:00:00 2001 From: Scot Hacker Date: Sun, 8 Apr 2018 00:49:01 -0700 Subject: [PATCH] Refactor Done and Delete actions in list and detail views --- .../templates/todo/include/toggle_delete.html | 2 +- todo/templates/todo/list_detail.html | 95 ++++++++----------- todo/templates/todo/task_detail.html | 7 +- todo/tests/test_utils.py | 38 +------- todo/urls.py | 10 ++ todo/utils.py | 35 +------ todo/views.py | 62 +++++++++--- 7 files changed, 108 insertions(+), 141 deletions(-) diff --git a/todo/templates/todo/include/toggle_delete.html b/todo/templates/todo/include/toggle_delete.html index f28def2..80248cb 100644 --- a/todo/templates/todo/include/toggle_delete.html +++ b/todo/templates/todo/include/toggle_delete.html @@ -4,7 +4,7 @@ View incomplete tasks {% else %} - View completed tasks + View completed tasks {% endif %} diff --git a/todo/templates/todo/list_detail.html b/todo/templates/todo/list_detail.html index 4a8998e..cb71f5a 100644 --- a/todo/templates/todo/list_detail.html +++ b/todo/templates/todo/list_detail.html @@ -22,63 +22,50 @@

In workgroup "{{ task_list.group }}" - drag rows to set priorities.

{% endif %} -
- {% csrf_token %} - - - - - - - - - - - +
DoneTaskCreatedDue onOwnerAssignedNoteCommDel
+ + + + + + + + + + {% for task in tasks %} + + + + + + + + {% endfor %} +
TaskCreatedDue onOwnerAssignedMark
+ {{ task.title|truncatewords:10 }} + + {{ task.created_date|date:"m/d/Y" }} + + + {{ task.due_date|date:"m/d/Y" }} + + + {{ task.created_by }} + + {% if task.assigned_to %}{{ task.assigned_to }}{% else %}Anyone{% endif %} + + + {% if view_completed %} + Not Done + {% else %} + Done + {% endif %} + +
- {% for task in tasks %} - - - - - - {{ task.title|truncatewords:10 }} - - - {{ task.created_date|date:"m/d/Y" }} - - - - {{ task.due_date|date:"m/d/Y" }} - - - - {{ task.created_by }} - - - {% if task.assigned_to %}{{ task.assigned_to }}{% else %}Anyone{% endif %} - - - {% if task.note %}≈{% endif %} - - - {% if task.comment_set.all.count > 0 %}{{ task.comment_set.all.count }}{% endif %} - - - - - - {% endfor %} - + {% include 'todo/include/toggle_delete.html' %} - - - {% include 'todo/include/toggle_delete.html' %} - -
{% else %}

No tasks on this list yet (add one!)

{% include 'todo/include/toggle_delete.html' %} diff --git a/todo/templates/todo/task_detail.html b/todo/templates/todo/task_detail.html index 689f9b6..067d7ae 100644 --- a/todo/templates/todo/task_detail.html +++ b/todo/templates/todo/task_detail.html @@ -20,8 +20,11 @@ - + + {% if task.completed %} Mark Not Done {% else %} Mark Done {% endif %} + + + Delete
  • diff --git a/todo/tests/test_utils.py b/todo/tests/test_utils.py index a321d93..aa505eb 100644 --- a/todo/tests/test_utils.py +++ b/todo/tests/test_utils.py @@ -3,7 +3,7 @@ import pytest from django.core import mail from todo.models import Task, Comment -from todo.utils import toggle_done, toggle_deleted, send_notify_mail, send_email_to_thread_participants +from todo.utils import send_notify_mail, send_email_to_thread_participants @pytest.fixture() @@ -12,42 +12,6 @@ def email_backend_setup(settings): settings.EMAIL_BACKEND = 'django.core.mail.backends.locmem.EmailBackend' -def test_toggle_done(todo_setup): - """Utility function takes an array of POSTed IDs and changes their `completed` status. - """ - u1_tasks = Task.objects.filter(created_by__username="u1") - completed = u1_tasks.filter(completed=True) - incomplete = u1_tasks.filter(completed=False) - - # Expected counts in fixture data - assert u1_tasks.count() == 3 - assert incomplete.count() == 2 - assert completed.count() == 1 - - # Mark incomplete tasks completed and check again - toggle_done([t.id for t in incomplete]) - now_completed = u1_tasks.filter(created_by__username="u1", completed=True) - assert now_completed.count() == 3 - - # Mark all incomplete and check again - toggle_done([t.id for t in now_completed]) - now_incomplete = u1_tasks.filter(created_by__username="u1", completed=False) - assert now_incomplete.count() == 3 - - -def test_toggle_deleted(todo_setup): - """Unlike toggle_done, delete means delete, so it's not really a toggle. - """ - u1_tasks = Task.objects.filter(created_by__username="u1") - assert u1_tasks.count() == 3 - t1 = u1_tasks.first() - t2 = u1_tasks.last() - - toggle_deleted([t1.id, t2.id, ]) - u1_tasks = Task.objects.filter(created_by__username="u1") - assert u1_tasks.count() == 1 - - def test_send_notify_mail_not_me(todo_setup, django_user_model, email_backend_setup): """Assign a task to someone else, mail should be sent. TODO: Future tests could check for email contents. diff --git a/todo/urls.py b/todo/urls.py index e124077..2c97a8d 100644 --- a/todo/urls.py +++ b/todo/urls.py @@ -56,6 +56,16 @@ urlpatterns = [ views.task_detail, name='task_detail'), + path( + 'toggle_done//', + views.toggle_done, + name='task_toggle_done'), + + path( + 'delete//', + views.delete_task, + name='delete_task'), + path( 'search/', views.search, diff --git a/todo/utils.py b/todo/utils.py index cce813b..4a68b1f 100644 --- a/todo/utils.py +++ b/todo/utils.py @@ -1,41 +1,8 @@ -import datetime - from django.contrib.sites.models import Site from django.core.mail import send_mail from django.template.loader import render_to_string -from todo.models import Task, Comment - - -def toggle_done(task_ids): - """Check for tasks in the mark_done POST array. If present, change status to complete. - Takes a list of task IDs. Returns list of status change strings. - """ - - _ret = [] - for task_id in task_ids: - i = Task.objects.get(id=task_id) - old_state = "completed" if i.completed else "incomplete" - i.completed = not i.completed # Invert the done state, either way - new_state = "completed" if i.completed else "incomplete" - i.completed_date = datetime.datetime.now() - i.save() - _ret.append("Task \"{i}\" changed from {o} to {n}.".format(i=i.title, o=old_state, n=new_state)) - - return _ret - - -def toggle_deleted(deleted_task_ids): - """Delete selected tasks. Returns list of status change strings. - """ - - _ret = [] - for task_id in deleted_task_ids: - i = Task.objects.get(id=task_id) - _ret.append("Task \"{i}\" deleted.".format(i=i.title)) - i.delete() - - return _ret +from todo.models import Comment def send_notify_mail(new_task): diff --git a/todo/views.py b/todo/views.py index 70e4d63..b2efbd8 100644 --- a/todo/views.py +++ b/todo/views.py @@ -12,6 +12,7 @@ from django.db.models import Q from django.http import HttpResponse from django.shortcuts import get_object_or_404, render, redirect from django.template.loader import render_to_string +from django.urls import reverse from django.utils import timezone from django.utils.text import slugify from django.views.decorators.csrf import csrf_exempt @@ -19,8 +20,6 @@ from django.views.decorators.csrf import csrf_exempt from todo.forms import AddTaskListForm, AddEditTaskForm, AddExternalTaskForm, SearchForm from todo.models import Task, TaskList, Comment from todo.utils import ( - toggle_done, - toggle_deleted, send_notify_mail, send_email_to_thread_participants, ) @@ -137,16 +136,6 @@ def list_detail(request, list_id=None, list_slug=None, view_completed=False): else: tasks = tasks.filter(completed=False) - if request.POST: - # Process completed and deleted tasks on each POST - results_changed = toggle_done(request.POST.getlist('toggle_done_tasks')) - for res in results_changed: - messages.success(request, res) - - results_changed = toggle_deleted(request.POST.getlist('toggle_deleted_tasks')) - for res in results_changed: - messages.success(request, res) - # ###################### # Add New Task Form # ###################### @@ -382,7 +371,6 @@ def external_add(request) -> HttpResponse: messages.warning(request, "Task saved but mail not sent. Contact your administrator.") messages.success(request, "Your trouble ticket has been submitted. We'll get back to you soon.") - return redirect(settings.TODO_PUBLIC_SUBMIT_REDIRECT) else: @@ -393,3 +381,51 @@ def external_add(request) -> HttpResponse: } return render(request, 'todo/add_task_external.html', context) + + +@login_required +def toggle_done(request, task_id: int) -> HttpResponse: + """Toggle the completed status of a task from done to undone, or vice versa. + Redirect to the list from which the task came. + """ + + task = get_object_or_404(Task, pk=task_id) + + # Permissions + if not ( + (task.created_by == request.user) or + (task.assigned_to == request.user) or + (task.task_list.group in request.user.groups.all()) + ): + raise PermissionDenied + + tlist = task.task_list + task.completed = not task.completed + task.save() + + messages.success(request, "Task status changed for '{}'".format(task.title)) + return redirect(reverse('todo:list_detail', kwargs={"list_id": tlist.id, "list_slug": tlist.slug})) + + + +@login_required +def delete_task(request, task_id: int) -> HttpResponse: + """Delete specified task. + Redirect to the list from which the task came. + """ + + task = get_object_or_404(Task, pk=task_id) + + # Permissions + if not ( + (task.created_by == request.user) or + (task.assigned_to == request.user) or + (task.task_list.group in request.user.groups.all()) + ): + raise PermissionDenied + + tlist = task.task_list + task.delete() + + messages.success(request, "Task '{}' has been deleted".format(task.title)) + return redirect(reverse('todo:list_detail', kwargs={"list_id": tlist.id, "list_slug": tlist.slug}))