5 files changed, 67 insertions, 15 deletions

diff --git a/src/mailman/app/membership.py b/src/mailman/app/membership.py
index 85a6cba08..046b33ea0 100644
--- a/src/mailman/app/membership.py
+++ b/src/mailman/app/membership.py
@@ -26,13 +26,16 @@ __all__ = [
 
 from email.utils import formataddr
 from mailman.app.notifications import (
-    send_goodbye_message, send_welcome_message)
+    send_admin_subscription_notice, send_goodbye_message,
+    send_welcome_message)
 from mailman.core.i18n import _
 from mailman.email.message import OwnerNotification
+from mailman.interfaces.address import IAddress
 from mailman.interfaces.bans import IBanManager
 from mailman.interfaces.member import (
     AlreadySubscribedError, MemberRole, MembershipIsBannedError,
     NotAMemberError, SubscriptionEvent)
+from mailman.interfaces.user import IUser
 from mailman.interfaces.usermanager import IUserManager
 from mailman.utilities.i18n import make
 from zope.component import getUtility
@@ -143,13 +146,23 @@ def delete_member(mlist, email, admin_notif=None, userack=None):
 def handle_SubscriptionEvent(event):
     if not isinstance(event, SubscriptionEvent):
         return
-    # Only send a notification message if the mailing list is configured to do
-    # so, and the member being added is a list member (as opposed to a
-    # moderator, non-member, or owner).
     member = event.member
+    # Only send notifications if a member (as opposed to a moderator,
+    # non-member, or owner) is being subscribed.
     if member.role is not MemberRole.member:
         return
     mlist = member.mailing_list
-    if not mlist.send_welcome_message:
-        return
-    send_welcome_message(mlist, member, member.preferred_language)
+    # Maybe send the list administrators a notification.
+    if mlist.admin_notify_mchanges:
+        subscriber = member.subscriber
+        if IAddress.providedBy(subscriber):
+            address = subscriber.email
+            display_name = subscriber.display_name
+        else:
+            assert IUser.providedBy(subscriber)
+            address = subscriber.preferred_address.email
+            display_name = subscriber.display_name
+        send_admin_subscription_notice(mlist, address, display_name)
+    # Maybe send a welcome message to the new member.
+    if mlist.send_welcome_message:
+        send_welcome_message(mlist, member, member.preferred_language)
diff --git a/src/mailman/app/moderator.py b/src/mailman/app/moderator.py
index eb848ea08..a5c0d50dd 100644
--- a/src/mailman/app/moderator.py
+++ b/src/mailman/app/moderator.py
@@ -34,7 +34,6 @@ import logging
 
 from email.utils import formataddr, formatdate, getaddresses, make_msgid
 from mailman.app.membership import add_member, delete_member
-from mailman.app.notifications import send_admin_subscription_notice
 from mailman.config import config
 from mailman.core.i18n import _
 from mailman.email.message import UserNotification
@@ -257,10 +256,6 @@ def handle_subscription(mlist, id, action, comment=None):
             # The address got subscribed in some other way after the original
             # request was made and accepted.
             pass
-        else:
-            if mlist.admin_notify_mchanges:
-                send_admin_subscription_notice(
-                    mlist, email, display_name, language)
         slog.info('%s: new %s, %s %s', mlist.fqdn_listname,
                   delivery_mode, formataddr((display_name, email)),
                   'via admin approval')
diff --git a/src/mailman/app/notifications.py b/src/mailman/app/notifications.py
index fc8a0549e..dde39ab21 100644
--- a/src/mailman/app/notifications.py
+++ b/src/mailman/app/notifications.py
@@ -123,7 +123,7 @@ def send_goodbye_message(mlist, address, language):
 
 
 
-def send_admin_subscription_notice(mlist, address, display_name, language):
+def send_admin_subscription_notice(mlist, address, display_name):
     """Send the list administrators a subscription notice.
 
     :param mlist: The mailing list.
@@ -132,8 +132,6 @@ def send_admin_subscription_notice(mlist, address, display_name, language):
     :type address: string
     :param display_name: The name of the subscriber.
     :type display_name: string
-    :param language: The language of the address's display name.
-    :type language: string
     """
     with _.using(mlist.preferred_language.code):
         subject = _('$mlist.display_name subscription notification')
diff --git a/src/mailman/app/tests/test_registrar.py b/src/mailman/app/tests/test_registrar.py
index e76009454..9e1000049 100644
--- a/src/mailman/app/tests/test_registrar.py
+++ b/src/mailman/app/tests/test_registrar.py
@@ -26,10 +26,12 @@ import unittest
 
 from mailman.app.lifecycle import create_list
 from mailman.interfaces.mailinglist import SubscriptionPolicy
+from mailman.interfaces.member import MemberRole
 from mailman.interfaces.pending import IPendings
 from mailman.interfaces.registrar import IRegistrar
 from mailman.interfaces.subscriptions import TokenOwner
 from mailman.interfaces.usermanager import IUserManager
+from mailman.testing.helpers import get_queue_messages
 from mailman.testing.layers import ConfigLayer
 from mailman.utilities.datetime import now
 from zope.component import getUtility
@@ -234,3 +236,41 @@ class TestRegistrar(unittest.TestCase):
         self._registrar.discard(token)
         # Trying to confirm the token now results in an exception.
         self.assertRaises(LookupError, self._registrar.confirm, token)
+
+    def test_admin_notify_mchanges(self):
+        # When a user gets subscribed via the subscription policy workflow,
+        # the list administrators get an email notification.
+        self._mlist.subscription_policy = SubscriptionPolicy.open
+        self._mlist.admin_notify_mchanges = True
+        self._mlist.send_welcome_message = False
+        token, token_owner, member = self._registrar.register(
+            self._anne, pre_verified=True)
+        # Anne is now a member.
+        self.assertEqual(member.address.email, 'anne@example.com')
+        # And there's a notification email waiting for Bart.
+        items = get_queue_messages('virgin')
+        self.assertEqual(len(items), 1)
+        message = items[0].msg
+        self.assertEqual(message['To'], 'ant-owner@example.com')
+        self.assertEqual(message['Subject'], 'Ant subscription notification')
+        self.assertEqual(message.get_payload(), """\
+anne@example.com has been successfully subscribed to Ant.""")
+
+    def test_no_admin_notify_mchanges(self):
+        # Even when a user gets subscribed via the subscription policy
+        # workflow, the list administrators won't get an email notification if
+        # they don't want one.
+        self._mlist.subscription_policy = SubscriptionPolicy.open
+        self._mlist.admin_notify_mchanges = False
+        self._mlist.send_welcome_message = False
+        # Bart is an administrator of the mailing list.
+        bart = getUtility(IUserManager).create_address(
+            'bart@example.com', 'Bart Person')
+        self._mlist.subscribe(bart, MemberRole.owner)
+        token, token_owner, member = self._registrar.register(
+            self._anne, pre_verified=True)
+        # Anne is now a member.
+        self.assertEqual(member.address.email, 'anne@example.com')
+        # There's no notification email waiting for Bart.
+        items = get_queue_messages('virgin')
+        self.assertEqual(len(items), 0)
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index 770a099d9..8ea80633d 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -12,6 +12,12 @@ Here is a history of user visible changes to Mailman.
 =============================
 (2015-XX-XX)
 
+Bugs
+----
+ * When the mailing list's `admin_notify_mchanges` is True, the list owners
+   now get the subscription notification.  (Closes: #1)
+
+
 3.0.0 -- "Show Don't Tell"
 ==========================
 (2015-04-28)