diff --git a/wagtail/models/__init__.py b/wagtail/models/__init__.py index f86fc2661b..9f3ed1e267 100644 --- a/wagtail/models/__init__.py +++ b/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)): diff --git a/wagtail/models/specific.py b/wagtail/models/specific.py index 5411e434ea..fb8b534255 100644 --- a/wagtail/models/specific.py +++ b/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): """ diff --git a/wagtail/tests/test_revision_model.py b/wagtail/tests/test_revision_model.py index 74b964a95f..bf91bb4f86 100644 --- a/wagtail/tests/test_revision_model.py +++ b/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) diff --git a/wagtail/tests/test_workflow.py b/wagtail/tests/test_workflow.py index 1ce3acdc02..51c481d2ca 100644 --- a/wagtail/tests/test_workflow.py +++ b/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).