Merge pull request #351 from mattbasta/icons

Icon changes for bug 744648

media/css/devreg/media.less

@@ -204,6 +204,7 @@
 }
 
 #icon_preview_32 {
+    display: none;
     line-height: 32px;
     width: 32px;
     height: 32px;
@@ -224,6 +225,7 @@
 }
 
 #icon_preview_64 {
+    display: none;
     line-height: 64px;
     width: 64px;
     height: 64px;
@@ -233,6 +235,21 @@
     }
 }
 
+#icon_preview_128 {
+    display: none;
+    line-height: 128px;
+    width: 128px;
+    height: 128px;
+    img {
+        max-width: 128px;
+        max-height: 128px;
+    }
+}
+
+#icon_preview .icon_preview_box.defunct {
+    display: none;
+}
+
 .edit-addon-section #icons_default {
     margin-bottom: 1em;
 }

media/js/devreg/devhub.js

@@ -564,6 +564,8 @@ function initUploadIcon() {
         $('#icon_preview_32 img').attr('src', $('img', $parent).attr('src'));
         $('#icon_preview_64 img').attr('src', $('img',
                 $parent).attr('src').replace(/32/, '64'));
+        $('#icon_preview_128 img').attr('src', $('img',
+                $parent).attr('src').replace(/32/, '128'));
 
         $error_list.html("");
     });

mkt/constants/submit.py

@@ -9,3 +9,7 @@
     ('done', _('Finished!')),
 ]
 APP_STEPS_TITLE = dict(APP_STEPS)
+
+# Size requirements for uploaded app icons
+APP_ICON_MIN_SIZE = (128, 128)
+

mkt/developers/templates/developers/apps/forms_shared/media.html

@@ -26,6 +26,16 @@
           {% if editable %}
             <div id="icon_preview">
               <div class="icon_preview_box">
+                <div class="icon_preview" id="icon_preview_128">
+                  <img src="{{ addon.get_icon_url(128) }}">
+                </div>
+                <div class="edit-media-details">
+                  {# L10n: The size of the icon #}
+                  {{ _('128x128px') }}
+                  {{ tip(None, _('Resized to fit all icon sizes.')) }}
+                </div>
+              </div>
+              <div class="icon_preview_box defunct">
                 <div class="icon_preview" id="icon_preview_64">
                   <img src="{{ addon.get_icon_url(64) }}">
                 </div>
@@ -35,7 +45,7 @@
                   {{ tip(None, _('Used in app detail pages.')) }}
                 </div>
               </div>
-              <div class="icon_preview_box">
+              <div class="icon_preview_box defunct">
                 <div class="icon_preview" id="icon_preview_32">
                   <img src="{{ addon.get_icon_url(32) }}">
                 </div>
@@ -63,7 +73,7 @@
             </div>
             <div class="edit-media-details">
               {% trans %}
-              PNG and JPG supported. Icons resized to 64x64 pixels if larger.
+              PNG and JPG supported. Icons must be at least 128x128px.
               {% endtrans %}
             </div>
             <ul class="errorlist">
@@ -72,8 +82,9 @@
           {% else %}
             {% call empty_unless(addon.icon_type) %}
               <div id="icon_preview_readonly">
-                <img src="{{ addon.get_icon_url(64) }}">
-                <img src="{{ addon.icon_url }}">
+                <img src="{{ addon.get_icon_url(128) }}">
+                {# <img src="{{ addon.get_icon_url(64) }}"> #}
+                {# <img src="{{ addon.icon_url }}"> #}
               </div>
             {% endcall %}
           {% endif %}

mkt/developers/tests/test_tasks.py

@@ -37,17 +37,17 @@ def test_resize_icon_shrink():
 def test_resize_icon_enlarge():
     """ Image stays the same, since the new size is bigger than both sides. """
 
-    resize_size = 100
-    final_size = (82, 31)
+    resize_size = 1000
+    final_size = (339, 128)
 
     _uploader(resize_size, final_size)
 
 
 def test_resize_icon_same():
     """ Image stays the same, since the new size is the same. """
 
-    resize_size = 82
-    final_size = (82, 31)
+    resize_size = 339
+    final_size = (339, 128)
 
     _uploader(resize_size, final_size)
 
@@ -56,14 +56,14 @@ def test_resize_icon_list():
     """ Resize multiple images at once. """
 
     resize_size = [32, 82, 100]
-    final_size = [(32, 12), (82, 31), (82, 31)]
+    final_size = [(32, 12), (82, 30), (100, 37)]
 
     _uploader(resize_size, final_size)
 
 
 def _uploader(resize_size, final_size):
     img = get_image_path('mozilla.png')
-    original_size = (82, 31)
+    original_size = (339, 128)
 
     src = tempfile.NamedTemporaryFile(mode='r+w+b', suffix=".png",
                                       delete=False)

mkt/developers/tests/test_views_edit.py

@@ -587,7 +587,7 @@ def test_edit_icon_log(self):
         eq_(log[0].action, amo.LOG.CHANGE_ICON.id)
 
     def test_edit_uploadedicon_noresize(self):
-        img = '%s/img/notifications/error.png' % settings.MEDIA_ROOT
+        img = '%s/img/mkt/logos/128.png' % settings.MEDIA_ROOT
         src_image = open(img, 'rb')
 
         data = dict(upload_image=src_image)
@@ -618,9 +618,9 @@ def test_edit_uploadedicon_noresize(self):
                                '%s' % (webapp.id / 1000))
         dest = os.path.join(dirname, '%s-64.png' % webapp.id)
 
-        assert os.path.exists(dest)
+        assert os.path.exists(dest), dest
 
-        eq_(Image.open(dest).size, (48, 48))
+        eq_(Image.open(dest).size, (64, 64))
 
     def test_no_video_types(self):
         res = self.client.get(self.get_url('media', edit=True))
@@ -727,7 +727,7 @@ def check_image_animated(self, url, msg):
 
         res = self.client.post(url, {'upload_image': filehandle})
         response_json = json.loads(res.content)
-        eq_(response_json['errors'][0], msg)
+        assert any(e == msg for e in response_json['errors'])
 
     def test_icon_animated(self):
         self.check_image_animated(self.icon_upload,
@@ -770,7 +770,8 @@ def add_json(self, handle, Video):
     def test_edit_preview_video_add_hash(self):
         Switch.objects.create(name='video-upload', active=True)
         res = self.add_json(open(video_files['good'], 'rb'))
-        assert res['upload_hash'].endswith('.video-webm')
+        assert not res['errors'], res['errors']
+        assert res['upload_hash'].endswith('.video-webm'), res['upload_hash']
 
     def test_edit_preview_video_add_hash_switch_off(self):
         res = self.add_json(open(video_files['good'], 'rb'))
@@ -784,7 +785,8 @@ def test_edit_preview_add_hash(self):
     def test_edit_preview_video_size(self):
         Switch.objects.create(name='video-upload', active=True)
         res = self.add_json(open(video_files['good'], 'rb'))
-        assert res['errors'][0].startswith('Please use')
+        assert any(e.startswith('Please use') for e in res['errors']), (
+                res['errors'])
 
     def test_edit_preview_video_add(self):
         Switch.objects.create(name='video-upload', active=True)

mkt/developers/tests/test_views_validation.py

@@ -150,25 +150,25 @@ def test_json_results_get(self, has_been_validated):
         eq_(self.client.get(self.json_url).status_code, 405)
 
 
-class TestValidateAddon(amo.tests.TestCase):
-    fixtures = ['base/users']
-
-    def setUp(self):
-        super(TestValidateAddon, self).setUp()
-        assert self.client.login(username='regular@mozilla.com',
-                                 password='password')
-
-    def test_login_required(self):
-        self.client.logout()
-        r = self.client.get(reverse('mkt.developers.validate_addon'))
-        eq_(r.status_code, 302)
-
-    def test_context(self):
-        r = self.client.get(reverse('mkt.developers.validate_addon'))
-        eq_(r.status_code, 200)
-        doc = pq(r.content)
-        eq_(doc('#upload-addon').attr('data-upload-url'),
-            reverse('mkt.developers.standalone_upload'))
+#class TestValidateAddon(amo.tests.TestCase):
+#    fixtures = ['base/users']
+#
+#    def setUp(self):
+#        super(TestValidateAddon, self).setUp()
+#        assert self.client.login(username='regular@mozilla.com',
+#                                 password='password')
+#
+#    def test_login_required(self):
+#        self.client.logout()
+#        r = self.client.get(reverse('mkt.developers.validate_addon'))
+#        eq_(r.status_code, 302)
+#
+#    def test_context(self):
+#        r = self.client.get(reverse('mkt.developers.validate_addon'))
+#        eq_(r.status_code, 200)
+#        doc = pq(r.content)
+#        eq_(doc('#upload-addon').attr('data-upload-url'),
+#            reverse('mkt.developers.standalone_upload'))
 
 
 class TestValidateFile(BaseUploadTest):

mkt/developers/urls.py

@@ -93,11 +93,12 @@ def paypal_patterns(prefix):
 urlpatterns = decorate(write, patterns('',
     # Redirect people who have /apps/ instead of /app/.
     ('^apps/\d+/.*',
-     lambda r: redirect(r.path.replace('addons', 'addon', 1))),
+     lambda r: redirect(r.path.replace('apps', 'app', 1))),
 
-    # Standalone validator:
-    url('^addon/validate/?$', views.validate_addon,
-        name='mkt.developers.validate_addon'),
+    # There's no validator yet, but this is where it will go.
+    ## Standalone validator:
+    #url('^addon/validate/?$', views.validate_addon,
+    #    name='mkt.developers.validate_addon'),
 
     # Redirect to /addons/ at the base.
     url('^submissions$', use_apps(views.dashboard),

mkt/developers/utils.py

@@ -9,6 +9,7 @@
 import waffle
 
 import amo
+import mkt.constants.submit as submit_constants
 from lib.video import library as video_library
 
 
@@ -46,6 +47,12 @@ def check_upload(file_obj, upload_type, content_type):
                 errors.append(_('Icons must be either PNG or JPG.'))
             else:
                 errors.append(_('Images must be either PNG or JPG.'))
+        elif is_icon:
+            # The upload is an image and it's intended to be an icon.
+            icon_width, icon_height = check.img.size
+            min_width, min_height = submit_constants.APP_ICON_MIN_SIZE
+            if icon_width < min_width or icon_height < min_height:
+                errors.append(_('The icon must be at least 128x128px.'))
 
         if check.is_animated():
             if is_icon:

mkt/submit/tests/test_views.py

@@ -524,6 +524,12 @@ def test_icon(self):
             assert os.path.exists(os.path.join(ad.get_icon_dir(), fn)), (
                     'Expected %s in %s' % (fn, os.listdir(ad.get_icon_dir())))
 
+    def test_icon_too_small(self):
+        self._step()
+        hash = self.upload_icon(get_image_path('non-animated-thin.png'))
+        # You get an empty hash when there's an upload error.
+        eq_(hash, '')
+
     def _setup_other_webapp(self):
         self._step()
         # Generate another webapp to test name uniqueness.