← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~bac/launchpad/bug-5927 into lp:launchpad

 

Brad Crittenden has proposed merging lp:~bac/launchpad/bug-5927 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #5927 assignee cannot see private bug
  https://bugs.launchpad.net/bugs/5927

For more details, see:
https://code.launchpad.net/~bac/launchpad/bug-5927/+merge/45403

= Summary =

Bug assignees should be allowed to see private bugs.

== Proposed fix ==

Since assignees already get bugnotifications, all that is required is to
change userCanView to explicitly allow bugtask assignees to view the
bug.  When the assignee is changed there is no residual subscription,
which would have been a problem if the assignee were given visibility
through the subscription model as is done for the bug supervisor.

== Pre-implementation notes ==

Talks with Curtis.

== Implementation details ==

As above.

== Tests ==

bin/test -vvm lp.bugs -t test_bugvisibility -t bugnotification

== Demo and Q/A ==

Mark a bug with no assignee private.  Ensure user 'bac' cannot see the
bug.  Assign bac to the bug.  Ensure bac can now see the bug.  Unassign
bac.  Ensure bac can no longer see the bug.  Repeat.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/tests/test_bugvisibility.py
  lib/lp/bugs/model/bug.py
  lib/lp/registry/tests/test_product.py
-- 
https://code.launchpad.net/~bac/launchpad/bug-5927/+merge/45403
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-5927 into lp:launchpad.
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2010-12-24 11:02:19 +0000
+++ lib/lp/bugs/model/bug.py	2011-01-06 16:01:03 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0611,W0212
@@ -1784,6 +1784,8 @@
         Note that Editing is also controlled by this check,
         because we permit editing of any bug one can see.
         """
+        assert user is not None, "User may not be None"
+
         if user.id in self._known_viewers:
             return True
         if not self.private:
@@ -1793,11 +1795,16 @@
             # Admins can view all bugs.
             return True
         else:
-            # This is a private bug. Only explicit subscribers may view it.
+            # This is a private bug. Explicit subscribers may view it.
             for subscription in self.subscriptions:
                 if user.inTeam(subscription.person):
                     self._known_viewers.add(user.id)
                     return True
+            # Assignees to bugtasks can also see the private bug.
+            for bugtask in self.bugtasks:
+                if user.inTeam(bugtask.assignee):
+                    self._known_viewers.add(user.id)
+                    return True
         return False
 
     def linkHWSubmission(self, submission):

=== added file 'lib/lp/bugs/tests/test_bugvisibility.py'
--- lib/lp/bugs/tests/test_bugvisibility.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/tests/test_bugvisibility.py	2011-01-06 16:01:03 +0000
@@ -0,0 +1,89 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for visibility of a bug."""
+
+from canonical.testing.layers import LaunchpadFunctionalLayer
+from lp.testing import (
+    celebrity_logged_in,
+    person_logged_in,
+    TestCaseWithFactory,
+    )
+
+
+class TestPublicBugVisibility(TestCaseWithFactory):
+    """Test visibility for a public bug."""
+
+    layer = LaunchpadFunctionalLayer
+
+    def setUp(self):
+        super(TestPublicBugVisibility, self).setUp()
+        owner = self.factory.makePerson(name="bugowner")
+        self.bug = self.factory.makeBug(owner=owner)
+
+    def test_publicBugAnonUser(self):
+        # userCanView does not get called for anonymous users.
+        self.assertRaises(AssertionError, self.bug.userCanView, None)
+
+    def test_publicBugRegularUser(self):
+        # A regular (non-privileged) user can view a public bug.
+        user = self.factory.makePerson()
+        self.assertTrue(self.bug.userCanView(user))
+
+
+class TestPrivateBugVisibility(TestCaseWithFactory):
+    """Test visibility for a private bug."""
+
+    layer = LaunchpadFunctionalLayer
+
+    def setUp(self):
+        super(TestPrivateBugVisibility, self).setUp()
+        self.owner = self.factory.makePerson(name="bugowner")
+        self.product_owner = self.factory.makePerson(name="productowner")
+        self.product = self.factory.makeProduct(
+            name="regular-product", owner=self.product_owner)
+        self.bug_team = self.factory.makeTeam(
+            name="bugteam", owner=self.product.owner)
+        self.bug_team_member = self.factory.makePerson(name="bugteammember")
+        with person_logged_in(self.product.owner):
+            self.bug_team.addMember(self.bug_team_member, self.product.owner)
+            self.product.setBugSupervisor(
+                bug_supervisor=self.bug_team,
+                user=self.product.owner)
+        self.bug = self.factory.makeBug(
+            owner=self.owner, private=True, product=self.product)
+
+    def test_privateBugRegularUser(self):
+        # A regular (non-privileged) user can not view a private bug.
+        user = self.factory.makePerson()
+        self.assertFalse(self.bug.userCanView(user))
+
+    def test_privateBugOwner(self):
+        # A regular (non-privileged) user can not view a private bug.
+        self.assertTrue(self.bug.userCanView(self.owner))
+
+    def test_privateBugSupervisor(self):
+        # A member of the bug supervisor team can not see a private bug.
+        self.assertFalse(self.bug.userCanView(self.bug_team_member))
+
+    def test_privateBugSupervisorPrivateBugsByDefault(self):
+        # A member of the bug supervisor team can see a private bug if the
+        # product is set to have private bugs by default.
+        self.product = self.factory.makeProduct(
+            name="priv-bugs-product", owner=self.product_owner)
+        with person_logged_in(self.product.owner):
+            self.product.setBugSupervisor(
+                bug_supervisor=self.bug_team,
+                user=self.product.owner)
+        with celebrity_logged_in('admin'):
+            self.product.private_bugs = True
+        self.bug = self.factory.makeBug(
+            owner=self.owner, product=self.product)
+        self.assertTrue(self.bug.userCanView(self.bug_team_member))
+
+    def test_privateBugAssignee(self):
+        # The bug assignee can see the private bug.
+        bug_assignee = self.factory.makePerson(name="bugassignee")
+        with person_logged_in(self.product.owner):
+            self.bug.default_bugtask.transitionToAssignee(bug_assignee)
+        self.assertTrue(self.bug.userCanView(bug_assignee))

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2010-12-09 07:14:34 +0000
+++ lib/lp/registry/tests/test_product.py	2011-01-06 16:01:03 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -10,7 +10,6 @@
 from lazr.lifecycle.snapshot import Snapshot
 import pytz
 import transaction
-from zope.component import getUtility
 
 from canonical.launchpad.ftests import syncUpdate
 from canonical.launchpad.testing.pages import (
@@ -22,7 +21,6 @@
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
     )
-from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.product import (
     IProduct,
     License,
@@ -345,9 +343,8 @@
     def testPersonCanSetSelfAsSupervisor(self):
         # A person can set themselves as bug supervisor for a product.
         # This is a regression test for bug 438985.
-        user = getUtility(IPersonSet).getByName(self.person.name)
         self.product.setBugSupervisor(
-            bug_supervisor=self.person, user=user)
+            bug_supervisor=self.person, user=self.person)
 
         self.assertEqual(
             self.product.bug_supervisor, self.person,