Merge lp:~wallyworld/launchpad/private-branch-unsubscribe-283167 into lp:launchpad

[Ian Booth]

[r=gmb][bug=283167] Branch owners can edit any subscriptions to the branch.

== Implementation ==

The bug report says that owners of private branches should be able to edit subscriptions to the branch. This mp does that, but for public branches too.

If the branch owner is a team, then the team owner can edit the subscriptions, but an arbitrary member of the owning team cannot.

== Tests ==

Added new tests to TestBranchSubscriptionCanBeUnsubscribedbyUser:
  test_branch_person_owner_can_unsubscribe
  test_branch_team_owner_can_unsubscribe

== Lint ==
Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/security.py
  lib/lp/code/model/tests/test_branchsubscription.py
  lib/lp/testing/factory.py

[Graham Binns (gmb)]

Hi Ian,

Only one very minor comment, otherwise r=me.

[1]

36 +

Minor nit, I know, but this blank line shouldn't be here.

review: Approve (code)

[Robert Collins (lifeless)]

The team ownership thing is inconsistent with every other use of teams; why do it that way?

=== modified file 'lib/lp/code/model/tests/test_branchsubscription.py'
--- lib/lp/code/model/tests/test_branchsubscription.py	2010-10-26 15:47:24 +0000
+++ lib/lp/code/model/tests/test_branchsubscription.py	2011-07-13 01:01:41 +0000
@@ -5,8 +5,6 @@
 
 __metaclass__ = type
 
-import unittest
-
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.app.errors import UserCannotUnsubscribePerson
 from lp.code.enums import (
@@ -119,6 +117,30 @@
             person=team, subscribed_by=subscribed_by)
         self.assertTrue(subscription.canBeUnsubscribedByUser(subscribed_by))
 
-
-def test_suite():
-    return unittest.TestLoader().loadTestsFromName(__name__)
+    def test_branch_person_owner_can_unsubscribe(self):
+        """Branch owner can unsubscribe someone from a branch."""
+        branch_owner = self.factory.makePerson()
+        branch = self.factory.makeBranch(owner=branch_owner)
+        subscribed_by = self.factory.makePerson()
+        subscriber = self.factory.makePerson()
+        subscription = self.factory.makeBranchSubscription(
+            branch=branch, person=subscriber, subscribed_by=subscribed_by)
+        self.assertTrue(subscription.canBeUnsubscribedByUser(branch_owner))
+
+    def test_branch_team_owner_can_unsubscribe(self):
+        """Branch team owner can unsubscribe someone from a branch.
+
+        If the owner of a branch is a team, then the team members can
+        unsubscribe someone.
+        """
+        team_owner = self.factory.makePerson()
+        team_member = self.factory.makePerson()
+        branch_owner = self.factory.makeTeam(
+            owner=team_owner, members=[team_member])
+        branch = self.factory.makeBranch(owner=branch_owner)
+        subscribed_by = self.factory.makePerson()
+        subscriber = self.factory.makePerson()
+        subscription = self.factory.makeBranchSubscription(
+            branch=branch, person=subscriber, subscribed_by=subscribed_by)
+        self.assertTrue(subscription.canBeUnsubscribedByUser(team_owner))
+        self.assertTrue(subscription.canBeUnsubscribedByUser(team_member))

=== modified file 'lib/lp/code/security.py'
--- lib/lp/code/security.py	2011-04-27 16:14:32 +0000
+++ lib/lp/code/security.py	2011-07-13 01:01:41 +0000
@@ -22,8 +22,12 @@
 
         Any team member can edit a branch subscription for their team.
         Launchpad Admins can also edit any branch subscription.
+        The owner of the subscribed branch can edit the subscription. If the
+        branch owner is a team, then members of the team can edit the
+        subscription.
         """
-        return (user.inTeam(self.obj.person) or
+        return (user.inTeam(self.obj.branch.owner) or
+                user.inTeam(self.obj.person) or
                 user.inTeam(self.obj.subscribed_by) or
                 user.in_admin or
                 user.in_bazaar_experts)
@@ -31,5 +35,3 @@
 
 class BranchSubscriptionView(BranchSubscriptionEdit):
     permission = 'launchpad.View'
-
-

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2011-07-07 15:47:36 +0000
+++ lib/lp/testing/factory.py	2011-07-13 01:01:41 +0000
@@ -1486,11 +1486,7 @@
 
     def makeBranchSubscription(self, branch=None, person=None,
                                subscribed_by=None):
-        """Create a BranchSubscription.
-
-        :param branch_title: The title to use for the created Branch
-        :param person_displayname: The displayname for the created Person
-        """
+        """Create a BranchSubscription."""
         if branch is None:
             branch = self.makeBranch()
         if person is None: