Allow default_locale to be set/changed via the API (#19290)

* Allow default_locale to be set/changed via the API

* rewrite some of the test comments to make it clearer what's going on
This commit is contained in:
Andrew Williamson 2022-05-27 13:05:40 +01:00 коммит произвёл GitHub
Родитель 72a92635cd
Коммит cc2fadf6b3
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
12 изменённых файлов: 330 добавлений и 49 удалений

Просмотреть файл

@ -311,6 +311,7 @@ is compatible with.
:<json object categories: Object holding the categories the add-on belongs to.
:<json array categories[app_name]: Array holding the :ref:`category slugs <category-list>` the add-on belongs to for a given :ref:`add-on application <addon-detail-application>`.
:<json string contributions_url: URL to the (external) webpage where the addon's authors collect monetary contributions. Only a limited number of services are `supported <https://github.com/mozilla/addons-server/blob/0b5db7d544a21f6b887e8e8032496778234ade33/src/olympia/constants/base.py#L214:L226>`_.
:<json string default_locale: The fallback locale for translated fields for this add-on. Note this only applies to the fields here - the default locale for :ref:`version release notes <version-create-request>` and custom license text is fixed to `en-US`.
:<json object|null description: The add-on description (See :ref:`translated fields <api-overview-translations>`). This field can contain some HTML tags.
:<json object|null developer_comments: Additional information about the add-on. (See :ref:`translated fields <api-overview-translations>`).
:<json object|null homepage: The add-on homepage (See :ref:`translated fields <api-overview-translations>` and :ref:`Outgoing Links <api-overview-outgoing>`).
@ -342,6 +343,7 @@ This endpoint allows an add-on's AMO metadata to be edited.
:<json object categories: Object holding the categories the add-on belongs to.
:<json array categories[app_name]: Array holding the :ref:`category slugs <category-list>` the add-on belongs to for a given :ref:`add-on application <addon-detail-application>`.
:<json string contributions_url: URL to the (external) webpage where the addon's authors collect monetary contributions. Only a limited number of services are `supported <https://github.com/mozilla/addons-server/blob/0b5db7d544a21f6b887e8e8032496778234ade33/src/olympia/constants/base.py#L214:L226>`_.
:<json string default_locale: The fallback locale for translated fields for this add-on. Note this only applies to the fields here - the default locale for :ref:`version release notes <version-create-request>` and custom license text is fixed to `en-US`.
:<json object|null description: The add-on description (See :ref:`translated fields <api-overview-translations>`). This field can contain some HTML tags.
:<json object|null developer_comments: Additional information about the add-on. (See :ref:`translated fields <api-overview-translations>`).
:<json object|null homepage: The add-on homepage (See :ref:`translated fields <api-overview-translations>` and :ref:`Outgoing Links <api-overview-outgoing>`).

Просмотреть файл

@ -442,6 +442,7 @@ These are `v5` specific changes - `v4` changes apply also.
* 2022-05-05: added the ability to list and edit add-on authors. https://github.com/mozilla/addons-server/issues/18231
* 2022-05-05: added the ability to delete add-on authors. https://github.com/mozilla/addons-server/issues/19163
* 2022-05-12: added the ability to add new add-on authors, as pending authors. https://github.com/mozilla/addons-server/issues/19164
* 2022-06-02: enabled setting ``default_locale`` via addon submission and edit endpoints. https://github.com/mozilla/addons-server/issues/18235
.. _`#11380`: https://github.com/mozilla/addons-server/issues/11380/
.. _`#11379`: https://github.com/mozilla/addons-server/issues/11379/

Просмотреть файл

@ -145,8 +145,12 @@ class LicenseNameSerializerField(serializers.Field):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.builtin_translation_field = self.builtin_translation_field_class()
self.custom_translation_field = self.custom_translation_field_class()
self.builtin_translation_field = self.builtin_translation_field_class(
*args, **kwargs
)
self.custom_translation_field = self.custom_translation_field_class(
*args, **kwargs
)
def bind(self, field_name, parent):
super().bind(field_name, parent)

Просмотреть файл

@ -933,7 +933,9 @@ class Addon(OnChangeMixin, ModelBase):
return addon
@classmethod
def resolve_webext_translations(cls, data, upload):
def resolve_webext_translations(
cls, data, upload, use_default_locale_fallback=True
):
"""Resolve all possible translations from an add-on.
This returns a modified `data` dictionary accordingly with proper
@ -955,15 +957,26 @@ class Addon(OnChangeMixin, ModelBase):
if isinstance(data[field], dict):
# if the field value is already a localized set of values don't override
continue
data[field] = {
locale: resolve_i18n_message(
data[field],
locale=locale,
default_locale=default_locale,
messages=messages,
)
for locale in messages
}
if messages:
data[field] = {
locale: value
for locale in messages
if (
value := resolve_i18n_message(
data[field],
locale=locale,
default_locale=use_default_locale_fallback
and default_locale,
messages=messages,
)
)
is not None
}
else:
# If we got a default_locale but no messages then the default_locale has
# been set via the serializer for a non-localized xpi, so format data
# correctly so the manifest values are assigned the correct locale.
data[field] = {default_locale: data[field]}
return data

Просмотреть файл

@ -39,6 +39,7 @@ from olympia.constants.applications import APPS_ALL, APP_IDS
from olympia.constants.base import ADDON_TYPE_CHOICES_API
from olympia.constants.categories import CATEGORIES_BY_ID
from olympia.constants.promoted import PROMOTED_GROUPS, RECOMMENDED
from olympia.core.languages import AMO_LANGUAGES
from olympia.files.models import File, FileUpload
from olympia.files.utils import parse_addon
from olympia.promoted.models import PromotedAddon
@ -77,8 +78,9 @@ from .tasks import resize_icon, resize_preview
from .utils import fetch_translations_from_addon
from .validators import (
AddonMetadataValidator,
AddonMetadataNewVersionValidator,
ValidateVersionLicense,
AddonDefaultLocaleValidator,
VersionAddonMetadataValidator,
VersionLicenseValidator,
VerifyMozillaTrademark,
)
@ -231,8 +233,8 @@ class ESPreviewSerializer(BaseESSerializer, PreviewSerializer):
class LicenseSerializer(serializers.ModelSerializer):
is_custom = serializers.SerializerMethodField()
name = LicenseNameSerializerField()
text = TranslationSerializerField()
name = LicenseNameSerializerField(allow_null=True)
text = TranslationSerializerField(allow_null=True)
url = serializers.SerializerMethodField()
slug = serializers.ReadOnlyField()
@ -393,7 +395,7 @@ class DeveloperVersionSerializer(VersionSerializer):
class Meta:
model = Version
validators = (ValidateVersionLicense(), AddonMetadataNewVersionValidator())
validators = (VersionLicenseValidator(), VersionAddonMetadataValidator())
fields = (
'id',
'channel',
@ -800,6 +802,9 @@ class AddonSerializer(serializers.ModelSerializer):
source='contributions', required=False
)
current_version = CurrentVersionSerializer(read_only=True)
default_locale = serializers.ChoiceField(
choices=list(AMO_LANGUAGES), required=False
)
description = TranslationSerializerField(required=False)
developer_comments = TranslationSerializerField(required=False)
edit_url = serializers.SerializerMethodField()
@ -856,14 +861,14 @@ class AddonSerializer(serializers.ModelSerializer):
url = serializers.SerializerMethodField()
version = DeveloperVersionSerializer(
write_only=True,
# Note: we're purposefully omitting AddonMetadataNewVersionValidator
validators=(CreateOnlyValidator(), ValidateVersionLicense()),
# Note: we're purposefully omitting VersionAddonMetadataValidator
validators=(CreateOnlyValidator(), VersionLicenseValidator()),
)
versions_url = serializers.SerializerMethodField()
class Meta:
model = Addon
validators = (AddonMetadataValidator(),)
validators = (AddonMetadataValidator(), AddonDefaultLocaleValidator())
fields = (
'id',
'authors',
@ -910,6 +915,7 @@ class AddonSerializer(serializers.ModelSerializer):
writeable_fields = (
'categories',
'contributions_url',
'default_locale',
'description',
'developer_comments',
'homepage',

Просмотреть файл

@ -60,6 +60,7 @@ from olympia.files.utils import parse_addon, parse_xpi
from olympia.files.tests.test_models import UploadMixin
from olympia.ratings.models import Rating
from olympia.tags.models import Tag
from olympia.translations.models import Translation
from olympia.users.models import EmailUserRestriction, UserProfile
from olympia.versions.models import (
ApplicationsVersions,
@ -940,7 +941,7 @@ class TestAddonViewSetCreate(UploadMixin, AddonViewSetCreateUpdateMixin, TestCas
self.url,
data={
'summary': {'en-US': 'replacement summary'},
'name': {'en-US': None}, # None should be ignored
'name': {}, # will override the name in the manifest
'version': {
'upload': self.upload.uuid,
'license': self.license.slug,
@ -971,8 +972,8 @@ class TestAddonViewSetCreate(UploadMixin, AddonViewSetCreateUpdateMixin, TestCas
)
assert response.status_code == 400, response.content
assert response.data == {
'name': ['This field is required for add-ons with listed versions.'],
'summary': ['This field is required for add-ons with listed versions.'],
'name': ['This field may not be null.'],
'summary': ['This field may not be null.'],
}
def test_not_authenticated(self):
@ -1365,6 +1366,119 @@ class TestAddonViewSetCreate(UploadMixin, AddonViewSetCreateUpdateMixin, TestCas
addon = Addon.objects.get()
assert [tag.tag_text for tag in addon.tags.all()] == ['music', 'zoom']
def test_default_locale_with_invalid_locale(self):
response = self.client.post(
self.url,
data={**self.minimal_data, 'default_locale': 'zz'},
)
assert response.status_code == 400
assert response.data == {'default_locale': ['"zz" is not a valid choice.']}
def test_default_locale(self):
# An xpi without localization - the values are in the manifest directly so will
# be intepretted as whatever locale is specified as the default locale.
response = self.client.post(
self.url,
data={
**self.minimal_data,
'default_locale': 'fr',
# the field will have a translation in de, but won't have a value in fr
'description': {'de': 'Das description'},
},
)
assert response.status_code == 400, response.data
error_string = 'A value in the default locale of "fr" is required.'
assert response.data == {
'description': [error_string],
}
# success cases, all tested with the post request with the different fields
# A field is provided with a value in new default
# B field already has a value in new default
# C field has no other translations
response = self.client.post(
self.url,
data={
**self.minimal_data,
'default_locale': 'fr',
'name': {'fr': 'nom française'}, # A - a value in fr
# B no summary provided, but has a value in the manifest already
# C no description and doesn't have other translations
},
)
assert response.status_code == 201, response.data
addon = Addon.objects.get()
assert addon.default_locale == 'fr'
# from the postdata
assert addon.name == 'nom française'
assert addon.name.locale == 'fr'
# summary value is from the manifest
assert addon.summary == 'just a test addon with the manifest.json format'
assert addon.summary.locale == 'fr'
# and there is no description either in the manifest or provided in post
assert addon.description is None
def test_default_locale_localized_xpi(self):
# This xpi has localized values in the xpi, but has been crafted to not have a
# name translation for de, which is valid if the default_locale is another lang,
# but won't be valid if the default_locale is de.
upload = self.get_upload(
'notify-link-clicks-i18n-missing.xpi',
user=self.user,
source=amo.UPLOAD_SOURCE_ADDON_API,
channel=amo.RELEASE_CHANNEL_UNLISTED,
)
# failure cases:
# A field doesn't have a value in the xpi in new default, or
# B field has other translations provided
response = self.client.post(
self.url,
data={
'version': {'upload': upload.uuid},
'default_locale': 'de',
# A no name provided for de, and our xpi is missing name in de
'support_url': {'it': 'https://it.support.test/'}, # B
},
)
assert response.status_code == 400, response.data
error_string = 'A value in the default locale of "de" is required.'
assert response.data == {
'name': [error_string],
'support_url': [error_string],
}
# success cases, all tested with the post request with the different fields:
# A field is provided with a value in new default
# B field already has a value in new default
# C field isn't required and has no other translations
response = self.client.post(
self.url,
data={
'version': {'upload': upload.uuid},
'default_locale': 'de',
'name': {'de': 'Das Name'}, # A
# B no summary provided, but the xpi already has a translation in de
# C no support_url provided and there aren't other translations
},
)
assert response.status_code == 201, response.data
with self.activate('fr'): # a locale the xpi doesn't have so we get defaults
addon = Addon.objects.get()
assert addon.default_locale == 'de'
# from the postdata
assert addon.name == 'Das Name'
assert addon.name.locale == 'de'
# summary is from the xpi translation json files
assert addon.summary == 'Benachrichtigt den Benutzer über Linkklicks'
assert addon.summary.locale == 'de'
# and there is no description either in the xpi, manifest or provided in post
assert addon.description is None
# homepage is defined directly in the manifest, and is not localized, so just
# testing the mix of translated and not translated is working as expected
assert str(addon.homepage).startswith('https://github.com/mdn/')
assert addon.homepage.locale == 'de'
class TestAddonViewSetCreateJWTAuth(TestAddonViewSetCreate):
client_class = APITestClientJWT
@ -1976,6 +2090,59 @@ class TestAddonViewSetUpdate(AddonViewSetCreateUpdateMixin, TestCase):
response = self.client.patch(self.url, data=data)
assert response.status_code == 200
def test_default_locale(self):
response = self.client.patch(self.url, data={'default_locale': 'zz'})
assert response.status_code == 400
assert response.data == {'default_locale': ['"zz" is not a valid choice.']}
Translation.objects.create(
id=self.addon.summary_id, locale='fr', localized_string='summary Française'
)
self.addon.description = 'description!'
self.addon.save()
# failure cases:
# A field is required and doesn't already have a value in new default
# B field is set to None in update
# C field isn't required, but has other translations already
# D field isn't required, but has other translations provided
response = self.client.patch(
self.url,
data={
'default_locale': 'fr',
# A no name, doesn't have a value in fr
'summary': {'fr': None}, # B summary has a value, but None would clear
# C no description, has a value in en-US already
'support_url': {'de': 'https://de.support.test/'}, # D
},
)
assert response.status_code == 400, response.data
error_string = 'A value in the default locale of "fr" is required.'
assert response.data == {
'name': [error_string],
'summary': [error_string],
'description': [error_string],
'support_url': [error_string],
}
self.addon.update(description=None)
# success cases - tested with different fields in the patch request:
# A field is provided with a value in new default in the postdata
# B field already has a value in new default - we created the Translation above
# C field isn't required and has no other translations - we set description=None
response = self.client.patch(
self.url,
data={
'default_locale': 'fr',
'name': {'fr': 'nom française'}, # A
# B no summary, but does have a value in fr already
# C no description, and isn't required
},
)
assert response.status_code == 200, response.data
self.addon.reload()
assert self.addon.default_locale == 'fr'
class TestAddonViewSetUpdateJWTAuth(TestAddonViewSetUpdate):
client_class = APITestClientJWT
@ -2466,6 +2633,17 @@ class VersionViewSetCreateUpdateMixin:
}
}
def test_custom_license_needs_name_and_text_empty_ignored(self):
# Check empty l10n is also ignored
response = self.request(custom_license={'name': {}, 'text': {}})
assert response.status_code == 400, response.content
assert response.data == {
'custom_license': {
'name': ['This field is required.'],
'text': ['This field is required.'],
}
}
def test_compatibility_list(self):
response = self.request(compatibility=[])
assert response.status_code == 400, response.content

Просмотреть файл

@ -4,8 +4,10 @@ from django.utils.translation import gettext
from rest_framework import exceptions
from olympia import amo
from olympia.amo.utils import find_language
from olympia.versions.models import License
from .models import Addon
from .utils import verify_mozilla_trademark
@ -20,7 +22,7 @@ class VerifyMozillaTrademark:
raise exceptions.ValidationError(exc.message)
class ValidateVersionLicense:
class VersionLicenseValidator:
requires_context = True
def __call__(self, data, serializer):
@ -143,7 +145,7 @@ class AddonMetadataValidator:
raise exceptions.ValidationError(missing_metadata, code='required')
class AddonMetadataNewVersionValidator(AddonMetadataValidator):
class VersionAddonMetadataValidator(AddonMetadataValidator):
def __call__(self, data, serializer):
try:
return super().__call__(data=data, serializer=serializer)
@ -156,3 +158,62 @@ class AddonMetadataNewVersionValidator(AddonMetadataValidator):
).format(missing_addon_metadata=list(exc.detail)),
code='required',
)
class AddonDefaultLocaleValidator:
requires_context = True
def __call__(self, data, serializer):
from olympia.api.fields import TranslationSerializerField
if 'default_locale' not in data:
return
new_default = data['default_locale']
fields = [
(name, field)
for name, field in serializer.fields.items()
if isinstance(field, TranslationSerializerField)
]
errors = {}
if not serializer.instance:
parsed_data = serializer.fields['version'].parsed_data
# Addon.resolve_webext_translations does find_language to check if l10n
if find_language(parsed_data.get('default_locale')):
xpi_trans = Addon.resolve_webext_translations(
parsed_data,
data['version']['upload'],
use_default_locale_fallback=False,
)
else:
xpi_trans = None
# confirm all the translated fields have a value in this new locale
for name, field in fields:
if serializer.instance:
# for an existing addon we need to consider all the existing values
existing = field.fetch_all_translations(
serializer.instance, field.get_source_field(serializer.instance)
)
all_translations = {
loc: val
for loc, val in {**existing, **data.get(name, {})}.items()
if val is not None
}
elif name in data:
# for a new addon a value in data overrides the xpi value/translations
all_translations = {
loc: val
for loc, val in data.get(name, {}).items()
if val is not None
}
else:
# else we need to check the xpi localization - if there is xpi l10n
all_translations = xpi_trans.get(name) if xpi_trans else None
if all_translations and new_default not in all_translations:
errors[name] = TranslationSerializerField.default_error_messages[
'default_locale_required'
].format(lang_code=new_default)
if errors:
raise exceptions.ValidationError(errors)

Просмотреть файл

@ -85,7 +85,7 @@ class TranslationSerializerField(fields.CharField):
default_error_messages = {
'unknown_locale': _('The language code "{lang_code}" is invalid.'),
'no_dict': _('You must provide an object of {lang-code:value}.'),
'no_dict': _('You must provide an object of {{lang-code:value}}.'),
'default_locale_required': _(
'A value in the default locale of "{lang_code}" is required.'
),
@ -96,6 +96,9 @@ class TranslationSerializerField(fields.CharField):
}
def __init__(self, *args, **kwargs):
# If not overriden, the field defaults to allow_null=True, and allow_null=False
# for new serializer instances (i.e. parent.instance=None) for dict style data.
self.allow_null_kwarg = kwargs.get('allow_null')
super().__init__(*args, **{'allow_null': True, **kwargs})
@property
@ -164,26 +167,24 @@ class TranslationSerializerField(fields.CharField):
def run_validation(self, data=fields.empty):
if data != fields.empty and not self.flat and not isinstance(data, dict):
raise exceptions.ValidationError(self.error_messages['no_dict'])
self.fail('no_dict')
if isinstance(data, dict):
obj = getattr(self.parent, 'instance', None)
if not obj and self.allow_null_kwarg is None:
self.allow_null = False
if not obj and len(data) == 0 and self.required:
self.fail('required')
for locale, value in data.items():
if locale.lower() not in settings.LANGUAGE_URL_MAP:
raise exceptions.ValidationError(
self.error_messages['unknown_locale'].format(lang_code=locale)
)
self.fail('unknown_locale', lang_code=locale)
data[locale] = super().run_validation(value)
obj = getattr(self.parent, 'instance', None)
default = default_locale(obj)
if default in data and data[default] is None:
# i.e. we're trying to delete the value in the default_locale
if self.required:
raise exceptions.ValidationError(
self.error_messages['default_locale_required'].format(
lang_code=default
)
)
self.fail('default_locale_required', lang_code=default)
else:
# even if not a required field, we need a default_locale value if
# there are other localizations.
@ -199,11 +200,7 @@ class TranslationSerializerField(fields.CharField):
if value is not None and locale != default
)
if any_other_locales:
raise exceptions.ValidationError(
self.error_messages['default_locale'].format(
lang_code=default
)
)
self.fail('default_locale', lang_code=default)
return data

Просмотреть файл

@ -172,18 +172,34 @@ class TestTranslationSerializerField(TestCase):
'The language code "unknown-locale" is invalid.'
]
def test_none_type_locale_is_allowed(self):
def test_none_type_locale_is_allowed_for_update(self):
# None values are valid because they are used to nullify existing
# translations in something like a PATCH.
data = {'en-US': None}
field = self.field_class(required=False)
field.bind('name', serializers.Serializer(instance=addon_factory()))
result = field.run_validation(data)
field.run_validation(result)
assert result == data
def test_none_type_locale_is_not_allowed_for_new_instances(self):
data = {'en-US': None}
field = self.field_class(required=False)
field.bind('name', serializers.Serializer())
with self.assertRaises(exceptions.ValidationError) as exc:
field.run_validation(data)
assert exc.exception.detail == ['This field may not be null.']
def test_none_type_locale_is_allowed_for_new_instances_when_allow_null_set(self):
data = {'en-US': None}
field = self.field_class(required=False, allow_null=True)
field.bind('name', serializers.Serializer())
assert field.run_validation(data) == data
def test_none_type_locale_is_not_allowed_for_required_fields(self):
data = {'en-US': None}
field = self.field_class(required=True)
field.bind('name', serializers.Serializer(instance=self.addon))
with self.assertRaises(exceptions.ValidationError) as exc:
field.run_validation(data)
assert exc.exception.detail == [

Двоичный файл не отображается.

Просмотреть файл

@ -1050,11 +1050,17 @@ class TestResolvei18nMessage:
result = utils.resolve_i18n_message('__MSG_foo__', messages, 'en')
assert result == 'bar'
def test_ignore_wrong_format(self):
def test_ignore_wrong_format_default(self):
messages = {'en-US': {'foo': 'bar'}}
result = utils.resolve_i18n_message('__MSG_foo__', messages, 'en', 'fr')
assert result == '__MSG_foo__'
def test_ignore_wrong_format_no_default(self):
messages = {'en-US': {'foo': 'bar'}}
result = utils.resolve_i18n_message('__MSG_foo__', messages, 'en')
assert result == '__MSG_foo__'
assert result is None
class TestGetBackgroundImages(TestCase):

Просмотреть файл

@ -1103,10 +1103,7 @@ def _update_version_in_json_manifest(content, new_version_number):
def extract_translations(file_obj):
"""Extract all translation messages from `file_obj`.
:param locale: if not `None` the list will be restricted only to `locale`.
"""
"""Extract all translation messages from `file_obj`."""
xpi = get_filepath(file_obj)
messages = {}
@ -1176,7 +1173,7 @@ def resolve_i18n_message(message, messages, locale, default_locale=None):
default_locale = find_language(default_locale)
msgid = match.group('msgid')
default = {'message': message}
default = {'message': message} if default_locale else {'message': None}
if locale in messages:
message = messages[locale].get(msgid, default)