File.status cleanup (bug 778779)

Author: Rob Hudson

apps/constants/base.py

@@ -111,6 +111,10 @@
 WEBAPPS_UNLISTED_STATUSES = (STATUS_DISABLED, STATUS_PENDING,
                              STATUS_PUBLIC_WAITING, STATUS_REJECTED)
 
+# The only statuses we use in the marketplace.
+MARKET_STATUSES = (STATUS_NULL, STATUS_PENDING, STATUS_PUBLIC, STATUS_DISABLED,
+                   STATUS_DELETED, STATUS_REJECTED, STATUS_PUBLIC_WAITING)
+
 # Types of administrative review queues for an add-on:
 ADMIN_REVIEW_FULL = 1
 ADMIN_REVIEW_PRELIM = 2

apps/files/models.py

@@ -150,20 +150,19 @@ def get_url_path(self, src, addon=None):
 
     @classmethod
     def from_upload(cls, upload, version, platform, parse_data={}):
+        is_webapp = version.addon.is_webapp()
         f = cls(version=version, platform=platform)
         upload.path = amo.utils.smart_path(nfd_str(upload.path))
-        ext = os.path.splitext(upload.path)[1]
-        f.filename = f.generate_filename(extension=ext or '.xpi')
+        f.filename = f.generate_filename(os.path.splitext(upload.path)[1])
         # Size in kilobytes.
-        f.size = int(max(1, round(storage.size(upload.path) / 1024, 0)))
+        f.size = int(max(1, round(storage.size(upload.path) / 1024)))
         data = cls.get_jetpack_metadata(upload.path)
         f.jetpack_version = data['sdkVersion']
         f.builder_version = data['builderVersion']
         f.no_restart = parse_data.get('no_restart', False)
         f.strict_compatibility = parse_data.get('strict_compatibility', False)
-        if version.addon.is_webapp():
-            # Files don't really matter for webapps, just make them public.
-            f.status = amo.STATUS_PUBLIC
+        if is_webapp:
+            f.status = amo.STATUS_PENDING
         elif version.addon.status == amo.STATUS_PUBLIC:
             if amo.VERSION_BETA.search(parse_data.get('version', '')):
                 f.status = amo.STATUS_BETA
@@ -220,25 +219,32 @@ def generate_hash(self, filename=None):
                 hash.update(chunk)
         return 'sha256:%s' % hash.hexdigest()
 
-    def generate_filename(self, extension='.xpi'):
+    def generate_filename(self, extension=None):
         """
         Files are in the format of:
         {addon_name}-{version}-{apps}-{platform}
         """
         parts = []
+        addon = self.version.addon
         # slugify drops unicode so we may end up with an empty string.
         # Apache did not like serving unicode filenames (bug 626587).
-        name = slugify(self.version.addon.name).replace('-', '_') or 'addon'
-        parts.append(name)
-        parts.append(self.version.version)
-
-        if self.version.compatible_apps:
-            apps = '+'.join([a.shortername for a in
-                             self.version.compatible_apps])
-            parts.append(apps)
-
-        if self.platform_id and self.platform_id != amo.PLATFORM_ALL.id:
-            parts.append(amo.PLATFORMS[self.platform_id].shortname)
+        if addon.is_webapp():
+            extension = extension or '.webapp'
+            parts.append(addon.app_slug)
+            parts.append(self.version.version)
+        else:
+            extension = extension or '.xpi'
+            name = slugify(addon.name).replace('-', '_') or 'addon'
+            parts.append(name)
+            parts.append(self.version.version)
+
+            if self.version.compatible_apps:
+                apps = '+'.join([a.shortername for a in
+                                 self.version.compatible_apps])
+                parts.append(apps)
+
+            if self.platform_id and self.platform_id != amo.PLATFORM_ALL.id:
+                parts.append(amo.PLATFORMS[self.platform_id].shortname)
 
         self.filename = '-'.join(parts) + extension
         return self.filename

apps/files/tests/test_models.py

@@ -225,6 +225,12 @@ def test_generate_filename(self):
         f = File.objects.get(id=67442)
         eq_(f.generate_filename(), 'delicious_bookmarks-2.1.072-fx.xpi')
 
+    def test_generate_filename_webapp(self):
+        f = File.objects.get(id=67442)
+        f.version.addon.app_slug = 'testing-123'
+        f.version.addon.type = amo.ADDON_WEBAPP
+        eq_(f.generate_filename(), 'testing-123-2.1.072.webapp')
+
     def test_pretty_filename(self):
         f = File.objects.get(id=67442)
         f.generate_filename()
@@ -260,7 +266,6 @@ def clean_files(self, f):
             with storage.open(f.file_path, 'w') as fp:
                 fp.write('sample data\n')
 
-
     def test_copy_to_mirror(self):
         f = File.objects.get(id=67442)
         self.clean_files(f)
@@ -574,7 +579,8 @@ def test_validator_sets_binary_via_extensions(self):
                 "id": "gkobes@gkobes",
             }
         })
-        upload = self.get_upload(filename='extension.xpi', validation=validation)
+        upload = self.get_upload(filename='extension.xpi',
+                                 validation=validation)
         version = Version.objects.filter(addon__pk=3615)[0]
         plat = Platform.objects.get(pk=amo.PLATFORM_LINUX.id)
         file_ = File.from_upload(upload, version, plat)
@@ -595,7 +601,8 @@ def test_validator_sets_binary_via_content(self):
                 "id": "gkobes@gkobes",
             }
         })
-        upload = self.get_upload(filename='extension.xpi', validation=validation)
+        upload = self.get_upload(filename='extension.xpi',
+                                 validation=validation)
         version = Version.objects.filter(addon__pk=3615)[0]
         plat = Platform.objects.get(pk=amo.PLATFORM_LINUX.id)
         file_ = File.from_upload(upload, version, plat)
@@ -616,7 +623,8 @@ def test_validator_sets_require_chrome(self):
                 "requires_chrome": True
             }
         })
-        upload = self.get_upload(filename='extension.xpi', validation=validation)
+        upload = self.get_upload(filename='extension.xpi',
+                                 validation=validation)
         version = Version.objects.filter(addon__pk=3615)[0]
         plat = Platform.objects.get(pk=amo.PLATFORM_LINUX.id)
         file_ = File.from_upload(upload, version, plat)

apps/files/utils.py

@@ -1,5 +1,4 @@
 import collections
-import cPickle as pickle
 import glob
 import hashlib
 import json
@@ -57,6 +56,7 @@ def get_filepath(fileorpath):
         return fileorpath.path
     return fileorpath
 
+
 def get_file(fileorpath):
     """Get a file-like object, whether given a FileUpload object or a path."""
     if hasattr(fileorpath, 'path'):  # FileUpload
@@ -438,17 +438,15 @@ def parse_addon(pkg, addon=None):
     or files.models.FileUpload.
     """
     name = getattr(pkg, 'name', pkg)
-    if (getattr(pkg, 'is_webapp', False) or
-        name.endswith('.webapp') or name.endswith('.json')):
+    if getattr(pkg, 'is_webapp', False) or name.endswith(('.webapp', '.json')):
         parsed = WebAppParser().parse(pkg, addon)
     elif name.endswith('.xml'):
         parsed = parse_search(pkg, addon)
     else:
         parsed = parse_xpi(pkg, addon)
 
     if addon and addon.type != parsed['type']:
-        raise forms.ValidationError(
-            _("<em:type> doesn't match add-on"))
+        raise forms.ValidationError(_("<em:type> doesn't match add-on"))
     return parsed
 
 

apps/versions/models.py

@@ -83,7 +83,7 @@ def from_upload(cls, upload, addon, platforms, send_signal=True):
             AV(version=v, min=app.min, max=app.max,
                application_id=app.id).save()
         if addon.type in [amo.ADDON_SEARCH, amo.ADDON_WEBAPP]:
-            # Search extensions are always for all platforms.
+            # Search extensions and webapps are always for all platforms.
             platforms = [Platform.objects.get(id=amo.PLATFORM_ALL.id)]
         else:
             platforms = cls._make_safe_platform_files(platforms)
@@ -92,8 +92,7 @@ def from_upload(cls, upload, addon, platforms, send_signal=True):
             File.from_upload(upload, v, platform, parse_data=data)
 
         v.disable_old_files()
-        # After the upload has been copied to all
-        # platforms, remove the upload.
+        # After the upload has been copied to all platforms, remove the upload.
         storage.delete(upload.path)
         if send_signal:
             version_uploaded.send(sender=v)
@@ -385,8 +384,9 @@ def rollup(xs):
     def disable_old_files(self):
         if not self.files.filter(status=amo.STATUS_BETA).exists():
             qs = File.objects.filter(version__addon=self.addon_id,
-                                     version__lt=self,
-                                     status=amo.STATUS_UNREVIEWED)
+                                     version__lt=self.id,
+                                     status__in=[amo.STATUS_UNREVIEWED,
+                                                 amo.STATUS_PENDING])
             # Use File.update so signals are triggered.
             for f in qs:
                 f.update(status=amo.STATUS_DISABLED)

mkt/developers/tests/test_tasks.py

@@ -25,7 +25,7 @@
 
 import mkt
 from mkt.developers import tasks
-from mkt.developers.tests.test_views import BaseWebAppTest
+from mkt.submit.tests.test_views import BaseWebAppTest
 from mkt.webapps.models import AddonExcludedRegion as AER
 
 

mkt/developers/tests/test_views.py

@@ -29,7 +29,6 @@
 from devhub.models import UserLog
 from files.models import FileUpload
 from files.tests.test_models import UploadTest as BaseUploadTest
-from lib.pay_server import client
 from market.models import AddonPremium, Price, Refund
 from mkt.developers import tasks
 from mkt.developers.models import ActivityLog
@@ -1457,69 +1456,6 @@ def assert_json_field(request, field, msg):
     eq_(content[field], msg)
 
 
-class BaseWebAppTest(BaseUploadTest, amo.tests.TestCase):
-    fixtures = ['base/apps', 'base/users', 'base/platforms']
-
-    def setUp(self):
-        super(BaseWebAppTest, self).setUp()
-        self.manifest = os.path.join(settings.ROOT, 'apps', 'devhub', 'tests',
-                                     'addons', 'mozball.webapp')
-        self.upload = self.get_upload(abspath=self.manifest)
-        self.url = reverse('submit.app.manifest')
-        assert self.client.login(username='regular@mozilla.com',
-                                 password='password')
-        self.client.post(reverse('submit.app.terms'),
-                         {'read_dev_agreement': True})
-
-    def post(self, desktop_platforms=[amo.PLATFORM_ALL], mobile_platforms=[],
-             expect_errors=False):
-        d = dict(upload=self.upload.pk,
-                 desktop_platforms=[p.id for p in desktop_platforms],
-                 mobile_platforms=[p.id for p in mobile_platforms])
-        r = self.client.post(self.url, d, follow=True)
-        eq_(r.status_code, 200)
-        if not expect_errors:
-            # Show any unexpected form errors.
-            if r.context and 'new_addon_form' in r.context:
-                eq_(r.context['new_addon_form'].errors.as_text(), '')
-        return r
-
-    def post_addon(self):
-        eq_(Addon.objects.count(), 0)
-        self.post()
-        return Addon.objects.get()
-
-
-class TestCreateWebApp(BaseWebAppTest):
-
-    def test_post_app_redirect(self):
-        r = self.post()
-        addon = Addon.objects.get()
-        self.assertRedirects(r, reverse('submit.app.details',
-                                        args=[addon.app_slug]))
-
-    def test_addon_from_uploaded_manifest(self):
-        addon = self.post_addon()
-        eq_(addon.type, amo.ADDON_WEBAPP)
-        eq_(addon.guid, None)
-        eq_(unicode(addon.name), 'MozillaBall')
-        eq_(addon.slug, 'app-%s' % addon.id)
-        eq_(addon.app_slug, 'mozillaball')
-        eq_(addon.summary, u'Exciting Open Web development action!')
-        eq_(Translation.objects.get(id=addon.summary.id, locale='it'),
-            u'Azione aperta emozionante di sviluppo di fotoricettore!')
-
-    def test_version_from_uploaded_manifest(self):
-        addon = self.post_addon()
-        eq_(addon.current_version.version, '1.0')
-
-    def test_file_from_uploaded_manifest(self):
-        addon = self.post_addon()
-        files = addon.current_version.files
-        eq_(files.count(), 1)
-        eq_(files.all()[0].status, amo.STATUS_PUBLIC)
-
-
 class TestDeleteApp(amo.tests.TestCase):
     fixtures = ['base/apps', 'base/users', 'webapps/337141-steamcube']