Merge branch 'api-clean' into 'master'

Remove old APIs handle_subscription() and hold_subscription().

See merge request !5

3 files changed, 74 insertions, 212 deletions

diff --git a/src/mailman/app/docs/moderator.rst b/src/mailman/app/docs/moderator.rst
index 43dc7688f..13dba1f37 100644
--- a/src/mailman/app/docs/moderator.rst
+++ b/src/mailman/app/docs/moderator.rst
@@ -232,130 +232,55 @@ The forwarded message is in the virgin queue, destined for the moderator.
     zack@example.com
 
 
-Holding subscription requests
-=============================
-
-For closed lists, subscription requests will also be held for moderator
-approval.  In this case, several pieces of information related to the
-subscription must be provided, including the subscriber's address and real
-name, what kind of delivery option they are choosing and their preferred
-language.
-
-    >>> from mailman.app.moderator import hold_subscription
-    >>> from mailman.interfaces.member import DeliveryMode
-    >>> from mailman.interfaces.subscriptions import RequestRecord
-    >>> req_id = hold_subscription(
-    ...     mlist,
-    ...     RequestRecord('fred@example.org', 'Fred Person',
-    ...                   DeliveryMode.regular, 'en'))
-
-
-Disposing of membership change requests
----------------------------------------
-
-Just as with held messages, the moderator can select one of several
-dispositions for this membership change request.  The most trivial is to
-simply defer a decision for now.
-
-    >>> from mailman.app.moderator import handle_subscription
-    >>> handle_subscription(mlist, req_id, Action.defer)
-    >>> requests.get_request(req_id) is not None
-    True
-
-The held subscription can also be discarded.
-
-    >>> handle_subscription(mlist, req_id, Action.discard)
-    >>> print(requests.get_request(req_id))
-    None
-
-Gwen tries to subscribe to the mailing list, but...
-
-    >>> req_id = hold_subscription(
-    ...     mlist,
-    ...     RequestRecord('gwen@example.org', 'Gwen Person',
-    ...                   DeliveryMode.regular, 'en'))
-
-
-...her request is rejected...
-
-    >>> handle_subscription(
-    ...     mlist, req_id, Action.reject, 'This is a closed list')
-    >>> messages = get_queue_messages('virgin')
-    >>> len(messages)
-    1
-
-...and she receives a rejection notice.
-
-    >>> print(messages[0].msg.as_string())
-    MIME-Version: 1.0
-    ...
-    Subject: Request to mailing list "A Test List" rejected
-    From: ant-bounces@example.com
-    To: gwen@example.org
-    ...
-    Your request to the ant@example.com mailing list
-    <BLANKLINE>
-        Subscription request
-    <BLANKLINE>
-    has been rejected by the list moderator.  The moderator gave the
-    following reason for rejecting your request:
-    <BLANKLINE>
-    "This is a closed list"
-    ...
-
-The subscription can also be accepted.  This subscribes the address to the
-mailing list.
-
-    >>> mlist.send_welcome_message = False
-    >>> req_id = hold_subscription(
-    ...     mlist,
-    ...     RequestRecord('herb@example.org', 'Herb Person',
-    ...                   DeliveryMode.regular, 'en'))
-
-The moderators accept the subscription request.
-
-    >>> handle_subscription(mlist, req_id, Action.accept)
-
-And now Herb is a member of the mailing list.
-
-    >>> print(mlist.members.get_member('herb@example.org').address)
-    Herb Person <herb@example.org>
-
-
 Holding unsubscription requests
 ===============================
 
 Some lists require moderator approval for unsubscriptions.  In this case, only
 the unsubscribing address is required.
 
-Herb now wants to leave the mailing list, but his request must be approved.
+Fred is a member of the mailing list...
+
+    >>> mlist.send_welcome_message = False
+    >>> from mailman.interfaces.usermanager import IUserManager
+    >>> fred = getUtility(IUserManager).create_address(
+    ...     'fred@example.com', 'Fred Person')
+    >>> from mailman.interfaces.registrar import IRegistrar
+    >>> registrar = IRegistrar(mlist)
+    >>> token, token_owner, member = registrar.register(
+    ...     fred, pre_verified=True, pre_confirmed=True, pre_approved=True)
+    >>> member
+    <Member: Fred Person <fred@example.com> on ant@example.com
+             as MemberRole.member>
+
+...but now that he wants to leave the mailing list, his request must be
+approved.
 
     >>> from mailman.app.moderator import hold_unsubscription
-    >>> req_id = hold_unsubscription(mlist, 'herb@example.org')
+    >>> req_id = hold_unsubscription(mlist, 'fred@example.com')
 
 As with subscription requests, the unsubscription request can be deferred.
 
     >>> from mailman.app.moderator import handle_unsubscription
     >>> handle_unsubscription(mlist, req_id, Action.defer)
-    >>> print(mlist.members.get_member('herb@example.org').address)
-    Herb Person <herb@example.org>
+    >>> print(mlist.members.get_member('fred@example.com').address)
+    Fred Person <fred@example.com>
 
 The held unsubscription can also be discarded, and the member will remain
 subscribed.
 
     >>> handle_unsubscription(mlist, req_id, Action.discard)
-    >>> print(mlist.members.get_member('herb@example.org').address)
-    Herb Person <herb@example.org>
+    >>> print(mlist.members.get_member('fred@example.com').address)
+    Fred Person <fred@example.com>
 
 The request can be rejected, in which case a message is sent to the member,
 and the person remains a member of the mailing list.
 
-    >>> req_id = hold_unsubscription(mlist, 'herb@example.org')
+    >>> req_id = hold_unsubscription(mlist, 'fred@example.com')
     >>> handle_unsubscription(mlist, req_id, Action.reject, 'No can do')
-    >>> print(mlist.members.get_member('herb@example.org').address)
-    Herb Person <herb@example.org>
+    >>> print(mlist.members.get_member('fred@example.com').address)
+    Fred Person <fred@example.com>
 
-Herb gets a rejection notice.
+Fred gets a rejection notice.
 ::
 
     >>> messages = get_queue_messages('virgin')
@@ -367,7 +292,7 @@ Herb gets a rejection notice.
     ...
     Subject: Request to mailing list "A Test List" rejected
     From: ant-bounces@example.com
-    To: herb@example.org
+    To: fred@example.com
     ...
     Your request to the ant@example.com mailing list
     <BLANKLINE>
@@ -382,10 +307,10 @@ Herb gets a rejection notice.
 The unsubscription request can also be accepted.  This removes the member from
 the mailing list.
 
-    >>> req_id = hold_unsubscription(mlist, 'herb@example.org')
+    >>> req_id = hold_unsubscription(mlist, 'fred@example.com')
     >>> mlist.send_goodbye_message = False
     >>> handle_unsubscription(mlist, req_id, Action.accept)
-    >>> print(mlist.members.get_member('herb@example.org'))
+    >>> print(mlist.members.get_member('fred@example.com'))
     None
 
 
@@ -399,13 +324,24 @@ Usually, the list administrators want to be notified when there are membership
 change requests they need to moderate.  These notifications are sent when the
 list is configured to send them.
 
+    >>> from mailman.interfaces.mailinglist import SubscriptionPolicy
     >>> mlist.admin_immed_notify = True
+    >>> mlist.subscription_policy = SubscriptionPolicy.moderate
+
+Gwen tries to subscribe to the mailing list.
 
-Iris tries to subscribe to the mailing list.
+    >>> gwen = getUtility(IUserManager).create_address(
+    ...     'gwen@example.com', 'Gwen Person')
+    >>> token, token_owner, member = registrar.register(
+    ...     gwen, pre_verified=True, pre_confirmed=True)
 
-    >>> req_id = hold_subscription(mlist,
-    ...     RequestRecord('iris@example.org', 'Iris Person',
-    ...                   DeliveryMode.regular, 'en'))
+Her subscription must be approved by the list administrator, so she is not yet
+a member of the mailing list.
+
+    >>> print(member)
+    None
+    >>> print(mlist.members.get_member('gwen@example.com'))
+    None
 
 There's now a message in the virgin queue, destined for the list owner.
 
@@ -415,14 +351,14 @@ There's now a message in the virgin queue, destined for the list owner.
     >>> print(messages[0].msg.as_string())
     MIME-Version: 1.0
     ...
-    Subject: New subscription request to A Test List from iris@example.org
+    Subject: New subscription request to A Test List from gwen@example.com
     From: ant-owner@example.com
     To: ant-owner@example.com
     ...
     Your authorization is required for a mailing list subscription request
     approval:
     <BLANKLINE>
-        For:  iris@example.org
+        For:  Gwen Person <gwen@example.com>
         List: ant@example.com
 
 Similarly, the administrator gets notifications on unsubscription requests.
@@ -455,7 +391,10 @@ receive a membership change notice.
 
     >>> mlist.admin_notify_mchanges = True
     >>> mlist.admin_immed_notify = False
-    >>> handle_subscription(mlist, req_id, Action.accept)
+    >>> token, token_owner, member = registrar.confirm(token)
+    >>> member
+    <Member: Gwen Person <gwen@example.com> on ant@example.com
+             as MemberRole.member>
     >>> messages = get_queue_messages('virgin')
     >>> len(messages)
     1
@@ -466,13 +405,13 @@ receive a membership change notice.
     From: noreply@example.com
     To: ant-owner@example.com
     ...
-    Iris Person <iris@example.org> has been successfully subscribed to A
+    Gwen Person <gwen@example.com> has been successfully subscribed to A
     Test List.
 
 Similarly when an unsubscription request is accepted, the administrators can
 get a notification.
 
-    >>> req_id = hold_unsubscription(mlist, 'iris@example.org')
+    >>> req_id = hold_unsubscription(mlist, 'gwen@example.com')
     >>> handle_unsubscription(mlist, req_id, Action.accept)
     >>> messages = get_queue_messages('virgin')
     >>> len(messages)
@@ -484,21 +423,21 @@ get a notification.
     From: noreply@example.com
     To: ant-owner@example.com
     ...
-    Iris Person <iris@example.org> has been removed from A Test List.
+    Gwen Person <gwen@example.com> has been removed from A Test List.
 
 
 Welcome messages
 ----------------
 
-When a member is subscribed to the mailing list via moderator approval, she
-can get a welcome message.
+When a member is subscribed to the mailing list, they can get a welcome
+message.
 
     >>> mlist.admin_notify_mchanges = False
     >>> mlist.send_welcome_message = True
-    >>> req_id = hold_subscription(mlist,
-    ...     RequestRecord('kate@example.org', 'Kate Person',
-    ...                   DeliveryMode.regular, 'en'))
-    >>> handle_subscription(mlist, req_id, Action.accept)
+    >>> herb = getUtility(IUserManager).create_address(
+    ...     'herb@example.com', 'Herb Person')
+    >>> token, token_owner, member = registrar.register(
+    ...     herb, pre_verified=True, pre_confirmed=True, pre_approved=True)
     >>> messages = get_queue_messages('virgin')
     >>> len(messages)
     1
@@ -507,7 +446,7 @@ can get a welcome message.
     ...
     Subject: Welcome to the "A Test List" mailing list
     From: ant-request@example.com
-    To: Kate Person <kate@example.org>
+    To: Herb Person <herb@example.com>
     ...
     Welcome to the "A Test List" mailing list!
     ...
@@ -520,7 +459,7 @@ Similarly, when the member's unsubscription request is approved, she'll get a
 goodbye message.
 
     >>> mlist.send_goodbye_message = True
-    >>> req_id = hold_unsubscription(mlist, 'kate@example.org')
+    >>> req_id = hold_unsubscription(mlist, 'herb@example.com')
     >>> handle_unsubscription(mlist, req_id, Action.accept)
     >>> messages = get_queue_messages('virgin')
     >>> len(messages)
@@ -530,5 +469,5 @@ goodbye message.
     ...
     Subject: You have been unsubscribed from the A Test List mailing list
     From: ant-bounces@example.com
-    To: kate@example.org
+    To: herb@example.com
     ...
diff --git a/src/mailman/app/moderator.py b/src/mailman/app/moderator.py
index a5c0d50dd..04054d5a2 100644
--- a/src/mailman/app/moderator.py
+++ b/src/mailman/app/moderator.py
@@ -20,10 +20,8 @@
 __all__ = [
     'handle_ListDeletingEvent',
     'handle_message',
-    'handle_subscription',
     'handle_unsubscription',
     'hold_message',
-    'hold_subscription',
     'hold_unsubscription',
     'send_rejection',
     ]
@@ -32,19 +30,16 @@ __all__ = [
 import time
 import logging
 
-from email.utils import formataddr, formatdate, getaddresses, make_msgid
-from mailman.app.membership import add_member, delete_member
+from email.utils import formatdate, getaddresses, make_msgid
+from mailman.app.membership import delete_member
 from mailman.config import config
 from mailman.core.i18n import _
 from mailman.email.message import UserNotification
 from mailman.interfaces.action import Action
-from mailman.interfaces.languages import ILanguageManager
 from mailman.interfaces.listmanager import ListDeletingEvent
-from mailman.interfaces.member import (
-    AlreadySubscribedError, DeliveryMode, NotAMemberError)
+from mailman.interfaces.member import NotAMemberError
 from mailman.interfaces.messages import IMessageStore
 from mailman.interfaces.requests import IListRequests, RequestType
-from mailman.interfaces.subscriptions import RequestRecord
 from mailman.utilities.datetime import now
 from mailman.utilities.i18n import make
 from zope.component import getUtility
@@ -194,78 +189,6 @@ def handle_message(mlist, id, action,
 
 
 
-def hold_subscription(mlist, record):
-    data = dict(when=now().isoformat(),
-                email=record.email,
-                display_name=record.display_name,
-                delivery_mode=record.delivery_mode.name,
-                language=record.language)
-    # Now hold this request.  We'll use the email address as the key.
-    requestsdb = IListRequests(mlist)
-    request_id = requestsdb.hold_request(
-        RequestType.subscription, record.email, data)
-    vlog.info('%s: held subscription request from %s',
-              mlist.fqdn_listname, record.email)
-    # Possibly notify the administrator in default list language
-    if mlist.admin_immed_notify:
-        email = record.email # XXX: seems unnecessary
-        subject = _(
-            'New subscription request to $mlist.display_name from $email')
-        text = make('subauth.txt',
-                    mailing_list=mlist,
-                    username=record.email,
-                    listname=mlist.fqdn_listname,
-                    admindb_url=mlist.script_url('admindb'),
-                    )
-        # This message should appear to come from the <list>-owner so as
-        # to avoid any useless bounce processing.
-        msg = UserNotification(
-            mlist.owner_address, mlist.owner_address,
-            subject, text, mlist.preferred_language)
-        msg.send(mlist, tomoderators=True)
-    return request_id
-
-
-
-def handle_subscription(mlist, id, action, comment=None):
-    requestdb = IListRequests(mlist)
-    if action is Action.defer:
-        # Nothing to do.
-        return
-    elif action is Action.discard:
-        # Nothing to do except delete the request from the database.
-        pass
-    elif action is Action.reject:
-        key, data = requestdb.get_request(id)
-        send_rejection(
-            mlist, _('Subscription request'),
-            data['email'],
-            comment or _('[No reason given]'),
-            lang=getUtility(ILanguageManager)[data['language']])
-    elif action is Action.accept:
-        key, data = requestdb.get_request(id)
-        delivery_mode = DeliveryMode[data['delivery_mode']]
-        email = data['email']
-        display_name = data['display_name']
-        language = getUtility(ILanguageManager)[data['language']]
-        try:
-            add_member(
-                mlist,
-                RequestRecord(email, display_name, delivery_mode, language))
-        except AlreadySubscribedError:
-            # The address got subscribed in some other way after the original
-            # request was made and accepted.
-            pass
-        slog.info('%s: new %s, %s %s', mlist.fqdn_listname,
-                  delivery_mode, formataddr((display_name, email)),
-                  'via admin approval')
-    else:
-        raise AssertionError('Unexpected action: {0}'.format(action))
-    # Delete the request from the database.
-    requestdb.delete_request(id)
-
-
-
 def hold_unsubscription(mlist, email):
     data = dict(email=email)
     requestsdb = IListRequests(mlist)
diff --git a/src/mailman/app/tests/test_moderation.py b/src/mailman/app/tests/test_moderation.py
index a371d8520..006348a3d 100644
--- a/src/mailman/app/tests/test_moderation.py
+++ b/src/mailman/app/tests/test_moderation.py
@@ -27,13 +27,12 @@ import unittest
 
 from mailman.app.lifecycle import create_list
 from mailman.app.moderator import (
-    handle_message, handle_subscription, handle_unsubscription, hold_message,
-    hold_subscription, hold_unsubscription)
+    handle_message, handle_unsubscription, hold_message, hold_unsubscription)
 from mailman.interfaces.action import Action
-from mailman.interfaces.member import DeliveryMode
 from mailman.interfaces.messages import IMessageStore
+from mailman.interfaces.registrar import IRegistrar
 from mailman.interfaces.requests import IListRequests
-from mailman.interfaces.subscriptions import RequestRecord
+from mailman.interfaces.usermanager import IUserManager
 from mailman.runners.incoming import IncomingRunner
 from mailman.runners.outgoing import OutgoingRunner
 from mailman.runners.pipeline import PipelineRunner
@@ -163,16 +162,17 @@ class TestUnsubscription(unittest.TestCase):
 
     def setUp(self):
         self._mlist = create_list('test@example.com')
-        self._request_db = IListRequests(self._mlist)
+        self._registrar = IRegistrar(self._mlist)
 
     def test_unsubscribe_defer(self):
         # When unsubscriptions must be approved by the moderator, but the
         # moderator defers this decision.
-        token = hold_subscription(
-            self._mlist,
-            RequestRecord('anne@example.org', 'Anne Person',
-                          DeliveryMode.regular, 'en'))
-        handle_subscription(self._mlist, token, Action.accept)
+        anne = getUtility(IUserManager).create_address(
+            'anne@example.org', 'Anne Person')
+        token, token_owner, member = self._registrar.register(
+            anne, pre_verified=True, pre_confirmed=True, pre_approved=True)
+        self.assertIsNone(token)
+        self.assertEqual(member.address.email, 'anne@example.org')
         # Now hold and handle an unsubscription request.
         token = hold_unsubscription(self._mlist, 'anne@example.org')
         handle_unsubscription(self._mlist, token, Action.defer)