diff options
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 3 | ||||
| -rw-r--r-- | src/mailman/model/subscriptions.py | 3 | ||||
| -rw-r--r-- | src/mailman/model/tests/test_subscriptions.py | 32 | ||||
| -rw-r--r-- | src/mailman/rest/docs/membership.rst | 152 | ||||
| -rw-r--r-- | src/mailman/rest/lists.py | 20 | ||||
| -rw-r--r-- | src/mailman/rest/tests/test_lists.py | 125 |
6 files changed, 223 insertions, 112 deletions
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index eaed35312..e2e3f17e1 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -187,6 +187,9 @@ REST With thanks to Anirudh Dahiya. (Closes #199) * Expose the system pipelines and chains via ``<api>/system/pipelines`` and ``<api>/system/chains`` respectively. Given by Simon Hanna. (Closes #66) + * Support mass unsubscription of members via ``DELETE`` on the + ``<api>/lists/<list-id>/roster/member`` resource. Given by Harshit + Bansal. (Closes #171) * Port to Falcon 1.0 (Closes #20) Other diff --git a/src/mailman/model/subscriptions.py b/src/mailman/model/subscriptions.py index e399e3285..d3fdc3a8a 100644 --- a/src/mailman/model/subscriptions.py +++ b/src/mailman/model/subscriptions.py @@ -161,7 +161,8 @@ class SubscriptionService: q_member = store.query(Member).filter( Member.list_id == list_id, Member.role == MemberRole.member) - for email in emails: + # De-duplicate. + for email in set(emails): unsubscribed = False # Join with a queries matching the email address and preferred # address of any subscribed user. diff --git a/src/mailman/model/tests/test_subscriptions.py b/src/mailman/model/tests/test_subscriptions.py index 49dd03625..b0a68e3f9 100644 --- a/src/mailman/model/tests/test_subscriptions.py +++ b/src/mailman/model/tests/test_subscriptions.py @@ -397,6 +397,38 @@ class TestSubscriptionService(unittest.TestCase): [address.email for address in bee_owners.addresses], ['anne_1@example.com']) + def test_unsubscribe_members_with_duplicates(self): + ant = create_list('ant@example.com') + ant.admin_immed_notify = False + anne = self._user_manager.create_user('anne@example.com') + set_preferred(anne) + ant.subscribe(anne, MemberRole.member) + # Now we try to unsubscribe Anne twice in the same call. That's okay + # because duplicates are ignored. + success, fail = self._service.unsubscribe_members( + ant.list_id, [ + 'anne@example.com', + 'anne@example.com', + ]) + self.assertEqual(success, set(['anne@example.com'])) + self.assertEqual(fail, set()) + + def test_unsubscribe_members_with_duplicate_failures(self): + ant = create_list('ant@example.com') + ant.admin_immed_notify = False + anne = self._user_manager.create_user('anne@example.com') + set_preferred(anne) + ant.subscribe(anne, MemberRole.member) + # Now we try to unsubscribe a nonmember twice in the same call. + # That's okay because duplicates are ignored. + success, fail = self._service.unsubscribe_members( + ant.list_id, [ + 'bart@example.com', + 'bart@example.com', + ]) + self.assertEqual(success, set()) + self.assertEqual(fail, set(['bart@example.com'])) + def test_find_members_issue_227(self): # A user is subscribed to a list with their preferred address. They # have a different secondary linked address which is not subscribed. diff --git a/src/mailman/rest/docs/membership.rst b/src/mailman/rest/docs/membership.rst index 0a5fb34d9..e19fa4623 100644 --- a/src/mailman/rest/docs/membership.rst +++ b/src/mailman/rest/docs/membership.rst @@ -771,53 +771,6 @@ Elly is no longer a member of the mailing list. set() -Mass Unsubscriptions -==================== -A batch of users can be unsubscribed from the mailing list via the REST API -just by supplying their email addresses. - - >>> m_list = create_list('mytest@example.com') - >>> subscribe(m_list, 'Joe') - <Member: Joe Person <jperson@example.com> on mytest@example.com as MemberRole.member> - >>> subscribe(m_list, 'Ken') - <Member: Ken Person <kperson@example.com> on mytest@example.com as MemberRole.member> - >>> subscribe(m_list, 'Mat') - <Member: Mat Person <mperson@example.com> on mytest@example.com as MemberRole.member> - >>> dump_json('http://localhost:9001/3.0/lists/mytest.example.com' - ... '/roster/member', {'emails': ['kperson@example.com', - ... 'mperson@example.com', - ... 'kperson@example.com', - ... 'tim@example.com']}, 'DELETE') - http_etag: "..." - kperson@example.com: Member already deleted. - tim@example.com: No such member. - >>> dump_json('http://localhost:9001/3.0/lists/mytest.example.com' - ... '/roster/member') - entry 0: - address: http://localhost:9001/3.0/addresses/jperson@example.com - delivery_mode: regular - email: jperson@example.com - http_etag: "..." - list_id: mytest.example.com - member_id: 10 - role: member - self_link: http://localhost:9001/3.0/members/10 - user: http://localhost:9001/3.0/users/7 - ... - total_size: 1 - -Ken and Mat have been unsubscribed successfully while tim@example.com can't -be unsubscribed since he was not a member of the list. Similarly Joe can be -unsubscribed from the mailing list. - - >>> dump_json('http://localhost:9001/3.0/lists/mytest.example.com' - ... '/roster/member', {'emails': ['jperson@example.com']}, 'DELETE') - content-length: 0 - date: ... - server: ... - status: 204 - - Changing delivery address ========================= @@ -856,10 +809,10 @@ addresses. email: herb@example.com http_etag: "..." list_id: ant.example.com - member_id: 13 + member_id: 10 role: member - self_link: http://localhost:9001/3.0/members/13 - user: http://localhost:9001/3.0/users/10 + self_link: http://localhost:9001/3.0/members/10 + user: http://localhost:9001/3.0/users/7 ... entry 9: address: http://localhost:9001/3.0/addresses/herb@example.com @@ -867,10 +820,10 @@ addresses. email: herb@example.com http_etag: "..." list_id: bee.example.com - member_id: 14 + member_id: 11 role: member - self_link: http://localhost:9001/3.0/members/14 - user: http://localhost:9001/3.0/users/10 + self_link: http://localhost:9001/3.0/members/11 + user: http://localhost:9001/3.0/users/7 http_etag: "..." start: 0 total_size: 10 @@ -884,13 +837,13 @@ Herb must iterate through his memberships explicitly. >>> memberships = [entry['self_link'] for entry in content['entries']] >>> for url in sorted(memberships): ... print(url) - http://localhost:9001/3.0/members/13 - http://localhost:9001/3.0/members/14 + http://localhost:9001/3.0/members/10 + http://localhost:9001/3.0/members/11 For each membership resource, the subscription address is changed by PATCH'ing the `address` attribute. - >>> dump_json('http://localhost:9001/3.0/members/13', { + >>> dump_json('http://localhost:9001/3.0/members/10', { ... 'address': 'hperson@example.com', ... }, method='PATCH') content-length: 0 @@ -898,7 +851,7 @@ the `address` attribute. server: ... status: 204 - >>> dump_json('http://localhost:9001/3.0/members/14', { + >>> dump_json('http://localhost:9001/3.0/members/11', { ... 'address': 'hperson@example.com', ... }, method='PATCH') content-length: 0 @@ -925,20 +878,20 @@ his membership ids have not changed. email: hperson@example.com http_etag: "..." list_id: ant.example.com - member_id: 13 + member_id: 10 role: member - self_link: http://localhost:9001/3.0/members/13 - user: http://localhost:9001/3.0/users/10 + self_link: http://localhost:9001/3.0/members/10 + user: http://localhost:9001/3.0/users/7 entry 1: address: http://localhost:9001/3.0/addresses/hperson@example.com delivery_mode: regular email: hperson@example.com http_etag: "..." list_id: bee.example.com - member_id: 14 + member_id: 11 role: member - self_link: http://localhost:9001/3.0/members/14 - user: http://localhost:9001/3.0/users/10 + self_link: http://localhost:9001/3.0/members/11 + user: http://localhost:9001/3.0/users/7 http_etag: "..." start: 0 total_size: 2 @@ -947,7 +900,7 @@ When changing his subscription address, Herb may also decide to change his mode of delivery. :: - >>> dump_json('http://localhost:9001/3.0/members/14', { + >>> dump_json('http://localhost:9001/3.0/members/11', { ... 'address': 'herb@example.com', ... 'delivery_mode': 'mime_digests', ... }, method='PATCH') @@ -964,10 +917,10 @@ mode of delivery. email: herb@example.com http_etag: "..." list_id: bee.example.com - member_id: 14 + member_id: 11 role: member - self_link: http://localhost:9001/3.0/members/14 - user: http://localhost:9001/3.0/users/10 + self_link: http://localhost:9001/3.0/members/11 + user: http://localhost:9001/3.0/users/7 http_etag: "..." start: 0 total_size: 1 @@ -980,22 +933,22 @@ The moderation action for a member can be changed by PATCH'ing the `moderation_action` attribute. When the member action falls back to the list default, there is no such attribute in the resource. - >>> dump_json('http://localhost:9001/3.0/members/13') + >>> dump_json('http://localhost:9001/3.0/members/10') address: http://localhost:9001/3.0/addresses/hperson@example.com delivery_mode: regular email: hperson@example.com http_etag: "..." list_id: ant.example.com - member_id: 13 + member_id: 10 role: member - self_link: http://localhost:9001/3.0/members/13 - user: http://localhost:9001/3.0/users/10 + self_link: http://localhost:9001/3.0/members/10 + user: http://localhost:9001/3.0/users/7 Patching the moderation action both changes it for the given user, and adds the attribute to the member's resource. :: - >>> dump_json('http://localhost:9001/3.0/members/13', { + >>> dump_json('http://localhost:9001/3.0/members/10', { ... 'moderation_action': 'hold', ... }, method='PATCH') content-length: 0 @@ -1003,7 +956,7 @@ the attribute to the member's resource. server: ... status: 204 - >>> dump_json('http://localhost:9001/3.0/members/13') + >>> dump_json('http://localhost:9001/3.0/members/10') address: http://localhost:9001/3.0/addresses/hperson@example.com ... moderation_action: hold @@ -1102,3 +1055,56 @@ As with list-centric bans, you can delete a global ban. http_etag: "..." start: 0 total_size: 0 + + +Mass Unsubscriptions +==================== + +A batch of users can be unsubscribed from the mailing list via the REST API +just by supplying their email addresses. +:: + >>> cat = create_list('cat@example.com') + >>> subscribe(cat, 'Isla') + <Member: Isla Person <iperson@example.com> on + cat@example.com as MemberRole.member> + >>> subscribe(cat, 'John') + <Member: John Person <jperson@example.com> on + cat@example.com as MemberRole.member> + >>> subscribe(cat, 'Kate') + <Member: Kate Person <kperson@example.com> on + cat@example.com as MemberRole.member> + +There are three new members of the mailing list. We try to mass delete them, +plus one other address that isn't a member of the list. We get back a +dictionary mapping email addresses to the success or failure of the removal +operation. It's okay that one of the addresses is removed twice. + + >>> dump_json( + ... 'http://localhost:9001/3.0/lists/cat.example.com/roster/member', { + ... 'emails': ['iperson@example.com', + ... 'jperson@example.com', + ... 'iperson@example.com', + ... 'zperson@example.com', + ... ]}, + ... 'DELETE') + http_etag: "..." + iperson@example.com: True + jperson@example.com: True + zperson@example.com: False + +And now only Kate is still a member. + + >>> dump_json( + ... 'http://localhost:9001/3.0/lists/cat.example.com/roster/member') + entry 0: + address: http://localhost:9001/3.0/addresses/kperson@example.com + delivery_mode: regular + email: kperson@example.com + http_etag: "..." + list_id: cat.example.com + member_id: 14 + role: member + self_link: http://localhost:9001/3.0/members/14 + user: http://localhost:9001/3.0/users/10 + ... + total_size: 1 diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py index 4f0c56569..69d14e548 100644 --- a/src/mailman/rest/lists.py +++ b/src/mailman/rest/lists.py @@ -251,23 +251,19 @@ class MembersOfList(MemberCollection): def on_delete(self, request, response): """Delete the members of the named mailing list.""" status = {} - success = [] - fail = [] try: validator = Validator(emails=list_of_strings_validator) arguments = validator(request) - emails = arguments.pop('emails') - except ValueError: - return bad_request(response, b'Invalid Input.') + except ValueError as error: + bad_request(response, str(error)) + return + emails = arguments.pop('emails') success, fail = getUtility(ISubscriptionService).unsubscribe_members( self._mlist.list_id, emails) - if len(fail) == 0: - return no_content(response) - for email in fail: - if email in success: - status[email] = 'Member already deleted.' - else: - status[email] = 'No such member.' + # There should be no email in both sets. + assert success.isdisjoint(fail), (success, fail) + status.update({email: True for email in success}) + status.update({email: False for email in fail}) okay(response, etag(status)) diff --git a/src/mailman/rest/tests/test_lists.py b/src/mailman/rest/tests/test_lists.py index 207743e50..891afa889 100644 --- a/src/mailman/rest/tests/test_lists.py +++ b/src/mailman/rest/tests/test_lists.py @@ -213,38 +213,111 @@ class TestLists(unittest.TestCase): '/owner/nobody@example.com') self.assertEqual(cm.exception.code, 404) - def test_list_mass_unsubscribe(self): + def test_list_mass_unsubscribe_all_succeed(self): with transaction(): - aperson = self._usermanager.create_address('aperson@test.com') - bperson = self._usermanager.create_address('bperson@test.com') - cperson = self._usermanager.create_address('cperson@test.com') - mlist = create_list('testlist@example.com') - mlist.subscribe(aperson) - mlist.subscribe(bperson) - mlist.subscribe(cperson) + aperson = self._usermanager.create_address('aperson@example.com') + bperson = self._usermanager.create_address('bperson@example.com') + cperson = self._usermanager.create_address('cperson@example.com') + self._mlist.subscribe(aperson) + self._mlist.subscribe(bperson) + self._mlist.subscribe(cperson) + resource, response = call_api( + 'http://localhost:9001/3.0/lists/test.example.com' + '/roster/member', { + 'emails': ['aperson@example.com', + 'bperson@example.com', + ]}, + 'DELETE') + self.assertEqual(response.status, 200) + # Remove variable data. + resource.pop('http_etag') + self.assertEqual(resource, {'aperson@example.com': True, + 'bperson@example.com': True, + }) + + def test_list_mass_unsubscribe_all_fail(self): + with transaction(): + aperson = self._usermanager.create_address('aperson@example.com') + bperson = self._usermanager.create_address('bperson@example.com') + cperson = self._usermanager.create_address('cperson@example.com') + self._mlist.subscribe(aperson) + self._mlist.subscribe(bperson) + self._mlist.subscribe(cperson) + resource, response = call_api( + 'http://localhost:9001/3.0/lists/test.example.com' + '/roster/member', { + 'emails': ['yperson@example.com', + 'zperson@example.com', + ]}, + 'DELETE') + self.assertEqual(response.status, 200) + # Remove variable data. + resource.pop('http_etag') + self.assertEqual(resource, {'yperson@example.com': False, + 'zperson@example.com': False, + }) + + def test_list_mass_unsubscribe_mixed_success(self): + with transaction(): + aperson = self._usermanager.create_address('aperson@example.com') + bperson = self._usermanager.create_address('bperson@example.com') + cperson = self._usermanager.create_address('cperson@example.com') + self._mlist.subscribe(aperson) + self._mlist.subscribe(bperson) + self._mlist.subscribe(cperson) + resource, response = call_api( + 'http://localhost:9001/3.0/lists/test.example.com' + '/roster/member', { + 'emails': ['aperson@example.com', + 'zperson@example.com', + ]}, + 'DELETE') + self.assertEqual(response.status, 200) + # Remove variable data. + resource.pop('http_etag') + self.assertEqual(resource, {'aperson@example.com': True, + 'zperson@example.com': False, + }) + + def test_list_mass_unsubscribe_with_duplicates(self): + with transaction(): + aperson = self._usermanager.create_address('aperson@example.com') + bperson = self._usermanager.create_address('bperson@example.com') + cperson = self._usermanager.create_address('cperson@example.com') + self._mlist.subscribe(aperson) + self._mlist.subscribe(bperson) + self._mlist.subscribe(cperson) + resource, response = call_api( + 'http://localhost:9001/3.0/lists/test.example.com' + '/roster/member', { + 'emails': ['aperson@example.com', + 'aperson@example.com', + 'bperson@example.com', + 'zperson@example.com', + ]}, + 'DELETE') + self.assertEqual(response.status, 200) + # Remove variable data. + resource.pop('http_etag') + self.assertEqual(resource, {'aperson@example.com': True, + 'bperson@example.com': True, + 'zperson@example.com': False, + }) + + def test_list_mass_unsubscribe_bogus_list(self): with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/lists/bogus.example.com' - '/roster/member', None, 'DELETE') + '/roster/member', + None, 'DELETE') self.assertEqual(cm.exception.code, 404) + + def test_list_mass_unsubscribe_with_no_data(self): with self.assertRaises(HTTPError) as cm: - call_api('http://localhost:9001/3.0/lists/testlist.example.com' - '/roster/member', None, 'DELETE') + call_api('http://localhost:9001/3.0/lists/test.example.com' + '/roster/member', + None, 'DELETE') self.assertEqual(cm.exception.code, 400) - self.assertEqual(cm.exception.reason, b'Invalid Input.') - resource, response = call_api( - 'http://127.0.0.1:9001/3.0/lists/testlist.example.com' - '/roster/member', {'emails': ['aperson@test.com']}, 'DELETE') - self.assertEqual(response.status, 204) - resource, response = call_api( - 'http://127.0.0.1:9001/3.0/lists/testlist.example.com' - '/roster/member', {'emails': ['bperson@test.com', - 'cperson@test.com', - 'bperson@test.com', - 'bogus@test.com']}, 'DELETE') - self.assertEqual(response.status, 200) - self.assertEqual(resource['bperson@test.com'], - 'Member already deleted.') - self.assertEqual(resource['bogus@test.com'], 'No such member.') + self.assertEqual(cm.exception.reason, b'Missing parameters: emails') class TestListArchivers(unittest.TestCase): |