Fix page/snippet cannot proceed a GroupApprovalTask if it's locked by someone outside of the group

Regression in d85db866cc

CHANGELOG.txt

@@ -24,6 +24,7 @@ Changelog
  * Fix: Image and Document multiple upload update forms now correctly use the progress button (longrunning) behaviour when clicked (Loveth Omokaro)
  * Fix: Prevent audit log report from failing on missing models (Andy Chosak)
  * Fix: Ensure that the privacy collection privacy edit button is styled as a button (Jatin Kumar)
+ * Fix: Fix page/snippet cannot proceed a `GroupApprovalTask` if it's locked by someone outside of the group (Sage Abdullah)
  * Docs: Add code block to make it easier to understand contribution docs (Suyash Singh)
  * Docs: Add new "Icons" page for icons customisation and reuse across the admin interface (Coen van der Kamp)
  * Docs: Fix broken formatting for MultiFieldPanel / FieldRowPanel permission kwarg docs (Matt Westcott)
@@ -58,6 +59,7 @@ Changelog
  * Fix: Fix image uploads on storage backends that require file pointer to be at the start of the file (Matt Westcott)
  * Fix: Fix "Edit this page" missing from userbar (Satvik Vashisht)
  * Fix: Prevent audit log report from failing on missing models (Andy Chosak)
+ * Fix: Fix page/snippet cannot proceed a `GroupApprovalTask` if it's locked by someone outside of the group (Sage Abdullah)
 
 
 4.2 (06.02.2023)

docs/releases/4.2.1.md

@@ -17,3 +17,4 @@ depth: 1
  * Fix image uploads on storage backends that require file pointer to be at the start of the file (Matt Westcott)
  * Fix "Edit this page" missing from userbar (Satvik Vashisht)
  * Prevent audit log report from failing on missing models (Andy Chosak)
+ * Fix page/snippet cannot proceed a `GroupApprovalTask` if it's locked by someone outside of the group (Sage Abdullah)

docs/releases/5.0.md

@@ -37,6 +37,7 @@ depth: 1
  * Image and Document multiple upload update forms now correctly use the progress button (longrunning) behaviour when clicked (Loveth Omokaro)
  * Prevent audit log report from failing on missing models (Andy Chosak)
  * Ensure that the privacy collection privacy edit button is styled as a button (Jatin Kumar)
+ * Fix page/snippet cannot proceed a `GroupApprovalTask` if it's locked by someone outside of the group (Sage Abdullah)
 
 ### Documentation
 

wagtail/models/__init__.py

@@ -3854,7 +3854,7 @@ class GroupApprovalTask(Task):
 
     def start(self, workflow_state, user=None):
         if (
-            isinstance(workflow_state, LockableMixin)
+            isinstance(workflow_state.content_object, LockableMixin)
             and workflow_state.content_object.locked_by
         ):
             # If the person who locked the object isn't in one of the groups, unlock the object

wagtail/tests/test_workflow.py

@@ -7,6 +7,7 @@ from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import ValidationError
 from django.db.utils import IntegrityError
 from django.test import TestCase, override_settings
+from django.utils import timezone
 from freezegun import freeze_time
 
 from wagtail.models import (
@@ -21,6 +22,7 @@ from wagtail.models import (
     WorkflowTask,
 )
 from wagtail.test.testapp.models import FullFeaturedSnippet, ModeratedModel, SimplePage
+from wagtail.test.utils.wagtail_tests import WagtailTestUtils
 
 
 class TestWorkflowModels(TestCase):
@@ -116,7 +118,7 @@ class TestWorkflowModels(TestCase):
         self.assertTrue(workflow_2.all_pages().filter(id=goodbye_page.id).exists())
 
 
-class TestPageWorkflows(TestCase):
+class TestPageWorkflows(WagtailTestUtils, TestCase):
     fixtures = ["test.json"]
 
     @classmethod
@@ -416,6 +418,37 @@ class TestPageWorkflows(TestCase):
             )[0],
         )
 
+    def test_start_workflow_group_approval_task_locked(self):
+        self.object.locked = True
+        self.object.locked_at = timezone.now()
+        self.object.locked_by = self.create_user("user1")
+        self.object.save()
+
+        # Create a workflow with one group approval task for the moderators group
+        moderators = Group.objects.get(name="Moderators")
+        workflow = Workflow.objects.create(name="test_workflow_foo")
+        task_1 = GroupApprovalTask.objects.create(name="test_task_1")
+        task_1.groups.add(moderators)
+        WorkflowTask.objects.create(workflow=workflow, task=task_1, sort_order=1)
+
+        # The object was locked by a non-moderator
+        self.assertFalse(self.object.locked_by.groups.filter(id=moderators.id).exists())
+
+        # Start the workflow as another user
+        self.object.save_revision()
+        workflow_state = workflow.start(self.object, self.create_user("user2"))
+
+        self.assertEqual(workflow_state.workflow, workflow)
+        self.assertEqual(workflow_state.content_object, self.object)
+        self.assertEqual(workflow_state.status, "in_progress")
+
+        self.object.refresh_from_db()
+
+        # The lock should be removed as otherwise the object would be stuck
+        self.assertFalse(self.object.locked)
+        self.assertIsNone(self.object.locked_at)
+        self.assertIsNone(self.object.locked_by)
+
 
 class TestSnippetWorkflows(TestPageWorkflows):
     fixtures = None
@@ -428,3 +461,21 @@ class TestSnippetWorkflows(TestPageWorkflows):
 
 class TestSnippetWorkflowsNotLockable(TestSnippetWorkflows):
     model = ModeratedModel
+
+    def test_start_workflow_group_approval_task_locked(self):
+        # Test normal GroupApprovalTask.start() as the object is not lockable
+
+        # Create a workflow with one group approval task for the moderators group
+        moderators = Group.objects.get(name="Moderators")
+        workflow = Workflow.objects.create(name="test_workflow_foo")
+        task_1 = GroupApprovalTask.objects.create(name="test_task_1")
+        task_1.groups.add(moderators)
+        WorkflowTask.objects.create(workflow=workflow, task=task_1, sort_order=1)
+
+        # Start the workflow
+        self.object.save_revision()
+        workflow_state = workflow.start(self.object, self.create_user("user2"))
+
+        self.assertEqual(workflow_state.workflow, workflow)
+        self.assertEqual(workflow_state.content_object, self.object)
+        self.assertEqual(workflow_state.status, "in_progress")