From d5804dce7d674e54dbd37cc6cace2e8cbae8f991 Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Tue, 26 Nov 2024 11:11:51 +0000 Subject: [PATCH 01/17] Save Workflow instance in forward op to fix migration application --- tasks/migrations/0012_taskworkflow_assign_summary_task.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/migrations/0012_taskworkflow_assign_summary_task.py b/tasks/migrations/0012_taskworkflow_assign_summary_task.py index 38f0dda86..26e22ed3b 100644 --- a/tasks/migrations/0012_taskworkflow_assign_summary_task.py +++ b/tasks/migrations/0012_taskworkflow_assign_summary_task.py @@ -12,7 +12,7 @@ def assign_new_summary_task_on_workflow(apps, schema_editor): title="Workflow summarising task", description="Workflow summarising task.", ) - TaskWorkflow.save() + task_workflow.save() class Migration(migrations.Migration): From 981fa4138c256351a16801a3250e190b52591465 Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Tue, 26 Nov 2024 11:44:38 +0000 Subject: [PATCH 02/17] Have TaskWorkflowTemplate.create_task_workflow() create summary task from data param --- tasks/models/workflow.py | 14 +++++++++----- tasks/tests/test_workflow_models.py | 15 +++++++++------ 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/tasks/models/workflow.py b/tasks/models/workflow.py index 35f149252..0ff7e3c6f 100644 --- a/tasks/models/workflow.py +++ b/tasks/models/workflow.py @@ -91,13 +91,17 @@ def get_task_templates(self) -> models.QuerySet: ) @atomic - def create_task_workflow(self) -> "TaskWorkflow": - """Create a workflow and it subtasks, using values from this template - workflow and its task templates.""" + def create_task_workflow(self, summary_data: dict) -> "TaskWorkflow": + """ + Create a workflow and it subtasks, using values from this template + workflow and its task templates. + The `summary_data` param is used to populate the details of the `TaskWorkflow.summary_task` instance. + """ + + summary_task = Task.objects.create(**summary_data) task_workflow = TaskWorkflow.objects.create( - title=self.title, - description=self.description, + summary_task=summary_task, creator_template=self, ) diff --git a/tasks/tests/test_workflow_models.py b/tasks/tests/test_workflow_models.py index 270e7b055..d80d646cc 100644 --- a/tasks/tests/test_workflow_models.py +++ b/tasks/tests/test_workflow_models.py @@ -13,8 +13,14 @@ def test_create_task_workflow_from_task_workflow_template( ): """Test creation of TaskWorkflow instances from TaskWorkflowTemplates using its `create_task_workflow()` method.""" + summary_data = { + "title": "Workflow title", + "description": "Workflow description", + } task_workflow = ( - task_workflow_template_three_task_template_items.create_task_workflow() + task_workflow_template_three_task_template_items.create_task_workflow( + summary_data, + ) ) # Test that workflow values are valid. @@ -22,11 +28,8 @@ def test_create_task_workflow_from_task_workflow_template( task_workflow.creator_template == task_workflow_template_three_task_template_items ) - assert task_workflow.title == task_workflow_template_three_task_template_items.title - assert ( - task_workflow.description - == task_workflow_template_three_task_template_items.description - ) + assert task_workflow.summary_task.title == summary_data["title"] + assert task_workflow.summary_task.description == summary_data["description"] assert task_workflow.get_items().count() == 3 # Validate that item positions are equivalent. From cf6f92acdc642ff7e594ffed60802cc4a2833adf Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Tue, 26 Nov 2024 13:36:17 +0000 Subject: [PATCH 03/17] Override TaskWorkflowTemplate.__str__ to display instance title --- tasks/models/workflow.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tasks/models/workflow.py b/tasks/models/workflow.py index 0ff7e3c6f..4084b11ca 100644 --- a/tasks/models/workflow.py +++ b/tasks/models/workflow.py @@ -82,6 +82,9 @@ class TaskWorkflowTemplate(Queue): instance generated from a template. """ + def __str__(self): + return self.title + def get_task_templates(self) -> models.QuerySet: """Get a QuerySet of the TaskTemplates associated through their TaskItemTemplate instances to this TaskWorkflowTemplate, ordered by the From 11793df8f6b6543bf0410524b1ec8c673e189407 Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Tue, 26 Nov 2024 13:42:50 +0000 Subject: [PATCH 04/17] Add UI capability to create workflows with or without workflow templates --- tasks/forms.py | 76 +++++++++++++++++++ .../{template_create.jinja => create.jinja} | 2 +- tasks/urls.py | 6 +- tasks/views.py | 63 ++++++++++++++- 4 files changed, 144 insertions(+), 3 deletions(-) rename tasks/jinja2/tasks/workflows/{template_create.jinja => create.jinja} (85%) diff --git a/tasks/forms.py b/tasks/forms.py index 894868276..39bcf2ea6 100644 --- a/tasks/forms.py +++ b/tasks/forms.py @@ -1,10 +1,19 @@ from crispy_forms_gds.helper import FormHelper +from crispy_forms_gds.layout import Fieldset from crispy_forms_gds.layout import Layout from crispy_forms_gds.layout import Size from crispy_forms_gds.layout import Submit +from django.db.models import TextChoices +from django.forms import CharField +from django.forms import Form +from django.forms import ModelChoiceField from django.forms import ModelForm +from django.forms import Textarea +from common.forms import BindNestedFormMixin +from common.forms import RadioNested from common.forms import delete_form_for +from common.validators import SymbolValidator from tasks.models import Task from tasks.models import TaskTemplate from tasks.models import TaskWorkflowTemplate @@ -82,6 +91,73 @@ def save(self, parent_task, user, commit=True): TaskDeleteForm = delete_form_for(Task) +class TaskWorkflowTemplateForm(Form): + workflow_template = ModelChoiceField( + label="", + queryset=TaskWorkflowTemplate.objects.all(), + help_text="Select a workflow template.", + error_messages={ + "required": "Select a workflow template", + }, + ) + + +class TaskWorkflowCreateForm(BindNestedFormMixin, Form): + class CreateType(TextChoices): + WITH_TEMPLATE = "WITH_TEMPLATE", "Yes" + WITHOUT_TEMPLATE = "WITHOUT_TEMPLATE", "No" + + title = CharField( + max_length=255, + validators=[SymbolValidator], + error_messages={ + "required": "Enter a title for the workflow", + }, + ) + + description = CharField( + validators=[SymbolValidator], + widget=Textarea(), + error_messages={ + "required": "Enter a description for the workflow", + }, + ) + + 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", + }, + ) + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + self.bind_nested_forms(*args, **kwargs) + + self.helper = FormHelper(self) + self.helper.label_size = Size.SMALL + self.helper.legend_size = Size.SMALL + self.helper.layout = Layout( + Fieldset( + "title", + "description", + ), + "create_type", + Submit( + "submit", + "Create", + data_module="govuk-button", + data_prevent_double_click="true", + ), + ) + + class TaskWorkflowTemplateBaseForm(ModelForm): class Meta: model = TaskWorkflowTemplate diff --git a/tasks/jinja2/tasks/workflows/template_create.jinja b/tasks/jinja2/tasks/workflows/create.jinja similarity index 85% rename from tasks/jinja2/tasks/workflows/template_create.jinja rename to tasks/jinja2/tasks/workflows/create.jinja index f2ec118c6..423b43452 100644 --- a/tasks/jinja2/tasks/workflows/template_create.jinja +++ b/tasks/jinja2/tasks/workflows/create.jinja @@ -2,7 +2,7 @@ {% from "components/breadcrumbs.jinja" import breadcrumbs %} -{% set page_title = "Create a workflow template" %} +{% set page_title = "Create a " ~ verbose_name %} {% block breadcrumb %} {{ breadcrumbs( diff --git a/tasks/urls.py b/tasks/urls.py index 89589ebd7..7cec677ec 100644 --- a/tasks/urls.py +++ b/tasks/urls.py @@ -72,7 +72,11 @@ workflow_ui_patterns = [ - # TODO + path( + "create/", + views.TaskWorkflowCreateView.as_view(), + name="task-workflow-ui-create", + ), ] workflow_template_ui_patterns = [ diff --git a/tasks/views.py b/tasks/views.py index 67e3e8e48..0ae25f2bb 100644 --- a/tasks/views.py +++ b/tasks/views.py @@ -9,6 +9,7 @@ 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 FormView from django.views.generic.edit import UpdateView from common.views import SortingMixin @@ -21,12 +22,14 @@ from tasks.forms import TaskTemplateDeleteForm from tasks.forms import TaskTemplateUpdateForm from tasks.forms import TaskUpdateForm +from tasks.forms import TaskWorkflowCreateForm from tasks.forms import TaskWorkflowTemplateCreateForm from tasks.forms import TaskWorkflowTemplateDeleteForm from tasks.forms import TaskWorkflowTemplateUpdateForm from tasks.models import Task from tasks.models import TaskItemTemplate from tasks.models import TaskTemplate +from tasks.models import TaskWorkflow from tasks.models import TaskWorkflowTemplate from tasks.signals import set_current_instigator @@ -257,6 +260,59 @@ def get_context_data(self, **kwargs): return context_data +class TaskWorkflowCreateView(PermissionRequiredMixin, FormView): + 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" + return context + + def form_valid(self, form): + summary_data = { + "title": form.cleaned_data["title"], + "description": form.cleaned_data["description"], + "creator": self.request.user, + } + create_type = form.cleaned_data["create_type"] + + if create_type == TaskWorkflowCreateForm.CreateType.WITH_TEMPLATE: + self.object = self.create_with_template( + template=form.cleaned_data["workflow_template"], + summary_data=summary_data, + ) + elif create_type == TaskWorkflowCreateForm.CreateType.WITHOUT_TEMPLATE: + self.object = self.create_without_template(summary_data) + + return super().form_valid(form) + + def create_with_template( + self, + template: TaskWorkflowTemplate, + summary_data: dict, + ) -> TaskWorkflow: + template = get_object_or_404( + TaskWorkflowTemplate, + pk=template.pk, + ) + return template.create_task_workflow(summary_data) + + @transaction.atomic + def create_without_template(self, summary_data: dict) -> TaskWorkflow: + summary_task = Task.objects.create(**summary_data) + return TaskWorkflow.objects.create( + summary_task=summary_task, + ) + + def get_success_url(self): + return reverse( + "workflow:task-workflow-ui-confirm-create", + kwargs={"pk": self.object.pk}, + ) + + class TaskWorkflowTemplateDetailView(PermissionRequiredMixin, DetailView): model = TaskWorkflowTemplate template_name = "tasks/workflows/template_detail.jinja" @@ -338,9 +394,14 @@ def demote_to_last(self, task_template_id: int) -> None: class TaskWorkflowTemplateCreateView(PermissionRequiredMixin, CreateView): model = TaskWorkflowTemplate permission_required = "tasks.add_taskworkflowtemplate" - template_name = "tasks/workflows/template_create.jinja" + template_name = "tasks/workflows/create.jinja" form_class = TaskWorkflowTemplateCreateForm + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["verbose_name"] = "workflow template" + return context + def get_success_url(self): return reverse( "workflow:task-workflow-template-ui-confirm-create", From d094dbb47a073a9752fad6fcf1a591ba3f959d5d Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Tue, 26 Nov 2024 13:56:04 +0000 Subject: [PATCH 05/17] Make confirm_create template usable for workflow and worklow template models --- .../tasks/workflows/confirm_create.jinja | 61 +++++++++++++++++++ .../workflows/template_confirm_create.jinja | 47 -------------- ...015_alter_taskworkflow_options_and_more.py | 21 +++++++ tasks/models/workflow.py | 45 +++++++++++++- 4 files changed, 125 insertions(+), 49 deletions(-) create mode 100644 tasks/jinja2/tasks/workflows/confirm_create.jinja delete mode 100644 tasks/jinja2/tasks/workflows/template_confirm_create.jinja create mode 100644 tasks/migrations/0015_alter_taskworkflow_options_and_more.py diff --git a/tasks/jinja2/tasks/workflows/confirm_create.jinja b/tasks/jinja2/tasks/workflows/confirm_create.jinja new file mode 100644 index 000000000..a84c0b9d6 --- /dev/null +++ b/tasks/jinja2/tasks/workflows/confirm_create.jinja @@ -0,0 +1,61 @@ +{% extends "common/confirm_create.jinja" %} + +{% from "components/breadcrumbs.jinja" import breadcrumbs %} +{% 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 ~ " created" %} + +{% block breadcrumb %} + {{ breadcrumbs( + request, + [ + { + "text": "Find and view workflow templates", + "href": object.get_url("list")}, + { + "text": "Create a " ~ object_name, + "href": object.get_url("create"), + }, + {"text": page_title} + ], + False, + ) }} +{% endblock %} + +{% block panel %} + {{ govukPanel({ + "titleText": object_name|capitalize ~ ": " ~ object, + "text": "You have created a new " ~ object_name, + "classes": "govuk-!-margin-bottom-7" + }) }} +{% endblock %} + +{% block button_group %} + {{ govukButton({ + "text": "View " ~ object_name, + "href": object.get_url("detail"), + "classes": "govuk-button" + }) }} + + {% if object_name == "workflow" %} + {{ govukButton({ + "text": "Create a task", + "href": "#TODO", + "classes": "govuk-button--secondary" + }) }} + {% elif object_name == "workflow template" %} + {{ govukButton({ + "text": "Create a task template", + "href": url("workflow:task-template-ui-create", kwargs={"workflow_template_pk": object.pk}), + "classes": "govuk-button--secondary" + }) }} + {% endif %} +{% endblock %} + +{% block actions %} +
  • + Find and view {{object_name}}s +
    • +{% endblock %} diff --git a/tasks/jinja2/tasks/workflows/template_confirm_create.jinja b/tasks/jinja2/tasks/workflows/template_confirm_create.jinja deleted file mode 100644 index 62f9ee5fe..000000000 --- a/tasks/jinja2/tasks/workflows/template_confirm_create.jinja +++ /dev/null @@ -1,47 +0,0 @@ -{% extends "common/confirm_create.jinja" %} - -{% from "components/breadcrumbs.jinja" import breadcrumbs %} -{% from "components/panel/macro.njk" import govukPanel %} -{% from "components/button/macro.njk" import govukButton %} - -{% set page_title = "Workflow template created" %} - -{% block breadcrumb %} - {{ breadcrumbs( - request, - [ - {"text": "Find and view workflow templates", "href": "#TODO"}, - { - "text": "Create a workflow template", - "href": url("workflow:task-workflow-template-ui-create"), - }, - {"text": page_title} - ], - False, - ) }} -{% endblock %} - -{% block panel %} - {{ govukPanel({ - "titleText": "Workflow template: " ~ object.title, - "text": "You have created a new workflow template", - "classes": "govuk-!-margin-bottom-7" - }) }} -{% endblock %} - -{% block button_group %} - {{ govukButton({ - "text": "View workflow template", - "href": url("workflow:task-workflow-template-ui-detail", kwargs={"pk": object.pk}), - "classes": "govuk-button" - }) }} - {{ govukButton({ - "text": "Create a task template", - "href": url("workflow:task-template-ui-create", kwargs={"workflow_template_pk": object.pk}), - "classes": "govuk-button--secondary" - }) }} -{% endblock %} - -{% block actions %} -
  • Find and view workflow templates
    • -{% endblock %} diff --git a/tasks/migrations/0015_alter_taskworkflow_options_and_more.py b/tasks/migrations/0015_alter_taskworkflow_options_and_more.py new file mode 100644 index 000000000..41e835e11 --- /dev/null +++ b/tasks/migrations/0015_alter_taskworkflow_options_and_more.py @@ -0,0 +1,21 @@ +# Generated by Django 4.2.15 on 2024-11-26 13:55 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("tasks", "0014_remove_taskworkflow_description_and_more"), + ] + + operations = [ + migrations.AlterModelOptions( + name="taskworkflow", + options={"verbose_name": "workflow"}, + ), + migrations.AlterModelOptions( + name="taskworkflowtemplate", + options={"verbose_name": "workflow template"}, + ), + ] diff --git a/tasks/models/workflow.py b/tasks/models/workflow.py index 4084b11ca..6b492eca9 100644 --- a/tasks/models/workflow.py +++ b/tasks/models/workflow.py @@ -28,14 +28,44 @@ class TaskWorkflow(Queue): ) """The template from which this workflow was created, if any.""" + class Meta: + verbose_name = "workflow" + + def __str__(self): + return self.summary_task.title + def get_tasks(self) -> models.QuerySet: """Get a QuerySet of the Tasks associated through their TaskItem instances to this TaskWorkflow, ordered by the position of the TaskItem.""" return Task.objects.filter(taskitem__queue=self).order_by("taskitem__position") - def __str__(self): - return self.title + def get_url(self, action: str = "detail"): + # if action == "detail": + # return reverse( + # "workflow:task-workflow-ui-detail", + # kwargs={"pk": self.pk}, + # ) + # elif action == "edit": + # return reverse( + # "workflow:task-workflow-ui-update", + # kwargs={"pk": self.pk}, + # ) + # elif action == "delete": + # return reverse( + # "workflow:task-workflow-ui-delete", + # kwargs={"pk": self.pk}, + # ) + if action == "create": + return reverse( + "workflow:task-workflow-ui-create", + ) + # elif action == "list": + # return reverse( + # "workflow:task-workflow-ui-list", + # ) + + return "#NOT-IMPLEMENTED" class TaskItem(QueueItem): @@ -82,6 +112,9 @@ class TaskWorkflowTemplate(Queue): instance generated from a template. """ + class Meta: + verbose_name = "workflow template" + def __str__(self): return self.title @@ -142,6 +175,14 @@ def get_url(self, action: str = "detail"): "workflow:task-workflow-template-ui-delete", kwargs={"pk": self.pk}, ) + elif action == "create": + return reverse( + "workflow:task-workflow-template-ui-create", + ) + # elif action == "list": + # return reverse( + # "workflow:task-workflow-template-ui-list", + # ) return "#NOT-IMPLEMENTED" From ab9f93706603ddfa33ad65a5c9b32ed2eca3ec1e Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Tue, 26 Nov 2024 14:26:26 +0000 Subject: [PATCH 06/17] Add workflow creation confirmation view --- tasks/urls.py | 5 +++++ tasks/views.py | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/tasks/urls.py b/tasks/urls.py index 7cec677ec..6aaf5aee0 100644 --- a/tasks/urls.py +++ b/tasks/urls.py @@ -77,6 +77,11 @@ views.TaskWorkflowCreateView.as_view(), name="task-workflow-ui-create", ), + path( + "/confirm-create/", + views.TaskWorkflowConfirmCreateView.as_view(), + name="task-workflow-ui-confirm-create", + ), ] workflow_template_ui_patterns = [ diff --git a/tasks/views.py b/tasks/views.py index 0ae25f2bb..05974de17 100644 --- a/tasks/views.py +++ b/tasks/views.py @@ -313,6 +313,12 @@ def get_success_url(self): ) +class TaskWorkflowConfirmCreateView(PermissionRequiredMixin, DetailView): + model = TaskWorkflow + template_name = "tasks/workflows/confirm_create.jinja" + permission_required = "tasks.add_taskworkflow" + + class TaskWorkflowTemplateDetailView(PermissionRequiredMixin, DetailView): model = TaskWorkflowTemplate template_name = "tasks/workflows/template_detail.jinja" @@ -411,7 +417,7 @@ def get_success_url(self): class TaskWorkflowTemplateConfirmCreateView(PermissionRequiredMixin, DetailView): model = TaskWorkflowTemplate - template_name = "tasks/workflows/template_confirm_create.jinja" + template_name = "tasks/workflows/confirm_create.jinja" permission_required = "tasks.add_taskworkflowtemplate" From 517a829ccc869cfee7c881a2509d420ec3b57cbd Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Tue, 26 Nov 2024 16:15:31 +0000 Subject: [PATCH 07/17] Add workflow create view & form unit tests --- conftest.py | 4 ++ tasks/tests/test_forms.py | 83 +++++++++++++++++++++++++++++++++++++++ tasks/tests/test_views.py | 71 +++++++++++++++++++++++++++++++++ 3 files changed, 158 insertions(+) diff --git a/conftest.py b/conftest.py index 56049cabb..0afdd2286 100644 --- a/conftest.py +++ b/conftest.py @@ -310,6 +310,10 @@ def policy_group(db) -> Group: ("tasks", "view_comment"), ("tasks", "change_comment"), ("tasks", "delete_comment"), + ("tasks", "add_taskworkflow"), + ("tasks", "change_taskworkflow"), + ("tasks", "delete_taskworkflow"), + ("tasks", "view_taskworkflow"), ("tasks", "add_taskworkflowtemplate"), ("tasks", "change_taskworkflowtemplate"), ("tasks", "delete_taskworkflowtemplate"), diff --git a/tasks/tests/test_forms.py b/tasks/tests/test_forms.py index d04806c63..6832ed0c1 100644 --- a/tasks/tests/test_forms.py +++ b/tasks/tests/test_forms.py @@ -1,8 +1,12 @@ +import pytest + from common.tests.factories import ProgressStateFactory from common.tests.factories import TaskFactory from tasks import forms from tasks.models import ProgressState +pytestmark = pytest.mark.django_db + def test_create_subtask_assigns_correct_parent_task(valid_user): """Tests that SubtaskCreateForm assigns the correct parent on form.save.""" @@ -20,3 +24,82 @@ def test_create_subtask_assigns_correct_parent_task(valid_user): new_subtask = form.save(parent_task_instance, user=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): + """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 = forms.TaskWorkflowCreateForm(form_data) + assert form.is_valid() + assert form.cleaned_data + + for key in form_data.keys(): + assert form.cleaned_data[key] == form_data[key] + + +@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", + ), + ( + { + "create_type": forms.TaskWorkflowCreateForm.CreateType.WITH_TEMPLATE, + "workflow_template": "", + }, + "create_type", + "Select a workflow template", + ), + ( + { + "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.", + ), + ], + ids=( + "missing_title", + "missing_description", + "missing_create_type", + "missing_workflow_template", + "invalid_workflow_template", + ), +) +def test_workflow_create_form_invalid_data(form_data, field, error_message): + """Tests that `TaskWorkflowCreateForm` raises expected form errors given + invalid form data.""" + + form = forms.TaskWorkflowCreateForm(form_data) + assert not form.is_valid() + assert error_message in form.errors[field] diff --git a/tasks/tests/test_views.py b/tasks/tests/test_views.py index 5bf1131ea..67331663d 100644 --- a/tasks/tests/test_views.py +++ b/tasks/tests/test_views.py @@ -6,10 +6,12 @@ from common.tests.factories import ProgressStateFactory from common.tests.factories import SubTaskFactory from common.tests.factories import TaskFactory +from tasks.forms import TaskWorkflowCreateForm from tasks.models import ProgressState from tasks.models import TaskItemTemplate from tasks.models import TaskLog from tasks.models import TaskTemplate +from tasks.models import TaskWorkflow from tasks.models import TaskWorkflowTemplate from tasks.tests.factories import TaskItemTemplateFactory @@ -545,3 +547,72 @@ def test_delete_task_template_view( f"Task template ID: {task_template_pk}" in soup.select(".govuk-panel__title")[0].text ) + + +@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_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 + ) + + assert not TaskWorkflow.objects.exists() + + 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__description=form_data["description"], + ) + + 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 + + confirmation_url = reverse( + "workflow:task-workflow-ui-confirm-create", + kwargs={"pk": created_workflow.pk}, + ) + assert create_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 str(created_workflow) in soup.select("h1.govuk-panel__title")[0].text From d6e91aba5d0300766f0dc57556f3154b9472268f Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Tue, 26 Nov 2024 16:47:29 +0000 Subject: [PATCH 08/17] Simplify TaskWorkflowCreateView.form_valid() --- tasks/views.py | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/tasks/views.py b/tasks/views.py index 05974de17..e38d0d373 100644 --- a/tasks/views.py +++ b/tasks/views.py @@ -279,33 +279,17 @@ def form_valid(self, form): create_type = form.cleaned_data["create_type"] if create_type == TaskWorkflowCreateForm.CreateType.WITH_TEMPLATE: - self.object = self.create_with_template( - template=form.cleaned_data["workflow_template"], - summary_data=summary_data, - ) + template = form.cleaned_data["workflow_template"] + self.object = template.create_task_workflow(summary_data) elif create_type == TaskWorkflowCreateForm.CreateType.WITHOUT_TEMPLATE: - self.object = self.create_without_template(summary_data) + with transaction.atomic(): + summary_task = Task.objects.create(**summary_data) + self.object = TaskWorkflow.objects.create( + summary_task=summary_task, + ) return super().form_valid(form) - def create_with_template( - self, - template: TaskWorkflowTemplate, - summary_data: dict, - ) -> TaskWorkflow: - template = get_object_or_404( - TaskWorkflowTemplate, - pk=template.pk, - ) - return template.create_task_workflow(summary_data) - - @transaction.atomic - def create_without_template(self, summary_data: dict) -> TaskWorkflow: - summary_task = Task.objects.create(**summary_data) - return TaskWorkflow.objects.create( - summary_task=summary_task, - ) - def get_success_url(self): return reverse( "workflow:task-workflow-ui-confirm-create", From 4ebc3ddd9c985242acfc3eca612f74c2557f342c Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Tue, 26 Nov 2024 16:48:51 +0000 Subject: [PATCH 09/17] Change factory faker provider of CategoryFactory.name to avoid recurrent duplicate key errors --- common/tests/factories.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/tests/factories.py b/common/tests/factories.py index 34f0bdbf2..d1c487b1b 100644 --- a/common/tests/factories.py +++ b/common/tests/factories.py @@ -1536,7 +1536,7 @@ class Meta: class CategoryFactory(factory.django.DjangoModelFactory): - name = factory.Faker("word") + name = factory.Faker("bs") class Meta: model = "tasks.Category" From a6592279513305a8cfdb5835390948e34a5bd198 Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Tue, 26 Nov 2024 17:18:19 +0000 Subject: [PATCH 10/17] Simplify workflow_create_form_valid_data test assertion --- tasks/tests/test_forms.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tasks/tests/test_forms.py b/tasks/tests/test_forms.py index 6832ed0c1..1dfd706fa 100644 --- a/tasks/tests/test_forms.py +++ b/tasks/tests/test_forms.py @@ -55,10 +55,7 @@ def test_workflow_create_form_valid_data(form_data, task_workflow_template): form = forms.TaskWorkflowCreateForm(form_data) assert form.is_valid() - assert form.cleaned_data - - for key in form_data.keys(): - assert form.cleaned_data[key] == form_data[key] + assert form.cleaned_data == form_data @pytest.mark.parametrize( From 80112cdff534b5292b899808858be526ddd40e6e Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Wed, 27 Nov 2024 11:01:12 +0000 Subject: [PATCH 11/17] Add title and description property methods to Workflow instances --- tasks/models/workflow.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tasks/models/workflow.py b/tasks/models/workflow.py index 6b492eca9..52e7e8a9e 100644 --- a/tasks/models/workflow.py +++ b/tasks/models/workflow.py @@ -32,8 +32,16 @@ class Meta: verbose_name = "workflow" def __str__(self): + return self.title + + @property + def title(self) -> str: return self.summary_task.title + @property + def description(self) -> str: + return self.summary_task.description + def get_tasks(self) -> models.QuerySet: """Get a QuerySet of the Tasks associated through their TaskItem instances to this TaskWorkflow, ordered by the position of the From 88f479293f3bcbb1766b3dbf8941ba0ed5724477 Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Wed, 27 Nov 2024 11:04:08 +0000 Subject: [PATCH 12/17] Add workflow detail view --- .../{template_detail.jinja => detail.jinja} | 35 +++++--- tasks/urls.py | 5 ++ tasks/views.py | 81 ++++++++++++++++++- 3 files changed, 107 insertions(+), 14 deletions(-) rename tasks/jinja2/tasks/workflows/{template_detail.jinja => detail.jinja} (62%) diff --git a/tasks/jinja2/tasks/workflows/template_detail.jinja b/tasks/jinja2/tasks/workflows/detail.jinja similarity index 62% rename from tasks/jinja2/tasks/workflows/template_detail.jinja rename to tasks/jinja2/tasks/workflows/detail.jinja index 00e04093d..db9be5e80 100644 --- a/tasks/jinja2/tasks/workflows/template_detail.jinja +++ b/tasks/jinja2/tasks/workflows/detail.jinja @@ -2,14 +2,23 @@ {% from "components/breadcrumbs.jinja" import breadcrumbs %} {% from "components/button/macro.njk" import govukButton %} -{% set page_title = "Workflow template: " ~ object.title %} -{% set list_include = "tasks/includes/task_queue.jinja" %} +{% set object_name = object._meta.verbose_name %} + +{% set page_title = object_name|capitalize ~ ": " ~ object.title %} -{% set edit_url = object.get_url("edit") %} +{% if object_name == "workflow" %} + {% set task_object_name = "task" %} + {% set task_object_create_url = "#TODO" %} +{% 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" %} {% block breadcrumb %} {{ breadcrumbs(request, [ - {"text": "Find and view workflow templates", "href": "#TODO"}, + {"text": "Find and view "~ object_name ~ "s", "href": object.get_url("list")}, {"text": page_title} ], with_workbasket=False, @@ -40,7 +49,7 @@ {{ object.title }}
    - Change title + Change title
    @@ -52,20 +61,20 @@ {{ object.description }}
    - Change description + Change description
    -

    Task templates

    +

    {{ task_object_name|capitalize }}s

    {{ govukButton({ - "text": "Create task template", - "href": url("workflow:task-template-ui-create", kwargs={"workflow_template_pk": object.pk}), + "text": "Create a " ~ task_object_name, + "href": task_object_create_url, "classes": "govuk-button align-right" }) }}
    @@ -76,20 +85,20 @@ {% if object_list %} {% include list_include %} {% else %} -

    There are no task templates for this workflow template.

    +

    There are no {{ task_object_name }}s for this {{ object_name }}.

    {% endif %}
    {{ govukButton({ - "text": "Delete workflow template", + "text": "Delete " ~ object_name, "href": object.get_url("delete"), "classes": "govuk-button--warning" }) }} {{ govukButton({ - "text": "Find and view workflow templates", - "href": "#TODO", + "text": "Find and view " ~ object_name ~ "s", + "href": object.get_url("list"), "classes": "govuk-button--secondary" }) }}
    diff --git a/tasks/urls.py b/tasks/urls.py index 6aaf5aee0..4ad833c41 100644 --- a/tasks/urls.py +++ b/tasks/urls.py @@ -72,6 +72,11 @@ workflow_ui_patterns = [ + path( + "/", + views.TaskWorkflowDetailView.as_view(), + name="task-workflow-ui-detail", + ), path( "create/", views.TaskWorkflowCreateView.as_view(), diff --git a/tasks/views.py b/tasks/views.py index e38d0d373..3a55e16d0 100644 --- a/tasks/views.py +++ b/tasks/views.py @@ -27,6 +27,7 @@ from tasks.forms import TaskWorkflowTemplateDeleteForm from tasks.forms import TaskWorkflowTemplateUpdateForm from tasks.models import Task +from tasks.models import TaskItem from tasks.models import TaskItemTemplate from tasks.models import TaskTemplate from tasks.models import TaskWorkflow @@ -260,6 +261,84 @@ def get_context_data(self, **kwargs): return context_data +class TaskWorkflowDetailView(PermissionRequiredMixin, DetailView): + model = TaskWorkflow + template_name = "tasks/workflows/detail.jinja" + permission_required = "tasks.view_taskworkflow" + + @cached_property + def task_workflow(self) -> TaskWorkflow: + return self.get_object() + + @property + def view_url(self) -> str: + return reverse( + "workflow:task-workflow-ui-detail", + kwargs={"pk": self.task_workflow.pk}, + ) + + def get_context_data(self, **kwargs): + context_data = super().get_context_data(**kwargs) + context_data["object_list"] = self.task_workflow.get_tasks() + 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) + + def promote(self, task_id: int) -> None: + task_item = get_object_or_404( + TaskItem, + task_id=task_id, + queue=self.task_workflow, + ) + try: + task_item.promote() + except OperationalError: + pass + + def demote(self, task_id: int) -> None: + task_item = get_object_or_404( + TaskItem, + task_id=task_id, + queue=self.task_workflow, + ) + try: + task_item.demote() + except OperationalError: + pass + + def promote_to_first(self, task_id: int) -> None: + task_item = get_object_or_404( + TaskItem, + task_id=task_id, + queue=self.task_workflow, + ) + try: + task_item.promote_to_first() + except OperationalError: + pass + + def demote_to_last(self, task_id: int) -> None: + task_item = get_object_or_404( + TaskItem, + task_id=task_id, + queue=self.task_workflow, + ) + try: + task_item.demote_to_last() + except OperationalError: + pass + + class TaskWorkflowCreateView(PermissionRequiredMixin, FormView): permission_required = "tasks.add_taskworkflow" template_name = "tasks/workflows/create.jinja" @@ -305,7 +384,7 @@ class TaskWorkflowConfirmCreateView(PermissionRequiredMixin, DetailView): class TaskWorkflowTemplateDetailView(PermissionRequiredMixin, DetailView): model = TaskWorkflowTemplate - template_name = "tasks/workflows/template_detail.jinja" + template_name = "tasks/workflows/detail.jinja" permission_required = "tasks.view_taskworkflowtemplate" @cached_property From a612c113d188e6f2562a538f688c739b2fa4418b Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Wed, 27 Nov 2024 11:05:03 +0000 Subject: [PATCH 13/17] Add workflow detail view unit tests --- tasks/tests/conftest.py | 26 ++++++++++++++ tasks/tests/factories.py | 3 ++ tasks/tests/test_views.py | 74 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 100 insertions(+), 3 deletions(-) diff --git a/tasks/tests/conftest.py b/tasks/tests/conftest.py index 6c324f913..f3b877727 100644 --- a/tasks/tests/conftest.py +++ b/tasks/tests/conftest.py @@ -5,7 +5,9 @@ from common.tests.factories import SubTaskFactory from common.tests.factories import TaskAssigneeFactory from common.tests.factories import TaskFactory +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 from tasks.models import TaskWorkflow @@ -128,3 +130,27 @@ def task_workflow_single_task_item(task_workflow) -> TaskWorkflow: assert task_workflow.get_items().get() == task_item return task_workflow + + +@pytest.fixture() +def task_workflow_three_task_items( + task_workflow, +) -> TaskWorkflow: + """Return a TaskWorkflow instance containing three TaskItems and related + Tasks.""" + + expected_count = 3 + + task_items = factories.TaskItemFactory.create_batch( + expected_count, + queue=task_workflow, + ) + + assert task_workflow.get_items().count() == expected_count + assert TaskItem.objects.filter(queue=task_workflow).count() == expected_count + assert ( + Task.objects.filter(taskitem__in=[item.pk for item in task_items]).count() + == expected_count + ) + + return task_workflow diff --git a/tasks/tests/factories.py b/tasks/tests/factories.py index 030cde8f1..7c703c8b4 100644 --- a/tasks/tests/factories.py +++ b/tasks/tests/factories.py @@ -14,6 +14,9 @@ class TaskWorkflowTemplateFactory(DjangoModelFactory): """Factory to create TaskWorkflowTemplate instances.""" + title = factory.Faker("sentence") + description = factory.Faker("sentence") + class Meta: model = TaskWorkflowTemplate diff --git a/tasks/tests/test_views.py b/tasks/tests/test_views.py index 67331663d..25e302e87 100644 --- a/tasks/tests/test_views.py +++ b/tasks/tests/test_views.py @@ -226,7 +226,7 @@ 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=workflow_template.title) + assert page.find("h1", text=f"Workflow template: {workflow_template.title}") assert page.find("a", text=task_template.title) @@ -256,7 +256,7 @@ def convert_to_index(position: int) -> int: access.""" return position - 1 - items = list(task_workflow_template_three_task_template_items.get_items()) + items = list(task_workflow_template_three_task_template_items.get_task_templates()) item_to_move = items[convert_to_index(item_position)] url = reverse( @@ -270,7 +270,9 @@ def convert_to_index(position: int) -> int: response = valid_user_client.post(url, form_data) assert response.status_code == 302 - reordered_items = task_workflow_template_three_task_template_items.get_items() + reordered_items = ( + task_workflow_template_three_task_template_items.get_task_templates() + ) for i, reordered_item in enumerate(reordered_items): expected_position = convert_to_index(expected_item_order[i]) expected_item = items[expected_position] @@ -549,6 +551,72 @@ def test_delete_task_template_view( ) +def test_workflow_detail_view_displays_tasks( + valid_user_client, + task_workflow_single_task_item, +): + workflow = task_workflow_single_task_item + task = task_workflow_single_task_item.get_tasks().get() + + url = reverse( + "workflow:task-workflow-ui-detail", + kwargs={"pk": workflow.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=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 + + 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 + + @pytest.mark.parametrize( "form_data", [ From 2d8c5972b7a0239225149df28562e3952e2f56f6 Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Wed, 27 Nov 2024 11:22:32 +0000 Subject: [PATCH 14/17] Add detail get_url action to Workflow model --- tasks/jinja2/tasks/includes/task_queue.jinja | 2 +- tasks/models/workflow.py | 20 ++++++++++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/tasks/jinja2/tasks/includes/task_queue.jinja b/tasks/jinja2/tasks/includes/task_queue.jinja index 904a3fdfc..1f55b66d3 100644 --- a/tasks/jinja2/tasks/includes/task_queue.jinja +++ b/tasks/jinja2/tasks/includes/task_queue.jinja @@ -47,7 +47,7 @@ {% endset -%} {%- set remove_cell_content %} - TODO + Remove {% endset -%} {{ table_rows.append([ diff --git a/tasks/models/workflow.py b/tasks/models/workflow.py index 52e7e8a9e..59b3b766b 100644 --- a/tasks/models/workflow.py +++ b/tasks/models/workflow.py @@ -49,11 +49,11 @@ def get_tasks(self) -> models.QuerySet: return Task.objects.filter(taskitem__queue=self).order_by("taskitem__position") def get_url(self, action: str = "detail"): - # if action == "detail": - # return reverse( - # "workflow:task-workflow-ui-detail", - # kwargs={"pk": self.pk}, - # ) + if action == "detail": + return reverse( + "workflow:task-workflow-ui-detail", + kwargs={"pk": self.pk}, + ) # elif action == "edit": # return reverse( # "workflow:task-workflow-ui-update", @@ -64,7 +64,7 @@ def get_url(self, action: str = "detail"): # "workflow:task-workflow-ui-delete", # kwargs={"pk": self.pk}, # ) - if action == "create": + elif action == "create": return reverse( "workflow:task-workflow-ui-create", ) @@ -218,6 +218,14 @@ def get_url(self, action: str = "detail"): return reverse("workflow:task-template-ui-detail", kwargs={"pk": self.pk}) elif action == "edit": return reverse("workflow:task-template-ui-update", kwargs={"pk": self.pk}) + elif action == "delete": + return reverse( + "workflow:task-template-ui-delete", + kwargs={ + "workflow_template_pk": self.taskitemtemplate.queue.pk, + "pk": self.pk, + }, + ) return "#NOT-IMPLEMENTED" From def12bcaa23a0a391ad37dcaf52febe233b2b4a8 Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Wed, 27 Nov 2024 12:30:05 +0000 Subject: [PATCH 15/17] Add workflow delete view --- tasks/forms.py | 4 ++ .../tasks/workflows/confirm_delete.jinja | 49 +++++++++++++++++++ .../{template_delete.jinja => delete.jinja} | 9 ++-- .../workflows/template_confirm_delete.jinja | 42 ---------------- tasks/models/workflow.py | 10 ++-- tasks/urls.py | 10 ++++ tasks/views.py | 44 ++++++++++++++++- 7 files changed, 115 insertions(+), 53 deletions(-) create mode 100644 tasks/jinja2/tasks/workflows/confirm_delete.jinja rename tasks/jinja2/tasks/workflows/{template_delete.jinja => delete.jinja} (63%) delete mode 100644 tasks/jinja2/tasks/workflows/template_confirm_delete.jinja diff --git a/tasks/forms.py b/tasks/forms.py index 39bcf2ea6..eb2a53fbe 100644 --- a/tasks/forms.py +++ b/tasks/forms.py @@ -16,6 +16,7 @@ from common.validators import SymbolValidator from tasks.models import Task from tasks.models import TaskTemplate +from tasks.models import TaskWorkflow from tasks.models import TaskWorkflowTemplate from workbaskets.models import WorkBasket @@ -158,6 +159,9 @@ def __init__(self, *args, **kwargs): ) +TaskWorkflowDeleteForm = delete_form_for(TaskWorkflow) + + class TaskWorkflowTemplateBaseForm(ModelForm): class Meta: model = TaskWorkflowTemplate diff --git a/tasks/jinja2/tasks/workflows/confirm_delete.jinja b/tasks/jinja2/tasks/workflows/confirm_delete.jinja new file mode 100644 index 000000000..ef25fbade --- /dev/null +++ b/tasks/jinja2/tasks/workflows/confirm_delete.jinja @@ -0,0 +1,49 @@ +{% 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 = verbose_name|capitalize ~ " deleted" %} + +{% if verbose_name == "workflow" %} + {% set list_url = "#TODO" %} + {% set create_url = url("workflow:task-workflow-ui-create") %} +{% elif verbose_name == "workflow template" %} + {% set list_url = "#TODO" %} + {% set create_url = url("workflow:task-workflow-template-ui-create") %} +{% endif %} + +{% block breadcrumb %} + {{ breadcrumbs( + request, + [ + {"text": "Find and view "~ verbose_name ~ "s", "href": list_url}, + {"text": page_title} + ], + False, + ) }} +{% endblock %} + +{% block panel %} + {{ govukPanel({ + "titleText": verbose_name|capitalize ~ " ID: " ~ deleted_pk, + "text": verbose_name|capitalize ~ " has been deleted", + "classes": "govuk-!-margin-bottom-7" + }) }} +{% endblock %} + +{% block button_group %} + {{ govukButton({ + "text": "Find and view " ~ verbose_name ~ "s", + "href": list_url, + "classes": "govuk-button" + }) }} + {{ govukButton({ + "text": "Create a " ~ verbose_name, + "href": create_url, + "classes": "govuk-button--secondary" + }) }} +{% endblock %} + +{% block actions %}{% endblock %} diff --git a/tasks/jinja2/tasks/workflows/template_delete.jinja b/tasks/jinja2/tasks/workflows/delete.jinja similarity index 63% rename from tasks/jinja2/tasks/workflows/template_delete.jinja rename to tasks/jinja2/tasks/workflows/delete.jinja index 5632034af..aff449a61 100644 --- a/tasks/jinja2/tasks/workflows/template_delete.jinja +++ b/tasks/jinja2/tasks/workflows/delete.jinja @@ -4,15 +4,16 @@ {% from "components/warning-text/macro.njk" import govukWarningText %} {% from "components/button/macro.njk" import govukButton %} -{% set page_title = "Delete workflow template: " ~ object.title %} +{% set object_name = object._meta.verbose_name %} +{% set page_title = "Delete " ~ object_name ~ ": " ~ object.title %} {% block breadcrumb %} {{ breadcrumbs( request, [ - {"text": "Find and view workflow templates", "href": "#TODO"}, + {"text": "Find and view "~ object_name ~ "s", "href": object.get_url("list")}, { - "text": "Workflow template: " ~ object.title, + "text": object_name|capitalize ~ ": " ~ object.title, "href": object.get_url("detail"), }, {"text": page_title} @@ -23,7 +24,7 @@ {% block form %} {{ govukWarningText({ - "text": "Are you sure you want to delete this workflow template?" + "text": "Are you sure you want to delete this " ~ object_name ~ "?", }) }} {% call django_form(action=object.get_url("delete")) %} diff --git a/tasks/jinja2/tasks/workflows/template_confirm_delete.jinja b/tasks/jinja2/tasks/workflows/template_confirm_delete.jinja deleted file mode 100644 index c46e0628c..000000000 --- a/tasks/jinja2/tasks/workflows/template_confirm_delete.jinja +++ /dev/null @@ -1,42 +0,0 @@ -{% 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 = "Workflow template deleted" %} - -{% block breadcrumb %} - {{ breadcrumbs( - request, - [ - {"text": "Find and view workflow templates", "href": "#TODO"}, - {"text": page_title} - ], - False, - ) }} -{% endblock %} - -{% block panel %} - {{ govukPanel({ - "titleText": "Workflow template ID: " ~ deleted_pk, - "text": "Workflow template has been deleted", - "classes": "govuk-!-margin-bottom-7" - }) }} -{% endblock %} - -{% block button_group %} - {{ govukButton({ - "text": "Find and view workflow templates", - "href": "#TODO", - "classes": "govuk-button" - }) }} - {{ govukButton({ - "text": "Create a workflow template", - "href": url("workflow:task-workflow-template-ui-create"), - "classes": "govuk-button--secondary" - }) }} -{% endblock %} - -{% block actions %} -{% endblock %} diff --git a/tasks/models/workflow.py b/tasks/models/workflow.py index 59b3b766b..c48e4c05e 100644 --- a/tasks/models/workflow.py +++ b/tasks/models/workflow.py @@ -59,11 +59,11 @@ def get_url(self, action: str = "detail"): # "workflow:task-workflow-ui-update", # kwargs={"pk": self.pk}, # ) - # elif action == "delete": - # return reverse( - # "workflow:task-workflow-ui-delete", - # kwargs={"pk": self.pk}, - # ) + elif action == "delete": + return reverse( + "workflow:task-workflow-ui-delete", + kwargs={"pk": self.pk}, + ) elif action == "create": return reverse( "workflow:task-workflow-ui-create", diff --git a/tasks/urls.py b/tasks/urls.py index 4ad833c41..cfcb80ecc 100644 --- a/tasks/urls.py +++ b/tasks/urls.py @@ -87,6 +87,16 @@ views.TaskWorkflowConfirmCreateView.as_view(), name="task-workflow-ui-confirm-create", ), + path( + "/delete/", + views.TaskWorkflowDeleteView.as_view(), + name="task-workflow-ui-delete", + ), + path( + "/confirm-delete/", + views.TaskWorkflowConfirmDeleteView.as_view(), + name="task-workflow-ui-confirm-delete", + ), ] workflow_template_ui_patterns = [ diff --git a/tasks/views.py b/tasks/views.py index 3a55e16d0..2f645807e 100644 --- a/tasks/views.py +++ b/tasks/views.py @@ -23,6 +23,7 @@ from tasks.forms import TaskTemplateUpdateForm from tasks.forms import TaskUpdateForm from tasks.forms import TaskWorkflowCreateForm +from tasks.forms import TaskWorkflowDeleteForm from tasks.forms import TaskWorkflowTemplateCreateForm from tasks.forms import TaskWorkflowTemplateDeleteForm from tasks.forms import TaskWorkflowTemplateUpdateForm @@ -382,6 +383,44 @@ class TaskWorkflowConfirmCreateView(PermissionRequiredMixin, DetailView): permission_required = "tasks.add_taskworkflow" +class TaskWorkflowDeleteView(PermissionRequiredMixin, DeleteView): + model = TaskWorkflow + template_name = "tasks/workflows/delete.jinja" + permission_required = "tasks.delete_taskworkflow" + form_class = TaskWorkflowDeleteForm + + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + kwargs["instance"] = self.object + return kwargs + + @transaction.atomic + def form_valid(self, form): + summary_task = self.object.summary_task + self.object.get_tasks().delete() + result = super().form_valid(form) + summary_task.delete() + return result + + def get_success_url(self): + return reverse( + "workflow:task-workflow-ui-confirm-delete", + kwargs={"pk": self.object.pk}, + ) + + +class TaskWorkflowConfirmDeleteView(PermissionRequiredMixin, TemplateView): + model = TaskWorkflow + template_name = "tasks/workflows/confirm_delete.jinja" + permission_required = "tasks.change_taskworkflow" + + def get_context_data(self, **kwargs): + context_data = super().get_context_data(**kwargs) + context_data["verbose_name"] = "workflow" + context_data["deleted_pk"] = self.kwargs["pk"] + return context_data + + class TaskWorkflowTemplateDetailView(PermissionRequiredMixin, DetailView): model = TaskWorkflowTemplate template_name = "tasks/workflows/detail.jinja" @@ -505,7 +544,7 @@ class TaskWorkflowTemplateConfirmUpdateView(PermissionRequiredMixin, DetailView) class TaskWorkflowTemplateDeleteView(PermissionRequiredMixin, DeleteView): model = TaskWorkflowTemplate - template_name = "tasks/workflows/template_delete.jinja" + template_name = "tasks/workflows/delete.jinja" permission_required = "tasks.delete_taskworkflowtemplate" form_class = TaskWorkflowTemplateDeleteForm @@ -528,11 +567,12 @@ def get_success_url(self): class TaskWorkflowTemplateConfirmDeleteView(PermissionRequiredMixin, TemplateView): model = TaskWorkflowTemplate - template_name = "tasks/workflows/template_confirm_delete.jinja" + template_name = "tasks/workflows/confirm_delete.jinja" permission_required = "tasks.change_taskworkflowtemplate" def get_context_data(self, **kwargs): context_data = super().get_context_data(**kwargs) + context_data["verbose_name"] = "workflow template" context_data["deleted_pk"] = self.kwargs["pk"] return context_data From a0c361fb2b9504ba6f0cd717b66696b3c566b5c4 Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Wed, 27 Nov 2024 12:30:36 +0000 Subject: [PATCH 16/17] Add workflow delete view unit test --- tasks/tests/test_views.py | 41 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/tasks/tests/test_views.py b/tasks/tests/test_views.py index 25e302e87..ad71e52d4 100644 --- a/tasks/tests/test_views.py +++ b/tasks/tests/test_views.py @@ -8,6 +8,8 @@ from common.tests.factories import TaskFactory from tasks.forms import TaskWorkflowCreateForm from tasks.models import ProgressState +from tasks.models import Task +from tasks.models import TaskItem from tasks.models import TaskItemTemplate from tasks.models import TaskLog from tasks.models import TaskTemplate @@ -354,7 +356,7 @@ def test_workflow_template_delete_view( task_workflow_template_single_task_template_item, ): """Tests that a workflow template can be deleted (along with related - TaskItemPosition and TaskTemplate objects) and that the corresponding + TaskItemTemplate and TaskTemplate objects) and that the corresponding confirmation view returns a HTTP 200 response.""" task_workflow_template_pk = task_workflow_template_single_task_template_item.pk @@ -684,3 +686,40 @@ def test_workflow_create_view( soup = BeautifulSoup(str(confirmation_response.content), "html.parser") assert str(created_workflow) in soup.select("h1.govuk-panel__title")[0].text + + +def test_workflow_delete_view( + valid_user_client, + task_workflow_single_task_item, +): + """Tests that a workflow can be deleted (along with related TaskItem and + Task objects) and that the corresponding confirmation view returns a HTTP + 200 response.""" + + workflow_pk = task_workflow_single_task_item.pk + summary_task_pk = task_workflow_single_task_item.summary_task.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=workflow_pk, + ).exists() + assert not TaskItem.objects.filter( + queue_id=workflow_pk, + ).exists() + assert not Task.objects.filter(pk__in=[summary_task_pk, task_pk]).exists() + + confirmation_url = reverse( + "workflow:task-workflow-ui-confirm-delete", + kwargs={"pk": 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: {workflow_pk}" in soup.select(".govuk-panel__title")[0].text From f0662d5be29522f30430054998708c9c13bf2f9e Mon Sep 17 00:00:00 2001 From: Dale Cannon Date: Thu, 28 Nov 2024 13:00:19 +0000 Subject: [PATCH 17/17] Make explicit create_task_workflow() params --- tasks/models/workflow.py | 24 +++++++++++++++--------- tasks/tests/test_views.py | 2 ++ tasks/tests/test_workflow_models.py | 18 +++++++++++------- tasks/views.py | 2 +- 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/tasks/models/workflow.py b/tasks/models/workflow.py index 6b492eca9..9a4be6de2 100644 --- a/tasks/models/workflow.py +++ b/tasks/models/workflow.py @@ -2,6 +2,7 @@ from django.db.transaction import atomic from django.urls import reverse +from common.models import User from tasks.models.queue import Queue from tasks.models.queue import QueueItem from tasks.models.task import Task @@ -127,15 +128,20 @@ def get_task_templates(self) -> models.QuerySet: ) @atomic - def create_task_workflow(self, summary_data: dict) -> "TaskWorkflow": - """ - Create a workflow and it subtasks, using values from this template - workflow and its task templates. - - The `summary_data` param is used to populate the details of the `TaskWorkflow.summary_task` instance. - """ - - summary_task = Task.objects.create(**summary_data) + def create_task_workflow( + self, + title: str, + description: str, + creator: User, + ) -> "TaskWorkflow": + """Create a workflow and it subtasks, using values from this template + workflow and its task templates.""" + + summary_task = Task.objects.create( + title=title, + description=description, + creator=creator, + ) task_workflow = TaskWorkflow.objects.create( summary_task=summary_task, creator_template=self, diff --git a/tasks/tests/test_views.py b/tasks/tests/test_views.py index 67331663d..412d5ab35 100644 --- a/tasks/tests/test_views.py +++ b/tasks/tests/test_views.py @@ -570,6 +570,7 @@ def test_delete_task_template_view( ) def test_workflow_create_view( form_data, + valid_user, valid_user_client, task_workflow_template_single_task_template_item, ): @@ -595,6 +596,7 @@ def test_workflow_create_view( created_workflow = TaskWorkflow.objects.get( summary_task__title=form_data["title"], summary_task__description=form_data["description"], + summary_task__creator=valid_user, ) if with_template: diff --git a/tasks/tests/test_workflow_models.py b/tasks/tests/test_workflow_models.py index d80d646cc..26d81cb93 100644 --- a/tasks/tests/test_workflow_models.py +++ b/tasks/tests/test_workflow_models.py @@ -9,17 +9,20 @@ def test_create_task_workflow_from_task_workflow_template( + valid_user, task_workflow_template_three_task_template_items, ): """Test creation of TaskWorkflow instances from TaskWorkflowTemplates using its `create_task_workflow()` method.""" - summary_data = { - "title": "Workflow title", - "description": "Workflow description", - } + + title = "Workflow title" + description = "Workflow description" + creator = valid_user task_workflow = ( task_workflow_template_three_task_template_items.create_task_workflow( - summary_data, + title=title, + description=description, + creator=creator, ) ) @@ -28,8 +31,9 @@ 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 == summary_data["title"] - assert task_workflow.summary_task.description == summary_data["description"] + assert task_workflow.summary_task.title == title + assert task_workflow.summary_task.description == description + assert task_workflow.summary_task.creator == creator assert task_workflow.get_items().count() == 3 # Validate that item positions are equivalent. diff --git a/tasks/views.py b/tasks/views.py index e38d0d373..3f7e4cf17 100644 --- a/tasks/views.py +++ b/tasks/views.py @@ -280,7 +280,7 @@ def form_valid(self, form): if create_type == TaskWorkflowCreateForm.CreateType.WITH_TEMPLATE: template = form.cleaned_data["workflow_template"] - self.object = template.create_task_workflow(summary_data) + 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)