diff --git a/pontoon/api/README.md b/pontoon/api/README.md index 2605655e5e..6615de1693 100644 --- a/pontoon/api/README.md +++ b/pontoon/api/README.md @@ -6,7 +6,7 @@ Pontoon provides a set of [RESTful](https://developer.mozilla.org/en-US/docs/Glo Most endpoints are publicly accessible and require no authentication. A few endpoints require an authenticated user. -Requests can be authenticated either with a session cookie or with a Personal Access Token (PAT). Write endpoints accept only a PAT. You can create a PAT from your [user settings](https://pontoon.mozilla.org/settings/) page (see the [User Accounts & Settings](https://github.com/mozilla/pontoon/blob/main/documentation/docs/localizer/users.md#personal-access-tokens) documentation for details). +Requests can be authenticated either with a session cookie or with a Personal Access Token (PAT). Session requests that write data are subject to Django's CSRF checks. The upload endpoints accept both; `POST /api/v2/pretranslate/`, which returns a machine pretranslation for a string, accepts only a PAT. You can create a PAT from your [user settings](https://pontoon.mozilla.org/settings/) page (see the [User Accounts & Settings](https://github.com/mozilla/pontoon/blob/main/documentation/docs/localizer/users.md#personal-access-tokens) documentation for details). Send the token in the `Authorization` header using the `Bearer` scheme: @@ -75,8 +75,8 @@ $ curl --globoff "https://example.com/api/v2/locales/?page_size=50" ## Write Endpoints -The following endpoints can write data and always require authentication with a Personal -Access Token. Session cookies are not accepted. +The following endpoints can write data and always require authentication, with a Personal +Access Token or with a session cookie and CSRF token. ### `POST /api/v2/upload/translations/` @@ -109,7 +109,8 @@ A successful request returns a summary of the import: "updated": 12, "unchanged": 3, "undefined_keys": [["obsolete_key"]], - "undefined_keys_count": 1 + "undefined_keys_count": 1, + "badge_updates": [{ "name": "Translation Champion", "level": 2 }] } ``` @@ -122,6 +123,8 @@ A successful request returns a summary of the import: Only the first 100 keys are listed. - `undefined_keys_count`: total number of keys with no matching string in Pontoon, before truncation. +- `badge_updates`: badges whose level the upload raised, each with its `name` and new + `level`. The user is also notified of each. Empty when no level changed. The upload is additive: strings missing from the uploaded file are left untouched, so partial files can be used to update a subset of translations. Re-uploading an unchanged @@ -154,7 +157,7 @@ Status codes: | ----- | ---------------------------------------------------------------------------------------------------------- | | `200` | Upload accepted (possibly with `"updated": 0`) | | `400` | Missing or invalid field, unsupported format, unparseable or empty file, or file too large | -| `403` | Missing token, invalid or expired token, or insufficient permission | +| `403` | Not authenticated, invalid or expired token, missing CSRF token, or insufficient permission | | `404` | Unknown or disabled project, unknown locale or resource, or project or resource not enabled for the locale | | `409` | A concurrent upload or review changed the same translations; retry the request | | `429` | Rate limit exceeded | @@ -192,7 +195,8 @@ A successful request returns a summary of the import: ], "failed_checks_count": 1, "undefined_keys": [["obsolete_key"]], - "undefined_keys_count": 1 + "undefined_keys_count": 1, + "badge_updates": [] } ``` @@ -217,6 +221,8 @@ A successful request returns a summary of the import: Only the first 100 keys are listed. - `undefined_keys_count`: total number of keys with no matching string in Pontoon, before truncation. +- `badge_updates`: badges whose level the upload raised, each with its `name` and new + `level`. The user is also notified of each. Empty when no level changed. Unlike the built-in pretranslation, strings with unreviewed suggestions are pretranslated. Suggestions that don't match the uploaded translation are kept as @@ -230,3 +236,81 @@ The requirements, limits and status codes of [`POST /api/v2/upload/translations/`](#post-apiv2uploadtranslations) also apply here, with one difference: the request is rejected with `403` unless the user is a member of the `pretranslators` group. + +### `POST /api/v2/upload/suggestions/` + +Store translations from an uploaded translation file as unreviewed suggestions, +authored by the authenticated user. + +The request body is the same `multipart/form-data` as +[`POST /api/v2/upload/translations/`](#post-apiv2uploadtranslations): + +```bash +$ curl -X POST \ + -H "Authorization: Bearer " \ + -F "project=firefox" \ + -F "locale=it" \ + -F "resource=browser/browser.ftl" \ + -F "uploadfile=@browser.ftl" \ + "https://example.com/api/v2/upload/suggestions/" +``` + +A successful request returns a summary of the import: + +```json +{ + "created": 9, + "restored": 1, + "unchanged": 4, + "failed_checks": [ + { + "key": ["entity_key"], + "errors": ["Ending newline mismatch"], + "warnings": [] + } + ], + "failed_checks_count": 1, + "undefined_keys": [["obsolete_key"]], + "undefined_keys_count": 1, + "badge_updates": [] +} +``` + +- `created`: number of suggestions added. +- `restored`: number of rejected translations matching the upload that were + un-rejected, becoming pending suggestions again. +- `unchanged`: number of uploaded translations that match existing unrejected + translations, in any review state, ignored. +- `failed_checks`: strings left untouched, because the uploaded translation has errors. + Each entry has the `key` of the string, in the same format as the `key` field of + entities, and the `errors` and `warnings` reported for it. Only the first 100 keys are + listed. +- `failed_checks_count`: total number of strings left untouched because of errors, + before truncation. +- `undefined_keys`: keys of translations with no matching string in Pontoon, ignored. + Each key is a list of strings, in the same format as the `key` field of entities. + Only the first 100 keys are listed. +- `undefined_keys_count`: total number of keys with no matching string in Pontoon, + before truncation. +- `badge_updates`: badges whose level the upload raised, each with its `name` and new + `level`. The user is also notified of each. Empty when no level changed. + +Nothing already in Pontoon is replaced or rejected: every uploaded translation is stored +as a suggestion, unless the string already has an unrejected translation with the same +value, whether approved, pretranslated or unreviewed. The fuzzy flag of the uploaded +file is ignored: the translation is stored as a plain suggestion. + +A rejected translation matching the upload is un-rejected instead of being suggested +again, so a translation rejected by mistake can be re-proposed. Its original author and +date are kept, and it becomes a pending suggestion, awaiting review like any other. + +Uploaded translations reported with errors are left out, as the editor rejects them as +well. Translations with warnings are stored, with their warnings, as a reviewer can +still accept them. + +NOTE: unlike in the UI, where any user can submit suggestions, this endpoint +requires translator rights. This is done to prevent abuse, since a malicious actor +could submit thousands of suggestions. + +The requirements, limits and status codes of +[`POST /api/v2/upload/translations/`](#post-apiv2uploadtranslations) also apply here. diff --git a/pontoon/api/serializers.py b/pontoon/api/serializers.py index e67d0874c2..91768e7b9f 100644 --- a/pontoon/api/serializers.py +++ b/pontoon/api/serializers.py @@ -419,6 +419,39 @@ def get_translation(self, obj): UPLOAD_KEYS_ERROR_LIMIT = 100 +def undefined_keys_field() -> serializers.ListField: + """Upload response field listing the keys that match no entity in Pontoon.""" + return serializers.ListField( + child=serializers.ListField(child=serializers.CharField()), + help_text=f"Keys of translations with no matching entity in Pontoon, ignored. " + f"Truncated to the first {UPLOAD_KEYS_ERROR_LIMIT} keys.", + ) + + +def undefined_keys_count_field() -> serializers.IntegerField: + """Upload response field counting the keys that match no entity in Pontoon.""" + return serializers.IntegerField( + help_text="Total number of keys with no matching entity in Pontoon, " + "before truncation." + ) + + +class BadgeUpdateSerializer(serializers.Serializer): + """A badge level the user reached through the upload.""" + + name = serializers.CharField(help_text="Name of the badge.") + level = serializers.IntegerField(help_text="Level reached.") + + +def badge_updates_field() -> BadgeUpdateSerializer: + """Upload response field listing the badge levels the user reached.""" + return BadgeUpdateSerializer( + many=True, + help_text="Badges whose level the upload raised, with the new level. " + "The user is also notified of each.", + ) + + class UploadTranslationsResponseSerializer(serializers.Serializer): """Result of a translation file upload.""" @@ -428,15 +461,9 @@ class UploadTranslationsResponseSerializer(serializers.Serializer): unchanged = serializers.IntegerField( help_text="Number of translations identical to the current ones, ignored." ) - undefined_keys = serializers.ListField( - child=serializers.ListField(child=serializers.CharField()), - help_text=f"Keys of translations with no matching entity in Pontoon, ignored. " - f"Truncated to the first {UPLOAD_KEYS_ERROR_LIMIT} keys.", - ) - undefined_keys_count = serializers.IntegerField( - help_text="Total number of keys with no matching entity in Pontoon, " - "before truncation." - ) + undefined_keys = undefined_keys_field() + undefined_keys_count = undefined_keys_count_field() + badge_updates = badge_updates_field() class FailedCheckSerializer(serializers.Serializer): @@ -489,12 +516,34 @@ class UploadPretranslationsResponseSerializer(serializers.Serializer): help_text="Total number of strings left untouched because of failing checks, " "before truncation." ) - undefined_keys = serializers.ListField( - child=serializers.ListField(child=serializers.CharField()), - help_text=f"Keys of translations with no matching entity in Pontoon, ignored. " - f"Truncated to the first {UPLOAD_KEYS_ERROR_LIMIT} keys.", + undefined_keys = undefined_keys_field() + undefined_keys_count = undefined_keys_count_field() + badge_updates = badge_updates_field() + + +class UploadSuggestionsResponseSerializer(serializers.Serializer): + """Result of a suggestion file upload.""" + + created = serializers.IntegerField( + help_text="Number of suggestions added by the upload." ) - undefined_keys_count = serializers.IntegerField( - help_text="Total number of keys with no matching entity in Pontoon, " + restored = serializers.IntegerField( + help_text="Number of rejected translations matching the upload that were " + "un-rejected, becoming pending suggestions again." + ) + unchanged = serializers.IntegerField( + help_text="Number of uploaded translations that the string already has as an " + "unrejected translation, in any review state, ignored." + ) + failed_checks = FailedCheckSerializer( + many=True, + help_text="Strings left untouched, because the uploaded translation has " + f"errors. Truncated to the first {UPLOAD_KEYS_ERROR_LIMIT} keys.", + ) + failed_checks_count = serializers.IntegerField( + help_text="Total number of strings left untouched because of errors, " "before truncation." ) + undefined_keys = undefined_keys_field() + undefined_keys_count = undefined_keys_count_field() + badge_updates = badge_updates_field() diff --git a/pontoon/api/tests/test_upload.py b/pontoon/api/tests/test_upload.py new file mode 100644 index 0000000000..fdca9db0dd --- /dev/null +++ b/pontoon/api/tests/test_upload.py @@ -0,0 +1,693 @@ +from datetime import timedelta + +import pytest + +from notifications.models import Notification +from rest_framework.test import APIClient +from rest_framework.throttling import SimpleRateThrottle + +from django.contrib.auth.hashers import make_password +from django.contrib.auth.models import Group +from django.core.cache import cache +from django.core.files.uploadedfile import SimpleUploadedFile +from django.db import IntegrityError +from django.utils.timezone import now + +from pontoon.api.models import PersonalAccessToken +from pontoon.api.serializers import UPLOAD_KEYS_ERROR_LIMIT +from pontoon.base import badge_utils +from pontoon.base.models import Project, Translation +from pontoon.sync import upload as sync_upload +from pontoon.sync.upload import UploadConflictError +from pontoon.test.factories import EntityFactory + + +TRANSLATIONS = "/api/v2/upload/translations/" +PRETRANSLATIONS = "/api/v2/upload/pretranslations/" +SUGGESTIONS = "/api/v2/upload/suggestions/" +ENDPOINTS = [TRANSLATIONS, PRETRANSLATIONS, SUGGESTIONS] + +# The importer each endpoint calls, as named in `pontoon.sync.upload`. +IMPORTERS = { + TRANSLATIONS: "import_uploaded_file", + PRETRANSLATIONS: "import_uploaded_pretranslations", + SUGGESTIONS: "import_uploaded_suggestions", +} + +PO_CONTENTS = 'msgid "test_key"\nmsgstr "new translation"' + + +def _pat_client(user): + token = PersonalAccessToken.objects.create( + user=user, + name="Upload Token", + token_hash="placeholder", + expires_at=now() + timedelta(days=1), + ) + token_unhashed = "unhashed-token" + token.token_hash = make_password(token_unhashed) + token.save() + + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"Bearer {token.id}_{token_unhashed}") + return client + + +def _po_file(contents=PO_CONTENTS, name="resource_a.po"): + return SimpleUploadedFile(name, contents.encode("utf-8")) + + +def _post(client, url, **data): + return client.post(url, data, format="multipart") + + +def _upload(client, url, project_locale, resource_path, contents=PO_CONTENTS): + return _post( + client, + url, + project=project_locale.project.slug, + locale=project_locale.locale.code, + resource=resource_path, + uploadfile=_po_file(contents), + ) + + +@pytest.fixture +def upload_translator(member, project_locale_a): + project_locale_a.locale.translators_group.user_set.add(member.user) + return member + + +@pytest.fixture +def pretranslator(upload_translator): + """A translator who may also upload pretranslations, so any endpoint accepts them.""" + upload_translator.user.groups.add(Group.objects.get(name="pretranslators")) + return upload_translator + + +@pytest.fixture +def upload_po_translation(translation_a): + translation_a.entity.key = ["test_key"] + translation_a.entity.save() + return translation_a + + +@pytest.fixture +def resource_path(upload_po_translation): + return upload_po_translation.entity.resource.path + + +@pytest.fixture +def untranslated_entity(upload_po_translation): + """An entity without translations, in the same resource as `upload_po_translation`.""" + return EntityFactory.create( + resource=upload_po_translation.entity.resource, + string="Other entity", + key=["other_key"], + ) + + +# Authentication and permissions, shared by all upload endpoints + + +@pytest.mark.django_db +@pytest.mark.parametrize("url", ENDPOINTS) +def test_upload_requires_authentication(url, project_locale_a, resource_path): + response = _upload(APIClient(), url, project_locale_a, resource_path) + + assert response.status_code == 403 + + +@pytest.mark.django_db +@pytest.mark.parametrize("url", ENDPOINTS) +def test_upload_session_auth(url, pretranslator, project_locale_a, resource_path): + """The translate app uploads with its session, without a token.""" + client = APIClient() + # force_authenticate() would bypass authentication_classes. + client.force_login(pretranslator.user) + + response = _upload(client, url, project_locale_a, resource_path) + + assert response.status_code == 200 + assert Translation.objects.filter(string="new translation").exists() + + +@pytest.mark.django_db +@pytest.mark.parametrize("url", ENDPOINTS) +def test_upload_session_auth_requires_csrf_token( + url, pretranslator, project_locale_a, resource_path +): + """Session requests are subject to CSRF checks, unlike token requests.""" + client = APIClient(enforce_csrf_checks=True) + client.force_login(pretranslator.user) + + response = _upload(client, url, project_locale_a, resource_path) + + assert response.status_code == 403 + assert "CSRF" in response.json()["detail"] + assert not Translation.objects.filter(string="new translation").exists() + + +@pytest.mark.django_db +@pytest.mark.parametrize("url", ENDPOINTS) +def test_upload_requires_translate_permission( + url, member, project_locale_a, resource_path +): + """Membership of the pretranslators group alone is not enough either.""" + member.user.groups.add(Group.objects.get(name="pretranslators")) + + response = _upload(_pat_client(member.user), url, project_locale_a, resource_path) + + assert response.status_code == 403 + assert not Translation.objects.filter(string="new translation").exists() + + +@pytest.mark.django_db +@pytest.mark.parametrize("url", ENDPOINTS) +def test_upload_readonly_project_locale( + url, pretranslator, project_locale_a, resource_path +): + project_locale_a.readonly = True + project_locale_a.save() + + response = _upload( + _pat_client(pretranslator.user), url, project_locale_a, resource_path + ) + + assert response.status_code == 403 + + +@pytest.mark.django_db +def test_upload_pretranslations_requires_pretranslators_group( + upload_translator, project_locale_a, resource_path +): + """Translator rights alone are not enough.""" + response = _upload( + _pat_client(upload_translator.user), + PRETRANSLATIONS, + project_locale_a, + resource_path, + ) + + assert response.status_code == 403 + assert not Translation.objects.filter(pretranslated=True).exists() + + +@pytest.mark.django_db +def test_upload_admin_can_upload(member, project_locale_a, resource_path): + member.user.is_superuser = True + member.user.save() + + assert not project_locale_a.locale.translators_group.user_set.filter( + pk=member.user.pk + ).exists() + + response = _upload( + _pat_client(member.user), TRANSLATIONS, project_locale_a, resource_path + ) + + assert response.status_code == 200 + assert response.json()["updated"] == 1 + + +# Request validation and target lookup, shared by all upload endpoints + + +@pytest.mark.django_db +@pytest.mark.parametrize("missing", ["project", "locale", "resource", "uploadfile"]) +def test_upload_missing_field(missing, upload_translator, project_locale_a): + data = { + "project": project_locale_a.project.slug, + "locale": project_locale_a.locale.code, + "resource": "resource_a.po", + "uploadfile": _po_file(), + } + del data[missing] + + response = _post(_pat_client(upload_translator.user), TRANSLATIONS, **data) + + assert response.status_code == 400 + assert missing in response.json() + + +@pytest.mark.django_db +def test_upload_incompatible_format(upload_translator, project_locale_a, resource_path): + response = _post( + _pat_client(upload_translator.user), + TRANSLATIONS, + project=project_locale_a.project.slug, + locale=project_locale_a.locale.code, + resource=resource_path, + uploadfile=_po_file(contents="irrelevant", name="resource_a.ftl"), + ) + + assert response.status_code == 400 + + +@pytest.mark.django_db +def test_upload_oversized_file(upload_translator, project_locale_a, resource_path): + response = _upload( + _pat_client(upload_translator.user), + TRANSLATIONS, + project_locale_a, + resource_path, + contents="#" * (5000 * 1000 + 1), + ) + + assert response.status_code == 400 + + +@pytest.mark.django_db +def test_upload_file_validated_after_authorization( + member, project_locale_a, resource_path +): + """An oversized file from a user without translator rights is a 403, not a 400.""" + response = _upload( + _pat_client(member.user), + TRANSLATIONS, + project_locale_a, + resource_path, + contents="#" * (5000 * 1000 + 1), + ) + + assert response.status_code == 403 + + +@pytest.mark.django_db +def test_upload_unparseable_file(upload_translator, project_locale_a, resource_path): + """Reject malformed files.""" + response = _upload( + _pat_client(upload_translator.user), + TRANSLATIONS, + project_locale_a, + resource_path, + contents="this is not valid gettext {{{ broken", + ) + + assert response.status_code == 400 + assert "uploadfile" in response.json() + + +@pytest.mark.django_db +def test_upload_file_without_translations( + upload_translator, project_locale_a, resource_path +): + """Reject files with no translations, rather than reporting a no-op.""" + response = _upload( + _pat_client(upload_translator.user), + TRANSLATIONS, + project_locale_a, + resource_path, + contents="# Just a comment\n", + ) + + assert response.status_code == 400 + assert response.json() == { + "uploadfile": ["No translations found in uploaded file."] + } + + +@pytest.mark.django_db +def test_upload_disabled_project(upload_translator, project_locale_a, resource_path): + project = project_locale_a.project + project.disabled = True + project.save() + + response = _upload( + _pat_client(upload_translator.user), + TRANSLATIONS, + project_locale_a, + resource_path, + contents='msgid "test_key"\nmsgstr "into disabled"', + ) + + assert response.status_code == 404 + assert not Translation.objects.filter(string="into disabled").exists() + + +@pytest.mark.django_db +def test_upload_private_project_not_visible( + upload_translator, project_locale_a, resource_path +): + project = project_locale_a.project + project.visibility = Project.Visibility.PRIVATE + project.save() + + response = _upload( + _pat_client(upload_translator.user), + TRANSLATIONS, + project_locale_a, + resource_path, + ) + + assert response.status_code == 404 + + +@pytest.mark.django_db +def test_upload_unknown_locale(upload_translator, project_locale_a): + response = _post( + _pat_client(upload_translator.user), + TRANSLATIONS, + project=project_locale_a.project.slug, + locale="does-not-exist", + resource="resource_a.po", + uploadfile=_po_file(), + ) + + assert response.status_code == 404 + + +@pytest.mark.django_db +def test_upload_locale_not_enabled_for_project(member, project_locale_a, locale_b): + locale_b.translators_group.user_set.add(member.user) + + response = _post( + _pat_client(member.user), + TRANSLATIONS, + project=project_locale_a.project.slug, + locale=locale_b.code, + resource="resource_a.po", + uploadfile=_po_file(), + ) + + assert response.status_code == 404 + + +@pytest.mark.django_db +def test_upload_unknown_resource(upload_translator, project_locale_a): + response = _post( + _pat_client(upload_translator.user), + TRANSLATIONS, + project=project_locale_a.project.slug, + locale=project_locale_a.locale.code, + resource="does_not_exist.po", + uploadfile=_po_file(name="does_not_exist.po"), + ) + + assert response.status_code == 404 + + +@pytest.mark.django_db +def test_upload_resource_not_enabled_for_locale( + upload_translator, project_locale_a, resource_a +): + """A resource with no TranslatedResource for the locale is not writable.""" + response = _upload( + _pat_client(upload_translator.user), + TRANSLATIONS, + project_locale_a, + resource_a.path, + ) + + assert response.status_code == 404 + assert not Translation.objects.filter(entity__resource=resource_a).exists() + + +@pytest.mark.django_db +@pytest.mark.parametrize("url", ENDPOINTS) +@pytest.mark.parametrize( + "error", + [ + IntegrityError("duplicate key value violates unique constraint"), + UploadConflictError(), + ], +) +def test_upload_concurrent_conflict( + monkeypatch, url, error, pretranslator, project_locale_a, resource_path +): + """A clash with a concurrent upload or review is reported as a conflict.""" + + def failing_import(*args, **kwargs): + raise error + + monkeypatch.setattr(sync_upload, IMPORTERS[url], failing_import) + + response = _upload( + _pat_client(pretranslator.user), url, project_locale_a, resource_path + ) + + assert response.status_code == 409 + + +# Responses + + +@pytest.mark.django_db +def test_upload_translations_response( + upload_translator, project_locale_a, resource_path +): + response = _upload( + _pat_client(upload_translator.user), + TRANSLATIONS, + project_locale_a, + resource_path, + ) + + assert response.status_code == 200 + assert response.json() == { + "updated": 1, + "unchanged": 0, + "undefined_keys": [], + "undefined_keys_count": 0, + "badge_updates": [], + } + + translation = Translation.objects.get(string="new translation") + + assert translation.approved + assert translation.user == upload_translator.user + + +@pytest.mark.django_db +def test_upload_pretranslations_response( + pretranslator, project_locale_a, untranslated_entity +): + response = _upload( + _pat_client(pretranslator.user), + PRETRANSLATIONS, + project_locale_a, + untranslated_entity.resource.path, + contents='msgid "other_key"\nmsgstr "pretranslation"', + ) + + assert response.status_code == 200 + assert response.json() == { + "created": 1, + "replaced": 0, + "converted": 0, + "unchanged": 0, + "skipped": 0, + "failed_checks": [], + "failed_checks_count": 0, + "undefined_keys": [], + "undefined_keys_count": 0, + "badge_updates": [], + } + + translation = Translation.objects.get(entity=untranslated_entity) + + assert translation.pretranslated + assert translation.user == pretranslator.user + + +@pytest.mark.django_db +def test_upload_suggestions_response( + upload_translator, project_locale_a, untranslated_entity +): + response = _upload( + _pat_client(upload_translator.user), + SUGGESTIONS, + project_locale_a, + untranslated_entity.resource.path, + contents='msgid "other_key"\nmsgstr "a suggestion"', + ) + + assert response.status_code == 200 + assert response.json() == { + "created": 1, + "restored": 0, + "unchanged": 0, + "failed_checks": [], + "failed_checks_count": 0, + "undefined_keys": [], + "undefined_keys_count": 0, + "badge_updates": [], + } + + translation = Translation.objects.get(entity=untranslated_entity) + + assert not translation.approved + assert not translation.pretranslated + assert translation.user == upload_translator.user + + +@pytest.mark.django_db +def test_upload_unknown_keys_reported( + upload_translator, project_locale_a, resource_path +): + """Unknown keys are reported in the format of entity keys.""" + response = _upload( + _pat_client(upload_translator.user), + TRANSLATIONS, + project_locale_a, + resource_path, + contents='msgid "test_key"\nmsgstr "new translation"\n\n' + 'msgid "no_such_key"\nmsgstr "x"\n\n' + 'msgid "another_missing"\nmsgstr "y"\n', + ) + + assert response.status_code == 200 + assert response.json() == { + "updated": 1, + "unchanged": 0, + "undefined_keys": [["no_such_key"], ["another_missing"]], + "undefined_keys_count": 2, + "badge_updates": [], + } + + +@pytest.mark.django_db +def test_upload_unknown_keys_truncated( + upload_translator, project_locale_a, resource_path +): + """Report at most UPLOAD_KEYS_ERROR_LIMIT unknown keys, alongside their total number.""" + unknown = 2 * UPLOAD_KEYS_ERROR_LIMIT + response = _upload( + _pat_client(upload_translator.user), + TRANSLATIONS, + project_locale_a, + resource_path, + contents="\n\n".join( + f'msgid "missing_{i}"\nmsgstr "x"' for i in range(unknown) + ), + ) + + assert response.status_code == 200 + body = response.json() + assert len(body["undefined_keys"]) == UPLOAD_KEYS_ERROR_LIMIT + assert body["undefined_keys_count"] == unknown + + +@pytest.mark.django_db +@pytest.mark.parametrize("url", [PRETRANSLATIONS, SUGGESTIONS]) +def test_upload_failed_checks_reported( + monkeypatch, url, pretranslator, project_locale_a, resource_path +): + """Translations left out for failing checks are reported with their messages.""" + + def failing_checks(entity, locale_code, string, use_tt_checks): + return {"pErrors": ["Test error"], "pWarnings": ["Test warning"]} + + monkeypatch.setattr(sync_upload, "run_checks", failing_checks) + + response = _upload( + _pat_client(pretranslator.user), url, project_locale_a, resource_path + ) + + assert response.status_code == 200 + assert response.json()["created"] == 0 + assert response.json()["failed_checks"] == [ + {"key": ["test_key"], "errors": ["Test error"], "warnings": ["Test warning"]} + ] + assert response.json()["failed_checks_count"] == 1 + + +# Badges + + +@pytest.mark.django_db +@pytest.mark.parametrize("url", ENDPOINTS) +def test_upload_badge_notification( + monkeypatch, url, pretranslator, project_locale_a, resource_path +): + """Crossing a badge threshold is reported in the response, and notified.""" + levels = iter([0, 1]) + monkeypatch.setattr( + badge_utils, "badges_translation_level", lambda user: next(levels) + ) + monkeypatch.setattr(badge_utils, "badges_review_level", lambda user: 0) + + response = _upload( + _pat_client(pretranslator.user), url, project_locale_a, resource_path + ) + + assert response.status_code == 200 + assert response.json()["badge_updates"] == [ + {"name": "Translation Champion", "level": 1} + ] + notification = Notification.objects.filter( + recipient=pretranslator.user, data__category="badge" + ).get() + assert "Translation Champion" in notification.description + + +@pytest.mark.django_db +def test_upload_no_badge_notification_below_threshold( + monkeypatch, upload_translator, project_locale_a, resource_path +): + """No notification when the upload doesn't move the user to a new badge level.""" + monkeypatch.setattr(badge_utils, "badges_translation_level", lambda user: 1) + monkeypatch.setattr(badge_utils, "badges_review_level", lambda user: 0) + + response = _upload( + _pat_client(upload_translator.user), + TRANSLATIONS, + project_locale_a, + resource_path, + ) + + assert response.status_code == 200 + assert response.json()["badge_updates"] == [] + assert not Notification.objects.filter( + recipient=upload_translator.user, data__category="badge" + ).exists() + + +# Throttling + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "rates", + [ + {"upload_burst": "2/minute", "upload_sustained": "1000/hour"}, + {"upload_burst": "60/minute", "upload_sustained": "2/hour"}, + ], +) +def test_upload_throttled( + monkeypatch, upload_translator, project_locale_a, resource_path, rates +): + # DRF copies the rates into a class attribute at import time, so overriding the + # REST_FRAMEWORK setting has no effect here. + monkeypatch.setattr(SimpleRateThrottle, "THROTTLE_RATES", rates) + cache.clear() + + client = _pat_client(upload_translator.user) + for expected_status in (200, 200, 429): + response = _upload(client, TRANSLATIONS, project_locale_a, resource_path) + assert response.status_code == expected_status + + cache.clear() + + +@pytest.mark.django_db +def test_upload_endpoints_share_the_quota( + monkeypatch, pretranslator, project_locale_a, resource_path +): + """All uploads count against the same per-user quota.""" + monkeypatch.setattr( + SimpleRateThrottle, + "THROTTLE_RATES", + {"upload_burst": "3/minute", "upload_sustained": "1000/hour"}, + ) + cache.clear() + + client = _pat_client(pretranslator.user) + for url, expected_status in zip( + [TRANSLATIONS, PRETRANSLATIONS, SUGGESTIONS, TRANSLATIONS], + (200, 200, 200, 429), + ): + response = _upload(client, url, project_locale_a, resource_path) + assert response.status_code == expected_status + + cache.clear() diff --git a/pontoon/api/tests/test_views.py b/pontoon/api/tests/test_views.py index efdd969ba1..4da13e3321 100644 --- a/pontoon/api/tests/test_views.py +++ b/pontoon/api/tests/test_views.py @@ -1,29 +1,18 @@ -from types import SimpleNamespace - import pytest -from notifications.models import Notification from rest_framework.test import APIClient -from rest_framework.throttling import SimpleRateThrottle from django.contrib.auth.hashers import make_password from django.contrib.auth.models import Group -from django.core.cache import cache -from django.core.files.uploadedfile import SimpleUploadedFile from django.db.models import Prefetch from django.utils.timezone import now, timedelta from pontoon.actionlog.models import ActionLog -from pontoon.api import views from pontoon.api.models import PersonalAccessToken -from pontoon.api.serializers import UPLOAD_KEYS_ERROR_LIMIT -from pontoon.base.models.changed_entity_locale import ChangedEntityLocale from pontoon.base.models.locale import Locale from pontoon.base.models.project import Project from pontoon.base.models.project_locale import ProjectLocale from pontoon.base.models.resource import Resource -from pontoon.base.models.translated_resource import TranslatedResource -from pontoon.base.models.translation import Translation from pontoon.base.models.translation_memory import TranslationMemoryEntry from pontoon.terminology.models import Term, TermTranslation from pontoon.test.factories import ( @@ -61,8 +50,6 @@ def test_user_actions_project_not_visible(member): @pytest.mark.django_db def test_user_actions_includes_implicit_flag(member): - from pontoon.actionlog.models import ActionLog - client = APIClient() client.force_authenticate(user=member.user) @@ -2008,1376 +1995,3 @@ def test_expired_pat_rejected_on_non_pretranslation_endpoint(member): ) assert response.status_code == 403 - - -def _pat_client(user, name="Upload Token"): - token = PersonalAccessToken.objects.create( - user=user, - name=name, - token_hash="placeholder", - expires_at=now() + timedelta(days=1), - ) - token_unhashed = "unhashed-token" - token.token_hash = make_password(token_unhashed) - token.save() - - client = APIClient() - client.credentials(HTTP_AUTHORIZATION=f"Bearer {token.id}_{token_unhashed}") - return client - - -def _upload(client, **data): - return client.post("/api/v2/upload/translations/", data, format="multipart") - - -def _po_file( - contents='msgid "test_key"\nmsgstr "new translation"', name="resource_a.po" -): - return SimpleUploadedFile(name, contents.encode("utf-8")) - - -@pytest.fixture -def upload_translator(member, project_locale_a): - project_locale_a.locale.translators_group.user_set.add(member.user) - return member - - -@pytest.fixture -def upload_po_translation(translation_a): - translation_a.entity.key = ["test_key"] - translation_a.entity.save() - return translation_a - - -@pytest.mark.django_db -def test_upload_api_requires_authentication(project_locale_a): - response = _upload( - APIClient(), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource="resource_a.po", - uploadfile=_po_file(), - ) - - assert response.status_code == 403 - - -@pytest.mark.django_db -def test_upload_api_session_auth_rejected(upload_translator, project_locale_a): - client = APIClient() - # force_authenticate() would bypass authentication_classes. - client.force_login(upload_translator.user) - - response = _upload( - client, - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource="resource_a.po", - uploadfile=_po_file(), - ) - - assert response.status_code == 403 - - -@pytest.mark.django_db -def test_upload_api_cannot_translate(member, project_locale_a, resource_a): - response = _upload( - _pat_client(member.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource="resource_a.po", - uploadfile=_po_file(), - ) - - assert response.status_code == 403 - - -@pytest.mark.django_db -def test_upload_api_readonly_project_locale( - upload_translator, project_locale_a, resource_a -): - project_locale_a.readonly = True - project_locale_a.save() - - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource="resource_a.po", - uploadfile=_po_file(), - ) - - assert response.status_code == 403 - - -@pytest.mark.django_db -def test_upload_api_missing_file(upload_translator, project_locale_a): - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource="resource_a.po", - ) - - assert response.status_code == 400 - assert "uploadfile" in response.json() - - -@pytest.mark.django_db -def test_upload_api_missing_project(upload_translator, project_locale_a): - response = _upload( - _pat_client(upload_translator.user), - locale=project_locale_a.locale.code, - resource="resource_a.po", - uploadfile=_po_file(), - ) - - assert response.status_code == 400 - assert "project" in response.json() - - -@pytest.mark.django_db -def test_upload_api_incompatible_format( - upload_translator, project_locale_a, upload_po_translation -): - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - uploadfile=_po_file(contents="irrelevant", name="resource_a.ftl"), - ) - - assert response.status_code == 400 - - -@pytest.mark.django_db -def test_upload_api_unparseable_file( - upload_translator, project_locale_a, upload_po_translation -): - """Reject malformed files.""" - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - uploadfile=_po_file(contents="this is not valid gettext {{{ broken"), - ) - - assert response.status_code == 400 - assert "uploadfile" in response.json() - - -@pytest.mark.django_db -def test_upload_api_unknown_keys_ignored( - upload_translator, project_locale_a, upload_po_translation -): - """Skip unknown keys and report them, importing the rest of the file.""" - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - uploadfile=_po_file( - contents='msgid "test_key"\nmsgstr "new translation"\n\n' - 'msgid "no_such_key"\nmsgstr "x"\n\n' - 'msgid "another_missing"\nmsgstr "y"\n' - ), - ) - - assert response.status_code == 200 - assert response.json() == { - "updated": 1, - "unchanged": 0, - "undefined_keys": [["no_such_key"], ["another_missing"]], - "undefined_keys_count": 2, - } - assert Translation.objects.filter(string="new translation").exists() - - -@pytest.mark.django_db -def test_upload_api_unknown_keys_truncated( - upload_translator, project_locale_a, upload_po_translation -): - """Report at most UPLOAD_KEYS_ERROR_LIMIT unknown keys, alongside their total number.""" - unknown = 2 * UPLOAD_KEYS_ERROR_LIMIT - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - uploadfile=_po_file( - contents="\n\n".join( - f'msgid "missing_{i}"\nmsgstr "x"' for i in range(unknown) - ) - ), - ) - - assert response.status_code == 200 - body = response.json() - assert len(body["undefined_keys"]) == UPLOAD_KEYS_ERROR_LIMIT - assert body["undefined_keys_count"] == unknown - - -@pytest.mark.django_db -def test_upload_api_badge_notification( - monkeypatch, upload_translator, project_locale_a, upload_po_translation -): - """Crossing a badge threshold through the API notifies the user.""" - levels = iter([0, 1]) - monkeypatch.setattr(views, "badges_translation_level", lambda user: next(levels)) - monkeypatch.setattr(views, "badges_review_level", lambda user: 0) - - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - uploadfile=_po_file(), - ) - - assert response.status_code == 200 - notification = Notification.objects.filter( - recipient=upload_translator.user, data__category="badge" - ).get() - assert "Translation Champion" in notification.description - - -@pytest.mark.django_db -def test_upload_api_no_badge_notification_below_threshold( - monkeypatch, upload_translator, project_locale_a, upload_po_translation -): - """No notification when the upload doesn't move the user to a new badge level.""" - monkeypatch.setattr(views, "badges_translation_level", lambda user: 1) - monkeypatch.setattr(views, "badges_review_level", lambda user: 0) - - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - uploadfile=_po_file(), - ) - - assert response.status_code == 200 - assert not Notification.objects.filter( - recipient=upload_translator.user, data__category="badge" - ).exists() - - -@pytest.mark.django_db -def test_upload_api_file_without_translations( - upload_translator, project_locale_a, upload_po_translation -): - """Reject files with no translations, rather than reporting a no-op.""" - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - uploadfile=_po_file(contents="# Just a comment\n"), - ) - - assert response.status_code == 400 - assert response.json() == { - "uploadfile": ["No translations found in uploaded file."] - } - - -@pytest.mark.django_db -def test_upload_api_disabled_project( - upload_translator, project_locale_a, upload_po_translation -): - """Reject disabled projects.""" - project = project_locale_a.project - project.disabled = True - project.save() - - response = _upload( - _pat_client(upload_translator.user), - project=project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - uploadfile=_po_file(contents='msgid "test_key"\nmsgstr "into disabled"'), - ) - - assert response.status_code == 404 - assert not Translation.objects.filter(string="into disabled").exists() - - -@pytest.mark.django_db -def test_upload_api_oversized_file( - upload_translator, project_locale_a, upload_po_translation -): - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - uploadfile=_po_file(contents="#" * (5000 * 1000 + 1)), - ) - - assert response.status_code == 400 - - -@pytest.mark.django_db -def test_upload_api_file_validated_after_authorization( - member, project_locale_a, resource_a -): - """An oversized file from a user without translator rights is a 403, not a 400.""" - response = _upload( - _pat_client(member.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=resource_a.path, - uploadfile=_po_file(contents="#" * (5000 * 1000 + 1)), - ) - - assert response.status_code == 403 - - -@pytest.mark.django_db -def test_upload_api_resource_not_enabled_for_locale( - upload_translator, project_locale_a, resource_a -): - """A resource with no TranslatedResource for the locale is not writable.""" - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=resource_a.path, - uploadfile=_po_file(), - ) - - assert response.status_code == 404 - assert not Translation.objects.filter(entity__resource=resource_a).exists() - - -@pytest.mark.django_db -def test_upload_api_concurrent_conflict( - monkeypatch, upload_translator, project_locale_a, upload_po_translation -): - """A uniqueness clash with a concurrent upload is reported as a conflict.""" - from django.db import IntegrityError - - from pontoon.sync import upload as sync_upload - - def raise_integrity_error(*args, **kwargs): - raise IntegrityError("duplicate key value violates unique constraint") - - monkeypatch.setattr(sync_upload, "import_uploaded_file", raise_integrity_error) - - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - uploadfile=_po_file(), - ) - - assert response.status_code == 409 - - -@pytest.mark.django_db -def test_upload_api_unknown_resource(upload_translator, project_locale_a): - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource="does_not_exist.po", - uploadfile=_po_file(name="does_not_exist.po"), - ) - - assert response.status_code == 404 - - -@pytest.mark.django_db -def test_upload_api_locale_not_enabled_for_project(member, project_locale_a, locale_b): - locale_b.translators_group.user_set.add(member.user) - - response = _upload( - _pat_client(member.user), - project=project_locale_a.project.slug, - locale=locale_b.code, - resource="resource_a.po", - uploadfile=_po_file(), - ) - - assert response.status_code == 404 - - -@pytest.mark.django_db -def test_upload_api_admin_can_upload(member, project_locale_a, upload_po_translation): - member.user.is_superuser = True - member.user.save() - - assert not project_locale_a.locale.translators_group.user_set.filter( - pk=member.user.pk - ).exists() - - response = _upload( - _pat_client(member.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - uploadfile=_po_file(), - ) - - assert response.status_code == 200 - assert response.json()["updated"] == 1 - - -@pytest.mark.django_db -def test_upload_api_unknown_locale(upload_translator, project_locale_a): - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale="does-not-exist", - resource="resource_a.po", - uploadfile=_po_file(), - ) - - assert response.status_code == 404 - - -@pytest.mark.django_db -def test_upload_api_private_project_not_visible( - upload_translator, project_locale_a, upload_po_translation -): - project = project_locale_a.project - project.visibility = Project.Visibility.PRIVATE - project.save() - - response = _upload( - _pat_client(upload_translator.user), - project=project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - uploadfile=_po_file(), - ) - - assert response.status_code == 404 - - -@pytest.mark.django_db -def test_upload_api_file(upload_translator, project_locale_a, upload_po_translation): - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - uploadfile=_po_file(), - ) - - assert response.status_code == 200 - assert response.json() == { - "updated": 1, - "unchanged": 0, - "undefined_keys": [], - "undefined_keys_count": 0, - } - - translation = Translation.objects.get(string="new translation") - - assert translation.entity.key == ["test_key"] - assert translation.entity.resource.path == "resource_a.po" - assert translation.approved - assert translation.user == upload_translator.user - assert not translation.warnings.exists() - - -@pytest.mark.django_db -def test_upload_api_no_changes( - upload_translator, project_locale_a, upload_po_translation -): - client = _pat_client(upload_translator.user) - kwargs = dict( - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - ) - - first = _upload(client, uploadfile=_po_file(), **kwargs) - assert first.status_code == 200 - assert first.json()["updated"] == 1 - - second = _upload(client, uploadfile=_po_file(), **kwargs) - assert second.status_code == 200 - assert second.json() == { - "updated": 0, - "unchanged": 1, - "undefined_keys": [], - "undefined_keys_count": 0, - } - - -@pytest.mark.django_db -def test_upload_api_logs_action( - upload_translator, project_locale_a, upload_po_translation -): - response = _upload( - _pat_client(upload_translator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - uploadfile=_po_file(), - ) - - assert response.status_code == 200 - assert ActionLog.objects.filter( - performed_by=upload_translator.user, - action_type=ActionLog.ActionType.TRANSLATION_CREATED, - ).exists() - - -@pytest.mark.django_db -@pytest.mark.parametrize( - "rates", - [ - {"upload_burst": "2/minute", "upload_sustained": "1000/hour"}, - {"upload_burst": "60/minute", "upload_sustained": "2/hour"}, - ], -) -def test_upload_api_throttled( - monkeypatch, upload_translator, project_locale_a, upload_po_translation, rates -): - # DRF copies the rates into a class attribute at import time, so overriding the - # REST_FRAMEWORK setting has no effect here. - monkeypatch.setattr(SimpleRateThrottle, "THROTTLE_RATES", rates) - cache.clear() - - client = _pat_client(upload_translator.user) - for expected_status in (200, 200, 429): - response = _upload( - client, - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=upload_po_translation.entity.resource.path, - uploadfile=_po_file(), - ) - assert response.status_code == expected_status - - cache.clear() - - -def _upload_pretranslations(client, **data): - return client.post("/api/v2/upload/pretranslations/", data, format="multipart") - - -@pytest.fixture -def pretranslator(upload_translator): - upload_translator.user.groups.add(Group.objects.get(name="pretranslators")) - return upload_translator - - -@pytest.fixture -def untranslated_entity(upload_po_translation): - """An entity without translations, in the same resource as `upload_po_translation`.""" - return EntityFactory.create( - resource=upload_po_translation.entity.resource, - string="Other entity", - key=["other_key"], - ) - - -def _upload_pretranslation(client, project_locale, resource_path, contents=None): - kwargs = {"contents": contents} if contents is not None else {} - return _upload_pretranslations( - client, - project=project_locale.project.slug, - locale=project_locale.locale.code, - resource=resource_path, - uploadfile=_po_file(**kwargs), - ) - - -@pytest.mark.django_db -def test_upload_pretranslations_requires_authentication( - project_locale_a, upload_po_translation -): - response = _upload_pretranslation( - APIClient(), project_locale_a, upload_po_translation.entity.resource.path - ) - - assert response.status_code == 403 - - -@pytest.mark.django_db -def test_upload_pretranslations_requires_pretranslators_group( - upload_translator, project_locale_a, upload_po_translation -): - """Translator rights alone are not enough.""" - response = _upload_pretranslation( - _pat_client(upload_translator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 403 - assert not Translation.objects.filter(pretranslated=True).exists() - - -@pytest.mark.django_db -def test_upload_pretranslations_requires_translate_permission( - member, project_locale_a, upload_po_translation -): - """Membership of the pretranslators group alone is not enough.""" - member.user.groups.add(Group.objects.get(name="pretranslators")) - - response = _upload_pretranslation( - _pat_client(member.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 403 - assert not Translation.objects.filter(pretranslated=True).exists() - - -@pytest.mark.django_db -def test_upload_pretranslations_readonly_project_locale( - pretranslator, project_locale_a, upload_po_translation -): - project_locale_a.readonly = True - project_locale_a.save() - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 403 - - -@pytest.mark.django_db -def test_upload_pretranslations_creates_pretranslation( - pretranslator, project_locale_a, untranslated_entity -): - """An untranslated string gets a new pretranslation, authored by the PAT user.""" - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - untranslated_entity.resource.path, - contents='msgid "other_key"\nmsgstr "pretranslation"', - ) - - assert response.status_code == 200 - assert response.json() == { - "created": 1, - "replaced": 0, - "converted": 0, - "unchanged": 0, - "skipped": 0, - "failed_checks": [], - "failed_checks_count": 0, - "undefined_keys": [], - "undefined_keys_count": 0, - } - - translation = Translation.objects.get(entity=untranslated_entity) - - assert translation.string == "pretranslation" - assert translation.pretranslated - assert translation.active - assert not translation.approved - assert translation.user == pretranslator.user - assert ActionLog.objects.filter( - performed_by=pretranslator.user, - action_type=ActionLog.ActionType.TRANSLATION_CREATED, - translation=translation, - ).exists() - - -@pytest.mark.django_db -def test_upload_pretranslations_skips_fuzzy_uploads( - pretranslator, project_locale_a, untranslated_entity -): - """A translation marked as fuzzy in the file is not stored as a pretranslation.""" - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - untranslated_entity.resource.path, - contents='#, fuzzy\nmsgid "other_key"\nmsgstr "pretranslation"', - ) - - assert response.status_code == 200 - assert response.json() == { - "created": 0, - "replaced": 0, - "converted": 0, - "unchanged": 0, - "skipped": 1, - "failed_checks": [], - "failed_checks_count": 0, - "undefined_keys": [], - "undefined_keys_count": 0, - } - assert not Translation.objects.filter(entity=untranslated_entity).exists() - - -@pytest.mark.django_db -def test_upload_pretranslations_fuzzy_upload_keeps_existing_pretranslation( - pretranslator, project_locale_a, upload_po_translation -): - """A fuzzy entry leaves a different, existing pretranslation in place.""" - upload_po_translation.approved = False - upload_po_translation.pretranslated = True - upload_po_translation.active = True - upload_po_translation.save() - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - contents='#, fuzzy\nmsgid "test_key"\nmsgstr "fuzzy translation"', - ) - - assert response.status_code == 200 - assert response.json()["skipped"] == 1 - - upload_po_translation.refresh_from_db() - - assert upload_po_translation.pretranslated - assert not upload_po_translation.rejected - assert upload_po_translation.active - assert Translation.objects.filter(entity=upload_po_translation.entity).count() == 1 - - -@pytest.mark.django_db -def test_upload_pretranslations_drops_replacement_with_errors( - monkeypatch, pretranslator, project_locale_a, upload_po_translation -): - """A replacement that fails checks is not stored, keeping the previous translation.""" - from pontoon.sync import upload as sync_upload - - def failing_checks(entity, locale_code, string, use_tt_checks): - return ( - {"pErrors": ["Test error", "Other error"]} - if string == "new translation" - else {} - ) - - monkeypatch.setattr(sync_upload, "run_checks", failing_checks) - - upload_po_translation.pretranslated = True - upload_po_translation.active = True - upload_po_translation.save() - ChangedEntityLocale.objects.all().delete() - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 200 - assert response.json()["replaced"] == 0 - assert response.json()["failed_checks"] == [ - {"key": ["test_key"], "errors": ["Test error", "Other error"], "warnings": []} - ] - assert response.json()["failed_checks_count"] == 1 - assert not Translation.objects.filter(string="new translation").exists() - - upload_po_translation.refresh_from_db() - - assert upload_po_translation.pretranslated - assert upload_po_translation.active - assert not upload_po_translation.rejected - assert not ChangedEntityLocale.objects.filter( - entity=upload_po_translation.entity - ).exists() - assert not ActionLog.objects.filter( - action_type=ActionLog.ActionType.TRANSLATION_CREATED, - performed_by=pretranslator.user, - ).exists() - - -@pytest.mark.django_db -def test_upload_pretranslations_keeps_matching_fuzzy_with_warnings( - monkeypatch, pretranslator, project_locale_a, upload_po_translation -): - """A matching fuzzy translation with warnings stays fuzzy and exported as it is.""" - from pontoon.sync import upload as sync_upload - - def failing_checks(entity, locale_code, string, use_tt_checks): - return {"pndbWarnings": ["Test warning"]} if string == "new translation" else {} - - monkeypatch.setattr(sync_upload, "run_checks", failing_checks) - - upload_po_translation.fuzzy = True - upload_po_translation.active = True - upload_po_translation.string = "new translation" - upload_po_translation.value = ["new translation"] - upload_po_translation.save() - ChangedEntityLocale.objects.all().delete() - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 200 - assert response.json()["converted"] == 0 - assert response.json()["failed_checks_count"] == 1 - - upload_po_translation.refresh_from_db() - - assert upload_po_translation.fuzzy - assert not upload_po_translation.pretranslated - assert not ChangedEntityLocale.objects.filter( - entity=upload_po_translation.entity - ).exists() - - -@pytest.mark.django_db -def test_upload_pretranslations_reports_missing_placeholder( - pretranslator, project_locale_a -): - """A dropped placeholder is caught, though it is not a check stored in the DB.""" - resource = ResourceFactory.create( - project=project_locale_a.project, - path="values/strings.xml", - format=Resource.Format.ANDROID, - ) - TranslatedResourceFactory.create(resource=resource, locale=project_locale_a.locale) - EntityFactory.create( - resource=resource, string="The page at {$arg1} says:", key=["page_at"] - ) - - response = _upload_pretranslations( - _pat_client(pretranslator.user), - project=project_locale_a.project.slug, - locale=project_locale_a.locale.code, - resource=resource.path, - uploadfile=SimpleUploadedFile( - "strings.xml", - b'\n' - b"\n" - b' La pagina sul server riporta:\n' - b"\n", - ), - ) - - assert response.status_code == 200 - assert response.json()["created"] == 0 - assert response.json()["failed_checks"] == [ - { - "key": ["page_at"], - "errors": [], - "warnings": ["Placeholder {$arg1} not found in translation"], - } - ] - assert not Translation.objects.filter(entity__resource=resource).exists() - - -@pytest.mark.django_db -def test_upload_pretranslations_skips_matching_translation_with_errors( - monkeypatch, pretranslator, project_locale_a, upload_po_translation -): - """A matching translation that fails checks is not converted, and is not deleted.""" - from pontoon.sync import upload as sync_upload - - def failing_checks(entity, locale_code, string, use_tt_checks): - return {"pErrors": ["Test error"]} if string == "new translation" else {} - - monkeypatch.setattr(sync_upload, "run_checks", failing_checks) - - upload_po_translation.fuzzy = True - upload_po_translation.active = True - upload_po_translation.string = "new translation" - upload_po_translation.value = ["new translation"] - upload_po_translation.save() - ChangedEntityLocale.objects.all().delete() - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 200 - assert response.json()["converted"] == 0 - assert response.json()["failed_checks"] == [ - {"key": ["test_key"], "errors": ["Test error"], "warnings": []} - ] - assert response.json()["failed_checks_count"] == 1 - - upload_po_translation.refresh_from_db() - - assert upload_po_translation.fuzzy - assert not upload_po_translation.pretranslated - assert not upload_po_translation.rejected - assert upload_po_translation.active - assert not ChangedEntityLocale.objects.filter( - entity=upload_po_translation.entity - ).exists() - - -@pytest.mark.django_db -def test_upload_pretranslations_updates_stats_and_marks_changed( - pretranslator, project_locale_a, untranslated_entity -): - """Stored pretranslations are counted in stats, and synced by the next sync.""" - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - untranslated_entity.resource.path, - contents='msgid "other_key"\nmsgstr "pretranslation"', - ) - - assert response.status_code == 200 - assert ( - TranslatedResource.objects.get( - resource=untranslated_entity.resource, locale=project_locale_a.locale - ).pretranslated_strings - == 1 - ) - assert ChangedEntityLocale.objects.filter( - entity=untranslated_entity, locale=project_locale_a.locale - ).exists() - - -@pytest.mark.django_db -def test_upload_pretranslations_drops_replacement_with_warnings( - monkeypatch, pretranslator, project_locale_a, upload_po_translation -): - """Warnings keep a pretranslation from being exported, so it is not stored either.""" - from pontoon.sync import upload as sync_upload - - def failing_checks(entity, locale_code, string, use_tt_checks): - return {"pndbWarnings": ["Test warning"]} if string == "new translation" else {} - - monkeypatch.setattr(sync_upload, "run_checks", failing_checks) - - upload_po_translation.pretranslated = True - upload_po_translation.active = True - upload_po_translation.save() - ChangedEntityLocale.objects.all().delete() - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 200 - assert response.json()["replaced"] == 0 - assert response.json()["failed_checks"] == [ - {"key": ["test_key"], "errors": [], "warnings": ["Test warning"]} - ] - assert not Translation.objects.filter(string="new translation").exists() - - upload_po_translation.refresh_from_db() - - assert upload_po_translation.pretranslated - assert upload_po_translation.active - assert not upload_po_translation.rejected - assert not ChangedEntityLocale.objects.filter( - entity=upload_po_translation.entity - ).exists() - - -@pytest.mark.django_db -def test_upload_pretranslations_updates_latest_translation( - pretranslator, project_locale_a, untranslated_entity -): - """Latest activity is updated, as it would be by Translation.save().""" - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - untranslated_entity.resource.path, - contents='msgid "other_key"\nmsgstr "pretranslation"', - ) - - assert response.status_code == 200 - - pretranslation = Translation.objects.get(entity=untranslated_entity) - project_locale_a.refresh_from_db() - - assert ( - TranslatedResource.objects.get( - resource=untranslated_entity.resource, locale=project_locale_a.locale - ).latest_translation - == pretranslation - ) - assert project_locale_a.latest_translation == pretranslation - - -@pytest.mark.django_db -def test_upload_pretranslations_skips_approved( - pretranslator, project_locale_a, upload_po_translation -): - """A string with an approved translation is left untouched.""" - upload_po_translation.approved = True - upload_po_translation.active = True - upload_po_translation.save() - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 200 - assert response.json()["skipped"] == 1 - assert response.json()["created"] == 0 - - upload_po_translation.refresh_from_db() - - assert upload_po_translation.approved - assert Translation.objects.filter(entity=upload_po_translation.entity).count() == 1 - - -@pytest.mark.django_db -def test_upload_pretranslations_replaces_fuzzy( - pretranslator, project_locale_a, upload_po_translation -): - """A different fuzzy translation is rejected and replaced.""" - upload_po_translation.fuzzy = True - upload_po_translation.active = True - upload_po_translation.save() - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 200 - assert response.json()["replaced"] == 1 - assert response.json()["skipped"] == 0 - - upload_po_translation.refresh_from_db() - - assert upload_po_translation.rejected - assert not upload_po_translation.fuzzy - assert not upload_po_translation.active - - new_translation = Translation.objects.get(string="new translation") - - assert new_translation.pretranslated - assert new_translation.active - assert not new_translation.fuzzy - - -@pytest.mark.django_db -def test_upload_pretranslations_converts_matching_fuzzy( - pretranslator, project_locale_a, upload_po_translation -): - """A fuzzy translation matching the upload becomes a pretranslation.""" - author = upload_po_translation.user - upload_po_translation.fuzzy = True - upload_po_translation.active = True - upload_po_translation.string = "new translation" - upload_po_translation.value = ["new translation"] - upload_po_translation.save() - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 200 - assert response.json()["converted"] == 1 - assert Translation.objects.filter(entity=upload_po_translation.entity).count() == 1 - - upload_po_translation.refresh_from_db() - - assert upload_po_translation.pretranslated - assert upload_po_translation.active - assert not upload_po_translation.fuzzy - assert not upload_po_translation.rejected - assert upload_po_translation.user == author - - -@pytest.mark.django_db -def test_upload_pretranslations_replaces_pretranslation( - pretranslator, project_locale_a, upload_po_translation -): - """A different pretranslation is rejected and replaced.""" - upload_po_translation.pretranslated = True - upload_po_translation.active = True - upload_po_translation.save() - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 200 - assert response.json()["replaced"] == 1 - assert response.json()["created"] == 0 - - upload_po_translation.refresh_from_db() - - assert upload_po_translation.rejected - assert not upload_po_translation.pretranslated - assert not upload_po_translation.active - assert upload_po_translation.rejected_user == pretranslator.user - - new_translation = Translation.objects.get(string="new translation") - - assert new_translation.pretranslated - assert new_translation.active - assert ActionLog.objects.filter( - performed_by=pretranslator.user, - action_type=ActionLog.ActionType.TRANSLATION_REJECTED, - translation=upload_po_translation, - ).exists() - - -@pytest.mark.django_db -def test_upload_pretranslations_reactivates_matching_pretranslation( - pretranslator, project_locale_a, upload_po_translation -): - """A matching pretranslation left inactive by a later suggestion is activated.""" - upload_po_translation.approved = False - upload_po_translation.pretranslated = True - upload_po_translation.active = False - upload_po_translation.string = "new translation" - upload_po_translation.value = ["new translation"] - upload_po_translation.save() - suggestion = TranslationFactory.create( - entity=upload_po_translation.entity, - locale=project_locale_a.locale, - string="a suggestion", - value=["a suggestion"], - active=True, - ) - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 200 - assert response.json()["unchanged"] == 0 - assert response.json()["converted"] == 1 - - upload_po_translation.refresh_from_db() - suggestion.refresh_from_db() - - assert upload_po_translation.pretranslated - assert upload_po_translation.active - assert not suggestion.active - assert not suggestion.rejected - - -@pytest.mark.django_db -def test_upload_pretranslations_unchanged( - pretranslator, project_locale_a, upload_po_translation -): - """An identical pretranslation is reported as unchanged.""" - upload_po_translation.pretranslated = True - upload_po_translation.active = True - upload_po_translation.string = "new translation" - upload_po_translation.value = ["new translation"] - upload_po_translation.save() - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 200 - assert response.json()["unchanged"] == 1 - assert Translation.objects.filter(entity=upload_po_translation.entity).count() == 1 - - -@pytest.mark.django_db -def test_upload_pretranslations_flags_matching_suggestion( - pretranslator, project_locale_a, upload_po_translation -): - """A suggestion matching the upload becomes a pretranslation, keeping its author.""" - author = upload_po_translation.user - upload_po_translation.string = "new translation" - upload_po_translation.value = ["new translation"] - upload_po_translation.save() - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 200 - assert response.json()["converted"] == 1 - assert Translation.objects.filter(entity=upload_po_translation.entity).count() == 1 - - upload_po_translation.refresh_from_db() - - assert upload_po_translation.pretranslated - assert upload_po_translation.active - assert not upload_po_translation.approved - assert upload_po_translation.user == author - - -@pytest.mark.django_db -def test_upload_pretranslations_keeps_other_suggestions( - pretranslator, project_locale_a, upload_po_translation -): - """Suggestions that do not match the upload are kept as unreviewed suggestions.""" - upload_po_translation.active = True - upload_po_translation.save() - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 200 - assert response.json()["created"] == 1 - - upload_po_translation.refresh_from_db() - - assert not upload_po_translation.rejected - assert not upload_po_translation.pretranslated - assert not upload_po_translation.active - - new_translation = Translation.objects.get(string="new translation") - - assert new_translation.pretranslated - assert new_translation.active - - -@pytest.mark.django_db -def test_upload_pretranslations_unknown_keys_ignored( - pretranslator, project_locale_a, upload_po_translation -): - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - contents='msgid "test_key"\nmsgstr "new translation"\n\n' - 'msgid "no_such_key"\nmsgstr "x"\n', - ) - - assert response.status_code == 200 - assert response.json() == { - "created": 1, - "replaced": 0, - "converted": 0, - "unchanged": 0, - "skipped": 0, - "failed_checks": [], - "failed_checks_count": 0, - "undefined_keys": [["no_such_key"]], - "undefined_keys_count": 1, - } - - -def _review_during_import(monkeypatch, review): - """Call `review` from inside a running pretranslation import. - - `import_uploaded_pretranslations()` calls `timezone.now()` after reading the - current translations and before writing anything, so this reproduces a review - landing in the window the conflict check guards. - """ - from pontoon.sync import upload as sync_upload - - real_now = sync_upload.timezone.now - reviewed = False - - def now_and_review(): - nonlocal reviewed - if not reviewed: - reviewed = True - review() - return real_now() - - monkeypatch.setattr( - sync_upload, "timezone", SimpleNamespace(now=now_and_review), raising=False - ) - - -def _approve_during_import(monkeypatch, translation, user): - _review_during_import(monkeypatch, lambda: translation.approve(user)) - - -@pytest.mark.django_db -def test_upload_pretranslations_conflicts_with_concurrent_approval_of_pretranslation( - monkeypatch, pretranslator, project_locale_a, upload_po_translation, admin -): - """A pretranslation approved mid-import is not rejected and replaced.""" - upload_po_translation.pretranslated = True - upload_po_translation.active = True - upload_po_translation.save() - - _approve_during_import(monkeypatch, upload_po_translation, admin) - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 409 - - # The import is rolled back, which also undoes the approval made inside it. - upload_po_translation.refresh_from_db() - - assert not upload_po_translation.rejected - assert upload_po_translation.pretranslated - assert upload_po_translation.active - assert Translation.objects.filter(entity=upload_po_translation.entity).count() == 1 - - -@pytest.mark.django_db -def test_upload_pretranslations_conflicts_with_concurrent_approval_of_suggestion( - monkeypatch, pretranslator, project_locale_a, upload_po_translation, admin -): - """A matching suggestion approved mid-import is not converted to a pretranslation.""" - upload_po_translation.active = True - upload_po_translation.string = "new translation" - upload_po_translation.value = ["new translation"] - upload_po_translation.save() - - _approve_during_import(monkeypatch, upload_po_translation, admin) - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 409 - - upload_po_translation.refresh_from_db() - - assert not upload_po_translation.pretranslated - assert not upload_po_translation.approved - assert Translation.objects.filter(entity=upload_po_translation.entity).count() == 1 - - -@pytest.mark.django_db -def test_upload_pretranslations_conflicts_with_concurrent_rejection_of_suggestion( - monkeypatch, pretranslator, project_locale_a, upload_po_translation, admin -): - """A matching suggestion rejected mid-import is not converted to a pretranslation.""" - upload_po_translation.active = True - upload_po_translation.string = "new translation" - upload_po_translation.value = ["new translation"] - upload_po_translation.save() - - _review_during_import(monkeypatch, lambda: upload_po_translation.reject(admin)) - - response = _upload_pretranslation( - _pat_client(pretranslator.user), - project_locale_a, - upload_po_translation.entity.resource.path, - ) - - assert response.status_code == 409 - - upload_po_translation.refresh_from_db() - - assert not upload_po_translation.rejected - assert not upload_po_translation.pretranslated - assert Translation.objects.filter(entity=upload_po_translation.entity).count() == 1 diff --git a/pontoon/api/urls.py b/pontoon/api/urls.py index 8f0fedc8c9..299b5567cc 100644 --- a/pontoon/api/urls.py +++ b/pontoon/api/urls.py @@ -61,6 +61,11 @@ views.UploadPretranslationsView.as_view(), name="upload-pretranslations", ), + path( + "upload/suggestions/", + views.UploadSuggestionsView.as_view(), + name="upload-suggestions", + ), path( # Terminology Search "search/terminology/", diff --git a/pontoon/api/views.py b/pontoon/api/views.py index c29501a065..921ad727dc 100644 --- a/pontoon/api/views.py +++ b/pontoon/api/views.py @@ -25,7 +25,7 @@ from pontoon.api.filters import TermFilter, TranslationMemoryFilter from pontoon.api.throttling import UPLOAD_THROTTLE_CLASSES from pontoon.base import forms -from pontoon.base.badge_utils import badges_review_level, badges_translation_level +from pontoon.base.badge_utils import badge_levels, new_badge_levels from pontoon.base.get_entities import get_entities_for_project_locale from pontoon.base.models import ( Entity, @@ -65,6 +65,7 @@ TermSerializer, TranslationMemorySerializer, UploadPretranslationsResponseSerializer, + UploadSuggestionsResponseSerializer, UploadTranslationsResponseSerializer, ) @@ -636,10 +637,40 @@ class UploadConflict(APIException): ) +def upload_schema( + response, + *, + accepted: str, + forbidden: str, + description: str, +): + """OpenAPI schema of an upload endpoint, with the error responses they all share.""" + return extend_schema( + request={"multipart/form-data": UPLOAD_REQUEST_SCHEMA}, + responses={ + 200: OpenApiResponse(response=response, description=accepted), + 400: OpenApiResponse( + description="Invalid parameters, or a file that is too large, " + "cannot be parsed, or contains no translations." + ), + 403: OpenApiResponse(description=forbidden), + 404: OpenApiResponse( + description="Unknown or disabled project, unknown locale or resource, " + "or a project or resource not enabled for the locale." + ), + 409: OpenApiResponse( + description="A concurrent upload or review changed the same " + "translations." + ), + 429: OpenApiResponse(description="Rate limit exceeded."), + }, + description=description, + ) + + class UploadView(APIView): """Shared behavior of endpoints writing translations from an uploaded file.""" - authentication_classes = [PersonalAccessTokenAuthentication] permission_classes = [IsAuthenticated] throttle_classes = UPLOAD_THROTTLE_CLASSES # Endpoints share a single upload quota per user. @@ -684,17 +715,37 @@ def upload_target(self, request) -> tuple[Project, Locale, Resource, UploadedFil return project, locale, resource, uploadfile def run_import(self, importer, *args): - """Run an import in a transaction, reporting its failures as API errors.""" + """ + Run an import in a transaction, reporting its failures as API errors. + + Returns the import result and the badge levels the user reached through it, + after notifying them of each. + """ from pontoon.sync.upload import UploadConflictError, UploadError + user = self.request.user + levels_before = badge_levels(user) try: with transaction.atomic(): - return importer(*args) + result = importer(*args) except UploadError as error: raise ValidationError({"uploadfile": [str(error)]}) except (IntegrityError, UploadConflictError): raise UploadConflict() + new_levels = new_badge_levels(user, levels_before) + for badge, level in new_levels: + send_badge_notification(user, badge, level) + return result, new_levels + + def badge_updates(self, levels: list[tuple[str, int]]) -> dict: + """Response field reporting the badge levels the user reached through the import.""" + return { + "badge_updates": [ + {"name": badge, "level": level} for badge, level in levels + ] + } + def undefined_keys(self, result) -> dict: """Response fields reporting the keys with no matching entity in Pontoon.""" return { @@ -704,33 +755,23 @@ def undefined_keys(self, result) -> dict: "undefined_keys_count": len(result.undefined_keys), } + def failed_checks(self, result) -> dict: + """Response fields reporting the keys left out because they fail checks.""" + return { + "failed_checks": [ + {"key": list(fc.key), "errors": fc.errors, "warnings": fc.warnings} + for fc in result.failed_checks[:UPLOAD_KEYS_ERROR_LIMIT] + ], + "failed_checks_count": len(result.failed_checks), + } + class UploadTranslationsView(UploadView): - @extend_schema( - request={"multipart/form-data": UPLOAD_REQUEST_SCHEMA}, - responses={ - 200: OpenApiResponse( - response=UploadTranslationsResponseSerializer, - description="Upload accepted. Reports the number of translations " - "updated and unchanged, and the keys not found in Pontoon.", - ), - 400: OpenApiResponse( - description="Invalid parameters, or a file that is too large, " - "cannot be parsed, or contains no translations." - ), - 403: OpenApiResponse( - description="Missing translate permission, or read-only project locale." - ), - 404: OpenApiResponse( - description="Unknown or disabled project, unknown locale or resource, " - "or a project or resource not enabled for the locale." - ), - 409: OpenApiResponse( - description="A concurrent upload or review changed the same " - "translations." - ), - 429: OpenApiResponse(description="Rate limit exceeded."), - }, + @upload_schema( + UploadTranslationsResponseSerializer, + accepted="Upload accepted. Reports the number of translations updated and " + "unchanged, and the keys not found in Pontoon.", + forbidden="Missing translate permission, or read-only project locale.", description=( "Update translations from an uploaded file, as the authenticated user. " "Requires translator rights for the target locale, and a project locale " @@ -744,31 +785,16 @@ def post(self, request): project, locale, resource, uploadfile = self.upload_target(request) - badge_levels_before = ( - badges_translation_level(request.user), - badges_review_level(request.user), - ) - - result = self.run_import( + result, badges = self.run_import( import_uploaded_file, project, locale, resource, uploadfile, request.user ) - for (badge, get_level), before in zip( - ( - ("Translation Champion", badges_translation_level), - ("Review Master", badges_review_level), - ), - badge_levels_before, - ): - after = get_level(request.user) - if after > before: - send_badge_notification(request.user, badge, after) - return Response( { "updated": result.updated, "unchanged": result.unchanged, **self.undefined_keys(result), + **self.badge_updates(badges), } ) @@ -776,32 +802,12 @@ def post(self, request): class UploadPretranslationsView(UploadView): permission_classes = [IsAuthenticated, IsPretranslator] - @extend_schema( - request={"multipart/form-data": UPLOAD_REQUEST_SCHEMA}, - responses={ - 200: OpenApiResponse( - response=UploadPretranslationsResponseSerializer, - description="Upload accepted. Reports how the pretranslations were " - "stored, and the keys not found in Pontoon.", - ), - 400: OpenApiResponse( - description="Invalid parameters, or a file that is too large, " - "cannot be parsed, or contains no translations." - ), - 403: OpenApiResponse( - description="Missing membership of the pretranslators group, missing " - "translate permission, or read-only project locale." - ), - 404: OpenApiResponse( - description="Unknown or disabled project, unknown locale or resource, " - "or a project or resource not enabled for the locale." - ), - 409: OpenApiResponse( - description="A concurrent upload or review changed the same " - "translations." - ), - 429: OpenApiResponse(description="Rate limit exceeded."), - }, + @upload_schema( + UploadPretranslationsResponseSerializer, + accepted="Upload accepted. Reports how the pretranslations were stored, and " + "the keys not found in Pontoon.", + forbidden="Missing membership of the pretranslators group, missing translate " + "permission, or read-only project locale.", description=( "Store translations from an uploaded translation file as pretranslations. " "This API requires the user to be a member of the `pretranslators` group, " @@ -822,7 +828,7 @@ def post(self, request): project, locale, resource, uploadfile = self.upload_target(request) - result = self.run_import( + result, badges = self.run_import( import_uploaded_pretranslations, project, locale, @@ -838,11 +844,59 @@ def post(self, request): "converted": result.converted, "unchanged": result.unchanged, "skipped": result.skipped, - "failed_checks": [ - {"key": list(fc.key), "errors": fc.errors, "warnings": fc.warnings} - for fc in result.failed_checks[:UPLOAD_KEYS_ERROR_LIMIT] - ], - "failed_checks_count": len(result.failed_checks), + **self.failed_checks(result), + **self.undefined_keys(result), + **self.badge_updates(badges), + } + ) + + +class UploadSuggestionsView(UploadView): + @upload_schema( + UploadSuggestionsResponseSerializer, + accepted="Upload accepted. Reports the number of suggestions created and " + "restored, the translations Pontoon already had, and the keys not found in " + "Pontoon.", + forbidden="Missing translate permission, or read-only project locale.", + description=( + "Store translations from an uploaded translation file as unreviewed " + "suggestions, authored by the authenticated user. Requires translator " + "rights for the target locale, and a project locale that is not " + "read-only, which is stricter than the editor, where any user can " + "suggest: a write API that did not require them would let a single " + "account flood a locale with suggestions. Nothing already in Pontoon is " + "replaced or rejected: every uploaded translation is stored as a " + "suggestion, unless the string already has an unrejected translation " + "with the same value, in any review state. A rejected translation " + "matching the upload is un-rejected instead, becoming a pending " + "suggestion again. " + "Keys not found in Pontoon are ignored, and the fuzzy flag of the " + "uploaded file is ignored as well, as a suggestion is unreviewed by " + "definition. Uploaded translations reported with errors are left out, as " + "the editor rejects them too; translations with warnings are stored." + ), + ) + def post(self, request): + from pontoon.sync.upload import import_uploaded_suggestions + + project, locale, resource, uploadfile = self.upload_target(request) + + result, badges = self.run_import( + import_uploaded_suggestions, + project, + locale, + resource, + uploadfile, + request.user, + ) + + return Response( + { + "created": result.created, + "restored": result.restored, + "unchanged": result.unchanged, + **self.failed_checks(result), **self.undefined_keys(result), + **self.badge_updates(badges), } ) diff --git a/pontoon/base/badge_utils.py b/pontoon/base/badge_utils.py index e8b3e2b2f2..3294acb2b1 100644 --- a/pontoon/base/badge_utils.py +++ b/pontoon/base/badge_utils.py @@ -76,3 +76,20 @@ def badges_review_level(user: User) -> int: if thresholds[level] <= count < thresholds[level + 1]: return level + 1 return 0 + + +def badge_levels(user: User) -> dict[str, int]: + """Current level of each badge awarded for translation and review activity.""" + return { + "Translation Champion": badges_translation_level(user), + "Review Master": badges_review_level(user), + } + + +def new_badge_levels(user: User, before: dict[str, int]) -> list[tuple[str, int]]: + """Badges whose level increased since `before`, each with its new level.""" + return [ + (badge, level) + for badge, level in badge_levels(user).items() + if level > before[badge] + ] diff --git a/pontoon/base/views.py b/pontoon/base/views.py index 77772cd2f6..a13851c7be 100755 --- a/pontoon/base/views.py +++ b/pontoon/base/views.py @@ -34,7 +34,7 @@ from pontoon.actionlog.models import ActionLog from pontoon.actionlog.utils import log_action from pontoon.base import forms, utils -from pontoon.base.badge_utils import badges_review_level, badges_translation_level +from pontoon.base.badge_utils import badge_levels, new_badge_levels from pontoon.base.get_entities import ( get_entities_for_project_locale, get_mismatched_filters, @@ -1067,8 +1067,7 @@ def upload(request): upload = request.FILES["uploadfile"] try: - translation_before_level = badges_translation_level(request.user) - review_before_level = badges_review_level(request.user) + levels_before = badge_levels(request.user) result = import_uploaded_file( project, locale, resource, upload, request.user ) @@ -1081,20 +1080,10 @@ def upload(request): else: messages.info(request, message, extra_tags="upload") - badge_levels = ( - ( - "Translation Champion", - translation_before_level, - badges_translation_level, - ), - ("Review Master", review_before_level, badges_review_level), - ) - for badge_name, before_level, get_level in badge_levels: - after_level = get_level(request.user) - if after_level > before_level: - send_badge_notification(request.user, badge_name, after_level) - message = json.dumps({"name": badge_name, "level": after_level}) - messages.info(request, message, extra_tags="badge") + for badge, level in new_badge_levels(request.user, levels_before): + send_badge_notification(request.user, badge, level) + message = json.dumps({"name": badge, "level": level}) + messages.info(request, message, extra_tags="badge") except Exception as error: messages.error(request, str(error)) else: diff --git a/pontoon/sync/tests/test_upload.py b/pontoon/sync/tests/test_upload.py new file mode 100644 index 0000000000..0848a903e2 --- /dev/null +++ b/pontoon/sync/tests/test_upload.py @@ -0,0 +1,1448 @@ +from datetime import timedelta +from types import SimpleNamespace + +import pytest + +from django.core.files.uploadedfile import SimpleUploadedFile +from django.db import connection, transaction +from django.test.utils import CaptureQueriesContext + +from pontoon.actionlog.models import ActionLog +from pontoon.base.models import ( + ChangedEntityLocale, + Resource, + TranslatedResource, + Translation, +) +from pontoon.checks.models import Error, Warning +from pontoon.sync import upload as sync_upload +from pontoon.sync.upload import ( + FailedCheck, + UploadConflictError, + UploadError, + import_uploaded_file, + import_uploaded_pretranslations, + import_uploaded_suggestions, +) +from pontoon.test.factories import ( + EntityFactory, + ResourceFactory, + TranslatedResourceFactory, + TranslationFactory, +) + + +PO_CONTENTS = 'msgid "test_key"\nmsgstr "new translation"' + + +def _po_file(contents=PO_CONTENTS, name="resource_a.po"): + return SimpleUploadedFile(name, contents.encode("utf-8")) + + +def _import( + importer, project_locale, resource, user, contents=PO_CONTENTS, upload=None +): + return importer( + project_locale.project, + project_locale.locale, + resource, + upload or _po_file(contents), + user, + ) + + +def _import_conflicts(importer, project_locale, resource, user): + """Run an import that must fail on a conflict, rolling it back like the API does.""" + with pytest.raises(UploadConflictError), transaction.atomic(): + _import(importer, project_locale, resource, user) + + +@pytest.fixture +def uploader(user_b): + """The user importing the file, distinct from the author of `po_translation`.""" + return user_b + + +@pytest.fixture +def po_translation(translation_a): + """An unreviewed suggestion for `test_key`, in a gettext resource.""" + translation_a.entity.key = ["test_key"] + translation_a.entity.save() + return translation_a + + +@pytest.fixture +def resource(po_translation): + return po_translation.entity.resource + + +@pytest.fixture +def untranslated_entity(po_translation): + """An entity without translations, in the same resource as `po_translation`.""" + return EntityFactory.create( + resource=po_translation.entity.resource, + string="Other entity", + key=["other_key"], + ) + + +@pytest.fixture +def android_entity(project_locale_a): + """An entity with a placeholder, in an Android resource enabled for the locale.""" + resource = ResourceFactory.create( + project=project_locale_a.project, + path="values/strings.xml", + format=Resource.Format.ANDROID, + ) + TranslatedResourceFactory.create(resource=resource, locale=project_locale_a.locale) + return EntityFactory.create( + resource=resource, string="The page at {$arg1} says:", key=["page_at"] + ) + + +def _android_upload_without_placeholder(): + return SimpleUploadedFile( + "strings.xml", + b'\n' + b"\n" + b' La pagina sul server riporta:\n' + b"\n", + ) + + +def _failing_checks(results): + """A `run_checks()` stand-in reporting `results` for the uploaded translation only.""" + + def run_checks(entity, locale_code, string, use_tt_checks): + return results if string == "new translation" else {} + + return run_checks + + +def _review_during_import(monkeypatch, review): + """Call `review` from inside a running import. + + The importers call `timezone.now()` after reading the current translations and + before writing anything, so this reproduces a review landing in the window the + conflict check guards. It runs in the same transaction, so it does not exercise + the locks themselves. + """ + real_now = sync_upload.timezone.now + reviewed = False + + def now_and_review(): + nonlocal reviewed + if not reviewed: + reviewed = True + review() + return real_now() + + monkeypatch.setattr( + sync_upload, "timezone", SimpleNamespace(now=now_and_review), raising=False + ) + + +def _approve_during_import(monkeypatch, translation, user): + _review_during_import(monkeypatch, lambda: translation.approve(user)) + + +# Translations + + +@pytest.mark.django_db +def test_upload_translations_file(project_locale_a, resource, po_translation, uploader): + result = _import(import_uploaded_file, project_locale_a, resource, uploader) + + assert result.updated == 1 + assert result.unchanged == 0 + assert result.undefined_keys == [] + + translation = Translation.objects.get(string="new translation") + + assert translation.entity.key == ["test_key"] + assert translation.approved + assert translation.user == uploader + assert not translation.warnings.exists() + assert ActionLog.objects.filter( + performed_by=uploader, + action_type=ActionLog.ActionType.TRANSLATION_CREATED, + translation=translation, + ).exists() + + +@pytest.mark.django_db +def test_upload_translations_unchanged( + project_locale_a, resource, po_translation, uploader +): + """Re-importing a file that changed nothing is reported as unchanged.""" + first = _import(import_uploaded_file, project_locale_a, resource, uploader) + assert first.updated == 1 + + second = _import(import_uploaded_file, project_locale_a, resource, uploader) + + assert second.updated == 0 + assert second.unchanged == 1 + assert Translation.objects.filter(string="new translation").count() == 1 + + +@pytest.mark.django_db +def test_upload_translations_unknown_keys_ignored( + project_locale_a, resource, po_translation, uploader +): + """Skip unknown keys and report them, importing the rest of the file.""" + result = _import( + import_uploaded_file, + project_locale_a, + resource, + uploader, + contents='msgid "test_key"\nmsgstr "new translation"\n\n' + 'msgid "no_such_key"\nmsgstr "x"\n\n' + 'msgid "another_missing"\nmsgstr "y"\n', + ) + + assert result.updated == 1 + assert result.undefined_keys == [("no_such_key",), ("another_missing",)] + assert result.undefined == 2 + assert Translation.objects.filter(string="new translation").exists() + + +@pytest.mark.django_db +def test_upload_translations_unparseable_file( + project_locale_a, resource, po_translation, uploader +): + with pytest.raises(UploadError, match="Could not parse uploaded file"): + _import( + import_uploaded_file, + project_locale_a, + resource, + uploader, + contents="this is not valid gettext {{{ broken", + ) + + +@pytest.mark.django_db +def test_upload_translations_file_without_translations( + project_locale_a, resource, po_translation, uploader +): + """A file with no translations is an error, rather than a no-op.""" + with pytest.raises(UploadError, match="No translations found"): + _import( + import_uploaded_file, + project_locale_a, + resource, + uploader, + contents="# Just a comment\n", + ) + + +# Pretranslations + + +@pytest.mark.django_db +def test_upload_pretranslations_creates_pretranslation( + project_locale_a, resource, untranslated_entity, uploader +): + """An untranslated string gets a new pretranslation, authored by the uploader.""" + result = _import( + import_uploaded_pretranslations, + project_locale_a, + resource, + uploader, + contents='msgid "other_key"\nmsgstr "pretranslation"', + ) + + assert result.created == 1 + assert result.replaced == 0 + assert result.converted == 0 + assert result.unchanged == 0 + assert result.skipped == 0 + assert result.failed_checks == [] + assert result.undefined_keys == [] + + translation = Translation.objects.get(entity=untranslated_entity) + + assert translation.string == "pretranslation" + assert translation.pretranslated + assert translation.active + assert not translation.approved + assert translation.user == uploader + assert ActionLog.objects.filter( + performed_by=uploader, + action_type=ActionLog.ActionType.TRANSLATION_CREATED, + translation=translation, + ).exists() + + +@pytest.mark.django_db +def test_upload_pretranslations_skips_fuzzy_uploads( + project_locale_a, resource, untranslated_entity, uploader +): + """A translation marked as fuzzy in the file is not stored as a pretranslation.""" + result = _import( + import_uploaded_pretranslations, + project_locale_a, + resource, + uploader, + contents='#, fuzzy\nmsgid "other_key"\nmsgstr "pretranslation"', + ) + + assert result.created == 0 + assert result.skipped == 1 + assert not Translation.objects.filter(entity=untranslated_entity).exists() + + +@pytest.mark.django_db +def test_upload_pretranslations_fuzzy_upload_keeps_existing_pretranslation( + project_locale_a, resource, po_translation, uploader +): + """A fuzzy entry leaves a different, existing pretranslation in place.""" + po_translation.approved = False + po_translation.pretranslated = True + po_translation.active = True + po_translation.save() + + result = _import( + import_uploaded_pretranslations, + project_locale_a, + resource, + uploader, + contents='#, fuzzy\nmsgid "test_key"\nmsgstr "fuzzy translation"', + ) + + assert result.skipped == 1 + + po_translation.refresh_from_db() + + assert po_translation.pretranslated + assert not po_translation.rejected + assert po_translation.active + assert Translation.objects.filter(entity=po_translation.entity).count() == 1 + + +@pytest.mark.django_db +def test_upload_pretranslations_drops_replacement_with_errors( + monkeypatch, project_locale_a, resource, po_translation, uploader +): + """A replacement that fails checks is not stored, keeping the previous translation.""" + monkeypatch.setattr( + sync_upload, + "run_checks", + _failing_checks({"pErrors": ["Test error", "Other error"]}), + ) + po_translation.pretranslated = True + po_translation.active = True + po_translation.save() + ChangedEntityLocale.objects.all().delete() + + result = _import( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + assert result.replaced == 0 + assert result.failed_checks == [ + FailedCheck( + key=("test_key",), errors=["Test error", "Other error"], warnings=[] + ) + ] + assert not Translation.objects.filter(string="new translation").exists() + + po_translation.refresh_from_db() + + assert po_translation.pretranslated + assert po_translation.active + assert not po_translation.rejected + assert not ChangedEntityLocale.objects.filter(entity=po_translation.entity).exists() + assert not ActionLog.objects.filter( + action_type=ActionLog.ActionType.TRANSLATION_CREATED, performed_by=uploader + ).exists() + + +@pytest.mark.django_db +def test_upload_pretranslations_keeps_matching_fuzzy_with_warnings( + monkeypatch, project_locale_a, resource, po_translation, uploader +): + """A matching fuzzy translation with warnings stays fuzzy and exported as it is.""" + monkeypatch.setattr( + sync_upload, "run_checks", _failing_checks({"pndbWarnings": ["Test warning"]}) + ) + po_translation.fuzzy = True + po_translation.active = True + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + ChangedEntityLocale.objects.all().delete() + + result = _import( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + assert result.converted == 0 + assert len(result.failed_checks) == 1 + + po_translation.refresh_from_db() + + assert po_translation.fuzzy + assert not po_translation.pretranslated + assert not ChangedEntityLocale.objects.filter(entity=po_translation.entity).exists() + + +@pytest.mark.django_db +def test_upload_pretranslations_reports_missing_placeholder( + project_locale_a, android_entity, uploader +): + """A dropped placeholder is caught, though it is not a check stored in the DB.""" + result = _import( + import_uploaded_pretranslations, + project_locale_a, + android_entity.resource, + uploader, + upload=_android_upload_without_placeholder(), + ) + + assert result.created == 0 + assert result.failed_checks == [ + FailedCheck( + key=("page_at",), + errors=[], + warnings=["Placeholder {$arg1} not found in translation"], + ) + ] + assert not Translation.objects.filter(entity=android_entity).exists() + + +@pytest.mark.django_db +def test_upload_pretranslations_skips_matching_translation_with_errors( + monkeypatch, project_locale_a, resource, po_translation, uploader +): + """A matching translation that fails checks is not converted, and is not deleted.""" + monkeypatch.setattr( + sync_upload, "run_checks", _failing_checks({"pErrors": ["Test error"]}) + ) + po_translation.fuzzy = True + po_translation.active = True + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + ChangedEntityLocale.objects.all().delete() + + result = _import( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + assert result.converted == 0 + assert result.failed_checks == [ + FailedCheck(key=("test_key",), errors=["Test error"], warnings=[]) + ] + + po_translation.refresh_from_db() + + assert po_translation.fuzzy + assert not po_translation.pretranslated + assert not po_translation.rejected + assert po_translation.active + assert not ChangedEntityLocale.objects.filter(entity=po_translation.entity).exists() + + +@pytest.mark.django_db +def test_upload_pretranslations_updates_stats_and_marks_changed( + project_locale_a, resource, untranslated_entity, uploader +): + """Stored pretranslations are counted in stats, and synced by the next sync.""" + _import( + import_uploaded_pretranslations, + project_locale_a, + resource, + uploader, + contents='msgid "other_key"\nmsgstr "pretranslation"', + ) + + assert ( + TranslatedResource.objects.get( + resource=resource, locale=project_locale_a.locale + ).pretranslated_strings + == 1 + ) + assert ChangedEntityLocale.objects.filter( + entity=untranslated_entity, locale=project_locale_a.locale + ).exists() + + +@pytest.mark.django_db +def test_upload_pretranslations_drops_replacement_with_warnings( + monkeypatch, project_locale_a, resource, po_translation, uploader +): + """Warnings keep a pretranslation from being exported, so it is not stored either.""" + monkeypatch.setattr( + sync_upload, "run_checks", _failing_checks({"pndbWarnings": ["Test warning"]}) + ) + po_translation.pretranslated = True + po_translation.active = True + po_translation.save() + ChangedEntityLocale.objects.all().delete() + + result = _import( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + assert result.replaced == 0 + assert result.failed_checks == [ + FailedCheck(key=("test_key",), errors=[], warnings=["Test warning"]) + ] + assert not Translation.objects.filter(string="new translation").exists() + + po_translation.refresh_from_db() + + assert po_translation.pretranslated + assert po_translation.active + assert not po_translation.rejected + assert not ChangedEntityLocale.objects.filter(entity=po_translation.entity).exists() + + +@pytest.mark.django_db +def test_upload_pretranslations_updates_latest_translation( + project_locale_a, resource, untranslated_entity, uploader +): + """Latest activity is updated, as it would be by Translation.save().""" + _import( + import_uploaded_pretranslations, + project_locale_a, + resource, + uploader, + contents='msgid "other_key"\nmsgstr "pretranslation"', + ) + + pretranslation = Translation.objects.get(entity=untranslated_entity) + project_locale_a.refresh_from_db() + + assert ( + TranslatedResource.objects.get( + resource=resource, locale=project_locale_a.locale + ).latest_translation + == pretranslation + ) + assert project_locale_a.latest_translation == pretranslation + + +@pytest.mark.django_db +def test_upload_pretranslations_skips_approved( + project_locale_a, resource, po_translation, uploader +): + """A string with an approved translation is left untouched.""" + po_translation.approved = True + po_translation.active = True + po_translation.save() + + result = _import( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + assert result.skipped == 1 + assert result.created == 0 + + po_translation.refresh_from_db() + + assert po_translation.approved + assert Translation.objects.filter(entity=po_translation.entity).count() == 1 + + +@pytest.mark.django_db +def test_upload_pretranslations_replaces_fuzzy( + project_locale_a, resource, po_translation, uploader +): + """A different fuzzy translation is rejected and replaced.""" + po_translation.fuzzy = True + po_translation.active = True + po_translation.save() + + result = _import( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + assert result.replaced == 1 + assert result.skipped == 0 + + po_translation.refresh_from_db() + + assert po_translation.rejected + assert not po_translation.fuzzy + assert not po_translation.active + + new_translation = Translation.objects.get(string="new translation") + + assert new_translation.pretranslated + assert new_translation.active + assert not new_translation.fuzzy + + +@pytest.mark.django_db +def test_upload_pretranslations_converts_matching_fuzzy( + project_locale_a, resource, po_translation, uploader +): + """A fuzzy translation matching the upload becomes a pretranslation.""" + author = po_translation.user + po_translation.fuzzy = True + po_translation.active = True + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + + result = _import( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + assert result.converted == 1 + assert Translation.objects.filter(entity=po_translation.entity).count() == 1 + + po_translation.refresh_from_db() + + assert po_translation.pretranslated + assert po_translation.active + assert not po_translation.fuzzy + assert not po_translation.rejected + assert po_translation.user == author + + +@pytest.mark.django_db +def test_upload_pretranslations_replaces_pretranslation( + project_locale_a, resource, po_translation, uploader +): + """A different pretranslation is rejected and replaced.""" + po_translation.pretranslated = True + po_translation.active = True + po_translation.save() + + result = _import( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + assert result.replaced == 1 + assert result.created == 0 + + po_translation.refresh_from_db() + + assert po_translation.rejected + assert not po_translation.pretranslated + assert not po_translation.active + assert po_translation.rejected_user == uploader + + new_translation = Translation.objects.get(string="new translation") + + assert new_translation.pretranslated + assert new_translation.active + assert ActionLog.objects.filter( + performed_by=uploader, + action_type=ActionLog.ActionType.TRANSLATION_REJECTED, + translation=po_translation, + ).exists() + + +@pytest.mark.django_db +def test_upload_pretranslations_reactivates_matching_pretranslation( + project_locale_a, resource, po_translation, uploader +): + """A matching pretranslation left inactive by a later suggestion is activated.""" + po_translation.approved = False + po_translation.pretranslated = True + po_translation.active = False + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + suggestion = TranslationFactory.create( + entity=po_translation.entity, + locale=project_locale_a.locale, + string="a suggestion", + value=["a suggestion"], + active=True, + ) + + result = _import( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + assert result.unchanged == 0 + assert result.converted == 1 + + po_translation.refresh_from_db() + suggestion.refresh_from_db() + + assert po_translation.pretranslated + assert po_translation.active + assert not suggestion.active + assert not suggestion.rejected + + +@pytest.mark.django_db +def test_upload_pretranslations_unchanged( + project_locale_a, resource, po_translation, uploader +): + """An identical pretranslation is reported as unchanged.""" + po_translation.pretranslated = True + po_translation.active = True + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + + result = _import( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + assert result.unchanged == 1 + assert Translation.objects.filter(entity=po_translation.entity).count() == 1 + + +@pytest.mark.django_db +def test_upload_pretranslations_flags_matching_suggestion( + project_locale_a, resource, po_translation, uploader +): + """A suggestion matching the upload becomes a pretranslation, keeping its author.""" + author = po_translation.user + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + + result = _import( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + assert result.converted == 1 + assert Translation.objects.filter(entity=po_translation.entity).count() == 1 + + po_translation.refresh_from_db() + + assert po_translation.pretranslated + assert po_translation.active + assert not po_translation.approved + assert po_translation.user == author + + +@pytest.mark.django_db +def test_upload_pretranslations_clears_stale_checks_of_matching_suggestion( + project_locale_a, resource, po_translation, uploader +): + """A converted suggestion loses the checks stored for it, as it passes them now.""" + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + # Checks stored when the translation was written, before they changed. + Warning.objects.create( + library="p", message="Stale warning", translation=po_translation + ) + Error.objects.create(library="p", message="Stale error", translation=po_translation) + + result = _import( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + assert result.converted == 1 + assert not po_translation.warnings.exists() + assert not po_translation.errors.exists() + + # Stale checks would have kept the pretranslation out of the stats and the export. + translated_resource = TranslatedResource.objects.get( + resource=resource, locale=project_locale_a.locale + ) + + assert translated_resource.pretranslated_strings == 1 + assert translated_resource.strings_with_warnings == 0 + assert translated_resource.strings_with_errors == 0 + + +@pytest.mark.django_db +def test_upload_pretranslations_keeps_other_suggestions( + project_locale_a, resource, po_translation, uploader +): + """Suggestions that do not match the upload are kept as unreviewed suggestions.""" + po_translation.active = True + po_translation.save() + + result = _import( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + assert result.created == 1 + + po_translation.refresh_from_db() + + assert not po_translation.rejected + assert not po_translation.pretranslated + assert not po_translation.active + + new_translation = Translation.objects.get(string="new translation") + + assert new_translation.pretranslated + assert new_translation.active + + +@pytest.mark.django_db +def test_upload_pretranslations_unknown_keys_ignored( + project_locale_a, resource, po_translation, uploader +): + result = _import( + import_uploaded_pretranslations, + project_locale_a, + resource, + uploader, + contents='msgid "test_key"\nmsgstr "new translation"\n\n' + 'msgid "no_such_key"\nmsgstr "x"\n', + ) + + assert result.created == 1 + assert result.undefined_keys == [("no_such_key",)] + + +@pytest.mark.django_db +def test_upload_pretranslations_conflicts_with_concurrent_approval_of_pretranslation( + monkeypatch, project_locale_a, resource, po_translation, uploader, admin +): + """A pretranslation approved mid-import is not rejected and replaced.""" + po_translation.pretranslated = True + po_translation.active = True + po_translation.save() + + _approve_during_import(monkeypatch, po_translation, admin) + _import_conflicts( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + # The import is rolled back, which also undoes the approval made inside it. + po_translation.refresh_from_db() + + assert not po_translation.rejected + assert po_translation.pretranslated + assert po_translation.active + assert Translation.objects.filter(entity=po_translation.entity).count() == 1 + + +@pytest.mark.django_db +def test_upload_pretranslations_conflicts_with_concurrent_approval_of_suggestion( + monkeypatch, project_locale_a, resource, po_translation, uploader, admin +): + """A matching suggestion approved mid-import is not converted to a pretranslation.""" + po_translation.active = True + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + + _approve_during_import(monkeypatch, po_translation, admin) + _import_conflicts( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + po_translation.refresh_from_db() + + assert not po_translation.pretranslated + assert not po_translation.approved + assert Translation.objects.filter(entity=po_translation.entity).count() == 1 + + +@pytest.mark.django_db +def test_upload_pretranslations_conflicts_with_concurrent_rejection_of_suggestion( + monkeypatch, project_locale_a, resource, po_translation, uploader, admin +): + """A matching suggestion rejected mid-import is not converted to a pretranslation.""" + po_translation.active = True + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + + _review_during_import(monkeypatch, lambda: po_translation.reject(admin)) + _import_conflicts( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + po_translation.refresh_from_db() + + assert not po_translation.rejected + assert not po_translation.pretranslated + assert Translation.objects.filter(entity=po_translation.entity).count() == 1 + + +@pytest.mark.django_db +@pytest.mark.parametrize("field", ["pretranslated", "fuzzy"]) +def test_upload_pretranslations_conflicts_with_concurrent_precedence_change( + monkeypatch, project_locale_a, resource, po_translation, uploader, field +): + """A suggestion that gains precedence mid-import is not just deactivated.""" + po_translation.active = True + po_translation.save() + + _review_during_import( + monkeypatch, + lambda: Translation.objects.filter(pk=po_translation.pk).update( + **{field: True} + ), + ) + _import_conflicts( + import_uploaded_pretranslations, project_locale_a, resource, uploader + ) + + # The import is rolled back, which also undoes the change made inside it. + po_translation.refresh_from_db() + + assert po_translation.active + assert not getattr(po_translation, field) + assert Translation.objects.filter(entity=po_translation.entity).count() == 1 + + +# Suggestions + + +@pytest.mark.django_db +def test_upload_suggestions_creates_suggestion( + project_locale_a, resource, untranslated_entity, uploader +): + """An untranslated string gets a new suggestion, authored by the uploader.""" + result = _import( + import_uploaded_suggestions, + project_locale_a, + resource, + uploader, + contents='msgid "other_key"\nmsgstr "a suggestion"', + ) + + assert result.created == 1 + assert result.restored == 0 + assert result.unchanged == 0 + assert result.failed_checks == [] + assert result.undefined_keys == [] + + translation = Translation.objects.get(entity=untranslated_entity) + + assert translation.string == "a suggestion" + assert translation.active + assert not translation.approved + assert not translation.pretranslated + assert not translation.fuzzy + assert not translation.rejected + assert translation.user == uploader + assert ActionLog.objects.filter( + performed_by=uploader, + action_type=ActionLog.ActionType.TRANSLATION_CREATED, + translation=translation, + ).exists() + + +@pytest.mark.django_db +def test_upload_suggestions_ignores_fuzzy_flag( + project_locale_a, resource, untranslated_entity, uploader +): + """A translation marked as fuzzy in the file is stored as a plain suggestion.""" + result = _import( + import_uploaded_suggestions, + project_locale_a, + resource, + uploader, + contents='#, fuzzy\nmsgid "other_key"\nmsgstr "a suggestion"', + ) + + assert result.created == 1 + + translation = Translation.objects.get(entity=untranslated_entity) + + assert not translation.fuzzy + assert translation.active + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "state", + [ + {}, + {"approved": True}, + {"pretranslated": True}, + {"fuzzy": True}, + ], +) +def test_upload_suggestions_skips_matching_translation( + project_locale_a, resource, po_translation, uploader, state +): + """An unrejected translation Pontoon has is not suggested again, in any state.""" + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.active = True + for name, value in state.items(): + setattr(po_translation, name, value) + po_translation.save() + + result = _import(import_uploaded_suggestions, project_locale_a, resource, uploader) + + assert result.unchanged == 1 + assert result.created == 0 + assert result.restored == 0 + assert Translation.objects.filter(entity=po_translation.entity).count() == 1 + + po_translation.refresh_from_db() + + # The matched translation keeps the review state it had, fuzzy included. + for name, value in state.items(): + assert getattr(po_translation, name) == value + assert not po_translation.rejected + assert po_translation.active + + +@pytest.mark.django_db +def test_upload_suggestions_keeps_approved_translation_active( + project_locale_a, resource, po_translation, uploader +): + """A suggestion for an approved string is stored, without becoming active.""" + po_translation.approved = True + po_translation.active = True + po_translation.save() + + result = _import(import_uploaded_suggestions, project_locale_a, resource, uploader) + + assert result.created == 1 + + po_translation.refresh_from_db() + suggestion = Translation.objects.get(string="new translation") + + assert po_translation.approved + assert po_translation.active + assert not po_translation.rejected + assert not suggestion.active + + +@pytest.mark.django_db +def test_upload_suggestions_deactivates_previous_suggestion( + project_locale_a, resource, po_translation, uploader +): + """As the newest suggestion, the uploaded one is shown instead of the previous.""" + po_translation.active = True + po_translation.save() + + result = _import(import_uploaded_suggestions, project_locale_a, resource, uploader) + + assert result.created == 1 + + po_translation.refresh_from_db() + suggestion = Translation.objects.get(string="new translation") + + assert suggestion.active + assert not po_translation.active + assert not po_translation.rejected + + +@pytest.mark.django_db +def test_upload_suggestions_drops_errors( + monkeypatch, project_locale_a, resource, po_translation, uploader +): + """A suggestion with errors is not stored, as the editor rejects it too.""" + monkeypatch.setattr( + sync_upload, "run_checks", _failing_checks({"pErrors": ["Test error"]}) + ) + + result = _import(import_uploaded_suggestions, project_locale_a, resource, uploader) + + assert result.created == 0 + assert result.failed_checks == [ + FailedCheck(key=("test_key",), errors=["Test error"], warnings=[]) + ] + assert not Translation.objects.filter(string="new translation").exists() + + +@pytest.mark.django_db +def test_upload_suggestions_stores_warnings( + monkeypatch, project_locale_a, resource, po_translation, uploader +): + """A suggestion with warnings is stored, with its warnings, for a reviewer to see.""" + monkeypatch.setattr( + sync_upload, "run_checks", _failing_checks({"pWarnings": ["Test warning"]}) + ) + + result = _import(import_uploaded_suggestions, project_locale_a, resource, uploader) + + assert result.created == 1 + assert result.failed_checks == [] + + suggestion = Translation.objects.get(string="new translation") + + assert [w.message for w in suggestion.warnings.all()] == ["Test warning"] + assert not suggestion.errors.exists() + + +@pytest.mark.django_db +def test_upload_suggestions_stores_missing_placeholder_warning( + project_locale_a, android_entity, uploader +): + """A dropped placeholder is only a warning, so the suggestion is still stored.""" + result = _import( + import_uploaded_suggestions, + project_locale_a, + android_entity.resource, + uploader, + upload=_android_upload_without_placeholder(), + ) + + assert result.created == 1 + assert result.failed_checks == [] + + suggestion = Translation.objects.get(entity=android_entity) + + assert [w.message for w in suggestion.warnings.all()] == [ + "Placeholder {$arg1} not found in translation" + ] + + +@pytest.mark.django_db +def test_upload_suggestions_unknown_keys_ignored( + project_locale_a, resource, po_translation, uploader +): + result = _import( + import_uploaded_suggestions, + project_locale_a, + resource, + uploader, + contents='msgid "test_key"\nmsgstr "new translation"\n\n' + 'msgid "no_such_key"\nmsgstr "x"\n', + ) + + assert result.created == 1 + assert result.undefined_keys == [("no_such_key",)] + + +@pytest.mark.django_db +def test_upload_suggestions_updates_stats_without_marking_changed( + project_locale_a, resource, untranslated_entity, uploader +): + """Suggestions are counted in stats, but never exported, so nothing is changed.""" + ChangedEntityLocale.objects.all().delete() + + _import( + import_uploaded_suggestions, + project_locale_a, + resource, + uploader, + contents='msgid "other_key"\nmsgstr "a suggestion"', + ) + + translated_resource = TranslatedResource.objects.get( + resource=resource, locale=project_locale_a.locale + ) + + # `po_translation` is an unreviewed suggestion of the other entity. + assert translated_resource.unreviewed_strings == 2 + assert translated_resource.approved_strings == 0 + assert not ChangedEntityLocale.objects.exists() + + +@pytest.mark.django_db +def test_upload_suggestions_updates_latest_translation( + project_locale_a, resource, untranslated_entity, uploader +): + """Latest activity is updated, as it would be by Translation.save().""" + _import( + import_uploaded_suggestions, + project_locale_a, + resource, + uploader, + contents='msgid "other_key"\nmsgstr "a suggestion"', + ) + + suggestion = Translation.objects.get(entity=untranslated_entity) + project_locale_a.refresh_from_db() + + assert ( + TranslatedResource.objects.get( + resource=resource, locale=project_locale_a.locale + ).latest_translation + == suggestion + ) + assert project_locale_a.latest_translation == suggestion + + +@pytest.mark.django_db +def test_upload_suggestions_restores_rejected_translation( + project_locale_a, resource, po_translation, uploader, admin +): + """A rejected translation matching the upload is un-rejected, not duplicated.""" + author = po_translation.user + date = po_translation.date + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + po_translation.reject(admin) + + result = _import(import_uploaded_suggestions, project_locale_a, resource, uploader) + + assert result.restored == 1 + assert result.created == 0 + assert result.unchanged == 0 + assert Translation.objects.filter(entity=po_translation.entity).count() == 1 + + po_translation.refresh_from_db() + + assert not po_translation.rejected + assert not po_translation.approved + assert not po_translation.pretranslated + assert not po_translation.fuzzy + assert po_translation.active + # The restored suggestion keeps its own author and date. + assert po_translation.user == author + assert po_translation.date == date + assert po_translation.unrejected_user == uploader + assert ActionLog.objects.filter( + performed_by=uploader, + action_type=ActionLog.ActionType.TRANSLATION_UNREJECTED, + translation=po_translation, + ).exists() + + +@pytest.mark.django_db +def test_upload_suggestions_restored_stays_behind_approved( + project_locale_a, resource, po_translation, uploader, admin +): + """Restoring a suggestion does not take the active slot from an approved one.""" + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + po_translation.reject(admin) + approved = TranslationFactory.create( + entity=po_translation.entity, + locale=project_locale_a.locale, + string="the approved one", + value=["the approved one"], + approved=True, + active=True, + ) + + result = _import(import_uploaded_suggestions, project_locale_a, resource, uploader) + + assert result.restored == 1 + + po_translation.refresh_from_db() + approved.refresh_from_db() + + assert not po_translation.rejected + assert not po_translation.active + assert approved.active + assert approved.approved + + +@pytest.mark.django_db +def test_upload_suggestions_restored_stays_behind_newer_suggestion( + project_locale_a, resource, po_translation, uploader, admin +): + """A restored suggestion older than the active one does not become active.""" + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + po_translation.reject(admin) + newer = TranslationFactory.create( + entity=po_translation.entity, + locale=project_locale_a.locale, + string="a newer suggestion", + value=["a newer suggestion"], + active=True, + date=po_translation.date + timedelta(days=1), + ) + + result = _import(import_uploaded_suggestions, project_locale_a, resource, uploader) + + assert result.restored == 1 + + po_translation.refresh_from_db() + newer.refresh_from_db() + + assert not po_translation.rejected + assert not po_translation.active + assert newer.active + + +@pytest.mark.django_db +def test_upload_suggestions_restore_counted_in_stats( + project_locale_a, resource, po_translation, uploader, admin +): + """A restored suggestion is unreviewed again, so it is counted in stats.""" + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + po_translation.reject(admin) + + translated_resource = TranslatedResource.objects.get( + resource=resource, locale=project_locale_a.locale + ) + + assert translated_resource.unreviewed_strings == 0 + + _import(import_uploaded_suggestions, project_locale_a, resource, uploader) + + translated_resource.refresh_from_db() + + assert translated_resource.unreviewed_strings == 1 + + +@pytest.mark.django_db +def test_upload_suggestions_does_not_restore_translation_with_errors( + monkeypatch, project_locale_a, resource, po_translation, uploader, admin +): + """A rejected translation that no longer passes checks stays rejected.""" + monkeypatch.setattr( + sync_upload, "run_checks", _failing_checks({"pErrors": ["Test error"]}) + ) + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + po_translation.reject(admin) + + result = _import(import_uploaded_suggestions, project_locale_a, resource, uploader) + + assert result.restored == 0 + assert result.failed_checks == [ + FailedCheck(key=("test_key",), errors=["Test error"], warnings=[]) + ] + + po_translation.refresh_from_db() + + assert po_translation.rejected + assert po_translation.unrejected_user is None + + +@pytest.mark.django_db +def test_upload_suggestions_refreshes_checks_of_restored_translation( + monkeypatch, project_locale_a, resource, po_translation, uploader, admin +): + """The checks stored for a restored translation are the ones just run for it.""" + monkeypatch.setattr( + sync_upload, "run_checks", _failing_checks({"pWarnings": ["Fresh warning"]}) + ) + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + po_translation.reject(admin) + # Checks stored when the translation was written, before they changed. + Warning.objects.create( + library="p", message="Stale warning", translation=po_translation + ) + Error.objects.create(library="p", message="Stale error", translation=po_translation) + + result = _import(import_uploaded_suggestions, project_locale_a, resource, uploader) + + assert result.restored == 1 + assert [w.message for w in po_translation.warnings.all()] == ["Fresh warning"] + assert not po_translation.errors.exists() + + +@pytest.mark.django_db +def test_upload_suggestions_conflicts_with_concurrent_approval( + monkeypatch, project_locale_a, resource, po_translation, uploader, admin +): + """A suggestion approved mid-import is not deactivated by the uploaded one.""" + po_translation.active = True + po_translation.save() + + _approve_during_import(monkeypatch, po_translation, admin) + _import_conflicts(import_uploaded_suggestions, project_locale_a, resource, uploader) + + # The import is rolled back, which also undoes the approval made inside it. + po_translation.refresh_from_db() + + assert po_translation.active + assert not po_translation.approved + assert Translation.objects.filter(entity=po_translation.entity).count() == 1 + + +@pytest.mark.django_db +@pytest.mark.parametrize("field", ["pretranslated", "fuzzy"]) +def test_upload_suggestions_conflicts_with_concurrent_precedence_change( + monkeypatch, project_locale_a, resource, po_translation, uploader, field +): + """A suggestion that gains precedence mid-import is not deactivated.""" + po_translation.active = True + po_translation.save() + + _review_during_import( + monkeypatch, + lambda: Translation.objects.filter(pk=po_translation.pk).update( + **{field: True} + ), + ) + _import_conflicts(import_uploaded_suggestions, project_locale_a, resource, uploader) + + # The import is rolled back, which also undoes the change made inside it. + po_translation.refresh_from_db() + + assert po_translation.active + assert not getattr(po_translation, field) + assert Translation.objects.filter(entity=po_translation.entity).count() == 1 + + +@pytest.mark.django_db +def test_upload_suggestions_conflicts_with_concurrent_rejection_of_approved( + monkeypatch, project_locale_a, resource, po_translation, uploader, admin +): + """The approved translation an inactive suggestion defers to is rejected mid-import.""" + po_translation.approved = True + po_translation.active = True + po_translation.save() + + _review_during_import(monkeypatch, lambda: po_translation.reject(admin)) + _import_conflicts(import_uploaded_suggestions, project_locale_a, resource, uploader) + + # The import is rolled back, which also undoes the rejection made inside it. + po_translation.refresh_from_db() + + assert po_translation.approved + assert po_translation.active + assert Translation.objects.filter(entity=po_translation.entity).count() == 1 + + +@pytest.mark.django_db +def test_upload_suggestions_conflicts_with_concurrent_unrejection( + monkeypatch, project_locale_a, resource, po_translation, uploader, admin +): + """A rejected translation un-rejected mid-import is not restored a second time.""" + po_translation.string = "new translation" + po_translation.value = ["new translation"] + po_translation.save() + po_translation.reject(admin) + + _review_during_import(monkeypatch, lambda: po_translation.unreject(admin)) + _import_conflicts(import_uploaded_suggestions, project_locale_a, resource, uploader) + + # The import is rolled back, which also undoes the un-rejection made inside it. + po_translation.refresh_from_db() + + assert po_translation.rejected + assert po_translation.unrejected_user is None + assert not ActionLog.objects.filter( + action_type=ActionLog.ActionType.TRANSLATION_UNREJECTED, + translation=po_translation, + ).exists() + + +@pytest.mark.django_db +def test_upload_suggestions_conflicts_with_concurrent_deletion( + monkeypatch, project_locale_a, resource, po_translation, uploader +): + """A suggestion deleted mid-import, which the upload would deactivate, is a conflict.""" + po_translation.active = True + po_translation.save() + + _review_during_import(monkeypatch, lambda: po_translation.delete()) + _import_conflicts(import_uploaded_suggestions, project_locale_a, resource, uploader) + + assert not Translation.objects.filter(string="new translation").exists() + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "importer", [import_uploaded_pretranslations, import_uploaded_suggestions] +) +def test_upload_locks_target_before_reading_translations( + importer, project_locale_a, resource, po_translation, uploader +): + """The lock that serializes concurrent imports is taken before the read it guards.""" + with CaptureQueriesContext(connection) as queries: + _import(importer, project_locale_a, resource, uploader) + + statements = [query["sql"] for query in queries.captured_queries] + lock = next( + i + for i, sql in enumerate(statements) + if "base_translatedresource" in sql and "FOR UPDATE" in sql + ) + read = next( + i for i, sql in enumerate(statements) if 'FROM "base_translation"' in sql + ) + + assert lock < read diff --git a/pontoon/sync/upload.py b/pontoon/sync/upload.py index decbff7516..5f7ef13695 100644 --- a/pontoon/sync/upload.py +++ b/pontoon/sync/upload.py @@ -1,7 +1,10 @@ +from collections.abc import Iterable from dataclasses import dataclass, field +from datetime import datetime from itertools import groupby from os.path import basename, join from tempfile import TemporaryDirectory +from typing import NamedTuple from moz.l10n.model import Id as L10nId from moz.l10n.resource import parse_resource @@ -17,10 +20,12 @@ Locale, Project, Resource as DbResource, + TranslatedResource, Translation, User, ) from pontoon.checks.libraries import run_checks +from pontoon.checks.utils import are_blocking_checks, get_failed_checks_db_objects from pontoon.sync.core.stats import update_stats from pontoon.sync.core.translations_from_repo import ( Updates, @@ -162,6 +167,294 @@ class FailedCheck: errors: list[str] warnings: list[str] + @classmethod + def from_check_results( + cls, key: L10nId, results: dict[str, list[str]] + ) -> "FailedCheck": + """The key and the messages of a `run_checks()` result, split by severity.""" + return cls( + key=key, + errors=[m for g, ms in results.items() if g.endswith("Errors") for m in ms], + warnings=[ + m for g, ms in results.items() if g.endswith("Warnings") for m in ms + ], + ) + + +def lock_import_target(db_res: DbResource, locale: Locale) -> None: + """ + Serialize imports targeting the same resource and locale. + + Two concurrent imports of the same target could each read a string's + translations, find no match, and both insert the same one. Neither insert is + caught by `lock_read_translations()`, which only re-checks rows that existed when + they were read. Taking this lock before reading closes that gap: a second import + blocks until the first commits, and then reads what it wrote. + + The lock is taken on the target's `TranslatedResource` row rather than on its + entities. That row is known to exist for any upload, `update_stats()` writes it + later in the same transaction anyway, and it is specific to the locale, so + imports of the same resource in other locales are not held back. + + Must run inside a transaction; the lock is held until it ends. + """ + list( + TranslatedResource.objects.select_for_update() + .filter(resource=db_res, locale=locale) + .values_list("pk", flat=True) + ) + + +def translations_by_entity( + locale: Locale, entity_ids: Iterable[int], *, include_rejected: bool +) -> dict[int, list[Translation]]: + """The locale's translations of the given entities, keyed by entity id.""" + translations = Translation.objects.filter(entity_id__in=entity_ids, locale=locale) + if not include_rejected: + translations = translations.filter(rejected=False) + return { + entity_id: list(txs) + for entity_id, txs in groupby( + translations.order_by("entity_id").iterator(), + key=lambda tx: tx.entity_id, + ) + } + + +class ReviewState(NamedTuple): + """ + The translation fields used to make a decision on how to treat imported + translations and to detect concurrent changes. + """ + + approved: bool + pretranslated: bool + fuzzy: bool + rejected: bool + active: bool + + @classmethod + def of(cls, tx: Translation) -> "ReviewState": + """The review state of `tx`.""" + return cls._make(getattr(tx, f) for f in cls._fields) + + +def read_state(translations: list[Translation]) -> dict[int, ReviewState]: + """The review state of each translation, keyed by translation id.""" + return {tx.pk: ReviewState.of(tx) for tx in translations} + + +@dataclass +class PendingChange: + """ + Changes staged for one entity, applied only once its checks are known: + - `new`: a translation to create, if the upload matches none Pontoon has + - `match`: the translation the upload matches, if Pontoon already has it + - `match_action`: the action to log for `match`, if any + - `reject_ids`: ids of the translations to reject + - `deactivate_ids`: ids of the translations to leave in place, but deactivate + - `read_state`: the review state of the entity's translations as read when the + change was decided, keyed by translation id + - `check_results`: the `run_checks()` result of the uploaded translation + """ + + key: L10nId + read_state: dict[int, ReviewState] + new: Translation | None = None + match: Translation | None = None + match_action: str | None = None + reject_ids: list[int] = field(default_factory=list) + deactivate_ids: list[int] = field(default_factory=list) + check_results: dict[str, list[str]] = field(default_factory=dict) + + @property + def translation(self) -> Translation: + """The row holding the uploaded translation, whether created or matched.""" + return self.new or self.match + + +def lock_read_translations(applied: list[PendingChange]) -> None: + """ + Lock the translations the staged changes were decided on, and check that none + has changed since it was read. + + Acquiring the lock waits for any concurrent transaction on these rows to commit, + so the state re-read here is the one the changes are written over. Comparing it + with the state read earlier catches a review or deletion that happened in between. + + Translations another import inserted in the meantime are not among these rows; + `lock_import_target()` keeps such imports from overlapping in the first place. + + Raises `UploadConflictError` if the review state of any translation differs from + the one read, or if one was deleted. + """ + expected = { + pk: state for change in applied for pk, state in change.read_state.items() + } + if not expected: + return + locked = ( + Translation.objects.select_for_update() + .filter(pk__in=expected) + .values_list("pk", *ReviewState._fields) + ) + current = {pk: ReviewState(*state) for pk, *state in locked} + if len(current) != len(expected) or any( + current[pk] != state for pk, state in expected.items() + ): + raise UploadConflictError() + + +def run_staged_checks( + pending: list[PendingChange], db_res: DbResource, locale: Locale +) -> None: + """ + Run the quality checks on each staged translation, recording the results on its + `PendingChange` as `check_results`. + + Checks run before anything is written, so that a translation failing them + can be left out, keeping in place the translation it would have replaced. + + The results are kept in memory rather than read back from the database + after the write, as `run_checks()` also reports checks from libraries that + `bulk_run_checks()` does not store. + """ + if not pending: + return + + entities = { + entity.pk: entity + for entity in Entity.objects.filter( + pk__in={change.translation.entity_id for change in pending} + ) + } + if db_res.format == DbResource.Format.DTD: + # compare-locales needs the other entities of the resource as a reference, + # and reloads them for each check unless they are cached on `db_res`. + prefetch_related_objects([db_res], "entities") + + for change in pending: + entity = entities[change.translation.entity_id] + entity.resource = db_res + change.check_results = run_checks( + entity, locale.code, change.translation.string, False + ) + + +def write_changes( + project: Project, + user: User, + now: datetime, + applied: list[PendingChange], + *, + match_fields: tuple[str, ...], + mark_changed: bool, +) -> None: + """ + Write the staged changes to the database, along with their check results, + action log entries and stats. Must run inside a transaction. + + `match_fields` are the only fields saved for matched translations. + `mark_changed` marks the written translations as changed for sync. + + Raises `UploadConflictError` if another transaction reviewed or deleted a + translation the changes were decided on after this import read it. + """ + from pontoon.checks.models import Error, Warning + + lock_read_translations(applied) + + reject_ids = [pk for change in applied for pk in change.reject_ids] + deactivate_ids = [pk for change in applied for pk in change.deactivate_ids] + matched = [change.match for change in applied if change.match is not None] + created = [change.new for change in applied if change.new is not None] + + actions: list[ActionLog] = [] + # Rejections and deactivations must be written before translations are activated, + # to keep a single active translation per entity and locale. + if reject_ids: + rejected = Translation.objects.filter(pk__in=reject_ids) + actions.extend( + ActionLog( + action_type=ActionLog.ActionType.TRANSLATION_REJECTED, + created_at=now, + performed_by=user, + translation=tx, + is_implicit_action=True, + ) + for tx in rejected + ) + # Only approved translations have TM entries, so there are none to remove here. + rejected.update( + active=False, + rejected=True, + rejected_user=user, + rejected_date=now, + pretranslated=False, + fuzzy=False, + ) + if deactivate_ids: + Translation.objects.filter(pk__in=deactivate_ids).update(active=False) + if matched: + Translation.objects.bulk_update(matched, list(match_fields)) + if created: + Translation.objects.bulk_create(created) + + actions.extend( + ActionLog( + action_type=ActionLog.ActionType.TRANSLATION_CREATED, + created_at=now, + performed_by=user, + translation=tx, + ) + for tx in created + ) + actions.extend( + ActionLog( + action_type=change.match_action, + created_at=now, + performed_by=user, + translation=change.match, + ) + for change in applied + if change.match is not None and change.match_action is not None + ) + if actions: + ActionLog.objects.bulk_create(actions) + + # Failed checks must be stored before stats are updated (bug 1521606). + matched_ids = [tx.pk for tx in matched] + if matched_ids: + # A matched translation's stored checks date from when it was written; the + # source string or the checks may have changed since. Replace them with the + # results just computed. + Warning.objects.filter(translation_id__in=matched_ids).delete() + Error.objects.filter(translation_id__in=matched_ids).delete() + warnings, errors = [], [] + for change in applied: + if change.check_results: + translation_warnings, translation_errors = get_failed_checks_db_objects( + change.translation, change.check_results + ) + warnings += translation_warnings + errors += translation_errors + if warnings: + Warning.objects.bulk_create(warnings) + if errors: + Error.objects.bulk_create(errors) + + if created: + # bulk_create() skips Translation.save(), which would do this + created[0].update_latest_translation() + + changed_pks = [tx.pk for tx in created + matched] + if changed_pks or reject_ids: + update_stats(project) + if mark_changed: + Translation.objects.filter( + pk__in=reject_ids + changed_pks + ).bulk_mark_changed() + @dataclass class PretranslationUploadResult: @@ -190,23 +483,6 @@ class PretranslationUploadResult: undefined_keys: list[L10nId] = field(default_factory=list) -@dataclass -class _PendingPretranslation: - """Changes staged for one entity, applied only if its checks pass.""" - - key: L10nId - match: Translation | None - new: Translation | None - reject_ids: list[int] - deactivate_ids: list[int] - replaces_translation: bool - - @property - def translation(self) -> Translation: - """The row holding the uploaded translation, whether created or matched.""" - return self.new or self.match - - def import_uploaded_pretranslations( project: Project, locale: Locale, @@ -230,8 +506,9 @@ def import_uploaded_pretranslations( translation never replaces a good one. A new pretranslation is not stored, a matching one is not converted, and the previous translation stays in place. - Raises `UploadConflictError` if a review approved a targeted translation after this - import read it, and that approval was committed first. + Must run inside a transaction. Raises `UploadConflictError` if a translation of a + targeted string was reviewed or deleted after this import read it, and that change + was committed first. This does not reuse `update_db_translations()`, which has the same overall shape, but makes a different call at nearly every decision point: @@ -248,24 +525,13 @@ def import_uploaded_pretranslations( upload_translations, entities, result.undefined_keys = parse_upload_for_entities( locale, db_res, upload ) - - current: dict[int, list[Translation]] = { - entity_id: list(txs) - for entity_id, txs in groupby( - Translation.objects.filter( - entity__resource=db_res, - entity__obsolete=False, - locale=locale, - rejected=False, - ) - .order_by("entity_id") - .iterator(), - key=lambda tx: tx.entity_id, - ) - } + lock_import_target(db_res, locale) + current = translations_by_entity( + locale, [entities[key] for key in upload_translations], include_rejected=False + ) now = timezone.now() - pending: list[_PendingPretranslation] = [] + pending: list[PendingChange] = [] for key, rt in upload_translations.items(): entity_id = entities[key] translations = current.get(entity_id, []) @@ -285,148 +551,172 @@ def import_uploaded_pretranslations( result.unchanged += 1 continue - reject_ids: list[int] = [] - deactivate_ids: list[int] = [] + change = PendingChange( + key=key, read_state=read_state(translations), match=match + ) for tx in translations: if tx is match: continue if tx.pretranslated or tx.fuzzy: - reject_ids.append(tx.pk) + change.reject_ids.append(tx.pk) elif tx.active: - deactivate_ids.append(tx.pk) + change.deactivate_ids.append(tx.pk) - new = None if match is None: - new = build_translation(rt, entity_id, locale.pk, user, now) - new.pretranslated = True - new.active = True - pending.append( - _PendingPretranslation( - key=key, - match=match, - new=new, - reject_ids=reject_ids, - deactivate_ids=deactivate_ids, - replaces_translation=any( - tx.pretranslated or tx.fuzzy for tx in translations - ), - ) - ) + change.new = build_translation(rt, entity_id, locale.pk, user, now) + change.new.pretranslated = True + change.new.active = True + pending.append(change) - # Checks run on the staged translations, before anything is written: a failing - # upload must not have discarded the translation it would replace. `run_checks()` - # reports more than `bulk_run_checks()` stores, as only some libraries are saved - # to the database, so its result is used here rather than the stored rows. - entities_by_id = { - entity.pk: entity - for entity in Entity.objects.filter( - pk__in={p.translation.entity_id for p in pending} - ) - } - if db_res.format == DbResource.Format.DTD: - # compare-locales needs the other entities of the resource as a reference, - # and reloads them for each check unless they are cached on `db_res`. - prefetch_related_objects([db_res], "entities") + run_staged_checks(pending, db_res, locale) - applied: list[_PendingPretranslation] = [] - for p in pending: - entity = entities_by_id[p.translation.entity_id] - entity.resource = db_res - failed = run_checks(entity, locale.code, p.translation.string, False) - if failed: + applied: list[PendingChange] = [] + for change in pending: + if are_blocking_checks(change.check_results, ignore_warnings=False): result.failed_checks.append( - FailedCheck( - key=p.key, - errors=[m for g, ms in failed.items() if "Errors" in g for m in ms], - warnings=[ - m for g, ms in failed.items() if "Warnings" in g for m in ms - ], - ) + FailedCheck.from_check_results(change.key, change.check_results) ) else: - applied.append(p) + applied.append(change) - for p in applied: - if p.match is not None: + for change in applied: + if change.match is not None: result.converted += 1 - p.match.pretranslated = True - p.match.fuzzy = False - p.match.active = True - elif p.replaces_translation: + change.match.pretranslated = True + change.match.fuzzy = False + change.match.active = True + elif change.reject_ids: result.replaced += 1 else: result.created += 1 - reject_ids = [pk for p in applied for pk in p.reject_ids] - deactivate_ids = [pk for p in applied for pk in p.deactivate_ids] - converted_translations = [p.match for p in applied if p.match is not None] - new_translations = [p.new for p in applied if p.new is not None] + write_changes( + project, + user, + now, + applied, + match_fields=("active", "fuzzy", "pretranslated"), + mark_changed=True, + ) + return result - actions: list[ActionLog] = [] - # Rejections and deactivations must be written before translations are activated, - # to keep a single active translation per entity and locale. - if reject_ids: - rejected = Translation.objects.filter(pk__in=reject_ids) - actions.extend( - ActionLog( - action_type=ActionLog.ActionType.TRANSLATION_REJECTED, - created_at=now, - performed_by=user, - translation=tx, - is_implicit_action=True, - ) - for tx in rejected - ) - # Only approved translations have TM entries, so there are none to remove here. - rejected.update( - active=False, - rejected=True, - rejected_user=user, - rejected_date=now, - pretranslated=False, - fuzzy=False, - ) - if deactivate_ids: - Translation.objects.filter(pk__in=deactivate_ids).update(active=False) - if converted_translations: - Translation.objects.bulk_update( - converted_translations, ["active", "fuzzy", "pretranslated"] - ) - if new_translations: - Translation.objects.bulk_create(new_translations) - - # A review may approve or reject one of these translations after `current` was - # loaded; abort if it did, rolling back the upload. A rejection is only a conflict - # for converted translations, as the other rejected rows were rejected above. - converted_ids = [tx.pk for tx in converted_translations] - modified_ids = reject_ids + deactivate_ids + converted_ids - if ( - modified_ids - and Translation.objects.filter( - Q(pk__in=modified_ids, approved=True) - | Q(pk__in=converted_ids, rejected=True) - ).exists() - ): - raise UploadConflictError() - actions.extend( - ActionLog( - action_type=ActionLog.ActionType.TRANSLATION_CREATED, - created_at=now, - performed_by=user, - translation=tx, - ) - for tx in new_translations +@dataclass +class SuggestionUploadResult: + """ + Summary of an uploaded suggestion file import: + - `created`: number of suggestions added + - `restored`: number of rejected translations matching the upload that were + un-rejected, becoming pending suggestions again + - `unchanged`: number of uploaded translations that the string already has as an + unrejected translation, in any review state + - `failed_checks`: keys of the strings left untouched because the uploaded + translation has errors, with the errors and warnings reported for each of them + - `undefined_keys`: keys of translations with no matching entity in Pontoon + """ + + created: int = 0 + restored: int = 0 + unchanged: int = 0 + failed_checks: list[FailedCheck] = field(default_factory=list) + undefined_keys: list[L10nId] = field(default_factory=list) + + +def import_uploaded_suggestions( + project: Project, + locale: Locale, + db_res: DbResource, + upload: File, + user: User, +) -> SuggestionUploadResult: + """ + Store translations from an uploaded file in the database as unreviewed suggestions. + + Nothing already in Pontoon is replaced or rejected: every uploaded translation is + stored as a suggestion, unless the string already has an unrejected translation + with the same value, whether approved, pretranslated or unreviewed. The fuzzy flag + of the uploaded file is ignored: the translation is stored as a plain suggestion. + + A rejected translation matching the upload is un-rejected instead of suggested + again, so that a translation rejected by mistake can be re-proposed. + + Uploaded translations reported with errors are left out, as the editor rejects them + as well. Translations with warnings are stored, with their warnings, as a reviewer + can still accept them. + + Must run inside a transaction. Raises `UploadConflictError` if a translation of a + targeted string was reviewed or deleted after this import read it, and that change + was committed first. + """ + result = SuggestionUploadResult() + upload_translations, entities, result.undefined_keys = parse_upload_for_entities( + locale, db_res, upload + ) + lock_import_target(db_res, locale) + # Rejected translations are read as well, to be restored rather than duplicated. + current = translations_by_entity( + locale, [entities[key] for key in upload_translations], include_rejected=True ) - if actions: - ActionLog.objects.bulk_create(actions) - if new_translations: - # bulk_create() skips Translation.save(), which would do this - new_translations[0].update_latest_translation() + now = timezone.now() + pending: list[PendingChange] = [] + for key, rt in upload_translations.items(): + entity_id = entities[key] + translations = current.get(entity_id, []) + matches = [ + tx + for tx in translations + if translations_equal(rt.value, rt.properties, tx.value, tx.properties) + ] + if any(not tx.rejected for tx in matches): + result.unchanged += 1 + continue - changed_pks = [tx.pk for tx in new_translations + converted_translations] - if changed_pks or reject_ids: - update_stats(project) - Translation.objects.filter(pk__in=reject_ids + changed_pks).bulk_mark_changed() + change = PendingChange(key=key, read_state=read_state(translations)) + if matches: + # Restore the most recent match, preserving its original author and date. + change.match = max(matches, key=lambda tx: tx.date) + change.match.rejected = False + change.match.unrejected_user = user + change.match.unrejected_date = now + change.match_action = ActionLog.ActionType.TRANSLATION_UNREJECTED + suggestion = change.match + else: + change.new = build_translation(rt, entity_id, locale.pk, user, now) + suggestion = change.new + + # The suggestion is the active translation of its string unless another + # unrejected translation takes precedence. + others = [ + tx for tx in translations if tx is not change.match and not tx.rejected + ] + suggestion.active = not any( + tx.approved or tx.pretranslated or tx.fuzzy or tx.date > suggestion.date + for tx in others + ) + if suggestion.active: + change.deactivate_ids = [tx.pk for tx in others if tx.active] + pending.append(change) + + run_staged_checks(pending, db_res, locale) + + applied: list[PendingChange] = [] + for change in pending: + if are_blocking_checks(change.check_results, ignore_warnings=True): + result.failed_checks.append( + FailedCheck.from_check_results(change.key, change.check_results) + ) + else: + applied.append(change) + result.restored = sum(1 for change in applied if change.match is not None) + result.created = len(applied) - result.restored + + write_changes( + project, + user, + now, + applied, + match_fields=("active", "rejected", "unrejected_user", "unrejected_date"), + mark_changed=False, + ) return result