Merge pull request #493 from magopian/1125890-allow-updateURL-for-unl…

…isted-addons

Allow updateURL for unlisted addons (bug 1125890)

apps/devhub/tasks.py

@@ -34,13 +34,13 @@
 
 @task
 @write
-def validator(upload_id, **kw):
+def validator(upload_id, listed=True, **kw):
     if not settings.VALIDATE_ADDONS:
         return None
     log.info('VALIDATING: %s' % upload_id)
     upload = FileUpload.objects.using('default').get(pk=upload_id)
     try:
-        result = run_validator(upload.path)
+        result = run_validator(upload.path, listed=listed)
         # Skip the "addon signed" warning if we're not signing.
         if not settings.SIGNING_SERVER:
             result = skip_signing_warning(result)
@@ -113,7 +113,7 @@ def file_validator(file_id, **kw):
 
 
 def run_validator(file_path, for_appversions=None, test_all_tiers=False,
-                  overrides=None, compat=False):
+                  overrides=None, compat=False, listed=True):
     """A pre-configured wrapper around the addon validator.
 
     *file_path*
@@ -139,6 +139,10 @@ def run_validator(file_path, for_appversions=None, test_all_tiers=False,
         validator to ignore certain tests that should not be run during bulk
         validation (see bug 735841).
 
+    *listed=True*
+        If the addon is unlisted, treat it as if it was a self hosted one
+        (don't fail on the presence of an updateURL).
+
     To validate the addon for compatibility with Firefox 5 and 6,
     you'd pass in::
 
@@ -174,7 +178,8 @@ def run_validator(file_path, for_appversions=None, test_all_tiers=False,
                             spidermonkey=settings.SPIDERMONKEY,
                             overrides=overrides,
                             timeout=settings.VALIDATOR_TIMEOUT,
-                            compat_test=compat)
+                            compat_test=compat,
+                            listed=listed)
     finally:
         if temp:
             os.remove(path)

apps/devhub/templates/devhub/addons/submit/upload.html

@@ -19,7 +19,21 @@ <h3>{{ _('Step 2. Upload Your Add-on') }}</h3>
   <div class="hidden">
     {{ new_addon_form.upload }}
   </div>
-  <input type="file" id="upload-addon" data-upload-url="{{ url('devhub.upload') }}">
+  <div class="list-addon">
+    <label>{{ _('Do you want your add-on listed on AMO?') }}</label>
+    <div>
+      <label for="{{ new_addon_form.is_listed.auto_id }}">
+        {{ new_addon_form.is_listed }}
+        {{ new_addon_form.is_listed.label }}
+        <span class="tip tooltip"
+              title="{{ new_addon_form.is_listed.help_text }}">?</span>
+      </label>
+    </div>
+  </div>
+  <input type="file" id="upload-addon"
+    data-upload-url="{{ url('devhub.upload') }}"
+    data-upload-url-listed="{{ url('devhub.upload') }}"
+    data-upload-url-unlisted="{{ url('devhub.upload_unlisted') }}">
 
   {{ new_addon_form.non_field_errors() }}
 
@@ -33,17 +47,6 @@ <h3>{{ _('Step 2. Upload Your Add-on') }}</h3>
         </div>
         {{ new_addon_form.upload }}
       </div>
-      <div class="list-addon">
-        <label>{{ _('Do you want your add-on listed on AMO?') }}</label>
-        <div>
-          <label for="{{ new_addon_form.is_listed.auto_id }}">
-            {{ new_addon_form.is_listed }}
-            {{ new_addon_form.is_listed.label }}
-            <span class="tip tooltip"
-                  title="{{ new_addon_form.is_listed.help_text }}">?</span>
-          </label>
-        </div>
-      </div>
       {% if is_admin %}
       <div class="admin-settings">
         <label>{{ _('Administrative overrides') }}</label>

apps/devhub/tests/test_views.py

@@ -1856,10 +1856,11 @@ def setUp(self):
         assert self.client.login(username='regular@mozilla.com',
                                  password='password')
         self.url = reverse('devhub.upload')
+        self.image_path = get_image_path('animated.png')
 
     def post(self):
         # Has to be a binary, non xpi file.
-        data = open(get_image_path('animated.png'), 'rb')
+        data = open(self.image_path, 'rb')
         return self.client.post(self.url, {'upload': data})
 
     def test_login_required(self):
@@ -1872,7 +1873,7 @@ def test_create_fileupload(self):
 
         upload = FileUpload.objects.get(name='animated.png')
         eq_(upload.name, 'animated.png')
-        data = open(get_image_path('animated.png'), 'rb').read()
+        data = open(self.image_path, 'rb').read()
         eq_(storage.open(upload.path).read(), data)
 
     def test_fileupload_user(self):
@@ -1906,6 +1907,15 @@ def test_redirect(self):
         url = reverse('devhub.upload_detail', args=[upload.pk, 'json'])
         self.assertRedirects(r, url)
 
+    @mock.patch('validator.validate.validate')
+    def test_upload_unlisted_addon(self, validate_mock):
+        """Unlisted addons are validated as "self hosted" addons."""
+        validate_mock.return_value = '{}'
+        self.url = reverse('devhub.upload_unlisted')
+        self.post()
+        # Make sure it was called with listed=False.
+        assert not validate_mock.call_args[1]['listed']
+
 
 class TestUploadDetail(BaseUploadTest):
     fixtures = ['base/appversion', 'base/users']

apps/devhub/urls.py

@@ -169,6 +169,8 @@
     url('^feed/all$', lambda r: redirect('devhub.feed_all', permanent=True)),
     url('^feed/%s$' % ADDON_ID, views.feed, name='devhub.feed'),
     url('^upload$', views.upload, name='devhub.upload'),
+    url('^upload/unlisted$', views.upload_unlisted,
+        name='devhub.upload_unlisted'),
     url('^upload/([^/]+)(?:/([^/]+))?$', views.upload_detail,
         name='devhub.upload_detail'),
     url('^standalone-upload$', views.standalone_upload,

apps/devhub/views.py

@@ -599,7 +599,7 @@ def file_perf_tests_start(request, addon_id, addon, file_id):
 @login_required
 @post_required
 @write
-def upload(request, addon_slug=None, is_standalone=False):
+def upload(request, addon_slug=None, is_standalone=False, is_listed=True):
     filedata = request.FILES['upload']
 
     fu = FileUpload.from_post(filedata, filedata.name, filedata.size)
@@ -615,7 +615,7 @@ def upload(request, addon_slug=None, is_standalone=False):
         ver = get_object_or_404(AppVersion, pk=request.POST['version_id'])
         tasks.compatibility_check.delay(fu.pk, app.guid, ver.version)
     else:
-        tasks.validator.delay(fu.pk)
+        tasks.validator.delay(fu.pk, listed=is_listed)
     if addon_slug:
         return redirect('devhub.upload_detail_for_addon',
                         addon_slug, fu.pk)
@@ -625,6 +625,14 @@ def upload(request, addon_slug=None, is_standalone=False):
         return redirect('devhub.upload_detail', fu.pk, 'json')
 
 
+@login_required
+@post_required
+@write
+def upload_unlisted(request, addon_slug=None, is_standalone=False):
+    return upload(request, addon_slug=addon_slug, is_standalone=is_standalone,
+                  is_listed=False)
+
+
 @login_required
 @post_required
 @json_view
@@ -1334,10 +1342,9 @@ def submit_addon(request, step):
                     _("Skipping to step 6 because the add-on won't be listed"))
             SubmitStep.objects.create(addon=addon, step=next_step)
             return redirect(_step_url(next_step), addon.slug)
-    template = 'upload.html'
     is_admin = acl.action_allowed(request, 'ReviewerAdminTools', 'View')
 
-    return render(request, 'devhub/addons/submit/%s' % template,
+    return render(request, 'devhub/addons/submit/upload.html',
                   {'step': step, 'new_addon_form': form, 'is_admin': is_admin})
 
 

static/js/zamboni/devhub.js

@@ -52,6 +52,7 @@ $(document).ready(function() {
     $('.perf-tests').exists(initPerfTests, [window.document]);
 
     // Add-on uploader
+    var $uploadAddon = $('#upload-addon');
     if($('#upload-addon').length) {
         var opt = {'cancel': $('.upload-file-cancel') };
         if($('#addon-compat-upload').length) {
@@ -62,7 +63,7 @@ $(document).ready(function() {
                                 $('#id_app_version option:selected').val());
             };
         }
-        $('#upload-addon').addonUploader(opt);
+        $uploadAddon.addonUploader(opt);
         $('#id_admin_override_validation').addClass('addon-upload-failure-dependant')
             .change(function () {
                 if ($(this).attr('checked')) {
@@ -82,6 +83,29 @@ $(document).ready(function() {
         $('.addon-upload-failure-dependant').attr('disabled', true);
     }
 
+    // is_listed checkbox: should the add-on be listed on AMO? If not, change
+    // the addon upload button's data-upload-url.
+    var url = $uploadAddon.attr('data-upload-url');
+    // If this add-on is unlisted, then tell the upload view so
+    // it'll run the validator with the "listed=False"
+    // parameter.
+    var $isListed = $('#id_is_listed');
+    function switchUploadUrl() {
+      if ($isListed.exists() &&
+          $isListed.is(':checked')) {
+        $uploadAddon.attr('data-upload-url', $uploadAddon.attr('data-upload-url-listed'));
+      } else {
+        $uploadAddon.attr('data-upload-url', $uploadAddon.attr('data-upload-url-unlisted'));
+      }
+      /* Don't allow submitting, need to reupload/revalidate the file. */
+      $('.addon-upload-dependant').attr('disabled', true);
+      $('.addon-upload-failure-dependant').attr({'disabled': true,
+                                                 'checked': false});
+      $('.upload-status').remove();
+    }
+    $isListed.bind('change', switchUploadUrl);
+    switchUploadUrl();
+
     if ($(".add-file-modal").length) {
         $modalFile = $(".add-file-modal").modal(".version-upload", {
             width: '450px',