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

[Curtis Hovey]

This is my branch to allow renamed teams with purged mailing lists to set
their contact address.

    lp:~sinzui/launchpad/delete-team-2
    Diff size: 62
    Launchpad bug:
          https://bugs.launchpad.net/bugs/619022
    Test command: ./bin/test -vv -t TestTeamContactAddress
    Pre-implementation: EdwinGrubbs
    Target release: 10.09

Allow renamed teams with purged mailing lists to set their contact address
--------------------------------------------------------------------------

This team had a list purged, then it was renamed. The team has the old mailing
list address but the mailing list does not. This is expected.
setContactAddress should not assume that it will always have an email address
for a mailing list. It is okay to be None, since the deletion loop does the
right thing, but the MasterStore step is not needed.

There was some discussion about whether the email address should be deleted
when the list is purged. It could be, but we need a reallocation mechanism
if the list is recreated. This issue was caused by a rename, and we still
have many examples where renaming persons breaks other things. We really
want to make rename safe for mailing lists and PPAs. Since renames can happen
via the SQL, we cannot entertain such an enhancement until we have a UI
that supports all the cases.

Rules
-----

    * Check is mailing_list_email is not None before getting the master store.

QA
--

    * Visit https://edge.launchpad.net/~ubuntu-l10n-nds
    * Delete the team (which implicitly sets the address to None).
    * Verify you see the people and teams search page.

Lint
----

Paste Linting changed files:
  lib/lp/registry/model/person.py
  lib/lp/registry/tests/test_team.py

I can clean up this file after the review.
./lib/lp/registry/model/person.py
      66: 'safe_hasattr' imported but unused
      64: E501 line too long (83 characters)
      68: E501 line too long (80 characters)
    1450: W291 trailing whitespace
    1491: W291 trailing whitespace
    1492: E501 line too long (82 characters)
    1522: E301 expected 1 blank line, found 0
    1559: E501 line too long (82 characters)
      64: Line exceeds 78 characters.
      68: Line exceeds 78 characters.
    1450: Line has trailing whitespace.
    1478: Line exceeds 78 characters.
    1491: Line has trailing whitespace.
    1492: Line exceeds 78 characters.
    1559: Line exceeds 78 characters. lint report

Test
----

    * lib/lp/registry/tests/test_team.py
      * Added a test that repeats the events that happened to the problem
        team and verified it can set its contact address.

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

    * lib/lp/registry/model/person.py
      * Added check to see if the address is None before

--
__Curtis C. Hovey_________
http://launchpad.net/

[Edwin Grubbs (edwin-grubbs)]

Hi Curtis,

This looks good. One minor comment: login_person and login_celebrity are not in alphabetical order.

-Edwin

review: Approve

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2010-08-17 13:53:04 +0000
+++ lib/lp/registry/model/person.py	2010-08-18 15:54:47 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 from __future__ import with_statement
 
@@ -2179,7 +2179,8 @@
         if self.mailing_list is not None:
             mailing_list_email = getUtility(IEmailAddressSet).getByEmail(
                 self.mailing_list.address)
-            mailing_list_email = IMasterObject(mailing_list_email)
+            if mailing_list_email is not None:
+                mailing_list_email = IMasterObject(mailing_list_email)
         else:
             mailing_list_email = None
         all_addresses = IMasterStore(self).find(

=== modified file 'lib/lp/registry/tests/test_team.py'
--- lib/lp/registry/tests/test_team.py	2010-04-19 21:16:12 +0000
+++ lib/lp/registry/tests/test_team.py	2010-08-18 15:54:47 +0000
@@ -6,7 +6,6 @@
 __metaclass__ = type
 
 import transaction
-from unittest import TestLoader
 
 from zope.component import getUtility
 
@@ -15,7 +14,8 @@
 from canonical.launchpad.interfaces.lpstorm import IMasterStore
 from canonical.testing import DatabaseFunctionalLayer
 
-from lp.testing import TestCaseWithFactory
+from lp.registry.interfaces.mailinglist import MailingListStatus
+from lp.testing import login_celebrity, login_person, TestCaseWithFactory
 
 
 class TestTeamContactAddress(TestCaseWithFactory):
@@ -81,6 +81,22 @@
         self.assertEqual(None, self.team.preferredemail)
         self.assertEqual([list_address], self.getAllEmailAddresses())
 
-
-def test_suite():
-    return TestLoader().loadTestsFromName(__name__)
+    def test_setContactAddress_after_purged_mailing_list_and_rename(self):
+        # This is the rare case where a list is purged for a team rename,
+        # then the contact address is set/unset sometime afterwards.
+        # The old mailing list address belongs the the team, but not the list.
+        # 1. Create then purge a mailing list.
+        list_address = self.createMailingListAndGetAddress()
+        mailing_list = self.team.mailing_list
+        mailing_list.deactivate()
+        mailing_list.transitionToStatus(MailingListStatus.INACTIVE)
+        mailing_list.purge()
+        transaction.commit()
+        # 2. Rename the team.
+        login_celebrity('admin')
+        self.team.name = 'beta'
+        login_person(self.team.teamowner)
+        # 3. Set the contact address.
+        self.team.setContactAddress(None)
+        self.assertEqual(None, self.team.preferredemail)
+        self.assertEqual([], self.getAllEmailAddresses())