-
Notifications
You must be signed in to change notification settings - Fork 4
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
base: TP2000-1471--task-workflow
Are you sure you want to change the base?
Changes from all commits
97f223a
d2feb1b
0bf3eff
cae9f2c
cc2ff3a
d0cef66
b850751
8546b9b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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() | ||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Placement of |
||
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 | ||
) |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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>", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
|
||||||
views.TaskWorkflowTaskConfirmCreateView.as_view(), | ||||||
name="task-workflow-task-ui-confirm-create", | ||||||
), | ||||||
] | ||||||
|
||||||
task_and_workflow_ui_patterns = [ | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -553,6 +553,51 @@ def get_context_data(self, **kwargs): | |
return context_data | ||
|
||
|
||
class TaskWorkflowTaskCreateView(PermissionRequiredMixin, CreateView): | ||
model = Task | ||
template_name = "layouts/create.jinja" | ||
permission_required = "tasks.add_task" | ||
form_class = TaskCreateForm | ||
|
||
def get_task_workflow(self): | ||
"""Get the associated TaskWorkflow via its pk in the URL.""" | ||
return TaskWorkflow.objects.get( | ||
pk=self.kwargs["task_workflow_pk"], | ||
) | ||
|
||
def get_context_data(self, **kwargs) -> dict: | ||
context = super().get_context_data(**kwargs) | ||
|
||
context["page_title"] = "Create a task" | ||
return context | ||
|
||
def form_valid(self, form) -> HttpResponseRedirect: | ||
with transaction.atomic(): | ||
self.object = form.save(user=self.request.user) | ||
TaskItem.objects.create( | ||
workflow=self.get_task_workflow(), | ||
task=self.object, | ||
) | ||
return HttpResponseRedirect(self.get_success_url(), self.object.pk) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the second parameter to the |
||
|
||
def get_success_url(self): | ||
return reverse( | ||
"workflow:task-workflow-task-ui-confirm-create", | ||
kwargs={"pk": self.object.pk}, | ||
) | ||
|
||
|
||
class TaskWorkflowTaskConfirmCreateView(PermissionRequiredMixin, DetailView): | ||
model = Task | ||
template_name = "tasks/confirm_create.jinja" | ||
permission_required = "tasks.add_task" | ||
|
||
def get_context_data(self, **kwargs) -> dict: | ||
context = super().get_context_data(**kwargs) | ||
context["verbose_name"] = "task" | ||
return context | ||
|
||
|
||
class TaskWorkflowTemplateDetailView( | ||
PermissionRequiredMixin, | ||
QueuedItemManagementMixin, | ||
|
There was a problem hiding this comment.
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, thereverse()
failing in a less straightforward way.