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

Proposed by Tim Penhey on 2010-05-25
Status: Merged
Approved by: Curtis Hovey on 2010-05-28
Approved revision: no longer in the source branch.
Merged at revision: 9417
Proposed branch: lp:~thumper/launchpad/branch-subscription-subscribed-by
Merge into: lp:launchpad/db-devel
Diff against target: 1649 lines (+395/-226)
50 files modified
database/sampledata/current-dev.sql (+4/-4)
database/sampledata/current.sql (+4/-4)
database/schema/patch-2207-60-0.sql (+18/-0)
lib/canonical/launchpad/mail/commands.py (+3/-1)
lib/canonical/launchpad/scripts/tests/test_garbo.py (+0/-2)
lib/canonical/launchpad/security.py (+1/-23)
lib/lp/app/errors.py (+18/-0)
lib/lp/bugs/interfaces/bug.py (+0/-7)
lib/lp/bugs/model/bug.py (+2/-1)
lib/lp/code/browser/branch.py (+0/-12)
lib/lp/code/browser/branchsubscription.py (+7/-24)
lib/lp/code/browser/codeimport.py (+2/-1)
lib/lp/code/configure.zcml (+1/-0)
lib/lp/code/doc/branch-merge-proposal-notifications.txt (+4/-4)
lib/lp/code/doc/branch-notifications.txt (+23/-39)
lib/lp/code/doc/branch-visibility.txt (+1/-1)
lib/lp/code/doc/branch.txt (+4/-4)
lib/lp/code/doc/codeimport.txt (+1/-1)
lib/lp/code/doc/codereviewcomment.txt (+1/-1)
lib/lp/code/interfaces/branch.py (+11/-3)
lib/lp/code/interfaces/branchsubscription.py (+12/-1)
lib/lp/code/mail/tests/test_branch.py (+1/-1)
lib/lp/code/mail/tests/test_branchmergeproposal.py (+3/-3)
lib/lp/code/mail/tests/test_codehandler.py (+1/-1)
lib/lp/code/mail/tests/test_codereviewcomment.py (+2/-2)
lib/lp/code/model/branch.py (+14/-5)
lib/lp/code/model/branchnamespace.py (+4/-2)
lib/lp/code/model/branchsubscription.py (+12/-1)
lib/lp/code/model/tests/test_branch.py (+14/-9)
lib/lp/code/model/tests/test_branchcollection.py (+9/-5)
lib/lp/code/model/tests/test_branchjob.py (+8/-4)
lib/lp/code/model/tests/test_branchmergeproposals.py (+20/-16)
lib/lp/code/model/tests/test_branchsubscription.py (+115/-0)
lib/lp/code/scripts/tests/test_scan_branches.py (+2/-1)
lib/lp/code/scripts/tests/test_sendbranchmail.py (+4/-2)
lib/lp/code/security.py (+37/-0)
lib/lp/code/stories/branches/xx-branch-edit.txt (+1/-1)
lib/lp/code/stories/branches/xx-branchmergeproposals.txt (+2/-2)
lib/lp/code/stories/branches/xx-person-branches.txt (+1/-1)
lib/lp/code/stories/branches/xx-subscribing-branches.txt (+7/-24)
lib/lp/code/stories/webservice/xx-branchsubscription.txt (+2/-0)
lib/lp/code/tests/test_branch.py (+1/-1)
lib/lp/codehosting/scanner/tests/test_bzrsync.py (+1/-1)
lib/lp/codehosting/scanner/tests/test_email.py (+6/-3)
lib/lp/codehosting/tests/test_jobs.py (+1/-1)
lib/lp/registry/browser/tests/packaging-views.txt (+1/-1)
lib/lp/registry/browser/tests/private-team-creation-views.txt (+1/-1)
lib/lp/registry/doc/private-team-roles.txt (+2/-2)
lib/lp/registry/model/distroseries.py (+1/-1)
lib/lp/testing/factory.py (+5/-2)
To merge this branch: bzr merge lp:~thumper/launchpad/branch-subscription-subscribed-by
Reviewer Review Type Date Requested Status
Stuart Bishop db 2010-05-28 Approve on 2010-05-29
Curtis Hovey (community) release-critical code 2010-05-27 Approve on 2010-05-28
Björn Tillenius db 2010-05-28 Pending
Review via email: mp+25937@code.launchpad.net

Commit message

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

Description of the change

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.

To post a comment you must log in.
Curtis Hovey (sinzui) wrote :
Download full text (3.2 KiB)

> === 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...

Read more...

review: Needs Information (release-critical)
Tim Penhey (thumper) wrote :

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) wrote :

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.

1=== modified file 'lib/canonical/launchpad/security.py'
2--- lib/canonical/launchpad/security.py 2010-05-27 04:44:39 +0000
3+++ lib/canonical/launchpad/security.py 2010-05-28 04:18:17 +0000
4@@ -24,8 +24,6 @@
5 IBranch, user_has_special_branch_access)
6 from lp.code.interfaces.branchmergeproposal import (
7 IBranchMergeProposal)
8-from lp.code.interfaces.branchsubscription import (
9- IBranchSubscription)
10 from lp.code.interfaces.sourcepackagerecipe import ISourcePackageRecipe
11 from lp.code.interfaces.sourcepackagerecipebuild import (
12 ISourcePackageRecipeBuild)
13@@ -61,8 +59,7 @@
14 from lp.services.worlddata.interfaces.language import ILanguage, ILanguageSet
15 from lp.translations.interfaces.languagepack import ILanguagePack
16 from canonical.launchpad.interfaces.launchpad import (
17- IBazaarApplication, IHasBug, IHasDrivers, ILaunchpadCelebrities,
18- IPersonRoles)
19+ IHasBug, IHasDrivers, ILaunchpadCelebrities, IPersonRoles)
20 from lp.registry.interfaces.role import IHasOwner
21 from lp.registry.interfaces.location import IPersonLocation
22 from lp.registry.interfaces.mailinglist import IMailingListSet
23@@ -1733,26 +1730,6 @@
24 self.obj.distribution).checkAuthenticated(user))
25
26
27-class BranchSubscriptionEdit(AuthorizationBase):
28- permission = 'launchpad.Edit'
29- usedfor = IBranchSubscription
30-
31- def checkAuthenticated(self, user):
32- """Is the user able to edit a branch subscription?
33-
34- Any team member can edit a branch subscription for their team.
35- Launchpad Admins can also edit any branch subscription.
36- """
37- return (user.inTeam(self.obj.person) or
38- user.inTeam(self.obj.subscribed_by) or
39- user.in_admin or
40- user.in_bazaar_experts)
41-
42-
43-class BranchSubscriptionView(BranchSubscriptionEdit):
44- permission = 'launchpad.View'
45-
46-
47 class BranchMergeProposalView(AuthorizationBase):
48 permission = 'launchpad.View'
49 usedfor = IBranchMergeProposal
50
51=== modified file 'lib/lp/code/browser/branchsubscription.py'
52--- lib/lp/code/browser/branchsubscription.py 2010-05-25 03:13:25 +0000
53+++ lib/lp/code/browser/branchsubscription.py 2010-05-28 09:31:03 +0000
54@@ -12,10 +12,9 @@
55 'BranchSubscriptionPrimaryContext',
56 ]
57
58-from zope.component import getUtility
59+
60 from zope.interface import implements
61
62-from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
63 from canonical.launchpad.webapp import (
64 action, canonical_url, LaunchpadEditFormView, LaunchpadFormView,
65 LaunchpadView)
66@@ -210,7 +209,8 @@
67 subscription = self.context.getSubscription(person)
68 if subscription is None:
69 self.context.subscribe(
70- person, notification_level, max_diff_lines, review_level, self.user)
71+ person, notification_level, max_diff_lines, review_level,
72+ self.user)
73
74 self.add_notification_message(
75 '%s has been subscribed to this branch with: '
76
77=== modified file 'lib/lp/code/configure.zcml'
78--- lib/lp/code/configure.zcml 2010-05-07 04:53:47 +0000
79+++ lib/lp/code/configure.zcml 2010-05-28 04:18:17 +0000
80@@ -11,6 +11,7 @@
81 xmlns:webservice="http://namespaces.canonical.com/webservice"
82 i18n_domain="launchpad">
83 <include package=".browser"/>
84+ <authorizations module="lp.code.security" />
85
86 <!-- Branch Merge Queue -->
87
88
89=== modified file 'lib/lp/code/interfaces/branchsubscription.py'
90--- lib/lp/code/interfaces/branchsubscription.py 2010-05-25 01:24:43 +0000
91+++ lib/lp/code/interfaces/branchsubscription.py 2010-05-28 04:18:17 +0000
92@@ -21,7 +21,8 @@
93 from lp.code.interfaces.branch import IBranch
94 from canonical.launchpad.fields import ParticipatingPersonChoice
95 from lazr.restful.declarations import (
96- export_as_webservice_entry, exported)
97+ REQUEST_USER, call_with, export_as_webservice_entry,
98+ export_read_operation, exported)
99 from lazr.restful.fields import Reference
100
101
102@@ -78,3 +79,8 @@
103 title=_('Subscribed by'), required=True,
104 vocabulary='ValidPersonOrTeam', readonly=True,
105 description=_("The person who created this subscription.")))
106+
107+ @call_with(user=REQUEST_USER)
108+ @export_read_operation()
109+ def canBeUnsubscribedByUser(user):
110+ """Can the user unsubscribe the subscriber from the branch?"""
111
112=== modified file 'lib/lp/code/model/branch.py'
113--- lib/lp/code/model/branch.py 2010-05-25 04:37:11 +0000
114+++ lib/lp/code/model/branch.py 2010-05-28 09:04:16 +0000
115@@ -45,6 +45,7 @@
116 from canonical.launchpad.webapp.interfaces import (
117 IStoreSelector, MAIN_STORE, SLAVE_FLAVOR)
118
119+from lp.app.errors import UserCannotUnsubscribePerson
120 from lp.code.bzr import (
121 BranchFormat, ControlFormat, CURRENT_BRANCH_FORMATS,
122 CURRENT_REPOSITORY_FORMATS, RepositoryFormat)
123@@ -705,9 +706,16 @@
124 def unsubscribe(self, person, unsubscribed_by):
125 """See `IBranch`."""
126 subscription = self.getSubscription(person)
127+ if subscription is None:
128+ # Silent success seems order of the day (like bugs).
129+ return
130+ if not subscription.canBeUnsubscribedByUser(unsubscribed_by):
131+ raise UserCannotUnsubscribePerson(
132+ '%s does not have permission to unsubscribe %s.' % (
133+ unsubscribed_by.displayname,
134+ person.displayname))
135 store = Store.of(subscription)
136- assert subscription is not None, "User is not subscribed."
137- BranchSubscription.delete(subscription.id)
138+ store.remove(subscription)
139 store.flush()
140
141 def getBranchRevision(self, sequence=None, revision=None,
142
143=== modified file 'lib/lp/code/model/branchsubscription.py'
144--- lib/lp/code/model/branchsubscription.py 2010-05-25 04:37:11 +0000
145+++ lib/lp/code/model/branchsubscription.py 2010-05-28 04:18:17 +0000
146@@ -13,13 +13,14 @@
147 from canonical.database.constants import DEFAULT
148 from canonical.database.sqlbase import SQLBase
149 from canonical.database.enumcol import EnumCol
150-
151+from canonical.launchpad.interfaces.launchpad import IPersonRoles
152 from lp.code.enums import (
153 BranchSubscriptionDiffSize, BranchSubscriptionNotificationLevel,
154 CodeReviewNotificationLevel)
155 from lp.code.interfaces.branchsubscription import IBranchSubscription
156 from lp.code.interfaces.branch import IBranchNavigationMenu
157 from lp.code.interfaces.branchtarget import IHasBranchTarget
158+from lp.code.security import BranchSubscriptionEdit
159 from lp.registry.interfaces.person import (
160 validate_person_not_private_membership)
161
162@@ -49,3 +50,10 @@
163 def target(self):
164 """See `IHasBranchTarget`."""
165 return self.branch.target
166+
167+ def canBeUnsubscribedByUser(self, user):
168+ """See `IBranchSubscription`."""
169+ if user is None:
170+ return False
171+ permission_check = BranchSubscriptionEdit(self)
172+ return permission_check.checkAuthenticated(IPersonRoles(user))
173
174=== added file 'lib/lp/code/model/tests/test_branchsubscription.py'
175--- lib/lp/code/model/tests/test_branchsubscription.py 1970-01-01 00:00:00 +0000
176+++ lib/lp/code/model/tests/test_branchsubscription.py 2010-05-28 09:04:16 +0000
177@@ -0,0 +1,115 @@
178+# Copyright 2010 Canonical Ltd. This software is licensed under the
179+# GNU Affero General Public License version 3 (see the file LICENSE).
180+
181+"""Tests for the BranchSubscrptions model object.."""
182+
183+from __future__ import with_statement
184+
185+__metaclass__ = type
186+
187+import unittest
188+
189+from canonical.testing.layers import DatabaseFunctionalLayer
190+from lp.app.errors import UserCannotUnsubscribePerson
191+from lp.code.enums import (
192+ BranchSubscriptionNotificationLevel, CodeReviewNotificationLevel)
193+from lp.testing import TestCaseWithFactory, person_logged_in
194+
195+
196+class TestBranchSubscriptions(TestCaseWithFactory):
197+ """Tests relating to branch subscriptions in general."""
198+
199+ layer = DatabaseFunctionalLayer
200+
201+ def test_subscribed_by_set(self):
202+ """The user subscribing is recorded along the subscriber."""
203+ subscriber = self.factory.makePerson()
204+ subscribed_by = self.factory.makePerson()
205+ branch = self.factory.makeAnyBranch()
206+ subscription = branch.subscribe(
207+ subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
208+ CodeReviewNotificationLevel.NOEMAIL, subscribed_by)
209+ self.assertEqual(subscriber, subscription.person)
210+ self.assertEqual(subscribed_by, subscription.subscribed_by)
211+
212+ def test_unsubscribe(self):
213+ """Test unsubscribing by the subscriber."""
214+ subscription = self.factory.makeBranchSubscription()
215+ subscriber = subscription.person
216+ branch = subscription.branch
217+ branch.unsubscribe(subscriber, subscriber)
218+ self.assertFalse(branch.hasSubscription(subscriber))
219+
220+ def test_unsubscribe_by_subscriber(self):
221+ """Test unsubscribing by the person who subscribed the user."""
222+ subscribed_by = self.factory.makePerson()
223+ subscription = self.factory.makeBranchSubscription(
224+ subscribed_by=subscribed_by)
225+ subscriber = subscription.person
226+ branch = subscription.branch
227+ branch.unsubscribe(subscriber, subscribed_by)
228+ self.assertFalse(branch.hasSubscription(subscriber))
229+
230+ def test_unsubscribe_by_unauthorized(self):
231+ """Test unsubscribing someone you shouldn't be able to."""
232+ subscription = self.factory.makeBranchSubscription()
233+ branch = subscription.branch
234+ self.assertRaises(
235+ UserCannotUnsubscribePerson,
236+ branch.unsubscribe,
237+ subscription.person,
238+ self.factory.makePerson())
239+
240+
241+class TestBranchSubscriptionCanBeUnsubscribedbyUser(TestCaseWithFactory):
242+ """Tests for BranchSubscription.canBeUnsubscribedByUser."""
243+
244+ layer = DatabaseFunctionalLayer
245+
246+ def test_none(self):
247+ """None for a user always returns False."""
248+ subscription = self.factory.makeBranchSubscription()
249+ self.assertFalse(subscription.canBeUnsubscribedByUser(None))
250+
251+ def test_self_subscriber(self):
252+ """The subscriber has permission to unsubscribe."""
253+ subscription = self.factory.makeBranchSubscription()
254+ self.assertTrue(
255+ subscription.canBeUnsubscribedByUser(subscription.person))
256+
257+ def test_non_subscriber_fails(self):
258+ """An unrelated person can't unsubscribe a user."""
259+ subscription = self.factory.makeBranchSubscription()
260+ editor = self.factory.makePerson()
261+ self.assertFalse(subscription.canBeUnsubscribedByUser(editor))
262+
263+ def test_subscribed_by(self):
264+ """If a user subscribes someone else, the user can unsubscribe."""
265+ subscribed_by = self.factory.makePerson()
266+ subscriber = self.factory.makePerson()
267+ subscription = self.factory.makeBranchSubscription(
268+ person=subscriber, subscribed_by=subscribed_by)
269+ self.assertTrue(subscription.canBeUnsubscribedByUser(subscribed_by))
270+
271+ def test_team_member_can_unsubscribe(self):
272+ """Any team member can unsubscribe the team from a branch."""
273+ team = self.factory.makeTeam()
274+ member = self.factory.makePerson()
275+ with person_logged_in(team.teamowner):
276+ team.addMember(member, team.teamowner)
277+ subscription = self.factory.makeBranchSubscription(
278+ person=team, subscribed_by=team.teamowner)
279+ self.assertTrue(subscription.canBeUnsubscribedByUser(member))
280+
281+ def test_team_subscriber_can_unsubscribe(self):
282+ """A team can be unsubscribed by the subscriber even if they are not a
283+ member."""
284+ team = self.factory.makeTeam()
285+ subscribed_by = self.factory.makePerson()
286+ subscription = self.factory.makeBranchSubscription(
287+ person=team, subscribed_by=subscribed_by)
288+ self.assertTrue(subscription.canBeUnsubscribedByUser(subscribed_by))
289+
290+
291+def test_suite():
292+ return unittest.TestLoader().loadTestsFromName(__name__)
293
294=== added file 'lib/lp/code/security.py'
295--- lib/lp/code/security.py 1970-01-01 00:00:00 +0000
296+++ lib/lp/code/security.py 2010-05-28 04:18:17 +0000
297@@ -0,0 +1,37 @@
298+# Copyright 2010 Canonical Ltd. This software is licensed under the
299+# GNU Affero General Public License version 3 (see the file LICENSE).
300+
301+"""Security adapters for the code module."""
302+
303+__metaclass__ = type
304+__all__ = [
305+ 'BranchSubscriptionEdit',
306+ 'BranchSubscriptionView',
307+ ]
308+
309+from canonical.launchpad.security import AuthorizationBase
310+
311+from lp.code.interfaces.branchsubscription import IBranchSubscription
312+
313+
314+
315+class BranchSubscriptionEdit(AuthorizationBase):
316+ permission = 'launchpad.Edit'
317+ usedfor = IBranchSubscription
318+
319+ def checkAuthenticated(self, user):
320+ """Is the user able to edit a branch subscription?
321+
322+ Any team member can edit a branch subscription for their team.
323+ Launchpad Admins can also edit any branch subscription.
324+ """
325+ return (user.inTeam(self.obj.person) or
326+ user.inTeam(self.obj.subscribed_by) or
327+ user.in_admin or
328+ user.in_bazaar_experts)
329+
330+
331+class BranchSubscriptionView(BranchSubscriptionEdit):
332+ permission = 'launchpad.View'
333+
334+
335
336=== modified file 'lib/lp/testing/factory.py'
337--- lib/lp/testing/factory.py 2010-05-25 01:24:43 +0000
338+++ lib/lp/testing/factory.py 2010-05-28 09:31:03 +0000
339@@ -986,7 +986,8 @@
340
341 return proposal
342
343- def makeBranchSubscription(self, branch=None, person=None, subscribed_by=None):
344+ def makeBranchSubscription(self, branch=None, person=None,
345+ subscribed_by=None):
346 """Create a BranchSubscription.
347
348 :param branch_title: The title to use for the created Branch
Tim Penhey (thumper) wrote :

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) wrote :

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

review: Approve (release-critical code)
Stuart Bishop (stub) wrote :

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

review: Approve (db)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'database/sampledata/current-dev.sql'
2--- database/sampledata/current-dev.sql 2010-05-27 22:18:16 +0000
3+++ database/sampledata/current-dev.sql 2010-05-29 09:01:05 +0000
4@@ -3131,10 +3131,10 @@
5
6 ALTER TABLE branchsubscription DISABLE TRIGGER ALL;
7
8-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);
9-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);
10-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);
11-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);
12+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);
13+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);
14+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);
15+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);
16
17
18 ALTER TABLE branchsubscription ENABLE TRIGGER ALL;
19
20=== modified file 'database/sampledata/current.sql'
21--- database/sampledata/current.sql 2010-05-27 22:18:16 +0000
22+++ database/sampledata/current.sql 2010-05-29 09:01:05 +0000
23@@ -3089,10 +3089,10 @@
24
25 ALTER TABLE branchsubscription DISABLE TRIGGER ALL;
26
27-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);
28-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);
29-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);
30-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);
31+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);
32+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);
33+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);
34+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);
35
36
37 ALTER TABLE branchsubscription ENABLE TRIGGER ALL;
38
39=== added file 'database/schema/patch-2207-60-0.sql'
40--- database/schema/patch-2207-60-0.sql 1970-01-01 00:00:00 +0000
41+++ database/schema/patch-2207-60-0.sql 2010-05-29 09:01:05 +0000
42@@ -0,0 +1,18 @@
43+-- Copyright 2010 Canonical Ltd. This software is licensed under the
44+-- GNU Affero General Public License version 3 (see the file LICENSE).
45+
46+SET client_min_messages=ERROR;
47+
48+ALTER TABLE BranchSubscription
49+ ADD COLUMN subscribed_by integer REFERENCES Person;
50+
51+UPDATE BranchSubscription
52+SET subscribed_by = person;
53+
54+ALTER TABLE BranchSubscription ALTER COLUMN subscribed_by SET NOT NULL;
55+
56+-- Index needed for person merging.
57+CREATE INDEX branchsubscription__subscribed_by__idx
58+ ON BranchSubscription(subscribed_by);
59+
60+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 60, 0);
61
62=== modified file 'lib/canonical/launchpad/mail/commands.py'
63--- lib/canonical/launchpad/mail/commands.py 2010-04-16 15:06:55 +0000
64+++ lib/canonical/launchpad/mail/commands.py 2010-05-29 09:01:05 +0000
65@@ -23,7 +23,7 @@
66 IDistributionSourcePackage, EmailProcessingError,
67 NotFoundError, CreateBugParams, IPillarNameSet,
68 BugTargetNotFound, IProjectGroup, ISourcePackage, IProductSeries,
69- BugTaskStatus, UserCannotUnsubscribePerson)
70+ BugTaskStatus)
71 from lazr.lifecycle.event import (
72 ObjectModifiedEvent, ObjectCreatedEvent)
73 from lazr.lifecycle.interfaces import (
74@@ -34,6 +34,8 @@
75 from canonical.launchpad.validators.name import valid_name
76 from canonical.launchpad.webapp.authorization import check_permission
77
78+from lp.app.errors import UserCannotUnsubscribePerson
79+
80
81 def normalize_arguments(string_args):
82 """Normalizes the string arguments.
83
84=== modified file 'lib/canonical/launchpad/scripts/tests/test_garbo.py'
85--- lib/canonical/launchpad/scripts/tests/test_garbo.py 2010-04-23 08:33:14 +0000
86+++ lib/canonical/launchpad/scripts/tests/test_garbo.py 2010-05-29 09:01:05 +0000
87@@ -19,7 +19,6 @@
88
89 from canonical.config import config
90 from canonical.database.constants import THIRTY_DAYS_AGO, UTC_NOW
91-from canonical.launchpad.database.emailaddress import EmailAddress
92 from canonical.launchpad.database.message import Message
93 from canonical.launchpad.database.oauth import OAuthNonce
94 from canonical.launchpad.database.openidconsumer import OpenIDConsumerNonce
95@@ -42,7 +41,6 @@
96 from lp.code.model.branchjob import BranchJob, BranchUpgradeJob
97 from lp.code.model.codeimportresult import CodeImportResult
98 from lp.registry.interfaces.person import IPersonSet, PersonCreationRationale
99-from lp.registry.model.person import Person
100 from lp.services.job.model.job import Job
101
102
103
104=== modified file 'lib/canonical/launchpad/security.py'
105--- lib/canonical/launchpad/security.py 2010-05-26 08:54:27 +0000
106+++ lib/canonical/launchpad/security.py 2010-05-29 09:01:05 +0000
107@@ -24,8 +24,6 @@
108 IBranch, user_has_special_branch_access)
109 from lp.code.interfaces.branchmergeproposal import (
110 IBranchMergeProposal)
111-from lp.code.interfaces.branchsubscription import (
112- IBranchSubscription)
113 from lp.code.interfaces.sourcepackagerecipe import ISourcePackageRecipe
114 from lp.code.interfaces.sourcepackagerecipebuild import (
115 ISourcePackageRecipeBuild)
116@@ -63,8 +61,7 @@
117 from lp.services.worlddata.interfaces.language import ILanguage, ILanguageSet
118 from lp.translations.interfaces.languagepack import ILanguagePack
119 from canonical.launchpad.interfaces.launchpad import (
120- IBazaarApplication, IHasBug, IHasDrivers, ILaunchpadCelebrities,
121- IPersonRoles)
122+ IHasBug, IHasDrivers, ILaunchpadCelebrities, IPersonRoles)
123 from lp.registry.interfaces.role import IHasOwner
124 from lp.registry.interfaces.location import IPersonLocation
125 from lp.registry.interfaces.mailinglist import IMailingListSet
126@@ -1749,25 +1746,6 @@
127 self.obj.distribution).checkAuthenticated(user))
128
129
130-class BranchSubscriptionEdit(AuthorizationBase):
131- permission = 'launchpad.Edit'
132- usedfor = IBranchSubscription
133-
134- def checkAuthenticated(self, user):
135- """Is the user able to edit a branch subscription?
136-
137- Any team member can edit a branch subscription for their team.
138- Launchpad Admins can also edit any branch subscription.
139- """
140- return (user.inTeam(self.obj.person) or
141- user.in_admin or
142- user.in_bazaar_experts)
143-
144-
145-class BranchSubscriptionView(BranchSubscriptionEdit):
146- permission = 'launchpad.View'
147-
148-
149 class BranchMergeProposalView(AuthorizationBase):
150 permission = 'launchpad.View'
151 usedfor = IBranchMergeProposal
152
153=== added file 'lib/lp/app/errors.py'
154--- lib/lp/app/errors.py 1970-01-01 00:00:00 +0000
155+++ lib/lp/app/errors.py 2010-05-29 09:01:05 +0000
156@@ -0,0 +1,18 @@
157+# Copyright 2010 Canonical Ltd. This software is licensed under the
158+# GNU Affero General Public License version 3 (see the file LICENSE).
159+
160+"""Cross application type errors for launchpad."""
161+
162+__metaclass__ = type
163+__all__ = [
164+ 'UserCannotUnsubscribePerson',
165+ ]
166+
167+from zope.security.interfaces import Unauthorized
168+
169+from lazr.restful.declarations import webservice_error
170+
171+
172+class UserCannotUnsubscribePerson(Unauthorized):
173+ """User does not have persmisson to unsubscribe person or team."""
174+ webservice_error(401)
175
176=== modified file 'lib/lp/bugs/interfaces/bug.py'
177--- lib/lp/bugs/interfaces/bug.py 2010-04-29 17:49:19 +0000
178+++ lib/lp/bugs/interfaces/bug.py 2010-05-29 09:01:05 +0000
179@@ -20,7 +20,6 @@
180 'IProjectGroupBugAddForm',
181 'InvalidBugTargetType',
182 'InvalidDuplicateValue',
183- 'UserCannotUnsubscribePerson',
184 ]
185
186 from zope.component import getUtility
187@@ -28,7 +27,6 @@
188 from zope.schema import (
189 Bool, Bytes, Choice, Datetime, Int, List, Object, Text, TextLine)
190 from zope.schema.vocabulary import SimpleVocabulary
191-from zope.security.interfaces import Unauthorized
192
193 from canonical.launchpad import _
194 from canonical.launchpad.fields import (
195@@ -805,11 +803,6 @@
196 webservice_error(417)
197
198
199-class UserCannotUnsubscribePerson(Unauthorized):
200- """User does not have persmisson to unsubscribe person or team."""
201- webservice_error(401)
202-
203-
204 # We are forced to define these now to avoid circular import problems.
205 IBugAttachment['bug'].schema = IBug
206 IBugWatch['bug'].schema = IBug
207
208=== modified file 'lib/lp/bugs/model/bug.py'
209--- lib/lp/bugs/model/bug.py 2010-05-25 15:48:47 +0000
210+++ lib/lp/bugs/model/bug.py 2010-05-29 09:01:05 +0000
211@@ -64,12 +64,13 @@
212 IStoreSelector, DEFAULT_FLAVOR, MAIN_STORE, NotFoundError)
213
214 from lp.answers.interfaces.questiontarget import IQuestionTarget
215+from lp.app.errors import UserCannotUnsubscribePerson
216 from lp.bugs.adapters.bugchange import (
217 BranchLinkedToBug, BranchUnlinkedFromBug, BugConvertedToQuestion,
218 BugWatchAdded, BugWatchRemoved, SeriesNominated, UnsubscribedFromBug)
219 from lp.bugs.interfaces.bug import (
220 IBug, IBugBecameQuestionEvent, IBugSet, IFileBugData,
221- InvalidDuplicateValue, UserCannotUnsubscribePerson)
222+ InvalidDuplicateValue)
223 from lp.bugs.interfaces.bugactivity import IBugActivitySet
224 from lp.bugs.interfaces.bugattachment import (
225 BugAttachmentType, IBugAttachmentSet)
226
227=== modified file 'lib/lp/code/browser/branch.py'
228--- lib/lp/code/browser/branch.py 2010-05-27 01:46:06 +0000
229+++ lib/lp/code/browser/branch.py 2010-05-29 09:01:05 +0000
230@@ -356,18 +356,6 @@
231
232 def initialize(self):
233 self.notices = []
234- self._add_subscription_notice()
235-
236- def _add_subscription_notice(self):
237- """Add the appropriate notice after posting the subscription form."""
238- if self.user and self.request.method == 'POST':
239- newsub = self.request.form.get('subscribe', None)
240- if newsub == 'Subscribe':
241- self.context.subscribe(self.user)
242- self.notices.append("You have subscribed to this branch.")
243- elif newsub == 'Unsubscribe':
244- self.context.unsubscribe(self.user)
245- self.notices.append("You have unsubscribed from this branch.")
246
247 def user_is_subscribed(self):
248 """Is the current user subscribed to this branch?"""
249
250=== modified file 'lib/lp/code/browser/branchsubscription.py'
251--- lib/lp/code/browser/branchsubscription.py 2010-04-20 01:21:10 +0000
252+++ lib/lp/code/browser/branchsubscription.py 2010-05-29 09:01:05 +0000
253@@ -12,10 +12,9 @@
254 'BranchSubscriptionPrimaryContext',
255 ]
256
257-from zope.component import getUtility
258+
259 from zope.interface import implements
260
261-from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
262 from canonical.launchpad.webapp import (
263 action, canonical_url, LaunchpadEditFormView, LaunchpadFormView,
264 LaunchpadView)
265@@ -118,7 +117,8 @@
266 review_level = data['review_level']
267
268 self.context.subscribe(
269- self.user, notification_level, max_diff_lines, review_level)
270+ self.user, notification_level, max_diff_lines, review_level,
271+ self.user)
272
273 self.add_notification_message(
274 'You have subscribed to this branch with: ',
275@@ -171,7 +171,7 @@
276 def unsubscribe(self, action, data):
277 # Be proactive in the checking to catch the stale post problem.
278 if self.context.hasSubscription(self.user):
279- self.context.unsubscribe(self.user)
280+ self.context.unsubscribe(self.user, self.user)
281 self.request.response.addNotification(
282 "You have unsubscribed from this branch.")
283 else:
284@@ -209,7 +209,8 @@
285 subscription = self.context.getSubscription(person)
286 if subscription is None:
287 self.context.subscribe(
288- person, notification_level, max_diff_lines, review_level)
289+ person, notification_level, max_diff_lines, review_level,
290+ self.user)
291
292 self.add_notification_message(
293 '%s has been subscribed to this branch with: '
294@@ -222,24 +223,6 @@
295 subscription.notification_level, subscription.max_diff_lines,
296 review_level)
297
298- def validate(self, data):
299- """Ensure that when a team is subscribed, the user is a member."""
300- celebs = getUtility(ILaunchpadCelebrities)
301- # An admin or bzr expert can subscribe anyone.
302- if self.user.inTeam(celebs.admin) or (
303- self.user.inTeam(celebs.bazaar_experts)):
304- return
305-
306- person = data.get('person')
307- if (person is not None and
308- person.isTeam() and
309- not self.user.inTeam(person)):
310- # A person can only subscribe a team if they are members
311- # of that team.
312- self.setFieldError(
313- 'person',
314- "You can only subscribe teams that you are a member of.")
315-
316
317 class BranchSubscriptionEditView(LaunchpadEditFormView):
318 """The view for editing branch subscriptions.
319@@ -273,7 +256,7 @@
320 @action("Unsubscribe", name="unsubscribe")
321 def unsubscribe_action(self, action, data):
322 """Unsubscribe the team from the branch."""
323- self.branch.unsubscribe(self.person)
324+ self.branch.unsubscribe(self.person, self.user)
325 self.request.response.addNotification(
326 "%s has been unsubscribed from this branch."
327 % self.person.displayname)
328
329=== modified file 'lib/lp/code/browser/codeimport.py'
330--- lib/lp/code/browser/codeimport.py 2010-04-28 02:49:58 +0000
331+++ lib/lp/code/browser/codeimport.py 2010-05-29 09:01:05 +0000
332@@ -392,7 +392,8 @@
333 self.user,
334 BranchSubscriptionNotificationLevel.FULL,
335 BranchSubscriptionDiffSize.NODIFF,
336- CodeReviewNotificationLevel.NOEMAIL)
337+ CodeReviewNotificationLevel.NOEMAIL,
338+ self.user)
339
340 self.next_url = canonical_url(code_import.branch)
341
342
343=== modified file 'lib/lp/code/configure.zcml'
344--- lib/lp/code/configure.zcml 2010-05-19 15:39:52 +0000
345+++ lib/lp/code/configure.zcml 2010-05-29 09:01:05 +0000
346@@ -11,6 +11,7 @@
347 xmlns:webservice="http://namespaces.canonical.com/webservice"
348 i18n_domain="launchpad">
349 <include package=".browser"/>
350+ <authorizations module="lp.code.security" />
351
352 <!-- Branch Merge Queue -->
353
354
355=== modified file 'lib/lp/code/doc/branch-merge-proposal-notifications.txt'
356--- lib/lp/code/doc/branch-merge-proposal-notifications.txt 2010-04-06 01:21:33 +0000
357+++ lib/lp/code/doc/branch-merge-proposal-notifications.txt 2010-05-29 09:01:05 +0000
358@@ -30,13 +30,13 @@
359 >>> _unused = bmp.source_branch.subscribe(source_subscriber,
360 ... BranchSubscriptionNotificationLevel.NOEMAIL,
361 ... BranchSubscriptionDiffSize.NODIFF,
362- ... CodeReviewNotificationLevel.STATUS)
363+ ... CodeReviewNotificationLevel.STATUS, source_subscriber)
364 >>> target_subscriber = factory.makePerson(
365 ... email='target@example.com', displayname='Target Subscriber')
366 >>> target_subscription = bmp.target_branch.subscribe(target_subscriber,
367 ... BranchSubscriptionNotificationLevel.NOEMAIL,
368 ... BranchSubscriptionDiffSize.NODIFF,
369- ... CodeReviewNotificationLevel.FULL)
370+ ... CodeReviewNotificationLevel.FULL, target_subscriber)
371
372 The owners of the branches are subscribed when the branches are created.
373
374@@ -69,8 +69,8 @@
375
376 Now we will unsubscribe the branch owners to simplify the rest of the test.
377
378- >>> bmp.source_branch.unsubscribe(source_owner)
379- >>> bmp.target_branch.unsubscribe(target_owner)
380+ >>> bmp.source_branch.unsubscribe(source_owner, source_owner)
381+ >>> bmp.target_branch.unsubscribe(target_owner, target_owner)
382 >>> recipients = bmp.getNotificationRecipients(
383 ... CodeReviewNotificationLevel.FULL)
384
385
386=== modified file 'lib/lp/code/doc/branch-notifications.txt'
387--- lib/lp/code/doc/branch-notifications.txt 2010-03-10 05:49:00 +0000
388+++ lib/lp/code/doc/branch-notifications.txt 2010-05-29 09:01:05 +0000
389@@ -40,7 +40,7 @@
390 ... branch.owner,
391 ... BranchSubscriptionNotificationLevel.FULL,
392 ... BranchSubscriptionDiffSize.WHOLEDIFF,
393- ... CodeReviewNotificationLevel.NOEMAIL)
394+ ... CodeReviewNotificationLevel.NOEMAIL, branch.owner)
395 >>> BranchMailer.forRevision(branch, 1, 'foo@canonical.com',
396 ... 'The contents.', None, 'Subject line').sendAll()
397
398@@ -71,7 +71,7 @@
399 You are subscribed to branch lp://dev/~name12/firefox/main.
400 To unsubscribe from this branch go to http://code.launchpad.dev/~name12/firefox/main/+edit-subscription
401 <BLANKLINE>
402- >>> branch.unsubscribe(branch.owner)
403+ >>> branch.unsubscribe(branch.owner, branch.owner)
404
405
406 == Subscriptions ==
407@@ -98,63 +98,55 @@
408 >>> from lp.code.interfaces.branch import IBranch, IBranchSet
409 >>> personset = getUtility(IPersonSet)
410
411- >>> branch.subscribe(
412- ... personset.getByEmail('no-priv@canonical.com'),
413+ >>> def subscribe_user_by_email(branch, email, level, size, level2):
414+ ... subscriber = personset.getByEmail(email)
415+ ... branch.subscribe(subscriber, level, size, level2, subscriber)
416+
417+ >>> subscribe_user_by_email(branch, 'no-priv@canonical.com',
418 ... BranchSubscriptionNotificationLevel.NOEMAIL,
419 ... BranchSubscriptionDiffSize.NODIFF,
420 ... CodeReviewNotificationLevel.NOEMAIL)
421- <BranchSubscription ...
422
423- >>> branch.subscribe(
424- ... personset.getByEmail('test@canonical.com'),
425+ >>> subscribe_user_by_email(branch, 'test@canonical.com',
426 ... BranchSubscriptionNotificationLevel.ATTRIBUTEONLY,
427 ... BranchSubscriptionDiffSize.NODIFF,
428 ... CodeReviewNotificationLevel.NOEMAIL)
429- <BranchSubscription ...
430
431- >>> branch.subscribe(
432- ... personset.getByEmail('carlos@canonical.com'),
433+ >>> subscribe_user_by_email(branch, 'carlos@canonical.com',
434 ... BranchSubscriptionNotificationLevel.DIFFSONLY,
435 ... BranchSubscriptionDiffSize.NODIFF,
436 ... CodeReviewNotificationLevel.NOEMAIL)
437- <BranchSubscription ...
438
439- >>> branch.subscribe(
440- ... personset.getByEmail('jeff.waugh@ubuntulinux.com'),
441+ >>> subscribe_user_by_email(branch, 'jeff.waugh@ubuntulinux.com',
442 ... BranchSubscriptionNotificationLevel.DIFFSONLY,
443 ... BranchSubscriptionDiffSize.HALFKLINES,
444 ... CodeReviewNotificationLevel.NOEMAIL)
445- <BranchSubscription ...
446
447- >>> branch.subscribe(
448- ... personset.getByEmail('celso.providelo@canonical.com'),
449+ >>> subscribe_user_by_email(branch, 'celso.providelo@canonical.com',
450 ... BranchSubscriptionNotificationLevel.DIFFSONLY,
451 ... BranchSubscriptionDiffSize.ONEKLINES,
452 ... CodeReviewNotificationLevel.NOEMAIL)
453- <BranchSubscription ...
454
455- >>> branch.subscribe(
456- ... personset.getByEmail('daf@canonical.com'),
457+ >>> subscribe_user_by_email(branch, 'daf@canonical.com',
458 ... BranchSubscriptionNotificationLevel.DIFFSONLY,
459 ... BranchSubscriptionDiffSize.FIVEKLINES,
460 ... CodeReviewNotificationLevel.NOEMAIL)
461- <BranchSubscription ...
462
463- >>> branch.subscribe(
464- ... personset.getByEmail('mark@example.com'),
465+ >>> subscribe_user_by_email(branch, 'mark@example.com',
466 ... BranchSubscriptionNotificationLevel.FULL,
467 ... BranchSubscriptionDiffSize.WHOLEDIFF,
468 ... CodeReviewNotificationLevel.NOEMAIL)
469- <BranchSubscription ...
470
471 Team are subscribed in the same way.
472
473- >>> branch.subscribe(
474- ... personset.getByName('launchpad'),
475+ >>> def subscribe_team_by_name(branch, name, level, size, level2):
476+ ... team = personset.getByName(name)
477+ ... branch.subscribe(team, level, size, level2, team.teamowner)
478+
479+ >>> subscribe_team_by_name(branch, 'launchpad',
480 ... BranchSubscriptionNotificationLevel.FULL,
481 ... BranchSubscriptionDiffSize.WHOLEDIFF,
482 ... CodeReviewNotificationLevel.NOEMAIL)
483- <BranchSubscription ...
484
485 And to make sure we have them:
486
487@@ -358,7 +350,7 @@
488 Unsubscribe everybody.
489
490 >>> for subscription in branch.subscriptions:
491- ... branch.unsubscribe(subscription.person)
492+ ... branch.unsubscribe(subscription.person, subscription.person)
493 >>> len(list(branch.subscriptions))
494 0
495
496@@ -373,29 +365,23 @@
497 If a team is registered, and that team has an email address assigned,
498 then that email address is used for the notifications.
499
500- >>> branch.subscribe(
501- ... personset.getByEmail('david.allouche@canonical.com'),
502+ >>> subscribe_user_by_email(branch, 'david.allouche@canonical.com',
503 ... BranchSubscriptionNotificationLevel.DIFFSONLY,
504 ... BranchSubscriptionDiffSize.HALFKLINES,
505 ... CodeReviewNotificationLevel.NOEMAIL)
506- <BranchSubscription ...
507
508- >>> branch.subscribe(
509- ... personset.getByName('vcs-imports'),
510+ >>> subscribe_team_by_name(branch, 'vcs-imports',
511 ... BranchSubscriptionNotificationLevel.DIFFSONLY,
512 ... BranchSubscriptionDiffSize.FIVEKLINES,
513 ... CodeReviewNotificationLevel.NOEMAIL)
514- <BranchSubscription ...
515
516 The ubuntu-team has an email address supplied (support@ubuntu.com), so
517 that is used rather than the email addresses of the seven members.
518
519- >>> branch.subscribe(
520- ... personset.getByName('ubuntu-team'),
521+ >>> subscribe_team_by_name(branch, 'ubuntu-team',
522 ... BranchSubscriptionNotificationLevel.DIFFSONLY,
523 ... BranchSubscriptionDiffSize.ONEKLINES,
524 ... CodeReviewNotificationLevel.NOEMAIL)
525- <BranchSubscription ...
526
527 >>> recipients = branch.getNotificationRecipients()
528 >>> for email in recipients.getEmails():
529@@ -427,12 +413,10 @@
530
531 Resubscribe our test user.
532
533- >>> branch.subscribe(
534- ... personset.getByEmail('test@canonical.com'),
535+ >>> subscribe_user_by_email(branch, 'test@canonical.com',
536 ... BranchSubscriptionNotificationLevel.ATTRIBUTEONLY,
537 ... BranchSubscriptionDiffSize.NODIFF,
538 ... CodeReviewNotificationLevel.NOEMAIL)
539- <BranchSubscription ...
540
541 >>> from zope.event import notify
542 >>> from lazr.lifecycle.event import ObjectModifiedEvent
543
544=== modified file 'lib/lp/code/doc/branch-visibility.txt'
545--- lib/lp/code/doc/branch-visibility.txt 2010-01-12 14:52:41 +0000
546+++ lib/lp/code/doc/branch-visibility.txt 2010-05-29 09:01:05 +0000
547@@ -138,7 +138,7 @@
548 >>> branch.subscribe(ubuntu_team,
549 ... BranchSubscriptionNotificationLevel.NOEMAIL,
550 ... BranchSubscriptionDiffSize.NODIFF,
551- ... CodeReviewNotificationLevel.NOEMAIL)
552+ ... CodeReviewNotificationLevel.NOEMAIL, ubuntu_team.teamowner)
553 <BranchSubscription ...>
554
555 >>> access.checkAccountAuthenticated(jdub.account)
556
557=== modified file 'lib/lp/code/doc/branch.txt'
558--- lib/lp/code/doc/branch.txt 2010-01-12 21:37:34 +0000
559+++ lib/lp/code/doc/branch.txt 2010-05-29 09:01:05 +0000
560@@ -310,7 +310,7 @@
561 ... subscriber,
562 ... BranchSubscriptionNotificationLevel.FULL,
563 ... BranchSubscriptionDiffSize.FIVEKLINES,
564- ... CodeReviewNotificationLevel.FULL)
565+ ... CodeReviewNotificationLevel.FULL, subscriber)
566 >>> verifyObject(IBranchSubscription, subscription)
567 True
568 >>> subscription.branch == branch and subscription.person == subscriber
569@@ -338,7 +338,7 @@
570 ... subscriber,
571 ... BranchSubscriptionNotificationLevel.FULL,
572 ... BranchSubscriptionDiffSize.FIVEKLINES,
573- ... CodeReviewNotificationLevel.NOEMAIL)
574+ ... CodeReviewNotificationLevel.NOEMAIL, subscriber)
575 >>> subscription == subscription2
576 True
577 >>> subscription2.review_level == CodeReviewNotificationLevel.NOEMAIL
578@@ -346,7 +346,7 @@
579
580 Unsubscribing is also supported.
581
582- >>> branch.unsubscribe(subscriber)
583+ >>> branch.unsubscribe(subscriber, subscriber)
584 >>> branch.subscribers.count()
585 1
586
587@@ -365,7 +365,7 @@
588 ... subscriber,
589 ... BranchSubscriptionNotificationLevel.FULL,
590 ... BranchSubscriptionDiffSize.FIVEKLINES,
591- ... CodeReviewNotificationLevel.NOEMAIL)
592+ ... CodeReviewNotificationLevel.NOEMAIL, subscriber)
593
594 >>> print_names(branch2.getSubscriptionsByLevel([
595 ... BranchSubscriptionNotificationLevel.FULL]))
596
597=== modified file 'lib/lp/code/doc/codeimport.txt'
598--- lib/lp/code/doc/codeimport.txt 2010-03-18 22:18:49 +0000
599+++ lib/lp/code/doc/codeimport.txt 2010-05-29 09:01:05 +0000
600@@ -258,7 +258,7 @@
601 ... nopriv,
602 ... BranchSubscriptionNotificationLevel.FULL,
603 ... BranchSubscriptionDiffSize.NODIFF,
604- ... CodeReviewNotificationLevel.FULL)
605+ ... CodeReviewNotificationLevel.FULL, nopriv)
606
607 >>> from lp.testing.mail_helpers import (
608 ... pop_notifications, print_emails)
609
610=== modified file 'lib/lp/code/doc/codereviewcomment.txt'
611--- lib/lp/code/doc/codereviewcomment.txt 2010-04-06 01:13:28 +0000
612+++ lib/lp/code/doc/codereviewcomment.txt 2010-05-29 09:01:05 +0000
613@@ -65,7 +65,7 @@
614 >>> _unused = merge_proposal.source_branch.subscribe(source_subscriber,
615 ... BranchSubscriptionNotificationLevel.NOEMAIL,
616 ... BranchSubscriptionDiffSize.NODIFF,
617- ... CodeReviewNotificationLevel.FULL)
618+ ... CodeReviewNotificationLevel.FULL, source_subscriber)
619 >>> from lp.testing.mail_helpers import (
620 ... pop_notifications, print_emails)
621 >>> _unused = pop_notifications()
622
623=== modified file 'lib/lp/code/interfaces/branch.py'
624--- lib/lp/code/interfaces/branch.py 2010-04-26 00:24:24 +0000
625+++ lib/lp/code/interfaces/branch.py 2010-05-29 09:01:05 +0000
626@@ -965,9 +965,10 @@
627 title=_("The level of code review notification emails."),
628 vocabulary=CodeReviewNotificationLevel))
629 @operation_returns_entry(Interface) # Really IBranchSubscription
630+ @call_with(subscribed_by=REQUEST_USER)
631 @export_write_operation()
632 def subscribe(person, notification_level, max_diff_lines,
633- code_review_level):
634+ code_review_level, subscribed_by):
635 """Subscribe this person to the branch.
636
637 :param person: The `Person` to subscribe.
638@@ -977,6 +978,8 @@
639 appear in a notification.
640 :param code_review_level: The kinds of code review activity that cause
641 notification.
642+ :param subscribed_by: The person who is subscribing the subscriber.
643+ Most often the subscriber themselves.
644 :return: new or existing BranchSubscription."""
645
646 @operation_parameters(
647@@ -995,9 +998,14 @@
648 person=Reference(
649 title=_("The person to unsubscribe"),
650 schema=IPerson))
651+ @call_with(unsubscribed_by=REQUEST_USER)
652 @export_write_operation()
653- def unsubscribe(person):
654- """Remove the person's subscription to this branch."""
655+ def unsubscribe(person, unsubscribed_by):
656+ """Remove the person's subscription to this branch.
657+
658+ :param person: The person or team to unsubscribe from the branch.
659+ :param unsubscribed_by: The person doing the unsubscribing.
660+ """
661
662 def getSubscriptionsByLevel(notification_levels):
663 """Return the subscriptions that are at the given notification levels.
664
665=== modified file 'lib/lp/code/interfaces/branchsubscription.py'
666--- lib/lp/code/interfaces/branchsubscription.py 2009-06-25 04:06:00 +0000
667+++ lib/lp/code/interfaces/branchsubscription.py 2010-05-29 09:01:05 +0000
668@@ -21,7 +21,8 @@
669 from lp.code.interfaces.branch import IBranch
670 from canonical.launchpad.fields import ParticipatingPersonChoice
671 from lazr.restful.declarations import (
672- export_as_webservice_entry, exported)
673+ REQUEST_USER, call_with, export_as_webservice_entry,
674+ export_read_operation, exported)
675 from lazr.restful.fields import Reference
676
677
678@@ -73,3 +74,13 @@
679 'Control the kind of review activity that triggers '
680 'notifications.'
681 )))
682+
683+ subscribed_by = exported(ParticipatingPersonChoice(
684+ title=_('Subscribed by'), required=True,
685+ vocabulary='ValidPersonOrTeam', readonly=True,
686+ description=_("The person who created this subscription.")))
687+
688+ @call_with(user=REQUEST_USER)
689+ @export_read_operation()
690+ def canBeUnsubscribedByUser(user):
691+ """Can the user unsubscribe the subscriber from the branch?"""
692
693=== modified file 'lib/lp/code/mail/tests/test_branch.py'
694--- lib/lp/code/mail/tests/test_branch.py 2010-04-26 00:22:16 +0000
695+++ lib/lp/code/mail/tests/test_branch.py 2010-05-29 09:01:05 +0000
696@@ -35,7 +35,7 @@
697 source_branch.owner, target_branch)
698 subscription = merge_proposal.source_branch.subscribe(
699 subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
700- CodeReviewNotificationLevel.FULL)
701+ CodeReviewNotificationLevel.FULL, subscriber)
702 return merge_proposal, subscription
703
704 def test_forBranchSubscriber(self):
705
706=== modified file 'lib/lp/code/mail/tests/test_branchmergeproposal.py'
707--- lib/lp/code/mail/tests/test_branchmergeproposal.py 2010-05-18 13:14:40 +0000
708+++ lib/lp/code/mail/tests/test_branchmergeproposal.py 2010-05-29 09:01:05 +0000
709@@ -67,7 +67,7 @@
710 email='baz.quxx@example.com')
711 bmp.source_branch.subscribe(subscriber,
712 BranchSubscriptionNotificationLevel.NOEMAIL, None,
713- CodeReviewNotificationLevel.FULL)
714+ CodeReviewNotificationLevel.FULL, subscriber)
715 bmp.source_branch.owner.name = 'bob'
716 bmp.source_branch.name = 'fix-foo-for-bar'
717 bmp.target_branch.owner.name = 'mary'
718@@ -176,7 +176,7 @@
719 bmp = request.merge_proposal
720 bmp.source_branch.subscribe(
721 bmp.registrant, BranchSubscriptionNotificationLevel.NOEMAIL, None,
722- CodeReviewNotificationLevel.FULL)
723+ CodeReviewNotificationLevel.FULL, bmp.registrant)
724 mailer = BMPMailer.forCreation(bmp, bmp.registrant)
725 ctrl = mailer.generateEmail(bmp.registrant,
726 bmp.registrant.preferredemail.email)
727@@ -259,7 +259,7 @@
728 diff_text=diff_text)
729 bmp.source_branch.subscribe(subscriber,
730 BranchSubscriptionNotificationLevel.NOEMAIL, None,
731- CodeReviewNotificationLevel.STATUS)
732+ CodeReviewNotificationLevel.STATUS, subscriber)
733 mailer = BMPMailer.forCreation(bmp, bmp.registrant)
734 ctrl = mailer.generateEmail('baz.quxx@example.com', subscriber)
735 self.assertEqual(0, len(ctrl.attachments))
736
737=== modified file 'lib/lp/code/mail/tests/test_codehandler.py'
738--- lib/lp/code/mail/tests/test_codehandler.py 2010-04-29 07:47:47 +0000
739+++ lib/lp/code/mail/tests/test_codehandler.py 2010-05-29 09:01:05 +0000
740@@ -346,7 +346,7 @@
741 subscriber = self.factory.makePerson()
742 bmp.source_branch.subscribe(
743 subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
744- CodeReviewNotificationLevel.FULL)
745+ CodeReviewNotificationLevel.FULL, subscriber)
746 email_addr = bmp.address
747 self.switchDbUser(config.processmail.dbuser)
748 self.code_handler.process(mail, email_addr, None)
749
750=== modified file 'lib/lp/code/mail/tests/test_codereviewcomment.py'
751--- lib/lp/code/mail/tests/test_codereviewcomment.py 2010-05-07 17:16:44 +0000
752+++ lib/lp/code/mail/tests/test_codereviewcomment.py 2010-05-29 09:01:05 +0000
753@@ -48,7 +48,7 @@
754 notification_level = CodeReviewNotificationLevel.FULL
755 comment.branch_merge_proposal.source_branch.subscribe(
756 subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
757- notification_level)
758+ notification_level, subscriber)
759 # Email is not sent on construction, so fake a root message id on the
760 # merge proposal.
761 login_person(comment.branch_merge_proposal.registrant)
762@@ -265,7 +265,7 @@
763 email='commenter@email.com', displayname='Commenter')
764 bmp.source_branch.subscribe(commenter,
765 BranchSubscriptionNotificationLevel.NOEMAIL, None,
766- CodeReviewNotificationLevel.FULL)
767+ CodeReviewNotificationLevel.FULL, commenter)
768 comment = bmp.createComment(commenter, 'hello')
769 return comment
770
771
772=== modified file 'lib/lp/code/model/branch.py'
773--- lib/lp/code/model/branch.py 2010-05-15 17:43:59 +0000
774+++ lib/lp/code/model/branch.py 2010-05-29 09:01:05 +0000
775@@ -45,6 +45,7 @@
776 from canonical.launchpad.webapp.interfaces import (
777 IStoreSelector, MAIN_STORE, SLAVE_FLAVOR)
778
779+from lp.app.errors import UserCannotUnsubscribePerson
780 from lp.code.bzr import (
781 BranchFormat, ControlFormat, CURRENT_BRANCH_FORMATS,
782 CURRENT_REPOSITORY_FORMATS, RepositoryFormat)
783@@ -659,7 +660,7 @@
784
785 # subscriptions
786 def subscribe(self, person, notification_level, max_diff_lines,
787- code_review_level):
788+ code_review_level, subscribed_by):
789 """See `IBranch`."""
790 # If the person is already subscribed, update the subscription with
791 # the specified notification details.
792@@ -668,7 +669,8 @@
793 subscription = BranchSubscription(
794 branch=self, person=person,
795 notification_level=notification_level,
796- max_diff_lines=max_diff_lines, review_level=code_review_level)
797+ max_diff_lines=max_diff_lines, review_level=code_review_level,
798+ subscribed_by=subscribed_by)
799 Store.of(subscription).flush()
800 else:
801 subscription.notification_level = notification_level
802@@ -701,12 +703,19 @@
803 """See `IBranch`."""
804 return self.getSubscription(person) is not None
805
806- def unsubscribe(self, person):
807+ def unsubscribe(self, person, unsubscribed_by):
808 """See `IBranch`."""
809 subscription = self.getSubscription(person)
810+ if subscription is None:
811+ # Silent success seems order of the day (like bugs).
812+ return
813+ if not subscription.canBeUnsubscribedByUser(unsubscribed_by):
814+ raise UserCannotUnsubscribePerson(
815+ '%s does not have permission to unsubscribe %s.' % (
816+ unsubscribed_by.displayname,
817+ person.displayname))
818 store = Store.of(subscription)
819- assert subscription is not None, "User is not subscribed."
820- BranchSubscription.delete(subscription.id)
821+ store.remove(subscription)
822 store.flush()
823
824 def getBranchRevision(self, sequence=None, revision=None,
825
826=== modified file 'lib/lp/code/model/branchnamespace.py'
827--- lib/lp/code/model/branchnamespace.py 2010-02-17 11:19:42 +0000
828+++ lib/lp/code/model/branchnamespace.py 2010-05-29 09:01:05 +0000
829@@ -106,7 +106,8 @@
830 implicit_subscription,
831 BranchSubscriptionNotificationLevel.NOEMAIL,
832 BranchSubscriptionDiffSize.NODIFF,
833- CodeReviewNotificationLevel.NOEMAIL)
834+ CodeReviewNotificationLevel.NOEMAIL,
835+ registrant)
836
837 # The registrant of the branch should also be automatically subscribed
838 # in order for them to get code review notifications. The implicit
839@@ -116,7 +117,8 @@
840 registrant,
841 BranchSubscriptionNotificationLevel.NOEMAIL,
842 BranchSubscriptionDiffSize.NODIFF,
843- CodeReviewNotificationLevel.FULL)
844+ CodeReviewNotificationLevel.FULL,
845+ registrant)
846
847 notify(ObjectCreatedEvent(branch))
848 return branch
849
850=== modified file 'lib/lp/code/model/branchsubscription.py'
851--- lib/lp/code/model/branchsubscription.py 2009-06-25 04:06:00 +0000
852+++ lib/lp/code/model/branchsubscription.py 2010-05-29 09:01:05 +0000
853@@ -13,13 +13,14 @@
854 from canonical.database.constants import DEFAULT
855 from canonical.database.sqlbase import SQLBase
856 from canonical.database.enumcol import EnumCol
857-
858+from canonical.launchpad.interfaces.launchpad import IPersonRoles
859 from lp.code.enums import (
860 BranchSubscriptionDiffSize, BranchSubscriptionNotificationLevel,
861 CodeReviewNotificationLevel)
862 from lp.code.interfaces.branchsubscription import IBranchSubscription
863 from lp.code.interfaces.branch import IBranchNavigationMenu
864 from lp.code.interfaces.branchtarget import IHasBranchTarget
865+from lp.code.security import BranchSubscriptionEdit
866 from lp.registry.interfaces.person import (
867 validate_person_not_private_membership)
868
869@@ -41,8 +42,18 @@
870 notNull=False, default=DEFAULT)
871 review_level = EnumCol(enum=CodeReviewNotificationLevel,
872 notNull=True, default=DEFAULT)
873+ subscribed_by = ForeignKey(
874+ dbName='subscribed_by', foreignKey='Person',
875+ storm_validator=validate_person_not_private_membership, notNull=True)
876
877 @property
878 def target(self):
879 """See `IHasBranchTarget`."""
880 return self.branch.target
881+
882+ def canBeUnsubscribedByUser(self, user):
883+ """See `IBranchSubscription`."""
884+ if user is None:
885+ return False
886+ permission_check = BranchSubscriptionEdit(self)
887+ return permission_check.checkAuthenticated(IPersonRoles(user))
888
889=== modified file 'lib/lp/code/model/tests/test_branch.py'
890--- lib/lp/code/model/tests/test_branch.py 2010-04-23 04:11:12 +0000
891+++ lib/lp/code/model/tests/test_branch.py 2010-05-29 09:01:05 +0000
892@@ -908,7 +908,7 @@
893 # The owner of the branch is subscribed to the branch when it is
894 # created. The tests here assume no initial connections, so
895 # unsubscribe the branch owner here.
896- self.branch.unsubscribe(self.branch.owner)
897+ self.branch.unsubscribe(self.branch.owner, self.branch.owner)
898
899 def test_deletable(self):
900 """A newly created branch can be deleted without any problems."""
901@@ -930,7 +930,7 @@
902 """A branch that has a subscription can be deleted."""
903 self.branch.subscribe(
904 self.user, BranchSubscriptionNotificationLevel.NOEMAIL, None,
905- CodeReviewNotificationLevel.NOEMAIL)
906+ CodeReviewNotificationLevel.NOEMAIL, self.user)
907 self.assertEqual(True, self.branch.canBeDeleted())
908
909 def test_codeImportCanStillBeDeleted(self):
910@@ -1068,7 +1068,7 @@
911 # The owner of the branch is subscribed to the branch when it is
912 # created. The tests here assume no initial connections, so
913 # unsubscribe the branch owner here.
914- self.branch.unsubscribe(self.branch.owner)
915+ self.branch.unsubscribe(self.branch.owner, self.branch.owner)
916
917 def test_plainBranch(self):
918 """Ensure that a fresh branch has no deletion requirements."""
919@@ -1081,8 +1081,9 @@
920 prerequisite_branch = self.factory.makeProductBranch(
921 product=self.branch.product)
922 # Remove the implicit subscriptions.
923- target_branch.unsubscribe(target_branch.owner)
924- prerequisite_branch.unsubscribe(prerequisite_branch.owner)
925+ target_branch.unsubscribe(target_branch.owner, target_branch.owner)
926+ prerequisite_branch.unsubscribe(
927+ prerequisite_branch.owner, prerequisite_branch.owner)
928 merge_proposal1 = self.branch.addLandingTarget(
929 self.branch.owner, target_branch, prerequisite_branch)
930 # Disable this merge proposal, to allow creating a new identical one
931@@ -1261,7 +1262,8 @@
932 """Deletion requirements for a code import branch are right"""
933 code_import = self.factory.makeCodeImport()
934 # Remove the implicit branch subscription first.
935- code_import.branch.unsubscribe(code_import.branch.owner)
936+ code_import.branch.unsubscribe(
937+ code_import.branch.owner, code_import.branch.owner)
938 self.assertEqual({}, code_import.branch.deletionRequirements())
939
940 def test_branchWithCodeImportDeletion(self):
941@@ -2303,7 +2305,8 @@
942 # Revisions created before the start date are not returned.
943 branch = self.factory.makeAnyBranch()
944 epoch = datetime(2009, 9, 10, tzinfo=UTC)
945- old = add_revision_to_branch(
946+ # Add some revisions before the epoch.
947+ add_revision_to_branch(
948 self.factory, branch, epoch - timedelta(days=1))
949 new = add_revision_to_branch(
950 self.factory, branch, epoch + timedelta(days=1))
951@@ -2318,7 +2321,8 @@
952 end_date = epoch + timedelta(days=2)
953 in_range = add_revision_to_branch(
954 self.factory, branch, end_date - timedelta(days=1))
955- too_new = add_revision_to_branch(
956+ # Add some revisions after the end_date.
957+ add_revision_to_branch(
958 self.factory, branch, end_date + timedelta(days=1))
959 result = branch.getMainlineBranchRevisions(epoch, end_date)
960 branch_revisions = [br for br, rev, ra in result]
961@@ -2354,7 +2358,8 @@
962 epoch = datetime(2009, 9, 10, tzinfo=UTC)
963 old = add_revision_to_branch(
964 self.factory, branch, epoch + timedelta(days=1))
965- merged = add_revision_to_branch(
966+ # Add some non mainline revision.
967+ add_revision_to_branch(
968 self.factory, branch, epoch + timedelta(days=2), mainline=False)
969 new = add_revision_to_branch(
970 self.factory, branch, epoch + timedelta(days=3))
971
972=== modified file 'lib/lp/code/model/tests/test_branchcollection.py'
973--- lib/lp/code/model/tests/test_branchcollection.py 2009-10-28 04:33:49 +0000
974+++ lib/lp/code/model/tests/test_branchcollection.py 2010-05-29 09:01:05 +0000
975@@ -364,7 +364,8 @@
976 branch.subscribe(
977 subscriber, BranchSubscriptionNotificationLevel.NOEMAIL,
978 BranchSubscriptionDiffSize.NODIFF,
979- CodeReviewNotificationLevel.NOEMAIL)
980+ CodeReviewNotificationLevel.NOEMAIL,
981+ subscriber)
982 collection = self.all_branches.subscribedBy(subscriber)
983 self.assertEqual([branch], list(collection.getBranches()))
984
985@@ -377,7 +378,7 @@
986 owned_branch = self.factory.makeAnyBranch(owner=person)
987 # Unsubscribe the owner, to demonstrate that we show owned branches
988 # even if they aren't subscribed.
989- owned_branch.unsubscribe(person)
990+ owned_branch.unsubscribe(person, person)
991 # Subscribe two other people to the owned branch to make sure
992 # that the BranchSubscription join is doing it right.
993 self.factory.makeBranchSubscription(branch=owned_branch)
994@@ -389,7 +390,8 @@
995 subscribed_branch.subscribe(
996 person, BranchSubscriptionNotificationLevel.NOEMAIL,
997 BranchSubscriptionDiffSize.NODIFF,
998- CodeReviewNotificationLevel.NOEMAIL)
999+ CodeReviewNotificationLevel.NOEMAIL,
1000+ person)
1001 related_branches = self.all_branches.relatedTo(person)
1002 self.assertEqual(
1003 sorted([owned_branch, registered_branch, subscribed_branch]),
1004@@ -548,7 +550,8 @@
1005 removeSecurityProxy(self.private_branch1).subscribe(
1006 subscriber, BranchSubscriptionNotificationLevel.NOEMAIL,
1007 BranchSubscriptionDiffSize.NODIFF,
1008- CodeReviewNotificationLevel.NOEMAIL)
1009+ CodeReviewNotificationLevel.NOEMAIL,
1010+ subscriber)
1011 branches = self.all_branches.visibleByUser(subscriber)
1012 self.assertEqual(
1013 sorted([self.public_branch, self.private_branch1]),
1014@@ -564,7 +567,8 @@
1015 removeSecurityProxy(private_branch).subscribe(
1016 team, BranchSubscriptionNotificationLevel.NOEMAIL,
1017 BranchSubscriptionDiffSize.NODIFF,
1018- CodeReviewNotificationLevel.NOEMAIL)
1019+ CodeReviewNotificationLevel.NOEMAIL,
1020+ team_owner)
1021 # Members of the team can see the private branch that the team is
1022 # subscribed to.
1023 branches = self.all_branches.visibleByUser(team_owner)
1024
1025=== modified file 'lib/lp/code/model/tests/test_branchjob.py'
1026--- lib/lp/code/model/tests/test_branchjob.py 2010-05-25 05:19:30 +0000
1027+++ lib/lp/code/model/tests/test_branchjob.py 2010-05-29 09:01:05 +0000
1028@@ -322,10 +322,12 @@
1029 def test_run_sends_mail(self):
1030 """Ensure RevisionMailJob.run sends mail with correct values."""
1031 branch = self.factory.makeAnyBranch()
1032- branch.subscribe(branch.registrant,
1033+ branch.subscribe(
1034+ branch.registrant,
1035 BranchSubscriptionNotificationLevel.FULL,
1036 BranchSubscriptionDiffSize.WHOLEDIFF,
1037- CodeReviewNotificationLevel.FULL)
1038+ CodeReviewNotificationLevel.FULL,
1039+ branch.registrant)
1040 job = RevisionMailJob.create(
1041 branch, 0, 'from@example.com', 'hello', False, 'subject')
1042 job.run()
1043@@ -515,10 +517,12 @@
1044 product = self.factory.makeProduct(name='foo')
1045 branch = self.factory.makeProductBranch(
1046 name='bar', product=product, owner=jrandom)
1047- branch.subscribe(branch.registrant,
1048+ branch.subscribe(
1049+ branch.registrant,
1050 BranchSubscriptionNotificationLevel.FULL,
1051 BranchSubscriptionDiffSize.WHOLEDIFF,
1052- CodeReviewNotificationLevel.FULL)
1053+ CodeReviewNotificationLevel.FULL,
1054+ branch.registrant)
1055 branch, tree = self.create_branch_and_tree(db_branch=branch)
1056 tree.branch.nick = 'nicholas'
1057 tree.lock_write()
1058
1059=== modified file 'lib/lp/code/model/tests/test_branchmergeproposals.py'
1060--- lib/lp/code/model/tests/test_branchmergeproposals.py 2010-05-07 04:53:47 +0000
1061+++ lib/lp/code/model/tests/test_branchmergeproposals.py 2010-05-29 09:01:05 +0000
1062@@ -722,16 +722,20 @@
1063 subscriber_set = set([source_owner, target_owner])
1064 self.assertEqual(subscriber_set, set(recipients.keys()))
1065 source_subscriber = self.factory.makePerson()
1066- bmp.source_branch.subscribe(source_subscriber,
1067+ bmp.source_branch.subscribe(
1068+ source_subscriber,
1069 BranchSubscriptionNotificationLevel.NOEMAIL, None,
1070- CodeReviewNotificationLevel.FULL)
1071+ CodeReviewNotificationLevel.FULL,
1072+ source_subscriber)
1073 recipients = bmp.getNotificationRecipients(
1074 CodeReviewNotificationLevel.STATUS)
1075 subscriber_set.add(source_subscriber)
1076 self.assertEqual(subscriber_set, set(recipients.keys()))
1077- bmp.source_branch.subscribe(source_subscriber,
1078+ bmp.source_branch.subscribe(
1079+ source_subscriber,
1080 BranchSubscriptionNotificationLevel.NOEMAIL, None,
1081- CodeReviewNotificationLevel.NOEMAIL)
1082+ CodeReviewNotificationLevel.NOEMAIL,
1083+ source_subscriber)
1084 # By specifying no email, they will no longer get email.
1085 subscriber_set.remove(source_subscriber)
1086 recipients = bmp.getNotificationRecipients(
1087@@ -744,11 +748,11 @@
1088 full_subscriber = self.factory.makePerson()
1089 bmp.source_branch.subscribe(full_subscriber,
1090 BranchSubscriptionNotificationLevel.NOEMAIL, None,
1091- CodeReviewNotificationLevel.FULL)
1092+ CodeReviewNotificationLevel.FULL, full_subscriber)
1093 status_subscriber = self.factory.makePerson()
1094 bmp.source_branch.subscribe(status_subscriber,
1095 BranchSubscriptionNotificationLevel.NOEMAIL, None,
1096- CodeReviewNotificationLevel.STATUS)
1097+ CodeReviewNotificationLevel.STATUS, status_subscriber)
1098 recipients = bmp.getNotificationRecipients(
1099 CodeReviewNotificationLevel.STATUS)
1100 # Both of the branch owners are now subscribed to their own
1101@@ -778,15 +782,15 @@
1102 source_subscriber = self.factory.makePerson()
1103 bmp.source_branch.subscribe(source_subscriber,
1104 BranchSubscriptionNotificationLevel.NOEMAIL, None,
1105- CodeReviewNotificationLevel.FULL)
1106+ CodeReviewNotificationLevel.FULL, source_subscriber)
1107 target_subscriber = self.factory.makePerson()
1108 bmp.target_branch.subscribe(target_subscriber,
1109 BranchSubscriptionNotificationLevel.NOEMAIL, None,
1110- CodeReviewNotificationLevel.FULL)
1111+ CodeReviewNotificationLevel.FULL, target_subscriber)
1112 prerequisite_subscriber = self.factory.makePerson()
1113 bmp.prerequisite_branch.subscribe(prerequisite_subscriber,
1114 BranchSubscriptionNotificationLevel.NOEMAIL, None,
1115- CodeReviewNotificationLevel.FULL)
1116+ CodeReviewNotificationLevel.FULL, prerequisite_subscriber)
1117 recipients = bmp.getNotificationRecipients(
1118 CodeReviewNotificationLevel.FULL)
1119 self.assertEqual(
1120@@ -832,7 +836,7 @@
1121 # Make sure that the registrant is subscribed.
1122 bmp.source_branch.subscribe(registrant,
1123 BranchSubscriptionNotificationLevel.NOEMAIL, None,
1124- CodeReviewNotificationLevel.FULL)
1125+ CodeReviewNotificationLevel.FULL, registrant)
1126 recipients = bmp.getNotificationRecipients(
1127 CodeReviewNotificationLevel.STATUS)
1128 reason = recipients[registrant]
1129@@ -879,7 +883,7 @@
1130 # we don't send them eamil.
1131 bmp = self.factory.makeBranchMergeProposal()
1132 owner = bmp.source_branch.owner
1133- bmp.source_branch.unsubscribe(owner)
1134+ bmp.source_branch.unsubscribe(owner, owner)
1135 recipients = bmp.getNotificationRecipients(
1136 CodeReviewNotificationLevel.STATUS)
1137 self.assertFalse(owner in recipients)
1138@@ -892,20 +896,20 @@
1139 eric = self.factory.makePerson()
1140 bmp.source_branch.subscribe(
1141 eric, BranchSubscriptionNotificationLevel.NOEMAIL, None,
1142- CodeReviewNotificationLevel.FULL)
1143+ CodeReviewNotificationLevel.FULL, eric)
1144 # Subscribe bob to the target branch only.
1145 bob = self.factory.makePerson()
1146 bmp.target_branch.subscribe(
1147 bob, BranchSubscriptionNotificationLevel.NOEMAIL, None,
1148- CodeReviewNotificationLevel.FULL)
1149+ CodeReviewNotificationLevel.FULL, bob)
1150 # Subscribe charlie to both.
1151 charlie = self.factory.makePerson()
1152 bmp.source_branch.subscribe(
1153 charlie, BranchSubscriptionNotificationLevel.NOEMAIL, None,
1154- CodeReviewNotificationLevel.FULL)
1155+ CodeReviewNotificationLevel.FULL, charlie)
1156 bmp.target_branch.subscribe(
1157 charlie, BranchSubscriptionNotificationLevel.NOEMAIL, None,
1158- CodeReviewNotificationLevel.FULL)
1159+ CodeReviewNotificationLevel.FULL, charlie)
1160 # Make both branches private.
1161 removeSecurityProxy(bmp.source_branch).private = True
1162 removeSecurityProxy(bmp.target_branch).private = True
1163@@ -1157,7 +1161,7 @@
1164 proposal.source_branch.subscribe(
1165 subscriber,
1166 BranchSubscriptionNotificationLevel.NOEMAIL, None,
1167- CodeReviewNotificationLevel.NOEMAIL)
1168+ CodeReviewNotificationLevel.NOEMAIL, subscriber)
1169 self.assertEqual(
1170 ['~albert/mike/work', '~albert/november/work'],
1171 self._get_merge_proposals(albert, visible_by_user=subscriber))
1172
1173=== added file 'lib/lp/code/model/tests/test_branchsubscription.py'
1174--- lib/lp/code/model/tests/test_branchsubscription.py 1970-01-01 00:00:00 +0000
1175+++ lib/lp/code/model/tests/test_branchsubscription.py 2010-05-29 09:01:05 +0000
1176@@ -0,0 +1,115 @@
1177+# Copyright 2010 Canonical Ltd. This software is licensed under the
1178+# GNU Affero General Public License version 3 (see the file LICENSE).
1179+
1180+"""Tests for the BranchSubscrptions model object.."""
1181+
1182+from __future__ import with_statement
1183+
1184+__metaclass__ = type
1185+
1186+import unittest
1187+
1188+from canonical.testing.layers import DatabaseFunctionalLayer
1189+from lp.app.errors import UserCannotUnsubscribePerson
1190+from lp.code.enums import (
1191+ BranchSubscriptionNotificationLevel, CodeReviewNotificationLevel)
1192+from lp.testing import TestCaseWithFactory, person_logged_in
1193+
1194+
1195+class TestBranchSubscriptions(TestCaseWithFactory):
1196+ """Tests relating to branch subscriptions in general."""
1197+
1198+ layer = DatabaseFunctionalLayer
1199+
1200+ def test_subscribed_by_set(self):
1201+ """The user subscribing is recorded along the subscriber."""
1202+ subscriber = self.factory.makePerson()
1203+ subscribed_by = self.factory.makePerson()
1204+ branch = self.factory.makeAnyBranch()
1205+ subscription = branch.subscribe(
1206+ subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
1207+ CodeReviewNotificationLevel.NOEMAIL, subscribed_by)
1208+ self.assertEqual(subscriber, subscription.person)
1209+ self.assertEqual(subscribed_by, subscription.subscribed_by)
1210+
1211+ def test_unsubscribe(self):
1212+ """Test unsubscribing by the subscriber."""
1213+ subscription = self.factory.makeBranchSubscription()
1214+ subscriber = subscription.person
1215+ branch = subscription.branch
1216+ branch.unsubscribe(subscriber, subscriber)
1217+ self.assertFalse(branch.hasSubscription(subscriber))
1218+
1219+ def test_unsubscribe_by_subscriber(self):
1220+ """Test unsubscribing by the person who subscribed the user."""
1221+ subscribed_by = self.factory.makePerson()
1222+ subscription = self.factory.makeBranchSubscription(
1223+ subscribed_by=subscribed_by)
1224+ subscriber = subscription.person
1225+ branch = subscription.branch
1226+ branch.unsubscribe(subscriber, subscribed_by)
1227+ self.assertFalse(branch.hasSubscription(subscriber))
1228+
1229+ def test_unsubscribe_by_unauthorized(self):
1230+ """Test unsubscribing someone you shouldn't be able to."""
1231+ subscription = self.factory.makeBranchSubscription()
1232+ branch = subscription.branch
1233+ self.assertRaises(
1234+ UserCannotUnsubscribePerson,
1235+ branch.unsubscribe,
1236+ subscription.person,
1237+ self.factory.makePerson())
1238+
1239+
1240+class TestBranchSubscriptionCanBeUnsubscribedbyUser(TestCaseWithFactory):
1241+ """Tests for BranchSubscription.canBeUnsubscribedByUser."""
1242+
1243+ layer = DatabaseFunctionalLayer
1244+
1245+ def test_none(self):
1246+ """None for a user always returns False."""
1247+ subscription = self.factory.makeBranchSubscription()
1248+ self.assertFalse(subscription.canBeUnsubscribedByUser(None))
1249+
1250+ def test_self_subscriber(self):
1251+ """The subscriber has permission to unsubscribe."""
1252+ subscription = self.factory.makeBranchSubscription()
1253+ self.assertTrue(
1254+ subscription.canBeUnsubscribedByUser(subscription.person))
1255+
1256+ def test_non_subscriber_fails(self):
1257+ """An unrelated person can't unsubscribe a user."""
1258+ subscription = self.factory.makeBranchSubscription()
1259+ editor = self.factory.makePerson()
1260+ self.assertFalse(subscription.canBeUnsubscribedByUser(editor))
1261+
1262+ def test_subscribed_by(self):
1263+ """If a user subscribes someone else, the user can unsubscribe."""
1264+ subscribed_by = self.factory.makePerson()
1265+ subscriber = self.factory.makePerson()
1266+ subscription = self.factory.makeBranchSubscription(
1267+ person=subscriber, subscribed_by=subscribed_by)
1268+ self.assertTrue(subscription.canBeUnsubscribedByUser(subscribed_by))
1269+
1270+ def test_team_member_can_unsubscribe(self):
1271+ """Any team member can unsubscribe the team from a branch."""
1272+ team = self.factory.makeTeam()
1273+ member = self.factory.makePerson()
1274+ with person_logged_in(team.teamowner):
1275+ team.addMember(member, team.teamowner)
1276+ subscription = self.factory.makeBranchSubscription(
1277+ person=team, subscribed_by=team.teamowner)
1278+ self.assertTrue(subscription.canBeUnsubscribedByUser(member))
1279+
1280+ def test_team_subscriber_can_unsubscribe(self):
1281+ """A team can be unsubscribed by the subscriber even if they are not a
1282+ member."""
1283+ team = self.factory.makeTeam()
1284+ subscribed_by = self.factory.makePerson()
1285+ subscription = self.factory.makeBranchSubscription(
1286+ person=team, subscribed_by=subscribed_by)
1287+ self.assertTrue(subscription.canBeUnsubscribedByUser(subscribed_by))
1288+
1289+
1290+def test_suite():
1291+ return unittest.TestLoader().loadTestsFromName(__name__)
1292
1293=== modified file 'lib/lp/code/scripts/tests/test_scan_branches.py'
1294--- lib/lp/code/scripts/tests/test_scan_branches.py 2010-04-28 05:40:02 +0000
1295+++ lib/lp/code/scripts/tests/test_scan_branches.py 2010-05-29 09:01:05 +0000
1296@@ -52,7 +52,8 @@
1297 db_branch.registrant,
1298 BranchSubscriptionNotificationLevel.FULL,
1299 BranchSubscriptionDiffSize.WHOLEDIFF,
1300- CodeReviewNotificationLevel.FULL)
1301+ CodeReviewNotificationLevel.FULL,
1302+ db_branch.registrant)
1303 transaction.commit()
1304
1305 self.run_script_and_assert_success()
1306
1307=== modified file 'lib/lp/code/scripts/tests/test_sendbranchmail.py'
1308--- lib/lp/code/scripts/tests/test_sendbranchmail.py 2010-04-28 05:40:02 +0000
1309+++ lib/lp/code/scripts/tests/test_sendbranchmail.py 2010-05-29 09:01:05 +0000
1310@@ -24,10 +24,12 @@
1311
1312 def createBranch(self):
1313 branch, tree = self.create_branch_and_tree()
1314- branch.subscribe(branch.registrant,
1315+ branch.subscribe(
1316+ branch.registrant,
1317 BranchSubscriptionNotificationLevel.FULL,
1318 BranchSubscriptionDiffSize.WHOLEDIFF,
1319- CodeReviewNotificationLevel.FULL)
1320+ CodeReviewNotificationLevel.FULL,
1321+ branch.registrant)
1322 transport = tree.bzrdir.root_transport
1323 transport.put_bytes('foo', 'bar')
1324 tree.add('foo')
1325
1326=== added file 'lib/lp/code/security.py'
1327--- lib/lp/code/security.py 1970-01-01 00:00:00 +0000
1328+++ lib/lp/code/security.py 2010-05-29 09:01:05 +0000
1329@@ -0,0 +1,37 @@
1330+# Copyright 2010 Canonical Ltd. This software is licensed under the
1331+# GNU Affero General Public License version 3 (see the file LICENSE).
1332+
1333+"""Security adapters for the code module."""
1334+
1335+__metaclass__ = type
1336+__all__ = [
1337+ 'BranchSubscriptionEdit',
1338+ 'BranchSubscriptionView',
1339+ ]
1340+
1341+from canonical.launchpad.security import AuthorizationBase
1342+
1343+from lp.code.interfaces.branchsubscription import IBranchSubscription
1344+
1345+
1346+
1347+class BranchSubscriptionEdit(AuthorizationBase):
1348+ permission = 'launchpad.Edit'
1349+ usedfor = IBranchSubscription
1350+
1351+ def checkAuthenticated(self, user):
1352+ """Is the user able to edit a branch subscription?
1353+
1354+ Any team member can edit a branch subscription for their team.
1355+ Launchpad Admins can also edit any branch subscription.
1356+ """
1357+ return (user.inTeam(self.obj.person) or
1358+ user.inTeam(self.obj.subscribed_by) or
1359+ user.in_admin or
1360+ user.in_bazaar_experts)
1361+
1362+
1363+class BranchSubscriptionView(BranchSubscriptionEdit):
1364+ permission = 'launchpad.View'
1365+
1366+
1367
1368=== modified file 'lib/lp/code/stories/branches/xx-branch-edit.txt'
1369--- lib/lp/code/stories/branches/xx-branch-edit.txt 2010-05-24 04:27:46 +0000
1370+++ lib/lp/code/stories/branches/xx-branch-edit.txt 2010-05-29 09:01:05 +0000
1371@@ -223,7 +223,7 @@
1372 >>> _unused = foogoo_svn.subscribe(sample_person,
1373 ... BranchSubscriptionNotificationLevel.ATTRIBUTEONLY,
1374 ... BranchSubscriptionDiffSize.NODIFF,
1375- ... CodeReviewNotificationLevel.NOEMAIL)
1376+ ... CodeReviewNotificationLevel.NOEMAIL, sample_person)
1377 >>> logout()
1378
1379 >>> nopriv_browser = setupBrowser(auth='Basic no-priv@canonical.com:test')
1380
1381=== modified file 'lib/lp/code/stories/branches/xx-branchmergeproposals.txt'
1382--- lib/lp/code/stories/branches/xx-branchmergeproposals.txt 2010-04-07 16:05:38 +0000
1383+++ lib/lp/code/stories/branches/xx-branchmergeproposals.txt 2010-05-29 09:01:05 +0000
1384@@ -19,7 +19,7 @@
1385 ... '~name12/firefox/main')
1386 >>> _unused = branch.subscribe(subscriber,
1387 ... BranchSubscriptionNotificationLevel.NOEMAIL, None,
1388- ... CodeReviewNotificationLevel.FULL)
1389+ ... CodeReviewNotificationLevel.FULL, subscriber)
1390
1391 Also subscribe to gnome-terminal, since Firefox tests are mixed with
1392 Gnome Terminal tests.
1393@@ -28,7 +28,7 @@
1394 ... '~name12/gnome-terminal/main')
1395 >>> _unused = branch.subscribe(subscriber,
1396 ... BranchSubscriptionNotificationLevel.NOEMAIL, None,
1397- ... CodeReviewNotificationLevel.FULL)
1398+ ... CodeReviewNotificationLevel.FULL, subscriber)
1399 >>> from lp.code.tests.helpers import make_erics_fooix_project
1400 >>> locals().update(make_erics_fooix_project(factory))
1401 >>> logout()
1402
1403=== modified file 'lib/lp/code/stories/branches/xx-person-branches.txt'
1404--- lib/lp/code/stories/branches/xx-person-branches.txt 2010-05-13 16:22:19 +0000
1405+++ lib/lp/code/stories/branches/xx-person-branches.txt 2010-05-29 09:01:05 +0000
1406@@ -107,7 +107,7 @@
1407
1408 >>> login(ANONYMOUS)
1409 >>> b2 = factory.makeAnyBranch(owner=eric)
1410- >>> ignored = b2.unsubscribe(eric)
1411+ >>> ignored = b2.unsubscribe(eric, eric)
1412 >>> logout()
1413
1414 >>> eric_browser.open('http://code.launchpad.dev/~eric')
1415
1416=== modified file 'lib/lp/code/stories/branches/xx-subscribing-branches.txt'
1417--- lib/lp/code/stories/branches/xx-subscribing-branches.txt 2010-04-20 04:12:30 +0000
1418+++ lib/lp/code/stories/branches/xx-subscribing-branches.txt 2010-05-29 09:01:05 +0000
1419@@ -190,25 +190,8 @@
1420 >>> browser.getControl('Person').value = 'landscape-developers'
1421 >>> browser.getControl('Subscribe').click()
1422
1423-The user can only subscribe teams that they are members of.
1424-
1425- >>> for message in get_feedback_messages(browser.contents):
1426- ... print extract_text(message)
1427- There is 1 error.
1428- You can only subscribe teams that you are a member of.
1429-
1430-Sample Person is a member of Landscape Developers.
1431-
1432- >>> browser = setupBrowser(auth='Basic test@canonical.com:test')
1433- >>> browser.open(
1434- ... 'http://code.launchpad.dev/~name12/gnome-terminal/main')
1435- >>> browser.getLink('Subscribe someone else').click()
1436- >>> browser.getControl('Notification Level').displayValue = [
1437- ... 'Branch attribute and revision notifications']
1438- >>> browser.getControl('Generated Diff Size Limit').displayValue = [
1439- ... '1000 lines']
1440- >>> browser.getControl('Person').value = 'landscape-developers'
1441- >>> browser.getControl('Subscribe').click()
1442+The user does not have to be in the team to subscribe them.
1443+
1444 >>> print_informational_message(browser.contents)
1445 Landscape Developers has been subscribed to this branch with:
1446 Send notifications for both branch attribute updates
1447@@ -234,10 +217,10 @@
1448 Editing a team subscription
1449 ===========================
1450
1451-In order to edit a team subscription the logged in user needs to be
1452-a member of the team that is subscribed. There is a link shown in
1453-the subscriptions portlet to edit the subscription of a team that
1454-the logged in user is a member of.
1455+In order to edit a team subscription the logged in user needs to be a member
1456+of the team that is subscribed, or must the the person who subscribed the team
1457+to the branch. There is a link shown in the subscriptions portlet to edit the
1458+subscription of a team that the logged in user is a member of.
1459
1460 XXX: thumper 2007-06-11
1461 There should be a central user subscriptions page. This could then
1462@@ -293,7 +276,7 @@
1463 ... CodeReviewNotificationLevel)
1464 >>> ignored = branch.subscribe(
1465 ... private_team, BranchSubscriptionNotificationLevel.NOEMAIL, None,
1466- ... CodeReviewNotificationLevel.NOEMAIL)
1467+ ... CodeReviewNotificationLevel.NOEMAIL, private_team.teamowner)
1468 >>> url = canonical_url(branch)
1469 >>> logout()
1470
1471
1472=== modified file 'lib/lp/code/stories/webservice/xx-branchsubscription.txt'
1473--- lib/lp/code/stories/webservice/xx-branchsubscription.txt 2009-06-16 03:40:05 +0000
1474+++ lib/lp/code/stories/webservice/xx-branchsubscription.txt 2010-05-29 09:01:05 +0000
1475@@ -38,6 +38,7 @@
1476 resource_type_link: u'http://.../#branch_subscription'
1477 review_level: u'No email'
1478 self_link: u'http://.../~farmer-bob/farm/corn/+subscription/farmer-joe'
1479+ subscribed_by_link: u'http://.../~salgado'
1480
1481 >>> def print_subscriber_count(branch):
1482 ... subscribers = webservice.get(
1483@@ -91,6 +92,7 @@
1484 resource_type_link: u'http://.../#branch_subscription'
1485 review_level: u'Status changes only'
1486 self_link: u'http://.../~farmer-bob/farm/corn/+subscription/farmer-joe'
1487+ subscribed_by_link: u'http://.../~salgado'
1488
1489
1490 We print the count, and even though subscribe was called again, there's still
1491
1492=== modified file 'lib/lp/code/tests/test_branch.py'
1493--- lib/lp/code/tests/test_branch.py 2010-05-27 01:46:06 +0000
1494+++ lib/lp/code/tests/test_branch.py 2010-05-29 09:01:05 +0000
1495@@ -134,7 +134,7 @@
1496 removeSecurityProxy(branch).subscribe(
1497 person, BranchSubscriptionNotificationLevel.NOEMAIL,
1498 BranchSubscriptionDiffSize.NODIFF,
1499- CodeReviewNotificationLevel.NOEMAIL)
1500+ CodeReviewNotificationLevel.NOEMAIL, person)
1501 self.assertAuthenticatedView(branch, person, True)
1502
1503 def test_privateBranchAnyoneElse(self):
1504
1505=== modified file 'lib/lp/codehosting/scanner/tests/test_bzrsync.py'
1506--- lib/lp/codehosting/scanner/tests/test_bzrsync.py 2010-05-03 09:31:06 +0000
1507+++ lib/lp/codehosting/scanner/tests/test_bzrsync.py 2010-05-29 09:01:05 +0000
1508@@ -85,7 +85,7 @@
1509 LaunchpadZopelessLayer.txn.begin()
1510 new_branch = self.factory.makeAnyBranch(*args, **kwargs)
1511 # Unsubscribe the implicit owner subscription.
1512- new_branch.unsubscribe(new_branch.owner)
1513+ new_branch.unsubscribe(new_branch.owner, new_branch.owner)
1514 LaunchpadZopelessLayer.txn.commit()
1515 return new_branch
1516
1517
1518=== modified file 'lib/lp/codehosting/scanner/tests/test_email.py'
1519--- lib/lp/codehosting/scanner/tests/test_email.py 2010-05-18 13:12:34 +0000
1520+++ lib/lp/codehosting/scanner/tests/test_email.py 2010-05-29 09:01:05 +0000
1521@@ -41,7 +41,8 @@
1522 test_user,
1523 BranchSubscriptionNotificationLevel.FULL,
1524 BranchSubscriptionDiffSize.FIVEKLINES,
1525- CodeReviewNotificationLevel.NOEMAIL)
1526+ CodeReviewNotificationLevel.NOEMAIL,
1527+ test_user)
1528 LaunchpadZopelessLayer.txn.commit()
1529 return branch
1530
1531@@ -151,7 +152,8 @@
1532 db_branch.registrant,
1533 BranchSubscriptionNotificationLevel.FULL,
1534 BranchSubscriptionDiffSize.WHOLEDIFF,
1535- CodeReviewNotificationLevel.FULL)
1536+ CodeReviewNotificationLevel.FULL,
1537+ db_branch.registrant)
1538 self.assertEqual(0, len(list(RevisionMailJob.iterReady())))
1539 notify(events.TipChanged(db_branch, tree.branch, True))
1540 self.assertEqual(1, len(list(RevisionMailJob.iterReady())))
1541@@ -164,7 +166,8 @@
1542 db_branch.registrant,
1543 BranchSubscriptionNotificationLevel.FULL,
1544 BranchSubscriptionDiffSize.WHOLEDIFF,
1545- CodeReviewNotificationLevel.FULL)
1546+ CodeReviewNotificationLevel.FULL,
1547+ db_branch.registrant)
1548 self.assertEqual(0, len(list(RevisionMailJob.iterReady())))
1549 notify(events.RevisionsRemoved(db_branch, tree.branch, ['x']))
1550 self.assertEqual(1, len(list(RevisionMailJob.iterReady())))
1551
1552=== modified file 'lib/lp/codehosting/tests/test_jobs.py'
1553--- lib/lp/codehosting/tests/test_jobs.py 2010-04-23 05:49:08 +0000
1554+++ lib/lp/codehosting/tests/test_jobs.py 2010-05-29 09:01:05 +0000
1555@@ -30,7 +30,7 @@
1556 branch.subscribe(branch.registrant,
1557 BranchSubscriptionNotificationLevel.FULL,
1558 BranchSubscriptionDiffSize.WHOLEDIFF,
1559- CodeReviewNotificationLevel.FULL)
1560+ CodeReviewNotificationLevel.FULL, branch.registrant)
1561 tree_transport = tree.bzrdir.root_transport
1562 tree_transport.put_bytes("hello.txt", "Hello World\n")
1563 tree.add('hello.txt')
1564
1565=== modified file 'lib/lp/registry/browser/tests/packaging-views.txt'
1566--- lib/lp/registry/browser/tests/packaging-views.txt 2010-04-16 18:00:31 +0000
1567+++ lib/lp/registry/browser/tests/packaging-views.txt 2010-05-29 09:01:05 +0000
1568@@ -350,5 +350,5 @@
1569 cnews
1570 libstdc++
1571 linux-source-2.6.15
1572+ hot
1573 thunderbird
1574- hot
1575
1576=== modified file 'lib/lp/registry/browser/tests/private-team-creation-views.txt'
1577--- lib/lp/registry/browser/tests/private-team-creation-views.txt 2009-08-03 21:46:09 +0000
1578+++ lib/lp/registry/browser/tests/private-team-creation-views.txt 2010-05-29 09:01:05 +0000
1579@@ -411,7 +411,7 @@
1580 ... team,
1581 ... BranchSubscriptionNotificationLevel.DIFFSONLY,
1582 ... BranchSubscriptionDiffSize.WHOLEDIFF,
1583- ... CodeReviewNotificationLevel.STATUS)
1584+ ... CodeReviewNotificationLevel.STATUS, team_owner)
1585 ... # A branch visibility rule.
1586 ... from lp.code.enums import BranchVisibilityRule
1587 ... from lp.registry.interfaces.product import IProductSet
1588
1589=== modified file 'lib/lp/registry/doc/private-team-roles.txt'
1590--- lib/lp/registry/doc/private-team-roles.txt 2010-04-12 08:04:31 +0000
1591+++ lib/lp/registry/doc/private-team-roles.txt 2010-05-29 09:01:05 +0000
1592@@ -130,7 +130,7 @@
1593 ... priv_team,
1594 ... BranchSubscriptionNotificationLevel.DIFFSONLY,
1595 ... BranchSubscriptionDiffSize.WHOLEDIFF,
1596- ... CodeReviewNotificationLevel.STATUS)
1597+ ... CodeReviewNotificationLevel.STATUS, team_owner)
1598 >>> print subscription.person.name
1599 private-team
1600
1601@@ -141,7 +141,7 @@
1602 ... pm_team,
1603 ... BranchSubscriptionNotificationLevel.DIFFSONLY,
1604 ... BranchSubscriptionDiffSize.WHOLEDIFF,
1605- ... CodeReviewNotificationLevel.STATUS)
1606+ ... CodeReviewNotificationLevel.STATUS, team_owner)
1607 Traceback (most recent call last):
1608 ...
1609 PrivatePersonLinkageError: Cannot link person
1610
1611=== modified file 'lib/lp/registry/model/distroseries.py'
1612--- lib/lp/registry/model/distroseries.py 2010-05-19 15:39:52 +0000
1613+++ lib/lp/registry/model/distroseries.py 2010-05-29 09:01:05 +0000
1614@@ -332,7 +332,7 @@
1615 origin = SQL(joins)
1616 condition = SQL(conditions + "AND packaging.id IS NULL")
1617 results = IStore(self).using(origin).find(find_spec, condition)
1618- results = results.order_by('score DESC')
1619+ results = results.order_by('score DESC', SourcePackageName.name)
1620 return [{
1621 'package': SourcePackage(
1622 sourcepackagename=spn, distroseries=self),
1623
1624=== modified file 'lib/lp/testing/factory.py'
1625--- lib/lp/testing/factory.py 2010-05-28 23:06:25 +0000
1626+++ lib/lp/testing/factory.py 2010-05-29 09:01:05 +0000
1627@@ -988,7 +988,8 @@
1628
1629 return proposal
1630
1631- def makeBranchSubscription(self, branch=None, person=None):
1632+ def makeBranchSubscription(self, branch=None, person=None,
1633+ subscribed_by=None):
1634 """Create a BranchSubscription.
1635
1636 :param branch_title: The title to use for the created Branch
1637@@ -998,9 +999,11 @@
1638 branch = self.makeBranch()
1639 if person is None:
1640 person = self.makePerson()
1641+ if subscribed_by is None:
1642+ subscribed_by = person
1643 return branch.subscribe(person,
1644 BranchSubscriptionNotificationLevel.NOEMAIL, None,
1645- CodeReviewNotificationLevel.NOEMAIL)
1646+ CodeReviewNotificationLevel.NOEMAIL, subscribed_by)
1647
1648 def makeDiff(self, diff_text=DIFF):
1649 return Diff.fromFile(StringIO(diff_text), len(diff_text))

Subscribers

People subscribed via source and target branches

to status/vote changes: