merge trunk and fix merge conflicts

5 files changed, 300 insertions, 51 deletions

diff --git a/src/mailman/rest/tests/test_listconf.py b/src/mailman/rest/tests/test_listconf.py
index 560c3ec47..0c36ef30a 100644
--- a/src/mailman/rest/tests/test_listconf.py
+++ b/src/mailman/rest/tests/test_listconf.py
@@ -26,7 +26,8 @@ import unittest
 
 from mailman.app.lifecycle import create_list
 from mailman.database.transaction import transaction
-from mailman.interfaces.mailinglist import IAcceptableAliasSet
+from mailman.interfaces.mailinglist import (
+    IAcceptableAliasSet, SubscriptionPolicy)
 from mailman.testing.helpers import call_api
 from mailman.testing.layers import RESTLayer
 
@@ -91,11 +92,28 @@ class TestConfiguration(unittest.TestCase):
         self.assertEqual(set(IAcceptableAliasSet(self._mlist).aliases),
                          set(aliases))
 
+
+    def test_patch_subscription_policy(self):
+        # The new subscription_policy value can be patched.
+        #
+        # To start with, the subscription policy is confirm by default.
+        resource, response = call_api(
+            'http://localhost:9001/3.0/lists/test@example.com/config')
+        self.assertEqual(resource['subscription_policy'], 'confirm')
+        # Let's patch it to do some moderation.
+        resource, response = call_api(
+            'http://localhost:9001/3.0/lists/test@example.com/config', dict(
+                subscription_policy='confirm_then_moderate'),
+            method='PATCH')
+        self.assertEqual(response.status, 204)
+        # And now we verify that it has the requested setting.
+        self.assertEqual(self._mlist.subscription_policy,
+                         SubscriptionPolicy.confirm_then_moderate)
+
     @unittest.expectedFailure
     def test_bad_description_update(self):
         resource, response = call_api(
             'http://localhost:9001/3.0/lists/test@example.com/config',
             dict(description='A description with , to check stuff'), 'PATCH')
-        self.assertEqual(response.status, 204)
         self.assertEqual(self._mlist.description,
                          'A description with , to check stuff')
diff --git a/src/mailman/rest/tests/test_membership.py b/src/mailman/rest/tests/test_membership.py
index a77dea3b5..4542677b6 100644
--- a/src/mailman/rest/tests/test_membership.py
+++ b/src/mailman/rest/tests/test_membership.py
@@ -100,6 +100,9 @@ class TestMembership(unittest.TestCase):
             call_api('http://localhost:9001/3.0/members', {
                 'list_id': 'test.example.com',
                 'subscriber': 'anne@example.com',
+                'pre_verified': True,
+                'pre_confirmed': True,
+                'pre_approved': True,
                 })
         self.assertEqual(cm.exception.code, 409)
         self.assertEqual(cm.exception.reason, b'Member already subscribed')
@@ -115,6 +118,9 @@ class TestMembership(unittest.TestCase):
             call_api('http://localhost:9001/3.0/members', {
                 'list_id': 'test.example.com',
                 'subscriber': 'ANNE@example.com',
+                'pre_verified': True,
+                'pre_confirmed': True,
+                'pre_approved': True,
                 })
         self.assertEqual(cm.exception.code, 409)
         self.assertEqual(cm.exception.reason, b'Member already subscribed')
@@ -130,6 +136,9 @@ class TestMembership(unittest.TestCase):
             call_api('http://localhost:9001/3.0/members', {
                 'list_id': 'test.example.com',
                 'subscriber': 'anne@example.com',
+                'pre_verified': True,
+                'pre_confirmed': True,
+                'pre_approved': True,
                 })
         self.assertEqual(cm.exception.code, 409)
         self.assertEqual(cm.exception.reason, b'Member already subscribed')
@@ -151,6 +160,9 @@ class TestMembership(unittest.TestCase):
             'list_id': 'test.example.com',
             'subscriber': 'hugh/person@example.com',
             'display_name': 'Hugh Person',
+            'pre_verified': True,
+            'pre_confirmed': True,
+            'pre_approved': True,
             })
         self.assertEqual(content, None)
         self.assertEqual(response.status, 201)
diff --git a/src/mailman/rest/tests/test_moderation.py b/src/mailman/rest/tests/test_moderation.py
index c77ae2aca..e1d1f9ab3 100644
--- a/src/mailman/rest/tests/test_moderation.py
+++ b/src/mailman/rest/tests/test_moderation.py
@@ -18,26 +18,29 @@
 """REST moderation tests."""
 
 __all__ = [
-    'TestModeration',
+    'TestPostModeration',
+    'TestSubscriptionModeration',
     ]
 
 
 import unittest
 
 from mailman.app.lifecycle import create_list
-from mailman.app.moderator import hold_message, hold_subscription
-from mailman.config import config
+from mailman.app.moderator import hold_message
 from mailman.database.transaction import transaction
-from mailman.interfaces.member import DeliveryMode
-from mailman.interfaces.subscriptions import RequestRecord
+from mailman.interfaces.mailinglist import SubscriptionPolicy
+from mailman.interfaces.registrar import IRegistrar
+from mailman.interfaces.usermanager import IUserManager
 from mailman.testing.helpers import (
-    call_api, specialized_message_from_string as mfs)
+    call_api, get_queue_messages, specialized_message_from_string as mfs)
 from mailman.testing.layers import RESTLayer
+from mailman.utilities.datetime import now
 from urllib.error import HTTPError
+from zope.component import getUtility
 
 
 
-class TestModeration(unittest.TestCase):
+class TestPostModeration(unittest.TestCase):
     layer = RESTLayer
 
     def setUp(self):
@@ -71,24 +74,6 @@ Something else.
             call_api('http://localhost:9001/3.0/lists/ant@example.com/held/99')
         self.assertEqual(cm.exception.code, 404)
 
-    def test_subscription_request_as_held_message(self):
-        # Provide the request id of a subscription request using the held
-        # message API returns a not-found even though the request id is
-        # in the database.
-        held_id = hold_message(self._mlist, self._msg)
-        subscribe_id = hold_subscription(
-            self._mlist,
-            RequestRecord('bperson@example.net', 'Bart Person',
-                          DeliveryMode.regular, 'en'))
-        config.db.store.commit()
-        url = 'http://localhost:9001/3.0/lists/ant@example.com/held/{0}'
-        with self.assertRaises(HTTPError) as cm:
-            call_api(url.format(subscribe_id))
-        self.assertEqual(cm.exception.code, 404)
-        # But using the held_id returns a valid response.
-        response, content = call_api(url.format(held_id))
-        self.assertEqual(response['message_id'], '<alpha>')
-
     def test_bad_held_message_action(self):
         # POSTing to a held message with a bad action.
         held_id = hold_message(self._mlist, self._msg)
@@ -99,43 +84,260 @@ Something else.
         self.assertEqual(cm.exception.msg,
                          b'Cannot convert parameters: action')
 
-    def test_bad_subscription_request_id(self):
-        # Bad request when request_id is not an integer.
+    def test_discard(self):
+        # Discarding a message removes it from the moderation queue.
+        with transaction():
+            held_id = hold_message(self._mlist, self._msg)
+        url = 'http://localhost:9001/3.0/lists/ant@example.com/held/{}'.format(
+            held_id)
+        content, response = call_api(url, dict(action='discard'))
+        self.assertEqual(response.status, 204)
+        # Now it's gone.
         with self.assertRaises(HTTPError) as cm:
-            call_api('http://localhost:9001/3.0/lists/ant@example.com/'
-                     'requests/bogus')
-        self.assertEqual(cm.exception.code, 400)
+            call_api(url, dict(action='discard'))
+        self.assertEqual(cm.exception.code, 404)
 
-    def test_missing_subscription_request_id(self):
-        # Bad request when the request_id is not in the database.
+
+
+class TestSubscriptionModeration(unittest.TestCase):
+    layer = RESTLayer
+    maxDiff = None
+
+    def setUp(self):
+        with transaction():
+            self._mlist = create_list('ant@example.com')
+            self._registrar = IRegistrar(self._mlist)
+            manager = getUtility(IUserManager)
+            self._anne = manager.create_address(
+                'anne@example.com', 'Anne Person')
+            self._bart = manager.make_user(
+                'bart@example.com', 'Bart Person')
+            preferred = list(self._bart.addresses)[0]
+            preferred.verified_on = now()
+            self._bart.preferred_address = preferred
+
+    def test_no_such_list(self):
+        # Try to get the requests of a nonexistent list.
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/lists/bee@example.com/'
+                     'requests')
+        self.assertEqual(cm.exception.code, 404)
+
+    def test_no_such_subscription_token(self):
+        # Bad request when the token is not in the database.
         with self.assertRaises(HTTPError) as cm:
             call_api('http://localhost:9001/3.0/lists/ant@example.com/'
-                     'requests/99')
+                     'requests/missing')
         self.assertEqual(cm.exception.code, 404)
 
     def test_bad_subscription_action(self):
         # POSTing to a held message with a bad action.
-        held_id = hold_subscription(
-            self._mlist,
-            RequestRecord('cperson@example.net', 'Cris Person',
-                          DeliveryMode.regular, 'en'))
-        config.db.store.commit()
-        url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{0}'
+        token, token_owner, member = self._registrar.register(self._anne)
+        # Anne's subscription request got held.
+        self.assertIsNone(member)
+        # Let's try to handle her request, but with a bogus action.
+        url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}'
         with self.assertRaises(HTTPError) as cm:
-            call_api(url.format(held_id), {'action': 'bogus'})
+            call_api(url.format(token), dict(
+                action='bogus',
+                ))
         self.assertEqual(cm.exception.code, 400)
         self.assertEqual(cm.exception.msg,
                          b'Cannot convert parameters: action')
 
+    def test_list_held_requests(self):
+        # We can view all the held requests.
+        with transaction():
+            token_1, token_owner, member = self._registrar.register(self._anne)
+            # Anne's subscription request got held.
+            self.assertIsNotNone(token_1)
+            self.assertIsNone(member)
+            token_2, token_owner, member = self._registrar.register(self._bart)
+            self.assertIsNotNone(token_2)
+            self.assertIsNone(member)
+        content, response = call_api(
+            'http://localhost:9001/3.0/lists/ant@example.com/requests')
+        self.assertEqual(response.status, 200)
+        self.assertEqual(content['total_size'], 2)
+        tokens = set(json['token'] for json in content['entries'])
+        self.assertEqual(tokens, {token_1, token_2})
+        emails = set(json['email'] for json in content['entries'])
+        self.assertEqual(emails, {'anne@example.com', 'bart@example.com'})
+
+    def test_individual_request(self):
+        # We can view an individual request.
+        with transaction():
+            token, token_owner, member = self._registrar.register(self._anne)
+            # Anne's subscription request got held.
+            self.assertIsNotNone(token)
+            self.assertIsNone(member)
+        url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}'
+        content, response = call_api(url.format(token))
+        self.assertEqual(response.status, 200)
+        self.assertEqual(content['token'], token)
+        self.assertEqual(content['token_owner'], token_owner.name)
+        self.assertEqual(content['email'], 'anne@example.com')
+
+    def test_accept(self):
+        # POST to the request to accept it.
+        with transaction():
+            token, token_owner, member = self._registrar.register(self._anne)
+        # Anne's subscription request got held.
+        self.assertIsNone(member)
+        url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}'
+        content, response = call_api(url.format(token), dict(
+            action='accept',
+            ))
+        self.assertEqual(response.status, 204)
+        # Anne is a member.
+        self.assertEqual(
+            self._mlist.members.get_member('anne@example.com').address,
+            self._anne)
+        # The request URL no longer exists.
+        with self.assertRaises(HTTPError) as cm:
+            call_api(url.format(token), dict(
+                action='accept',
+                ))
+        self.assertEqual(cm.exception.code, 404)
+
+    def test_accept_bad_token(self):
+        # Try to accept a request with a bogus token.
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/lists/ant@example.com'
+                     '/requests/bogus',
+                     dict(action='accept'))
+        self.assertEqual(cm.exception.code, 404)
+
+    def test_accept_by_moderator_clears_request_queue(self):
+        # After accepting a message held for moderator approval, there are no
+        # more requests to handle.
+        #
+        # We start with nothing in the queue.
+        content, response = call_api(
+            'http://localhost:9001/3.0/lists/ant@example.com/requests')
+        self.assertEqual(content['total_size'], 0)
+        # Anne tries to subscribe to a list that only requests moderator
+        # approval.
+        with transaction():
+            self._mlist.subscription_policy = SubscriptionPolicy.moderate
+            token, token_owner, member = self._registrar.register(
+                self._anne,
+                pre_verified=True, pre_confirmed=True)
+        # There's now one request in the queue, and it's waiting on moderator
+        # approval.
+        content, response = call_api(
+            'http://localhost:9001/3.0/lists/ant@example.com/requests')
+        self.assertEqual(content['total_size'], 1)
+        json = content['entries'][0]
+        self.assertEqual(json['token_owner'], 'moderator')
+        self.assertEqual(json['email'], 'anne@example.com')
+        # The moderator approves the request.
+        url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}'
+        content, response = call_api(url.format(token), {'action': 'accept'})
+        self.assertEqual(response.status, 204)
+        # And now the request queue is empty.
+        content, response = call_api(
+            'http://localhost:9001/3.0/lists/ant@example.com/requests')
+        self.assertEqual(content['total_size'], 0)
+
     def test_discard(self):
-        # Discarding a message removes it from the moderation queue.
+        # POST to the request to discard it.
         with transaction():
-            held_id = hold_message(self._mlist, self._msg)
-        url = 'http://localhost:9001/3.0/lists/ant@example.com/held/{}'.format(
-            held_id)
-        content, response = call_api(url, dict(action='discard'))
+            token, token_owner, member = self._registrar.register(self._anne)
+        # Anne's subscription request got held.
+        self.assertIsNone(member)
+        url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}'
+        content, response = call_api(url.format(token), dict(
+            action='discard',
+            ))
         self.assertEqual(response.status, 204)
-        # Now it's gone.
+        # Anne is not a member.
+        self.assertIsNone(self._mlist.members.get_member('anne@example.com'))
+        # The request URL no longer exists.
         with self.assertRaises(HTTPError) as cm:
-            call_api(url, dict(action='discard'))
+            call_api(url.format(token), dict(
+                action='discard',
+                ))
+        self.assertEqual(cm.exception.code, 404)
+
+    def test_defer(self):
+        # Defer the decision for some other moderator.
+        with transaction():
+            token, token_owner, member = self._registrar.register(self._anne)
+        # Anne's subscription request got held.
+        self.assertIsNone(member)
+        url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}'
+        content, response = call_api(url.format(token), dict(
+            action='defer',
+            ))
+        self.assertEqual(response.status, 204)
+        # Anne is not a member.
+        self.assertIsNone(self._mlist.members.get_member('anne@example.com'))
+        # The request URL still exists.
+        content, response = call_api(url.format(token), dict(
+            action='defer',
+            ))
+        self.assertEqual(response.status, 204)
+        # And now we can accept it.
+        content, response = call_api(url.format(token), dict(
+            action='accept',
+            ))
+        self.assertEqual(response.status, 204)
+        # Anne is a member.
+        self.assertEqual(
+            self._mlist.members.get_member('anne@example.com').address,
+            self._anne)
+        # The request URL no longer exists.
+        with self.assertRaises(HTTPError) as cm:
+            call_api(url.format(token), dict(
+                action='accept',
+                ))
+        self.assertEqual(cm.exception.code, 404)
+
+    def test_defer_bad_token(self):
+        # Try to accept a request with a bogus token.
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/lists/ant@example.com'
+                     '/requests/bogus',
+                     dict(action='defer'))
+        self.assertEqual(cm.exception.code, 404)
+
+    def test_reject(self):
+        # POST to the request to reject it.  This leaves a bounce message in
+        # the virgin queue.
+        with transaction():
+            token, token_owner, member = self._registrar.register(self._anne)
+        # Anne's subscription request got held.
+        self.assertIsNone(member)
+        # Clear out the virgin queue, which currently contains the
+        # confirmation message sent to Anne.
+        get_queue_messages('virgin')
+        url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}'
+        content, response = call_api(url.format(token), dict(
+            action='reject',
+            ))
+        self.assertEqual(response.status, 204)
+        # Anne is not a member.
+        self.assertIsNone(self._mlist.members.get_member('anne@example.com'))
+        # The request URL no longer exists.
+        with self.assertRaises(HTTPError) as cm:
+            call_api(url.format(token), dict(
+                action='reject',
+                ))
+        self.assertEqual(cm.exception.code, 404)
+        # And the rejection message to Anne is now in the virgin queue.
+        items = get_queue_messages('virgin')
+        self.assertEqual(len(items), 1)
+        message = items[0].msg
+        self.assertEqual(message['From'], 'ant-bounces@example.com')
+        self.assertEqual(message['To'], 'anne@example.com')
+        self.assertEqual(message['Subject'],
+                         'Request to mailing list "Ant" rejected')
+
+    def test_reject_bad_token(self):
+        # Try to accept a request with a bogus token.
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/lists/ant@example.com'
+                     '/requests/bogus',
+                     dict(action='reject'))
         self.assertEqual(cm.exception.code, 404)
diff --git a/src/mailman/rest/tests/test_users.py b/src/mailman/rest/tests/test_users.py
index af2c9f0d1..ac8d018e8 100644
--- a/src/mailman/rest/tests/test_users.py
+++ b/src/mailman/rest/tests/test_users.py
@@ -376,7 +376,8 @@ class TestLP1074374(unittest.TestCase):
         call_api('http://localhost:9001/3.0/members', dict(
             list_id='test.example.com',
             subscriber='anne@example.com',
-            role='member'))
+            role='member',
+            pre_verified=True, pre_confirmed=True, pre_approved=True))
         # This is not the Anne you're looking for.  (IOW, the new Anne is a
         # different user).
         content, response = call_api(
diff --git a/src/mailman/rest/tests/test_validator.py b/src/mailman/rest/tests/test_validator.py
index c670fc77c..2d515f828 100644
--- a/src/mailman/rest/tests/test_validator.py
+++ b/src/mailman/rest/tests/test_validator.py
@@ -24,8 +24,11 @@ __all__ = [
 
 import unittest
 
-from mailman.rest.validator import list_of_strings_validator
+from mailman.interfaces.usermanager import IUserManager
+from mailman.rest.validator import (
+    list_of_strings_validator, subscriber_validator)
 from mailman.testing.layers import RESTLayer
+from zope.component import getUtility
 
 
 
@@ -46,3 +49,16 @@ class TestValidators(unittest.TestCase):
         # Strings are required.
         self.assertRaises(ValueError, list_of_strings_validator, 7)
         self.assertRaises(ValueError, list_of_strings_validator, ['ant', 7])
+
+    def test_subscriber_validator_uuid(self):
+        # Convert from an existing user id to a UUID.
+        anne = getUtility(IUserManager).make_user('anne@example.com')
+        uuid = subscriber_validator(str(anne.user_id.int))
+        self.assertEqual(anne.user_id, uuid)
+
+    def test_subscriber_validator_bad_uuid(self):
+        self.assertRaises(ValueError, subscriber_validator, 'not-a-thing')
+
+    def test_subscriber_validator_email_address(self):
+        self.assertEqual(subscriber_validator('anne@example.com'),
+                         'anne@example.com')