5 files changed, 31 insertions, 21 deletions

diff --git a/src/mailman/interfaces/user.py b/src/mailman/interfaces/user.py
index bdeccce3f..27a8cbaba 100644
--- a/src/mailman/interfaces/user.py
+++ b/src/mailman/interfaces/user.py
@@ -114,6 +114,4 @@ class IUser(Interface):
         """Merge this user's attributes and memberships, and then delete it.
 
         In case of conflict, the current user's properties are preserved.
-        If an IAddress is given, the merge will be performed on the addresses'
-        linked user.
         """
diff --git a/src/mailman/model/preferences.py b/src/mailman/model/preferences.py
index 22d22ed94..b69282e37 100644
--- a/src/mailman/model/preferences.py
+++ b/src/mailman/model/preferences.py
@@ -19,8 +19,8 @@
 
 from mailman import public
 from mailman.database.model import Model
-from mailman.database.types import Enum
 from mailman.database.transaction import dbconnection
+from mailman.database.types import Enum
 from mailman.interfaces.languages import ILanguageManager
 from mailman.interfaces.member import DeliveryMode, DeliveryStatus
 from mailman.interfaces.preferences import IPreferences
@@ -69,10 +69,9 @@ class Preferences(Model):
     @dbconnection
     def absorb(self, store, preferences):
         """See `IPreferences`."""
-        column_names = [ c.name for c in self.__table__.columns
-                         if not c.primary_key ]
+        column_names = [c.name for c in self.__table__.columns
+                        if not c.primary_key]
         for cname in column_names:
             if (getattr(self, cname) is None and
                 getattr(preferences, cname) is not None):
                 setattr(self, cname, getattr(preferences, cname))
-        store.delete(preferences)
diff --git a/src/mailman/model/tests/test_preferences.py b/src/mailman/model/tests/test_preferences.py
index 17e121454..929833595 100644
--- a/src/mailman/model/tests/test_preferences.py
+++ b/src/mailman/model/tests/test_preferences.py
@@ -25,6 +25,7 @@ __all__ = [
 import unittest
 
 from mailman.app.lifecycle import create_list
+from mailman.config import config
 from mailman.database.transaction import transaction
 from mailman.interfaces.languages import ILanguageManager
 from mailman.interfaces.member import DeliveryMode, DeliveryStatus
@@ -61,7 +62,7 @@ class TestPreferences(unittest.TestCase):
             'receive_own_postings': True,
             'delivery_mode': DeliveryMode.mime_digests,
             'delivery_status': DeliveryStatus.by_user,
-        }
+            }
         bill_prefs = self._bill.preferences
         for name, value in attributes.items():
             setattr(bill_prefs, name, value)
@@ -73,17 +74,17 @@ class TestPreferences(unittest.TestCase):
         # Only overwrite the pref if it is unset in the absorber
         anne_prefs = self._anne.preferences
         bill_prefs = self._bill.preferences
-        self.assertEqual(self._anne.preferences.acknowledge_posts, None)
-        self.assertEqual(self._anne.preferences.hide_address, None)
-        self.assertEqual(self._anne.preferences.receive_list_copy, None)
+        self.assertIsNone(self._anne.preferences.acknowledge_posts)
+        self.assertIsNone(self._anne.preferences.hide_address)
+        self.assertIsNone(self._anne.preferences.receive_list_copy)
         anne_prefs.acknowledge_posts = False
         bill_prefs.acknowledge_posts = True
         anne_prefs.hide_address = True
         bill_prefs.receive_list_copy = True
         self._anne.preferences.absorb(self._bill.preferences)
         # set for both anne and bill, don't overwrite
-        self.assertEqual(self._anne.preferences.acknowledge_posts, False)
+        self.assertFalse(self._anne.preferences.acknowledge_posts)
         # set only for anne
-        self.assertEqual(self._anne.preferences.hide_address, True)
+        self.assertTrue(self._anne.preferences.hide_address)
         # set only for bill, overwrite anne's default value
-        self.assertEqual(self._anne.preferences.receive_list_copy, True)
+        self.assertTrue(self._anne.preferences.receive_list_copy)
diff --git a/src/mailman/model/tests/test_user.py b/src/mailman/model/tests/test_user.py
index deb017620..8c509c60a 100644
--- a/src/mailman/model/tests/test_user.py
+++ b/src/mailman/model/tests/test_user.py
@@ -30,6 +30,8 @@ from mailman.interfaces.usermanager import IUserManager
 from mailman.model.preferences import Preferences
 from mailman.testing.helpers import set_preferred
 from mailman.testing.layers import ConfigLayer
+from mailman.utilities.datetime import now
+from sqlalchemy import inspect
 from zope.component import getUtility
 
 
@@ -136,8 +138,13 @@ class TestUser(unittest.TestCase):
             list(a.email for a in self._anne.addresses))
         self.assertIn('bill2@example.com',
             list(a.email for a in self._anne.addresses))
-        # the preferred address shouldn't change
+        # The preferred address shouldn't change.
         self.assertEqual(self._anne.preferred_address, anne_addr)
+        self.assertEqual(
+            self._manager.get_user('bill@example.com'), self._anne)
+        self.assertEqual(
+            self._manager.get_user('bill2@example.com'), self._anne)
+        self.assertIsNone(self._manager.get_user_by_id(bill.user_id))
 
     def test_absorb_memberships(self):
         mlist2 = create_list('test2@example.com')
@@ -150,20 +157,21 @@ class TestUser(unittest.TestCase):
             bill_address = list(bill.addresses)[0]
             bill_address.verified_on = now()
             bill.preferred_address = bill_address
-        # Subscribe both users to self._mlist
+        # Subscribe both users to self._mlist.
         self._mlist.subscribe(self._anne, MemberRole.member)
         self._mlist.subscribe(bill, MemberRole.moderator)
-        # Subscribe only bill to mlist2
+        # Subscribe only bill to mlist2.
         mlist2.subscribe(bill, MemberRole.owner)
-        # Subscribe only bill's address to mlist3
+        # Subscribe only bill's address to mlist3.
         mlist3.subscribe(bill.preferred_address, MemberRole.moderator)
-        # Do the absorption
+        # Do the absorption.
         self._anne.absorb(bill)
-        # check that bill has been deleted
+        # Check that bill has been deleted.
         self.assertEqual(len(list(self._manager.users)), 1)
-        # check that there is no leftover membership from user bill
+        self.assertEqual(list(self._manager.users)[0], self._anne)
+        # Check that there is no leftover membership from user bill.
         self.assertEqual(len(list(self._manager.members)), 3)
-        # check that anne is subscribed to all lists
+        # Check that anne is subscribed to all lists.
         self.assertEqual(self._anne.memberships.member_count, 3)
         memberships = {}
         for member in self._anne.memberships.members:
@@ -200,6 +208,9 @@ class TestUser(unittest.TestCase):
         self.assertIsNone(self._anne.preferences.acknowledge_posts)
         self._anne.absorb(bill)
         self.assertEqual(self._anne.preferences.acknowledge_posts, True)
+        # Check that Bill's preferences were deleted (requires a DB flush).
+        config.db.store.flush()
+        self.assertTrue(inspect(bill.preferences).deleted)
 
     def test_absorb_properties(self):
         props = {
diff --git a/src/mailman/model/user.py b/src/mailman/model/user.py
index 783d7f7b7..fe4f1c6d4 100644
--- a/src/mailman/model/user.py
+++ b/src/mailman/model/user.py
@@ -199,6 +199,7 @@ class User(Model):
                 store.delete(member)
         # Merge the user preferences
         self.preferences.absorb(user.preferences)
+        store.delete(user.preferences)
         # Merge display_name, password and is_server_owner attributes.
         for prop in ('display_name', 'password', 'is_server_owner'):
             if getattr(user, prop) and not getattr(self, prop):