Add support for saving revisions of snippets (#751)

Author: James <[email protected]>

README.md

@@ -72,7 +72,7 @@ With your preferred virtualenv activated, install testing dependencies:
 #### Using pip
 
 ```sh
-pip install pip>=21.3
+pip install "pip>=21.3"
 pip install -e '.[testing]' -U
 ```
 

docs/how-to/installation.md

@@ -54,10 +54,13 @@ class MyPage(Page):
     # ...
 ```
 
-## Disabling default publication of translated pages
+## Saving new translations as drafts
 
-Live pages that are submitted for translation are made live immediately. If you wish live pages submitted for
-translation to remain as drafts, set `WAGTAILLOCALIZE_SYNC_LIVE_STATUS_ON_TRANSLATE = False` in your settings file.
+Live versions of models that support drafts (i.e. subclasses of `Page`, and models which inherit `DraftStateMixin` and `RevisionMixin`)
+which are submitted for translation are made live immediately by default.
+
+If you would like to ensure that live instances which are newly submitted for translation remain as drafts for manual
+publication, set `WAGTAILLOCALIZE_SYNC_LIVE_STATUS_ON_TRANSLATE = False` in your settings file.
 
 ## Control translation cleanup mode
 

wagtail_localize/migrations/0016_rename_page_revision_translationlog_revision.py

@@ -0,0 +1,17 @@
+# Generated by Django 4.2.9 on 2024-01-06 13:20
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("wagtail_localize", "0015_translationcontext_field_path"),
+    ]
+
+    operations = [
+        migrations.RenameField(
+            model_name="translationlog",
+            old_name="page_revision",
+            new_name="revision",
+        ),
+    ]

wagtail_localize/models.py

@@ -41,8 +41,10 @@
 from wagtail.coreutils import find_available_slug
 from wagtail.fields import StreamField
 from wagtail.models import (
+    DraftStateMixin,
     Page,
     PageLogEntry,
+    RevisionMixin,
     TranslatableMixin,
     _copy,
     get_translatable_models,
@@ -727,7 +729,7 @@ def create_or_update_translation(
 
         Raises:
             SourceDeletedError: if the source object has been deleted.
-            CannotSaveDraftError: if the `publish` parameter was set to `False` when translating a non-page object.
+            CannotSaveDraftError: if the `publish` parameter was set to `False` when translating a non-DraftStateMixin object.
             MissingTranslationError: if a translation is missing and `fallback `is not `True`.
             MissingRelatedObjectError: if a related object is not translated and `fallback `is not `True`.
 
@@ -737,9 +739,8 @@ def create_or_update_translation(
         original = self.as_instance()
         created = False
 
-        # Only pages can be saved as draft
-        # To-Do: add support for models using DraftStateMixin
-        if not publish and not isinstance(original, Page):
+        # Only models with DraftStateMixin can be saved as a draft
+        if not publish and not isinstance(original, DraftStateMixin):
             raise CannotSaveDraftError
 
         try:
@@ -797,19 +798,31 @@ def create_or_update_translation(
                     translation.save()
 
                     # Create a new revision
-                    page_revision = translation.save_revision(user=user)
+                    new_revision = translation.save_revision(user=user)
 
                     self.sync_view_restrictions(original, translation)
 
                     if publish:
-                        transaction.on_commit(page_revision.publish)
+                        transaction.on_commit(new_revision.publish)
+
+                # Note: DraftStateMixin requires RevisionMixin, so RevisionMixin is checked here for typing
+                elif isinstance(translation, RevisionMixin):
+                    # We copied another instance which may be live, so we make sure this one matches the desired state
+                    translation.live = publish
+                    translation.save()
+
+                    # Create a new revision of the Snippet
+                    new_revision = translation.save_revision(user=user)
+
+                    if publish:
+                        transaction.on_commit(new_revision.publish)
 
                 else:
-                    # Note: we don't need to run full_clean for Pages as Wagtail does that in Page.save()
+                    # Note: we don't need to run full_clean for DraftStateMixin objects or Pages as Wagtail does that in RevisionMixin.save_revision()
                     translation.full_clean()
 
                     translation.save()
-                    page_revision = None
+                    new_revision = None
 
         except ValidationError as e:
             # If the validation error's field matches the context of a translation,
@@ -855,9 +868,7 @@ def create_or_update_translation(
             raise
 
         # Log that the translation was made
-        TranslationLog.objects.create(
-            source=self, locale=locale, page_revision=page_revision
-        )
+        TranslationLog.objects.create(source=self, locale=locale, revision=new_revision)
 
         return translation, created
 
@@ -1355,8 +1366,7 @@ class TranslationLog(models.Model):
         source (ForeignKey to TranslationSource): The source that was used for translation.
         locale (ForeignKey to Locale): The Locale that the source was translated into.
         created_at (DateTimeField): The date/time the translation was done.
-        page_revision (ForeignKey to PageRevision): If the translation was of a page, this links to the PageRevision
-            that was created
+        revision (ForeignKey to Revision): If the translation was of a page, this links to the Revision that was created
     """
 
     source = models.ForeignKey(
@@ -1368,7 +1378,7 @@ class TranslationLog(models.Model):
         related_name="translation_logs",
     )
     created_at = models.DateTimeField(auto_now_add=True)
-    page_revision = models.ForeignKey(
+    revision = models.ForeignKey(
         "wagtailcore.Revision",
         on_delete=models.SET_NULL,
         null=True,
@@ -1377,7 +1387,9 @@ class TranslationLog(models.Model):
     )
 
     def __str__(self):
-        return f"TranslationLog: {self.source_id}, {self.locale_id}, {self.page_revision_id} "
+        return (
+            f"TranslationLog: {self.source_id}, {self.locale_id}, {self.revision_id} "
+        )
 
     def get_instance(self):
         """

wagtail_localize/operations.py

@@ -3,7 +3,7 @@
 from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.db import transaction
-from wagtail.models import Page
+from wagtail.models import DraftStateMixin, Page
 
 from wagtail_localize.models import Translation, TranslationSource
 
@@ -79,12 +79,9 @@ def create_translations(self, instance, include_related_objects=True):
             # Determine whether to publish the translation.
             if getattr(settings, "WAGTAILLOCALIZE_SYNC_LIVE_STATUS_ON_TRANSLATE", True):
                 publish = getattr(instance, "live", True)
-            elif isinstance(instance, Page):
-                publish = False
             else:
-                # we cannot save drafts for non-Page models, so set this to True
-                # To-Do: add support for models using DraftStateMixin
-                publish = True
+                # If the model can't be saved as a draft, then we have to publish it
+                publish = not isinstance(instance, DraftStateMixin)
 
             try:
                 translation.save_target(user=self.user, publish=publish)

wagtail_localize/test/migrations/0005_testsnippet_expire_at_testsnippet_expired_and_more.py

@@ -0,0 +1,124 @@
+# Generated by Django 4.2.9 on 2024-01-06 13:20
+
+import uuid
+
+import django.db.models.deletion
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("wagtailcore", "0077_alter_revision_user"),
+        ("wagtail_localize_test", "0004_testparentalsnippet"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="testsnippet",
+            name="expire_at",
+            field=models.DateTimeField(
+                blank=True, null=True, verbose_name="expiry date/time"
+            ),
+        ),
+        migrations.AddField(
+            model_name="testsnippet",
+            name="expired",
+            field=models.BooleanField(
+                default=False, editable=False, verbose_name="expired"
+            ),
+        ),
+        migrations.AddField(
+            model_name="testsnippet",
+            name="first_published_at",
+            field=models.DateTimeField(
+                blank=True, db_index=True, null=True, verbose_name="first published at"
+            ),
+        ),
+        migrations.AddField(
+            model_name="testsnippet",
+            name="go_live_at",
+            field=models.DateTimeField(
+                blank=True, null=True, verbose_name="go live date/time"
+            ),
+        ),
+        migrations.AddField(
+            model_name="testsnippet",
+            name="has_unpublished_changes",
+            field=models.BooleanField(
+                default=False, editable=False, verbose_name="has unpublished changes"
+            ),
+        ),
+        migrations.AddField(
+            model_name="testsnippet",
+            name="last_published_at",
+            field=models.DateTimeField(
+                editable=False, null=True, verbose_name="last published at"
+            ),
+        ),
+        migrations.AddField(
+            model_name="testsnippet",
+            name="latest_revision",
+            field=models.ForeignKey(
+                blank=True,
+                editable=False,
+                null=True,
+                on_delete=django.db.models.deletion.SET_NULL,
+                related_name="+",
+                to="wagtailcore.revision",
+                verbose_name="latest revision",
+            ),
+        ),
+        migrations.AddField(
+            model_name="testsnippet",
+            name="live",
+            field=models.BooleanField(
+                default=True, editable=False, verbose_name="live"
+            ),
+        ),
+        migrations.AddField(
+            model_name="testsnippet",
+            name="live_revision",
+            field=models.ForeignKey(
+                blank=True,
+                editable=False,
+                null=True,
+                on_delete=django.db.models.deletion.SET_NULL,
+                related_name="+",
+                to="wagtailcore.revision",
+                verbose_name="live revision",
+            ),
+        ),
+        migrations.CreateModel(
+            name="TestNoDraftModel",
+            fields=[
+                (
+                    "id",
+                    models.AutoField(
+                        auto_created=True,
+                        primary_key=True,
+                        serialize=False,
+                        verbose_name="ID",
+                    ),
+                ),
+                (
+                    "translation_key",
+                    models.UUIDField(default=uuid.uuid4, editable=False),
+                ),
+                ("field", models.CharField(blank=True, max_length=10)),
+                (
+                    "locale",
+                    models.ForeignKey(
+                        editable=False,
+                        on_delete=django.db.models.deletion.PROTECT,
+                        related_name="+",
+                        to="wagtailcore.locale",
+                    ),
+                ),
+            ],
+            options={
+                "abstract": False,
+                "unique_together": {("translation_key", "locale")},
+            },
+        ),
+    ]

wagtail_localize/test/models.py

@@ -16,7 +16,13 @@
 from wagtail.embeds.blocks import EmbedBlock
 from wagtail.fields import RichTextField, StreamField
 from wagtail.images.blocks import ImageChooserBlock
-from wagtail.models import Orderable, Page, TranslatableMixin
+from wagtail.models import (
+    DraftStateMixin,
+    Orderable,
+    Page,
+    RevisionMixin,
+    TranslatableMixin,
+)
 from wagtail.snippets.blocks import SnippetChooserBlock
 from wagtail.snippets.models import register_snippet
 
@@ -27,7 +33,7 @@
 
 
 @register_snippet
-class TestSnippet(TranslatableMixin, ClusterableModel):
+class TestSnippet(TranslatableMixin, DraftStateMixin, RevisionMixin, ClusterableModel):
     field = models.TextField(gettext_lazy("field"))
     # To test field level validation of snippets
     small_charfield = models.CharField(max_length=10, blank=True)
@@ -45,6 +51,18 @@ class TestSnippet(TranslatableMixin, ClusterableModel):
     ]
 
 
+class TestNoDraftModel(TranslatableMixin):
+    field = models.CharField(max_length=10, blank=True)
+
+    translatable_fields = [
+        TranslatableField("field"),
+    ]
+
+    panels = [
+        FieldPanel("field"),
+    ]
+
+
 class TestSnippetOrderable(TranslatableMixin, Orderable):
     parent = ParentalKey(
         "TestSnippet",