Refactor Done and Delete actions in list and detail views

todo/templates/todo/include/toggle_delete.html

@@ -4,7 +4,7 @@
       <a href="{% url 'todo:list_detail' list_id list_slug %}" class="btn btn-sm btn-warning">View incomplete tasks</a>
 
     {% else %}
-      <a href="{% url 'todo:list_detail_completed' list_id list_slug %}" class="btn btn-sm btn-info">View completed tasks</a>
+      <a href="{% url 'todo:list_detail_completed' list_id list_slug %}" class="btn btn-sm btn-warning">View completed tasks</a>
 
     {% endif %}
 

todo/templates/todo/list_detail.html

@@ -22,28 +22,18 @@
       <p><small><i>In workgroup "{{ task_list.group }}" - drag rows to set priorities.</i></small></p>
     {% endif %}
 
-    <form action="" name="show_tasks" method="post">
-        {% csrf_token %}
       <table class="table" id="tasktable">
         <tr class="nodrop">
-            <th>Done</th>
           <th>Task</th>
           <th>Created</th>
           <th>Due on</th>
           <th>Owner</th>
           <th>Assigned</th>
-            <th>Note</th>
+          <th>Mark</th>
-            <th>Comm</th>
-            <th>Del</th>
         </tr>
 
         {% for task in tasks %}
           <tr id="{{ task.id }}">
-              <td>
-                  <input type="checkbox" name="toggle_done_tasks"
-                  value="{{ task.id }}" id="{{ task.id }}"
-                  {% if task.completed %}checked{% endif %}>
-              </td>
             <td>
                 <a href="{% url 'todo:task_detail' task.id %}">{{ task.title|truncatewords:10 }}</a>
             </td>
@@ -62,23 +52,20 @@
               {% if task.assigned_to %}{{ task.assigned_to }}{% else %}Anyone{% endif %}
             </td>
             <td>
-                 {% if task.note %}&asymp;{% endif %}
+                <a href="{% url "todo:task_toggle_done" task.id %}" class="btn btn-info btn-sm">
-              </td>
+                  {% if view_completed %}
-              <td>
+                    Not Done
-                {% if task.comment_set.all.count > 0 %}{{ task.comment_set.all.count }}{% endif %}
+                  {% else %}
-              </td>
+                    Done
-              <td>
+                  {% endif %}
-                <input type="checkbox" name="toggle_deleted_tasks" value="{{ task.id }}" id="{{ task.id }}">
+                </a>
               </td>
           </tr>
         {% endfor %}
       </table>
 
-        <input type="submit" name="process_tasks" value="Process selection"  class="btn btn-sm btn-success">
-
       {% include 'todo/include/toggle_delete.html' %}
 
-    </form>
   {% else %}
     <h4>No tasks on this list yet (add one!)</h4>
     {% include 'todo/include/toggle_delete.html' %}

todo/templates/todo/task_detail.html

@@ -20,8 +20,11 @@
             <button class="btn btn-sm btn-primary" id="EditTaskButton" type="button"
             data-toggle="collapse" data-target="#AddEditTask">Edit Task</button>
 
-            <input class="btn btn-sm btn-info" id="CompleteTaskButton" type="submit" name="toggle_done"
+            <a href="{% url "todo:task_toggle_done" task.id %}" class="btn btn-info btn-sm">
-              value="{% if task.completed %}Mark Incomplete{% else %}Mark Done{% endif %}">
+              {% if task.completed %} Mark Not Done {% else %} Mark Done {% endif %}
+            </a>
+
+            <a href="{% url "todo:delete_task" task.id %}" class="btn btn-danger btn-sm">Delete</a>
           </form>
         </li>
         <li class="list-group-item">

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.

todo/urls.py

@@ -56,6 +56,16 @@ urlpatterns = [
         views.task_detail,
         name='task_detail'),
 
+    path(
+        'toggle_done/<int:task_id>/',
+        views.toggle_done,
+        name='task_toggle_done'),
+
+    path(
+        'delete/<int:task_id>/',
+        views.delete_task,
+        name='delete_task'),
+
     path(
         'search/',
         views.search,

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
+from todo.models import 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
 
 
 def send_notify_mail(new_task):

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}))