Shows mobile platform choices when submitting a mobile add-on (bug 624226)

apps/amo/__init__.py

@@ -375,11 +375,27 @@ class PLATFORM_SUN:
     shortname = 'solaris'
     api_name = 'SunOS'
 
+
+class PLATFORM_ANDROID:
+    id = 7
+    name = _(u'Android')
+    shortname = u'android'
+    api_name = u'Android'
+
+
+class PLATFORM_MAEMO:
+    id = 8
+    name = _(u'Maemo')
+    shortname = u'maemo'
+    api_name = u'Maemo'
+
 # Order matters
 PLATFORMS = {PLATFORM_ANY.id: PLATFORM_ANY, PLATFORM_ALL.id: PLATFORM_ALL,
              PLATFORM_LINUX.id: PLATFORM_LINUX, PLATFORM_MAC.id: PLATFORM_MAC,
              PLATFORM_BSD.id: PLATFORM_BSD, PLATFORM_WIN.id: PLATFORM_WIN,
-             PLATFORM_SUN.id: PLATFORM_SUN}
+             PLATFORM_SUN.id: PLATFORM_SUN,
+             PLATFORM_ANDROID.id: PLATFORM_ANDROID,
+             PLATFORM_MAEMO.id: PLATFORM_MAEMO}
 
 SUPPORTED_PLATFORMS = {PLATFORM_ALL.id: PLATFORM_ALL,
                        PLATFORM_LINUX.id: PLATFORM_LINUX,

apps/amo/fixtures/base/appversion.json

@@ -381,5 +381,27 @@
         },
         "model": "applications.appversion",
         "pk": 282
+    },
+    {
+        "pk": 324,
+        "model": "applications.appversion",
+        "fields": {
+            "version_int": 2000000001000,
+            "application": 60,
+            "version": "2.0a1pre",
+            "modified": null,
+            "created": null
+        }
+    },
+    {
+        "pk": 352,
+        "model": "applications.appversion",
+        "fields": {
+            "version_int": 4000000102000,
+            "application": 60,
+            "version": "4.0b2pre",
+            "modified": null,
+            "created": null
+        }
     }
 ]

apps/devhub/tests/addons/unopenable.xpi

@@ -0,0 +1 @@
+This is not a valid XPI!

apps/devhub/tests/test_views.py

@@ -3419,13 +3419,24 @@ class TestUpload(files.tests.UploadTest):
 
 
 class TestUploadDetail(files.tests.UploadTest):
-    fixtures = ['base/apps', 'base/users']
+    fixtures = ['base/apps', 'base/appversion', 'base/users']
 
     def post(self):
         # Has to be a binary, non xpi file.
         data = open(get_image_path('animated.png'), 'rb')
         return self.client.post(reverse('devhub.upload'), {'upload': data})
 
+    def validation_ok(self):
+        return {
+            'errors': 0,
+            'success': True,
+            'warnings': 0,
+            'notices': 0,
+            'message_tree': {},
+            'messages': [],
+            'rejected': False
+        }
+
     @attr('validator')
     def test_detail_json(self):
         self.post()
@@ -3457,6 +3468,61 @@ class TestUploadDetail(files.tests.UploadTest):
         eq_(suite.attr('data-validateurl'),
             reverse('devhub.upload_detail', args=[upload.uuid, 'json']))
 
+    @mock.patch('devhub.tasks._validator')
+    def test_multi_app_addon_cannot_have_platforms(self, v):
+        v.return_value = json.dumps(self.validation_ok())
+        addon = os.path.join(settings.ROOT, 'apps', 'devhub', 'tests',
+                             'addons', 'mobile-2.9.10-fx+fn.xpi')
+        with open(addon, 'rb') as f:
+            r = self.client.post(reverse('devhub.upload'),
+                                 {'upload': f})
+        eq_(r.status_code, 302)
+        upload = FileUpload.objects.get()
+        r = self.client.get(reverse('devhub.upload_detail',
+                                    args=[upload.uuid, 'json']))
+        eq_(r.status_code, 200)
+        data = json.loads(r.content)
+        eq_(data['new_platform_choices'], [
+                {'text': unicode(amo.PLATFORM_ALL.name), 'checked': True,
+                 'value': amo.PLATFORM_ALL.id}])
+
+    @mock.patch('devhub.tasks._validator')
+    def test_new_platform_choices_for_mobile(self, v):
+        v.return_value = json.dumps(self.validation_ok())
+        addon = os.path.join(settings.ROOT, 'apps', 'devhub', 'tests',
+                             'addons', 'mobile-0.1-fn.xpi')
+        with open(addon, 'rb') as f:
+            r = self.client.post(reverse('devhub.upload'),
+                                 {'upload': f})
+        eq_(r.status_code, 302)
+        upload = FileUpload.objects.get()
+        r = self.client.get(reverse('devhub.upload_detail',
+                                    args=[upload.uuid, 'json']))
+        eq_(r.status_code, 200)
+        data = json.loads(r.content)
+        eq_(data['new_platform_choices'], [
+                {'text': unicode(amo.PLATFORM_ALL.name), 'checked': True,
+                 'value': amo.PLATFORM_ALL.id},
+                {'text': unicode(amo.PLATFORM_MAEMO.name),
+                 'value': amo.PLATFORM_MAEMO.id},
+                {'text': unicode(amo.PLATFORM_ANDROID.name),
+                 'value': amo.PLATFORM_ANDROID.id}])
+
+    @mock.patch('devhub.tasks._validator')
+    def test_unparsable_xpi(self, v):
+        v.return_value = json.dumps(self.validation_ok())
+        addon = os.path.join(settings.ROOT, 'apps', 'devhub', 'tests',
+                             'addons', 'unopenable.xpi')
+        with open(addon, 'rb') as f:
+            r = self.client.post(reverse('devhub.upload'),
+                                 {'upload': f})
+        upload = FileUpload.objects.get()
+        r = self.client.get(reverse('devhub.upload_detail',
+                                    args=[upload.uuid, 'json']))
+        eq_(r.status_code, 200)
+        data = json.loads(r.content)
+        eq_(data['new_platform_choices'], None)
+
 
 class TestUploadValidation(files.tests.UploadTest):
     fixtures = ['base/apps', 'base/users',

apps/devhub/views.py

@@ -10,6 +10,7 @@ import uuid
 
 from django import http
 from django.conf import settings
+from django import forms as django_forms
 from django.db.models import Count
 from django.shortcuts import get_object_or_404, redirect
 from django.utils.http import urlquote
@@ -34,6 +35,7 @@ from addons.models import Addon, AddonUser
 from addons.views import BaseFilter
 from devhub.models import ActivityLog, RssKey, SubmitStep
 from files.models import File, FileUpload
+from files.utils import parse_addon
 from translations.models import delete_translation
 from versions.models import License, Version
 
@@ -532,13 +534,38 @@ def json_upload_detail(upload):
     validation = json.loads(upload.validation) if upload.validation else ""
     url = reverse('devhub.upload_detail', args=[upload.uuid, 'json'])
     full_report_url = reverse('devhub.upload_detail', args=[upload.uuid])
+    new_platform_choices = None
 
     if validation:
         prepare_validation_results(validation)
+        if validation['errors'] == 0:
+            try:
+                app_ids = [a.id for a in parse_addon(upload.path)['apps']]
+                if amo.MOBILE.id in app_ids:
+                    # For multiple apps, choosing a platform no longer makes
+                    # sense, so only allow ALL
+                    new_platform_choices = [
+                        dict(value=amo.PLATFORM_ALL.id,
+                             text=unicode(amo.PLATFORM_ALL.name),
+                             checked=True)]
+                    if len(app_ids) == 1:
+                        # For mobile only add-ons, show mobile platforms:
+                        new_platform_choices.extend([
+                            dict(value=amo.PLATFORM_MAEMO.id,
+                                 text=unicode(amo.PLATFORM_MAEMO.name)),
+                            dict(value=amo.PLATFORM_ANDROID.id,
+                                 text=unicode(amo.PLATFORM_ANDROID.name))])
+            except django_forms.ValidationError:
+                # XPI parsing errors will be reported in the form submission
+                # (next request).
+                # TODO(Kumar) It would be nicer to present errors to the user
+                # right here to avoid confusion about platform selection.
+                log.exception("XPI parsing error, ignored")
 
     r = dict(upload=upload.uuid, validation=validation,
              error=upload.task_error, url=url,
-             full_report_url=full_report_url)
+             full_report_url=full_report_url,
+             new_platform_choices=new_platform_choices)
     return r
 
 

media/js/zamboni/devhub.js

@@ -883,9 +883,30 @@ function addonUploaded(json) {
         }
 
         if (json.validation.detected_type == 'search') {
+            // TODO(Kumar) this probably broke the versions page
+            // which does not use #create-addon. Remove the id.
             $("#create-addon .platform").hide();
         } else {
             $("#create-addon .platform:hidden").show();
+            if (json.new_platform_choices) {
+                // e.g. after uploading a Mobile add-on
+                $('.platform ul').empty();
+                $.each(json.new_platform_choices, function(i, pl) {
+                    var li = $(format('<li><label><input name="platforms" ' +
+                                      'type="checkbox" class="platform" />' +
+                                      '{0}</label></li>', [pl.text])),
+                        id = format('id_platforms_{0}', [i]),
+                        label = $('label', li),
+                        input = $('input', li);
+                    label.attr('for', id);
+                    input.attr('id', id);
+                    input.attr('value', pl.value);
+                    if (pl.checked) {
+                        input.attr('checked', 'checked');
+                    }
+                    $('.platform ul').append(li);
+                });
+            }
         }
 
         statusclass = v.errors ? 'status-fail' : 'status-pass';

media/js/zamboni/tests/devhub_tests.js

@@ -1026,4 +1026,68 @@ asyncTest('customized', function() {
 });
 
 
+module('switch addon platforms', {
+    setup: function() {
+        this.sandbox = tests.createSandbox('#addon-platform-switching');
+    },
+    teardown: function() {
+        this.sandbox.remove();
+    }
+});
+
+test('mobile', function() {
+    addonUploaded({
+        validation: {
+            "errors": 0,
+            "detected_type": "mobile",
+            "success": true,
+            "warnings": 0,
+            "notices": 0,
+            "message_tree": {},
+            "messages": [],
+            "rejected": false
+        },
+        new_platform_choices: [
+            {value: 1, checked: true, text: 'All Platforms'},
+            {value: 2, checked: false, text: 'Maemo'},
+            {value: 3, checked: false, text: 'Android'}
+        ]
+    });
+    equals($('.platform input:eq(0)', this.sandbox).attr('value'), '1');
+    equals($('.platform input:eq(0)', this.sandbox).attr('id'),
+           'id_platforms_0');
+    equals($('.platform input:eq(0)', this.sandbox).attr('checked'), true);
+    equals($('.platform li:eq(0)', this.sandbox).text(),
+           'All Platforms');
+    equals($('.platform input:eq(1)', this.sandbox).attr('value'), '2');
+    equals($('.platform input:eq(1)', this.sandbox).attr('id'),
+           'id_platforms_1');
+    equals($('.platform li:eq(1)', this.sandbox).text(), 'Maemo');
+    equals($('.platform input:eq(2)', this.sandbox).attr('value'), '3');
+    equals($('.platform input:eq(2)', this.sandbox).attr('id'),
+           'id_platforms_2');
+    equals($('.platform li:eq(2)', this.sandbox).text(), 'Android');
+});
+
+test('non-ascii', function() {
+    addonUploaded({
+        validation: {
+            "errors": 0,
+            "detected_type": "mobile",
+            "success": true,
+            "warnings": 0,
+            "notices": 0,
+            "message_tree": {},
+            "messages": [],
+            "rejected": false
+        },
+        new_platform_choices: [
+            {value: 1, checked: true, text: 'フォクすけといっしょ'}
+        ]
+    });
+    equals($('.platform li:eq(0)', this.sandbox).text(),
+           'フォクすけといっしょ');
+});
+
+
 });

migrations/139-add-mobile-platforms.sql

@@ -0,0 +1,23 @@
+UPDATE translations_seq SET id=LAST_INSERT_ID(id + 1);
+SELECT LAST_INSERT_ID() FROM translations_seq INTO @id;
+insert into translations (id, locale, localized_string) VALUES
+                         ((SELECT @id), 'en-US', 'Android');
+INSERT INTO platforms (id, name) VALUES (7, (SELECT @id));
+
+UPDATE translations_seq SET id=LAST_INSERT_ID(id + 1);
+SELECT LAST_INSERT_ID() FROM translations_seq INTO @id;
+INSERT INTO translations (id, locale, localized_string) VALUES
+                         ((SELECT @id), 'en-US', 'android');
+UPDATE platforms SET shortname = @id WHERE id=7;
+
+UPDATE translations_seq SET id=LAST_INSERT_ID(id + 1);
+SELECT LAST_INSERT_ID() FROM translations_seq INTO @id;
+INSERT INTO translations (id, locale, localized_string) VALUES
+                         ((SELECT @id), 'en-US', 'Maemo');
+INSERT INTO platforms (id, name) VALUES (8, (SELECT @id));
+
+UPDATE translations_seq SET id=LAST_INSERT_ID(id + 1);
+SELECT LAST_INSERT_ID() FROM translations_seq INTO @id;
+INSERT INTO translations (id, locale, localized_string) VALUES
+                         ((SELECT @id), 'en-US', 'maemo');
+UPDATE platforms SET shortname = @id WHERE id=8;

templates/qunit.html

@@ -123,6 +123,24 @@
       </ul>
     </form>
   </div>
+  <div id="addon-platform-switching">
+    <div class="platform">
+      <ul>
+        <li><label for="id_platforms_0">
+          <input checked="checked" name="platforms" value="100" id="id_platforms_0" type="checkbox" class="platform" />
+          All Platforms</label></li>
+        <li><label for="id_platforms_1">
+          <input name="platforms" value="101" id="id_platforms_1" type="checkbox" class="platform" />
+          Linux</label></li>
+        <li><label for="id_platforms_2">
+          <input name="platforms" value="102" id="id_platforms_2" type="checkbox" class="platform" />
+          Mac OS X</label></li>
+        <li><label for="id_platforms_3">
+          <input name="platforms" value="103" id="id_platforms_3" type="checkbox" class="platform" />
+          Windows</label></li>
+      </ul>
+    </div>
+  </div>
   <div id="slugified-field">
     <input id="id_name" />
     <span id="slug_edit" class="edit_with_prefix edit_initially_hidden">