launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/delete-recipes-during-merge into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #669288 person merge code needs to handle colliding recipes
  https://bugs.launchpad.net/bugs/669288
  #683798 Recipes for merged team still showing up in +recipes page
  https://bugs.launchpad.net/bugs/683798

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/delete-recipes-during-merge/+merge/47630

Also delete SourcePackageRecipes while merging people.
-- 
https://code.launchpad.net/~stevenk/launchpad/delete-recipes-during-merge/+merge/47630
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/delete-recipes-during-merge into lp:launchpad.

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2011-01-19 21:32:21 +0000
+++ lib/lp/registry/model/person.py	2011-01-27 04:29:00 +0000
@@ -3371,6 +3371,15 @@
             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)
+        for recipe in recipes:
+            recipe.destroySelf()
+
     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
@@ -3871,8 +3880,7 @@
         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-01-18 14:59:44 +0000
+++ lib/lp/registry/tests/test_person.py	2011-01-27 04:29:00 +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,36 @@
             test_team,
             target_team)
 
+    def test_merge_deletes_recipes(self):
+        # When person/teams are merged, recipes they owned are deleted.
+        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(0, 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
+        # deleted.
+        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(1, recipes.count())
+        self.assertEqual(u'TO', recipes[0].description)
 
 class TestPersonSetCreateByOpenId(TestCaseWithFactory):
     layer = DatabaseFunctionalLayer