Replaces all traces of addon nomination date with version nom. date (…

…bug 638855)

apps/addons/admin.py

@@ -11,7 +11,7 @@ class AddonAdmin(admin.ModelAdmin):
     fieldsets = (
         (None, {
             'fields': ('name', 'guid', 'default_locale', 'type', 'status',
-                       'highest_status', 'nomination_date'),
+                       'highest_status'),
         }),
         ('Details', {
             'fields': ('summary', 'description', 'homepage', 'eula',

apps/addons/models.py

@@ -179,8 +179,6 @@ class Addon(amo.models.OnChangeMixin, amo.models.ModelBase):
 
     nomination_message = models.TextField(null=True,
                                           db_column='nominationmessage')
-    nomination_date = models.DateTimeField(null=True,
-                                           db_column='nominationdate')
     target_locale = models.CharField(
         max_length=255, db_index=True, blank=True, null=True,
         help_text="For dictionaries and language packs")
@@ -871,7 +869,15 @@ def version_changed(sender, **kw):
 @Addon.on_change
 def watch_status(old_attr={}, new_attr={}, instance=None,
                  sender=None, **kw):
-    """Set nominationdate if self.status asks for full review."""
+    """Set nomination date if self.status asks for full review.
+
+    The nomination date will only be set when the status of the addon changes.
+    The nomination date cannot be reset, say, when a developer cancels their
+    request for full review and re-requests full review.
+
+    If a version is rejected after nomination, the developer has to upload a
+    new version.
+    """
     new_status = new_attr.get('status')
     if not new_status:
         return
@@ -884,7 +890,6 @@ def watch_status(old_attr={}, new_attr={}, instance=None,
                 latest.update(nomination=datetime.now())
         except Version.DoesNotExist:
             pass
-        addon.update(nomination_date=datetime.now())
 
 
 class MiniAddonManager(AddonManager):

apps/devhub/tests/test_views.py

@@ -1752,6 +1752,9 @@ def setUp(self):
     def get_addon(self):
         return Addon.objects.no_cache().get(pk=3615)
 
+    def get_version(self):
+        return self.get_addon().versions.get()
+
     def get_step(self):
         return SubmitStep.objects.get(addon=self.get_addon())
 
@@ -2204,13 +2207,13 @@ def test_prelim_review(self):
         assert_raises(SubmitStep.DoesNotExist, self.get_step)
 
     def test_full_review(self):
-        self.get_addon().update(nomination_date=None)
+        self.get_version().update(nomination=None)
         d = dict(review_type=amo.STATUS_NOMINATED)
         r = self.client.post(self.url, d)
         eq_(r.status_code, 302)
         addon = self.get_addon()
         eq_(addon.status, amo.STATUS_NOMINATED)
-        assert_close_to_now(addon.nomination_date)
+        assert_close_to_now(self.get_version().nomination)
         assert_raises(SubmitStep.DoesNotExist, self.get_step)
 
     def test_nomination_date_is_only_set_once(self):
@@ -2219,10 +2222,10 @@ def test_nomination_date_is_only_set_once(self):
         r = self.client.post(self.url, dict(review_type=amo.STATUS_NOMINATED))
         eq_(r.status_code, 302)
         nomdate = datetime.now() - timedelta(days=5)
-        self.get_addon().update(nomination_date=nomdate, _signal=False)
+        self.get_version().update(nomination=nomdate, _signal=False)
         # Update something else in the addon:
         self.get_addon().update(slug='foobar')
-        eq_(self.get_addon().nomination_date.timetuple()[0:5],
+        eq_(self.get_version().nomination.timetuple()[0:5],
             nomdate.timetuple()[0:5])
 
 
@@ -3099,6 +3102,9 @@ def setUp(self):
     def get_addon(self):
         return Addon.objects.get(id=self.addon.id)
 
+    def get_version(self):
+        return Version.objects.get(pk=self.version.id)
+
     def check(self, old_status, url, new_status):
         self.addon.update(status=old_status)
         r = self.client.post(url)
@@ -3131,21 +3137,19 @@ def test_lite_to_lite(self):
         self.check_400(self.lite_url)
 
     def test_lite_to_public(self):
-        eq_(self.addon.nomination_date, None)
+        eq_(self.version.nomination, None)
         self.check(amo.STATUS_LITE, self.public_url,
                    amo.STATUS_LITE_AND_NOMINATED)
-        self.addon = Addon.objects.get(pk=self.addon.id)
-        assert_close_to_now(self.addon.nomination_date)
+        assert_close_to_now(self.get_version().nomination)
 
     def test_purgatory_to_lite(self):
         self.check(amo.STATUS_PURGATORY, self.lite_url, amo.STATUS_UNREVIEWED)
 
     def test_purgatory_to_public(self):
-        eq_(self.addon.nomination_date, None)
+        eq_(self.version.nomination, None)
         self.check(amo.STATUS_PURGATORY, self.public_url,
                    amo.STATUS_NOMINATED)
-        self.addon = Addon.objects.get(pk=self.addon.id)
-        assert_close_to_now(self.addon.nomination_date)
+        assert_close_to_now(self.get_version().nomination)
 
     def test_lite_and_nominated_to_public(self):
         self.addon.update(status=amo.STATUS_LITE_AND_NOMINATED)
@@ -3159,23 +3163,27 @@ def test_lite_and_nominated(self):
     def test_renominate_for_full_review(self):
         # When a version is rejected, the addon is disabled.
         # The author must upload a new version and re-nominate.
-        self.addon.update(
-                # Pretend it was nominated in the past:
-                nomination_date=datetime.now() - timedelta(days=30))
+        # However, renominating the *same* version does not adjust the
+        # nomination date.
+        orig_date = nomination=datetime.now() - timedelta(days=30)
+        # Pretend it was nominated in the past:
+        self.version.update(nomination=orig_date)
         self.check(amo.STATUS_NULL, self.public_url, amo.STATUS_NOMINATED)
-        assert_close_to_now(self.get_addon().nomination_date)
+        eq_(self.get_version().nomination.timetuple()[0:5],
+            orig_date.timetuple()[0:5])
 
-    def test_renomination_resets_nomination_date(self):
+    def test_renomination_doesnt_reset_nomination_date(self):
         # Nominate:
         self.addon.update(status=amo.STATUS_LITE_AND_NOMINATED)
         # Pretend it was nominated in the past:
-        self.addon.update(nomination_date=datetime.now() - timedelta(days=30),
-                          _signal=False)
+        orig_date = datetime.now() - timedelta(days=30)
+        self.version.update(nomination=orig_date, _signal=False)
         # Reject it:
         self.addon.update(status=amo.STATUS_NULL)
         # Re-nominate:
         self.addon.update(status=amo.STATUS_LITE_AND_NOMINATED)
-        assert_close_to_now(self.get_addon().nomination_date)
+        eq_(self.get_version().nomination.timetuple()[0:5],
+            orig_date.timetuple()[0:5])
 
 
 class TestRedirects(test_utils.TestCase):

apps/editors/models.py

@@ -129,11 +129,11 @@ def base_query(self):
         q = super(ViewFullReviewQueue, self).base_query()
         q['select'].update({
             'waiting_time_days':
-                'TIMESTAMPDIFF(DAY, addons.nominationdate, NOW())',
+                'TIMESTAMPDIFF(DAY, versions.nomination, NOW())',
             'waiting_time_hours':
-                'TIMESTAMPDIFF(HOUR, addons.nominationdate, NOW())',
+                'TIMESTAMPDIFF(HOUR, versions.nomination, NOW())',
             'waiting_time_min':
-                'TIMESTAMPDIFF(MINUTE, addons.nominationdate, NOW())'
+                'TIMESTAMPDIFF(MINUTE, versions.nomination, NOW())'
         })
         q['where'].extend(['files.status <> %s' % amo.STATUS_BETA,
                            'addons.status IN (%s, %s)' % (

apps/editors/tests/test_models.py

@@ -167,7 +167,7 @@ def test_any_nominated_file_shows_up(self):
 
     def test_waiting_time(self):
         self.new_file(name='Addon 1', version=u'0.1')
-        Addon.objects.update(nomination_date=datetime.utcnow())
+        Version.objects.update(nomination=datetime.utcnow())
         row = self.Queue.objects.all()[0]
         eq_(row.waiting_time_days, 0)
         # Time zone will be off, hard to test this.

apps/editors/tests/test_views.py

@@ -682,34 +682,29 @@ def test_search_by_app_version(self):
         eq_(self.named_addons(r), [u'Bieber For Mobile 4.0b2pre'])
 
     def test_age_of_submission(self):
-        Addon.objects.update(
-                nomination_date=datetime.now() - timedelta(days=1))
-        bieber = (Addon.objects
-                  .filter(name__localized_string='Justin Bieber Persona'))
+        Version.objects.update(nomination=datetime.now() - timedelta(days=1))
+        title = 'Justin Bieber Persona'
+        bieber = (Version.objects.filter(addon__name__localized_string=title))
         # Exclude anything out of range:
-        bieber.update(nomination_date=datetime.now() - timedelta(days=5))
+        bieber.update(nomination=datetime.now() - timedelta(days=5))
         r = self.search({'waiting_time_days': 2})
         addons = self.named_addons(r)
-        assert 'Justin Bieber Persona' not in addons, (
-                                'Unexpected results: %r' % addons)
+        assert title not in addons, ('Unexpected results: %r' % addons)
         # Include anything submitted up to requested days:
-        bieber.update(nomination_date=datetime.now() - timedelta(days=2))
+        bieber.update(nomination=datetime.now() - timedelta(days=2))
         r = self.search({'waiting_time_days': 5})
         addons = self.named_addons(r)
-        assert 'Justin Bieber Persona' in addons, (
-                                'Unexpected results: %r' % addons)
+        assert title in addons, ('Unexpected results: %r' % addons)
         # Special case: exclude anything under 10 days:
-        bieber.update(nomination_date=datetime.now() - timedelta(days=8))
+        bieber.update(nomination=datetime.now() - timedelta(days=8))
         r = self.search({'waiting_time_days': '10+'})
         addons = self.named_addons(r)
-        assert 'Justin Bieber Persona' not in addons, (
-                                'Unexpected results: %r' % addons)
+        assert title not in addons, ('Unexpected results: %r' % addons)
         # Special case: include anything 10 days and over:
-        bieber.update(nomination_date=datetime.now() - timedelta(days=12))
+        bieber.update(nomination=datetime.now() - timedelta(days=12))
         r = self.search({'waiting_time_days': '10+'})
         addons = self.named_addons(r)
-        assert 'Justin Bieber Persona' in addons, (
-                                'Unexpected results: %r' % addons)
+        assert title in addons, ('Unexpected results: %r' % addons)
 
     def test_form(self):
         r = self.search({})

migrations/166-drop-addon-nomination_date.sql

@@ -0,0 +1,7 @@
+-- Moves nomination date from addon to version per bug 638855.
+-- Also see migration 163 which adds the nomination column to versions.
+UPDATE versions v
+    INNER JOIN addons a ON (a.id = v.addon_id AND v.nomination IS NULL)
+    SET v.nomination = a.nominationdate;
+-- Not deleting addons.nominationdate as a sacrificial token
+-- to please the gods of Remora