launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02480
[Merge] lp:~stevenk/launchpad/merge-recipes-during-merge into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/merge-recipes-during-merge into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#506630 account merge code for branches doesn't handle source package branches
https://bugs.launchpad.net/bugs/506630
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/merge-recipes-during-merge/+merge/48264
Readd the code that deals with recipes on merge -- but merges them into the target person, rather than just deleting them. I also refactored the branch merging code to deal with package branches at the same time.
--
https://code.launchpad.net/~stevenk/launchpad/merge-recipes-during-merge/+merge/48264
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/merge-recipes-during-merge into lp:launchpad.
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2011-02-01 01:40:34 +0000
+++ lib/lp/registry/model/person.py 2011-02-02 03:07:59 +0000
@@ -3340,37 +3340,36 @@
(decorator_table.lower(), person_pointer_column.lower()))
def _mergeBranches(self, cur, from_id, to_id):
- # XXX MichaelHudson 2010-01-13 bug=506630: This code does not account
- # for package branches.
- cur.execute('''
- SELECT product, name FROM Branch WHERE owner = %(to_id)d
- ''' % vars())
- possible_conflicts = set(tuple(r) for r in cur.fetchall())
- cur.execute('''
- SELECT id, product, name FROM Branch WHERE owner = %(from_id)d
- ORDER BY id
- ''' % vars())
- for id, product, name in list(cur.fetchall()):
- new_name = name
- suffix = 1
- while (product, new_name) in possible_conflicts:
- new_name = '%s-%d' % (name, suffix)
- suffix += 1
- possible_conflicts.add((product, new_name))
- new_name = new_name.encode('US-ASCII')
- name = name.encode('US-ASCII')
- cur.execute('''
- UPDATE Branch SET owner = %(to_id)s, name = %(new_name)s
- WHERE owner = %(from_id)s AND name = %(name)s
- AND (%(product)s IS NULL OR product = %(product)s)
- ''', dict(to_id=to_id, from_id=from_id,
- name=name, new_name=new_name, product=product))
+ # Avoid circular imports.
+ from lp.code.model.branch import Branch
+ store = IMasterStore(Branch)
+ branches = store.find(Branch, Branch.owner == from_id)
+ for branch in branches:
+ branch.setOwner(to_id, to_id)
def _mergeBranchMergeQueues(self, cur, from_id, to_id):
cur.execute('''
UPDATE BranchMergeQueue SET owner = %(to_id)s WHERE owner =
%(from_id)s''', dict(to_id=to_id, from_id=from_id))
+ def _mergeSourcePackageRecipes(self, cur, from_id, to_id):
+ from lp.code.model.sourcepackagerecipe import SourcePackageRecipe
+ store = IMasterStore(SourcePackageRecipe)
+ recipes = store.find(
+ SourcePackageRecipe, SourcePackageRecipe.owner == from_id)
+ to_recipes = store.find(
+ SourcePackageRecipe, SourcePackageRecipe.owner == to_id)
+ existing_names = [r.name for r in to_recipes]
+ for recipe in recipes:
+ new_name = recipe.name
+ count = 1
+ while new_name in existing_names:
+ new_name = '%s-%s' % (recipe.name, count)
+ count += 1
+ nr = removeSecurityProxy(recipe)
+ nr.owner = to_id
+ nr.name = new_name
+
def _mergeMailingListSubscriptions(self, cur, from_id, to_id):
# Update MailingListSubscription. Note that since all the from_id
# email addresses are set to NEW, all the subscriptions must be
@@ -3865,14 +3864,13 @@
# Update the Branches that will not conflict, and fudge the names of
# ones that *do* conflict.
- self._mergeBranches(cur, from_id, to_id)
+ self._mergeBranches(cur, from_person, to_person)
skip.append(('branch', 'owner'))
self._mergeBranchMergeQueues(cur, from_id, to_id)
skip.append(('branchmergequeue', 'owner'))
- # XXX MichaelHudson 2010-01-13: Write _mergeSourcePackageRecipes!
- #self._mergeSourcePackageRecipes(cur, from_id, to_id))
+ self._mergeSourcePackageRecipes(cur, from_id, to_id)
skip.append(('sourcepackagerecipe', 'owner'))
self._mergeMailingListSubscriptions(cur, from_id, to_id)
=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py 2011-02-01 01:40:34 +0000
+++ lib/lp/registry/tests/test_person.py 2011-02-02 03:07:59 +0000
@@ -17,10 +17,6 @@
from canonical.database.sqlbase import cursor
from canonical.launchpad.database.account import Account
from canonical.launchpad.database.emailaddress import EmailAddress
-from canonical.launchpad.ftests import (
- ANONYMOUS,
- login,
- )
from canonical.launchpad.interfaces.account import (
AccountCreationRationale,
AccountStatus,
@@ -62,9 +58,14 @@
from lp.registry.model.person import Person
from lp.registry.model.structuralsubscription import StructuralSubscription
from lp.services.openid.model.openididentifier import OpenIdIdentifier
-from lp.soyuz.enums import ArchivePurpose
+from lp.soyuz.enums import (
+ ArchivePurpose,
+ ArchiveStatus,
+ )
from lp.testing import (
+ ANONYMOUS,
celebrity_logged_in,
+ login,
login_person,
logout,
person_logged_in,
@@ -681,6 +682,66 @@
test_team,
target_team)
+ def test_merge_moves_branches(self):
+ # When person/teams are merged, branches owned by the from person
+ # are copied.
+ person = self.factory.makePerson()
+ branch = self.factory.makeBranch()
+ self._do_premerge(branch.owner, person)
+ login_person(person)
+ self.person_set.merge(branch.owner, person)
+ branches = person.getBranches()
+ self.assertEqual(1, branches.count())
+
+ def test_merge_with_duplicated_branches(self):
+ # When person/teams are merged, if the from person has a branch with
+ # the same name as the to person, the from person's branches are
+ # renamed.
+ product = self.factory.makeProduct()
+ from_branch = self.factory.makeBranch(name='foo', product=product)
+ to_branch = self.factory.makeBranch(name='foo', product=product)
+ mergee = to_branch.owner
+ self._do_premerge(from_branch.owner, mergee)
+ login_person(mergee)
+ self.person_set.merge(from_branch.owner, mergee)
+ branches = [b.name for b in mergee.getBranches()]
+ self.assertEqual(2, len(branches))
+ self.assertEqual([u'foo', u'foo-1'], branches)
+
+ def test_merge_moves_recipes(self):
+ # When person/teams are merged, recipes owned by the from person are
+ # copied.
+ person = self.factory.makePerson()
+ recipe = self.factory.makeSourcePackageRecipe()
+ # Disable the recipe owners PPA
+ with person_logged_in(recipe.owner):
+ recipe.owner.archive.status = ArchiveStatus.DELETED
+ self._do_premerge(recipe.owner, person)
+ login_person(person)
+ self.person_set.merge(recipe.owner, person)
+ self.assertEqual(1, person.getRecipes().count())
+
+ def test_merge_with_duplicated_recipes(self):
+ # When person/teams are merged, if the from person has a recipe with
+ # the same name as the to person, the from person's recipes are
+ # renamed.
+ merge_from = self.factory.makeSourcePackageRecipe(
+ name=u'foo', description=u'FROM')
+ merge_to = self.factory.makeSourcePackageRecipe(
+ name=u'foo', description=u'TO')
+ mergee = merge_to.owner
+ # Disable merge_from's PPA
+ with person_logged_in(merge_from.owner):
+ merge_from.owner.archive.status = ArchiveStatus.DELETED
+ self._do_premerge(merge_from.owner, mergee)
+ login_person(mergee)
+ self.person_set.merge(merge_from.owner, merge_to.owner)
+ recipes = mergee.getRecipes()
+ self.assertEqual(2, recipes.count())
+ descriptions = [r.description for r in recipes]
+ self.assertEqual([u'TO', u'FROM'], descriptions)
+ self.assertEqual(u'foo-1', recipes[1].name)
+
class TestPersonSetCreateByOpenId(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
