dedupe activity log (bug 641582)

apps/amo/cron.py

@@ -217,6 +217,7 @@ def _delete_anonymous_collections(items, **kw):
     Collection.objects.filter(type=amo.COLLECTION_ANONYMOUS,
                               pk__in=items).delete()
 
+
 @task
 def _delete_incomplete_addons(items, **kw):
     log.info('[%s@%s] Deleting incomplete add-ons' %
@@ -293,7 +294,11 @@ def _migrate_editor_eventlog(items, **kw):
 @cronjobs.register
 def migrate_approvals():
     a = MigrationTracker('approvals')
-    id = a.get() or 0
+    id = a.get()
+    if not id:
+        log.warning('No last position reported from redis for '
+                    'migrating approvals, exiting.')
+        return
 
     items = (Approval.objects.filter(pk__gt=id).order_by('id')
                              .values_list('id', flat=True))

apps/amo/management/commands/dedupe_approvals.py

@@ -0,0 +1,14 @@
+from django.core.management.base import BaseCommand
+
+from addons.models import Addon
+from amo.utils import chunked
+from amo.tasks import dedupe_approvals
+
+
+class Command(BaseCommand):
+    help = 'Dedupes activity for addon approvals log'
+
+    def handle(self, *args, **options):
+        pks = Addon.objects.values_list('pk', flat=True).order_by('id')
+        for chunk in chunked(pks, 100):
+            dedupe_approvals(chunk)

apps/amo/tasks.py

@@ -1,3 +1,5 @@
+import json
+
 from django.conf import settings
 
 import commonware.log
@@ -5,7 +7,10 @@ import celery.task
 from celeryutils import task
 from hera.contrib.django_utils import flush_urls
 
-from . import cron
+from addons.models import Addon
+from devhub.models import ActivityLog
+from editors.helpers import LOG_STATUSES
+
 
 log = commonware.log.getLogger('z.task')
 
@@ -36,3 +41,34 @@ def flush_front_end_cache_urls(urls, **kw):
                 urls[index] = u"%s%s" % (settings.SITE_URL, url)
 
     flush_urls(urls)
+
+
+@task
+def dedupe_approvals(items, **kw):
+    log.info('[%s@%s] Deduping approval items starting with addon: %s' %
+             (len(items), dedupe_approvals.rate_limit, items[0]))
+    for addon in Addon.objects.filter(pk__in=items):
+        last = {}
+        for activity in (ActivityLog.objects.for_addons(addon)
+                                    .order_by('-id')
+                                    .filter(action__in=LOG_STATUSES)):
+            arguments = json.loads(activity._arguments)
+            current = {
+                'action': activity.action,
+                'created': activity.created.date(),
+                'user': activity.user.pk,
+                'addon': arguments[0]['addons.addon'],
+                'version': arguments[1]['versions.version'],
+            }
+            if activity._details:
+                details = json.loads(activity._details)
+                current.update({
+                    'reviewtype': details['reviewtype'],
+                    'comments': details['comments'],
+                })
+
+            if last and last == current:
+                log.info('Deleting duplicate activity log %s '
+                         'from addon %s' % (activity.pk, addon.pk))
+                activity.delete()
+            last = current.copy()

apps/amo/tests/test_cron.py

@@ -4,13 +4,16 @@ from nose.tools import eq_
 import test_utils
 
 import amo
+from amo.cron import gc, remove_extra_cats
+from amo.tasks import dedupe_approvals
 from addons.models import Addon, AddonCategory, Category
 from bandwagon.models import Collection
 from cake.models import Session
 from devhub.models import ActivityLog
 from files.models import TestResult, TestResultCache
 from stats.models import AddonShareCount, Contribution
-from amo.cron import gc, remove_extra_cats
+from users.models import UserProfile
+from versions.models import Version
 
 
 class GarbageTest(test_utils.TestCase):
@@ -81,3 +84,36 @@ class RemoveExtraCatTest(test_utils.TestCase):
         eq_(self.addon.categories.count(), 2)
         remove_extra_cats()
         eq_(self.addon.categories.count(), 2)
+
+
+class TestDedupeApprovals(test_utils.TestCase):
+    fixtures = ['base/users']
+
+    def setUp(self):
+        self.addon = Addon.objects.create(type=amo.ADDON_EXTENSION)
+        self.version = Version.objects.create(addon=self.addon)
+        amo.set_user(UserProfile.objects.get(username='editor'))
+
+    def test_dedupe(self):
+        for x in range(0, 4):
+            amo.log(amo.LOG.APPROVE_VERSION, self.addon, self.version)
+        dedupe_approvals([self.addon.pk])
+        eq_(ActivityLog.objects.for_addons(self.addon).count(), 1)
+
+    def test_dedupe_mix(self):
+        for x in range(0, 4):
+            amo.log(amo.LOG.APPROVE_VERSION, self.addon, self.version)
+        for x in range(0, 3):
+            amo.log(amo.LOG.REJECT_VERSION, self.addon, self.version)
+        for x in range(0, 5):
+            amo.log(amo.LOG.APPROVE_VERSION, self.addon, self.version)
+        dedupe_approvals([self.addon.pk])
+        eq_(ActivityLog.objects.for_addons(self.addon).count(), 3)
+
+    def test_dedupe_date(self):
+        # Test that a log spanning
+        old = amo.log(amo.LOG.APPROVE_VERSION, self.addon, self.version)
+        old.update(created=datetime.today() - timedelta(days=1))
+        amo.log(amo.LOG.APPROVE_VERSION, self.addon, self.version)
+        dedupe_approvals([self.addon.pk])
+        eq_(ActivityLog.objects.for_addons(self.addon).count(), 2)