diff --git a/common/models/mixins/__init__.py b/common/models/mixins/__init__.py index d2e203dd5..0592410b9 100644 --- a/common/models/mixins/__init__.py +++ b/common/models/mixins/__init__.py @@ -1,6 +1,7 @@ """Mixins for models.""" from django.db import models +from django.db.models import signals class TimestampedMixin(models.Model): @@ -11,3 +12,37 @@ class TimestampedMixin(models.Model): class Meta: abstract = True + + +class WithSignalManagerMixin: + """A mixin that overrides default Manager methods to send pre_save signals + for model instances.""" + + def bulk_create(self, objs, **kwargs) -> list: + for obj in objs: + signals.pre_save.send(sender=self.model, instance=obj) + return super().bulk_create(objs, **kwargs) + + +class WithSignalQuerysetMixin: + """A mixin that overrides default QuerySet methods to send pre_save signals + for model instances.""" + + def update(self, **kwargs) -> int: + old_instances_map = {} + pk_list = [] + for old_instance in self.iterator(): + old_instances_map[old_instance.pk] = old_instance + pk_list.append(old_instance.pk) + + rows_updated = super().update(**kwargs) + + for new_instance in self.model.objects.filter(pk__in=pk_list): + old_instance = old_instances_map.get(new_instance.pk) + signals.pre_save.send( + sender=self.model, + instance=new_instance, + old_instance=old_instance, + ) + + return rows_updated diff --git a/common/tests/factories.py b/common/tests/factories.py index f3335c637..34f0bdbf2 100644 --- a/common/tests/factories.py +++ b/common/tests/factories.py @@ -7,6 +7,7 @@ import factory from django.contrib.auth import get_user_model +from django.db.models import signals from factory.fuzzy import FuzzyChoice from factory.fuzzy import FuzzyText from faker import Faker @@ -1565,6 +1566,7 @@ class SubTaskFactory(TaskFactory): parent_task = factory.SubFactory(TaskFactory) +@factory.django.mute_signals(signals.pre_save) class TaskAssigneeFactory(factory.django.DjangoModelFactory): user = factory.SubFactory(UserFactory) assignment_type = FuzzyChoice(TaskAssignee.AssignmentType.values) diff --git a/conftest.py b/conftest.py index 54ff9db39..8c9ed10c0 100644 --- a/conftest.py +++ b/conftest.py @@ -290,7 +290,7 @@ def api_client() -> APIClient: @pytest.fixture def policy_group(db) -> Group: - group = factories.UserGroupFactory.create(name="Policy") + group = factories.UserGroupFactory.create(name="Tariff Managers") for app_label, codename in [ ("common", "add_trackedmodel"), @@ -301,6 +301,9 @@ def policy_group(db) -> Group: ("publishing", "consume_from_packaging_queue"), ("publishing", "manage_packaging_queue"), ("publishing", "view_envelope"), + ("tasks", "add_task"), + ("tasks", "change_task"), + ("tasks", "delete_task"), ("tasks", "add_taskassignee"), ("tasks", "change_taskassignee"), ("tasks", "add_comment"), diff --git a/tasks/admin.py b/tasks/admin.py index 593611372..01086b346 100644 --- a/tasks/admin.py +++ b/tasks/admin.py @@ -6,6 +6,7 @@ from tasks.models import ProgressState from tasks.models import Task from tasks.models import TaskAssignee +from tasks.models import TaskLog class TaskAdminMixin: @@ -77,6 +78,19 @@ def task_id(self, obj): return self.link_to_task(obj.task) +class TaskLogAdmin(admin.ModelAdmin): + list_display = ["task", "action", "created_at"] + list_filter = ["action"] + readonly_fields = [ + "id", + "action", + "description", + "task", + "instigator", + "created_at", + ] + + admin.site.register(Task, TaskAdmin) admin.site.register(Category, CategoryAdmin) @@ -84,3 +98,5 @@ def task_id(self, obj): admin.site.register(ProgressState, ProgressStateAdmin) admin.site.register(TaskAssignee, TaskAssigneeAdmin) + +admin.site.register(TaskLog, TaskLogAdmin) diff --git a/tasks/filters.py b/tasks/filters.py new file mode 100644 index 000000000..b65c79793 --- /dev/null +++ b/tasks/filters.py @@ -0,0 +1,28 @@ +from django.forms import CheckboxSelectMultiple +from django.urls import reverse_lazy +from django_filters import ModelMultipleChoiceFilter + +from common.filters import TamatoFilter +from tasks.models import ProgressState +from tasks.models import Task + + +class TaskFilter(TamatoFilter): + + search_fields = ( + "id", + "title", + "description", + ) + clear_url = reverse_lazy("workflow:task-ui-list") + + progress_state = ModelMultipleChoiceFilter( + label="Status", + help_text="Select all that apply", + queryset=ProgressState.objects.all(), + widget=CheckboxSelectMultiple, + ) + + class Meta: + model = Task + fields = ["search", "category", "progress_state"] diff --git a/tasks/forms.py b/tasks/forms.py new file mode 100644 index 000000000..9334f1ef6 --- /dev/null +++ b/tasks/forms.py @@ -0,0 +1,70 @@ +from crispy_forms_gds.helper import FormHelper +from crispy_forms_gds.layout import Layout +from crispy_forms_gds.layout import Size +from crispy_forms_gds.layout import Submit +from django.forms import ModelForm + +from common.forms import delete_form_for +from tasks.models import Task +from workbaskets.models import WorkBasket + + +class TaskBaseForm(ModelForm): + class Meta: + model = Task + exclude = ["parent_task", "creator"] + + error_messages = { + "title": { + "required": "Enter a title", + }, + "description": { + "required": "Enter a description", + }, + "progress_state": { + "required": "Select a status", + }, + } + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.init_fields() + self.init_layout() + + 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) + self.helper.label_size = Size.SMALL + self.helper.legend_size = Size.SMALL + self.helper.layout = Layout( + "title", + "description", + "category", + "progress_state", + "workbasket", + Submit( + "submit", + "Save", + data_module="govuk-button", + data_prevent_double_click="true", + ), + ) + + +class TaskCreateForm(TaskBaseForm): + def save(self, user, commit=True): + instance = super().save(commit=False) + instance.creator = user + if commit: + instance.save() + return instance + + +class TaskUpdateForm(TaskBaseForm): + pass + + +TaskDeleteForm = delete_form_for(Task) diff --git a/tasks/jinja2/tasks/confirm_create.jinja b/tasks/jinja2/tasks/confirm_create.jinja new file mode 100644 index 000000000..f4feb04b4 --- /dev/null +++ b/tasks/jinja2/tasks/confirm_create.jinja @@ -0,0 +1,42 @@ +{% extends "common/confirm_create.jinja" %} + +{% from "components/breadcrumbs/macro.njk" import govukBreadcrumbs %} +{% from "components/panel/macro.njk" import govukPanel %} +{% from "components/button/macro.njk" import govukButton %} + +{% set page_title = "Task created" %} + +{% block breadcrumb %} + {{ govukBreadcrumbs({ + "items": [ + {"text": "Home", "href": url("home")}, + {"text": "Create a new task", "href": url("workflow:task-ui-create") }, + {"text": "Task created"} + ] + }) }} +{% endblock %} + +{% block panel %} + {{ govukPanel({ + "titleText": "Task: " ~ object.title, + "text": "You have created a new task", + "classes": "govuk-!-margin-bottom-7" + }) }} +{% endblock %} + +{% block button_group %} + {{ govukButton({ + "text": "View task", + "href": url('workflow:task-ui-detail', kwargs={"pk": object.pk}), + "classes": "govuk-button" + }) }} + {{ govukButton({ + "text": "Return to homepage", + "href": url("home"), + "classes": "govuk-button--secondary" + }) }} +{% endblock %} + +{% block actions %} +
  • Find and view tasks
    • +{% endblock %} diff --git a/tasks/jinja2/tasks/confirm_delete.jinja b/tasks/jinja2/tasks/confirm_delete.jinja new file mode 100644 index 000000000..65b53069a --- /dev/null +++ b/tasks/jinja2/tasks/confirm_delete.jinja @@ -0,0 +1,41 @@ +{% extends "layouts/confirm.jinja" %} + +{% from "components/breadcrumbs/macro.njk" import govukBreadcrumbs %} +{% from "components/panel/macro.njk" import govukPanel %} +{% from "components/button/macro.njk" import govukButton %} + +{% set page_title = "Task deleted" %} + +{% block breadcrumb %} + {{ govukBreadcrumbs({ + "items": [ + {"text": "Find and view tasks", "href": url("workflow:task-ui-list")}, + {"text": page_title} + ] + }) }} +{% endblock %} + +{% block panel %} + {{ govukPanel({ + "titleText": "Task ID: " ~ deleted_pk, + "text": "Task has been deleted", + "classes": "govuk-!-margin-bottom-7" + }) }} +{% endblock %} + +{% block button_group %} + {{ govukButton({ + "text": "Find and view tasks", + "href": url("workflow:task-ui-list"), + "classes": "govuk-button" + }) }} + {{ govukButton({ + "text": "Return to homepage", + "href": url("home"), + "classes": "govuk-button--secondary" + }) }} +{% endblock %} + +{% block actions %} +
  • Create a task
    • +{% endblock %} diff --git a/tasks/jinja2/tasks/confirm_update.jinja b/tasks/jinja2/tasks/confirm_update.jinja new file mode 100644 index 000000000..7e6b712ff --- /dev/null +++ b/tasks/jinja2/tasks/confirm_update.jinja @@ -0,0 +1,41 @@ +{% extends "common/confirm_update.jinja" %} + +{% from "components/breadcrumbs/macro.njk" import govukBreadcrumbs %} +{% from "components/panel/macro.njk" import govukPanel %} +{% from "components/button/macro.njk" import govukButton %} + +{% set page_title = "Task updated" %} + +{% block breadcrumb %} + {{ govukBreadcrumbs({ + "items": [ + {"text": "Task: " ~ object.title, "href": url("workflow:task-ui-detail", kwargs={"pk": object.pk})}, + {"text": page_title} + ] + }) }} +{% endblock %} + +{% block panel %} + {{ govukPanel({ + "titleText": "Task: " ~ object.title, + "text": "Task has been updated", + "classes": "govuk-!-margin-bottom-7" + }) }} +{% endblock %} + +{% block button_group %} + {{ govukButton({ + "text": "View task", + "href": url('workflow:task-ui-detail', kwargs={"pk": object.pk}), + "classes": "govuk-button" + }) }} + {{ govukButton({ + "text": "Return to homepage", + "href": url("home"), + "classes": "govuk-button--secondary" + }) }} +{% endblock %} + +{% block actions %} +
  • Find and view tasks
    • +{% endblock %} diff --git a/tasks/jinja2/tasks/delete.jinja b/tasks/jinja2/tasks/delete.jinja new file mode 100644 index 000000000..9cc761efc --- /dev/null +++ b/tasks/jinja2/tasks/delete.jinja @@ -0,0 +1,26 @@ +{% extends "common/delete.jinja" %} + +{% from "components/breadcrumbs.jinja" import breadcrumbs %} +{% from "components/warning-text/macro.njk" import govukWarningText %} +{% from "components/button/macro.njk" import govukButton %} + +{% set page_title = "Delete Task:" ~ object.title %} + +{% 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 %} + {{ govukWarningText({ + "text": "Are you sure you want to delete this task?" + }) }} + + {% call django_form(action=url("workflow:task-ui-delete", kwargs={"pk": object.pk})) %} + {{ crispy(form) }} + {% endcall %} +{% endblock %} diff --git a/tasks/jinja2/tasks/detail.jinja b/tasks/jinja2/tasks/detail.jinja new file mode 100644 index 000000000..665b4564b --- /dev/null +++ b/tasks/jinja2/tasks/detail.jinja @@ -0,0 +1,115 @@ +{% extends "layouts/layout.jinja" %} +{% from "components/breadcrumbs.jinja" import breadcrumbs %} +{% from "components/button/macro.njk" import govukButton %} + +{% set page_title = "Task: " ~ object.title %} + +{% set edit_url = url("workflow:task-ui-update", kwargs={"pk": object.pk}) %} + +{% block breadcrumb %} + {{ breadcrumbs(request, [ + {"text": "Find and view tasks", "href": url("workflow:task-ui-list")}, + {"text": page_title} + ] + ) }} +{% endblock %} + +{% block content %} +
    +
    +

    {{ page_title }}

    +
    +
    + +

    Details

    + +
    +
    +
    + ID +
    +
    + {{ object.pk }} +
    +
    + +
    +
    + Title +
    +
    + {{ object.title }} +
    +
    + Change title +
    +
    + +
    +
    + Description +
    +
    + {{ object.description }} +
    +
    + Change description +
    +
    + +
    +
    + Category +
    +
    + {{ object.category }} +
    +
    + Change category +
    +
    + +
    +
    + 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

    +
    + {{ crispy(filter.form) }} +
    +
    + +
    + {% 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:

    + + {% 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())