1 files changed, 130 insertions, 85 deletions

diff --git a/src/mailman/model/tests/test_user.py b/src/mailman/model/tests/test_user.py
index 5bb684331..e55267f8f 100644
--- a/src/mailman/model/tests/test_user.py
+++ b/src/mailman/model/tests/test_user.py
@@ -30,7 +30,6 @@ 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
 
@@ -124,135 +123,181 @@ class TestUser(unittest.TestCase):
             id=user.preferences.id)
         self.assertEqual(preferences.count(), 0)
 
+    def test_absorb_not_a_user(self):
+        bart = self._manager.create_address('bart@example.com')
+        self.assertRaises(TypeError, self._anne.absorb, bart)
+
     def test_absorb_addresses(self):
-        anne_addr = self._anne.preferred_address
+        # Absorbing the user absorbs all of the users addresses.  I.e. they
+        # are relinked to the absorbing user.
+        anne_preferred = self._anne.preferred_address
         with transaction():
             # This has to happen in a transaction so that both the user and
             # the preferences objects get valid ids.
-            bill = self._manager.create_user(
-                'bill@example.com', 'Bill Person')
-            bill_addr_2 = self._manager.create_address('bill2@example.com')
-            bill.link(bill_addr_2)
-        self._anne.absorb(bill)
-        self.assertIn(
-            'bill@example.com',
-            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.
-        self.assertEqual(self._anne.preferred_address, anne_addr)
+            bart = self._manager.create_user('bart@example.com', 'Bart Person')
+            bart_secondary = self._manager.create_address(
+                'bart.person@example.com')
+            bart.link(bart_secondary)
+        # Absorb the Bart user into Anne.
+        self._anne.absorb(bart)
+        # Bart's primary and secondary addresses are now linked to Anne.
+        anne_addresses = list(
+            address.email for address in self._anne.addresses)
+        self.assertIn('bart@example.com', anne_addresses)
+        self.assertIn('bart.person@example.com', anne_addresses)
+        # Anne's preferred address shouldn't change.
+        self.assertEqual(self._anne.preferred_address, anne_preferred)
+        # Check the reverse linkages by getting Bart's addresses from the user
+        # manager.  They should both point back to the Anne user.
         self.assertEqual(
-            self._manager.get_user('bill@example.com'), self._anne)
+            self._manager.get_user('bart@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))
+            self._manager.get_user('bart.person@example.com'), self._anne)
+        # The Bart user has been deleted.
+        self.assertIsNone(self._manager.get_user_by_id(bart.user_id))
 
     def test_absorb_memberships(self):
+        # When a user is absorbed, all of their user-subscribed memberships
+        # are relinked to the absorbing user.
         mlist2 = create_list('test2@example.com')
         mlist3 = create_list('test3@example.com')
         with transaction():
             # This has to happen in a transaction so that both the user and
             # the preferences objects get valid ids.
-            bill = self._manager.create_user(
-                'bill@example.com', 'Bill Person')
-            bill_address = list(bill.addresses)[0]
-            bill_address.verified_on = now()
-            bill.preferred_address = bill_address
+            bart = self._manager.create_user('bart@example.com', 'Bart Person')
+            set_preferred(bart)
         # Subscribe both users to self._mlist.
         self._mlist.subscribe(self._anne, MemberRole.member)
-        self._mlist.subscribe(bill, MemberRole.moderator)
-        # Subscribe only bill to mlist2.
-        mlist2.subscribe(bill, MemberRole.owner)
-        # Subscribe only bill's address to mlist3.
-        mlist3.subscribe(bill.preferred_address, MemberRole.moderator)
+        self._mlist.subscribe(bart, MemberRole.moderator)
+        # Subscribe only Bart to mlist2.
+        mlist2.subscribe(bart, MemberRole.owner)
+        # Subscribe only Bart's address to mlist3.
+        mlist3.subscribe(bart.preferred_address, MemberRole.moderator)
+        # There are now 4 memberships, one with Anne two with Bart's user and
+        # one with Bart's address.
+        all_members = list(self._manager.members)
+        self.assertEqual(len(all_members), 4, all_members)
         # Do the absorption.
-        self._anne.absorb(bill)
-        # Check that bill has been deleted.
-        self.assertEqual(len(list(self._manager.users)), 1)
-        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.
-        self.assertEqual(self._anne.memberships.member_count, 3)
-        memberships = {}
-        for member in self._anne.memberships.members:
-            memberships[member.list_id] = member
-        self.assertEqual(
-            set(memberships.keys()),
-            set([
-                'test.example.com',
-                'test2.example.com',
-                'test3.example.com',
-                ]))
-        # The subscription to test@example.com already existed, it must not be
-        # overwritten.
-        self.assertEqual(
-            memberships['test.example.com'].role, MemberRole.member)
-        # Check that the subscription roles were imported
+        self._anne.absorb(bart)
+        # The Bart user has been deleted, leaving only the Anne user in the
+        # user manager.
+        all_users = list(self._manager.users)
+        self.assertEqual(len(all_users), 1)
+        self.assertEqual(all_users[0], self._anne)
+        # There are no leftover memberships for user Bart.  Anne owns all the
+        # memberships.
+        all_members = list(self._manager.members)
+        self.assertEqual(len(all_members), 4, all_members)
+        self.assertEqual(self._anne.memberships.member_count, 4)
+        memberships = {(member.list_id, member.role): member
+                       for member in self._anne.memberships.members}
+        # Note that Anne is now both a member and moderator of the test list.
+        self.assertEqual(set(memberships), set([
+            ('test.example.com', MemberRole.member),
+            ('test.example.com', MemberRole.moderator),
+            ('test2.example.com', MemberRole.owner),
+            ('test3.example.com', MemberRole.moderator),
+            ]))
+        # Both of Bart's previous user subscriptions are now transferred to
+        # the Anne user.
         self.assertEqual(
-            memberships['test2.example.com'].role, MemberRole.owner)
-        self.assertEqual(
-            memberships['test3.example.com'].role, MemberRole.moderator)
-        # The user bill was subscribed, the subscription must thus be
-        # transferred to anne's primary address.
-        self.assertEqual(
-            memberships['test2.example.com'].address,
+            memberships[('test.example.com', MemberRole.moderator)].address,
             self._anne.preferred_address)
-        # The address was subscribed, it must not be changed
         self.assertEqual(
-            memberships['test3.example.com'].address.email,
-            'bill@example.com')
+            memberships[('test2.example.com', MemberRole.owner)].address,
+            self._anne.preferred_address)
+        # Bart's address was subscribed; it must not have been changed.  Of
+        # course, Anne now controls bart@example.com.
+        key = ('test3.example.com', MemberRole.moderator)
+        self.assertEqual(memberships[key].address.email, 'bart@example.com')
+        self.assertEqual(self._manager.get_user('bart@example.com'),
+                         self._anne)
+
+    def test_absorb_duplicates(self):
+        # Duplicate memberships, where the list-id and role match, are
+        # ignored.  Here we subscribe Anne to the test list as a member, and
+        # Bart as both a member and an owner.  Anne's member membership
+        # remains unchanged, but she gains an owner membership.
+        with transaction():
+            bart = self._manager.create_user('bart@example.com')
+            set_preferred(bart)
+        self._mlist.subscribe(self._anne, MemberRole.member)
+        self._mlist.subscribe(bart, MemberRole.member)
+        self._mlist.subscribe(bart, MemberRole.owner)
+        # There are now three memberships.
+        all_members = list(self._manager.members)
+        self.assertEqual(len(all_members), 3, all_members)
+        # Do the absorption.
+        self._anne.absorb(bart)
+        # There are now only 2 memberships, both owned by Anne.
+        all_members = list(self._manager.members)
+        self.assertEqual(len(all_members), 2, all_members)
+        memberships = set([
+            (member.list_id, member.role, member.address.email)
+            for member in all_members
+            ])
+        self.assertEqual(memberships, set([
+            ('test.example.com', MemberRole.member, 'anne@example.com'),
+            ('test.example.com', MemberRole.owner, 'anne@example.com'),
+            ]))
 
     def test_absorb_preferences(self):
         with transaction():
             # This has to happen in a transaction so that both the user and
             # the preferences objects get valid ids.
-            bill = self._manager.create_user(
-                'bill@example.com', 'Bill Person')
-        bill.preferences.acknowledge_posts = True
+            bart = self._manager.create_user('bart@example.com', 'Bart Person')
+        bart.preferences.acknowledge_posts = True
         self.assertIsNone(self._anne.preferences.acknowledge_posts)
-        self._anne.absorb(bill)
+        self._anne.absorb(bart)
         self.assertEqual(self._anne.preferences.acknowledge_posts, True)
-        # Check that Bill's preferences were deleted (requires a DB flush).
+        # Check that Bart's preferences were deleted (requires a DB flush).
         config.db.store.flush()
-        self.assertTrue(inspect(bill.preferences).deleted)
+        self.assertTrue(inspect(bart.preferences).deleted)
 
     def test_absorb_properties(self):
-        props = {
+        properties = {
             'password': 'dummy',
             'is_server_owner': True
-        }
+            }
         with transaction():
             # This has to happen in a transaction so that both the user and
             # the preferences objects get valid ids.
-            bill = self._manager.create_user(
-                'bill@example.com', 'Bill Person')
-        for prop, value in props.items():
-            setattr(bill, prop, value)
-        self._anne.absorb(bill)
-        for prop, value in props.items():
-            self.assertEqual(getattr(self._anne, prop), value)
-        # This was not empty so it must not be overwritten
+            bart = self._manager.create_user('bart@example.com', 'Bart Person')
+        for name, value in properties.items():
+            setattr(bart, name, value)
+        self._anne.absorb(bart)
+        for name, value in properties.items():
+            self.assertEqual(getattr(self._anne, name), value)
+        # This was not empty so it must not have been overwritten.
         self.assertEqual(self._anne.display_name, 'Anne Person')
 
+    def test_absorb_display_name(self):
+        # Bart has no display name, but once he absorbs Cate, he gains her
+        # display_name.
+        with transaction():
+            bart = self._manager.create_user('bart@example.com')
+            cate = self._manager.create_user('cate@example.com', 'Cate Person')
+        self.assertEqual(bart.display_name, '')
+        bart.absorb(cate)
+        self.assertEqual(bart.display_name, 'Cate Person')
+
     def test_absorb_delete_user(self):
-        # Make sure the user was deleted
+        # Make sure the user was deleted after being absorbed.
         with transaction():
             # This has to happen in a transaction so that both the user and
             # the preferences objects get valid ids.
-            bill = self._manager.create_user(
-                'bill@example.com', 'Bill Person')
-        bill_user_id = bill.user_id
-        self._anne.absorb(bill)
-        self.assertIsNone(self._manager.get_user_by_id(bill_user_id))
+            bart = self._manager.create_user('bart@example.com', 'Bart Person')
+        bart_user_id = bart.user_id
+        self._anne.absorb(bart)
+        self.assertIsNone(self._manager.get_user_by_id(bart_user_id))
 
     def test_absorb_self(self):
-        # Absorbing oneself should be a no-op (it must not delete the user)
+        # Absorbing oneself should be a no-op (it must not delete the user).
         self._mlist.subscribe(self._anne)
         self._anne.absorb(self._anne)
         new_anne = self._manager.get_user_by_id(self._anne.user_id)
         self.assertIsNotNone(new_anne)
         self.assertEqual(
-            [a.email for a in new_anne.addresses], ['anne@example.com'])
+            [address.email for address in new_anne.addresses],
+            ['anne@example.com'])
         self.assertEqual(new_anne.memberships.member_count, 1)