From 340439cbe8f585652cb9b00cc8ea1b1989abd09c Mon Sep 17 00:00:00 2001
From: Paul Pepper
Date: Thu, 29 Aug 2024 18:16:03 +0100
Subject: [PATCH 01/57] App initial commit.
---
settings/common.py | 1 +
workflow/__init__.py | 0
workflow/admin.py | 1 +
workflow/apps.py | 6 ++++++
workflow/migrations/__init__.py | 0
workflow/models.py | 1 +
workflow/tests.py | 1 +
workflow/views.py | 1 +
8 files changed, 11 insertions(+)
create mode 100644 workflow/__init__.py
create mode 100644 workflow/admin.py
create mode 100644 workflow/apps.py
create mode 100644 workflow/migrations/__init__.py
create mode 100644 workflow/models.py
create mode 100644 workflow/tests.py
create mode 100644 workflow/views.py
diff --git a/settings/common.py b/settings/common.py
index bbf3cdb60..6bdf41f7c 100644
--- a/settings/common.py
+++ b/settings/common.py
@@ -141,6 +141,7 @@
"exporter.apps.ExporterConfig",
"crispy_forms",
"crispy_forms_gds",
+ "workflow",
]
APPS_THAT_MUST_COME_LAST = ["django.forms"]
diff --git a/workflow/__init__.py b/workflow/__init__.py
new file mode 100644
index 000000000..e69de29bb
diff --git a/workflow/admin.py b/workflow/admin.py
new file mode 100644
index 000000000..846f6b406
--- /dev/null
+++ b/workflow/admin.py
@@ -0,0 +1 @@
+# Register your models here.
diff --git a/workflow/apps.py b/workflow/apps.py
new file mode 100644
index 000000000..bd2134053
--- /dev/null
+++ b/workflow/apps.py
@@ -0,0 +1,6 @@
+from django.apps import AppConfig
+
+
+class WorkflowConfig(AppConfig):
+ default_auto_field = "django.db.models.BigAutoField"
+ name = "workflow"
diff --git a/workflow/migrations/__init__.py b/workflow/migrations/__init__.py
new file mode 100644
index 000000000..e69de29bb
diff --git a/workflow/models.py b/workflow/models.py
new file mode 100644
index 000000000..6b2021999
--- /dev/null
+++ b/workflow/models.py
@@ -0,0 +1 @@
+# Create your models here.
diff --git a/workflow/tests.py b/workflow/tests.py
new file mode 100644
index 000000000..a39b155ac
--- /dev/null
+++ b/workflow/tests.py
@@ -0,0 +1 @@
+# Create your tests here.
diff --git a/workflow/views.py b/workflow/views.py
new file mode 100644
index 000000000..60f00ef0e
--- /dev/null
+++ b/workflow/views.py
@@ -0,0 +1 @@
+# Create your views here.
From 301644a7d6c516417756174e128dd4ed1b41f9af Mon Sep 17 00:00:00 2001
From: Dale Cannon <118175145+dalecannon@users.noreply.github.com>
Date: Thu, 19 Sep 2024 17:15:04 +0100
Subject: [PATCH 02/57] TP2000-1472 Revise task-related models (#1288)
* Rename UserAssignment model to TaskAssignee
* Add TaskCategory and TaskProgressState models
* Add parent_task field to Task model
* Add creator field to Task model
* Add TaskLog model
* Don't require that tasks have a workbasket
* Make progress_state a required field on Task model
* Order task category by name and display progress state by value
* Remove Task prefix from Category and ProgressState models
* Remove workflow app
---
.../UnassignUserForm.js | 6 +-
.../UnassignUserForm.test.js.snap | 8 +-
common/tests/factories.py | 42 +++--
common/tests/test_views.py | 6 +-
common/views/pages.py | 6 +-
conftest.py | 14 +-
package-lock.json | 39 ++---
package.json | 2 +-
settings/common.py | 1 -
...0003_rename_userassignment_taskassignee.py | 60 +++++++
...askprogressstate_task_category_and_more.py | 134 ++++++++++++++
tasks/migrations/0005_task_parent_task.py | 26 +++
tasks/migrations/0006_task_creator.py | 27 +++
tasks/migrations/0007_create_tasklog.py | 69 ++++++++
.../migrations/0008_alter_task_workbasket.py | 27 +++
tasks/models.py | 165 ++++++++++++++++--
tasks/tests/conftest.py | 38 ++--
tasks/tests/test_models.py | 158 +++++++++++++----
workbaskets/forms.py | 36 ++--
workbaskets/models.py | 15 +-
workbaskets/tests/test_forms.py | 20 +--
workbaskets/tests/test_models.py | 32 ++--
workbaskets/tests/test_views.py | 38 ++--
workbaskets/views/ui.py | 15 +-
workflow/__init__.py | 0
workflow/admin.py | 1 -
workflow/apps.py | 6 -
workflow/migrations/__init__.py | 0
workflow/models.py | 1 -
workflow/tests.py | 1 -
workflow/views.py | 1 -
31 files changed, 803 insertions(+), 191 deletions(-)
create mode 100644 tasks/migrations/0003_rename_userassignment_taskassignee.py
create mode 100644 tasks/migrations/0004_taskcategory_taskprogressstate_task_category_and_more.py
create mode 100644 tasks/migrations/0005_task_parent_task.py
create mode 100644 tasks/migrations/0006_task_creator.py
create mode 100644 tasks/migrations/0007_create_tasklog.py
create mode 100644 tasks/migrations/0008_alter_task_workbasket.py
delete mode 100644 workflow/__init__.py
delete mode 100644 workflow/admin.py
delete mode 100644 workflow/apps.py
delete mode 100644 workflow/migrations/__init__.py
delete mode 100644 workflow/models.py
delete mode 100644 workflow/tests.py
delete mode 100644 workflow/views.py
diff --git a/common/static/common/js/components/WorkbasketUserAssignment/UnassignUserForm.js b/common/static/common/js/components/WorkbasketUserAssignment/UnassignUserForm.js
index 7a9fae768..e2d5c2779 100644
--- a/common/static/common/js/components/WorkbasketUserAssignment/UnassignUserForm.js
+++ b/common/static/common/js/components/WorkbasketUserAssignment/UnassignUserForm.js
@@ -5,8 +5,8 @@ import accessibleAutocomplete from "accessible-autocomplete";
import PropTypes from "prop-types";
function UnassignUserForm({ users }) {
- const elementId = "assignments-select";
- const elementName = "assignments";
+ const elementId = "assignees-select";
+ const elementName = "assignees";
const label = "Unassign user";
const hint = "Select a user to unassign";
@@ -66,7 +66,7 @@ UnassignUserForm.propTypes = {
PropTypes.shape({
pk: PropTypes.number.isRequired,
name: PropTypes.string.isRequired,
- })
+ }),
),
};
diff --git a/common/static/common/js/components/WorkbasketUserAssignment/tests/__snapshots__/UnassignUserForm.test.js.snap b/common/static/common/js/components/WorkbasketUserAssignment/tests/__snapshots__/UnassignUserForm.test.js.snap
index 9427e9111..c25959a29 100644
--- a/common/static/common/js/components/WorkbasketUserAssignment/tests/__snapshots__/UnassignUserForm.test.js.snap
+++ b/common/static/common/js/components/WorkbasketUserAssignment/tests/__snapshots__/UnassignUserForm.test.js.snap
@@ -16,20 +16,20 @@ exports[`UnassignUserForm renders form 1`] = `
>
Select a user to unassign
+
+
+
+
+
+
+
+
+
+ Status
+
+
+ {{ object.progress_state }}
+
+
+ Change status
+
+
+
+
+
+ Created
+
+
+ {{ "{:%d %b %Y %H:%M}".format(object.created_at) }}
+
+
+
+
+
+ Created by
+
+
+ {{ object.creator.get_displayname() }}
+
+
+
+
+
+ {{ govukButton({
+ "text": "Delete task",
+ "href": url("workflow:task-ui-delete", kwargs={"pk": object.pk}),
+ "classes": "govuk-button--warning"
+ }) }}
+ {{ govukButton({
+ "text": "Find and view tasks",
+ "href": url("workflow:task-ui-list"),
+ "classes": "govuk-button--secondary"
+ }) }}
+
+{% endblock %}
diff --git a/tasks/jinja2/tasks/edit.jinja b/tasks/jinja2/tasks/edit.jinja
new file mode 100644
index 000000000..9ce73f211
--- /dev/null
+++ b/tasks/jinja2/tasks/edit.jinja
@@ -0,0 +1,19 @@
+{% extends "layouts/form.jinja" %}
+{% from "components/breadcrumbs.jinja" import breadcrumbs %}
+
+{% set page_title = "Edit task details" %}
+
+{% block breadcrumb %}
+ {{ breadcrumbs(request, [
+ {"text": "Find and view tasks", "href": url("workflow:task-ui-list")},
+ {"text": "Task: " ~ object.title, "href": url("workflow:task-ui-detail", kwargs={"pk": object.pk})},
+ {"text": page_title}
+ ])
+ }}
+{% endblock %}
+
+{% block form %}
+ {% call django_form() %}
+ {{ crispy(form) }}
+ {% endcall %}
+{% endblock %}
diff --git a/tasks/jinja2/tasks/list.jinja b/tasks/jinja2/tasks/list.jinja
new file mode 100644
index 000000000..fa120a392
--- /dev/null
+++ b/tasks/jinja2/tasks/list.jinja
@@ -0,0 +1,78 @@
+{% extends "layouts/layout.jinja" %}
+
+{% from "components/table/macro.njk" import govukTable %}
+{% from "components/create_sortable_anchor.jinja" import create_sortable_anchor %}
+
+{% set page_title = "Find and view tasks" %}
+
+{% set base_url = url("workflow:task-ui-list") %}
+
+{% block content %}
+ {{ page_title }}
+
+ Search for tasks.
+ Alternatively, create a new task.
+
+
+
+
+
Search and filter
+
+
+
+
+ {% if paginator.count > 0 %}
+ {% include "includes/common/pagination-list-summary.jinja" %}
+ {% endif %}
+ {% if object_list %}
+ {% set table_rows = [] %}
+ {% for object in object_list %}
+ {%- set task_linked_id -%}
+
{{ object.pk }}
+ {%- endset -%}
+
+ {%- set workbasket_linked_id -%}
+
{{ object.workbasket.pk }}
+ {%- endset -%}
+
+ {{ table_rows.append([
+ {"text": task_linked_id},
+ {"text": object.title},
+ {"text": object.category},
+ {"text": object.progress_state},
+ {"text": workbasket_linked_id if object.workbasket else "-"},
+ {"text": "{:%d %b %Y}".format(object.created_at)},
+ ]) or "" }}
+ {% endfor %}
+
+ {{ govukTable({
+ "head": [
+ {"text": "ID"},
+ {"text": "Title"},
+ {"text": "Category"},
+ {"text": "Status"},
+ {"text": "Workbasket"},
+ {"text": create_sortable_anchor(request, "created_at", "Created", base_url)},
+ ],
+ "rows": table_rows
+ }) }}
+ {% else %}
+
There are no results for your search, please:
+
+ - check the spelling of your keywords
+ - use more general keywords
+ - select or deselect different filters
+
+ {% endif %}
+ {% include "includes/common/pagination.jinja" %}
+
+
+{% endblock %}
diff --git a/tasks/migrations/0007_create_tasklog.py b/tasks/migrations/0007_create_tasklog.py
index 15a81debf..05659f3fa 100644
--- a/tasks/migrations/0007_create_tasklog.py
+++ b/tasks/migrations/0007_create_tasklog.py
@@ -40,6 +40,7 @@ class Migration(migrations.Migration):
("TASK_UNASSIGNED", "Task Unassigned"),
("PROGRESS_STATE_UPDATED", "Progress State Updated"),
],
+ editable=False,
max_length=100,
),
),
@@ -56,7 +57,8 @@ class Migration(migrations.Migration):
"task",
models.ForeignKey(
editable=False,
- on_delete=django.db.models.deletion.PROTECT,
+ null=True,
+ on_delete=django.db.models.deletion.SET_NULL,
related_name="logs",
to="tasks.task",
),
diff --git a/tasks/models.py b/tasks/models.py
index 424b026b3..28cad75a1 100644
--- a/tasks/models.py
+++ b/tasks/models.py
@@ -3,13 +3,25 @@
from django.conf import settings
from django.contrib.auth import get_user_model
from django.db import models
+from django.db import transaction
+from django.urls import reverse
from common.models.mixins import TimestampedMixin
+from common.models.mixins import WithSignalManagerMixin
+from common.models.mixins import WithSignalQuerysetMixin
from workbaskets.models import WorkBasket
User = get_user_model()
+class TaskManager(WithSignalManagerMixin, models.Manager):
+ pass
+
+
+class TaskQueryset(WithSignalQuerysetMixin, models.QuerySet):
+ pass
+
+
class Task(TimestampedMixin):
title = models.CharField(max_length=255)
description = models.TextField()
@@ -44,9 +56,14 @@ class Task(TimestampedMixin):
related_name="created_tasks",
)
+ objects = TaskManager.from_queryset(TaskQueryset)()
+
def __str__(self):
return self.title
+ def get_url(self):
+ return reverse("workflow:task-ui-detail", kwargs={"pk": self.pk})
+
class Category(models.Model):
name = models.CharField(
@@ -78,7 +95,11 @@ def __str__(self):
return self.get_name_display()
-class TaskAssigneeQueryset(models.QuerySet):
+class TaskAssigneeManager(WithSignalManagerMixin, models.Manager):
+ pass
+
+
+class TaskAssigneeQueryset(WithSignalQuerysetMixin, models.QuerySet):
def assigned(self):
return self.exclude(unassigned_at__isnull=False)
@@ -122,7 +143,7 @@ class AssignmentType(models.TextChoices):
null=True,
)
- objects = TaskAssigneeQueryset.as_manager()
+ objects = TaskAssigneeManager.from_queryset(TaskAssigneeQueryset)()
def __str__(self):
return (
@@ -134,13 +155,17 @@ def is_assigned(self):
return True if not self.unassigned_at else False
@classmethod
- def unassign_user(cls, user, task):
+ def unassign_user(cls, user, task, instigator):
+ from tasks.signals import set_current_instigator
+
try:
assignment = cls.objects.get(user=user, task=task)
if assignment.unassigned_at:
return False
- assignment.unassigned_at = datetime.now()
- assignment.save(update_fields=["unassigned_at"])
+ set_current_instigator(instigator)
+ with transaction.atomic():
+ assignment.unassigned_at = datetime.now()
+ assignment.save(update_fields=["unassigned_at"])
return True
except cls.DoesNotExist:
return False
@@ -218,11 +243,13 @@ class AuditActionType(models.TextChoices):
action = models.CharField(
max_length=100,
choices=AuditActionType.choices,
+ editable=False,
)
description = models.TextField(editable=False)
task = models.ForeignKey(
Task,
- on_delete=models.PROTECT,
+ null=True,
+ on_delete=models.SET_NULL,
editable=False,
related_name="logs",
)
diff --git a/tasks/signals.py b/tasks/signals.py
new file mode 100644
index 000000000..af6392408
--- /dev/null
+++ b/tasks/signals.py
@@ -0,0 +1,67 @@
+import threading
+
+from django.db.models.signals import pre_save
+from django.dispatch import receiver
+
+from tasks.models import Task
+from tasks.models import TaskAssignee
+from tasks.models import TaskLog
+
+_thread_locals = threading.local()
+
+
+def get_current_instigator():
+ return getattr(_thread_locals, "instigator", None)
+
+
+def set_current_instigator(instigator):
+ _thread_locals.instigator = instigator
+
+
+@receiver(pre_save, sender=Task)
+def create_tasklog_for_task_update(sender, instance, old_instance=None, **kwargs):
+ """
+ Creates a `TaskLog` entry when a `Task` is being updated and the update
+ action is a `TaskLog.AuditActionType`.
+
+ Note that this signal is triggered before the `Task` instance is saved.
+ """
+ if instance._state.adding:
+ return
+
+ old_instance = old_instance or Task.objects.get(pk=instance.pk)
+
+ if instance.progress_state != old_instance.progress_state:
+ TaskLog.objects.create(
+ task=instance,
+ action=TaskLog.AuditActionType.PROGRESS_STATE_UPDATED,
+ instigator=get_current_instigator(),
+ progress_state=instance.progress_state,
+ )
+
+
+@receiver(pre_save, sender=TaskAssignee)
+def create_tasklog_for_task_assignee(sender, instance, old_instance=None, **kwargs):
+ """
+ Creates a `TaskLog` entry when a user is assigned to or unassigned from a
+ `Task`.
+
+ Note that this signal is triggered before the `TaskAssignee` instance is saved.
+ """
+ if instance._state.adding:
+ return TaskLog.objects.create(
+ task=instance.task,
+ action=TaskLog.AuditActionType.TASK_ASSIGNED,
+ instigator=get_current_instigator(),
+ assignee=instance.user,
+ )
+
+ old_instance = old_instance or TaskAssignee.objects.get(pk=instance.pk)
+
+ if instance.unassigned_at and instance.unassigned_at != old_instance.unassigned_at:
+ return TaskLog.objects.create(
+ task=instance.task,
+ action=TaskLog.AuditActionType.TASK_UNASSIGNED,
+ instigator=get_current_instigator(),
+ assignee=old_instance.user,
+ )
diff --git a/tasks/tests/test_models.py b/tasks/tests/test_models.py
index 2201e0c2d..ec8f6fb81 100644
--- a/tasks/tests/test_models.py
+++ b/tasks/tests/test_models.py
@@ -29,9 +29,9 @@ def test_task_assignee_unassign_user_classmethod(task_assignee):
user = task_assignee.user
task = task_assignee.task
- assert TaskAssignee.unassign_user(user=user, task=task)
+ assert TaskAssignee.unassign_user(user=user, task=task, instigator=user)
# User has already been unassigned
- assert not TaskAssignee.unassign_user(user=user, task=task)
+ assert not TaskAssignee.unassign_user(user=user, task=task, instigator=user)
def test_task_assignee_assigned_queryset(
@@ -41,7 +41,7 @@ def test_task_assignee_assigned_queryset(
user = task_assignee.user
task = task_assignee.task
- TaskAssignee.unassign_user(user=user, task=task)
+ TaskAssignee.unassign_user(user=user, task=task, instigator=user)
assert not TaskAssignee.objects.assigned()
@@ -53,7 +53,7 @@ def test_task_assignee_unassigned_queryset(
user = task_assignee.user
task = task_assignee.task
- TaskAssignee.unassign_user(user=user, task=task)
+ TaskAssignee.unassign_user(user=user, task=task, instigator=user)
assert TaskAssignee.objects.unassigned().count() == 1
diff --git a/tasks/tests/test_views.py b/tasks/tests/test_views.py
new file mode 100644
index 000000000..b7ecd58c8
--- /dev/null
+++ b/tasks/tests/test_views.py
@@ -0,0 +1,35 @@
+from django.urls import reverse
+
+from common.tests.factories import ProgressStateFactory
+from common.tests.factories import TaskFactory
+from tasks.models import ProgressState
+from tasks.models import TaskLog
+
+
+def test_task_update_view_update_progress_state(valid_user_client):
+ """Tests that `TaskUpdateView` updates `Task.progress_state` and that a
+ related `TaskLog` entry is also created."""
+ instance = TaskFactory.create(progress_state__name=ProgressState.State.TO_DO)
+ new_progress_state = ProgressStateFactory.create(
+ name=ProgressState.State.IN_PROGRESS,
+ )
+ form_data = {
+ "progress_state": new_progress_state.pk,
+ "title": instance.title,
+ "description": instance.description,
+ }
+ url = reverse(
+ "workflow:task-ui-update",
+ kwargs={"pk": instance.pk},
+ )
+ response = valid_user_client.post(url, form_data)
+ assert response.status_code == 302
+
+ instance.refresh_from_db()
+
+ assert instance.progress_state == new_progress_state
+ assert TaskLog.objects.get(
+ task=instance,
+ action=TaskLog.AuditActionType.PROGRESS_STATE_UPDATED,
+ instigator=response.wsgi_request.user,
+ )
diff --git a/tasks/urls.py b/tasks/urls.py
new file mode 100644
index 000000000..176b76bff
--- /dev/null
+++ b/tasks/urls.py
@@ -0,0 +1,33 @@
+from django.urls import include
+from django.urls import path
+
+from tasks import views
+
+app_name = "workflow"
+
+ui_patterns = [
+ path("", views.TaskListView.as_view(), name="task-ui-list"),
+ path("/", views.TaskDetailView.as_view(), name="task-ui-detail"),
+ path("create/", views.TaskCreateView.as_view(), name="task-ui-create"),
+ path(
+ "/confirm-create",
+ views.TaskConfirmCreateView.as_view(),
+ name="task-ui-confirm-create",
+ ),
+ path("/update/", views.TaskUpdateView.as_view(), name="task-ui-update"),
+ path(
+ "/confirm-update",
+ views.TaskConfirmUpdateView.as_view(),
+ name="task-ui-confirm-update",
+ ),
+ path("/delete/", views.TaskDeleteView.as_view(), name="task-ui-delete"),
+ path(
+ "/confirm-delete",
+ views.TaskConfirmDeleteView.as_view(),
+ name="task-ui-confirm-delete",
+ ),
+]
+
+urlpatterns = [
+ path("tasks/", include(ui_patterns)),
+]
diff --git a/tasks/views.py b/tasks/views.py
new file mode 100644
index 000000000..c2394b226
--- /dev/null
+++ b/tasks/views.py
@@ -0,0 +1,114 @@
+from django.contrib.auth.mixins import PermissionRequiredMixin
+from django.db import transaction
+from django.http import HttpResponseRedirect
+from django.urls import reverse
+from django.views.generic.base import TemplateView
+from django.views.generic.detail import DetailView
+from django.views.generic.edit import CreateView
+from django.views.generic.edit import DeleteView
+from django.views.generic.edit import UpdateView
+
+from common.views import SortingMixin
+from common.views import WithPaginationListView
+from tasks.filters import TaskFilter
+from tasks.forms import TaskCreateForm
+from tasks.forms import TaskDeleteForm
+from tasks.forms import TaskUpdateForm
+from tasks.models import Task
+from tasks.signals import set_current_instigator
+
+
+class TaskListView(PermissionRequiredMixin, SortingMixin, WithPaginationListView):
+ model = Task
+ template_name = "tasks/list.jinja"
+ permission_required = "tasks.view_task"
+ paginate_by = 20
+ filterset_class = TaskFilter
+ sort_by_fields = ["created_at"]
+
+ def get_queryset(self):
+ queryset = Task.objects.all()
+ ordering = self.get_ordering()
+ if ordering:
+ ordering = (ordering,)
+ queryset = queryset.order_by(*ordering)
+ return queryset
+
+
+class TaskDetailView(PermissionRequiredMixin, DetailView):
+ model = Task
+ template_name = "tasks/detail.jinja"
+ permission_required = "tasks.view_task"
+
+
+class TaskCreateView(PermissionRequiredMixin, CreateView):
+ model = Task
+ template_name = "layouts/create.jinja"
+ permission_required = "tasks.add_task"
+ form_class = TaskCreateForm
+
+ def get_context_data(self, **kwargs):
+ context = super().get_context_data(**kwargs)
+ context["page_title"] = "Create a task"
+ return context
+
+ def form_valid(self, form):
+ self.object = form.save(user=self.request.user)
+ return HttpResponseRedirect(self.get_success_url())
+
+ def get_success_url(self):
+ return reverse("workflow:task-ui-confirm-create", kwargs={"pk": self.object.pk})
+
+
+class TaskConfirmCreateView(PermissionRequiredMixin, DetailView):
+ model = Task
+ template_name = "tasks/confirm_create.jinja"
+ permission_required = "tasks.add_task"
+
+
+class TaskUpdateView(PermissionRequiredMixin, UpdateView):
+ model = Task
+ template_name = "tasks/edit.jinja"
+ permission_required = "tasks.change_task"
+ form_class = TaskUpdateForm
+
+ def form_valid(self, form):
+ set_current_instigator(self.request.user)
+ with transaction.atomic():
+ self.object = form.save()
+ return HttpResponseRedirect(self.get_success_url())
+
+ def get_success_url(self):
+ return reverse("workflow:task-ui-confirm-update", kwargs={"pk": self.object.pk})
+
+
+class TaskConfirmUpdateView(PermissionRequiredMixin, DetailView):
+ model = Task
+ template_name = "tasks/confirm_update.jinja"
+ permission_required = "tasks.change_task"
+
+
+class TaskDeleteView(PermissionRequiredMixin, DeleteView):
+ model = Task
+ template_name = "tasks/delete.jinja"
+ permission_required = "tasks.delete_task"
+ form_class = TaskDeleteForm
+
+ def get_form_kwargs(self):
+ kwargs = super().get_form_kwargs()
+ kwargs["instance"] = self.object
+ return kwargs
+
+ def get_success_url(self):
+ return reverse("workflow:task-ui-confirm-delete", kwargs={"pk": self.object.pk})
+
+
+class TaskConfirmDeleteView(PermissionRequiredMixin, TemplateView):
+ model = Task
+ template_name = "tasks/confirm_delete.jinja"
+ permission_required = "tasks.delete_task"
+
+ def get_context_data(self, **kwargs):
+ context_data = super().get_context_data(**kwargs)
+ context_data["deleted_pk"] = self.kwargs["pk"]
+ return context_data
diff --git a/urls.py b/urls.py
index 61e6aca58..d82888efc 100644
--- a/urls.py
+++ b/urls.py
@@ -37,6 +37,7 @@
path("", include("regulations.urls")),
path("", include("reports.urls")),
path("", include("taric_parsers.urls")),
+ path("", include("tasks.urls", namespace="workflow")),
path("", include("workbaskets.urls", namespace="workbaskets")),
path("", include("reference_documents.urls", namespace="reference_documents")),
]
diff --git a/workbaskets/forms.py b/workbaskets/forms.py
index 7d6c0827d..b2fb5002a 100644
--- a/workbaskets/forms.py
+++ b/workbaskets/forms.py
@@ -11,6 +11,7 @@
from crispy_forms_gds.layout import Submit
from django import forms
from django.contrib.auth import get_user_model
+from django.db import transaction
from django.db.models import Q
from django.urls import reverse
@@ -20,6 +21,7 @@
from tasks.models import Comment
from tasks.models import Task
from tasks.models import TaskAssignee
+from tasks.signals import set_current_instigator
from workbaskets import models
from workbaskets import validators
from workbaskets.util import serialize_uploaded_data
@@ -248,7 +250,10 @@ def init_layout(self):
),
)
+ @transaction.atomic
def assign_users(self, task):
+ set_current_instigator(self.request.user)
+
assignment_type = self.cleaned_data["assignment_type"]
assignees = [
@@ -309,7 +314,10 @@ def init_layout(self):
),
)
+ @transaction.atomic
def unassign_users(self):
+ set_current_instigator(self.request.user)
+
assignees = self.cleaned_data["assignees"]
for assignee in assignees:
assignee.unassigned_at = datetime.now()
diff --git a/workbaskets/tests/test_models.py b/workbaskets/tests/test_models.py
index a9e23c086..c837459e6 100644
--- a/workbaskets/tests/test_models.py
+++ b/workbaskets/tests/test_models.py
@@ -462,7 +462,7 @@ def test_unassigned_workbasket_cannot_be_queued():
)
assert workbasket.is_fully_assigned()
- TaskAssignee.unassign_user(user=worker, task=task)
+ TaskAssignee.unassign_user(user=worker, task=task, instigator=worker)
assert not workbasket.is_fully_assigned()
diff --git a/workbaskets/tests/test_views.py b/workbaskets/tests/test_views.py
index 8baa0f142..9abc1721c 100644
--- a/workbaskets/tests/test_views.py
+++ b/workbaskets/tests/test_views.py
@@ -27,6 +27,7 @@
from measures.models import Measure
from tasks.models import Comment
from tasks.models import TaskAssignee
+from tasks.models import TaskLog
from workbaskets import models
from workbaskets.tasks import check_workbasket_sync
from workbaskets.validators import WorkflowStatus
@@ -2275,18 +2276,36 @@ def test_disabled_packaging_for_unassigned_workbasket(
assert not packaging_button.has_attr("disabled")
-def test_workbasket_assign_users_view(valid_user, valid_user_client, user_workbasket):
- valid_user.user_permissions.add(
- Permission.objects.get(codename="add_taskassignee"),
- )
- response = valid_user_client.get(
- reverse(
- "workbaskets:workbasket-ui-assign-users",
- kwargs={"pk": user_workbasket.pk},
- ),
+def test_workbasket_assign_users_view(valid_user, valid_user_client, new_workbasket):
+ """Tests that a user can be assigned to a workbasket and that a `TaskLog`
+ entry is created together with the `TaskAssignee` instance."""
+ url = reverse(
+ "workbaskets:workbasket-ui-assign-users",
+ kwargs={"pk": new_workbasket.pk},
)
+
+ form_data = {
+ "users": [valid_user.pk],
+ "assignment_type": TaskAssignee.AssignmentType.WORKBASKET_WORKER,
+ }
+
+ response = valid_user_client.get(url)
assert response.status_code == 200
- assert "Assign users to workbasket" in str(response.content)
+
+ response = valid_user_client.post(url, form_data)
+ assert response.status_code == 302
+
+ assert TaskAssignee.objects.get(
+ user=valid_user,
+ assignment_type=TaskAssignee.AssignmentType.WORKBASKET_WORKER,
+ task__workbasket=new_workbasket,
+ )
+
+ assert TaskLog.objects.get(
+ task__workbasket=new_workbasket,
+ action=TaskLog.AuditActionType.TASK_ASSIGNED,
+ instigator=response.wsgi_request.user,
+ )
def test_workbasket_assign_users_view_without_permission(client, user_workbasket):
@@ -2301,18 +2320,35 @@ def test_workbasket_assign_users_view_without_permission(client, user_workbasket
assert response.status_code == 403
-def test_workbasket_unassign_users_view(valid_user, valid_user_client, user_workbasket):
- valid_user.user_permissions.add(
- Permission.objects.get(codename="change_taskassignee"),
- )
- response = valid_user_client.get(
- reverse(
- "workbaskets:workbasket-ui-unassign-users",
- kwargs={"pk": user_workbasket.pk},
- ),
+def test_workbasket_unassign_users_view(valid_user, valid_user_client):
+ """Tests that a user can be unassigned from a workbasket and that a
+ `TaskLog` entry is created together with the updated `TaskAssignee`
+ instance."""
+ assignee = factories.TaskAssigneeFactory.create(user=valid_user)
+ workbasket = assignee.task.workbasket
+
+ url = reverse(
+ "workbaskets:workbasket-ui-unassign-users",
+ kwargs={"pk": workbasket.pk},
)
+ form_data = {
+ "assignees": [assignee.pk],
+ }
+
+ response = valid_user_client.get(url)
assert response.status_code == 200
- assert "Unassign users from workbasket" in str(response.content)
+
+ response = valid_user_client.post(url, form_data)
+ assert response.status_code == 302
+
+ assignee.refresh_from_db()
+
+ assert not assignee.is_assigned
+ assert TaskLog.objects.get(
+ task__workbasket=workbasket,
+ action=TaskLog.AuditActionType.TASK_UNASSIGNED,
+ instigator=response.wsgi_request.user,
+ )
def test_workbasket_unassign_users_view_without_permission(client, user_workbasket):
diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py
index 3b7f682a8..62705411a 100644
--- a/workbaskets/views/ui.py
+++ b/workbaskets/views/ui.py
@@ -1661,7 +1661,6 @@ def get_form_kwargs(self):
)
return kwargs
- @atomic
def form_valid(self, form):
form.unassign_users()
return redirect(self.get_success_url())
From ddfea35343385d3b0444aecf03388d7b28dd4b49 Mon Sep 17 00:00:00 2001
From: Marya Shariq
Date: Mon, 11 Nov 2024 14:46:09 +0000
Subject: [PATCH 05/57] TP2000-1487: Create Subtask Form (#1319)
* create 'subtask create form' with predetermined parent
* add test to check that SubtaskCreateForm.save adds correct parent task when creating subtask
---
tasks/forms.py | 10 ++++++++++
tasks/tests/test_forms.py | 22 ++++++++++++++++++++++
tasks/urls.py | 5 +++++
tasks/views.py | 21 +++++++++++++++++++++
4 files changed, 58 insertions(+)
create mode 100644 tasks/tests/test_forms.py
diff --git a/tasks/forms.py b/tasks/forms.py
index 9334f1ef6..c15f3be06 100644
--- a/tasks/forms.py
+++ b/tasks/forms.py
@@ -67,4 +67,14 @@ class TaskUpdateForm(TaskBaseForm):
pass
+class SubTaskCreateForm(TaskBaseForm):
+ def save(self, parent_task, user, commit=True):
+ instance = super().save(commit=False)
+ instance.creator = user
+ instance.parent_task = parent_task
+ if commit:
+ instance.save()
+ return instance
+
+
TaskDeleteForm = delete_form_for(Task)
diff --git a/tasks/tests/test_forms.py b/tasks/tests/test_forms.py
new file mode 100644
index 000000000..d04806c63
--- /dev/null
+++ b/tasks/tests/test_forms.py
@@ -0,0 +1,22 @@
+from common.tests.factories import ProgressStateFactory
+from common.tests.factories import TaskFactory
+from tasks import forms
+from tasks.models import ProgressState
+
+
+def test_create_subtask_assigns_correct_parent_task(valid_user):
+ """Tests that SubtaskCreateForm assigns the correct parent on form.save."""
+ parent_task_instance = TaskFactory.create()
+ progress_state = ProgressStateFactory.create(
+ name=ProgressState.State.IN_PROGRESS,
+ )
+
+ subtask_form_data = {
+ "progress_state": progress_state.pk,
+ "title": "subtask test title",
+ "description": "subtask test description",
+ }
+ form = forms.SubTaskCreateForm(data=subtask_form_data)
+ new_subtask = form.save(parent_task_instance, user=valid_user)
+
+ assert new_subtask.parent_task.pk == parent_task_instance.pk
diff --git a/tasks/urls.py b/tasks/urls.py
index 176b76bff..0f5aff2c5 100644
--- a/tasks/urls.py
+++ b/tasks/urls.py
@@ -26,6 +26,11 @@
views.TaskConfirmDeleteView.as_view(),
name="task-ui-confirm-delete",
),
+ path(
+ "/sub-tasks/create",
+ views.SubTaskCreateView.as_view(),
+ name="subtask-ui-create",
+ ),
]
urlpatterns = [
diff --git a/tasks/views.py b/tasks/views.py
index c2394b226..5f9b2bfad 100644
--- a/tasks/views.py
+++ b/tasks/views.py
@@ -11,6 +11,7 @@
from common.views import SortingMixin
from common.views import WithPaginationListView
from tasks.filters import TaskFilter
+from tasks.forms import SubTaskCreateForm
from tasks.forms import TaskCreateForm
from tasks.forms import TaskDeleteForm
from tasks.forms import TaskUpdateForm
@@ -112,3 +113,23 @@ def get_context_data(self, **kwargs):
context_data = super().get_context_data(**kwargs)
context_data["deleted_pk"] = self.kwargs["pk"]
return context_data
+
+
+class SubTaskCreateView(PermissionRequiredMixin, CreateView):
+ model = Task
+ template_name = "layouts/create.jinja"
+ permission_required = "tasks.add_task"
+ form_class = SubTaskCreateForm
+
+ def get_context_data(self, **kwargs):
+ context = super().get_context_data(**kwargs)
+ context["page_title"] = "Create a subtask"
+ return context
+
+ def form_valid(self, form):
+ parent_task = Task.objects.filter(pk=self.kwargs["pk"]).first()
+ self.object = form.save(parent_task, user=self.request.user)
+ return HttpResponseRedirect(self.get_success_url())
+
+ def get_success_url(self):
+ return reverse("workflow:task-ui-confirm-create", kwargs={"pk": self.object.pk})
From ead85bceccbcf5af6a8ecabf2f824ecde9bfc340 Mon Sep 17 00:00:00 2001
From: Paul Pepper <85895113+paulpepper-trade@users.noreply.github.com>
Date: Tue, 12 Nov 2024 14:05:07 +0000
Subject: [PATCH 06/57] TP2000-1540, TP2000-1541, TP2000-1546 & TP2000-1553
queues, workflows and workflow task items (#1321)
* Split models file and add stubbed queue support.
* Removed queue from base class; Create TaskItem using TaskItemTemplate position
* Amend TaskItem.position exception error message
* Override QueueItem metaclass to enforce queue field on conrete subclasses
* Export queue-related models from tasks
* Implement Queue instance methods using new get_related_objects util method
* Implement QueueItem instance methods
* Add Queue.get_items() and add return type annotations
* Add concrete test-only models for Queue and QueueItem
* Add queue tests
* Add queue and item tests
* Refactor TableLock into a decorator and context manager
* Override QueueItem's model manager create method to set queue positions on behalf of subclasses
* Amend Queue and QueueItem tests
* Docstring updates.
* Support only a single reverse FK relationship to QueueItem for queue instances
* Support default State instance on ProgressState and Task.progress_state
* Remove redundant model manager. Add task and task template util methods.
* Add workflow unit tests.
* Rely on Task.progress_state default in WB assign users view
---------
Co-authored-by: Dale Cannon
---
common/tests/test_util.py | 41 +++-
common/util.py | 59 +++--
settings/test.py | 7 +-
...late_taskworkflow_tasktemplate_and_more.py | 165 +++++++++++++
tasks/models/__init__.py | 41 ++++
tasks/models/logs.py | 99 ++++++++
tasks/models/queue.py | 230 ++++++++++++++++++
tasks/{models.py => models/task.py} | 147 +++--------
tasks/models/workflow.py | 124 ++++++++++
tasks/tests/test_queue/__init__.py | 0
tasks/tests/test_queue/models.py | 24 ++
tasks/tests/test_queue_models.py | 151 ++++++++++++
tasks/tests/test_workflow_models.py | 162 ++++++++++++
workbaskets/views/ui.py | 4 -
14 files changed, 1122 insertions(+), 132 deletions(-)
create mode 100644 tasks/migrations/0009_taskworkflowtemplate_taskworkflow_tasktemplate_and_more.py
create mode 100644 tasks/models/__init__.py
create mode 100644 tasks/models/logs.py
create mode 100644 tasks/models/queue.py
rename tasks/{models.py => models/task.py} (60%)
create mode 100644 tasks/models/workflow.py
create mode 100644 tasks/tests/test_queue/__init__.py
create mode 100644 tasks/tests/test_queue/models.py
create mode 100644 tasks/tests/test_queue_models.py
create mode 100644 tasks/tests/test_workflow_models.py
diff --git a/common/tests/test_util.py b/common/tests/test_util.py
index ba956d454..b7d5be1b9 100644
--- a/common/tests/test_util.py
+++ b/common/tests/test_util.py
@@ -27,7 +27,7 @@
"port": 1234,
"dbname": "dbname",
},
- "engine://username:password@host:1234/dbname",
+ "engine://username:password@host:1234/dbname", # /PS-IGNORE
),
(
{
@@ -379,3 +379,42 @@ def test_xml_fromstring_vulnerabilities(file_name):
string = open(file).read()
with pytest.raises((XMLSyntaxError, DTDForbidden)):
util.xml_fromstring(string)
+
+
+@mock.patch("django.db.transaction.get_connection")
+def test_tablelock_as_context_manager(mock_get_connection):
+ mock_cursor = mock.MagicMock()
+ mock_get_connection.return_value.cursor.return_value.__enter__.return_value = (
+ mock_cursor
+ )
+
+ instance = factories.TestModel1Factory.create()
+ model = instance._meta.model
+ lock = util.TableLock.EXCLUSIVE
+
+ with util.TableLock(model, lock=lock):
+ assert model.objects.select_for_update().get(pk=instance.pk)
+ mock_cursor.execute.assert_called_once_with(
+ f"LOCK TABLE {model._meta.db_table} IN {lock} MODE",
+ )
+
+
+@mock.patch("django.db.transaction.get_connection")
+def test_tablelock_as_decorator(mock_get_connection):
+ mock_cursor = mock.MagicMock()
+ mock_get_connection.return_value.cursor.return_value.__enter__.return_value = (
+ mock_cursor
+ )
+
+ instance = factories.TestModel1Factory.create()
+ model = instance._meta.model
+ lock = util.TableLock.EXCLUSIVE
+
+ @util.TableLock.acquire_lock(model, lock=lock)
+ def decorated_function():
+ return True
+
+ assert decorated_function()
+ mock_cursor.execute.assert_called_once_with(
+ f"LOCK TABLE {model._meta.db_table} IN {lock} MODE",
+ )
diff --git a/common/util.py b/common/util.py
index 667f96b79..e8e0de4f0 100644
--- a/common/util.py
+++ b/common/util.py
@@ -432,8 +432,8 @@ def get_field_tuple(model: Model, field_name: str) -> Tuple[str, Any]:
class TableLock:
- """Provides a decorator for locking database tables for the duration of a
- decorated function."""
+ """Provides a decorator and context manager for locking database tables for
+ the duration of a decorated function or context block."""
ACCESS_SHARE = "ACCESS SHARE"
ROW_SHARE = "ROW SHARE"
@@ -455,6 +455,29 @@ class TableLock:
ACCESS_EXCLUSIVE,
)
+ def __init__(self, *models, lock=None):
+ if lock is None:
+ lock = self.ACCESS_EXCLUSIVE
+
+ if lock not in self.LOCK_TYPES:
+ raise ValueError("%s is not a PostgreSQL supported lock mode.")
+
+ self.lock = lock
+ self.models = models
+
+ def __enter__(self):
+ with atomic():
+ with transaction.get_connection().cursor() as cursor:
+ for model in self.models:
+ if isinstance(model, str):
+ model = apps.get_model(model)
+ cursor.execute(
+ f"LOCK TABLE {model._meta.db_table} IN {self.lock} MODE",
+ )
+
+ def __exit__(self, exc_type, exc_val, exc_tb):
+ pass
+
@classmethod
def acquire_lock(cls, *models, lock=None):
"""
@@ -469,24 +492,11 @@ def myview(request)
PostgreSQL's LOCK Documentation:
http://www.postgresql.org/docs/8.3/interactive/sql-lock.html
"""
- if lock is None:
- lock = cls.ACCESS_EXCLUSIVE
-
- if lock not in cls.LOCK_TYPES:
- raise ValueError("%s is not a PostgreSQL supported lock mode.")
@wrapt.decorator
def wrapper(wrapped, instance, args, kwargs):
- with atomic():
- with transaction.get_connection().cursor() as cursor:
- for model in models:
- if isinstance(model, str):
- model = apps.get_model(model)
- cursor.execute(
- f"LOCK TABLE {model._meta.db_table} IN {lock} MODE",
- )
-
- return wrapped(*args, **kwargs)
+ with cls(*models, lock=lock):
+ return wrapped(*args, **kwargs)
return wrapper
@@ -694,3 +704,18 @@ def wrapper(wrapped, instance, args, kwargs):
return result
return wrapper
+
+
+def get_related_names(instance, related_model) -> list[str]:
+ """
+ Return a list of related names of reverse foreign-key relationships to
+ (subclasses of) the specified `related_model` for the given `instance`.
+
+ If a reverse foreign-key relationship exists but no related name has been defined, a default name in the format `relatedmodel_set` will be returned.
+ If no such relationships exist, an empty list if returned.
+ """
+ related_names = []
+ for field in instance._meta.get_fields():
+ if field.one_to_many and issubclass(field.related_model, related_model):
+ related_names.append(field.get_accessor_name())
+ return related_names
diff --git a/settings/test.py b/settings/test.py
index 03b732549..5973d5380 100644
--- a/settings/test.py
+++ b/settings/test.py
@@ -8,7 +8,12 @@
CSRF_COOKIE_SECURE = False
SESSION_COOKIE_SECURE = False
-INSTALLED_APPS.append("common.tests")
+INSTALLED_APPS.extend(
+ [
+ "common.tests",
+ "tasks.tests.test_queue",
+ ],
+)
# Bucket settings are belt-and-braces to guard against running in a real bucket
HMRC_BUCKET_STORAGE_NAME = os.environ.get("TEST_HMRC_BUCKET_STORAGE_NAME", "test-hmrc")
diff --git a/tasks/migrations/0009_taskworkflowtemplate_taskworkflow_tasktemplate_and_more.py b/tasks/migrations/0009_taskworkflowtemplate_taskworkflow_tasktemplate_and_more.py
new file mode 100644
index 000000000..7eaf5611d
--- /dev/null
+++ b/tasks/migrations/0009_taskworkflowtemplate_taskworkflow_tasktemplate_and_more.py
@@ -0,0 +1,165 @@
+# Generated by Django 4.2.15 on 2024-11-04 12:23
+
+import django.db.models.deletion
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ("tasks", "0008_alter_task_workbasket"),
+ ]
+
+ operations = [
+ migrations.CreateModel(
+ name="TaskWorkflowTemplate",
+ fields=[
+ (
+ "id",
+ models.BigAutoField(
+ auto_created=True,
+ primary_key=True,
+ serialize=False,
+ verbose_name="ID",
+ ),
+ ),
+ ("title", models.CharField(max_length=255)),
+ ("description", models.TextField(blank=True)),
+ ],
+ options={
+ "abstract": False,
+ },
+ ),
+ migrations.CreateModel(
+ name="TaskWorkflow",
+ fields=[
+ (
+ "id",
+ models.BigAutoField(
+ auto_created=True,
+ primary_key=True,
+ serialize=False,
+ verbose_name="ID",
+ ),
+ ),
+ ("title", models.CharField(max_length=255)),
+ ("description", models.TextField(blank=True)),
+ (
+ "creator_template",
+ models.ForeignKey(
+ null=True,
+ on_delete=django.db.models.deletion.SET_NULL,
+ to="tasks.taskworkflowtemplate",
+ ),
+ ),
+ ],
+ options={
+ "abstract": False,
+ },
+ ),
+ migrations.CreateModel(
+ name="TaskTemplate",
+ fields=[
+ (
+ "id",
+ models.BigAutoField(
+ auto_created=True,
+ primary_key=True,
+ serialize=False,
+ verbose_name="ID",
+ ),
+ ),
+ ("created_at", models.DateTimeField(auto_now_add=True)),
+ ("updated_at", models.DateTimeField(auto_now=True)),
+ ("title", models.CharField(max_length=255)),
+ ("description", models.TextField()),
+ (
+ "category",
+ models.ForeignKey(
+ blank=True,
+ null=True,
+ on_delete=django.db.models.deletion.PROTECT,
+ to="tasks.category",
+ ),
+ ),
+ ],
+ options={
+ "abstract": False,
+ },
+ ),
+ migrations.CreateModel(
+ name="TaskItemTemplate",
+ fields=[
+ (
+ "id",
+ models.BigAutoField(
+ auto_created=True,
+ primary_key=True,
+ serialize=False,
+ verbose_name="ID",
+ ),
+ ),
+ (
+ "position",
+ models.PositiveSmallIntegerField(db_index=True, editable=False),
+ ),
+ (
+ "queue",
+ models.ForeignKey(
+ on_delete=django.db.models.deletion.CASCADE,
+ related_name="queue_items",
+ to="tasks.taskworkflowtemplate",
+ ),
+ ),
+ (
+ "task_template",
+ models.OneToOneField(
+ on_delete=django.db.models.deletion.CASCADE,
+ to="tasks.tasktemplate",
+ ),
+ ),
+ ],
+ options={
+ "ordering": ["queue", "position"],
+ "abstract": False,
+ },
+ ),
+ migrations.CreateModel(
+ name="TaskItem",
+ fields=[
+ (
+ "id",
+ models.BigAutoField(
+ auto_created=True,
+ primary_key=True,
+ serialize=False,
+ verbose_name="ID",
+ ),
+ ),
+ (
+ "position",
+ models.PositiveSmallIntegerField(db_index=True, editable=False),
+ ),
+ (
+ "queue",
+ models.ForeignKey(
+ on_delete=django.db.models.deletion.CASCADE,
+ related_name="queue_items",
+ to="tasks.taskworkflow",
+ ),
+ ),
+ (
+ "task",
+ models.OneToOneField(
+ on_delete=django.db.models.deletion.CASCADE,
+ to="tasks.task",
+ ),
+ ),
+ ],
+ options={
+ "ordering": ["queue", "position"],
+ "abstract": False,
+ },
+ ),
+ ]
diff --git a/tasks/models/__init__.py b/tasks/models/__init__.py
new file mode 100644
index 000000000..af60eda88
--- /dev/null
+++ b/tasks/models/__init__.py
@@ -0,0 +1,41 @@
+"""Models used by all apps in the project."""
+
+from tasks.models.logs import TaskLog
+from tasks.models.queue import Queue
+from tasks.models.queue import QueueItem
+from tasks.models.queue import RequiredFieldError
+from tasks.models.task import Category
+from tasks.models.task import Comment
+from tasks.models.task import ProgressState
+from tasks.models.task import Task
+from tasks.models.task import TaskAssignee
+from tasks.models.workflow import TaskWorkflow
+from tasks.models.workflow import TaskItem
+from tasks.models.workflow import TaskWorkflowTemplate
+from tasks.models.workflow import TaskItemTemplate
+from tasks.models.workflow import TaskTemplate
+
+
+__all__ = [
+ # tasks.models.logs
+ "TaskLog",
+
+ # tasks.models.queue
+ "Queue",
+ "QueueItem",
+ "RequiredFieldError",
+
+ # tasks.models.task
+ "Category",
+ "Comment",
+ "ProgressState",
+ "Task",
+ "TaskAssignee",
+
+ # tasks.models.workflow
+ "TaskWorkflow",
+ "TaskItem",
+ "TaskWorkflowTemplate",
+ "TaskItemTemplate",
+ "TaskTemplate",
+]
diff --git a/tasks/models/logs.py b/tasks/models/logs.py
new file mode 100644
index 000000000..30c4bc046
--- /dev/null
+++ b/tasks/models/logs.py
@@ -0,0 +1,99 @@
+from django.conf import settings
+from django.contrib.auth import get_user_model
+from django.db import models
+
+from common.models.mixins import TimestampedMixin
+from tasks.models.task import Task
+
+User = get_user_model()
+
+
+class TaskLogManager(models.Manager):
+ def create(
+ self,
+ task: Task,
+ action: "TaskLog.AuditActionType",
+ instigator: User,
+ **kwargs,
+ ) -> "TaskLog":
+ """
+ Creates a new `TaskLog` instance with a generated description based on
+ `action`, saving it to the database and returning the created instance.
+
+ A TaskLog's `description` is generated using a template retrieved from `TaskLog.AUDIT_ACTION_MAP` that maps an `action` to its corresponding description.
+ Additional `kwargs` are required to format the description template depending on the provided action.
+ """
+
+ if action not in self.model.AuditActionType:
+ raise ValueError(
+ f"The action '{action}' is an invalid TaskLog.AuditActionType value.",
+ )
+
+ description_template = self.model.AUDIT_ACTION_MAP.get(action)
+ if not description_template:
+ raise ValueError(
+ f"No description template found for action '{action}' in TaskLog.AUDIT_ACTION_MAP.",
+ )
+
+ context = {"instigator": instigator}
+
+ if action in {
+ self.model.AuditActionType.TASK_ASSIGNED,
+ self.model.AuditActionType.TASK_UNASSIGNED,
+ }:
+ assignee = kwargs.pop("assignee", None)
+ if not assignee:
+ raise ValueError(f"Missing 'assignee' in kwargs for action '{action}'.")
+ context["assignee"] = assignee
+
+ elif action == self.model.AuditActionType.PROGRESS_STATE_UPDATED:
+ progress_state = kwargs.pop("progress_state", None)
+ if not progress_state:
+ raise ValueError(
+ f"Missing 'progress_state' in kwargs for action '{action}'.",
+ )
+ context["progress_state"] = progress_state
+
+ description = description_template.format(**context)
+
+ return super().create(
+ task=task,
+ action=action,
+ instigator=instigator,
+ description=description,
+ **kwargs,
+ )
+
+
+class TaskLog(TimestampedMixin):
+ class AuditActionType(models.TextChoices):
+ TASK_ASSIGNED = ("TASK_ASSIGNED",)
+ TASK_UNASSIGNED = ("TASK_UNASSIGNED",)
+ PROGRESS_STATE_UPDATED = ("PROGRESS_STATE_UPDATED",)
+
+ AUDIT_ACTION_MAP = {
+ AuditActionType.TASK_ASSIGNED: "{instigator} assigned {assignee}",
+ AuditActionType.TASK_UNASSIGNED: "{instigator} unassigned {assignee}",
+ AuditActionType.PROGRESS_STATE_UPDATED: "{instigator} changed the status to {progress_state}",
+ }
+
+ action = models.CharField(
+ max_length=100,
+ choices=AuditActionType.choices,
+ editable=False,
+ )
+ description = models.TextField(editable=False)
+ task = models.ForeignKey(
+ Task,
+ null=True,
+ on_delete=models.SET_NULL,
+ editable=False,
+ related_name="logs",
+ )
+ instigator = models.ForeignKey(
+ settings.AUTH_USER_MODEL,
+ on_delete=models.PROTECT,
+ editable=False,
+ )
+
+ objects = TaskLogManager()
diff --git a/tasks/models/queue.py b/tasks/models/queue.py
new file mode 100644
index 000000000..0fc517f26
--- /dev/null
+++ b/tasks/models/queue.py
@@ -0,0 +1,230 @@
+from __future__ import annotations
+
+from typing import Self
+
+from django.core.exceptions import ObjectDoesNotExist
+from django.db import models
+from django.db.transaction import atomic
+
+from common.util import TableLock
+from common.util import get_related_names
+
+
+class RequiredFieldError(Exception):
+ pass
+
+
+class Queue(models.Model):
+ """
+ A (FIFO) queue.
+
+ Note: This abstract class only supports a single reverse foreign-key relationship
+ to `QueueItem` for each instance. As such, all instance methods in this class use
+ the first-returned related name to access related `QueueItem` objects. This means that
+ if there are multiple relationships, only the first one will be considered when
+ retrieving items from the queue,
+ """
+
+ class Meta:
+ abstract = True
+
+ def get_items(self) -> models.QuerySet:
+ """Get all queue items as a queryset."""
+ related_name = get_related_names(self, QueueItem)[0]
+ return getattr(self, related_name).all()
+
+ def get_first(self) -> QueueItem | None:
+ """Get the first item in the queue."""
+ return self.get_items().first()
+
+ def get_last(self) -> QueueItem | None:
+ """Get the last item in the queue."""
+ return self.get_items().last()
+
+ def get_item(self, position: int) -> QueueItem | None:
+ """Get the item at `position` position in the queue."""
+ try:
+ return self.get_items().get(position=position)
+ except ObjectDoesNotExist:
+ return None
+
+ @property
+ def max_position(self) -> int:
+ """
+ Returns the highest item position in the queue.
+
+ If the queue is empty it returns zero.
+ """
+ max = self.get_items().aggregate(
+ max_position=models.Max("position"),
+ )["max_position"]
+ return max if max is not None else 0
+
+
+class QueueItemMetaClass(models.base.ModelBase):
+ def __new__(cls, name, bases, attrs):
+ new_class = super().__new__(cls, name, bases, attrs)
+
+ if (
+ "QueueItem" in [base.__name__ for base in bases]
+ and not new_class._meta.abstract
+ ):
+ queue_field = attrs.get("queue", None)
+ if not queue_field or not isinstance(queue_field, models.ForeignKey):
+ raise RequiredFieldError(
+ f"{name} must have a 'queue' ForeignKey field.",
+ )
+
+ return new_class
+
+
+class QueueItemManager(models.Manager):
+ @atomic
+ def create(self, **kwargs) -> QueueItem:
+ """Create a new item instance in a queue, given by the `queue` named
+ param, and place it in last position."""
+
+ with TableLock(self.model, lock=TableLock.EXCLUSIVE):
+ queue = kwargs.pop("queue")
+ position = kwargs.pop("position", (queue.get_items().count() + 1))
+
+ if position <= 0:
+ raise ValueError(
+ "QueueItem.position must be a positive integer greater than zero.",
+ )
+
+ return super().create(
+ queue=queue,
+ position=position,
+ **kwargs,
+ )
+
+
+class QueueItem(models.Model, metaclass=QueueItemMetaClass):
+ """Item that is a member of a Queue."""
+
+ class Meta:
+ abstract = True
+ ordering = ["queue", "position"]
+
+ position = models.PositiveSmallIntegerField(
+ db_index=True,
+ editable=False,
+ )
+ """
+ 1-based positioning - 1 is the first position.
+ """
+
+ objects = QueueItemManager()
+
+ @atomic
+ def delete(self):
+ """Remove and delete instance from its queue, shuffling all successive
+ queued instances up one position."""
+ instance = self.__class__.objects.select_for_update(nowait=True).get(pk=self.pk)
+
+ self.__class__.objects.select_for_update(nowait=True).filter(
+ position__gt=instance.position,
+ ).update(position=models.F("position") - 1)
+
+ return super().delete()
+
+ @atomic
+ def promote(self) -> Self:
+ """
+ Promote the instance by one place up the queue.
+
+ No change is made if the instance is already in its queue's first place.
+
+ Returns the promoted instance with any database updates applied.
+ """
+ instance = self.__class__.objects.select_for_update(nowait=True).get(pk=self.pk)
+
+ if instance.position == 1:
+ return instance
+
+ item_to_demote = self.__class__.objects.select_for_update(nowait=True).get(
+ position=instance.position - 1,
+ )
+ item_to_demote.position += 1
+ instance.position -= 1
+ self.__class__.objects.bulk_update([instance, item_to_demote], ["position"])
+ instance.refresh_from_db()
+
+ return instance
+
+ @atomic
+ def demote(self) -> Self:
+ """
+ Demote the instance by one place down the queue.
+
+ No change is made if the instance is already in its queue's last place.
+
+ Returns the demoted instance with any database updates applied.
+ """
+ instance = self.__class__.objects.select_for_update(nowait=True).get(pk=self.pk)
+
+ if instance.position == self.queue.max_position:
+ return instance
+
+ item_to_promote = self.__class__.objects.select_for_update(nowait=True).get(
+ position=instance.position + 1,
+ )
+ item_to_promote.position -= 1
+ instance.position += 1
+ self.__class__.objects.bulk_update([instance, item_to_promote], ["position"])
+ instance.refresh_from_db()
+
+ return instance
+
+ @atomic
+ def promote_to_first(self) -> Self:
+ """
+ Promote the instance to the first place in the queue so that it occupies
+ position 1.
+
+ No change is made if the instance is already in its queue's first place.
+
+ Returns the promoted instance with any database updates applied.
+ """
+
+ instance = self.__class__.objects.select_for_update(nowait=True).get(pk=self.pk)
+
+ if instance.position == 1:
+ return instance
+
+ self.__class__.objects.select_for_update(nowait=True).filter(
+ models.Q(position__lt=instance.position),
+ ).update(position=models.F("position") + 1)
+
+ instance.position = 1
+ instance.save(update_fields=["position"])
+ instance.refresh_from_db()
+
+ return instance
+
+ @atomic
+ def demote_to_last(self) -> Self:
+ """
+ Demote the instance to the last place in the queue so that it occupies
+ position of queue length.
+
+ No change is made if the instance is already in its queue's last place.
+
+ Returns the demoted instance with any database updates applied.
+ """
+ instance = self.__class__.objects.select_for_update(nowait=True).get(pk=self.pk)
+
+ last_place = self.queue.max_position
+ if instance.position == last_place:
+ return instance
+
+ self.__class__.objects.select_for_update(nowait=True).filter(
+ position__gt=instance.position,
+ ).update(position=models.F("position") - 1)
+
+ instance.position = last_place
+ instance.save(update_fields=["position"])
+ instance.refresh_from_db()
+
+ return instance
diff --git a/tasks/models.py b/tasks/models/task.py
similarity index 60%
rename from tasks/models.py
rename to tasks/models/task.py
index 28cad75a1..14c8d3201 100644
--- a/tasks/models.py
+++ b/tasks/models/task.py
@@ -14,6 +14,32 @@
User = get_user_model()
+class ProgressState(models.Model):
+ class State(models.TextChoices):
+ TO_DO = "TO_DO", "To do"
+ IN_PROGRESS = "IN_PROGRESS", "In progress"
+ DONE = "DONE", "Done"
+
+ DEFAULT_STATE_NAME = State.TO_DO
+ """The name of the default `State` object for `ProgressState`."""
+
+ name = models.CharField(
+ max_length=255,
+ choices=State.choices,
+ unique=True,
+ )
+
+ def __str__(self):
+ return self.get_name_display()
+
+ @classmethod
+ def get_default_state_id(cls):
+ """Get the id / pk of the default `State` object for `ProgressState`."""
+ # Failsafe get_or_create() avoids attempt to get non-existant instance.
+ default, _ = cls.objects.get_or_create(name=cls.DEFAULT_STATE_NAME)
+ return default.id
+
+
class TaskManager(WithSignalManagerMixin, models.Manager):
pass
@@ -22,7 +48,13 @@ class TaskQueryset(WithSignalQuerysetMixin, models.QuerySet):
pass
-class Task(TimestampedMixin):
+class TaskBase(TimestampedMixin):
+ """Abstract model mixin containing model fields common to TaskTemplate and
+ Task models."""
+
+ class Meta:
+ abstract = True
+
title = models.CharField(max_length=255)
description = models.TextField()
category = models.ForeignKey(
@@ -31,8 +63,12 @@ class Task(TimestampedMixin):
null=True,
on_delete=models.PROTECT,
)
+
+
+class Task(TaskBase):
progress_state = models.ForeignKey(
- "ProgressState",
+ ProgressState,
+ default=ProgressState.get_default_state_id,
on_delete=models.PROTECT,
)
parent_task = models.ForeignKey(
@@ -79,22 +115,6 @@ def __str__(self):
return self.name
-class ProgressState(models.Model):
- class State(models.TextChoices):
- TO_DO = "TO_DO", "To do"
- IN_PROGRESS = "IN_PROGRESS", "In progress"
- DONE = "DONE", "Done"
-
- name = models.CharField(
- max_length=255,
- choices=State.choices,
- unique=True,
- )
-
- def __str__(self):
- return self.get_name_display()
-
-
class TaskAssigneeManager(WithSignalManagerMixin, models.Manager):
pass
@@ -171,97 +191,6 @@ def unassign_user(cls, user, task, instigator):
return False
-class TaskLogManager(models.Manager):
- def create(
- self,
- task: Task,
- action: "TaskLog.AuditActionType",
- instigator: User,
- **kwargs,
- ) -> "TaskLog":
- """
- Creates a new `TaskLog` instance with a generated description based on
- `action`, saving it to the database and returning the created instance.
-
- A TaskLog's `description` is generated using a template retrieved from `TaskLog.AUDIT_ACTION_MAP` that maps an `action` to its corresponding description.
- Additional `kwargs` are required to format the description template depending on the provided action.
- """
-
- if action not in self.model.AuditActionType:
- raise ValueError(
- f"The action '{action}' is an invalid TaskLog.AuditActionType value.",
- )
-
- description_template = self.model.AUDIT_ACTION_MAP.get(action)
- if not description_template:
- raise ValueError(
- f"No description template found for action '{action}' in TaskLog.AUDIT_ACTION_MAP.",
- )
-
- context = {"instigator": instigator}
-
- if action in {
- self.model.AuditActionType.TASK_ASSIGNED,
- self.model.AuditActionType.TASK_UNASSIGNED,
- }:
- assignee = kwargs.pop("assignee", None)
- if not assignee:
- raise ValueError(f"Missing 'assignee' in kwargs for action '{action}'.")
- context["assignee"] = assignee
-
- elif action == self.model.AuditActionType.PROGRESS_STATE_UPDATED:
- progress_state = kwargs.pop("progress_state", None)
- if not progress_state:
- raise ValueError(
- f"Missing 'progress_state' in kwargs for action '{action}'.",
- )
- context["progress_state"] = progress_state
-
- description = description_template.format(**context)
-
- return super().create(
- task=task,
- action=action,
- instigator=instigator,
- description=description,
- **kwargs,
- )
-
-
-class TaskLog(TimestampedMixin):
- class AuditActionType(models.TextChoices):
- TASK_ASSIGNED = ("TASK_ASSIGNED",)
- TASK_UNASSIGNED = ("TASK_UNASSIGNED",)
- PROGRESS_STATE_UPDATED = ("PROGRESS_STATE_UPDATED",)
-
- AUDIT_ACTION_MAP = {
- AuditActionType.TASK_ASSIGNED: "{instigator} assigned {assignee}",
- AuditActionType.TASK_UNASSIGNED: "{instigator} unassigned {assignee}",
- AuditActionType.PROGRESS_STATE_UPDATED: "{instigator} changed the status to {progress_state}",
- }
-
- action = models.CharField(
- max_length=100,
- choices=AuditActionType.choices,
- editable=False,
- )
- description = models.TextField(editable=False)
- task = models.ForeignKey(
- Task,
- null=True,
- on_delete=models.SET_NULL,
- editable=False,
- related_name="logs",
- )
- instigator = models.ForeignKey(
- settings.AUTH_USER_MODEL,
- on_delete=models.PROTECT,
- editable=False,
- )
-
- objects = TaskLogManager()
-
-
class Comment(TimestampedMixin):
author = models.ForeignKey(
settings.AUTH_USER_MODEL,
diff --git a/tasks/models/workflow.py b/tasks/models/workflow.py
new file mode 100644
index 000000000..4087a2540
--- /dev/null
+++ b/tasks/models/workflow.py
@@ -0,0 +1,124 @@
+from django.db import models
+from django.db.transaction import atomic
+
+from tasks.models.queue import Queue
+from tasks.models.queue import QueueItem
+from tasks.models.task import Task
+from tasks.models.task import TaskBase
+
+# ---------------
+# - Base classes.
+# ---------------
+
+
+class TaskWorkflowBase(Queue):
+ """Abstract model base class containing model fields common to TaskWorkflow
+ and TaskWorkflowTemplate."""
+
+ class Meta:
+ abstract = True
+
+ title = models.CharField(
+ max_length=255,
+ )
+ description = models.TextField(
+ blank=True,
+ )
+
+
+# ----------------------
+# - Workflows and tasks.
+# ----------------------
+
+
+class TaskWorkflow(TaskWorkflowBase):
+ """Workflow of ordered Tasks."""
+
+ creator_template = models.ForeignKey(
+ "tasks.TaskWorkflowTemplate",
+ null=True,
+ on_delete=models.SET_NULL,
+ )
+
+ def get_tasks(self) -> models.QuerySet:
+ """Get a QuerySet of the Tasks associated through their TaskItem
+ instances to this TaskWorkflow."""
+ return Task.objects.filter(taskitem__queue=self)
+
+
+class TaskItem(QueueItem):
+ """Task item queue management for Task instances (these should always be
+ subtasks)."""
+
+ queue = models.ForeignKey(
+ TaskWorkflow,
+ related_name="queue_items",
+ on_delete=models.CASCADE,
+ )
+ task = models.OneToOneField(
+ "tasks.Task",
+ on_delete=models.CASCADE,
+ )
+ """The Task instance managed by this TaskItem."""
+
+
+# ----------------------------------------
+# - Template workflows and template tasks.
+# ----------------------------------------
+
+
+class TaskWorkflowTemplate(TaskWorkflowBase):
+ """Template used to create TaskWorkflow instance."""
+
+ def get_task_templates(self) -> models.QuerySet:
+ """Get a QuerySet of the TaskTemplates associated through their
+ TaskItemTemplate instances to this TaskWorkflowTemplate."""
+ return TaskTemplate.objects.filter(taskitemtemplate__queue=self)
+
+ @atomic
+ def create_task_workflow(self) -> "TaskWorkflow":
+ """Create a workflow and it subtasks, using values from this template
+ workflow and its task templates."""
+
+ task_workflow = TaskWorkflow.objects.create(
+ title=self.title,
+ description=self.description,
+ creator_template=self,
+ )
+
+ task_item_templates = TaskItemTemplate.objects.select_related(
+ "task_template",
+ ).filter(queue=self)
+ for task_item_template in task_item_templates:
+ task_template = task_item_template.task_template
+ task = Task.objects.create(
+ title=task_template.title,
+ description=task_template.description,
+ category=task_template.category,
+ )
+ TaskItem.objects.create(
+ position=task_item_template.position,
+ queue=task_workflow,
+ task=task,
+ )
+
+ return task_workflow
+
+
+class TaskItemTemplate(QueueItem):
+ """Queue item management for TaskTemplate instances."""
+
+ queue = models.ForeignKey(
+ TaskWorkflowTemplate,
+ related_name="queue_items",
+ on_delete=models.CASCADE,
+ )
+ task_template = models.OneToOneField(
+ "tasks.TaskTemplate",
+ on_delete=models.CASCADE,
+ )
+
+
+class TaskTemplate(TaskBase):
+ """Template used to create Task instances from within a template
+ workflow."""
diff --git a/tasks/tests/test_queue/__init__.py b/tasks/tests/test_queue/__init__.py
new file mode 100644
index 000000000..e69de29bb
diff --git a/tasks/tests/test_queue/models.py b/tasks/tests/test_queue/models.py
new file mode 100644
index 000000000..1e53ca973
--- /dev/null
+++ b/tasks/tests/test_queue/models.py
@@ -0,0 +1,24 @@
+from django.db import models
+
+from tasks.models import Queue
+from tasks.models import QueueItem
+
+
+class TestQueue(Queue):
+ """Concrete subclass of Queue."""
+
+ class Meta:
+ abstract = False
+
+
+class TestQueueItem(QueueItem):
+ """Concrete subclass of QueueItem."""
+
+ class Meta:
+ abstract = False
+
+ queue = models.ForeignKey(
+ TestQueue,
+ related_name="queue_items",
+ on_delete=models.CASCADE,
+ )
diff --git a/tasks/tests/test_queue_models.py b/tasks/tests/test_queue_models.py
new file mode 100644
index 000000000..3a878695d
--- /dev/null
+++ b/tasks/tests/test_queue_models.py
@@ -0,0 +1,151 @@
+import pytest
+from factory import SubFactory
+from factory.django import DjangoModelFactory
+
+from tasks.tests.test_queue.models import TestQueue
+from tasks.tests.test_queue.models import TestQueueItem
+
+pytestmark = pytest.mark.django_db
+
+
+class TestQueueFactory(DjangoModelFactory):
+ """Factory for TestQueue."""
+
+ class Meta:
+ abstract = False
+ model = TestQueue
+
+
+class TestQueueItemFactory(DjangoModelFactory):
+ """Factory for TestQueueItem."""
+
+ class Meta:
+ model = TestQueueItem
+ abstract = False
+
+ queue = SubFactory(TestQueueFactory)
+
+
+@pytest.fixture()
+def queue() -> TestQueue:
+ """Return an instance of TestQueue that contains no TestQueueItems."""
+ queue = TestQueueFactory.create()
+
+ assert not queue.get_items().exists()
+
+ return queue
+
+
+@pytest.fixture()
+def single_item_queue(queue) -> TestQueue:
+ """Return an instance of TestQueue containing a single TestQueueItem."""
+ TestQueueItemFactory.create(queue=queue)
+
+ assert queue.get_items().count() == 1
+
+ return queue
+
+
+@pytest.fixture()
+def three_item_queue(queue) -> TestQueue:
+ """Return an instance of TestQueue containing three TestQueueItem
+ instances."""
+ TestQueueItemFactory.create(queue=queue)
+ TestQueueItemFactory.create(queue=queue)
+ TestQueueItemFactory.create(queue=queue)
+
+ assert queue.get_items().count() == 3
+
+ return queue
+
+
+def test_empty_queue(queue):
+ assert queue.max_position == 0
+ assert queue.get_first() == None
+ assert queue.get_item(1) == None
+ assert queue.get_last() == None
+
+
+def test_non_empty_queue(queue):
+ first_item = TestQueueItemFactory.create(queue=queue)
+ second_item = TestQueueItemFactory.create(queue=queue)
+ third_item = TestQueueItemFactory.create(queue=queue)
+
+ assert first_item.position == 1
+ assert second_item.position == 2
+ assert third_item.position == 3
+ assert {first_item, second_item, third_item} == set(queue.get_items())
+ assert queue.max_position == 3
+ assert queue.get_items().count() == 3
+ assert queue.get_first() == first_item
+ assert queue.get_last() == third_item
+ assert queue.get_item(2) == second_item
+
+
+def test_item_delete(three_item_queue):
+ three_item_queue.get_first().delete()
+
+ assert three_item_queue.get_items().count() == 2
+
+ three_item_queue.get_item(1).delete()
+ three_item_queue.get_last().delete()
+
+ assert three_item_queue.get_items().count() == 0
+
+
+def test_item_promote(three_item_queue):
+ item = three_item_queue.get_last()
+
+ assert item.position == 3
+
+ item = item.promote()
+ item = item.promote()
+
+ assert item.position == 1
+
+ item = item.promote()
+
+ assert item.position == 1
+
+
+def test_item_demote(three_item_queue):
+ item = three_item_queue.get_first()
+
+ assert item.position == 1
+
+ item = item.demote()
+ item = item.demote()
+
+ assert item.position == 3
+
+ item = item.demote()
+
+ assert item.position == 3
+
+
+def test_item_demote_to_last(three_item_queue):
+ item = three_item_queue.get_first()
+
+ assert item.position == 1
+
+ item = item.demote_to_last()
+
+ assert item.position == 3
+
+ item = item.demote_to_last()
+
+ assert item.position == 3
+
+
+def test_item_promote_to_first(three_item_queue):
+ item = three_item_queue.get_last()
+
+ assert item.position == 3
+
+ item = item.promote_to_first()
+
+ assert item.position == 1
+
+ item = item.promote_to_first()
+
+ assert item.position == 1
diff --git a/tasks/tests/test_workflow_models.py b/tasks/tests/test_workflow_models.py
new file mode 100644
index 000000000..eb6716b02
--- /dev/null
+++ b/tasks/tests/test_workflow_models.py
@@ -0,0 +1,162 @@
+import factory
+import pytest
+from django.core.exceptions import ObjectDoesNotExist
+from factory import SubFactory
+from factory.django import DjangoModelFactory
+
+from common.tests.factories import CategoryFactory
+from tasks.models import TaskItemTemplate
+from tasks.models import TaskTemplate
+from tasks.models import TaskWorkflowTemplate
+
+pytestmark = pytest.mark.django_db
+
+
+class TaskWorkflowTemplateFactory(DjangoModelFactory):
+ """Factory to create TaskWorkflowTemplate instances."""
+
+ class Meta:
+ model = TaskWorkflowTemplate
+
+
+class TaskTemplateFactory(DjangoModelFactory):
+ """Factory to create TaskTemplate instances."""
+
+ title = factory.Faker("sentence")
+ description = factory.Faker("sentence")
+ category = factory.SubFactory(CategoryFactory)
+
+ class Meta:
+ model = TaskTemplate
+
+
+class TaskItemTemplateFactory(DjangoModelFactory):
+ """Factory to create TaskItemTemplate instances."""
+
+ class Meta:
+ model = TaskItemTemplate
+
+ queue = SubFactory(TaskWorkflowTemplateFactory)
+ task_template = SubFactory(TaskTemplateFactory)
+
+
+@pytest.fixture()
+def task_workflow_template() -> TaskWorkflowTemplate:
+ """Return an empty TaskWorkflowTemplate instance (containing no items)."""
+ return TaskWorkflowTemplateFactory.create()
+
+
+@pytest.fixture()
+def task_workflow_template_three_task_items(
+ task_workflow_template,
+) -> TaskWorkflowTemplate:
+ """Return a TaskWorkflowTemplate instance containing three TaskItemTemplate
+ and related TaskTemplates."""
+
+ task_item_templates = []
+ for _ in range(3):
+ task_template = TaskTemplateFactory.create()
+ task_item_template = TaskItemTemplateFactory.create(
+ queue=task_workflow_template,
+ task_template=task_template,
+ )
+ task_item_templates.append(task_item_template)
+
+ assert task_workflow_template.get_items().count() == 3
+ assert (
+ TaskItemTemplate.objects.filter(
+ queue=task_workflow_template,
+ ).count()
+ == 3
+ )
+ assert (
+ TaskTemplate.objects.filter(
+ taskitemtemplate__in=task_item_templates,
+ ).count()
+ == 3
+ )
+
+ return task_workflow_template
+
+
+def test_create_task_workflow_from_task_workflow_template(
+ task_workflow_template_three_task_items,
+):
+ """Test creation of TaskWorkflow instances from TaskWorkflowTemplates using
+ its `create_task_workflow()` method."""
+ task_workflow = task_workflow_template_three_task_items.create_task_workflow()
+
+ # Test that workflow values are valid.
+ assert task_workflow.creator_template == task_workflow_template_three_task_items
+ assert task_workflow.title == task_workflow_template_three_task_items.title
+ assert (
+ task_workflow.description == task_workflow_template_three_task_items.description
+ )
+ assert task_workflow.get_items().count() == 3
+
+ # Validate that item positions are equivalent.
+ zipped_items = zip(
+ task_workflow_template_three_task_items.get_items(),
+ task_workflow.get_items(),
+ )
+ for item_template, item in zipped_items:
+ assert item_template.position == item.position
+
+ # Validate that object values are equivalent.
+ zipped_objs = zip(
+ task_workflow_template_three_task_items.get_task_templates(),
+ task_workflow.get_tasks(),
+ )
+ for task_template, task in zipped_objs:
+ assert task_template.title == task.title
+ assert task_template.description == task.description
+ assert task_template.category == task.category
+
+
+def test_delete_task_item_template():
+ task_item_template = TaskItemTemplateFactory.create()
+ task_item_template_id = task_item_template.id
+ task_template_id = task_item_template.task_template.id
+
+ assert TaskItemTemplate.objects.get(id=task_item_template_id)
+ assert TaskTemplate.objects.get(id=task_template_id)
+
+ task_item_template.delete()
+
+ with pytest.raises(ObjectDoesNotExist):
+ TaskItemTemplate.objects.get(id=task_item_template_id)
+ assert TaskTemplate.objects.get(id=task_template_id)
+
+
+def test_delete_task_template():
+ task_item_template = TaskItemTemplateFactory.create()
+ task_item_template_id = task_item_template.id
+ task_template = task_item_template.task_template
+ task_template_id = task_item_template.task_template.id
+
+ assert TaskItemTemplate.objects.get(id=task_item_template_id)
+ assert TaskTemplate.objects.get(id=task_template_id)
+
+ task_template.delete()
+
+ with pytest.raises(ObjectDoesNotExist):
+ TaskItemTemplate.objects.get(id=task_item_template_id)
+ with pytest.raises(ObjectDoesNotExist):
+ assert TaskTemplate.objects.get(id=task_template_id)
+
+
+def test_delete_task_workflow_template(task_workflow_template_three_task_items):
+ task_item_template_ids = [
+ item.id for item in task_workflow_template_three_task_items.get_items()
+ ]
+ task_template_ids = [
+ task.id for task in task_workflow_template_three_task_items.get_task_templates()
+ ]
+
+ assert len(task_item_template_ids) == 3
+ assert len(task_template_ids) == 3
+
+ task_workflow_template_three_task_items.delete()
+
+ assert not TaskItemTemplate.objects.filter(id__in=task_item_template_ids).exists()
+ assert TaskTemplate.objects.filter(id__in=task_template_ids).count() == 3
diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py
index 62705411a..6281bbb15 100644
--- a/workbaskets/views/ui.py
+++ b/workbaskets/views/ui.py
@@ -64,7 +64,6 @@
from quotas.models import QuotaSuspension
from regulations.models import Regulation
from tasks.models import Comment
-from tasks.models import ProgressState
from tasks.models import Task
from tasks.models import TaskAssignee
from workbaskets import forms
@@ -1624,9 +1623,6 @@ def form_valid(self, form):
defaults={
"title": self.workbasket.title,
"description": self.workbasket.reason,
- "progress_state": ProgressState.objects.get(
- name=ProgressState.State.TO_DO,
- ),
"creator": self.request.user,
},
)
From 4d7dda9c6cb1e5c0e7fb1659b5bab07ac17c683d Mon Sep 17 00:00:00 2001
From: Dale Cannon <118175145+dalecannon@users.noreply.github.com>
Date: Tue, 12 Nov 2024 14:31:13 +0000
Subject: [PATCH 07/57] TP2000-1543 Prototype workflow template detail view
(#1322)
* Implement prototype TaskWorkflowTemplate detail view
* Rename workflow template jinja template
* Add TaskWorkflowTemplateDetailView test
---
common/static/common/scss/application.scss | 1 +
conftest.py | 1 +
.../tasks/workflows/template_detail.jinja | 97 +++++++++++++++++++
tasks/static/tasks/scss/_tasks.scss | 49 ++++++++++
tasks/tests/test_views.py | 19 ++++
tasks/urls.py | 15 ++-
tasks/views.py | 14 +++
webpack.config.js | 1 +
8 files changed, 195 insertions(+), 2 deletions(-)
create mode 100644 tasks/jinja2/tasks/workflows/template_detail.jinja
create mode 100644 tasks/static/tasks/scss/_tasks.scss
diff --git a/common/static/common/scss/application.scss b/common/static/common/scss/application.scss
index dbe60d797..c93689c49 100644
--- a/common/static/common/scss/application.scss
+++ b/common/static/common/scss/application.scss
@@ -28,6 +28,7 @@ $govuk-image-url-function: frontend-image-url;
@import "regulations";
@import "workbaskets";
@import "reference_documents";
+@import "tasks";
@import "versions";
@import "fake-link";
@import "violations";
diff --git a/conftest.py b/conftest.py
index 8c9ed10c0..eefff698c 100644
--- a/conftest.py
+++ b/conftest.py
@@ -310,6 +310,7 @@ def policy_group(db) -> Group:
("tasks", "view_comment"),
("tasks", "change_comment"),
("tasks", "delete_comment"),
+ ("tasks", "view_taskworkflowtemplate"),
]:
group.permissions.add(
Permission.objects.get(
diff --git a/tasks/jinja2/tasks/workflows/template_detail.jinja b/tasks/jinja2/tasks/workflows/template_detail.jinja
new file mode 100644
index 000000000..9e9073fd5
--- /dev/null
+++ b/tasks/jinja2/tasks/workflows/template_detail.jinja
@@ -0,0 +1,97 @@
+{% extends "layouts/layout.jinja" %}
+{% from "components/breadcrumbs.jinja" import breadcrumbs %}
+{% from "components/button/macro.njk" import govukButton %}
+
+{% set page_title = "Workflow template: " ~ object.title %}
+
+{% set edit_url = "#TODO" %}
+
+{% block breadcrumb %}
+ {{ breadcrumbs(request, [
+ {"text": "Find and view workflow templates", "href": "#TODO"},
+ {"text": page_title}
+ ],
+ with_workbasket=False,
+ ) }}
+{% endblock %}
+
+{% block content %}
+
+
+ Details
+
+
+
+
-
+ ID
+
+ -
+ {{ object.pk }}
+
+
+
+
+
+
+
+
+ Task templates
+
+ {% if not task_template_items %}
+ There are currently no task templates for this workflow template.
+ {% else %}
+
+ {% for task_template_item in task_template_items -%}
+ -
+
+
+ {% endfor %}
+
+ {% endif %}
+
+
+ {{ govukButton({
+ "text": "Delete workflow template",
+ "href": "#TODO",
+ "classes": "govuk-button--warning"
+ }) }}
+ {{ govukButton({
+ "text": "Find and view workflow templates",
+ "href": "#TODO",
+ "classes": "govuk-button--secondary"
+ }) }}
+
+{% endblock %}
diff --git a/tasks/static/tasks/scss/_tasks.scss b/tasks/static/tasks/scss/_tasks.scss
new file mode 100644
index 000000000..563247648
--- /dev/null
+++ b/tasks/static/tasks/scss/_tasks.scss
@@ -0,0 +1,49 @@
+// Source: https://design-system.service.gov.uk/components/task-list/
+
+.govuk-task-list {
+ @include govuk-font($size: 19);
+ margin-top: 0;
+ @include govuk-responsive-margin(6, "bottom");
+ padding: 0;
+ list-style-type: none;
+}
+
+.govuk-task-list__item {
+ display: table;
+ position: relative;
+ width: 100%;
+ margin-bottom: 0;
+ padding-top: govuk-spacing(2);
+ padding-bottom: govuk-spacing(2);
+ border-bottom: 1px solid $govuk-border-colour;
+}
+
+.govuk-task-list__item:first-child {
+ border-top: 1px solid $govuk-border-colour;
+}
+
+.govuk-task-list__item--with-link:hover {
+ background: govuk-colour("light-grey");
+}
+
+.govuk-task-list__name-and-hint {
+ display: table-cell;
+ vertical-align: top;
+ @include govuk-text-colour;
+}
+
+.govuk-task-list__link::after {
+ content: "";
+ display: block;
+ position: absolute;
+ top: 0;
+ right: 0;
+ bottom: 0;
+ left: 0;
+}
+
+.govuk-task-list__hint {
+ margin-top: govuk-spacing(1);
+ color: $govuk-secondary-text-colour;
+}
+
diff --git a/tasks/tests/test_views.py b/tasks/tests/test_views.py
index b7ecd58c8..252f256c8 100644
--- a/tasks/tests/test_views.py
+++ b/tasks/tests/test_views.py
@@ -1,9 +1,11 @@
+from bs4 import BeautifulSoup
from django.urls import reverse
from common.tests.factories import ProgressStateFactory
from common.tests.factories import TaskFactory
from tasks.models import ProgressState
from tasks.models import TaskLog
+from tasks.tests.test_workflow_models import TaskItemTemplateFactory
def test_task_update_view_update_progress_state(valid_user_client):
@@ -33,3 +35,20 @@ def test_task_update_view_update_progress_state(valid_user_client):
action=TaskLog.AuditActionType.PROGRESS_STATE_UPDATED,
instigator=response.wsgi_request.user,
)
+
+
+def test_workflow_template_detail_view_displays_task_templates(valid_user_client):
+ task_item_template = TaskItemTemplateFactory.create()
+ task_template = task_item_template.task_template
+ workflow_template = task_item_template.queue
+
+ url = reverse(
+ "workflow:task-workflow-template-ui-detail",
+ kwargs={"pk": workflow_template.pk},
+ )
+ response = valid_user_client.get(url)
+ assert response.status_code == 200
+
+ page = BeautifulSoup(response.content.decode(response.charset), "html.parser")
+ assert page.find("h1", text=workflow_template.title)
+ assert page.find("a", text=task_template.title)
diff --git a/tasks/urls.py b/tasks/urls.py
index 0f5aff2c5..74d2e10ef 100644
--- a/tasks/urls.py
+++ b/tasks/urls.py
@@ -5,7 +5,7 @@
app_name = "workflow"
-ui_patterns = [
+task_ui_patterns = [
path("", views.TaskListView.as_view(), name="task-ui-list"),
path("/", views.TaskDetailView.as_view(), name="task-ui-detail"),
path("create/", views.TaskCreateView.as_view(), name="task-ui-create"),
@@ -33,6 +33,17 @@
),
]
+workflow_template_ui_patterns = [
+ path(
+ "templates//",
+ views.TaskWorkflowTemplateDetailView.as_view(),
+ name="task-workflow-template-ui-detail",
+ ),
+]
+
+workflow_ui_patterns = workflow_template_ui_patterns
+
urlpatterns = [
- path("tasks/", include(ui_patterns)),
+ path("tasks/", include(task_ui_patterns)),
+ path("workflows/", include(workflow_ui_patterns)),
]
diff --git a/tasks/views.py b/tasks/views.py
index 5f9b2bfad..01d06fd61 100644
--- a/tasks/views.py
+++ b/tasks/views.py
@@ -16,6 +16,7 @@
from tasks.forms import TaskDeleteForm
from tasks.forms import TaskUpdateForm
from tasks.models import Task
+from tasks.models import TaskWorkflowTemplate
from tasks.signals import set_current_instigator
@@ -133,3 +134,16 @@ def form_valid(self, form):
def get_success_url(self):
return reverse("workflow:task-ui-confirm-create", kwargs={"pk": self.object.pk})
+
+
+class TaskWorkflowTemplateDetailView(PermissionRequiredMixin, DetailView):
+ model = TaskWorkflowTemplate
+ template_name = "tasks/workflows/template_detail.jinja"
+ permission_required = "tasks.view_taskworkflowtemplate"
+
+ def get_context_data(self, **kwargs):
+ context_data = super().get_context_data(**kwargs)
+ context_data["task_template_items"] = (
+ self.get_object().get_items().select_related("task_template")
+ )
+ return context_data
diff --git a/webpack.config.js b/webpack.config.js
index 5c98a53be..8dbf2b028 100644
--- a/webpack.config.js
+++ b/webpack.config.js
@@ -81,6 +81,7 @@ module.exports = {
"regulations/static/regulations/scss",
"workbaskets/static/workbaskets/scss",
"reference_documents/static/reference_documents/scss",
+ "tasks/static/tasks/scss",
],
},
},
From 77760e5b917a5c1c285ecc0b86524945d1dbfd53 Mon Sep 17 00:00:00 2001
From: Paul Pepper
Date: Tue, 12 Nov 2024 14:52:37 +0000
Subject: [PATCH 08/57] PR template tweaks
---
.github/pull_request_template.md | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md
index a4620a412..1bb09902a 100644
--- a/.github/pull_request_template.md
+++ b/.github/pull_request_template.md
@@ -1,5 +1,6 @@
-# TP-??? Your PR title here
+# TP2000-??? Your PR title here
+## Checklist
+- Requires migrations? Yes / No
+- Requires dependency updates? Yes / No
+
@@ -85,6 +113,18 @@
+
+
Created
@@ -92,6 +132,9 @@
{{ "{:%d %b %Y %H:%M}".format(object.created_at) }}
+
+
+
@@ -101,28 +144,64 @@
{{ object.creator.get_displayname() if object.creator else "-" }}
+
+
+
+
+
+
+
+ Assignees
+
+
+
+ {{ display_assignees(current_assignees) }}
+
+
+
+
+
-{% if not object.parent_task %}
- {{ govukButton({
- "text": "Delete task",
- "href": url("workflow:task-ui-delete", kwargs={"pk": object.pk}),
- "classes": "govuk-button--warning"
- }) }}
+ {% if not object.parent_task %}
+ {{ govukButton({
+ "text": "Delete task",
+ "href": url("workflow:task-ui-delete", kwargs={"pk": object.pk}),
+ "classes": "govuk-button--warning"
+ }) }}
{% else %}
{{ govukButton({
- "text": "Delete subtask",
- "href": url("workflow:subtask-ui-delete", kwargs={"pk": object.pk}),
- "classes": "govuk-button--warning"
- }) }}
+ "text": "Delete subtask",
+ "href": url("workflow:subtask-ui-delete", kwargs={"pk": object.pk}),
+ "classes": "govuk-button--warning"
+ }) }}
{% endif %}
+
{{ govukButton({
"text": "Find and view tasks",
"href": url("workflow:task-ui-list"),
"classes": "govuk-button--secondary"
}) }}
+
{% if not object.parent_task %}
{{ govukButton({
"text": "Create subtask",
@@ -131,4 +210,17 @@
}) }}
{% endif %}
+
+{% set assign_users_link = url("workflow:task-ui-assign-users", kwargs={"pk": object.pk}) %}
+{% set unassign_users_link = url("workflow:task-ui-unassign-users", kwargs={"pk": object.pk}) %}
+
+
{% endblock %}
diff --git a/tasks/migrations/0020_alter_taskassignee_assignment_type.py b/tasks/migrations/0020_alter_taskassignee_assignment_type.py
new file mode 100644
index 000000000..46b091033
--- /dev/null
+++ b/tasks/migrations/0020_alter_taskassignee_assignment_type.py
@@ -0,0 +1,26 @@
+# Generated by Django 4.2.17 on 2024-12-23 14:55
+
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ("tasks", "0019_alter_task_options_alter_tasktemplate_options_and_more"),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name="taskassignee",
+ name="assignment_type",
+ field=models.CharField(
+ choices=[
+ ("WORKBASKET_WORKER", "Workbasket worker"),
+ ("WORKBASKET_REVIEWER", "Workbasket reviewer"),
+ ("GENERAL", "General"),
+ ],
+ max_length=50,
+ ),
+ ),
+ ]
diff --git a/tasks/models/task.py b/tasks/models/task.py
index a93970dd7..294ed5150 100644
--- a/tasks/models/task.py
+++ b/tasks/models/task.py
@@ -200,9 +200,82 @@ def workbasket_reviewers(self):
class TaskAssignee(TimestampedMixin):
+ """
+ Model used to assocate Task instances with one or more Users.
+
+ The original intent was to associate two mandatory user roles to a workbasket:
+ - Worker who creates data in the workbasket - instances have
+ `assignment_type = AssignmentType.WORKBASKET_WORKER`
+ - Reviewer of workbasket data - instances have
+ `assignment_type = AssignmentType.WORKBASKET_REVIEWER`
+
+ In retrospect, these users should be assigned directly to the workbasket,
+ not via a Task, which includes an unnecessary level of indirection.
+
+ Current Task management introduces AssignmentType.GENERAL. TaskAssignee
+ instances with
+ `assignment_type = AssignmentType.GENERAL`
+ are actual task assignments, rather than the legacy approach to assigning
+ users to worker or reviewer roles.
+
+ Workbasket and new task assignment should be separated by introducing a new
+ Django Model, say, WorkBasketAssignee, and old assignments should be
+ migrated to instances of the new, replacement model.
+
+ class WorkBasketAssignee(TimestampedMixin):
+ class AssignmentType(models.TextChoices):
+ WORKBASKET_WORKER = "WORKBASKET_WORKER", "Workbasket worker"
+ WORKBASKET_REVIEWER = "WORKBASKET_REVIEWER", "Workbasket reviewer"
+
+ workbasket = models.ForeignKey(
+ WorkBasket,
+ blank=True,
+ null=True,
+ on_delete=models.CASCADE,
+ related_name="workbasketassignees",
+ )
+ user = models.ForeignKey(
+ settings.AUTH_USER_MODEL,
+ on_delete=models.PROTECT,
+ related_name="assigned_to",
+ )
+ assignment_type = models.CharField(
+ choices=AssignmentType.choices,
+ max_length=50,
+ )
+ unassigned_at = models.DateTimeField(
+ auto_now=False,
+ blank=True,
+ null=True,
+ )
+
+ @property
+ def is_assigned(self) -> bool:
+ return True if not self.unassigned_at else False
+
+ @classmethod
+ def unassign_user(cls, user, workbasket) -> bool:
+ try:
+ assignment = cls.objects.get(user=user, workbasket=workbasket)
+ except cls.DoesNotExist:
+ return False
+
+ if assignment.unassigned_at:
+ return False
+
+ with transaction.atomic():
+ assignment.unassigned_at = make_aware(datetime.now())
+ assignment.save(update_fields=["unassigned_at"])
+ return True
+
+ AssignmentType can then be stripped from TaskAssignee, since there'll only
+ be one type of assignee against tasks.
+ """
+
class AssignmentType(models.TextChoices):
WORKBASKET_WORKER = "WORKBASKET_WORKER", "Workbasket worker"
WORKBASKET_REVIEWER = "WORKBASKET_REVIEWER", "Workbasket reviewer"
+ GENERAL = "GENERAL", "General"
user = models.ForeignKey(
settings.AUTH_USER_MODEL,
diff --git a/tasks/models/workflow.py b/tasks/models/workflow.py
index b0bb33c54..0df851a6d 100644
--- a/tasks/models/workflow.py
+++ b/tasks/models/workflow.py
@@ -186,6 +186,7 @@ def create_task_workflow(
title=task_template.title,
description=task_template.description,
category=task_template.category,
+ creator=creator,
)
TaskItem.objects.create(
position=task_item_template.position,
diff --git a/tasks/signals.py b/tasks/signals.py
index af6392408..1f13fa0a4 100644
--- a/tasks/signals.py
+++ b/tasks/signals.py
@@ -15,6 +15,9 @@ def get_current_instigator():
def set_current_instigator(instigator):
+ """Sets the current user (`instigator`) who is instigating a task
+ action / change. This is normally done from the view that handles the
+ change. When the change is saved, instigator details are logged."""
_thread_locals.instigator = instigator
diff --git a/tasks/urls.py b/tasks/urls.py
index ad00db22f..22817acdb 100644
--- a/tasks/urls.py
+++ b/tasks/urls.py
@@ -27,6 +27,16 @@
views.TaskConfirmDeleteView.as_view(),
name="task-ui-confirm-delete",
),
+ path(
+ "/assign-users/",
+ views.TaskAssignUsersView.as_view(),
+ name="task-ui-assign-users",
+ ),
+ path(
+ f"/unassign-users/",
+ views.TaskUnassignUsersView.as_view(),
+ name="task-ui-unassign-users",
+ ),
# Subtask urls
path(
"/subtasks/create/",
diff --git a/tasks/views.py b/tasks/views.py
index 4df9a8517..6ef88084b 100644
--- a/tasks/views.py
+++ b/tasks/views.py
@@ -1,4 +1,5 @@
from django.conf import settings
+from django.contrib.auth import get_user_model
from django.contrib.auth.mixins import PermissionRequiredMixin
from django.db import OperationalError
from django.db import transaction
@@ -19,6 +20,7 @@
from tasks.filters import TaskFilter
from tasks.filters import TaskWorkflowFilter
from tasks.filters import WorkflowTemplateFilter
+from tasks.forms import AssignUsersForm
from tasks.forms import SubTaskCreateForm
from tasks.forms import TaskCreateForm
from tasks.forms import TaskDeleteForm
@@ -32,9 +34,11 @@
from tasks.forms import TaskWorkflowTemplateDeleteForm
from tasks.forms import TaskWorkflowTemplateUpdateForm
from tasks.forms import TaskWorkflowUpdateForm
+from tasks.forms import UnassignUsersForm
from tasks.models import Queue
from tasks.models import QueueItem
from tasks.models import Task
+from tasks.models import TaskAssignee
from tasks.models import TaskItem
from tasks.models import TaskItemTemplate
from tasks.models import TaskTemplate
@@ -42,6 +46,8 @@
from tasks.models import TaskWorkflowTemplate
from tasks.signals import set_current_instigator
+User = get_user_model()
+
class TaskListView(PermissionRequiredMixin, SortingMixin, WithPaginationListView):
model = Task
@@ -65,6 +71,36 @@ class TaskDetailView(PermissionRequiredMixin, DetailView):
template_name = "tasks/detail.jinja"
permission_required = "tasks.view_task"
+ def get_context_data(self, **kwargs) -> dict:
+ context = super().get_context_data(**kwargs)
+
+ # TODO: Factor out queries and place in TaskAssigeeQuerySet.
+ current_assignees = TaskAssignee.objects.filter(
+ task=self.get_object(),
+ # TODO:
+ # Using all task assignees is temporary for illustration as it
+ # doesn't align with the new approach of assigning users to tasks
+ # rather than assigning users to workbaskets (the old approach,
+ # uses tasks as an intermediary joining object).
+ # assignment_type=TaskAssignee.AssignmentType.GENERAL,
+ ).assigned()
+
+ context["current_assignees"] = [
+ {"pk": assignee.pk, "name": assignee.user.get_full_name()}
+ for assignee in current_assignees.order_by(
+ "user__first_name",
+ "user__last_name",
+ )
+ ]
+ context["assignable_users"] = [
+ {"pk": user.pk, "name": user.get_full_name()}
+ for user in User.objects.active_tms().exclude(
+ pk__in=current_assignees.values_list("user__pk", flat=True),
+ )
+ ]
+
+ return context
+
class TaskCreateView(PermissionRequiredMixin, CreateView):
model = Task
@@ -161,6 +197,61 @@ def get_context_data(self, **kwargs):
return context_data
+class TaskAssignUsersView(PermissionRequiredMixin, FormView):
+ permission_required = "tasks.add_taskassignee"
+ template_name = "tasks/assign_users.jinja"
+ form_class = AssignUsersForm
+
+ @property
+ def task(self):
+ return Task.objects.get(pk=self.kwargs["pk"])
+
+ def get_context_data(self, *args, **kwargs):
+ context = super().get_context_data(*args, **kwargs)
+ context["page_title"] = "Assign users to task"
+ return context
+
+ def form_valid(self, form):
+ form.assign_users(task=self.task, user_instigator=self.request.user)
+ return HttpResponseRedirect(self.get_success_url())
+
+ def get_success_url(self):
+ return reverse(
+ "workflow:task-ui-detail",
+ kwargs={"pk": self.kwargs["pk"]},
+ )
+
+
+class TaskUnassignUsersView(PermissionRequiredMixin, FormView):
+ permission_required = "tasks.change_taskassignee"
+ template_name = "tasks/assign_users.jinja"
+ form_class = UnassignUsersForm
+
+ @property
+ def task(self):
+ return Task.objects.get(pk=self.kwargs["pk"])
+
+ def get_context_data(self, *args, **kwargs):
+ context = super().get_context_data(*args, **kwargs)
+ context["page_title"] = "Unassign users from task"
+ return context
+
+ def get_form_kwargs(self):
+ kwargs = super().get_form_kwargs()
+ kwargs["task"] = self.task
+ return kwargs
+
+ def form_valid(self, form):
+ form.unassign_users(self.request.user)
+ return HttpResponseRedirect(self.get_success_url())
+
+ def get_success_url(self):
+ return reverse(
+ "workflow:task-ui-detail",
+ kwargs={"pk": self.kwargs["pk"]},
+ )
+
+
class SubTaskCreateView(PermissionRequiredMixin, CreateView):
model = Task
template_name = "layouts/create.jinja"
From 6590627c229af185650639d7a741b57a815677d0 Mon Sep 17 00:00:00 2001
From: Paul Pepper
Date: Tue, 11 Feb 2025 15:49:21 +0000
Subject: [PATCH 45/57] Renumber clashing migration
---
...013_alter_user_managers.py => 0014_alter_user_managers.py} | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
rename common/migrations/{0013_alter_user_managers.py => 0014_alter_user_managers.py} (73%)
diff --git a/common/migrations/0013_alter_user_managers.py b/common/migrations/0014_alter_user_managers.py
similarity index 73%
rename from common/migrations/0013_alter_user_managers.py
rename to common/migrations/0014_alter_user_managers.py
index 38149a5cd..1ff782ad2 100644
--- a/common/migrations/0013_alter_user_managers.py
+++ b/common/migrations/0014_alter_user_managers.py
@@ -1,4 +1,4 @@
-# Generated by Django 4.2.18 on 2025-02-11 14:47
+# Generated by Django 4.2.18 on 2025-02-11 15:48
from django.db import migrations
@@ -8,7 +8,7 @@
class Migration(migrations.Migration):
dependencies = [
- ("common", "0012_user_sso_uuid"),
+ ("common", "0013_versiongroup_common_vers_current_04c358_idx"),
]
operations = [
From 16ba584c6e0bd4d3488d0b38ad9f104c43702676 Mon Sep 17 00:00:00 2001
From: Marya Shariq
Date: Wed, 12 Feb 2025 08:31:59 +0000
Subject: [PATCH 46/57] Tp2000 1616 workflow task create view (#1363)
* added new urls for task workflow task create
* added 2 views for task workflow task create and confirm create
* added URL and views for workflow task create and confirm create
* failing unit test
* fixed failing test for workflow task create and confirm create view
* modified TaskBaseForm to explicitly add 'fields' attribute as per Django documentation guidelines
* removed redundant comment
* Rename migration file after naming conflict
* Remove last committed migration
* addressing PR comments including reordering assert statements in tests and changing URL to match convention
---------
Co-authored-by: Lauren Mullally
Co-authored-by: Paul Pepper
---
tasks/forms.py | 8 ++-
tasks/jinja2/tasks/workflows/detail.jinja | 2 +-
tasks/tests/test_views.py | 78 +++++++++++++++++++++++
tasks/urls.py | 10 +++
tasks/views.py | 45 +++++++++++++
5 files changed, 141 insertions(+), 2 deletions(-)
diff --git a/tasks/forms.py b/tasks/forms.py
index 1332679e6..5b6b4c71b 100644
--- a/tasks/forms.py
+++ b/tasks/forms.py
@@ -35,7 +35,13 @@
class TaskBaseForm(ModelForm):
class Meta:
model = Task
- exclude = ["parent_task", "creator"]
+ fields = [
+ "title",
+ "description",
+ "progress_state",
+ "category",
+ "workbasket",
+ ]
error_messages = {
"title": {
diff --git a/tasks/jinja2/tasks/workflows/detail.jinja b/tasks/jinja2/tasks/workflows/detail.jinja
index db9be5e80..b9f395a0a 100644
--- a/tasks/jinja2/tasks/workflows/detail.jinja
+++ b/tasks/jinja2/tasks/workflows/detail.jinja
@@ -8,7 +8,7 @@
{% if object_name == "workflow" %}
{% set task_object_name = "task" %}
- {% set task_object_create_url = "#TODO" %}
+ {% set task_object_create_url = url("workflow:task-workflow-task-ui-create", kwargs={"task_workflow_pk": object.pk}) %}
{% elif object_name == "workflow template" %}
{% set task_object_name = "task template" %}
{% set task_object_create_url = url("workflow:task-template-ui-create", kwargs={"workflow_template_pk": object.pk}) %}
diff --git a/tasks/tests/test_views.py b/tasks/tests/test_views.py
index 6a1a77155..8a5a4af6a 100644
--- a/tasks/tests/test_views.py
+++ b/tasks/tests/test_views.py
@@ -815,3 +815,81 @@ def test_task_and_workflow_list_view(valid_user_client, task, task_workflow):
assert table.select("tr:nth-child(2) > td:nth-child(1) > a:nth-child(1)")[
0
].text == str(task_workflow.summary_task.pk)
+
+
+def test_create_workflow_task_view(valid_user_client, task_workflow):
+ """Test the view for creating new Tasks for an existing workflow and the
+ confirmation view that a successful creation redirects to."""
+
+ assert task_workflow.get_tasks().count() == 0
+
+ progress_state = ProgressStateFactory.create()
+
+ create_url = reverse(
+ "workflow:task-workflow-task-ui-create",
+ kwargs={"task_workflow_pk": task_workflow.pk},
+ )
+
+ form_data = {
+ "title": factory.Faker("sentence"),
+ "description": factory.Faker("sentence"),
+ "progress_state": progress_state.pk,
+ }
+ create_response = valid_user_client.post(create_url, form_data)
+
+ assert task_workflow.get_tasks().count() == 1
+ assert create_response.status_code == 302
+
+ created_workflow_task = task_workflow.get_tasks().get()
+ confirmation_url = reverse(
+ "workflow:task-workflow-task-ui-confirm-create",
+ kwargs={"pk": created_workflow_task.pk},
+ )
+ assert create_response.url == confirmation_url
+
+ confirmation_response = valid_user_client.get(confirmation_url)
+ assert confirmation_response.status_code == 200
+
+ soup = BeautifulSoup(
+ confirmation_response.content.decode(confirmation_response.charset),
+ "html.parser",
+ )
+ assert created_workflow_task.title in soup.select("h1.govuk-panel__title")[0].text
+
+
+def test_workflow_delete_view_deletes_related_tasks(
+ valid_user_client,
+ task_workflow_single_task_item,
+):
+ """Tests that a workflow can be deleted (along with related Task and
+ TaskItem objects) and that the corresponding confirmation view returns a
+ HTTP 200 response."""
+
+ task_workflow_pk = task_workflow_single_task_item.pk
+ task_pk = task_workflow_single_task_item.get_tasks().get().pk
+
+ delete_url = task_workflow_single_task_item.get_url("delete")
+ delete_response = valid_user_client.post(delete_url)
+ assert delete_response.status_code == 302
+
+ assert not TaskWorkflow.objects.filter(
+ pk=task_workflow_pk,
+ ).exists()
+ assert not TaskItem.objects.filter(
+ workflow_id=task_workflow_pk,
+ ).exists()
+ assert not Task.objects.filter(pk=task_pk).exists()
+
+ confirmation_url = reverse(
+ "workflow:task-workflow-ui-confirm-delete",
+ kwargs={"pk": task_workflow_pk},
+ )
+ assert delete_response.url == confirmation_url
+
+ confirmation_response = valid_user_client.get(confirmation_url)
+ assert confirmation_response.status_code == 200
+
+ soup = BeautifulSoup(str(confirmation_response.content), "html.parser")
+ assert (
+ f"Workflow ID: {task_workflow_pk}" in soup.select(".govuk-panel__title")[0].text
+ )
diff --git a/tasks/urls.py b/tasks/urls.py
index 22817acdb..9acadb182 100644
--- a/tasks/urls.py
+++ b/tasks/urls.py
@@ -121,6 +121,16 @@
views.TaskWorkflowConfirmDeleteView.as_view(),
name="task-workflow-ui-confirm-delete",
),
+ path(
+ "/task/create/",
+ views.TaskWorkflowTaskCreateView.as_view(),
+ name="task-workflow-task-ui-create",
+ ),
+ path(
+ "task//confirm-create/",
+ views.TaskWorkflowTaskConfirmCreateView.as_view(),
+ name="task-workflow-task-ui-confirm-create",
+ ),
]
task_and_workflow_ui_patterns = [
diff --git a/tasks/views.py b/tasks/views.py
index 6ef88084b..f660413d5 100644
--- a/tasks/views.py
+++ b/tasks/views.py
@@ -649,6 +649,51 @@ def get_context_data(self, **kwargs):
return context_data
+class TaskWorkflowTaskCreateView(PermissionRequiredMixin, CreateView):
+ model = Task
+ template_name = "layouts/create.jinja"
+ permission_required = "tasks.add_task"
+ form_class = TaskCreateForm
+
+ def get_task_workflow(self):
+ """Get the associated TaskWorkflow via its pk in the URL."""
+ return TaskWorkflow.objects.get(
+ pk=self.kwargs["task_workflow_pk"],
+ )
+
+ def get_context_data(self, **kwargs) -> dict:
+ context = super().get_context_data(**kwargs)
+
+ context["page_title"] = "Create a task"
+ return context
+
+ def form_valid(self, form) -> HttpResponseRedirect:
+ with transaction.atomic():
+ self.object = form.save(user=self.request.user)
+ TaskItem.objects.create(
+ workflow=self.get_task_workflow(),
+ task=self.object,
+ )
+ return HttpResponseRedirect(self.get_success_url())
+
+ def get_success_url(self):
+ return reverse(
+ "workflow:task-workflow-task-ui-confirm-create",
+ kwargs={"pk": self.object.pk},
+ )
+
+
+class TaskWorkflowTaskConfirmCreateView(PermissionRequiredMixin, DetailView):
+ model = Task
+ template_name = "tasks/confirm_create.jinja"
+ permission_required = "tasks.add_task"
+
+ def get_context_data(self, **kwargs) -> dict:
+ context = super().get_context_data(**kwargs)
+ context["verbose_name"] = "task"
+ return context
+
+
class TaskWorkflowTemplateDetailView(
PermissionRequiredMixin,
QueuedItemManagementMixin,
From b1401a8b3822bf96b42dce8d325522adaaffe381 Mon Sep 17 00:00:00 2001
From: Dale Cannon <118175145+dalecannon@users.noreply.github.com>
Date: Wed, 12 Feb 2025 13:41:21 +0000
Subject: [PATCH 47/57] TP2000-1653 Enhance AutoCompleteField to support a
broader range of models (#1382)
* Ensure only authenticated and authorised users can access WorkBasketViewSet API endpoints
* Add a search filter for workbaskets
* Add autocomplete_label property to workbasket model
* Add new workbasket API endpoint optimised for use with AutoCompleteField in forms
* Support other types of model other than TrackedModels in AutoCompleteWidget
* Enable specifying a custom API source URL when using an AutoCompleteField
* Make workbasket an AutoCompleteField on Task forms
* Delegate validation to to_python so that any validation errors will be displayed as form errors
* Add note about search rank normalisation
* Add comment to caught exception for clarity
---
common/fields.py | 48 +++++++++++++++++----
common/tests/test_fields.py | 55 ++++++++++++++++++++++++
common/widgets.py | 8 +++-
tasks/forms.py | 17 +++++++-
workbaskets/filters.py | 69 +++++++++++++++++++++++++++++++
workbaskets/models.py | 4 ++
workbaskets/tests/test_filters.py | 32 ++++++++++++++
workbaskets/tests/test_views.py | 18 ++++++++
workbaskets/views/api.py | 39 +++++++++++++++++
9 files changed, 280 insertions(+), 10 deletions(-)
create mode 100644 common/tests/test_fields.py
create mode 100644 workbaskets/filters.py
create mode 100644 workbaskets/tests/test_filters.py
diff --git a/common/fields.py b/common/fields.py
index 88cd07cf7..d8fa44863 100644
--- a/common/fields.py
+++ b/common/fields.py
@@ -5,6 +5,7 @@
from dateutil.relativedelta import relativedelta
from django.contrib.postgres.fields import DateRangeField
from django.contrib.postgres.fields import DateTimeRangeField
+from django.core.exceptions import ValidationError
from django.db import models
from django.db.models.expressions import RawSQL
from django.forms import ModelChoiceField
@@ -146,20 +147,51 @@ class TaricDateTimeRangeField(DateTimeRangeField):
class AutoCompleteField(ModelChoiceField):
- def __init__(self, *args, **kwargs):
- qs = kwargs["queryset"]
- prefix = getattr(qs.model, "url_pattern_name_prefix", None)
- if not prefix:
- prefix = qs.model._meta.model_name
+ """
+ A form field that provides an AutoComplete widget for selecting a model
+ instance.
+
+ Args:
+ - queryset (QuerySet): A queryset of model instances that will populate the valid choices for the field.
+ - label (str): (Optional) A label for the field.
+ - help_text (str): (Optional) Help text for the field.
+ - url_pattern_name (str): (Optional) A custom pattern name to use for resolving the API source URL of AutoCompleteWidget.
+ - attrs (dict): (Optional) Additional attributes to pass to AutoCompleteWidget, e.g {"min-length": 2}.
+ """
+
+ def __init__(self, queryset, url_pattern_name=None, *args, **kwargs):
self.widget = AutocompleteWidget(
attrs={
"label": kwargs.get("label", ""),
"help_text": kwargs.get("help_text", ""),
- "source_url": reverse_lazy(f"{prefix}-list"),
+ "source_url": reverse_lazy(
+ self.get_url_pattern_name(queryset, url_pattern_name),
+ ),
**kwargs.pop("attrs", {}),
},
)
- super().__init__(*args, **kwargs)
+ super().__init__(queryset=queryset, *args, **kwargs)
+
+ def get_url_pattern_name(self, queryset, url_pattern_name: str | None) -> str:
+ """
+ Determines the URL pattern name to use for resolving the API source URL
+ of AutocompleteWidget.
+
+ If a custom name isn't provided, the name will be based on the
+ attributes of the model derived from the queryset.
+ """
+
+ if url_pattern_name is not None:
+ return url_pattern_name
+
+ prefix = getattr(queryset.model, "url_pattern_name_prefix", None)
+ if not prefix:
+ prefix = queryset.model._meta.model_name
+
+ return f"{prefix}-list"
def prepare_value(self, value):
- return self.to_python(value)
+ try:
+ return self.to_python(value)
+ except ValidationError:
+ return None
diff --git a/common/tests/test_fields.py b/common/tests/test_fields.py
new file mode 100644
index 000000000..f30ca312d
--- /dev/null
+++ b/common/tests/test_fields.py
@@ -0,0 +1,55 @@
+import pytest
+from django.urls import reverse_lazy
+
+from common.fields import AutoCompleteField
+from footnotes.models import Footnote
+from workbaskets.models import WorkBasket
+
+pytestmark = pytest.mark.django_db
+
+
+@pytest.mark.parametrize(
+ ("queryset, url_pattern_name, label, help_text, attrs, expected_url"),
+ [
+ (
+ Footnote.objects.all(),
+ None,
+ "Footnotes",
+ "Search for footnotes",
+ {"min-length": 2},
+ reverse_lazy("footnote-list"),
+ ),
+ (
+ WorkBasket.objects.all(),
+ "workbaskets:workbasket-autocomplete-list",
+ "Workbaskets",
+ "Search for workbaskets",
+ {},
+ reverse_lazy("workbaskets:workbasket-autocomplete-list"),
+ ),
+ ],
+)
+def test_autocompletefield_url_pattern_name(
+ queryset,
+ url_pattern_name,
+ label,
+ help_text,
+ attrs,
+ expected_url,
+):
+ """Tests that a custom URL pattern name can be provided for resolving the
+ API source URL of the field's autocomplete widget."""
+ field = AutoCompleteField(
+ queryset=queryset,
+ url_pattern_name=url_pattern_name,
+ label=label,
+ help_text=help_text,
+ attrs=attrs,
+ )
+
+ assert not field.queryset.difference(queryset)
+ assert field.label == label
+ assert field.help_text == help_text
+ for key, value in attrs.items():
+ assert field.widget.attrs.get(key) == value
+ assert field.widget.attrs.get("source_url") == expected_url
diff --git a/common/widgets.py b/common/widgets.py
index 05be8f890..a2b5e1629 100644
--- a/common/widgets.py
+++ b/common/widgets.py
@@ -44,13 +44,19 @@ class AutocompleteWidget(widgets.Widget):
template_name = "components/autocomplete.jinja"
def get_context(self, name, value, attrs=None):
+ from common.models import TrackedModel
+
if attrs is None:
attrs = {}
+
display_string = ""
- if value:
+
+ if isinstance(value, TrackedModel):
display_string = value.structure_code
if value.structure_description:
display_string = f"{display_string} - {value.structure_description}"
+ else:
+ display_string = value.autocomplete_label if value else ""
return {
"widget": {
diff --git a/tasks/forms.py b/tasks/forms.py
index 5b6b4c71b..d9e94bb97 100644
--- a/tasks/forms.py
+++ b/tasks/forms.py
@@ -17,6 +17,7 @@
from django.forms import Textarea
from django.utils.timezone import make_aware
+from common.fields import AutoCompleteField
from common.forms import BindNestedFormMixin
from common.forms import RadioNested
from common.forms import delete_form_for
@@ -55,6 +56,21 @@ class Meta:
},
}
+ workbasket = AutoCompleteField(
+ label="Workbasket",
+ help_text=(
+ "Search for a workbasket by typing in the workbasket's ID, TOPS/Jira number or description. "
+ "A dropdown list will appear after a few seconds. You can then select the correct workbasket from the dropdown list."
+ ),
+ queryset=WorkBasket.objects.editable(),
+ url_pattern_name="workbaskets:workbasket-autocomplete-list",
+ attrs={"min_length": 2},
+ error_messages={
+ "invalid_choice": "Select a workbasket that is in the editing state",
+ },
+ required=False,
+ )
+
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.init_fields()
@@ -62,7 +78,6 @@ def __init__(self, *args, **kwargs):
def init_fields(self):
self.fields["progress_state"].label = "Status"
- self.fields["workbasket"].queryset = WorkBasket.objects.editable()
def init_layout(self):
self.helper = FormHelper(self)
diff --git a/workbaskets/filters.py b/workbaskets/filters.py
new file mode 100644
index 000000000..0d3b08061
--- /dev/null
+++ b/workbaskets/filters.py
@@ -0,0 +1,69 @@
+from django.contrib.postgres.search import SearchRank
+from django.contrib.postgres.search import SearchVector
+from django.db.models import Case
+from django.db.models import FloatField
+from django.db.models import Q
+from django.db.models import Value
+from django.db.models import When
+
+from common.filters import TamatoFilterBackend
+from common.filters import TamatoFilterMixin
+
+
+class WorkBasketFilterMixin(TamatoFilterMixin):
+ search_fields = ("title", "reason")
+
+ def search_queryset(self, queryset, search_term):
+ """
+ Filters the queryset to results with `search_fields` (including PK)
+ containing or matching the `search_term`.
+
+ Results are ordered first by relevancy then by PK to favour newer
+ objects in the event of a tied rank value. Exact search term matches are
+ therefore prioritised over partial substring matches.
+
+ The search rank is normalised to penalise longer documents and those
+ with a high unique word count, improving relevance scoring.
+ """
+ NORMALISATION_LOG_LENGTH = 1
+ NORMALISATION_UNIQUE_WORDS = 8
+
+ search_term = self.get_search_term(search_term)
+ search_vector = SearchVector(*self.search_fields)
+ search_rank = SearchRank(
+ search_vector,
+ search_term,
+ normalization=Value(NORMALISATION_LOG_LENGTH).bitor(
+ Value(NORMALISATION_UNIQUE_WORDS),
+ ),
+ )
+
+ vector_queryset = queryset.annotate(search=search_vector)
+
+ exact_match_query = Q(search=search_term)
+ partial_match_query = Q(search__icontains=search_term)
+ query = exact_match_query | partial_match_query
+
+ try:
+ pk_query = Q(pk=int(search_term))
+ query |= pk_query
+ search_rank = Case(
+ When(pk=search_term, then=1),
+ default=search_rank,
+ output_field=FloatField(),
+ )
+ except ValueError:
+ # search_term cannot be converted to an integer
+ pass
+
+ return (
+ vector_queryset.annotate(
+ rank=search_rank,
+ )
+ .filter(query)
+ .order_by("-rank", "-pk")
+ )
+
+
+class WorkBasketAutoCompleteFilterBackEnd(TamatoFilterBackend, WorkBasketFilterMixin):
+ pass
diff --git a/workbaskets/models.py b/workbaskets/models.py
index aab2b464b..8ba2c7e32 100644
--- a/workbaskets/models.py
+++ b/workbaskets/models.py
@@ -491,6 +491,10 @@ def commodity_measure_changes_hash(self):
hash.update(value)
return hash.hexdigest()
+ @property
+ def autocomplete_label(self):
+ return f"({self.pk}) {self.title} - {self.reason}"
+
def __str__(self):
return f"({self.pk}) [{self.status}]"
diff --git a/workbaskets/tests/test_filters.py b/workbaskets/tests/test_filters.py
new file mode 100644
index 000000000..78913e832
--- /dev/null
+++ b/workbaskets/tests/test_filters.py
@@ -0,0 +1,32 @@
+import pytest
+
+from common.tests.factories import WorkBasketFactory
+from workbaskets.filters import WorkBasketAutoCompleteFilterBackEnd
+from workbaskets.models import WorkBasket
+
+pytestmark = pytest.mark.django_db
+
+
+def test_workbasket_autocomplete_filter_backend():
+ """Tests that WorkBasketAutoCompleteFilterBackEnd filters workbaskets by
+ exact and partial match of search term."""
+ workbasket1 = WorkBasketFactory.create(
+ pk=1111,
+ title="wb1",
+ reason="samedescription",
+ )
+ workbasket2 = WorkBasketFactory.create(
+ pk=2222,
+ title="wb2",
+ reason="samedescription",
+ )
+ queryset = WorkBasket.objects.all()
+
+ filter = WorkBasketAutoCompleteFilterBackEnd()
+ results = filter.search_queryset(queryset=queryset, search_term=str(workbasket1.pk))
+ assert workbasket1 in results
+ assert workbasket2 not in results
+
+ results = filter.search_queryset(queryset=queryset, search_term="same")
+ assert workbasket1 in results
+ assert workbasket2 in results
diff --git a/workbaskets/tests/test_views.py b/workbaskets/tests/test_views.py
index 124b75781..6971de859 100644
--- a/workbaskets/tests/test_views.py
+++ b/workbaskets/tests/test_views.py
@@ -43,6 +43,24 @@
pytestmark = pytest.mark.django_db
+def test_workbasket_autocomplete_api_endpoint(valid_user_api_client):
+ """Tests that workbasket autocomplete API endpoint allows searching for
+ workbaskets."""
+ factories.WorkBasketFactory.create(reason="irrelevant_workbasket")
+ workbasket = factories.WorkBasketFactory.create(reason="test")
+
+ autocomplete_api_url = reverse("workbaskets:workbasket-autocomplete-list")
+ response = valid_user_api_client.get(
+ path=autocomplete_api_url,
+ data={"search": "test"},
+ )
+
+ assert response.status_code == 200
+ assert response.data["count"] == 1
+ assert response.data["results"][0]["value"] == workbasket.pk
+ assert response.data["results"][0]["label"] == workbasket.autocomplete_label
+
+
def test_workbasket_create_form_creates_workbasket_object(
valid_user_api_client,
):
diff --git a/workbaskets/views/api.py b/workbaskets/views/api.py
index 4af575a24..9ce8cc81f 100644
--- a/workbaskets/views/api.py
+++ b/workbaskets/views/api.py
@@ -1,7 +1,13 @@
+from rest_framework import permissions
from rest_framework import renderers
+from rest_framework import status
from rest_framework import viewsets
+from rest_framework.decorators import action
+from rest_framework.response import Response
from common.renderers import TaricXMLRenderer
+from common.serializers import AutoCompleteSerializer
+from workbaskets.filters import WorkBasketAutoCompleteFilterBackEnd
from workbaskets.models import WorkBasket
from workbaskets.serializers import WorkBasketSerializer
@@ -17,9 +23,42 @@ class WorkBasketViewSet(viewsets.ModelViewSet):
renderers.BrowsableAPIRenderer,
TaricXMLRenderer,
]
+ permission_classes = [
+ permissions.IsAuthenticated,
+ permissions.DjangoModelPermissions,
+ ]
search_fields = ["title"]
def get_template_names(self, *args, **kwargs):
if self.detail:
return ["workbaskets/taric/workbasket_detail.xml"]
return ["workbaskets/taric/workbasket_list.xml"]
+
+ @action(
+ detail=False,
+ methods=["GET"],
+ url_path="autocomplete",
+ url_name="autocomplete-list",
+ )
+ def autocomplete(self, request):
+ """
+ Read-only API endpoint that allows users to search for workbaskets by
+ ID, title or reason (i.e description) using an autocomplete form field.
+
+ It returns a paginated JSON array of workbaskets that match the search
+ query.
+ """
+ filter_backend = WorkBasketAutoCompleteFilterBackEnd()
+ queryset = filter_backend.filter_queryset(
+ request,
+ WorkBasket.objects.all(),
+ self,
+ )
+
+ page = self.paginate_queryset(queryset)
+ if page is not None:
+ serializer = AutoCompleteSerializer(page, many=True)
+ return self.get_paginated_response(serializer.data)
+
+ serializer = AutoCompleteSerializer(queryset, many=True)
+ return Response(serializer.data, status=status.HTTP_200_OK)
From 56f1d4bd2632ead4056255c07967de1ff8ba0b23 Mon Sep 17 00:00:00 2001
From: Paul Pepper
Date: Wed, 12 Feb 2025 17:28:01 +0000
Subject: [PATCH 48/57] Restrict factory to workbasket assignees
---
common/tests/factories.py | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/common/tests/factories.py b/common/tests/factories.py
index d1c487b1b..5dfb18593 100644
--- a/common/tests/factories.py
+++ b/common/tests/factories.py
@@ -1569,7 +1569,12 @@ class SubTaskFactory(TaskFactory):
@factory.django.mute_signals(signals.pre_save)
class TaskAssigneeFactory(factory.django.DjangoModelFactory):
user = factory.SubFactory(UserFactory)
- assignment_type = FuzzyChoice(TaskAssignee.AssignmentType.values)
+ assignment_type = FuzzyChoice(
+ [
+ TaskAssignee.AssignmentType.WORKBASKET_WORKER,
+ TaskAssignee.AssignmentType.WORKBASKET_REVIEWER,
+ ],
+ )
task = factory.SubFactory(TaskFactory)
class Meta:
From 280ee8158ded5052b48d5807bcc32b3528a53e41 Mon Sep 17 00:00:00 2001
From: Dale Cannon <118175145+dalecannon@users.noreply.github.com>
Date: Thu, 13 Feb 2025 10:23:05 +0000
Subject: [PATCH 49/57] TP2000-1651 Fix sorting on list views (#1380)
* Amend breadcrumb formatting on Task delete view
* Avoid dereferencing null category on task and workflow list views
* Add parent task breadcrumb on subtask create view
* Amend workflow and worklfow template name references on their respective list views
* Add a return to workflow template button on task template confirm create view
* Truncate long descriptions on workflow template list view
* Use sentence case for table column headers on workflow template list view
* Show associated workbasket on Task detail view
* Preserve filtering when sorting on all Task & Workflow list views
* Add page caption and link to parent task for Subtask detail views
* Add a return to workflow template button on task template detail view
* Remove workbasket breadcrumb from task and workflow views
* Rename SortingMixin.clean_query_params() to build_sorting_urls()
* Add util function to get resolved URL of current view
* Add capability to SortingMixin to build sorting URLs
* Update usage of create_sortable_anchor
* Fix tests related to sorting changes
* Reposition Auto end-date measures summary row above comments
---
.../includes/certificates/tabs/measures.jinja | 8 ++-
certificates/tests/test_views.py | 1 +
certificates/views.py | 1 +
.../tabs/measures-declarable.jinja | 10 ++--
.../commodities/tabs/measures-defined.jinja | 8 ++-
.../components/create_sortable_anchor.jinja | 22 +++------
common/util.py | 10 ++++
common/views/mixins.py | 40 +++++++++++++++
.../includes/footnotes/tabs/measures.jinja | 8 ++-
footnotes/views.py | 1 +
.../includes/geo_areas/tabs/measures.jinja | 6 +--
geo_areas/views.py | 1 +
measures/jinja2/includes/measures/list.jinja | 10 ++--
measures/views/search.py | 15 +-----
.../includes/quotas/tabs/measures.jinja | 4 +-
quotas/jinja2/quotas/definitions.jinja | 1 -
quotas/jinja2/quotas/tables/definitions.jinja | 4 +-
quotas/views/base.py | 2 +-
.../includes/regulations/tabs/measures.jinja | 8 ++-
regulations/views.py | 1 +
tasks/jinja2/tasks/create.jinja | 26 ++++++++++
tasks/jinja2/tasks/delete.jinja | 18 +++----
tasks/jinja2/tasks/detail.jinja | 49 ++++++++++++++++---
tasks/jinja2/tasks/edit.jinja | 5 +-
tasks/jinja2/tasks/list.jinja | 12 +++--
tasks/jinja2/tasks/workflows/list.jinja | 14 ++++--
.../workflows/task-and-workflow-list.jinja | 12 +++--
.../task_template_confirm_create.jinja | 4 +-
.../workflows/task_template_detail.jinja | 5 ++
.../tasks/workflows/template_list.jinja | 20 +++++---
tasks/views.py | 28 ++++++++---
.../workbaskets/auto_end_date_measures.jinja | 8 ++-
workbaskets/jinja2/workbaskets/changes.jinja | 8 ++-
.../workbaskets/checks/missing_measures.jinja | 4 +-
.../workbaskets/summary-workbasket.jinja | 25 +++++-----
.../jinja2/workbaskets/violations.jinja | 8 ++-
workbaskets/views/ui.py | 32 ++++++------
37 files changed, 277 insertions(+), 162 deletions(-)
create mode 100644 tasks/jinja2/tasks/create.jinja
diff --git a/certificates/jinja2/includes/certificates/tabs/measures.jinja b/certificates/jinja2/includes/certificates/tabs/measures.jinja
index ddca45753..f8e2f7ea2 100644
--- a/certificates/jinja2/includes/certificates/tabs/measures.jinja
+++ b/certificates/jinja2/includes/certificates/tabs/measures.jinja
@@ -44,18 +44,16 @@
]) or "" }}
{% endfor %}
- {% set base_url = url('certificate-ui-detail-measures', kwargs={"certificate_type__sid":object.certificate_type.sid, "sid":object.sid}) %}
-
{% set commodity_code %}
- {{ create_sortable_anchor(request, "goods_nomenclature", "Commodity code", base_url) }}
+ {{ create_sortable_anchor(request, "Commodity code", sorting_urls["goods_nomenclature"]) }}
{% endset %}
{% set geo_area %}
- {{ create_sortable_anchor(request, "geo_area", "Geographical area", base_url) }}
+ {{ create_sortable_anchor(request, "Geographical area", sorting_urls["geo_area"]) }}
{% endset %}
{% set start_date %}
- {{ create_sortable_anchor(request, "start_date", "Start date", base_url) }}
+ {{ create_sortable_anchor(request, "Start date", sorting_urls["start_date"]) }}
{% endset %}
{% if paginator.count > 0 %}
diff --git a/certificates/tests/test_views.py b/certificates/tests/test_views.py
index 8c2fe80cf..d89d8ed76 100644
--- a/certificates/tests/test_views.py
+++ b/certificates/tests/test_views.py
@@ -373,6 +373,7 @@ def test_certificate_detail_measures_view_sorting_commodity(valid_user_client):
)
measures.append(measure)
commodity_codes = [measure.goods_nomenclature.item_id for measure in measures]
+ commodity_codes.sort()
url = reverse(
"certificate-ui-detail-measures",
diff --git a/certificates/views.py b/certificates/views.py
index a2caf6c4b..fb5b96409 100644
--- a/certificates/views.py
+++ b/certificates/views.py
@@ -168,6 +168,7 @@ class CertificateDetailMeasures(SortingMixin, WithPaginationListMixin, ListView)
paginate_by = 20
sort_by_fields = ["goods_nomenclature", "geo_area", "start_date"]
custom_sorting = {
+ "goods_nomenclature": "goods_nomenclature__item_id",
"geo_area": "geographical_area__area_id",
"start_date": "valid_between",
}
diff --git a/commodities/jinja2/includes/commodities/tabs/measures-declarable.jinja b/commodities/jinja2/includes/commodities/tabs/measures-declarable.jinja
index 62a54928a..95eeda96e 100644
--- a/commodities/jinja2/includes/commodities/tabs/measures-declarable.jinja
+++ b/commodities/jinja2/includes/commodities/tabs/measures-declarable.jinja
@@ -62,22 +62,20 @@
]) or "" }}
{% endfor %}
-{% set base_url = url('commodity-ui-detail-measures-declarable', args=[commodity.sid]) %}
-
{% set comm_code %}
- {{ create_sortable_anchor(request, "commodity", "Commodity code", base_url) }}
+ {{ create_sortable_anchor(request, "Commodity code", sorting_urls["commodity"]) }}
{% endset %}
{% set measure_type %}
- {{ create_sortable_anchor(request, "measure_type", "Measure type", base_url) }}
+ {{ create_sortable_anchor(request, "Measure type", sorting_urls["measure_type"]) }}
{% endset %}
{% set geo_area %}
- {{ create_sortable_anchor(request, "geo_area", "Geographical area", base_url) }}
+ {{ create_sortable_anchor(request, "Geographical area", sorting_urls["geo_area"]) }}
{% endset %}
{% set start_date %}
- {{ create_sortable_anchor(request, "start_date", "Start date", base_url) }}
+ {{ create_sortable_anchor(request, "Start date", sorting_urls["start_date"]) }}
{% endset %}
{% if paginator.count > 0 %}
diff --git a/commodities/jinja2/includes/commodities/tabs/measures-defined.jinja b/commodities/jinja2/includes/commodities/tabs/measures-defined.jinja
index 1b0b19ac6..41d379a62 100644
--- a/commodities/jinja2/includes/commodities/tabs/measures-defined.jinja
+++ b/commodities/jinja2/includes/commodities/tabs/measures-defined.jinja
@@ -56,18 +56,16 @@
]) or "" }}
{% endfor %}
-{% set base_url = url('commodity-ui-detail-measures-as-defined', args=[commodity.sid]) %}
-
{% set measure_type %}
- {{ create_sortable_anchor(request, "measure_type", "Measure type", base_url) }}
+ {{ create_sortable_anchor(request, "Measure type", sorting_urls["measure_type"]) }}
{% endset %}
{% set geo_area %}
- {{ create_sortable_anchor(request, "geo_area", "Geographical area", base_url) }}
+ {{ create_sortable_anchor(request, "Geographical area", sorting_urls["geo_area"]) }}
{% endset %}
{% set start_date %}
- {{ create_sortable_anchor(request, "start_date", "Start date", base_url) }}
+ {{ create_sortable_anchor(request, "Start date", sorting_urls["start_date"]) }}
{% endset %}
{% if paginator.count > 0 %}
diff --git a/common/jinja2/components/create_sortable_anchor.jinja b/common/jinja2/components/create_sortable_anchor.jinja
index b83f591dc..3db8ae818 100644
--- a/common/jinja2/components/create_sortable_anchor.jinja
+++ b/common/jinja2/components/create_sortable_anchor.jinja
@@ -1,21 +1,11 @@
-
-{% macro create_sortable_anchor(request, sort_by, title, base_url, query_params=False, fragment="") %}
-
- {% if query_params %}
- {% set query_string_prefix = "&"%}
- {% else %}
- {% set query_string_prefix = "?"%}
- {% endif %}
-
- {% if request.GET.sort_by == sort_by and request.GET.ordered == "desc" %}
-
-
- {{ title }}
-
+{% macro create_sortable_anchor(request, title, url, fragment="") %}
+ {% if request.GET.ordered == "desc" %}
+
+ {{ title }}
+
{% else %}
-
+
{{ title }}
{% endif %}
-
{% endmacro %}
diff --git a/common/util.py b/common/util.py
index aef0adf8f..2f3539872 100644
--- a/common/util.py
+++ b/common/util.py
@@ -47,7 +47,9 @@
from django.db.models.functions.text import Lower
from django.db.models.functions.text import Upper
from django.db.transaction import atomic
+from django.http import HttpRequest
from django.template import loader
+from django.urls import reverse
from django.utils import timezone
from lxml import etree
from psycopg.types.range import DateRange
@@ -809,3 +811,11 @@ def get_related_names(instance, related_model) -> list[str]:
if field.one_to_many and issubclass(field.related_model, related_model):
related_names.append(field.get_accessor_name())
return related_names
+
+
+def get_current_view_url(request: HttpRequest) -> str:
+ """Returns the resolved URL of the current view."""
+ view_name = request.resolver_match.view_name
+ args = request.resolver_match.args
+ kwargs = request.resolver_match.kwargs
+ return reverse(view_name, args=args, kwargs=kwargs)
diff --git a/common/views/mixins.py b/common/views/mixins.py
index 6cfff811e..11581c534 100644
--- a/common/views/mixins.py
+++ b/common/views/mixins.py
@@ -8,11 +8,13 @@
from django.db.models import Model
from django.db.models import QuerySet
from django.http import Http404
+from django.http import QueryDict
from common.business_rules import BusinessRule
from common.business_rules import BusinessRuleViolation
from common.models import TrackedModel
from common.pagination import build_pagination_list
+from common.util import get_current_view_url
class WithPaginationListMixin:
@@ -162,3 +164,41 @@ def get_ordering(self):
else:
return None
+
+ def remove_sorting_params(self) -> QueryDict:
+ """Returns a cleaned copy of GET params having removed 'sort_by' and
+ 'ordered'."""
+ query_params = self.request.GET.copy()
+ query_params.pop("sort_by", None)
+ query_params.pop("ordered", None)
+ return query_params
+
+ def build_sorting_urls(self) -> dict[str, str]:
+ """
+ Returns a dictionary mapping a sort_by_field to its sorting URL.
+
+ Appends `sort_by` and `ordered` query params to the current view URL for each field in `sort_by_fields`,
+ while keeping other query params (such as those used for filtering) in place.
+ """
+ urls = {}
+ view_url = get_current_view_url(self.request)
+ query_params = self.remove_sorting_params().urlencode()
+ ordering = self.get_ordering()
+
+ for sort_by in self.sort_by_fields:
+ if ordering and ordering.startswith("-"):
+ sorting_params = f"sort_by={sort_by}&ordered=asc"
+ else:
+ sorting_params = f"sort_by={sort_by}&ordered=desc"
+ urls[sort_by] = (
+ f"{view_url}?{query_params}&{sorting_params}"
+ if query_params
+ else f"{view_url}?{sorting_params}"
+ )
+
+ return urls
+
+ def get_context_data(self, **kwargs):
+ context = super().get_context_data(**kwargs)
+ context["sorting_urls"] = self.build_sorting_urls()
+ return context
diff --git a/footnotes/jinja2/includes/footnotes/tabs/measures.jinja b/footnotes/jinja2/includes/footnotes/tabs/measures.jinja
index 40d0d9e21..36b2fef14 100644
--- a/footnotes/jinja2/includes/footnotes/tabs/measures.jinja
+++ b/footnotes/jinja2/includes/footnotes/tabs/measures.jinja
@@ -44,18 +44,16 @@
]) or "" }}
{% endfor %}
- {% set base_url = url('footnote-ui-detail-measures', kwargs={"footnote_type__footnote_type_id":object.footnote_type.footnote_type_id, "footnote_id":object.footnote_id}) %}
-
{% set commodity_code %}
- {{ create_sortable_anchor(request, "goods_nomenclature", "Commodity code", base_url) }}
+ {{ create_sortable_anchor(request, "Commodity code", sorting_urls["goods_nomenclature"]) }}
{% endset %}
{% set geo_area %}
- {{ create_sortable_anchor(request, "geo_area", "Geographical area", base_url) }}
+ {{ create_sortable_anchor(request, "Geographical area", sorting_urls["geo_area"]) }}
{% endset %}
{% set start_date %}
- {{ create_sortable_anchor(request, "start_date", "Start date", base_url) }}
+ {{ create_sortable_anchor(request, "Start date", sorting_urls["start_date"]) }}
{% endset %}
{% if paginator.count > 0 %}
diff --git a/footnotes/views.py b/footnotes/views.py
index 5e701d8f4..add4f964a 100644
--- a/footnotes/views.py
+++ b/footnotes/views.py
@@ -203,6 +203,7 @@ class FootnoteDetailMeasures(SortingMixin, WithPaginationListMixin, ListView):
paginate_by = 20
sort_by_fields = ["goods_nomenclature", "geo_area", "start_date"]
custom_sorting = {
+ "goods_nomenclature": "goods_nomenclature__item_id",
"geo_area": "geographical_area__area_id",
"start_date": "valid_between",
}
diff --git a/geo_areas/jinja2/includes/geo_areas/tabs/measures.jinja b/geo_areas/jinja2/includes/geo_areas/tabs/measures.jinja
index 4edd73849..c491c78c9 100644
--- a/geo_areas/jinja2/includes/geo_areas/tabs/measures.jinja
+++ b/geo_areas/jinja2/includes/geo_areas/tabs/measures.jinja
@@ -40,14 +40,12 @@
]) or "" }}
{% endfor %}
- {% set base_url = url('geo_area-ui-detail-measures', kwargs={"sid": object.sid}) %}
-
{% set commodity_code %}
- {{ create_sortable_anchor(request, "goods_nomenclature", "Commodity code", base_url) }}
+ {{ create_sortable_anchor(request, "Commodity code", sorting_urls["goods_nomenclature"]) }}
{% endset %}
{% set start_date %}
- {{ create_sortable_anchor(request, "start_date", "Start date", base_url) }}
+ {{ create_sortable_anchor(request, "Start date", sorting_urls["start_date"]) }}
{% endset %}
{% if paginator.count > 0 %}
diff --git a/geo_areas/views.py b/geo_areas/views.py
index cd17b212e..546caaf78 100644
--- a/geo_areas/views.py
+++ b/geo_areas/views.py
@@ -148,6 +148,7 @@ class GeoAreaDetailMeasures(SortingMixin, WithPaginationListMixin, ListView):
paginate_by = 20
sort_by_fields = ["goods_nomenclature", "start_date"]
custom_sorting = {
+ "goods_nomenclature": "goods_nomenclature__item_id",
"start_date": "valid_between",
}
diff --git a/measures/jinja2/includes/measures/list.jinja b/measures/jinja2/includes/measures/list.jinja
index bbb3a31e4..37886c290 100644
--- a/measures/jinja2/includes/measures/list.jinja
+++ b/measures/jinja2/includes/measures/list.jinja
@@ -13,23 +13,23 @@
{%- endset %}
{% set sid %}
- {{ create_sortable_anchor(request, "sid", "SID", base_url, query_params) }}
+ {{ create_sortable_anchor(request, "SID", sorting_urls["sid"]) }}
{% endset %}
{% set measure_type %}
- {{ create_sortable_anchor(request, "measure_type", "Type", base_url, query_params) }}
+ {{ create_sortable_anchor(request, "Type", sorting_urls["measure_type"]) }}
{% endset %}
{% set geo_area %}
- {{ create_sortable_anchor(request, "geo_area", "Geographical area", base_url, query_params) }}
+ {{ create_sortable_anchor(request, "Geographical area", sorting_urls["geo_area"]) }}
{% endset %}
{% set start_date %}
- {{ create_sortable_anchor(request, "start_date", "Start date", base_url, query_params) }}
+ {{ create_sortable_anchor(request, "Start date", sorting_urls["start_date"]) }}
{% endset %}
{% set end_date %}
- {{ create_sortable_anchor(request, "end_date", "End date", base_url, query_params) }}
+ {{ create_sortable_anchor(request, "End date", sorting_urls["end_date"]) }}
{% endset %}
{# sets out form #}
diff --git a/measures/views/search.py b/measures/views/search.py
index dec0f6ba2..eeee5bb7b 100644
--- a/measures/views/search.py
+++ b/measures/views/search.py
@@ -84,16 +84,6 @@ def get_form_kwargs(self):
kwargs["objects"] = page.object_list
return kwargs
- def cleaned_query_params(self):
- # Remove the sort_by and ordered params in order to stop them being duplicated in the base url
- if "sort_by" and "ordered" in self.filterset.data:
- cleaned_filterset = self.filterset.data.copy()
- cleaned_filterset.pop("sort_by")
- cleaned_filterset.pop("ordered")
- return cleaned_filterset
- else:
- return self.filterset.data
-
def selected_filter_formatter(self) -> List[List[str]]:
"""
A function that formats the selected filter choices into nicely written
@@ -242,6 +232,7 @@ def get_context_data(self, **kwargs):
),
"selected_filter_lists": self.selected_filter_formatter(),
"workbasket": self.workbasket,
+ "sorting_urls": self.build_sorting_urls(),
},
)
if context["has_previous_page"]:
@@ -253,10 +244,6 @@ def get_context_data(self, **kwargs):
pk__in=self.measure_selections,
).values_list("sid", flat=True)
- context["query_params"] = True
- context["base_url"] = (
- f'{reverse("measure-ui-list")}?{urlencode(self.cleaned_query_params())}'
- )
return context
def get_initial(self):
diff --git a/quotas/jinja2/includes/quotas/tabs/measures.jinja b/quotas/jinja2/includes/quotas/tabs/measures.jinja
index 761e6a562..33fabd967 100644
--- a/quotas/jinja2/includes/quotas/tabs/measures.jinja
+++ b/quotas/jinja2/includes/quotas/tabs/measures.jinja
@@ -16,10 +16,8 @@
]) or "" }}
{% endfor %}
-{% set base_url = url('quota-ui-detail', args=[object.sid]) %}
-
{% set commodity_code %}
- {{ create_sortable_anchor(request, "goods_nomenclature", "Commodity code", base_url, "#measures") }}
+ {{ create_sortable_anchor(request, "Commodity code", sorting_urls["goods_nomenclature"], "#measures") }}
{% endset %}
diff --git a/quotas/jinja2/quotas/definitions.jinja b/quotas/jinja2/quotas/definitions.jinja
index 614da2ec4..80974d22e 100644
--- a/quotas/jinja2/quotas/definitions.jinja
+++ b/quotas/jinja2/quotas/definitions.jinja
@@ -61,7 +61,6 @@
{{ fake_tabs(links) }}
- {% set base_url = url('quota_definition-ui-list', args=[quota.sid] ) %}
{% if quota_type == "blocking_periods" %}
{% include "quotas/tables/blocking_periods.jinja" %}
diff --git a/quotas/jinja2/quotas/tables/definitions.jinja b/quotas/jinja2/quotas/tables/definitions.jinja
index 494a25dca..3f6f48aed 100644
--- a/quotas/jinja2/quotas/tables/definitions.jinja
+++ b/quotas/jinja2/quotas/tables/definitions.jinja
@@ -65,11 +65,11 @@
{% endfor %}
{% set sid %}
- {{ create_sortable_anchor(request, "sid", "Quota definition SID", base_url) }}
+ {{ create_sortable_anchor(request, "Quota definition SID", sorting_urls["sid"]) }}
{% endset %}
{% set start_date %}
- {{ create_sortable_anchor(request, "valid_between", "Start date", base_url) }}
+ {{ create_sortable_anchor(request, "Start date", sorting_urls["valid_between"]) }}
{% endset %}
{{ govukTable({
diff --git a/quotas/views/base.py b/quotas/views/base.py
index 49519047b..0b7891f1a 100644
--- a/quotas/views/base.py
+++ b/quotas/views/base.py
@@ -190,7 +190,7 @@ class QuotaList(QuotaOrderNumberMixin, TamatoListView):
filterset_class = QuotaFilter
-class QuotaDetail(QuotaOrderNumberMixin, TrackedModelDetailView, SortingMixin):
+class QuotaDetail(QuotaOrderNumberMixin, SortingMixin, TrackedModelDetailView):
template_name = "quotas/detail.jinja"
sort_by_fields = ["goods_nomenclature"]
diff --git a/regulations/jinja2/includes/regulations/tabs/measures.jinja b/regulations/jinja2/includes/regulations/tabs/measures.jinja
index 00f24c85a..ffcd52ed9 100644
--- a/regulations/jinja2/includes/regulations/tabs/measures.jinja
+++ b/regulations/jinja2/includes/regulations/tabs/measures.jinja
@@ -45,18 +45,16 @@
]) or "" }}
{% endfor %}
- {% set base_url = url('regulation-ui-detail-measures', kwargs={"role_type": object.role_type, "regulation_id": object.regulation_id}) %}
-
{% set commodity_code %}
- {{ create_sortable_anchor(request, "goods_nomenclature", "Commodity code", base_url) }}
+ {{ create_sortable_anchor(request, "Commodity code", sorting_urls["goods_nomenclature"]) }}
{% endset %}
{% set geo_area %}
- {{ create_sortable_anchor(request, "geo_area", "Geographical area", base_url) }}
+ {{ create_sortable_anchor(request, "Geographical area", sorting_urls["geo_area"]) }}
{% endset %}
{% set start_date %}
- {{ create_sortable_anchor(request, "start_date", "Start date", base_url) }}
+ {{ create_sortable_anchor(request, "Start date", sorting_urls["start_date"]) }}
{% endset %}
{% if paginator.count > 0 %}
diff --git a/regulations/views.py b/regulations/views.py
index 0634d0d8d..0270cf8da 100644
--- a/regulations/views.py
+++ b/regulations/views.py
@@ -79,6 +79,7 @@ class RegulationDetailMeasures(SortingMixin, WithPaginationListMixin, ListView):
paginate_by = 20
sort_by_fields = ["goods_nomenclature", "geo_area", "start_date"]
custom_sorting = {
+ "goods_nomenclature": "goods_nomenclature__item_id",
"geo_area": "geographical_area__area_id",
"start_date": "valid_between",
}
diff --git a/tasks/jinja2/tasks/create.jinja b/tasks/jinja2/tasks/create.jinja
new file mode 100644
index 000000000..2e25bdbaf
--- /dev/null
+++ b/tasks/jinja2/tasks/create.jinja
@@ -0,0 +1,26 @@
+{% extends 'layouts/form.jinja' %}
+
+{% block breadcrumb %}
+ {% if parent_task %}
+ {{ breadcrumbs(request, [
+ {"text": "Find and view tasks", "href": url("workflow:task-ui-list")},
+ {"text": "Task: " ~ parent_task.title, "href": parent_task.get_url("detail")},
+ {"text": page_title}
+ ],
+ with_workbasket=False,
+ ) }}
+ {% else %}
+ {{ breadcrumbs(request, [
+ {"text": "Find and view tasks", "href": url("workflow:task-ui-list")},
+ {"text": page_title}
+ ],
+ with_workbasket=False,
+ ) }}
+ {% endif %}
+{% endblock %}
+
+{% block form %}
+ {% call django_form() %}
+ {{ crispy(form) }}
+ {% endcall %}
+{% endblock %}
diff --git a/tasks/jinja2/tasks/delete.jinja b/tasks/jinja2/tasks/delete.jinja
index 574aa28a5..0de6bacb0 100644
--- a/tasks/jinja2/tasks/delete.jinja
+++ b/tasks/jinja2/tasks/delete.jinja
@@ -4,7 +4,14 @@
{% from "components/warning-text/macro.njk" import govukWarningText %}
{% from "components/button/macro.njk" import govukButton %}
-{% set page_title = "Delete my Task:" ~ object.title %}
+{% block breadcrumb %}
+ {{ breadcrumbs(request, [
+ {"text": "Find and view tasks", "href": url("workflow:task-ui-list")},
+ {"text": verbose_name|capitalize ~ ": " ~ object.title, "href": url("workflow:task-ui-detail", kwargs={"pk": object.pk})},
+ {"text": page_title}
+ ])
+ }}
+{% endblock %}
{% block content %}
@@ -30,12 +37,3 @@
{% endblock %}
{% endblock %}
-
-{% block breadcrumb %}
- {{ breadcrumbs(request, [
- {"text": "Find and view tasks", "href": url("workflow:task-ui-list")},
- {"text": verbose_name ~ " :" ~ object.title, "href": url("workflow:task-ui-detail", kwargs={"pk": object.pk})},
- {"text": page_title}
- ])
- }}
-{% endblock %}
diff --git a/tasks/jinja2/tasks/detail.jinja b/tasks/jinja2/tasks/detail.jinja
index a3f94a83c..0750d487e 100644
--- a/tasks/jinja2/tasks/detail.jinja
+++ b/tasks/jinja2/tasks/detail.jinja
@@ -4,11 +4,36 @@
{% set page_title = "Task: " ~ object.title %}
-{%if object.parent_task%}
+{% if object.parent_task %}
{% set edit_url = url("workflow:subtask-ui-update", kwargs={"pk": object.pk}) %}
+ {% set title_caption = "Subtask" %}
+ {% set parent_summary_list_row %}
+
+ {% endset %}
{% else %}
{% set edit_url = url("workflow:task-ui-update", kwargs={"pk": object.pk}) %}
-{% endif%}
+{% endif %}
+
+{% set workbasket_linked_id %}
+ {% if object.workbasket -%}
+
{{ object.workbasket.pk }}
+ {% else %}
+ -
+ {% endif -%}
+{% endset %}
{% macro display_assignees(assignees) %}
{% if not assignees %}
@@ -28,7 +53,8 @@
{{ breadcrumbs(request, [
{"text": "Find and view tasks", "href": url("workflow:task-ui-list")},
{"text": page_title}
- ]
+ ],
+ with_workbasket=False,
) }}
{% endblock %}
@@ -46,6 +72,13 @@
{% block content %}
+ {% if title_caption %}
+ {% block page_subtitle %}
+
+ {{ title_caption }}
+
+ {% endblock %}
+ {% endif %}
{{ page_title }}
@@ -94,7 +127,7 @@
Category
- {{ object.category }}
+ {{ object.category or "-" }}
Change category
@@ -118,13 +151,17 @@
Workbasket
- {{ workbasket_summary_link }}
+ {{ workbasket_linked_id }}
- Change workbasket
+ Change status
+ {% if object.parent_task %}
+ {{ parent_summary_list_row }}
+ {% endif %}
+
Created
diff --git a/tasks/jinja2/tasks/edit.jinja b/tasks/jinja2/tasks/edit.jinja
index d2d0bde91..0f8ebe1ee 100644
--- a/tasks/jinja2/tasks/edit.jinja
+++ b/tasks/jinja2/tasks/edit.jinja
@@ -7,8 +7,9 @@
{"text": "Find and view tasks", "href": url("workflow:task-ui-list")},
{"text": "Task: " ~ object.title, "href": url("workflow:task-ui-detail", kwargs={"pk": object.pk})},
{"text": page_title}
- ])
- }}
+ ],
+ with_workbasket=False,
+ ) }}
{% endblock %}
{% block form %}
diff --git a/tasks/jinja2/tasks/list.jinja b/tasks/jinja2/tasks/list.jinja
index f516e5ca1..b85fe2a7a 100644
--- a/tasks/jinja2/tasks/list.jinja
+++ b/tasks/jinja2/tasks/list.jinja
@@ -5,7 +5,13 @@
{% set page_title = "Find and view tasks" %}
-{% set base_url = url("workflow:task-ui-list") %}
+{% block breadcrumb %}
+ {{ breadcrumbs(request, [
+ {"text": page_title}
+ ],
+ with_workbasket=False,
+ ) }}
+{% endblock %}
{% block content %}
{{ page_title }}
@@ -50,7 +56,7 @@
{{ table_rows.append([
{"text": task_linked_id},
{"text": object.title},
- {"text": object.category},
+ {"text": object.category or "-"},
{"text": object.progress_state},
{"text": workbasket_linked_id},
{"text": "{:%d %b %Y}".format(object.created_at)},
@@ -64,7 +70,7 @@
{"text": "Category"},
{"text": "Status"},
{"text": "Workbasket"},
- {"text": create_sortable_anchor(request, "created_at", "Created", base_url)},
+ {"text": create_sortable_anchor(request, "Created", sorting_urls["created_at"])},
],
"rows": table_rows
}) }}
diff --git a/tasks/jinja2/tasks/workflows/list.jinja b/tasks/jinja2/tasks/workflows/list.jinja
index fd76656f0..18b7a72a9 100644
--- a/tasks/jinja2/tasks/workflows/list.jinja
+++ b/tasks/jinja2/tasks/workflows/list.jinja
@@ -3,9 +3,15 @@
{% from "components/table/macro.njk" import govukTable %}
{% from "components/create_sortable_anchor.jinja" import create_sortable_anchor %}
-{% set page_title = "Find and view tasks workflows" %}
+{% set page_title = "Find and view workflows" %}
-{% set base_url = url("workflow:task-workflow-ui-list") %}
+{% block breadcrumb %}
+ {{ breadcrumbs(request, [
+ {"text": page_title}
+ ],
+ with_workbasket=False,
+ ) }}
+{% endblock %}
{% block content %}
{{ page_title }}
@@ -53,7 +59,7 @@
{{ table_rows.append([
{"text": task_linked_id},
{"text": object.title},
- {"text": object.category},
+ {"text": object.category or "-"},
{"text": object.progress_state},
{"text": workbasket_linked_id},
{"text": "{:%d %b %Y}".format(object.created_at)},
@@ -67,7 +73,7 @@
{"text": "Category"},
{"text": "Status"},
{"text": "Workbasket"},
- {"text": create_sortable_anchor(request, "created_at", "Created", base_url)},
+ {"text": create_sortable_anchor(request, "Created", sorting_urls["created_at"])},
],
"rows": table_rows
}) }}
diff --git a/tasks/jinja2/tasks/workflows/task-and-workflow-list.jinja b/tasks/jinja2/tasks/workflows/task-and-workflow-list.jinja
index e0e5339ae..b9f7f8d1d 100644
--- a/tasks/jinja2/tasks/workflows/task-and-workflow-list.jinja
+++ b/tasks/jinja2/tasks/workflows/task-and-workflow-list.jinja
@@ -5,7 +5,13 @@
{% set page_title = "Find and view tasks and workflows" %}
-{% set base_url = url("workflow:task-and-workflow-ui-list") %}
+{% block breadcrumb %}
+ {{ breadcrumbs(request, [
+ {"text": page_title}
+ ],
+ with_workbasket=False,
+ ) }}
+{% endblock %}
{% block content %}
{{ page_title }}
@@ -20,7 +26,7 @@
Search and filter
-
@@ -86,7 +92,7 @@
{"text": "Category"},
{"text": "Status"},
{"text": "Workbasket"},
- {"text": create_sortable_anchor(request, "created_at", "Created", base_url)},
+ {"text": create_sortable_anchor(request, "Created", sorting_urls["created_at"])},
],
"rows": table_rows
}) }}
diff --git a/tasks/jinja2/tasks/workflows/task_template_confirm_create.jinja b/tasks/jinja2/tasks/workflows/task_template_confirm_create.jinja
index 0c532419c..e3dcac8b0 100644
--- a/tasks/jinja2/tasks/workflows/task_template_confirm_create.jinja
+++ b/tasks/jinja2/tasks/workflows/task_template_confirm_create.jinja
@@ -42,8 +42,8 @@
"classes": "govuk-button"
}) }}
{{ govukButton({
- "text": "Return to homepage",
- "href": url("home"),
+ "text": "Return to workflow template",
+ "href": url("workflow:task-workflow-template-ui-detail", kwargs={"pk": task_workflow_template.pk}),
"classes": "govuk-button--secondary"
}) }}
{% endblock %}
diff --git a/tasks/jinja2/tasks/workflows/task_template_detail.jinja b/tasks/jinja2/tasks/workflows/task_template_detail.jinja
index f42ff3f41..286cd9ad5 100644
--- a/tasks/jinja2/tasks/workflows/task_template_detail.jinja
+++ b/tasks/jinja2/tasks/workflows/task_template_detail.jinja
@@ -68,5 +68,10 @@
"classes": "govuk-button--warning"
}) }}
+ {{ govukButton({
+ "text": "Return to workflow template",
+ "href": url("workflow:task-workflow-template-ui-detail", kwargs={"pk": task_workflow_template.pk}),
+ "classes": "govuk-button--secondary"
+ }) }}
{% endblock %}
diff --git a/tasks/jinja2/tasks/workflows/template_list.jinja b/tasks/jinja2/tasks/workflows/template_list.jinja
index 3be85f30e..3900678c5 100644
--- a/tasks/jinja2/tasks/workflows/template_list.jinja
+++ b/tasks/jinja2/tasks/workflows/template_list.jinja
@@ -3,15 +3,21 @@
{% from "components/table/macro.njk" import govukTable %}
{% from "components/create_sortable_anchor.jinja" import create_sortable_anchor %}
-{% set page_title = "Find and view task workflow templates" %}
+{% set page_title = "Find and view workflow templates" %}
-{% set base_url = url("workflow:task-workflow-template-ui-list") %}
+{% block breadcrumb %}
+ {{ breadcrumbs(request, [
+ {"text": page_title}
+ ],
+ with_workbasket=False,
+ ) }}
+{% endblock %}
{% block content %}
{{ page_title }}
- Search for task workflow templates.
- Alternatively, create a new task workflow template.
+ Search for workflow templates.
+ Alternatively, create a new workflow template.
@@ -39,7 +45,7 @@
{{ table_rows.append([
{"text": workflow_template_linked_id},
{"text": object.title},
- {"text": object.description},
+ {"text": object.description|truncate(75)},
{"text": object.creator.get_displayname() if object.creator else "Unknown"},
{"text": object.created_at.strftime(datetime_format)},
{"text": object.updated_at.strftime(datetime_format)},
@@ -52,8 +58,8 @@
{"text": "Title"},
{"text": "Description"},
{"text": "Created by"},
- {"text": create_sortable_anchor(request, "created_at", "Created", base_url)},
- {"text": create_sortable_anchor(request, "updated_at", "Last Updated", base_url)},
+ {"text": create_sortable_anchor(request, "Created", sorting_urls["created_at"])},
+ {"text": create_sortable_anchor(request, "Last updated", sorting_urls["updated_at"])},
],
"rows": table_rows
}) }}
diff --git a/tasks/views.py b/tasks/views.py
index f660413d5..f7027a915 100644
--- a/tasks/views.py
+++ b/tasks/views.py
@@ -65,6 +65,10 @@ def get_queryset(self):
queryset = queryset.order_by(*ordering)
return queryset
+ def get_context_data(self, *, object_list=None, **kwargs):
+ context = super().get_context_data(object_list=object_list, **kwargs)
+ return context
+
class TaskDetailView(PermissionRequiredMixin, DetailView):
model = Task
@@ -104,7 +108,7 @@ def get_context_data(self, **kwargs) -> dict:
class TaskCreateView(PermissionRequiredMixin, CreateView):
model = Task
- template_name = "layouts/create.jinja"
+ template_name = "tasks/create.jinja"
permission_required = "tasks.add_task"
form_class = TaskCreateForm
@@ -254,27 +258,31 @@ def get_success_url(self):
class SubTaskCreateView(PermissionRequiredMixin, CreateView):
model = Task
- template_name = "layouts/create.jinja"
+ template_name = "tasks/create.jinja"
permission_required = "tasks.add_task"
form_class = SubTaskCreateForm
+ @property
+ def parent_task(self) -> Task:
+ return Task.objects.get(pk=self.kwargs["parent_task_pk"])
+
def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
context["page_title"] = (
f"Create a subtask for task {self.kwargs['parent_task_pk']}"
)
+ context["parent_task"] = self.parent_task
return context
def form_valid(self, form):
- parent_task = Task.objects.filter(pk=self.kwargs["parent_task_pk"]).first()
- if parent_task.parent_task:
+ if self.parent_task.parent_task:
form.add_error(
None,
"You cannot make a subtask from a subtask.",
)
return self.form_invalid(form)
else:
- self.object = form.save(parent_task, user=self.request.user)
+ self.object = form.save(self.parent_task, user=self.request.user)
return HttpResponseRedirect(self.get_success_url())
def get_success_url(self):
@@ -385,6 +393,10 @@ def get_queryset(self):
queryset = queryset.order_by(*ordering)
return queryset
+ def get_context_data(self, *, object_list=None, **kwargs):
+ context = super().get_context_data(object_list=object_list, **kwargs)
+ return context
+
class TaskAndWorkflowListView(
PermissionRequiredMixin,
@@ -406,6 +418,10 @@ def get_queryset(self):
queryset = queryset.order_by(*ordering)
return queryset
+ def get_context_data(self, *, object_list=None, **kwargs):
+ context = super().get_context_data(object_list=object_list, **kwargs)
+ return context
+
class TaskWorkflowTemplateListView(
PermissionRequiredMixin,
@@ -421,9 +437,7 @@ class TaskWorkflowTemplateListView(
def get_context_data(self, **kwargs) -> dict:
context_data = super().get_context_data(**kwargs)
-
context_data["datetime_format"] = settings.DATETIME_FORMAT
-
return context_data
def get_queryset(self):
diff --git a/workbaskets/jinja2/includes/workbaskets/auto_end_date_measures.jinja b/workbaskets/jinja2/includes/workbaskets/auto_end_date_measures.jinja
index d9e81311b..bcf27a4db 100644
--- a/workbaskets/jinja2/includes/workbaskets/auto_end_date_measures.jinja
+++ b/workbaskets/jinja2/includes/workbaskets/auto_end_date_measures.jinja
@@ -29,18 +29,16 @@
]) or "" }}
{% endfor %}
- {% set base_url = url('workbaskets:workbasket-ui-auto-end-date-measures') %}
-
{% set commodity_code %}
- {{ create_sortable_anchor(request, "goods_nomenclature", "Commodity code", base_url) }}
+ {{ create_sortable_anchor(request, "Commodity code", sorting_urls["goods_nomenclature"], "#measures") }}
{% endset %}
{% set start_date %}
- {{ create_sortable_anchor(request, "start_date", "Measure start date", base_url) }}
+ {{ create_sortable_anchor(request, "Measure start date", sorting_urls["start_date"], "#measures") }}
{% endset %}
{% set measure_sid %}
- {{ create_sortable_anchor(request, "sid", "Measure SID", base_url) }}
+ {{ create_sortable_anchor(request, "Measure SID", sorting_urls["sid"], "#measures") }}
{% endset %}
{% if object_list %}
diff --git a/workbaskets/jinja2/workbaskets/changes.jinja b/workbaskets/jinja2/workbaskets/changes.jinja
index 96cbca35c..4fd4c401e 100644
--- a/workbaskets/jinja2/workbaskets/changes.jinja
+++ b/workbaskets/jinja2/workbaskets/changes.jinja
@@ -6,18 +6,16 @@
{% from "includes/workbaskets/navigation.jinja" import create_workbasket_detail_navigation with context %}
{% from "macros/checkbox_item.jinja" import checkbox_item %}
-{% set base_url = url("workbaskets:workbasket-ui-changes", args=[workbasket.pk]) ~ "?page=" ~ page_obj.number %}
-
{% set component %}
- {{ create_sortable_anchor(request, "component", "Item", base_url, True) }}
+ {{ create_sortable_anchor(request, "Item", sorting_urls["component"]) }}
{% endset %}
{% set action %}
- {{ create_sortable_anchor(request, "action", "Action", base_url, True) }}
+ {{ create_sortable_anchor(request, "Action", sorting_urls["action"]) }}
{% endset %}
{% set activity_date %}
- {{ create_sortable_anchor(request, "activity_date", "Activity date", base_url, True) }}
+ {{ create_sortable_anchor(request, "Activity date", sorting_urls["activity_date"]) }}
{% endset %}
{% set page_title %} Workbasket {{ workbasket.id }} - {{ workbasket.status }} {% endset %}
diff --git a/workbaskets/jinja2/workbaskets/checks/missing_measures.jinja b/workbaskets/jinja2/workbaskets/checks/missing_measures.jinja
index 6a9068614..8279f3939 100644
--- a/workbaskets/jinja2/workbaskets/checks/missing_measures.jinja
+++ b/workbaskets/jinja2/workbaskets/checks/missing_measures.jinja
@@ -121,10 +121,8 @@
- {% set base_url = url("workbaskets:workbasket-ui-missing-measures-check") %}
-
{% set commodity_code %}
- {{ create_sortable_anchor(request, "commodity", "Commodity code", base_url) }}
+ {{ create_sortable_anchor(request, "Commodity code", sorting_urls["commodity"]) }}
{% endset %}
{% set table_rows = [] %}
diff --git a/workbaskets/jinja2/workbaskets/summary-workbasket.jinja b/workbaskets/jinja2/workbaskets/summary-workbasket.jinja
index 8d54bf4f9..ddc37e738 100644
--- a/workbaskets/jinja2/workbaskets/summary-workbasket.jinja
+++ b/workbaskets/jinja2/workbaskets/summary-workbasket.jinja
@@ -15,8 +15,6 @@
{% set assign_users_link = url("workbaskets:workbasket-ui-assign-users", kwargs={"pk": workbasket.pk}) %}
{% set unassign_users_link = url("workbaskets:workbasket-ui-unassign-users", kwargs={"pk": workbasket.pk}) %}
-{% set base_url = url("workbaskets:current-workbasket") ~ "?page=" ~ page_obj.number %}
-
{% macro display_assigned_users(assigned_users, assignment_type) %}
{% if not assigned_users and assignment_type == "workers" %}
No users have been assigned to this workbasket yet.
@@ -156,6 +154,17 @@
+
+
Auto end-date measures
+
Automatically end-date or delete measures and footnote associations on commodities which have been ended in this workbasket.
+
+ Auto end-date measures
+
+
+
{% if workbasket.tasks.exists() and can_add_comment %}
Activity
@@ -164,21 +173,11 @@
{% if comments %}
- Sort by {{ create_sortable_anchor(request, "comments", sort_by_title, base_url, query_params="True") }}
+ Sort by {{ create_sortable_anchor(request, sort_by_label, sorting_urls["comments"]) }}
{% endif %}
{% endif %}
-
-
Auto end-date measures
-
Automatically end-date or delete measures and footnote associations on commodities which have been ended in this workbasket.
-
- Auto end-date measures
-
-
{% if comments and can_view_comment %}
diff --git a/workbaskets/jinja2/workbaskets/violations.jinja b/workbaskets/jinja2/workbaskets/violations.jinja
index b85e2a923..37c63cb85 100644
--- a/workbaskets/jinja2/workbaskets/violations.jinja
+++ b/workbaskets/jinja2/workbaskets/violations.jinja
@@ -33,18 +33,16 @@
]) or "" }}
{% endfor %}
-{% set base_url = url('workbaskets:workbasket-ui-violations' ) %}
-
{% set item %}
- {{ create_sortable_anchor(request, "model", "Item", base_url) }}
+ {{ create_sortable_anchor(request, "Item", sorting_urls["model"]) }}
{% endset %}
{% set violation %}
- {{ create_sortable_anchor(request, "check_name", "Violation", base_url) }}
+ {{ create_sortable_anchor(request, "Violation", sorting_urls["check_name"]) }}
{% endset %}
{% set activity_date %}
- {{ create_sortable_anchor(request, "date", "Activity date", base_url) }}
+ {{ create_sortable_anchor(request, "Activity date", sorting_urls["date"]) }}
{% endset %}
{{ govukTable({
diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py
index b7a45f40e..ba239c0d3 100644
--- a/workbaskets/views/ui.py
+++ b/workbaskets/views/ui.py
@@ -2,7 +2,6 @@
import re
from datetime import date
from functools import cached_property
-from typing import Tuple
import boto3
import django_filters
@@ -388,9 +387,11 @@ class EditWorkbasketView(PermissionRequiredMixin, TemplateView):
@method_decorator(require_current_workbasket, name="dispatch")
-class CurrentWorkBasket(FormView):
+class CurrentWorkBasket(SortingMixin, FormView):
template_name = "workbaskets/summary-workbasket.jinja"
form_class = forms.WorkBasketCommentCreateForm
+ sort_by_fields = ["comments"]
+ custom_sorting = {"comments": "created_at"}
@property
def workbasket(self) -> WorkBasket:
@@ -398,27 +399,28 @@ def workbasket(self) -> WorkBasket:
@cached_property
def comments(self):
+ comments = Comment.objects.filter(task__workbasket=self.workbasket)
ordering = self.get_comments_ordering()[0]
- return Comment.objects.filter(task__workbasket=self.workbasket).order_by(
- ordering,
- )
+ if ordering:
+ comments = comments.order_by(ordering)
+ return comments
@cached_property
def paginator(self):
return Paginator(self.comments, per_page=20)
- def get_comments_ordering(self) -> Tuple[str, str]:
- """Returns the ordering for `self.comments` based on `ordered` GET param
- together with the title to use for the sort by filter (which will be the
- opposite of the applied ordering)."""
- ordered = self.request.GET.get("ordered")
- if ordered == "desc":
+ def get_comments_ordering(self) -> tuple[str, str]:
+ """Reverses the ordering value returned by `super().get_ordering()` to
+ list newest comments first by default and includes a custom label to use
+ as the sorting anchor's title."""
+ ordering = super().get_ordering()
+ if ordering and ordering.startswith("-"):
ordering = "created_at"
- new_sort_by_title = "Newest first"
+ sort_by_label = "Newest first"
else:
ordering = "-created_at"
- new_sort_by_title = "Oldest first"
- return ordering, new_sort_by_title
+ sort_by_label = "Oldest first"
+ return ordering, sort_by_label
def form_valid(self, form):
form.save(user=self.request.user, workbasket=self.workbasket)
@@ -477,7 +479,7 @@ def get_context_data(self, **kwargs):
"can_add_comment": can_add_comment,
"can_view_comment": can_view_comment,
"comments": page.object_list,
- "sort_by_title": self.get_comments_ordering()[1],
+ "sort_by_label": self.get_comments_ordering()[1],
"paginator": self.paginator,
"page_obj": page,
"page_links": page_links,
From 562a3c1cc42b6b63c59264da54c2dc3150e58f51 Mon Sep 17 00:00:00 2001
From: Dale Cannon
Date: Thu, 13 Feb 2025 17:05:21 +0000
Subject: [PATCH 50/57] Pre-sort objects in view sorting tests to guarantee
consistent ordering
---
footnotes/tests/test_views.py | 4 +++-
geo_areas/tests/test_views.py | 4 +++-
regulations/tests/test_views.py | 4 +++-
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/footnotes/tests/test_views.py b/footnotes/tests/test_views.py
index a432a0edf..ee4627cd7 100644
--- a/footnotes/tests/test_views.py
+++ b/footnotes/tests/test_views.py
@@ -347,7 +347,9 @@ def test_footnote_detail_measures_view_sorting_commodity(valid_user_client):
associated_footnote=footnote,
)
measures.append(measure)
- commodity_codes = [measure.goods_nomenclature.item_id for measure in measures]
+ commodity_codes = sorted(
+ [measure.goods_nomenclature.item_id for measure in measures],
+ )
url = reverse(
"footnote-ui-detail-measures",
kwargs={
diff --git a/geo_areas/tests/test_views.py b/geo_areas/tests/test_views.py
index 58f5acf17..bdfe199e5 100644
--- a/geo_areas/tests/test_views.py
+++ b/geo_areas/tests/test_views.py
@@ -579,7 +579,9 @@ def test_geo_area_detail_measures_view_sorting_commodity(valid_user_client):
3,
geographical_area=geo_area,
)
- commodity_codes = [measure.goods_nomenclature.item_id for measure in measures]
+ commodity_codes = sorted(
+ [measure.goods_nomenclature.item_id for measure in measures],
+ )
url = reverse(
"geo_area-ui-detail-measures",
diff --git a/regulations/tests/test_views.py b/regulations/tests/test_views.py
index f6f55405d..889aa51cd 100644
--- a/regulations/tests/test_views.py
+++ b/regulations/tests/test_views.py
@@ -149,7 +149,9 @@ def test_regulation_detail_measures_view_sorting_commodity(valid_user_client):
3,
generating_regulation=regulation,
)
- commodity_codes = [measure.goods_nomenclature.item_id for measure in measures]
+ commodity_codes = sorted(
+ [measure.goods_nomenclature.item_id for measure in measures],
+ )
url = reverse(
"regulation-ui-detail-measures",
From 380d87b326bfeb3dfa02bb4e263badbd19f6c96f Mon Sep 17 00:00:00 2001
From: Marya Shariq
Date: Thu, 20 Feb 2025 11:16:34 +0000
Subject: [PATCH 51/57] UI text changes for new 'tickets' tile on homepage
(#1414)
* UI text changes for new 'tickets' tile on homepage
* two unit tests added to check visibility of 'tickets' tile for users with specific permissions
---
common/jinja2/common/homepage.jinja | 29 +++++++----------
common/tests/test_views.py | 50 +++++++++++++++++++++++++++--
2 files changed, 60 insertions(+), 19 deletions(-)
diff --git a/common/jinja2/common/homepage.jinja b/common/jinja2/common/homepage.jinja
index 7cc40fa84..236d807aa 100644
--- a/common/jinja2/common/homepage.jinja
+++ b/common/jinja2/common/homepage.jinja
@@ -144,29 +144,24 @@
{% endif %}
+
+ {% if request.user.groups.filter(name='Tariff Managers').exists() or request.user.is_superuser %}
- Tasks and Workflows - TEST ONLY
-
- Tasks
-
-
+ Tickets
+
Workflows
-
-
- Tasks and Workflows
-
-
+ >Tickets
+
+ {% if request.user.is_superuser %}
+
Workflow templates
-
+ >Ticket templates
+
+ {% endif %}
+ {% endif %}
Resources
diff --git a/common/tests/test_views.py b/common/tests/test_views.py
index 5b65d32ea..8bd62e88f 100644
--- a/common/tests/test_views.py
+++ b/common/tests/test_views.py
@@ -378,8 +378,7 @@ def test_resources_view_displays_resources(heading, expected_url, valid_user_cli
@override_settings(SSO_ENABLED=True)
def test_admin_login_shows_404_when_sso_enabled(superuser_client):
"""Test to check that when staff SSO is enabled, the login page shows a 404
- but the rest of the admin site is still available.
- """
+ but the rest of the admin site is still available."""
response = superuser_client.get(reverse("admin:login"))
assert response.status_code == 404
@@ -387,3 +386,50 @@ def test_admin_login_shows_404_when_sso_enabled(superuser_client):
assert response.status_code == 200
page = BeautifulSoup(response.content.decode(response.charset), "html.parser")
assert page.find("h1", string="Site administration")
+
+
+@pytest.mark.parametrize(
+ "client_name",
+ [
+ "client",
+ "superuser_client",
+ "valid_user_client",
+ ],
+)
+def test_tickets_tile_visibility_based_on_user_permissions(client_name, request):
+ """This test checks if 'Tickets' tile is only visible to either Tariff
+ Managers (valid_user_client used as proxy) or superusers."""
+ client = request.getfixturevalue(client_name)
+ response = client.get(reverse("home"))
+
+ page = BeautifulSoup(response.content.decode(response.charset), "html.parser")
+ card = page.find("h3", string="Tickets")
+
+ if client_name == "client":
+ assert not card
+ else:
+ assert card
+
+
+@pytest.mark.parametrize(
+ "client_name",
+ [
+ "valid_user_client",
+ "superuser_client",
+ ],
+)
+def test_ticket_templates_link_only_visible_to_superuser(client_name, request):
+ """This test checks if the 'Ticket Templates' link is only visible to
+ superusers."""
+ print(f"Using the following client: {client_name}")
+ client = request.getfixturevalue(client_name)
+ response = client.get(reverse("home"))
+
+ page = BeautifulSoup(response.content.decode(response.charset), "html.parser")
+ link_url = reverse("workflow:task-workflow-template-ui-list")
+ ticket_templates_link = page.find("a", href=link_url)
+
+ if client_name == "valid_user_client":
+ assert not ticket_templates_link
+ else:
+ assert ticket_templates_link
From 48ef119f5e5d6c8152b92cd83e249b9530ca3e16 Mon Sep 17 00:00:00 2001
From: marya
Date: Thu, 20 Feb 2025 14:08:15 +0000
Subject: [PATCH 52/57] references to subtasks removed from UI
---
tasks/jinja2/tasks/detail.jinja | 8 --------
1 file changed, 8 deletions(-)
diff --git a/tasks/jinja2/tasks/detail.jinja b/tasks/jinja2/tasks/detail.jinja
index 0750d487e..4034cf800 100644
--- a/tasks/jinja2/tasks/detail.jinja
+++ b/tasks/jinja2/tasks/detail.jinja
@@ -238,14 +238,6 @@
"href": url("workflow:task-ui-list"),
"classes": "govuk-button--secondary"
}) }}
-
- {% if not object.parent_task %}
- {{ govukButton({
- "text": "Create subtask",
- "href": url("workflow:subtask-ui-create", kwargs={"parent_task_pk": object.pk}),
- "classes": "govuk-button--secondary"
- }) }}
- {% endif %}
{% set assign_users_link = url("workflow:task-ui-assign-users", kwargs={"pk": object.pk}) %}
From b64597d3eaff326548cdcdfdcab596de97be9d2a Mon Sep 17 00:00:00 2001
From: marya
Date: Thu, 20 Feb 2025 14:12:09 +0000
Subject: [PATCH 53/57] Revert "references to subtasks removed from UI"
This reverts commit 48ef119f5e5d6c8152b92cd83e249b9530ca3e16.
---
tasks/jinja2/tasks/detail.jinja | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/tasks/jinja2/tasks/detail.jinja b/tasks/jinja2/tasks/detail.jinja
index 4034cf800..0750d487e 100644
--- a/tasks/jinja2/tasks/detail.jinja
+++ b/tasks/jinja2/tasks/detail.jinja
@@ -238,6 +238,14 @@
"href": url("workflow:task-ui-list"),
"classes": "govuk-button--secondary"
}) }}
+
+ {% if not object.parent_task %}
+ {{ govukButton({
+ "text": "Create subtask",
+ "href": url("workflow:subtask-ui-create", kwargs={"parent_task_pk": object.pk}),
+ "classes": "govuk-button--secondary"
+ }) }}
+ {% endif %}
{% set assign_users_link = url("workflow:task-ui-assign-users", kwargs={"pk": object.pk}) %}
From fe6675af93be7ca1fe47a8a8ff172cab90c3dd61 Mon Sep 17 00:00:00 2001
From: Dale Cannon <118175145+dalecannon@users.noreply.github.com>
Date: Fri, 28 Feb 2025 16:41:51 +0000
Subject: [PATCH 54/57] TP2000-1725 Redesign ticket detail view (#1417)
* Remove queued item management from ticket detail view
* Reconcile detail.jinja for ticket and ticket template detail views
* Display step assignees on ticket detail view
* Add heading caption to ticket detail view
* Remove details summary list
* Add description detail
* Add datetime formatting jinja filters
* Add display_details macros
* Add css classes for stacked summary list
* Add details column
* Add edit button
* Update ticket detail view unit tests
* Use coloured tags for status
* Align ticket template step list table with ticket step list table
* Amend content
---
common/jinja2.py | 8 ++
common/static/common/scss/_components.scss | 26 ++++
common/util.py | 28 ++++
tasks/jinja2/tasks/includes/task_list.jinja | 26 ++++
tasks/jinja2/tasks/includes/task_queue.jinja | 8 +-
.../jinja2/tasks/macros/display_details.jinja | 42 ++++++
tasks/jinja2/tasks/workflows/detail.jinja | 131 ++++++++----------
tasks/tests/factories.py | 2 +
tasks/tests/test_views.py | 67 +++------
tasks/views.py | 32 ++---
10 files changed, 221 insertions(+), 149 deletions(-)
create mode 100644 tasks/jinja2/tasks/includes/task_list.jinja
create mode 100644 tasks/jinja2/tasks/macros/display_details.jinja
diff --git a/common/jinja2.py b/common/jinja2.py
index 6d9909e47..b794b0382 100644
--- a/common/jinja2.py
+++ b/common/jinja2.py
@@ -16,6 +16,10 @@
from webpack_loader.contrib.jinja2ext import _render_bundle
from webpack_loader.templatetags.webpack_loader import webpack_static
+from common.util import format_date
+from common.util import format_datetime
+from common.util import format_short_date
+from common.util import format_short_datetime
from govuk_frontend_jinja.templates import Environment
from govuk_frontend_jinja.templates import NunjucksExtension
from workbaskets.models import WorkBasket
@@ -127,6 +131,10 @@ def environment(**kwargs):
{
"localtime": template_localtime,
"pluralize": pluralize,
+ "format_date": format_date,
+ "format_short_date": format_short_date,
+ "format_datetime": format_datetime,
+ "format_short_datetime": format_short_datetime,
},
)
diff --git a/common/static/common/scss/_components.scss b/common/static/common/scss/_components.scss
index a0b8ed9d9..a3024cd52 100644
--- a/common/static/common/scss/_components.scss
+++ b/common/static/common/scss/_components.scss
@@ -165,3 +165,29 @@
.govuk-checkboxes__label:before {
background-color: white
}
+
+.govuk-summary-list__row__stacked {
+ @extend .govuk-summary-list__row;
+ display: block;
+}
+
+.govuk-summary-list__key__stacked {
+ @extend .govuk-summary-list__key;
+ display: block;
+ width: 100%;
+ padding: 0;
+ margin: 0;
+}
+
+.govuk-summary-list__value__stacked {
+ @extend .govuk-summary-list__value;
+ display: block;
+ padding: 0;
+ margin: 0;
+}
+
+.govuk-summary-list__value__stacked:last-child {
+ width: 100%;
+ padding-bottom: govuk-spacing(2);
+ margin: 0;
+}
diff --git a/common/util.py b/common/util.py
index 2f3539872..69d431bdf 100644
--- a/common/util.py
+++ b/common/util.py
@@ -690,6 +690,34 @@ def format_date_string(date_string: str, short_format=False) -> str:
return ""
+def format_date(value: datetime) -> str:
+ """Formats a datetime object using `settings.DATE_FORMAT`."""
+ if isinstance(value, datetime):
+ return value.strftime(settings.DATE_FORMAT)
+ return value
+
+
+def format_short_date(value: datetime) -> str:
+ """Formats a datetime object using `settings.SHORT_DATE_FORMAT`."""
+ if isinstance(value, datetime):
+ return value.strftime(settings.SHORT_DATE_FORMAT)
+ return value
+
+
+def format_datetime(value: datetime) -> str:
+ """Formats a datetime object using `settings.DATETIME_FORMAT`."""
+ if isinstance(value, datetime):
+ return value.strftime(settings.DATETIME_FORMAT)
+ return value
+
+
+def format_short_datetime(value: datetime) -> str:
+ """Formats a datetime object using `settings.SHORT_DATETIME_FORMAT`."""
+ if isinstance(value, datetime):
+ return value.strftime(settings.SHORT_DATETIME_FORMAT)
+ return value
+
+
def log_timing(logger_function: typing.Callable):
"""
Decorator function to log start and end times of a decorated function.
diff --git a/tasks/jinja2/tasks/includes/task_list.jinja b/tasks/jinja2/tasks/includes/task_list.jinja
new file mode 100644
index 000000000..3e1a48c37
--- /dev/null
+++ b/tasks/jinja2/tasks/includes/task_list.jinja
@@ -0,0 +1,26 @@
+{% from "components/table/macro.njk" import govukTable %}
+{% from "tasks/macros/display_details.jinja" import display_assignees %}
+{% from "tasks/macros/display_details.jinja" import display_status %}
+
+{% set table_rows = [] %}
+
+{% for object in object_list %}
+ {% set object_link -%}
+ {{ object.title }}
+ {%- endset %}
+
+ {{ table_rows.append([
+ {"html": object_link},
+ {"text": display_assignees(object.assignees.assigned())},
+ {"text": display_status(object.progress_state.__str__())},
+ ]) or "" }}
+{% endfor %}
+
+{{ govukTable({
+ "head": [
+ {"text": "Name"},
+ {"text": "Assignee"},
+ {"text": "Status"},
+ ],
+ "rows": table_rows
+}) }}
diff --git a/tasks/jinja2/tasks/includes/task_queue.jinja b/tasks/jinja2/tasks/includes/task_queue.jinja
index 1f55b66d3..b8dcc46e9 100644
--- a/tasks/jinja2/tasks/includes/task_queue.jinja
+++ b/tasks/jinja2/tasks/includes/task_queue.jinja
@@ -28,10 +28,6 @@
{{ obj.title }}
{% endset -%}
- {%- set description_cell_content %}
- {{ obj.description|truncate(50) }}
- {% endset -%}
-
{%- set order_cell_content %}
{% if object_list|length == 1 %}
{# No buttons needed for a single item #}
@@ -54,7 +50,6 @@
{"html": up_down_cell_content},
{"text": loop.index},
{"html": title_cell_content},
- {"html": description_cell_content},
{"html": order_cell_content},
{"html": remove_cell_content},
]) or "" }}
@@ -68,8 +63,7 @@
"head": [
{"text": ""},
{"text": "Position"},
- {"text": "Title"},
- {"text": "Description"},
+ {"text": "Name"},
{"text": "Order"},
{"text": "Remove"},
],
diff --git a/tasks/jinja2/tasks/macros/display_details.jinja b/tasks/jinja2/tasks/macros/display_details.jinja
new file mode 100644
index 000000000..841324fbe
--- /dev/null
+++ b/tasks/jinja2/tasks/macros/display_details.jinja
@@ -0,0 +1,42 @@
+{% from "components/tag/macro.njk" import govukTag %}
+
+{% macro display_assignees(assignees) -%}
+ {% if not assignees %}
+ None
+ {% else %}
+ {% for assignee in assignees %}
+ {{ assignee.user.get_displayname() }}{% if not loop.last %}, {% endif %}
+ {% endfor %}
+ {% endif %}
+{%- endmacro %}
+
+{% macro display_workbasket(workbasket) -%}
+ {% if not workbasket %}
+ None
+ {% else %}
+ {{ workbasket.pk }} - {{ workbasket.status }}
+ {% endif %}
+{%- endmacro %}
+
+{% macro display_summary_row_stacked(key, value) -%}
+
+
{{ key }}
+ {{ value }}
+
+{%- endmacro %}
+
+{% macro display_status(status) %}
+ {% set status_to_class_map = ({
+ "In progress": "govuk-tag--blue",
+ "Done": "govuk-tag--green",
+ "To do": "govuk-tag--purple",
+ }) %}
+
+ {{ govukTag({
+ "text": status,
+ "classes": status_to_class_map[status],
+ }) }}
+{% endmacro %}
diff --git a/tasks/jinja2/tasks/workflows/detail.jinja b/tasks/jinja2/tasks/workflows/detail.jinja
index b9f395a0a..1835e9856 100644
--- a/tasks/jinja2/tasks/workflows/detail.jinja
+++ b/tasks/jinja2/tasks/workflows/detail.jinja
@@ -1,24 +1,16 @@
{% extends "layouts/layout.jinja" %}
{% from "components/breadcrumbs.jinja" import breadcrumbs %}
{% from "components/button/macro.njk" import govukButton %}
+{% from "tasks/macros/display_details.jinja" import display_assignees %}
+{% from "tasks/macros/display_details.jinja" import display_workbasket %}
+{% from "tasks/macros/display_details.jinja" import display_summary_row_stacked %}
+{% from "tasks/macros/display_details.jinja" import display_status %}
-{% set object_name = object._meta.verbose_name %}
-
-{% set page_title = object_name|capitalize ~ ": " ~ object.title %}
-
-{% if object_name == "workflow" %}
- {% set task_object_name = "task" %}
- {% set task_object_create_url = url("workflow:task-workflow-task-ui-create", kwargs={"task_workflow_pk": object.pk}) %}
-{% elif object_name == "workflow template" %}
- {% set task_object_name = "task template" %}
- {% set task_object_create_url = url("workflow:task-template-ui-create", kwargs={"workflow_template_pk": object.pk}) %}
-{% endif %}
-
-{% set list_include = "tasks/includes/task_queue.jinja" %}
+{% set page_title = verbose_name|capitalize ~ " " ~ object.id %}
{% block breadcrumb %}
{{ breadcrumbs(request, [
- {"text": "Find and view "~ object_name ~ "s", "href": object.get_url("list")},
+ {"text": "Find and view " ~ verbose_name ~ "s", "href": object.get_url("list")},
{"text": page_title}
],
with_workbasket=False,
@@ -26,80 +18,73 @@
{% endblock %}
{% block content %}
- {{ page_title }}
-
- Details
-
-
-
-
-
- ID
-
- -
- {{ object.pk }}
-
-
-
-
-
-
-
-
-
-
{{ task_object_name|capitalize }}s
+
+ {{ object.title }}
+ {{ page_title }}
+
-
+
{{ govukButton({
- "text": "Create a " ~ task_object_name,
- "href": task_object_create_url,
- "classes": "govuk-button align-right"
+ "text": "Edit " ~ verbose_name,
+ "href": object.get_url("edit"),
+ "classes": "align-right",
}) }}
+
+
-
+
+
Description
+
{{ object.description }}
+
+
Steps
{% if object_list %}
{% include list_include %}
{% else %}
-
There are no {{ task_object_name }}s for this {{ object_name }}.
+
There are no steps for this {{ verbose_name }}.
{% endif %}
-
-
+
+ {% if verbose_name == "ticket template" %}
+
+
+
+ {{ govukButton({
+ "text": "Add a step",
+ "href": url("workflow:task-template-ui-create", kwargs={"workflow_template_pk": object.pk}),
+ }) }}
+ {{ govukButton({
+ "text": "Delete " ~ verbose_name,
+ "href": object.get_url("delete"),
+ "classes": "govuk-button--warning"
+ }) }}
+
+
+
+ {% endif %}
+
{% endblock %}
diff --git a/tasks/tests/factories.py b/tasks/tests/factories.py
index 95d66f29a..9d97e84d0 100644
--- a/tasks/tests/factories.py
+++ b/tasks/tests/factories.py
@@ -4,6 +4,7 @@
from common.tests.factories import CategoryFactory
from common.tests.factories import TaskFactory
+from common.tests.factories import UserFactory
from tasks.models import TaskItem
from tasks.models import TaskItemTemplate
from tasks.models import TaskTemplate
@@ -16,6 +17,7 @@ class TaskWorkflowTemplateFactory(DjangoModelFactory):
title = factory.Faker("sentence")
description = factory.Faker("sentence")
+ creator = factory.SubFactory(UserFactory)
class Meta:
model = TaskWorkflowTemplate
diff --git a/tasks/tests/test_views.py b/tasks/tests/test_views.py
index 8a5a4af6a..e0e4d6570 100644
--- a/tasks/tests/test_views.py
+++ b/tasks/tests/test_views.py
@@ -7,6 +7,7 @@
from common.tests.factories import ProgressStateFactory
from common.tests.factories import SubTaskFactory
from common.tests.factories import TaskFactory
+from common.util import format_date
from tasks.forms import TaskWorkflowCreateForm
from tasks.models import ProgressState
from tasks.models import Task
@@ -219,7 +220,6 @@ def test_delete_subtask_missing_user_permissions(
def test_workflow_template_detail_view_displays_task_templates(valid_user_client):
task_item_template = TaskItemTemplateFactory.create()
- task_template = task_item_template.task_template
workflow_template = task_item_template.workflow_template
url = reverse(
@@ -230,8 +230,14 @@ def test_workflow_template_detail_view_displays_task_templates(valid_user_client
assert response.status_code == 200
page = BeautifulSoup(response.content.decode(response.charset), "html.parser")
- assert page.find("h1", text=f"Workflow template: {workflow_template.title}")
- assert page.find("a", text=task_template.title)
+
+ assert f"Ticket template {workflow_template.id}" in page.find("h1").text
+ assert page.find("p", text=workflow_template.description)
+ assert page.find("dd", text=workflow_template.creator.get_displayname())
+ assert page.find("dd", text=format_date(workflow_template.created_at))
+
+ template_rows = page.select(".govuk-table__body > .govuk-table__row")
+ assert len(template_rows) == workflow_template.get_task_templates().count()
@pytest.mark.parametrize(
@@ -584,7 +590,7 @@ def test_workflow_detail_view_displays_tasks(
task_workflow_single_task_item,
):
workflow = task_workflow_single_task_item
- task = task_workflow_single_task_item.get_tasks().get()
+ workbasket = workflow.summary_task.workbasket
url = reverse(
"workflow:task-workflow-ui-detail",
@@ -594,55 +600,14 @@ def test_workflow_detail_view_displays_tasks(
assert response.status_code == 200
page = BeautifulSoup(response.content.decode(response.charset), "html.parser")
- assert page.find("h1", text=f"Workflow: {workflow.title}")
- assert page.find("a", text=task.title)
-
-
-@pytest.mark.parametrize(
- ("action", "item_position", "expected_item_order"),
- [
- ("promote", 1, [1, 2, 3]),
- ("promote", 2, [2, 1, 3]),
- ("demote", 2, [1, 3, 2]),
- ("demote", 3, [1, 2, 3]),
- ("promote_to_first", 3, [3, 1, 2]),
- ("demote_to_last", 1, [2, 3, 1]),
- ],
-)
-def test_workflow_detail_view_reorder_items(
- action,
- item_position,
- expected_item_order,
- valid_user_client,
- task_workflow_three_task_items,
-):
- """Tests that `TaskWorkflowDetailView` handles POST requests to promote or
- demote tasks."""
- def convert_to_index(position: int) -> int:
- """Converts a 1-based item position to a 0-based index for items array
- access."""
- return position - 1
-
- items = list(task_workflow_three_task_items.get_tasks())
- item_to_move = items[convert_to_index(item_position)]
-
- url = reverse(
- "workflow:task-workflow-ui-detail",
- kwargs={"pk": task_workflow_three_task_items.pk},
- )
- form_data = {
- action: item_to_move.id,
- }
-
- response = valid_user_client.post(url, form_data)
- assert response.status_code == 302
+ assert f"Ticket {workflow.id}" in page.find("h1").text
+ assert page.find("p", text=workflow.description)
+ assert page.find("a", text=f"{workbasket.pk} - {workbasket.status}")
+ assert page.find("dd", text=format_date(workflow.summary_task.created_at))
- reordered_items = task_workflow_three_task_items.get_tasks()
- for i, reordered_item in enumerate(reordered_items):
- expected_position = convert_to_index(expected_item_order[i])
- expected_item = items[expected_position]
- assert reordered_item.id == expected_item.id
+ step_rows = page.select(".govuk-table__body > .govuk-table__row")
+ assert len(step_rows) == workflow.get_tasks().count()
@pytest.mark.parametrize(
diff --git a/tasks/views.py b/tasks/views.py
index f7027a915..77a16440d 100644
--- a/tasks/views.py
+++ b/tasks/views.py
@@ -521,15 +521,11 @@ def demote_to_last(self, lookup_id: int) -> None:
class TaskWorkflowDetailView(
PermissionRequiredMixin,
- QueuedItemManagementMixin,
DetailView,
):
template_name = "tasks/workflows/detail.jinja"
permission_required = "tasks.view_taskworkflow"
model = TaskWorkflow
- queued_item_model = TaskItem
- item_lookup_field = "task_id"
- queue_field = queued_item_model.queue_field
@property
def view_url(self) -> str:
@@ -540,21 +536,15 @@ def view_url(self) -> str:
def get_context_data(self, **kwargs):
context_data = super().get_context_data(**kwargs)
- context_data["object_list"] = self.queue.get_tasks()
+ context_data.update(
+ {
+ "object_list": self.get_object().get_tasks(),
+ "verbose_name": "ticket",
+ "list_include": "tasks/includes/task_list.jinja",
+ },
+ )
return context_data
- def post(self, request, *args, **kwargs):
- if "promote" in request.POST:
- self.promote(request.POST.get("promote"))
- elif "demote" in request.POST:
- self.demote(request.POST.get("demote"))
- elif "promote_to_first" in request.POST:
- self.promote_to_first(request.POST.get("promote_to_first"))
- elif "demote_to_last" in request.POST:
- self.demote_to_last(request.POST.get("demote_to_last"))
-
- return HttpResponseRedirect(self.view_url)
-
class TaskWorkflowCreateView(PermissionRequiredMixin, FormView):
permission_required = "tasks.add_taskworkflow"
@@ -729,7 +719,13 @@ def view_url(self) -> str:
def get_context_data(self, **kwargs):
context_data = super().get_context_data(**kwargs)
- context_data["object_list"] = self.queue.get_task_templates()
+ context_data.update(
+ {
+ "object_list": self.queue.get_task_templates(),
+ "verbose_name": "ticket template",
+ "list_include": "tasks/includes/task_queue.jinja",
+ },
+ )
return context_data
def post(self, request, *args, **kwargs):
From 7ca53f0caf1d22e90386f1fade25b6c88b9255de Mon Sep 17 00:00:00 2001
From: Marya Shariq
Date: Fri, 28 Feb 2025 16:43:17 +0000
Subject: [PATCH 55/57] Tp2000 1726 delete ticket view (#1420)
* wip
* added list url
* removed redundant comments
* modified breadcrumbs
* modified unit tests
---
tasks/jinja2/tasks/workflows/delete.jinja | 9 ++++-----
tasks/tests/test_views.py | 4 ++--
tasks/views.py | 9 +++++++--
3 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/tasks/jinja2/tasks/workflows/delete.jinja b/tasks/jinja2/tasks/workflows/delete.jinja
index aff449a61..fb49ee889 100644
--- a/tasks/jinja2/tasks/workflows/delete.jinja
+++ b/tasks/jinja2/tasks/workflows/delete.jinja
@@ -4,16 +4,15 @@
{% from "components/warning-text/macro.njk" import govukWarningText %}
{% from "components/button/macro.njk" import govukButton %}
-{% set object_name = object._meta.verbose_name %}
-{% set page_title = "Delete " ~ object_name ~ ": " ~ object.title %}
+{% set page_title = "Delete " ~ verbose_name ~ ": " ~ object.title %}
{% block breadcrumb %}
{{ breadcrumbs(
request,
[
- {"text": "Find and view "~ object_name ~ "s", "href": object.get_url("list")},
+ {"text": "Find and view "~ verbose_name ~ "s", "href": object.get_url("list")},
{
- "text": object_name|capitalize ~ ": " ~ object.title,
+ "text": verbose_name|capitalize ~ ": " ~ object.title,
"href": object.get_url("detail"),
},
{"text": page_title}
@@ -24,7 +23,7 @@
{% block form %}
{{ govukWarningText({
- "text": "Are you sure you want to delete this " ~ object_name ~ "?",
+ "text": "Are you sure you want to delete this " ~ verbose_name ~ "?",
}) }}
{% call django_form(action=object.get_url("delete")) %}
diff --git a/tasks/tests/test_views.py b/tasks/tests/test_views.py
index e0e4d6570..af76bbbe4 100644
--- a/tasks/tests/test_views.py
+++ b/tasks/tests/test_views.py
@@ -748,7 +748,7 @@ def test_workflow_delete_view(
assert confirmation_response.status_code == 200
soup = BeautifulSoup(str(confirmation_response.content), "html.parser")
- assert f"Workflow ID: {workflow_pk}" in soup.select(".govuk-panel__title")[0].text
+ assert f"Ticket ID: {workflow_pk}" in soup.select(".govuk-panel__title")[0].text
def test_workflow_list_view(valid_user_client, task_workflow):
@@ -856,5 +856,5 @@ def test_workflow_delete_view_deletes_related_tasks(
soup = BeautifulSoup(str(confirmation_response.content), "html.parser")
assert (
- f"Workflow ID: {task_workflow_pk}" in soup.select(".govuk-panel__title")[0].text
+ f"Ticket ID: {task_workflow_pk}" in soup.select(".govuk-panel__title")[0].text
)
diff --git a/tasks/views.py b/tasks/views.py
index 77a16440d..8a72ab26b 100644
--- a/tasks/views.py
+++ b/tasks/views.py
@@ -620,6 +620,11 @@ def get_form_kwargs(self):
kwargs["instance"] = self.object
return kwargs
+ def get_context_data(self, **kwargs):
+ context_data = super().get_context_data(**kwargs)
+ context_data["verbose_name"] = "ticket"
+ return context_data
+
@transaction.atomic
def form_valid(self, form):
summary_task = self.object.summary_task
@@ -644,10 +649,10 @@ def get_context_data(self, **kwargs):
context_data = super().get_context_data(**kwargs)
context_data.update(
{
- "verbose_name": "workflow",
+ "verbose_name": "ticket",
"deleted_pk": self.kwargs["pk"],
"create_url": reverse("workflow:task-workflow-ui-create"),
- "list_url": "#NOT-IMPLEMENTED",
+ "list_url": reverse("workflow:task-workflow-ui-list"),
},
)
return context_data
From 7ebc168eb31a9039c3c567840db346b5e22d433f Mon Sep 17 00:00:00 2001
From: Lauren Mullally <46787754+LaurenMullally@users.noreply.github.com>
Date: Mon, 3 Mar 2025 14:35:50 +0000
Subject: [PATCH 56/57] TP200-1722 Create New Ticket (#1419)
* Amend template, form and include additional fields
* Amend model to take in eif date and policy contact
* amend form's mandatory fields and error messages
* Amend mandatory fields
* Fix form tests
* Amend view tests
* Amend workflow model tests
* Squish the migrations
* Amend ticket name error message
* Amend policy contact character limit and squash migrations
* Re route create form's success url and amend tests
---
tasks/forms.py | 51 ++++++++-------
tasks/jinja2/tasks/workflows/create.jinja | 2 +-
...ow_eif_date_taskworkflow_policy_contact.py | 24 +++++++
tasks/models/workflow.py | 14 +++++
tasks/tests/test_forms.py | 62 ++++++-------------
tasks/tests/test_views.py | 58 +++++------------
tasks/tests/test_workflow_models.py | 15 ++++-
tasks/views.py | 24 +++----
8 files changed, 122 insertions(+), 128 deletions(-)
create mode 100644 tasks/migrations/0021_taskworkflow_eif_date_taskworkflow_policy_contact.py
diff --git a/tasks/forms.py b/tasks/forms.py
index d9e94bb97..e026872cd 100644
--- a/tasks/forms.py
+++ b/tasks/forms.py
@@ -7,7 +7,6 @@
from crispy_forms_gds.layout import Submit
from django.contrib.auth import get_user_model
from django.db import transaction
-from django.db.models import TextChoices
from django.forms import CharField
from django.forms import CheckboxSelectMultiple
from django.forms import Form
@@ -19,7 +18,7 @@
from common.fields import AutoCompleteField
from common.forms import BindNestedFormMixin
-from common.forms import RadioNested
+from common.forms import DateInputFieldFixed
from common.forms import delete_form_for
from common.validators import SymbolValidator
from tasks.models import Task
@@ -245,36 +244,42 @@ class TaskWorkflowTemplateForm(Form):
class TaskWorkflowCreateForm(BindNestedFormMixin, Form):
- class CreateType(TextChoices):
- WITH_TEMPLATE = "WITH_TEMPLATE", "Yes"
- WITHOUT_TEMPLATE = "WITHOUT_TEMPLATE", "No"
- title = CharField(
+ ticket_name = CharField(
+ required=True,
max_length=255,
validators=[SymbolValidator],
error_messages={
- "required": "Enter a title for the workflow",
+ "required": "Enter a title for the ticket",
+ },
+ )
+
+ work_type = ModelChoiceField(
+ required=True,
+ help_text="Choose the most appropriate category, this will generate a pre-defined set of steps to complete the work",
+ queryset=TaskWorkflowTemplate.objects.all(),
+ error_messages={
+ "required": "Choose a work type",
},
)
description = CharField(
+ required=False,
validators=[SymbolValidator],
widget=Textarea(),
- error_messages={
- "required": "Enter a description for the workflow",
- },
+ help_text="This field is optional",
)
- create_type = RadioNested(
- label="Do you want to use a workflow template?",
- choices=CreateType.choices,
- nested_forms={
- CreateType.WITH_TEMPLATE.value: [TaskWorkflowTemplateForm],
- CreateType.WITHOUT_TEMPLATE.value: [],
- },
- error_messages={
- "required": "Select if you want to use a workflow template",
- },
+ entry_into_force_date = DateInputFieldFixed(
+ required=False,
+ help_text="This field is optional",
+ )
+
+ policy_contact = CharField(
+ required=False,
+ max_length=255,
+ validators=[SymbolValidator],
+ help_text="This field is optional",
)
def __init__(self, *args, **kwargs):
@@ -287,10 +292,12 @@ def __init__(self, *args, **kwargs):
self.helper.legend_size = Size.SMALL
self.helper.layout = Layout(
Fieldset(
- "title",
+ "ticket_name",
+ "work_type",
"description",
+ "entry_into_force_date",
+ "policy_contact",
),
- "create_type",
Submit(
"submit",
"Create",
diff --git a/tasks/jinja2/tasks/workflows/create.jinja b/tasks/jinja2/tasks/workflows/create.jinja
index a2affcceb..f2f0e13d8 100644
--- a/tasks/jinja2/tasks/workflows/create.jinja
+++ b/tasks/jinja2/tasks/workflows/create.jinja
@@ -2,7 +2,7 @@
{% from "components/breadcrumbs.jinja" import breadcrumbs %}
-{% set page_title = "Create a " ~ verbose_name %}
+{% set page_title = "Create a new " ~ verbose_name %}
{% block breadcrumb %}
{{ breadcrumbs(
diff --git a/tasks/migrations/0021_taskworkflow_eif_date_taskworkflow_policy_contact.py b/tasks/migrations/0021_taskworkflow_eif_date_taskworkflow_policy_contact.py
new file mode 100644
index 000000000..ed635ddd8
--- /dev/null
+++ b/tasks/migrations/0021_taskworkflow_eif_date_taskworkflow_policy_contact.py
@@ -0,0 +1,24 @@
+# Generated by Django 4.2.18 on 2025-03-03 10:36
+
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ("tasks", "0020_alter_taskassignee_assignment_type"),
+ ]
+
+ operations = [
+ migrations.AddField(
+ model_name="taskworkflow",
+ name="eif_date",
+ field=models.DateField(blank=True, null=True),
+ ),
+ migrations.AddField(
+ model_name="taskworkflow",
+ name="policy_contact",
+ field=models.CharField(blank=True, max_length=40, null=True),
+ ),
+ ]
diff --git a/tasks/models/workflow.py b/tasks/models/workflow.py
index 0df851a6d..f089d5106 100644
--- a/tasks/models/workflow.py
+++ b/tasks/models/workflow.py
@@ -1,3 +1,5 @@
+from datetime import date
+
from django.conf import settings
from django.db import models
from django.db.transaction import atomic
@@ -26,10 +28,16 @@ class TaskWorkflow(Queue):
workflow."""
creator_template = models.ForeignKey(
"tasks.TaskWorkflowTemplate",
+ blank=False,
null=True,
on_delete=models.SET_NULL,
)
"""The template from which this workflow was created, if any."""
+ eif_date = models.DateField(
+ blank=True,
+ null=True,
+ )
+ policy_contact = models.CharField(max_length=40, blank=True, null=True)
class Meta(Queue.Meta):
abstract = False
@@ -163,6 +171,9 @@ def create_task_workflow(
title: str,
description: str,
creator: User,
+ # Take in additional data
+ eif_date: date,
+ policy_contact: str,
) -> "TaskWorkflow":
"""Create a workflow and it subtasks, using values from this template
workflow and its task templates."""
@@ -175,6 +186,9 @@ def create_task_workflow(
task_workflow = TaskWorkflow.objects.create(
summary_task=summary_task,
creator_template=self,
+ # Pass new data to the workflow instance that will be made
+ eif_date=eif_date,
+ policy_contact=policy_contact,
)
task_item_templates = TaskItemTemplate.objects.select_related(
diff --git a/tasks/tests/test_forms.py b/tasks/tests/test_forms.py
index b4e7972f8..db30ff822 100644
--- a/tasks/tests/test_forms.py
+++ b/tasks/tests/test_forms.py
@@ -26,71 +26,45 @@ def test_create_subtask_assigns_correct_parent_task(valid_user):
assert new_subtask.parent_task.pk == parent_task_instance.pk
-@pytest.mark.parametrize(
- "form_data",
- [
- {
- "title": "Test workflow 1",
- "description": "Workflow created without using template",
- "create_type": forms.TaskWorkflowCreateForm.CreateType.WITHOUT_TEMPLATE,
- },
- {
- "title": "Test workflow 2",
- "description": "Workflow created using template",
- "create_type": forms.TaskWorkflowCreateForm.CreateType.WITH_TEMPLATE,
- "workflow_template": "",
- },
- ],
- ids=(
- "without_template",
- "with_template",
- ),
-)
-def test_workflow_create_form_valid_data(form_data, task_workflow_template):
+def test_workflow_create_form_valid_data(task_workflow_template):
"""Tests that `TaskWorkflowCreateForm` returns expected cleaned_data given
valid form data."""
- if "workflow_template" in form_data:
- form_data["workflow_template"] = task_workflow_template
+ form_data = {
+ "ticket_name": "Test ticket 1",
+ "description": "Ticket created with all fields",
+ "work_type": task_workflow_template,
+ "entry_into_force_date_0": 12,
+ "entry_into_force_date_1": 12,
+ "entry_into_force_date_2": 2026,
+ "policy_contact": "Fake Contact Name",
+ }
form = forms.TaskWorkflowCreateForm(form_data)
assert form.is_valid()
- assert form.cleaned_data == form_data
@pytest.mark.parametrize(
("form_data", "field", "error_message"),
[
- ({"title": ""}, "title", "Enter a title for the workflow"),
- ({"description": ""}, "description", "Enter a description for the workflow"),
- (
- {"create_type": ""},
- "create_type",
- "Select if you want to use a workflow template",
- ),
+ ({"ticket_name": ""}, "ticket_name", "Enter a title for the ticket"),
(
- {
- "create_type": forms.TaskWorkflowCreateForm.CreateType.WITH_TEMPLATE,
- "workflow_template": "",
- },
- "create_type",
- "Select a workflow template",
+ {"work_type": ""},
+ "work_type",
+ "Choose a work type",
),
(
{
- "create_type": forms.TaskWorkflowCreateForm.CreateType.WITH_TEMPLATE,
"workflow_template": "invalidchoice",
},
- "create_type",
- "Select a valid choice. That choice is not one of the available choices.",
+ "work_type",
+ "Choose a work type",
),
],
ids=(
"missing_title",
- "missing_description",
- "missing_create_type",
- "missing_workflow_template",
- "invalid_workflow_template",
+ "missing_work_type",
+ "invalid_work_type",
),
)
def test_workflow_create_form_invalid_data(form_data, field, error_message):
diff --git a/tasks/tests/test_views.py b/tasks/tests/test_views.py
index af76bbbe4..27123ca24 100644
--- a/tasks/tests/test_views.py
+++ b/tasks/tests/test_views.py
@@ -8,7 +8,6 @@
from common.tests.factories import SubTaskFactory
from common.tests.factories import TaskFactory
from common.util import format_date
-from tasks.forms import TaskWorkflowCreateForm
from tasks.models import ProgressState
from tasks.models import Task
from tasks.models import TaskItem
@@ -610,66 +609,39 @@ def test_workflow_detail_view_displays_tasks(
assert len(step_rows) == workflow.get_tasks().count()
-@pytest.mark.parametrize(
- "form_data",
- [
- {
- "title": "Test workflow 1",
- "description": "Workflow created without using template",
- "create_type": TaskWorkflowCreateForm.CreateType.WITHOUT_TEMPLATE,
- },
- {
- "title": "Test workflow 2",
- "description": "Workflow created using template",
- "create_type": TaskWorkflowCreateForm.CreateType.WITH_TEMPLATE,
- },
- ],
- ids=(
- "without_template",
- "with_template",
- ),
-)
def test_workflow_create_view(
- form_data,
valid_user,
valid_user_client,
task_workflow_template_single_task_template_item,
):
- """Tests that a new workflow can be created (with or without workflow
- template) and that the corresponding confirmation view returns a HTTP 200
- response."""
-
- with_template = (
- form_data["create_type"] == TaskWorkflowCreateForm.CreateType.WITH_TEMPLATE
- )
-
- if with_template:
- form_data["workflow_template"] = (
- task_workflow_template_single_task_template_item.pk
- )
+ """Tests that a new workflow can be created and that the corresponding
+ confirmation view returns a HTTP 200 response."""
assert not TaskWorkflow.objects.exists()
+ form_data = {
+ "ticket_name": "Test workflow 1",
+ "description": "Workflow created",
+ "work_type": task_workflow_template_single_task_template_item.pk,
+ }
+
create_url = reverse("workflow:task-workflow-ui-create")
create_response = valid_user_client.post(create_url, form_data)
assert create_response.status_code == 302
created_workflow = TaskWorkflow.objects.get(
- summary_task__title=form_data["title"],
+ summary_task__title=form_data["ticket_name"],
summary_task__description=form_data["description"],
summary_task__creator=valid_user,
)
- if with_template:
- assert (
- created_workflow.get_tasks().count()
- == task_workflow_template_single_task_template_item.get_task_templates().count()
- )
- else:
- assert created_workflow.get_tasks().count() == 0
+ assert (
+ created_workflow.get_tasks().count()
+ == task_workflow_template_single_task_template_item.get_task_templates().count()
+ )
confirmation_url = reverse(
- "workflow:task-workflow-ui-confirm-create",
+ "workflow:task-workflow-ui-detail",
kwargs={"pk": created_workflow.pk},
)
assert create_response.url == confirmation_url
@@ -678,7 +650,7 @@ def test_workflow_create_view(
assert confirmation_response.status_code == 200
soup = BeautifulSoup(str(confirmation_response.content), "html.parser")
- assert str(created_workflow) in soup.select("h1.govuk-panel__title")[0].text
+ assert str(created_workflow) in soup.select("h1")[0].text
def test_workflow_update_view(
diff --git a/tasks/tests/test_workflow_models.py b/tasks/tests/test_workflow_models.py
index 26d81cb93..dab1dfec9 100644
--- a/tasks/tests/test_workflow_models.py
+++ b/tasks/tests/test_workflow_models.py
@@ -1,3 +1,5 @@
+import datetime
+
import pytest
from django.core.exceptions import ObjectDoesNotExist
@@ -15,14 +17,19 @@ def test_create_task_workflow_from_task_workflow_template(
"""Test creation of TaskWorkflow instances from TaskWorkflowTemplates using
its `create_task_workflow()` method."""
- title = "Workflow title"
+ ticket_name = "Workflow title"
description = "Workflow description"
creator = valid_user
+ eif_date = datetime.date(2026, 12, 12)
+ policy_contact = "Policy Contact"
+
task_workflow = (
task_workflow_template_three_task_template_items.create_task_workflow(
- title=title,
+ title=ticket_name,
description=description,
creator=creator,
+ eif_date=eif_date,
+ policy_contact=policy_contact,
)
)
@@ -31,10 +38,12 @@ def test_create_task_workflow_from_task_workflow_template(
task_workflow.creator_template
== task_workflow_template_three_task_template_items
)
- assert task_workflow.summary_task.title == title
+ assert task_workflow.summary_task.title == ticket_name
assert task_workflow.summary_task.description == description
assert task_workflow.summary_task.creator == creator
assert task_workflow.get_items().count() == 3
+ assert task_workflow.eif_date == eif_date
+ assert task_workflow.policy_contact == policy_contact
# Validate that item positions are equivalent.
zipped_items = zip(
diff --git a/tasks/views.py b/tasks/views.py
index 8a72ab26b..f47ff1234 100644
--- a/tasks/views.py
+++ b/tasks/views.py
@@ -547,39 +547,33 @@ def get_context_data(self, **kwargs):
class TaskWorkflowCreateView(PermissionRequiredMixin, FormView):
+ # Feb 2025 - Workflows will now be called Tickets in the UI only.
permission_required = "tasks.add_taskworkflow"
template_name = "tasks/workflows/create.jinja"
form_class = TaskWorkflowCreateForm
def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
- context["verbose_name"] = "workflow"
+ context["verbose_name"] = "ticket"
context["list_url"] = "#NOT-IMPLEMENTED"
return context
def form_valid(self, form):
- summary_data = {
- "title": form.cleaned_data["title"],
+ data = {
+ "title": form.cleaned_data["ticket_name"],
"description": form.cleaned_data["description"],
"creator": self.request.user,
+ "eif_date": form.cleaned_data["entry_into_force_date"],
+ "policy_contact": form.cleaned_data["policy_contact"],
}
- create_type = form.cleaned_data["create_type"]
-
- if create_type == TaskWorkflowCreateForm.CreateType.WITH_TEMPLATE:
- template = form.cleaned_data["workflow_template"]
- self.object = template.create_task_workflow(**summary_data)
- elif create_type == TaskWorkflowCreateForm.CreateType.WITHOUT_TEMPLATE:
- with transaction.atomic():
- summary_task = Task.objects.create(**summary_data)
- self.object = TaskWorkflow.objects.create(
- summary_task=summary_task,
- )
+ template = form.cleaned_data["work_type"]
+ self.object = template.create_task_workflow(**data)
return super().form_valid(form)
def get_success_url(self):
return reverse(
- "workflow:task-workflow-ui-confirm-create",
+ "workflow:task-workflow-ui-detail",
kwargs={"pk": self.object.pk},
)
From 57bbf8e6b00bebbd8bfebe1af3524fe7b297b3f0 Mon Sep 17 00:00:00 2001
From: Dale Cannon <118175145+dalecannon@users.noreply.github.com>
Date: Wed, 5 Mar 2025 13:32:01 +0000
Subject: [PATCH 57/57] TP2000-1724 Redesign ticket edit view (#1421)
* Rename workflow as ticket
* Add EIF date and policy contact fields to ticket edit form
* Add back link to ticket edit form
* Update confirmation view
* Display EIF date and policy contact values on ticket detail view
* Update form and view tests
* Remove step assignees column on ticket detail view
* Add margin to stacked summary list value
* Hide step column headers on ticket detail view
---
common/static/common/scss/_components.scss | 6 ++--
common/util.py | 12 +++----
tasks/forms.py | 35 ++++++++++++++-----
tasks/jinja2/tasks/includes/task_list.jinja | 7 ++--
.../tasks/workflows/confirm_update.jinja | 25 +++++--------
tasks/jinja2/tasks/workflows/detail.jinja | 4 +--
tasks/jinja2/tasks/workflows/edit.jinja | 8 ++---
tasks/tests/test_forms.py | 12 +++++--
tasks/tests/test_views.py | 19 ++++++----
tasks/views.py | 20 +++++++++++
10 files changed, 95 insertions(+), 53 deletions(-)
diff --git a/common/static/common/scss/_components.scss b/common/static/common/scss/_components.scss
index a3024cd52..db21c71dc 100644
--- a/common/static/common/scss/_components.scss
+++ b/common/static/common/scss/_components.scss
@@ -175,15 +175,15 @@
@extend .govuk-summary-list__key;
display: block;
width: 100%;
- padding: 0;
- margin: 0;
+ padding: 0 !important;
+ margin: 0 !important;
}
.govuk-summary-list__value__stacked {
@extend .govuk-summary-list__value;
display: block;
padding: 0;
- margin: 0;
+ margin-bottom: govuk-spacing(2) !important;
}
.govuk-summary-list__value__stacked:last-child {
diff --git a/common/util.py b/common/util.py
index 69d431bdf..d011a19f3 100644
--- a/common/util.py
+++ b/common/util.py
@@ -690,16 +690,16 @@ def format_date_string(date_string: str, short_format=False) -> str:
return ""
-def format_date(value: datetime) -> str:
- """Formats a datetime object using `settings.DATE_FORMAT`."""
- if isinstance(value, datetime):
+def format_date(value: datetime | date) -> str:
+ """Formats a datetime or date object using `settings.DATE_FORMAT`."""
+ if isinstance(value, (datetime, date)):
return value.strftime(settings.DATE_FORMAT)
return value
-def format_short_date(value: datetime) -> str:
- """Formats a datetime object using `settings.SHORT_DATE_FORMAT`."""
- if isinstance(value, datetime):
+def format_short_date(value: datetime | date) -> str:
+ """Formats a datetime or date object using `settings.SHORT_DATE_FORMAT`."""
+ if isinstance(value, (datetime, date)):
return value.strftime(settings.SHORT_DATE_FORMAT)
return value
diff --git a/tasks/forms.py b/tasks/forms.py
index e026872cd..9b259bd42 100644
--- a/tasks/forms.py
+++ b/tasks/forms.py
@@ -1,6 +1,7 @@
from datetime import datetime
from crispy_forms_gds.helper import FormHelper
+from crispy_forms_gds.layout import HTML
from crispy_forms_gds.layout import Fieldset
from crispy_forms_gds.layout import Layout
from crispy_forms_gds.layout import Size
@@ -312,23 +313,31 @@ def __init__(self, *args, **kwargs):
class TaskWorkflowUpdateForm(ModelForm):
title = CharField(
+ label="Ticket name",
max_length=255,
validators=[SymbolValidator],
error_messages={
- "required": "Enter a title",
+ "required": "Enter a ticket name",
},
)
description = CharField(
validators=[SymbolValidator],
widget=Textarea(),
- error_messages={
- "required": "Enter a description",
- },
+ required=False,
+ )
+ eif_date = DateInputFieldFixed(
+ label="Entry into force date",
+ required=False,
+ )
+ policy_contact = CharField(
+ required=False,
+ max_length=40,
+ validators=[SymbolValidator],
)
class Meta:
model = TaskWorkflow
- fields = []
+ fields = ["eif_date", "policy_contact"]
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
@@ -346,22 +355,32 @@ def init_layout(self):
self.helper.layout = Layout(
"title",
"description",
+ "eif_date",
+ "policy_contact",
Submit(
"submit",
- "Update",
+ "Save",
data_module="govuk-button",
data_prevent_double_click="true",
+ css_class="govuk-!-margin-right-2",
+ ),
+ HTML(
+ f'Back',
),
)
@transaction.atomic
def save(self, commit=True):
- summary_task = self.instance.summary_task
+ instance = super().save(commit)
+
+ summary_task = instance.summary_task
summary_task.title = self.cleaned_data["title"]
summary_task.description = self.cleaned_data["description"]
+
if commit:
summary_task.save()
- return self.instance
+
+ return instance
class TaskWorkflowTemplateBaseForm(ModelForm):
diff --git a/tasks/jinja2/tasks/includes/task_list.jinja b/tasks/jinja2/tasks/includes/task_list.jinja
index 3e1a48c37..6aaff7228 100644
--- a/tasks/jinja2/tasks/includes/task_list.jinja
+++ b/tasks/jinja2/tasks/includes/task_list.jinja
@@ -1,5 +1,4 @@
{% from "components/table/macro.njk" import govukTable %}
-{% from "tasks/macros/display_details.jinja" import display_assignees %}
{% from "tasks/macros/display_details.jinja" import display_status %}
{% set table_rows = [] %}
@@ -11,16 +10,14 @@
{{ table_rows.append([
{"html": object_link},
- {"text": display_assignees(object.assignees.assigned())},
{"text": display_status(object.progress_state.__str__())},
]) or "" }}
{% endfor %}
{{ govukTable({
"head": [
- {"text": "Name"},
- {"text": "Assignee"},
- {"text": "Status"},
+ {"text": "Name", "classes": "govuk-visually-hidden"},
+ {"text": "Status", "classes": "govuk-visually-hidden"},
],
"rows": table_rows
}) }}
diff --git a/tasks/jinja2/tasks/workflows/confirm_update.jinja b/tasks/jinja2/tasks/workflows/confirm_update.jinja
index 9c4efc761..1aeaed0e9 100644
--- a/tasks/jinja2/tasks/workflows/confirm_update.jinja
+++ b/tasks/jinja2/tasks/workflows/confirm_update.jinja
@@ -4,15 +4,14 @@
{% from "components/panel/macro.njk" import govukPanel %}
{% from "components/button/macro.njk" import govukButton %}
-{% set object_name = object._meta.verbose_name %}
-{% set page_title = object_name|capitalize ~ " updated" %}
+{% set page_title = verbose_name|capitalize ~ " updated" %}
{% block breadcrumb %}
{{ breadcrumbs(
request,
[
- {"text": "Find and view " ~ object_name ~ "s", "href": object.get_url("list")},
- {"text": object_name|capitalize ~ ": " ~ object.title, "href": object.get_url("detail")},
+ {"text": "Find and view " ~ verbose_name ~ "s", "href": object.get_url("list")},
+ {"text": verbose_name|capitalize ~ " " ~ object.id, "href": object.get_url("detail")},
{"text": page_title}
],
with_workbasket=False,
@@ -21,27 +20,21 @@
{% block panel %}
{{ govukPanel({
- "titleText": object_name|capitalize ~ ": " ~ object,
- "text": "You have updated the " ~ object_name,
+ "titleText": verbose_name|capitalize ~ " " ~ object.id,
+ "text": "You have updated the " ~ verbose_name,
"classes": "govuk-!-margin-bottom-7"
}) }}
{% endblock %}
{% block button_group %}
{{ govukButton({
- "text": "View " ~ object_name,
+ "text": "View " ~ verbose_name,
"href": object.get_url("detail"),
"classes": "govuk-button"
}) }}
- {% if object_name == "workflow" %}
+ {% if verbose_name == "ticket template" %}
{{ govukButton({
- "text": "Create a task",
- "href": "#TODO",
- "classes": "govuk-button--secondary"
- }) }}
- {% elif object_name == "workflow template" %}
- {{ govukButton({
- "text": "Create a task template",
+ "text": "Add a step",
"href": url("workflow:task-template-ui-create", kwargs={"workflow_template_pk": object.pk}),
"classes": "govuk-button--secondary"
}) }}
@@ -49,5 +42,5 @@
{% endblock %}
{% block actions %}
- Find and view {{object_name}}s
+ Find and view {{verbose_name}}s
{% endblock %}
diff --git a/tasks/jinja2/tasks/workflows/detail.jinja b/tasks/jinja2/tasks/workflows/detail.jinja
index 1835e9856..16c1b852a 100644
--- a/tasks/jinja2/tasks/workflows/detail.jinja
+++ b/tasks/jinja2/tasks/workflows/detail.jinja
@@ -59,8 +59,8 @@
{{ display_summary_row_stacked("Assignee", display_assignees(object.summary_task.assignees.assigned())) }}
{{ display_summary_row_stacked("Work type", object.creator_template.title) }}
{{ display_summary_row_stacked("Created date", object.summary_task.created_at|format_date) }}
- {{ display_summary_row_stacked("Entry into force date", "None") }}
- {{ display_summary_row_stacked("Policy contact", "None") }}
+ {{ display_summary_row_stacked("Entry into force date", object.eif_date|format_date) }}
+ {{ display_summary_row_stacked("Policy contact", object.policy_contact or "None") }}
{% elif verbose_name == "ticket template" %}
{{ display_summary_row_stacked("Created by", object.creator.get_displayname()) }}
{{ display_summary_row_stacked("Created date", object.created_at|format_date) }}
diff --git a/tasks/jinja2/tasks/workflows/edit.jinja b/tasks/jinja2/tasks/workflows/edit.jinja
index f4789b63d..25feda85b 100644
--- a/tasks/jinja2/tasks/workflows/edit.jinja
+++ b/tasks/jinja2/tasks/workflows/edit.jinja
@@ -1,14 +1,12 @@
{% extends "layouts/form.jinja" %}
{% from "components/breadcrumbs.jinja" import breadcrumbs %}
-{% set object_name = object._meta.verbose_name %}
-
-{% set page_title = "Edit " ~ object_name ~ " details" %}
+{% set page_title = "Edit " ~ verbose_name ~ " " ~ object.id %}
{% block breadcrumb %}
{{ breadcrumbs(request, [
- {"text": "Find and view " ~ object_name ~ "s", "href": object.get_url("list")},
- {"text": object_name|capitalize ~ ": " ~ object.title, "href": object.get_url("detail")},
+ {"text": "Find and view " ~ verbose_name ~ "s", "href": object.get_url("list")},
+ {"text": verbose_name|capitalize ~ " " ~ object.id, "href": object.get_url("detail")},
{"text": page_title}
],
with_workbasket=False,
diff --git a/tasks/tests/test_forms.py b/tasks/tests/test_forms.py
index db30ff822..eb717e9ac 100644
--- a/tasks/tests/test_forms.py
+++ b/tasks/tests/test_forms.py
@@ -1,3 +1,5 @@
+from datetime import date
+
import pytest
from common.tests.factories import ProgressStateFactory
@@ -80,13 +82,19 @@ def test_workflow_update_form_save(task_workflow):
"""Tests that the details of `TaskWorkflow.summary_task` are updated when
calling form.save()."""
form_data = {
- "title": "Updated workflow title",
- "description": "Updated workflow description",
+ "title": "Updated title",
+ "description": "Updated description",
+ "eif_date_0": date.today().day,
+ "eif_date_1": date.today().month,
+ "eif_date_2": date.today().year,
+ "policy_contact": "Policy contact",
}
form = forms.TaskWorkflowUpdateForm(data=form_data, instance=task_workflow)
assert form.is_valid()
workflow = form.save()
+ assert workflow.eif_date == date.today()
+ assert workflow.policy_contact == form_data["policy_contact"]
assert workflow.summary_task.title == form_data["title"]
assert workflow.summary_task.description == form_data["description"]
diff --git a/tasks/tests/test_views.py b/tasks/tests/test_views.py
index 27123ca24..5adde8201 100644
--- a/tasks/tests/test_views.py
+++ b/tasks/tests/test_views.py
@@ -1,3 +1,5 @@
+from datetime import date
+
import factory
import pytest
from bs4 import BeautifulSoup
@@ -21,8 +23,6 @@
pytestmark = pytest.mark.django_db
-pytestmark = pytest.mark.django_db
-
def test_task_update_view_update_progress_state(valid_user_client):
"""Tests that `TaskUpdateView` updates `Task.progress_state` and that a
@@ -355,7 +355,9 @@ def test_workflow_template_update_view(
assert confirmation_response.status_code == 200
soup = BeautifulSoup(str(confirmation_response.content), "html.parser")
- assert task_workflow_template.title in soup.select("h1.govuk-panel__title")[0].text
+ assert (
+ str(task_workflow_template.id) in soup.select("h1.govuk-panel__title")[0].text
+ )
def test_workflow_template_delete_view(
@@ -589,6 +591,9 @@ def test_workflow_detail_view_displays_tasks(
task_workflow_single_task_item,
):
workflow = task_workflow_single_task_item
+ workflow.policy_contact = "Policy contact"
+ workflow.eif_date = date.today()
+ workflow.save()
workbasket = workflow.summary_task.workbasket
url = reverse(
@@ -604,6 +609,8 @@ def test_workflow_detail_view_displays_tasks(
assert page.find("p", text=workflow.description)
assert page.find("a", text=f"{workbasket.pk} - {workbasket.status}")
assert page.find("dd", text=format_date(workflow.summary_task.created_at))
+ assert page.find("dd", text=format_date(workflow.eif_date))
+ assert page.find("dd", text=workflow.policy_contact)
step_rows = page.select(".govuk-table__body > .govuk-table__row")
assert len(step_rows) == workflow.get_tasks().count()
@@ -661,8 +668,8 @@ def test_workflow_update_view(
confirmation view returns a HTTP 200 response."""
form_data = {
- "title": "Updated workflow title",
- "description": "Updated workflow description",
+ "title": "Updated title",
+ "description": "Updated description",
}
update_url = task_workflow.get_url("edit")
@@ -683,7 +690,7 @@ def test_workflow_update_view(
assert confirmation_response.status_code == 200
soup = BeautifulSoup(str(confirmation_response.content), "html.parser")
- assert str(task_workflow) in soup.select("h1.govuk-panel__title")[0].text
+ assert str(task_workflow.id) in soup.select("h1.govuk-panel__title")[0].text
def test_workflow_delete_view(
diff --git a/tasks/views.py b/tasks/views.py
index f47ff1234..71dd09e25 100644
--- a/tasks/views.py
+++ b/tasks/views.py
@@ -590,6 +590,11 @@ class TaskWorkflowUpdateView(PermissionRequiredMixin, UpdateView):
permission_required = "tasks.change_taskworkflow"
form_class = TaskWorkflowUpdateForm
+ def get_context_data(self, **kwargs):
+ context = super().get_context_data(**kwargs)
+ context["verbose_name"] = "ticket"
+ return context
+
def get_success_url(self):
return reverse(
"workflow:task-workflow-ui-confirm-update",
@@ -602,6 +607,11 @@ class TaskWorkflowConfirmUpdateView(PermissionRequiredMixin, DetailView):
template_name = "tasks/workflows/confirm_update.jinja"
permission_required = "tasks.change_taskworkflow"
+ def get_context_data(self, **kwargs):
+ context = super().get_context_data(**kwargs)
+ context["verbose_name"] = "ticket"
+ return context
+
class TaskWorkflowDeleteView(PermissionRequiredMixin, DeleteView):
model = TaskWorkflow
@@ -775,6 +785,11 @@ class TaskWorkflowTemplateUpdateView(PermissionRequiredMixin, UpdateView):
permission_required = "tasks.change_taskworkflowtemplate"
form_class = TaskWorkflowTemplateUpdateForm
+ def get_context_data(self, **kwargs):
+ context = super().get_context_data(**kwargs)
+ context["verbose_name"] = "ticket template"
+ return context
+
def get_success_url(self):
return reverse(
"workflow:task-workflow-template-ui-confirm-update",
@@ -787,6 +802,11 @@ class TaskWorkflowTemplateConfirmUpdateView(PermissionRequiredMixin, DetailView)
template_name = "tasks/workflows/confirm_update.jinja"
permission_required = "tasks.change_taskworkflowtemplate"
+ def get_context_data(self, **kwargs):
+ context = super().get_context_data(**kwargs)
+ context["verbose_name"] = "ticket template"
+ return context
+
class TaskWorkflowTemplateDeleteView(PermissionRequiredMixin, DeleteView):
model = TaskWorkflowTemplate