diff --git a/README.md b/README.md index 3dca2dd8e..a2e2f5347 100644 --- a/README.md +++ b/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 ``` diff --git a/docs/how-to/installation.md b/docs/how-to/installation.md index a61cf16e0..e051d0d1c 100644 --- a/docs/how-to/installation.md +++ b/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 diff --git a/wagtail_localize/migrations/0016_rename_page_revision_translationlog_revision.py b/wagtail_localize/migrations/0016_rename_page_revision_translationlog_revision.py new file mode 100644 index 000000000..3f15b371c --- /dev/null +++ b/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", + ), + ] diff --git a/wagtail_localize/models.py b/wagtail_localize/models.py index 722e81558..962c21bd7 100644 --- a/wagtail_localize/models.py +++ b/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): """ diff --git a/wagtail_localize/operations.py b/wagtail_localize/operations.py index 1df4ac2f1..1ea943722 100644 --- a/wagtail_localize/operations.py +++ b/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) diff --git a/wagtail_localize/test/migrations/0005_testsnippet_expire_at_testsnippet_expired_and_more.py b/wagtail_localize/test/migrations/0005_testsnippet_expire_at_testsnippet_expired_and_more.py new file mode 100644 index 000000000..0f6d37051 --- /dev/null +++ b/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")}, + }, + ), + ] diff --git a/wagtail_localize/test/models.py b/wagtail_localize/test/models.py index 4ff6fc9d8..8340d2b71 100644 --- a/wagtail_localize/test/models.py +++ b/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", diff --git a/wagtail_localize/tests/test_edit_translation.py b/wagtail_localize/tests/test_edit_translation.py index 72649189a..d4c5918e2 100644 --- a/wagtail_localize/tests/test_edit_translation.py +++ b/wagtail_localize/tests/test_edit_translation.py @@ -30,7 +30,7 @@ from wagtail.documents.models import Document from wagtail.images.models import Image from wagtail.images.tests.utils import get_test_image_file -from wagtail.models import Locale, Page +from wagtail.models import Locale, Page, Revision from wagtail.test.utils import WagtailTestUtils from wagtail_localize.models import ( @@ -1589,11 +1589,15 @@ def test_edit_snippet_translation(self): self.assertEqual( props["object"]["title"], f"TestSnippet object ({self.fr_snippet.id})" ) - self.assertTrue(props["object"]["isLive"]) + self.assertFalse( + props["object"]["isLive"] + ) # Draftable snippet is saved as draft + self.assertIsNone( + props["object"]["lastPublishedDate"] + ) # and so it won't have a last published date initially self.assertFalse(props["object"]["isLocked"]) - # Snippets don't have last published, live URL, breadcrumb, or tabs - self.assertIsNone(props["object"]["lastPublishedDate"]) + # Snippets don't have live URL, breadcrumb, or tabs self.assertIsNone(props["object"]["liveUrl"]) self.assertEqual(props["breadcrumb"], []) self.assertEqual(props["tabs"], [{"label": "Content", "slug": "content"}]) @@ -1814,7 +1818,7 @@ def test_publish_page_translation(self): log = TranslationLog.objects.get() self.assertEqual(log.source, self.page_source) self.assertEqual(log.locale, self.fr_locale) - self.assertEqual(log.page_revision, latest_revision) + self.assertEqual(log.revision, latest_revision) def test_publish_page_translation_with_missing_translations(self): # Same as the above test except we only fill in one field. We should be given a warning but the publish should be published. @@ -1856,7 +1860,7 @@ def test_publish_page_translation_with_missing_translations(self): log = TranslationLog.objects.get() self.assertEqual(log.source, self.page_source) self.assertEqual(log.locale, self.fr_locale) - self.assertEqual(log.page_revision, latest_revision) + self.assertEqual(log.revision, latest_revision) def test_publish_page_translation_with_new_field_error(self): translation = StringTranslation.objects.create( @@ -1951,7 +1955,7 @@ def test_publish_page_translation_with_known_field_error(self): log = TranslationLog.objects.get() self.assertEqual(log.source, self.page_source) self.assertEqual(log.locale, self.fr_locale) - self.assertEqual(log.page_revision, self.fr_page.get_latest_revision()) + self.assertEqual(log.revision, self.fr_page.get_latest_revision()) def test_publish_page_translation_with_invalid_segment_override(self): # Set the email address override to something invalid @@ -2047,7 +2051,7 @@ def test_publish_snippet_translation(self): log = TranslationLog.objects.get() self.assertEqual(log.source, self.snippet_source) self.assertEqual(log.locale, self.fr_locale) - self.assertIsNone(log.page_revision) + self.assertIsInstance(log.revision, Revision) def test_cant_publish_snippet_translation_without_perms(self): self.moderators_group.permissions.filter( diff --git a/wagtail_localize/tests/test_submit_translations.py b/wagtail_localize/tests/test_submit_translations.py index 12286892b..322fb3a6e 100644 --- a/wagtail_localize/tests/test_submit_translations.py +++ b/wagtail_localize/tests/test_submit_translations.py @@ -924,6 +924,7 @@ def test_post_submit_snippet_translation_draft(self): # The translated snippet should've been created translated_snippet = self.en_snippet.get_translation(self.fr_locale) self.assertEqual(translated_snippet.field, "Test snippet") + self.assertFalse(translated_snippet.live) self.assertRedirects( response, diff --git a/wagtail_localize/tests/test_translation_model.py b/wagtail_localize/tests/test_translation_model.py index 4d8980695..3fc1c4d64 100644 --- a/wagtail_localize/tests/test_translation_model.py +++ b/wagtail_localize/tests/test_translation_model.py @@ -32,6 +32,7 @@ from wagtail_localize.segments import RelatedObjectSegmentValue from wagtail_localize.strings import StringValue from wagtail_localize.test.models import ( + TestNoDraftModel, TestPage, TestParentalSnippet, TestSnippet, @@ -614,6 +615,10 @@ def test_save_target(self, _mock_on_commit): self.assertTrue(translated_page.live) + log = TranslationLog.objects.get() + self.assertEqual(log.source, self.source) + self.assertEqual(log.revision, translated_page.live_revision) + def test_save_target_as_draft(self): self.translation.save_target(publish=False) @@ -680,9 +685,9 @@ def test_save_target_snippet(self): translated_snippet = snippet.get_translation(self.fr_locale) self.assertEqual(translated_snippet.field, "Contenu de test") - def test_save_target_cant_save_snippet_as_draft(self): - snippet = TestSnippet.objects.create(field="Test content") - source, created = TranslationSource.get_or_create_from_instance(snippet) + def test_save_target_cant_save_non_draftable_as_draft(self): + snippet = TestNoDraftModel.objects.create(field="Test content") + source, _ = TranslationSource.get_or_create_from_instance(snippet) translation = Translation.objects.create( source=source, target_locale=self.fr_locale, @@ -699,6 +704,72 @@ def test_save_target_cant_save_snippet_as_draft(self): with self.assertRaises(CannotSaveDraftError): translation.save_target(publish=False) + def test_save_target_can_save_draftable_as_draft(self): + snippet = TestSnippet.objects.create(field="Test content") + source, _ = TranslationSource.get_or_create_from_instance(snippet) + translation = Translation.objects.create( + source=source, + target_locale=self.fr_locale, + ) + + translation.save_target(publish=False) + + self.assertFalse(snippet.get_translation(self.fr_locale).live) + + def test_save_target_can_update_draftable_as_draft(self): + snippet = TestSnippet.objects.create(field="Test content") + source, _ = TranslationSource.get_or_create_from_instance(snippet) + translation = Translation.objects.create( + source=source, + target_locale=self.fr_locale, + ) + + translation.save_target(publish=False) + + field_context = TranslationContext.objects.get(path="field") + StringTranslation.objects.create( + translation_of=self.test_content_string, + context=field_context, + locale=self.fr_locale, + data="Contenu de test", + ) + + translation.save_target(publish=False) + + self.assertFalse(snippet.get_translation(self.fr_locale).live) + self.assertEqual( + snippet.get_translation(self.fr_locale).field, "Contenu de test" + ) + + @patch.object(transaction, "on_commit", side_effect=lambda func: func()) + def test_save_target_can_publish_draftable(self, _mock_on_commit): + snippet = TestSnippet.objects.create(field="Test content") + source, _ = TranslationSource.get_or_create_from_instance(snippet) + translation = Translation.objects.create( + source=source, + target_locale=self.fr_locale, + ) + + field_context = TranslationContext.objects.get(path="field") + StringTranslation.objects.create( + translation_of=self.test_content_string, + context=field_context, + locale=self.fr_locale, + data="Contenu de test", + ) + + translation.save_target() + + french_snippet = snippet.get_translation(self.fr_locale) + + self.assertTrue(french_snippet.live) + self.assertEqual(french_snippet.field, "Contenu de test") + + log = TranslationLog.objects.get() + self.assertEqual(log.source, source) + self.assertEqual(log.locale, self.fr_locale) + self.assertEqual(log.revision, french_snippet.live_revision) + def test_uuid_snippet_save_target(self): foreign_key_target = TestUUIDModel.objects.create(charfield="Some Test") snippet = TestUUIDSnippet.objects.create(field=foreign_key_target) diff --git a/wagtail_localize/tests/test_translationsource_model.py b/wagtail_localize/tests/test_translationsource_model.py index 745a0acc2..e9ba14752 100644 --- a/wagtail_localize/tests/test_translationsource_model.py +++ b/wagtail_localize/tests/test_translationsource_model.py @@ -67,6 +67,15 @@ def test_create(self): json.loads(source.content_json), { "pk": self.snippet.pk, + "expire_at": None, + "expired": False, + "first_published_at": None, + "go_live_at": None, + "has_unpublished_changes": False, + "last_published_at": None, + "latest_revision": None, + "live": True, + "live_revision": None, "field": "This is some test content", "small_charfield": "", "test_snippet_orderable": [], @@ -120,6 +129,15 @@ def test_create(self): json.loads(source.content_json), { "pk": self.snippet.pk, + "expire_at": None, + "expired": False, + "go_live_at": None, + "has_unpublished_changes": False, + "first_published_at": None, + "last_published_at": None, + "latest_revision": None, + "live": True, + "live_revision": None, "field": "This is some test content", "small_charfield": "", "test_snippet_orderable": [], diff --git a/wagtail_localize/views/edit_translation.py b/wagtail_localize/views/edit_translation.py index e269c3d9c..d7af70c9c 100644 --- a/wagtail_localize/views/edit_translation.py +++ b/wagtail_localize/views/edit_translation.py @@ -45,7 +45,7 @@ from wagtail.fields import StreamField from wagtail.images.blocks import ImageChooserBlock from wagtail.images.models import AbstractImage -from wagtail.models import Page, TranslatableMixin +from wagtail.models import DraftStateMixin, Page, TranslatableMixin from wagtail.snippets.blocks import SnippetChooserBlock from wagtail.snippets.models import get_snippet_models from wagtail.snippets.permissions import get_permission_name, user_can_edit_snippet_type @@ -481,7 +481,7 @@ def widget_from_block(block, content_components=None): } -def edit_translation(request, translation, instance): +def edit_translation(request, translation: Translation, instance): if isinstance(instance, Page): # Page # Note: Edit permission is already checked by the edit page view @@ -509,6 +509,29 @@ def edit_translation(request, translation, instance): can_lock = page_perms.can_lock() can_unlock = page_perms.can_unlock() can_delete = page_perms.can_delete() + elif isinstance(instance, DraftStateMixin): + # Draftable Snippet + # Note: Edit permission is already checked by the edit snippet view + page_perms = None + + is_page = False + is_live = bool(instance.live_revision) + is_locked = False + last_published_at = instance.live_revision.created_at if is_live else None + last_published_by = instance.live_revision.user if is_live else None + live_url = None + + can_publish = ( + request.user.has_perm(get_permission_name("publish", instance.__class__)) + or request.user.is_superuser + ) + + can_unpublish = False # Snippets can't be unpublished + can_lock = False + can_unlock = False + can_delete = request.user.has_perm( + get_permission_name("delete", instance.__class__) + ) else: # Snippet # Note: Edit permission is already checked by the edit snippet view @@ -533,8 +556,17 @@ def edit_translation(request, translation, instance): if request.method == "POST": if request.POST.get("action") == "publish": - if isinstance(instance, Page): - if not page_perms.can_publish(): + if isinstance(instance, DraftStateMixin): + if isinstance(instance, Page): + if not page_perms.can_publish(): + raise PermissionDenied + + elif ( + not request.user.has_perm( + get_permission_name("publish", instance.__class__) + ) + and not request.user.is_superuser + ): raise PermissionDenied try: @@ -781,7 +813,9 @@ def get_source_object_info(segment): else: return { "title": str(instance), - "isLive": True, + "isLive": instance.live + if isinstance(instance, DraftStateMixin) + else True, "editUrl": get_edit_url(instance), "createTranslationRequestUrl": get_submit_translation_url(instance), } @@ -801,7 +835,9 @@ def get_dest_object_info(segment): else: return { "title": str(instance), - "isLive": True, + "isLive": instance.live + if isinstance(instance, DraftStateMixin) + else True, "editUrl": get_edit_url(instance), }