Merge lp:~thumper/launchpad/branch-move-methods into lp:launchpad

Proposed by Tim Penhey
Status: Merged
Merged at revision: not available
Proposed branch: lp:~thumper/launchpad/branch-move-methods
Merge into: lp:launchpad
Diff against target: None lines
To merge this branch: bzr merge lp:~thumper/launchpad/branch-move-methods
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+9559@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :
Download full text (3.2 KiB)

= Summary =

This branch adds setOwner and setTarget to IBranch.

IBranch.owner and IBranch.product are changed to be read only in the interface
and API.

== Implementation details ==

Both setOwner and setTarget defer to the IBranchNamespace.moveBranch method.
This method does the validation checks for both owner setting, name clashes,
and visibility policy checks.

The LaunchpadEditForm view updateContextFromData was modified to take a boolean
to determine whether or not the ObjectModifiedEvent listeners should be
notified. This is because the setOwner call needs to remove the 'owner' key
out of the dict as the updateContextFromData will fail if it is there, but we
still want to show that the change has occurred in the email.

A number of changes are just noise in the move to make owner and product read
only through the interface.

== Tests ==

TestBranchSetOwner
TestBranchSetTarget
TestBranchNamespaceMoveBranch
test_retargetBranch

== Demo and Q/A ==

Not much has chanced in the UI. If you can still change the owner of the
branch, then we have succeeded.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/webapp/launchpadform.py
  lib/lp/code/interfaces/branchtarget.py
  lib/lp/code/configure.zcml
  lib/lp/code/browser/branch.py
  lib/lp/code/interfaces/branch.py
  lib/lp/registry/doc/private-team-roles.txt
  lib/lp/code/interfaces/branchnamespace.py
  lib/lp/registry/browser/tests/private-team-creation-views.txt
  lib/lp/code/model/branchtarget.py
  lib/lp/codehosting/tests/test_acceptance.py
  lib/lp/code/model/branch.py
  lib/lp/code/browser/tests/test_branchmergeproposallisting.py
  lib/lp/code/model/tests/test_branchtarget.py
  lib/lp/code/model/tests/test_branchnamespace.py
  lib/lp/code/model/branchnamespace.py
  lib/lp/code/doc/branch.txt
  lib/lp/code/model/tests/test_revision.py
  lib/lp/code/scripts/tests/test_revisionkarma.py
  lib/canonical/launchpad/interfaces/_schema_circular_imports.py
  lib/lp/code/model/tests/test_branch.py

== Pyflakes notices ==

lib/canonical/launchpad/webapp/launchpadform.py
    22: 'action' imported but unused
    94: redefinition of unused 'action' from line 22
    195: redefinition of unused 'action' from line 22

lib/lp/code/model/branch.py
    73: 'IPerson' imported but unused

lib/lp/code/model/tests/test_branch.py
    61: 'IProductSet' imported but unused

== Pylint notices ==

lib/canonical/launchpad/webapp/launchpadform.py
    245: [W0211, LaunchpadFormView.validate_none] Static method with 'self' as
first argument

lib/lp/code/browser/branch.py
    48: [F0401] Unable to import 'lazr.uri' (No module named uri)

lib/lp/code/model/branch.py
    186: [C0301] Line too long (79/78)
    73: [W0611] Unused import IPerson

lib/lp/code/browser/tests/test_branchmergeproposallisting.py
    16: [C0301] Line too long (79/78)

lib/lp/code/scripts/tests/test_revisionkarma.py
    52: [C0322, TestRevisionKarma.test_junkBranchMoved] Operator not preceded
by a space
    project=self.factory.makeProduct()
    ^

lib/lp/code/model/tests/test_branch.py...

Read more...

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :
Download full text (39.3 KiB)

I think, on reviewing this branch, that I'd rather you'd have done the
setOwner thing and moveBranch thing in separate branches, if that
would have been possible. Oh well!

Approved, subject to hiding retargetBranch a little harder. Comments
below.

Cheers,
mwh

> === modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
> --- lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-07-17 00:26:05 +0000
> +++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-08-03 03:36:16 +0000
> @@ -75,6 +75,11 @@
> IBranch['linkSpecification'].queryTaggedValue(
> LAZR_WEBSERVICE_EXPORTED)['params']['spec'].schema= ISpecification
> IBranch['product'].schema = IProduct
> +IBranch['setTarget'].queryTaggedValue(
> + LAZR_WEBSERVICE_EXPORTED)['params']['project'].schema= IProduct
> +IBranch['setTarget'].queryTaggedValue(
> + LAZR_WEBSERVICE_EXPORTED)['params']['source_package'].schema= \
> + ISourcePackage
> IBranch['spec_links'].value_type.schema = ISpecificationBranch
> IBranch['subscribe'].queryTaggedValue(
> LAZR_WEBSERVICE_EXPORTED)['return_type'].schema = IBranchSubscription

I guess once the lp.foo dependencies are properly sorted out, we won't
need this kind of rubbish so much any more....

> === modified file 'lib/canonical/launchpad/webapp/launchpadform.py'
> --- lib/canonical/launchpad/webapp/launchpadform.py 2009-06-25 05:30:52 +0000
> +++ lib/canonical/launchpad/webapp/launchpadform.py 2009-08-03 03:36:16 +0000
> @@ -372,7 +372,7 @@
>
> render_context = True
>
> - def updateContextFromData(self, data, context=None):
> + def updateContextFromData(self, data, context=None, notify_modified=True):
> """Update the context object based on form data.
>
> If no context is given, the view's context is used.

It's micro-optimization, but I don't think there's any reason to take
the snapshot if you're not going to use it.

> @@ -391,7 +391,7 @@
>
> was_changed = form.applyChanges(context, self.form_fields,
> data, self.adapters)
> - if was_changed:
> + if was_changed and notify_modified:
> field_names = [form_field.__name__
> for form_field in self.form_fields]
> notify(ObjectModifiedEvent(

> === modified file 'lib/lp/code/browser/branch.py'
> --- lib/lp/code/browser/branch.py 2009-07-19 04:41:14 +0000
> +++ lib/lp/code/browser/branch.py 2009-08-03 03:36:16 +0000
> @@ -36,12 +36,15 @@
> from zope.app.form.browser import TextAreaWidget
> from zope.traversing.interfaces import IPathAdapter
> from zope.component import getUtility, queryAdapter
> +from zope.event import notify
> from zope.formlib import form
> -from zope.interface import Interface, implements
> +from zope.interface import Interface, implements, providedBy
> from zope.publisher.interfaces import NotFound
> from zope.schema import Choice, Text
> from lazr.delegates import delegates
> from lazr.enum import EnumeratedType, Item
> +from lazr.lifecycle.event import ObjectModifiedEvent
> +from lazr.lifecycle.snapshot import Snapshot
> from lazr.uri import URI
>
> from...

review: Approve
Revision history for this message
Tim Penhey (thumper) wrote :
Download full text (13.9 KiB)

On Mon, 03 Aug 2009 17:54:59 Michael Hudson wrote:
> Review: Approve
> I think, on reviewing this branch, that I'd rather you'd have done the
> setOwner thing and moveBranch thing in separate branches, if that
> would have been possible. Oh well!

Probably, but it wasn't until I realised what was needed that it got done
together. I guess I could have worked harder to separate the bits. Perhaps
next time :)

> > === modified file
> > 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py' ---
> > lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-07-17
> > 00:26:05 +0000 +++
> > lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-08-03
> > 03:36:16 +0000 @@ -75,6 +75,11 @@
> > IBranch['linkSpecification'].queryTaggedValue(
> > LAZR_WEBSERVICE_EXPORTED)['params']['spec'].schema= ISpecification
> > IBranch['product'].schema = IProduct
> > +IBranch['setTarget'].queryTaggedValue(
> > + LAZR_WEBSERVICE_EXPORTED)['params']['project'].schema= IProduct
> > +IBranch['setTarget'].queryTaggedValue(
> > + LAZR_WEBSERVICE_EXPORTED)['params']['source_package'].schema= \
> > + ISourcePackage
> > IBranch['spec_links'].value_type.schema = ISpecificationBranch
> > IBranch['subscribe'].queryTaggedValue(
> > LAZR_WEBSERVICE_EXPORTED)['return_type'].schema =
> > IBranchSubscription
>
> I guess once the lp.foo dependencies are properly sorted out, we won't
> need this kind of rubbish so much any more....

Unlikely, as IProduct imports some bits that need IBranch, so we have the
classical circular dependency.

> > === modified file 'lib/canonical/launchpad/webapp/launchpadform.py'
> > --- lib/canonical/launchpad/webapp/launchpadform.py 2009-06-25 05:30:52
> > +0000 +++ lib/canonical/launchpad/webapp/launchpadform.py 2009-08-03
> > 03:36:16 +0000 @@ -372,7 +372,7 @@
> >
> > render_context = True
> >
> > - def updateContextFromData(self, data, context=None):
> > + def updateContextFromData(self, data, context=None,
> > notify_modified=True): """Update the context object based on form data.
> >
> > If no context is given, the view's context is used.
>
> It's micro-optimization, but I don't think there's any reason to take
> the snapshot if you're not going to use it.

Added.

> > === modified file 'lib/lp/code/browser/branch.py'
> > --- lib/lp/code/browser/branch.py 2009-07-19 04:41:14 +0000
> > +++ lib/lp/code/browser/branch.py 2009-08-03 03:36:16 +0000
> > @@ -36,12 +36,15 @@
> > from zope.app.form.browser import TextAreaWidget
> > from zope.traversing.interfaces import IPathAdapter
> > from zope.component import getUtility, queryAdapter
> > +from zope.event import notify
> > from zope.formlib import form
> > -from zope.interface import Interface, implements
> > +from zope.interface import Interface, implements, providedBy
> > from zope.publisher.interfaces import NotFound
> > from zope.schema import Choice, Text
> > from lazr.delegates import delegates
> > from lazr.enum import EnumeratedType, Item
> > +from lazr.lifecycle.event import ObjectModifiedEvent
> > +from lazr.lifecycle.snapshot import Snapshot
> > from lazr.uri import URI
> >
> > from canonical.cachedpr...

1=== modified file 'lib/canonical/launchpad/webapp/launchpadform.py'
2--- lib/canonical/launchpad/webapp/launchpadform.py 2009-07-31 02:32:22 +0000
3+++ lib/canonical/launchpad/webapp/launchpadform.py 2009-08-04 00:25:53 +0000
4@@ -386,8 +386,9 @@
5 """
6 if context is None:
7 context = self.context
8- context_before_modification = Snapshot(
9- context, providing=providedBy(context))
10+ if notify_modified:
11+ context_before_modification = Snapshot(
12+ context, providing=providedBy(context))
13
14 was_changed = form.applyChanges(context, self.form_fields,
15 data, self.adapters)
16
17=== modified file 'lib/lp/code/browser/branch.py'
18--- lib/lp/code/browser/branch.py 2009-07-31 02:32:22 +0000
19+++ lib/lp/code/browser/branch.py 2009-08-04 00:27:41 +0000
20@@ -601,6 +601,10 @@
21 @action('Change Branch', name='change')
22 def change_action(self, action, data):
23 # If the owner or product has changed, add an explicit notification.
24+ # We take our own snapshot here to make sure that the snapshot records
25+ # changes to the owner and private, and we notify the listeners
26+ # explicitly below rather than the notification that would normally be
27+ # sent in updateContextFromData.
28 branch_before_modification = Snapshot(
29 self.context, providing=providedBy(self.context))
30 if 'owner' in data:
31
32=== modified file 'lib/lp/code/interfaces/branch.py'
33--- lib/lp/code/interfaces/branch.py 2009-07-31 01:07:04 +0000
34+++ lib/lp/code/interfaces/branch.py 2009-08-05 01:49:53 +0000
35@@ -32,7 +32,6 @@
36 'IBranchNavigationMenu',
37 'IBranchSet',
38 'NoSuchBranch',
39- 'UnknownBranchTargetError',
40 'UnknownBranchTypeError',
41 'user_has_special_branch_access',
42 ]
43@@ -122,10 +121,6 @@
44 """Raised when the user specifies an unrecognized branch type."""
45
46
47-class UnknownBranchTargetError(Exception):
48- """Raised when an unexpected type is passed through as a branch target."""
49-
50-
51 class BranchCreationForbidden(BranchCreationException):
52 """A Branch visibility policy forbids branch creation.
53
54@@ -445,6 +440,10 @@
55 Only one of `project` or `source_package` can be set, and if neither
56 is set, the branch gets moved into the junk namespace of the branch
57 owner.
58+
59+ :raises BranchTargetError: if both project and source_package are set,
60+ or if either the project or source_package fail to be adapted to an
61+ `IBranchTarget`.
62 """
63
64 reviewer = exported(
65
66=== modified file 'lib/lp/code/interfaces/branchtarget.py'
67--- lib/lp/code/interfaces/branchtarget.py 2009-07-28 05:20:23 +0000
68+++ lib/lp/code/interfaces/branchtarget.py 2009-08-04 00:13:03 +0000
69@@ -113,12 +113,3 @@
70
71 def getBugTask(bug):
72 """Get the BugTask for a given bug related to the branch target."""
73-
74- def retargetBranch(branch):
75- """Set the branch target to refer to this target.
76-
77- This only updates the target related attributes of the branch.
78-
79- No name clashes are checked, nor privacy policies. Those are handled
80- by the IBranchNamespace.moveBranch method.
81- """
82
83=== modified file 'lib/lp/code/model/branchnamespace.py'
84--- lib/lp/code/model/branchnamespace.py 2009-07-28 05:20:23 +0000
85+++ lib/lp/code/model/branchnamespace.py 2009-08-04 00:14:50 +0000
86@@ -177,7 +177,7 @@
87 # attributes are readonly through the interface.
88 naked_branch = removeSecurityProxy(branch)
89 naked_branch.owner = self.owner
90- self.target.retargetBranch(naked_branch)
91+ self.target._retargetBranch(naked_branch)
92 naked_branch.name = new_name
93
94 def createBranchWithPrefix(self, branch_type, prefix, registrant,
95
96=== modified file 'lib/lp/code/model/branchtarget.py'
97--- lib/lp/code/model/branchtarget.py 2009-07-28 05:20:23 +0000
98+++ lib/lp/code/model/branchtarget.py 2009-08-04 00:14:25 +0000
99@@ -129,15 +129,15 @@
100 # those cases.
101 return bug.default_bugtask
102
103- def retargetBranch(self, branch):
104- """See `IBranchTarget`."""
105- # Since product, distroseries and sourcepackagename are not writable
106- # as defined by the interface, we need to rip off the security proxy
107- # here.
108- naked_branch = removeSecurityProxy(branch)
109- naked_branch.product = None
110- naked_branch.distroseries = self.sourcepackage.distroseries
111- naked_branch.sourcepackagename = self.sourcepackage.sourcepackagename
112+ def _retargetBranch(self, branch):
113+ """Set the branch target to refer to this target.
114+
115+ This only updates the target related attributes of the branch, and
116+ expects a branch without a security proxy as a parameter.
117+ """
118+ branch.product = None
119+ branch.distroseries = self.sourcepackage.distroseries
120+ branch.sourcepackagename = self.sourcepackage.sourcepackagename
121
122
123 class PersonBranchTarget(_BaseBranchTarget):
124@@ -193,15 +193,15 @@
125 """See `IBranchTarget`."""
126 return bug.default_bugtask
127
128- def retargetBranch(self, branch):
129- """See `IBranchTarget`."""
130- # Since product, distroseries and sourcepackagename are not writable
131- # as defined by the interface, we need to rip off the security proxy
132- # here.
133- naked_branch = removeSecurityProxy(branch)
134- naked_branch.product = None
135- naked_branch.distroseries = None
136- naked_branch.sourcepackagename = None
137+ def _retargetBranch(self, branch):
138+ """Set the branch target to refer to this target.
139+
140+ This only updates the target related attributes of the branch, and
141+ expects a branch without a security proxy as a parameter.
142+ """
143+ branch.product = None
144+ branch.distroseries = None
145+ branch.sourcepackagename = None
146
147
148 class ProductBranchTarget(_BaseBranchTarget):
149@@ -286,15 +286,15 @@
150 task = bug.bugtasks[0]
151 return task
152
153- def retargetBranch(self, branch):
154- """See `IBranchTarget`."""
155- # Since product, distroseries and sourcepackagename are not writable
156- # as defined by the interface, we need to rip off the security proxy
157- # here.
158- naked_branch = removeSecurityProxy(branch)
159- naked_branch.product = self.product
160- naked_branch.distroseries = None
161- naked_branch.sourcepackagename = None
162+ def _retargetBranch(self, branch):
163+ """Set the branch target to refer to this target.
164+
165+ This only updates the target related attributes of the branch, and
166+ expects a branch without a security proxy as a parameter.
167+ """
168+ branch.product = self.product
169+ branch.distroseries = None
170+ branch.sourcepackagename = None
171
172
173 def get_canonical_url_data_for_target(branch_target):
174
175=== modified file 'lib/lp/code/model/tests/test_branch.py'
176--- lib/lp/code/model/tests/test_branch.py 2009-08-02 23:17:10 +0000
177+++ lib/lp/code/model/tests/test_branch.py 2009-08-05 02:03:41 +0000
178@@ -40,7 +40,7 @@
179 from lp.code.interfaces.branch import (
180 BranchCannotBePrivate, BranchCannotBePublic,
181 BranchCreatorNotMemberOfOwnerTeam, BranchCreatorNotOwner,
182- CannotDeleteBranch, DEFAULT_BRANCH_STATUS_IN_LISTING)
183+ BranchTargetError, CannotDeleteBranch, DEFAULT_BRANCH_STATUS_IN_LISTING)
184 from lp.code.interfaces.branchlookup import IBranchLookup
185 from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
186 from lp.code.interfaces.branchmergeproposal import InvalidBranchMergeProposal
187@@ -58,7 +58,6 @@
188 from lp.code.model.codeimport import CodeImport, CodeImportSet
189 from lp.code.model.codereviewcomment import CodeReviewComment
190 from lp.registry.interfaces.person import IPersonSet
191-from lp.registry.interfaces.product import IProductSet
192 from lp.registry.model.product import ProductSet
193 from lp.registry.model.sourcepackage import SourcePackage
194 from lp.soyuz.interfaces.publishing import PackagePublishingPocket
195@@ -1722,6 +1721,17 @@
196
197 layer = DatabaseFunctionalLayer
198
199+ def test_not_both_project_and_source_package(self):
200+ # Only one of project or source_package can be passed in, not both.
201+ branch = self.factory.makePersonalBranch()
202+ project = self.factory.makeProduct()
203+ source_package = self.factory.makeSourcePackage()
204+ login_person(branch.owner)
205+ self.assertRaises(
206+ BranchTargetError,
207+ branch.setTarget,
208+ user=branch.owner, project=project, source_package=source_package)
209+
210 def test_junk_branch_to_project_branch(self):
211 # A junk branch can be moved to a project.
212 branch = self.factory.makePersonalBranch()
213
214=== modified file 'lib/lp/code/model/tests/test_branchtarget.py'
215--- lib/lp/code/model/tests/test_branchtarget.py 2009-07-28 05:20:23 +0000
216+++ lib/lp/code/model/tests/test_branchtarget.py 2009-08-04 00:23:05 +0000
217@@ -49,19 +49,19 @@
218 def test_retargetBranch_packageBranch(self):
219 # Retarget an existing package branch to this target.
220 branch = self.factory.makePackageBranch()
221- self.target.retargetBranch(branch)
222+ self.target._retargetBranch(removeSecurityProxy(branch))
223 self.assertEqual(self.target, branch.target)
224
225 def test_retargetBranch_productBranch(self):
226 # Retarget an existing product branch to this target.
227 branch = self.factory.makeProductBranch()
228- self.target.retargetBranch(branch)
229+ self.target._retargetBranch(removeSecurityProxy(branch))
230 self.assertEqual(self.target, branch.target)
231
232 def test_retargetBranch_personalBranch(self):
233 # Retarget an existing personal branch to this target.
234 branch = self.factory.makePersonalBranch()
235- self.target.retargetBranch(branch)
236+ self.target._retargetBranch(removeSecurityProxy(branch))
237 self.assertEqual(self.target, branch.target)
238
239
240@@ -236,7 +236,7 @@
241 # match the target as the target is the branch owner for a personal
242 # branch.
243 branch = self.factory.makePackageBranch(owner=self.original)
244- self.target.retargetBranch(branch)
245+ self.target._retargetBranch(removeSecurityProxy(branch))
246 self.assertEqual(self.target, branch.target)
247
248 def test_retargetBranch_productBranch(self):
249@@ -245,7 +245,7 @@
250 # match the target as the target is the branch owner for a personal
251 # branch.
252 branch = self.factory.makeProductBranch(owner=self.original)
253- self.target.retargetBranch(branch)
254+ self.target._retargetBranch(removeSecurityProxy(branch))
255 self.assertEqual(self.target, branch.target)
256
257 def test_retargetBranch_personalBranch(self):
258@@ -254,7 +254,7 @@
259 # match the target as the target is the branch owner for a personal
260 # branch.
261 branch = self.factory.makePersonalBranch(owner=self.original)
262- self.target.retargetBranch(branch)
263+ self.target._retargetBranch(removeSecurityProxy(branch))
264 self.assertEqual(self.target, branch.target)
265
266
267
268=== modified file 'lib/lp/code/scripts/tests/test_revisionkarma.py'
269--- lib/lp/code/scripts/tests/test_revisionkarma.py 2009-08-02 23:17:10 +0000
270+++ lib/lp/code/scripts/tests/test_revisionkarma.py 2009-08-04 00:36:50 +0000
271@@ -49,7 +49,7 @@
272 branch.createBranchRevision(1, rev)
273 # Once the branch is connected to the revision, we now specify
274 # a product for the branch.
275- project=self.factory.makeProduct()
276+ project = self.factory.makeProduct()
277 branch.setTarget(user=branch.owner, project=project)
278 # Commit and switch to the script db user.
279 transaction.commit()
280
281=== modified file 'lib/lp/codehosting/tests/test_acceptance.py'
282--- lib/lp/codehosting/tests/test_acceptance.py 2009-08-03 01:41:07 +0000
283+++ lib/lp/codehosting/tests/test_acceptance.py 2009-08-04 00:40:11 +0000
284@@ -14,7 +14,6 @@
285 from bzrlib.tests import TestCaseWithTransport
286 from bzrlib.urlutils import local_path_from_url
287 from bzrlib.workingtree import WorkingTree
288-from zope.security.proxy import removeSecurityProxy
289
290 from lp.codehosting.bzrutils import DenyingServer
291 from lp.codehosting.tests.helpers import (
292@@ -383,7 +382,7 @@
293 # rename as far as bzr is concerned: the URL changes.
294 LaunchpadZopelessTestSetup().txn.begin()
295 branch = self.getDatabaseBranch('testuser', None, 'test-branch')
296- removeSecurityProxy(branch).product = Product.byName('firefox')
297+ branch.setTarget(user=branch.owner, project=Product.byName('firefox'))
298 LaunchpadZopelessTestSetup().txn.commit()
299
300 self.assertNotBranch(

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
2--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-07-17 00:26:05 +0000
3+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-07-31 01:07:04 +0000
4@@ -75,6 +75,11 @@
5 IBranch['linkSpecification'].queryTaggedValue(
6 LAZR_WEBSERVICE_EXPORTED)['params']['spec'].schema= ISpecification
7 IBranch['product'].schema = IProduct
8+IBranch['setTarget'].queryTaggedValue(
9+ LAZR_WEBSERVICE_EXPORTED)['params']['project'].schema= IProduct
10+IBranch['setTarget'].queryTaggedValue(
11+ LAZR_WEBSERVICE_EXPORTED)['params']['source_package'].schema= \
12+ ISourcePackage
13 IBranch['spec_links'].value_type.schema = ISpecificationBranch
14 IBranch['subscribe'].queryTaggedValue(
15 LAZR_WEBSERVICE_EXPORTED)['return_type'].schema = IBranchSubscription
16
17=== modified file 'lib/canonical/launchpad/webapp/launchpadform.py'
18--- lib/canonical/launchpad/webapp/launchpadform.py 2009-06-25 05:30:52 +0000
19+++ lib/canonical/launchpad/webapp/launchpadform.py 2009-07-31 02:32:22 +0000
20@@ -372,7 +372,7 @@
21
22 render_context = True
23
24- def updateContextFromData(self, data, context=None):
25+ def updateContextFromData(self, data, context=None, notify_modified=True):
26 """Update the context object based on form data.
27
28 If no context is given, the view's context is used.
29@@ -391,7 +391,7 @@
30
31 was_changed = form.applyChanges(context, self.form_fields,
32 data, self.adapters)
33- if was_changed:
34+ if was_changed and notify_modified:
35 field_names = [form_field.__name__
36 for form_field in self.form_fields]
37 notify(ObjectModifiedEvent(
38
39=== modified file 'lib/lp/code/browser/branch.py'
40--- lib/lp/code/browser/branch.py 2009-07-19 04:41:14 +0000
41+++ lib/lp/code/browser/branch.py 2009-07-31 02:32:22 +0000
42@@ -36,12 +36,15 @@
43 from zope.app.form.browser import TextAreaWidget
44 from zope.traversing.interfaces import IPathAdapter
45 from zope.component import getUtility, queryAdapter
46+from zope.event import notify
47 from zope.formlib import form
48-from zope.interface import Interface, implements
49+from zope.interface import Interface, implements, providedBy
50 from zope.publisher.interfaces import NotFound
51 from zope.schema import Choice, Text
52 from lazr.delegates import delegates
53 from lazr.enum import EnumeratedType, Item
54+from lazr.lifecycle.event import ObjectModifiedEvent
55+from lazr.lifecycle.snapshot import Snapshot
56 from lazr.uri import URI
57
58 from canonical.cachedproperty import cachedproperty
59@@ -574,7 +577,6 @@
60 the user to be able to edit it.
61 """
62 use_template(IBranch, include=[
63- 'owner',
64 'name',
65 'url',
66 'description',
67@@ -582,6 +584,7 @@
68 'whiteboard',
69 ])
70 private = copy_field(IBranch['private'], readonly=False)
71+ owner = copy_field(IBranch['owner'], readonly=False)
72
73
74 class BranchEditFormView(LaunchpadEditFormView):
75@@ -598,9 +601,12 @@
76 @action('Change Branch', name='change')
77 def change_action(self, action, data):
78 # If the owner or product has changed, add an explicit notification.
79+ branch_before_modification = Snapshot(
80+ self.context, providing=providedBy(self.context))
81 if 'owner' in data:
82- new_owner = data['owner']
83+ new_owner = data.pop('owner')
84 if new_owner != self.context.owner:
85+ self.context.setOwner(new_owner, self.user)
86 self.request.response.addNotification(
87 "The branch owner has been changed to %s (%s)"
88 % (new_owner.displayname, new_owner.name))
89@@ -616,7 +622,13 @@
90 else:
91 self.request.response.addNotification(
92 "The branch is now publicly accessible.")
93- if self.updateContextFromData(data):
94+ if self.updateContextFromData(data, notify_modified=False):
95+ # Notify the object has changed with the snapshot that was taken
96+ # earler.
97+ field_names = [
98+ form_field.__name__ for form_field in self.form_fields]
99+ notify(ObjectModifiedEvent(
100+ self.context, branch_before_modification, field_names))
101 # Only specify that the context was modified if there
102 # was in fact a change.
103 self.context.date_last_modified = UTC_NOW
104
105=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposallisting.py'
106--- lib/lp/code/browser/tests/test_branchmergeproposallisting.py 2009-07-17 00:26:05 +0000
107+++ lib/lp/code/browser/tests/test_branchmergeproposallisting.py 2009-08-02 23:25:37 +0000
108@@ -8,14 +8,15 @@
109 from unittest import TestLoader
110
111 import transaction
112+from zope.security.proxy import removeSecurityProxy
113
114+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
115+from canonical.testing import DatabaseFunctionalLayer
116 from lp.code.browser.branchmergeproposallisting import (
117 ActiveReviewsView, BranchMergeProposalListingView, PersonActiveReviewsView,
118 ProductActiveReviewsView)
119 from lp.code.enums import CodeReviewVote
120 from lp.testing import ANONYMOUS, login, login_person, TestCaseWithFactory
121-from canonical.launchpad.webapp.servers import LaunchpadTestRequest
122-from canonical.testing import DatabaseFunctionalLayer
123
124 _default = object()
125
126@@ -201,8 +202,7 @@
127 self.assertReviewGroupForUser(
128 self.bmp.registrant, self._view.OTHER)
129 team = self.factory.makeTeam(self.bmp.registrant)
130- login_person(self.bmp.source_branch.owner)
131- self.bmp.source_branch.owner = team
132+ removeSecurityProxy(self.bmp.source_branch).owner = team
133 self.assertReviewGroupForUser(
134 self.bmp.registrant, ActiveReviewsView.MINE)
135
136
137=== modified file 'lib/lp/code/configure.zcml'
138--- lib/lp/code/configure.zcml 2009-07-23 02:06:55 +0000
139+++ lib/lp/code/configure.zcml 2009-07-30 00:30:02 +0000
140@@ -430,9 +430,9 @@
141 "/>
142 <require
143 permission="launchpad.Edit"
144- attributes="destroySelf setPrivate"
145+ attributes="destroySelf setPrivate setOwner setTarget"
146 set_attributes="name url mirror_status_message
147- owner author description product lifecycle_status
148+ description lifecycle_status
149 last_mirrored last_mirrored_id last_mirror_attempt
150 mirror_failures pull_disabled next_mirror_time
151 last_scanned last_scanned_id revision_count branch_type
152
153=== modified file 'lib/lp/code/doc/branch.txt'
154--- lib/lp/code/doc/branch.txt 2009-06-16 03:31:05 +0000
155+++ lib/lp/code/doc/branch.txt 2009-08-02 23:59:31 +0000
156@@ -116,9 +116,9 @@
157 >>> print new_branch.owner.name
158 registrant
159
160-A user can create a branch where the owner is either themselves,
161-or a team that they are a member of. The registrant is not writable,
162-whereas the owner is.
163+A user can create a branch where the owner is either themselves, or a team
164+that they are a member of. Neither the owner nor the registrant are writable,
165+but the owner can be set using the `setOwner` method.
166
167 >>> login('admin@canonical.com')
168 >>> new_branch.registrant = factory.makePerson()
169@@ -126,7 +126,8 @@
170 ...
171 ForbiddenAttribute: ('registrant', <Branch ...>)
172
173- >>> new_branch.owner = factory.makePerson(name='new-owner')
174+ >>> team = factory.makeTeam(name='new-owner', owner=new_branch.owner)
175+ >>> new_branch.setOwner(new_owner=team, user=new_branch.owner)
176 >>> print new_branch.registrant.name
177 registrant
178 >>> print new_branch.owner.name
179
180=== modified file 'lib/lp/code/interfaces/branch.py'
181--- lib/lp/code/interfaces/branch.py 2009-07-29 03:44:05 +0000
182+++ lib/lp/code/interfaces/branch.py 2009-07-31 01:07:04 +0000
183@@ -19,6 +19,7 @@
184 'BranchCreatorNotMemberOfOwnerTeam',
185 'BranchCreatorNotOwner',
186 'BranchExists',
187+ 'BranchTargetError',
188 'BranchTypeError',
189 'CannotDeleteBranch',
190 'DEFAULT_BRANCH_STATUS_IN_LISTING',
191@@ -31,6 +32,7 @@
192 'IBranchNavigationMenu',
193 'IBranchSet',
194 'NoSuchBranch',
195+ 'UnknownBranchTargetError',
196 'UnknownBranchTypeError',
197 'user_has_special_branch_access',
198 ]
199@@ -90,7 +92,7 @@
200 """Raised when creating a branch that already exists."""
201
202 def __init__(self, existing_branch):
203- # XXX: JonathanLange 2008-12-04 spec=package-branches: This error
204+ # XXX: TimPenhey 2009-07-12 bug=405214: This error
205 # message logic is incorrect, but the exact text is being tested
206 # in branch-xmlrpc.txt.
207 params = {'name': existing_branch.name}
208@@ -108,6 +110,10 @@
209 BranchCreationException.__init__(self, message)
210
211
212+class BranchTargetError(Exception):
213+ """Raised when there is an error determining a branch target."""
214+
215+
216 class CannotDeleteBranch(Exception):
217 """The branch cannot be deleted at this time."""
218
219@@ -116,6 +122,10 @@
220 """Raised when the user specifies an unrecognized branch type."""
221
222
223+class UnknownBranchTargetError(Exception):
224+ """Raised when an unexpected type is passed through as a branch target."""
225+
226+
227 class BranchCreationForbidden(BranchCreationException):
228 """A Branch visibility policy forbids branch creation.
229
230@@ -402,14 +412,41 @@
231 title=_("The user that registered the branch."),
232 required=True, readonly=True,
233 vocabulary='ValidPersonOrTeam'))
234+
235 owner = exported(
236 ParticipatingPersonChoice(
237 title=_('Owner'),
238- required=True,
239+ required=True, readonly=True,
240 vocabulary='UserTeamsParticipationPlusSelf',
241 description=_("Either yourself or a team you are a member of. "
242 "This controls who can modify the branch.")))
243
244+ @call_with(user=REQUEST_USER)
245+ @operation_parameters(
246+ new_owner=Reference(
247+ title=_("The new owner of the branch."),
248+ schema=IPerson))
249+ @export_write_operation()
250+ def setOwner(new_owner, user):
251+ """Set the owner of the branch to be `new_owner`."""
252+
253+ @call_with(user=REQUEST_USER)
254+ @operation_parameters(
255+ project=Reference(
256+ title=_("The project the branch belongs to."),
257+ schema=Interface, required=False), # Really IProduct
258+ source_package=Reference(
259+ title=_("The source package the branch belongs to."),
260+ schema=Interface, required=False)) # Really ISourcePackage
261+ @export_write_operation()
262+ def setTarget(user, project=None, source_package=None):
263+ """Set the target of the branch to be `project` or `source_package`.
264+
265+ Only one of `project` or `source_package` can be set, and if neither
266+ is set, the branch gets moved into the junk namespace of the branch
267+ owner.
268+ """
269+
270 reviewer = exported(
271 PublicPersonChoice(
272 title=_('Default Review Team'),
273@@ -456,7 +493,7 @@
274 product = exported(
275 ReferenceChoice(
276 title=_('Project'),
277- required=False,
278+ required=False, readonly=True,
279 vocabulary='Product',
280 schema=Interface,
281 description=_("The project this branch belongs to.")),
282
283=== modified file 'lib/lp/code/interfaces/branchnamespace.py'
284--- lib/lp/code/interfaces/branchnamespace.py 2009-06-25 04:06:00 +0000
285+++ lib/lp/code/interfaces/branchnamespace.py 2009-07-27 10:19:21 +0000
286@@ -70,6 +70,24 @@
287 def isNameUsed(name):
288 """Is 'name' already used in this namespace?"""
289
290+ def moveBranch(branch, mover, new_name=None, rename_if_necessary=False):
291+ """Move the branch into this namespace.
292+
293+ :param branch: The `IBranch` to move.
294+ :param mover: The `IPerson` doing the moving.
295+ :param new_name: A new name for the branch.
296+ :param rename_if_necessary: Rename the branch if the branch name
297+ exists already in this namespace.
298+ :raises BranchCreatorNotMemberOfOwnerTeam: if the namespace owner is
299+ a team, and 'mover' is not in that team.
300+ :raises BranchCreatorNotOwner: if the namespace owner is an individual
301+ and 'mover' is not the owner.
302+ :raises BranchCreationForbidden: if 'mover' is not allowed to create
303+ a branch in this namespace due to privacy rules.
304+ :raises BranchExists: if a branch with the 'name' exists already in
305+ the namespace, and 'rename_if_necessary' is False.
306+ """
307+
308
309 class IBranchNamespacePolicy(Interface):
310 """Methods relating to branch creation and validation."""
311@@ -133,11 +151,13 @@
312 validation constraints on IBranch.name.
313 """
314
315- def validateMove(branch, mover):
316+ def validateMove(branch, mover, name=None):
317 """Check that 'mover' can move 'branch' into this namespace.
318
319 :param branch: An `IBranch` that might be moved.
320 :param mover: The `IPerson` who would move it.
321+ :param name: A new name for the branch. If None, the branch name is
322+ used.
323 :raises BranchCreatorNotMemberOfOwnerTeam: if the namespace owner is
324 a team, and 'mover' is not in that team.
325 :raises BranchCreatorNotOwner: if the namespace owner is an individual
326
327=== modified file 'lib/lp/code/interfaces/branchtarget.py'
328--- lib/lp/code/interfaces/branchtarget.py 2009-07-17 00:26:05 +0000
329+++ lib/lp/code/interfaces/branchtarget.py 2009-07-28 05:20:23 +0000
330@@ -113,3 +113,12 @@
331
332 def getBugTask(bug):
333 """Get the BugTask for a given bug related to the branch target."""
334+
335+ def retargetBranch(branch):
336+ """Set the branch target to refer to this target.
337+
338+ This only updates the target related attributes of the branch.
339+
340+ No name clashes are checked, nor privacy policies. Those are handled
341+ by the IBranchNamespace.moveBranch method.
342+ """
343
344=== modified file 'lib/lp/code/model/branch.py'
345--- lib/lp/code/model/branch.py 2009-07-19 04:41:14 +0000
346+++ lib/lp/code/model/branch.py 2009-07-31 01:07:04 +0000
347@@ -57,7 +57,7 @@
348 from lp.code.event.branchmergeproposal import NewBranchMergeProposalEvent
349 from lp.code.interfaces.branch import (
350 bazaar_identity, BranchCannotBePrivate, BranchCannotBePublic,
351- BranchTypeError, CannotDeleteBranch,
352+ BranchTargetError, BranchTypeError, CannotDeleteBranch,
353 DEFAULT_BRANCH_STATUS_IN_LISTING, IBranch,
354 IBranchNavigationMenu, IBranchSet)
355 from lp.code.interfaces.branchcollection import IAllBranches
356@@ -71,7 +71,7 @@
357 from lp.code.interfaces.seriessourcepackagebranch import (
358 IFindOfficialBranchLinks)
359 from lp.registry.interfaces.person import (
360- validate_person_not_private_membership, validate_public_person)
361+ IPerson, validate_person_not_private_membership, validate_public_person)
362
363
364 class Branch(SQLBase):
365@@ -113,6 +113,12 @@
366 owner = ForeignKey(
367 dbName='owner', foreignKey='Person',
368 storm_validator=validate_person_not_private_membership, notNull=True)
369+
370+ def setOwner(self, new_owner, user):
371+ """See `IBranch`."""
372+ new_namespace = self.target.getNamespace(new_owner)
373+ new_namespace.moveBranch(self, user, rename_if_necessary=True)
374+
375 reviewer = ForeignKey(
376 dbName='reviewer', foreignKey='Person',
377 storm_validator=validate_public_person, default=None)
378@@ -162,6 +168,28 @@
379 target = self.product
380 return IBranchTarget(target)
381
382+ def setTarget(self, user, project=None, source_package=None):
383+ """See `IBranch`."""
384+ if project is not None:
385+ if source_package is not None:
386+ raise BranchTargetError(
387+ 'Cannot specify both a project and a source package.')
388+ else:
389+ target = IBranchTarget(project)
390+ if target is None:
391+ raise BranchTargetError(
392+ '%r is not a valid project target' % project)
393+ elif source_package is not None:
394+ target = IBranchTarget(source_package)
395+ if target is None:
396+ raise BranchTargetError(
397+ '%r is not a valid source package target' % source_package)
398+ else:
399+ target = IBranchTarget(self.owner)
400+ # Person targets are always valid.
401+ namespace = target.getNamespace(self.owner)
402+ namespace.moveBranch(self, user, rename_if_necessary=True)
403+
404 @property
405 def namespace(self):
406 """See `IBranch`."""
407
408=== modified file 'lib/lp/code/model/branchnamespace.py'
409--- lib/lp/code/model/branchnamespace.py 2009-06-25 04:06:00 +0000
410+++ lib/lp/code/model/branchnamespace.py 2009-07-28 05:20:23 +0000
411@@ -16,6 +16,7 @@
412 from zope.component import getUtility
413 from zope.event import notify
414 from zope.interface import implements
415+from zope.security.proxy import removeSecurityProxy
416
417 from lazr.lifecycle.event import ObjectCreatedEvent
418 from storm.locals import And
419@@ -160,6 +161,25 @@
420 self.validateBranchName(name)
421 self.validateRegistrant(mover)
422
423+ def moveBranch(self, branch, mover, new_name=None,
424+ rename_if_necessary=False):
425+ """See `IBranchNamespace`."""
426+ # Check to see if the branch is already in this namespace.
427+ old_namespace = branch.namespace
428+ if self.name == old_namespace.name:
429+ return
430+ if new_name is None:
431+ new_name = branch.name
432+ if rename_if_necessary:
433+ new_name = self.findUnusedName(new_name)
434+ self.validateMove(branch, mover, new_name)
435+ # Remove the security proxy of the branch as the owner and target
436+ # attributes are readonly through the interface.
437+ naked_branch = removeSecurityProxy(branch)
438+ naked_branch.owner = self.owner
439+ self.target.retargetBranch(naked_branch)
440+ naked_branch.name = new_name
441+
442 def createBranchWithPrefix(self, branch_type, prefix, registrant,
443 url=None):
444 """See `IBranchNamespace`."""
445
446=== modified file 'lib/lp/code/model/branchtarget.py'
447--- lib/lp/code/model/branchtarget.py 2009-07-17 00:26:05 +0000
448+++ lib/lp/code/model/branchtarget.py 2009-07-28 05:20:23 +0000
449@@ -13,7 +13,8 @@
450
451 from zope.component import getUtility
452 from zope.interface import implements
453-from zope.security.proxy import isinstance as zope_isinstance
454+from zope.security.proxy import (
455+ removeSecurityProxy, isinstance as zope_isinstance)
456
457 from lp.code.interfaces.branchcollection import IAllBranches
458 from lp.code.interfaces.branchtarget import (
459@@ -128,6 +129,16 @@
460 # those cases.
461 return bug.default_bugtask
462
463+ def retargetBranch(self, branch):
464+ """See `IBranchTarget`."""
465+ # Since product, distroseries and sourcepackagename are not writable
466+ # as defined by the interface, we need to rip off the security proxy
467+ # here.
468+ naked_branch = removeSecurityProxy(branch)
469+ naked_branch.product = None
470+ naked_branch.distroseries = self.sourcepackage.distroseries
471+ naked_branch.sourcepackagename = self.sourcepackage.sourcepackagename
472+
473
474 class PersonBranchTarget(_BaseBranchTarget):
475 implements(IBranchTarget)
476@@ -182,6 +193,16 @@
477 """See `IBranchTarget`."""
478 return bug.default_bugtask
479
480+ def retargetBranch(self, branch):
481+ """See `IBranchTarget`."""
482+ # Since product, distroseries and sourcepackagename are not writable
483+ # as defined by the interface, we need to rip off the security proxy
484+ # here.
485+ naked_branch = removeSecurityProxy(branch)
486+ naked_branch.product = None
487+ naked_branch.distroseries = None
488+ naked_branch.sourcepackagename = None
489+
490
491 class ProductBranchTarget(_BaseBranchTarget):
492 implements(IBranchTarget)
493@@ -265,6 +286,16 @@
494 task = bug.bugtasks[0]
495 return task
496
497+ def retargetBranch(self, branch):
498+ """See `IBranchTarget`."""
499+ # Since product, distroseries and sourcepackagename are not writable
500+ # as defined by the interface, we need to rip off the security proxy
501+ # here.
502+ naked_branch = removeSecurityProxy(branch)
503+ naked_branch.product = self.product
504+ naked_branch.distroseries = None
505+ naked_branch.sourcepackagename = None
506+
507
508 def get_canonical_url_data_for_target(branch_target):
509 """Return the `ICanonicalUrlData` for an `IBranchTarget`."""
510
511=== modified file 'lib/lp/code/model/tests/test_branch.py'
512--- lib/lp/code/model/tests/test_branch.py 2009-07-23 02:06:55 +0000
513+++ lib/lp/code/model/tests/test_branch.py 2009-08-02 23:17:10 +0000
514@@ -39,6 +39,7 @@
515 BranchVisibilityRule, CodeReviewNotificationLevel)
516 from lp.code.interfaces.branch import (
517 BranchCannotBePrivate, BranchCannotBePublic,
518+ BranchCreatorNotMemberOfOwnerTeam, BranchCreatorNotOwner,
519 CannotDeleteBranch, DEFAULT_BRANCH_STATUS_IN_LISTING)
520 from lp.code.interfaces.branchlookup import IBranchLookup
521 from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
522@@ -213,8 +214,7 @@
523 # attribute is updated too.
524 branch = self.factory.makeAnyBranch()
525 new_owner = self.factory.makePerson()
526- login('admin@canonical.com')
527- branch.owner = new_owner
528+ removeSecurityProxy(branch).owner = new_owner
529 # Call the function that is normally called through the event system
530 # to auto reload the fields updated by the db triggers.
531 update_trigger_modified_fields(branch)
532@@ -1054,9 +1054,9 @@
533
534 def setUp(self):
535 TestCaseWithFactory.setUp(self, 'admin@canonical.com')
536- self.product = getUtility(IProductSet).getByName('firefox')
537+ self.product = self.factory.makeProduct()
538
539- self.user = getUtility(IPersonSet).getByName('no-priv')
540+ self.user = self.factory.makePerson()
541 self.source = self.factory.makeProductBranch(
542 name='source-branch', owner=self.user, product=self.product)
543 self.target = self.factory.makeProductBranch(
544@@ -1069,7 +1069,7 @@
545
546 def test_junkSource(self):
547 """Junk branches cannot be used as a source for merge proposals."""
548- self.source.product = None
549+ self.source.setTarget(user=self.source.owner)
550 self.assertRaises(
551 InvalidBranchMergeProposal, self.source.addLandingTarget,
552 self.user, self.target)
553@@ -1078,12 +1078,13 @@
554 """The product of the target branch must match the product of the
555 source branch.
556 """
557- self.target.product = None
558+ self.target.setTarget(user=self.target.owner)
559 self.assertRaises(
560 InvalidBranchMergeProposal, self.source.addLandingTarget,
561 self.user, self.target)
562
563- self.target.product = getUtility(IProductSet).getByName('bzr')
564+ project = self.factory.makeProduct()
565+ self.target.setTarget(user=self.target.owner, project=project)
566 self.assertRaises(
567 InvalidBranchMergeProposal, self.source.addLandingTarget,
568 self.user, self.target)
569@@ -1097,12 +1098,13 @@
570 def test_dependentBranchSameProduct(self):
571 """The dependent branch, if it is there, must be for the same product.
572 """
573- self.dependent.product = None
574+ self.dependent.setTarget(user=self.dependent.owner)
575 self.assertRaises(
576 InvalidBranchMergeProposal, self.source.addLandingTarget,
577 self.user, self.target, self.dependent)
578
579- self.dependent.product = getUtility(IProductSet).getByName('bzr')
580+ project = self.factory.makeProduct()
581+ self.dependent.setTarget(user=self.dependent.owner, project=project)
582 self.assertRaises(
583 InvalidBranchMergeProposal, self.source.addLandingTarget,
584 self.user, self.target, self.dependent)
585@@ -1650,5 +1652,138 @@
586 self.assertEqual(branch.spec_links.count(), 0)
587
588
589+class TestBranchSetOwner(TestCaseWithFactory):
590+ """Tests for IBranch.setOwner."""
591+
592+ layer = DatabaseFunctionalLayer
593+
594+ def test_owner_sets_team(self):
595+ # The owner of the branch can set the owner of the branch to be a team
596+ # they are a member of.
597+ branch = self.factory.makeAnyBranch()
598+ team = self.factory.makeTeam(owner=branch.owner)
599+ login_person(branch.owner)
600+ branch.setOwner(team, branch.owner)
601+ self.assertEqual(team, branch.owner)
602+
603+ def test_owner_cannot_set_nonmember_team(self):
604+ # The owner of the branch cannot set the owner to be a team they are
605+ # not a member of.
606+ branch = self.factory.makeAnyBranch()
607+ team = self.factory.makeTeam()
608+ login_person(branch.owner)
609+ self.assertRaises(
610+ BranchCreatorNotMemberOfOwnerTeam,
611+ branch.setOwner,
612+ team, branch.owner)
613+
614+ def test_owner_cannot_set_other_user(self):
615+ # The owner of the branch cannot set the new owner to be another
616+ # person.
617+ branch = self.factory.makeAnyBranch()
618+ person = self.factory.makePerson()
619+ login_person(branch.owner)
620+ self.assertRaises(
621+ BranchCreatorNotOwner,
622+ branch.setOwner,
623+ person, branch.owner)
624+
625+ def test_admin_can_set_any_team_or_person(self):
626+ # A Launchpad admin can set the branch to be owned by any team or
627+ # person.
628+ branch = self.factory.makeAnyBranch()
629+ team = self.factory.makeTeam()
630+ # To get a random administrator, choose the admin team owner.
631+ admin = getUtility(ILaunchpadCelebrities).admin.teamowner
632+ login_person(admin)
633+ branch.setOwner(team, admin)
634+ self.assertEqual(team, branch.owner)
635+ person = self.factory.makePerson()
636+ branch.setOwner(person, admin)
637+ self.assertEqual(person, branch.owner)
638+
639+ def test_bazaar_experts_can_set_any_team_or_person(self):
640+ # A bazaar expert can set the branch to be owned by any team or
641+ # person.
642+ branch = self.factory.makeAnyBranch()
643+ team = self.factory.makeTeam()
644+ # To get a random administrator, choose the admin team owner.
645+ experts = getUtility(ILaunchpadCelebrities).bazaar_experts.teamowner
646+ login_person(experts)
647+ branch.setOwner(team, experts)
648+ self.assertEqual(team, branch.owner)
649+ person = self.factory.makePerson()
650+ branch.setOwner(person, experts)
651+ self.assertEqual(person, branch.owner)
652+
653+
654+class TestBranchSetTarget(TestCaseWithFactory):
655+ """Tests for IBranch.setTarget."""
656+
657+ layer = DatabaseFunctionalLayer
658+
659+ def test_junk_branch_to_project_branch(self):
660+ # A junk branch can be moved to a project.
661+ branch = self.factory.makePersonalBranch()
662+ project = self.factory.makeProduct()
663+ login_person(branch.owner)
664+ branch.setTarget(user=branch.owner, project=project)
665+ self.assertEqual(project, branch.target.context)
666+
667+ def test_junk_branch_to_package_branch(self):
668+ # A junk branch can be moved to a source package.
669+ branch = self.factory.makePersonalBranch()
670+ source_package = self.factory.makeSourcePackage()
671+ login_person(branch.owner)
672+ branch.setTarget(user=branch.owner, source_package=source_package)
673+ self.assertEqual(source_package, branch.target.context)
674+
675+ def test_project_branch_to_other_project_branch(self):
676+ # Move a branch from one project to another.
677+ branch = self.factory.makeProductBranch()
678+ project = self.factory.makeProduct()
679+ login_person(branch.owner)
680+ branch.setTarget(user=branch.owner, project=project)
681+ self.assertEqual(project, branch.target.context)
682+
683+ def test_project_branch_to_package_branch(self):
684+ # Move a branch from a project to a package.
685+ branch = self.factory.makeProductBranch()
686+ source_package = self.factory.makeSourcePackage()
687+ login_person(branch.owner)
688+ branch.setTarget(user=branch.owner, source_package=source_package)
689+ self.assertEqual(source_package, branch.target.context)
690+
691+ def test_project_branch_to_junk_branch(self):
692+ # Move a branch from a project to junk.
693+ branch = self.factory.makeProductBranch()
694+ login_person(branch.owner)
695+ branch.setTarget(user=branch.owner)
696+ self.assertEqual(branch.owner, branch.target.context)
697+
698+ def test_package_branch_to_other_package_branch(self):
699+ # Move a branch from one package to another.
700+ branch = self.factory.makePackageBranch()
701+ source_package = self.factory.makeSourcePackage()
702+ login_person(branch.owner)
703+ branch.setTarget(user=branch.owner, source_package=source_package)
704+ self.assertEqual(source_package, branch.target.context)
705+
706+ def test_package_branch_to_project_branch(self):
707+ # Move a branch from a package to a project.
708+ branch = self.factory.makePackageBranch()
709+ project = self.factory.makeProduct()
710+ login_person(branch.owner)
711+ branch.setTarget(user=branch.owner, project=project)
712+ self.assertEqual(project, branch.target.context)
713+
714+ def test_package_branch_to_junk_branch(self):
715+ # Move a branch from a package to junk.
716+ branch = self.factory.makePackageBranch()
717+ login_person(branch.owner)
718+ branch.setTarget(user=branch.owner)
719+ self.assertEqual(branch.owner, branch.target.context)
720+
721+
722 def test_suite():
723 return TestLoader().loadTestsFromName(__name__)
724
725=== renamed file 'lib/lp/code/tests/test_branchnamespace.py' => 'lib/lp/code/model/tests/test_branchnamespace.py'
726--- lib/lp/code/tests/test_branchnamespace.py 2009-06-25 04:06:00 +0000
727+++ lib/lp/code/model/tests/test_branchnamespace.py 2009-07-29 02:16:43 +0000
728@@ -1871,5 +1871,70 @@
729 BranchCreatorNotOwner, self.albert, self.doug)
730
731
732+class TestBranchNamespaceMoveBranch(TestCaseWithFactory):
733+ """Test the IBranchNamespace.moveBranch method.
734+
735+ The edge cases of the validateMove are tested in the NamespaceMixin for
736+ each of the namespaces.
737+ """
738+
739+ layer = DatabaseFunctionalLayer
740+
741+ def assertNamespacesEqual(self, expected, result):
742+ """Assert that the namespaces refer to the same thing.
743+
744+ The name of the namespace contains the user name and the context
745+ parts, so is the easiest thing to check.
746+ """
747+ self.assertEqual(expected.name, result.name)
748+
749+ def test_move_to_same_namespace(self):
750+ # Moving to the same namespace is effectively a no-op. No exceptions
751+ # about matching branch names should be raised.
752+ branch = self.factory.makeAnyBranch()
753+ namespace = branch.namespace
754+ namespace.moveBranch(branch, branch.owner)
755+ self.assertNamespacesEqual(namespace, branch.namespace)
756+
757+ def test_name_clash_raises(self):
758+ # A name clash will raise an exception.
759+ branch = self.factory.makeAnyBranch(name="test")
760+ another = self.factory.makeAnyBranch(owner=branch.owner, name="test")
761+ namespace = another.namespace
762+ self.assertRaises(
763+ BranchExists, namespace.moveBranch, branch, branch.owner)
764+
765+ def test_move_with_rename(self):
766+ # A name clash with 'rename_if_necessary' set to True will cause the
767+ # branch to be renamed instead of raising an error.
768+ branch = self.factory.makeAnyBranch(name="test")
769+ another = self.factory.makeAnyBranch(owner=branch.owner, name="test")
770+ namespace = another.namespace
771+ namespace.moveBranch(branch, branch.owner, rename_if_necessary=True)
772+ self.assertEqual("test-1", branch.name)
773+ self.assertNamespacesEqual(namespace, branch.namespace)
774+
775+ def test_move_with_new_name(self):
776+ # A new name for the branch can be specified as part of the move.
777+ branch = self.factory.makeAnyBranch(name="test")
778+ another = self.factory.makeAnyBranch(owner=branch.owner, name="test")
779+ namespace = another.namespace
780+ namespace.moveBranch(branch, branch.owner, new_name="foo")
781+ self.assertEqual("foo", branch.name)
782+ self.assertNamespacesEqual(namespace, branch.namespace)
783+
784+ def test_sets_branch_owner(self):
785+ # Moving to a new namespace may change the owner of the branch if the
786+ # owner of the namespace is different.
787+ branch = self.factory.makeAnyBranch(name="test")
788+ team = self.factory.makeTeam(branch.owner)
789+ product = self.factory.makeProduct()
790+ namespace = ProductNamespace(team, product)
791+ namespace.moveBranch(branch, branch.owner)
792+ self.assertEqual(team, branch.owner)
793+ # And for paranoia.
794+ self.assertNamespacesEqual(namespace, branch.namespace)
795+
796+
797 def test_suite():
798 return unittest.TestLoader().loadTestsFromName(__name__)
799
800=== modified file 'lib/lp/code/model/tests/test_branchtarget.py'
801--- lib/lp/code/model/tests/test_branchtarget.py 2009-06-25 04:06:00 +0000
802+++ lib/lp/code/model/tests/test_branchtarget.py 2009-07-28 05:20:23 +0000
803@@ -46,6 +46,24 @@
804 branches = self.target.collection.getBranches()
805 self.assertEqual([branch], list(branches))
806
807+ def test_retargetBranch_packageBranch(self):
808+ # Retarget an existing package branch to this target.
809+ branch = self.factory.makePackageBranch()
810+ self.target.retargetBranch(branch)
811+ self.assertEqual(self.target, branch.target)
812+
813+ def test_retargetBranch_productBranch(self):
814+ # Retarget an existing product branch to this target.
815+ branch = self.factory.makeProductBranch()
816+ self.target.retargetBranch(branch)
817+ self.assertEqual(self.target, branch.target)
818+
819+ def test_retargetBranch_personalBranch(self):
820+ # Retarget an existing personal branch to this target.
821+ branch = self.factory.makePersonalBranch()
822+ self.target.retargetBranch(branch)
823+ self.assertEqual(self.target, branch.target)
824+
825
826 class TestPackageBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):
827
828@@ -212,6 +230,33 @@
829 # The default merge target is always None.
830 self.assertIs(None, self.target.default_merge_target)
831
832+ def test_retargetBranch_packageBranch(self):
833+ # Retarget an existing package branch to this target. Override the
834+ # mixin tests, and specify the owner of the branch. This is needed to
835+ # match the target as the target is the branch owner for a personal
836+ # branch.
837+ branch = self.factory.makePackageBranch(owner=self.original)
838+ self.target.retargetBranch(branch)
839+ self.assertEqual(self.target, branch.target)
840+
841+ def test_retargetBranch_productBranch(self):
842+ # Retarget an existing product branch to this target. Override the
843+ # mixin tests, and specify the owner of the branch. This is needed to
844+ # match the target as the target is the branch owner for a personal
845+ # branch.
846+ branch = self.factory.makeProductBranch(owner=self.original)
847+ self.target.retargetBranch(branch)
848+ self.assertEqual(self.target, branch.target)
849+
850+ def test_retargetBranch_personalBranch(self):
851+ # Retarget an existing personal branch to this target. Override the
852+ # mixin tests, and specify the owner of the branch. This is needed to
853+ # match the target as the target is the branch owner for a personal
854+ # branch.
855+ branch = self.factory.makePersonalBranch(owner=self.original)
856+ self.target.retargetBranch(branch)
857+ self.assertEqual(self.target, branch.target)
858+
859
860 class TestProductBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):
861
862
863=== modified file 'lib/lp/code/model/tests/test_revision.py'
864--- lib/lp/code/model/tests/test_revision.py 2009-06-25 04:06:00 +0000
865+++ lib/lp/code/model/tests/test_revision.py 2009-08-02 23:17:10 +0000
866@@ -146,7 +146,8 @@
867 branch.createBranchRevision(1, rev)
868 # Once the branch is connected to the revision, we now specify
869 # a product for the branch.
870- branch.product = self.factory.makeProduct()
871+ project = self.factory.makeProduct()
872+ branch.setTarget(user=branch.owner, project=project)
873 # The revision is now identified as needing karma allocated.
874 self.assertEqual(
875 [rev], list(RevisionSet.getRevisionsNeedingKarmaAllocated()))
876
877=== modified file 'lib/lp/code/scripts/tests/test_revisionkarma.py'
878--- lib/lp/code/scripts/tests/test_revisionkarma.py 2009-06-25 04:06:00 +0000
879+++ lib/lp/code/scripts/tests/test_revisionkarma.py 2009-08-02 23:17:10 +0000
880@@ -49,7 +49,8 @@
881 branch.createBranchRevision(1, rev)
882 # Once the branch is connected to the revision, we now specify
883 # a product for the branch.
884- branch.product = self.factory.makeProduct()
885+ project=self.factory.makeProduct()
886+ branch.setTarget(user=branch.owner, project=project)
887 # Commit and switch to the script db user.
888 transaction.commit()
889 LaunchpadZopelessLayer.switchDbUser(config.revisionkarma.dbuser)
890
891=== modified file 'lib/lp/codehosting/tests/test_acceptance.py'
892--- lib/lp/codehosting/tests/test_acceptance.py 2009-07-17 00:26:05 +0000
893+++ lib/lp/codehosting/tests/test_acceptance.py 2009-08-02 23:38:51 +0000
894@@ -8,13 +8,13 @@
895 import atexit
896 import os
897 import unittest
898-from xml.dom.minidom import parseString
899 import xmlrpclib
900
901 import bzrlib.branch
902 from bzrlib.tests import TestCaseWithTransport
903 from bzrlib.urlutils import local_path_from_url
904 from bzrlib.workingtree import WorkingTree
905+from zope.security.proxy import removeSecurityProxy
906
907 from lp.codehosting.bzrutils import DenyingServer
908 from lp.codehosting.tests.helpers import (
909@@ -382,7 +382,8 @@
910 # rename as far as bzr is concerned: the URL changes.
911 LaunchpadZopelessTestSetup().txn.begin()
912 branch = self.getDatabaseBranch('testuser', None, 'test-branch')
913- branch.product = database.Product.byName('firefox')
914+ removeSecurityProxy(branch).product = database.Product.byName(
915+ 'firefox')
916 LaunchpadZopelessTestSetup().txn.commit()
917
918 self.assertNotBranch(
919
920=== modified file 'lib/lp/registry/browser/tests/private-team-creation-views.txt'
921--- lib/lp/registry/browser/tests/private-team-creation-views.txt 2009-06-16 17:12:30 +0000
922+++ lib/lp/registry/browser/tests/private-team-creation-views.txt 2009-08-03 01:02:44 +0000
923@@ -400,8 +400,7 @@
924 ... bugtask = bug.default_bugtask
925 ... bugtask.transitionToAssignee(team)
926 ... # A branch.
927- ... branch = factory.makeBranch()
928- ... branch.owner = team
929+ ... branch = factory.makeBranch(owner=team)
930 ... # A branch subscription.
931 ... from lp.code.enums import (
932 ... BranchSubscriptionDiffSize,
933
934=== modified file 'lib/lp/registry/doc/private-team-roles.txt'
935--- lib/lp/registry/doc/private-team-roles.txt 2009-06-19 01:27:32 +0000
936+++ lib/lp/registry/doc/private-team-roles.txt 2009-08-03 00:41:34 +0000
937@@ -23,9 +23,10 @@
938 -----------------
939
940 >>> # Create the necessary teams.
941- >>> from lp.registry.interfaces.person import PersonVisibility
942 >>> team_owner = factory.makePerson(name='team-owner')
943- >>> login('foo.bar@canonical.com')
944+ >>> from lp.registry.interfaces.person import IPersonSet, PersonVisibility
945+ >>> admin_user = getUtility(IPersonSet).getByEmail('admin@canonical.com')
946+ >>> login_person(admin_user)
947 >>> priv_team = factory.makeTeam(name='private-team',
948 ... owner=team_owner,
949 ... visibility=PersonVisibility.PRIVATE)
950@@ -102,12 +103,12 @@
951 Private teams can be assigned as the owner of a branch
952
953 >>> branch = factory.makeBranch()
954- >>> branch.owner = priv_team
955+ >>> branch.setOwner(priv_team, user=admin_user)
956
957 But private membership teams cannot own a branch.
958
959 >>> branch = factory.makeBranch()
960- >>> branch.owner = pm_team
961+ >>> branch.setOwner(pm_team, user=admin_user)
962 Traceback (most recent call last):
963 ...
964 PrivatePersonLinkageError: Cannot link person