diff options
| author | Barry Warsaw | 2012-11-05 11:01:04 -0500 |
|---|---|---|
| committer | Barry Warsaw | 2012-11-05 11:01:04 -0500 |
| commit | a3c1fad102fc1fc454ddfa2bd66068b9aab636fe (patch) | |
| tree | 39e9e9e59b9470cd93569bcbd1b9238e9579916a | |
| parent | e56ca2098b727c762e2572b044fc8c943770b5b4 (diff) | |
| download | mailman-a3c1fad102fc1fc454ddfa2bd66068b9aab636fe.tar.gz mailman-a3c1fad102fc1fc454ddfa2bd66068b9aab636fe.tar.zst mailman-a3c1fad102fc1fc454ddfa2bd66068b9aab636fe.zip | |
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 2 | ||||
| -rw-r--r-- | src/mailman/rest/docs/users.rst | 7 | ||||
| -rw-r--r-- | src/mailman/rest/tests/test_users.py | 87 | ||||
| -rw-r--r-- | src/mailman/rest/users.py | 9 | ||||
| -rw-r--r-- | src/mailman/testing/helpers.py | 1 |
5 files changed, 103 insertions, 3 deletions
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 3c7580126..0fa00ab86 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -34,6 +34,8 @@ REST respectively. The POST data is ignored. It is not an error to verify or unverify an address more than once, but verifying an already verified address does not change its `.verified_on` date. (LP: #1054730) + * Deleting a user through the REST API also deletes all the user's linked + addresses and memberships. (LP: #1074374) Configuration ------------- diff --git a/src/mailman/rest/docs/users.rst b/src/mailman/rest/docs/users.rst index 888bc43fd..efd664f27 100644 --- a/src/mailman/rest/docs/users.rst +++ b/src/mailman/rest/docs/users.rst @@ -230,6 +230,13 @@ Cris's resource cannot be retrieved either by email address... ... HTTPError: HTTP Error 404: 404 Not Found +Cris's address records no longer exist either. + + >>> dump_json('http://localhost:9001/3.0/addresses/cris@example.com') + Traceback (most recent call last): + ... + HTTPError: HTTP Error 404: 404 Not Found + Missing users ============= diff --git a/src/mailman/rest/tests/test_users.py b/src/mailman/rest/tests/test_users.py index 4595c69d8..cf83e096c 100644 --- a/src/mailman/rest/tests/test_users.py +++ b/src/mailman/rest/tests/test_users.py @@ -22,15 +22,18 @@ from __future__ import absolute_import, print_function, unicode_literals __metaclass__ = type __all__ = [ 'TestUsers', + 'TestLP1074374', ] import unittest from urllib2 import HTTPError +from zope.component import getUtility from mailman.app.lifecycle import create_list from mailman.database.transaction import transaction +from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import call_api from mailman.testing.layers import RESTLayer @@ -48,3 +51,87 @@ class TestUsers(unittest.TestCase): with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/users/99', method='DELETE') self.assertEqual(cm.exception.code, 404) + + + +class TestLP1074374(unittest.TestCase): + """LP: #1074374 - deleting a user left their address records active.""" + + layer = RESTLayer + + def setUp(self): + self.user_manager = getUtility(IUserManager) + with transaction(): + self.mlist = create_list('test@example.com') + self.anne = self.user_manager.create_user( + 'anne@example.com', 'Anne Person') + + def test_deleting_user_deletes_address(self): + with transaction(): + user_id = self.anne.user_id + call_api('http://localhost:9001/3.0/users/anne@example.com', + method='DELETE') + # The user record is gone. + self.assertIsNone(self.user_manager.get_user_by_id(user_id)) + self.assertIsNone(self.user_manager.get_user('anne@example.com')) + # Anne's address is also gone. + self.assertIsNone(self.user_manager.get_address('anne@example.com')) + + def test_deleting_user_deletes_addresses(self): + # All of Anne's linked addresses are deleted when her user record is + # deleted. So, register and link another address to Anne. + with transaction(): + self.anne.register('aperson@example.org') + call_api('http://localhost:9001/3.0/users/anne@example.com', + method='DELETE') + self.assertIsNone(self.user_manager.get_user('anne@example.com')) + self.assertIsNone(self.user_manager.get_user('aperson@example.org')) + + def test_lp_1074374(self): + # Specific steps to reproduce the bug: + # - create a user through the REST API (well, we did that outside the + # REST API here, but that should be fine) + # - delete that user through the API + # - repeating step 1 gives a 500 status code + # - /3.0/addresses still contains the original address + # - /3.0/members gives a 500 + with transaction(): + user_id = self.anne.user_id + address = list(self.anne.addresses)[0] + self.mlist.subscribe(address) + call_api('http://localhost:9001/3.0/users/anne@example.com', + method='DELETE') + content, response = call_api('http://localhost:9001/3.0/addresses') + # There are no addresses, and thus no entries in the returned JSON. + self.assertNotIn('entries', content) + self.assertEqual(content['total_size'], 0) + # There are also no members. + content, response = call_api('http://localhost:9001/3.0/members') + self.assertNotIn('entries', content) + self.assertEqual(content['total_size'], 0) + # Now we can create a new user record for Anne, and subscribe her to + # the mailing list, this time all through the API. + call_api('http://localhost:9001/3.0/users', dict( + email='anne@example.com', + password='bbb')) + call_api('http://localhost:9001/3.0/members', dict( + list_id='test.example.com', + subscriber='anne@example.com', + role='member')) + # This is not the Anne you're looking for. (IOW, the new Anne is a + # different user). + content, response = call_api( + 'http://localhost:9001/3.0/users/anne@example.com') + self.assertNotEqual(user_id, content['user_id']) + # Anne has an address record. + content, response = call_api('http://localhost:9001/3.0/addresses') + self.assertEqual(content['total_size'], 1) + self.assertEqual(content['entries'][0]['email'], 'anne@example.com') + # Anne is also a member of the mailing list. + content, response = call_api('http://localhost:9001/3.0/members') + self.assertEqual(content['total_size'], 1) + member = content['entries'][0] + self.assertEqual(member['address'], 'anne@example.com') + self.assertEqual(member['delivery_mode'], 'regular') + self.assertEqual(member['list_id'], 'test.example.com') + self.assertEqual(member['role'], 'member') diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py index 94817da49..25a49defa 100644 --- a/src/mailman/rest/users.py +++ b/src/mailman/rest/users.py @@ -170,10 +170,15 @@ class AUser(_UserBase): @resource.DELETE() def delete_user(self, request): - """Delete the named user.""" + """Delete the named user, all her memberships, and addresses.""" if self._user is None: return http.not_found() - getUtility(IUserManager).delete_user(self._user) + for member in self._user.memberships.members: + member.unsubscribe() + user_manager = getUtility(IUserManager) + for address in self._user.addresses: + user_manager.delete_address(address) + user_manager.delete_user(self._user) return no_content() @resource.child() diff --git a/src/mailman/testing/helpers.py b/src/mailman/testing/helpers.py index 99f4b8961..b233e7bd4 100644 --- a/src/mailman/testing/helpers.py +++ b/src/mailman/testing/helpers.py @@ -23,7 +23,6 @@ __metaclass__ = type __all__ = [ 'LogFileMark', 'TestableMaster', - 'body_line_iterator', 'call_api', 'chdir', 'configuration', |
