From e3a095efc0956f0ae381ef512e0350cea6ce8b51 Mon Sep 17 00:00:00 2001 From: Dan Braghis Date: Thu, 11 Jun 2020 15:58:04 +0100 Subject: [PATCH] Handle "needs changes" workflow state in workflow status --- .../scss/layouts/page-editor.scss | 4 +- .../templates/wagtailadmin/pages/create.html | 7 +- .../wagtailadmin/shared/workflow_status.html | 23 ++++-- .../workflows/workflow_status.html | 6 +- wagtail/admin/tests/test_workflows.py | 74 ++++++++++++++++--- wagtail/admin/views/pages.py | 4 +- wagtail/core/models.py | 4 + wagtail/core/tests/test_workflow.py | 2 + 8 files changed, 95 insertions(+), 29 deletions(-) diff --git a/wagtail/admin/static_src/wagtailadmin/scss/layouts/page-editor.scss b/wagtail/admin/static_src/wagtailadmin/scss/layouts/page-editor.scss index abb1a5b115..3394589553 100644 --- a/wagtail/admin/static_src/wagtailadmin/scss/layouts/page-editor.scss +++ b/wagtail/admin/static_src/wagtailadmin/scss/layouts/page-editor.scss @@ -590,11 +590,11 @@ footer .preview { fill: $color-teal; } - .needs_changes .icon { + .needs_changes .icon, + .rejected .icon { fill: $color-orange; } - .rejected .icon, .cancelled .icon { fill: $color-red-dark; } diff --git a/wagtail/admin/templates/wagtailadmin/pages/create.html b/wagtail/admin/templates/wagtailadmin/pages/create.html index 21044e603f..9fbece80ca 100644 --- a/wagtail/admin/templates/wagtailadmin/pages/create.html +++ b/wagtail/admin/templates/wagtailadmin/pages/create.html @@ -10,12 +10,9 @@
{% explorer_breadcrumb parent_page include_self=1 %} -
+
-

- {% icon "doc-empty-inverse" class_name="header-title-icon" %} - {% trans 'New' %} {{ content_type.model_class.get_verbose_name }} -

+

{% trans 'New' %} {{ content_type.model_class.get_verbose_name }}

diff --git a/wagtail/admin/templates/wagtailadmin/shared/workflow_status.html b/wagtail/admin/templates/wagtailadmin/shared/workflow_status.html index 08331dde1a..d4719cbab8 100644 --- a/wagtail/admin/templates/wagtailadmin/shared/workflow_status.html +++ b/wagtail/admin/templates/wagtailadmin/shared/workflow_status.html @@ -1,14 +1,21 @@ {% load i18n wagtailadmin_tags %} -{% if workflow_state and page.current_workflow_task %} - {% else %} {% if page.get_latest_revision %} diff --git a/wagtail/admin/templates/wagtailadmin/workflows/workflow_status.html b/wagtail/admin/templates/wagtailadmin/workflows/workflow_status.html index d20654aaa9..2f3c4406e4 100644 --- a/wagtail/admin/templates/wagtailadmin/workflows/workflow_status.html +++ b/wagtail/admin/templates/wagtailadmin/workflows/workflow_status.html @@ -24,7 +24,11 @@ diff --git a/wagtail/admin/tests/test_workflows.py b/wagtail/admin/tests/test_workflows.py index 3df6778953..c8fc38d107 100644 --- a/wagtail/admin/tests/test_workflows.py +++ b/wagtail/admin/tests/test_workflows.py @@ -251,7 +251,6 @@ class TestWorkflowsEditView(TestCase, WagtailTestUtils): moderators.user_set.add(self.moderator) moderators.permissions.add(Permission.objects.get(codename="change_workflow")) - def get(self, params={}): return self.client.get(reverse('wagtailadmin_workflows:edit', args=[self.workflow.id]), params) @@ -571,7 +570,6 @@ class TestCreateTaskView(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) - class TestSelectTaskTypeView(TestCase, WagtailTestUtils): def setUp(self): @@ -749,7 +747,7 @@ class TestSubmitToWorkflow(TestCase, WagtailTestUtils): response = self.client.get(edit_url) workflow_status_url = reverse('wagtailadmin_pages:workflow_status', args=(self.page.id, )) self.assertContains(response, workflow_status_url) - self.assertContains(response, 'Awaiting\n \n {}'.format(self.page.current_workflow_task.name)) + self.assertRegex(response.content.decode('utf-8'), r'Awaiting[\s|\n]+{}'.format(self.page.current_workflow_task.name)) self.assertNotContains(response, 'Draft') @mock.patch.object(EmailMultiAlternatives, 'send', side_effect=IOError('Server down')) @@ -904,8 +902,6 @@ class TestApproveRejectWorkflow(TestCase, WagtailTestUtils): } return self.client.post(reverse('wagtailadmin_pages:edit', args=(self.page.id,)), post_data) - - @override_settings(WAGTAIL_FINISH_WORKFLOW_ACTION='') def test_approve_task_and_workflow(self): """ @@ -1080,7 +1076,6 @@ class TestApproveRejectWorkflow(TestCase, WagtailTestUtils): self.assertNotContains(response, "Hello world!") - class TestNotificationPreferences(TestCase, WagtailTestUtils): def setUp(self): delete_existing_workflows() @@ -1601,7 +1596,8 @@ class TestWorkflowUsageView(TestCase, WagtailTestUtils): self.assertNotIn(self.child_page_with_another_workflow.id, object_set) -class TestWorkflowStatusModal(TestCase, WagtailTestUtils): +@freeze_time("2020-06-01 12:00:00") +class TestWorkflowStatus(TestCase, WagtailTestUtils): def setUp(self): delete_existing_workflows() self.submitter = get_user_model().objects.create_user( @@ -1625,7 +1621,7 @@ class TestWorkflowStatusModal(TestCase, WagtailTestUtils): password='password', ) - self.login(user=self.submitter) + self.login(self.superuser) # Create a page root_page = Page.objects.get(id=2) @@ -1642,6 +1638,8 @@ class TestWorkflowStatusModal(TestCase, WagtailTestUtils): WorkflowPage.objects.create(workflow=self.workflow, page=self.page) + self.edit_url = reverse('wagtailadmin_pages:edit', args=(self.page.id, )) + def create_workflow_and_tasks(self): workflow = Workflow.objects.create(name='test_workflow') task_1 = GroupApprovalTask.objects.create(name='test_task_1') @@ -1653,14 +1651,25 @@ class TestWorkflowStatusModal(TestCase, WagtailTestUtils): WorkflowTask.objects.create(workflow=workflow, task=task_2, sort_order=2) return workflow, task_1, task_2 - def submit(self): + def submit(self, action='action-submit'): post_data = { 'title': str(self.page.title), 'slug': str(self.page.slug), 'content': str(self.page.content), - 'action-submit': "True", + action: "True", } - return self.client.post(reverse('wagtailadmin_pages:edit', args=(self.page.id,)), post_data) + return self.client.post(self.edit_url, post_data) + + def workflow_action(self, action): + post_data = { + 'action': action, + 'next': self.edit_url + } + return self.client.post( + reverse('wagtailadmin_pages:workflow_action', args=(self.page.id, action, self.page.current_workflow_task_state.id)), + post_data, + follow=True + ) def test_workflow_status_modal(self): workflow_status_url = reverse('wagtailadmin_pages:workflow_status', args=(self.page.id, )) @@ -1682,3 +1691,46 @@ class TestWorkflowStatusModal(TestCase, WagtailTestUtils): self.assertIn(reverse('wagtailadmin_pages:history', args=(self.page.id, )), html) self.assertTemplateUsed(response, 'wagtailadmin/workflows/workflow_status.html') + + def test_status_through_workflow_cycle(self): + self.login(self.superuser) + response = self.client.get(self.edit_url) + self.assertContains(response, 'Draft', 1) + + self.page.save_revision() + response = self.client.get(self.edit_url) + self.assertContains(response, 'Draft saved', 1) + + self.submit() + response = self.client.get(self.edit_url) + self.assertRegex(response.content.decode('utf-8'), r'Awaiting[\s|\n]+{}'.format(self.task_1.name)) + + response = self.workflow_action('approve') + self.assertRegex(response.content.decode('utf-8'), r'Awaiting[\s|\n]+{}'.format(self.task_2.name)) + + response = self.workflow_action('reject') + self.assertContains(response, 'Changes requested') + + # resubmit + self.submit() + response = self.client.get(self.edit_url) + self.assertRegex(response.content.decode('utf-8'), r'Awaiting[\s|\n]+{}'.format(self.task_2.name)) + + response = self.workflow_action('approve') + self.assertContains(response, 'Published') + + def test_status_after_cancel(self): + # start workflow, then cancel + self.submit() + self.submit('action-cancel-workflow') + response = self.client.get(self.edit_url) + self.assertContains(response, 'Draft saved') + + def test_status_after_restart(self): + self.submit() + response = self.workflow_action('approve') + self.assertRegex(response.content.decode('utf-8'), r'Awaiting[\s|\n]+{}'.format(self.task_2.name)) + self.workflow_action('reject') + self.submit('action-restart-workflow') + response = self.client.get(self.edit_url) + self.assertRegex(response.content.decode('utf-8'), r'Awaiting[\s|\n]+{}'.format(self.task_1.name)) diff --git a/wagtail/admin/views/pages.py b/wagtail/admin/views/pages.py index aaefcea57a..657d7e3a43 100644 --- a/wagtail/admin/views/pages.py +++ b/wagtail/admin/views/pages.py @@ -700,7 +700,7 @@ def edit(request, page_id): 'next': next_url, 'has_unsaved_changes': has_unsaved_changes, 'page_locked': page_perms.page_locked(), - 'workflow_state': workflow_state, + 'workflow_state': workflow_state if workflow_state and workflow_state.is_active else None, 'current_task_state': page.current_workflow_task_state, }) @@ -1321,7 +1321,7 @@ def workflow_action(request, page_id, action_name, task_state_id): def workflow_status(request, page_id): page = get_object_or_404(Page, id=page_id) - if not page.workflow_in_progress: + if not page.current_workflow_state: raise PermissionDenied workflow_tasks = [] diff --git a/wagtail/core/models.py b/wagtail/core/models.py index b1fa8f267c..5158bcfa6f 100644 --- a/wagtail/core/models.py +++ b/wagtail/core/models.py @@ -3384,6 +3384,10 @@ class WorkflowState(models.Model): return tasks + @property + def is_active(self): + return self.status not in [self.STATUS_APPROVED, self.STATUS_CANCELLED] + class Meta: verbose_name = _('Workflow state') verbose_name_plural = _('Workflow states') diff --git a/wagtail/core/tests/test_workflow.py b/wagtail/core/tests/test_workflow.py index 77c34767db..21bde08529 100644 --- a/wagtail/core/tests/test_workflow.py +++ b/wagtail/core/tests/test_workflow.py @@ -208,6 +208,7 @@ class TestWorkflows(TestCase): self.assertEqual(workflow_state.status, workflow_state.STATUS_IN_PROGRESS) self.assertEqual(workflow_state.current_task_state.status, workflow_state.current_task_state.STATUS_IN_PROGRESS) self.assertEqual(workflow_state.current_task_state.task, task_2) + self.assertTrue(workflow_state.is_active) def test_tasks_with_status_on_resubmission(self): # test that a Workflow rejected and resumed shows the status of the latest tasks when _`all_tasks_with_status` is called @@ -242,6 +243,7 @@ class TestWorkflows(TestCase): self.assertEqual(workflow_state.status, WorkflowState.STATUS_CANCELLED) self.assertEqual(workflow_state.current_task_state.status, TaskState.STATUS_CANCELLED) self.assertFalse(TaskState.objects.filter(workflow_state=workflow_state, status=TaskState.STATUS_IN_PROGRESS).exists()) + self.assertFalse(workflow_state.is_active) def test_task_workflows(self): workflow = Workflow.objects.create(name='test_workflow')