Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tp2000 1616 workflow task create view #1363

Open
wants to merge 8 commits into
base: TP2000-1471--task-workflow
Choose a base branch
from
7 changes: 7 additions & 0 deletions tasks/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
class TaskBaseForm(ModelForm):
class Meta:
model = Task
include = [
marya-shariq marked this conversation as resolved.
Show resolved Hide resolved
"title",
"description",
"progress_state",
"category",
"workbasket",
]
exclude = ["parent_task", "creator"]

error_messages = {
Expand Down
2 changes: 1 addition & 1 deletion tasks/jinja2/tasks/workflows/detail.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

{% if object_name == "workflow" %}
{% set task_object_name = "task" %}
{% set task_object_create_url = "#TODO" %}
{% set task_object_create_url = url("workflow:task-workflow-task-ui-create", kwargs={"task_workflow_pk": object.pk}) %}
{% elif object_name == "workflow template" %}
{% set task_object_name = "task template" %}
{% set task_object_create_url = url("workflow:task-template-ui-create", kwargs={"workflow_template_pk": object.pk}) %}
Expand Down
75 changes: 75 additions & 0 deletions tasks/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -815,3 +815,78 @@ def test_task_and_workflow_list_view(valid_user_client, task, task_workflow):
assert table.select("tr:nth-child(2) > td:nth-child(1) > a:nth-child(1)")[
0
].text == str(task_workflow.summary_task.pk)


def test_create_workflow_task_view(valid_user_client, task_workflow):
"""Test the view for creating new Tasks for an existing workflow and the
confirmation view that a successful creation redirects to."""

assert task_workflow.get_tasks().count() == 0

progress_state = ProgressStateFactory.create()

create_url = reverse(
"workflow:task-workflow-task-ui-create",
kwargs={"task_workflow_pk": task_workflow.pk},
)

form_data = {
"title": factory.Faker("sentence"),
"description": factory.Faker("sentence"),
"progress_state": progress_state.pk,
}
create_response = valid_user_client.post(create_url, form_data)
created_workflow_task = task_workflow.get_tasks().get()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an assert that checks for a single task count further down, so if this blows up because there isn't exactly one task instance, then that assert will never be applied. The assertion could be performed against task count before getting the (single) task.

Checking the response's status code straight after the call to post() would also usefully catch failures in a more descriptive way, rather than, say, the reverse() failing in a less straightforward way.

confirmation_url = reverse(
"workflow:task-workflow-task-ui-confirm-create",
kwargs={"pk": created_workflow_task.pk},
)

assert create_response.status_code == 302
assert task_workflow.get_tasks().count() == 1
assert create_response.url == confirmation_url

confirmation_response = valid_user_client.get(confirmation_url)

soup = BeautifulSoup(str(confirmation_response.content), "html.parser")

assert confirmation_response.status_code == 200
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to previous comment, before attempting to access the response's content, the assert should first perform its validation checking.

Placement of asserts in the other test, test_workflow_delete_view_deletes_related_tasks, is better because it places them close to the side-effect or invariance they're validating.

assert created_workflow_task.title in soup.select("h1.govuk-panel__title")[0].text


def test_workflow_delete_view_deletes_related_tasks(
valid_user_client,
task_workflow_single_task_item,
):
"""Tests that a workflow can be deleted (along with related Task and
TaskItem objects) and that the corresponding confirmation view returns a
HTTP 200 response."""

task_workflow_pk = task_workflow_single_task_item.pk
task_pk = task_workflow_single_task_item.get_tasks().get().pk

delete_url = task_workflow_single_task_item.get_url("delete")
delete_response = valid_user_client.post(delete_url)
assert delete_response.status_code == 302

assert not TaskWorkflow.objects.filter(
pk=task_workflow_pk,
).exists()
assert not TaskItem.objects.filter(
workflow_id=task_workflow_pk,
).exists()
assert not Task.objects.filter(pk=task_pk).exists()

confirmation_url = reverse(
"workflow:task-workflow-ui-confirm-delete",
kwargs={"pk": task_workflow_pk},
)
assert delete_response.url == confirmation_url

confirmation_response = valid_user_client.get(confirmation_url)
assert confirmation_response.status_code == 200

soup = BeautifulSoup(str(confirmation_response.content), "html.parser")
assert (
f"Workflow ID: {task_workflow_pk}" in soup.select(".govuk-panel__title")[0].text
)
10 changes: 10 additions & 0 deletions tasks/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,16 @@
views.TaskWorkflowConfirmDeleteView.as_view(),
name="task-workflow-ui-confirm-delete",
),
path(
"<int:task_workflow_pk>/task/create/",
views.TaskWorkflowTaskCreateView.as_view(),
name="task-workflow-task-ui-create",
),
path(
"task/confirm-create/<int:pk>",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For URL pattern consistency with other confirm-create views (for instance, subtask creation uses "subtasks/<int:pk>/confirm-create/",):

Suggested change
"task/confirm-create/<int:pk>",
"task/<int:pk>/confirm-create/",

views.TaskWorkflowTaskConfirmCreateView.as_view(),
name="task-workflow-task-ui-confirm-create",
),
]

task_and_workflow_ui_patterns = [
Expand Down
47 changes: 47 additions & 0 deletions tasks/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,53 @@ def get_context_data(self, **kwargs):
return context_data


class TaskWorkflowTaskCreateView(PermissionRequiredMixin, CreateView):
model = Task
template_name = "layouts/create.jinja"
permission_required = "tasks.add_task"
form_class = TaskCreateForm

def get_task_workflow(self):
"""Get the associated TaskWorkflow via its pk in the URL."""
return TaskWorkflow.objects.get(
pk=self.kwargs["task_workflow_pk"],
)

def get_context_data(self, **kwargs) -> dict:
context = super().get_context_data(**kwargs)

context["page_title"] = "Create a task"
# context["task_workflow"] = self.get_task_workflow()
marya-shariq marked this conversation as resolved.
Show resolved Hide resolved
return context

def form_valid(self, form) -> HttpResponseRedirect:
with transaction.atomic():
self.object = form.save(user=self.request.user)
TaskItem.objects.create(
workflow=self.get_task_workflow(),
task=self.object,
)
return HttpResponseRedirect(self.get_success_url(), self.object.pk)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the second parameter to the HttpResponseRedirect constructor correct or needed?


def get_success_url(self):
return reverse(
"workflow:task-workflow-task-ui-confirm-create",
kwargs={"pk": self.object.pk},
)


class TaskWorkflowTaskConfirmCreateView(PermissionRequiredMixin, DetailView):
model = Task
template_name = "tasks/confirm_create.jinja"
permission_required = "tasks.add_task"

def get_context_data(self, **kwargs) -> dict:
context = super().get_context_data(**kwargs)
context["verbose_name"] = "task"
# context["task_workflow"] = self.get_object().taskitem.workflow
marya-shariq marked this conversation as resolved.
Show resolved Hide resolved
return context


class TaskWorkflowTemplateDetailView(
PermissionRequiredMixin,
QueuedItemManagementMixin,
Expand Down