Component: Consolidate commit code

Reuse the same commit code in component, translation and addons. This
makes the commit behave consistently (for example it properly pushes and
sends signals).

Fixes #3708
Fixes #3707

weblate/addons/base.py

@@ -239,15 +239,6 @@ def trigger_alerts(self, component):
         else:
             component.delete_alert(self.alert)
 
-    def get_commit_message(self, component):
-        return render_template(
-            component.addon_message,
-            # Compatibility with older
-            hook_name=self.verbose,
-            addon_name=self.verbose,
-            component=component,
-        )
-
     def commit_and_push(self, component, files=None):
         if files is None:
             files = list(
@@ -259,9 +250,11 @@ def commit_and_push(self, component, files=None):
             files += self.extra_files
         repository = component.repository
         with repository.lock:
-            if repository.needs_commit():
-                repository.commit(self.get_commit_message(component), files=files)
-                component.push_if_needed(None)
+            component.commit_files(
+                template=component.addon_message,
+                extra_context={"addon_name": self.verbose},
+                files=files,
+            )
 
     def render_repo_filename(self, template, translation):
         component = translation.component

weblate/addons/git.py

@@ -216,5 +216,8 @@ def post_commit(self, component):
             method(component, repository)
             # Commit any left files, those were most likely generated
             # by addon and do not exactly match patterns above
-            if repository.needs_commit():
-                repository.commit(self.get_commit_message(component))
+            component.commit_files(
+                template=component.addon_message,
+                extra_context={"addon_name": self.verbose},
+                signals=False,
+            )

weblate/trans/models/component.py

@@ -25,9 +25,10 @@
 from collections import Counter
 from contextlib import contextmanager
 from copy import copy
+from datetime import datetime
 from glob import glob
 from itertools import chain
-from typing import List, Optional
+from typing import Any, Dict, List, Optional
 from urllib.parse import urlparse
 
 from celery import current_task
@@ -38,7 +39,6 @@
 from django.db import models, transaction
 from django.db.models import Count, Q
 from django.urls import reverse
-from django.utils import timezone
 from django.utils.encoding import force_str
 from django.utils.functional import cached_property
 from django.utils.translation import gettext as _
@@ -1392,6 +1392,42 @@ def commit_pending(self, reason, user, skip_push=False):
 
         return True
 
+    def commit_files(
+        self,
+        template: str,
+        author: Optional[str] = None,
+        timestamp: Optional[datetime] = None,
+        files: Optional[List[str]] = None,
+        signals: bool = True,
+        skip_push: bool = False,
+        extra_context: Optional[Dict[str, Any]] = None,
+    ):
+        """Commits files to the repository."""
+        # Is there something to commit?
+        if not self.repository.needs_commit(*files or []):
+            return False
+
+        # Handle context
+        context = {"component": self, "author": author}
+        if extra_context:
+            context.update(extra_context)
+
+        # Generate commit message
+        message = render_template(template, **context)
+
+        # Actual commit
+        self.repository.commit(message, author, timestamp, files)
+
+        # Send post commit signal
+        if signals:
+            vcs_post_commit.send(sender=self.__class__, component=self)
+
+        # Push if we should
+        if not skip_push:
+            self.push_if_needed()
+
+        return True
+
     def handle_parse_error(self, error, translation=None):
         """Handler for parse errors."""
         error_message = getattr(error, "strerror", "")
@@ -2119,18 +2155,19 @@ def create_template_if_missing(self):
             fullname, self.project.source_language, self.get_new_base_filename()
         )
 
-        message = render_template(
-            self.add_message,
-            translation=Translation(
-                filename=self.template,
-                language_code=self.project.source_language.code,
-                language=self.project.source_language,
-                component=self,
-            ),
-        )
         with self.repository.lock:
-            self.repository.commit(
-                message, "Weblate <noreply@weblate.org>", timezone.now(), [fullname]
+            self.commit_files(
+                template=self.add_message,
+                author="Weblate <noreply@weblate.org>",
+                extra_context={
+                    "translation": Translation(
+                        filename=self.template,
+                        language_code=self.project.source_language.code,
+                        language=self.project.source_language,
+                        component=self,
+                    )
+                },
+                files=[fullname],
             )
 
     def after_save(
@@ -2443,13 +2480,12 @@ def add_new_language(self, language, request, send_signal=True):
                     sender=self.__class__, translation=translation
                 )
             translation.check_sync(force=True, request=request)
-            translation.commit_template = "add"
             translation.git_commit(
                 request.user if request else None,
                 request.user.get_author_name()
                 if request
                 else "Weblate <noreply@weblate.org>",
-                timezone.now(),
+                template=self.add_message,
             )
             self.update_source_checks()
             translation.invalidate_cache()

weblate/trans/models/translation.py

@@ -20,6 +20,7 @@
 import codecs
 import os
 import tempfile
+from datetime import datetime
 from typing import BinaryIO, Optional
 
 from django.core.cache import cache
@@ -49,7 +50,7 @@
     STATE_TRANSLATED,
     Unit,
 )
-from weblate.trans.signals import store_post_load, vcs_post_commit, vcs_pre_commit
+from weblate.trans.signals import store_post_load, vcs_pre_commit
 from weblate.trans.util import split_plural
 from weblate.trans.validators import validate_check_flags
 from weblate.utils.errors import report_error
@@ -144,7 +145,6 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.stats = TranslationStats(self)
         self.addon_commit_files = []
-        self.commit_template = ""
         self.was_new = 0
         self.reason = ""
 
@@ -518,45 +518,10 @@ def commit_pending(self, reason, user, skip_push=False, force=False, signals=Tru
 
         return True
 
-    def get_commit_message(self, author, template=None, **kwargs):
+    def get_commit_message(self, author: str, template: str, **kwargs):
         """Format commit message based on project configuration."""
-        if template is None:
-            if self.commit_template == "add":
-                template = self.component.add_message
-                self.commit_template = ""
-            elif self.commit_template == "delete":
-                template = self.component.delete_message
-                self.commit_template = ""
-            else:
-                template = self.component.commit_message
-
         return render_template(template, translation=self, author=author, **kwargs)
 
-    def __git_commit(self, author, timestamp, signals=True):
-        """Commit translation to git."""
-        # Format commit message
-        msg = self.get_commit_message(author)
-
-        # Pre commit hook
-        vcs_pre_commit.send(sender=self.__class__, translation=self, author=author)
-
-        # Create list of files to commit
-        files = self.filenames
-
-        # Do actual commit
-        if self.repo_needs_commit():
-            self.component.repository.commit(
-                msg, author, timestamp, files + self.addon_commit_files
-            )
-        self.addon_commit_files = []
-
-        # Post commit hook
-        if signals:
-            vcs_post_commit.send(sender=self.__class__, component=self.component)
-
-        # Store updated hash
-        self.store_hash()
-
     def needs_commit(self):
         """Check whether there are some not committed changes."""
         return self.unit_set.filter(pending=True).exists()
@@ -578,24 +543,41 @@ def filenames(self):
     def repo_needs_commit(self):
         return self.component.repository.needs_commit(*self.filenames)
 
-    def git_commit(self, user, author, timestamp, skip_push=False, signals=True):
+    def git_commit(
+        self,
+        user,
+        author: str,
+        timestamp: Optional[datetime] = None,
+        skip_push=False,
+        signals=True,
+        template: Optional[str] = None,
+    ):
         """Wrapper for committing translation to git."""
         repository = self.component.repository
+        if template is None:
+            template = self.component.commit_message
         with repository.lock:
-            # Is there something for commit?
-            if not self.repo_needs_commit():
-                return False
+            # Pre commit hook
+            vcs_pre_commit.send(sender=self.__class__, translation=self, author=author)
 
             # Do actual commit with git lock
-            self.log_info("committing %s as %s", self.filenames, author)
-            Change.objects.create(
-                action=Change.ACTION_COMMIT, translation=self, user=user
-            )
-            self.__git_commit(author, timestamp, signals=signals)
+            if not self.component.commit_files(
+                template=template,
+                author=author,
+                timestamp=timestamp,
+                skip_push=skip_push,
+                signals=signals,
+                files=self.filenames + self.addon_commit_files,
+                extra_context={"translation": self},
+            ):
+                self.log_info("committed %s as %s", self.filenames, author)
+                Change.objects.create(
+                    action=Change.ACTION_COMMIT, translation=self, user=user
+                )
 
-            # Push if we should
-            if not skip_push:
-                self.component.push_if_needed()
+            # Store updated hash
+            self.store_hash()
+            self.addon_commit_files = []
 
         return True
 
@@ -894,17 +876,13 @@ def handle_source(self, request, fileobj):
                         os.unlink(temp.name)
 
             # Commit changes
-            if component.repository.needs_commit(*filenames):
-                component.repository.commit(
-                    self.get_commit_message(
-                        request.user.get_author_name(),
-                        template=component.addon_message,
-                        addon_name="Source update",
-                    ),
-                    files=filenames,
-                )
+            if component.commit_files(
+                template=component.addon_message,
+                files=filenames,
+                author=request.user.get_author_name(),
+                extra_context={"addon_name": "Source update"},
+            ):
                 component.create_translations(request=request, force=True)
-                component.push_if_needed(None)
         return (0, 0, self.unit_set.count(), self.unit_set.count())
 
     def handle_replace(self, request, fileobj):
@@ -924,8 +902,7 @@ def handle_replace(self, request, fileobj):
             )
 
             # Commit to VCS
-            if self.repo_needs_commit():
-                self.__git_commit(request.user.get_author_name(), timezone.now())
+            if self.git_commit(request.user, request.user.get_author_name()):
 
                 # Drop store cache
                 self.drop_store_cache()
@@ -1040,10 +1017,13 @@ def remove(self, user):
 
         # Remove file from VCS
         if any((os.path.exists(name) for name in self.filenames)):
-            self.commit_template = "delete"
             with self.component.repository.lock:
                 self.component.repository.remove(
-                    self.filenames, self.get_commit_message(author), author
+                    self.filenames,
+                    self.get_commit_message(
+                        author, template=self.component.delete_message
+                    ),
+                    author,
                 )
 
         # Delete from the database
@@ -1071,8 +1051,7 @@ def new_unit(self, request, key, value):
             )
             self.store.new_unit(key, value)
             self.component.create_translations(request=request)
-            self.__git_commit(request.user.get_author_name(), timezone.now())
-            self.component.push_if_needed()
+            self.git_commit(request.user, request.user.get_author_name())
 
 
 class GhostTranslation:

weblate/trans/tests/test_remote.py

@@ -23,7 +23,6 @@
 from unittest import SkipTest
 
 from django.db import transaction
-from django.utils import timezone
 
 from weblate.trans.models import Component
 from weblate.trans.tests.test_views import ViewTestCase
@@ -111,9 +110,7 @@ def push_replace(self, content, mode):
 
         # Do changes in first repo
         with transaction.atomic():
-            translation.git_commit(
-                self.request.user, "TEST <test@example.net>", timezone.now()
-            )
+            translation.git_commit(self.request.user, "TEST <test@example.net>")
         self.assertFalse(translation.needs_commit())
         translation.component.do_push(self.request)