launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03270
[Merge] lp:~sinzui/launchpad/person-merge-job-4 into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/person-merge-job-4 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #697530 in Launchpad itself: "failing to +adminteammerge two teams "Constraint not satisfied""
https://bugs.launchpad.net/launchpad/+bug/697530
Bug #736421 in Launchpad itself: "Person:+delete timeouts : Connect the UI the merge jobs"
https://bugs.launchpad.net/launchpad/+bug/736421
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/person-merge-job-4/+merge/57037
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
--
https://code.launchpad.net/~sinzui/launchpad/person-merge-job-4/+merge/57037
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/person-merge-job-4 into lp:launchpad.
=== 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-09 02:58:29 +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-09 02:58:29 +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-09 02:58:29 +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-09 02:58:29 +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/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-09 02:58:29 +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(
- "del@xxxxxx", 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-09 02:58:29 +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-09 02:58:29 +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-09 02:58:29 +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-09 02:58:29 +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-09 02:58:29 +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/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-09 02:58:29 +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('andrew.bennetts@xxxxxxxxxxxxxxx').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.
=== 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-09 02:58:29 +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-09 02:58:29 +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.
