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

[Tim Penhey]

[Tim Penhey (thumper)]

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

[Michael Hudson-Doyle (mwhudson)]

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

[Tim Penhey (thumper)]

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

=== 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-07-31 01:07:04 +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
 
=== 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-07-31 02:32:22 +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.
@@ -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-07-31 02:32:22 +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.cachedproperty import cachedproperty
@@ -574,7 +577,6 @@
     the user to be able to edit it.
     """
     use_template(IBranch, include=[
-        'owner',
         'name',
         'url',
         'description',
@@ -582,6 +584,7 @@
         'whiteboard',
         ])
     private = copy_field(IBranch['private'], readonly=False)
+    owner = copy_field(IBranch['owner'], readonly=False)
 
 
 class BranchEditFormView(LaunchpadEditFormView):
@@ -598,9 +601,12 @@
     @action('Change Branch', name='change')
     def change_action(self, action, data):
         # If the owner or product has changed, add an explicit notification.
+        branch_before_modification = Snapshot(
+            self.context, providing=providedBy(self.context))
         if 'owner' in data:
-            new_owner = data['owner']
+            new_owner = data.pop('owner')
             if new_owner != self.context.owner:
+                self.context.setOwner(new_owner, self.user)
                 self.request.response.addNotification(
                     "The branch owner has been changed to %s (%s)"
                     % (new_owner.displayname, new_owner.name))
@@ -616,7 +622,13 @@
                 else:
                     self.request.response.addNotification(
                         "The branch is now publicly accessible.")
-        if self.updateContextFromData(data):
+        if self.updateContextFromData(data, notify_modified=False):
+            # Notify the object has changed with the snapshot that was taken
+            # earler.
+            field_names = [
+                form_field.__name__ for form_field in self.form_fields]
+            notify(ObjectModifiedEvent(
+                self.context, branch_before_modification, field_names))
             # Only specify that the context was modified if there
             # was in fact a change.
             self.context.date_last_modified = UTC_NOW
 
=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposallisting.py'
--- lib/lp/code/browser/tests/test_branchmergeproposallisting.py	2009-07-17 00:26:05 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposallisting.py	2009-08-02 23:25:37 +0000
@@ -8,14 +8,15 @@
 from unittest import TestLoader
 
 import transaction
+from zope.security.proxy import removeSecurityProxy
 
+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
+from canonical.testing import DatabaseFunctionalLayer
 from lp.code.browser.branchmergeproposallisting import (
     ActiveReviewsView, BranchMergeProposalListingView, PersonActiveReviewsView,
     ProductActiveReviewsView)
 from lp.code.enums import CodeReviewVote
 from lp.testing import ANONYMOUS, login, login_person, TestCaseWithFactory
-from canonical.launchpad.webapp.servers import LaunchpadTestRequest
-from canonical.testing import DatabaseFunctionalLayer
 
 _default = object()
 
@@ -201,8 +202,7 @@
         self.assertReviewGroupForUser(
             self.bmp.registrant, self._view.OTHER)
         team = self.factory.makeTeam(self.bmp.registrant)
-        login_person(self.bmp.source_branch.owner)
-        self.bmp.source_branch.owner = team
+        removeSecurityProxy(self.bmp.source_branch).owner = team
         self.assertReviewGroupForUser(
             self.bmp.registrant, ActiveReviewsView.MINE)
 
 
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2009-07-23 02:06:55 +0000
+++ lib/lp/code/configure.zcml	2009-07-30 00:30:02 +0000
@@ -430,9 +430,9 @@
                     "/>
     <require
         permission="launchpad.Edit"
-        attributes="destroySelf setPrivate"
+        attributes="destroySelf setPrivate setOwner setTarget"
         set_attributes="name url mirror_status_message
-                        owner author description product lifecycle_status
+                        description lifecycle_status
                         last_mirrored last_mirrored_id last_mirror_attempt
                         mirror_failures pull_disabled next_mirror_time
                         last_scanned last_scanned_id revision_count branch_type
 
=== modified file 'lib/lp/code/doc/branch.txt'
--- lib/lp/code/doc/branch.txt	2009-06-16 03:31:05 +0000
+++ lib/lp/code/doc/branch.txt	2009-08-02 23:59:31 +0000
@@ -116,9 +116,9 @@
     >>> print new_branch.owner.name
     registrant
 
-A user can create a branch where the owner is either themselves,
-or a team that they are a member of.  The registrant is not writable,
-whereas the owner is.
+A user can create a branch where the owner is either themselves, or a team
+that they are a member of.  Neither the owner nor the registrant are writable,
+but the owner can be set using the `setOwner` method.
 
     >>> login('[e-mail]')
     >>> new_branch.registrant = factory.makePerson()
@@ -126,7 +126,8 @@
       ...
     ForbiddenAttribute: ('registrant', <Branch ...>)
 
-    >>> new_branch.owner = factory.makePerson(name='new-owner')
+    >>> team = factory.makeTeam(name='new-owner', owner=new_branch.owner)
+    >>> new_branch.setOwner(new_owner=team, user=new_branch.owner)
     >>> print new_branch.registrant.name
     registrant
     >>> print new_branch.owner.name
 
=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2009-07-29 03:44:05 +0000
+++ lib/lp/code/interfaces/branch.py	2009-07-31 01:07:04 +0000
@@ -19,6 +19,7 @@
     'BranchCreatorNotMemberOfOwnerTeam',
     'BranchCreatorNotOwner',
     'BranchExists',
+    'BranchTargetError',
     'BranchTypeError',
     'CannotDeleteBranch',
     'DEFAULT_BRANCH_STATUS_IN_LISTING',
@@ -31,6 +32,7 @@
     'IBranchNavigationMenu',
     'IBranchSet',
     'NoSuchBranch',
+    'UnknownBranchTargetError',
     'UnknownBranchTypeError',
     'user_has_special_branch_access',
     ]
@@ -90,7 +92,7 @@
     """Raised when creating a branch that already exists."""
 
     def __init__(self, existing_branch):
-        # XXX: JonathanLange 2008-12-04 spec=package-branches: This error
+        # XXX: TimPenhey 2009-07-12 bug=405214: This error
         # message logic is incorrect, but the exact text is being tested
         # in branch-xmlrpc.txt.
         params = {'name': existing_branch.name}
@@ -108,6 +110,10 @@
         BranchCreationException.__init__(self, message)
 
 
+class BranchTargetError(Exception):
+    """Raised when there is an error determining a branch target."""
+
+
 class CannotDeleteBranch(Exception):
     """The branch cannot be deleted at this time."""
 
@@ -116,6 +122,10 @@
     """Raised when the user specifies an unrecognized branch type."""
 
 
+class UnknownBranchTargetError(Exception):
+    """Raised when an unexpected type is passed through as a branch target."""
+
+
 class BranchCreationForbidden(BranchCreationException):
     """A Branch visibility policy forbids branch creation.
 
@@ -402,14 +412,41 @@
             title=_("The user that registered the branch."),
             required=True, readonly=True,
             vocabulary='ValidPersonOrTeam'))
+
     owner = exported(
         ParticipatingPersonChoice(
             title=_('Owner'),
-            required=True,
+            required=True, readonly=True,
             vocabulary='UserTeamsParticipationPlusSelf',
             description=_("Either yourself or a team you are a member of. "
                           "This controls who can modify the branch.")))
 
+    @call_with(user=REQUEST_USER)
+    @operation_parameters(
+        new_owner=Reference(
+            title=_("The new owner of the branch."),
+            schema=IPerson))
+    @export_write_operation()
+    def setOwner(new_owner, user):
+        """Set the owner of the branch to be `new_owner`."""
+
+    @call_with(user=REQUEST_USER)
+    @operation_parameters(
+        project=Reference(
+            title=_("The project the branch belongs to."),
+            schema=Interface, required=False), # Really IProduct
+        source_package=Reference(
+            title=_("The source package the branch belongs to."),
+            schema=Interface, required=False)) # Really ISourcePackage
+    @export_write_operation()
+    def setTarget(user, project=None, source_package=None):
+        """Set the target of the branch to be `project` or `source_package`.
+
+        Only one of `project` or `source_package` can be set, and if neither
+        is set, the branch gets moved into the junk namespace of the branch
+        owner.
+        """
+
     reviewer = exported(
         PublicPersonChoice(
             title=_('Default Review Team'),
@@ -456,7 +493,7 @@
     product = exported(
         ReferenceChoice(
             title=_('Project'),
-            required=False,
+            required=False, readonly=True,
             vocabulary='Product',
             schema=Interface,
             description=_("The project this branch belongs to.")),
 
=== modified file 'lib/lp/code/interfaces/branchnamespace.py'
--- lib/lp/code/interfaces/branchnamespace.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/interfaces/branchnamespace.py	2009-07-27 10:19:21 +0000
@@ -70,6 +70,24 @@
     def isNameUsed(name):
         """Is 'name' already used in this namespace?"""
 
+    def moveBranch(branch, mover, new_name=None, rename_if_necessary=False):
+        """Move the branch into this namespace.
+
+        :param branch: The `IBranch` to move.
+        :param mover: The `IPerson` doing the moving.
+        :param new_name: A new name for the branch.
+        :param rename_if_necessary: Rename the branch if the branch name
+            exists already in this namespace.
+        :raises BranchCreatorNotMemberOfOwnerTeam: if the namespace owner is
+            a team, and 'mover' is not in that team.
+        :raises BranchCreatorNotOwner: if the namespace owner is an individual
+            and 'mover' is not the owner.
+        :raises BranchCreationForbidden: if 'mover' is not allowed to create
+            a branch in this namespace due to privacy rules.
+        :raises BranchExists: if a branch with the 'name' exists already in
+            the namespace, and 'rename_if_necessary' is False.
+        """
+
 
 class IBranchNamespacePolicy(Interface):
     """Methods relating to branch creation and validation."""
@@ -133,11 +151,13 @@
             validation constraints on IBranch.name.
         """
 
-    def validateMove(branch, mover):
+    def validateMove(branch, mover, name=None):
         """Check that 'mover' can move 'branch' into this namespace.
 
         :param branch: An `IBranch` that might be moved.
         :param mover: The `IPerson` who would move it.
+        :param name: A new name for the branch.  If None, the branch name is
+            used.
         :raises BranchCreatorNotMemberOfOwnerTeam: if the namespace owner is
             a team, and 'mover' is not in that team.
         :raises BranchCreatorNotOwner: if the namespace owner is an individual
 
=== modified file 'lib/lp/code/interfaces/branchtarget.py'
--- lib/lp/code/interfaces/branchtarget.py	2009-07-17 00:26:05 +0000
+++ lib/lp/code/interfaces/branchtarget.py	2009-07-28 05:20:23 +0000
@@ -113,3 +113,12 @@
 
     def getBugTask(bug):
         """Get the BugTask for a given bug related to the branch target."""
+
+    def retargetBranch(branch):
+        """Set the branch target to refer to this target.
+
+        This only updates the target related attributes of the branch.
+
+        No name clashes are checked, nor privacy policies.  Those are handled
+        by the IBranchNamespace.moveBranch method.
+        """
 
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2009-07-19 04:41:14 +0000
+++ lib/lp/code/model/branch.py	2009-07-31 01:07:04 +0000
@@ -57,7 +57,7 @@
 from lp.code.event.branchmergeproposal import NewBranchMergeProposalEvent
 from lp.code.interfaces.branch import (
     bazaar_identity, BranchCannotBePrivate, BranchCannotBePublic,
-    BranchTypeError, CannotDeleteBranch,
+    BranchTargetError, BranchTypeError, CannotDeleteBranch,
     DEFAULT_BRANCH_STATUS_IN_LISTING, IBranch,
     IBranchNavigationMenu, IBranchSet)
 from lp.code.interfaces.branchcollection import IAllBranches
@@ -71,7 +71,7 @@
 from lp.code.interfaces.seriessourcepackagebranch import (
     IFindOfficialBranchLinks)
 from lp.registry.interfaces.person import (
-    validate_person_not_private_membership, validate_public_person)
+    IPerson, validate_person_not_private_membership, validate_public_person)
 
 
 class Branch(SQLBase):
@@ -113,6 +113,12 @@
     owner = ForeignKey(
         dbName='owner', foreignKey='Person',
         storm_validator=validate_person_not_private_membership, notNull=True)
+
+    def setOwner(self, new_owner, user):
+        """See `IBranch`."""
+        new_namespace = self.target.getNamespace(new_owner)
+        new_namespace.moveBranch(self, user, rename_if_necessary=True)
+
     reviewer = ForeignKey(
         dbName='reviewer', foreignKey='Person',
         storm_validator=validate_public_person, default=None)
@@ -162,6 +168,28 @@
             target = self.product
         return IBranchTarget(target)
 
+    def setTarget(self, user, project=None, source_package=None):
+        """See `IBranch`."""
+        if project is not None:
+            if source_package is not None:
+                raise BranchTargetError(
+                    'Cannot specify both a project and a source package.')
+            else:
+                target = IBranchTarget(project)
+                if target is None:
+                    raise BranchTargetError(
+                        '%r is not a valid project target' % project)
+        elif source_package is not None:
+            target = IBranchTarget(source_package)
+            if target is None:
+                raise BranchTargetError(
+                    '%r is not a valid source package target' % source_package)
+        else:
+            target = IBranchTarget(self.owner)
+            # Person targets are always valid.
+        namespace = target.getNamespace(self.owner)
+        namespace.moveBranch(self, user, rename_if_necessary=True)
+
     @property
     def namespace(self):
         """See `IBranch`."""
 
=== modified file 'lib/lp/code/model/branchnamespace.py'
--- lib/lp/code/model/branchnamespace.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/model/branchnamespace.py	2009-07-28 05:20:23 +0000
@@ -16,6 +16,7 @@
 from zope.component import getUtility
 from zope.event import notify
 from zope.interface import implements
+from zope.security.proxy import removeSecurityProxy
 
 from lazr.lifecycle.event import ObjectCreatedEvent
 from storm.locals import And
@@ -160,6 +161,25 @@
         self.validateBranchName(name)
         self.validateRegistrant(mover)
 
+    def moveBranch(self, branch, mover, new_name=None,
+                   rename_if_necessary=False):
+        """See `IBranchNamespace`."""
+        # Check to see if the branch is already in this namespace.
+        old_namespace = branch.namespace
+        if self.name == old_namespace.name:
+            return
+        if new_name is None:
+            new_name = branch.name
+        if rename_if_necessary:
+            new_name = self.findUnusedName(new_name)
+        self.validateMove(branch, mover, new_name)
+        # Remove the security proxy of the branch as the owner and target
+        # attributes are readonly through the interface.
+        naked_branch = removeSecurityProxy(branch)
+        naked_branch.owner = self.owner
+        self.target.retargetBranch(naked_branch)
+        naked_branch.name = new_name
+
     def createBranchWithPrefix(self, branch_type, prefix, registrant,
                                url=None):
         """See `IBranchNamespace`."""
 
=== modified file 'lib/lp/code/model/branchtarget.py'
--- lib/lp/code/model/branchtarget.py	2009-07-17 00:26:05 +0000
+++ lib/lp/code/model/branchtarget.py	2009-07-28 05:20:23 +0000
@@ -13,7 +13,8 @@
 
 from zope.component import getUtility
 from zope.interface import implements
-from zope.security.proxy import isinstance as zope_isinstance
+from zope.security.proxy import (
+    removeSecurityProxy, isinstance as zope_isinstance)
 
 from lp.code.interfaces.branchcollection import IAllBranches
 from lp.code.interfaces.branchtarget import (
@@ -128,6 +129,16 @@
         # those cases.
         return bug.default_bugtask
 
+    def retargetBranch(self, branch):
+        """See `IBranchTarget`."""
+        # Since product, distroseries and sourcepackagename are not writable
+        # as defined by the interface, we need to rip off the security proxy
+        # here.
+        naked_branch = removeSecurityProxy(branch)
+        naked_branch.product = None
+        naked_branch.distroseries = self.sourcepackage.distroseries
+        naked_branch.sourcepackagename = self.sourcepackage.sourcepackagename
+
 
 class PersonBranchTarget(_BaseBranchTarget):
     implements(IBranchTarget)
@@ -182,6 +193,16 @@
         """See `IBranchTarget`."""
         return bug.default_bugtask
 
+    def retargetBranch(self, branch):
+        """See `IBranchTarget`."""
+        # Since product, distroseries and sourcepackagename are not writable
+        # as defined by the interface, we need to rip off the security proxy
+        # here.
+        naked_branch = removeSecurityProxy(branch)
+        naked_branch.product = None
+        naked_branch.distroseries = None
+        naked_branch.sourcepackagename = None
+
 
 class ProductBranchTarget(_BaseBranchTarget):
     implements(IBranchTarget)
@@ -265,6 +286,16 @@
             task = bug.bugtasks[0]
         return task
 
+    def retargetBranch(self, branch):
+        """See `IBranchTarget`."""
+        # Since product, distroseries and sourcepackagename are not writable
+        # as defined by the interface, we need to rip off the security proxy
+        # here.
+        naked_branch = removeSecurityProxy(branch)
+        naked_branch.product = self.product
+        naked_branch.distroseries = None
+        naked_branch.sourcepackagename = None
+
 
 def get_canonical_url_data_for_target(branch_target):
     """Return the `ICanonicalUrlData` for an `IBranchTarget`."""
 
=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py	2009-07-23 02:06:55 +0000
+++ lib/lp/code/model/tests/test_branch.py	2009-08-02 23:17:10 +0000
@@ -39,6 +39,7 @@
     BranchVisibilityRule, CodeReviewNotificationLevel)
 from lp.code.interfaces.branch import (
     BranchCannotBePrivate, BranchCannotBePublic,
+    BranchCreatorNotMemberOfOwnerTeam, BranchCreatorNotOwner,
     CannotDeleteBranch, DEFAULT_BRANCH_STATUS_IN_LISTING)
 from lp.code.interfaces.branchlookup import IBranchLookup
 from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
@@ -213,8 +214,7 @@
         # attribute is updated too.
         branch = self.factory.makeAnyBranch()
         new_owner = self.factory.makePerson()
-        login('[e-mail]')
-        branch.owner = new_owner
+        removeSecurityProxy(branch).owner = new_owner
         # Call the function that is normally called through the event system
         # to auto reload the fields updated by the db triggers.
         update_trigger_modified_fields(branch)
@@ -1054,9 +1054,9 @@
 
     def setUp(self):
         TestCaseWithFactory.setUp(self, '[e-mail]')
-        self.product = getUtility(IProductSet).getByName('firefox')
+        self.product = self.factory.makeProduct()
 
-        self.user = getUtility(IPersonSet).getByName('no-priv')
+        self.user = self.factory.makePerson()
         self.source = self.factory.makeProductBranch(
             name='source-branch', owner=self.user, product=self.product)
         self.target = self.factory.makeProductBranch(
@@ -1069,7 +1069,7 @@
 
     def test_junkSource(self):
         """Junk branches cannot be used as a source for merge proposals."""
-        self.source.product = None
+        self.source.setTarget(user=self.source.owner)
         self.assertRaises(
             InvalidBranchMergeProposal, self.source.addLandingTarget,
             self.user, self.target)
@@ -1078,12 +1078,13 @@
         """The product of the target branch must match the product of the
         source branch.
         """
-        self.target.product = None
+        self.target.setTarget(user=self.target.owner)
         self.assertRaises(
             InvalidBranchMergeProposal, self.source.addLandingTarget,
             self.user, self.target)
 
-        self.target.product = getUtility(IProductSet).getByName('bzr')
+        project = self.factory.makeProduct()
+        self.target.setTarget(user=self.target.owner, project=project)
         self.assertRaises(
             InvalidBranchMergeProposal, self.source.addLandingTarget,
             self.user, self.target)
@@ -1097,12 +1098,13 @@
     def test_dependentBranchSameProduct(self):
         """The dependent branch, if it is there, must be for the same product.
         """
-        self.dependent.product = None
+        self.dependent.setTarget(user=self.dependent.owner)
         self.assertRaises(
             InvalidBranchMergeProposal, self.source.addLandingTarget,
             self.user, self.target, self.dependent)
 
-        self.dependent.product = getUtility(IProductSet).getByName('bzr')
+        project = self.factory.makeProduct()
+        self.dependent.setTarget(user=self.dependent.owner, project=project)
         self.assertRaises(
             InvalidBranchMergeProposal, self.source.addLandingTarget,
             self.user, self.target, self.dependent)
@@ -1650,5 +1652,138 @@
         self.assertEqual(branch.spec_links.count(), 0)
 
 
+class TestBranchSetOwner(TestCaseWithFactory):
+    """Tests for IBranch.setOwner."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_owner_sets_team(self):
+        # The owner of the branch can set the owner of the branch to be a team
+        # they are a member of.
+        branch = self.factory.makeAnyBranch()
+        team = self.factory.makeTeam(owner=branch.owner)
+        login_person(branch.owner)
+        branch.setOwner(team, branch.owner)
+        self.assertEqual(team, branch.owner)
+
+    def test_owner_cannot_set_nonmember_team(self):
+        # The owner of the branch cannot set the owner to be a team they are
+        # not a member of.
+        branch = self.factory.makeAnyBranch()
+        team = self.factory.makeTeam()
+        login_person(branch.owner)
+        self.assertRaises(
+            BranchCreatorNotMemberOfOwnerTeam,
+            branch.setOwner,
+            team, branch.owner)
+
+    def test_owner_cannot_set_other_user(self):
+        # The owner of the branch cannot set the new owner to be another
+        # person.
+        branch = self.factory.makeAnyBranch()
+        person = self.factory.makePerson()
+        login_person(branch.owner)
+        self.assertRaises(
+            BranchCreatorNotOwner,
+            branch.setOwner,
+            person, branch.owner)
+
+    def test_admin_can_set_any_team_or_person(self):
+        # A Launchpad admin can set the branch to be owned by any team or
+        # person.
+        branch = self.factory.makeAnyBranch()
+        team = self.factory.makeTeam()
+        # To get a random administrator, choose the admin team owner.
+        admin = getUtility(ILaunchpadCelebrities).admin.teamowner
+        login_person(admin)
+        branch.setOwner(team, admin)
+        self.assertEqual(team, branch.owner)
+        person = self.factory.makePerson()
+        branch.setOwner(person, admin)
+        self.assertEqual(person, branch.owner)
+
+    def test_bazaar_experts_can_set_any_team_or_person(self):
+        # A bazaar expert can set the branch to be owned by any team or
+        # person.
+        branch = self.factory.makeAnyBranch()
+        team = self.factory.makeTeam()
+        # To get a random administrator, choose the admin team owner.
+        experts = getUtility(ILaunchpadCelebrities).bazaar_experts.teamowner
+        login_person(experts)
+        branch.setOwner(team, experts)
+        self.assertEqual(team, branch.owner)
+        person = self.factory.makePerson()
+        branch.setOwner(person, experts)
+        self.assertEqual(person, branch.owner)
+
+
+class TestBranchSetTarget(TestCaseWithFactory):
+    """Tests for IBranch.setTarget."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_junk_branch_to_project_branch(self):
+        # A junk branch can be moved to a project.
+        branch = self.factory.makePersonalBranch()
+        project = self.factory.makeProduct()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner, project=project)
+        self.assertEqual(project, branch.target.context)
+
+    def test_junk_branch_to_package_branch(self):
+        # A junk branch can be moved to a source package.
+        branch = self.factory.makePersonalBranch()
+        source_package = self.factory.makeSourcePackage()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner, source_package=source_package)
+        self.assertEqual(source_package, branch.target.context)
+
+    def test_project_branch_to_other_project_branch(self):
+        # Move a branch from one project to another.
+        branch = self.factory.makeProductBranch()
+        project = self.factory.makeProduct()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner, project=project)
+        self.assertEqual(project, branch.target.context)
+
+    def test_project_branch_to_package_branch(self):
+        # Move a branch from a project to a package.
+        branch = self.factory.makeProductBranch()
+        source_package = self.factory.makeSourcePackage()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner, source_package=source_package)
+        self.assertEqual(source_package, branch.target.context)
+
+    def test_project_branch_to_junk_branch(self):
+        # Move a branch from a project to junk.
+        branch = self.factory.makeProductBranch()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner)
+        self.assertEqual(branch.owner, branch.target.context)
+
+    def test_package_branch_to_other_package_branch(self):
+        # Move a branch from one package to another.
+        branch = self.factory.makePackageBranch()
+        source_package = self.factory.makeSourcePackage()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner, source_package=source_package)
+        self.assertEqual(source_package, branch.target.context)
+
+    def test_package_branch_to_project_branch(self):
+        # Move a branch from a package to a project.
+        branch = self.factory.makePackageBranch()
+        project = self.factory.makeProduct()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner, project=project)
+        self.assertEqual(project, branch.target.context)
+
+    def test_package_branch_to_junk_branch(self):
+        # Move a branch from a package to junk.
+        branch = self.factory.makePackageBranch()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner)
+        self.assertEqual(branch.owner, branch.target.context)
+
+
 def test_suite():
     return TestLoader().loadTestsFromName(__name__)
 
=== renamed file 'lib/lp/code/tests/test_branchnamespace.py' => 'lib/lp/code/model/tests/test_branchnamespace.py'
--- lib/lp/code/tests/test_branchnamespace.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/model/tests/test_branchnamespace.py	2009-07-29 02:16:43 +0000
@@ -1871,5 +1871,70 @@
             BranchCreatorNotOwner, self.albert, self.doug)
 
 
+class TestBranchNamespaceMoveBranch(TestCaseWithFactory):
+    """Test the IBranchNamespace.moveBranch method.
+
+    The edge cases of the validateMove are tested in the NamespaceMixin for
+    each of the namespaces.
+    """
+
+    layer = DatabaseFunctionalLayer
+
+    def assertNamespacesEqual(self, expected, result):
+        """Assert that the namespaces refer to the same thing.
+
+        The name of the namespace contains the user name and the context
+        parts, so is the easiest thing to check.
+        """
+        self.assertEqual(expected.name, result.name)
+
+    def test_move_to_same_namespace(self):
+        # Moving to the same namespace is effectively a no-op.  No exceptions
+        # about matching branch names should be raised.
+        branch = self.factory.makeAnyBranch()
+        namespace = branch.namespace
+        namespace.moveBranch(branch, branch.owner)
+        self.assertNamespacesEqual(namespace, branch.namespace)
+
+    def test_name_clash_raises(self):
+        # A name clash will raise an exception.
+        branch = self.factory.makeAnyBranch(name="test")
+        another = self.factory.makeAnyBranch(owner=branch.owner, name="test")
+        namespace = another.namespace
+        self.assertRaises(
+            BranchExists, namespace.moveBranch, branch, branch.owner)
+
+    def test_move_with_rename(self):
+        # A name clash with 'rename_if_necessary' set to True will cause the
+        # branch to be renamed instead of raising an error.
+        branch = self.factory.makeAnyBranch(name="test")
+        another = self.factory.makeAnyBranch(owner=branch.owner, name="test")
+        namespace = another.namespace
+        namespace.moveBranch(branch, branch.owner, rename_if_necessary=True)
+        self.assertEqual("test-1", branch.name)
+        self.assertNamespacesEqual(namespace, branch.namespace)
+
+    def test_move_with_new_name(self):
+        # A new name for the branch can be specified as part of the move.
+        branch = self.factory.makeAnyBranch(name="test")
+        another = self.factory.makeAnyBranch(owner=branch.owner, name="test")
+        namespace = another.namespace
+        namespace.moveBranch(branch, branch.owner, new_name="foo")
+        self.assertEqual("foo", branch.name)
+        self.assertNamespacesEqual(namespace, branch.namespace)
+
+    def test_sets_branch_owner(self):
+        # Moving to a new namespace may change the owner of the branch if the
+        # owner of the namespace is different.
+        branch = self.factory.makeAnyBranch(name="test")
+        team = self.factory.makeTeam(branch.owner)
+        product = self.factory.makeProduct()
+        namespace = ProductNamespace(team, product)
+        namespace.moveBranch(branch, branch.owner)
+        self.assertEqual(team, branch.owner)
+        # And for paranoia.
+        self.assertNamespacesEqual(namespace, branch.namespace)
+
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)
 
=== modified file 'lib/lp/code/model/tests/test_branchtarget.py'
--- lib/lp/code/model/tests/test_branchtarget.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/model/tests/test_branchtarget.py	2009-07-28 05:20:23 +0000
@@ -46,6 +46,24 @@
         branches = self.target.collection.getBranches()
         self.assertEqual([branch], list(branches))
 
+    def test_retargetBranch_packageBranch(self):
+        # Retarget an existing package branch to this target.
+        branch = self.factory.makePackageBranch()
+        self.target.retargetBranch(branch)
+        self.assertEqual(self.target, branch.target)
+
+    def test_retargetBranch_productBranch(self):
+        # Retarget an existing product branch to this target.
+        branch = self.factory.makeProductBranch()
+        self.target.retargetBranch(branch)
+        self.assertEqual(self.target, branch.target)
+
+    def test_retargetBranch_personalBranch(self):
+        # Retarget an existing personal branch to this target.
+        branch = self.factory.makePersonalBranch()
+        self.target.retargetBranch(branch)
+        self.assertEqual(self.target, branch.target)
+
 
 class TestPackageBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):
 
@@ -212,6 +230,33 @@
         # The default merge target is always None.
         self.assertIs(None, self.target.default_merge_target)
 
+    def test_retargetBranch_packageBranch(self):
+        # Retarget an existing package branch to this target.  Override the
+        # mixin tests, and specify the owner of the branch.  This is needed to
+        # match the target as the target is the branch owner for a personal
+        # branch.
+        branch = self.factory.makePackageBranch(owner=self.original)
+        self.target.retargetBranch(branch)
+        self.assertEqual(self.target, branch.target)
+
+    def test_retargetBranch_productBranch(self):
+        # Retarget an existing product branch to this target.  Override the
+        # mixin tests, and specify the owner of the branch.  This is needed to
+        # match the target as the target is the branch owner for a personal
+        # branch.
+        branch = self.factory.makeProductBranch(owner=self.original)
+        self.target.retargetBranch(branch)
+        self.assertEqual(self.target, branch.target)
+
+    def test_retargetBranch_personalBranch(self):
+        # Retarget an existing personal branch to this target.  Override the
+        # mixin tests, and specify the owner of the branch.  This is needed to
+        # match the target as the target is the branch owner for a personal
+        # branch.
+        branch = self.factory.makePersonalBranch(owner=self.original)
+        self.target.retargetBranch(branch)
+        self.assertEqual(self.target, branch.target)
+
 
 class TestProductBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):
 
 
=== modified file 'lib/lp/code/model/tests/test_revision.py'
--- lib/lp/code/model/tests/test_revision.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/model/tests/test_revision.py	2009-08-02 23:17:10 +0000
@@ -146,7 +146,8 @@
         branch.createBranchRevision(1, rev)
         # Once the branch is connected to the revision, we now specify
         # a product for the branch.
-        branch.product = self.factory.makeProduct()
+        project = self.factory.makeProduct()
+        branch.setTarget(user=branch.owner, project=project)
         # The revision is now identified as needing karma allocated.
         self.assertEqual(
             [rev], list(RevisionSet.getRevisionsNeedingKarmaAllocated()))
 
=== modified file 'lib/lp/code/scripts/tests/test_revisionkarma.py'
--- lib/lp/code/scripts/tests/test_revisionkarma.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/scripts/tests/test_revisionkarma.py	2009-08-02 23:17:10 +0000
@@ -49,7 +49,8 @@
         branch.createBranchRevision(1, rev)
         # Once the branch is connected to the revision, we now specify
         # a product for the branch.
-        branch.product = self.factory.makeProduct()
+        project=self.factory.makeProduct()
+        branch.setTarget(user=branch.owner, project=project)
         # Commit and switch to the script db user.
         transaction.commit()
         LaunchpadZopelessLayer.switchDbUser(config.revisionkarma.dbuser)
 
=== modified file 'lib/lp/codehosting/tests/test_acceptance.py'
--- lib/lp/codehosting/tests/test_acceptance.py	2009-07-17 00:26:05 +0000
+++ lib/lp/codehosting/tests/test_acceptance.py	2009-08-02 23:38:51 +0000
@@ -8,13 +8,13 @@
 import atexit
 import os
 import unittest
-from xml.dom.minidom import parseString
 import xmlrpclib
 
 import bzrlib.branch
 from bzrlib.tests import TestCaseWithTransport
 from bzrlib.urlutils import local_path_from_url
 from bzrlib.workingtree import WorkingTree
+from zope.security.proxy import removeSecurityProxy
 
 from lp.codehosting.bzrutils import DenyingServer
 from lp.codehosting.tests.helpers import (
@@ -382,7 +382,8 @@
         # rename as far as bzr is concerned: the URL changes.
         LaunchpadZopelessTestSetup().txn.begin()
         branch = self.getDatabaseBranch('testuser', None, 'test-branch')
-        branch.product = database.Product.byName('firefox')
+        removeSecurityProxy(branch).product = database.Product.byName(
+            'firefox')
         LaunchpadZopelessTestSetup().txn.commit()
 
         self.assertNotBranch(
 
=== modified file 'lib/lp/registry/browser/tests/private-team-creation-views.txt'
--- lib/lp/registry/browser/tests/private-team-creation-views.txt	2009-06-16 17:12:30 +0000
+++ lib/lp/registry/browser/tests/private-team-creation-views.txt	2009-08-03 01:02:44 +0000
@@ -400,8 +400,7 @@
     ...     bugtask = bug.default_bugtask
     ...     bugtask.transitionToAssignee(team)
     ...     # A branch.
-    ...     branch = factory.makeBranch()
-    ...     branch.owner = team
+    ...     branch = factory.makeBranch(owner=team)
     ...     # A branch subscription.
     ...     from lp.code.enums import (
     ...         BranchSubscriptionDiffSize,
 
=== modified file 'lib/lp/registry/doc/private-team-roles.txt'
--- lib/lp/registry/doc/private-team-roles.txt	2009-06-19 01:27:32 +0000
+++ lib/lp/registry/doc/private-team-roles.txt	2009-08-03 00:41:34 +0000
@@ -23,9 +23,10 @@
 -----------------
 
     >>> # Create the necessary teams.
-    >>> from lp.registry.interfaces.person import PersonVisibility
     >>> team_owner = factory.makePerson(name='team-owner')
-    >>> login('[e-mail]')
+    >>> from lp.registry.interfaces.person import IPersonSet, PersonVisibility
+    >>> admin_user = getUtility(IPersonSet).getByEmail('[e-mail]')
+    >>> login_person(admin_user)
     >>> priv_team = factory.makeTeam(name='private-team',
     ...     owner=team_owner,
     ...     visibility=PersonVisibility.PRIVATE)
@@ -102,12 +103,12 @@
 Private teams can be assigned as the owner of a branch
 
     >>> branch = factory.makeBranch()
-    >>> branch.owner = priv_team
+    >>> branch.setOwner(priv_team, user=admin_user)
 
 But private membership teams cannot own a branch.
 
     >>> branch = factory.makeBranch()
-    >>> branch.owner = pm_team
+    >>> branch.setOwner(pm_team, user=admin_user)
     Traceback (most recent call last):
     ...
     PrivatePersonLinkageError: Cannot link person