Merge lp:~michael.nelson/rnr-server/708841-caching-issue into lp:rnr-server

[Michael Nelson]

Overview
========

Fixes bug 708841 by ensuring that whenever the hide property of a review is changed, we update the stats on the corresponding softwareitem.

[Danny Tamez (zematynnad)]

everything looks good

review: Approve

[lp:~michael.nelson/rnr-server/708841-caching-issue]

118. By Michael Nelson on 2011-01-28

Ensure hidden reviews are not included in stats.

119. By Michael Nelson on 2011-01-28

Fixed issue where Django returns None for average of zero items.

120. By Michael Nelson on 2011-01-28

Merged trunk.

=== modified file 'src/reviewsapp/models/reviews.py'
--- src/reviewsapp/models/reviews.py	2011-01-28 15:39:42 +0000
+++ src/reviewsapp/models/reviews.py	2011-01-28 16:27:06 +0000
@@ -77,10 +77,16 @@
         app_label = 'reviewsapp'
 
     def update_stats(self):
-        stats = Review.objects.filter(softwareitem=self).aggregate(
-                                            average=Avg('rating'),
-                                            total=Count('rating'))
+        stats = Review.objects.filter(
+            softwareitem=self, hide=False).aggregate(
+                average=Avg('rating'), total=Count('rating'))
         self.ratings_total = stats['total']
+
+        # If there are no visible reviews, the average will be returned
+        # as None.
+        if stats['average'] is None:
+            stats['average'] = 0
+
         self.ratings_average = str(stats['average'])
         self.date_ratings_updated = datetime.utcnow()
         self.save()
@@ -181,6 +187,7 @@
         if self.review.hide != should_hide:
             self.review.hide = should_hide
             self.review.save()
+            self.review.softwareitem.update_stats()
 
     def can_be_moderated_by(self, user):
         """Return whether the user is allowed to moderate this object."""
@@ -303,6 +310,7 @@
         if self.reviewmoderation_set.approved().count() == 0:
             self.hide = True
             self.save()
+            self.softwareitem.update_stats()
 
         return flagged_object
 
 
=== modified file 'src/reviewsapp/tests/test_models.py'
--- src/reviewsapp/tests/test_models.py	2011-01-24 09:53:02 +0000
+++ src/reviewsapp/tests/test_models.py	2011-01-28 16:27:06 +0000
@@ -24,6 +24,7 @@
     'ReviewModelTestCase',
     'ReviewModerationTestCase',
     'ReviewFlagTestCase',
+    'SoftwareItemTestCase',
     ]
 
 from django.db import IntegrityError
@@ -147,6 +148,8 @@
         # Flagging an item which has not previously been approved
         # will hide that item.
         review = self.factory.makeReview()
+        stats_updated_orig = review.softwareitem.date_ratings_updated
+
         self.assertFalse(review.hide)
         flagger = self.factory.makeUser()
 
@@ -154,6 +157,9 @@
             flagger, 'Offensive language', "The word 'blue' is offensive.")
 
         self.assertTrue(review.hide)
+        # The stats were updated for the software item.
+        self.assertNotEquals(
+            stats_updated_orig, review.softwareitem.date_ratings_updated)
 
     def test_flag_does_not_hide_approved_items(self):
         # Flagging an item which has been previously approved does not
@@ -161,6 +167,7 @@
         review_moderation = self.factory.makeReviewModeration(
             status=ReviewModeration.APPROVED_STATUS, num_flags=0)
         review = review_moderation.review
+        stats_updated_orig = review.softwareitem.date_ratings_updated
         flagger = self.factory.makeUser()
         self.assertFalse(review.hide)
 
@@ -168,6 +175,9 @@
             flagger, 'Offensive language', "The word 'blue' is offensive.")
 
         self.assertFalse(review.hide)
+        # The stats were not updated for the software item.
+        self.assertEquals(
+            stats_updated_orig, review.softwareitem.date_ratings_updated)
 
     def test_multiple_flag_ordering(self):
         # When an object is flagged multiple times default ordering is
@@ -233,6 +243,7 @@
         # Moderating with approval unhides a review.
         review_moderation = self.factory.makeReviewModeration(num_flags=1)
         review = review_moderation.review
+        stats_updated_orig = review.softwareitem.date_ratings_updated
         self.assertTrue(review.hide)
 
         review_moderation.moderate(
@@ -241,11 +252,15 @@
 
         review = Review.objects.get(id=review.id)
         self.assertFalse(review.hide)
+        # The stats were updated for the software item.
+        self.assertNotEquals(
+            stats_updated_orig, review.softwareitem.date_ratings_updated)
 
     def test_moderate_rejection_hides_review(self):
         # Moderating with rejection unhides a review.
         review_moderation = self.factory.makeReviewModeration(num_flags=0)
         review = review_moderation.review
+        stats_updated_orig = review.softwareitem.date_ratings_updated
         self.assertFalse(review.hide)
 
         review_moderation.moderate(
@@ -253,6 +268,9 @@
             "whatever")
 
         review = Review.objects.get(id=review.id)
+        # The stats were updated for the software item.
+        self.assertNotEquals(
+            stats_updated_orig, review.softwareitem.date_ratings_updated)
         self.assertTrue(review.hide)
 
     def test_moderate_non_pending_moderation_errors(self):
@@ -302,3 +320,32 @@
             review_moderation.moderate, reviewer,
             ReviewModeration.APPROVED_STATUS, "Great.")
 
+
+class SoftwareItemTestCase(TestCaseWithFactory):
+
+    def test_update_stats(self):
+        # Updating the stats calculates the average and total.
+        software_item = self.factory.makeSoftwareItem()
+        self.factory.makeReview(
+            software_item=software_item, rating="5")
+        self.factory.makeReview(
+            software_item=software_item, rating="3")
+
+        software_item.update_stats()
+
+        self.assertEqual('4.0', software_item.ratings_average)
+        self.assertEqual(2, software_item.ratings_total)
+
+    def test_update_stats_ignores_hidden(self):
+        # Updating the stats ignores hidden reviews.
+        software_item = self.factory.makeSoftwareItem()
+        self.factory.makeReview(
+            software_item=software_item, rating="5")
+        self.factory.makeReview(
+            software_item=software_item, rating="3", hide=True)
+
+        software_item.update_stats()
+
+        self.assertEqual('5.0', software_item.ratings_average)
+        self.assertEqual(1, software_item.ratings_total)
+