Convert task_delete and task_done views from GET to POST

README.md

@@ -168,6 +168,8 @@ The previous `tox` system was removed with the v2 release, since we no longer ai
 
 # Version History
 
+**2.2.1** Convert task delete and toggle_done views to POST only
+
 **2.2.0** Re-instate enforcement of TODO_STAFF_ONLY setting
 
 **2.1.1** Correct Python version requirement in documentation to Python 3.6

todo/__init__.py

@@ -1,7 +1,7 @@
 """
 A multi-user, multi-group task management and assignment system for Django.
 """
-__version__ = '2.2.0'
+__version__ = '2.2.1'
 
 __author__ = 'Scot Hacker'
 __email__ = 'shacker@birdhouse.org'

todo/templates/todo/list_detail.html

@@ -52,14 +52,17 @@
               {% if task.assigned_to %}{{ task.assigned_to }}{% else %}Anyone{% endif %}
             </td>
             <td>
-                <a href="{% url "todo:task_toggle_done" task.id %}" class="btn btn-info btn-sm">
+              <form method="post" action="{% url "todo:task_toggle_done" task.id %}" role="form">
+                {% csrf_token %}
+                <button class="btn btn-info btn-sm" type="submit" name="toggle_done">
                   {% if view_completed %}
                     Not Done
                   {% else %}
                     Done
                   {% endif %}
-                </a>
-              </td>
+                </button>
+              </form>
+            </td>
           </tr>
         {% endfor %}
       </table>

todo/templates/todo/task_detail.html

@@ -12,21 +12,40 @@
     </div>
 
     <div class="col-sm-4">
-      <ul class="list-group">
-        <li class="list-group-item">
-          <form action="" method="post">
-            {% csrf_token %}
+      <div class="mb-2">
 
-            <button class="btn btn-sm btn-primary" id="EditTaskButton" type="button"
-            data-toggle="collapse" data-target="#AddEditTask">Edit Task</button>
+        <button
+          class="btn btn-sm btn-primary"
+          id="EditTaskButton"
+          type="button"
+          data-toggle="collapse"
+          data-target="#AddEditTask"
+        >
+            Edit Task
+        </button>
 
-            <a href="{% url "todo:task_toggle_done" task.id %}" class="btn btn-info btn-sm">
+        <form method="post" action="{% url "todo:task_toggle_done" task.id %}" role="form" style="display:inline;">
+          {% csrf_token %}
+          <div style="display:inline;">
+            <button class="btn btn-info btn-sm" type="submit" name="toggle_done">
               {% if task.completed %} Mark Not Done {% else %} Mark Done {% endif %}
-            </a>
+            </button>
+          </div>
+        </form>
+
+        <form method="post" action="{% url "todo:delete_task" task.id %}" role="form" style="display:inline;">
+          {% csrf_token %}
+          <div style="display:inline;">
+            <button class="btn btn-danger btn-sm" type="submit" name="submit_delete">
+              Delete
+            </button>
+          </div>
+        </form>
+
+      </div>
+
+      <ul class="list-group">
 
-            <a href="{% url "todo:delete_task" task.id %}" class="btn btn-danger btn-sm">Delete</a>
-          </form>
-        </li>
         <li class="list-group-item">
           <strong>Assigned to:</strong>
           {% if task.assigned_to %} {{ task.assigned_to.get_full_name }} {% else %} Anyone {% endif %}
@@ -37,7 +56,7 @@
         <li class="list-group-item">
           <strong>Due date:</strong> {{ task.due_date }}
         </li>
-        
+
         {% if task.completed %}
           <li class="list-group-item">
             <strong>Completed on:</strong> {{ task.completed_date}}
@@ -47,7 +66,7 @@
             <strong>Completed:</strong> {{ task.completed|yesno:"Yes,No" }}
           </li>
         {% endif %}
-          
+
         <li class="list-group-item">
           <strong>In list:</strong>
           <a href="{% url 'todo:list_detail' task.task_list.id task.task_list.slug %}">

todo/tests/test_views.py

@@ -1,5 +1,4 @@
 import bleach
-import json
 import pytest
 
 from django.contrib.auth import get_user_model
@@ -14,8 +13,6 @@ Next permissions tests - some views should respond for staffers only.
 After that, view contents and behaviors.
 """
 
-# ### SMOKETESTS ###
-
 
 @pytest.mark.django_db
 def test_todo_setup(todo_setup):
@@ -85,6 +82,31 @@ def test_view_task_detail(todo_setup, admin_client):
     assert response.status_code == 200
 
 
+def test_del_task(todo_setup, admin_user, client):
+    task = Task.objects.first()
+    url = reverse("todo:delete_task", kwargs={"task_id": task.id})
+    # View accepts POST, not GET
+    client.login(username="admin", password="password")
+    response = client.get(url)
+    assert response.status_code == 403
+    response = client.post(url)
+    assert not Task.objects.filter(id=task.id).exists()
+
+
+def test_task_toggle_done(todo_setup, admin_user, client):
+    task = Task.objects.first()
+    assert not task.completed
+    url = reverse("todo:task_toggle_done", kwargs={"task_id": task.id})
+    # View accepts POST, not GET
+    client.login(username="admin", password="password")
+    response = client.get(url)
+    assert response.status_code == 403
+
+    client.post(url)
+    task.refresh_from_db()
+    assert task.completed
+
+
 def test_view_search(todo_setup, admin_client):
     url = reverse("todo:search")
     response = admin_client.get(url)

todo/views/delete_task.py

@@ -16,20 +16,27 @@ def delete_task(request, task_id: int) -> HttpResponse:
     Redirect to the list from which the task came.
     """
 
-    task = get_object_or_404(Task, pk=task_id)
+    if request.method == "POST":
+        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())
-    ):
+        redir_url = reverse(
+            "todo:list_detail",
+            kwargs={"list_id": task.task_list.id, "list_slug": task.task_list.slug},
+        )
+
+        # Permissions
+        if not (
+            (task.created_by == request.user)
+            or (request.user.is_superuser)
+            or (task.assigned_to == request.user)
+            or (task.task_list.group in request.user.groups.all())
+        ):
+            raise PermissionDenied
+
+        task.delete()
+
+        messages.success(request, "Task '{}' has been deleted".format(task.title))
+        return redirect(redir_url)
+
+    else:
         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})
-    )

todo/views/list_lists.py

@@ -20,7 +20,7 @@ def list_lists(request) -> HttpResponse:
     searchform = SearchForm(auto_id=False)
 
     # Make sure user belongs to at least one group.
-    if request.user.groups.all().count() == 0:
+    if not request.user.groups.all().exists():
         messages.warning(
             request,
             "You do not yet belong to any groups. Ask your administrator to add you to one.",

todo/views/toggle_done.py

@@ -17,23 +17,27 @@ def toggle_done(request, task_id: int) -> HttpResponse:
     Redirect to the list from which the task came.
     """
 
-    task = get_object_or_404(Task, pk=task_id)
+    if request.method == "POST":
+        task = get_object_or_404(Task, pk=task_id)
 
-    # Permissions
-    if not (
-        (request.user.is_superuser)
-        or (task.created_by == request.user)
-        or (task.assigned_to == request.user)
-        or (task.task_list.group in request.user.groups.all())
-    ):
-        raise PermissionDenied
-
-    toggle_task_completed(task.id)
-    messages.success(request, "Task status changed for '{}'".format(task.title))
-
-    return redirect(
-        reverse(
+        redir_url = reverse(
             "todo:list_detail",
             kwargs={"list_id": task.task_list.id, "list_slug": task.task_list.slug},
         )
-    )
+
+        # Permissions
+        if not (
+            (task.created_by == request.user)
+            or (request.user.is_superuser)
+            or (task.assigned_to == request.user)
+            or (task.task_list.group in request.user.groups.all())
+        ):
+            raise PermissionDenied
+
+        toggle_task_completed(task.id)
+        messages.success(request, "Task status changed for '{}'".format(task.title))
+
+        return redirect(redir_url)
+
+    else:
+        raise PermissionDenied