diff --git a/apps/amo/tests/__init__.py b/apps/amo/tests/__init__.py index 5ce5b56f266..22bca918764 100644 --- a/apps/amo/tests/__init__.py +++ b/apps/amo/tests/__init__.py @@ -217,7 +217,7 @@ def decorated(request, *args, **kwargs): def days_ago(days): - return datetime.now() - timedelta(days=days) + return datetime.now().replace(microsecond=0) - timedelta(days=days) class TestCase(RedisTest, test_utils.TestCase): @@ -382,6 +382,8 @@ def assertCloseToNow(self, dt, now=None): """ Make sure the datetime is within a minute from `now`. """ + if not dt: + raise AssertionError('Expected datetime, got: %s' % dt) dt_later_ts = time.mktime((dt + timedelta(minutes=1)).timetuple()) dt_earlier_ts = time.mktime((dt - timedelta(minutes=1)).timetuple()) if not now: diff --git a/apps/versions/models.py b/apps/versions/models.py index 319725913d2..edc83cdedc8 100644 --- a/apps/versions/models.py +++ b/apps/versions/models.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- +import datetime import os import django.dispatch @@ -453,14 +454,32 @@ def inherit_nomination(sender, instance, **kw): """ if kw.get('raw'): return - if (instance.nomination is None - and instance.addon.status in (amo.STATUS_NOMINATED, - amo.STATUS_LITE_AND_NOMINATED) - and not instance.is_beta): - last_ver = (Version.objects.filter(addon=instance.addon) - .exclude(nomination=None).order_by('-nomination')) - if last_ver.exists(): - instance.update(nomination=last_ver[0].nomination) + addon = instance.addon + if (addon.type == amo.ADDON_WEBAPP and addon.is_packaged): + # If prior version's file is pending, inherit nomination. Otherwise, + # set nomination to now. + last_ver = (Version.objects.filter(addon=addon) + .exclude(pk=instance.pk) + .order_by('-nomination')) + if (last_ver.exists() and + last_ver[0].all_files[0].status == amo.STATUS_PENDING): + instance.update(nomination=last_ver[0].nomination, _signal=False) + log.debug('[Webapp:%s] Inheriting nomination from prior pending ' + 'version' % addon.id) + elif addon.status == amo.STATUS_PUBLIC and not instance.nomination: + log.debug('[Webapp:%s] Setting nomination date to now for new ' + 'version.' % addon.id) + instance.update(nomination=datetime.datetime.now(), _signal=False) + else: + if (instance.nomination is None + and addon.status in (amo.STATUS_NOMINATED, + amo.STATUS_LITE_AND_NOMINATED) + and not instance.is_beta): + last_ver = (Version.objects.filter(addon=addon) + .exclude(nomination=None).order_by('-nomination')) + if last_ver.exists(): + instance.update(nomination=last_ver[0].nomination, + _signal=False) def update_incompatible_versions(sender, instance, **kw): @@ -507,25 +526,27 @@ def clear_compatversion_cache_on_delete(sender, instance, **kw): version_uploaded = django.dispatch.Signal() -models.signals.post_save.connect(update_status, sender=Version, - dispatch_uid='version_update_status') -models.signals.post_save.connect(inherit_nomination, sender=Version, - dispatch_uid='version_inherit_nomination') -models.signals.post_delete.connect(update_status, sender=Version, - dispatch_uid='version_update_status') -models.signals.post_save.connect(update_incompatible_versions, sender=Version, - dispatch_uid='version_update_incompat') -models.signals.post_delete.connect(update_incompatible_versions, - sender=Version, - dispatch_uid='version_update_incompat') -models.signals.pre_delete.connect(cleanup_version, sender=Version, - dispatch_uid='cleanup_version') -models.signals.post_save.connect(clear_compatversion_cache_on_save, - sender=Version, - dispatch_uid='clear_compatversion_cache_save') -models.signals.post_delete.connect(clear_compatversion_cache_on_delete, - sender=Version, - dispatch_uid='clear_compatversion_cache_del') +models.signals.post_save.connect( + update_status, sender=Version, dispatch_uid='version_update_status') +models.signals.post_save.connect( + inherit_nomination, sender=Version, + dispatch_uid='version_inherit_nomination') +models.signals.post_delete.connect( + update_status, sender=Version, dispatch_uid='version_update_status') +models.signals.post_save.connect( + update_incompatible_versions, sender=Version, + dispatch_uid='version_update_incompat') +models.signals.post_delete.connect( + update_incompatible_versions, sender=Version, + dispatch_uid='version_update_incompat') +models.signals.pre_delete.connect( + cleanup_version, sender=Version, dispatch_uid='cleanup_version') +models.signals.post_save.connect( + clear_compatversion_cache_on_save, sender=Version, + dispatch_uid='clear_compatversion_cache_save') +models.signals.post_delete.connect( + clear_compatversion_cache_on_delete, sender=Version, + dispatch_uid='clear_compatversion_cache_del') class LicenseManager(amo.models.ManagerBase): diff --git a/migrations/547-set-version-nominations-for-apps.sql b/migrations/547-set-version-nominations-for-apps.sql new file mode 100644 index 00000000000..1dab113d608 --- /dev/null +++ b/migrations/547-set-version-nominations-for-apps.sql @@ -0,0 +1,5 @@ +UPDATE versions, addons +SET versions.nomination=versions.created +WHERE versions.addon_id=addons.id AND + addons.addontype_id=11 AND + versions.nomination IS NULL; diff --git a/mkt/reviewers/tests/test_views.py b/mkt/reviewers/tests/test_views.py index 5f227566a40..1e5505f01e7 100644 --- a/mkt/reviewers/tests/test_views.py +++ b/mkt/reviewers/tests/test_views.py @@ -218,9 +218,11 @@ class TestAppQueue(AppReviewerTest, AccessMixin, FlagsMixin, SearchMixin, def setUp(self): self.apps = [app_factory(name='XXX', - status=amo.STATUS_PENDING), + status=amo.STATUS_PENDING, + version_kw={'nomination': self.days_ago(2)}), app_factory(name='YYY', - status=amo.STATUS_PENDING), + status=amo.STATUS_PENDING, + version_kw={'nomination': self.days_ago(1)}), app_factory(name='ZZZ')] self.apps[0].update(created=self.days_ago(2)) self.apps[1].update(created=self.days_ago(1)) @@ -478,10 +480,12 @@ class TestUpdateQueue(AppReviewerTest, AccessMixin, FlagsMixin, SearchMixin, def setUp(self): app1 = app_factory(is_packaged=True, name='XXX', version_kw={'version': '1.0', - 'created': self.days_ago(2)}) + 'created': self.days_ago(2), + 'nomination': self.days_ago(2)}) app2 = app_factory(is_packaged=True, name='YYY', version_kw={'version': '1.0', - 'created': self.days_ago(2)}) + 'created': self.days_ago(2), + 'nomination': self.days_ago(2)}) version_factory(addon=app1, version='1.1', created=self.days_ago(1), nomination=self.days_ago(1), @@ -498,8 +502,8 @@ def review_url(self, app): return reverse('reviewers.apps.review', args=[app.app_slug]) def test_template_links(self): - self.apps[0].versions.latest().files.update(created=self.days_ago(2)) - self.apps[1].versions.latest().files.update(created=self.days_ago(1)) + self.apps[0].versions.latest().update(nomination=self.days_ago(2)) + self.apps[1].versions.latest().update(nomination=self.days_ago(1)) r = self.client.get(self.url) eq_(r.status_code, 200) links = pq(r.content)('#addon-queue tbody')('tr td:nth-of-type(2) a') @@ -594,8 +598,8 @@ def test_escalated_not_in_queue(self): def test_order(self): self.apps[0].update(created=self.days_ago(10)) self.apps[1].update(created=self.days_ago(5)) - self.apps[0].versions.latest().files.update(created=self.days_ago(1)) - self.apps[1].versions.latest().files.update(created=self.days_ago(4)) + self.apps[0].versions.latest().update(nomination=self.days_ago(1)) + self.apps[1].versions.latest().update(nomination=self.days_ago(4)) res = self.client.get(self.url) apps = list(res.context['addons']) eq_(apps[0].app, self.apps[1]) diff --git a/mkt/reviewers/views.py b/mkt/reviewers/views.py index fa373f169b1..701f88ae431 100644 --- a/mkt/reviewers/views.py +++ b/mkt/reviewers/views.py @@ -333,9 +333,8 @@ def _do_sort(request, qs): """Column sorting logic based on request GET parameters.""" sort, order = clean_sort_param(request) - if qs.model in [EscalationQueue, RereviewQueue] and sort != 'created': - # For some queues, want to use queue's created field, not the app's - # created field. + if qs.model is not Webapp and sort != 'created': + # For when `Webapp` isn't the base model of the queryset. sort = 'addon__' + sort if order == 'asc': @@ -355,13 +354,16 @@ def _do_sort(request, qs): @permission_required('Apps', 'Review') def queue_apps(request): excluded_ids = EscalationQueue.uncached.values_list('addon', flat=True) - qs = (Webapp.uncached.filter(type=amo.ADDON_WEBAPP, - disabled_by_user=False, - status=amo.STATUS_PENDING) - .exclude(id__in=excluded_ids)) - - qs, search_form = _get_search_form(request, _do_sort(request, qs)) - apps = [QueuedApp(app, app.created) for app in qs] + qs = (Version.uncached.filter(addon__type=amo.ADDON_WEBAPP, + addon__disabled_by_user=False, + addon__status=amo.STATUS_PENDING) + .exclude(addon__id__in=excluded_ids) + .order_by('nomination', 'created') + .select_related('addon').no_transforms()) + + qs, search_form = _queue_to_apps(request, qs) + apps = [QueuedApp(app, app.current_version.nomination) + for app in Webapp.version_and_file_transformer(qs)] return _queue(request, apps, 'pending', search_form) @@ -403,7 +405,7 @@ def queue_updates(request): .filter(id__in=addon_ids)) qs, search_form = _get_search_form(request, _do_sort(request, qs)) - apps = [QueuedApp(app, app.all_versions[0].all_files[0].created) + apps = [QueuedApp(app, app.all_versions[0].nomination) for app in Webapp.version_and_file_transformer(qs)] apps = sorted(apps, key=lambda a: a.created) return _queue(request, apps, 'updates', search_form) diff --git a/mkt/webapps/models.py b/mkt/webapps/models.py index 266c2bd5b04..baf63fbfaf4 100644 --- a/mkt/webapps/models.py +++ b/mkt/webapps/models.py @@ -752,6 +752,26 @@ def update_cached_manifests(sender, **kw): update_cached_manifests.delay(sender.id) +@Webapp.on_change +def watch_status(old_attr={}, new_attr={}, instance=None, sender=None, **kw): + """Set nomination date when app is pending review.""" + new_status = new_attr.get('status') + if not new_status: + return + addon = instance + if new_status == amo.STATUS_PENDING and old_attr['status'] != new_status: + # We always set nomination date when app switches to PENDING, even if + # previously rejected. + try: + latest = addon.versions.latest() + log.debug('[Webapp:%s] Setting nomination date to now.' % addon.id) + latest.update(nomination=datetime.datetime.now()) + except Version.DoesNotExist: + log.debug('[Webapp:%s] Missing version, no nomination set.' + % addon.id) + pass + + class ImageAsset(amo.models.ModelBase): addon = models.ForeignKey(Addon, related_name='image_assets') filetype = models.CharField(max_length=25, default='image/png') diff --git a/mkt/webapps/tests/test_models.py b/mkt/webapps/tests/test_models.py index a4c44785b51..2c29798e480 100644 --- a/mkt/webapps/tests/test_models.py +++ b/mkt/webapps/tests/test_models.py @@ -379,6 +379,39 @@ def test_app_type_hosted(self): def test_app_type_packaged(self): eq_(Webapp(is_packaged=True).app_type, 'packaged') + def test_nomination_new(self): + app = app_factory() + app.update(status=amo.STATUS_NULL) + app.versions.latest().update(nomination=None) + app.update(status=amo.STATUS_PENDING) + assert app.versions.latest().nomination + + def test_nomination_rejected(self): + app = app_factory() + app.update(status=amo.STATUS_REJECTED) + app.versions.latest().update(nomination=self.days_ago(1)) + app.update(status=amo.STATUS_PENDING) + self.assertCloseToNow(app.versions.latest().nomination) + + def test_nomination_pkg_pending_new_version(self): + # New versions while pending inherit version nomination. + app = app_factory() + app.update(status=amo.STATUS_PENDING, is_packaged=True) + old_ver = app.versions.latest() + old_ver.update(nomination=self.days_ago(1)) + old_ver.all_files[0].update(status=amo.STATUS_PENDING) + v = Version.objects.create(addon=app, version='1.9') + eq_(v.nomination, old_ver.nomination) + + def test_nomination_pkg_public_new_version(self): + # New versions while public get a new version nomination. + app = app_factory() + app.update(is_packaged=True) + old_ver = app.versions.latest() + old_ver.update(nomination=self.days_ago(1)) + v = Version.objects.create(addon=app, version='1.9') + self.assertCloseToNow(v.nomination) + class TestPackagedAppManifestUpdates(amo.tests.TestCase): # Note: More extensive tests for `Addon.update_names` are in the Addon