launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/extend-subscription-check into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/extend-subscription-check/+merge/80173

Disallow a team to change their subscription policy to OPEN or DELEGATED if they are subscribed to, or assigned to any private bugs.
-- 
https://code.launchpad.net/~stevenk/launchpad/extend-subscription-check/+merge/80173
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/extend-subscription-check into lp:launchpad.

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2011-10-04 07:42:19 +0000
+++ lib/lp/registry/interfaces/person.py	2011-10-24 05:21:24 +0000
@@ -69,6 +69,7 @@
     Reference,
     )
 from lazr.restful.interface import copy_field
+from storm.expr import Join
 from zope.component import getUtility
 from zope.formlib.form import NoInputData
 from zope.interface import (
@@ -101,6 +102,7 @@
     IHasMugshot,
     IPrivacy,
     )
+from canonical.launchpad.interfaces.lpstorm import IStore
 from canonical.launchpad.interfaces.validation import validate_new_team_email
 from canonical.launchpad.webapp.authorization import check_permission
 from canonical.launchpad.webapp.interfaces import ILaunchpadApplication
@@ -522,7 +524,7 @@
 
 
 def team_subscription_policy_can_transition(team, policy):
-    """Can the team can change its subscription policy
+    """Can the team can change its subscription policy?
 
     Returns True when the policy can change. or raises an error. OPEN teams
     cannot be members of MODERATED or RESTRICTED teams. OPEN teams
@@ -544,12 +546,41 @@
                 raise TeamSubscriptionPolicyError(
                     "The team subscription policy cannot be %s because one "
                     "or more if its super teams are not open." % policy)
-        # The team can be open if it has PPAs.
+        # The team can not be open if it has PPAs.
         for ppa in team.ppas:
             if ppa.status != ArchiveStatus.DELETED:
                 raise TeamSubscriptionPolicyError(
                     "The team subscription policy cannot be %s because it "
                     "has one or more active PPAs." % policy)
+        # Circular imports.
+        from lp.bugs.model.bug import Bug
+        from lp.bugs.model.bugsubscription import BugSubscription
+        from lp.bugs.model.bugtask import BugTask
+        # The team can not be open if it is subscribed to private bugs.
+        subscribed_private_bugs = IStore(BugSubscription).using(
+            BugSubscription,
+            Join(Bug,
+                Bug.id == BugSubscription.bug_id)
+            ).find(
+                BugSubscription,
+                BugSubscription.person_id == team.id,
+                Bug.private == True)
+        if subscribed_private_bugs.count():
+            raise TeamSubscriptionPolicyError(
+                "The team subscription policy cannot be %s because it is "
+                "subscribed to one or more private bugs." % policy)
+        assigned_private_bugs = IStore(Bug).using(
+            Bug,
+            Join(BugTask,
+                BugTask.bugID == Bug.id)
+            ).find(
+                Bug,
+                Bug.private == True,
+                BugTask.assignee == team)
+        if assigned_private_bugs.count():
+            raise TeamSubscriptionPolicyError(
+                "The team subscription policy cannot be %s because it is "
+                "assigned to one or more private bugs." % policy)
     elif team.subscriptionpolicy in OPEN_TEAM_POLICY:
         # The team can become MODERATED or RESTRICTED if its member teams
         # are not OPEN.

=== modified file 'lib/lp/registry/tests/test_team.py'
--- lib/lp/registry/tests/test_team.py	2011-05-10 17:55:40 +0000
+++ lib/lp/registry/tests/test_team.py	2011-10-24 05:21:24 +0000
@@ -36,6 +36,7 @@
 from lp.testing import (
     login_celebrity,
     login_person,
+    person_logged_in,
     TestCaseWithFactory,
     )
 
@@ -394,6 +395,30 @@
         self.assertEqual(
             None, self.field.validate(TeamSubscriptionPolicy.OPEN))
 
+    def test_closed_team_with_private_bugs_cannot_become_open(self):
+        # The team cannot become open if it is subscribed to private bugs.
+        self.setUpTeams()
+        bug = self.factory.makeBug(owner=self.team.teamowner, private=True)
+        with person_logged_in(self.team.teamowner):
+            bug.subscribe(self.team, self.team.teamowner)
+        self.assertFalse(
+            self.field.constraint(TeamSubscriptionPolicy.OPEN))
+        self.assertRaises(
+            TeamSubscriptionPolicyError, self.field.validate,
+            TeamSubscriptionPolicy.OPEN)
+
+    def test_closed_team_with_private_bugs_assigned_cannot_become_open(self):
+        # The team cannot become open if it is assigned private bugs.
+        self.setUpTeams()
+        bug = self.factory.makeBug(owner=self.team.teamowner, private=True)
+        with person_logged_in(self.team.teamowner):
+            bug.default_bugtask.transitionToAssignee(self.team)
+        self.assertFalse(
+            self.field.constraint(TeamSubscriptionPolicy.OPEN))
+        self.assertRaises(
+            TeamSubscriptionPolicyError, self.field.validate,
+            TeamSubscriptionPolicy.OPEN)
+
 
 class TestTeamSubscriptionPolicyChoiceRestrcted(
                                    TestTeamSubscriptionPolicyChoiceModerated):