Edit: Separate getting unit from actions

The checksum was necessary in the actions in times we used it to
reference unitdata. This is now gone and there is no reason to include
it there.

weblate/trans/forms.py

@@ -389,16 +389,16 @@ class ChecksumForm(forms.Form):
 
     checksum = ChecksumField(required=True)
 
-    def __init__(self, translation, *args, **kwargs):
-        self.translation = translation
+    def __init__(self, unit_set, *args, **kwargs):
+        self.unit_set = unit_set
         super().__init__(*args, **kwargs)
 
     def clean_checksum(self):
         """Validate whether checksum is valid and fetches unit for it."""
         if "checksum" not in self.cleaned_data:
             return
 
-        unit_set = self.translation.unit_set
+        unit_set = self.unit_set
 
         try:
             self.cleaned_data["unit"] = unit_set.filter(
@@ -410,6 +410,12 @@ def clean_checksum(self):
             )
 
 
+class UnitForm(forms.Form):
+    def __init__(self, unit: Unit, *args, **kwargs):
+        self.unit = unit
+        super().__init__(*args, **kwargs)
+
+
 class FuzzyField(forms.BooleanField):
     help_as_icon = True
 
@@ -423,7 +429,7 @@ def __init__(self, *args, **kwargs):
         self.widget.attrs["class"] = "fuzzy_checkbox"
 
 
-class TranslationForm(ChecksumForm):
+class TranslationForm(UnitForm):
     """Form used for translation of single string."""
 
     contentsum = ChecksumField(required=True)
@@ -441,10 +447,9 @@ class TranslationForm(ChecksumForm):
         widget=forms.RadioSelect,
     )
 
-    def __init__(self, user, translation, unit, *args, **kwargs):
+    def __init__(self, user, unit: Unit, *args, **kwargs):
         if unit is not None:
             kwargs["initial"] = {
-                "checksum": unit.checksum,
                 "contentsum": hash_to_checksum(unit.content_hash),
                 "translationsum": hash_to_checksum(unit.get_target_hash()),
                 "target": unit,
@@ -453,7 +458,7 @@ def __init__(self, user, translation, unit, *args, **kwargs):
             }
             kwargs["auto_id"] = "id_{0}_%s".format(unit.checksum)
         tabindex = kwargs.pop("tabindex", 100)
-        super().__init__(translation, *args, **kwargs)
+        super().__init__(unit, *args, **kwargs)
         self.user = user
         self.fields["target"].widget.attrs["tabindex"] = tabindex
         self.fields["target"].widget.profile = user.profile
@@ -466,7 +471,6 @@ def __init__(self, user, translation, unit, *args, **kwargs):
         self.helper.form_tag = False
         self.helper.disable_csrf = True
         self.helper.layout = Layout(
-            Field("checksum"),
             Field("target"),
             Field("fuzzy"),
             Field("contentsum"),
@@ -482,11 +486,11 @@ def clean(self):
         super().clean()
 
         # Check required fields
-        required = {"unit", "target", "contentsum", "translationsum"}
+        required = {"target", "contentsum", "translationsum"}
         if not required.issubset(self.cleaned_data):
             return
 
-        unit = self.cleaned_data["unit"]
+        unit = self.unit
 
         if self.cleaned_data["contentsum"] != unit.content_hash:
             raise ValidationError(
@@ -519,10 +523,10 @@ def clean(self):
 
 
 class ZenTranslationForm(TranslationForm):
-    def __init__(self, user, translation, unit, *args, **kwargs):
-        super().__init__(user, translation, unit, *args, **kwargs)
+    def __init__(self, user, unit, *args, **kwargs):
+        super().__init__(user, unit, *args, **kwargs)
         self.helper.form_action = reverse(
-            "save_zen", kwargs=translation.get_reverse_url_kwargs()
+            "save_zen", kwargs=unit.translation.get_reverse_url_kwargs()
         )
         self.helper.form_tag = True
         self.helper.disable_csrf = False
@@ -698,7 +702,7 @@ def clean_offset(self):
     def items(self):
         items = []
         # Skip checksum and offset as these change
-        ignored = {"checksum", "offset"}
+        ignored = {"offset"}
         for param in sorted(self.cleaned_data):
             value = self.cleaned_data[param]
             # We don't care about empty values or ignored
@@ -733,7 +737,6 @@ def reset_offset(self):
         """Reset offset to avoid using form as default for new search."""
         data = copy.copy(self.data)
         data["offset"] = "1"
-        data["checksum"] = ""
         self.data = data
         return self
 
@@ -743,46 +746,46 @@ class PositionSearchForm(SearchForm):
     offset_kwargs = {"template": "snippets/position-field.html"}
 
 
-class MergeForm(ChecksumForm):
+class MergeForm(UnitForm):
     """Simple form for merging translation of two units."""
 
     merge = forms.IntegerField()
 
     def clean(self):
         super().clean()
-        if "unit" not in self.cleaned_data or "merge" not in self.cleaned_data:
+        if "merge" not in self.cleaned_data:
             return None
         try:
-            project = self.translation.component.project
+            unit = self.unit
+            translation = unit.translation
+            project = translation.component.project
             self.cleaned_data["merge_unit"] = merge_unit = Unit.objects.get(
                 pk=self.cleaned_data["merge"],
                 translation__component__project=project,
-                translation__language=self.translation.language,
+                translation__language=translation.language,
+                id_hash=unit.id_hash,
+                content_hash=unit.content_hash,
             )
-            unit = self.cleaned_data["unit"]
-            if (
-                unit.id_hash != merge_unit.id_hash
-                and unit.content_hash != merge_unit.content_hash
-                and unit.source != merge_unit.source
-            ):
+            # Compare in Python to ensure case sensitiveness on MySQL
+            if unit.source != merge_unit.source:
                 raise ValidationError(_("Could not find merged string."))
         except Unit.DoesNotExist:
             raise ValidationError(_("Could not find merged string."))
         return self.cleaned_data
 
 
-class RevertForm(ChecksumForm):
+class RevertForm(UnitForm):
     """Form for reverting edits."""
 
     revert = forms.IntegerField()
 
     def clean(self):
         super().clean()
-        if "unit" not in self.cleaned_data or "revert" not in self.cleaned_data:
+        if "revert" not in self.cleaned_data:
             return None
         try:
             self.cleaned_data["revert_change"] = Change.objects.get(
-                pk=self.cleaned_data["revert"], unit=self.cleaned_data["unit"]
+                pk=self.cleaned_data["revert"], unit=self.unit,
             )
         except Change.DoesNotExist:
             raise ValidationError(_("Could not find reverted change."))

weblate/trans/tests/test_edit.py

@@ -85,8 +85,8 @@ def test_plurals(self):
             target_1="Opice má %d banány.\n",
             target_2="Opice má %d banánů.\n",
         )
-        # We should get to second message
-        self.assert_redirects_offset(response, self.translate_url, 2)
+        # We should get to next message
+        self.assert_redirects_offset(response, self.translate_url, 3)
         # Check translations
         unit = self.get_unit("Orangutan")
         plurals = unit.get_target_plurals()

weblate/trans/views/edit.py

@@ -25,7 +25,12 @@
 from django.contrib.messages import get_messages
 from django.core.exceptions import PermissionDenied
 from django.db.models import Q
-from django.http import HttpResponse, HttpResponseRedirect, JsonResponse
+from django.http import (
+    HttpResponse,
+    HttpResponseBadRequest,
+    HttpResponseRedirect,
+    JsonResponse,
+)
 from django.shortcuts import get_object_or_404, redirect
 from django.utils.encoding import force_str
 from django.utils.http import urlencode
@@ -41,6 +46,7 @@
 from weblate.trans.forms import (
     AntispamForm,
     AutoForm,
+    ChecksumForm,
     CommentForm,
     ContextForm,
     MergeForm,
@@ -125,7 +131,6 @@ def search(base, unit_set, request, form_class=SearchForm):
     search_result = {
         "form": form,
         "offset": form.cleaned_data.get("offset", 1),
-        "checksum": form.cleaned_data.get("checksum"),
     }
     search_url = form.urlencode()
     session_key = "search_{0}_{1}".format(base.cache_key, search_url)
@@ -252,20 +257,19 @@ def perform_translation(unit, form, request):
 
 
 @session_ratelimit_post("translate")
-def handle_translate(request, translation, this_unit_url, next_unit_url):
+def handle_translate(request, unit, this_unit_url, next_unit_url):
     """Save translation or suggestion to database and backend."""
     # Antispam protection
     antispam = AntispamForm(request.POST)
     if not antispam.is_valid():
         # Silently redirect to next entry
         return HttpResponseRedirect(next_unit_url)
 
-    form = TranslationForm(request.user, translation, None, request.POST)
+    form = TranslationForm(request.user, unit, request.POST)
     if not form.is_valid():
         show_form_errors(request, form)
         return None
 
-    unit = form.cleaned_data["unit"]
     go_next = True
 
     if "suggest" in request.POST:
@@ -281,14 +285,13 @@ def handle_translate(request, translation, this_unit_url, next_unit_url):
     return HttpResponseRedirect(this_unit_url)
 
 
-def handle_merge(translation, request, next_unit_url):
+def handle_merge(unit, request, next_unit_url):
     """Handle unit merging."""
-    mergeform = MergeForm(translation, request.GET)
+    mergeform = MergeForm(unit, request.GET)
     if not mergeform.is_valid():
         messages.error(request, _("Invalid merge request!"))
         return None
 
-    unit = mergeform.cleaned_data["unit"]
     merged = mergeform.cleaned_data["merge_unit"]
 
     if not request.user.has_perm("unit.edit", unit):
@@ -301,13 +304,12 @@ def handle_merge(translation, request, next_unit_url):
     return HttpResponseRedirect(next_unit_url)
 
 
-def handle_revert(translation, request, next_unit_url):
-    revertform = RevertForm(translation, request.GET)
+def handle_revert(unit, request, next_unit_url):
+    revertform = RevertForm(unit, request.GET)
     if not revertform.is_valid():
         messages.error(request, _("Invalid revert request!"))
         return None
 
-    unit = revertform.cleaned_data["unit"]
     change = revertform.cleaned_data["revert_change"]
 
     if not request.user.has_perm("unit.edit", unit):
@@ -417,11 +419,12 @@ def translate(request, project, component, lang):
     offset = search_result["offset"]
 
     # Checksum unit access
-    if search_result["checksum"]:
+    checksum_form = ChecksumForm(unit_set, request.GET or request.POST)
+    if checksum_form.is_valid():
+        unit = checksum_form.cleaned_data["unit"]
         try:
-            unit = unit_set.get(id_hash=search_result["checksum"])
             offset = search_result["ids"].index(unit.id) + 1
-        except (Unit.DoesNotExist, ValueError):
+        except ValueError:
             messages.warning(request, _("No string matched your search!"))
             return redirect(translation)
     else:
@@ -464,9 +467,7 @@ def translate(request, project, component, lang):
             and "downvote" not in request.POST
         ):
             # Handle translation
-            response = handle_translate(
-                request, translation, this_unit_url, next_unit_url
-            )
+            response = handle_translate(request, unit, this_unit_url, next_unit_url)
         elif not locked or "delete" in request.POST or "spam" in request.POST:
             # Handle accepting/deleting suggestions
             response = handle_suggestions(
@@ -475,11 +476,11 @@ def translate(request, project, component, lang):
 
     # Handle translation merging
     elif "merge" in request.GET and not locked:
-        response = handle_merge(translation, request, next_unit_url)
+        response = handle_merge(unit, request, next_unit_url)
 
     # Handle reverting
     elif "revert" in request.GET and not locked:
-        response = handle_revert(translation, request, this_unit_url)
+        response = handle_revert(unit, request, this_unit_url)
 
     # Pass possible redirect further
     if response is not None:
@@ -495,7 +496,7 @@ def translate(request, project, component, lang):
     antispam = AntispamForm()
 
     # Prepare form
-    form = TranslationForm(request.user, translation, unit)
+    form = TranslationForm(request.user, unit)
     sort = get_sort_name(request)
 
     return render(
@@ -688,7 +689,7 @@ def get_zen_unitdata(translation, request):
                 else None
             ),
             "form": ZenTranslationForm(
-                request.user, translation, unit, tabindex=100 + (unit.position * 10)
+                request.user, unit, tabindex=100 + (unit.position * 10)
             ),
             "offset": offset + pos + 1,
         }
@@ -755,17 +756,22 @@ def load_zen(request, project, component, lang):
 def save_zen(request, project, component, lang):
     """Save handler for zen mode."""
     translation = get_translation(request, project, component, lang)
+    unit_set = translation.unit_set
+
+    checksum_form = ChecksumForm(unit_set, request.POST)
+    if not checksum_form.is_valid():
+        show_form_errors(request, checksum_form)
+        return HttpResponseBadRequest("Invalid checksum")
 
-    form = TranslationForm(request.user, translation, None, request.POST)
-    unit = None
+    unit = checksum_form.cleaned_data["unit"]
     translationsum = ""
+
+    form = TranslationForm(request.user, unit, request.POST)
     if not form.is_valid():
         show_form_errors(request, form)
-    elif not request.user.has_perm("unit.edit", form.cleaned_data["unit"]):
+    elif not request.user.has_perm("unit.edit", unit):
         messages.error(request, _("Insufficient privileges for saving translations."))
     else:
-        unit = form.cleaned_data["unit"]
-
         perform_translation(unit, form, request)
 
         translationsum = hash_to_checksum(unit.get_target_hash())