diff options
| -rw-r--r-- | src/mailman/app/moderator.py | 22 | ||||
| -rw-r--r-- | src/mailman/app/subscriptions.py | 10 | ||||
| -rw-r--r-- | src/mailman/app/tests/test_registrar.py | 2 | ||||
| -rw-r--r-- | src/mailman/rest/docs/sub-moderation.rst | 9 | ||||
| -rw-r--r-- | src/mailman/rest/sub_moderation.py | 14 | ||||
| -rw-r--r-- | src/mailman/rest/tests/test_moderation.py | 48 |
6 files changed, 79 insertions, 26 deletions
diff --git a/src/mailman/app/moderator.py b/src/mailman/app/moderator.py index 0e4d59479..eb848ea08 100644 --- a/src/mailman/app/moderator.py +++ b/src/mailman/app/moderator.py @@ -25,6 +25,7 @@ __all__ = [ 'hold_message', 'hold_subscription', 'hold_unsubscription', + 'send_rejection', ] @@ -125,8 +126,9 @@ def handle_message(mlist, id, action, language = member.preferred_language else: language = None - _refuse(mlist, _('Posting of your message titled "$subject"'), - sender, comment or _('[No reason given]'), language) + send_rejection( + mlist, _('Posting of your message titled "$subject"'), + sender, comment or _('[No reason given]'), language) elif action is Action.accept: # Start by getting the message from the message store. msg = message_store.get_message_by_id(message_id) @@ -236,10 +238,11 @@ def handle_subscription(mlist, id, action, comment=None): pass elif action is Action.reject: key, data = requestdb.get_request(id) - _refuse(mlist, _('Subscription request'), - data['email'], - comment or _('[No reason given]'), - lang=getUtility(ILanguageManager)[data['language']]) + 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']] @@ -307,8 +310,9 @@ def handle_unsubscription(mlist, id, action, comment=None): pass elif action is Action.reject: key, data = requestdb.get_request(id) - _refuse(mlist, _('Unsubscription request'), email, - comment or _('[No reason given]')) + send_rejection( + mlist, _('Unsubscription request'), email, + comment or _('[No reason given]')) elif action is Action.accept: key, data = requestdb.get_request(id) try: @@ -324,7 +328,7 @@ def handle_unsubscription(mlist, id, action, comment=None): -def _refuse(mlist, request, recip, comment, origmsg=None, lang=None): +def send_rejection(mlist, request, recip, comment, origmsg=None, lang=None): # As this message is going to the requester, try to set the language to # his/her language choice, if they are a member. Otherwise use the list's # preferred language. diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index 8794fb78a..1593b4d58 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -159,9 +159,13 @@ class SubscriptionWorkflow(Workflow): def _set_token(self, token_owner): assert isinstance(token_owner, TokenOwner) + pendings = getUtility(IPendings) + # Clear out the previous pending token if there is one. + if self.token is not None: + pendings.confirm(self.token) # Create a new token to prevent replay attacks. It seems like this - # should produce the same token, but it won't because the pending adds - # a bit of randomization. + # would produce the same token, but it won't because the pending adds a + # bit of randomization. self.token_owner = token_owner if token_owner is TokenOwner.no_one: self.token = None @@ -173,7 +177,7 @@ class SubscriptionWorkflow(Workflow): when=now().replace(microsecond=0).isoformat(), token_owner=token_owner.name, ) - self.token = getUtility(IPendings).add(pendable, timedelta(days=3650)) + self.token = pendings.add(pendable, timedelta(days=3650)) def _step_sanity_checks(self): # Ensure that we have both an address and a user, even if the address diff --git a/src/mailman/app/tests/test_registrar.py b/src/mailman/app/tests/test_registrar.py index a2ab2c686..e76009454 100644 --- a/src/mailman/app/tests/test_registrar.py +++ b/src/mailman/app/tests/test_registrar.py @@ -59,7 +59,7 @@ class TestRegistrar(unittest.TestCase): self.assertEqual(self._pendings.count, 1) record = self._pendings.confirm(token, expunge=False) self.assertEqual(record['list_id'], self._mlist.list_id) - self.assertEqual(record['address'], 'anne@example.com') + self.assertEqual(record['email'], 'anne@example.com') def test_subscribe(self): # Registering a subscription request where no confirmation or diff --git a/src/mailman/rest/docs/sub-moderation.rst b/src/mailman/rest/docs/sub-moderation.rst index 0468d75b5..52b2b89fd 100644 --- a/src/mailman/rest/docs/sub-moderation.rst +++ b/src/mailman/rest/docs/sub-moderation.rst @@ -65,13 +65,12 @@ You can view an individual membership change request by providing the token >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/' ... 'requests/{}'.format(token)) - delivery_mode: regular display_name: Anne Person email: anne@example.com http_etag: "..." - language: en - request_id: ... - type: subscription + list_id: ant.example.com + token: ... + token_owner: moderator when: 2005-08-01T07:49:23 @@ -99,11 +98,9 @@ Anne's subscription request is accepted. Anne is now a member of the mailing list. - >>> transaction.abort() >>> ant.members.get_member('anne@example.com') <Member: Anne Person <anne@example.com> on ant@example.com as MemberRole.member> - >>> transaction.abort() There are no more membership change requests. diff --git a/src/mailman/rest/sub_moderation.py b/src/mailman/rest/sub_moderation.py index 12bd8ab26..ebb09b9b3 100644 --- a/src/mailman/rest/sub_moderation.py +++ b/src/mailman/rest/sub_moderation.py @@ -22,12 +22,14 @@ __all__ = [ ] +from mailman.app.moderator import send_rejection from mailman.interfaces.action import Action from mailman.interfaces.pending import IPendings from mailman.interfaces.registrar import IRegistrar from mailman.rest.helpers import ( CollectionMixin, bad_request, child, etag, no_content, not_found, okay) from mailman.rest.validator import Validator, enum_validator +from mailman.utilities.i18n import _ from zope.component import getUtility @@ -98,8 +100,16 @@ class IndividualRequest(_ModerationBase): else: no_content(response) elif action is Action.reject: - # XXX - no_content(response) + # Like discard but sends a rejection notice to the user. + pendable = self._pendings.confirm(self._token, expunge=True) + if pendable is None: + not_found(response) + else: + no_content(response) + send_rejection( + self._mlist, _('Subscription request'), + pendable['email'], + _('[No reason given]')) diff --git a/src/mailman/rest/tests/test_moderation.py b/src/mailman/rest/tests/test_moderation.py index cc1f67f7e..e1d1f9ab3 100644 --- a/src/mailman/rest/tests/test_moderation.py +++ b/src/mailman/rest/tests/test_moderation.py @@ -28,6 +28,7 @@ import unittest from mailman.app.lifecycle import create_list from mailman.app.moderator import hold_message from mailman.database.transaction import transaction +from mailman.interfaces.mailinglist import SubscriptionPolicy from mailman.interfaces.registrar import IRegistrar from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import ( @@ -100,6 +101,7 @@ Something else. class TestSubscriptionModeration(unittest.TestCase): layer = RESTLayer + maxDiff = None def setUp(self): with transaction(): @@ -206,6 +208,38 @@ class TestSubscriptionModeration(unittest.TestCase): 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): # POST to the request to discard it. with transaction(): @@ -275,9 +309,9 @@ class TestSubscriptionModeration(unittest.TestCase): token, token_owner, member = self._registrar.register(self._anne) # Anne's subscription request got held. self.assertIsNone(member) - # There are currently no messages in the virgin queue. - items = get_queue_messages('virgin') - self.assertEqual(len(items), 0) + # 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', @@ -291,10 +325,14 @@ class TestSubscriptionModeration(unittest.TestCase): action='reject', )) self.assertEqual(cm.exception.code, 404) - # And the rejection message is now in the virgin queue. + # And the rejection message to Anne is now in the virgin queue. items = get_queue_messages('virgin') self.assertEqual(len(items), 1) - self.assertEqual(str(items[0].msg), '') + 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. |