Merge lp:~thumper/launchpad/branch-subscription-subscribed-by into lp:launchpad/db-devel

[Tim Penhey]

Allow people to subscribe teams they are not a member of, and record them as the user who subscribed the team.

Add a subscribed_by to the BranchSubscription table.

This work also changes the permissions and abilities for subscribing and unsubscribing from branches.

Work in progress for now.

[Curtis Hovey (sinzui)]

> === modified file 'lib/lp/code/browser/branch.py'
> --- lib/lp/code/browser/branch.py 2010-05-20 04:01:34 +0000
> +++ lib/lp/code/browser/branch.py 2010-05-27 04:47:25 +0000
> @@ -354,6 +354,7 @@
>
> def initialize(self):
> self.notices = []
> + # TODO: FIXME - this is almost certainly not what we want.
> self._add_subscription_notice()

What do you want to do here?

> === modified file 'lib/lp/code/browser/branchsubscription.py'
> --- lib/lp/code/browser/branchsubscription.py 2010-04-20 01:21:10 +0000
> +++ lib/lp/code/browser/branchsubscription.py 2010-05-27 04:47:25 +0000
> ...
>
> @@ -209,7 +210,7 @@
> subscription = self.context.getSubscription(person)
> if subscription is None:
> self.context.subscribe(
> - person, notification_level, max_diff_lines, review_level)
> + person, notification_level, max_diff_lines, review_level, self.user)

Wrap the code at 78 characters.

> === modified file 'lib/lp/code/model/branchsubscription.py'
> --- lib/lp/code/model/branchsubscription.py 2009-06-25 04:06:00 +0000
> +++ lib/lp/code/model/branchsubscription.py 2010-05-27 04:47:25 +0000
> @@ -41,6 +41,9 @@
> notNull=False, default=DEFAULT)
> review_level = EnumCol(enum=CodeReviewNotificationLevel,
> notNull=True, default=DEFAULT)
> + subscribed_by = ForeignKey(
> + dbName='subscribed_by', foreignKey='Person',
> + storm_validator=validate_person_not_private_membership, notNull=True)

I was suggested in a recent review to drop the dbName since it is identical
to the column name. I did so, but now I ponder explicit is better than
implicit again.

> === modified file 'lib/lp/registry/browser/tests/packaging-views.txt'
> --- lib/lp/registry/browser/tests/packaging-views.txt 2010-04-16 18:00:31 +0000
> +++ lib/lp/registry/browser/tests/packaging-views.txt 2010-05-27 04:47:25 +0000
> @@ -350,5 +350,5 @@
> cnews
> libstdc++
> linux-source-2.6.15
> + hot
> thunderbird
> - hot

What caused this? This look like me from last week?

> === modified file 'lib/lp/registry/model/distroseries.py'
> --- lib/lp/registry/model/distroseries.py 2010-05-12 23:23:19 +0000
> +++ lib/lp/registry/model/distroseries.py 2010-05-27 04:47:25 +0000
> @@ -332,7 +332,7 @@
> origin = SQL(joins)
> condition = SQL(conditions + "AND packaging.id IS NULL")
> results = IStore(self).using(origin).find(find_spec, condition)
> - results = results.order_by('score DESC')
> + results = results.order_by('score DESC', SourcePackageName.name)
> return [{
> 'package': SourcePackage(
> sourcepackagename=spn, distroseries=self),

This is me from last week.

> === modified file 'lib/lp/testing/factory.py'
> --- lib/lp/testing/factory.py 2010-05-25 13:13:02 +0000
> +++ lib/lp/testing/factory.py 2010-05-27 04:47:25 +0000
> @@ -986,7 +986,7 @@
>
> return proposal
>
> - def makeBranchSubscription(self, branch=None, person=None):
> + def makeBranchSubscription(self, branch=None, person=None, subscri...

review: Needs Information (release-critical)

[Tim Penhey (thumper)]

On Fri, 28 May 2010 13:58:22 you wrote:
> Review: Needs Information release-critical

Thanks Curtis,

> > === modified file 'lib/lp/code/browser/branch.py'
> > --- lib/lp/code/browser/branch.py 2010-05-20 04:01:34 +0000
> > +++ lib/lp/code/browser/branch.py 2010-05-27 04:47:25 +0000
> > @@ -354,6 +354,7 @@
> >
> > def initialize(self):
> > self.notices = []
> >
> > + # TODO: FIXME - this is almost certainly not what we want.
> >
> > self._add_subscription_notice()
>
> What do you want to do here?

Haha, that was my own personal reminder in that I *think* that the following
function call isn't needed, nor called, nor would do what is really wanted,
and it was a reminder to check.

> > === modified file 'lib/lp/code/model/branchsubscription.py'
> > --- lib/lp/code/model/branchsubscription.py 2009-06-25 04:06:00 +0000
> > +++ lib/lp/code/model/branchsubscription.py 2010-05-27 04:47:25 +0000
> > @@ -41,6 +41,9 @@
> >
> > notNull=False, default=DEFAULT)
> >
> > review_level = EnumCol(enum=CodeReviewNotificationLevel,
> >
> > notNull=True, default=DEFAULT)
> >
> > + subscribed_by = ForeignKey(
> > + dbName='subscribed_by', foreignKey='Person',
> > + storm_validator=validate_person_not_private_membership,
> > notNull=True)
>
> I was suggested in a recent review to drop the dbName since it is identical
> to the column name. I did so, but now I ponder explicit is better than
> implicit again.

I'm relatively unconcerned about this point. I think at one stage the dbName
was required for ForeignKey but now may not be needed. I'm happy to remove
the dbName param if you think it best.

> > === modified file 'lib/lp/registry/browser/tests/packaging-views.txt'
> > --- lib/lp/registry/browser/tests/packaging-views.txt 2010-04-16 18:00:31
> > +0000 +++ lib/lp/registry/browser/tests/packaging-views.txt 2010-05-27
> > 04:47:25 +0000 @@ -350,5 +350,5 @@
> >
> > cnews
> > libstdc++
> > linux-source-2.6.15
> >
> > + hot
> >
> > thunderbird
> >
> > - hot
>
> What caused this? This look like me from last week?

Caused by someone.

> > === modified file 'lib/lp/registry/model/distroseries.py'
> > --- lib/lp/registry/model/distroseries.py 2010-05-12 23:23:19 +0000
> > +++ lib/lp/registry/model/distroseries.py 2010-05-27 04:47:25 +0000
> > @@ -332,7 +332,7 @@
> >
> > origin = SQL(joins)
> > condition = SQL(conditions + "AND packaging.id IS NULL")
> > results = IStore(self).using(origin).find(find_spec, condition)
> >
> > - results = results.order_by('score DESC')
> > + results = results.order_by('score DESC', SourcePackageName.name)
> >
> > return [{
> >
> > 'package': SourcePackage(
> >
> > sourcepackagename=spn, distroseries=self),
>
> This is me from last week.

Yes this was the source of it. It was failing locally for me consistently.

I'm back on to this branch now to add the extra tests and model validation.

[Tim Penhey (thumper)]

On Fri, 28 May 2010 13:58:22 you wrote:
> Review: Needs Information release-critical
> > === modified file 'lib/lp/code/browser/branchsubscription.py'
> Wrap the code at 78 characters.

Done.

> > === modified file 'lib/lp/testing/factory.py'
> Wrap the code at 78 characters.

Done.

Also added the checking.

[Tim Penhey (thumper)]

On Fri, 28 May 2010 13:58:22 you wrote:
> Review: Needs Information release-critical
>
> > === modified file 'lib/lp/code/browser/branch.py'
> > --- lib/lp/code/browser/branch.py 2010-05-20 04:01:34 +0000
> > +++ lib/lp/code/browser/branch.py 2010-05-27 04:47:25 +0000
> > @@ -354,6 +354,7 @@
> >
> > def initialize(self):
> > self.notices = []
> >
> > + # TODO: FIXME - this is almost certainly not what we want.
> >
> > self._add_subscription_notice()
>
> What do you want to do here?

Looks like it isn't used:

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py 2010-05-25 03:13:25 +0000
+++ lib/lp/code/browser/branch.py 2010-05-28 09:43:31 +0000
@@ -354,19 +354,6 @@

     def initialize(self):
         self.notices = []
- # TODO: FIXME - this is almost certainly not what we want.
- self._add_subscription_notice()
-
- def _add_subscription_notice(self):
- """Add the appropriate notice after posting the subscription form."""
- if self.user and self.request.method == 'POST':
- newsub = self.request.form.get('subscribe', None)
- if newsub == 'Subscribe':
- self.context.subscribe(self.user, self.user)
- self.notices.append("You have subscribed to this branch.")
- elif newsub == 'Unsubscribe':
- self.context.unsubscribe(self.user, self.user)
- self.notices.append("You have unsubscribed from this
branch.")

     def user_is_subscribed(self):
         """Is the current user subscribed to this branch?"""

[Curtis Hovey (sinzui)]

Thanks for addressing this issue Tim. This branch is good to land.

review: Approve (release-critical code)

[Stuart Bishop (stub)]

Looks good. patch-2207-60-0.sql

review: Approve (db)

=== modified file 'database/sampledata/current-dev.sql'
--- database/sampledata/current-dev.sql	2010-05-27 22:18:16 +0000
+++ database/sampledata/current-dev.sql	2010-05-29 09:01:05 +0000
@@ -3131,10 +3131,10 @@
 
 ALTER TABLE branchsubscription DISABLE TRIGGER ALL;
 
-INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level) VALUES (1, 12, 20, '2006-10-16 18:31:43.079375', 1, NULL, 0);
-INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level) VALUES (2, 12, 24, '2006-10-16 18:31:43.080236', 1, NULL, 0);
-INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level) VALUES (4, 64, 29, '2007-05-28 02:41:07.938677', 1, NULL, 0);
-INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level) VALUES (5, 64, 30, '2007-05-28 02:41:07.938677', 1, NULL, 0);
+INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level, subscribed_by) VALUES (1, 12, 20, '2006-10-16 18:31:43.079375', 1, NULL, 0, 12);
+INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level, subscribed_by) VALUES (2, 12, 24, '2006-10-16 18:31:43.080236', 1, NULL, 0, 12);
+INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level, subscribed_by) VALUES (4, 64, 29, '2007-05-28 02:41:07.938677', 1, NULL, 0, 64);
+INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level, subscribed_by) VALUES (5, 64, 30, '2007-05-28 02:41:07.938677', 1, NULL, 0, 64);
 
 
 ALTER TABLE branchsubscription ENABLE TRIGGER ALL;
 
=== modified file 'database/sampledata/current.sql'
--- database/sampledata/current.sql	2010-05-27 22:18:16 +0000
+++ database/sampledata/current.sql	2010-05-29 09:01:05 +0000
@@ -3089,10 +3089,10 @@
 
 ALTER TABLE branchsubscription DISABLE TRIGGER ALL;
 
-INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level) VALUES (1, 12, 20, '2006-10-16 18:31:43.079375', 1, NULL, 0);
-INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level) VALUES (2, 12, 24, '2006-10-16 18:31:43.080236', 1, NULL, 0);
-INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level) VALUES (4, 64, 29, '2007-05-28 02:41:07.938677', 1, NULL, 0);
-INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level) VALUES (5, 64, 30, '2007-05-28 02:41:07.938677', 1, NULL, 0);
+INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level, subscribed_by) VALUES (1, 12, 20, '2006-10-16 18:31:43.079375', 1, NULL, 0, 12);
+INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level, subscribed_by) VALUES (2, 12, 24, '2006-10-16 18:31:43.080236', 1, NULL, 0, 12);
+INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level, subscribed_by) VALUES (4, 64, 29, '2007-05-28 02:41:07.938677', 1, NULL, 0, 64);
+INSERT INTO branchsubscription (id, person, branch, date_created, notification_level, max_diff_lines, review_level, subscribed_by) VALUES (5, 64, 30, '2007-05-28 02:41:07.938677', 1, NULL, 0, 64);
 
 
 ALTER TABLE branchsubscription ENABLE TRIGGER ALL;
 
=== added file 'database/schema/patch-2207-60-0.sql'
--- database/schema/patch-2207-60-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2207-60-0.sql	2010-05-29 09:01:05 +0000
@@ -0,0 +1,18 @@
+-- Copyright 2010 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+ALTER TABLE BranchSubscription
+   ADD COLUMN subscribed_by integer REFERENCES Person;
+
+UPDATE BranchSubscription
+SET subscribed_by = person;
+
+ALTER TABLE BranchSubscription ALTER COLUMN subscribed_by SET NOT NULL;
+
+-- Index needed for person merging.
+CREATE INDEX branchsubscription__subscribed_by__idx
+   ON BranchSubscription(subscribed_by);
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 60, 0);
 
=== modified file 'lib/canonical/launchpad/mail/commands.py'
--- lib/canonical/launchpad/mail/commands.py	2010-04-16 15:06:55 +0000
+++ lib/canonical/launchpad/mail/commands.py	2010-05-29 09:01:05 +0000
@@ -23,7 +23,7 @@
         IDistributionSourcePackage, EmailProcessingError,
         NotFoundError, CreateBugParams, IPillarNameSet,
         BugTargetNotFound, IProjectGroup, ISourcePackage, IProductSeries,
-        BugTaskStatus, UserCannotUnsubscribePerson)
+        BugTaskStatus)
 from lazr.lifecycle.event import (
     ObjectModifiedEvent, ObjectCreatedEvent)
 from lazr.lifecycle.interfaces import (
@@ -34,6 +34,8 @@
 from canonical.launchpad.validators.name import valid_name
 from canonical.launchpad.webapp.authorization import check_permission
 
+from lp.app.errors import UserCannotUnsubscribePerson
+
 
 def normalize_arguments(string_args):
     """Normalizes the string arguments.
 
=== modified file 'lib/canonical/launchpad/scripts/tests/test_garbo.py'
--- lib/canonical/launchpad/scripts/tests/test_garbo.py	2010-04-23 08:33:14 +0000
+++ lib/canonical/launchpad/scripts/tests/test_garbo.py	2010-05-29 09:01:05 +0000
@@ -19,7 +19,6 @@
 
 from canonical.config import config
 from canonical.database.constants import THIRTY_DAYS_AGO, UTC_NOW
-from canonical.launchpad.database.emailaddress import EmailAddress
 from canonical.launchpad.database.message import Message
 from canonical.launchpad.database.oauth import OAuthNonce
 from canonical.launchpad.database.openidconsumer import OpenIDConsumerNonce
@@ -42,7 +41,6 @@
 from lp.code.model.branchjob import BranchJob, BranchUpgradeJob
 from lp.code.model.codeimportresult import CodeImportResult
 from lp.registry.interfaces.person import IPersonSet, PersonCreationRationale
-from lp.registry.model.person import Person
 from lp.services.job.model.job import Job
 
 
 
=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2010-05-26 08:54:27 +0000
+++ lib/canonical/launchpad/security.py	2010-05-29 09:01:05 +0000
@@ -24,8 +24,6 @@
     IBranch, user_has_special_branch_access)
 from lp.code.interfaces.branchmergeproposal import (
     IBranchMergeProposal)
-from lp.code.interfaces.branchsubscription import (
-    IBranchSubscription)
 from lp.code.interfaces.sourcepackagerecipe import ISourcePackageRecipe
 from lp.code.interfaces.sourcepackagerecipebuild import (
     ISourcePackageRecipeBuild)
@@ -63,8 +61,7 @@
 from lp.services.worlddata.interfaces.language import ILanguage, ILanguageSet
 from lp.translations.interfaces.languagepack import ILanguagePack
 from canonical.launchpad.interfaces.launchpad import (
-    IBazaarApplication, IHasBug, IHasDrivers, ILaunchpadCelebrities,
-    IPersonRoles)
+    IHasBug, IHasDrivers, ILaunchpadCelebrities, IPersonRoles)
 from lp.registry.interfaces.role import IHasOwner
 from lp.registry.interfaces.location import IPersonLocation
 from lp.registry.interfaces.mailinglist import IMailingListSet
@@ -1749,25 +1746,6 @@
             self.obj.distribution).checkAuthenticated(user))
 
 
-class BranchSubscriptionEdit(AuthorizationBase):
-    permission = 'launchpad.Edit'
-    usedfor = IBranchSubscription
-
-    def checkAuthenticated(self, user):
-        """Is the user able to edit a branch subscription?
-
-        Any team member can edit a branch subscription for their team.
-        Launchpad Admins can also edit any branch subscription.
-        """
-        return (user.inTeam(self.obj.person) or
-                user.in_admin or
-                user.in_bazaar_experts)
-
-
-class BranchSubscriptionView(BranchSubscriptionEdit):
-    permission = 'launchpad.View'
-
-
 class BranchMergeProposalView(AuthorizationBase):
     permission = 'launchpad.View'
     usedfor = IBranchMergeProposal
 
=== added file 'lib/lp/app/errors.py'
--- lib/lp/app/errors.py	1970-01-01 00:00:00 +0000
+++ lib/lp/app/errors.py	2010-05-29 09:01:05 +0000
@@ -0,0 +1,18 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Cross application type errors for launchpad."""
+
+__metaclass__ = type
+__all__ = [
+    'UserCannotUnsubscribePerson',
+    ]
+
+from zope.security.interfaces import Unauthorized
+
+from lazr.restful.declarations import webservice_error
+
+
+class UserCannotUnsubscribePerson(Unauthorized):
+    """User does not have persmisson to unsubscribe person or team."""
+    webservice_error(401)
 
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py	2010-04-29 17:49:19 +0000
+++ lib/lp/bugs/interfaces/bug.py	2010-05-29 09:01:05 +0000
@@ -20,7 +20,6 @@
     'IProjectGroupBugAddForm',
     'InvalidBugTargetType',
     'InvalidDuplicateValue',
-    'UserCannotUnsubscribePerson',
     ]
 
 from zope.component import getUtility
@@ -28,7 +27,6 @@
 from zope.schema import (
     Bool, Bytes, Choice, Datetime, Int, List, Object, Text, TextLine)
 from zope.schema.vocabulary import SimpleVocabulary
-from zope.security.interfaces import Unauthorized
 
 from canonical.launchpad import _
 from canonical.launchpad.fields import (
@@ -805,11 +803,6 @@
     webservice_error(417)
 
 
-class UserCannotUnsubscribePerson(Unauthorized):
-    """User does not have persmisson to unsubscribe person or team."""
-    webservice_error(401)
-
-
 # We are forced to define these now to avoid circular import problems.
 IBugAttachment['bug'].schema = IBug
 IBugWatch['bug'].schema = IBug
 
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2010-05-25 15:48:47 +0000
+++ lib/lp/bugs/model/bug.py	2010-05-29 09:01:05 +0000
@@ -64,12 +64,13 @@
     IStoreSelector, DEFAULT_FLAVOR, MAIN_STORE, NotFoundError)
 
 from lp.answers.interfaces.questiontarget import IQuestionTarget
+from lp.app.errors import UserCannotUnsubscribePerson
 from lp.bugs.adapters.bugchange import (
     BranchLinkedToBug, BranchUnlinkedFromBug, BugConvertedToQuestion,
     BugWatchAdded, BugWatchRemoved, SeriesNominated, UnsubscribedFromBug)
 from lp.bugs.interfaces.bug import (
     IBug, IBugBecameQuestionEvent, IBugSet, IFileBugData,
-    InvalidDuplicateValue, UserCannotUnsubscribePerson)
+    InvalidDuplicateValue)
 from lp.bugs.interfaces.bugactivity import IBugActivitySet
 from lp.bugs.interfaces.bugattachment import (
     BugAttachmentType, IBugAttachmentSet)
 
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2010-05-27 01:46:06 +0000
+++ lib/lp/code/browser/branch.py	2010-05-29 09:01:05 +0000
@@ -356,18 +356,6 @@
 
     def initialize(self):
         self.notices = []
-        self._add_subscription_notice()
-
-    def _add_subscription_notice(self):
-        """Add the appropriate notice after posting the subscription form."""
-        if self.user and self.request.method == 'POST':
-            newsub = self.request.form.get('subscribe', None)
-            if newsub == 'Subscribe':
-                self.context.subscribe(self.user)
-                self.notices.append("You have subscribed to this branch.")
-            elif newsub == 'Unsubscribe':
-                self.context.unsubscribe(self.user)
-                self.notices.append("You have unsubscribed from this branch.")
 
     def user_is_subscribed(self):
         """Is the current user subscribed to this branch?"""
 
=== modified file 'lib/lp/code/browser/branchsubscription.py'
--- lib/lp/code/browser/branchsubscription.py	2010-04-20 01:21:10 +0000
+++ lib/lp/code/browser/branchsubscription.py	2010-05-29 09:01:05 +0000
@@ -12,10 +12,9 @@
     'BranchSubscriptionPrimaryContext',
     ]
 
-from zope.component import getUtility
+
 from zope.interface import implements
 
-from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.webapp import (
     action, canonical_url, LaunchpadEditFormView, LaunchpadFormView,
     LaunchpadView)
@@ -118,7 +117,8 @@
             review_level = data['review_level']
 
             self.context.subscribe(
-                self.user, notification_level, max_diff_lines, review_level)
+                self.user, notification_level, max_diff_lines, review_level,
+                self.user)
 
             self.add_notification_message(
                 'You have subscribed to this branch with: ',
@@ -171,7 +171,7 @@
     def unsubscribe(self, action, data):
         # Be proactive in the checking to catch the stale post problem.
         if self.context.hasSubscription(self.user):
-            self.context.unsubscribe(self.user)
+            self.context.unsubscribe(self.user, self.user)
             self.request.response.addNotification(
                 "You have unsubscribed from this branch.")
         else:
@@ -209,7 +209,8 @@
         subscription = self.context.getSubscription(person)
         if subscription is None:
             self.context.subscribe(
-                person, notification_level, max_diff_lines, review_level)
+                person, notification_level, max_diff_lines, review_level,
+                self.user)
 
             self.add_notification_message(
                 '%s has been subscribed to this branch with: '
@@ -222,24 +223,6 @@
                 subscription.notification_level, subscription.max_diff_lines,
                 review_level)
 
-    def validate(self, data):
-        """Ensure that when a team is subscribed, the user is a member."""
-        celebs = getUtility(ILaunchpadCelebrities)
-        # An admin or bzr expert can subscribe anyone.
-        if self.user.inTeam(celebs.admin) or (
-            self.user.inTeam(celebs.bazaar_experts)):
-            return
-
-        person = data.get('person')
-        if (person is not None and
-            person.isTeam() and
-            not self.user.inTeam(person)):
-            # A person can only subscribe a team if they are members
-            # of that team.
-            self.setFieldError(
-                'person',
-                "You can only subscribe teams that you are a member of.")
-
 
 class BranchSubscriptionEditView(LaunchpadEditFormView):
     """The view for editing branch subscriptions.
@@ -273,7 +256,7 @@
     @action("Unsubscribe", name="unsubscribe")
     def unsubscribe_action(self, action, data):
         """Unsubscribe the team from the branch."""
-        self.branch.unsubscribe(self.person)
+        self.branch.unsubscribe(self.person, self.user)
         self.request.response.addNotification(
             "%s has been unsubscribed from this branch."
             % self.person.displayname)
 
=== modified file 'lib/lp/code/browser/codeimport.py'
--- lib/lp/code/browser/codeimport.py	2010-04-28 02:49:58 +0000
+++ lib/lp/code/browser/codeimport.py	2010-05-29 09:01:05 +0000
@@ -392,7 +392,8 @@
             self.user,
             BranchSubscriptionNotificationLevel.FULL,
             BranchSubscriptionDiffSize.NODIFF,
-            CodeReviewNotificationLevel.NOEMAIL)
+            CodeReviewNotificationLevel.NOEMAIL,
+            self.user)
 
         self.next_url = canonical_url(code_import.branch)
 
 
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2010-05-19 15:39:52 +0000
+++ lib/lp/code/configure.zcml	2010-05-29 09:01:05 +0000
@@ -11,6 +11,7 @@
     xmlns:webservice="http://namespaces.canonical.com/webservice"
     i18n_domain="launchpad">
   <include package=".browser"/>
+  <authorizations module="lp.code.security" />
 
   <!-- Branch Merge Queue -->
 
 
=== modified file 'lib/lp/code/doc/branch-merge-proposal-notifications.txt'
--- lib/lp/code/doc/branch-merge-proposal-notifications.txt	2010-04-06 01:21:33 +0000
+++ lib/lp/code/doc/branch-merge-proposal-notifications.txt	2010-05-29 09:01:05 +0000
@@ -30,13 +30,13 @@
     >>> _unused = bmp.source_branch.subscribe(source_subscriber,
     ...     BranchSubscriptionNotificationLevel.NOEMAIL,
     ...     BranchSubscriptionDiffSize.NODIFF,
-    ...     CodeReviewNotificationLevel.STATUS)
+    ...     CodeReviewNotificationLevel.STATUS, source_subscriber)
     >>> target_subscriber = factory.makePerson(
     ...     email='[e-mail]', displayname='Target Subscriber')
     >>> target_subscription = bmp.target_branch.subscribe(target_subscriber,
     ...     BranchSubscriptionNotificationLevel.NOEMAIL,
     ...     BranchSubscriptionDiffSize.NODIFF,
-    ...     CodeReviewNotificationLevel.FULL)
+    ...     CodeReviewNotificationLevel.FULL, target_subscriber)
 
 The owners of the branches are subscribed when the branches are created.
 
@@ -69,8 +69,8 @@
 
 Now we will unsubscribe the branch owners to simplify the rest of the test.
 
-    >>> bmp.source_branch.unsubscribe(source_owner)
-    >>> bmp.target_branch.unsubscribe(target_owner)
+    >>> bmp.source_branch.unsubscribe(source_owner, source_owner)
+    >>> bmp.target_branch.unsubscribe(target_owner, target_owner)
     >>> recipients = bmp.getNotificationRecipients(
     ...     CodeReviewNotificationLevel.FULL)
 
 
=== modified file 'lib/lp/code/doc/branch-notifications.txt'
--- lib/lp/code/doc/branch-notifications.txt	2010-03-10 05:49:00 +0000
+++ lib/lp/code/doc/branch-notifications.txt	2010-05-29 09:01:05 +0000
@@ -40,7 +40,7 @@
     ...     branch.owner,
     ...     BranchSubscriptionNotificationLevel.FULL,
     ...     BranchSubscriptionDiffSize.WHOLEDIFF,
-    ...     CodeReviewNotificationLevel.NOEMAIL)
+    ...     CodeReviewNotificationLevel.NOEMAIL, branch.owner)
     >>> BranchMailer.forRevision(branch, 1, '[e-mail]',
     ...     'The contents.', None, 'Subject line').sendAll()
 
@@ -71,7 +71,7 @@
     You are subscribed to branch lp://dev/~name12/firefox/main.
     To unsubscribe from this branch go to http://code.launchpad.dev/~name12/firefox/main/+edit-subscription
     <BLANKLINE>
-    >>> branch.unsubscribe(branch.owner)
+    >>> branch.unsubscribe(branch.owner, branch.owner)
 
 
 == Subscriptions ==
@@ -98,63 +98,55 @@
     >>> from lp.code.interfaces.branch import IBranch, IBranchSet
     >>> personset = getUtility(IPersonSet)
 
-    >>> branch.subscribe(
-    ...     personset.getByEmail('[e-mail]'),
+    >>> def subscribe_user_by_email(branch, email, level, size, level2):
+    ...     subscriber = personset.getByEmail(email)
+    ...     branch.subscribe(subscriber, level, size, level2, subscriber)
+
+    >>> subscribe_user_by_email(branch, '[e-mail]',
     ...     BranchSubscriptionNotificationLevel.NOEMAIL,
     ...     BranchSubscriptionDiffSize.NODIFF,
     ...     CodeReviewNotificationLevel.NOEMAIL)
-    <BranchSubscription ...
 
-    >>> branch.subscribe(
-    ...     personset.getByEmail('[e-mail]'),
+    >>> subscribe_user_by_email(branch, '[e-mail]',
     ...     BranchSubscriptionNotificationLevel.ATTRIBUTEONLY,
     ...     BranchSubscriptionDiffSize.NODIFF,
     ...     CodeReviewNotificationLevel.NOEMAIL)
-    <BranchSubscription ...
 
-    >>> branch.subscribe(
-    ...     personset.getByEmail('[e-mail]'),
+    >>> subscribe_user_by_email(branch, '[e-mail]',
     ...     BranchSubscriptionNotificationLevel.DIFFSONLY,
     ...     BranchSubscriptionDiffSize.NODIFF,
     ...     CodeReviewNotificationLevel.NOEMAIL)
-    <BranchSubscription ...
 
-    >>> branch.subscribe(
-    ...     personset.getByEmail('[e-mail]'),
+    >>> subscribe_user_by_email(branch, '[e-mail]',
     ...     BranchSubscriptionNotificationLevel.DIFFSONLY,
     ...     BranchSubscriptionDiffSize.HALFKLINES,
     ...     CodeReviewNotificationLevel.NOEMAIL)
-    <BranchSubscription ...
 
-    >>> branch.subscribe(
-    ...     personset.getByEmail('[e-mail]'),
+    >>> subscribe_user_by_email(branch, '[e-mail]',
     ...     BranchSubscriptionNotificationLevel.DIFFSONLY,
     ...     BranchSubscriptionDiffSize.ONEKLINES,
     ...     CodeReviewNotificationLevel.NOEMAIL)
-    <BranchSubscription ...
 
-    >>> branch.subscribe(
-    ...     personset.getByEmail('[e-mail]'),
+    >>> subscribe_user_by_email(branch, '[e-mail]',
     ...     BranchSubscriptionNotificationLevel.DIFFSONLY,
     ...     BranchSubscriptionDiffSize.FIVEKLINES,
     ...     CodeReviewNotificationLevel.NOEMAIL)
-    <BranchSubscription ...
 
-    >>> branch.subscribe(
-    ...     personset.getByEmail('[e-mail]'),
+    >>> subscribe_user_by_email(branch, '[e-mail]',
     ...     BranchSubscriptionNotificationLevel.FULL,
     ...     BranchSubscriptionDiffSize.WHOLEDIFF,
     ...     CodeReviewNotificationLevel.NOEMAIL)
-    <BranchSubscription ...
 
 Team are subscribed in the same way.
 
-    >>> branch.subscribe(
-    ...     personset.getByName('launchpad'),
+    >>> def subscribe_team_by_name(branch, name, level, size, level2):
+    ...     team = personset.getByName(name)
+    ...     branch.subscribe(team, level, size, level2, team.teamowner)
+
+    >>> subscribe_team_by_name(branch, 'launchpad',
     ...     BranchSubscriptionNotificationLevel.FULL,
     ...     BranchSubscriptionDiffSize.WHOLEDIFF,
     ...     CodeReviewNotificationLevel.NOEMAIL)
-    <BranchSubscription ...
 
 And to make sure we have them:
 
@@ -358,7 +350,7 @@
 Unsubscribe everybody.
 
     >>> for subscription in branch.subscriptions:
-    ...     branch.unsubscribe(subscription.person)
+    ...     branch.unsubscribe(subscription.person, subscription.person)
     >>> len(list(branch.subscriptions))
     0
 
@@ -373,29 +365,23 @@
 If a team is registered, and that team has an email address assigned,
 then that email address is used for the notifications.
 
-    >>> branch.subscribe(
-    ...     personset.getByEmail('[e-mail]'),
+    >>> subscribe_user_by_email(branch, '[e-mail]',
     ...     BranchSubscriptionNotificationLevel.DIFFSONLY,
     ...     BranchSubscriptionDiffSize.HALFKLINES,
     ...     CodeReviewNotificationLevel.NOEMAIL)
-    <BranchSubscription ...
 
-    >>> branch.subscribe(
-    ...     personset.getByName('vcs-imports'),
+    >>> subscribe_team_by_name(branch, 'vcs-imports',
     ...     BranchSubscriptionNotificationLevel.DIFFSONLY,
     ...     BranchSubscriptionDiffSize.FIVEKLINES,
     ...     CodeReviewNotificationLevel.NOEMAIL)
-    <BranchSubscription ...
 
 The ubuntu-team has an email address supplied ([e-mail]), so
 that is used rather than the email addresses of the seven members.
 
-    >>> branch.subscribe(
-    ...     personset.getByName('ubuntu-team'),
+    >>> subscribe_team_by_name(branch, 'ubuntu-team',
     ...     BranchSubscriptionNotificationLevel.DIFFSONLY,
     ...     BranchSubscriptionDiffSize.ONEKLINES,
     ...     CodeReviewNotificationLevel.NOEMAIL)
-    <BranchSubscription ...
 
     >>> recipients = branch.getNotificationRecipients()
     >>> for email in recipients.getEmails():
@@ -427,12 +413,10 @@
 
 Resubscribe our test user.
 
-    >>> branch.subscribe(
-    ...     personset.getByEmail('[e-mail]'),
+    >>> subscribe_user_by_email(branch, '[e-mail]',
     ...     BranchSubscriptionNotificationLevel.ATTRIBUTEONLY,
     ...     BranchSubscriptionDiffSize.NODIFF,
     ...     CodeReviewNotificationLevel.NOEMAIL)
-    <BranchSubscription ...
 
     >>> from zope.event import notify
     >>> from lazr.lifecycle.event import ObjectModifiedEvent
 
=== modified file 'lib/lp/code/doc/branch-visibility.txt'
--- lib/lp/code/doc/branch-visibility.txt	2010-01-12 14:52:41 +0000
+++ lib/lp/code/doc/branch-visibility.txt	2010-05-29 09:01:05 +0000
@@ -138,7 +138,7 @@
     >>> branch.subscribe(ubuntu_team,
     ...     BranchSubscriptionNotificationLevel.NOEMAIL,
     ...     BranchSubscriptionDiffSize.NODIFF,
-    ...     CodeReviewNotificationLevel.NOEMAIL)
+    ...     CodeReviewNotificationLevel.NOEMAIL, ubuntu_team.teamowner)
     <BranchSubscription ...>
 
     >>> access.checkAccountAuthenticated(jdub.account)
 
=== modified file 'lib/lp/code/doc/branch.txt'
--- lib/lp/code/doc/branch.txt	2010-01-12 21:37:34 +0000
+++ lib/lp/code/doc/branch.txt	2010-05-29 09:01:05 +0000
@@ -310,7 +310,7 @@
     ...     subscriber,
     ...     BranchSubscriptionNotificationLevel.FULL,
     ...     BranchSubscriptionDiffSize.FIVEKLINES,
-    ...     CodeReviewNotificationLevel.FULL)
+    ...     CodeReviewNotificationLevel.FULL, subscriber)
     >>> verifyObject(IBranchSubscription, subscription)
     True
     >>> subscription.branch == branch and subscription.person == subscriber
@@ -338,7 +338,7 @@
     ...     subscriber,
     ...     BranchSubscriptionNotificationLevel.FULL,
     ...     BranchSubscriptionDiffSize.FIVEKLINES,
-    ...     CodeReviewNotificationLevel.NOEMAIL)
+    ...     CodeReviewNotificationLevel.NOEMAIL, subscriber)
     >>> subscription == subscription2
     True
     >>> subscription2.review_level == CodeReviewNotificationLevel.NOEMAIL
@@ -346,7 +346,7 @@
 
     Unsubscribing is also supported.
 
-    >>> branch.unsubscribe(subscriber)
+    >>> branch.unsubscribe(subscriber, subscriber)
     >>> branch.subscribers.count()
     1
 
@@ -365,7 +365,7 @@
     ...     subscriber,
     ...     BranchSubscriptionNotificationLevel.FULL,
     ...     BranchSubscriptionDiffSize.FIVEKLINES,
-    ...     CodeReviewNotificationLevel.NOEMAIL)
+    ...     CodeReviewNotificationLevel.NOEMAIL, subscriber)
 
     >>> print_names(branch2.getSubscriptionsByLevel([
     ...     BranchSubscriptionNotificationLevel.FULL]))
 
=== modified file 'lib/lp/code/doc/codeimport.txt'
--- lib/lp/code/doc/codeimport.txt	2010-03-18 22:18:49 +0000
+++ lib/lp/code/doc/codeimport.txt	2010-05-29 09:01:05 +0000
@@ -258,7 +258,7 @@
     ...     nopriv,
     ...     BranchSubscriptionNotificationLevel.FULL,
     ...     BranchSubscriptionDiffSize.NODIFF,
-    ...     CodeReviewNotificationLevel.FULL)
+    ...     CodeReviewNotificationLevel.FULL, nopriv)
 
     >>> from lp.testing.mail_helpers import (
     ...     pop_notifications, print_emails)
 
=== modified file 'lib/lp/code/doc/codereviewcomment.txt'
--- lib/lp/code/doc/codereviewcomment.txt	2010-04-06 01:13:28 +0000
+++ lib/lp/code/doc/codereviewcomment.txt	2010-05-29 09:01:05 +0000
@@ -65,7 +65,7 @@
     >>> _unused = merge_proposal.source_branch.subscribe(source_subscriber,
     ...     BranchSubscriptionNotificationLevel.NOEMAIL,
     ...     BranchSubscriptionDiffSize.NODIFF,
-    ...     CodeReviewNotificationLevel.FULL)
+    ...     CodeReviewNotificationLevel.FULL, source_subscriber)
     >>> from lp.testing.mail_helpers import (
     ...     pop_notifications, print_emails)
     >>> _unused = pop_notifications()
 
=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2010-04-26 00:24:24 +0000
+++ lib/lp/code/interfaces/branch.py	2010-05-29 09:01:05 +0000
@@ -965,9 +965,10 @@
             title=_("The level of code review notification emails."),
             vocabulary=CodeReviewNotificationLevel))
     @operation_returns_entry(Interface) # Really IBranchSubscription
+    @call_with(subscribed_by=REQUEST_USER)
     @export_write_operation()
     def subscribe(person, notification_level, max_diff_lines,
-                  code_review_level):
+                  code_review_level, subscribed_by):
         """Subscribe this person to the branch.
 
         :param person: The `Person` to subscribe.
@@ -977,6 +978,8 @@
             appear in a notification.
         :param code_review_level: The kinds of code review activity that cause
             notification.
+        :param subscribed_by: The person who is subscribing the subscriber.
+            Most often the subscriber themselves.
         :return: new or existing BranchSubscription."""
 
     @operation_parameters(
@@ -995,9 +998,14 @@
         person=Reference(
             title=_("The person to unsubscribe"),
             schema=IPerson))
+    @call_with(unsubscribed_by=REQUEST_USER)
     @export_write_operation()
-    def unsubscribe(person):
-        """Remove the person's subscription to this branch."""
+    def unsubscribe(person, unsubscribed_by):
+        """Remove the person's subscription to this branch.
+
+        :param person: The person or team to unsubscribe from the branch.
+        :param unsubscribed_by: The person doing the unsubscribing.
+        """
 
     def getSubscriptionsByLevel(notification_levels):
         """Return the subscriptions that are at the given notification levels.
 
=== modified file 'lib/lp/code/interfaces/branchsubscription.py'
--- lib/lp/code/interfaces/branchsubscription.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/interfaces/branchsubscription.py	2010-05-29 09:01:05 +0000
@@ -21,7 +21,8 @@
 from lp.code.interfaces.branch import IBranch
 from canonical.launchpad.fields import ParticipatingPersonChoice
 from lazr.restful.declarations import (
-    export_as_webservice_entry, exported)
+    REQUEST_USER, call_with, export_as_webservice_entry,
+    export_read_operation, exported)
 from lazr.restful.fields import Reference
 
 
@@ -73,3 +74,13 @@
                 'Control the kind of review activity that triggers '
                 'notifications.'
                 )))
+
+    subscribed_by = exported(ParticipatingPersonChoice(
+        title=_('Subscribed by'), required=True,
+        vocabulary='ValidPersonOrTeam', readonly=True,
+        description=_("The person who created this subscription.")))
+
+    @call_with(user=REQUEST_USER)
+    @export_read_operation()
+    def canBeUnsubscribedByUser(user):
+        """Can the user unsubscribe the subscriber from the branch?"""
 
=== modified file 'lib/lp/code/mail/tests/test_branch.py'
--- lib/lp/code/mail/tests/test_branch.py	2010-04-26 00:22:16 +0000
+++ lib/lp/code/mail/tests/test_branch.py	2010-05-29 09:01:05 +0000
@@ -35,7 +35,7 @@
             source_branch.owner, target_branch)
         subscription = merge_proposal.source_branch.subscribe(
             subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL, subscriber)
         return merge_proposal, subscription
 
     def test_forBranchSubscriber(self):
 
=== modified file 'lib/lp/code/mail/tests/test_branchmergeproposal.py'
--- lib/lp/code/mail/tests/test_branchmergeproposal.py	2010-05-18 13:14:40 +0000
+++ lib/lp/code/mail/tests/test_branchmergeproposal.py	2010-05-29 09:01:05 +0000
@@ -67,7 +67,7 @@
             email='[e-mail]')
         bmp.source_branch.subscribe(subscriber,
             BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL, subscriber)
         bmp.source_branch.owner.name = 'bob'
         bmp.source_branch.name = 'fix-foo-for-bar'
         bmp.target_branch.owner.name = 'mary'
@@ -176,7 +176,7 @@
         bmp = request.merge_proposal
         bmp.source_branch.subscribe(
             bmp.registrant, BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL, bmp.registrant)
         mailer = BMPMailer.forCreation(bmp, bmp.registrant)
         ctrl = mailer.generateEmail(bmp.registrant,
                                     bmp.registrant.preferredemail.email)
@@ -259,7 +259,7 @@
             diff_text=diff_text)
         bmp.source_branch.subscribe(subscriber,
             BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.STATUS)
+            CodeReviewNotificationLevel.STATUS, subscriber)
         mailer = BMPMailer.forCreation(bmp, bmp.registrant)
         ctrl = mailer.generateEmail('[e-mail]', subscriber)
         self.assertEqual(0, len(ctrl.attachments))
 
=== modified file 'lib/lp/code/mail/tests/test_codehandler.py'
--- lib/lp/code/mail/tests/test_codehandler.py	2010-04-29 07:47:47 +0000
+++ lib/lp/code/mail/tests/test_codehandler.py	2010-05-29 09:01:05 +0000
@@ -346,7 +346,7 @@
         subscriber = self.factory.makePerson()
         bmp.source_branch.subscribe(
             subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL, subscriber)
         email_addr = bmp.address
         self.switchDbUser(config.processmail.dbuser)
         self.code_handler.process(mail, email_addr, None)
 
=== modified file 'lib/lp/code/mail/tests/test_codereviewcomment.py'
--- lib/lp/code/mail/tests/test_codereviewcomment.py	2010-05-07 17:16:44 +0000
+++ lib/lp/code/mail/tests/test_codereviewcomment.py	2010-05-29 09:01:05 +0000
@@ -48,7 +48,7 @@
             notification_level = CodeReviewNotificationLevel.FULL
         comment.branch_merge_proposal.source_branch.subscribe(
             subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            notification_level)
+            notification_level, subscriber)
         # Email is not sent on construction, so fake a root message id on the
         # merge proposal.
         login_person(comment.branch_merge_proposal.registrant)
@@ -265,7 +265,7 @@
             email='[e-mail]', displayname='Commenter')
         bmp.source_branch.subscribe(commenter,
             BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL, commenter)
         comment = bmp.createComment(commenter, 'hello')
         return comment
 
 
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2010-05-15 17:43:59 +0000
+++ lib/lp/code/model/branch.py	2010-05-29 09:01:05 +0000
@@ -45,6 +45,7 @@
 from canonical.launchpad.webapp.interfaces import (
     IStoreSelector, MAIN_STORE, SLAVE_FLAVOR)
 
+from lp.app.errors import UserCannotUnsubscribePerson
 from lp.code.bzr import (
     BranchFormat, ControlFormat, CURRENT_BRANCH_FORMATS,
     CURRENT_REPOSITORY_FORMATS, RepositoryFormat)
@@ -659,7 +660,7 @@
 
     # subscriptions
     def subscribe(self, person, notification_level, max_diff_lines,
-                  code_review_level):
+                  code_review_level, subscribed_by):
         """See `IBranch`."""
         # If the person is already subscribed, update the subscription with
         # the specified notification details.
@@ -668,7 +669,8 @@
             subscription = BranchSubscription(
                 branch=self, person=person,
                 notification_level=notification_level,
-                max_diff_lines=max_diff_lines, review_level=code_review_level)
+                max_diff_lines=max_diff_lines, review_level=code_review_level,
+                subscribed_by=subscribed_by)
             Store.of(subscription).flush()
         else:
             subscription.notification_level = notification_level
@@ -701,12 +703,19 @@
         """See `IBranch`."""
         return self.getSubscription(person) is not None
 
-    def unsubscribe(self, person):
+    def unsubscribe(self, person, unsubscribed_by):
         """See `IBranch`."""
         subscription = self.getSubscription(person)
+        if subscription is None:
+            # Silent success seems order of the day (like bugs).
+            return
+        if not subscription.canBeUnsubscribedByUser(unsubscribed_by):
+            raise UserCannotUnsubscribePerson(
+                '%s does not have permission to unsubscribe %s.' % (
+                    unsubscribed_by.displayname,
+                    person.displayname))
         store = Store.of(subscription)
-        assert subscription is not None, "User is not subscribed."
-        BranchSubscription.delete(subscription.id)
+        store.remove(subscription)
         store.flush()
 
     def getBranchRevision(self, sequence=None, revision=None,
 
=== modified file 'lib/lp/code/model/branchnamespace.py'
--- lib/lp/code/model/branchnamespace.py	2010-02-17 11:19:42 +0000
+++ lib/lp/code/model/branchnamespace.py	2010-05-29 09:01:05 +0000
@@ -106,7 +106,8 @@
                 implicit_subscription,
                 BranchSubscriptionNotificationLevel.NOEMAIL,
                 BranchSubscriptionDiffSize.NODIFF,
-                CodeReviewNotificationLevel.NOEMAIL)
+                CodeReviewNotificationLevel.NOEMAIL,
+                registrant)
 
         # The registrant of the branch should also be automatically subscribed
         # in order for them to get code review notifications.  The implicit
@@ -116,7 +117,8 @@
             registrant,
             BranchSubscriptionNotificationLevel.NOEMAIL,
             BranchSubscriptionDiffSize.NODIFF,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL,
+            registrant)
 
         notify(ObjectCreatedEvent(branch))
         return branch
 
=== modified file 'lib/lp/code/model/branchsubscription.py'
--- lib/lp/code/model/branchsubscription.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/model/branchsubscription.py	2010-05-29 09:01:05 +0000
@@ -13,13 +13,14 @@
 from canonical.database.constants import DEFAULT
 from canonical.database.sqlbase import SQLBase
 from canonical.database.enumcol import EnumCol
-
+from canonical.launchpad.interfaces.launchpad import IPersonRoles
 from lp.code.enums import (
     BranchSubscriptionDiffSize, BranchSubscriptionNotificationLevel,
     CodeReviewNotificationLevel)
 from lp.code.interfaces.branchsubscription import IBranchSubscription
 from lp.code.interfaces.branch import IBranchNavigationMenu
 from lp.code.interfaces.branchtarget import IHasBranchTarget
+from lp.code.security import BranchSubscriptionEdit
 from lp.registry.interfaces.person import (
     validate_person_not_private_membership)
 
@@ -41,8 +42,18 @@
                              notNull=False, default=DEFAULT)
     review_level = EnumCol(enum=CodeReviewNotificationLevel,
                                  notNull=True, default=DEFAULT)
+    subscribed_by = ForeignKey(
+        dbName='subscribed_by', foreignKey='Person',
+        storm_validator=validate_person_not_private_membership, notNull=True)
 
     @property
     def target(self):
         """See `IHasBranchTarget`."""
         return self.branch.target
+
+    def canBeUnsubscribedByUser(self, user):
+        """See `IBranchSubscription`."""
+        if user is None:
+            return False
+        permission_check = BranchSubscriptionEdit(self)
+        return permission_check.checkAuthenticated(IPersonRoles(user))
 
=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py	2010-04-23 04:11:12 +0000
+++ lib/lp/code/model/tests/test_branch.py	2010-05-29 09:01:05 +0000
@@ -908,7 +908,7 @@
         # The owner of the branch is subscribed to the branch when it is
         # created.  The tests here assume no initial connections, so
         # unsubscribe the branch owner here.
-        self.branch.unsubscribe(self.branch.owner)
+        self.branch.unsubscribe(self.branch.owner, self.branch.owner)
 
     def test_deletable(self):
         """A newly created branch can be deleted without any problems."""
@@ -930,7 +930,7 @@
         """A branch that has a subscription can be deleted."""
         self.branch.subscribe(
             self.user, BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.NOEMAIL)
+            CodeReviewNotificationLevel.NOEMAIL, self.user)
         self.assertEqual(True, self.branch.canBeDeleted())
 
     def test_codeImportCanStillBeDeleted(self):
@@ -1068,7 +1068,7 @@
         # The owner of the branch is subscribed to the branch when it is
         # created.  The tests here assume no initial connections, so
         # unsubscribe the branch owner here.
-        self.branch.unsubscribe(self.branch.owner)
+        self.branch.unsubscribe(self.branch.owner, self.branch.owner)
 
     def test_plainBranch(self):
         """Ensure that a fresh branch has no deletion requirements."""
@@ -1081,8 +1081,9 @@
         prerequisite_branch = self.factory.makeProductBranch(
             product=self.branch.product)
         # Remove the implicit subscriptions.
-        target_branch.unsubscribe(target_branch.owner)
-        prerequisite_branch.unsubscribe(prerequisite_branch.owner)
+        target_branch.unsubscribe(target_branch.owner, target_branch.owner)
+        prerequisite_branch.unsubscribe(
+            prerequisite_branch.owner, prerequisite_branch.owner)
         merge_proposal1 = self.branch.addLandingTarget(
             self.branch.owner, target_branch, prerequisite_branch)
         # Disable this merge proposal, to allow creating a new identical one
@@ -1261,7 +1262,8 @@
         """Deletion requirements for a code import branch are right"""
         code_import = self.factory.makeCodeImport()
         # Remove the implicit branch subscription first.
-        code_import.branch.unsubscribe(code_import.branch.owner)
+        code_import.branch.unsubscribe(
+            code_import.branch.owner, code_import.branch.owner)
         self.assertEqual({}, code_import.branch.deletionRequirements())
 
     def test_branchWithCodeImportDeletion(self):
@@ -2303,7 +2305,8 @@
         # Revisions created before the start date are not returned.
         branch = self.factory.makeAnyBranch()
         epoch = datetime(2009, 9, 10, tzinfo=UTC)
-        old = add_revision_to_branch(
+        # Add some revisions before the epoch.
+        add_revision_to_branch(
             self.factory, branch, epoch - timedelta(days=1))
         new = add_revision_to_branch(
             self.factory, branch, epoch + timedelta(days=1))
@@ -2318,7 +2321,8 @@
         end_date = epoch + timedelta(days=2)
         in_range = add_revision_to_branch(
             self.factory, branch, end_date - timedelta(days=1))
-        too_new = add_revision_to_branch(
+        # Add some revisions after the end_date.
+        add_revision_to_branch(
             self.factory, branch, end_date + timedelta(days=1))
         result = branch.getMainlineBranchRevisions(epoch, end_date)
         branch_revisions = [br for br, rev, ra in result]
@@ -2354,7 +2358,8 @@
         epoch = datetime(2009, 9, 10, tzinfo=UTC)
         old = add_revision_to_branch(
             self.factory, branch, epoch + timedelta(days=1))
-        merged = add_revision_to_branch(
+        # Add some non mainline revision.
+        add_revision_to_branch(
             self.factory, branch, epoch + timedelta(days=2), mainline=False)
         new = add_revision_to_branch(
             self.factory, branch, epoch + timedelta(days=3))
 
=== modified file 'lib/lp/code/model/tests/test_branchcollection.py'
--- lib/lp/code/model/tests/test_branchcollection.py	2009-10-28 04:33:49 +0000
+++ lib/lp/code/model/tests/test_branchcollection.py	2010-05-29 09:01:05 +0000
@@ -364,7 +364,8 @@
         branch.subscribe(
             subscriber, BranchSubscriptionNotificationLevel.NOEMAIL,
             BranchSubscriptionDiffSize.NODIFF,
-            CodeReviewNotificationLevel.NOEMAIL)
+            CodeReviewNotificationLevel.NOEMAIL,
+            subscriber)
         collection = self.all_branches.subscribedBy(subscriber)
         self.assertEqual([branch], list(collection.getBranches()))
 
@@ -377,7 +378,7 @@
         owned_branch = self.factory.makeAnyBranch(owner=person)
         # Unsubscribe the owner, to demonstrate that we show owned branches
         # even if they aren't subscribed.
-        owned_branch.unsubscribe(person)
+        owned_branch.unsubscribe(person, person)
         # Subscribe two other people to the owned branch to make sure
         # that the BranchSubscription join is doing it right.
         self.factory.makeBranchSubscription(branch=owned_branch)
@@ -389,7 +390,8 @@
         subscribed_branch.subscribe(
             person, BranchSubscriptionNotificationLevel.NOEMAIL,
             BranchSubscriptionDiffSize.NODIFF,
-            CodeReviewNotificationLevel.NOEMAIL)
+            CodeReviewNotificationLevel.NOEMAIL,
+            person)
         related_branches = self.all_branches.relatedTo(person)
         self.assertEqual(
             sorted([owned_branch, registered_branch, subscribed_branch]),
@@ -548,7 +550,8 @@
         removeSecurityProxy(self.private_branch1).subscribe(
             subscriber, BranchSubscriptionNotificationLevel.NOEMAIL,
             BranchSubscriptionDiffSize.NODIFF,
-            CodeReviewNotificationLevel.NOEMAIL)
+            CodeReviewNotificationLevel.NOEMAIL,
+            subscriber)
         branches = self.all_branches.visibleByUser(subscriber)
         self.assertEqual(
             sorted([self.public_branch, self.private_branch1]),
@@ -564,7 +567,8 @@
         removeSecurityProxy(private_branch).subscribe(
             team, BranchSubscriptionNotificationLevel.NOEMAIL,
             BranchSubscriptionDiffSize.NODIFF,
-            CodeReviewNotificationLevel.NOEMAIL)
+            CodeReviewNotificationLevel.NOEMAIL,
+            team_owner)
         # Members of the team can see the private branch that the team is
         # subscribed to.
         branches = self.all_branches.visibleByUser(team_owner)
 
=== modified file 'lib/lp/code/model/tests/test_branchjob.py'
--- lib/lp/code/model/tests/test_branchjob.py	2010-05-25 05:19:30 +0000
+++ lib/lp/code/model/tests/test_branchjob.py	2010-05-29 09:01:05 +0000
@@ -322,10 +322,12 @@
     def test_run_sends_mail(self):
         """Ensure RevisionMailJob.run sends mail with correct values."""
         branch = self.factory.makeAnyBranch()
-        branch.subscribe(branch.registrant,
+        branch.subscribe(
+            branch.registrant,
             BranchSubscriptionNotificationLevel.FULL,
             BranchSubscriptionDiffSize.WHOLEDIFF,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL,
+            branch.registrant)
         job = RevisionMailJob.create(
             branch, 0, '[e-mail]', 'hello', False, 'subject')
         job.run()
@@ -515,10 +517,12 @@
         product = self.factory.makeProduct(name='foo')
         branch = self.factory.makeProductBranch(
             name='bar', product=product, owner=jrandom)
-        branch.subscribe(branch.registrant,
+        branch.subscribe(
+            branch.registrant,
             BranchSubscriptionNotificationLevel.FULL,
             BranchSubscriptionDiffSize.WHOLEDIFF,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL,
+            branch.registrant)
         branch, tree = self.create_branch_and_tree(db_branch=branch)
         tree.branch.nick = 'nicholas'
         tree.lock_write()
 
=== modified file 'lib/lp/code/model/tests/test_branchmergeproposals.py'
--- lib/lp/code/model/tests/test_branchmergeproposals.py	2010-05-07 04:53:47 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposals.py	2010-05-29 09:01:05 +0000
@@ -722,16 +722,20 @@
         subscriber_set = set([source_owner, target_owner])
         self.assertEqual(subscriber_set, set(recipients.keys()))
         source_subscriber = self.factory.makePerson()
-        bmp.source_branch.subscribe(source_subscriber,
+        bmp.source_branch.subscribe(
+            source_subscriber,
             BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL,
+            source_subscriber)
         recipients = bmp.getNotificationRecipients(
             CodeReviewNotificationLevel.STATUS)
         subscriber_set.add(source_subscriber)
         self.assertEqual(subscriber_set, set(recipients.keys()))
-        bmp.source_branch.subscribe(source_subscriber,
+        bmp.source_branch.subscribe(
+            source_subscriber,
             BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.NOEMAIL)
+            CodeReviewNotificationLevel.NOEMAIL,
+            source_subscriber)
         # By specifying no email, they will no longer get email.
         subscriber_set.remove(source_subscriber)
         recipients = bmp.getNotificationRecipients(
@@ -744,11 +748,11 @@
         full_subscriber = self.factory.makePerson()
         bmp.source_branch.subscribe(full_subscriber,
             BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL, full_subscriber)
         status_subscriber = self.factory.makePerson()
         bmp.source_branch.subscribe(status_subscriber,
             BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.STATUS)
+            CodeReviewNotificationLevel.STATUS, status_subscriber)
         recipients = bmp.getNotificationRecipients(
             CodeReviewNotificationLevel.STATUS)
         # Both of the branch owners are now subscribed to their own
@@ -778,15 +782,15 @@
         source_subscriber = self.factory.makePerson()
         bmp.source_branch.subscribe(source_subscriber,
             BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL, source_subscriber)
         target_subscriber = self.factory.makePerson()
         bmp.target_branch.subscribe(target_subscriber,
             BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL, target_subscriber)
         prerequisite_subscriber = self.factory.makePerson()
         bmp.prerequisite_branch.subscribe(prerequisite_subscriber,
             BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL, prerequisite_subscriber)
         recipients = bmp.getNotificationRecipients(
             CodeReviewNotificationLevel.FULL)
         self.assertEqual(
@@ -832,7 +836,7 @@
         # Make sure that the registrant is subscribed.
         bmp.source_branch.subscribe(registrant,
             BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL, registrant)
         recipients = bmp.getNotificationRecipients(
             CodeReviewNotificationLevel.STATUS)
         reason = recipients[registrant]
@@ -879,7 +883,7 @@
         # we don't send them eamil.
         bmp = self.factory.makeBranchMergeProposal()
         owner = bmp.source_branch.owner
-        bmp.source_branch.unsubscribe(owner)
+        bmp.source_branch.unsubscribe(owner, owner)
         recipients = bmp.getNotificationRecipients(
             CodeReviewNotificationLevel.STATUS)
         self.assertFalse(owner in recipients)
@@ -892,20 +896,20 @@
         eric = self.factory.makePerson()
         bmp.source_branch.subscribe(
             eric, BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL, eric)
         # Subscribe bob to the target branch only.
         bob = self.factory.makePerson()
         bmp.target_branch.subscribe(
             bob, BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL, bob)
         # Subscribe charlie to both.
         charlie = self.factory.makePerson()
         bmp.source_branch.subscribe(
             charlie, BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL, charlie)
         bmp.target_branch.subscribe(
             charlie, BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL, charlie)
         # Make both branches private.
         removeSecurityProxy(bmp.source_branch).private = True
         removeSecurityProxy(bmp.target_branch).private = True
@@ -1157,7 +1161,7 @@
         proposal.source_branch.subscribe(
             subscriber,
             BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.NOEMAIL)
+            CodeReviewNotificationLevel.NOEMAIL, subscriber)
         self.assertEqual(
             ['~albert/mike/work', '~albert/november/work'],
             self._get_merge_proposals(albert, visible_by_user=subscriber))
 
=== added file 'lib/lp/code/model/tests/test_branchsubscription.py'
--- lib/lp/code/model/tests/test_branchsubscription.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/model/tests/test_branchsubscription.py	2010-05-29 09:01:05 +0000
@@ -0,0 +1,115 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the BranchSubscrptions model object.."""
+
+from __future__ import with_statement
+
+__metaclass__ = type
+
+import unittest
+
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.app.errors import UserCannotUnsubscribePerson
+from lp.code.enums import (
+    BranchSubscriptionNotificationLevel, CodeReviewNotificationLevel)
+from lp.testing import TestCaseWithFactory, person_logged_in
+
+
+class TestBranchSubscriptions(TestCaseWithFactory):
+    """Tests relating to branch subscriptions in general."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_subscribed_by_set(self):
+        """The user subscribing is recorded along the subscriber."""
+        subscriber = self.factory.makePerson()
+        subscribed_by = self.factory.makePerson()
+        branch = self.factory.makeAnyBranch()
+        subscription = branch.subscribe(
+            subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
+            CodeReviewNotificationLevel.NOEMAIL, subscribed_by)
+        self.assertEqual(subscriber, subscription.person)
+        self.assertEqual(subscribed_by, subscription.subscribed_by)
+
+    def test_unsubscribe(self):
+        """Test unsubscribing by the subscriber."""
+        subscription = self.factory.makeBranchSubscription()
+        subscriber = subscription.person
+        branch = subscription.branch
+        branch.unsubscribe(subscriber, subscriber)
+        self.assertFalse(branch.hasSubscription(subscriber))
+
+    def test_unsubscribe_by_subscriber(self):
+        """Test unsubscribing by the person who subscribed the user."""
+        subscribed_by = self.factory.makePerson()
+        subscription = self.factory.makeBranchSubscription(
+            subscribed_by=subscribed_by)
+        subscriber = subscription.person
+        branch = subscription.branch
+        branch.unsubscribe(subscriber, subscribed_by)
+        self.assertFalse(branch.hasSubscription(subscriber))
+
+    def test_unsubscribe_by_unauthorized(self):
+        """Test unsubscribing someone you shouldn't be able to."""
+        subscription = self.factory.makeBranchSubscription()
+        branch = subscription.branch
+        self.assertRaises(
+            UserCannotUnsubscribePerson,
+            branch.unsubscribe,
+            subscription.person,
+            self.factory.makePerson())
+
+
+class TestBranchSubscriptionCanBeUnsubscribedbyUser(TestCaseWithFactory):
+    """Tests for BranchSubscription.canBeUnsubscribedByUser."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_none(self):
+        """None for a user always returns False."""
+        subscription = self.factory.makeBranchSubscription()
+        self.assertFalse(subscription.canBeUnsubscribedByUser(None))
+
+    def test_self_subscriber(self):
+        """The subscriber has permission to unsubscribe."""
+        subscription = self.factory.makeBranchSubscription()
+        self.assertTrue(
+            subscription.canBeUnsubscribedByUser(subscription.person))
+
+    def test_non_subscriber_fails(self):
+        """An unrelated person can't unsubscribe a user."""
+        subscription = self.factory.makeBranchSubscription()
+        editor = self.factory.makePerson()
+        self.assertFalse(subscription.canBeUnsubscribedByUser(editor))
+
+    def test_subscribed_by(self):
+        """If a user subscribes someone else, the user can unsubscribe."""
+        subscribed_by = self.factory.makePerson()
+        subscriber = self.factory.makePerson()
+        subscription = self.factory.makeBranchSubscription(
+            person=subscriber, subscribed_by=subscribed_by)
+        self.assertTrue(subscription.canBeUnsubscribedByUser(subscribed_by))
+
+    def test_team_member_can_unsubscribe(self):
+        """Any team member can unsubscribe the team from a branch."""
+        team = self.factory.makeTeam()
+        member = self.factory.makePerson()
+        with person_logged_in(team.teamowner):
+            team.addMember(member, team.teamowner)
+        subscription = self.factory.makeBranchSubscription(
+            person=team, subscribed_by=team.teamowner)
+        self.assertTrue(subscription.canBeUnsubscribedByUser(member))
+
+    def test_team_subscriber_can_unsubscribe(self):
+        """A team can be unsubscribed by the subscriber even if they are not a
+        member."""
+        team = self.factory.makeTeam()
+        subscribed_by = self.factory.makePerson()
+        subscription = self.factory.makeBranchSubscription(
+            person=team, subscribed_by=subscribed_by)
+        self.assertTrue(subscription.canBeUnsubscribedByUser(subscribed_by))
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)
 
=== modified file 'lib/lp/code/scripts/tests/test_scan_branches.py'
--- lib/lp/code/scripts/tests/test_scan_branches.py	2010-04-28 05:40:02 +0000
+++ lib/lp/code/scripts/tests/test_scan_branches.py	2010-05-29 09:01:05 +0000
@@ -52,7 +52,8 @@
             db_branch.registrant,
             BranchSubscriptionNotificationLevel.FULL,
             BranchSubscriptionDiffSize.WHOLEDIFF,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL,
+            db_branch.registrant)
         transaction.commit()
 
         self.run_script_and_assert_success()
 
=== modified file 'lib/lp/code/scripts/tests/test_sendbranchmail.py'
--- lib/lp/code/scripts/tests/test_sendbranchmail.py	2010-04-28 05:40:02 +0000
+++ lib/lp/code/scripts/tests/test_sendbranchmail.py	2010-05-29 09:01:05 +0000
@@ -24,10 +24,12 @@
 
     def createBranch(self):
         branch, tree = self.create_branch_and_tree()
-        branch.subscribe(branch.registrant,
+        branch.subscribe(
+            branch.registrant,
             BranchSubscriptionNotificationLevel.FULL,
             BranchSubscriptionDiffSize.WHOLEDIFF,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL,
+            branch.registrant)
         transport = tree.bzrdir.root_transport
         transport.put_bytes('foo', 'bar')
         tree.add('foo')
 
=== added file 'lib/lp/code/security.py'
--- lib/lp/code/security.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/security.py	2010-05-29 09:01:05 +0000
@@ -0,0 +1,37 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Security adapters for the code module."""
+
+__metaclass__ = type
+__all__ = [
+    'BranchSubscriptionEdit',
+    'BranchSubscriptionView',
+    ]
+
+from canonical.launchpad.security import AuthorizationBase
+
+from lp.code.interfaces.branchsubscription import IBranchSubscription
+
+
+
+class BranchSubscriptionEdit(AuthorizationBase):
+    permission = 'launchpad.Edit'
+    usedfor = IBranchSubscription
+
+    def checkAuthenticated(self, user):
+        """Is the user able to edit a branch subscription?
+
+        Any team member can edit a branch subscription for their team.
+        Launchpad Admins can also edit any branch subscription.
+        """
+        return (user.inTeam(self.obj.person) or
+                user.inTeam(self.obj.subscribed_by) or
+                user.in_admin or
+                user.in_bazaar_experts)
+
+
+class BranchSubscriptionView(BranchSubscriptionEdit):
+    permission = 'launchpad.View'
+
+
 
=== modified file 'lib/lp/code/stories/branches/xx-branch-edit.txt'
--- lib/lp/code/stories/branches/xx-branch-edit.txt	2010-05-24 04:27:46 +0000
+++ lib/lp/code/stories/branches/xx-branch-edit.txt	2010-05-29 09:01:05 +0000
@@ -223,7 +223,7 @@
     >>> _unused = foogoo_svn.subscribe(sample_person,
     ...     BranchSubscriptionNotificationLevel.ATTRIBUTEONLY,
     ...     BranchSubscriptionDiffSize.NODIFF,
-    ...     CodeReviewNotificationLevel.NOEMAIL)
+    ...     CodeReviewNotificationLevel.NOEMAIL, sample_person)
     >>> logout()
 
     >>> nopriv_browser = setupBrowser(auth='Basic [e-mail]:test')
 
=== modified file 'lib/lp/code/stories/branches/xx-branchmergeproposals.txt'
--- lib/lp/code/stories/branches/xx-branchmergeproposals.txt	2010-04-07 16:05:38 +0000
+++ lib/lp/code/stories/branches/xx-branchmergeproposals.txt	2010-05-29 09:01:05 +0000
@@ -19,7 +19,7 @@
     ...     '~name12/firefox/main')
     >>> _unused = branch.subscribe(subscriber,
     ...     BranchSubscriptionNotificationLevel.NOEMAIL, None,
-    ...     CodeReviewNotificationLevel.FULL)
+    ...     CodeReviewNotificationLevel.FULL, subscriber)
 
 Also subscribe to gnome-terminal, since Firefox tests are mixed with
 Gnome Terminal tests.
@@ -28,7 +28,7 @@
     ...     '~name12/gnome-terminal/main')
     >>> _unused = branch.subscribe(subscriber,
     ...     BranchSubscriptionNotificationLevel.NOEMAIL, None,
-    ...     CodeReviewNotificationLevel.FULL)
+    ...     CodeReviewNotificationLevel.FULL, subscriber)
     >>> from lp.code.tests.helpers import make_erics_fooix_project
     >>> locals().update(make_erics_fooix_project(factory))
     >>> logout()
 
=== modified file 'lib/lp/code/stories/branches/xx-person-branches.txt'
--- lib/lp/code/stories/branches/xx-person-branches.txt	2010-05-13 16:22:19 +0000
+++ lib/lp/code/stories/branches/xx-person-branches.txt	2010-05-29 09:01:05 +0000
@@ -107,7 +107,7 @@
 
     >>> login(ANONYMOUS)
     >>> b2 = factory.makeAnyBranch(owner=eric)
-    >>> ignored = b2.unsubscribe(eric)
+    >>> ignored = b2.unsubscribe(eric, eric)
     >>> logout()
 
     >>> eric_browser.open('http://code.launchpad.dev/~eric')
 
=== modified file 'lib/lp/code/stories/branches/xx-subscribing-branches.txt'
--- lib/lp/code/stories/branches/xx-subscribing-branches.txt	2010-04-20 04:12:30 +0000
+++ lib/lp/code/stories/branches/xx-subscribing-branches.txt	2010-05-29 09:01:05 +0000
@@ -190,25 +190,8 @@
     >>> browser.getControl('Person').value = 'landscape-developers'
     >>> browser.getControl('Subscribe').click()
 
-The user can only subscribe teams that they are members of.
-
-    >>> for message in get_feedback_messages(browser.contents):
-    ...     print extract_text(message)
-    There is 1 error.
-    You can only subscribe teams that you are a member of.
-
-Sample Person is a member of Landscape Developers.
-
-    >>> browser = setupBrowser(auth='Basic [e-mail]:test')
-    >>> browser.open(
-    ...     'http://code.launchpad.dev/~name12/gnome-terminal/main')
-    >>> browser.getLink('Subscribe someone else').click()
-    >>> browser.getControl('Notification Level').displayValue = [
-    ...     'Branch attribute and revision notifications']
-    >>> browser.getControl('Generated Diff Size Limit').displayValue = [
-    ...     '1000 lines']
-    >>> browser.getControl('Person').value = 'landscape-developers'
-    >>> browser.getControl('Subscribe').click()
+The user does not have to be in the team to subscribe them.
+
     >>> print_informational_message(browser.contents)
     Landscape Developers has been subscribed to this branch with:
     Send notifications for both branch attribute updates
@@ -234,10 +217,10 @@
 Editing a team subscription
 ===========================
 
-In order to edit a team subscription the logged in user needs to be
-a member of the team that is subscribed.  There is a link shown in
-the subscriptions portlet to edit the subscription of a team that
-the logged in user is a member of.
+In order to edit a team subscription the logged in user needs to be a member
+of the team that is subscribed, or must the the person who subscribed the team
+to the branch.  There is a link shown in the subscriptions portlet to edit the
+subscription of a team that the logged in user is a member of.
 
 XXX: thumper 2007-06-11
 There should be a central user subscriptions page.  This could then
@@ -293,7 +276,7 @@
     ...     CodeReviewNotificationLevel)
     >>> ignored = branch.subscribe(
     ...     private_team, BranchSubscriptionNotificationLevel.NOEMAIL, None,
-    ...     CodeReviewNotificationLevel.NOEMAIL)
+    ...     CodeReviewNotificationLevel.NOEMAIL, private_team.teamowner)
     >>> url = canonical_url(branch)
     >>> logout()
 
 
=== modified file 'lib/lp/code/stories/webservice/xx-branchsubscription.txt'
--- lib/lp/code/stories/webservice/xx-branchsubscription.txt	2009-06-16 03:40:05 +0000
+++ lib/lp/code/stories/webservice/xx-branchsubscription.txt	2010-05-29 09:01:05 +0000
@@ -38,6 +38,7 @@
     resource_type_link: u'http://.../#branch_subscription'
     review_level: u'No email'
     self_link: u'http://.../~farmer-bob/farm/corn/+subscription/farmer-joe'
+    subscribed_by_link: u'http://.../~salgado'
 
     >>> def print_subscriber_count(branch):
     ...     subscribers = webservice.get(
@@ -91,6 +92,7 @@
     resource_type_link: u'http://.../#branch_subscription'
     review_level: u'Status changes only'
     self_link: u'http://.../~farmer-bob/farm/corn/+subscription/farmer-joe'
+    subscribed_by_link: u'http://.../~salgado'
 
 
 We print the count, and even though subscribe was called again, there's still
 
=== modified file 'lib/lp/code/tests/test_branch.py'
--- lib/lp/code/tests/test_branch.py	2010-05-27 01:46:06 +0000
+++ lib/lp/code/tests/test_branch.py	2010-05-29 09:01:05 +0000
@@ -134,7 +134,7 @@
         removeSecurityProxy(branch).subscribe(
             person, BranchSubscriptionNotificationLevel.NOEMAIL,
             BranchSubscriptionDiffSize.NODIFF,
-            CodeReviewNotificationLevel.NOEMAIL)
+            CodeReviewNotificationLevel.NOEMAIL, person)
         self.assertAuthenticatedView(branch, person, True)
 
     def test_privateBranchAnyoneElse(self):
 
=== modified file 'lib/lp/codehosting/scanner/tests/test_bzrsync.py'
--- lib/lp/codehosting/scanner/tests/test_bzrsync.py	2010-05-03 09:31:06 +0000
+++ lib/lp/codehosting/scanner/tests/test_bzrsync.py	2010-05-29 09:01:05 +0000
@@ -85,7 +85,7 @@
         LaunchpadZopelessLayer.txn.begin()
         new_branch = self.factory.makeAnyBranch(*args, **kwargs)
         # Unsubscribe the implicit owner subscription.
-        new_branch.unsubscribe(new_branch.owner)
+        new_branch.unsubscribe(new_branch.owner, new_branch.owner)
         LaunchpadZopelessLayer.txn.commit()
         return new_branch
 
 
=== modified file 'lib/lp/codehosting/scanner/tests/test_email.py'
--- lib/lp/codehosting/scanner/tests/test_email.py	2010-05-18 13:12:34 +0000
+++ lib/lp/codehosting/scanner/tests/test_email.py	2010-05-29 09:01:05 +0000
@@ -41,7 +41,8 @@
             test_user,
             BranchSubscriptionNotificationLevel.FULL,
             BranchSubscriptionDiffSize.FIVEKLINES,
-            CodeReviewNotificationLevel.NOEMAIL)
+            CodeReviewNotificationLevel.NOEMAIL,
+            test_user)
         LaunchpadZopelessLayer.txn.commit()
         return branch
 
@@ -151,7 +152,8 @@
             db_branch.registrant,
             BranchSubscriptionNotificationLevel.FULL,
             BranchSubscriptionDiffSize.WHOLEDIFF,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL,
+            db_branch.registrant)
         self.assertEqual(0, len(list(RevisionMailJob.iterReady())))
         notify(events.TipChanged(db_branch, tree.branch, True))
         self.assertEqual(1, len(list(RevisionMailJob.iterReady())))
@@ -164,7 +166,8 @@
             db_branch.registrant,
             BranchSubscriptionNotificationLevel.FULL,
             BranchSubscriptionDiffSize.WHOLEDIFF,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL,
+            db_branch.registrant)
         self.assertEqual(0, len(list(RevisionMailJob.iterReady())))
         notify(events.RevisionsRemoved(db_branch, tree.branch, ['x']))
         self.assertEqual(1, len(list(RevisionMailJob.iterReady())))
 
=== modified file 'lib/lp/codehosting/tests/test_jobs.py'
--- lib/lp/codehosting/tests/test_jobs.py	2010-04-23 05:49:08 +0000
+++ lib/lp/codehosting/tests/test_jobs.py	2010-05-29 09:01:05 +0000
@@ -30,7 +30,7 @@
         branch.subscribe(branch.registrant,
             BranchSubscriptionNotificationLevel.FULL,
             BranchSubscriptionDiffSize.WHOLEDIFF,
-            CodeReviewNotificationLevel.FULL)
+            CodeReviewNotificationLevel.FULL, branch.registrant)
         tree_transport = tree.bzrdir.root_transport
         tree_transport.put_bytes("hello.txt", "Hello World\n")
         tree.add('hello.txt')
 
=== modified file 'lib/lp/registry/browser/tests/packaging-views.txt'
--- lib/lp/registry/browser/tests/packaging-views.txt	2010-04-16 18:00:31 +0000
+++ lib/lp/registry/browser/tests/packaging-views.txt	2010-05-29 09:01:05 +0000
@@ -350,5 +350,5 @@
     cnews
     libstdc++
     linux-source-2.6.15
+    hot
     thunderbird
-    hot
 
=== modified file 'lib/lp/registry/browser/tests/private-team-creation-views.txt'
--- lib/lp/registry/browser/tests/private-team-creation-views.txt	2009-08-03 21:46:09 +0000
+++ lib/lp/registry/browser/tests/private-team-creation-views.txt	2010-05-29 09:01:05 +0000
@@ -411,7 +411,7 @@
     ...         team,
     ...         BranchSubscriptionNotificationLevel.DIFFSONLY,
     ...         BranchSubscriptionDiffSize.WHOLEDIFF,
-    ...         CodeReviewNotificationLevel.STATUS)
+    ...         CodeReviewNotificationLevel.STATUS, team_owner)
     ...     # A branch visibility rule.
     ...     from lp.code.enums import BranchVisibilityRule
     ...     from lp.registry.interfaces.product import IProductSet
 
=== modified file 'lib/lp/registry/doc/private-team-roles.txt'
--- lib/lp/registry/doc/private-team-roles.txt	2010-04-12 08:04:31 +0000
+++ lib/lp/registry/doc/private-team-roles.txt	2010-05-29 09:01:05 +0000
@@ -130,7 +130,7 @@
     ...     priv_team,
     ...     BranchSubscriptionNotificationLevel.DIFFSONLY,
     ...     BranchSubscriptionDiffSize.WHOLEDIFF,
-    ...     CodeReviewNotificationLevel.STATUS)
+    ...     CodeReviewNotificationLevel.STATUS, team_owner)
     >>> print subscription.person.name
     private-team
 
@@ -141,7 +141,7 @@
     ...     pm_team,
     ...     BranchSubscriptionNotificationLevel.DIFFSONLY,
     ...     BranchSubscriptionDiffSize.WHOLEDIFF,
-    ...     CodeReviewNotificationLevel.STATUS)
+    ...     CodeReviewNotificationLevel.STATUS, team_owner)
     Traceback (most recent call last):
     ...
     PrivatePersonLinkageError: Cannot link person
 
=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py	2010-05-19 15:39:52 +0000
+++ lib/lp/registry/model/distroseries.py	2010-05-29 09:01:05 +0000
@@ -332,7 +332,7 @@
         origin = SQL(joins)
         condition = SQL(conditions + "AND packaging.id IS NULL")
         results = IStore(self).using(origin).find(find_spec, condition)
-        results = results.order_by('score DESC')
+        results = results.order_by('score DESC', SourcePackageName.name)
         return [{
                  'package': SourcePackage(
                     sourcepackagename=spn, distroseries=self),
 
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-05-28 23:06:25 +0000
+++ lib/lp/testing/factory.py	2010-05-29 09:01:05 +0000
@@ -988,7 +988,8 @@
 
         return proposal
 
-    def makeBranchSubscription(self, branch=None, person=None):
+    def makeBranchSubscription(self, branch=None, person=None,
+                               subscribed_by=None):
         """Create a BranchSubscription.
 
         :param branch_title: The title to use for the created Branch
@@ -998,9 +999,11 @@
             branch = self.makeBranch()
         if person is None:
             person = self.makePerson()
+        if subscribed_by is None:
+            subscribed_by = person
         return branch.subscribe(person,
             BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.NOEMAIL)
+            CodeReviewNotificationLevel.NOEMAIL, subscribed_by)
 
     def makeDiff(self, diff_text=DIFF):
         return Diff.fromFile(StringIO(diff_text), len(diff_text))