launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/reject-mail-from-teams into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #770329 in Launchpad itself: "Mailing lists must reject emails that claim to be from teams."
  https://bugs.launchpad.net/launchpad/+bug/770329

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/reject-mail-from-teams/+merge/61341

== Implementation ==

Updated the isRegisteredInLaunchpad() method to check that the person associated with an email address is not a team. 

The solution involves an object navigation from EmailAddress->Person which means an extra SQL lookup. I considered fixing this but the solution would require a chunk of new code since the interfaces currently in use - IEmailAddressSet, IPersonSet - have getByEmail() methods which return single objects (EmailAddress or Person). I think it's ok as is since unlike the case with page impressions, we are not getting 10000000's of incoming emails per day and the extra lookup is at the end of an XMLRPC request anyway.

== Tests ==

Added new test test_isRegisteredInLaunchpad_team() to test_mailinglistapi

== Lint ==

Linting changed files:
  lib/lp/registry/tests/test_mailinglistapi.py
  lib/lp/registry/xmlrpc/mailinglist.py

./lib/lp/registry/xmlrpc/mailinglist.py
      47: redefinition of unused 'ENABLED' from line 45
      48: redefinition of unused 'BYUSER' from line 45

-- 
https://code.launchpad.net/~wallyworld/launchpad/reject-mail-from-teams/+merge/61341
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/reject-mail-from-teams into lp:launchpad.

=== modified file 'lib/lp/registry/tests/test_mailinglistapi.py'
--- lib/lp/registry/tests/test_mailinglistapi.py	2010-12-14 22:57:54 +0000
+++ lib/lp/registry/tests/test_mailinglistapi.py	2011-05-18 01:53:25 +0000
@@ -83,3 +83,7 @@
         email = config.mailman.archive_address
         self.factory.makePerson(email=email)
         self.assertFalse(self.api.isRegisteredInLaunchpad(email))
+
+    def test_isRegisteredInLaunchpad_team(self):
+        self.factory.makeTeam(email='me@xxxxxxxxx')
+        self.assertFalse(self.api.isRegisteredInLaunchpad('me@xxxxxxxxx'))

=== modified file 'lib/lp/registry/xmlrpc/mailinglist.py'
--- lib/lp/registry/xmlrpc/mailinglist.py	2011-05-12 21:33:10 +0000
+++ lib/lp/registry/xmlrpc/mailinglist.py	2011-05-18 01:53:25 +0000
@@ -226,13 +226,14 @@
         email_address = getUtility(IEmailAddressSet).getByEmail(address)
         return (email_address is not None and
                 email_address.personID is not None and
+                not email_address.person.is_team and
                 email_address.status in (EmailAddressStatus.VALIDATED,
                                          EmailAddressStatus.PREFERRED))
 
     def inGoodStanding(self, address):
         """See `IMailingListAPIView`."""
         person = getUtility(IPersonSet).getByEmail(address)
-        if person is None or person.isTeam():
+        if person is None or person.is_team:
             return False
         return person.personal_standing in (PersonalStanding.GOOD,
                                             PersonalStanding.EXCELLENT)