Merge pull request #3390 from diox/extensions-locales-simpler

Use the right language when adding name/description for FxOS Add-ons (bug 1215094)

mkt/developers/tests/test_views.py

@@ -1140,7 +1140,7 @@ def test_success(self):
         qs = list(Translation.objects.filter(localized_string__isnull=False)
                   .values_list('locale', flat=True)
                   .filter(id=self.webapp.name_id))
-        eq_(qs, ['en-US', 'es'])
+        eq_(qs, ['en-us', 'es'])
 
     def test_delete_default_locale(self):
         r = self.client.post(self.url, {'locale': self.webapp.default_locale})

mkt/extensions/migrations/0016_reset_extensions_translations_locale.py

@@ -0,0 +1,31 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+def reset_extensions_translations_locales(apps, schema_editor):
+    """Reset the locale field for all translations on existing Extensions. This
+    is done to fix bug 1215094: some translations were created with the wrong
+    language - the one from the request, instead of the one from the
+    default_language field."""
+
+    Extension = apps.get_model('extensions', 'Extension')
+    Translation = apps.get_model('translations', 'Translation')
+    extensions = Extension.objects.all()
+    for extension in extensions:
+        translations_ids = filter(
+            None, [extension.name_id, extension.description_id])
+        lang = extension.default_language.lower()
+        Translation.objects.filter(id__in=translations_ids).update(locale=lang)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('extensions', '0015_extension_author'),
+    ]
+
+    operations = [
+        migrations.RunPython(reset_extensions_translations_locales),
+    ]

mkt/extensions/models.py

@@ -165,19 +165,36 @@ def extract_manifest_fields(cls, manifest_data, fields=None):
         if fields is None:
             fields = cls.manifest_is_source_of_truth_fields
         data = {k: manifest_data[k] for k in fields if k in manifest_data}
+
+        # Determine default language to use for translations.
+        # Web Extensions Manifest contains locales (e.g. "en_US"), not
+        # languages (e.g. "en-US"). The field is also called differently as a
+        # result (default_locale vs default_language), so we need to transform
+        # both the key and the value before adding it to data. A default value
+        # needs to be set to correctly generate the translated fields below.
+        default_language = to_language(manifest_data.get(
+            'default_locale', cls._meta.get_field('default_language').default))
         if 'default_language' in fields:
-            # Manifest contains locales (e.g. "en_US"), not languages
-            # (e.g. "en-US"). The field is also called differently as a result
-            # (default_locale vs default_language), so we need to transform
-            # both the key and the value before adding it to data.
-            default_locale = manifest_data.get('default_locale')
-            if default_locale:
-                data['default_language'] = to_language(default_locale)
+            data['default_language'] = default_language
+
+        # Be nice and strip leading / trailing whitespace chars from
+        # strings.
         for key, value in data.items():
-            # Be nice and strip leading / trailing whitespace chars from
-            # strings.
             if isinstance(value, basestring):
                 data[key] = value.strip()
+
+        # Translated fields should not be extracted as simple strings,
+        # otherwise we end up setting a locale on the translation that is
+        # dependent on the locale of the thread. Use dicts instead, always
+        # setting default_language as the language for now (since we don't
+        # support i18n in web extensions yet).
+        for field in cls._meta.translated_fields:
+            field_name = field.name
+            if field_name in data:
+                data[field_name] = {
+                    default_language: manifest_data[field_name]
+                }
+
         return data
 
     @classmethod

mkt/extensions/tests/test_models.py

@@ -81,6 +81,8 @@ def test_upload_new(self):
         eq_(extension.name, u'My Lîttle Extension')
         eq_(extension.default_language, 'en-GB')
         eq_(extension.description, u'A Dummÿ Extension')
+        eq_(extension.name.locale, 'en-gb')
+        eq_(extension.description.locale, 'en-gb')
         eq_(extension.slug, u'my-lîttle-extension')
         eq_(extension.status, STATUS_PENDING)
         ok_(extension.uuid)
@@ -685,6 +687,9 @@ def test_update_fields_from_manifest_when_version_is_made_public(self):
         eq_(extension.author, u'New Authôr')
         eq_(extension.description, u'New Descriptîon')
         eq_(extension.name, u'New Nâme')
+        # Locale should be en-US since none is specified in the manifest.
+        eq_(extension.name.locale, 'en-us')
+        eq_(extension.description.locale, 'en-us')
 
     def test_update_manifest_when_public_version_is_hard_deleted(self):
         old_manifest = {

mkt/extensions/tests/test_views.py

@@ -190,25 +190,51 @@ def test_create_logged_in(self):
         data = response.json
 
         eq_(data['author'], u'Mozillâ')
-        eq_(data['description'], {'en-US': u'A Dummÿ Extension'})
+        # The extension.zip package has "default_locale": "en_GB".
+        eq_(data['description'], {'en-GB': u'A Dummÿ Extension'})
         eq_(data['device_types'], ['firefoxos'])
         eq_(data['disabled'], False)
         eq_(data['last_updated'], None)  # The extension is not public yet.
         eq_(data['latest_version']['size'], 268)
         eq_(data['latest_version']['version'], '0.1')
-        eq_(data['name'], {'en-US': u'My Lîttle Extension'})
+        eq_(data['name'], {'en-GB': u'My Lîttle Extension'})
         eq_(data['slug'], u'my-lîttle-extension')
         eq_(data['status'], 'pending')
         eq_(Extension.objects.without_deleted().count(), 1)
         eq_(ExtensionVersion.objects.without_deleted().count(), 1)
         extension = Extension.objects.without_deleted().get(pk=data['id'])
+        eq_(extension.default_language, 'en-GB')
+        eq_(extension.description, u'A Dummÿ Extension')
+        eq_(extension.description.locale, 'en-gb')
+        eq_(extension.name, u'My Lîttle Extension')
+        eq_(extension.name.locale, 'en-gb')
         eq_(extension.status, STATUS_PENDING)
         eq_(list(extension.authors.all()), [self.user])
 
         note = extension.threads.get().notes.get()
         eq_(note.body, u'add-on has arrivedÄ')
         eq_(note.note_type, comm.SUBMISSION)
 
+    def test_create_logged_in_with_lang(self):
+        upload = self.get_upload(
+            abspath=self.packaged_app_path('extension.zip'), user=self.user)
+        eq_(upload.valid, True)
+        response = self.client.post(self.list_url, json.dumps({
+            'lang': 'es',
+            'validation_id': upload.pk
+        }))
+        eq_(response.status_code, 201)
+        data = response.json
+
+        eq_(data['author'], u'Mozillâ')
+        # The extension.zip package has "default_locale": "en_GB".
+        eq_(data['description'], {'en-GB': u'A Dummÿ Extension'})
+        eq_(data['name'], {'en-GB': u'My Lîttle Extension'})
+        extension = Extension.objects.without_deleted().get(pk=data['id'])
+        eq_(extension.default_language, 'en-GB')
+        eq_(extension.description.locale, 'en-gb')
+        eq_(extension.name.locale, 'en-gb')
+
     def test_create_upload_has_no_user(self):
         upload = self.get_upload(
             abspath=self.packaged_app_path('extension.zip'), user=None)

mkt/site/fixtures/data/prices2.json

@@ -5,7 +5,7 @@
     "fields": {
       "localized_string_clean": null,
       "created": "2010-07-02 01:26:32",
-      "locale": "en-US",
+      "locale": "en-us",
       "modified": "2010-01-02 12:34:56",
       "id": 1241126,
       "localized_string": ""
@@ -17,7 +17,7 @@
     "fields": {
       "localized_string_clean": null,
       "created": "2010-07-02 01:26:32",
-      "locale": "en-US",
+      "locale": "en-us",
       "modified": "2010-01-02 12:34:56",
       "id": 1241127,
       "localized_string": ""

mkt/site/fixtures/data/webapp_337141.json

@@ -137,7 +137,7 @@
         "fields": {
             "localized_string": "We sell your ish \r\nhttp://omg.org/yes",
             "created": "2007-04-05 08:08:48",
-            "locale": "en-US",
+            "locale": "en-us",
             "modified": "2009-03-26 07:41:10",
             "id": 2326784,
             "localized_string_clean": null
@@ -149,7 +149,7 @@
         "fields": {
             "localized_string_clean": null,
             "created": "2011-10-18 16:28:24",
-            "locale": "en-US",
+            "locale": "en-us",
             "modified": "2011-10-18 16:28:57",
             "id": 2425897,
             "localized_string": "Something Something Steamcube!"
@@ -173,7 +173,7 @@
         "fields": {
             "localized_string_clean": null,
             "created": "2011-10-18 16:28:24",
-            "locale": "en-US",
+            "locale": "en-us",
             "modified": "2011-10-18 16:28:57",
             "id": 2425898,
             "localized_string": "Something Something Steamcube description!"
@@ -185,7 +185,7 @@
         "fields": {
             "localized_string_clean": null,
             "created": "2011-10-18 16:28:24",
-            "locale": "en-US",
+            "locale": "en-us",
             "modified": "2011-10-18 16:28:57",
             "id": 2425899,
             "localized_string": "foo@bar.com"

mkt/translations/fields.py

@@ -163,14 +163,15 @@ def __set__(self, instance, value):
 
     def translation_from_string(self, instance, lang, string):
         """Create, save, and return a Translation from a string."""
+        lang = lang.lower()
         try:
             trans = getattr(instance, self.field.name)
             trans_id = getattr(instance, self.field.attname)
             if trans is None and trans_id is not None:
                 # This locale doesn't have a translation set, but there are
                 # translations in another locale, so we have an id already.
                 translation = self.model.new(string, lang, id=trans_id)
-            elif to_language(trans.locale) == lang.lower():
+            elif to_language(trans.locale) == lang:
                 # Replace the translation in the current language.
                 trans.localized_string = string
                 translation = trans

mkt/translations/tests/test_models.py

@@ -184,7 +184,7 @@ def test_update_translation(self):
 
     def test_create_with_dict(self):
         # Set translations with a dict.
-        strings = {'en-US': 'right language', 'de': 'wrong language'}
+        strings = {'en-us': 'right language', 'de': 'wrong language'}
         o = self.TranslatedModel.objects.create(name=strings)
 
         # Make sure we get the English text since we're in en-US.
@@ -232,14 +232,27 @@ def get_model():
         translation.activate('sr-Latn')
         trans_eq(get_model().name, 'yes I speak serbian', 'sr-Latn')
 
+    def test_update_with_dict_locale_in_different_case(self):
+        def get_model():
+            return self.TranslatedModel.objects.get(id=1)
+
+        obj = get_model()
+        eq_(obj.name.locale, 'en-us')
+        trans_eq(obj.name, 'some name', 'en-US')
+        obj.name = {'en-US': u'yes I speak englîsh'}
+        obj.save()
+        obj = get_model()
+        eq_(obj.name.locale, 'en-us')
+        trans_eq(obj.name, u'yes I speak englîsh', 'en-US')
+
     def test_dict_bad_locale(self):
         m = self.TranslatedModel.objects.get(id=1)
         m.name = {'de': 'oof', 'xxx': 'bam', 'es': 'si'}
         m.save()
 
         ts = Translation.objects.filter(id=m.name_id)
         eq_(sorted(ts.values_list('locale', flat=True)),
-            ['de', 'en-US', 'es'])
+            ['de', 'en-us', 'es'])
 
     def test_sorting(self):
         """Test translation comparisons in Python code."""
@@ -351,7 +364,7 @@ def test_purifed_linkified_fields_in_template(self):
     def test_require_locale(self):
         obj = self.TranslatedModel.objects.get(id=1)
         eq_(unicode(obj.no_locale), 'blammo')
-        eq_(obj.no_locale.locale, 'en-US')
+        eq_(obj.no_locale.locale, 'en-us')
 
         # Switch the translation to a locale we wouldn't pick up by default.
         obj.no_locale.locale = 'fr'

mkt/translations/tests/testapp/fixtures/testapp/test_models.json

@@ -8,7 +8,7 @@
         "model": "translations.translation",
         "fields": {
             "id": 1,
-            "locale": "en-US",
+            "locale": "en-us",
             "localized_string": "some name",
             "created": "2007-03-05 16:06:55",
             "modified": "2007-03-21 13:42:36"
@@ -19,7 +19,7 @@
         "model": "translations.translation",
         "fields": {
             "id": 2,
-            "locale": "en-US",
+            "locale": "en-us",
             "localized_string": "some description",
             "created": "2007-03-05 16:06:55",
             "modified": "2007-03-21 13:42:36"
@@ -52,7 +52,7 @@
         "model": "translations.translation",
         "fields": {
             "id": 3,
-            "locale": "en-US",
+            "locale": "en-us",
             "localized_string": "speak American",
             "created": "2007-03-05 16:06:55",
             "modified": "2007-03-21 13:42:36"
@@ -63,7 +63,7 @@
         "model": "translations.translation",
         "fields": {
             "id": 4,
-            "locale": "en-US",
+            "locale": "en-us",
             "localized_string": "hot dogs",
             "created": "2007-03-05 16:06:55",
             "modified": "2007-03-21 13:42:36"
@@ -74,7 +74,7 @@
         "model": "translations.translation",
         "fields": {
             "id": 10,
-            "locale": "en-US",
+            "locale": "en-us",
             "localized_string": "blammo",
             "created": "2007-03-05 16:06:55",
             "modified": "2007-03-21 13:42:36"
@@ -85,7 +85,7 @@
         "model": "translations.purifiedtranslation",
         "fields": {
             "id": 20,
-            "locale": "en-US",
+            "locale": "en-us",
             "localized_string": "<i>x</i> http://yyy.com",
             "created": "2007-03-05 16:06:55",
             "modified": "2007-03-21 13:42:36"
@@ -96,7 +96,7 @@
         "model": "translations.linkifiedtranslation",
         "fields": {
             "id": 30,
-            "locale": "en-US",
+            "locale": "en-us",
             "localized_string": "<i>x</i> http://yyy.com",
             "created": "2007-03-05 16:06:55",
             "modified": "2007-03-21 13:42:36"

mkt/webapps/models.py

@@ -874,14 +874,13 @@ def update_names(self, new_names):
         Don't forget to call save() in your calling method.
         """
         updated_locales = {}
-        locales = dict(Translation.objects.filter(id=self.name_id)
-                                          .values_list('locale',
-                                                       'localized_string'))
+        qs = Translation.objects.filter(id=self.name_id)
+        locales = {to_language(locale): localized for locale, localized
+                   in qs.values_list('locale', 'localized_string')}
         msg_c = []  # For names that were created.
         msg_d = []  # For deletes.
         msg_u = []  # For updates.
 
-        # Normalize locales.
         names = {}
         for locale, name in new_names.iteritems():
             loc = find_language(locale)

mkt/webapps/tests/test_models.py

@@ -554,8 +554,8 @@ def test_remove(self):
         app.remove_locale('el')
         qs = (Translation.objects.filter(localized_string__isnull=False)
               .values_list('locale', flat=True))
-        eq_(sorted(qs.filter(id=app.name_id)), ['en-US'])
-        eq_(sorted(qs.filter(id=app.description_id)), ['en-US', 'ja'])
+        eq_(sorted(qs.filter(id=app.name_id)), ['en-us'])
+        eq_(sorted(qs.filter(id=app.description_id)), ['en-us', 'ja'])
 
     def test_remove_version_locale(self):
         app = app_factory()

mkt/webpay/webpay_jwt.py

@@ -12,6 +12,7 @@
 from lib.crypto.webpay import sign_webpay_jwt
 from mkt.site.helpers import absolutify
 from mkt.translations.models import Translation
+from mkt.translations.utils import to_language
 from mkt.webpay.utils import make_external_id, strip_tags
 
 
@@ -96,8 +97,9 @@ def localized_properties(self):
         for attr in ('name', 'description'):
             tr_object = getattr(self.webapp, attr)
             for tr in Translation.objects.filter(id=tr_object.id):
-                props.setdefault(tr.locale, {})
-                props[tr.locale][attr] = tr.localized_string
+                language = to_language(tr.locale)
+                props.setdefault(language, {})
+                props[language][attr] = tr.localized_string
 
         return props