launchpad-reviewers team mailing list archive

[Riccardo Padovani <riccardo@xxxxxxxxxxxxx>]

Riccardo Padovani has proposed merging lp:~rpadovani/launchpad/unhide-comments into lp:launchpad.

Commit message:
Show hidden comments to their owners

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rpadovani/launchpad/unhide-comments/+merge/270950

Summary

Fix bug #1391394, impossible to unhide my comment

Proposed fix

Now owner of comments could see the comment also if it's hidden


Tests

I'm a bit confused about how implementing the test.
I see there is a TestBugCommentVisibility class in bugs/browser/tests/test_bugcomment.py, and there is a method called makeHiddenMessage().

But, as far as I understand, the message created here is owned by admin.

So I think I need to create a method that creates a comment with a non-admin account, then I need to set visibility of the comment to false (comment.visible = False could work?) and then test if the comment is visible to the owner and then test if it's hide to a normal user?

Could work as approach?

-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rpadovani/launchpad/unhide-comments into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2015-08-11 00:54:41 +0000
+++ lib/lp/bugs/browser/bugtask.py	2015-09-14 13:20:38 +0000
@@ -295,7 +295,8 @@
         role = PersonRoles(user)
         strip_invisible = not (role.in_admin or role.in_registry_experts)
     if strip_invisible:
-        visible_comments = [c for c in visible_comments if c.visible]
+        visible_comments = [c for c in visible_comments if c.visible or
+                            c.owner == user]
 
     return visible_comments
 

=== modified file 'lib/lp/bugs/browser/tests/test_bugcomment.py'
--- lib/lp/bugs/browser/tests/test_bugcomment.py	2012-12-26 01:32:19 +0000
+++ lib/lp/bugs/browser/tests/test_bugcomment.py	2015-09-14 13:20:38 +0000
@@ -211,12 +211,12 @@
 
     layer = DatabaseFunctionalLayer
 
-    def makeHiddenMessage(self):
+    def makeHiddenMessage(self, comment_owner=None):
         """Required by the mixin."""
         with celebrity_logged_in('admin'):
             bug = self.factory.makeBug()
             comment = self.factory.makeBugComment(
-                    bug=bug, body=self.comment_text)
+                    bug=bug, body=self.comment_text, owner=comment_owner)
             comment.visible = False
         return bug
 

=== modified file 'lib/lp/coop/answersbugs/visibility.py'
--- lib/lp/coop/answersbugs/visibility.py	2012-12-11 04:08:19 +0000
+++ lib/lp/coop/answersbugs/visibility.py	2015-09-14 13:20:38 +0000
@@ -58,6 +58,12 @@
         view = self.getView(context=context)
         self.assertNotIn(self.html_comment_text, view.contents)
 
+    def test_comment_owner_could_see_hidden_comment(self):
+        owner = self.factory.makePerson()
+        context = self.makeHiddenMessage(comment_owner=owner)
+        view = self.getView(context=context, user=owner)
+        self.assertIn(self.html_comment_text, view.contents)
+
 
 class TestHideMessageControlMixin: