Implement TUI support for multiple users in each position (#80)
All checks were successful
continuous-integration/drone/push Build is passing
All checks were successful
continuous-integration/drone/push Build is passing
Co-authored-by: Justin Chung <20733699+justin13888@users.noreply.github.com> Co-authored-by: Max Erenberg <merenber@csclub.uwaterloo.ca> Reviewed-on: #80 Co-authored-by: Justin Chung <j24chung@csclub.uwaterloo.ca> Co-committed-by: Justin Chung <j24chung@csclub.uwaterloo.ca>
This commit is contained in:
parent
f84965c8e1
commit
5e8f1b5ba5
@ -16,13 +16,22 @@ def positions():
|
||||
def get():
|
||||
resp = http_get('/api/positions')
|
||||
result = handle_sync_response(resp)
|
||||
print_colon_kv(result.items())
|
||||
print_colon_kv([
|
||||
(position, ', '.join(usernames))
|
||||
for position, usernames in result.items()
|
||||
])
|
||||
|
||||
|
||||
@positions.command(short_help='Update positions')
|
||||
def set(**kwargs):
|
||||
body = {k.replace('_', '-'): v for k, v in kwargs.items()}
|
||||
print_body = {k: v or '' for k, v in body.items()}
|
||||
body = {
|
||||
k.replace('_', '-'): v.replace(' ', '').split(',') if v else None
|
||||
for k, v in kwargs.items()
|
||||
}
|
||||
print_body = {
|
||||
k: ', '.join(v) if v else ''
|
||||
for k, v in body.items()
|
||||
}
|
||||
click.echo('The positions will be updated:')
|
||||
print_colon_kv(print_body.items())
|
||||
click.confirm('Do you want to continue?', abort=True)
|
||||
|
@ -19,8 +19,8 @@ class GetPositionsController(Controller):
|
||||
positions = tui_utils.handle_sync_response(resp, self)
|
||||
except Controller.RequestFailed:
|
||||
return
|
||||
for pos, username in positions.items():
|
||||
self.model.positions[pos] = username
|
||||
for pos, usernames in positions.items():
|
||||
self.model.positions[pos] = ','.join(usernames)
|
||||
|
||||
def target():
|
||||
self.view.flash_text.set_text('')
|
||||
|
@ -17,7 +17,7 @@ class SetPositionsController(Controller):
|
||||
body = {}
|
||||
for pos, field in self.view.position_fields.items():
|
||||
if field.edit_text != '':
|
||||
body[pos] = field.edit_text
|
||||
body[pos] = field.edit_text.replace(' ', '').split(',')
|
||||
model = TransactionModel(
|
||||
UpdateMemberPositionsTransaction.operations,
|
||||
'POST', '/api/positions', json=body
|
||||
@ -37,8 +37,8 @@ class SetPositionsController(Controller):
|
||||
positions = tui_utils.handle_sync_response(resp, self)
|
||||
except Controller.RequestFailed:
|
||||
return
|
||||
for pos, username in positions.items():
|
||||
self.model.positions[pos] = username
|
||||
for pos, usernames in positions.items():
|
||||
self.model.positions[pos] = ','.join(usernames)
|
||||
|
||||
def target():
|
||||
self.view.flash_text.set_text('')
|
||||
|
@ -87,6 +87,9 @@ def space_colon_kv(pairs: List[Tuple[str, str]]) -> List[str]:
|
||||
maxlen = max(len(key) for key, val in pairs)
|
||||
for key, val in pairs:
|
||||
if key != '':
|
||||
if not val:
|
||||
lines.append(key + ':')
|
||||
continue
|
||||
prefix = key + ': '
|
||||
else:
|
||||
# assume this is a continuation from the previous line
|
||||
|
@ -2,6 +2,7 @@ from flask import Blueprint, request
|
||||
from zope import component
|
||||
|
||||
from .utils import authz_restrict_to_syscom, create_streaming_response
|
||||
from ceo_common.errors import BadRequest
|
||||
from ceo_common.interfaces import ILDAPService, IConfig
|
||||
from ceod.transactions.members import UpdateMemberPositionsTransaction
|
||||
|
||||
@ -15,7 +16,9 @@ def get_positions():
|
||||
positions = {}
|
||||
for user in ldap_srv.get_users_with_positions():
|
||||
for position in user.positions:
|
||||
positions[position] = user.uid
|
||||
if position not in positions:
|
||||
positions[position] = []
|
||||
positions[position].append(user.uid)
|
||||
|
||||
return positions
|
||||
|
||||
@ -29,23 +32,31 @@ def update_positions():
|
||||
required = cfg.get('positions_required')
|
||||
available = cfg.get('positions_available')
|
||||
|
||||
# remove falsy values
|
||||
body = {
|
||||
positions: username for positions, username in body.items()
|
||||
if username
|
||||
}
|
||||
# remove falsy values and parse multiple users in each position
|
||||
# Example: "user1,user2, user3" -> ["user1","user2","user3"]
|
||||
position_to_usernames = {}
|
||||
for position, usernames in body.items():
|
||||
if not usernames:
|
||||
continue
|
||||
if type(usernames) is list:
|
||||
position_to_usernames[position] = usernames
|
||||
elif type(usernames) is str:
|
||||
position_to_usernames[position] = usernames.replace(' ', '').split(',')
|
||||
else:
|
||||
raise BadRequest('usernames must be a list or comma-separated string')
|
||||
|
||||
for position in body.keys():
|
||||
# check for duplicates (i.e. one username specified twice in the same list)
|
||||
for usernames in position_to_usernames.values():
|
||||
if len(usernames) != len(set(usernames)):
|
||||
raise BadRequest('username may only be specified at most once for a position')
|
||||
|
||||
for position in position_to_usernames.keys():
|
||||
if position not in available:
|
||||
return {
|
||||
'error': f'unknown position: {position}'
|
||||
}, 400
|
||||
raise BadRequest(f'unknown position: {position}')
|
||||
|
||||
for position in required:
|
||||
if position not in body:
|
||||
return {
|
||||
'error': f'missing required position: {position}'
|
||||
}, 400
|
||||
if position not in position_to_usernames:
|
||||
raise BadRequest(f'missing required position: {position}')
|
||||
|
||||
txn = UpdateMemberPositionsTransaction(body)
|
||||
txn = UpdateMemberPositionsTransaction(position_to_usernames)
|
||||
return create_streaming_response(txn)
|
||||
|
@ -1,5 +1,5 @@
|
||||
from collections import defaultdict
|
||||
from typing import Dict
|
||||
from typing import Dict, List
|
||||
|
||||
from zope import component
|
||||
|
||||
@ -20,15 +20,19 @@ class UpdateMemberPositionsTransaction(AbstractTransaction):
|
||||
'subscribe_to_mailing_lists',
|
||||
]
|
||||
|
||||
def __init__(self, positions_reversed: Dict[str, str]):
|
||||
def __init__(self, position_to_usernames: Dict[str, List[str]]):
|
||||
# positions_reversed is position -> username
|
||||
super().__init__()
|
||||
self.ldap_srv = component.getUtility(ILDAPService)
|
||||
|
||||
# Reverse the dict so it's easier to use (username -> positions)
|
||||
self.positions = defaultdict(list)
|
||||
for position, username in positions_reversed.items():
|
||||
self.positions[username].append(position)
|
||||
for position, usernames in position_to_usernames.items():
|
||||
if isinstance(usernames, list):
|
||||
for username in usernames:
|
||||
self.positions[username].append(position)
|
||||
else:
|
||||
raise TypeError("Username(s) under each position must be a list")
|
||||
|
||||
# a cached Dict of the Users who need to be modified (username -> User)
|
||||
self.users: Dict[str, IUser] = {}
|
||||
@ -42,7 +46,7 @@ class UpdateMemberPositionsTransaction(AbstractTransaction):
|
||||
mailing_lists = cfg.get('auxiliary mailing lists_exec')
|
||||
|
||||
# position -> username
|
||||
new_positions_reversed = {} # For returning result
|
||||
new_position_to_usernames = {} # For returning result
|
||||
|
||||
# retrieve User objects and cache them
|
||||
for username in self.positions:
|
||||
@ -64,7 +68,9 @@ class UpdateMemberPositionsTransaction(AbstractTransaction):
|
||||
|
||||
self.old_positions[username] = old_positions
|
||||
for position in new_positions:
|
||||
new_positions_reversed[position] = username
|
||||
if position not in new_position_to_usernames:
|
||||
new_position_to_usernames[position] = []
|
||||
new_position_to_usernames[position].append(username)
|
||||
yield 'update_positions_ldap'
|
||||
|
||||
# update exec group in LDAP
|
||||
@ -97,7 +103,7 @@ class UpdateMemberPositionsTransaction(AbstractTransaction):
|
||||
else:
|
||||
yield 'subscribe_to_mailing_lists'
|
||||
|
||||
self.finish(new_positions_reversed)
|
||||
self.finish(new_position_to_usernames)
|
||||
|
||||
def rollback(self):
|
||||
if 'update_exec_group_ldap' in self.finished_operations:
|
||||
|
@ -4,6 +4,8 @@ x-common: &common
|
||||
image: python:3.9-bullseye
|
||||
volumes:
|
||||
- .:$PWD:z
|
||||
security_opt:
|
||||
- label:disable
|
||||
environment:
|
||||
FLASK_APP: ceod.api
|
||||
FLASK_ENV: development
|
||||
|
@ -61,9 +61,9 @@ paths:
|
||||
program:
|
||||
$ref: "#/components/schemas/Program"
|
||||
terms:
|
||||
$ref: "#/components/schemas/Terms"
|
||||
$ref: "#/components/schemas/TermsOrNumTerms"
|
||||
non_member_terms:
|
||||
$ref: "#/components/schemas/NonMemberTerms"
|
||||
$ref: "#/components/schemas/TermsOrNumTerms"
|
||||
forwarding_addresses:
|
||||
$ref: "#/components/schemas/ForwardingAddresses"
|
||||
responses:
|
||||
@ -161,17 +161,11 @@ paths:
|
||||
- type: object
|
||||
properties:
|
||||
terms:
|
||||
type: array
|
||||
description: Terms for which this user will be a member
|
||||
items:
|
||||
$ref: "#/components/schemas/Term"
|
||||
$ref: "#/components/schemas/TermsOrNumTerms"
|
||||
- type: object
|
||||
properties:
|
||||
non_member_terms:
|
||||
type: array
|
||||
description: Terms for which this user will be a club rep
|
||||
items:
|
||||
$ref: "#/components/schemas/Term"
|
||||
$ref: "#/components/schemas/TermsOrNumTerms"
|
||||
example: {"terms": ["f2021"]}
|
||||
responses:
|
||||
"200":
|
||||
@ -383,11 +377,14 @@ paths:
|
||||
schema:
|
||||
type: object
|
||||
additionalProperties:
|
||||
type: string
|
||||
type: array
|
||||
description: list of usernames
|
||||
items:
|
||||
type: string
|
||||
example:
|
||||
president: user0
|
||||
vice-president: user1
|
||||
sysadmin: user2
|
||||
president: ["user1"]
|
||||
vice-president: ["user2", "user3"]
|
||||
sysadmin: ["user4"]
|
||||
treasurer:
|
||||
post:
|
||||
tags: ['positions']
|
||||
@ -404,11 +401,18 @@ paths:
|
||||
schema:
|
||||
type: object
|
||||
additionalProperties:
|
||||
type: string
|
||||
oneOf:
|
||||
- type: string
|
||||
description: username or comma-separated list of usernames
|
||||
- type: array
|
||||
description: list of usernames
|
||||
items:
|
||||
type: string
|
||||
example:
|
||||
president: user0
|
||||
vice-president: user1
|
||||
sysadmin: user2
|
||||
president: user1
|
||||
vice-president: user2, user3
|
||||
secretary: ["user4", "user5"]
|
||||
sysadmin: ["user6"]
|
||||
treasurer:
|
||||
responses:
|
||||
"200":
|
||||
@ -422,7 +426,7 @@ paths:
|
||||
{"status": "in progress", "operation": "update_positions_ldap"}
|
||||
{"status": "in progress", "operation": "update_exec_group_ldap"}
|
||||
{"status": "in progress", "operation": "subscribe_to_mailing_list"}
|
||||
{"status": "completed", "result": "OK"}
|
||||
{"status": "completed", "result": {"president": ["user1"],"vice-president": ["user2", "user3"],"secretary": ["user4". "user5"],"sysadmin": ["user6"]}}
|
||||
"400":
|
||||
description: Failed
|
||||
content:
|
||||
@ -883,14 +887,15 @@ components:
|
||||
example: MAT/Mathematics Computer Science
|
||||
Terms:
|
||||
type: array
|
||||
description: Terms for which this user was a member
|
||||
items:
|
||||
$ref: "#/components/schemas/Term"
|
||||
NonMemberTerms:
|
||||
type: array
|
||||
description: Terms for which this user was a club rep
|
||||
description: List of terms
|
||||
items:
|
||||
$ref: "#/components/schemas/Term"
|
||||
TermsOrNumTerms:
|
||||
oneOf:
|
||||
- type: integer
|
||||
description: number of additional terms to add
|
||||
example: 1
|
||||
- $ref: "#/components/schemas/Terms"
|
||||
LoginShell:
|
||||
type: string
|
||||
description: Login shell
|
||||
@ -939,7 +944,7 @@ components:
|
||||
terms:
|
||||
$ref: "#/components/schemas/Terms"
|
||||
non_member_terms:
|
||||
$ref: "#/components/schemas/NonMemberTerms"
|
||||
$ref: "#/components/schemas/Terms"
|
||||
forwarding_addresses:
|
||||
$ref: "#/components/schemas/ForwardingAddresses"
|
||||
groups:
|
||||
|
File diff suppressed because one or more lines are too long
@ -6,7 +6,7 @@ gunicorn==20.1.0
|
||||
Jinja2==3.1.2
|
||||
ldap3==2.9.1
|
||||
mysql-connector-python==8.0.26
|
||||
psycopg2==2.9.1
|
||||
psycopg2-binary==2.9.1
|
||||
python-augeas==1.1.0
|
||||
requests==2.26.0
|
||||
requests-gssapi==1.2.3
|
||||
|
@ -1,5 +1,6 @@
|
||||
import os
|
||||
import shutil
|
||||
import time
|
||||
|
||||
from click.testing import CliRunner
|
||||
from mysql.connector import connect
|
||||
@ -10,10 +11,15 @@ from ceo.cli import cli
|
||||
|
||||
|
||||
def mysql_attempt_connection(host, username, password):
|
||||
# Sometimes, when running the tests locally, I've observed a race condition
|
||||
# where another client can't "see" a database right after we create it.
|
||||
# I only observed this after upgrading the containers to bullseye (MariaDB
|
||||
# version: 10.5.18).
|
||||
time.sleep(0.05)
|
||||
with connect(
|
||||
host=host,
|
||||
user=username,
|
||||
password=password,
|
||||
host=host,
|
||||
user=username,
|
||||
password=password,
|
||||
) as con, con.cursor() as cur:
|
||||
cur.execute("SHOW DATABASES")
|
||||
response = cur.fetchall()
|
||||
@ -34,7 +40,7 @@ def test_mysql(cli_setup, cfg, ldap_user):
|
||||
|
||||
# create database for user
|
||||
result = runner.invoke(cli, ['mysql', 'create', username], input='y\n')
|
||||
print(result.output)
|
||||
#print(result.output) # noqa: E265
|
||||
assert result.exit_code == 0
|
||||
assert os.path.isfile(info_file_path)
|
||||
|
||||
|
@ -26,9 +26,9 @@ def test_members_get(cli_setup, ldap_user):
|
||||
f"home directory: {ldap_user.home_directory}\n"
|
||||
f"is a club: {ldap_user.is_club()}\n"
|
||||
f"is a club rep: {ldap_user.is_club_rep}\n"
|
||||
f"forwarding addresses: \n"
|
||||
f"forwarding addresses:\n"
|
||||
f"member terms: {','.join(ldap_user.terms)}\n"
|
||||
f"groups: \n"
|
||||
f"groups:\n"
|
||||
)
|
||||
assert result.exit_code == 0
|
||||
assert result.output == expected
|
||||
|
@ -44,17 +44,17 @@ vice-president: test_1
|
||||
sysadmin: test_2
|
||||
secretary: test_3
|
||||
webmaster: test_4
|
||||
treasurer:
|
||||
cro:
|
||||
librarian:
|
||||
imapd:
|
||||
offsck:
|
||||
treasurer:
|
||||
cro:
|
||||
librarian:
|
||||
imapd:
|
||||
offsck:
|
||||
Do you want to continue? [y/N]: y
|
||||
Update positions in LDAP... Done
|
||||
Update executive group in LDAP... Done
|
||||
Subscribe to mailing lists... Done
|
||||
Transaction successfully completed.
|
||||
'''[1:] # noqa: W291
|
||||
'''[1:]
|
||||
|
||||
result = runner.invoke(cli, ['positions', 'get'])
|
||||
assert result.exit_code == 0
|
||||
@ -71,3 +71,71 @@ webmaster: test_4
|
||||
for user in users:
|
||||
user.remove_from_ldap()
|
||||
group.remove_from_ldap()
|
||||
|
||||
|
||||
def test_positions_multiple_users(cli_setup, g_admin_ctx):
|
||||
runner = CliRunner()
|
||||
|
||||
# Setup test data
|
||||
users = []
|
||||
with g_admin_ctx():
|
||||
for i in range(5):
|
||||
user = User(
|
||||
uid=f'test_{i}',
|
||||
cn=f'Test {i}',
|
||||
given_name='Test',
|
||||
sn=str(i),
|
||||
program='Math',
|
||||
terms=['w2023'],
|
||||
)
|
||||
user.add_to_ldap()
|
||||
users.append(user)
|
||||
group = Group(
|
||||
cn='exec',
|
||||
description='Test Group',
|
||||
gid_number=10500,
|
||||
)
|
||||
group.add_to_ldap()
|
||||
|
||||
result = runner.invoke(cli, [
|
||||
'positions', 'set',
|
||||
'--president', 'test_0',
|
||||
'--vice-president', 'test_1,test_2',
|
||||
'--sysadmin', 'test_2',
|
||||
'--secretary', 'test_3, test_4, test_2',
|
||||
], input='y\n')
|
||||
|
||||
assert result.exit_code == 0
|
||||
assert result.output == '''
|
||||
The positions will be updated:
|
||||
president: test_0
|
||||
vice-president: test_1, test_2
|
||||
sysadmin: test_2
|
||||
secretary: test_3, test_4, test_2
|
||||
treasurer:
|
||||
cro:
|
||||
librarian:
|
||||
imapd:
|
||||
webmaster:
|
||||
offsck:
|
||||
Do you want to continue? [y/N]: y
|
||||
Update positions in LDAP... Done
|
||||
Update executive group in LDAP... Done
|
||||
Subscribe to mailing lists... Done
|
||||
Transaction successfully completed.
|
||||
'''[1:]
|
||||
|
||||
result = runner.invoke(cli, ['positions', 'get'])
|
||||
assert result.exit_code == 0
|
||||
assert result.output == '''
|
||||
president: test_0
|
||||
secretary: test_2, test_3, test_4
|
||||
sysadmin: test_2
|
||||
vice-president: test_1, test_2
|
||||
'''[1:]
|
||||
|
||||
# Cleanup test data
|
||||
with g_admin_ctx():
|
||||
for user in users:
|
||||
user.remove_from_ldap()
|
||||
group.remove_from_ldap()
|
||||
|
@ -7,8 +7,8 @@ def test_get_positions(client, ldap_user, g_admin_ctx):
|
||||
status, data = client.get('/api/positions')
|
||||
assert status == 200
|
||||
expected = {
|
||||
'president': ldap_user.uid,
|
||||
'treasurer': ldap_user.uid,
|
||||
'president': [ldap_user.uid],
|
||||
'treasurer': [ldap_user.uid],
|
||||
}
|
||||
assert data == expected
|
||||
|
||||
@ -20,7 +20,7 @@ def test_set_positions(cfg, client, g_admin_ctx, mock_mailman_server):
|
||||
|
||||
users = []
|
||||
with g_admin_ctx():
|
||||
for uid in ['test_1', 'test_2', 'test_3', 'test_4']:
|
||||
for uid in ['test1', 'test2', 'test3', 'test4']:
|
||||
user = User(uid=uid, cn='Some Name', given_name='Some', sn='Name',
|
||||
terms=['s2021'])
|
||||
user.add_to_ldap()
|
||||
@ -31,23 +31,23 @@ def test_set_positions(cfg, client, g_admin_ctx, mock_mailman_server):
|
||||
try:
|
||||
# missing required position
|
||||
status, _ = client.post('/api/positions', json={
|
||||
'vice-president': 'test_1',
|
||||
'vice-president': 'test1',
|
||||
})
|
||||
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',
|
||||
'president': 'test1',
|
||||
'vice-president': 'test2',
|
||||
'sysadmin': 'test3',
|
||||
'no-such-position': 'test3',
|
||||
})
|
||||
assert status == 400
|
||||
|
||||
status, data = client.post('/api/positions', json={
|
||||
'president': 'test_1',
|
||||
'vice-president': 'test_2',
|
||||
'sysadmin': 'test_3',
|
||||
'president': 'test1',
|
||||
'vice-president': 'test2',
|
||||
'sysadmin': 'test3',
|
||||
})
|
||||
assert status == 200
|
||||
expected = [
|
||||
@ -55,16 +55,16 @@ def test_set_positions(cfg, client, g_admin_ctx, mock_mailman_server):
|
||||
{"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",
|
||||
"president": ["test1"],
|
||||
"vice-president": ["test2"],
|
||||
"sysadmin": ["test3"],
|
||||
}},
|
||||
]
|
||||
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']
|
||||
expected = ['test1', 'test2', 'test3']
|
||||
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]
|
||||
@ -72,20 +72,33 @@ def test_set_positions(cfg, client, g_admin_ctx, mock_mailman_server):
|
||||
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',
|
||||
'president': 'test1',
|
||||
'vice-president': 'test2',
|
||||
'sysadmin': 'test2',
|
||||
'treasurer': 'test4',
|
||||
})
|
||||
assert data[-1]['status'] == 'completed'
|
||||
# make sure old exec was removed from group
|
||||
expected = ['test_1', 'test_2', 'test_4']
|
||||
expected = ['test1', 'test2', 'test4']
|
||||
_, 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
|
||||
|
||||
# multiple users per position
|
||||
status, data = client.post('/api/positions', json={
|
||||
'president': 'test1',
|
||||
'vice-president': ['test2', 'test3'],
|
||||
'sysadmin': 'test2, test3,test4',
|
||||
})
|
||||
assert status == 200
|
||||
assert data[-1] == {'status': 'completed', 'result': {
|
||||
'president': ['test1'],
|
||||
'vice-president': ['test2', 'test3'],
|
||||
'sysadmin': ['test2', 'test3', 'test4'],
|
||||
}}
|
||||
finally:
|
||||
with g_admin_ctx():
|
||||
for user in users:
|
||||
|
Loading…
x
Reference in New Issue
Block a user