Merge ef3d01f55f into a09bba67cd

wagtail/models/__init__.py

@@ -290,7 +290,14 @@ class RevisionMixin(models.Model):
         "latest_revision",
     ]
 
-    @property
+    _revisions = GenericRelation(
+        "wagtailcore.Revision",
+        content_type_field="content_type",
+        object_id_field="object_id",
+        for_concrete_model=False,
+    )
+
+    @cached_property
     def revisions(self):
         """
         Returns revisions that belong to the object.
@@ -302,10 +309,7 @@ class RevisionMixin(models.Model):
         ``related_query_name`` of the ``GenericRelation`` and add custom logic
         (e.g. to always use the specific instance in ``Page``).
         """
-        return Revision.objects.filter(
-            content_type=self.get_content_type(),
-            object_id=self.pk,
-        )
+        return Revision.objects.for_instance(self)
 
     def get_base_content_type(self):
         parents = self._meta.get_parent_list()
@@ -915,9 +919,19 @@ class LockableMixin(models.Model):
             return BasicLock(self)
 
 
-class WorkflowMixin:
+class WorkflowMixin(models.Model):
     """A mixin that allows a model to have workflows."""
 
+    _workflow_states = GenericRelation(
+        "wagtailcore.WorkflowState",
+        content_type_field="base_content_type",
+        object_id_field="object_id",
+        for_concrete_model=False,
+    )
+
+    class Meta:
+        abstract = True
+
     @classmethod
     def check(cls, **kwargs):
         return [
@@ -990,7 +1004,7 @@ class WorkflowMixin:
         """Returns the active workflow assigned to the object."""
         return self.get_default_workflow()
 
-    @property
+    @cached_property
     def workflow_states(self):
         """
         Returns workflow states that belong to the object.
@@ -1199,9 +1213,16 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
         verbose_name=_("latest revision created at"), null=True, editable=False
     )
 
-    _revisions = GenericRelation("wagtailcore.Revision", related_query_name="page")
+    _revisions = GenericRelation(
+        "wagtailcore.Revision",
+        content_type_field="content_type",
+        object_id_field="object_id",
+        related_query_name="page",
+        for_concrete_model=False,
+    )
 
-    # Add GenericRelation to allow WorkflowState.objects.filter(page=...) queries.
+    # Override WorkflowMixin's GenericRelation to specify related_query_name
+    # so we can do WorkflowState.objects.filter(page=...) queries.
     # There is no need to override the workflow_states property, as the default
     # implementation in WorkflowMixin already ensures that the queryset uses the
     # base Page content type.
@@ -1344,7 +1365,7 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
     def __str__(self):
         return self.title
 
-    @property
+    @cached_property
     def revisions(self):
         # Always use the specific page instance when querying for revisions as
         # they are always saved with the specific content_type.
@@ -1707,7 +1728,7 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
         clean=True,
     ):
         # Raise error if this is not the specific version of the page
-        if not isinstance(self, self.specific_class):
+        if not self.is_specific:
             raise RuntimeError(
                 "page.save_revision() must be called on the specific version of the page. "
                 "Call page.specific.save_revision() instead."
@@ -2460,7 +2481,7 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
             and self.specific_class.permissions_for_user
             != type(self).permissions_for_user
         )
-        if is_overridden and not isinstance(self, self.specific_class):
+        if is_overridden and not self.is_specific:
             return self.specific_deferred.permissions_for_user(user)
         return PagePermissionTester(user, self)
 
@@ -2789,12 +2810,23 @@ class RevisionQuerySet(models.QuerySet):
         return self.exclude(self.page_revisions_q())
 
     def for_instance(self, instance):
-        return self.filter(
-            content_type=ContentType.objects.get_for_model(
-                instance, for_concrete_model=False
-            ),
-            object_id=str(instance.pk),
-        )
+        """
+        Filters to only Revisions for the given instance
+        """
+        try:
+            # Use RevisionMixin.get_base_content_type() if available
+            return self.filter(
+                base_content_type=instance.get_base_content_type(),
+                object_id=str(instance.pk),
+            )
+        except AttributeError:
+            # Fallback to ContentType for the model
+            return self.filter(
+                content_type=ContentType.objects.get_for_model(
+                    instance, for_concrete_model=False
+                ),
+                object_id=str(instance.pk),
+            )
 
 
 class RevisionsManager(models.Manager.from_queryset(RevisionQuerySet)):

wagtail/models/specific.py

@@ -118,6 +118,10 @@ class SpecificMixin:
         """
         return self.cached_content_type.model_class()
 
+    @property
+    def is_specific(self):
+        return self.specific_class is not None and isinstance(self, self.specific_class)
+
     @property
     def cached_content_type(self):
         """

wagtail/tests/test_revision_model.py

@@ -173,15 +173,15 @@ class TestRevisableModel(TestCase):
     def test_revision_cascade_on_object_delete(self):
         page = self.create_page()
         full_featured_snippet = FullFeaturedSnippet.objects.create(text="foo")
+        # The RevisionMixin should provide a default `GenericRelation` so that
+        # revisions are deleted when the object is deleted, even if the
+        # model does not explicitly define a `GenericRelation` to `Revision`.
         cases = [
-            # Tuple of (instance, cascades)
-            # For models that define a GenericRelation to Revision, the revision
-            # should be deleted when the instance is deleted.
-            (page, True),
-            (full_featured_snippet, True),
-            (self.instance, False),  # No GenericRelation to Revision
+            page,
+            full_featured_snippet,
+            self.instance,  # No explicit GenericRelation to Revision
         ]
-        for instance, cascades in cases:
+        for instance in cases:
             with self.subTest(instance=instance):
                 revision = instance.save_revision()
                 query = {
@@ -190,4 +190,4 @@ class TestRevisableModel(TestCase):
                 }
                 self.assertEqual(Revision.objects.filter(**query).first(), revision)
                 instance.delete()
-                self.assertIs(Revision.objects.filter(**query).exists(), not cascades)
+                self.assertIs(Revision.objects.filter(**query).exists(), False)

wagtail/tests/test_workflow.py

@@ -449,7 +449,7 @@ class TestPageWorkflows(WagtailTestUtils, TestCase):
         self.assertIsNone(self.object.locked_at)
         self.assertIsNone(self.object.locked_by)
 
-    def test_workflow_state_cascade_on_object_delete(self, cascades=True):
+    def test_workflow_state_cascade_on_object_delete(self):
         data = self.start_workflow()
         query = {
             "base_content_type": self.object.get_base_content_type(),
@@ -460,7 +460,7 @@ class TestPageWorkflows(WagtailTestUtils, TestCase):
             data["workflow_state"],
         )
         self.object.delete()
-        self.assertIs(WorkflowState.objects.filter(**query).exists(), not cascades)
+        self.assertIs(WorkflowState.objects.filter(**query).exists(), False)
 
 
 class TestSnippetWorkflows(TestPageWorkflows):
@@ -493,9 +493,6 @@ class TestSnippetWorkflowsNotLockable(TestSnippetWorkflows):
         self.assertEqual(workflow_state.content_object, self.object)
         self.assertEqual(workflow_state.status, "in_progress")
 
-    def test_workflow_state_cascade_on_object_delete(self):
-        # We expect the cascade to not happen as the model does not define
-        # a GenericRelation to WorkflowState. However, workflows should still
-        # work as expected.
-        # See https://github.com/wagtail/wagtail/issues/11300 for more details.
-        return super().test_workflow_state_cascade_on_object_delete(cascades=False)
+    # The ModeratedModel does not explicitly define a `GenericRelation` to
+    # `WorkflowState`, but the `WorkflowState` should still be deleted when the
+    # object is deleted (test_workflow_state_cascade_on_object_delete passes).