Merge branch 'reviews'

apps/access/acl.py

@@ -1,3 +1,7 @@
+import amo
+from addons.models import Addon
+
+
 def match_rules(rules, app, action):
     """
     This will match rules found in Group.
@@ -25,3 +29,23 @@ def action_allowed(request, app, action):
 
     return any(match_rules(group.rules, app, action)
         for group in getattr(request, 'groups', ()))
+
+
+def check_ownership(request, addon, require_owner=False):
+    """Check if request.user has owner permissions for the add-on."""
+    if not request.user.is_authenticated():
+        return False
+
+    if not require_owner and action_allowed(request, 'Admin', 'EditAnyAddon'):
+        return True
+
+    if addon.status == amo.STATUS_DISABLED:
+        return False
+
+    roles = (amo.AUTHOR_ROLE_ADMINOWNER, amo.AUTHOR_ROLE_ADMIN,
+             amo.AUTHOR_ROLE_OWNER, amo.AUTHOR_ROLE_DEV)
+    if not require_owner:
+        roles += (amo.AUTHOR_ROLE_VIEWER,)
+
+    return bool(addon.authors.filter(addonuser__role__in=roles,
+                                     user=request.user))

apps/access/tests.py

@@ -1,11 +1,13 @@
 from django.http import HttpRequest
-from nose.tools import assert_false
+
+import mock
+from nose.tools import assert_false, eq_
 
 import amo
 from cake.models import Session
 from test_utils import TestCase
 
-from .acl import match_rules, action_allowed
+from .acl import match_rules, action_allowed, check_ownership
 
 
 def test_match_rules():
@@ -83,3 +85,37 @@ def test_admin_login(self):
         c.login(session=session)
         response = c.get('/en-US/admin/models/')
         self.assertContains(response, 'login-form')
+
+
+class TestCheckOwnership(TestCase):
+
+    def setUp(self):
+        self.request = mock.Mock()
+        self.request.groups = ()
+        self.addon = mock.Mock()
+
+    def test_unauthenticated(self):
+        self.request.user.is_authenticated = lambda: False
+        eq_(False, check_ownership(self.request, self.addon))
+
+    @mock.patch('access.acl.action_allowed')
+    def test_admin(self, allowed):
+        eq_(True, check_ownership(self.request, self.addon))
+        eq_(True, check_ownership(self.request, self.addon,
+                                  require_owner=True))
+
+    def test_addon_status(self):
+        self.addon.status = amo.STATUS_DISABLED
+        eq_(False, check_ownership(self.request, self.addon))
+
+    def test_author_roles(self):
+        f = self.addon.authors.filter
+        roles = (amo.AUTHOR_ROLE_ADMINOWNER, amo.AUTHOR_ROLE_ADMIN,
+                 amo.AUTHOR_ROLE_OWNER, amo.AUTHOR_ROLE_DEV)
+
+        check_ownership(self.request, self.addon, True)
+        eq_(f.call_args[1]['addonuser__role__in'], roles)
+
+        check_ownership(self.request, self.addon)
+        eq_(f.call_args[1]['addonuser__role__in'],
+            roles + (amo.AUTHOR_ROLE_VIEWER,))

apps/addons/models.py

@@ -102,7 +102,7 @@ class Addon(amo.models.ModelBase):
     the_reason = TranslatedField()
     the_future = TranslatedField()
 
-    average_rating = models.CharField(max_length=255, default=0,
+    average_rating = models.CharField(max_length=255, default=0, null=True,
                                       db_column='averagerating')
     bayesian_rating = models.FloatField(default=0, db_index=True,
                                         db_column='bayesianrating')

apps/addons/tests/test_models.py

@@ -211,10 +211,12 @@ def test_review_replies(self):
         addon = Addon.objects.get(id=3615)
         u = UserProfile.objects.get(pk=2519)
         version = addon.current_version
-        new_review = Review(version=version, user=u, rating=2, body='hello')
+        new_review = Review(version=version, user=u, rating=2, body='hello',
+                            addon=addon)
         new_review.save()
         new_reply = Review(version=version, user=addon.authors.all()[0],
-                           reply_to=new_review, rating=2, body='my reply')
+                           addon=addon, reply_to=new_review,
+                           rating=2, body='my reply')
         new_reply.save()
 
         review_list = [r.pk for r in addon.reviews]

apps/amo/tests/test_redirects.py

@@ -11,7 +11,7 @@
 
 class TestRedirects(test.TestCase):
 
-    fixtures = ['amo/test_redirects', 'base/global-stats']
+    fixtures = ['reviews/test_models', 'base/global-stats']
 
     def test_persona_category(self):
         """`/personas/film and tv` should go to /personas/film-and-tv"""

apps/cronjobs/management/commands/cron.py

@@ -26,7 +26,7 @@ def handle(self, *args, **opts):
 
         if not args:
             log.error("Cron called but doesn't know what to do.")
-            print 'Try one of these: %s' % ', '.join(registered)
+            print 'Try one of these:\n%s' % '\n'.join(sorted(registered))
             sys.exit(1)
 
         script, args = args[0], args[1:]

apps/reviews/cron.py

@@ -0,0 +1,31 @@
+import logging
+
+from celery.messaging import establish_connection
+
+import cronjobs
+from amo.utils import chunked
+from addons.models import Addon
+
+from . import tasks
+from .models import Review
+
+log = logging.getLogger('z.cron')
+
+
+@cronjobs.register
+def reviews_denorm():
+    """Set is_latest and previous_count for all reviews."""
+    pairs = list(set(Review.objects.values_list('addon', 'user')))
+    with establish_connection() as conn:
+        for chunk in chunked(pairs, 50):
+            tasks.update_denorm.apply_async(args=chunk, connection=conn)
+
+
+@cronjobs.register
+def addon_reviews_ratings():
+    """Update all add-on total_reviews and average/bayesian ratings."""
+    addons = Addon.objects.values_list('id', flat=True)
+    with establish_connection() as conn:
+        for chunk in chunked(addons, 100):
+            tasks.cron_review_aggregate.apply_async(args=chunk,
+                                                    connection=conn)