select a reasonable default for Firefox - add-on packager (bug 691393)

apps/devhub/forms.py

@@ -1,3 +1,4 @@
+# -*- coding: utf8 -*-
 import path
 import re
 import socket
@@ -105,7 +106,7 @@ class LicenseRadioInput(forms.widgets.RadioInput):
         super(LicenseRadioInput, self).__init__(name, value, attrs, choice,
                                                 index)
         license = choice[1]  # Choice is a tuple (object.id, object).
-        link = '<a class="xx extra" href="%s" target="_blank">%s</a>'
+        link = u'<a class="xx extra" href="%s" target="_blank">%s</a>'
         if hasattr(license, 'url'):
             details = link % (license.url, _('Details'))
             self.choice_label = mark_safe(self.choice_label + details)
@@ -222,7 +223,7 @@ class PolicyForm(TranslationFormMixin, AMOModelForm):
 
     def _has_field(self, name):
         # If there's a eula in any language, this addon has a eula.
-        n = getattr(self.addon, '%s_id' % name)
+        n = getattr(self.addon, u'%s_id' % name)
         return any(map(bool, Translation.objects.filter(id=n)))
 
     class Meta:
@@ -481,7 +482,7 @@ class NewVersionForm(NewAddonForm):
             xpi = parse_addon(self.cleaned_data['upload'], self.addon)
             if self.addon.versions.filter(version=xpi['version']):
                 raise forms.ValidationError(
-                    _('Version %s already exists') % xpi['version'])
+                    _(u'Version %s already exists') % xpi['version'])
             self._clean_all_platforms()
         return self.cleaned_data
 
@@ -784,6 +785,14 @@ class PackagerCompatForm(forms.Form):
         self.fields['min_ver'].queryset = qs.filter(~Q(version__contains='*'))
         self.fields['max_ver'].queryset = qs.all()
 
+        # Unreasonably hardcode a reasonable default Firefox minVersion.
+        if self.app == amo.FIREFOX and not self.data.get('min_ver'):
+            try:
+                self.fields['min_ver'].initial = qs.filter(
+                    version=settings.FIREFOX_MINVER)[0]
+            except (IndexError, AttributeError):
+                pass
+
     def clean_min_ver(self):
         if self.cleaned_data['enabled'] and not self.cleaned_data['min_ver']:
             raise_required()

apps/devhub/templates/devhub/package_addon.html

@@ -60,7 +60,7 @@
         that you intend to test your add-on with.
         {% endtrans %}
       </p>
-      <div class="supported-apps">
+      <div id="supported-apps">
         <label>{{ _('Supported Applications and Versions') }}</label>
         {{ compat_forms.management_form }}
         {{ compat_forms.non_form_errors() }}

apps/devhub/tests/test_views_packager.py

@@ -1,6 +1,9 @@
 import json
 import os
 
+from django.conf import settings
+
+from mock import patch
 from nose.tools import eq_
 from pyquery import PyQuery as pq
 
@@ -9,6 +12,7 @@ from amo.tests import assert_required, formset, initial
 from amo.urlresolvers import reverse
 from addons.models import BlacklistedSlug
 from addons.utils import ReverseNameLookup
+from applications.models import AppVersion
 from devhub.views import packager_path
 
 
@@ -19,6 +23,7 @@ class TestPackager(amo.tests.TestCase):
     def setUp(self):
         assert self.client.login(username='regular@mozilla.com',
                                  password='password')
+
         self.url = reverse('devhub.package_addon')
 
         ctx = self.client.get(self.url).context['compat_forms']
@@ -49,7 +54,7 @@ class TestPackager(amo.tests.TestCase):
         """Ensure that the initial forms for each application are present."""
         r = self.client.get(self.url)
         eq_(r.status_code, 200)
-        rows = pq(r.content)('.supported-apps li.row')
+        rows = pq(r.content)('#supported-apps li.row')
         classes = [a.short for a in amo.APP_USAGE]
         eq_(rows.length, len(classes))
         for app_class, label in zip(classes, rows('label.app')):
@@ -181,6 +186,29 @@ class TestPackager(amo.tests.TestCase):
         eq_(r.context['compat_forms'].errors[0]['__all__'][0],
             'Min version must be less than Max version.')
 
+    @patch.object(settings, 'FIREFOX_MINVER', '3.6')
+    def test_default_firefox_minver(self):
+        eq_(len(AppVersion.objects.filter(version='3.6')), 1)
+        r = self.client.get(self.url)
+        eq_(r.status_code, 200)
+        form = r.context['compat_forms'].forms[0]
+        eq_(form.fields['min_ver'].initial.version, '3.6')
+
+    @patch.object(settings, 'FIREFOX_MINVER', '999.0')
+    def test_no_default_firefox_minver(self):
+        r = self.client.get(self.url)
+        eq_(r.status_code, 200)
+        form = r.context['compat_forms'].forms[0]
+        eq_(form.fields['min_ver'].initial, None)
+
+    @patch.object(settings, 'FIREFOX_MINVER', '3.6')
+    def test_no_default_firefox_minver_on_post(self):
+        self.compat_form['min_ver'] = '114'
+        r = self.client.post(self.url, self._form_data())
+        form = r.context['compat_forms'].forms[0]
+        assert form.fields['min_ver'].initial.version != '3.6', (
+            'The Firefox minVer default should not be set on POST.')
+
 
 class TestPackagerDownload(amo.tests.TestCase):
     fixtures = ['base/apps', 'base/users']

media/css/impala/devhub/packager.less

@@ -49,7 +49,7 @@
     textarea {
         width: auto;
     }
-    .supported-apps {
+    #supported-apps {
         > label {
             display: block;
             margin: 2em 0 .5em;
@@ -59,7 +59,7 @@
             padding: .5em 0;
             label.app, span.row {
                 opacity: .5;
-                .transition(opacity);
+                .transition(opacity, .2s);
             }
             label.app.choice {
                 background-position: 0 .25em;

media/js/zamboni/packager.js

@@ -2,8 +2,18 @@ $(document).ready(function() {
 
     var $pkgr = $('#packager');
     if ($pkgr.length) {
+        // Upon keypress, generates a package name slug from add-on name.
+        $pkgr.delegate('#id_name', 'keyup blur', function() {
+            var slug = makeslug($('#id_name').val(), '_');
+            $('#id_package_name').val(slug);
+        }).delegate('#id_package_name', 'blur', function() {
+            var $this = $(this);
+            $this.val(makeslug($this.val(), '_'));
+        });
+
         // Adds a 'selected' class upon clicking an application checkbox.
-        $pkgr.delegate('.app input:checkbox', 'change', function() {
+        var $supported = $('#supported-apps');
+        $supported.delegate('input:checkbox', 'change', function() {
             var $this = $(this),
                 $li = $this.closest('li');
             if ($this.is(':checked')) {
@@ -12,15 +22,11 @@ $(document).ready(function() {
                 $li.removeClass('selected');
             }
         });
-        $pkgr.find('.app input:checkbox').trigger('change');
+        $supported.find('input:checkbox').trigger('change');
 
-        // Upon keypress, generates a package name slug from add-on name.
-        $pkgr.delegate('#id_name', 'keyup blur', function() {
-            var slug = makeslug($('#id_name').val(), '_');
-            $('#id_package_name').val(slug);
-        }).delegate('#id_package_name', 'blur', function() {
-            var $this = $(this);
-            $this.val(makeslug($this.val(), '_'));
+        $supported.delegate('select', 'change', function() {
+            var $checkbox = $(this).closest('li.row').find('input:checkbox');
+            $checkbox.attr('checked', true).trigger('change');
         });
     }
 

settings.py

@@ -1128,6 +1128,9 @@ COMPAT = (
 # Latest nightly version of Firefox.
 NIGHTLY_VERSION = COMPAT[0]['main']
 
+# Default minimum version of Firefox for Add-on Packager.
+FIREFOX_MINVER = COMPAT[4]['main']
+
 # URL for reporting arecibo errors too. If not set, won't be sent.
 ARECIBO_SERVER_URL = ""
 # Make AMO group posts and wait for 60 seconds when we get lots of errors.