Merge lp:~sinzui/launchpad/delete-team-3 into lp:launchpad

[Curtis Hovey]

This is my branch to unblock owners from deleting their teams.

    lp:~sinzui/launchpad/delete-team-3
    Diff size: 492
    Launchpad bug:
        https://bugs.launchpad.net/bugs/599464
        https://bugs.launchpad.net/bugs/523985
        https://bugs.launchpad.net/bugs/589125
        https://bugs.launchpad.net/bugs/577079
    Test command: ./bin/test -vv --layer=DatabaseFunctional \
        -t peoplemerge-views -t mailinglist-views
        -t test_peoplemerge -t test_teammembership
    Pre-implementation: barry
    Target release: 10.11

Unblock owners from deleting their teams
-----------------------------------------

The registry team continues to help owners delete there teams. Owners often
cannot delete the team because it has an unpurged mailing list or super
teams:

523985 teams cannot purge their mailing list
    Owners cannot purge mailing lists, a registry admin can. But in the
    case of deleting a team with a deactivated the mailing list, we know
    no one can use the list, so it is okay to automatically purge.

599464 Can't delete team with super teams
    The code doing the delete action *thinks* it is doing a merge action.
    Merge cannot handle super teams because there is a chance that a cyclic
    team member error will occur. But since delete removes the team members,
    there is never any chance of a cyclic error. Actually, the delete step
    should remove the team from the super teams immediately after the
    membership is removed.

589125 Message about deactivating the mailing list when deleting a team
        needs bling
    The sentence saying that the delete cannot be performed until the mailing
    list is purged could be bold to alert the user.

577079 PersonSet.merge must delete the team email address
    The subclass that delete teams removes the email address, but this
    rule applies to all team merges. Move the rule to the base class.

Rules
-----

On closer examination, these issues affect users merging teams too. An owner
who is merging his team into the ~registry team is doing a delete, and he
is stuck as well. (The delete rules preselect the teams used in a merge).

    * Allow owners to purge lists.
    * Update the rules to permit the owner to delete a team when the view
      cane safely purge the mailing list.
    * Allow admins/owners to retract their team's membership in another team.
    * Update the merge rules to remove the team from its super teams
      before it starts the merge into ~registry.

QA
--

    * Visit open team delete questions that are blocked by lists and
      super teams.
    * Delete all the teams.
    * Rejoice that this is that last time this will ever need to be done
      by a Registry Admins.

Lint
----

Linting changed files:
  lib/lp/registry/browser/peoplemerge.py
  lib/lp/registry/browser/team.py
  lib/lp/registry/browser/tests/mailinglist-views.txt
  lib/lp/registry/browser/tests/peoplemerge-views.txt
  lib/lp/registry/browser/tests/test_peoplemerge.py
  lib/lp/registry/interfaces/person.py
  lib/lp/registry/model/person.py
  lib/lp/registry/templates/team-delete.pt
  lib/lp/registry/tests/test_teammembership.py

Test
----

lib/lp/registry/browser/tests/mailinglist-views.txt
    * Updated tests to document that an owner can purge a mailing list.
    * This test was broken! The setup of the owner never worked (the
      view was testing anonymous). The setup of logged in user was testing
      the view implementation of security; the test now logs the correct
      user in to verify the permissions.

lib/lp/registry/browser/tests/peoplemerge-views.txt
    * Removed doctest. There is a revised test added to test_peoplemerge

lib/lp/registry/browser/tests/test_peoplemerge.py
    * Added tests for AdminTeamMergeView's handling of team email
      addresses, mailing lists, and super teams.

lib/lp/registry/tests/test_teammembership.py
   * Added tests to verify that memberships can be retracted and that the
     TeamMembershipStatus is correct.
   * Updated the tests to run on the correct layer.

Implementation
--------------

lib/lp/registry/browser/peoplemerge.py
    * Moved the delete team email address rule from the delete view to
      AdminTeamMergeView.
    * Extended AdminTeamMergeView to purge mailing lists when possible and
      to remove super teams when the merge is with Registry Admins.

lib/lp/registry/browser/team.py
    * Replaced the view implementation of security with standard security
      checkers. This change implicitly allows the team owner to purge his
      list.

lib/lp/registry/interfaces/person.py
    * Added retractTeamMembership() so that teams can leave other teams.
    * This is a partial fix for bugs 239486 and 656782 where users cannot
      retract memberships in PENDING or INVITED states. Savvy user can
      use the API to fix the issue. The real fix will require a lot of
      UI work.

lib/lp/registry/model/person.py
    * Reimplemented Person.leave() to be a specific case of
      Person.retractTeamMembership().
    * Added retractTeamMembership() as a stormification of the Person.leave()
      method.
    * PENDING and INVITED states are supported because there are edge cases
      in production where users want to delete a team, but it has a
      membership stuck in a another team's join queue.

lib/lp/registry/templates/team-delete.pt
    * Made the mailing list message bold so that users immediately know why
      there is not Delete button shown on the page.

[Henning Eggers (henninge)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Curtis,
thank you for your many fixes and the extensive cover letter. Much appreciated.

An overall note on tests comments: "Verify ..." is noise on a test comment
because "verifying" is what tests are supposed to be doing. Our style guide
even mentions that. They are fixed easily, though:
# Verify that inactive lists do not block merges.
becomes
# Inactive lists do not block merges.

Am 01.11.2010 14:12, schrieb Curtis Hovey:
> === modified file 'lib/lp/registry/browser/peoplemerge.py'
> --- lib/lp/registry/browser/peoplemerge.py 2010-09-23 15:34:05 +0000
> +++ lib/lp/registry/browser/peoplemerge.py 2010-11-01 13:11:51 +0000
> @@ -34,13 +34,14 @@
> LaunchpadView,
> )
> from canonical.launchpad.webapp.interfaces import ILaunchBag
> -from lp.registry.interfaces.mailinglist import MailingListStatus
> +from lp.registry.interfaces.mailinglist import PURGE_STATES
> from lp.registry.interfaces.person import (
> IAdminPeopleMergeSchema,
> IAdminTeamMergeSchema,
> IPersonSet,
> IRequestPeopleMerge,
> )
> +from lp.services.propertycache import cachedproperty
>
>
> class RequestPeopleMergeView(LaunchpadFormView):
> @@ -216,7 +217,27 @@
> def hasMailingList(self, team):
> return (
> team.mailing_list is not None
> - and team.mailing_list.status != MailingListStatus.PURGED)
> + and team.mailing_list.status not in PURGE_STATES)
> +
> + @cachedproperty
> + def registry_experts(self):
> + return getUtility(ILaunchpadCelebrities).registry_experts
> +
> + def doMerge(self, data):
> + """Purge the non-transferable team data and merge."""
> + # A team cannot have more than one mailing list. The old list will
> + # remain in the archive.
> + if self.dupe_person.mailing_list is not None:
> + self.dupe_person.mailing_list.purge()
> + # A team cannot have more than one email address; they are not

Well "it is not transferable" sounds more reasonable to me because it is just
one address.

> + # transferable because the identity of the team has changed.
> + self.dupe_person.setContactAddress(None)
> + # The registry experts does not want to acquire super teams from a
> + # merge.
> + if self.target_person is self.registry_experts:
> + for team in self.dupe_person.teams_participated_in:
> + self.dupe_person.retractTeamMembership(team, self.user)
> + super(AdminTeamMergeView, self).doMerge(data)
>
> def validate(self, data):
> """Check there are no mailing lists associated with the dupe team."""
> @@ -228,9 +249,14 @@
>
> super(AdminTeamMergeView, self).validate(data)
> dupe_team = data['dupe_person']
> - # Our code doesn't know how to merge a team's superteams, so we
> - # prohibit that here.
> - if dupe_team.super_teams.count() > 0:
> + target_team = data['target_person']
> + # Merge cannot reconcile cyclic membership in super teams.
> + # Super team memberships are automatically removed when merging into
> + # the re...

review: Approve (code)

[Curtis Hovey (sinzui)]

Thanks for the review.

On Mon, 2010-11-01 at 17:46 +0000, Henning Eggers wrote:
> > + @call_with(user=REQUEST_USER)
> > + @operation_parameters(
> > + team=copy_field(ITeamMembership['team'],
> > + comment=Text()))
> > + @export_write_operation()
> > + def retractTeamMembership(team, user, comment=None):
> > + """Retract this team's membership in the given team.
> > +
> > + This is the team equivalent of user.leave(team).
>
> Apart from the fact that parameters are not documented, I find this
> description misleading. In fact "leave" is specialized version of
> "retractTeamMembership" and I first wondered why you introduced the latter
> instead of revamping the former. I realized that it is for API consistency.
> But the comments should reflect that "leave" now is a convenience method to
> "retractTeamMembership".

Thanks for prompting me to revise this. My change implicitly fixed the
situation where the user wants to retract a PROPOSED membership in a
team. I updated the docstrings to explain what really happen.

{{{
=== modified file 'lib/lp/registry/browser/peoplemerge.py'
--- lib/lp/registry/browser/peoplemerge.py 2010-10-31 22:03:40 +0000
+++ lib/lp/registry/browser/peoplemerge.py 2010-11-01 19:59:54 +0000
@@ -229,8 +229,7 @@
         # remain in the archive.
         if self.dupe_person.mailing_list is not None:
             self.dupe_person.mailing_list.purge()
- # A team cannot have more than one email address; they are not
- # transferable because the identity of the team has changed.
+ # Team email addresses are not transferable.
         self.dupe_person.setContactAddress(None)
         # The registry experts does not want to acquire super teams from a
         # merge.
@@ -255,8 +254,8 @@
         # 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.
- if (target_team != self.registry_experts
- and dupe_team.super_teams.count() > 0):
+ 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)))

=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py 2010-10-31 22:21:20 +0000
+++ lib/lp/registry/browser/team.py 2010-11-01 20:10:37 +0000
@@ -742,8 +742,10 @@
         or INACTIVE states. Further, the user doing the purging, must be
         an owner, Launchpad administrator or mailing list expert.
         """
- if (check_permission('launchpad.Moderate', self.context) or
- check_permission('launchpad.MailingListManager', self.context)):
+ is_moderator = check_permission('launchpad.Moderate', self.context)
+ is_mailing_list_manager = check_permission(
+ 'launchpad.MailingListManager', self.context)
+ if is_moderator or is_mailing_list_manager:
             return self.getListInState(*PURGE_STATES) is not None
         else:
             return...

=== modified file 'lib/lp/registry/browser/peoplemerge.py'
--- lib/lp/registry/browser/peoplemerge.py	2010-09-23 15:34:05 +0000
+++ lib/lp/registry/browser/peoplemerge.py	2010-11-03 22:16:52 +0000
@@ -34,13 +34,17 @@
     LaunchpadView,
     )
 from canonical.launchpad.webapp.interfaces import ILaunchBag
-from lp.registry.interfaces.mailinglist import MailingListStatus
+from lp.registry.interfaces.mailinglist import (
+    MailingListStatus,
+    PURGE_STATES,
+    )
 from lp.registry.interfaces.person import (
     IAdminPeopleMergeSchema,
     IAdminTeamMergeSchema,
     IPersonSet,
     IRequestPeopleMerge,
     )
+from lp.services.propertycache import cachedproperty
 
 
 class RequestPeopleMergeView(LaunchpadFormView):
@@ -214,9 +218,32 @@
     schema = IAdminTeamMergeSchema
 
     def hasMailingList(self, team):
+        unused_states = [state for state in PURGE_STATES]
+        unused_states.append(MailingListStatus.PURGED)
         return (
             team.mailing_list is not None
-            and team.mailing_list.status != MailingListStatus.PURGED)
+            and team.mailing_list.status not in unused_states)
+
+    @cachedproperty
+    def registry_experts(self):
+        return getUtility(ILaunchpadCelebrities).registry_experts
+
+    def doMerge(self, data):
+        """Purge the non-transferable team data and merge."""
+        # 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.
+        if self.target_person is self.registry_experts:
+            for team in self.dupe_person.teams_participated_in:
+                self.dupe_person.retractTeamMembership(team, self.user)
+        super(AdminTeamMergeView, self).doMerge(data)
 
     def validate(self, data):
         """Check there are no mailing lists associated with the dupe team."""
@@ -228,9 +255,14 @@
 
         super(AdminTeamMergeView, self).validate(data)
         dupe_team = data['dupe_person']
-        # Our code doesn't know how to merge a team's superteams, so we
-        # prohibit that here.
-        if dupe_team.super_teams.count() > 0:
+        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)))
@@ -330,9 +362,6 @@
     @action('Delete', name='delete', condition=canDelete)
     def merge_action(self, action, data):
         base = super(DeleteTeamView, self)
-        # Delete is implemented as a merge process, but email addresses should
-        # be deleted because ~registry can never claim them.
-        self.context.setContactAddress(None)
         base.deactivate_members_and_merge_action.success(data)
 
 

=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py	2010-09-25 14:29:32 +0000
+++ lib/lp/registry/browser/team.py	2010-11-03 22:16:52 +0000
@@ -44,7 +44,6 @@
 from canonical.launchpad import _
 from canonical.launchpad.interfaces.authtoken import LoginTokenType
 from canonical.launchpad.interfaces.emailaddress import IEmailAddressSet
-from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.interfaces.logintoken import ILoginTokenSet
 from canonical.launchpad.interfaces.validation import validate_new_team_email
 from canonical.launchpad.validators import LaunchpadValidationError
@@ -78,7 +77,6 @@
     )
 from lp.registry.interfaces.person import (
     ImmutableVisibilityError,
-    IPerson,
     IPersonSet,
     ITeam,
     ITeamContactAddressForm,
@@ -742,15 +740,15 @@
 
         The list must exist and be in one of the REGISTERED, DECLINED, FAILED,
         or INACTIVE states.  Further, the user doing the purging, must be
-        a Launchpad administrator or mailing list expert.
+        an owner, Launchpad administrator or mailing list expert.
         """
-        requester = IPerson(self.request.principal, None)
-        celebrities = getUtility(ILaunchpadCelebrities)
-        if (requester is None or
-            (not requester.inTeam(celebrities.admin) and
-             not requester.inTeam(celebrities.mailing_list_experts))):
+        is_moderator = check_permission('launchpad.Moderate', self.context)
+        is_mailing_list_manager = check_permission(
+            'launchpad.MailingListManager', self.context)
+        if is_moderator or is_mailing_list_manager:
+            return self.getListInState(*PURGE_STATES) is not None
+        else:
             return False
-        return self.getListInState(*PURGE_STATES) is not None
 
 
 class TeamMailingListSubscribersView(LaunchpadView):

=== modified file 'lib/lp/registry/browser/tests/mailinglist-views.txt'
--- lib/lp/registry/browser/tests/mailinglist-views.txt	2010-08-28 23:01:18 +0000
+++ lib/lp/registry/browser/tests/mailinglist-views.txt	2010-11-03 22:16:52 +0000
@@ -76,7 +76,7 @@
     >>> team_one, list_one = factory.makeTeamAndMailingList(
     ...     'team-one', 'no-priv')
 
-    >>> the_owner = team_one.teamowner.preferredemail.email
+    >>> the_owner = team_one.teamowner
 
     >>> from canonical.launchpad.interfaces.launchpad import (
     ...     ILaunchpadCelebrities)
@@ -85,9 +85,9 @@
     >>> an_admin = list(celebrities.admin.allmembers)[0]
 
     >>> def create_view(principal, form=None):
+    ...     login_person(principal)
     ...     return create_initialized_view(
-    ...         team_one, '+mailinglist',
-    ...         form=form, principal=principal)
+    ...         team_one, '+mailinglist', form=form)
 
 Nobody can purge an active mailing list, the team owner...
 
@@ -133,15 +133,13 @@
     >>> logout()
     >>> transaction.commit()
 
-The team owner cannot purge his list...
+The team owner can purge his list, as well as a Launchpad administrator and
+a mailing list expert.
 
     >>> login(ANONYMOUS)
     >>> view = create_view(the_owner)
     >>> view.list_can_be_purged
-    False
-
-...but a Launchpad administrator, or mailing list expert can purge the mailing
-list.
+    True
 
     >>> view = create_view(an_admin)
     >>> view.list_can_be_purged

=== modified file 'lib/lp/registry/browser/tests/peoplemerge-views.txt'
--- lib/lp/registry/browser/tests/peoplemerge-views.txt	2010-10-18 22:24:59 +0000
+++ lib/lp/registry/browser/tests/peoplemerge-views.txt	2010-11-03 22:16:52 +0000
@@ -9,7 +9,8 @@
 Create a member of the registry team that is not a member of the admins
 team.
 
-    >>> from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+    >>> from canonical.launchpad.interfaces.launchpad import (
+    ...     ILaunchpadCelebrities)
     >>> from lp.registry.interfaces.person import IPersonSet
     >>> registry_experts = getUtility(ILaunchpadCelebrities).registry_experts
     >>> person_set = getUtility(IPersonSet)
@@ -248,33 +249,6 @@
     >>> print find_tag_by_id(content, 'field.actions.delete')
     None
 
-The registry experts can delete a team with an email address. The email
-address is deleted instead of being transferred to the registry experts team.
-
-    >>> from canonical.launchpad.interfaces.emailaddress import (
-    ...     IEmailAddressSet, EmailAddressStatus)
-    >>> login_person(registry_expert)
-    >>> admin = getUtility(ILaunchpadCelebrities).admin
-    >>> registry_expert.inTeam(admin)
-    False
-    >>> deletable_team = factory.makeTeam()
-    >>> email = factory.makeEmail(
-    ...     "[e-mail]", deletable_team,
-    ...     email_status=EmailAddressStatus.NEW)
-    >>> for email in getUtility(IEmailAddressSet).getByPerson(deletable_team):
-    ...     print email.email, email.status.title
-    [e-mail] New Email Address
-    >>> form = {'field.actions.delete': 'Delete'}
-    >>> view = create_initialized_view(deletable_team, '+delete', form=form)
-    >>> view.errors
-    []
-    >>> for notification in view.request.response.notifications:
-    ...     print notification.message
-    Team deleted.
-    >>> emails = getUtility(IEmailAddressSet).getByPerson(registry_experts)
-    >>> emails.count()
-    0
-
 Private teams can be deleted by admins.
 
     >>> from lp.registry.interfaces.person import PersonVisibility

=== modified file 'lib/lp/registry/browser/tests/test_peoplemerge.py'
--- lib/lp/registry/browser/tests/test_peoplemerge.py	2010-10-26 15:47:24 +0000
+++ lib/lp/registry/browser/tests/test_peoplemerge.py	2010-11-03 22:16:52 +0000
@@ -6,14 +6,24 @@
 
 from zope.component import getUtility
 
+from canonical.launchpad.interfaces.emailaddress import (
+    EmailAddressStatus,
+    IEmailAddressSet,
+    )
+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.registry.interfaces.person import IPersonSet
+from lp.registry.interfaces.mailinglist import MailingListStatus
 from lp.testing import (
+    login_celebrity,
     login_person,
     person_logged_in,
     TestCaseWithFactory,
     )
-from lp.testing.views import create_view
+from lp.testing.views import (
+    create_initialized_view,
+    create_view,
+    )
 
 
 class TestRequestPeopleMergeMultipleEmailsView(TestCaseWithFactory):
@@ -71,3 +81,54 @@
             method='POST')
         view.processForm()
         self.verify_user_must_reselect_email_addresses(view)
+
+
+class TestAdminTeamMergeView(TestCaseWithFactory):
+    """Test the AdminTeamMergeView rules."""
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestAdminTeamMergeView, self).setUp()
+        self.person_set = getUtility(IPersonSet)
+        self.dupe_team = self.factory.makeTeam()
+        self.target_team = self.factory.makeTeam()
+        login_celebrity('registry_experts')
+
+    def getView(self):
+        form = {
+            'field.dupe_person': self.dupe_team.name,
+            'field.target_person': self.target_team.name,
+            'field.actions.deactivate_members_and_merge': 'Merge',
+            }
+        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(
+            "[e-mail]", self.dupe_team, email_status=EmailAddressStatus.NEW)
+        view = self.getView()
+        self.assertEqual([], view.errors)
+        self.assertEqual(self.target_team, self.dupe_team.merged)
+        emails = getUtility(IEmailAddressSet).getByPerson(self.target_team)
+        self.assertEqual(0, emails.count())
+
+    def test_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()
+        self.dupe_team.join(super_team, self.dupe_team.teamowner)
+        view = self.getView()
+        self.assertEqual([], view.errors)
+        self.assertEqual(self.target_team, self.dupe_team.merged)

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2010-11-03 20:24:36 +0000
+++ lib/lp/registry/interfaces/person.py	2010-11-03 22:16:52 +0000
@@ -1454,14 +1454,11 @@
     def leave(team):
         """Leave the given team.
 
-        If there's a membership entry for this person on the given team and
-        its status is either APPROVED or ADMIN, we change the status to
-        DEACTIVATED and remove the relevant entries in teamparticipation.
+        This is a convenience method for retractTeamMembership() that allows
+        a user to leave the given team, or to cancel a PENDING membership
+        request.
 
-        Teams cannot call this method because they're not allowed to
-        login and thus can't 'leave' another team. Instead, they have their
-        subscription deactivated (using the setMembershipData() method) by
-        a team administrator.
+        :param team: The team to leave.
         """
 
     @operation_parameters(
@@ -1523,7 +1520,7 @@
         :return: A tuple containing a boolean indicating when the
             membership status changed and the current `TeamMembershipStatus`.
             This depends on the desired status passed as an argument, the
-            subscription policy and the user's priveleges.
+            subscription policy and the user's privileges.
         """
 
     @operation_parameters(
@@ -1550,6 +1547,22 @@
         to INVITATION_DECLINED.
         """
 
+    def retractTeamMembership(team, user, comment=None):
+        """Retract this team's membership in the given team.
+
+        If there's a membership entry for this team on the given team and
+        its status is either APPROVED, ADMIN, PENDING, or INVITED, the status
+        is changed and the relevant entries in TeamParticipation.
+
+        APPROVED and ADMIN status are changed to DEACTIVATED.
+        PENDING status is changed to DECLINED.
+        INVITED status is changes to INVITATION_DECLINED.
+
+        :param team: The team to leave.
+        :param user: The user making the retraction.
+        :param comment: An optional explanation about why the change was made.
+        """
+
     def renewTeamMembership(team):
         """Renew the TeamMembership for this person on the given team.
 

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2010-11-02 20:10:56 +0000
+++ lib/lp/registry/model/person.py	2010-11-03 22:16:52 +0000
@@ -1234,17 +1234,7 @@
     def leave(self, team):
         """See `IPerson`."""
         assert not ITeam.providedBy(self)
-
-        self._inTeam_cache = {} # Flush the cache used by the inTeam method
-
-        active = [TeamMembershipStatus.ADMIN, TeamMembershipStatus.APPROVED]
-        tm = TeamMembership.selectOneBy(person=self, team=team)
-        if tm is None or tm.status not in active:
-            # Ok, we're done. You are not an active member and still
-            # not being.
-            return
-
-        tm.setStatus(TeamMembershipStatus.DEACTIVATED, self)
+        self.retractTeamMembership(team, self)
 
     def join(self, team, requester=None, may_subscribe_to_list=True):
         """See `IPerson`."""
@@ -1397,6 +1387,28 @@
             TeamMembershipStatus.INVITATION_DECLINED,
             getUtility(ILaunchBag).user, comment=comment)
 
+    def retractTeamMembership(self, team, user, comment=None):
+        """See `IPerson`"""
+        # Include PROPOSED and INVITED so that teams can retract mistakes
+        # without involving members of the other team.
+        active_and_transitioning = {
+            TeamMembershipStatus.ADMIN: TeamMembershipStatus.DEACTIVATED,
+            TeamMembershipStatus.APPROVED: TeamMembershipStatus.DEACTIVATED,
+            TeamMembershipStatus.PROPOSED: TeamMembershipStatus.DECLINED,
+            TeamMembershipStatus.INVITED:
+                TeamMembershipStatus.INVITATION_DECLINED,
+            }
+        constraints = And(
+            TeamMembership.personID == self.id,
+            TeamMembership.teamID == team.id,
+            TeamMembership.status.is_in(active_and_transitioning.keys()))
+        tm = Store.of(self).find(TeamMembership, constraints).one()
+        if tm is not None:
+            # Flush the cache used by the inTeam method.
+            self._inTeam_cache = {}
+            new_status = active_and_transitioning[tm.status]
+            tm.setStatus(new_status, user, comment=comment)
+
     def renewTeamMembership(self, team):
         """Renew the TeamMembership for this person on the given team.
 

=== modified file 'lib/lp/registry/stories/mailinglists/lifecycle.txt'
--- lib/lp/registry/stories/mailinglists/lifecycle.txt	2010-10-18 22:24:59 +0000
+++ lib/lp/registry/stories/mailinglists/lifecycle.txt	2010-11-03 22:16:52 +0000
@@ -60,18 +60,7 @@
 address until Mailman has acknowledged successful creation.
 
     >>> browser.reload()
-    >>> browser.getLink(url='+contactaddress').click()
-    >>> browser.getControl('The Launchpad mailing list')
-    Traceback (most recent call last):
-    ...
-    LookupError: ...
-
     >>> browser.getLink(url='+mailinglist').click()
-    >>> browser.getControl('Apply for Mailing List')
-    Traceback (most recent call last):
-    ...
-    LookupError: ...
-
     >>> print mailing_list_status_message(browser.contents)
     This team's mailing list will be available within a few minutes.
 
@@ -113,11 +102,6 @@
     >>> from canonical.launchpad.testing.pages import strip_label
 
     >>> browser.getLink(url='+mailinglist').click()
-    >>> browser.getControl('Apply for Mailing List')
-    Traceback (most recent call last):
-    ...
-    LookupError: ...
-
     >>> print extract_text(find_tag_by_id(browser.contents,
     ...                                   'mailing_list_not_contact_address'))
     The mailing list is not set as the team contact address. You can
@@ -137,15 +121,6 @@
     >>> print browser.getLink(url='+mailinglist').url
     http://launchpad.dev/~landscape-developers/+mailinglist
 
-Although of course, it's not available to someone with no permission to
-manipulate the list.
-
-    >>> user_browser.open('http://launchpad.dev/~landscape-developers')
-    >>> user_browser.getLink(url='+mailinglist')
-    Traceback (most recent call last):
-    ...
-    LinkNotFoundError
-
 When the mailing list is not the team's contact address, the mailing
 list configuration screen displays a message to this effect.
 
@@ -324,7 +299,7 @@
     ...     print mailing_list.status.name
     ...     logout()
 
-The team owner cannot purge a mailing list.
+The team owner can see that he can purge or reactivate mailing list.
 
     >>> user_browser.open('http://launchpad.dev/~aardvarks/+mailinglist')
     >>> user_browser.getControl('Apply for Mailing List').click()
@@ -345,14 +320,13 @@
 
     >>> user_browser.getLink(url='+mailinglist').click()
     >>> print purge_text(user_browser)
-    None
-
-But a Launchpad administrator can purge a list.
-
-    >>> admin_browser.open('http://launchpad.dev/~aardvarks/+mailinglist')
-    >>> print purge_text(admin_browser)
     You can purge this mailing list...
 
+    >>> user_browser.getControl('Reactivate this Mailing List')
+    <SubmitControl name='field.actions.reactivate_list' type='submit'>
+    >>> user_browser.getControl('Purge this Mailing List')
+    <SubmitControl name='field.actions.purge_list' type='submit'>
+
 Mailing list experts can also purge mailing lists.  Sample Person is
 trustworthy enough to become a mailing list expert, but not a Launchpad
 administrator.  He's given mailing list expert authority so that he can
@@ -377,35 +351,12 @@
     >>> print purge_text(expert_browser)
     You can purge this mailing list...
 
-Nobody can purge an active mailing list.
-
-    >>> user_browser.open('http://launchpad.dev/~aardvarks/+mailinglist')
-    >>> user_browser.getControl('Reactivate this Mailing List').click()
-    >>> act()
-    >>> print_list_state()
-    ACTIVE
-
-    >>> admin_browser.reload()
-    >>> expert_browser.reload()
-    >>> print find_tag_by_id(admin_browser.contents, 'mailing_list_purge')
-    None
-    >>> print find_tag_by_id(expert_browser.contents, 'mailing_list_purge')
-    None
-
-    >>> act()
-    >>> print_list_state()
-    ACTIVE
-    >>> admin_browser.reload()
-    >>> expert_browser.reload()
-    >>> print find_tag_by_id(admin_browser.contents, 'mailing_list_purge')
-    None
-    >>> print find_tag_by_id(expert_browser.contents, 'mailing_list_purge')
-    None
-
 A constructing, modified, updating, or deactivating or mod-failed list cannot
 be purged.
 
     >>> from zope.security.proxy import removeSecurityProxy
+    >>> from lp.registry.interfaces.mailinglist import MailingListStatus
+
     >>> def set_list_state(team_name, status):
     ...     login('[e-mail]')
     ...     mailing_list = getUtility(IMailingListSet).get(team_name)
@@ -424,38 +375,6 @@
     ...         expert_browser.open(url)
     ...         print purge_text(expert_browser)
 
-    >>> from lp.registry.interfaces.mailinglist import MailingListStatus
-    >>> show_states(MailingListStatus.CONSTRUCTING,
-    ...             MailingListStatus.MODIFIED,
-    ...             MailingListStatus.UPDATING,
-    ...             MailingListStatus.DEACTIVATING,
-    ...             MailingListStatus.MOD_FAILED)
-    CONSTRUCTING
-    None
-    None
-    MODIFIED
-    None
-    None
-    UPDATING
-    None
-    None
-    DEACTIVATING
-    None
-    None
-    MOD_FAILED
-    None
-    None
-
-A declined or failed list can be purged.
-
-    >>> show_states(MailingListStatus.DECLINED, MailingListStatus.FAILED)
-    DECLINED
-    You can purge this mailing list...
-    You can purge this mailing list...
-    FAILED
-    You can purge this mailing list...
-    You can purge this mailing list...
-
 A purged list acts as if it doesn't even exist.
 
     >>> set_list_state('aardvarks', MailingListStatus.PURGED)
@@ -468,32 +387,10 @@
     >>> expert_browser.getControl('Apply for Mailing List')
     <SubmitControl name='field.actions.request_list_creation' type='submit'>
 
-To the team owner, an inactive list can merely be reactivated, it still cannot
-be purged.
+The team owner can see that an inactive list can be reactivated or purged.
 
     >>> set_list_state('aardvarks', MailingListStatus.INACTIVE)
 
-    >>> user_browser.open('http://launchpad.dev/~aardvarks/+mailinglist')
-    >>> user_browser.getControl('Reactivate this Mailing List')
-    <SubmitControl name='field.actions.reactivate_list' type='submit'>
-    >>> print purge_text(user_browser)
-    None
-
-However to the LP administrator or mailing list expert, the list can also be
-purged.
-
-    >>> expert_browser.open('http://launchpad.dev/~aardvarks/+mailinglist')
-    >>> expert_browser.getControl('Reactivate this Mailing List')
-    <SubmitControl name='field.actions.reactivate_list' type='submit'>
-    >>> print purge_text(expert_browser)
-    You can purge this mailing list...
-
-    >>> admin_browser.open('http://launchpad.dev/~aardvarks/+mailinglist')
-    >>> admin_browser.getControl('Reactivate this Mailing List')
-    <SubmitControl name='field.actions.reactivate_list' type='submit'>
-    >>> print purge_text(admin_browser)
-    You can purge this mailing list...
-
 
 Actions on purged mailing lists
 ===============================
@@ -522,21 +419,13 @@
     <Link text='View archive'
           url='http://lists.launchpad.dev/aardvarks'>
 
-A team with an active mailing list cannot be renamed.
-
-    >>> user_browser.open('http://launchpad.dev/~aardvarks/+edit')
-    >>> user_browser.getControl(name='field.name').value = 'antelopes'
-    Traceback (most recent call last):
-    ...
-    LookupError: name 'field.name'
-
-But once the mailing list has been purged, the team can be renamed.
+Once the mailing list has been purged, the team can be renamed.
 
     >>> user_browser.open('http://launchpad.dev/~aardvarks/+mailinglist')
     >>> user_browser.getControl('Deactivate this Mailing List').click()
     >>> act()
-    >>> admin_browser.open('http://launchpad.dev/~aardvarks/+mailinglist')
-    >>> admin_browser.getControl('Purge this Mailing List').click()
+    >>> user_browser.open('http://launchpad.dev/~aardvarks/+mailinglist')
+    >>> user_browser.getControl('Purge this Mailing List').click()
 
     >>> user_browser.open('http://launchpad.dev/~aardvarks/+edit')
     >>> user_browser.getControl(name='field.name').value = 'antelopes'
@@ -571,8 +460,8 @@
 But if the mailing list for the team being merge has been purged, then the
 merge is allowed.
 
-    >>> user_browser.open('http://launchpad.dev/~antelopes/+mailinglist')
-    >>> user_browser.getControl('Deactivate this Mailing List').click()
+    >>> admin_browser.open('http://launchpad.dev/~antelopes/+mailinglist')
+    >>> admin_browser.getControl('Deactivate this Mailing List').click()
     >>> act()
     >>> admin_browser.open('http://launchpad.dev/~antelopes/+mailinglist')
     >>> admin_browser.getControl('Purge this Mailing List').click()

=== modified file 'lib/lp/registry/templates/team-delete.pt'
--- lib/lp/registry/templates/team-delete.pt	2009-12-24 01:26:58 +0000
+++ lib/lp/registry/templates/team-delete.pt	2010-11-03 22:16:52 +0000
@@ -12,8 +12,8 @@
       </p>
 
       <p tal:condition="view/has_mailing_list">
-        This team cannot be deleted until its mailing list is first
-        deactivated, then purged after the deactivation is confirmed.
+        <strong>This team cannot be deleted until its mailing list is first
+        deactivated, then purged after the deactivation is confirmed.</strong>
       </p>
 
       <p tal:condition="context/activemembers/count">

=== modified file 'lib/lp/registry/tests/test_teammembership.py'
--- lib/lp/registry/tests/test_teammembership.py	2010-10-04 19:50:45 +0000
+++ lib/lp/registry/tests/test_teammembership.py	2010-11-03 22:16:52 +0000
@@ -30,7 +30,7 @@
     setUp,
     tearDown,
     )
-from canonical.testing.layers import LaunchpadFunctionalLayer
+from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.registry.interfaces.person import (
     IPersonSet,
     TeamSubscriptionPolicy,
@@ -45,7 +45,7 @@
 
 
 class TestTeamMembershipSet(unittest.TestCase):
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         login('[e-mail]')
@@ -157,7 +157,7 @@
 
 class TeamParticipationTestCase(unittest.TestCase):
     """Tests for team participation using 5 teams."""
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         login('[e-mail]')
@@ -190,6 +190,7 @@
                     team5
                        no-priv
     """
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         """Setup the team hierarchy."""
@@ -256,6 +257,7 @@
                      team5
                        no-priv
     """
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         """Setup the team hierarchy."""
@@ -320,6 +322,7 @@
                      team5
                        no-priv
     """
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         """Setup the team hierarchy."""
@@ -381,7 +384,7 @@
 
 
 class TestTeamMembership(unittest.TestCase):
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def test_teams_not_kicked_from_themselves_bug_248498(self):
         """The self-participation of a team must not be removed.
@@ -443,7 +446,7 @@
 
 class TestTeamMembershipSetStatus(unittest.TestCase):
     """Test the behaviour of TeamMembership's setStatus()."""
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         login('[e-mail]')
@@ -651,9 +654,52 @@
         team1_on_team2.setStatus(TeamMembershipStatus.ADMIN, self.foobar)
         self.assertEqual(team1_on_team2.status, TeamMembershipStatus.ADMIN)
 
+    def test_invited_member_can_be_declined(self):
+        # A team can decline an invited member.
+        self.team2.addMember(self.team1, self.no_priv)
+        tm = getUtility(ITeamMembershipSet).getByPersonAndTeam(
+            self.team1, self.team2)
+        tm.setStatus(
+            TeamMembershipStatus.INVITATION_DECLINED, self.team2.teamowner)
+        self.assertEqual(TeamMembershipStatus.INVITATION_DECLINED, tm.status)
+
+    def test_retractTeamMembership_invited(self):
+        # A team can retract a membership invitation.
+        self.team2.addMember(self.team1, self.no_priv)
+        self.team1.retractTeamMembership(self.team2, self.team1.teamowner)
+        tm = getUtility(ITeamMembershipSet).getByPersonAndTeam(
+            self.team1, self.team2)
+        self.assertEqual(TeamMembershipStatus.INVITATION_DECLINED, tm.status)
+
+    def test_retractTeamMembership_proposed(self):
+        # A team can retract the proposed membership in a team.
+        self.team2.subscriptionpolicy = TeamSubscriptionPolicy.MODERATED
+        self.team1.join(self.team2, self.team1.teamowner)
+        self.team1.retractTeamMembership(self.team2, self.team1.teamowner)
+        tm = getUtility(ITeamMembershipSet).getByPersonAndTeam(
+            self.team1, self.team2)
+        self.assertEqual(TeamMembershipStatus.DECLINED, tm.status)
+
+    def test_retractTeamMembership_active(self):
+        # A team can retract the membership in a team.
+        self.team1.join(self.team2, self.team1.teamowner)
+        self.team1.retractTeamMembership(self.team2, self.team1.teamowner)
+        tm = getUtility(ITeamMembershipSet).getByPersonAndTeam(
+            self.team1, self.team2)
+        self.assertEqual(TeamMembershipStatus.DEACTIVATED, tm.status)
+
+    def test_retractTeamMembership_admin(self):
+        # A team can retract the membership in a team.
+        self.team1.join(self.team2, self.team1.teamowner)
+        tm = getUtility(ITeamMembershipSet).getByPersonAndTeam(
+            self.team1, self.team2)
+        tm.setStatus(TeamMembershipStatus.ADMIN, self.team2.teamowner)
+        self.team1.retractTeamMembership(self.team2, self.team1.teamowner)
+        self.assertEqual(TeamMembershipStatus.DEACTIVATED, tm.status)
+
 
 class TestCheckTeamParticipationScript(unittest.TestCase):
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def _runScript(self, expected_returncode=0):
         process = subprocess.Popen(
@@ -760,6 +806,6 @@
     suite = unittest.TestLoader().loadTestsFromName(__name__)
     bug_249185 = LayeredDocFileSuite(
         'bug-249185.txt', optionflags=default_optionflags,
-        layer=LaunchpadFunctionalLayer, setUp=setUp, tearDown=tearDown)
+        layer=DatabaseFunctionalLayer, setUp=setUp, tearDown=tearDown)
     suite.addTest(bug_249185)
     return suite