launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/comment-display-rules into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/comment-display-rules/+merge/58760


-- 
https://code.launchpad.net/~jcsackett/launchpad/comment-display-rules/+merge/58760
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/comment-display-rules into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-04-13 11:05:45 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-04-21 22:01:18 +0000
@@ -147,6 +147,7 @@
     any,
     NULL,
     )
+from canonical.launchpad.utilities.personroles import PersonRoles
 from canonical.launchpad.webapp import (
     canonical_url,
     enabled_with_permission,
@@ -371,7 +372,7 @@
     return comments
 
 
-def get_visible_comments(comments):
+def get_visible_comments(comments, user=None):
     """Return comments, filtering out empty or duplicated ones."""
     visible_comments = []
     previous_comment = None
@@ -396,6 +397,13 @@
     commenters = set(comment.owner for comment in visible_comments)
     getUtility(IPersonSet).getValidPersons(commenters)
 
+    # If a user is supplied, we can also strip out comments that the user
+    # cannot see, because they have been marked invisible.
+    if user is not None:
+        role = PersonRoles(user)
+        if not (role.in_admin or role.in_registry_experts):
+            visible_comments = [c for c in visible_comments if c.visible]
+
     return visible_comments
 
 
@@ -775,7 +783,9 @@
                 self.context, truncate=True, for_display=True,
                 slice_info=[
                     slice(None, oldest_count), slice(new_count, None)])
-        visible_comments = get_visible_comments(comments)
+
+        visible_comments = get_visible_comments(
+            comments, user=self.user)
 
         event_groups = group_comments_with_activity(
             comments=visible_comments,

=== added file 'lib/lp/bugs/browser/tests/test_bugcomment_visibility.py'
--- lib/lp/bugs/browser/tests/test_bugcomment_visibility.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/browser/tests/test_bugcomment_visibility.py	2011-04-21 22:01:18 +0000
@@ -0,0 +1,76 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the various rules around bug comment visibility."""
+
+__metaclass__ = type
+
+from zope.component import getUtility
+
+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.testing import (
+    BrowserTestCase,
+    person_logged_in,
+    )
+from lp.testing.views import create_view, create_initialized_view
+
+
+class TestBugCommentVisibility(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    def _makeBugWithHiddenComment(self, bugbody=None):
+        administrator = getUtility(ILaunchpadCelebrities).admin.teamowner
+        bug = self.factory.makeBug()
+        with person_logged_in(administrator):
+            comment = self.factory.makeBugComment(bug=bug, body=bugbody)
+            comment.visible = False
+        return bug
+
+    def _getUserForTest(self, team=None):
+        person = self.factory.makePerson()
+        if team is not None:
+            with person_logged_in(team.teamowner):
+                team.addMember(person, team.teamowner)
+        return person
+
+    def test_admin_can_see_comments(self):
+        comment_text = "You can't see me."
+        bug = self._makeBugWithHiddenComment(comment_text)
+        admin_team = getUtility(ILaunchpadCelebrities).admin
+        administrator = self._getUserForTest(admin_team)
+        view = self.getViewBrowser(
+            context=bug.default_bugtask, user=administrator)
+        self.assertTrue(
+           comment_text in view.contents,
+           "Administrator cannot see the hidden comment.")
+        
+    def test_registry_can_see_comments(self):
+        comment_text = "You can't see me."
+        bug = self._makeBugWithHiddenComment(comment_text)
+        registry_team = getUtility(ILaunchpadCelebrities).registry_experts
+        registry_expert = self._getUserForTest(registry_team)
+        view = self.getViewBrowser(
+            context=bug.default_bugtask, user=registry_expert)
+        self.assertTrue(
+           comment_text in view.contents,
+           "Registy member cannot see the hidden comment.")
+
+
+    def test_anon_cannot_see_comments(self):
+        comment_text = "You can't see me."
+        bug = self._makeBugWithHiddenComment(comment_text)
+        view = self.getViewBrowser(context=bug.default_bugtask)
+        self.assertFalse(
+           comment_text in view.contents,
+           "Anonymous person can see the hidden comment.")
+
+    def test_random_cannot_see_comments(self):
+        comment_text = "You can't see me."
+        bug = self._makeBugWithHiddenComment(comment_text)
+        user = self._getUserForTest()
+        view = self.getViewBrowser(context=bug.default_bugtask, user=user)
+        self.assertFalse(
+           comment_text in view.contents,
+           "Random user can see the hidden comment.")

=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt	2011-04-14 14:39:55 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt	2011-04-21 22:01:18 +0000
@@ -254,11 +254,7 @@
           <tal:is-comment
               define="comment comment_or_activity/comment|nothing"
               condition="comment">
-            <tal:comment-box condition="python: (
-                                       view.user_is_admin or
-                                       comment.visible)"
-                             replace="structure comment/@@+box">
-            </tal:comment-box>
+            <tal:comment-box replace="structure comment/@@+box" />
           </tal:is-comment>
 
           <tal:is-activity