Avoid purging Revisions in use by third-party packages (#10961)

* Resolves #10678 Avoid purging Revisions in use by third-party packages

---------
Co-authored-by: MeghanaNalla <123588774+MeghanaNalla@users.noreply.github.com>
Co-authored-by: sag​e <laymonage@gmail.com>
Co-authored-by: Storm B. Heg <storm@stormbase.digital>

docs/reference/management_commands.md

@@ -52,7 +52,14 @@ This command deletes old revisions which are not in moderation, live, approved t
 revision. If the `days` argument is supplied, only revisions older than the specified number of
 days will be deleted.
 
-If the `pages` argument is supplied, only revisions of page models will be deleted. If the `non-pages` argument is supplied, only revisions of non-page models will be deleted. If both or neither arguments are supplied, revisions of all models will be deleted.
+To prevent deleting important revisions when they become stale, you can refer to such revisions in a model using a `ForeignKey` with {attr}`on_delete=models.PROTECT <django.db.models.PROTECT>`.
+
+```{versionadded} 5.2
+Support for respecting `on_delete=models.PROTECT` is added.
+```
+
+If the `pages` argument is supplied, only revisions of page models will be deleted. If the `non-pages` argument is supplied, only revisions of non-page models will be deleted. If both or neither arguments are supplied, revisions of all models will be deleted. 
+If deletion of a revision is not desirable, mark `Revision` with `on_delete=models.PROTECT`. 
 
 ```{versionadded} 5.1
 Support for deleting revisions of non-page models is added.

docs/reference/pages/model_reference.md

@@ -681,6 +681,8 @@ Every time a page is edited, a new `Revision` is created and saved to the databa
 -   The content of the page is JSON-serialisable and stored in the {attr}`~Revision.content` field.
 -   You can retrieve a `Revision` as an instance of the object's model by calling the {meth}`~Revision.as_object` method.
 
+You can use the [`purge_revisions`](purge_revisions) command to delete old revisions that are no longer in use.
+
 ### Database fields
 
 ```{eval-rst}

wagtail/management/commands/purge_revisions.py

@@ -1,6 +1,7 @@
 from django.conf import settings
 from django.core.management.base import BaseCommand
 from django.db.models import Q
+from django.db.models.deletion import ProtectedError
 from django.utils import timezone
 
 from wagtail.models import Revision, WorkflowState
@@ -31,7 +32,9 @@ class Command(BaseCommand):
         pages = options.get("pages")
         non_pages = options.get("non_pages")
 
-        revisions_deleted = purge_revisions(days=days, pages=pages, non_pages=non_pages)
+        revisions_deleted, protected_error_count = purge_revisions(
+            days=days, pages=pages, non_pages=non_pages
+        )
 
         if revisions_deleted:
             self.stdout.write(
@@ -39,6 +42,12 @@ class Command(BaseCommand):
                     "Successfully deleted %s revisions" % revisions_deleted
                 )
             )
+            self.stdout.write(
+                self.style.SUCCESS(
+                    "Ignored %s revisions because one or more protected relations exist that prevent deletion."
+                    % protected_error_count
+                )
+            )
         else:
             self.stdout.write("No revisions deleted")
 
@@ -74,11 +83,15 @@ def purge_revisions(days=None, pages=True, non_pages=True):
         purgeable_revisions = purgeable_revisions.filter(created_at__lt=purgeable_until)
 
     deleted_revisions_count = 0
+    protected_error_count = 0
 
     for revision in purgeable_revisions.iterator():
         # don't delete the latest revision
         if not revision.is_latest_revision():
-            revision.delete()
-            deleted_revisions_count += 1
+            try:
+                revision.delete()
+                deleted_revisions_count += 1
+            except ProtectedError:
+                protected_error_count += 1
 
-    return deleted_revisions_count
+    return deleted_revisions_count, protected_error_count

wagtail/test/testapp/migrations/0029_variousondeletemodel_cascading_toy.py

@@ -5,7 +5,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [
         ("tests", "0028_fullfeaturedsnippet_some_number"),
     ]

wagtail/test/testapp/migrations/0030_purgerevisionsprotectedtestmodel.py

@@ -0,0 +1,36 @@
+# Generated by Django 4.0.10 on 2023-10-09 07:24
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("wagtailcore", "0089_log_entry_data_json_null_to_object"),
+        ("tests", "0029_variousondeletemodel_cascading_toy"),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name="PurgeRevisionsProtectedTestModel",
+            fields=[
+                (
+                    "id",
+                    models.AutoField(
+                        auto_created=True,
+                        primary_key=True,
+                        serialize=False,
+                        verbose_name="ID",
+                    ),
+                ),
+                (
+                    "revision",
+                    models.OneToOneField(
+                        on_delete=django.db.models.deletion.PROTECT,
+                        related_name="+",
+                        to="wagtailcore.revision",
+                    ),
+                ),
+            ],
+        ),
+    ]

wagtail/test/testapp/models.py

@@ -2181,3 +2181,9 @@ class FeatureCompleteToy(index.Indexed, models.Model):
 
     def __str__(self):
         return f"{self.name} ({self.release_date})"
+
+
+class PurgeRevisionsProtectedTestModel(models.Model):
+    revision = models.OneToOneField(
+        "wagtailcore.Revision", on_delete=models.PROTECT, related_name="+"
+    )

wagtail/tests/test_management_commands.py

@@ -24,6 +24,7 @@ from wagtail.test.testapp.models import (
     DraftStateModel,
     EventPage,
     FullFeaturedSnippet,
+    PurgeRevisionsProtectedTestModel,
     SecretPage,
     SimplePage,
 )
@@ -167,7 +168,6 @@ class TestMovePagesCommand(TestCase):
 
 
 class TestSetUrlPathsCommand(TestCase):
-
     fixtures = ["test.json"]
 
     def run_command(self):
@@ -728,6 +728,18 @@ class TestPurgeRevisionsCommandForPages(TestCase):
         # revision is now older than 30 days, so should be deleted
         self.assertRevisionNotExists(old_revision)
 
+    def test_purge_revisions_protected_error(self):
+        revision_old = self.object.save_revision()
+        PurgeRevisionsProtectedTestModel.objects.create(revision=revision_old)
+        revision_purged = self.object.save_revision()
+        self.object.save_revision()
+
+        self.run_command()
+        # revision should not be deleted, as it is protected
+        self.assertRevisionExists(revision_old)
+        # Any other revisions are deleted
+        self.assertRevisionNotExists(revision_purged)
+
 
 class TestPurgeRevisionsCommandForSnippets(TestPurgeRevisionsCommandForPages):
     def get_object(self):