From 53d4229bff96677a54fd443322c166c2bdcc3054 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Thu, 16 Apr 2015 15:28:06 -0400 Subject: Checkpointing. --- src/mailman/app/subscriptions.py | 3 +- src/mailman/app/tests/test_subscriptions.py | 16 ++ src/mailman/interfaces/pending.py | 6 + src/mailman/interfaces/registrar.py | 23 +- src/mailman/model/docs/pending.rst | 19 +- src/mailman/model/pending.py | 5 + src/mailman/rest/docs/moderation.rst | 361 ---------------------------- src/mailman/rest/docs/post-moderation.rst | 192 +++++++++++++++ src/mailman/rest/lists.py | 3 +- src/mailman/rest/moderation.py | 252 ------------------- src/mailman/rest/post_moderation.py | 155 ++++++++++++ src/mailman/rest/tests/test_moderation.py | 219 +++++++++++++---- 12 files changed, 577 insertions(+), 677 deletions(-) delete mode 100644 src/mailman/rest/docs/moderation.rst create mode 100644 src/mailman/rest/docs/post-moderation.rst delete mode 100644 src/mailman/rest/moderation.py create mode 100644 src/mailman/rest/post_moderation.py (limited to 'src') diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index 1c6b96016..46ce549af 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -24,7 +24,6 @@ __all__ = [ ] - import uuid import logging @@ -170,6 +169,8 @@ class SubscriptionWorkflow(Workflow): pendable = Pendable( list_id=self.mlist.list_id, address=self.address.email, + hold_date=now().replace(microsecond=0).isoformat(), + token_owner=token_owner.name, ) self.token = getUtility(IPendings).add(pendable, timedelta(days=3650)) diff --git a/src/mailman/app/tests/test_subscriptions.py b/src/mailman/app/tests/test_subscriptions.py index 732266ac3..6b5d88026 100644 --- a/src/mailman/app/tests/test_subscriptions.py +++ b/src/mailman/app/tests/test_subscriptions.py @@ -57,6 +57,22 @@ class TestSubscriptionWorkflow(unittest.TestCase): self.assertEqual(workflow.token_owner, TokenOwner.no_one) self.assertIsNone(workflow.member) + def test_pended_data(self): + # There is a Pendable associated with the held request, and it has + # some data associated with it. + anne = self._user_manager.create_address(self._anne) + workflow = SubscriptionWorkflow(self._mlist, anne) + try: + workflow.run_thru('send_confirmation') + except StopIteration: + pass + self.assertIsNotNone(workflow.token) + pendable = getUtility(IPendings).confirm(workflow.token, expunge=False) + self.assertEqual(pendable['list_id'], 'test.example.com') + self.assertEqual(pendable['address'], 'anne@example.com') + self.assertEqual(pendable['hold_date'], '2005-08-01T07:49:23') + self.assertEqual(pendable['token_owner'], 'subscriber') + def test_user_or_address_required(self): # The `subscriber` attribute must be a user or address. workflow = SubscriptionWorkflow(self._mlist) diff --git a/src/mailman/interfaces/pending.py b/src/mailman/interfaces/pending.py index 222f0dfbf..c921123de 100644 --- a/src/mailman/interfaces/pending.py +++ b/src/mailman/interfaces/pending.py @@ -95,4 +95,10 @@ class IPendings(Interface): def evict(): """Remove all pended items whose lifetime has expired.""" + def __iter__(): + """An iterator over all pendables. + + Each element is a 2-tuple of the form (token, dict). + """ + count = Attribute('The number of pendables in the pendings database.') diff --git a/src/mailman/interfaces/registrar.py b/src/mailman/interfaces/registrar.py index 6f049b9d7..959e0bf6a 100644 --- a/src/mailman/interfaces/registrar.py +++ b/src/mailman/interfaces/registrar.py @@ -75,12 +75,13 @@ class IRegistrar(Interface): :param subscriber: The user or address to subscribe. :type email: ``IUser`` or ``IAddress`` - :return: None if the workflow completes with the member being - subscribed. If the workflow is paused for user confirmation or - moderator approval, a 3-tuple is returned where the first element - is a ``TokenOwner`` the second element is the token hash, and the - third element is the subscribed member. - :rtype: None or 2-tuple of (TokenOwner, str) + :return: A 3-tuple is returned where the first element is the token + hash, the second element is a ``TokenOwner`, and the third element + is the subscribed member. If the subscriber got subscribed + immediately, the token will be None and the member will be + an ``IMember``. If the subscription got held, the token + will be a hash and the member will be None. + :rtype: (str-or-None, ``TokenOwner``, ``IMember``-or-None) :raises MembershipIsBannedError: when the address being subscribed appears in the global or list-centric bans. """ @@ -94,9 +95,13 @@ class IRegistrar(Interface): :param token: A token matching a workflow. :type token: string - :return: The new token for any follow up confirmation, or None if the - user was subscribed. - :rtype: str or None + :return: A 3-tuple is returned where the first element is the token + hash, the second element is a ``TokenOwner`, and the third element + is the subscribed member. If the subscriber got subscribed + immediately, the token will be None and the member will be + an ``IMember``. If the subscription is still being held, the token + will be a hash and the member will be None. + :rtype: (str-or-None, ``TokenOwner``, ``IMember``-or-None) :raises LookupError: when no workflow is associated with the token. """ diff --git a/src/mailman/model/docs/pending.rst b/src/mailman/model/docs/pending.rst index 4a14edb2a..03eee3772 100644 --- a/src/mailman/model/docs/pending.rst +++ b/src/mailman/model/docs/pending.rst @@ -43,10 +43,9 @@ There's exactly one entry in the pendings database now. >>> pendingdb.count 1 -There's not much you can do with tokens except to *confirm* them, which -basically means returning the `IPendable` structure (as a dictionary) from the -database that matches the token. If the token isn't in the database, None is -returned. +You can *confirm* the pending, which means returning the `IPendable` structure +(as a dictionary) from the database that matches the token. If the token +isn't in the database, None is returned. >>> pendable = pendingdb.confirm(b'missing') >>> print(pendable) @@ -83,6 +82,18 @@ expunge it. >>> print(pendingdb.confirm(token_1)) None +You can iterate over all the pendings in the database. + + >>> pendables = list(pendingdb) + >>> def sort_key(item): + ... token, pendable = item + ... return pendable['type'] + >>> sorted_pendables = sorted(pendables, key=sort_key) + >>> for token, pendable in sorted_pendables: + ... print(pendable['type']) + three + two + An event can be given a lifetime when it is pended, otherwise it just uses a default lifetime. diff --git a/src/mailman/model/pending.py b/src/mailman/model/pending.py index bbe95d5f0..04b63f2ca 100644 --- a/src/mailman/model/pending.py +++ b/src/mailman/model/pending.py @@ -166,6 +166,11 @@ class Pendings: store.delete(keyvalue) store.delete(pending) + @dbconnection + def __iter__(self, store): + for pending in store.query(Pended).all(): + yield pending.token, self.confirm(pending.token, expunge=False) + @property @dbconnection def count(self, store): diff --git a/src/mailman/rest/docs/moderation.rst b/src/mailman/rest/docs/moderation.rst deleted file mode 100644 index 0a9fd59a8..000000000 --- a/src/mailman/rest/docs/moderation.rst +++ /dev/null @@ -1,361 +0,0 @@ -========== -Moderation -========== - -There are two kinds of moderation tasks a list administrator may need to -perform. Messages which are held for approval can be accepted, rejected, -discarded, or deferred. Subscription (and sometimes unsubscription) requests -can similarly be accepted, discarded, rejected, or deferred. - - -Message moderation -================== - -Viewing the list of held messages ---------------------------------- - -Held messages can be moderated through the REST API. A mailing list starts -with no held messages. - - >>> ant = create_list('ant@example.com') - >>> transaction.commit() - >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/held') - http_etag: "..." - start: 0 - total_size: 0 - -When a message gets held for moderator approval, it shows up in this list. -:: - - >>> msg = message_from_string("""\ - ... From: anne@example.com - ... To: ant@example.com - ... Subject: Something - ... Message-ID: - ... - ... Something else. - ... """) - - >>> from mailman.app.moderator import hold_message - >>> request_id = hold_message(ant, msg, {'extra': 7}, 'Because') - >>> transaction.commit() - - >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/held') - entry 0: - extra: 7 - hold_date: 2005-08-01T07:49:23 - http_etag: "..." - message_id: - msg: From: anne@example.com - To: ant@example.com - Subject: Something - Message-ID: - X-Message-ID-Hash: GCSMSG43GYWWVUMO6F7FBUSSPNXQCJ6M - - Something else. - - reason: Because - request_id: 1 - sender: anne@example.com - subject: Something - http_etag: "..." - start: 0 - total_size: 1 - -You can get an individual held message by providing the *request id* for that -message. This will include the text of the message. -:: - - >>> def url(request_id): - ... return ('http://localhost:9001/3.0/lists/' - ... 'ant@example.com/held/{0}'.format(request_id)) - - >>> dump_json(url(request_id)) - extra: 7 - hold_date: 2005-08-01T07:49:23 - http_etag: "..." - message_id: - msg: From: anne@example.com - To: ant@example.com - Subject: Something - Message-ID: - X-Message-ID-Hash: GCSMSG43GYWWVUMO6F7FBUSSPNXQCJ6M - - Something else. - - reason: Because - request_id: 1 - sender: anne@example.com - subject: Something - - -Disposing of held messages --------------------------- - -Individual messages can be moderated through the API by POSTing back to the -held message's resource. The POST data requires an action of one of the -following: - - * discard - throw the message away. - * reject - bounces the message back to the original author. - * defer - defer any action on the message (continue to hold it) - * accept - accept the message for posting. - -Let's see what happens when the above message is deferred. - - >>> dump_json(url(request_id), { - ... 'action': 'defer', - ... }) - content-length: 0 - date: ... - server: ... - status: 204 - -The message is still in the moderation queue. - - >>> dump_json(url(request_id)) - extra: 7 - hold_date: 2005-08-01T07:49:23 - http_etag: "..." - message_id: - msg: From: anne@example.com - To: ant@example.com - Subject: Something - Message-ID: - X-Message-ID-Hash: GCSMSG43GYWWVUMO6F7FBUSSPNXQCJ6M - - Something else. - - reason: Because - request_id: 1 - sender: anne@example.com - subject: Something - -The held message can be discarded. - - >>> dump_json(url(request_id), { - ... 'action': 'discard', - ... }) - content-length: 0 - date: ... - server: ... - status: 204 - -Messages can also be accepted via the REST API. Let's hold a new message for -moderation. -:: - - >>> del msg['message-id'] - >>> msg['Message-ID'] = '' - >>> request_id = hold_message(ant, msg) - >>> transaction.commit() - - >>> results = call_http(url(request_id)) - >>> print(results['message_id']) - - - >>> dump_json(url(request_id), { - ... 'action': 'accept', - ... }) - content-length: 0 - date: ... - server: ... - status: 204 - - >>> from mailman.testing.helpers import get_queue_messages - >>> messages = get_queue_messages('pipeline') - >>> len(messages) - 1 - >>> print(messages[0].msg['message-id']) - - -Messages can be rejected via the REST API too. These bounce the message back -to the original author. -:: - - >>> del msg['message-id'] - >>> msg['Message-ID'] = '' - >>> request_id = hold_message(ant, msg) - >>> transaction.commit() - - >>> results = call_http(url(request_id)) - >>> print(results['message_id']) - - - >>> dump_json(url(request_id), { - ... 'action': 'reject', - ... }) - content-length: 0 - date: ... - server: ... - status: 204 - - >>> from mailman.testing.helpers import get_queue_messages - >>> messages = get_queue_messages('virgin') - >>> len(messages) - 1 - >>> print(messages[0].msg['subject']) - Request to mailing list "Ant" rejected - - -Subscription moderation -======================= - -Viewing subscription requests ------------------------------ - -Subscription and unsubscription requests can be moderated via the REST API as -well. A mailing list starts with no pending subscription or unsubscription -requests. - - >>> ant.admin_immed_notify = False - >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/requests') - http_etag: "..." - start: 0 - total_size: 0 - -When Anne tries to subscribe to the Ant list, her subscription is held for -moderator approval. -:: - - >>> from mailman.app.moderator import hold_subscription - >>> from mailman.interfaces.member import DeliveryMode - >>> from mailman.interfaces.subscriptions import RequestRecord - - >>> sub_req_id = hold_subscription( - ... ant, RequestRecord('anne@example.com', 'Anne Person', - ... DeliveryMode.regular, 'en')) - >>> transaction.commit() - -The subscription request is available from the mailing list. - - >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/requests') - entry 0: - delivery_mode: regular - display_name: Anne Person - email: anne@example.com - http_etag: "..." - language: en - request_id: ... - type: subscription - when: 2005-08-01T07:49:23 - http_etag: "..." - start: 0 - total_size: 1 - - -Viewing unsubscription requests -------------------------------- - -Bart tries to leave a mailing list, but he may not be allowed to. - - >>> from mailman.testing.helpers import subscribe - >>> from mailman.app.moderator import hold_unsubscription - >>> bart = subscribe(ant, 'Bart', email='bart@example.com') - >>> unsub_req_id = hold_unsubscription(ant, 'bart@example.com') - >>> transaction.commit() - -The unsubscription request is also available from the mailing list. - - >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/requests') - entry 0: - delivery_mode: regular - display_name: Anne Person - email: anne@example.com - http_etag: "..." - language: en - request_id: ... - type: subscription - when: 2005-08-01T07:49:23 - entry 1: - email: bart@example.com - http_etag: "..." - request_id: ... - type: unsubscription - http_etag: "..." - start: 0 - total_size: 2 - - -Viewing individual requests ---------------------------- - -You can view an individual membership change request by providing the -request id. Anne's subscription request looks like this. - - >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/' - ... 'requests/{}'.format(sub_req_id)) - delivery_mode: regular - display_name: Anne Person - email: anne@example.com - http_etag: "..." - language: en - request_id: ... - type: subscription - when: 2005-08-01T07:49:23 - -Bart's unsubscription request looks like this. - - >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/' - ... 'requests/{}'.format(unsub_req_id)) - email: bart@example.com - http_etag: "..." - request_id: ... - type: unsubscription - - -Disposing of subscription requests ----------------------------------- - -Similar to held messages, you can dispose of held subscription and -unsubscription requests by POSTing back to the request's resource. The POST -data requires an action of one of the following: - - * discard - throw the request away. - * reject - the request is denied and a notification is sent to the email - address requesting the membership change. - * defer - defer any action on this membership change (continue to hold it). - * accept - accept the membership change. - -Anne's subscription request is accepted. - - >>> dump_json('http://localhost:9001/3.0/lists/' - ... 'ant@example.com/requests/{}'.format(sub_req_id), - ... {'action': 'accept'}) - content-length: 0 - date: ... - server: ... - status: 204 - -Anne is now a member of the mailing list. - - >>> transaction.abort() - >>> ant.members.get_member('anne@example.com') - on ant@example.com - as MemberRole.member> - >>> transaction.abort() - -Bart's unsubscription request is discarded. - - >>> dump_json('http://localhost:9001/3.0/lists/' - ... 'ant@example.com/requests/{}'.format(unsub_req_id), - ... {'action': 'discard'}) - content-length: 0 - date: ... - server: ... - status: 204 - -Bart is still a member of the mailing list. - - >>> transaction.abort() - >>> print(ant.members.get_member('bart@example.com')) - on ant@example.com - as MemberRole.member> - >>> transaction.abort() - -There are no more membership change requests. - - >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/requests') - http_etag: "..." - start: 0 - total_size: 0 diff --git a/src/mailman/rest/docs/post-moderation.rst b/src/mailman/rest/docs/post-moderation.rst new file mode 100644 index 000000000..f082de157 --- /dev/null +++ b/src/mailman/rest/docs/post-moderation.rst @@ -0,0 +1,192 @@ +=============== +Post Moderation +=============== + +Messages which are held for approval can be accepted, rejected, discarded, or +deferred by the list moderators. + + +Viewing the list of held messages +================================= + +Held messages can be moderated through the REST API. A mailing list starts +with no held messages. + + >>> ant = create_list('ant@example.com') + >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/held') + http_etag: "..." + start: 0 + total_size: 0 + +When a message gets held for moderator approval, it shows up in this list. +:: + + >>> msg = message_from_string("""\ + ... From: anne@example.com + ... To: ant@example.com + ... Subject: Something + ... Message-ID: + ... + ... Something else. + ... """) + + >>> from mailman.app.moderator import hold_message + >>> request_id = hold_message(ant, msg, {'extra': 7}, 'Because') + >>> transaction.commit() + + >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/held') + entry 0: + extra: 7 + hold_date: 2005-08-01T07:49:23 + http_etag: "..." + message_id: + msg: From: anne@example.com + To: ant@example.com + Subject: Something + Message-ID: + X-Message-ID-Hash: GCSMSG43GYWWVUMO6F7FBUSSPNXQCJ6M + + Something else. + + reason: Because + request_id: 1 + sender: anne@example.com + subject: Something + http_etag: "..." + start: 0 + total_size: 1 + +You can get an individual held message by providing the *request id* for that +message. This will include the text of the message. +:: + + >>> def url(request_id): + ... return ('http://localhost:9001/3.0/lists/' + ... 'ant@example.com/held/{0}'.format(request_id)) + + >>> dump_json(url(request_id)) + extra: 7 + hold_date: 2005-08-01T07:49:23 + http_etag: "..." + message_id: + msg: From: anne@example.com + To: ant@example.com + Subject: Something + Message-ID: + X-Message-ID-Hash: GCSMSG43GYWWVUMO6F7FBUSSPNXQCJ6M + + Something else. + + reason: Because + request_id: 1 + sender: anne@example.com + subject: Something + + +Disposing of held messages +========================== + +Individual messages can be moderated through the API by POSTing back to the +held message's resource. The POST data requires an action of one of the +following: + + * discard - throw the message away. + * reject - bounces the message back to the original author. + * defer - defer any action on the message (continue to hold it) + * accept - accept the message for posting. + +Let's see what happens when the above message is deferred. + + >>> dump_json(url(request_id), { + ... 'action': 'defer', + ... }) + content-length: 0 + date: ... + server: ... + status: 204 + +The message is still in the moderation queue. + + >>> dump_json(url(request_id)) + extra: 7 + hold_date: 2005-08-01T07:49:23 + http_etag: "..." + message_id: + msg: From: anne@example.com + To: ant@example.com + Subject: Something + Message-ID: + X-Message-ID-Hash: GCSMSG43GYWWVUMO6F7FBUSSPNXQCJ6M + + Something else. + + reason: Because + request_id: 1 + sender: anne@example.com + subject: Something + +The held message can be discarded. + + >>> dump_json(url(request_id), { + ... 'action': 'discard', + ... }) + content-length: 0 + date: ... + server: ... + status: 204 + +Messages can also be accepted via the REST API. Let's hold a new message for +moderation. +:: + + >>> del msg['message-id'] + >>> msg['Message-ID'] = '' + >>> request_id = hold_message(ant, msg) + >>> transaction.commit() + + >>> results = call_http(url(request_id)) + >>> print(results['message_id']) + + + >>> dump_json(url(request_id), { + ... 'action': 'accept', + ... }) + content-length: 0 + date: ... + server: ... + status: 204 + + >>> from mailman.testing.helpers import get_queue_messages + >>> messages = get_queue_messages('pipeline') + >>> len(messages) + 1 + >>> print(messages[0].msg['message-id']) + + +Messages can be rejected via the REST API too. These bounce the message back +to the original author. +:: + + >>> del msg['message-id'] + >>> msg['Message-ID'] = '' + >>> request_id = hold_message(ant, msg) + >>> transaction.commit() + + >>> results = call_http(url(request_id)) + >>> print(results['message_id']) + + + >>> dump_json(url(request_id), { + ... 'action': 'reject', + ... }) + content-length: 0 + date: ... + server: ... + status: 204 + + >>> from mailman.testing.helpers import get_queue_messages + >>> messages = get_queue_messages('virgin') + >>> len(messages) + 1 + >>> print(messages[0].msg['subject']) + Request to mailing list "Ant" rejected diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py index f6bc27917..0607102cb 100644 --- a/src/mailman/rest/lists.py +++ b/src/mailman/rest/lists.py @@ -42,7 +42,8 @@ from mailman.rest.helpers import ( CollectionMixin, GetterSetter, NotFound, bad_request, child, created, etag, no_content, not_found, okay, paginate, path_to) from mailman.rest.members import AMember, MemberCollection -from mailman.rest.moderation import HeldMessages, SubscriptionRequests +from mailman.rest.post_moderation import HeldMessages +from mailman.rest.sub_moderation import SubscriptionRequests from mailman.rest.validator import Validator from operator import attrgetter from zope.component import getUtility diff --git a/src/mailman/rest/moderation.py b/src/mailman/rest/moderation.py deleted file mode 100644 index 984e2d34a..000000000 --- a/src/mailman/rest/moderation.py +++ /dev/null @@ -1,252 +0,0 @@ -# Copyright (C) 2012-2015 by the Free Software Foundation, Inc. -# -# This file is part of GNU Mailman. -# -# GNU Mailman is free software: you can redistribute it and/or modify it under -# the terms of the GNU General Public License as published by the Free -# Software Foundation, either version 3 of the License, or (at your option) -# any later version. -# -# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -# more details. -# -# You should have received a copy of the GNU General Public License along with -# GNU Mailman. If not, see . - -"""REST API for Message moderation.""" - -__all__ = [ - 'HeldMessage', - 'HeldMessages', - 'MembershipChangeRequest', - 'SubscriptionRequests', - ] - - -from mailman.app.moderator import ( - handle_message, handle_subscription, handle_unsubscription) -from mailman.interfaces.action import Action -from mailman.interfaces.messages import IMessageStore -from mailman.interfaces.requests import IListRequests, RequestType -from mailman.rest.helpers import ( - CollectionMixin, bad_request, child, etag, no_content, not_found, okay) -from mailman.rest.validator import Validator, enum_validator -from zope.component import getUtility - - -HELD_MESSAGE_REQUESTS = (RequestType.held_message,) -MEMBERSHIP_CHANGE_REQUESTS = (RequestType.subscription, - RequestType.unsubscription) - - - -class _ModerationBase: - """Common base class.""" - - def _make_resource(self, request_id, expected_request_types): - requests = IListRequests(self._mlist) - results = requests.get_request(request_id) - if results is None: - return None - key, data = results - resource = dict(key=key, request_id=request_id) - # Flatten the IRequest payload into the JSON representation. - resource.update(data) - # Check for a matching request type, and insert the type name into the - # resource. - request_type = RequestType[resource.pop('_request_type')] - if request_type not in expected_request_types: - return None - resource['type'] = request_type.name - # This key isn't what you think it is. Usually, it's the Pendable - # record's row id, which isn't helpful at all. If it's not there, - # that's fine too. - resource.pop('id', None) - return resource - - - -class _HeldMessageBase(_ModerationBase): - """Held messages are a little different.""" - - def _make_resource(self, request_id): - resource = super(_HeldMessageBase, self)._make_resource( - request_id, HELD_MESSAGE_REQUESTS) - if resource is None: - return None - # Grab the message and insert its text representation into the - # resource. XXX See LP: #967954 - key = resource.pop('key') - msg = getUtility(IMessageStore).get_message_by_id(key) - resource['msg'] = msg.as_string() - # Some of the _mod_* keys we want to rename and place into the JSON - # resource. Others we can drop. Since we're mutating the dictionary, - # we need to make a copy of the keys. When you port this to Python 3, - # you'll need to list()-ify the .keys() dictionary view. - for key in list(resource): - if key in ('_mod_subject', '_mod_hold_date', '_mod_reason', - '_mod_sender', '_mod_message_id'): - resource[key[5:]] = resource.pop(key) - elif key.startswith('_mod_'): - del resource[key] - # Also, held message resources will always be this type, so ignore - # this key value. - del resource['type'] - return resource - - -class HeldMessage(_HeldMessageBase): - """Resource for moderating a held message.""" - - def __init__(self, mlist, request_id): - self._mlist = mlist - self._request_id = request_id - - def on_get(self, request, response): - try: - request_id = int(self._request_id) - except ValueError: - bad_request(response) - return - resource = self._make_resource(request_id) - if resource is None: - not_found(response) - else: - okay(response, etag(resource)) - - def on_post(self, request, response): - try: - validator = Validator(action=enum_validator(Action)) - arguments = validator(request) - except ValueError as error: - bad_request(response, str(error)) - return - requests = IListRequests(self._mlist) - try: - request_id = int(self._request_id) - except ValueError: - bad_request(response) - return - results = requests.get_request(request_id, RequestType.held_message) - if results is None: - not_found(response) - else: - handle_message(self._mlist, request_id, **arguments) - no_content(response) - - - -class HeldMessages(_HeldMessageBase, CollectionMixin): - """Resource for messages held for moderation.""" - - def __init__(self, mlist): - self._mlist = mlist - self._requests = None - - def _resource_as_dict(self, request): - """See `CollectionMixin`.""" - return self._make_resource(request.id) - - def _get_collection(self, request): - requests = IListRequests(self._mlist) - self._requests = requests - return list(requests.of_type(RequestType.held_message)) - - def on_get(self, request, response): - """/lists/listname/held""" - resource = self._make_collection(request) - okay(response, etag(resource)) - - @child(r'^(?P[^/]+)') - def message(self, request, segments, **kw): - return HeldMessage(self._mlist, kw['id']) - - - -class MembershipChangeRequest(_ModerationBase): - """Resource for moderating a membership change.""" - - def __init__(self, mlist, request_id): - self._mlist = mlist - self._request_id = request_id - - def on_get(self, request, response): - try: - request_id = int(self._request_id) - except ValueError: - bad_request(response) - return - resource = self._make_resource(request_id, MEMBERSHIP_CHANGE_REQUESTS) - if resource is None: - not_found(response) - else: - # Remove unnecessary keys. - del resource['key'] - okay(response, etag(resource)) - - def on_post(self, request, response): - try: - validator = Validator(action=enum_validator(Action)) - arguments = validator(request) - except ValueError as error: - bad_request(response, str(error)) - return - requests = IListRequests(self._mlist) - try: - request_id = int(self._request_id) - except ValueError: - bad_request(response) - return - results = requests.get_request(request_id) - if results is None: - not_found(response) - return - key, data = results - try: - request_type = RequestType[data['_request_type']] - except ValueError: - bad_request(response) - return - if request_type is RequestType.subscription: - handle_subscription(self._mlist, request_id, **arguments) - elif request_type is RequestType.unsubscription: - handle_unsubscription(self._mlist, request_id, **arguments) - else: - bad_request(response) - return - no_content(response) - - -class SubscriptionRequests(_ModerationBase, CollectionMixin): - """Resource for membership change requests.""" - - def __init__(self, mlist): - self._mlist = mlist - self._requests = None - - def _resource_as_dict(self, request): - """See `CollectionMixin`.""" - resource = self._make_resource(request.id, MEMBERSHIP_CHANGE_REQUESTS) - # Remove unnecessary keys. - del resource['key'] - return resource - - def _get_collection(self, request): - requests = IListRequests(self._mlist) - self._requests = requests - items = [] - for request_type in MEMBERSHIP_CHANGE_REQUESTS: - for request in requests.of_type(request_type): - items.append(request) - return items - - def on_get(self, request, response): - """/lists/listname/requests""" - resource = self._make_collection(request) - okay(response, etag(resource)) - - @child(r'^(?P[^/]+)') - def subscription(self, request, segments, **kw): - return MembershipChangeRequest(self._mlist, kw['id']) diff --git a/src/mailman/rest/post_moderation.py b/src/mailman/rest/post_moderation.py new file mode 100644 index 000000000..6156fa39f --- /dev/null +++ b/src/mailman/rest/post_moderation.py @@ -0,0 +1,155 @@ +# Copyright (C) 2012-2015 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see . + +"""REST API for held message moderation.""" + +__all__ = [ + 'HeldMessage', + 'HeldMessages', + ] + + +from mailman.app.moderator import handle_message +from mailman.interfaces.action import Action +from mailman.interfaces.messages import IMessageStore +from mailman.interfaces.requests import IListRequests, RequestType +from mailman.rest.helpers import ( + CollectionMixin, bad_request, child, etag, no_content, not_found, okay) +from mailman.rest.validator import Validator, enum_validator +from zope.component import getUtility + + + +class _ModerationBase: + """Common base class.""" + + def _make_resource(self, request_id): + requests = IListRequests(self._mlist) + results = requests.get_request(request_id) + if results is None: + return None + key, data = results + resource = dict(key=key, request_id=request_id) + # Flatten the IRequest payload into the JSON representation. + resource.update(data) + # Check for a matching request type, and insert the type name into the + # resource. + request_type = RequestType[resource.pop('_request_type')] + if request_type is not RequestType.held_message: + return None + resource['type'] = RequestType.held_message.name + # This key isn't what you think it is. Usually, it's the Pendable + # record's row id, which isn't helpful at all. If it's not there, + # that's fine too. + resource.pop('id', None) + return resource + + + +class _HeldMessageBase(_ModerationBase): + """Held messages are a little different.""" + + def _make_resource(self, request_id): + resource = super(_HeldMessageBase, self)._make_resource(request_id) + if resource is None: + return None + # Grab the message and insert its text representation into the + # resource. XXX See LP: #967954 + key = resource.pop('key') + msg = getUtility(IMessageStore).get_message_by_id(key) + resource['msg'] = msg.as_string() + # Some of the _mod_* keys we want to rename and place into the JSON + # resource. Others we can drop. Since we're mutating the dictionary, + # we need to make a copy of the keys. When you port this to Python 3, + # you'll need to list()-ify the .keys() dictionary view. + for key in list(resource): + if key in ('_mod_subject', '_mod_hold_date', '_mod_reason', + '_mod_sender', '_mod_message_id'): + resource[key[5:]] = resource.pop(key) + elif key.startswith('_mod_'): + del resource[key] + # Also, held message resources will always be this type, so ignore + # this key value. + del resource['type'] + return resource + + +class HeldMessage(_HeldMessageBase): + """Resource for moderating a held message.""" + + def __init__(self, mlist, request_id): + self._mlist = mlist + self._request_id = request_id + + def on_get(self, request, response): + try: + request_id = int(self._request_id) + except ValueError: + bad_request(response) + return + resource = self._make_resource(request_id) + if resource is None: + not_found(response) + else: + okay(response, etag(resource)) + + def on_post(self, request, response): + try: + validator = Validator(action=enum_validator(Action)) + arguments = validator(request) + except ValueError as error: + bad_request(response, str(error)) + return + requests = IListRequests(self._mlist) + try: + request_id = int(self._request_id) + except ValueError: + bad_request(response) + return + results = requests.get_request(request_id, RequestType.held_message) + if results is None: + not_found(response) + else: + handle_message(self._mlist, request_id, **arguments) + no_content(response) + + + +class HeldMessages(_HeldMessageBase, CollectionMixin): + """Resource for messages held for moderation.""" + + def __init__(self, mlist): + self._mlist = mlist + self._requests = None + + def _resource_as_dict(self, request): + """See `CollectionMixin`.""" + return self._make_resource(request.id) + + def _get_collection(self, request): + requests = IListRequests(self._mlist) + self._requests = requests + return list(requests.of_type(RequestType.held_message)) + + def on_get(self, request, response): + """/lists/listname/held""" + resource = self._make_collection(request) + okay(response, etag(resource)) + + @child(r'^(?P[^/]+)') + def message(self, request, segments, **kw): + return HeldMessage(self._mlist, kw['id']) diff --git a/src/mailman/rest/tests/test_moderation.py b/src/mailman/rest/tests/test_moderation.py index c77ae2aca..ab7383251 100644 --- a/src/mailman/rest/tests/test_moderation.py +++ b/src/mailman/rest/tests/test_moderation.py @@ -18,26 +18,28 @@ """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.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 +73,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'], '') - 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 +83,180 @@ 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 + + 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. + token_1, token_owner, member = self._registrar.register(self._anne) + # Anne's subscription request got held. + self.assertIsNone(member) + token_2, token_owner, member = self._registrar.register(self._bart) + 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) + import pdb; pdb.set_trace() + + def test_accept(self): + # POST to the request to accept it. + 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/{}' + with self.assertRaises(HTTPError) as cm: + call_api(url.format(token), dict( + action='accept', + )) + self.assertEqual(cm.exception.code, 204) + # Anne is a member. + self.assertEqual( + self._mlist.get_members('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_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. + # POST to the request to discard it. + 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/{}' 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, 204) + # Anne is not a member. + self.assertIsNone(self._mlist.get_members('anne@example.com')) + # The request URL no longer exists. + with self.assertRaises(HTTPError) as cm: + 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. + 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/{}' + with self.assertRaises(HTTPError) as cm: + call_api(url.format(token), dict( + action='defer', + )) + self.assertEqual(cm.exception.code, 204) + # Anne is not a member. + self.assertIsNone(self._mlist.get_members('anne@example.com')) + # The request URL still exists. + with self.assertRaises(HTTPError) as cm: + call_api(url.format(token), dict( + action='defer', + )) + self.assertEqual(cm.exception.code, 204) + # And now we can accept it. + with self.assertRaises(HTTPError) as cm: + call_api(url.format(token), dict( + action='accept', + )) + self.assertEqual(cm.exception.code, 204) + # Anne is a member. + self.assertEqual( + self._mlist.get_members('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_reject(self): + # POST to the request to reject it. This leaves a bounce message in + # the virgin queue. + 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) + url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}' + with self.assertRaises(HTTPError) as cm: + call_api(url.format(token), dict( + action='reject', + )) + self.assertEqual(cm.exception.code, 204) + # Anne is not a member. + self.assertIsNone(self._mlist.get_members('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 is now in the virgin queue. + items = get_queue_messages('virgin') + self.assertEqual(len(items), 1) + self.assertEqual(str(items[0].msg), '') -- cgit v1.2.3-70-g09d2 From 63f0f56219875f1001f8b9eb4572436b46bea30d Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Thu, 16 Apr 2015 15:28:16 -0400 Subject: Checkpointing. --- src/mailman/rest/docs/sub-moderation.rst | 113 ++++++++++++++++++++++ src/mailman/rest/sub_moderation.py | 155 +++++++++++++++++++++++++++++++ 2 files changed, 268 insertions(+) create mode 100644 src/mailman/rest/docs/sub-moderation.rst create mode 100644 src/mailman/rest/sub_moderation.py (limited to 'src') diff --git a/src/mailman/rest/docs/sub-moderation.rst b/src/mailman/rest/docs/sub-moderation.rst new file mode 100644 index 000000000..6d3d4993c --- /dev/null +++ b/src/mailman/rest/docs/sub-moderation.rst @@ -0,0 +1,113 @@ +========================= + Subscription moderation +========================= + +Subscription (and sometimes unsubscription) requests can similarly be +accepted, discarded, rejected, or deferred by the list moderators. + + +Viewing subscription requests +============================= + +A mailing list starts with no pending subscription or unsubscription requests. + + >>> ant = create_list('ant@example.com') + >>> ant.admin_immed_notify = False + >>> from mailman.interfaces.mailinglist import SubscriptionPolicy + >>> ant.subscription_policy = SubscriptionPolicy.moderate + >>> transaction.commit() + >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/requests') + http_etag: "..." + start: 0 + total_size: 0 + +When Anne tries to subscribe to the Ant list, her subscription is held for +moderator approval. + + >>> from mailman.interfaces.registrar import IRegistrar + >>> from mailman.interfaces.usermanager import IUserManager + >>> from zope.component import getUtility + >>> registrar = IRegistrar(ant) + >>> manager = getUtility(IUserManager) + >>> anne = manager.create_address('anne@example.com', 'Anne Person') + >>> token, token_owner, member = registrar.register( + ... anne, pre_verified=True, pre_confirmed=True) + >>> print(member) + None + +The message is being held for moderator approval. + + >>> token_owner.name + TokenOwner.moderator + +The subscription request can be viewed in the REST API. + + >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/requests') + entry 0: + delivery_mode: regular + display_name: Anne Person + email: anne@example.com + http_etag: "..." + language: en + request_id: ... + type: subscription + when: 2005-08-01T07:49:23 + http_etag: "..." + start: 0 + total_size: 1 + + +Viewing individual requests +=========================== + +You can view an individual membership change request by providing the token +(a.k.a. request id). Anne's subscription request looks like this. + + >>> 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 + when: 2005-08-01T07:49:23 + + +Disposing of subscription requests +================================== + +Moderators can dispose of held subscription requests by POSTing back to the +request's resource. The POST data requires an action of one of the following: + + * discard - throw the request away. + * reject - the request is denied and a notification is sent to the email + address requesting the membership change. + * defer - defer any action on this membership change (continue to hold it). + * accept - accept the membership change. + +Anne's subscription request is accepted. + + >>> dump_json('http://localhost:9001/3.0/lists/' + ... 'ant@example.com/requests/{}'.format(token), + ... {'action': 'accept'}) + content-length: 0 + date: ... + server: ... + status: 204 + +Anne is now a member of the mailing list. + + >>> transaction.abort() + >>> ant.members.get_member('anne@example.com') + on ant@example.com + as MemberRole.member> + >>> transaction.abort() + +There are no more membership change requests. + + >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/requests') + http_etag: "..." + start: 0 + total_size: 0 diff --git a/src/mailman/rest/sub_moderation.py b/src/mailman/rest/sub_moderation.py new file mode 100644 index 000000000..4f8cf2e8e --- /dev/null +++ b/src/mailman/rest/sub_moderation.py @@ -0,0 +1,155 @@ +# Copyright (C) 2012-2015 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see . + +"""REST API for held subscription requests.""" + +__all__ = [ + 'SubscriptionRequests', + ] + + +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 zope.component import getUtility + + + +class _ModerationBase: + """Common base class.""" + + def _make_resource(self, token): + requests = IListRequests(self._mlist) + results = requests.get_request(request_id) + if results is None: + return None + key, data = results + resource = dict(key=key, request_id=request_id) + # Flatten the IRequest payload into the JSON representation. + resource.update(data) + # Check for a matching request type, and insert the type name into the + # resource. + request_type = RequestType[resource.pop('_request_type')] + if request_type not in expected_request_types: + return None + resource['type'] = request_type.name + # This key isn't what you think it is. Usually, it's the Pendable + # record's row id, which isn't helpful at all. If it's not there, + # that's fine too. + resource.pop('id', None) + return resource + + + +class IndividualRequest(_ModerationBase): + """Resource for moderating a membership change.""" + + def __init__(self, mlist, token): + self._mlist = mlist + self._registrar = IRegistrar(self._mlist) + self._token = token + + def on_get(self, request, response): + try: + token, token_owner, member = self._registrar.confirm(self._token) + except LookupError: + not_found(response) + return + try: + request_id = int(self._request_id) + except ValueError: + bad_request(response) + return + resource = self._make_resource(request_id, MEMBERSHIP_CHANGE_REQUESTS) + if resource is None: + not_found(response) + else: + # Remove unnecessary keys. + del resource['key'] + okay(response, etag(resource)) + + def on_post(self, request, response): + try: + validator = Validator(action=enum_validator(Action)) + arguments = validator(request) + except ValueError as error: + bad_request(response, str(error)) + return + requests = IListRequests(self._mlist) + try: + request_id = int(self._request_id) + except ValueError: + bad_request(response) + return + results = requests.get_request(request_id) + if results is None: + not_found(response) + return + key, data = results + try: + request_type = RequestType[data['_request_type']] + except ValueError: + bad_request(response) + return + if request_type is RequestType.subscription: + handle_subscription(self._mlist, request_id, **arguments) + elif request_type is RequestType.unsubscription: + handle_unsubscription(self._mlist, request_id, **arguments) + else: + bad_request(response) + return + no_content(response) + + +class SubscriptionRequests(_ModerationBase, CollectionMixin): + """Resource for membership change requests.""" + + def __init__(self, mlist): + self._mlist = mlist + + def _resource_as_dict(self, request): + """See `CollectionMixin`.""" + resource = self._make_resource(request.id, MEMBERSHIP_CHANGE_REQUESTS) + # Remove unnecessary keys. + del resource['key'] + return resource + + def _get_collection(self, request): + # There's currently no better way to query the pendings database for + # all the entries that are associated with subscription holds on this + # mailing list. Brute force for now. + items = [] + for token, pendable in getUtility(IPendings): + token_owner = pendable.get('token_owner') + if token_owner is None: + continue + item = dict(token=token) + item.update(pendable) + items.append(item) + return items + + def on_get(self, request, response): + """/lists/listname/requests""" + resource = self._make_collection(request) + okay(response, etag(resource)) + + @child(r'^(?P[^/]+)') + def subscription(self, request, segments, **kw): + return IndividualRequest(self._mlist, kw['token']) -- cgit v1.2.3-70-g09d2 From c2d15f3c7c2e86912f62326b51977abca0c9bb5e Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Thu, 16 Apr 2015 15:37:24 -0400 Subject: Updates. --- src/mailman/rest/docs/sub-moderation.rst | 4 +-- src/mailman/rest/sub_moderation.py | 42 ++++++-------------------------- 2 files changed, 10 insertions(+), 36 deletions(-) (limited to 'src') diff --git a/src/mailman/rest/docs/sub-moderation.rst b/src/mailman/rest/docs/sub-moderation.rst index 6d3d4993c..a658ac410 100644 --- a/src/mailman/rest/docs/sub-moderation.rst +++ b/src/mailman/rest/docs/sub-moderation.rst @@ -37,8 +37,8 @@ moderator approval. The message is being held for moderator approval. - >>> token_owner.name - TokenOwner.moderator + >>> print(token_owner.name) + moderator The subscription request can be viewed in the REST API. diff --git a/src/mailman/rest/sub_moderation.py b/src/mailman/rest/sub_moderation.py index 4f8cf2e8e..7bc6554a6 100644 --- a/src/mailman/rest/sub_moderation.py +++ b/src/mailman/rest/sub_moderation.py @@ -35,25 +35,13 @@ from zope.component import getUtility class _ModerationBase: """Common base class.""" + def __init__(self): + self._pendings = getUtility(IPendings) + def _make_resource(self, token): - requests = IListRequests(self._mlist) - results = requests.get_request(request_id) - if results is None: - return None - key, data = results - resource = dict(key=key, request_id=request_id) - # Flatten the IRequest payload into the JSON representation. - resource.update(data) - # Check for a matching request type, and insert the type name into the - # resource. - request_type = RequestType[resource.pop('_request_type')] - if request_type not in expected_request_types: - return None - resource['type'] = request_type.name - # This key isn't what you think it is. Usually, it's the Pendable - # record's row id, which isn't helpful at all. If it's not there, - # that's fine too. - resource.pop('id', None) + pendable = self._pendings.confirm(token, expunge=False) + resource = dict(token=token) + resource.update(pendable) return resource @@ -122,28 +110,14 @@ class SubscriptionRequests(_ModerationBase, CollectionMixin): """Resource for membership change requests.""" def __init__(self, mlist): + super().__init__() self._mlist = mlist - def _resource_as_dict(self, request): - """See `CollectionMixin`.""" - resource = self._make_resource(request.id, MEMBERSHIP_CHANGE_REQUESTS) - # Remove unnecessary keys. - del resource['key'] - return resource - def _get_collection(self, request): # There's currently no better way to query the pendings database for # all the entries that are associated with subscription holds on this # mailing list. Brute force for now. - items = [] - for token, pendable in getUtility(IPendings): - token_owner = pendable.get('token_owner') - if token_owner is None: - continue - item = dict(token=token) - item.update(pendable) - items.append(item) - return items + return [token for token, pendable in getUtility(IPendings)] def on_get(self, request, response): """/lists/listname/requests""" -- cgit v1.2.3-70-g09d2 From 3857294c3dc8fdf294f23179926f062c4e0a6c90 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Fri, 17 Apr 2015 11:30:23 -0400 Subject: Check pointing new subscription moderation REST API. --- src/mailman/app/subscriptions.py | 5 +- src/mailman/app/tests/test_subscriptions.py | 5 +- src/mailman/rest/docs/post-moderation.rst | 1 + src/mailman/rest/docs/sub-moderation.rst | 8 +- src/mailman/rest/sub_moderation.py | 89 ++++++++++--------- src/mailman/rest/tests/test_moderation.py | 131 ++++++++++++++++++---------- 6 files changed, 147 insertions(+), 92 deletions(-) (limited to 'src') diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index 46ce549af..8794fb78a 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -168,8 +168,9 @@ class SubscriptionWorkflow(Workflow): return pendable = Pendable( list_id=self.mlist.list_id, - address=self.address.email, - hold_date=now().replace(microsecond=0).isoformat(), + email=self.address.email, + display_name=self.address.display_name, + when=now().replace(microsecond=0).isoformat(), token_owner=token_owner.name, ) self.token = getUtility(IPendings).add(pendable, timedelta(days=3650)) diff --git a/src/mailman/app/tests/test_subscriptions.py b/src/mailman/app/tests/test_subscriptions.py index 6b5d88026..2d5a3733b 100644 --- a/src/mailman/app/tests/test_subscriptions.py +++ b/src/mailman/app/tests/test_subscriptions.py @@ -69,8 +69,9 @@ class TestSubscriptionWorkflow(unittest.TestCase): self.assertIsNotNone(workflow.token) pendable = getUtility(IPendings).confirm(workflow.token, expunge=False) self.assertEqual(pendable['list_id'], 'test.example.com') - self.assertEqual(pendable['address'], 'anne@example.com') - self.assertEqual(pendable['hold_date'], '2005-08-01T07:49:23') + self.assertEqual(pendable['email'], 'anne@example.com') + self.assertEqual(pendable['display_name'], '') + self.assertEqual(pendable['when'], '2005-08-01T07:49:23') self.assertEqual(pendable['token_owner'], 'subscriber') def test_user_or_address_required(self): diff --git a/src/mailman/rest/docs/post-moderation.rst b/src/mailman/rest/docs/post-moderation.rst index f082de157..6dd96e71a 100644 --- a/src/mailman/rest/docs/post-moderation.rst +++ b/src/mailman/rest/docs/post-moderation.rst @@ -13,6 +13,7 @@ Held messages can be moderated through the REST API. A mailing list starts with no held messages. >>> ant = create_list('ant@example.com') + >>> transaction.commit() >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/held') http_etag: "..." start: 0 diff --git a/src/mailman/rest/docs/sub-moderation.rst b/src/mailman/rest/docs/sub-moderation.rst index a658ac410..0468d75b5 100644 --- a/src/mailman/rest/docs/sub-moderation.rst +++ b/src/mailman/rest/docs/sub-moderation.rst @@ -42,15 +42,15 @@ The message is being held for moderator approval. The subscription request can be viewed in the REST API. + >>> transaction.commit() >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/requests') entry 0: - 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 http_etag: "..." start: 0 diff --git a/src/mailman/rest/sub_moderation.py b/src/mailman/rest/sub_moderation.py index 7bc6554a6..12bd8ab26 100644 --- a/src/mailman/rest/sub_moderation.py +++ b/src/mailman/rest/sub_moderation.py @@ -38,8 +38,11 @@ class _ModerationBase: def __init__(self): self._pendings = getUtility(IPendings) - def _make_resource(self, token): + def _resource_as_dict(self, token): pendable = self._pendings.confirm(token, expunge=False) + if pendable is None: + # This token isn't in the database. + raise LookupError resource = dict(token=token) resource.update(pendable) return resource @@ -50,28 +53,20 @@ class IndividualRequest(_ModerationBase): """Resource for moderating a membership change.""" def __init__(self, mlist, token): + super().__init__() self._mlist = mlist self._registrar = IRegistrar(self._mlist) self._token = token def on_get(self, request, response): + # Get the pended record associated with this token, if it exists in + # the pending table. try: - token, token_owner, member = self._registrar.confirm(self._token) + resource = self._resource_as_dict(self._token) except LookupError: not_found(response) return - try: - request_id = int(self._request_id) - except ValueError: - bad_request(response) - return - resource = self._make_resource(request_id, MEMBERSHIP_CHANGE_REQUESTS) - if resource is None: - not_found(response) - else: - # Remove unnecessary keys. - del resource['key'] - okay(response, etag(resource)) + okay(response, etag(resource)) def on_post(self, request, response): try: @@ -80,32 +75,34 @@ class IndividualRequest(_ModerationBase): except ValueError as error: bad_request(response, str(error)) return - requests = IListRequests(self._mlist) - try: - request_id = int(self._request_id) - except ValueError: - bad_request(response) - return - results = requests.get_request(request_id) - if results is None: - not_found(response) - return - key, data = results - try: - request_type = RequestType[data['_request_type']] - except ValueError: - bad_request(response) - return - if request_type is RequestType.subscription: - handle_subscription(self._mlist, request_id, **arguments) - elif request_type is RequestType.unsubscription: - handle_unsubscription(self._mlist, request_id, **arguments) - else: - bad_request(response) - return - no_content(response) + action = arguments['action'] + if action is Action.defer: + # At least see if the token is in the database. + pendable = self._pendings.confirm(self._token, expunge=False) + if pendable is None: + not_found(response) + else: + no_content(response) + elif action is Action.accept: + try: + self._registrar.confirm(self._token) + except LookupError: + not_found(response) + else: + no_content(response) + elif action is Action.discard: + # At least see if the token is in the database. + pendable = self._pendings.confirm(self._token, expunge=True) + if pendable is None: + not_found(response) + else: + no_content(response) + elif action is Action.reject: + # XXX + no_content(response) + class SubscriptionRequests(_ModerationBase, CollectionMixin): """Resource for membership change requests.""" @@ -116,8 +113,20 @@ class SubscriptionRequests(_ModerationBase, CollectionMixin): def _get_collection(self, request): # There's currently no better way to query the pendings database for # all the entries that are associated with subscription holds on this - # mailing list. Brute force for now. - return [token for token, pendable in getUtility(IPendings)] + # mailing list. Brute force iterating over all the pendables. + collection = [] + for token, pendable in getUtility(IPendings): + if 'token_owner' not in pendable: + # This isn't a subscription hold. + continue + list_id = pendable.get('list_id') + if list_id != self._mlist.list_id: + # Either there isn't a list_id field, in which case it can't + # be a subscription hold, or this is a hold for some other + # mailing list. + continue + collection.append(token) + return collection def on_get(self, request, response): """/lists/listname/requests""" diff --git a/src/mailman/rest/tests/test_moderation.py b/src/mailman/rest/tests/test_moderation.py index ab7383251..cc1f67f7e 100644 --- a/src/mailman/rest/tests/test_moderation.py +++ b/src/mailman/rest/tests/test_moderation.py @@ -145,30 +145,51 @@ class TestSubscriptionModeration(unittest.TestCase): def test_list_held_requests(self): # We can view all the held requests. - token_1, token_owner, member = self._registrar.register(self._anne) - # Anne's subscription request got held. - self.assertIsNone(member) - token_2, token_owner, member = self._registrar.register(self._bart) + 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) - import pdb; pdb.set_trace() + 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. - token, token_owner, member = self._registrar.register(self._anne) + 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/{}' - with self.assertRaises(HTTPError) as cm: - call_api(url.format(token), dict( - action='accept', - )) - self.assertEqual(cm.exception.code, 204) + content, response = call_api(url.format(token), dict( + action='accept', + )) + self.assertEqual(response.status, 204) # Anne is a member. self.assertEqual( - self._mlist.get_members('anne@example.com').address, + self._mlist.members.get_member('anne@example.com').address, self._anne) # The request URL no longer exists. with self.assertRaises(HTTPError) as cm: @@ -177,19 +198,27 @@ class TestSubscriptionModeration(unittest.TestCase): )) 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_discard(self): # POST to the request to discard it. - token, token_owner, member = self._registrar.register(self._anne) + 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/{}' - with self.assertRaises(HTTPError) as cm: - call_api(url.format(token), dict( - action='discard', - )) - self.assertEqual(cm.exception.code, 204) + content, response = call_api(url.format(token), dict( + action='discard', + )) + self.assertEqual(response.status, 204) # Anne is not a member. - self.assertIsNone(self._mlist.get_members('anne@example.com')) + 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( @@ -199,32 +228,30 @@ class TestSubscriptionModeration(unittest.TestCase): def test_defer(self): # Defer the decision for some other moderator. - token, token_owner, member = self._registrar.register(self._anne) + 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/{}' - with self.assertRaises(HTTPError) as cm: - call_api(url.format(token), dict( - action='defer', - )) - self.assertEqual(cm.exception.code, 204) + content, response = call_api(url.format(token), dict( + action='defer', + )) + self.assertEqual(response.status, 204) # Anne is not a member. - self.assertIsNone(self._mlist.get_members('anne@example.com')) + self.assertIsNone(self._mlist.members.get_member('anne@example.com')) # The request URL still exists. - with self.assertRaises(HTTPError) as cm: - call_api(url.format(token), dict( - action='defer', - )) - self.assertEqual(cm.exception.code, 204) + content, response = call_api(url.format(token), dict( + action='defer', + )) + self.assertEqual(response.status, 204) # And now we can accept it. - with self.assertRaises(HTTPError) as cm: - call_api(url.format(token), dict( - action='accept', - )) - self.assertEqual(cm.exception.code, 204) + content, response = call_api(url.format(token), dict( + action='accept', + )) + self.assertEqual(response.status, 204) # Anne is a member. self.assertEqual( - self._mlist.get_members('anne@example.com').address, + self._mlist.members.get_member('anne@example.com').address, self._anne) # The request URL no longer exists. with self.assertRaises(HTTPError) as cm: @@ -233,23 +260,31 @@ class TestSubscriptionModeration(unittest.TestCase): )) 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. - token, token_owner, member = self._registrar.register(self._anne) + with transaction(): + 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) url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}' - with self.assertRaises(HTTPError) as cm: - call_api(url.format(token), dict( - action='reject', - )) - self.assertEqual(cm.exception.code, 204) + content, response = call_api(url.format(token), dict( + action='reject', + )) + self.assertEqual(response.status, 204) # Anne is not a member. - self.assertIsNone(self._mlist.get_members('anne@example.com')) + 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( @@ -260,3 +295,11 @@ class TestSubscriptionModeration(unittest.TestCase): items = get_queue_messages('virgin') self.assertEqual(len(items), 1) self.assertEqual(str(items[0].msg), '') + + 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) -- cgit v1.2.3-70-g09d2 From b37f33558b54c298916f70ff274dd06d18d3e38a Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Fri, 17 Apr 2015 14:12:00 -0400 Subject: Branch his ready. --- src/mailman/app/moderator.py | 22 ++++++++------ src/mailman/app/subscriptions.py | 10 +++++-- src/mailman/app/tests/test_registrar.py | 2 +- src/mailman/rest/docs/sub-moderation.rst | 9 ++---- src/mailman/rest/sub_moderation.py | 14 +++++++-- src/mailman/rest/tests/test_moderation.py | 48 +++++++++++++++++++++++++++---- 6 files changed, 79 insertions(+), 26 deletions(-) (limited to 'src') 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') 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. -- cgit v1.2.3-70-g09d2