Allow addmember and removemember to accept multiple usernames (#67)

Closes #57.

Reviewed-on: #67

ceo/cli/groups.py

@@ -68,75 +68,78 @@ def get(group_name):
     print_group_lines(result)
 
 
-@groups.command(short_help='Add a member to a group')
+@groups.command(short_help='Add one or more members to a group')
 @click.argument('group_name')
 @click.argument('username')
+@click.argument('usernames', nargs=-1)
 @click.option('--no-subscribe', is_flag=True, default=False,
-              help='Do not subscribe the member to any auxiliary mailing lists.')
-def addmember(group_name, username, no_subscribe):
-    click.confirm(f'Are you sure you want to add {username} to {group_name}?',
-                  abort=True)
+              help='Do not subscribe the member(s) to any auxiliary mailing lists.')
+def addmember(group_name, username, usernames, no_subscribe):
+    usernames = [username, *usernames]
+    if len(usernames) == 1:
+        click.confirm(f'Are you sure you want to add {username} to {group_name}?',
+                      abort=True)
+    else:
+        click.echo(f'The following users will be added to {group_name}:')
+        click.echo(', '.join(usernames))
+        click.confirm('Do you want to continue?', abort=True)
     base_domain = component.getUtility(IConfig).get('base_domain')
-    url = f'/api/groups/{group_name}/members/{username}'
     operations = AddMemberToGroupTransaction.operations
-
     if no_subscribe:
-        url += '?subscribe_to_lists=false'
         operations.remove('subscribe_user_to_auxiliary_mailing_lists')
-    resp = http_post(url)
-    data = handle_stream_response(resp, operations)
-    result = data[-1]['result']
-    lines = []
-    for i, group in enumerate(result['added_to_groups']):
-        if i == 0:
-            prefix = 'Added to groups'
-        else:
-            prefix = ''
-        lines.append((prefix, group))
-    for i, mailing_list in enumerate(result.get('subscribed_to_lists', [])):
-        if i == 0:
-            prefix = 'Subscribed to lists'
-        else:
-            prefix = ''
-        if '@' not in mailing_list:
-            mailing_list += '@' + base_domain
-        lines.append((prefix, mailing_list))
-    print_colon_kv(lines)
-
-
-@groups.command(short_help='Remove a member from a group')
+    for username in usernames:
+        url = f'/api/groups/{group_name}/members/{username}'
+        if no_subscribe:
+            url += '?subscribe_to_lists=false'
+        resp = http_post(url)
+        data = handle_stream_response(resp, operations)
+        result = data[-1]['result']
+        click.echo(f'Added {username} to ' + ', '.join(result['added_to_groups']))
+        if result.get('subscribed_to_lists'):
+            mailing_lists = [
+                mailing_list + '@' + base_domain
+                if '@' not in mailing_list
+                else mailing_list
+                for mailing_list in result['subscribed_to_lists']
+            ]
+            click.echo(f'Subscribed {username} to ' + ', '.join(mailing_lists))
+
+
+@groups.command(short_help='Remove one or more members from a group')
 @click.argument('group_name')
 @click.argument('username')
+@click.argument('usernames', nargs=-1)
 @click.option('--no-unsubscribe', is_flag=True, default=False,
-              help='Do not unsubscribe the member from any auxiliary mailing lists.')
-def removemember(group_name, username, no_unsubscribe):
-    click.confirm(f'Are you sure you want to remove {username} from {group_name}?',
-                  abort=True)
+              help='Do not unsubscribe the member(s) from any auxiliary mailing lists.')
+def removemember(group_name, username, usernames, no_unsubscribe):
+    usernames = [username, *usernames]
+    if len(usernames) == 1:
+        click.confirm(f'Are you sure you want to remove {username} from {group_name}?',
+                      abort=True)
+    else:
+        click.echo(f'The following users will be removed from {group_name}:')
+        click.echo(', '.join(usernames))
+        click.confirm('Do you want to continue?', abort=True)
     base_domain = component.getUtility(IConfig).get('base_domain')
-    url = f'/api/groups/{group_name}/members/{username}'
     operations = RemoveMemberFromGroupTransaction.operations
     if no_unsubscribe:
-        url += '?unsubscribe_from_lists=false'
         operations.remove('unsubscribe_user_from_auxiliary_mailing_lists')
-    resp = http_delete(url)
-    data = handle_stream_response(resp, operations)
-    result = data[-1]['result']
-    lines = []
-    for i, group in enumerate(result['removed_from_groups']):
-        if i == 0:
-            prefix = 'Removed from groups'
-        else:
-            prefix = ''
-        lines.append((prefix, group))
-    for i, mailing_list in enumerate(result.get('unsubscribed_from_lists', [])):
-        if i == 0:
-            prefix = 'Unsubscribed from lists'
-        else:
-            prefix = ''
-        if '@' not in mailing_list:
-            mailing_list += '@' + base_domain
-        lines.append((prefix, mailing_list))
-    print_colon_kv(lines)
+    for username in usernames:
+        url = f'/api/groups/{group_name}/members/{username}'
+        if no_unsubscribe:
+            url += '?unsubscribe_from_lists=false'
+        resp = http_delete(url)
+        data = handle_stream_response(resp, operations)
+        result = data[-1]['result']
+        click.echo(f'Removed {username} from ' + ', '.join(result['removed_from_groups']))
+        if result.get('unsubscribed_from_lists'):
+            mailing_lists = [
+                mailing_list + '@' + base_domain
+                if '@' not in mailing_list
+                else mailing_list
+                for mailing_list in result['unsubscribed_from_lists']
+            ]
+            click.echo(f'Unsubscribed {username} from ' + ', '.join(mailing_lists))
 
 
 @groups.command(short_help='Delete a group')

tests/ceo/cli/test_groups.py

@@ -47,7 +47,7 @@ def test_groups(cli_setup, ldap_user):
         "Add user to auxiliary groups... Skipped\n"
         "Subscribe user to auxiliary mailing lists... Skipped\n"
         "Transaction successfully completed.\n"
-        "Added to groups: test_group_1\n"
+        f"Added {ldap_user.uid} to test_group_1\n"
     )
     assert result.exit_code == 0
     assert result.output == expected
@@ -68,7 +68,7 @@ def test_groups(cli_setup, ldap_user):
         "Remove user from auxiliary groups... Skipped\n"
         "Unsubscribe user from auxiliary mailing lists... Skipped\n"
         "Transaction successfully completed.\n"
-        "Removed from groups: test_group_1\n"
+        f"Removed {ldap_user.uid} from test_group_1\n"
     )
     assert result.exit_code == 0
     assert result.output == expected
@@ -92,6 +92,28 @@ def delete_group(group_name):
     assert result.exit_code == 0
 
 
+def test_groups_multiple_members(cli_setup, new_user_gen):
+    runner = CliRunner()
+    create_group('test_group_1', 'Test Group 1')
+    with new_user_gen() as user1, new_user_gen() as user2:
+        result = runner.invoke(cli, [
+            'groups', 'addmember', 'test_group_1', user1.uid, user2.uid
+        ], input='y\n')
+        assert result.exit_code == 0
+        lines = result.output.splitlines()
+        assert f'Added {user1.uid} to test_group_1' in lines
+        assert f'Added {user2.uid} to test_group_1' in lines
+
+        result = runner.invoke(cli, [
+            'groups', 'removemember', 'test_group_1', user1.uid, user2.uid
+        ], input='y\n')
+        assert result.exit_code == 0
+        lines = result.output.splitlines()
+        assert f'Removed {user1.uid} from test_group_1' in lines
+        assert f'Removed {user2.uid} from test_group_1' in lines
+    delete_group('test_group_1')
+
+
 def test_groups_with_auxiliary_groups_and_mailing_lists(cli_setup, ldap_user):
     runner = CliRunner()
     # make sure auxiliary groups + mailing lists exist in ceod_test_local.ini
@@ -109,11 +131,8 @@ def test_groups_with_auxiliary_groups_and_mailing_lists(cli_setup, ldap_user):
         "Add user to auxiliary groups... Done\n"
         "Subscribe user to auxiliary mailing lists... Done\n"
         "Transaction successfully completed.\n"
-        "Added to groups:     syscom\n"
-        "                     office\n"
-        "                     staff\n"
-        "Subscribed to lists: syscom@csclub.internal\n"
-        "                     syscom-alerts@csclub.internal\n"
+        f"Added {ldap_user.uid} to syscom, office, staff\n"
+        f"Subscribed {ldap_user.uid} to syscom@csclub.internal, syscom-alerts@csclub.internal\n"
     )
     assert result.exit_code == 0
     assert result.output == expected
@@ -128,11 +147,8 @@ def test_groups_with_auxiliary_groups_and_mailing_lists(cli_setup, ldap_user):
         "Remove user from auxiliary groups... Done\n"
         "Unsubscribe user from auxiliary mailing lists... Done\n"
         "Transaction successfully completed.\n"
-        "Removed from groups:     syscom\n"
-        "                         office\n"
-        "                         staff\n"
-        "Unsubscribed from lists: syscom@csclub.internal\n"
-        "                         syscom-alerts@csclub.internal\n"
+        f"Removed {ldap_user.uid} from syscom, office, staff\n"
+        f"Unsubscribed {ldap_user.uid} from syscom@csclub.internal, syscom-alerts@csclub.internal\n"
     )
     assert result.exit_code == 0
     assert result.output == expected
@@ -142,14 +158,14 @@ def test_groups_with_auxiliary_groups_and_mailing_lists(cli_setup, ldap_user):
         'groups', 'addmember', 'syscom', ldap_user.uid, '--no-subscribe',
     ], input='y\n')
     assert result.exit_code == 0
-    assert 'Subscribed to lists' not in result.output
+    assert 'Subscribed' not in result.output
 
     runner = CliRunner()
     result = runner.invoke(cli, [
         'groups', 'removemember', 'syscom', ldap_user.uid, '--no-unsubscribe',
     ], input='y\n')
     assert result.exit_code == 0
-    assert 'Unsubscribed from lists' not in result.output
+    assert 'Unsubscribed' not in result.output
 
     delete_group('syscom')
     delete_group('office')