chore(autotranslate): code cleanups

- avoid using kwargs for class parameters, instead pass them directly
- directly filter the queryset instead of building filters and applying
  them later
- improved error handling in the form

Author: nijel

weblate/trans/autotranslate.py

@@ -14,9 +14,7 @@
 from django.db.models.functions import MD5, Lower
 from django.utils.translation import gettext, ngettext
 
-from weblate.machinery.base import (
-    MachineTranslationError,
-)
+from weblate.machinery.base import MachineTranslationError
 from weblate.machinery.models import MACHINERY
 from weblate.trans.actions import ActionEvents
 from weblate.trans.models import Category, Component, Suggestion, Translation, Unit
@@ -30,16 +28,11 @@
 from weblate.utils.stats import ProjectLanguage
 
 if TYPE_CHECKING:
-    from collections.abc import Iterable
+    from collections.abc import Sequence
 
     from weblate.auth.models import User
-    from weblate.machinery.base import (
-        BatchMachineTranslation,
-        UnitMemoryResultDict,
-    )
-    from weblate.utils.state import (
-        StringState,
-    )
+    from weblate.machinery.base import BatchMachineTranslation, UnitMemoryResultDict
+    from weblate.utils.state import StringState
 
 
 class BaseAutoTranslate:
@@ -54,7 +47,6 @@ def __init__(
         mode: str,
         component_wide: bool = False,
         unit_ids: list[int] | None = None,
-        **kwargs,
     ) -> None:
         self.user: User | None = user
         self.q: str = q
@@ -89,8 +81,19 @@ def set_progress(self, current: int) -> None:
 
 
 class AutoTranslate(BaseAutoTranslate):
-    def __init__(self, *, translation: Translation, **kwargs) -> None:
-        super().__init__(**kwargs)
+    def __init__(
+        self,
+        *,
+        translation: Translation,
+        user: User | None,
+        q: str,
+        mode: str,
+        component_wide: bool = False,
+        unit_ids: list[int] | None = None,
+    ) -> None:
+        super().__init__(
+            user=user, q=q, mode=mode, component_wide=component_wide, unit_ids=unit_ids
+        )
         self.translation: Translation = translation
         translation.component.start_batched_checks()
         self.progress_base = 0
@@ -151,12 +154,11 @@ def post_process(self) -> None:
     @transaction.atomic
     def process_others(self, source_component_id: int | None) -> None:
         """Perform automatic translation based on other components."""
-        kwargs = {
-            "translation__plural": self.translation.plural,
-            "state__gte": STATE_TRANSLATED,
-        }
+        sources = Unit.objects.filter(
+            translation__plural=self.translation.plural,
+            state__gte=STATE_TRANSLATED,
+        )
         source_language = self.translation.component.source_language
-        exclude = {}
         if source_component_id:
             component = Component.objects.get(id=source_component_id)
 
@@ -169,23 +171,20 @@ def process_others(self, source_component_id: int | None) -> None:
             if component.source_language != source_language:
                 msg = "Component have different source languages."
                 raise PermissionDenied(msg)
-            kwargs["translation__component"] = component
+            sources = sources.filter(translation__component=component)
         else:
             project = self.translation.component.project
-            kwargs["translation__component__project"] = project
-            kwargs["translation__component__source_language"] = source_language
-            exclude["translation"] = self.translation
-        sources = Unit.objects.filter(**kwargs)
+            sources = sources.filter(
+                translation__component__project=project,
+                translation__component__source_language=source_language,
+            ).exclude(translation=self.translation)
 
         # Use memory_db for the query in case it exists. This is supposed
         # to be a read-only replica for offloading expensive translation
         # queries.
         if "memory_db" in settings.DATABASES:
             sources = sources.using("memory_db")
 
-        if exclude:
-            sources = sources.exclude(**exclude)
-
         # Get source MD5s
         source_md5s = list(
             self.get_units()
@@ -356,7 +355,7 @@ def perform(
 
 
 class BatchAutoTranslate(BaseAutoTranslate):
-    translations: Iterable[Translation]
+    translations: Sequence[Translation]
 
     def __init__(
         self,

weblate/trans/forms.py

@@ -1113,7 +1113,7 @@ def clean_component(self):
                 raise ValidationError(gettext("Component not found!"))
             try:
                 result = self.components.get(slug=component, project=self.project)
-            except Component.DoesNotExist as error:
+            except (Component.DoesNotExist, Component.MultipleObjectsReturned) as error:
                 raise ValidationError(gettext("Component not found!")) from error
         else:
             try:

weblate/trans/tasks.py

@@ -17,11 +17,9 @@
 from django.conf import settings
 from django.contrib.staticfiles.storage import staticfiles_storage
 from django.core.cache import cache
+from django.core.exceptions import PermissionDenied
 from django.db import IntegrityError, transaction
-from django.db.models import (
-    Count,
-    F,
-)
+from django.db.models import Count, F
 from django.http import Http404
 from django.utils import timezone
 from django.utils.timezone import make_aware
@@ -572,13 +570,17 @@ def auto_translate(  # noqa: PLR0913
             component_wide=component_wide,
             unit_ids=unit_ids,
         )
-        message = auto.perform(
-            auto_source=auto_source,
-            engines=engines,
-            threshold=threshold,
-            source_component_id=source_component_id,
-        )
-        result.update({"message": message})
+        try:
+            message = auto.perform(
+                auto_source=auto_source,
+                engines=engines,
+                threshold=threshold,
+                source_component_id=source_component_id,
+            )
+        except PermissionDenied as error:
+            result.update({"message": str(error)})
+        else:
+            result.update({"message": message})
         return result
 
 

weblate/trans/tests/test_autotranslate.py

@@ -11,25 +11,30 @@
 
 from weblate.lang.models import Language
 from weblate.trans.actions import ActionEvents
-from weblate.trans.models import Change, Component, PendingUnitChange
+from weblate.trans.models import Change, Component, PendingUnitChange, Project
 from weblate.trans.tasks import auto_translate
 from weblate.trans.tests.test_views import ViewTestCase
 from weblate.utils.stats import ProjectLanguage
 
 
 class AutoTranslationTest(ViewTestCase):
+    use_component_id: bool = False
+
     def setUp(self) -> None:
         super().setUp()
         # Need extra power
         self.user.is_superuser = True
         self.user.save()
         self.project.translation_review = True
         self.project.save()
+        self.component2 = self.create_second_component()
+
+    def create_second_component(self, project: Project | None = None) -> Component:
         with override_settings(CREATE_GLOSSARIES=self.CREATE_GLOSSARIES):
-            self.component2 = Component.objects.create(
+            return Component.objects.create(
                 name="Test 2",
                 slug="test-2",
-                project=self.project,
+                project=self.project if project is None else project,
                 repo=self.git_repo_path,
                 push=self.git_repo_path,
                 vcs="git",
@@ -63,6 +68,8 @@ def perform_auto(
             kwargs["q"] = "state:<translated"
         if "mode" not in kwargs:
             kwargs["mode"] = "translate"
+        if self.use_component_id:
+            kwargs["component"] = self.component.id
         response = self.client.post(url, kwargs, follow=True)
         if expected == 0:
             expected_string = (
@@ -128,11 +135,16 @@ def test_autotranslate_component(self) -> None:
         self.assertEqual(de_translation.stats.translated, initial_stats + 1)
 
     def test_autotranslate_category(self) -> None:
-        category = self.create_category(project=self.project)
-        self.component.category = self.component2.category = category
+        self.component.category = self.create_category(project=self.project)
+        category = self.component.category
+        if self.component2.project != self.project:
+            category = self.create_category(project=self.component2.project)
+        self.component2.category = category
         self.component.save()
         self.component2.save()
+
         self.make_different("de")
+
         self.perform_auto(
             path_params={"path": category.get_url_path()},
             expected=2,
@@ -141,7 +153,7 @@ def test_autotranslate_category(self) -> None:
 
     def test_autotranslate_project_language(self) -> None:
         project_language = ProjectLanguage(
-            self.project,
+            self.component2.project,
             language=Language.objects.get(code="cs"),
         )
         self.make_different("de")
@@ -307,7 +319,13 @@ def test_command_add(self) -> None:
 
     def test_command_different(self) -> None:
         self.make_different()
-        call_command("auto_translate", "test", "test-2", "cs", source="test/test")
+        call_command(
+            "auto_translate",
+            self.component2.project.slug,
+            self.component2.slug,
+            "cs",
+            source=self.component.full_slug,
+        )
 
     def test_command_errors(self) -> None:
         with self.assertRaises(CommandError):
@@ -320,6 +338,16 @@ def test_command_errors(self) -> None:
             call_command("auto_translate", "test", "test", "xxx")
 
 
+class AutoTranslationCrossProjectTest(AutoTranslationTest):
+    use_component_id: bool = True
+
+    def create_second_component(self, project: Project | None = None) -> Component:
+        project = Project.objects.create(
+            name="Other", slug="other", translation_review=True
+        )
+        return super().create_second_component(project=project)
+
+
 class AutoTranslationMtTest(ViewTestCase):
     def setUp(self) -> None:
         super().setUp()