From f1fae34757596aff8632ec226bff8a1eebfc7779 Mon Sep 17 00:00:00 2001 From: chris48s Date: Fri, 6 May 2022 16:31:58 +0100 Subject: [PATCH 1/4] fix 'ready for review'/'published' status bug --- .../importer.py | 17 +++++- .../0006_languagecloudfile_revision.py | 25 ++++++++ wagtail_localize_rws_languagecloud/models.py | 57 ++++++++++++++----- .../tests/test_models.py | 30 +++++++++- 4 files changed, 111 insertions(+), 18 deletions(-) create mode 100644 wagtail_localize_rws_languagecloud/migrations/0006_languagecloudfile_revision.py diff --git a/wagtail_localize_rws_languagecloud/importer.py b/wagtail_localize_rws_languagecloud/importer.py index 4c1b760..baffaf1 100644 --- a/wagtail_localize_rws_languagecloud/importer.py +++ b/wagtail_localize_rws_languagecloud/importer.py @@ -1,6 +1,10 @@ import polib -from django.core.exceptions import SuspiciousOperation, ValidationError +from django.core.exceptions import ( + ObjectDoesNotExist, + SuspiciousOperation, + ValidationError, +) from django.db import transaction from wagtail.core.models import Page @@ -72,4 +76,15 @@ def import_po(self, translation, target_file): ) self.db_source_file.internal_status = LanguageCloudFile.STATUS_IMPORTED + + try: + instance = translation.get_target_instance() + if isinstance(instance, Page): + self.db_source_file.revision = instance.get_latest_revision() + except ObjectDoesNotExist: + # we will hit this case if we failed with a + # `MissingRelatedObjectError` or `ValidationError` + # trying to call `save_target()` a few lines up + pass + self.db_source_file.save() diff --git a/wagtail_localize_rws_languagecloud/migrations/0006_languagecloudfile_revision.py b/wagtail_localize_rws_languagecloud/migrations/0006_languagecloudfile_revision.py new file mode 100644 index 0000000..9393943 --- /dev/null +++ b/wagtail_localize_rws_languagecloud/migrations/0006_languagecloudfile_revision.py @@ -0,0 +1,25 @@ +# Generated by Django 3.1.14 on 2022-05-06 14:08 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("wagtailcore", "0066_collection_management_permissions"), + ("wagtail_localize_rws_languagecloud", "0005_languagecloudprojectsettings"), + ] + + operations = [ + migrations.AddField( + model_name="languagecloudfile", + name="revision", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to="wagtailcore.pagerevision", + ), + ), + ] diff --git a/wagtail_localize_rws_languagecloud/models.py b/wagtail_localize_rws_languagecloud/models.py index 104cf3f..266e660 100644 --- a/wagtail_localize_rws_languagecloud/models.py +++ b/wagtail_localize_rws_languagecloud/models.py @@ -2,7 +2,7 @@ from django.db import models from django.utils.functional import cached_property from django.utils.translation import gettext_lazy -from wagtail.core.models import Page +from wagtail.core.models import Page, PageRevision from wagtail_localize.components import register_translation_component from wagtail_localize.models import Translation, TranslationSource @@ -95,6 +95,12 @@ class LanguageCloudFile(StatusModel): project = models.ForeignKey(LanguageCloudProject, on_delete=models.CASCADE) lc_source_file_id = models.CharField(blank=True, max_length=255) create_attempts = models.IntegerField(default=0) + revision = models.ForeignKey( + PageRevision, + on_delete=models.CASCADE, + blank=True, + null=True, + ) class Meta: unique_together = [ @@ -111,11 +117,41 @@ def is_failed(self): return self.lc_source_file_id == "" and self.create_attempts >= 3 @property - def instance_is_published(self): + def published_status(self): + PUBLISHED = gettext_lazy("Translations published") + RECEIVED = gettext_lazy("Translations received") + READY_TO_REVIEW = gettext_lazy("Translations ready for review") + instance = self.translation.get_target_instance() + if not isinstance(instance, Page): - return True - return not instance.has_unpublished_changes + # snippets are always published immediately + return PUBLISHED + + if not self.revision: + """ + There's a couple of reasons we can end up here: + 1. For legacy reasons: we didn't always save a revision on this object. + Objects created with <=0.8.0 willl have a NULL revision + 2. If we threw an error trying to call save_target() in import_po() + + In this case, say we've got the translations back + but we don't know if they are published or not + """ + return RECEIVED + + if ( + self.revision == instance.get_latest_revision() + and not instance.has_unpublished_changes + ): + return PUBLISHED + + if (self.revision != instance.get_latest_revision()) and ( + self.revision.created_at < instance.get_latest_revision().created_at + ): + return PUBLISHED + + return READY_TO_REVIEW @property def combined_status(self): @@ -134,17 +170,8 @@ def combined_status(self): if self.project.lc_project_status == LanguageCloudStatus.ARCHIVED: return gettext_lazy("LanguageCloud project archived") - if ( - self.internal_status == LanguageCloudFile.STATUS_IMPORTED - and not self.instance_is_published - ): - return gettext_lazy("Translations ready for review") - - if ( - self.internal_status == LanguageCloudFile.STATUS_IMPORTED - and self.instance_is_published - ): - return gettext_lazy("Translations published") + if self.internal_status == LanguageCloudFile.STATUS_IMPORTED: + return self.published_status if self.internal_status == LanguageCloudFile.STATUS_ERROR: return gettext_lazy("Error importing PO file") diff --git a/wagtail_localize_rws_languagecloud/tests/test_models.py b/wagtail_localize_rws_languagecloud/tests/test_models.py index 81936d5..2d59c4d 100644 --- a/wagtail_localize_rws_languagecloud/tests/test_models.py +++ b/wagtail_localize_rws_languagecloud/tests/test_models.py @@ -76,7 +76,33 @@ def test_translations_ready(self): self.file.save() self.assertEqual(self.file.combined_status, "Translations ready for review") - def test_translations_published(self): + def test_translations_no_revision(self): + self.project.lc_project_id = "12345" + self.file.lc_source_file_id = "67890" + self.file.internal_status = LanguageCloudFile.STATUS_IMPORTED + importer = Importer(self.file, logging.getLogger("dummy")) + importer.import_po(self.translation, str(self.po_file)) + self.project.save() + self.file.revision = None + self.file.save() + self.assertEqual(self.file.combined_status, "Translations received") + + def test_translations_published_directly(self): + self.project.lc_project_id = "12345" + self.file.lc_source_file_id = "67890" + self.file.internal_status = LanguageCloudFile.STATUS_IMPORTED + importer = Importer(self.file, logging.getLogger("dummy")) + importer.import_po(self.translation, str(self.po_file)) + instance = self.translation.source.get_translated_instance( + self.translation.target_locale + ) + instance.get_latest_revision().publish() # publish the translated revision directly + self.project.save() + self.file.save() + self.translation.save() + self.assertEqual(self.file.combined_status, "Translations published") + + def test_translations_published_with_edits(self): self.project.lc_project_id = "12345" self.file.lc_source_file_id = "67890" self.file.internal_status = LanguageCloudFile.STATUS_IMPORTED @@ -85,7 +111,7 @@ def test_translations_published(self): instance = self.translation.source.get_translated_instance( self.translation.target_locale ) - instance.get_latest_revision().publish() + instance.save_revision().publish() # save a new revision and publish that self.project.save() self.file.save() self.translation.save() From 7da961891c01e39eb63a5ebcfe6021857cf30734 Mon Sep 17 00:00:00 2001 From: chris48s Date: Fri, 6 May 2022 16:41:30 +0100 Subject: [PATCH 2/4] drop dependency on 0066_collection_management_permissions --- .../migrations/0006_languagecloudfile_revision.py | 1 - 1 file changed, 1 deletion(-) diff --git a/wagtail_localize_rws_languagecloud/migrations/0006_languagecloudfile_revision.py b/wagtail_localize_rws_languagecloud/migrations/0006_languagecloudfile_revision.py index 9393943..51def35 100644 --- a/wagtail_localize_rws_languagecloud/migrations/0006_languagecloudfile_revision.py +++ b/wagtail_localize_rws_languagecloud/migrations/0006_languagecloudfile_revision.py @@ -7,7 +7,6 @@ class Migration(migrations.Migration): dependencies = [ - ("wagtailcore", "0066_collection_management_permissions"), ("wagtail_localize_rws_languagecloud", "0005_languagecloudprojectsettings"), ] From 08e893cce7845a3ced3857d56b49b69432a4d7f4 Mon Sep 17 00:00:00 2001 From: chris48s Date: Mon, 16 May 2022 10:06:44 +0100 Subject: [PATCH 3/4] add a test for 'more recent drafts but not published' case --- .../tests/test_models.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/wagtail_localize_rws_languagecloud/tests/test_models.py b/wagtail_localize_rws_languagecloud/tests/test_models.py index 2d59c4d..bdfd29c 100644 --- a/wagtail_localize_rws_languagecloud/tests/test_models.py +++ b/wagtail_localize_rws_languagecloud/tests/test_models.py @@ -117,6 +117,21 @@ def test_translations_published_with_edits(self): self.translation.save() self.assertEqual(self.file.combined_status, "Translations published") + def test_translations_not_yet_published_with_later_edits(self): + self.project.lc_project_id = "12345" + self.file.lc_source_file_id = "67890" + self.file.internal_status = LanguageCloudFile.STATUS_IMPORTED + importer = Importer(self.file, logging.getLogger("dummy")) + importer.import_po(self.translation, str(self.po_file)) + instance = self.translation.source.get_translated_instance( + self.translation.target_locale + ) + instance.save_revision() # save a new revision, but don't publish it yet + self.project.save() + self.file.save() + self.translation.save() + self.assertEqual(self.file.combined_status, "Translations ready for review") + def test_project_create_failed(self): self.project.create_attempts = 3 self.project.save() From d529eff5eb3bcb2bb79d23531e490b4b6e6b39f6 Mon Sep 17 00:00:00 2001 From: chris48s Date: Mon, 16 May 2022 10:10:50 +0100 Subject: [PATCH 4/4] update published_status logic --- wagtail_localize_rws_languagecloud/models.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/wagtail_localize_rws_languagecloud/models.py b/wagtail_localize_rws_languagecloud/models.py index 266e660..b34c28f 100644 --- a/wagtail_localize_rws_languagecloud/models.py +++ b/wagtail_localize_rws_languagecloud/models.py @@ -140,16 +140,19 @@ def published_status(self): """ return RECEIVED - if ( - self.revision == instance.get_latest_revision() - and not instance.has_unpublished_changes - ): + if self.revision == instance.live_revision: + # The PageRevision attached to this LanguageCloudFile is published return PUBLISHED - if (self.revision != instance.get_latest_revision()) and ( - self.revision.created_at < instance.get_latest_revision().created_at - ): - return PUBLISHED + recent_revisions = ( + instance.revisions.all() + .filter(created_at__gt=self.revision.created_at) + .order_by("-created_at", "-id") + ) + for revision in recent_revisions: + if revision == instance.live_revision: + # Some other more recent PageRevision is published + return PUBLISHED return READY_TO_REVIEW