add unit tests

ceo_common/interfaces/IUser.py

@@ -53,11 +53,8 @@ class IUser(Interface):
     def add_non_member_terms(terms: List[str]):
         """Add non-member terms for this user."""
 
-    def add_position(position: str):
-        """Add a position to this user."""
-
-    def remove_position(position: str):
-        """Remove a position from this user."""
+    def set_positions(self, positions: List[str]):
+        """Set the positions for this user."""
 
     def change_password(password: str):
         """Replace this user's password."""

ceo_common/model/Config.py

@@ -28,5 +28,5 @@ class Config:
         if val.lower() in ['false', 'no']:
             return False
         if section.startswith('auxiliary ') or section == 'positions':
-            return val.split(',')
+            return [item.strip() for item in val.split(',')]
         return val

ceod/api/positions.py

@@ -1,4 +1,3 @@
-import json
 from flask import Blueprint, request
 from zope import component
 
@@ -18,7 +17,7 @@ def get_positions():
         for position in user.positions:
             positions[position] = user.uid
 
-    return json.dumps(positions)
+    return positions
 
 
 @bp.route('/', methods=['POST'], strict_slashes=False)
@@ -34,7 +33,7 @@ def update_positions():
         if position not in available:
             return {
                 'error': f'unknown position: {position}'
-            }, 404
+            }, 400
 
     for position in required:
         if position not in body:

ceod/model/Group.py

@@ -112,3 +112,4 @@ class Group:
         ]
         with self.ldap_srv.entry_ctx_for_group(self) as entry:
             entry.uniqueMember = DNs
+        self.members = usernames

ceod/model/User.py

@@ -67,9 +67,8 @@ class User:
             'login_shell': self.login_shell,
             'home_directory': self.home_directory,
             'is_club': self.is_club(),
+            'program': self.program or 'Unknown',
         }
-        if self.program:
-            data['program'] = self.program
         if self.terms:
             data['terms'] = self.terms
         if self.non_member_terms:
@@ -158,16 +157,6 @@ class User:
             entry.nonMemberTerm.add(terms)
         self.non_member_terms.extend(terms)
 
-    def add_position(self, position: str):
-        with self.ldap_srv.entry_ctx_for_user(self) as entry:
-            entry.position.add(position)
-        self.positions.append(position)
-
-    def remove_position(self, position: str):
-        with self.ldap_srv.entry_ctx_for_user(self) as entry:
-            entry.position.delete(position)
-        self.positions.remove(position)
-
     def set_positions(self, positions: List[str]):
         with self.ldap_srv.entry_ctx_for_user(self) as entry:
             entry.position = positions

tests/MockMailmanServer.py

@@ -35,6 +35,10 @@ class MockMailmanServer:
     def stop(self):
         self.loop.call_soon_threadsafe(self.loop.stop)
 
+    def clear(self):
+        for key in self.subscriptions:
+            self.subscriptions[key].clear()
+
     async def subscribe(self, request):
         body = await request.post()
         subscriber = body['subscriber']

tests/ceod/api/test_positions.py

@@ -0,0 +1,93 @@
+from ceod.model import User, Group
+
+
+def test_get_positions(client, ldap_user, g_admin_ctx):
+    with g_admin_ctx():
+        ldap_user.set_positions(['president', 'treasurer'])
+    status, data = client.get('/api/positions')
+    assert status == 200
+    expected = {
+        'president': ldap_user.uid,
+        'treasurer': ldap_user.uid,
+    }
+    assert data == expected
+
+
+def test_set_positions(cfg, client, g_admin_ctx, mock_mailman_server):
+    mock_mailman_server.clear()
+    mailing_lists = cfg.get('auxiliary mailing lists_exec')
+    base_domain = cfg.get('base_domain')
+
+    users = []
+    with g_admin_ctx():
+        for uid in ['test_1', 'test_2', 'test_3', 'test_4']:
+            user = User(uid=uid, cn='Some Name', terms=['s2021'])
+            user.add_to_ldap()
+            users.append(user)
+        exec_group = Group(cn='exec', gid_number=10013)
+        exec_group.add_to_ldap()
+
+    try:
+        # missing required position
+        status, _ = client.post('/api/positions', json={
+            'vice-president': 'test_1',
+        })
+        assert status == 400
+
+        # non-existent position
+        status, _ = client.post('/api/positions', json={
+            'president': 'test_1',
+            'vice-president': 'test_2',
+            'sysadmin': 'test_3',
+            'no-such-position': 'test_3',
+        })
+        assert status == 400
+
+        status, data = client.post('/api/positions', json={
+            'president': 'test_1',
+            'vice-president': 'test_2',
+            'sysadmin': 'test_3',
+        })
+        assert status == 200
+        expected = [
+            {"status": "in progress", "operation": "update_positions_ldap"},
+            {"status": "in progress", "operation": "update_exec_group_ldap"},
+            {"status": "in progress", "operation": "subscribe_to_mailing_lists"},
+            {"status": "completed", "result": {
+                "president": "test_1",
+                "vice-president": "test_2",
+                "sysadmin": "test_3",
+            }},
+        ]
+        assert data == expected
+        # make sure execs were added to exec group
+        status, data = client.get('/api/groups/exec')
+        assert status == 200
+        expected = ['test_1', 'test_2', 'test_3']
+        assert sorted([item['uid'] for item in data['members']]) == expected
+        # make sure execs were subscribed to mailing lists
+        addresses = [f'{uid}@{base_domain}' for uid in expected]
+        for mailing_list in mailing_lists:
+            assert sorted(mock_mailman_server.subscriptions[mailing_list]) == addresses
+
+        _, data = client.post('/api/positions', json={
+            'president': 'test_1',
+            'vice-president': 'test_2',
+            'sysadmin': 'test_2',
+            'treasurer': 'test_4',
+        })
+        assert data[-1]['status'] == 'completed'
+        # make sure old exec was removed from group
+        expected = ['test_1', 'test_2', 'test_4']
+        _, data = client.get('/api/groups/exec')
+        assert sorted([item['uid'] for item in data['members']]) == expected
+        # make sure old exec was removed from mailing lists
+        addresses = [f'{uid}@{base_domain}' for uid in expected]
+        for mailing_list in mailing_lists:
+            assert sorted(mock_mailman_server.subscriptions[mailing_list]) == addresses
+    finally:
+        with g_admin_ctx():
+            for user in users:
+                user.remove_from_ldap()
+            exec_group.remove_from_ldap()
+        mock_mailman_server.clear()

tests/ceod/model/test_group.py

@@ -37,6 +37,10 @@ def test_group_members(ldap_group, ldap_srv):
     with pytest.raises(UserNotInGroupError):
         group.remove_member('member1')
 
+    group.set_members(['member3'])
+    assert group.members == ['member3']
+    assert ldap_srv.get_group(group.cn).members == group.members
+
 
 def test_group_to_dict(ldap_group, ldap_user, g_admin_ctx):
     group = ldap_group

tests/ceod/model/test_user.py

@@ -116,16 +116,14 @@ def test_user_terms(ldap_user, ldap_srv):
 def test_user_positions(ldap_user, ldap_srv):
     user = ldap_user
 
-    user.add_position('treasurer')
-    assert user.positions == ['treasurer']
-    assert ldap_srv.get_user(user.uid).positions == user.positions
-    user.add_position('cro')
-    assert user.positions == ['treasurer', 'cro']
+    old_positions = user.positions[:]
+
+    new_positions = ['treasurer', 'cro']
+    user.set_positions(new_positions)
+    assert user.positions == new_positions
     assert ldap_srv.get_user(user.uid).positions == user.positions
 
-    user.remove_position('cro')
-    assert user.positions == ['treasurer']
-    assert ldap_srv.get_user(user.uid).positions == user.positions
+    user.set_positions(old_positions)
 
 
 def test_user_change_password(krb_user):

tests/ceod_dev.ini

@@ -55,4 +55,5 @@ exec = exec
 
 [positions]
 required = president,vice-president,sysadmin
-available = president,vice-president,treasurer,secretary,sysadmin,cro,librarian,imapd,webmaster,offsck
+available = president,vice-president,treasurer,secretary,
+            sysadmin,cro,librarian,imapd,webmaster,offsck

tests/ceod_test_local.ini

@@ -49,3 +49,8 @@ syscom = office,staff
 [auxiliary mailing lists]
 syscom = syscom,syscom-alerts
 exec = exec
+
+[positions]
+required = president,vice-president,sysadmin
+available = president,vice-president,treasurer,secretary,
+            sysadmin,cro,librarian,imapd,webmaster,offsck