4 files changed, 51 insertions, 53 deletions

diff --git a/src/mailman/app/notifications.py b/src/mailman/app/notifications.py
index b20cc6432..e28714eb5 100644
--- a/src/mailman/app/notifications.py
+++ b/src/mailman/app/notifications.py
@@ -76,14 +76,19 @@ def send_welcome_message(mlist, member, language, text=''):
     """
     welcome_message = _get_message(mlist.welcome_message_uri, mlist, language)
     options_url = member.options_url
-    # Get the text from the template.
+    # Try to find a non-empty display name.  We first look at the directly
+    # subscribed record, which will either be the address or the user.  That's
+    # handled automatically by going through member.subscriber.  If that
+    # doesn't give us something useful, try whatever user is linked to the
+    # subscriber.
     if member.subscriber.display_name:
         display_name = member.subscriber.display_name
-    elif member.user:
-        display_name = ('' if member.user.display_name is None
-            else member.user.display_name)
+    # If an unlinked address is subscribed tehre will be no .user.
+    elif member.user is not None and member.user.display_name:
+        display_name = member.user.display_name
     else:
         display_name = ''
+    # Get the text from the template.
     text = expand(welcome_message, dict(
         fqdn_listname=mlist.fqdn_listname,
         list_name=mlist.display_name,
diff --git a/src/mailman/app/tests/test_notifications.py b/src/mailman/app/tests/test_notifications.py
index 75d87255e..248ead6cc 100644
--- a/src/mailman/app/tests/test_notifications.py
+++ b/src/mailman/app/tests/test_notifications.py
@@ -32,7 +32,8 @@ from mailman.config import config
 from mailman.interfaces.languages import ILanguageManager
 from mailman.interfaces.member import MemberRole
 from mailman.interfaces.usermanager import IUserManager
-from mailman.testing.helpers import get_queue_messages, subscribe, set_preferred
+from mailman.testing.helpers import (
+    get_queue_messages, set_preferred, subscribe)
 from mailman.testing.layers import ConfigLayer
 from mailman.utilities.datetime import now
 from zope.component import getUtility
@@ -145,86 +146,71 @@ Welcome to the Test List mailing list.
         self.assertEqual(len(messages), 0)
 
     def test_member_susbcribed_address_has_display_name(self):
-        address = getUtility(IUserManager).create_address('anne@example.com',
-            'Anne Person')
+        address = getUtility(IUserManager).create_address(
+            'anne@example.com', 'Anne Person')
         address.verified_on = now()
         self._mlist.subscribe(address)
-        messages = get_queue_messages('virgin')
+        messages = get_queue_messages('virgin', expected_count=1)
         message = messages[0].msg
-        self.assertEqual(message['to'],
-            'Anne Person <anne@example.com>')
+        self.assertEqual(message['to'], 'Anne Person <anne@example.com>')
 
     def test_member_subscribed_address_has_no_display_name(self):
-        address = getUtility(IUserManager).create_address('anne@example.com',
-            '')
+        address = getUtility(IUserManager).create_address('anne@example.com')
         address.verified_on = now()
         self._mlist.subscribe(address)
-        messages = get_queue_messages('virgin')
+        messages = get_queue_messages('virgin', expected_count=1)
         message = messages[0].msg
-        self.assertEqual(message['to'],
-            'anne@example.com')
+        self.assertEqual(message['to'], 'anne@example.com')
 
     def test_member_is_user_and_has_display_name(self):
-        user = getUtility(IUserManager).create_user('anne@example.com',
-            'Anne Person')
+        user = getUtility(IUserManager).create_user(
+            'anne@example.com', 'Anne Person')
         set_preferred(user)
         self._mlist.subscribe(user)
-        messages = get_queue_messages('virgin')
+        messages = get_queue_messages('virgin', expected_count=1)
         message = messages[0].msg
-        self.assertEqual(message['to'],
-            'Anne Person <anne@example.com>')
+        self.assertEqual(message['to'], 'Anne Person <anne@example.com>')
 
     def test_member_is_user_and_has_no_display_name(self):
-        user = getUtility(IUserManager).create_user('anne@example.com',
-            '')
+        user = getUtility(IUserManager).create_user('anne@example.com')
         set_preferred(user)
         self._mlist.subscribe(user)
-        messages = get_queue_messages('virgin')
+        messages = get_queue_messages('virgin', expected_count=1)
         message = messages[0].msg
-        self.assertEqual(message['to'],
-            'anne@example.com')
+        self.assertEqual(message['to'], 'anne@example.com')
 
     def test_member_has_linked_user_display_name(self):
-        user = getUtility(IUserManager).create_user('anne@example.com',
-            'Anne Person')
+        user = getUtility(IUserManager).create_user(
+            'anne@example.com', 'Anne Person')
         set_preferred(user)
-        self._mlist.subscribe(user)
-        address = getUtility(IUserManager).create_address('anne2@example.com',
-            '')
+        address = getUtility(IUserManager).create_address('anne2@example.com')
         address.verified_on = now()
         user.link(address)
         self._mlist.subscribe(address)
-        messages = get_queue_messages('virgin')
-        message = messages[1].msg
-        self.assertEqual(message['to'],
-            'Anne Person <anne2@example.com>')
+        messages = get_queue_messages('virgin', expected_count=1)
+        message = messages[0].msg
+        self.assertEqual(message['to'], 'Anne Person <anne2@example.com>')
 
     def test_member_has_no_linked_display_name(self):
-        user = getUtility(IUserManager).create_user('anne@example.com',
-            '')
+        user = getUtility(IUserManager).create_user('anne@example.com')
         set_preferred(user)
-        self._mlist.subscribe(user)
-        address = getUtility(IUserManager).create_address('anne2@example.com',
-            '')
+        address = getUtility(IUserManager).create_address('anne2@example.com')
         address.verified_on = now()
         user.link(address)
         self._mlist.subscribe(address)
-        messages = get_queue_messages('virgin')
-        message = messages[1].msg
-        self.assertEqual(message['to'],
-            'anne2@example.com')
+        messages = get_queue_messages('virgin', expected_count=1)
+        message = messages[0].msg
+        self.assertEqual(message['to'], 'anne2@example.com')
 
     def test_member_has_address_and_user_display_name(self):
-        user = getUtility(IUserManager).create_user('anne@example.com',
-            'Anne Person')
+        user = getUtility(IUserManager).create_user(
+            'anne@example.com', 'Anne Person')
         set_preferred(user)
-        self._mlist.subscribe(user)
-        address = getUtility(IUserManager).create_address('anne2@example.com',
-            'Anne Person 2')
+        address = getUtility(IUserManager).create_address(
+            'anne2@example.com', 'Anne X Person')
         address.verified_on = now()
         user.link(address)
         self._mlist.subscribe(address)
-        messages = get_queue_messages('virgin')
-        message = messages[1].msg
-        self.assertEqual(message['to'],
-            'Anne Person 2 <anne2@example.com>')
+        messages = get_queue_messages('virgin', expected_count=1)
+        message = messages[0].msg
+        self.assertEqual(message['to'], 'Anne X Person <anne2@example.com>')
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index a8a7ca89f..95e01019e 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -73,6 +73,8 @@ Bugs
    Bompard.  (Closes #203)
  * Don't let crashes in IArchiver plugins break handlers or runners.
    (Closes #208)
+ * Fix "None" as display name in welcome message.  Given by Aditya Divekar.
+   (Closes #194)
 
 Configuration
 -------------
diff --git a/src/mailman/testing/helpers.py b/src/mailman/testing/helpers.py
index 631424661..e0d46cd20 100644
--- a/src/mailman/testing/helpers.py
+++ b/src/mailman/testing/helpers.py
@@ -122,12 +122,14 @@ class _Bag:
             setattr(self, key, value)
 
 
-def get_queue_messages(queue_name, sort_on=None):
+def get_queue_messages(queue_name, sort_on=None, expected_count=None):
     """Return and clear all the messages in the given queue.
 
     :param queue_name: A string naming a queue.
     :param sort_on: The message header to sort on.  If None (the default),
         no sorting is performed.
+    :param expected_count: If given and there aren't exactly this number of
+        messages in the queue, raise an AssertionError.
     :return: A list of 2-tuples where each item contains the message and
         message metadata.
     """
@@ -137,6 +139,9 @@ def get_queue_messages(queue_name, sort_on=None):
         msg, msgdata = queue.dequeue(filebase)
         messages.append(_Bag(msg=msg, msgdata=msgdata))
         queue.finish(filebase)
+    if expected_count is not None:
+        assert len(messages) == expected_count, 'Wanted {}, got {}'.format(
+            expected_count, len(messages))
     if sort_on is not None:
         messages.sort(key=lambda item: str(item.msg[sort_on]))
     return messages