Merge lp:~sinzui/launchpad/person-merge-job-4 into lp:launchpad

[Curtis Hovey]

Enable async person/team merges from the UI.

    Launchpad bug:
        https://bugs.launchpad.net/bugs/736421
        https://bugs.launchpad.net/bugs/697530
    Pre-implementation: allenap
    Test command: ./bin/test -vv \
      -t test_is_merge_pending -t peoplemerge
      -t /xx-adminpeoplemerge -t xx-adminteammerge

- Update call-sites to use mergeAsync() where appropriate.
- UI to show if a user is the subject of a scheduled merge request.

--------------------------------------------------------------------

RULES

    * Add an info notification to the person/team index page that states
      the team is involved in a merge.
    * Ensure a merge cannot be requested is the user/team is involved
      in a queued merge
    * Change the view the call Person.merge_async() and show a message
      explaining that the merge is queued.
    * ADDENDUM. Fix the ambiguous "Constraint not satisfied" message.

QA

    * Using one of the unmergeable users, perform a merge.
    * Verify (quickly) you see the notification that the user is being merged.
    * Verify the user is merged after a few minutes.
    * Using and undeletable teams, perform a delete.
    * Verify (quickly) you see the notification that the team is being merged.
    * Verify the team is merged after a few minutes.

LINT

    lib/canonical/launchpad/browser/logintoken.py
    lib/lp/registry/browser/peoplemerge.py
    lib/lp/registry/browser/person.py
    lib/lp/registry/browser/tests/peoplemerge-views.txt
    lib/lp/registry/browser/tests/test_peoplemerge.py
    lib/lp/registry/browser/tests/test_person_view.py
    lib/lp/registry/browser/tests/test_team.py
    lib/lp/registry/interfaces/person.py
    lib/lp/registry/interfaces/persontransferjob.py
    lib/lp/registry/model/person.py
    lib/lp/registry/stories/person/xx-adminpeoplemerge.txt
    lib/lp/registry/stories/team/xx-adminteammerge.txt
    lib/lp/registry/tests/test_person.py

IMPLEMENTATION

Fixed the signature of mergeAsync() and create() methods and updated the
documentation. Added reviewer to Merge() to be compatible with the
aforementioned methods.
    lib/lp/registry/interfaces/person.py
    lib/lp/registry/interfaces/persontransferjob.py

Revised the rule for is_merge_pending because we care about merges in
the duplicate role. It is okay for a person or team to have several
dupes pending to merge. We expect this to be the case for team deletions
which are merges into ~registry.
    lib/lp/registry/model/person.py
    lib/lp/registry/tests/test_person.py

I deleted the duplicate validate() from the direct subclass. I think it
was left over from a refactoring. I discovered that the common validate()
is used by forms that do not provide a target user since it is assumed to
be the requester. If fixed it to fix the tests I was writing.
Added a check to ensure the duplicate and target do not have a pending merge.
Updated the logintoken and peoplemerge views to use merge_async().
Changed the messages to explain the merge is queued.
I was startled by the "Constraint not satisfied" in a test, since that is
a UI problem. I found a bug was reported about it so I fixed the message.
    lib/canonical/launchpad/browser/logintoken.py
    lib/lp/registry/browser/peoplemerge.py
    lib/lp/registry/browser/tests/peoplemerge-views.txt
    lib/lp/registry/browser/tests/test_peoplemerge.py

I deleted corner cases from these story tests because there are testing in
the view tests. I updated the stories to verify the messages users see.
    lib/lp/registry/stories/person/xx-adminpeoplemerge.txt
    lib/lp/registry/stories/team/xx-adminteammerge.txt

Added a notification to explain that the team or person is being merged.
    lib/lp/registry/browser/person.py
    lib/lp/registry/browser/tests/test_person_view.py
    lib/lp/registry/browser/tests/test_team.py

[Benji York (benji)]

This branch looks good.

One thing you might want to look at, I /think/ there is a hole in the
testing around the "is_merge_pending" property. I don't see that there
are any tests that the property is ever true.

review: Approve (code)

[Curtis Hovey (sinzui)]

There is a False test and a True test in lib/lp/registry/tests/test_person.py:

    def test_no_merge_pending(self):
        # is_merge_pending returns False when this person is not the "from"
        # person of an active merge job.
        person = self.factory.makePerson()
        self.assertFalse(person.is_merge_pending)

    def test_is_merge_pending(self):
        # is_merge_pending returns True when this person is being merged with
        # another person in an active merge job.
        from_person = self.factory.makePerson()
        to_person = self.factory.makePerson()
        getUtility(IPersonSet).mergeAsync(from_person, to_person)
        self.assertTrue(from_person.is_merge_pending)
        self.assertFalse(to_person.is_merge_pending)

[Benji York (benji)]

On Mon, Apr 11, 2011 at 5:00 PM, Curtis Hovey
<email address hidden> wrote:
> There is a False test and a True test in lib/lp/registry/tests/test_person.py:

Ah. I missed that. Thanks.
--
Benji York

=== modified file 'lib/canonical/launchpad/browser/logintoken.py'
--- lib/canonical/launchpad/browser/logintoken.py	2011-02-16 17:31:11 +0000
+++ lib/canonical/launchpad/browser/logintoken.py	2011-04-12 02:08:36 +0000
@@ -605,10 +605,11 @@
         if emailset.getByPerson(self.dupe):
             self.mergeCompleted = False
             return
-
-        # Call Stuart's magic function which will reassign all of the dupe
-        # account's stuff to the user account.
-        getUtility(IPersonSet).merge(self.dupe, requester)
+        getUtility(IPersonSet).mergeAsync(
+            self.dupe, requester, reviewer=requester)
+        merge_message = _(
+            'A merge is queued and is expected to complete in a few minutes.')
+        self.request.response.addInfoNotification(merge_message)
         self.mergeCompleted = True
 
 
 
=== modified file 'lib/lp/registry/browser/peoplemerge.py'
--- lib/lp/registry/browser/peoplemerge.py	2011-03-25 23:42:33 +0000
+++ lib/lp/registry/browser/peoplemerge.py	2011-04-12 02:08:36 +0000
@@ -56,14 +56,14 @@
     def validate(self, data):
         """Check that user is not attempting to merge a person into itself."""
         dupe_person = data.get('dupe_person')
-        target_person = data.get('target_person')
-
-        if dupe_person == target_person and dupe_person is not None:
-            self.addError(_("You can't merge ${name} into itself.",
-                  mapping=dict(name=dupe_person.name)))
-        # We cannot merge if there is a PPA with published
-        # packages on the duplicate, unless that PPA is removed.
-        if dupe_person is not None:
+        target_person = data.get('target_person') or self.user
+        if dupe_person is None:
+            self.setFieldError(
+                'dupe_person', 'The duplicate is not a valid person or team.')
+        else:
+            if dupe_person == target_person:
+                self.addError(_("You can't merge ${name} into itself.",
+                      mapping=dict(name=dupe_person.name)))
             dupe_person_ppas = getUtility(IArchiveSet).getPPAOwnedByPerson(
                 dupe_person, statuses=[ArchiveStatus.ACTIVE,
                                        ArchiveStatus.DELETING])
@@ -73,6 +73,12 @@
                     "can be merged. It may take ten minutes to remove the "
                     "deleted PPA's files.",
                     mapping=dict(name=dupe_person.name)))
+            if dupe_person.is_merge_pending:
+                self.addError(_("${name} is already queued for merging.",
+                      mapping=dict(name=dupe_person.name)))
+        if target_person is not None and target_person.is_merge_pending:
+            self.addError(_("${name} is already queued for merging.",
+                  mapping=dict(name=target_person.name)))
 
 
 class AdminMergeBaseView(ValidatingMergeView):
@@ -84,7 +90,8 @@
     # subclasses.
     should_confirm_email_reassignment = False
     should_confirm_member_deactivation = False
-    merge_message = _('Merge completed successfully.')
+    merge_message = _(
+        'A merge is queued and is expected to complete in a few minutes.')
 
     dupe_person_emails = ()
     dupe_person = None
@@ -98,25 +105,6 @@
     def success_url(self):
         return canonical_url(self.target_person)
 
-    def validate(self, data):
-        """Check that user is not attempting to merge a person into itself."""
-        dupe_person = data.get('dupe_person')
-        target_person = data.get('target_person')
-        if dupe_person == target_person and dupe_person is not None:
-            self.addError(_("You can't merge ${name} into itself.",
-                  mapping=dict(name=dupe_person.name)))
-        # We cannot merge the teams if there is a PPA with published
-        # packages on the duplicate person, unless that PPA is removed.
-        dupe_person_ppas = getUtility(IArchiveSet).getPPAOwnedByPerson(
-            dupe_person, statuses=[ArchiveStatus.ACTIVE,
-                                   ArchiveStatus.DELETING])
-        if dupe_person_ppas is not None:
-            self.addError(_(
-                "${name} has a PPA that must be deleted before it "
-                "can be merged. It may take ten minutes to remove the "
-                "deleted PPA's files.",
-                mapping=dict(name=dupe_person.name)))
-
     def render(self):
         # Subclasses may define other actions that they will render manually
         # only in certain circunstances, so don't include them in the list of
@@ -151,7 +139,7 @@
                 naked_email.personID = self.target_person.id
                 naked_email.accountID = self.target_person.accountID
                 naked_email.status = EmailAddressStatus.NEW
-        getUtility(IPersonSet).merge(
+        job = getUtility(IPersonSet).mergeAsync(
             self.dupe_person, self.target_person, reviewer=self.user)
         self.request.response.addInfoNotification(self.merge_message)
         self.next_url = self.success_url
@@ -264,7 +252,7 @@
 
     page_title = 'Delete'
     field_names = ['dupe_person', 'target_person']
-    merge_message = _('Team deleted.')
+    merge_message = _('The team is queued to be deleted.')
 
     @property
     def label(self):
 
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2011-03-11 00:06:42 +0000
+++ lib/lp/registry/browser/person.py	2011-04-12 02:08:36 +0000
@@ -3391,6 +3391,14 @@
 
     def initialize(self):
         super(PersonIndexView, self).initialize()
+        if self.context.is_merge_pending:
+            if self.context.is_team:
+                merge_action = 'merged or deleted'
+            else:
+                merge_action = 'merged'
+            self.request.response.addInfoNotification(
+                "%s is queued to be be %s in a few minutes." % (
+                self.context.displayname, merge_action))
         if self.request.method == "POST":
             self.processForm()
 
 
=== modified file 'lib/lp/registry/browser/tests/peoplemerge-views.txt'
--- lib/lp/registry/browser/tests/peoplemerge-views.txt	2011-03-25 13:44:19 +0000
+++ lib/lp/registry/browser/tests/peoplemerge-views.txt	2011-04-12 02:08:36 +0000
@@ -31,13 +31,10 @@
     ...     person_set, '+adminteammerge', form=form)
     >>> len(view.errors)
     0
-
-The old team is now renamed but the target team remains.
-
-    >>> print person_set.getByName('name21')
-    None
-    >>> print person_set.getByName('ubuntu-team').displayname
-    Ubuntu Team
+    >>> for notification in view.request.response.notifications:
+    ...     print notification.message
+    A merge is queued and is expected to complete in a few minutes.
+
 
 Attempting to merge a non-existent team results in an error.
 
@@ -76,12 +73,9 @@
     >>> view = create_initialized_view(
     ...     person_set, '+requestmerge', form=form)
     >>> len(view.errors)
-    1
-
-    # XXX Salgado, 2009-07-08, bug=396410: There should be a meaningful
-    # error message here instead of this one.
+    2
     >>> print view.getFieldError('dupe_person')
-    Constraint not satisfied
+    The duplicate is not a valid person or team.
 
 Admins, on the other hand, are allowed to merge people without a single email
 address.
@@ -181,18 +175,15 @@
 
     >>> for notification in view.request.response.notifications:
     ...     print notification.message
-    Team deleted.
+    The team is queued to be deleted.
 
     >>> print view.next_url
     http://launchpad.dev/people
 
-    >>> print person_set.getByName('deletable')
-    None
-
 The delete team view cannot be hacked to delete another team, or change
 the team that the merge operation is performed with.
 
-    >>> deletable_team = factory.makeTeam(owner=team_owner, name='deletable')
+    >>> deletable_team = factory.makeTeam(owner=team_owner, name='delete-me')
     >>> transaction.commit()
     >>> form = {
     ...     'field.target_person': 'rosetta-admins',
@@ -206,9 +197,6 @@
     >>> view.request.response.notifications
     []
 
-    >>> print deletable_team.name
-    deletable
-
 A team with a mailing list cannot be deleted.
 
     >>> team, mailing_list = factory.makeTeamAndMailingList(
@@ -246,4 +234,4 @@
     []
     >>> for notification in view.request.response.notifications:
     ...     print notification.message
-    Team deleted.
+    The team is queued to be deleted.
 
=== modified file 'lib/lp/registry/browser/tests/person-views.txt'
--- lib/lp/registry/browser/tests/person-views.txt	2010-12-06 23:18:20 +0000
+++ lib/lp/registry/browser/tests/person-views.txt	2011-04-12 02:08:36 +0000
@@ -121,6 +121,7 @@
 email addresses state is LOGIN_REQUIRED, there is no description, nor
 are there any email addresses.
 
+    >>> login(ANONYMOUS)
     >>> view = create_initialized_view(mark, '+index')
     >>> view.email_address_visibility.is_login_required
     True
 
=== modified file 'lib/lp/registry/browser/tests/test_peoplemerge.py'
--- lib/lp/registry/browser/tests/test_peoplemerge.py	2011-03-25 13:44:19 +0000
+++ lib/lp/registry/browser/tests/test_peoplemerge.py	2011-04-12 02:08:36 +0000
@@ -6,15 +6,12 @@
 
 from zope.component import getUtility
 
-from canonical.launchpad.interfaces.emailaddress import (
-    EmailAddressStatus,
-    IEmailAddressSet,
-    )
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.registry.interfaces.person import (
     IPersonSet,
     TeamSubscriptionPolicy,
     )
+from lp.registry.interfaces.persontransferjob import IPersonMergeJobSource
 from lp.testing import (
     login_celebrity,
     login_person,
@@ -27,33 +24,70 @@
     )
 
 
-class TestRequestPeopleMergeView(TestCaseWithFactory):
+class TestValidatingMergeView(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer
 
     def setUp(self):
-        super(TestRequestPeopleMergeView, self).setUp()
+        super(TestValidatingMergeView, self).setUp()
         self.person_set = getUtility(IPersonSet)
         self.dupe = self.factory.makePerson(name='dupe')
-        self.target = self.factory.makeTeam(name='target')
+        self.target = self.factory.makePerson(name='target')
+
+    def getForm(self, dupe_name=None):
+        if dupe_name is None:
+            dupe_name = self.dupe.name
+        return {
+            'field.dupe_person': dupe_name,
+            'field.target_person': self.target.name,
+            'field.actions.continue': 'Continue',
+            }
 
     def test_cannot_merge_person_with_ppas(self):
         # A team with a PPA cannot be merged.
         login_celebrity('admin')
         archive = self.dupe.createPPA()
         login_celebrity('registry_experts')
-        form = {
-            'field.dupe_person': self.dupe.name,
-            'field.target_person': self.target.name,
-            'field.actions.continue': 'Continue',
-            }
+        view = create_initialized_view(
+            self.person_set, '+requestmerge', form=self.getForm())
+        self.assertEqual(
+            [u"dupe has a PPA that must be deleted before it can be "
+              "merged. It may take ten minutes to remove the deleted PPA's "
+              "files."],
+            view.errors)
+
+    def test_cannot_merge_person_with_itself(self):
+        # A IPerson cannot be merged with itself.
+        login_person(self.target)
+        form = self.getForm(dupe_name=self.target.name)
         view = create_initialized_view(
             self.person_set, '+requestmerge', form=form)
         self.assertEqual(
-            [u"dupe has a PPA that must be deleted before it can be "
-              "merged. It may take ten minutes to remove the deleted PPA's "
-              "files."],
-            view.errors)
+            ["You can't merge target into itself."], view.errors)
+
+    def test_cannot_merge_dupe_person_with_an_existing_merge_job(self):
+        # A merge cannot be requested for an IPerson if it there is a job
+        # queued to merge it into another IPerson.
+        job_source = getUtility(IPersonMergeJobSource)
+        duplicate_job = job_source.create(
+            from_person=self.dupe, to_person=self.target)
+        login_person(self.target)
+        view = create_initialized_view(
+            self.person_set, '+requestmerge', form=self.getForm())
+        self.assertEqual(
+            ["dupe is already queued for merging."], view.errors)
+
+    def test_cannot_merge_target_person_with_an_existing_merge_job(self):
+        # A merge cannot be requested for an IPerson if it there is a job
+        # queued to merge it into another IPerson.
+        job_source = getUtility(IPersonMergeJobSource)
+        duplicate_job = job_source.create(
+            from_person=self.target, to_person=self.dupe)
+        login_person(self.target)
+        view = create_initialized_view(
+            self.person_set, '+requestmerge', form=self.getForm())
+        self.assertEqual(
+            ["target is already queued for merging."], view.errors)
 
 
 class TestRequestPeopleMergeMultipleEmailsView(TestCaseWithFactory):
@@ -135,16 +169,6 @@
         return create_initialized_view(
             self.person_set, '+adminteammerge', form=form)
 
-    def test_merge_team_with_email_address(self):
-        # Team email addresses are not transferred.
-        self.factory.makeEmail(
-            "[e-mail]", self.dupe_team, email_status=EmailAddressStatus.NEW)
-        view = self.getView()
-        self.assertEqual([], view.errors)
-        self.assertEqual(self.target_team, self.dupe_team.merged)
-        emails = getUtility(IEmailAddressSet).getByPerson(self.target_team)
-        self.assertEqual(0, emails.count())
-
     def test_cannot_merge_team_with_ppa(self):
         # A team with a PPA cannot be merged.
         login_celebrity('admin')
 
=== modified file 'lib/lp/registry/browser/tests/test_person_view.py'
--- lib/lp/registry/browser/tests/test_person_view.py	2011-03-08 23:06:21 +0000
+++ lib/lp/registry/browser/tests/test_person_view.py	2011-04-12 02:08:36 +0000
@@ -6,10 +6,7 @@
 import transaction
 from storm.expr import LeftJoin
 from storm.store import Store
-from testtools.matchers import (
-    Equals,
-    LessThan,
-    )
+from testtools.matchers import LessThan
 from zope.component import getUtility
 
 from canonical.config import config
@@ -43,6 +40,7 @@
     PersonVisibility,
     IPersonSet,
     )
+from lp.registry.interfaces.persontransferjob import IPersonMergeJobSource
 from lp.registry.interfaces.teammembership import (
     ITeamMembershipSet,
     TeamMembershipStatus,
@@ -69,6 +67,22 @@
     )
 
 
+class TestPersonIndexView(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_is_merge_pending(self):
+        dupe_person = self.factory.makePerson(name='finch')
+        target_person = self.factory.makePerson()
+        job_source = getUtility(IPersonMergeJobSource)
+        job_source.create(from_person=dupe_person, to_person=target_person)
+        view = create_initialized_view(dupe_person, name="+index")
+        notifications = view.request.response.notifications
+        message = 'Finch is queued to be be merged in a few minutes.'
+        self.assertEqual(1, len(notifications))
+        self.assertEqual(message, notifications[0].message)
+
+
 class TestPersonViewKarma(TestCaseWithFactory):
 
     layer = LaunchpadZopelessLayer
 
=== modified file 'lib/lp/registry/browser/tests/test_team.py'
--- lib/lp/registry/browser/tests/test_team.py	2010-12-03 00:03:15 +0000
+++ lib/lp/registry/browser/tests/test_team.py	2011-04-12 02:08:36 +0000
@@ -3,9 +3,12 @@
 
 __metaclass__ = type
 
+from zope.component import getUtility
+
 from canonical.launchpad.webapp.publisher import canonical_url
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.registry.browser.person import TeamOverviewMenu
+from lp.registry.interfaces.persontransferjob import IPersonMergeJobSource
 from lp.testing import (
     login_person,
     TestCaseWithFactory,
@@ -149,3 +152,17 @@
     def test_add_member_step_title(self):
         view = create_initialized_view(self.team, '+index')
         self.assertEqual('Search', view.add_member_step_title)
+
+    def test_is_merge_pending(self):
+        target_team = self.factory.makeTeam()
+        job_source = getUtility(IPersonMergeJobSource)
+        job_source.create(
+            from_person=self.team, to_person=target_team,
+            reviewer=target_team.teamowner)
+        view = create_initialized_view(self.team, name="+index")
+        notifications = view.request.response.notifications
+        message = (
+            'Test Team is queued to be be merged or deleted '
+            'in a few minutes.')
+        self.assertEqual(1, len(notifications))
+        self.assertEqual(message, notifications[0].message)
 
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2011-03-23 16:28:51 +0000
+++ lib/lp/registry/interfaces/person.py	2011-04-12 02:08:36 +0000
@@ -1428,12 +1428,14 @@
         "The number of real people who are members of this team.")
     # activemembers.value_type.schema will be set to IPerson once
     # IPerson is defined.
-    activemembers = Attribute('List of direct members with ADMIN or APPROVED status')
+    activemembers = Attribute(
+        'List of direct members with ADMIN or APPROVED status')
     # For the API we need eager loading
     api_activemembers = exported(
         doNotSnapshot(
             CollectionField(
-                title=_("List of direct members with ADMIN or APPROVED status"),
+                title=_(
+                    "List of direct members with ADMIN or APPROVED status"),
                 value_type=Reference(schema=Interface))),
         exported_as='members')
     adminmembers = exported(
@@ -2170,7 +2172,7 @@
     def latest_teams(limit=5):
         """Return the latest teams registered, up to the limit specified."""
 
-    def mergeAsync(from_person, to_person):
+    def mergeAsync(from_person, to_person, reviewer=None):
         """Merge a person/team into another asynchronously.
 
         This schedules a call to `merge()` to happen outside of the current
@@ -2179,10 +2181,13 @@
         guaranteed to succeed. If either user is in a pending person merge
         job, None is returned.
 
+        :param from_person: An IPerson or ITeam that is a duplicate.
+        :param to_person: An IPerson or ITeam that is a master.
+        :param reviewer: An IPerson who approved the ITeam merger.
         :return: A `PersonMergeJob` or None.
         """
 
-    def merge(from_person, to_person):
+    def merge(from_person, to_person, reviewer=None):
         """Merge a person/team into another.
 
         The old person/team (from_person) will be left as an atavism.
@@ -2196,9 +2201,9 @@
         passing deactivate_members=True. In that case the user who's
         performing the merge must be provided as well.
 
-        We are not yet game to delete the `from_person` entry from the
-        database yet. We will let it roll for a while and see what cruft
-        develops. -- StuartBishop 20050812
+        :param from_person: An IPerson or ITeam that is a duplicate.
+        :param to_person: An IPerson or ITeam that is a master.
+        :param reviewer: An IPerson who approved the ITeam merger.
         """
 
     def getValidPersons(persons):
 
=== modified file 'lib/lp/registry/interfaces/persontransferjob.py'
--- lib/lp/registry/interfaces/persontransferjob.py	2011-03-26 21:50:45 +0000
+++ lib/lp/registry/interfaces/persontransferjob.py	2011-04-12 02:08:36 +0000
@@ -102,7 +102,7 @@
 class IPersonMergeJobSource(IJobSource):
     """An interface for acquiring IPersonMergeJobs."""
 
-    def create(from_person, to_person, review=None):
+    def create(from_person, to_person, reviewer=None):
         """Create a new IPersonMergeJob.
 
         None is returned if either the from_person or to_person are already
@@ -111,7 +111,7 @@
 
         :param from_person: An IPerson or ITeam that is a duplicate.
         :param to_person: An IPerson or ITeam that is a master.
-        :param review: An IPerson who approved ITeam merger.
+        :param reviewer: An IPerson who approved ITeam merger.
         """
 
     def find(from_person=None, to_person=None, any_person=False):
 
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2011-04-06 17:08:45 +0000
+++ lib/lp/registry/model/person.py	2011-04-12 02:08:36 +0000
@@ -2076,8 +2076,7 @@
     def is_merge_pending(self):
         """See `IPublicPerson`."""
         return not getUtility(
-            IPersonMergeJobSource).find(
-                from_person=self, to_person=self, any_person=True).is_empty()
+            IPersonMergeJobSource).find(from_person=self).is_empty()
 
     def visibilityConsistencyWarning(self, new_value):
         """Warning used when changing the team's visibility.
@@ -3836,10 +3835,10 @@
             naked_from_team.retractTeamMembership(team, reviewer)
         IStore(from_team).flush()
 
-    def mergeAsync(self, from_person, to_person):
+    def mergeAsync(self, from_person, to_person, reviewer=None):
         """See `IPersonSet`."""
         return getUtility(IPersonMergeJobSource).create(
-            from_person=from_person, to_person=to_person)
+            from_person=from_person, to_person=to_person, reviewer=reviewer)
 
     def merge(self, from_person, to_person, reviewer=None):
         """See `IPersonSet`."""
 
=== modified file 'lib/lp/registry/stories/person/merge-people.txt'
--- lib/lp/registry/stories/person/merge-people.txt	2010-01-12 00:19:20 +0000
+++ lib/lp/registry/stories/person/merge-people.txt	2011-04-12 02:08:36 +0000
@@ -12,12 +12,6 @@
     >>> foo = factory.makePerson(name='foo', email='[e-mail]')
     >>> logout()
 
-Workaround while https://launchpad.net/launchpad/+bug/39016 is not
-fixed.
-
-    >>> from lp.services.mail import stub
-    >>> stub.test_emails[:] = []
-
 First we have to go to the +requestmerge page.
 
     >>> browser.addHeader('Authorization', 'Basic [e-mail]:test')
@@ -25,23 +19,6 @@
     >>> print browser.title
     Merge Launchpad accounts
 
-If we try to merge a nonexistent account, we'll get an error page.
-
-    >>> browser.getControl('Duplicated Account').value = 'bar'
-    >>> browser.getControl('Continue').click()
-    >>> print "\n".join(get_feedback_messages(browser.contents))
-    There is 1 error.
-    Invalid value
-
-The same will happen if we try to merge a team.
-
-    >>> browser.getControl(
-    ...     'Duplicated Account').value = '[e-mail]'
-    >>> browser.getControl('Continue').click()
-    >>> print "\n".join(get_feedback_messages(browser.contents))
-    There is 1 error.
-    Invalid value
-
 In preparation for the actual merge (below), we add an extra email
 address to the 'foo' account, to show how things work when the dupe
 account has more than one email address.
@@ -70,6 +47,7 @@
 
 Make sure we haven't got leftovers from a previous test
 
+    >>> from lp.services.mail import stub
     >>> len(stub.test_emails)
     0
 
@@ -164,18 +142,6 @@
     >>> assert not stub.test_emails
     >>> token_url = get_token_url_from_email(raw_msg)
 
-    # XXX: Steve Alexander 2006-10-21: To prevent spurious failures in PQM.
-    #>>> browser.open(token_url)
-    #>>> browser.getControl('Confirm').click()
-    #>>> 'The accounts have been merged successfully' in browser.contents
-    #True
-
-Revisiting the mergerequest-sent page now safely redirects the user:
-
-    #>>> browser.open('http://launchpad.dev/people/+mergerequest-sent?dupe=55')
-    #>>> browser.url
-    #'http://launchpad.dev/~no-priv'
-
 If the duplicate account has multiple email addresses and has chosen
 to hide them the process is slightly different.  We cannot display the
 hidden addresses so instead we just inform the user to check all of
 
=== modified file 'lib/lp/registry/stories/person/xx-adminpeoplemerge.txt'
--- lib/lp/registry/stories/person/xx-adminpeoplemerge.txt	2010-10-10 15:39:28 +0000
+++ lib/lp/registry/stories/person/xx-adminpeoplemerge.txt	2011-04-12 02:08:36 +0000
@@ -1,4 +1,5 @@
-= Merging people or teams =
+Merging people or teams
+=======================
 
 Launchpad admins can merge any two people or teams, without the need of
 any email address confirmation or something like that.  There's one page
@@ -35,17 +36,6 @@
     >>> admin_browser.url
     'http://launchpad.dev/~salgado'
 
-    >>> from zope.component import getUtility
-    >>> from canonical.launchpad.ftests import ANONYMOUS, login, logout
-    >>> from lp.registry.interfaces.person import IPersonSet
-    >>> login(ANONYMOUS)
-    >>> person_set = getUtility(IPersonSet)
-    >>> person_set.getByEmail('[e-mail]').name
-    u'salgado'
-
-    >>> spiv = person_set.getByName('spiv-merged', ignore_merged=False)
-    >>> spiv.merged.name
-    u'salgado'
-
-    >>> logout()
+    >>> print get_feedback_messages(admin_browser.contents)[0]
+    A merge is queued and is expected to complete in a few minutes.
 
 
=== removed file 'lib/lp/registry/stories/person/xx-merge-person-with-hidden-email.txt'
--- lib/lp/registry/stories/person/xx-merge-person-with-hidden-email.txt	2009-09-10 23:58:31 +0000
+++ lib/lp/registry/stories/person/xx-merge-person-with-hidden-email.txt	1970-01-01 00:00:00 +0000
@@ -1,45 +0,0 @@
-= Merging people with hidden email addresses =
-
-In order to merge FooBarDupe into FooBar we need to send an email to
-FooBarDupe's email address with instructions for FooBar to complete the
-merge. The merge should succeed even if FooBarDupe has chosen to hide his
-email addresses.
-
-    # First we'll create profiles for FooBar and FooBarDupe, the latter with
-    # hidden email addresses.
-    >>> login(ANONYMOUS)
-    >>> foobar = factory.makePerson(
-    ...     email='[e-mail]', name='foobar', password='test')
-    >>> foobar_dupe = factory.makePerson(name='foobar-dupe')
-    >>> login_person(foobar_dupe)
-    >>> foobar_dupe.hide_email_addresses = True
-    >>> logout()
-
-FooBar will now request that FooBarDupe be merged into his account.
-
-    >>> browser = setupBrowser(auth="Basic [e-mail]:test")
-    >>> browser.open('http://launchpad.dev/people/+requestmerge')
-    >>> browser.getControl('Duplicated Account').value = 'foobar-dupe'
-    >>> browser.getControl('Continue').click()
-    >>> browser.url
-    'http://launchpad.dev/people/+mergerequest-sent?dupe=...
-    >>> print extract_text(find_main_content(browser.contents))
-    An email message was sent to...
-    Please follow the instructions on that message to complete the merge.
-
-Following the link included in the email sent to FooBarDupe's email he'll
-be able to complete the merge.
-
-    >>> from lp.services.mail import stub
-    >>> from canonical.launchpad.ftests.logintoken import (
-    ...     get_token_url_from_email)
-    >>> len(stub.test_emails)
-    1
-    >>> from_addr, to_addrs, raw_msg = stub.test_emails.pop()
-    >>> token_url = get_token_url_from_email(raw_msg)
-    >>> browser.open(token_url)
-    >>> browser.url
-    'http://launchpad.dev/token/.../+accountmerge'
-    >>> browser.getControl('Confirm').click()
-    >>> print "\n".join(get_feedback_messages(browser.contents))
-    The accounts have been merged successfully...
 
=== modified file 'lib/lp/registry/stories/team/xx-adminteammerge.txt'
--- lib/lp/registry/stories/team/xx-adminteammerge.txt	2010-10-18 22:24:59 +0000
+++ lib/lp/registry/stories/team/xx-adminteammerge.txt	2011-04-12 02:08:36 +0000
@@ -1,4 +1,5 @@
-= Merging Teams =
+Merging Teams
+=============
 
 There's a separate page for merging teams.  We can't merge teams with
 active members, so the user will first have to confirm that the
@@ -32,70 +33,5 @@
     >>> registry_browser.url
     'http://launchpad.dev/~guadamen'
 
-    >>> from canonical.launchpad.ftests import ANONYMOUS, login, logout
-    >>> from zope.component import getUtility
-    >>> from lp.registry.interfaces.mailinglist import IMailingListSet
-    >>> login(ANONYMOUS)
-    >>> new_team.merged.name
-    u'guadamen'
-    >>> logout()
-
-
-==  Non-display of merged teams ==
-
-Merged teams are not visible anymore.
-
-    >>> browser.open("http://launchpad.dev/~new-team-merged")
-    Traceback (most recent call last):
-    ...
-    NotFound:...
-
-
-== Corner cases ==
-
-If the duplicated team is associated with a mailing list, it can't be
-merged, though.
-
-    >>> login(ANONYMOUS)
-    >>> guadamen = getUtility(IPersonSet).getByName('guadamen')
-    >>> mailing_list = getUtility(IMailingListSet).new(guadamen)
-    >>> logout()
-
-    >>> registry_browser.open('http://launchpad.dev/people/+adminteammerge')
-    >>> registry_browser.getControl('Duplicated Team').value = 'guadamen'
-    >>> registry_browser.getControl('Target Team').value = 'ubuntu-team'
-    >>> registry_browser.getControl('Merge').click()
-
-    >>> registry_browser.url
-    'http://launchpad.dev/people/+adminteammerge'
-
-    >>> print get_feedback_messages(registry_browser.contents)
-    [u'There is 1 error.',
-     u"guadamen is associated with a Launchpad mailing list;
-       we can't merge it."]
-
-We also can't (for obvious reasons) merge any person/team into itself.
-
-    >>> registry_browser.getControl('Duplicated Team').value = 'shipit-admins'
-    >>> registry_browser.getControl('Target Team').value = 'shipit-admins'
-    >>> registry_browser.getControl('Merge').click()
-
-    >>> registry_browser.url
-    'http://launchpad.dev/people/+adminteammerge'
-
-    >>> print get_feedback_messages(registry_browser.contents)
-    [u'There is 1 error.', u"You can't merge shipit-admins into itself."]
-
-Nor can we merge a person into a team or a team into a person.
-
-    >>> registry_browser.getControl('Duplicated Team').value = 'ubuntu-team'
-    >>> registry_browser.getControl('Target Team').value = 'salgado'
-    >>> registry_browser.getControl('Merge').click()
-
-    >>> registry_browser.url
-    'http://launchpad.dev/people/+adminteammerge'
-
-    # Yes, the error message is not of much help, but this UI is only for
-    # admins and they're supposed to know what they're doing.
-    >>> print get_feedback_messages(registry_browser.contents)
-    [u'There is 1 error.', u'Constraint not satisfied']
+    >>> print get_feedback_messages(registry_browser.contents)[0]
+    A merge is queued and is expected to complete in a few minutes.
 
=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2011-04-05 12:10:01 +0000
+++ lib/lp/registry/tests/test_person.py	2011-04-12 02:08:36 +0000
@@ -289,14 +289,14 @@
         person = self.factory.makePerson()
         self.assertFalse(person.is_merge_pending)
 
-    def test_merge_pending(self):
+    def test_is_merge_pending(self):
         # is_merge_pending returns True when this person is being merged with
         # another person in an active merge job.
         from_person = self.factory.makePerson()
         to_person = self.factory.makePerson()
         getUtility(IPersonSet).mergeAsync(from_person, to_person)
         self.assertTrue(from_person.is_merge_pending)
-        self.assertTrue(to_person.is_merge_pending)
+        self.assertFalse(to_person.is_merge_pending)
 
     def test_mergeAsync_success(self):
         # mergeAsync returns a job with the from and to persons.