8 files changed, 180 insertions, 73 deletions

diff --git a/src/mailman/app/registrar.py b/src/mailman/app/registrar.py
index a7c9dbb1b..a22e5df8c 100644
--- a/src/mailman/app/registrar.py
+++ b/src/mailman/app/registrar.py
@@ -25,6 +25,8 @@ __all__ = [
     ]
 
 
+import logging
+
 from pkg_resources import resource_string
 from zope.component import getUtility
 from zope.interface import implements
@@ -33,13 +35,16 @@ from mailman.core.i18n import _
 from mailman.email.message import UserNotification
 from mailman.interfaces.address import IEmailValidator
 from mailman.interfaces.listmanager import IListManager
-from mailman.interfaces.member import MemberRole
+from mailman.interfaces.member import DeliveryMode, MemberRole
 from mailman.interfaces.pending import IPendable, IPendings
 from mailman.interfaces.registrar import IRegistrar
 from mailman.interfaces.usermanager import IUserManager
 from mailman.utilities.datetime import now
 
 
+log = logging.getLogger('mailman.error')
+
+
 
 class PendableRegistration(dict):
     implements(IPendable)
@@ -52,8 +57,10 @@ class Registrar:
 
     implements(IRegistrar)
 
-    def register(self, mlist, email, real_name=None):
+    def register(self, mlist, email, real_name=None, delivery_mode=None):
         """See `IUserRegistrar`."""
+        if delivery_mode is None:
+            delivery_mode = DeliveryMode.regular
         # First, do validation on the email address.  If the address is
         # invalid, it will raise an exception, otherwise it just returns.
         getUtility(IEmailValidator).validate(email)
@@ -61,7 +68,8 @@ class Registrar:
         pendable = PendableRegistration(
             type=PendableRegistration.PEND_KEY,
             email=email,
-            real_name=real_name)
+            real_name=real_name,
+            delivery_mode=delivery_mode.name)
         pendable['list_name'] = mlist.fqdn_listname
         token = getUtility(IPendings).add(pendable)
         # There are three ways for a user to confirm their subscription.  They
@@ -92,6 +100,13 @@ class Registrar:
         email = pendable.get('email', missing)
         real_name = pendable.get('real_name', missing)
         list_name = pendable.get('list_name', missing)
+        pended_delivery_mode = pendable.get('delivery_mode', 'regular')
+        try:
+            delivery_mode = DeliveryMode[pended_delivery_mode]
+        except ValueError:
+            log.error('Invalid pended delivery_mode for {0}: {1}',
+                      email, pended_delivery_mode)
+            delivery_mode = DeliveryMode.regular
         if pendable.get('type') != PendableRegistration.PEND_KEY:
             # It seems like it would be very difficult to accurately guess
             # tokens, or brute force an attack on the SHA1 hash, so we'll just
@@ -137,7 +152,8 @@ class Registrar:
         if list_name is not None:
             mlist = getUtility(IListManager).get(list_name)
             if mlist:
-                mlist.subscribe(address, MemberRole.member)
+                member = mlist.subscribe(address, MemberRole.member)
+                member.preferences.delivery_mode = delivery_mode
         return True
 
     def discard(self, token):
diff --git a/src/mailman/commands/docs/membership.rst b/src/mailman/commands/docs/membership.rst
index d05f12eee..d3643c2b1 100644
--- a/src/mailman/commands/docs/membership.rst
+++ b/src/mailman/commands/docs/membership.rst
@@ -29,7 +29,7 @@ The mail command ``join`` subscribes an email address to the mailing list.
         join address=myotheraddress@example.com
     <BLANKLINE>
     >>> print join.argument_description
-    [digest=<yes|no>] [address=<address>]
+    [digest=<no|mime|plain>]
 
 
 No address to join
diff --git a/src/mailman/commands/eml_membership.py b/src/mailman/commands/eml_membership.py
index 88f7d5722..b4f24b514 100644
--- a/src/mailman/commands/eml_membership.py
+++ b/src/mailman/commands/eml_membership.py
@@ -17,7 +17,7 @@
 
 """The email commands 'join' and 'subscribe'."""
 
-from __future__ import absolute_import, unicode_literals
+from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 __all__ = [
@@ -47,7 +47,8 @@ class Join:
     implements(IEmailCommand)
 
     name = 'join'
-    argument_description = '[digest=<yes|no>] [address=<address>]'
+    # XXX 2012-02-29 BAW: DeliveryMode.summary is not yet supported.
+    argument_description = '[digest=<no|mime|plain>]'
     description = _("""\
 Join this mailing list.  You will be asked to confirm your subscription
 request and you may be issued a provisional password.
@@ -63,16 +64,16 @@ example:
     def process(self, mlist, msg, msgdata, arguments, results):
         """See `IEmailCommand`."""
         # Parse the arguments.
-        address, delivery_mode = self._parse_arguments(arguments)
-        # address could be None or the empty string.
-        if not address:
-            real_name, address = parseaddr(msg['from'])
+        delivery_mode = self._parse_arguments(arguments, results)
+        if delivery_mode is ContinueProcessing.no:
+            return ContinueProcessing.no
+        real_name, address = parseaddr(msg['from'])
         # Address could be None or the empty string.
         if not address:
             address = msg.sender
         if not address:
-            print >> results, _(
-                '$self.name: No valid address found to subscribe')
+            print(_('$self.name: No valid address found to subscribe'),
+                  file=results)
             return ContinueProcessing.no
         # Have we already seen one join request from this user during the
         # processing of this email?
@@ -82,60 +83,44 @@ example:
             return ContinueProcessing.yes
         joins.add(address)
         results.joins = joins
-        members = getUtility(ISubscriptionService).find_members(address)
         person = formataddr((real_name, address))
-        if len(members) > 0 and members[0].role is MemberRole.member:
-            assert(members[0].address.email == address)
-            print >> results, _('$person is already a member')
+        # Is this person already a member of the list?  Search for all
+        # matching memberships.
+        members = getUtility(ISubscriptionService).find_members(
+            address, mlist.fqdn_listname, MemberRole.member)
+        if len(members) > 0:
+            print(_('$person is already a member'), file=results)
         else:
-            getUtility(IRegistrar).register(mlist, address, real_name)
-            print >> results, _('Confirmation email sent to $person')
+            getUtility(IRegistrar).register(mlist, address, 
+                                            real_name, delivery_mode)
+            print(_('Confirmation email sent to $person'), file=results)
         return ContinueProcessing.yes
 
-    def _parse_arguments(self, arguments):
+    def _parse_arguments(self, arguments, results):
         """Parse command arguments.
 
         :param arguments: The sequences of arguments as given to the
             `process()` method.
-        :return: address, delivery_mode
+        :param results: The results object.
+        :return: The delivery mode, None, or ContinueProcessing.no on error.
         """
-        address = None
-        delivery_mode = None
+        mode = DeliveryMode.regular
         for argument in arguments:
             parts = argument.split('=', 1)
-            if parts[0].lower() == 'digest':
-                if digest is not None:
-                    print >> results, self.name, \
-                          _('duplicate argument: $argument')
-                    return ContinueProcessing.no
-                if len(parts) == 0:
-                    # We treat just plain 'digest' as 'digest=yes'.  We don't
-                    # yet support the other types of digest delivery.
-                    delivery_mode = DeliveryMode.mime_digests
-                else:
-                    if parts[1].lower() == 'yes':
-                        delivery_mode = DeliveryMode.mime_digests
-                    elif parts[1].lower() == 'no':
-                        delivery_mode = DeliveryMode.regular
-                    else:
-                        print >> results, self.name, \
-                              _('bad argument: $argument')
-                        return ContinueProcessing.no
-            elif parts[0].lower() == 'address':
-                if address is not None:
-                    print >> results, self.name, \
-                          _('duplicate argument $argument')
-                    return ContinueProcessing.no
-                if len(parts) == 0:
-                    print >> results, self.name, \
-                          _('missing argument value: $argument')
-                    return ContinueProcessing.no
-                if len(parts) > 1:
-                    print >> results, self.name, \
-                          _('too many argument values: $argument')
-                    return ContinueProcessing.no
-                address = parts[1]
-        return address, delivery_mode
+            if len(parts) != 2 or parts[0] != 'digest':
+                print(self.name, _('bad argument: $argument'),
+                      file=results)
+                return ContinueProcessing.no
+            mode = {
+                'no': DeliveryMode.regular,
+                'plain': DeliveryMode.plaintext_digests,
+                'mime': DeliveryMode.mime_digests,
+                }.get(parts[1])
+            if mode is None:
+                print(self.name, _('bad argument: $argument'),
+                      file=results)
+                return ContinueProcessing.no
+        return mode
 
 
 
@@ -160,19 +145,21 @@ class Leave:
         """See `IEmailCommand`."""
         email = msg.sender
         if not email:
-            print >> results, _(
-                '$self.name: No valid email address found to unsubscribe')
+            print(_('$self.name: No valid email address found to unsubscribe'),
+                  file=results)
             return ContinueProcessing.no
         user_manager = getUtility(IUserManager)
         user = user_manager.get_user(email)
         if user is None:
-            print >> results, _('No registered user for email address: $email')
+            print(_('No registered user for email address: $email'),
+                  file=results)
             return ContinueProcessing.no
         # The address that the -leave command was sent from, must be verified.
         # Otherwise you could link a bogus address to anyone's account, and
         # then send a leave command from that address.
         if user_manager.get_address(email).verified_on is None:
-            print >> results, _('Invalid or unverified email address: $email')
+            print(_('Invalid or unverified email address: $email'),
+                  file=results)
             return ContinueProcessing.no
         for user_address in user.addresses:
             # Only recognize verified addresses.
@@ -183,12 +170,13 @@ class Leave:
                 break
         else:
             # None of the user's addresses are subscribed to this mailing list.
-            print >> results, _(
-                '$self.name: $email is not a member of $mlist.fqdn_listname')
+            print(_(
+                '$self.name: $email is not a member of $mlist.fqdn_listname'),
+                file=results)
             return ContinueProcessing.no
         member.unsubscribe()
         person = formataddr((user.real_name, email))
-        print >> results, _('$person left $mlist.fqdn_listname')
+        print(_('$person left $mlist.fqdn_listname'), file=results)
         return ContinueProcessing.yes
 
 
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index 1370c2d9b..8955e4798 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -50,6 +50,9 @@ Commands
  * `bin/mailman shell` is an alias for `withlist`.
  * The `confirm` email command now properly handles `Re:`-like prefixes, even
    if they contain non-ASCII characters.  (LP: #685261)
+ * The `join` email command no longer accepts an `address=` argument.  Its
+   `digest=` argument now accepts the following values: `no` (for regular
+   delivery), `mime`, or `plain`
 
 Bug fixes
 ---------
diff --git a/src/mailman/interfaces/registrar.py b/src/mailman/interfaces/registrar.py
index 846ad2014..0f4fe2edf 100644
--- a/src/mailman/interfaces/registrar.py
+++ b/src/mailman/interfaces/registrar.py
@@ -42,7 +42,7 @@ class IRegistrar(Interface):
     syntax checking, or confirmation, while this interface does.
     """
 
-    def register(mlist, email, real_name=None):
+    def register(mlist, email, real_name=None, delivery_mode=None):
         """Register the email address, requesting verification.
 
         No `IAddress` or `IUser` is created during this step, but after
@@ -60,6 +60,9 @@ class IRegistrar(Interface):
         :type email: str
         :param real_name: The optional real name of the user.
         :type real_name: str
+        :param delivery_mode: The optional delivery mode for this
+            registration.  If not given, regular delivery is used.
+        :type delivery_mode: `DeliveryMode`
         :return: The confirmation token string.
         :rtype: str
         :raises InvalidEmailAddressError: if the address is not allowed.
diff --git a/src/mailman/model/docs/registration.rst b/src/mailman/model/docs/registration.rst
index 9605fcbea..e9228d359 100644
--- a/src/mailman/model/docs/registration.rst
+++ b/src/mailman/model/docs/registration.rst
@@ -100,10 +100,11 @@ But this address is waiting for confirmation.
     >>> pendingdb = getUtility(IPendings)
 
     >>> dump_msgdata(pendingdb.confirm(token, expunge=False))
-    email    : aperson@example.com
-    list_name: alpha@example.com
-    real_name: Anne Person
-    type     : registration
+    delivery_mode: regular
+    email        : aperson@example.com
+    list_name    : alpha@example.com
+    real_name    : Anne Person
+    type         : registration
 
 
 Verification by email
diff --git a/src/mailman/runners/tests/test_join.py b/src/mailman/runners/tests/test_join.py
index 369880a32..8cbd8659f 100644
--- a/src/mailman/runners/tests/test_join.py
+++ b/src/mailman/runners/tests/test_join.py
@@ -31,11 +31,15 @@ from zope.component import getUtility
 
 from mailman.app.lifecycle import create_list
 from mailman.config import config
+from mailman.interfaces.member import DeliveryMode
+from mailman.interfaces.registrar import IRegistrar
+from mailman.interfaces.subscriptions import ISubscriptionService
 from mailman.interfaces.usermanager import IUserManager
 from mailman.runners.command import CommandRunner
 from mailman.testing.helpers import (
     get_queue_messages,
     make_testable_runner,
+    reset_the_world,
     specialized_message_from_string as mfs)
 from mailman.testing.layers import ConfigLayer
 
@@ -51,6 +55,9 @@ class TestJoin(unittest.TestCase):
         self._commandq = config.switchboards['command']
         self._runner = make_testable_runner(CommandRunner, 'command')
 
+    def tearDown(self):
+        reset_the_world()
+
     def test_double_confirmation(self):
         # A join request comes in using both the -join address and the word
         # 'subscribe' in the first line of the body.  This should produce just
@@ -72,15 +79,14 @@ subscribe
         # second one is the confirmation message of her join request.
         messages = get_queue_messages('virgin', sort_on='subject')
         self.assertEqual(len(messages), 2)
-        # 'The' comes before 'confirm'
-        self.assertTrue(str(messages[0].msg['subject']).startswith('confirm'))
-        self.assertEqual(messages[1].msg['subject'],
+        self.assertTrue(str(messages[1].msg['subject']).startswith('confirm'))
+        self.assertEqual(messages[0].msg['subject'],
                          'The results of your email commands')
         # Search the contents of the results message.  There should be just
         # one 'Confirmation email' line.
         confirmation_lines = []
         in_results = False
-        for line in body_line_iterator(messages[1].msg, decode=True):
+        for line in body_line_iterator(messages[0].msg, decode=True):
             line = line.strip()
             if in_results:
                 if line.startswith('- Done'):
@@ -131,3 +137,93 @@ Subject: join
         self.assertEqual(len(confirmation_lines), 1)
         # And the confirmation line should name Anne's email address.
         self.assertTrue('anne@example.org' in confirmation_lines[0])
+
+
+
+class TestJoinWithDigests(unittest.TestCase):
+    """Test mailing list joins with the digests=<no|mime|plain> argument."""
+
+    layer = ConfigLayer
+
+    def setUp(self):
+        self._mlist = create_list('test@example.com')
+        self._commandq = config.switchboards['command']
+        self._runner = make_testable_runner(CommandRunner, 'command')
+
+    def tearDown(self):
+        reset_the_world()
+
+    def _confirm(self):
+        # There will be two messages in the queue - the confirmation messages,
+        # and a reply to Anne notifying her of the status of her command
+        # email.  We need to dig the confirmation token out of the Subject
+        # header of the latter so that we can confirm the subscription.
+        messages = get_queue_messages('virgin', sort_on='subject')
+        self.assertEqual(len(messages), 2)
+        subject_words = str(messages[1].msg['subject']).split()
+        self.assertEqual(subject_words[0], 'confirm')
+        token = subject_words[1]
+        status = getUtility(IRegistrar).confirm(token)
+        self.assertTrue(status, 'Confirmation failed')
+        # Now, make sure that Anne is a member of the list and is receiving
+        # digest deliveries.
+        members = getUtility(ISubscriptionService).find_members(
+            'anne@example.org')
+        self.assertEqual(len(members), 1)
+        return members[0]
+
+    def test_join_with_implicit_no_digests(self):
+        # Test the digest=mime argument to the join command.
+        msg = mfs("""\
+From: anne@example.org
+To: test-request@example.com
+
+join
+""")
+        self._commandq.enqueue(msg, dict(listname='test@example.com'))
+        self._runner.run()
+        anne = self._confirm()
+        self.assertEqual(anne.address.email, 'anne@example.org')
+        self.assertEqual(anne.delivery_mode, DeliveryMode.regular)
+
+    def test_join_with_explicit_no_digests(self):
+        # Test the digest=mime argument to the join command.
+        msg = mfs("""\
+From: anne@example.org
+To: test-request@example.com
+
+join digest=no
+""")
+        self._commandq.enqueue(msg, dict(listname='test@example.com'))
+        self._runner.run()
+        anne = self._confirm()
+        self.assertEqual(anne.address.email, 'anne@example.org')
+        self.assertEqual(anne.delivery_mode, DeliveryMode.regular)
+
+    def test_join_with_mime_digests(self):
+        # Test the digest=mime argument to the join command.
+        msg = mfs("""\
+From: anne@example.org
+To: test-request@example.com
+
+join digest=mime
+""")
+        self._commandq.enqueue(msg, dict(listname='test@example.com'))
+        self._runner.run()
+        anne = self._confirm()
+        self.assertEqual(anne.address.email, 'anne@example.org')
+        self.assertEqual(anne.delivery_mode, DeliveryMode.mime_digests)
+
+    def test_join_with_plain_digests(self):
+        # Test the digest=mime argument to the join command.
+        msg = mfs("""\
+From: anne@example.org
+To: test-request@example.com
+
+join digest=plain
+""")
+        self._commandq.enqueue(msg, dict(listname='test@example.com'))
+        self._runner.run()
+        anne = self._confirm()
+        self.assertEqual(anne.address.email, 'anne@example.org')
+        self.assertEqual(anne.delivery_mode, DeliveryMode.plaintext_digests)
diff --git a/src/mailman/testing/helpers.py b/src/mailman/testing/helpers.py
index de35be8be..e61aca605 100644
--- a/src/mailman/testing/helpers.py
+++ b/src/mailman/testing/helpers.py
@@ -127,7 +127,7 @@ def get_queue_messages(queue_name, sort_on=None):
         messages.append(_Bag(msg=msg, msgdata=msgdata))
         queue.finish(filebase)
     if sort_on is not None:
-        messages.sort(key=lambda item: item.msg[sort_on])
+        messages.sort(key=lambda item: str(item.msg[sort_on]))
     return messages