launchpad-reviewers team mailing list archive

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

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/bug-comment-visibility/+merge/51637

Summary
=======

There's a method in bugs, setCommentVisibility, that is used by Admin's to hide comment spam on bugs. Because we'd like to bother admins as little as possible when on Maintenance Rotation, and spam is one of the more common issues, it would be nice if setCommentVisibility were useable by people in registry experts.

Preimplementation
=================

Spoke with Curtis Hovey

Proposed Fix
============

Update the permissions so anyone in registry experts (which includes everyone on Maintenance rotation without including everyone in the LP community) can use setCommentVisibility. Down the road, we can add an lp-dev utility or something to make using this attribute super easy for people on maintenance rotation.

Implementation
==============

lib/lp/bugs/security.py
-----------------------
Update permissions to allow registry experts to have launchpad.Admin rights on bug comment visibility.

lib/lp/bugs/tests/test_bug_messages_webservice.py
-------------------------------------------------
Tests.

Tests
=====

bin/test -t test_bug_messages_webservice

Demo & QA
=========

If you're in ~registry or ~admins, you should be able to hide bug comments (e.g. spam) over the API using setCommentVisibility. If you're logged out or not in ~registry, you should get a 401.

Lint
====

make lint output:

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/security.py
  lib/lp/bugs/tests/test_bug_messages_webservice.py

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

=== modified file 'lib/lp/bugs/security.py'
--- lib/lp/bugs/security.py	2011-02-01 18:52:06 +0000
+++ lib/lp/bugs/security.py	2011-02-28 21:40:34 +0000
@@ -15,6 +15,7 @@
 from lp.bugs.interfaces.bug import IBug
 from lp.bugs.interfaces.bugattachment import IBugAttachment
 from lp.bugs.interfaces.bugbranch import IBugBranch
+from lp.bugs.interfaces.bugmessage import IBugMessage
 from lp.bugs.interfaces.bugnomination import IBugNomination
 from lp.bugs.interfaces.bugsubscription import IBugSubscription
 from lp.bugs.interfaces.bugsubscriptionfilter import IBugSubscriptionFilter
@@ -22,6 +23,7 @@
 from lp.bugs.interfaces.bugwatch import IBugWatch
 from lp.bugs.interfaces.structuralsubscription import IStructuralSubscription
 
+
 class EditBugNominationStatus(AuthorizationBase):
     permission = 'launchpad.Driver'
     usedfor = IBugNomination
@@ -159,6 +161,20 @@
     usedfor = IMessage
 
 
+class SetMessageVisibility(AuthorizationBase):
+    permission = 'launchpad.Admin'
+    usedfor = IBugMessage
+
+    def checkAuthenticated(self, user):
+        """Admins and registry admins can set bug comment visibility."""
+        return (user.in_admin or user.in_registry_experts)
+
+
+class SetBugCommentVisibility(SetMessageVisibility):
+
+    usedfor = IBug
+
+
 class ViewBugTracker(AnonymousAuthorization):
     """Anyone can view a bug tracker."""
     usedfor = IBugTracker

=== added file 'lib/lp/bugs/tests/test_bug_messages_webservice.py'
--- lib/lp/bugs/tests/test_bug_messages_webservice.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/tests/test_bug_messages_webservice.py	2011-02-28 21:40:34 +0000
@@ -0,0 +1,106 @@
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Webservice unit tests related to Launchpad Bug messages."""
+
+__metaclass__ = type
+
+import transaction
+
+from lazr.restfulclient.errors import HTTPError
+from zope.component import getUtility
+from zope.security.management import endInteraction
+
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.bugs.interfaces.bugmessage import IBugMessageSet
+from lp.registry.interfaces.person import IPersonSet
+from lp.testing import (
+    launchpadlib_for,
+    person_logged_in,
+    TestCaseWithFactory,
+    )
+
+
+class TestSetCommentVisibility(TestCaseWithFactory):
+    """Tests who can successfully set comment visibility."""
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestSetCommentVisibility, self).setUp()
+        self.person_set = getUtility(IPersonSet)
+        admins = self.person_set.getByName('admins')
+        self.admin = admins.teamowner
+        with person_logged_in(self.admin):
+            self.bug = self.factory.makeBug()
+            self.message = self.factory.makeBugComment(
+                bug=self.bug,
+                subject='foo',
+                body='bar')
+        transaction.commit()
+
+    def _get_bug_for_user(self, user=None):
+        """Convenience function to get the api bug reference."""
+        endInteraction()
+        if user is not None:
+            lp = launchpadlib_for("test", user)
+        else:
+            lp = launchpadlib_for("test")
+
+        bug_entry = lp.load(
+            'http://api.launchpad.dev/1.0/bugs/%s/' % self.bug.id)
+        return bug_entry
+
+    def _set_visibility(self, bug):
+        """Method to set visibility; needed for assertRaises."""
+        bug.setCommentVisibility(
+            comment_number=1,
+            visible=False)
+
+    def assertCommentHidden(self):
+        bug_msg_set = getUtility(IBugMessageSet)
+        with person_logged_in(self.admin):
+            bug_message = bug_msg_set.getByBugAndMessage(
+                self.bug, self.message)
+            self.assertFalse(bug_message.visible)
+
+    def test_random_user_cannot_set_visible(self):
+        # Logged in users without privs can't set bug comment
+        # visibility.
+        nopriv = self.person_set.getByName('no-priv')
+        bug = self._get_bug_for_user(nopriv)
+        self.assertRaises(
+            HTTPError,
+            self._set_visibility,
+            bug)
+
+    def test_anon_cannot_set_visible(self):
+        # Anonymous users can't set bug comment
+        # visibility.
+        bug = self._get_bug_for_user()
+        self.assertRaises(
+            HTTPError,
+            self._set_visibility,
+            bug)
+
+    def test_registry_admin_can_set_visible(self):
+        # Members of registry experts can set bug comment
+        # visibility.
+        registry = self.person_set.getByName('registry')
+        person = self.factory.makePerson()
+        with person_logged_in(registry.teamowner):
+            registry.addMember(person, registry.teamowner)
+        bug = self._get_bug_for_user(person)
+        self._set_visibility(bug)
+        self.assertCommentHidden()
+
+    def test_admin_can_set_visible(self):
+        # Admins can set bug comment
+        # visibility.
+        admins = self.person_set.getByName('admins')
+        person = self.factory.makePerson()
+        with person_logged_in(admins.teamowner):
+            admins.addMember(person, admins.teamowner)
+        bug = self._get_bug_for_user(person)
+        self._set_visibility(bug)
+        self.assertCommentHidden()