diff options
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 2 | ||||
| -rw-r--r-- | src/mailman/model/tests/test_user.py | 32 | ||||
| -rw-r--r-- | src/mailman/model/user.py | 4 | ||||
| -rw-r--r-- | src/mailman/model/usermanager.py | 1 | ||||
| -rw-r--r-- | src/mailman/rest/tests/test_users.py | 21 | ||||
| -rw-r--r-- | src/mailman/rest/users.py | 2 |
6 files changed, 54 insertions, 8 deletions
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 0c1f6bc63..1fda90a89 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -26,6 +26,8 @@ Bugs a 409 error instead of a 500 error. Found by Ankush Sharma. (LP: #1425359) * ``mailman lists --domain`` was not properly handling its arguments. Given by Manish Gill. (LP: #1166911) + * When deleting a user object, make sure their preferences are also deleted. + Given by Abhishek. (LP: #1418276) Configuration ------------- diff --git a/src/mailman/model/tests/test_user.py b/src/mailman/model/tests/test_user.py index a05b69644..3cdac106b 100644 --- a/src/mailman/model/tests/test_user.py +++ b/src/mailman/model/tests/test_user.py @@ -25,10 +25,13 @@ __all__ = [ import unittest from mailman.app.lifecycle import create_list +from mailman.config import config +from mailman.database.transaction import transaction from mailman.interfaces.address import ( AddressAlreadyLinkedError, AddressNotLinkedError) from mailman.interfaces.user import UnverifiedAddressError from mailman.interfaces.usermanager import IUserManager +from mailman.model.preferences import Preferences from mailman.testing.layers import ConfigLayer from mailman.utilities.datetime import now from zope.component import getUtility @@ -41,8 +44,9 @@ class TestUser(unittest.TestCase): layer = ConfigLayer def setUp(self): + self._manager = getUtility(IUserManager) self._mlist = create_list('test@example.com') - self._anne = getUtility(IUserManager).create_user( + self._anne = self._manager.create_user( 'anne@example.com', 'Anne Person') preferred = list(self._anne.addresses)[0] preferred.verified_on = now() @@ -79,7 +83,7 @@ class TestUser(unittest.TestCase): self._anne.user_id = 'foo' def test_addresses_may_only_be_linked_to_one_user(self): - user = getUtility(IUserManager).create_user() + user = self._manager.create_user() # Anne's preferred address is already linked to her. with self.assertRaises(AddressAlreadyLinkedError) as cm: user.link(self._anne.preferred_address) @@ -88,23 +92,39 @@ class TestUser(unittest.TestCase): def test_unlink_from_address_not_linked_to(self): # You cannot unlink an address from a user if that address is not # already linked to the user. - user = getUtility(IUserManager).create_user() + user = self._manager.create_user() with self.assertRaises(AddressNotLinkedError) as cm: user.unlink(self._anne.preferred_address) self.assertEqual(cm.exception.address, self._anne.preferred_address) def test_unlink_address_which_is_not_linked(self): # You cannot unlink an address which is not linked to any user. - address = getUtility(IUserManager).create_address('bart@example.com') - user = getUtility(IUserManager).create_user() + address = self._manager.create_address('bart@example.com') + user = self._manager.create_user() with self.assertRaises(AddressNotLinkedError) as cm: user.unlink(address) self.assertEqual(cm.exception.address, address) def test_set_unverified_preferred_address(self): # A user's preferred address cannot be set to an unverified address. - new_preferred = getUtility(IUserManager).create_address( + new_preferred = self._manager.create_address( 'anne.person@example.com') with self.assertRaises(UnverifiedAddressError) as cm: self._anne.preferred_address = new_preferred self.assertEqual(cm.exception.address, new_preferred) + + def test_preferences_deletion_on_user_deletion(self): + # LP: #1418276 - deleting a user did not delete their preferences. + with transaction(): + # This has to happen in a transaction so that both the user and + # the preferences objects get valid ids. + user = self._manager.create_user() + # The user's preference is in the database. + preferences = config.db.store.query(Preferences).filter_by( + id=user.preferences.id) + self.assertEqual(preferences.count(), 1) + self._manager.delete_user(user) + # The user's preference has been deleted. + preferences = config.db.store.query(Preferences).filter_by( + id=user.preferences.id) + self.assertEqual(preferences.count(), 0) diff --git a/src/mailman/model/user.py b/src/mailman/model/user.py index b74ea6d06..66197d72e 100644 --- a/src/mailman/model/user.py +++ b/src/mailman/model/user.py @@ -83,7 +83,9 @@ class User(Model): 'Duplicate user id {0}'.format(user_id)) self._user_id = user_id self.display_name = ('' if display_name is None else display_name) - self.preferences = preferences + if preferences is not None: + store.add(preferences) + self.preferences = preferences store.add(self) def __repr__(self): diff --git a/src/mailman/model/usermanager.py b/src/mailman/model/usermanager.py index 11557bc25..aefd955ed 100644 --- a/src/mailman/model/usermanager.py +++ b/src/mailman/model/usermanager.py @@ -49,6 +49,7 @@ class UserManager: @dbconnection def delete_user(self, store, user): """See `IUserManager`.""" + store.delete(user.preferences) store.delete(user) @dbconnection diff --git a/src/mailman/rest/tests/test_users.py b/src/mailman/rest/tests/test_users.py index 030e74bcb..af2c9f0d1 100644 --- a/src/mailman/rest/tests/test_users.py +++ b/src/mailman/rest/tests/test_users.py @@ -36,6 +36,7 @@ from mailman.testing.helpers import call_api, configuration from mailman.testing.layers import RESTLayer from urllib.error import HTTPError from zope.component import getUtility +from mailman.model.preferences import Preferences @@ -211,6 +212,26 @@ class TestUsers(unittest.TestCase): content, response = call_api('http://localhost:9001/3.0/users') self.assertEqual(content['total_size'], 1) + def test_preferences_deletion_on_user_deletion(self): + # LP: #1418276 - deleting a user did not delete their preferences. + with transaction(): + anne = getUtility(IUserManager).create_user( + 'anne@example.com', 'Anne Person') + # Anne's preference is in the database. + preferences = config.db.store.query(Preferences).filter_by( + id=anne.preferences.id) + self.assertEqual(preferences.count(), 1) + # Delete the user via REST. + content, response = call_api( + 'http://localhost:9001/3.0/users/anne@example.com', + method='DELETE') + self.assertEqual(response.status, 204) + # The user's preference has been deleted. + with transaction(): + preferences = config.db.store.query(Preferences).filter_by( + id=anne.preferences.id) + self.assertEqual(preferences.count(), 0) + class TestLogin(unittest.TestCase): diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py index 018c8441d..a912b6129 100644 --- a/src/mailman/rest/users.py +++ b/src/mailman/rest/users.py @@ -185,7 +185,7 @@ class AUser(_UserBase): return UserAddresses(self._user) def on_delete(self, request, response): - """Delete the named user, all her memberships, and addresses.""" + """Delete the named user and all associated resources.""" if self._user is None: not_found(response) return |