4 files changed, 55 insertions, 20 deletions

diff --git a/src/mailman/app/membership.py b/src/mailman/app/membership.py
index 83c1b7f8b..fdeb6e9c4 100644
--- a/src/mailman/app/membership.py
+++ b/src/mailman/app/membership.py
@@ -17,7 +17,7 @@
 
 """Application support for membership management."""
 
-from __future__ import absolute_import, unicode_literals
+from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 __all__ = [
@@ -173,8 +173,4 @@ def handle_SubscriptionEvent(event):
     mlist = member.mailing_list
     if not mlist.send_welcome_message:
         return
-    # What language should the welcome message be sent in?
-    language = member.preferred_language
-    if language is None:
-        language = mlist.preferred_language
-    send_welcome_message(mlist, member, language)
+    send_welcome_message(mlist, member, member.preferred_language)
diff --git a/src/mailman/app/tests/test_membership.py b/src/mailman/app/tests/test_membership.py
index b97944351..95e8de1d0 100644
--- a/src/mailman/app/tests/test_membership.py
+++ b/src/mailman/app/tests/test_membership.py
@@ -17,10 +17,13 @@
 
 """Tests of application level membership functions."""
 
-from __future__ import absolute_import, unicode_literals
+from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 __all__ = [
+    'TestAddMember',
+    'TestAddMemberPassword',
+    'TestDeleteMember',
     ]
 
 
@@ -29,17 +32,18 @@ import unittest
 from zope.component import getUtility
 
 from mailman.app.lifecycle import create_list
-from mailman.app.membership import add_member
+from mailman.app.membership import add_member, delete_member
 from mailman.core.constants import system_preferences
 from mailman.interfaces.bans import IBanManager
 from mailman.interfaces.member import (
-    AlreadySubscribedError, DeliveryMode, MemberRole, MembershipIsBannedError)
+    AlreadySubscribedError, DeliveryMode, MemberRole, MembershipIsBannedError,
+    NotAMemberError)
 from mailman.interfaces.usermanager import IUserManager
 from mailman.testing.layers import ConfigLayer
 
 
 
-class AddMemberTest(unittest.TestCase):
+class TestAddMember(unittest.TestCase):
     layer = ConfigLayer
 
     def setUp(self):
@@ -70,10 +74,13 @@ class AddMemberTest(unittest.TestCase):
         # Test that members who are banned by specific address cannot
         # subscribe to the mailing list.
         IBanManager(self._mlist).ban('anne@example.com')
-        self.assertRaises(
-            MembershipIsBannedError,
-            add_member, self._mlist, 'anne@example.com', 'Anne Person',
-            '123', DeliveryMode.regular, system_preferences.preferred_language)
+        with self.assertRaises(MembershipIsBannedError) as cm:
+            add_member(self._mlist, 'anne@example.com', 'Anne Person',
+                       '123', DeliveryMode.regular,
+                       system_preferences.preferred_language)
+        self.assertEqual(
+            str(cm.exception),
+            'anne@example.com is not allowed to subscribe to test@example.com')
 
     def test_add_member_globally_banned(self):
         # Test that members who are banned by specific address cannot
@@ -165,7 +172,7 @@ class AddMemberTest(unittest.TestCase):
 
 
 
-class AddMemberPasswordTest(unittest.TestCase):
+class TestAddMemberPassword(unittest.TestCase):
     layer = ConfigLayer
 
     def setUp(self):
@@ -177,3 +184,19 @@ class AddMemberPasswordTest(unittest.TestCase):
                             'Anne Person', 'abc', DeliveryMode.regular,
                             system_preferences.preferred_language)
         self.assertEqual(member.user.password, '{plaintext}abc')
+
+
+
+class TestDeleteMember(unittest.TestCase):
+    layer = ConfigLayer
+
+    def setUp(self):
+        self._mlist = create_list('test@example.com')
+
+    def test_delete_member_not_a_member(self):
+        # Try to delete an address which is not a member of the mailing list.
+        with self.assertRaises(NotAMemberError) as cm:
+            delete_member(self._mlist, 'noperson@example.com')
+        self.assertEqual(
+            str(cm.exception),
+            'noperson@example.com is not a member of test@example.com')
diff --git a/src/mailman/model/member.py b/src/mailman/model/member.py
index 84f99d5ac..438796811 100644
--- a/src/mailman/model/member.py
+++ b/src/mailman/model/member.py
@@ -136,7 +136,7 @@ class Member(Model):
                 if self._address is None
                 else getUtility(IUserManager).get_user(self._address.email))
 
-    def _lookup(self, preference):
+    def _lookup(self, preference, default=None):
         pref = getattr(self.preferences, preference)
         if pref is not None:
             return pref
@@ -147,7 +147,9 @@ class Member(Model):
             pref = getattr(self.address.user.preferences, preference)
             if pref is not None:
                 return pref
-        return getattr(system_preferences, preference)
+        if default is None:
+            return getattr(system_preferences, preference)
+        return default
 
     @property
     def acknowledge_posts(self):
@@ -157,7 +159,13 @@ class Member(Model):
     @property
     def preferred_language(self):
         """See `IMember`."""
-        return self._lookup('preferred_language')
+        missing = object()
+        language = self._lookup('preferred_language', missing)
+        if language is missing:
+            language = ((self.mailing_list and
+                         self.mailing_list.preferred_language) or
+                        system_preferences.preferred_language)
+        return language
 
     @property
     def receive_list_copy(self):
diff --git a/src/mailman/model/tests/test_member.py b/src/mailman/model/tests/test_member.py
index 7ac015cd4..5bd3d1594 100644
--- a/src/mailman/model/tests/test_member.py
+++ b/src/mailman/model/tests/test_member.py
@@ -17,19 +17,21 @@
 
 """Test members."""
 
-from __future__ import absolute_import, unicode_literals
+from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 __all__ = [
+    'TestMember',
     ]
 
 
 import unittest
 
 from mailman.app.lifecycle import create_list
-from mailman.interfaces.member import MembershipError
+from mailman.interfaces.member import MemberRole, MembershipError
 from mailman.interfaces.user import UnverifiedAddressError
 from mailman.interfaces.usermanager import IUserManager
+from mailman.model.member import Member
 from mailman.testing.layers import ConfigLayer
 from mailman.utilities.datetime import now
 
@@ -94,3 +96,9 @@ class TestMember(unittest.TestCase):
         # The new address is not verified.
         self.assertRaises(MembershipError,
                           setattr, member, 'address', bart_address)
+
+    def test_member_ctor_value_error(self):
+        # ValueError when passing in anything but a user or address.
+        self.assertRaises(ValueError, Member, MemberRole.member,
+                          self._mlist.list_id,
+                          'aperson@example.com')