Cleaned up status handling on version changes (bug 800086)

Author: Rob Hudson

apps/addons/models.py

@@ -819,7 +819,7 @@ def get_icon_url(self, size, use_default=True):
 
     def update_status(self, using=None):
         if (self.status in [amo.STATUS_NULL, amo.STATUS_DELETED]
-            or self.is_disabled or self.is_persona()):
+            or self.is_disabled or self.is_persona() or self.is_webapp()):
             return
 
         def logit(reason, old=self.status):

mkt/developers/tests/test_views_versions.py

@@ -6,6 +6,7 @@
 import amo.tests
 from addons.models import Addon, AddonUser
 from devhub.models import ActivityLog
+from files.models import File
 from users.models import UserProfile
 from versions.models import Version
 
@@ -156,6 +157,20 @@ def test_unique_version(self):
         self.assertFormError(res, 'upload_form', 'upload',
                              'Version 1.0 already exists')
 
+    def test_pending_on_new_version(self):
+        # Test app rejection, then new version, updates app status to pending.
+        self.app.current_version.update(version='0.9',
+                                        created=self.days_ago(1))
+        self.app.update(status=amo.STATUS_REJECTED)
+        files = File.objects.filter(version__addon=self.app)
+        files.update(status=amo.STATUS_DISABLED)
+        self._post(302)
+        version = self.app.versions.latest()
+        eq_(version.version, '1.0')
+        eq_(version.all_files[0].status, amo.STATUS_PENDING)
+        self.app.update_status()
+        eq_(self.app.status, amo.STATUS_PENDING)
+
 
 class TestEditVersion(amo.tests.TestCase):
     fixtures = ['base/users']

mkt/webapps/models.py

@@ -372,6 +372,38 @@ def mark_done(self):
         """When the submission process is done, update status accordingly."""
         self.update(status=amo.WEBAPPS_UNREVIEWED_STATUS)
 
+    def update_status(self, using=None):
+        if self.is_deleted or self.is_disabled:
+            return
+
+        def _log(reason, old=self.status):
+            log.info(u'Update app status [%s]: %s => %s (%s).' % (
+                self.id, old, self.status, reason))
+            amo.log(amo.LOG.CHANGE_STATUS, self.get_status_display(), self)
+
+        # Handle the case of no versions.
+        if not self.versions.exists():
+            self.update(status=amo.STATUS_NULL)
+            _log('no versions')
+            return
+
+        # Handle the case of versions with no files.
+        if not self.versions.filter(files__isnull=False).exists():
+            self.update(status=amo.STATUS_NULL)
+            _log('no versions with files')
+            return
+
+        # If there are no public versions and at least one pending, set status
+        # to pending.
+        has_public = (
+            self.versions.filter(files__status=amo.STATUS_PUBLIC).exists())
+        has_pending = (
+            self.versions.filter(files__status=amo.STATUS_PENDING).exists())
+        if not has_public and has_pending:
+            self.update(status=amo.STATUS_PENDING)
+            _log('has pending but no public files')
+            return
+
     def authors_other_addons(self, app=None):
         """Return other apps by the same author."""
         return (self.__class__.objects.visible()

mkt/webapps/tests/test_models.py

@@ -4,6 +4,7 @@
 import zipfile
 
 from django.conf import settings
+from django.db.models.signals import post_delete, post_save
 
 import mock
 import waffle
@@ -12,7 +13,8 @@
 
 import amo
 from addons.models import (Addon, AddonCategory, AddonDeviceType, AddonPremium,
-                           BlacklistedSlug, Category, Preview)
+                           BlacklistedSlug, Category, Preview, version_changed)
+from addons.signals import version_changed as version_changed_signal
 from amo.tests import app_factory, version_factory
 from constants.applications import DEVICE_TYPES
 from editors.models import RereviewQueue
@@ -21,7 +23,7 @@
 from lib.crypto.tests import mock_sign
 from market.models import Price
 from users.models import UserProfile
-from versions.models import Version
+from versions.models import update_status, Version
 
 import mkt
 from mkt.submit.tests.test_views import BasePackagedAppTest, BaseWebAppTest
@@ -928,3 +930,63 @@ def test_packaged_reviewer(self, sign):
                                          reviewer=True)
         eq_(sign.call_args[0][0], self.app.current_version.pk)
         eq_(sign.call_args[1]['reviewer'], True)
+
+
+class TestUpdateStatus(amo.tests.TestCase):
+
+    def setUp(self):
+        # Disabling signals to simplify these tests and because create doesn't
+        # call the signals anyway.
+        version_changed_signal.disconnect(version_changed,
+                                          dispatch_uid='version_changed')
+        post_save.disconnect(update_status, sender=Version,
+                             dispatch_uid='version_update_status')
+        post_delete.disconnect(update_status, sender=Version,
+                               dispatch_uid='version_update_status')
+
+    def tearDown(self):
+        version_changed_signal.connect(version_changed,
+                                       dispatch_uid='version_changed')
+        post_save.connect(update_status, sender=Version,
+                          dispatch_uid='version_update_status')
+        post_delete.connect(update_status, sender=Version,
+                            dispatch_uid='version_update_status')
+
+    def test_no_versions(self):
+        app = Webapp.objects.create(status=amo.STATUS_PUBLIC)
+        app.update_status()
+        eq_(app.status, amo.STATUS_NULL)
+
+    def test_version_no_files(self):
+        app = Webapp.objects.create(status=amo.STATUS_PUBLIC)
+        Version(addon=app).save()
+        app.update_status()
+        eq_(app.status, amo.STATUS_NULL)
+
+    def test_only_version_deleted(self):
+        app = amo.tests.app_factory(status=amo.STATUS_REJECTED)
+        app.current_version.delete()
+        app.update_status()
+        eq_(app.status, amo.STATUS_NULL)
+
+    def test_other_version_deleted(self):
+        app = amo.tests.app_factory(status=amo.STATUS_REJECTED)
+        amo.tests.version_factory(addon=app)
+        app.current_version.delete()
+        app.update_status()
+        eq_(app.status, amo.STATUS_REJECTED)
+
+    def test_one_version_pending(self):
+        app = amo.tests.app_factory(status=amo.STATUS_REJECTED,
+                                    file_kw=dict(status=amo.STATUS_DISABLED))
+        amo.tests.version_factory(addon=app,
+                                  file_kw=dict(status=amo.STATUS_PENDING))
+        app.update_status()
+        eq_(app.status, amo.STATUS_PENDING)
+
+    def test_one_version_public(self):
+        app = amo.tests.app_factory(status=amo.STATUS_PUBLIC)
+        amo.tests.version_factory(addon=app,
+                                  file_kw=dict(status=amo.STATUS_DISABLED))
+        app.update_status()
+        eq_(app.status, amo.STATUS_PUBLIC)