From 0e027cf867ceca21f04ea5de80f9b8d5656cee9b Mon Sep 17 00:00:00 2001 From: Patrick Smith Date: Tue, 18 Jun 2024 16:23:31 +0100 Subject: [PATCH 1/6] Added option to machine translate on page translation sync. Working in concert with publishing option. --- .../admin/update_translations.html | 14 +++++++++ wagtail_localize/views/edit_translation.py | 14 ++++++--- wagtail_localize/views/update_translations.py | 29 +++++++++++++++++-- 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/wagtail_localize/templates/wagtail_localize/admin/update_translations.html b/wagtail_localize/templates/wagtail_localize/admin/update_translations.html index 191494903..38b15f7fc 100644 --- a/wagtail_localize/templates/wagtail_localize/admin/update_translations.html +++ b/wagtail_localize/templates/wagtail_localize/admin/update_translations.html @@ -49,6 +49,20 @@

{{ field.help_text }}

+ {% elif field.name == 'use_machine_translation'%} + {% if machine_translator %} +
  • +
    +
    +
    + + +
    +
    +
    +

    {{ field.help_text }}

    +
    • + {% endif %} {% else %}
  • {% include "wagtailadmin/shared/field.html" %}
    • {% endif %} diff --git a/wagtail_localize/views/edit_translation.py b/wagtail_localize/views/edit_translation.py index 4f2ff0231..c75cca160 100644 --- a/wagtail_localize/views/edit_translation.py +++ b/wagtail_localize/views/edit_translation.py @@ -1370,12 +1370,11 @@ def upload_pofile(request, translation_id): return redirect(next_url) -@require_POST -def machine_translate(request, translation_id): +def apply_machine_translation(translation_id, user): translation = get_object_or_404(Translation, id=translation_id) instance = translation.get_target_instance() - if not user_can_edit_instance(request.user, instance): + if not user_can_edit_instance(user, instance): raise PermissionDenied translator = get_machine_translator() @@ -1426,12 +1425,19 @@ def machine_translate(request, translation_id): "data": translations[string].data, "translation_type": StringTranslation.TRANSLATION_TYPE_MACHINE, "tool_name": translator.display_name, - "last_translated_by": request.user, + "last_translated_by": user, "has_error": False, "field_error": "", }, ) + return True + return False + +@require_POST +def machine_translate(request, translation_id): + if apply_machine_translation(translation_id, request.user): + translator = get_machine_translator() messages.success( request, _("Successfully translated with {}.").format(translator.display_name), diff --git a/wagtail_localize/views/update_translations.py b/wagtail_localize/views/update_translations.py index 09093da6d..93f99a866 100644 --- a/wagtail_localize/views/update_translations.py +++ b/wagtail_localize/views/update_translations.py @@ -17,16 +17,35 @@ from wagtail.snippets.models import get_snippet_models from wagtail.utils.version import get_main_version +from wagtail_localize.machine_translators import get_machine_translator from wagtail_localize.models import TranslationSource +from wagtail_localize.views.edit_translation import apply_machine_translation from wagtail_localize.views.submit_translations import TranslationComponentManager +if get_machine_translator(): + PUBLISH_TRANSLATIONS_HELP = ( + "Select this to publish immediately. Changes will be published " + "in the original language unless you also select use machine translation." + ) +else: + PUBLISH_TRANSLATIONS_HELP = ( + "Select this to publish immediately. Changes will be published " + "in the original language until you translate and publish them." + ) + +USE_MACHINE_TRANSLATION_HELP = "Apply machine translations to the incoming changes." + + class UpdateTranslationsForm(forms.Form): publish_translations = forms.BooleanField( label=gettext_lazy("Publish immediately"), - help_text=gettext_lazy( - "This will apply the updates and publish immediately, before any new translations happen." - ), + help_text=gettext_lazy(PUBLISH_TRANSLATIONS_HELP), + required=False, + ) + use_machine_translation = forms.BooleanField( + label=gettext_lazy("Use machine translation"), + help_text=gettext_lazy(USE_MACHINE_TRANSLATION_HELP), required=False, ) @@ -109,6 +128,7 @@ def get_context_data(self, **kwargs): enabled=True ).select_related("target_locale") ], + "machine_translator": get_machine_translator(), "last_sync_date": self.object.last_updated_at, "form": self.get_form(), "next_url": self.get_success_url(), @@ -133,6 +153,9 @@ def form_valid(self, form): self.object.update_from_db() enabled_translations = self.object.translations.filter(enabled=True) + if form.cleaned_data["use_machine_translation"]: + for translation in enabled_translations.select_related("target_locale"): + apply_machine_translation(translation.id, self.request.user) if form.cleaned_data["publish_translations"]: for translation in enabled_translations.select_related("target_locale"): From 9e9973f0a2601ee0f5024734141d188d4c2df30b Mon Sep 17 00:00:00 2001 From: Patrick Smith Date: Wed, 19 Jun 2024 12:00:14 +0100 Subject: [PATCH 2/6] Added tests for use machine translation option on syncing translated pages. --- .../tests/test_update_translations.py | 70 ++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/wagtail_localize/tests/test_update_translations.py b/wagtail_localize/tests/test_update_translations.py index c828aae00..fdc756ce1 100644 --- a/wagtail_localize/tests/test_update_translations.py +++ b/wagtail_localize/tests/test_update_translations.py @@ -10,7 +10,12 @@ from wagtail.models import Locale, Page, PageViewRestriction from wagtail.test.utils import WagtailTestUtils -from wagtail_localize.models import StringSegment, Translation, TranslationSource +from wagtail_localize.models import ( + StringSegment, + StringTranslation, + Translation, + TranslationSource, +) from wagtail_localize.test.models import NonTranslatableSnippet, TestSnippet from .utils import assert_permission_denied, make_test_page @@ -612,3 +617,66 @@ def test_post_update_page_translation_will_run_update_target_view_restrictions( ) # one call from the first post, and one from above self.assertEqual(update_target_view_restrictions.call_count, 2) + + def test_post_update_page_translation_with_publish_translations_and_use_machine_translation( + self, + ): + self.en_blog_post.test_charfield = "Edited blog post" + self.en_blog_post.save_revision().publish() + + response = self.client.post( + reverse( + "wagtail_localize:update_translations", + args=[self.page_source.id], + ), + { + "publish_translations": "on", + "use_machine_translation": "on", + }, + ) + + self.assertRedirects( + response, reverse("wagtailadmin_explore", args=[self.en_blog_index.id]) + ) + + # The FR version should be translated (Dummy translator will reverse) and updated + self.fr_blog_post.refresh_from_db() + self.assertEqual(self.fr_blog_post.test_charfield, "post blog Edited") + + def test_post_update_page_translation_with_use_machine_translation(self): + self.en_blog_post.test_charfield = "Edited blog post" + self.en_blog_post.save_revision().publish() + + response = self.client.post( + reverse( + "wagtail_localize:update_translations", + args=[self.page_source.id], + ), + { + "use_machine_translation": "on", + }, + ) + + self.assertRedirects( + response, reverse("wagtailadmin_explore", args=[self.en_blog_index.id]) + ) + + self.fr_blog_post.refresh_from_db() + self.assertEqual(self.fr_blog_post.test_charfield, "Test content") + + # Check that the translation is done, awaiting publication + fr = Locale.objects.get(language_code="fr") + translation_source = TranslationSource.objects.get_for_instance_or_none( + self.en_blog_post + ) + translation = translation_source.translations.get(target_locale=fr) + + string_segment = translation.source.stringsegment_set.get( + string__data="Edited blog post" + ) + string_translation = StringTranslation.objects.get( + translation_of_id=string_segment.string_id, + locale=fr, + context_id=string_segment.context_id, + ) + self.assertEqual(string_translation.data, "post blog Edited") From 98783eed353f743b06f02259be8fa7999eb4aa40 Mon Sep 17 00:00:00 2001 From: Patrick Smith Date: Wed, 4 Sep 2024 11:36:44 +0100 Subject: [PATCH 3/6] Updated help style as per @zerolab's feedback --- wagtail_localize/views/update_translations.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/wagtail_localize/views/update_translations.py b/wagtail_localize/views/update_translations.py index 93f99a866..bff9a7f2d 100644 --- a/wagtail_localize/views/update_translations.py +++ b/wagtail_localize/views/update_translations.py @@ -25,13 +25,14 @@ if get_machine_translator(): PUBLISH_TRANSLATIONS_HELP = ( - "Select this to publish immediately. Changes will be published " - "in the original language unless you also select use machine translation." + "Apply the updates and publish immediately. The changes will use " + "the original language until translated unless you also select " + '"Use machine translation".' ) else: PUBLISH_TRANSLATIONS_HELP = ( - "Select this to publish immediately. Changes will be published " - "in the original language until you translate and publish them." + "Apply the updates and publish immediately. The changes will use " + "the original language until translated." ) USE_MACHINE_TRANSLATION_HELP = "Apply machine translations to the incoming changes." From f09d540085933bf62959ee2bdb3fce37e490cf4b Mon Sep 17 00:00:00 2001 From: Patrick Smith Date: Wed, 4 Sep 2024 16:17:08 +0100 Subject: [PATCH 4/6] apply_machine_translations now takes a translator arg to apply --- wagtail_localize/views/edit_translation.py | 23 ++++++++++--------- wagtail_localize/views/update_translations.py | 13 ++++++++++- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/wagtail_localize/views/edit_translation.py b/wagtail_localize/views/edit_translation.py index c75cca160..b2a130767 100644 --- a/wagtail_localize/views/edit_translation.py +++ b/wagtail_localize/views/edit_translation.py @@ -1370,18 +1370,14 @@ def upload_pofile(request, translation_id): return redirect(next_url) -def apply_machine_translation(translation_id, user): +def apply_machine_translation(translation_id, user, machine_translator): translation = get_object_or_404(Translation, id=translation_id) instance = translation.get_target_instance() if not user_can_edit_instance(user, instance): raise PermissionDenied - translator = get_machine_translator() - if translator is None: - raise Http404 - - if not translator.can_translate( + if not machine_translator.can_translate( translation.source.locale, translation.target_locale ): raise Http404 @@ -1410,7 +1406,7 @@ def apply_machine_translation(translation_id, user): ) if segments: - translations = translator.translate( + translations = machine_translator.translate( translation.source.locale, translation.target_locale, segments.keys() ) @@ -1424,7 +1420,7 @@ def apply_machine_translation(translation_id, user): defaults={ "data": translations[string].data, "translation_type": StringTranslation.TRANSLATION_TYPE_MACHINE, - "tool_name": translator.display_name, + "tool_name": machine_translator.display_name, "last_translated_by": user, "has_error": False, "field_error": "", @@ -1436,11 +1432,16 @@ def apply_machine_translation(translation_id, user): @require_POST def machine_translate(request, translation_id): - if apply_machine_translation(translation_id, request.user): - translator = get_machine_translator() + machine_translator = get_machine_translator() + if machine_translator is None: + raise Http404 + + if apply_machine_translation(translation_id, request.user, machine_translator): messages.success( request, - _("Successfully translated with {}.").format(translator.display_name), + _("Successfully translated with {}.").format( + machine_translator.display_name + ), ) else: diff --git a/wagtail_localize/views/update_translations.py b/wagtail_localize/views/update_translations.py index bff9a7f2d..c105fefc4 100644 --- a/wagtail_localize/views/update_translations.py +++ b/wagtail_localize/views/update_translations.py @@ -50,6 +50,14 @@ class UpdateTranslationsForm(forms.Form): required=False, ) + def clean(self): + cleaned_data = super().clean() + if ( + cleaned_data.get("use_machine_translation") + and get_machine_translator() is None + ): + raise ValidationError(_("A machine translator could not be found.")) + class UpdateTranslationsView(SingleObjectMixin, TemplateView): template_name = "wagtail_localize/admin/update_translations.html" @@ -155,8 +163,11 @@ def form_valid(self, form): enabled_translations = self.object.translations.filter(enabled=True) if form.cleaned_data["use_machine_translation"]: + machine_translator = get_machine_translator() for translation in enabled_translations.select_related("target_locale"): - apply_machine_translation(translation.id, self.request.user) + apply_machine_translation( + translation.id, self.request.user, machine_translator + ) if form.cleaned_data["publish_translations"]: for translation in enabled_translations.select_related("target_locale"): From ba487d65260dbd67619d239a6018677afdc40c08 Mon Sep 17 00:00:00 2001 From: Patrick Smith Date: Wed, 4 Sep 2024 16:37:04 +0100 Subject: [PATCH 5/6] configure form fields on init --- .../admin/update_translations.html | 14 -------------- wagtail_localize/views/update_translations.py | 15 +++++++++------ 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/wagtail_localize/templates/wagtail_localize/admin/update_translations.html b/wagtail_localize/templates/wagtail_localize/admin/update_translations.html index 38b15f7fc..191494903 100644 --- a/wagtail_localize/templates/wagtail_localize/admin/update_translations.html +++ b/wagtail_localize/templates/wagtail_localize/admin/update_translations.html @@ -49,20 +49,6 @@

    {{ field.help_text }}

    - {% elif field.name == 'use_machine_translation'%} - {% if machine_translator %} -
  • -
    -
    -
    - - -
    -
    -
    -

    {{ field.help_text }}

    -
    • - {% endif %} {% else %}
  • {% include "wagtailadmin/shared/field.html" %}
    • {% endif %} diff --git a/wagtail_localize/views/update_translations.py b/wagtail_localize/views/update_translations.py index c105fefc4..b427187cf 100644 --- a/wagtail_localize/views/update_translations.py +++ b/wagtail_localize/views/update_translations.py @@ -23,7 +23,9 @@ from wagtail_localize.views.submit_translations import TranslationComponentManager -if get_machine_translator(): +HAS_MACHINE_TRANSLATOR = get_machine_translator() is not None + +if HAS_MACHINE_TRANSLATOR: PUBLISH_TRANSLATIONS_HELP = ( "Apply the updates and publish immediately. The changes will use " "the original language until translated unless you also select " @@ -50,12 +52,14 @@ class UpdateTranslationsForm(forms.Form): required=False, ) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + if not HAS_MACHINE_TRANSLATOR: + self.fields["use_machine_translation"].widget = forms.HiddenInput() + def clean(self): cleaned_data = super().clean() - if ( - cleaned_data.get("use_machine_translation") - and get_machine_translator() is None - ): + if cleaned_data.get("use_machine_translation") and not HAS_MACHINE_TRANSLATOR: raise ValidationError(_("A machine translator could not be found.")) @@ -137,7 +141,6 @@ def get_context_data(self, **kwargs): enabled=True ).select_related("target_locale") ], - "machine_translator": get_machine_translator(), "last_sync_date": self.object.last_updated_at, "form": self.get_form(), "next_url": self.get_success_url(), From bede4220e3629c2ec7dd77919aa20a3fdc5a596e Mon Sep 17 00:00:00 2001 From: Patrick Smith Date: Thu, 5 Sep 2024 10:58:54 +0100 Subject: [PATCH 6/6] added tests for form state with and without macine translator --- .../tests/test_update_translations.py | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/wagtail_localize/tests/test_update_translations.py b/wagtail_localize/tests/test_update_translations.py index fdc756ce1..98a2c2668 100644 --- a/wagtail_localize/tests/test_update_translations.py +++ b/wagtail_localize/tests/test_update_translations.py @@ -5,6 +5,7 @@ from django.contrib.auth.models import Group, Permission from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError +from django.forms.widgets import CheckboxInput, HiddenInput from django.test import TestCase, override_settings from django.urls import reverse from wagtail.models import Locale, Page, PageViewRestriction @@ -618,6 +619,39 @@ def test_post_update_page_translation_will_run_update_target_view_restrictions( # one call from the first post, and one from above self.assertEqual(update_target_view_restrictions.call_count, 2) + @mock.patch( + "wagtail_localize.views.update_translations.HAS_MACHINE_TRANSLATOR", False + ) + def test_update_translations_form_without_machine_translator(self): + response = self.client.get( + reverse( + "wagtail_localize:update_translations", + args=[self.snippet_source.id], + ) + ) + + self.assertEqual(response.status_code, 200) + + self.assertIsInstance( + response.context["form"].fields["use_machine_translation"].widget, + HiddenInput, + ) + + def test_update_translations_form_with_machine_translator(self): + response = self.client.get( + reverse( + "wagtail_localize:update_translations", + args=[self.snippet_source.id], + ) + ) + + self.assertEqual(response.status_code, 200) + + self.assertIsInstance( + response.context["form"].fields["use_machine_translation"].widget, + CheckboxInput, + ) + def test_post_update_page_translation_with_publish_translations_and_use_machine_translation( self, ):