Merge lp:~sinzui/launchpad/deactivate-all-members-fix-0 into lp:launchpad

[Curtis Hovey]

deactivateAllMembers() must use TM._cleanTeamParticipation.

    Launchpad bug:
        https://bugs.launchpad.net/bugs/733881
    Pre-implementation: jcsackett
    Test command: ./bin/test -vv \
      -t team_membership -t registry/tests/test_person \
      -t registry/tests/test_team -t doc/person-merge -t peoplemerge \t
      -t doc/teammembership -t doc/vocabularies

Oopses like "AssertionError: sinzui is an indirect member of ubuntu-
translations-coordinators but sinzui is not a participant in any direct member
of ubuntu-translations-coordinators" was caused by a deletion on team. All
the users in the delete team were members of ~rosetta-admins via another
team. The TeamParticipation entries for ~rosetta-admins should not have been
deleted.

Person.deactivateAllMembers() was called many steps before merge was called
to ensure there were no TP entries for the team. This method does a simple
delete of TP based on the active membership. It does not recursively check if
the user has an other path to participation in a team via another team
membership.

TeamMembership._cleanTeamParticipation does does build a exclude list of
participation entries. The fix might be to replace the custom query with a
call to TM._cleanTeamParticipation(child, parent). I see store.invalidate() is
called twice; I think once will suffice. The final storm call to remove the
direct participants may not be needed either.

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

RULES

    * Remove the redundant call to store.invalidate().
    * Replace the inline sql with a call to TM._cleanTeamParticipation
    * Remove the code to that deals with the direct participants if
      the master function does it.
    * ADDENDUM: Move this method to ITeamMembershipSet to encourage the
      one true way to manange TeamParticipation

QA

    * On staging or QA, staging, delete ~launchpad-chr.
    * Verify that ~sinzui/+participation still works.

LINT

    lib/lp/registry/configure.zcml
    lib/lp/registry/browser/peoplemerge.py
    lib/lp/registry/doc/person-merge.txt
    lib/lp/registry/doc/teammembership.txt
    lib/lp/registry/doc/vocabularies.txt
    lib/lp/registry/interfaces/person.py
    lib/lp/registry/interfaces/teammembership.py
    lib/lp/registry/model/person.py
    lib/lp/registry/model/teammembership.py
    lib/lp/registry/tests/test_person.py
    lib/lp/registry/tests/test_team.py
    lib/lp/registry/tests/test_teammembership.py
^ lint does not like some of the doctests. I can clean them up before
landing this branch.

IMPLEMENTATION

Added a test to verify the expected behaviour of mass member deactivation.
Refactored deactivateAllMembers to use TM._cleanTeamParticipation, which
allowed me to delete 2/3 of the method. The storm bug mentioned in the
comment was fixed a long time ago, so I fixed the enums. I then moved this
method to TMSet.deactivateActiveMemberships to keep the TeamParticipation
rules in one module. The existing tests from duplicated the existing
tests, but there was no test to verify that the comment and user were
saved with the mass deactivation. I revised one of the old tests to verify
that deactivateActiveMemberships stores the right information.
    lib/lp/registry/interfaces/teammembership.py
    lib/lp/registry/model/teammembership.py
    lib/lp/registry/tests/test_teammembership.py
    lib/lp/registry/tests/test_team.py

Updated call sites to use TMSet.deactivateActiveMemberships. Removed
the old method which also permited the removal of an interface, and the
deletion of an duplicate tests.
    lib/lp/registry/configure.zcml
    lib/lp/registry/browser/peoplemerge.py
    lib/lp/registry/doc/person-merge.txt
    lib/lp/registry/doc/teammembership.txt
    lib/lp/registry/doc/vocabularies.txt
    lib/lp/registry/interfaces/person.py
    lib/lp/registry/model/person.py
    lib/lp/registry/tests/test_person.py

[j.c.sackett (jcsackett)]

This looks good to land. Moving the deactivation code into TeamMembership is a big win, and I like how much simpler this makes a fairly complicated process.

Thanks, Curtis.

review: Approve

=== modified file 'lib/lp/registry/browser/peoplemerge.py'
--- lib/lp/registry/browser/peoplemerge.py	2011-02-16 17:31:11 +0000
+++ lib/lp/registry/browser/peoplemerge.py	2011-03-17 20:19:01 +0000
@@ -47,6 +47,7 @@
     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
@@ -317,7 +318,9 @@
             'issues with this change.'
             % (self.target_person.unique_displayname,
                canonical_url(self.target_person)))
-        self.dupe_person.deactivateAllMembers(comment, self.user)
+        membershipset = getUtility(ITeamMembershipSet)
+        membershipset.deactivateActiveMemberships(
+            self.dupe_person, comment, self.user)
         flush_database_updates()
         self.doMerge(data)
 

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2011-03-10 19:11:04 +0000
+++ lib/lp/registry/configure.zcml	2011-03-17 20:19:01 +0000
@@ -893,9 +893,6 @@
                 permission="launchpad.Moderate"
                 set_attributes="name"/>
             <require
-                permission="launchpad.Moderate"
-                interface="lp.registry.interfaces.person.IPersonModerate" />
-            <require
                 permission="launchpad.Commercial"
                 set_schema="lp.registry.interfaces.person.IPersonCommAdminWriteRestricted"/>
             <require

=== modified file 'lib/lp/registry/doc/person-merge.txt'
--- lib/lp/registry/doc/person-merge.txt	2011-01-04 16:08:57 +0000
+++ lib/lp/registry/doc/person-merge.txt	2011-03-17 20:19:01 +0000
@@ -1,10 +1,11 @@
-= Merging =
+Merging
+=======
 
 For many reasons (i.e. a gina run) we could have duplicated accounts in
-Launchpad. Once a duplicated account is identified, we need to allow the user
-to merge two accounts into a single one, because both represent the same
-person and they're there just because each of those was created using a
-different email address.
+Launchpad. Once a duplicated account is identified, we need to allow the
+user to merge two accounts into a single one, because both represent the
+same person and they're there just because each of those was created
+using a different email address.
 
     >>> from zope.component import getUtility
     >>> from canonical.database.sqlbase import sqlvalues
@@ -19,7 +20,8 @@
     >>> marilize = personset.getByName('marilize')
 
 
-== Sanity checks ==
+Sanity checks
+-------------
 
 We can't merge an account that still has email addresses attached to it
 
@@ -29,13 +31,15 @@
     AssertionError: ...
 
 
-== Preparing test person for the merge ==
+Preparing test person for the merge
+-----------------------------------
 
 Merging people involves updating the merged person relationships. Let's
 put the person we will merge into some of those.
 
     # To assign marilize as the ubuntu team owner, we must log on as the
     # previous owner.
+
     >>> login('[e-mail]')
 
     >>> ubuntu_team = personset.getByName('ubuntu-team')
@@ -55,20 +59,23 @@
     >>> saved_marilize_karma_id = marilize_karma.id
     >>> print marilize_karma.person.name
     marilize
+
     >>> sampleperson_old_karma = sample.karma
 
-Branches whose owner is being merged are uniquified by appending '-N' where N
-is a unique integer. We create "peoplemerge" and "peoplemerge-1" branches owned
-by marilize, and a "peoplemerge" and "peoplemerge-1" branches owned by 'Sample
-Person' to test that branch name uniquifying works.
-
-Branches with smaller IDs will be processed first, so we create "peoplemerge"
-first, and it will be renamed "peoplemerge-2". The extant "peoplemerge-1"
-branch will be renamed "peoplemerge-1-1". The "peoplemerge-0" branch will not
-be renamed since it will not conflict.
-
-That is not a particularly sensible way of renaming branches, but it is simple
-to implement, and it be should extremely rare for the case to occur.
+Branches whose owner is being merged are uniquified by appending '-N'
+where N is a unique integer. We create "peoplemerge" and "peoplemerge-1"
+branches owned by marilize, and a "peoplemerge" and "peoplemerge-1"
+branches owned by 'Sample Person' to test that branch name uniquifying
+works.
+
+Branches with smaller IDs will be processed first, so we create
+"peoplemerge" first, and it will be renamed "peoplemerge-2". The extant
+"peoplemerge-1" branch will be renamed "peoplemerge-1-1". The
+"peoplemerge-0" branch will not be renamed since it will not conflict.
+
+That is not a particularly sensible way of renaming branches, but it is
+simple to implement, and it be should extremely rare for the case to
+occur.
 
     >>> peoplemerge = factory.makePersonalBranch(
     ...     name='peoplemerge', owner=sample)
@@ -81,12 +88,13 @@
     >>> peoplemerge11 = factory.makePersonalBranch(
     ...     name='peoplemerge-1', owner=marilize)
 
-'Sample Person' is a deactivated member of the 'Ubuntu Translators' team,
-while marilize is an active member. After the merge, 'Sample Person' will be an
-active member of that team.
+'Sample Person' is a deactivated member of the 'Ubuntu Translators'
+team, while marilize is an active member. After the merge, 'Sample
+Person' will be an active member of that team.
 
     >>> sample in ubuntu_translators.inactivemembers
     True
+
     >>> marilize in ubuntu_translators.activemembers
     True
 
@@ -102,11 +110,13 @@
     u'Marilize Coetzee'
 
 
-== Do the merge! ==
+Do the merge!
+-------------
 
     # Now we remove the only email address marilize had, so that we can merge
     # it.  First we need to change its status, though, because we can't delete
     # a person's preferred email.
+
     >>> from canonical.launchpad.interfaces.emailaddress import (
     ...     EmailAddressStatus)
     >>> from canonical.launchpad.interfaces.lpstorm import IMasterObject
@@ -119,13 +129,15 @@
     >>> personset.merge(marilize, sample)
 
 
-== Merge results ==
+Merge results
+-------------
 
 Check that 'Sample Person' has indeed become an active member of 'Ubuntu
 Translators'
 
     >>> sample in ubuntu_translators.activemembers
     True
+
     >>> sample.inTeam(ubuntu_translators)
     True
 
@@ -136,27 +148,32 @@
     >>> sample_junk = get_branch_namespace(sample)
     >>> sample_junk.getByName('peoplemerge') == peoplemerge
     True
+
     >>> sample_junk.getByName('peoplemerge-0') == peoplemerge0
     True
+
     >>> sample_junk.getByName('peoplemerge-1') == peoplemerge1
     True
+
     >>> sample_junk.getByName('peoplemerge-2') == peoplemerge2
     True
+
     >>> sample_junk.getByName('peoplemerge-1-1') == peoplemerge11
     True
 
-The Karma that was previously assigned to marilize is now assigned to name12
-(Sample Person).
+The Karma that was previously assigned to marilize is now assigned to
+name12 (Sample Person).
 
     >>> from canonical.database.sqlbase import flush_database_caches
     >>> flush_database_caches()
     >>> saved_marilize_karma_id == marilize_karma.id
     True
+
     >>> print marilize_karma.person.name
     name12
 
-Note that we don't bother migrating karma caches - it will just be reset next
-time the caches are rebuilt.
+Note that we don't bother migrating karma caches - it will just be reset
+next time the caches are rebuilt.
 
     >>> sample.karma == sampleperson_old_karma
     True
@@ -199,8 +216,8 @@
     >>> results.get_one()[0] is None
     True
 
-An email is sent to the user informing him that he should review his email
-and mailing list subscription settings.
+An email is sent to the user informing him that he should review his
+email and mailing list subscription settings.
 
     >>> from lp.registry.interfaces.personnotification import (
     ...     IPersonNotificationSet)
@@ -209,11 +226,14 @@
     >>> notifications = notification_set.getNotificationsToSend()
     >>> notifications.count()
     1
+
     >>> notification = notifications[0]
     >>> print notification.person.name
     name12
+
     >>> print notification.subject
     Launchpad accounts merged
+
     >>> print notification.body
     The Launchpad account named 'marilize-merged' was merged into the account
     named 'name12'. ...
@@ -226,13 +246,13 @@
 Person decoration
 -----------------
 
-Several tables "extend" the Person table by having additional information
-that is UNIQUEly keyed to Person.id. We have a utility function that merges
-information in those tables, we test it here.
+Several tables "extend" the Person table by having additional
+information that is UNIQUEly keyed to Person.id. We have a utility
+function that merges information in those tables, we test it here.
 
-We will use PersonLocation as an example. There are many permutations and
-combinations, we will exercise them all, and in each case we'll create, and
-then delete, the needed two people.
+We will use PersonLocation as an example. There are many permutations
+and combinations, we will exercise them all, and in each case we'll
+create, and then delete, the needed two people.
 
     >>> from lp.registry.model.person import PersonSet, Person
     >>> from lp.registry.interfaces.person import PersonCreationRationale
@@ -272,11 +292,12 @@
     >>> winner, loser = endless_supply_of_players.next()
     >>> print decorator_refs(store, winner, loser)
     <BLANKLINE>
+
     >>> personset._merge_person_decoration(winner, loser, skip,
     ...     'PersonLocation', 'person', ['last_modified_by',])
 
-"Skip" should have been updated with the table and unique reference column
-name.
+"Skip" should have been updated with the table and unique reference
+column name.
 
     >>> print skip
     [('personlocation', 'person')]
@@ -286,42 +307,44 @@
     >>> print decorator_refs(store, winner, loser)
     <BLANKLINE>
 
-OK, now, this time, we will add some decorator information to the winner but
-not the loser.
+OK, now, this time, we will add some decorator information to the winner
+but not the loser.
 
     >>> winner, loser = endless_supply_of_players.next()
     >>> winner.setLocation(None, None, 'America/Santiago', winner)
     >>> print decorator_refs(store, winner, loser)
     winner, winner,
+
     >>> personset._merge_person_decoration(winner, loser, skip,
     ...     'PersonLocation', 'person', ['last_modified_by',])
 
-There should now still be one decorator, with all columns pointing to the
-winner:
+There should now still be one decorator, with all columns pointing to
+the winner:
 
     >>> print decorator_refs(store, winner, loser)
     winner, winner,
 
-This time, we will have a decorator for the person that is being merged INTO
-another person, but nothing on the target person.
+This time, we will have a decorator for the person that is being merged
+INTO another person, but nothing on the target person.
 
     >>> winner, loser = endless_supply_of_players.next()
     >>> loser.setLocation(None, None, 'America/Santiago', loser)
     >>> print decorator_refs(store, winner, loser)
     loser, loser,
+
     >>> personset._merge_person_decoration(winner, loser, skip,
     ...     'PersonLocation', 'person', ['last_modified_by',])
 
-There should now still be one decorator, with all columns pointing to the
-winner:
+There should now still be one decorator, with all columns pointing to
+the winner:
 
     >>> print decorator_refs(store, winner, loser)
     winner, winner,
 
 Now, we want to show what happens when there is a decorator for both the
-to_person and the from_person. We expect that the from_person record will
-remain as noise but non-unique columns will have been updated to point to
-the winner, and the to_person will be unaffected.
+to_person and the from_person. We expect that the from_person record
+will remain as noise but non-unique columns will have been updated to
+point to the winner, and the to_person will be unaffected.
 
     >>> winner, loser = endless_supply_of_players.next()
     >>> winner.setLocation(None, None, 'America/Santiago', winner)
@@ -329,6 +352,7 @@
     >>> print decorator_refs(store, winner, loser)
     winner, winner,
     loser, loser,
+
     >>> personset._merge_person_decoration(winner, loser, skip,
     ...     'PersonLocation', 'person', ['last_modified_by',])
     >>> print decorator_refs(store, winner, loser)
@@ -336,12 +360,13 @@
     loser, winner,
 
 
-== Merging teams ==
+Merging teams
+-------------
 
 Merging of teams is also possible and uses the same API used for merging
 people.  Note, though, that when merging teams, its polls will not be
-carried over to the remaining team.  Team memberships, on the other hand,
-are carried over just like when merging people.
+carried over to the remaining team.  Team memberships, on the other
+hand, are carried over just like when merging people.
 
     >>> from datetime import datetime, timedelta
     >>> import pytz
@@ -357,37 +382,45 @@
     ...     PollSecrecy.OPEN, allowspoilt=True)
 
     # test_team has a superteam, one active member and a poll.
+
     >>> [team.name for team in test_team.super_teams]
     [u'launchpad']
+
     >>> test_team.teamowner.name
     u'name12'
+
     >>> [member.name for member in test_team.allmembers]
     [u'name12']
+
     >>> list(IPollSubset(test_team).getAll())
     [<Poll at ...]
 
     # Landscape-developers has no super teams, two members and no polls.
+
     >>> landscape = personset.getByName('landscape-developers')
     >>> [team.name for team in landscape.super_teams]
     []
+
     >>> landscape.teamowner.name
     u'name12'
+
     >>> [member.name for member in landscape.allmembers]
     [u'salgado', u'name12']
+
     >>> list(IPollSubset(landscape).getAll())
     []
 
-Now we try to merge them, but since test_team has active members it can't be
-merged.
+Now we try to merge them, but since test_team has active members it
+can't be merged.
 
     >>> personset.merge(test_team, landscape)
     Traceback (most recent call last):
     ...
     AssertionError: Only teams without active members can be merged
 
-A team with a mailing list can only be merged if has no mailing list or if the
-mailing list is in the PURGED state.  This state indicates that there are no
-artifacts of the list on the Mailman side.
+A team with a mailing list can only be merged if has no mailing list or
+if the mailing list is in the PURGED state.  This state indicates that
+there are no artifacts of the list on the Mailman side.
 
     >>> from lp.registry.interfaces.mailinglist import IMailingListSet
     >>> mailing_list = getUtility(IMailingListSet).new(landscape)
@@ -395,21 +428,23 @@
     >>> mailman.act()
     >>> print mailing_list.status.name
     ACTIVE
+
     >>> ubuntu_team = personset.getByName('ubuntu-team')
     >>> personset.merge(landscape, ubuntu_team)
     Traceback (most recent call last):
     ...
     AssertionError: Can't merge teams which have mailing lists...
 
-Purging the associated mailing list doesn't delete it, but puts it into a
-state that essentially treats it as nonexistent.  It still can't be merged
-until all the members are deactivated though.
+Purging the associated mailing list doesn't delete it, but puts it into
+a state that essentially treats it as nonexistent.  It still can't be
+merged until all the members are deactivated though.
 
     >>> mailing_list.deactivate()
     >>> mailman.act()
     >>> mailing_list.purge()
     >>> print mailing_list.status.name
     PURGED
+
     >>> personset.merge(landscape, ubuntu_team)
     Traceback (most recent call last):
     ...
@@ -418,26 +453,34 @@
 For cases like this we have an API which takes care of deactivating all
 active members of the team so that we can perform the merge.
 
+    >>> from lp.registry.interfaces.teammembership import ITeamMembershipSet
+
     # Only admins can use that API, though, so we need to login as an admin.
+
     >>> login('[e-mail]')
 
     >>> comment = "We're merging this team into another one."
-    >>> test_team.deactivateAllMembers(comment, personset.getByName('name16'))
+    >>> membershipset = getUtility(ITeamMembershipSet)
+    >>> membershipset.deactivateActiveMemberships(
+    ...     test_team, comment, personset.getByName('name16'))
     >>> for team in test_team.super_teams:
     ...     test_team.retractTeamMembership(team, test_team.teamowner)
 
-
     >>> personset.merge(test_team, landscape)
 
     # The resulting Landscape-developers no new super teams, has
     # no polls and its members are still the same two from before the
     # merge.
+
     >>> landscape.teamowner.name
     u'name12'
+
     >>> [member.name for member in landscape.allmembers]
     [u'salgado', u'name12']
+
     >>> [team.name for team in landscape.super_teams]
     []
+
     >>> list(IPollSubset(landscape).getAll())
     []
 

=== modified file 'lib/lp/registry/doc/teammembership.txt'
--- lib/lp/registry/doc/teammembership.txt	2011-02-28 23:25:52 +0000
+++ lib/lp/registry/doc/teammembership.txt	2011-03-17 20:19:01 +0000
@@ -895,7 +895,8 @@
 If a team is merged it will not show up in the set of administered teams.
 
     >>> login('[e-mail]')
-    >>> cprov_team.deactivateAllMembers("Merging", foobar)
+    >>> membershipset.deactivateActiveMemberships(
+    ...     cprov_team, "Merging", foobar)
     >>> personset.merge(cprov_team, guadamen_team)
     >>> [team.name for team in cprov.getAdministratedTeams()]
     [u'canonical-partner-dev', u'guadamen', u'launchpad-buildd-admins']

=== modified file 'lib/lp/registry/doc/vocabularies.txt'
--- lib/lp/registry/doc/vocabularies.txt	2010-12-21 21:59:19 +0000
+++ lib/lp/registry/doc/vocabularies.txt	2011-03-17 20:19:01 +0000
@@ -1,4 +1,5 @@
-= Registry vocabularies =
+Registry vocabularies
+=====================
 
     >>> from canonical.database.sqlbase import flush_database_updates
     >>> from canonical.launchpad.webapp.interfaces import IOpenLaunchBag
@@ -21,16 +22,18 @@
     >>> product_vocabulary = get_naked_vocab(None, "Product")
 
 
-== ActiveMailingList ==
+ActiveMailingList
+-----------------
 
-The active mailing lists vocabulary matches and returns only those mailing
-lists which are active.
+The active mailing lists vocabulary matches and returns only those
+mailing lists which are active.
 
     >>> list_vocabulary = get_naked_vocab(None, 'ActiveMailingList')
     >>> from canonical.launchpad.webapp.testing import verifyObject
     >>> from canonical.launchpad.webapp.vocabulary import IHugeVocabulary
     >>> verifyObject(IHugeVocabulary, list_vocabulary)
     True
+
     >>> list_vocabulary.displayname
     'Select an active mailing list.'
 
@@ -38,17 +41,15 @@
 
     >>> list(list_vocabulary)
     []
+
     >>> len(list_vocabulary)
     0
+
     >>> list(list_vocabulary.search())
     []
 
 Mailing lists are not active when they are first registered.
 
-    >>> # These are the convoluted steps to create some mailing lists.  We
-    >>> # can't use the shortcuts that other related tests use because those
-    >>> # leave the list in the ACTIVE state and we want to check things
-    >>> # before they get to that state.
     >>> personset = getUtility(IPersonSet)
     >>> ddaa = personset.getByName('ddaa')
     >>> carlos = personset.getByName('carlos')
@@ -70,13 +71,15 @@
     >>> list_three = listset.new(team_three)
     >>> list(list_vocabulary)
     []
+
     >>> len(list_vocabulary)
     0
+
     >>> list(list_vocabulary.search())
     []
 
-Mailing lists become active once they have been constructed by Mailman (which
-indicates so by transitioning the state to ACTIVE).
+Mailing lists become active once they have been constructed by Mailman
+(which indicates so by transitioning the state to ACTIVE).
 
     >>> list_one.startConstructing()
     >>> list_two.startConstructing()
@@ -88,11 +91,12 @@
     >>> sorted(mailing_list.team.displayname
     ...        for mailing_list in list_vocabulary)
     [u'Bass Players', u'Drummers', u'Guitar Players']
+
     >>> len(list_vocabulary)
     3
 
-Searching for active lists is done through the vocabulary as well.  With a
-search term of None, all active lists are returned.
+Searching for active lists is done through the vocabulary as well.  With
+a search term of None, all active lists are returned.
 
     >>> sorted(mailing_list.team.displayname
     ...        for mailing_list in list_vocabulary.search(None))
@@ -104,27 +108,31 @@
     ...        for mailing_list in list_vocabulary.search('player'))
     [u'Bass Players', u'Guitar Players']
 
-The IHugeVocabulary interface also requires a search method that returns a
-CountableIterator.
+The IHugeVocabulary interface also requires a search method that returns
+a CountableIterator.
 
     >>> iter = list_vocabulary.searchForTerms('player')
     >>> from canonical.launchpad.webapp.vocabulary import CountableIterator
     >>> isinstance(iter, CountableIterator)
     True
+
     >>> sorted((term.value.team.name, term.token, term.title)
     ...         for term in iter)
     [(u'bass-players', 'bass-players', u'Bass Players'),
      (u'guitar-players', 'guitar-players', u'Guitar Players')]
 
-The vocabulary supports accessing mailing lists by 'term', where the term must
-be a mailing list.  The returned term's value is the mailing list object, the
-token is the team name and the title is the team's display name.
+The vocabulary supports accessing mailing lists by 'term', where the
+term must be a mailing list.  The returned term's value is the mailing
+list object, the token is the team name and the title is the team's
+display name.
 
     >>> term_1 = list_vocabulary.getTerm(list_two)
     >>> term_1.value.team.displayname
     u'Guitar Players'
+
     >>> term_1.token
     'guitar-players'
+
     >>> term_1.title
     u'Guitar Players'
 
@@ -140,13 +148,14 @@
     >>> term_2 = list_vocabulary.getTermByToken(term_1.token)
     >>> term_2.value.team.displayname
     u'Guitar Players'
+
     >>> term_3 = list_vocabulary.getTerm(list_one)
     >>> term_4 = list_vocabulary.getTermByToken(term_3.token)
     >>> term_4.value.team.displayname
     u'Bass Players'
 
-If you try to get the term by a token not represented in the vocabulary, you
-get an exception.
+If you try to get the term by a token not represented in the vocabulary,
+you get an exception.
 
     >>> list_vocabulary.getTermByToken('turntablists')
     Traceback (most recent call last):
@@ -158,8 +167,8 @@
     >>> list_three in list_vocabulary
     True
 
-You are not allowed to ask whether a non-mailing list object is contained in
-this vocabulary.
+You are not allowed to ask whether a non-mailing list object is
+contained in this vocabulary.
 
     >>> team_three in list_vocabulary
     Traceback (most recent call last):
@@ -179,11 +188,13 @@
 
     >>> list(list_vocabulary.search('flautists'))
     []
+
     >>> list(list_vocabulary.search('cellists'))
     []
 
 
-=== DistroSeriesVocabulary ===
+DistroSeriesVocabulary
+......................
 
 Reflects the available distribution series.  Results are ordered by
 `name`
@@ -227,7 +238,8 @@
     []
 
 
-=== PersonActiveMembership ===
+PersonActiveMembership
+......................
 
 All the teams the person is an active member of.
 
@@ -236,6 +248,7 @@
     ...     foo_bar, 'PersonActiveMembership')
     >>> len(person_active_membership)
     10
+
     >>> for term in person_active_membership:
     ...     print term.token, term.value.displayname, term.title
     canonical-partner-dev Canonical Partner Developers
@@ -253,6 +266,7 @@
     >>> launchpad_team = person_set.getByName('launchpad')
     >>> launchpad_team in person_active_membership
     True
+
     >>> mirrors_admins = person_set.getByName('mirrors-admins')
     >>> mirrors_admins in person_active_membership
     False
@@ -313,7 +327,8 @@
     LookupError:...
 
 
-=== PersonTeamParticipations ===
+PersonTeamParticipations
+........................
 
 This vocabulary contains all the teams a person participates in. Either
 through direct or indirect participations.
@@ -322,6 +337,7 @@
     >>> [membership.team.name
     ...  for membership in sample_person.team_memberships]
     [u'hwdb-team', u'landscape-developers', u'launchpad-users', u'name20']
+
     >>> [team.name for team in sample_person.teams_participated_in]
     [u'hwdb-team', u'landscape-developers', u'launchpad-users', u'name18',
      u'name20']
@@ -338,16 +354,17 @@
     name20: Warty Security Team (name20)
 
 
-=== Milestone ===
+Milestone
+.........
 
 All the milestone in a context.
 
 A MilestoneVolcabulary contains different milestones, depending on the
 current context. It is pointless to present the large number of all
 active milestones known in Launchpad in a vocabulary. Hence a
-MilestoneVolcabulary contains only those milestones that are related
-to the current context. If no context is given, or if the context does
-not have any milestones, a MilestoneVocabulary is empty...
+MilestoneVolcabulary contains only those milestones that are related to
+the current context. If no context is given, or if the context does not
+have any milestones, a MilestoneVocabulary is empty...
 
     >>> milestones = get_naked_vocab(None, 'Milestone')
     >>> len(milestones)
@@ -359,14 +376,15 @@
     >>> len(milestones)
     0
 
-...but if the context is an IPerson, the MilestoneVocabulary contains all
-milestones. IPerson related pages showing milestone lists retrieve the
-milestones from RelevantMilestonesMixin.getMilestoneWidgetValues()
+...but if the context is an IPerson, the MilestoneVocabulary contains
+all milestones. IPerson related pages showing milestone lists retrieve
+the milestones from RelevantMilestonesMixin.getMilestoneWidgetValues()
 but we need the big default vocabulary for form input validation.
 
     >>> all_milestones = get_naked_vocab(sample_person, 'Milestone')
     >>> len(all_milestones)
     3
+
     >>> for term in all_milestones:
     ...     print "%s: %s" % (term.value.target.name, term.value.name)
     debian: 3.1
@@ -403,6 +421,7 @@
     >>> firefox_task = bug_one.bugtasks[0]
     >>> firefox_task.bugtargetdisplayname
     u'Mozilla Firefox'
+
     >>> firefox_task_milestones = get_naked_vocab(
     ...     firefox_task, 'Milestone')
     >>> for term in firefox_task_milestones:
@@ -414,14 +433,15 @@
     >>> debian_woody_task = bug_two.bugtasks[-1]
     >>> debian_woody_task.bugtargetdisplayname
     u'mozilla-firefox (Debian Woody)'
+
     >>> debian_woody_milestones = get_naked_vocab(
     ...     debian_woody_task, 'Milestone')
     >>> debian_woody = debian_woody_task.distroseries
     >>> len(debian_woody_milestones)
     2
 
-If one of the milestones is disabled, it won't be included in the vocabulary
-anymore.
+If one of the milestones is disabled, it won't be included in the
+vocabulary anymore.
 
     >>> milestone = debian_woody.milestones[0]
     >>> milestone.active = False
@@ -429,8 +449,9 @@
     >>> len(get_naked_vocab(debian_woody_task, 'Milestone'))
     1
 
-If the milestone was used in a bugtask before it was marked inactive, though,
-it'll still show up on the vocabulary so that users can change it.
+If the milestone was used in a bugtask before it was marked inactive,
+though, it'll still show up on the vocabulary so that users can change
+it.
 
     >>> debian_woody_task.milestone = milestone
     >>> flush_database_updates()
@@ -466,8 +487,8 @@
     firefox: 1.0
     firefox: firefox-milestone-no-series
 
-If the context is a specification, only milestones from that specification
-target are in the vocabulary.
+If the context is a specification, only milestones from that
+specification target are in the vocabulary.
 
     >>> canvas_spec = firefox.getSpecification('canvas')
     >>> spec_target_milestones = get_naked_vocab(
@@ -485,18 +506,21 @@
     >>> one_dot_o = firefox.milestones[0]
     >>> one_dot_o.name
     u'1.0'
+
     >>> one_dot_o.active = False
 
     >>> firefox_milestones = get_naked_vocab(firefox, 'Milestone')
     >>> len(firefox_milestones)
     0
+
     >>> firefox_task_milestones = get_naked_vocab(
     ...     firefox_task, 'Milestone')
     >>> len(firefox_task_milestones)
     0
 
 
-=== ProjectProductsVocabulary ===
+ProjectProductsVocabulary
+.........................
 
 All the products in a project.
 
@@ -510,7 +534,8 @@
     thunderbird: Mozilla Thunderbird
 
 
-=== ProjectGroupVocabulary ===
+ProjectGroupVocabulary
+......................
 
 The list of selectable projects. The results are ordered by displayname.
 
@@ -520,6 +545,7 @@
 
     >>> [p.title for p in project_vocabulary.search('mozilla')]
     [u'The Mozilla Project']
+
     >>> mozilla = project_vocabulary.getTermByToken('mozilla')
     >>> mozilla.title
     u'The Mozilla Project'
@@ -533,6 +559,7 @@
 
     >>> [p.title for p in project_vocabulary.search('mozilla')]
     [u'The Mozilla Project']
+
     >>> moz_project.active = False
     >>> flush_database_updates()
     >>> moz_project in project_vocabulary
@@ -540,11 +567,13 @@
 
     >>> [p.title for p in project_vocabulary.search('mozilla')]
     []
+
     >>> moz_project.active = True
     >>> flush_database_updates()
 
 
-=== ProductReleaseVocabulary ===
+ProductReleaseVocabulary
+........................
 
 The list of selectable products releases.
 
@@ -554,6 +583,7 @@
 
     >>> list(productrelease_vocabulary.search(None))
     []
+
     >>> evolution_releases = productrelease_vocabulary.search("evolution")
     >>> l = [release_term.title for release_term in evolution_releases]
     >>> release = productrelease_vocabulary.getTermByToken(
@@ -562,10 +592,11 @@
     u'evolution trunk 2.1.6'
 
 
-=== PersonAccountToMergeVocabulary ===
+PersonAccountToMergeVocabulary
+..............................
 
-All non-merged people with at least one email address. This vocabulary is
-meant to be used only in the people merge form.
+All non-merged people with at least one email address. This vocabulary
+is meant to be used only in the people merge form.
 
     >>> vocab = get_naked_vocab(None, "PersonAccountToMerge")
     >>> vocab.displayname
@@ -576,8 +607,8 @@
     >>> list(vocab.search(None))
     []
 
-Searching for 'Launchpad Administrators' will return an empty list, because
-teams are not part of this vocabulary.
+Searching for 'Launchpad Administrators' will return an empty list,
+because teams are not part of this vocabulary.
 
     >>> [item.name for item in list(vocab.search('Launchpad Administrators'))]
     []
@@ -632,6 +663,7 @@
     True
 
     # Here we cheat because IPerson.merged is a readonly attribute.
+
     >>> naked_cprov = removeSecurityProxy(cprov)
     >>> naked_cprov.merged = 1
     >>> naked_cprov.syncUpdate()
@@ -653,7 +685,8 @@
     True
 
 
-== AdminMergeablePerson ==
+AdminMergeablePerson
+--------------------
 
 The set of non-merged people.
 
@@ -661,18 +694,21 @@
     >>> vocab.displayname
     'Select a Person to Merge'
 
-Unlike PersonAccountToMerge, this vocabulary includes people who don't have a
-single email address, as it's fine for admins to merge them.
+Unlike PersonAccountToMerge, this vocabulary includes people who don't
+have a single email address, as it's fine for admins to merge them.
 
     >>> print fooperson.preferredemail
     None
+
     >>> list(fooperson.validatedemails) + list(fooperson.guessedemails)
     []
+
     >>> fooperson in vocab
     True
 
 
-=== NonMergedPeopleAndTeams ===
+NonMergedPeopleAndTeams
+.......................
 
 All non-merged people and teams.
 
@@ -683,8 +719,8 @@
     >>> list(vocab.search(None))
     []
 
-This vocabulary includes both validated and unvalidated profiles, as well
-as teams:
+This vocabulary includes both validated and unvalidated profiles, as
+well as teams:
 
     >>> [(p.name, p.is_valid_person) for p in vocab.search('matsubara')]
     [(u'matsubara', False)]
@@ -704,12 +740,13 @@
     False
 
 
-=== ValidPersonOrTeam ===
+ValidPersonOrTeam
+.................
 
 All 'valid' persons or teams. This is currently defined as people with a
 password, a preferred email address and not merged (Person.merged is
-None).  It also includes all public teams and private teams the
-user has permission to view.
+None).  It also includes all public teams and private teams the user has
+permission to view.
 
     >>> vocab = get_naked_vocab(None, "ValidPersonOrTeam")
     >>> vocab.displayname
@@ -723,11 +760,12 @@
 
     >>> vocab.getTermByToken('name16').value.displayname
     u'Foo Bar'
+
     >>> vocab.getTermByToken('[e-mail]').value.displayname
     u'Foo Bar'
 
-Almost all teams have the word 'team' as part of their names, so a search
-for 'team' should give us some of them.  Notice that the
+Almost all teams have the word 'team' as part of their names, so a
+search for 'team' should give us some of them.  Notice that the
 PRIVATE_TEAM 'myteam' is not included in the results.
 
     >>> login_person(sample_person)
@@ -740,8 +778,11 @@
 
 Valid teams do not include teams that have been merged.
 
+    >>> from lp.registry.interfaces.teammembership import ITeamMembershipSet
     >>> login_person(foo_bar)
-    >>> ephemeral.deactivateAllMembers("Merging", foo_bar)
+    >>> membershipset = getUtility(ITeamMembershipSet)
+    >>> membershipset.deactivateActiveMemberships(
+    ...     ephemeral, "Merging", foo_bar)
     >>> person_set.merge(ephemeral, foo_bar)
     >>> login_person(sample_person)
     >>> sorted(person.name for person in vocab.search('team'))
@@ -750,7 +791,8 @@
      u'testing-spanish-team', u'ubuntu-security', u'ubuntu-team',
      u'warty-gnome']
 
-A PRIVATE team is displayed when the logged in user is a member of the team.
+A PRIVATE team is displayed when the logged in user is a member of the
+team.
 
     >>> commercial = person_set.getByEmail('[e-mail]')
     >>> vocab = get_naked_vocab(commercial, "ValidPersonOrTeam")
@@ -776,7 +818,8 @@
      u'testing-spanish-team', u'ubuntu-security', u'ubuntu-team',
      u'warty-gnome']
 
-The PRIVATE team can be looked up via getTermByToken for a member of the team.
+The PRIVATE team can be looked up via getTermByToken for a member of the
+team.
 
     >>> term = vocab.getTermByToken('private-team')
     >>> print term.title
@@ -818,15 +861,16 @@
     [...u'private-team'...]
 
 A search for 'support' will give us only the persons which have support
-as part of their name or displayname, or the beginning of
-one of its email addresses.
+as part of their name or displayname, or the beginning of one of its
+email addresses.
 
     >>> login('[e-mail]')
     >>> vocab = get_naked_vocab(None, "ValidPersonOrTeam")
     >>> sorted(person.name for person in vocab.search('support'))
     [u'ubuntu-team']
 
-Matsubara doesn't have a preferred email address; he's not a valid Person.
+Matsubara doesn't have a preferred email address; he's not a valid
+Person.
 
     >>> sorted(person.name for person in vocab.search('matsubara'))
     []
@@ -841,13 +885,14 @@
     >>> [cjwatson] = vocab.search('cjwatson')
     >>> cjwatson.name, cjwatson.preferredemail.email
     (u'kamion', u'[e-mail]')
+
     >>> [ircid.nickname for ircid in cjwatson.ircnicknames]
     [u'cjwatson']
 
-Since there are so many people and teams a vocabulary that includes
-them all is not very useful when displaying in the user interface.  So
-we limit the number of results.  The results are ordered by
-displayname and the first set of those are the ones returned
+Since there are so many people and teams a vocabulary that includes them
+all is not very useful when displaying in the user interface.  So we
+limit the number of results.  The results are ordered by displayname and
+the first set of those are the ones returned
 
     >>> login(ANONYMOUS)
     >>> [person.displayname for person in vocab.search('team')]
@@ -859,10 +904,12 @@
     >>> login(ANONYMOUS)
     >>> vocab.LIMIT
     100
+
     >>> vocab.LIMIT = 4
 
     # The limit gets applied in multiple subselects, so the result
     # can actually be less than the limit.
+
     >>> [person.displayname for person in vocab.search('team')]
     [u'HWDB Team', u'No Team Memberships', u'testing Spanish team']
 
@@ -910,15 +957,17 @@
      u'ubuntu-team', u'warty-gnome']
 
 
-=== ValidTeam ===
+ValidTeam
+.........
 
 The valid team vocabulary is just like the ValidPersonOrTeam vocabulary,
-except that its terms are limited only to teams.  No non-team Persons will be
-returned.
+except that its terms are limited only to teams.  No non-team Persons
+will be returned.
 
     >>> vocab = get_naked_vocab(None, 'ValidTeam')
     >>> vocab.displayname
     'Select a Team'
+
     >>> sorted((team.displayname, team.teamowner.displayname)
     ...        for team in vocab.search(None))
     [(u'Bass Players', u'David Allouche'),
@@ -956,8 +1005,9 @@
      (u'Warty Security Team', u'Mark Shuttleworth'),
      (u'testing Spanish team', u'Carlos Perell\xf3 Mar\xedn')]
 
-Like with ValidPersonOrTeam, you can narrow your search down by providing some
-text to match against the team name.  Still, you only get teams back.
+Like with ValidPersonOrTeam, you can narrow your search down by
+providing some text to match against the team name.  Still, you only get
+teams back.
 
     >>> sorted((team.displayname, team.teamowner.displayname)
     ...         for team in vocab.search('spanish'))
@@ -987,7 +1037,8 @@
      (u'Warty Security Team', u'Mark Shuttleworth'),
      (u'testing Spanish team', u'Carlos Perell\xf3 Mar\xedn')]
 
-A user who is a member of a private team will see that team in his search.
+A user who is a member of a private team will see that team in his
+search.
 
     >>> login('[e-mail]')
     >>> sorted((team.displayname, team.teamowner.displayname)
@@ -1011,7 +1062,8 @@
     [(u'Ubuntu Team', u'Mark Shuttleworth')]
 
 
-=== ValidPerson ===
+ValidPerson
+...........
 
 All 'valid' persons who are not a team.
 
@@ -1019,19 +1071,23 @@
     >>> vocab = get_naked_vocab(None, "ValidPerson")
     >>> vocab.displayname
     'Select a Person'
+
     >>> people = vocab.search(None)
     >>> people.count() > 0
     True
+
     >>> invalid_people = [
     ...     person for person in people if not person.is_valid_person]
     >>> print len(invalid_people)
     0
 
-There are two 'Carlos' in the sample data but only one is a valid person.
+There are two 'Carlos' in the sample data but only one is a valid
+person.
 
     >>> carlos_people = vocab.search('Carlos')
     >>> print len(list(carlos_people))
     1
+
     >>> invalid_carlos = [
     ...     person for person in carlos_people if not person.is_valid_person]
     >>> print len(invalid_carlos)
@@ -1039,29 +1095,27 @@
 
 ValidPerson does not include teams.
 
-    >>> # Create a new team.
     >>> carlos = getUtility(IPersonSet).getByName('carlos')
     >>> carlos_team = factory.makeTeam(
     ...     owner=carlos, name='carlos-team')
     >>> person_or_team_vocab = get_naked_vocab(None, "ValidPersonOrTeam")
     >>> carlos_people_or_team = person_or_team_vocab.search('carlos')
-    >>> # The people or team search yields our one Carlos person and
-    >>> # the new team.
     >>> print len(list(carlos_people_or_team))
     2
+
     >>> carlos_team in carlos_people_or_team
     True
 
-    >>> # But the ValidPersonVocabulary only has the original Carlos
-    >>> # person, not the new team.
     >>> carlos_people = vocab.search('carlos')
     >>> print len(list(carlos_people))
     1
+
     >>> carlos_team in carlos_people
     False
 
 
-=== DistributionOrProductVocabulary ===
+DistributionOrProductVocabulary
+...............................
 
 All products and distributions. Note that the value type is
 heterogeneous.
@@ -1077,12 +1131,14 @@
 
     >>> vocab.getTermByToken('firefox').token
     'firefox'
+
     >>> login('[e-mail]')
     >>> product_set['firefox'].setAliases(['iceweasel'])
     >>> current_user = launchbag.user
     >>> login_person(current_user)
     >>> vocab.getTermByToken('iceweasel').token
     'firefox'
+
     >>> [term.token for term in vocab.searchForTerms(query='iceweasel')]
     ['firefox']
 
@@ -1101,14 +1157,17 @@
     ...     if 'Tomcat' in term.title:
     ...         print term.title, '- class', term.value.__class__.__name__
     Tomcat (Product) - class Product
+
     >>> tomcat = product_set.getByName('tomcat')
     >>> tomcat in vocab
     True
+
     >>> tomcat.active = False
     >>> flush_database_updates()
     >>> vocab = get_naked_vocab(None, "DistributionOrProduct")
     >>> tomcat in vocab
     False
+
     >>> tomcat.active = True
     >>> flush_database_updates()
     >>> vocab = get_naked_vocab(None, "DistributionOrProduct")
@@ -1122,10 +1181,11 @@
     False
 
 
-=== DistributionOrProductOrProjectGroupVocabulary ===
+DistributionOrProductOrProjectGroupVocabulary
+.............................................
 
-All products, project groups and distributions. Note that the value type is
-heterogeneous.
+All products, project groups and distributions. Note that the value type
+is heterogeneous.
 
     >>> vocab = get_naked_vocab(None, "DistributionOrProductOrProjectGroup")
     >>> for term in vocab:
@@ -1138,6 +1198,7 @@
 
     >>> vocab.getTermByToken('ubuntu').token
     'ubuntu'
+
     >>> from lp.registry.interfaces.distribution import (
     ...     IDistributionSet)
     >>> login('[e-mail]')
@@ -1145,6 +1206,7 @@
     >>> login_person(current_user)
     >>> vocab.getTermByToken('ubantoo').token
     'ubuntu'
+
     >>> [term.token for term in vocab.searchForTerms(query='ubantoo')]
     ['ubuntu']
 
@@ -1153,6 +1215,7 @@
     >>> tomcat = product_set.getByName('tomcat')
     >>> tomcat in vocab
     True
+
     >>> tomcat.active = False
     >>> tomcat in vocab
     False
@@ -1160,6 +1223,7 @@
     >>> apache = getUtility(IProjectGroupSet).getByName('apache')
     >>> apache in vocab
     True
+
     >>> apache.active = False
     >>> apache in vocab
     False
@@ -1179,13 +1243,15 @@
     ...     if 'Apache' in term.title:
     ...         print term.title, '- class', term.value.__class__.__name__
     Apache (ProjectGroup) - class ProjectGroup
+
     >>> for term in vocab:
     ...     if 'Tomcat' in term.title:
     ...         print term.title, '- class', term.value.__class__.__name__
     Tomcat (Product) - class Product
 
 
-== FeaturedProjectVocabulary ==
+FeaturedProjectVocabulary
+-------------------------
 
 The featured project vocabulary contains all the projects that are
 featured on Launchpad. It is a subset of the
@@ -1220,19 +1286,20 @@
     False
 
 
-== CommercialProjectsVocabulary ==
+CommercialProjectsVocabulary
+----------------------------
 
 The commercial projects vocabulary contains all commercial projects,
-ordered by displayname.  Note:  a project is considered commercial if
-it has a proprietary license or no license.  That's why some of these
+ordered by displayname.  Note:  a project is considered commercial if it
+has a proprietary license or no license.  That's why some of these
 clearly FOSS project in our test data show up as commercial.
 
 For a normal user (one who does not have launchpad.Commercial
-permission) the owned commercial project vocabulary is a list of
-project the user either owns or manages.
+permission) the owned commercial project vocabulary is a list of project
+the user either owns or manages.
 
-The test data has one project with a proprietary license.  Let's
-change bzr's so we will get more interesting results.
+The test data has one project with a proprietary license.  Let's change
+bzr's so we will get more interesting results.
 
     >>> from lp.registry.interfaces.product import License
     >>> bzr = product_set.getByName('bzr')
@@ -1284,3 +1351,5 @@
     ...     print term.value.displayname
     Bazaar
     Mega Money Maker
+
+

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2011-03-01 09:41:39 +0000
+++ lib/lp/registry/interfaces/person.py	2011-03-17 20:19:01 +0000
@@ -1716,13 +1716,6 @@
         """
 
 
-class IPersonModerate(Interface):
-    """IPerson attributes that require launchpad.Moderate."""
-
-    def deactivateAllMembers(comment, reviewer):
-        """Deactivate all the members of this team."""
-
-
 class IPersonCommAdminWriteRestricted(Interface):
     """IPerson attributes that require launchpad.Admin permission to set."""
 
@@ -1771,7 +1764,7 @@
 
 class IPerson(IPersonPublic, IPersonViewRestricted, IPersonEditRestricted,
               IPersonCommAdminWriteRestricted, IPersonSpecialRestricted,
-              IPersonModerate, IHasStanding, ISetLocation, IRootContext):
+              IHasStanding, ISetLocation, IRootContext):
     """A Person."""
     export_as_webservice_entry(plural_name='people')
 

=== modified file 'lib/lp/registry/interfaces/teammembership.py'
--- lib/lp/registry/interfaces/teammembership.py	2011-03-01 03:52:30 +0000
+++ lib/lp/registry/interfaces/teammembership.py	2011-03-17 20:19:01 +0000
@@ -302,6 +302,16 @@
         TeamMembership and I'll return None.
         """
 
+    def deactivateActiveMemberships(team, comment, reviewer):
+        """Deactivate all team members in ACTIVE_STATES.
+
+        This is a convenience method used before teams are deleted.
+
+        :param team: The team to deactivate.
+        :param comment: An explanation for the deactivation.
+        :param reviewer: The user doing the deactivation.
+        """
+
 
 class ITeamParticipation(Interface):
     """A TeamParticipation.

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2011-03-09 18:18:02 +0000
+++ lib/lp/registry/model/person.py	2011-03-17 20:19:01 +0000
@@ -246,10 +246,7 @@
     SSHKeyCompromisedError,
     SSHKeyType,
     )
-from lp.registry.interfaces.teammembership import (
-    ACTIVE_STATES,
-    TeamMembershipStatus,
-    )
+from lp.registry.interfaces.teammembership import TeamMembershipStatus
 from lp.registry.interfaces.wikiname import (
     IWikiName,
     IWikiNameSet,
@@ -1547,89 +1544,6 @@
         tm.dateexpires += timedelta(days=team.defaultrenewalperiod)
         tm.sendSelfRenewalNotification()
 
-    def deactivateAllMembers(self, comment, reviewer):
-        """Deactivate all members of this team.
-
-        This method circuments the TeamMembership.setStatus() method
-        to improve performance; therefore, it does not send out any
-        status change noticiations to the team admins.
-
-        :param comment: Explanation of the change.
-        :param reviewer: Person who made the change.
-        """
-        assert self.is_team, "This method is only available for teams."
-        now = datetime.now(pytz.timezone('UTC'))
-        store = Store.of(self)
-        cur = cursor()
-
-        # Deactivate the approved/admin team members.
-        # XXX: EdwinGrubbs 2009-07-08 bug=397072
-        # There are problems using storm to write an update
-        # statement using DBITems in the comparison.
-        cur.execute("""
-            UPDATE TeamMembership
-            SET status=%(status)s,
-                last_changed_by=%(last_changed_by)s,
-                last_change_comment=%(comment)s,
-                date_last_changed=%(date_last_changed)s
-            WHERE
-                TeamMembership.team = %(team)s
-                AND TeamMembership.status IN %(original_statuses)s
-            """,
-            dict(
-                status=TeamMembershipStatus.DEACTIVATED,
-                last_changed_by=reviewer.id,
-                comment=comment,
-                date_last_changed=now,
-                team=self.id,
-                original_statuses=(
-                    TeamMembershipStatus.ADMIN.value,
-                    TeamMembershipStatus.APPROVED.value)))
-
-        # Since we've updated the database behind Storm's back,
-        # flush its caches.
-        store.invalidate()
-
-        # Remove all indirect TeamParticipation entries resulting from this
-        # team. If this were just a select, it would be a complicated but
-        # feasible set of joins. Since it's a delete, we have to use
-        # some sub selects.
-        cur.execute('''
-            DELETE FROM TeamParticipation
-                WHERE
-                    -- The person needs to be a member of the team in question
-                    person IN
-                        (SELECT person from TeamParticipation WHERE
-                            team = %(team)s) AND
-
-                    -- The teams being deleted should be teams that this team
-                    -- is a member of.
-                    team IN
-                        (SELECT team from TeamMembership WHERE
-                            person = %(team)s) AND
-
-                    -- The person needs to not have direct membership in the
-                    -- team.
-                    NOT EXISTS
-                        (SELECT tm1.person from TeamMembership tm1
-                            WHERE
-                                tm1.person = TeamParticipation.person and
-                                tm1.team = TeamParticipation.team and
-                                tm1.status IN %(active_states)s);
-            ''', dict(team=self.id, active_states=ACTIVE_STATES))
-
-        # Since we've updated the database behind Storm's back yet again,
-        # we need to flush its caches, again.
-        store.invalidate()
-
-        # Remove all members from the TeamParticipation table
-        # except for the team, itself.
-        participants = store.find(
-            TeamParticipation,
-            TeamParticipation.teamID == self.id,
-            TeamParticipation.personID != self.id)
-        participants.remove()
-
     def setMembershipData(self, person, status, reviewer, expires=None,
                           comment=None):
         """See `IPerson`."""

=== modified file 'lib/lp/registry/model/teammembership.py'
--- lib/lp/registry/model/teammembership.py	2011-03-09 18:18:02 +0000
+++ lib/lp/registry/model/teammembership.py	2011-03-17 20:19:01 +0000
@@ -30,6 +30,7 @@
 from canonical.database.datetimecol import UtcDateTimeCol
 from canonical.database.enumcol import EnumCol
 from canonical.database.sqlbase import (
+    cursor,
     flush_database_updates,
     SQLBase,
     sqlvalues,
@@ -493,6 +494,33 @@
                     TeamMembershipRenewalPolicy.AUTOMATIC)
         return IStore(TeamMembership).find(TeamMembership, *conditions)
 
+    def deactivateActiveMemberships(self, team, comment, reviewer):
+        """See `ITeamMembershipSet`."""
+        now = datetime.now(pytz.timezone('UTC'))
+        store = Store.of(team)
+        cur = cursor()
+        all_members = list(team.activemembers)
+        cur.execute("""
+            UPDATE TeamMembership
+            SET status=%(status)s,
+                last_changed_by=%(last_changed_by)s,
+                last_change_comment=%(comment)s,
+                date_last_changed=%(date_last_changed)s
+            WHERE
+                TeamMembership.team = %(team)s
+                AND TeamMembership.status IN %(original_statuses)s
+            """,
+            dict(
+                status=TeamMembershipStatus.DEACTIVATED,
+                last_changed_by=reviewer.id,
+                comment=comment,
+                date_last_changed=now,
+                team=team.id,
+                original_statuses=ACTIVE_STATES))
+        for member in all_members:
+            # store.invalidate() is called for each iteration.
+            _cleanTeamParticipation(member, team)
+
 
 class TeamParticipation(SQLBase):
     implements(ITeamParticipation)

=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2011-03-08 22:31:58 +0000
+++ lib/lp/registry/tests/test_person.py	2011-03-17 20:19:01 +0000
@@ -55,6 +55,7 @@
     PersonVisibility,
     )
 from lp.registry.interfaces.product import IProductSet
+from lp.registry.interfaces.teammembership import ITeamMembershipSet
 from lp.registry.model.karma import (
     KarmaCategory,
     KarmaTotalCache,
@@ -684,8 +685,9 @@
         self.assertEqual(oldest_date, person.datecreated)
 
     def _doMerge(self, test_team, target_team):
-        test_team.deactivateAllMembers(
-            comment='',
+        membershipset = getUtility(ITeamMembershipSet)
+        membershipset.deactivateActiveMemberships(
+            test_team, comment='',
             reviewer=test_team.teamowner)
         self.person_set.merge(test_team, target_team)
 

=== modified file 'lib/lp/registry/tests/test_team.py'
--- lib/lp/registry/tests/test_team.py	2011-02-01 19:16:43 +0000
+++ lib/lp/registry/tests/test_team.py	2011-03-17 20:19:01 +0000
@@ -425,68 +425,3 @@
         members = list(team.approvedmembers)
         self.assertEqual(1, len(members))
         self.assertEqual(user, members[0])
-
-
-class TestMembershipManagement(TestCaseWithFactory):
-
-    layer = DatabaseFunctionalLayer
-
-    def test_deactivateAllMembers_cleans_up_teamparticipation_deactivated(
-            self):
-        superteam = self.factory.makeTeam(name='super')
-        targetteam = self.factory.makeTeam(name='target')
-        login_celebrity('admin')
-        targetteam.join(superteam, targetteam.teamowner)
-
-        # Now we create a deactivated link for the target team's teamowner.
-        targetteam.teamowner.join(superteam, targetteam.teamowner)
-        targetteam.teamowner.leave(superteam)
-
-        self.assertEqual(
-                sorted([superteam, targetteam]),
-                sorted([team for team in
-                    targetteam.teamowner.teams_participated_in]))
-        targetteam.deactivateAllMembers(
-            comment='test',
-            reviewer=targetteam.teamowner)
-        self.assertEqual(
-            [],
-            sorted([team for team in
-                targetteam.teamowner.teams_participated_in]))
-
-    def test_deactivateAllMembers_cleans_up_teamparticipation_teamowner(self):
-        superteam = self.factory.makeTeam(name='super')
-        targetteam = self.factory.makeTeam(name='target')
-        login_celebrity('admin')
-        targetteam.join(superteam, targetteam.teamowner)
-        self.assertEqual(
-                sorted([superteam, targetteam]),
-                sorted([team for team
-                            in targetteam.teamowner.teams_participated_in]))
-        targetteam.deactivateAllMembers(
-            comment='test',
-            reviewer=targetteam.teamowner)
-        self.assertEqual(
-            [],
-            sorted([team for team
-                in targetteam.teamowner.teams_participated_in]))
-
-    def test_deactivateAllMembers_cleans_up_team_participation(self):
-        superteam = self.factory.makeTeam(name='super')
-        sharedteam = self.factory.makeTeam(name='shared')
-        anotherteam = self.factory.makeTeam(name='another')
-        targetteam = self.factory.makeTeam(name='target')
-        person = self.factory.makePerson()
-        login_celebrity('admin')
-        person.join(targetteam)
-        person.join(sharedteam)
-        person.join(anotherteam)
-        targetteam.join(superteam, targetteam.teamowner)
-        targetteam.join(sharedteam, targetteam.teamowner)
-        self.assertTrue(superteam in person.teams_participated_in)
-        targetteam.deactivateAllMembers(
-            comment='test',
-            reviewer=targetteam.teamowner)
-        self.assertEqual(
-            sorted([sharedteam, anotherteam]),
-            sorted([team for team in person.teams_participated_in]))

=== modified file 'lib/lp/registry/tests/test_teammembership.py'
--- lib/lp/registry/tests/test_teammembership.py	2011-03-01 03:52:30 +0000
+++ lib/lp/registry/tests/test_teammembership.py	2011-03-17 20:19:01 +0000
@@ -51,16 +51,18 @@
     TeamParticipation,
     )
 from lp.testing import (
+    login_celebrity,
     person_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.mail_helpers import pop_notifications
 
 
-class TestTeamMembershipSet(TestCase):
+class TestTeamMembershipSet(TestCaseWithFactory):
     layer = DatabaseFunctionalLayer
 
     def setUp(self):
+        super(TestTeamMembershipSet, self).setUp()
         login('[e-mail]')
         self.membershipset = getUtility(ITeamMembershipSet)
         self.personset = getUtility(IPersonSet)
@@ -166,6 +168,24 @@
             sample_person_on_motu.status, TeamMembershipStatus.EXPIRED)
         self.failIf(sample_person.inTeam(motu))
 
+    def test_deactivateActiveMemberships(self):
+        superteam = self.factory.makeTeam(name='super')
+        targetteam = self.factory.makeTeam(name='target')
+        member = self.factory.makePerson()
+        login_celebrity('admin')
+        targetteam.join(superteam, targetteam.teamowner)
+        targetteam.addMember(member, targetteam.teamowner)
+        targetteam.teamowner.join(superteam, targetteam.teamowner)
+        self.membershipset.deactivateActiveMemberships(
+            targetteam, comment='test', reviewer=targetteam.teamowner)
+        membership = self.membershipset.getByPersonAndTeam(member, targetteam)
+        self.assertEqual('test', membership.last_change_comment)
+        self.assertEqual(targetteam.teamowner, membership.last_changed_by)
+        self.assertEqual([], list(targetteam.allmembers))
+        self.assertEqual(
+            [superteam], list(targetteam.teamowner.teams_participated_in))
+        self.assertEqual([], list(member.teams_participated_in))
+
 
 class TeamParticipationTestCase(TestCaseWithFactory):
     """Tests for team participation using 5 teams."""
@@ -465,6 +485,31 @@
             previous_count-10,
             self.getTeamParticipationCount())
 
+    def testTeam3_deactivateActiveMemberships(self):
+        # Removing all the members of team2 will not remove memberships
+        # to super teams from other paths.
+        non_member = self.factory.makePerson()
+        self.team3.addMember(non_member, self.foo_bar, force_team_add=True)
+        previous_count = self.getTeamParticipationCount()
+        membershipset = getUtility(ITeamMembershipSet)
+        membershipset.deactivateActiveMemberships(
+            self.team3, 'gone', self.foo_bar)
+        self.assertEqual([], list(self.team3.allmembers))
+        self.assertParticipantsEquals(
+            ['name16', 'no-priv', 'team2', 'team3', 'team4', 'team5'],
+            self.team1)
+        self.assertParticipantsEquals(
+            ['name16', 'no-priv', 'team3', 'team4', 'team5'], self.team2)
+        self.assertParticipantsEquals(
+            [], self.team3)
+        self.assertParticipantsEquals(
+            ['name16', 'no-priv', 'team5'], self.team4)
+        self.assertParticipantsEquals(['name16', 'no-priv'], self.team5)
+        self.assertParticipantsEquals(
+            ['name16', 'no-priv', 'team2', 'team3', 'team4', 'team5'],
+            self.team6)
+        self.assertEqual(previous_count - 8, self.getTeamParticipationCount())
+
 
 class TestTeamMembership(TestCaseWithFactory):
     layer = DatabaseFunctionalLayer