launchpad-reviewers team mailing list archive

[Martin Pool <mbp@xxxxxxxxxxxxx>]

Martin Pool has proposed merging lp:~mbp/launchpad/678090-affected-count into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #678090 in Launchpad itself: "Affected people from duplicates aren't included in the master bug's affected count"
  https://bugs.launchpad.net/launchpad/+bug/678090

For more details, see:
https://code.launchpad.net/~mbp/launchpad/678090-affected-count/+merge/81108

This changes the "affects N people" on a bug page to show the total people affected across all bugs.

It does this by actually counting them, which may or may not be a performance problem.  I have checked there is only one query, and actually by rearranging the code and using a cachedproperty it now does fewer queries than previously.

Robert suggested just adding up the cached affected-user-count across all duplicates, but that's incorrect in a way that's likely to generate knock-on bugs, and it wasn't obvious how to implement it.
-- 
https://code.launchpad.net/~mbp/launchpad/678090-affected-count/+merge/81108
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~mbp/launchpad/678090-affected-count into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-11-01 16:02:45 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-11-03 04:23:30 +0000
@@ -3531,36 +3531,38 @@
         else:
             return self.context.users_affected_count
 
-    @property
+    @cachedproperty
     def affected_statement(self):
         """The default "this bug affects" statement to show.
 
         The outputs of this method should be mirrored in
         MeTooChoiceSource._getSourceNames() (Javascript).
         """
-        if self.other_users_affected_count == 1:
-            if self.current_user_affected_status is None:
+        total_affected = self.context.users_affected_count_with_dupes
+        me_affected = self.current_user_affected_status
+        if me_affected is None:
+            if total_affected == 1:
                 return "This bug affects 1 person. Does this bug affect you?"
-            elif self.current_user_affected_status:
-                return "This bug affects you and 1 other person"
-            else:
-                return "This bug affects 1 person, but not you"
-        elif self.other_users_affected_count > 1:
-            if self.current_user_affected_status is None:
+            elif total_affected > 1:
                 return (
                     "This bug affects %d people. Does this bug "
-                    "affect you?" % (self.other_users_affected_count))
-            elif self.current_user_affected_status:
+                    "affect you?" % (total_affected))
+            else:
+                return "Does this bug affect you?"
+        elif me_affected is True:
+            if total_affected == 1:
+                return "This bug affects you and 1 other person"
+            elif total_affected > 1:
                 return "This bug affects you and %d other people" % (
-                    self.other_users_affected_count)
+                    total_affected - 1)
             else:
+                return "This bug affects you"
+        else:
+            if total_affected == 1:
+                return "This bug affects 1 person, but not you"
+            elif total_affected > 1:
                 return "This bug affects %d people, but not you" % (
-                    self.other_users_affected_count)
-        else:
-            if self.current_user_affected_status is None:
-                return "Does this bug affect you?"
-            elif self.current_user_affected_status:
-                return "This bug affects you"
+                    total_affected)
             else:
                 return "This bug doesn't affect you"