Ratings and Reviews server

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

  1. 708841-caching-issue
  2. Merge into trunk
Proposed by Michael Nelson
Status: Merged
Merged at revision: 118
Proposed branch: lp:~michael.nelson/rnr-server/708841-caching-issue
Merge into: lp:rnr-server
Diff against target: 155 lines (+58/-3)
2 files modified
src/reviewsapp/models/reviews.py (+11/-3)
src/reviewsapp/tests/test_models.py (+47/-0)
To merge this branch: bzr merge lp:~michael.nelson/rnr-server/708841-caching-issue
Reviewer Review Type Date Requested Status
Danny Tamez (community) Approve
Review via email: mp+47816@code.launchpad.net

Description of the change

Overview
========

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

To post a comment you must log in.
Revision history for this message
Danny Tamez (zematynnad) wrote :

everything looks good

review: Approve
lp:~michael.nelson/rnr-server/708841-caching-issue updated
118. By Michael Nelson

Ensure hidden reviews are not included in stats.

119. By Michael Nelson

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

120. By Michael Nelson

Merged trunk.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/reviewsapp/models/reviews.py'
2--- src/reviewsapp/models/reviews.py 2011-01-28 15:39:42 +0000
3+++ src/reviewsapp/models/reviews.py 2011-01-28 16:27:06 +0000
4@@ -77,10 +77,16 @@
5 app_label = 'reviewsapp'
6
7 def update_stats(self):
8- stats = Review.objects.filter(softwareitem=self).aggregate(
9- average=Avg('rating'),
10- total=Count('rating'))
11+ stats = Review.objects.filter(
12+ softwareitem=self, hide=False).aggregate(
13+ average=Avg('rating'), total=Count('rating'))
14 self.ratings_total = stats['total']
15+
16+ # If there are no visible reviews, the average will be returned
17+ # as None.
18+ if stats['average'] is None:
19+ stats['average'] = 0
20+
21 self.ratings_average = str(stats['average'])
22 self.date_ratings_updated = datetime.utcnow()
23 self.save()
24@@ -181,6 +187,7 @@
25 if self.review.hide != should_hide:
26 self.review.hide = should_hide
27 self.review.save()
28+ self.review.softwareitem.update_stats()
29
30 def can_be_moderated_by(self, user):
31 """Return whether the user is allowed to moderate this object."""
32@@ -303,6 +310,7 @@
33 if self.reviewmoderation_set.approved().count() == 0:
34 self.hide = True
35 self.save()
36+ self.softwareitem.update_stats()
37
38 return flagged_object
39
40
41=== modified file 'src/reviewsapp/tests/test_models.py'
42--- src/reviewsapp/tests/test_models.py 2011-01-24 09:53:02 +0000
43+++ src/reviewsapp/tests/test_models.py 2011-01-28 16:27:06 +0000
44@@ -24,6 +24,7 @@
45 'ReviewModelTestCase',
46 'ReviewModerationTestCase',
47 'ReviewFlagTestCase',
48+ 'SoftwareItemTestCase',
49 ]
50
51 from django.db import IntegrityError
52@@ -147,6 +148,8 @@
53 # Flagging an item which has not previously been approved
54 # will hide that item.
55 review = self.factory.makeReview()
56+ stats_updated_orig = review.softwareitem.date_ratings_updated
57+
58 self.assertFalse(review.hide)
59 flagger = self.factory.makeUser()
60
61@@ -154,6 +157,9 @@
62 flagger, 'Offensive language', "The word 'blue' is offensive.")
63
64 self.assertTrue(review.hide)
65+ # The stats were updated for the software item.
66+ self.assertNotEquals(
67+ stats_updated_orig, review.softwareitem.date_ratings_updated)
68
69 def test_flag_does_not_hide_approved_items(self):
70 # Flagging an item which has been previously approved does not
71@@ -161,6 +167,7 @@
72 review_moderation = self.factory.makeReviewModeration(
73 status=ReviewModeration.APPROVED_STATUS, num_flags=0)
74 review = review_moderation.review
75+ stats_updated_orig = review.softwareitem.date_ratings_updated
76 flagger = self.factory.makeUser()
77 self.assertFalse(review.hide)
78
79@@ -168,6 +175,9 @@
80 flagger, 'Offensive language', "The word 'blue' is offensive.")
81
82 self.assertFalse(review.hide)
83+ # The stats were not updated for the software item.
84+ self.assertEquals(
85+ stats_updated_orig, review.softwareitem.date_ratings_updated)
86
87 def test_multiple_flag_ordering(self):
88 # When an object is flagged multiple times default ordering is
89@@ -233,6 +243,7 @@
90 # Moderating with approval unhides a review.
91 review_moderation = self.factory.makeReviewModeration(num_flags=1)
92 review = review_moderation.review
93+ stats_updated_orig = review.softwareitem.date_ratings_updated
94 self.assertTrue(review.hide)
95
96 review_moderation.moderate(
97@@ -241,11 +252,15 @@
98
99 review = Review.objects.get(id=review.id)
100 self.assertFalse(review.hide)
101+ # The stats were updated for the software item.
102+ self.assertNotEquals(
103+ stats_updated_orig, review.softwareitem.date_ratings_updated)
104
105 def test_moderate_rejection_hides_review(self):
106 # Moderating with rejection unhides a review.
107 review_moderation = self.factory.makeReviewModeration(num_flags=0)
108 review = review_moderation.review
109+ stats_updated_orig = review.softwareitem.date_ratings_updated
110 self.assertFalse(review.hide)
111
112 review_moderation.moderate(
113@@ -253,6 +268,9 @@
114 "whatever")
115
116 review = Review.objects.get(id=review.id)
117+ # The stats were updated for the software item.
118+ self.assertNotEquals(
119+ stats_updated_orig, review.softwareitem.date_ratings_updated)
120 self.assertTrue(review.hide)
121
122 def test_moderate_non_pending_moderation_errors(self):
123@@ -302,3 +320,32 @@
124 review_moderation.moderate, reviewer,
125 ReviewModeration.APPROVED_STATUS, "Great.")
126
127+
128+class SoftwareItemTestCase(TestCaseWithFactory):
129+
130+ def test_update_stats(self):
131+ # Updating the stats calculates the average and total.
132+ software_item = self.factory.makeSoftwareItem()
133+ self.factory.makeReview(
134+ software_item=software_item, rating="5")
135+ self.factory.makeReview(
136+ software_item=software_item, rating="3")
137+
138+ software_item.update_stats()
139+
140+ self.assertEqual('4.0', software_item.ratings_average)
141+ self.assertEqual(2, software_item.ratings_total)
142+
143+ def test_update_stats_ignores_hidden(self):
144+ # Updating the stats ignores hidden reviews.
145+ software_item = self.factory.makeSoftwareItem()
146+ self.factory.makeReview(
147+ software_item=software_item, rating="5")
148+ self.factory.makeReview(
149+ software_item=software_item, rating="3", hide=True)
150+
151+ software_item.update_stats()
152+
153+ self.assertEqual('5.0', software_item.ratings_average)
154+ self.assertEqual(1, software_item.ratings_total)
155+

Subscribers

People subscribed via source and target branches