launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01102
[Merge] lp:~sinzui/launchpad/mailman-xmlrpc-0 into lp:launchpad/devel
Curtis Hovey has proposed merging lp:~sinzui/launchpad/mailman-xmlrpc-0 into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#457606 UnknownSender OOPS in MailingListAPIView.holdMessage()
https://bugs.launchpad.net/bugs/457606
#641711 move MailingListAPIView to lp.registry.xmlrpc
https://bugs.launchpad.net/bugs/641711
This is my branch to fix isRegisteredInLaunchpad to handle personless emails.
lp:~sinzui/launchpad/mailman-xmlrpc-0
Diff size: 234
Launchpad bug:
https://bugs.launchpad.net/bugs/457606
https://bugs.launchpad.net/bugs/641711
Test command: ./bin/test -vv \
-t test_mailinglistapi -t mailinglist-xmlrpc
Pre-implementation: no one
Target release: 10.10
Fix isRegisteredInLaunchpad to handle personless emails
-------------------------------------------------------
Mailman occasionally tries to place a message in the moderation queue
because sender's email address is registered, but Launchpad oopses because
the email address does not have a person. There are personless accounts
from SSO and shipit.
Rules
-----
* Move MailingListAPIView to lp.registry where the interfaces and
tests are located.
* Update isRegisteredInLaunchpad() to exclude email addresses where
the person is None.
QA
--
* Send an email using the address in one of the oopses (both are SSO
users) and verify staging does not oops and that the message does
not appear in the moderation queue.
Lint
----
Linting changed files:
lib/canonical/launchpad/xmlrpc/__init__.py
lib/lp/registry/configure.zcml
lib/lp/registry/tests/test_doc.py
lib/lp/registry/tests/test_mailinglistapi.py
lib/lp/registry/xmlrpc/mailinglist.py
lib/lp/testing/factory.py
Test
----
* lib/lp/registry/tests/test_doc.py
* Updated import and hushed linter.
* lib/lp/registry/tests/test_mailinglistapi.py
* Added tests for isRegisteredInLaunchpad()
* Updated test to use TestCaseWithFactory
Implementation
--------------
* lib/canonical/launchpad/xmlrpc/__init__.py
* Removed glob import
* lib/lp/registry/configure.zcml
* Updated import
* lib/lp/registry/xmlrpc/mailinglist.py
* Added a check for email_address.personID is not None
* Fixed glob imports
* lib/lp/testing/factory.py
* Login as the user during setup in the factory so that the test
does not need to login.
--
__Curtis C. Hovey_________
http://launchpad.net/
--
https://code.launchpad.net/~sinzui/launchpad/mailman-xmlrpc-0/+merge/35906
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/mailman-xmlrpc-0 into lp:launchpad/devel.
=== modified file 'lib/canonical/launchpad/xmlrpc/__init__.py'
--- lib/canonical/launchpad/xmlrpc/__init__.py 2010-08-20 20:31:18 +0000
+++ lib/canonical/launchpad/xmlrpc/__init__.py 2010-09-18 02:11:12 +0000
@@ -7,7 +7,6 @@
from canonical.launchpad.xmlrpc.application import *
from canonical.launchpad.xmlrpc.authserver import *
-from canonical.launchpad.xmlrpc.mailinglist import *
from lp.bugs.xmlrpc.bug import *
from lp.code.xmlrpc.branch import *
from lp.code.xmlrpc.codeimportscheduler import *
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2010-09-13 12:09:30 +0000
+++ lib/lp/registry/configure.zcml 2010-09-18 02:11:12 +0000
@@ -1034,7 +1034,7 @@
<xmlrpc:view
for="lp.registry.interfaces.mailinglist.IMailingListApplication"
interface="lp.registry.interfaces.mailinglist.IMailingListAPIView"
- class="canonical.launchpad.xmlrpc.MailingListAPIView"
+ class="lp.registry.xmlrpc.mailinglist.MailingListAPIView"
permission="zope.Public"/>
<securedutility
class="lp.registry.xmlrpc.softwarecenteragent.SoftwareCenterAgentApplication"
=== modified file 'lib/lp/registry/tests/test_doc.py'
--- lib/lp/registry/tests/test_doc.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/tests/test_doc.py 2010-09-18 02:11:12 +0000
@@ -44,22 +44,28 @@
# architecture. Don't use ServerProxy. We do this because it's easier to
# debug because when things go horribly wrong, you see the errors on
# stdout instead of in an OOPS report living in some log file somewhere.
- from canonical.launchpad.xmlrpc import MailingListAPIView
+ from lp.registry.xmlrpc.mailinglist import MailingListAPIView
+
class ImpedenceMatchingView(MailingListAPIView):
+
@mailinglists_helper.fault_catcher
def getPendingActions(self):
return super(ImpedenceMatchingView, self).getPendingActions()
+
@mailinglists_helper.fault_catcher
def reportStatus(self, statuses):
return super(ImpedenceMatchingView, self).reportStatus(statuses)
+
@mailinglists_helper.fault_catcher
def getMembershipInformation(self, teams):
return super(
ImpedenceMatchingView, self).getMembershipInformation(teams)
+
@mailinglists_helper.fault_catcher
def isLaunchpadMember(self, address):
return super(ImpedenceMatchingView, self).isLaunchpadMember(
address)
+
@mailinglists_helper.fault_catcher
def isTeamPublic(self, team_name):
return super(ImpedenceMatchingView, self).isTeamPublic(team_name)
@@ -189,7 +195,7 @@
def test_suite():
suite = build_test_suite(here, special, layer=DatabaseFunctionalLayer)
- launchpadlib_path = os.path.join(os.path.pardir, 'doc', 'launchpadlib')
+ launchpadlib_path = os.path.join(os.path.pardir, 'doc', 'launchpadlib')
lplib_suite = build_doctest_suite(here, launchpadlib_path,
layer=DatabaseFunctionalLayer)
suite.addTest(lplib_suite)
=== modified file 'lib/lp/registry/tests/test_mailinglistapi.py'
--- lib/lp/registry/tests/test_mailinglistapi.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/tests/test_mailinglistapi.py 2010-09-18 02:11:12 +0000
@@ -3,47 +3,43 @@
"""Unit tests for the private MailingList API."""
+from __future__ import with_statement
+
__metaclass__ = type
__all__ = []
-import unittest
-
import transaction
-from canonical.launchpad.ftests import (
- ANONYMOUS,
- login,
- login_person,
- logout,
- )
-from canonical.launchpad.xmlrpc.mailinglist import (
+from canonical.launchpad.interfaces.emailaddress import EmailAddressStatus
+from canonical.testing import DatabaseFunctionalLayer
+from lp.registry.tests.mailinglists_helper import new_team
+from lp.registry.xmlrpc.mailinglist import (
BYUSER,
ENABLED,
MailingListAPIView,
)
-from canonical.testing import LaunchpadFunctionalLayer
-from lp.registry.tests.mailinglists_helper import new_team
-from lp.testing.factory import LaunchpadObjectFactory
-
-
-class MailingListAPITestCase(unittest.TestCase):
+from lp.testing import (
+ person_logged_in,
+ TestCaseWithFactory,
+ )
+
+
+class MailingListAPITestCase(TestCaseWithFactory):
"""Tests for MailingListAPIView."""
- layer = LaunchpadFunctionalLayer
+ layer = DatabaseFunctionalLayer
def setUp(self):
"""Create a team with a list and subscribe self.member to it."""
- login('foo.bar@xxxxxxxxxxxxx')
+ super(MailingListAPITestCase, self).setUp()
self.team, self.mailing_list = new_team('team-a', with_list=True)
- self.member = LaunchpadObjectFactory().makePersonByName('Bob')
- self.member.join(self.team)
+ self.member = self.factory.makePersonByName('Bob')
+ with person_logged_in(self.member):
+ self.member.join(self.team)
self.mailing_list.subscribe(self.member)
self.api = MailingListAPIView(None, None)
- def tearDown(self):
- logout()
-
def _assertMembership(self, expected):
"""Assert that the named team has exactly the expected membership."""
transaction.commit()
@@ -56,10 +52,9 @@
def test_getMembershipInformation_with_hidden_email(self):
"""Verify that hidden email addresses are still reported correctly."""
- login_person(self.member)
- self.member.hide_email_addresses = True
+ with person_logged_in(self.member):
+ self.member.hide_email_addresses = True
# API runs without a logged in user.
- login(ANONYMOUS)
self._assertMembership([
('archive@xxxxxxxxxxxxxxxx', '', 0, ENABLED),
('bob.person@xxxxxxxxxxx', 'Bob Person', 0, ENABLED),
@@ -67,6 +62,18 @@
('no-priv@xxxxxxxxxxxxx', u'No Privileges Person', 0, BYUSER),
])
-
-def test_suite():
- return unittest.TestLoader().loadTestsFromName(__name__)
+ def test_isRegisteredInLaunchpad_person_with_preferred_email(self):
+ self.factory.makePerson(email='me@xxxxxxxxx')
+ self.assertTrue(self.api.isRegisteredInLaunchpad('me@xxxxxxxxx'))
+
+ def test_isRegisteredInLaunchpad_email_without_preferred_email(self):
+ self.factory.makePerson(
+ email='me@xxxxxxxxx', email_address_status=EmailAddressStatus.NEW)
+ self.assertFalse(self.api.isRegisteredInLaunchpad('me@xxxxxxxxx'))
+
+ def test_isRegisteredInLaunchpad_email_no_email_address(self):
+ self.assertFalse(self.api.isRegisteredInLaunchpad('me@xxxxxxxxx'))
+
+ def test_isRegisteredInLaunchpad_email_without_person(self):
+ self.factory.makeAccount('Me', email='me@xxxxxxxxx')
+ self.assertFalse(self.api.isRegisteredInLaunchpad('me@xxxxxxxxx'))
=== renamed file 'lib/canonical/launchpad/xmlrpc/mailinglist.py' => 'lib/lp/registry/xmlrpc/mailinglist.py'
--- lib/canonical/launchpad/xmlrpc/mailinglist.py 2010-08-25 20:04:40 +0000
+++ lib/lp/registry/xmlrpc/mailinglist.py 2010-09-18 02:11:12 +0000
@@ -17,21 +17,25 @@
from canonical.config import config
from canonical.encoding import escape_nonascii_uniquely
-from canonical.launchpad.interfaces import (
+from canonical.launchpad.interfaces.emailaddress import (
EmailAddressStatus,
IEmailAddressSet,
+ )
+from canonical.launchpad.interfaces.message import IMessageSet
+from canonical.launchpad.webapp import LaunchpadXMLRPCView
+from canonical.launchpad.xmlrpc import faults
+from lp.registry.interfaces.mailinglist import (
IMailingListAPIView,
IMailingListSet,
IMessageApprovalSet,
- IMessageSet,
+ MailingListStatus,
+ PostedMessageStatus,
+ )
+from lp.registry.interfaces.person import (
IPersonSet,
- MailingListStatus,
PersonalStanding,
- PostedMessageStatus,
+ PersonVisibility,
)
-from canonical.launchpad.webapp import LaunchpadXMLRPCView
-from canonical.launchpad.xmlrpc import faults
-from lp.registry.interfaces.person import PersonVisibility
# Not all developers will have built the Mailman instance (via
# 'make mailman_instance'). In that case, this import will fail, but in that
@@ -221,6 +225,7 @@
return False
email_address = getUtility(IEmailAddressSet).getByEmail(address)
return (email_address is not None and
+ email_address.personID is not None and
email_address.status in (EmailAddressStatus.VALIDATED,
EmailAddressStatus.PREFERRED))
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-09-17 03:22:23 +0000
+++ lib/lp/testing/factory.py 2010-09-18 02:11:12 +0000
@@ -632,8 +632,9 @@
if not use_default_autosubscribe_policy:
# Shut off list auto-subscription so that we have direct control
# over subscriptions in the doctests.
- person.mailing_list_auto_subscribe_policy = \
- MailingListAutoSubscribePolicy.NEVER
+ with person_logged_in(person):
+ person.mailing_list_auto_subscribe_policy = (
+ MailingListAutoSubscribePolicy.NEVER)
account = IMasterStore(Account).get(Account, person.accountID)
getUtility(IEmailAddressSet).new(
alternative_address, person, EmailAddressStatus.VALIDATED,
