launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03074
[Merge] lp:~sinzui/launchpad/person-merge-job-2 into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/person-merge-job-2 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #741094 in Launchpad itself: "Move pre-merge person rules into the model."
https://bugs.launchpad.net/launchpad/+bug/741094
Bug #742524 in Launchpad itself: "Teams can have impossible email addresses"
https://bugs.launchpad.net/launchpad/+bug/742524
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/person-merge-job-2/+merge/54873
Move pre-merge team rules into the model.
Launchpad bug:
https://bugs.launchpad.net/bugs/741094
https://bugs.launchpad.net/bugs/742524
Pre-implementation: lifeless, jcsackett
Test command: ./bin/test -vv \
-t TestPersonSetMerge -t peoplemerge \
-t xx-adminteammerge -t merge-people -t xx-merge-person \
-t xx-adminpeoplemerge
Before bug 736421 can be fixed, the pre-merge work that can timeout needs to
be moved into the model so that PersonMergeJob can put the person or team in a
sane state for merging.
--------------------------------------------------------------------
RULES
* Moved the mailing list, members and super team cleanup blocks
into the model.
* ADDENDUM: Ensure the mailing list email address is deleted when the
mailing list is purged.
QA
* Try to delete a team with an active PPA, and mailing list using the
UI.
* Verify the page tells you it cannot be done.
* delete a team with a deleted PPA, deactivated mailing list, members
and is a member of another team.
* Verify it succeeds.
LINT
lib/canonical/launchpad/database/emailaddress.py
lib/lp/registry/browser/peoplemerge.py
lib/lp/registry/browser/tests/peoplemerge-views.txt
lib/lp/registry/browser/tests/test_peoplemerge.py
lib/lp/registry/interfaces/persontransferjob.py
lib/lp/registry/model/mailinglist.py
lib/lp/registry/model/person.py
lib/lp/registry/model/persontransferjob.py
lib/lp/registry/tests/test_person.py
lib/lp/registry/tests/test_person_merge_job.py
IMPLEMENTATION
Testing revealed that the mailing list email address was not being removed
when the mailing list was purged. The addresses cause no problem for the team,
but teams can acquired an impossible address from merges and renames. I
reported bug 742524.
lib/canonical/launchpad/database/emailaddress.py
lib/lp/registry/model/mailinglist.py
The view was passing the logged in user as the reviewer for the team
membership changes. I updated IPersonMergeJobSource to accept the reviewer
as metadata and added a property PersonMergeJob to use it. This is the same
approach used by IMembershipNotificationJob and MembershipNotificationJob.
lib/lp/registry/interfaces/persontransferjob.py
lib/lp/registry/model/persontransferjob.py
Moved the pre-merge team rules to a private method called by merge(). I added
the reviewer keyword arguments so that we have a record of who changed the
memberships was was done by the view. I fixed the type checking asserts, the
real rules were always team to team, user to user. I was able to remove the
membership asserts since the private method ensures membership is correct.
There is one logic change, the view only managed super teams for delete
operations, but the model does it for all merges. We recently fixed the code
that did this and we are confident it works.
lib/lp/registry/model/person.py
lib/lp/registry/tests/test_person.py
lib/lp/registry/tests/test_person_merge_job.py
Removed unneeded logic from views and view tests.
lib/lp/registry/browser/peoplemerge.py
lib/lp/registry/browser/tests/peoplemerge-views.txt
lib/lp/registry/browser/tests/test_peoplemerge.py
--
https://code.launchpad.net/~sinzui/launchpad/person-merge-job-2/+merge/54873
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/person-merge-job-2 into lp:launchpad.
=== modified file 'lib/canonical/launchpad/database/emailaddress.py'
--- lib/canonical/launchpad/database/emailaddress.py 2011-02-23 20:26:53 +0000
+++ lib/canonical/launchpad/database/emailaddress.py 2011-03-25 15:25:55 +0000
@@ -67,6 +67,7 @@
def destroySelf(self):
"""See `IEmailAddress`."""
# Import this here to avoid circular references.
+ from lp.registry.interfaces.mailinglist import MailingListStatus
from lp.registry.model.mailinglist import (
MailingListSubscription)
@@ -74,7 +75,9 @@
raise UndeletableEmailAddress(
"This is a person's preferred email, so it can't be deleted.")
mailing_list = self.person and self.person.mailing_list
- if mailing_list is not None and mailing_list.address == self.email:
+ if (mailing_list is not None
+ and mailing_list.status != MailingListStatus.PURGED
+ and mailing_list.address == self.email):
raise UndeletableEmailAddress(
"This is the email address of a team's mailing list, so it "
"can't be deleted.")
=== modified file 'lib/lp/registry/browser/peoplemerge.py'
--- lib/lp/registry/browser/peoplemerge.py 2011-03-23 16:28:51 +0000
+++ lib/lp/registry/browser/peoplemerge.py 2011-03-25 15:25:55 +0000
@@ -18,7 +18,6 @@
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
-from canonical.database.sqlbase import flush_database_updates
from canonical.launchpad import _
from canonical.launchpad.interfaces.authtoken import LoginTokenType
from canonical.launchpad.interfaces.emailaddress import (
@@ -47,7 +46,6 @@
IPersonSet,
IRequestPeopleMerge,
)
-from lp.registry.interfaces.teammembership import ITeamMembershipSet
from lp.services.propertycache import cachedproperty
from lp.soyuz.enums import ArchiveStatus
from lp.soyuz.interfaces.archive import IArchiveSet
@@ -143,19 +141,18 @@
Before merging this moves each email address of the duplicate person
to the target person, and resets them to `NEW`.
"""
- for email in self.dupe_person_emails:
- email = IMasterObject(email)
- # EmailAddress.person and EmailAddress.account are readonly
- # fields, so we need to remove the security proxy here.
- naked_email = removeSecurityProxy(email)
- naked_email.personID = self.target_person.id
- naked_email.accountID = self.target_person.accountID
- # XXX: Guilherme Salgado 2007-10-15: Maybe this status change
- # should be done only when merging people but not when merging
- # teams.
- naked_email.status = EmailAddressStatus.NEW
- flush_database_updates()
- getUtility(IPersonSet).merge(self.dupe_person, self.target_person)
+ if not self.dupe_person.is_team:
+ # Transfer user email addresses. Team addresses will be deleted.
+ for email in self.dupe_person_emails:
+ email = IMasterObject(email)
+ # EmailAddress.person and EmailAddress.account are readonly
+ # fields, so we need to remove the security proxy here.
+ naked_email = removeSecurityProxy(email)
+ naked_email.personID = self.target_person.id
+ naked_email.accountID = self.target_person.accountID
+ naked_email.status = EmailAddressStatus.NEW
+ getUtility(IPersonSet).merge(
+ self.dupe_person, self.target_person, reviewer=self.user)
self.request.response.addInfoNotification(self.merge_message)
self.next_url = self.success_url
@@ -219,49 +216,6 @@
def registry_experts(self):
return getUtility(ILaunchpadCelebrities).registry_experts
- def doMerge(self, data):
- """Purge the non-transferable team data and merge.
-
- For the duplicate team:
-
- - If a mailing list exists, and is REGISTERED, DECLINED, FAILED or
- INACTIVE, it is purged.
-
- - Unsets the contact address.
-
- If the target team is the Registry Experts:
-
- - The duplicate team is withdrawn from all teams that it is itself a
- member of.
-
- """
- # A team cannot have more than one mailing list. The old list will
- # remain in the archive.
- purge_list = (self.dupe_person.mailing_list is not None
- and self.dupe_person.mailing_list.status in PURGE_STATES)
- if purge_list:
- self.dupe_person.mailing_list.purge()
- # Team email addresses are not transferable.
- self.dupe_person.setContactAddress(None)
- # The registry experts does not want to acquire super teams from a
- # merge. This operation requires unrestricted access to ensure
- # the user who has permission to delete a team can remove the
- # team from other teams.
- if self.target_person == self.registry_experts:
- all_super_teams = set(self.dupe_person.teams_participated_in)
- indirect_super_teams = set(
- self.dupe_person.teams_indirectly_participated_in)
- super_teams = all_super_teams - indirect_super_teams
- naked_dupe_person = removeSecurityProxy(self.dupe_person)
- for team in super_teams:
- naked_dupe_person.retractTeamMembership(team, self.user)
- del naked_dupe_person
- # We have sent another series of calls to the db, potentially a long
- # sequence depending on the merge. We want everything synced up
- # before proceeding.
- flush_database_updates()
- super(AdminTeamMergeView, self).doMerge(data)
-
def validate(self, data):
"""Check there are no mailing lists associated with the dupe team."""
# If errors have already been discovered there is no need to continue,
@@ -273,16 +227,6 @@
super(AdminTeamMergeView, self).validate(data)
dupe_team = data['dupe_person']
target_team = data['target_person']
- # Merge cannot reconcile cyclic membership in super teams.
- # Super team memberships are automatically removed when merging into
- # the registry experts team. When merging into any other team, an
- # error must be raised to explain that the user must remove the teams
- # himself.
- super_teams_count = dupe_team.super_teams.count()
- if target_team != self.registry_experts and super_teams_count > 0:
- self.addError(_(
- "${name} has super teams, so it can't be merged.",
- mapping=dict(name=dupe_team.name)))
# We cannot merge the teams if there is a mailing list on the
# duplicate person, unless that mailing list is purged.
if self.hasMailingList(dupe_team):
@@ -305,24 +249,14 @@
# merge.
self.should_confirm_member_deactivation = True
return
- self.doMerge(data)
+ super(AdminTeamMergeView, self).doMerge(data)
@action('Deactivate Members and Merge',
name='deactivate_members_and_merge')
def deactivate_members_and_merge_action(self, action, data):
"""Deactivate all members of the team to be merged and merge them."""
self.setUpPeople(data)
- comment = (
- 'Deactivating all members as this team is being merged into %s. '
- 'Please contact the administrators of <%s> if you have any '
- 'issues with this change.'
- % (self.target_person.unique_displayname,
- canonical_url(self.target_person)))
- membershipset = getUtility(ITeamMembershipSet)
- membershipset.deactivateActiveMemberships(
- self.dupe_person, comment, self.user)
- flush_database_updates()
- self.doMerge(data)
+ super(AdminTeamMergeView, self).doMerge(data)
class DeleteTeamView(AdminTeamMergeView):
@@ -545,10 +479,5 @@
token = logintokenset.new(
self.user, login, removeSecurityProxy(email).email,
LoginTokenType.ACCOUNTMERGE)
-
- # XXX: SteveAlexander 2006-03-07: An experiment to see if this
- # improves problems with merge people tests.
- import canonical.database.sqlbase
- canonical.database.sqlbase.flush_database_updates()
token.sendMergeRequestEmail()
self.next_url = './+mergerequest-sent?dupe=%d' % dupeaccount.id
=== modified file 'lib/lp/registry/browser/tests/peoplemerge-views.txt'
--- lib/lp/registry/browser/tests/peoplemerge-views.txt 2010-10-31 22:21:20 +0000
+++ lib/lp/registry/browser/tests/peoplemerge-views.txt 2011-03-25 15:25:55 +0000
@@ -39,26 +39,9 @@
>>> print person_set.getByName('ubuntu-team').displayname
Ubuntu Team
-Teams which have superteams cannot be merged into other teams. This
-restriction was added as a fix to https://bugs.launchpad.net/bugs/290681.
-
- >>> parent_team = factory.makeTeam()
- >>> child_team = factory.makeTeam(name='child-team')
- >>> random_team = factory.makeTeam()
- >>> transaction.commit()
- >>> login('foo.bar@xxxxxxxxxxxxx')
- >>> ignored = parent_team.addMember(
- ... child_team, reviewer=parent_team.teamowner, force_team_add=True)
- >>> form = {'field.dupe_person': child_team.name,
- ... 'field.target_person': random_team.name,
- ... 'field.actions.deactivate_members_and_merge': 'Merge'}
- >>> view = create_initialized_view(
- ... person_set, '+adminteammerge', form=form)
- >>> view.errors
- [u"child-team has super teams, so it can't be merged."]
-
Attempting to merge a non-existent team results in an error.
+ >>> transaction.commit()
>>> form = {'field.dupe_person': 'not-a-real-team',
... 'field.target_person': 'another-fake-team',
... 'field.actions.merge': 'Merge'}
=== modified file 'lib/lp/registry/browser/tests/test_peoplemerge.py'
--- lib/lp/registry/browser/tests/test_peoplemerge.py 2011-01-07 15:16:52 +0000
+++ lib/lp/registry/browser/tests/test_peoplemerge.py 2011-03-25 15:25:55 +0000
@@ -10,15 +10,12 @@
EmailAddressStatus,
IEmailAddressSet,
)
-from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
from canonical.testing.layers import DatabaseFunctionalLayer
-from lp.registry.interfaces.mailinglist import MailingListStatus
from lp.registry.interfaces.person import (
IPersonSet,
TeamSubscriptionPolicy,
)
from lp.testing import (
- celebrity_logged_in,
login_celebrity,
login_person,
person_logged_in,
@@ -138,16 +135,6 @@
return create_initialized_view(
self.person_set, '+adminteammerge', form=form)
- def test_merge_team_with_inactive_mailing_list(self):
- # Inactive lists do not block merges.
- mailing_list = self.factory.makeMailingList(
- self.dupe_team, self.dupe_team.teamowner)
- mailing_list.deactivate()
- mailing_list.transitionToStatus(MailingListStatus.INACTIVE)
- view = self.getView()
- self.assertEqual([], view.errors)
- self.assertEqual(self.target_team, self.dupe_team.merged)
-
def test_merge_team_with_email_address(self):
# Team email addresses are not transferred.
self.factory.makeEmail(
@@ -158,33 +145,6 @@
emails = getUtility(IEmailAddressSet).getByPerson(self.target_team)
self.assertEqual(0, emails.count())
- def test_merge_team_with_super_teams_into_registry_experts(self):
- # Super team memberships are removed.
- self.target_team = getUtility(ILaunchpadCelebrities).registry_experts
- super_team = self.factory.makeTeam()
- login_celebrity('admin')
- self.dupe_team.join(super_team, self.dupe_team.teamowner)
- login_person(self.dupe_team.teamowner)
- form = {
- 'field.dupe_person': self.dupe_team.name,
- 'field.target_person': self.target_team.name,
- 'field.actions.merge': 'Merge',
- }
- view = self.getView()
- self.assertEqual([], view.errors)
- self.assertEqual(self.target_team, self.dupe_team.merged)
-
- def test_owner_delete_team_with_super_teams(self):
- # Super team memberships are removed.
- self.target_team = getUtility(ILaunchpadCelebrities).registry_experts
- super_team = self.factory.makeTeam()
- login_celebrity('admin')
- self.dupe_team.join(super_team, self.dupe_team.teamowner)
- login_person(self.dupe_team.teamowner)
- view = self.getView()
- self.assertEqual([], view.errors)
- self.assertEqual(self.target_team, self.dupe_team.merged)
-
def test_cannot_merge_team_with_ppa(self):
# A team with a PPA cannot be merged.
login_celebrity('admin')
@@ -198,18 +158,6 @@
"files."],
view.errors)
- def test_registry_delete_team_with_super_teams(self):
- # Registry admins can delete teams with super team memberships.
- self.target_team = getUtility(ILaunchpadCelebrities).registry_experts
- super_team = self.factory.makeTeam()
- # Use admin to avoid the team invitation dance. The Registry admin
- # is logged back in.
- with celebrity_logged_in('admin'):
- self.dupe_team.join(super_team, super_team.teamowner)
- view = self.getView()
- self.assertEqual([], view.errors)
- self.assertEqual(self.target_team, self.dupe_team.merged)
-
class TestAdminPeopleMergeView(TestCaseWithFactory):
"""Test the AdminPeopleMergeView rules."""
=== modified file 'lib/lp/registry/interfaces/persontransferjob.py'
--- lib/lp/registry/interfaces/persontransferjob.py 2011-03-23 16:28:51 +0000
+++ lib/lp/registry/interfaces/persontransferjob.py 2011-03-25 15:25:55 +0000
@@ -97,13 +97,18 @@
class IPersonMergeJobSource(IJobSource):
- """An interface for acquiring IMembershipNotificationJobs."""
+ """An interface for acquiring IPersonMergeJobs."""
- def create(from_person, to_person):
+ def create(from_person, to_person, review=None):
"""Create a new IMembershipNotificationJob.
None is returned if either the from_person or to_person are already
- in a pending merge.
+ in a pending merge. The review keyword argument is required if
+ the from_person and to_person are teams.
+
+ :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.
"""
def find(from_person=None, to_person=None, any_person=False):
=== modified file 'lib/lp/registry/model/mailinglist.py'
--- lib/lp/registry/model/mailinglist.py 2011-01-24 19:22:06 +0000
+++ lib/lp/registry/model/mailinglist.py 2011-03-25 15:25:55 +0000
@@ -588,6 +588,8 @@
# a bit tortured, so just do it here.
if self.status in PURGE_STATES:
self.status = MailingListStatus.PURGED
+ email = getUtility(IEmailAddressSet).getByEmail(self.address)
+ removeSecurityProxy(email).destroySelf()
else:
assert self.status != MailingListStatus.PURGED, 'Already purged'
raise UnsafeToPurge(self)
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2011-03-23 16:28:51 +0000
+++ lib/lp/registry/model/person.py 2011-03-25 15:25:55 +0000
@@ -216,6 +216,7 @@
IMailingListSet,
MailingListStatus,
PostedMessageStatus,
+ PURGE_STATES,
)
from lp.registry.interfaces.mailinglistsubscription import (
MailingListAutoSubscribePolicy,
@@ -246,7 +247,10 @@
SSHKeyCompromisedError,
SSHKeyType,
)
-from lp.registry.interfaces.teammembership import TeamMembershipStatus
+from lp.registry.interfaces.teammembership import (
+ ITeamMembershipSet,
+ TeamMembershipStatus,
+ )
from lp.registry.interfaces.wikiname import (
IWikiName,
IWikiNameSet,
@@ -3800,44 +3804,60 @@
WHERE id = %(to_id)d
''' % vars())
+ def _purgeUnmergableTeamArtifacts(self, from_team, to_team, reviewer):
+ """Purge team artifacts that cannot be merged, but can be removed."""
+ # A team cannot have more than one mailing list.
+ mailing_list = getUtility(IMailingListSet).get(from_team.name)
+ if mailing_list is not None and mailing_list.status in PURGE_STATES:
+ from_team.mailing_list.purge()
+ elif mailing_list is not None:
+ raise AssertionError(
+ "Teams with active mailing lists cannot be merged.")
+ # Team email addresses are not transferable.
+ from_team.setContactAddress(None)
+ # Memberships in the team are not transferable
+ comment = (
+ 'Deactivating all members as this team is being merged into %s.'
+ % to_team.name)
+ membershipset = getUtility(ITeamMembershipSet)
+ membershipset.deactivateActiveMemberships(
+ from_team, comment, reviewer)
+ # Memberships in other teams are not transferable.
+ all_super_teams = set(from_team.teams_participated_in)
+ indirect_super_teams = set(
+ from_team.teams_indirectly_participated_in)
+ super_teams = all_super_teams - indirect_super_teams
+ naked_from_team = removeSecurityProxy(from_team)
+ for team in super_teams:
+ naked_from_team.retractTeamMembership(team, reviewer)
+ IStore(from_team).flush()
+
def mergeAsync(self, from_person, to_person):
"""See `IPersonSet`."""
return getUtility(IPersonMergeJobSource).create(
from_person=from_person, to_person=to_person)
- def merge(self, from_person, to_person):
+ def merge(self, from_person, to_person, reviewer=None):
"""See `IPersonSet`."""
- # Sanity checks
- if not IPerson.providedBy(from_person):
- raise TypeError('from_person is not a person.')
- if not IPerson.providedBy(to_person):
- raise TypeError('to_person is not a person.')
- # If the team has a mailing list, the mailing list better be in the
- # purged state, otherwise the team can't be merged.
- mailing_list = getUtility(IMailingListSet).get(from_person.name)
- assert (mailing_list is None or
- mailing_list.status == MailingListStatus.PURGED), (
- "Can't merge teams which have mailing lists into other teams.")
-
- if getUtility(IEmailAddressSet).getByPerson(from_person).count() > 0:
- raise AssertionError('from_person still has email addresses.')
-
+ # since we are doing direct SQL manipulation, make sure all
+ # changes have been flushed to the database
+ store = Store.of(from_person)
+ store.flush()
+ if (from_person.is_team and not to_person.is_team
+ or not from_person.is_team and to_person.is_team):
+ raise AssertionError("Users cannot be merged with teams.")
+ if from_person.is_team and reviewer is None:
+ raise AssertionError("Team merged require a reviewer.")
if getUtility(IArchiveSet).getPPAOwnedByPerson(
from_person, statuses=[ArchiveStatus.ACTIVE,
ArchiveStatus.DELETING]) is not None:
raise AssertionError(
'from_person has a ppa in ACTIVE or DELETING status')
-
- if from_person.is_team and from_person.allmembers.count() > 0:
- raise AssertionError(
- "Only teams without active members can be merged")
-
- if from_person.is_team and from_person.super_teams.count() > 0:
- raise AssertionError(
- "Only teams without super teams can be merged.")
- # since we are doing direct SQL manipulation, make sure all
- # changes have been flushed to the database
- store = Store.of(from_person)
+ if from_person.is_team:
+ self._purgeUnmergableTeamArtifacts(
+ from_person, to_person, reviewer)
+ if getUtility(IEmailAddressSet).getByPerson(from_person).count() > 0:
+ raise AssertionError('from_person still has email addresses.')
# Get a database cursor.
cur = cursor()
=== modified file 'lib/lp/registry/model/persontransferjob.py'
--- lib/lp/registry/model/persontransferjob.py 2011-03-23 16:28:51 +0000
+++ lib/lp/registry/model/persontransferjob.py 2011-03-25 15:25:55 +0000
@@ -339,12 +339,17 @@
class_job_type = PersonTransferJobType.MERGE
@classmethod
- def create(cls, from_person, to_person):
+ def create(cls, from_person, to_person, reviewer=None):
"""See `IPersonMergeJobSource`."""
if from_person.is_merge_pending or to_person.is_merge_pending:
return None
+ if from_person.is_team:
+ metadata = {'reviewer': reviewer.id}
+ else:
+ metadata = {}
return super(PersonMergeJob, cls).create(
- minor_person=from_person, major_person=to_person, metadata={})
+ minor_person=from_person, major_person=to_person,
+ metadata=metadata)
@classmethod
def find(cls, from_person=None, to_person=None, any_person=False):
@@ -378,6 +383,13 @@
return self.major_person
@property
+ def reviewer(self):
+ if 'reviewer' in self.metadata:
+ return getUtility(IPersonSet).get(self.metadata['reviewer'])
+ else:
+ return None
+
+ @property
def log_name(self):
return self.__class__.__name__
@@ -392,7 +404,8 @@
from_person_name, to_person_name)
getUtility(IPersonSet).merge(
- from_person=self.from_person, to_person=self.to_person)
+ from_person=self.from_person, to_person=self.to_person,
+ reviewer=self.reviewer)
log.debug(
"%s has merged ~%s into ~%s", self.log_name,
=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py 2011-03-23 16:28:51 +0000
+++ lib/lp/registry/tests/test_person.py 2011-03-25 15:25:55 +0000
@@ -24,6 +24,7 @@
from canonical.launchpad.interfaces.emailaddress import (
EmailAddressAlreadyTaken,
EmailAddressStatus,
+ IEmailAddressSet,
InvalidEmailAddress,
)
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
@@ -46,6 +47,7 @@
PrivatePersonLinkageError,
)
from lp.registry.interfaces.karma import IKarmaCacheManager
+from lp.registry.interfaces.mailinglist import MailingListStatus
from lp.registry.interfaces.nameblacklist import INameBlacklistSet
from lp.registry.interfaces.person import (
ImmutableVisibilityError,
@@ -55,7 +57,6 @@
PersonVisibility,
)
from lp.registry.interfaces.product import IProductSet
-from lp.registry.interfaces.teammembership import ITeamMembershipSet
from lp.registry.model.karma import (
KarmaCategory,
KarmaTotalCache,
@@ -692,36 +693,59 @@
self.person_set.merge(duplicate, person)
self.assertEqual(oldest_date, person.datecreated)
- def _doMerge(self, test_team, target_team):
- membershipset = getUtility(ITeamMembershipSet)
- membershipset.deactivateActiveMemberships(
- test_team, comment='',
- reviewer=test_team.teamowner)
- self.person_set.merge(test_team, target_team)
+ def test_team_with_active_mailing_list_raises_error(self):
+ # A team with an active mailing list cannot be merged.
+ target_team = self.factory.makeTeam()
+ test_team = self.factory.makeTeam()
+ mailing_list = self.factory.makeMailingList(
+ test_team, test_team.teamowner)
+ self.assertRaises(
+ AssertionError, self.person_set.merge, test_team, target_team)
+
+ def test_team_with_inactive_mailing_list(self):
+ # A team with an inactive mailing list can be merged.
+ target_team = self.factory.makeTeam()
+ test_team = self.factory.makeTeam()
+ mailing_list = self.factory.makeMailingList(
+ test_team, test_team.teamowner)
+ mailing_list.deactivate()
+ mailing_list.transitionToStatus(MailingListStatus.INACTIVE)
+ self.person_set.merge(test_team, target_team, test_team.teamowner)
+ self.assertEqual(target_team, test_team.merged)
+ self.assertEqual(MailingListStatus.PURGED, mailing_list.status)
+ emails = getUtility(IEmailAddressSet).getByPerson(target_team).count()
+ self.assertEqual(0, emails)
+
+ def test_team_with_members(self):
+ # Team members are removed before merging.
+ target_team = self.factory.makeTeam()
+ test_team = self.factory.makeTeam()
+ former_member = self.factory.makePerson()
+ with person_logged_in(test_team.teamowner):
+ test_team.addMember(former_member, test_team.teamowner)
+ self.person_set.merge(test_team, target_team, test_team.teamowner)
+ self.assertEqual(target_team, test_team.merged)
+ self.assertEqual([], list(former_member.super_teams))
def test_team_without_super_teams_is_fine(self):
# A team with no members and no super teams
# merges without errors.
test_team = self.factory.makeTeam()
target_team = self.factory.makeTeam()
-
login_person(test_team.teamowner)
- self._doMerge(test_team, target_team)
+ self.person_set.merge(test_team, target_team, test_team.teamowner)
- def test_team_with_super_teams_raises_error(self):
- # A team with no members but with superteams
- # raises an assertion error.
+ def test_team_with_super_teams(self):
+ # A team with with superteams can be merged, but the memberships
+ # are not transferred.
test_team = self.factory.makeTeam()
super_team = self.factory.makeTeam()
target_team = self.factory.makeTeam()
-
login_person(test_team.teamowner)
test_team.join(super_team, test_team.teamowner)
- self.assertRaises(
- AssertionError,
- self._doMerge,
- test_team,
- target_team)
+ self.person_set.merge(test_team, target_team, test_team.teamowner)
+ self.assertEqual(target_team, test_team.merged)
+ self.assertEqual([], list(target_team.super_teams))
def test_merge_moves_branches(self):
# When person/teams are merged, branches owned by the from person
=== modified file 'lib/lp/registry/tests/test_person_merge_job.py'
--- lib/lp/registry/tests/test_person_merge_job.py 2011-03-23 16:28:51 +0000
+++ lib/lp/registry/tests/test_person_merge_job.py 2011-03-25 15:25:55 +0000
@@ -105,7 +105,9 @@
from_team = self.factory.makeTeam(name='null')
with person_logged_in(from_team.teamowner):
from_team.teamowner.leave(from_team)
- self.job_source.create(from_person=from_team, to_person=to_team)
+ self.job_source.create(
+ from_person=from_team, to_person=to_team,
+ reviewer=from_team.teamowner)
transaction.commit()
out, err, exit_code = run_script(
