Handle "needs changes" workflow state in workflow status

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;
     }

wagtail/admin/templates/wagtailadmin/pages/create.html

@@ -10,12 +10,9 @@
     <header class="merged tab-merged">
         {% explorer_breadcrumb parent_page include_self=1 %}
 
-        <div class="row row-flush">
+        <div class="row">
             <div class="left col9 header-title">
-                <h1>
-                    {% icon "doc-empty-inverse" class_name="header-title-icon" %}
-                    {% trans 'New' %} <span>{{ content_type.model_class.get_verbose_name }}</span>
-                </h1>
+                <h1>{% trans 'New' %} <span>{{ content_type.model_class.get_verbose_name }}</span></h1>
             </div>
         </div>
     </header>

wagtail/admin/templates/wagtailadmin/shared/workflow_status.html

@@ -1,14 +1,21 @@
 {% load i18n wagtailadmin_tags %}
-{% if workflow_state and page.current_workflow_task %}
-    <button data-url="{% url 'wagtailadmin_pages:workflow_status' page.id %}" class="button button-small button-nobg button-strokeonhover text-notransform action-workflow-status">
-        {% if workflow_state.status == "in_progress" %}
-            {% trans "Awaiting" %}
+{% if workflow_state %}
+    <button data-url="{% url 'wagtailadmin_pages:workflow_status' page.id %}"
+            class="button button-small button-nobg button-strokeonhover text-notransform action-workflow-status">
+        {% if workflow_state.status == 'needs_changes' %}
+            {% icon name="warning" %}
+            {% trans "Changes requested" %}
+            {% include "wagtailadmin/shared/last_updated.html" with since_text=since_text last_updated=workflow_state.current_task_state.finished_at time_prefix="at" %}
         {% else %}
-            {{ workflow_state.status_display }}
+            {% if workflow_state.status == "in_progress" %}
+                {% trans "Awaiting" %}
+                {% trans "since" as since_text %}
+            {% else %}
+                {{ workflow_state.get_status_display }}
+            {% endif %}
+            {{ workflow_state.current_task_state.task.name }}
+            {% include "wagtailadmin/shared/last_updated.html" with since_text=since_text last_updated=workflow_state.current_task_state.started_at %}
         {% endif %}
-        {{ page.current_workflow_task.name }}
-        {% trans "since" as since_text %}
-        {% include "wagtailadmin/shared/last_updated.html" with since_text=since_text last_updated=workflow_state.current_task_state.started_at %}
     </button>
 {% else %}
     {% if page.get_latest_revision %}

wagtail/admin/templates/wagtailadmin/workflows/workflow_status.html

@@ -24,7 +24,11 @@
     <ul class="workflow-timeline">
     {% for task in workflow_tasks %}
         <li data-wagtail-tooltip="{{ task.name }}: {{ task.status_display }}" {% if task.status %}class="{{ task.status }}"{% endif %}>
-            {% icon name="success" class_name="default" %} {{ task.name }}
+            {% if task.status == "rejected" %}
+                {% icon name="warning" class_name="default" %}
+            {% else %}
+                {% icon name="success" class_name="default" %}
+            {% endif %}{{ task.name }}
         </li>
     {% endfor %}
     </ul>

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))

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 = []

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')

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')