From b533fbfbadf384ad3e61999b0b84c0195317bc7f Mon Sep 17 00:00:00 2001 From: Lucas Ellenberger Date: Tue, 3 Sep 2024 10:13:30 -0700 Subject: [PATCH 01/12] Init users/upsert. --- autograder/api/config.py | 2 +- autograder/api/constants.py | 10 +++- autograder/api/users/upsert.py | 27 +++++++++++ autograder/cli/common.py | 3 ++ autograder/cli/users/upsert.py | 83 ++++++++++++++++++++++++++++++++++ 5 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 autograder/api/users/upsert.py create mode 100644 autograder/cli/users/upsert.py diff --git a/autograder/api/config.py b/autograder/api/config.py index e6096e6a..fb2db27c 100644 --- a/autograder/api/config.py +++ b/autograder/api/config.py @@ -209,7 +209,7 @@ def get_argument_parser( 'Only show results from users with this role (all roles if unknown (default)).', required = False, parser_options = {'action': 'store', 'default': 'unknown', - 'choices': autograder.api.constants.ROLES}) + 'choices': autograder.api.constants.COURSE_ROLES}) PARAM_FORCE = APIParam('force', 'Force the operation, overwriting and existing resources.', diff --git a/autograder/api/constants.py b/autograder/api/constants.py index bce70616..3a01e2f4 100644 --- a/autograder/api/constants.py +++ b/autograder/api/constants.py @@ -6,7 +6,15 @@ API_RESPONSE_KEY_MESSAGE = 'message' API_RESPONSE_KEY_CONTENT = API_REQUEST_JSON_KEY -ROLES = [ +SERVER_ROLES = [ + 'unknown', + 'user', + 'creator', + 'admin', + 'owner', +] + +COURSE_ROLES = [ 'unknown', 'other', 'student', diff --git a/autograder/api/users/upsert.py b/autograder/api/users/upsert.py new file mode 100644 index 00000000..fef32133 --- /dev/null +++ b/autograder/api/users/upsert.py @@ -0,0 +1,27 @@ +import autograder.api.common +import autograder.api.config + +API_ENDPOINT = 'users/upsert' +API_PARAMS = [ + autograder.api.config.PARAM_USER_EMAIL, + autograder.api.config.PARAM_USER_PASS, + + autograder.api.config.PARAM_DRY_RUN, + autograder.api.config.PARAM_SKIP_EMAILS, + + autograder.api.config.APIParam('raw-users', + 'A list of users to upsert.', + required = True, cli_param = False), +] + +DESCRIPTION = 'Upsert one or more users to the server (Update if exists, otherwise insert).' + +def send(arguments, **kwargs): + return autograder.api.common.handle_api_request(arguments, API_PARAMS, API_ENDPOINT, **kwargs) + +def _get_parser(): + parser = autograder.api.config.get_argument_parser( + description = DESCRIPTION, + params = API_PARAMS) + + return parser diff --git a/autograder/cli/common.py b/autograder/cli/common.py index 91b40da7..3604d007 100644 --- a/autograder/cli/common.py +++ b/autograder/cli/common.py @@ -69,3 +69,6 @@ def list_add_users(result, table = False): error['index'], error['email'], error['message'])) list_sync_users(result, table = table) + +def list_user_op_results(result, table = False): + print(result) diff --git a/autograder/cli/users/upsert.py b/autograder/cli/users/upsert.py new file mode 100644 index 00000000..ca6769fc --- /dev/null +++ b/autograder/cli/users/upsert.py @@ -0,0 +1,83 @@ +import sys + +import autograder.api.config +import autograder.api.users.upsert +import autograder.cli.common + +def run(arguments): + arguments = vars(arguments) + + password = arguments['new-pass'] + if (password != ''): + password = autograder.util.hash.sha256_hex(password) + + arguments['raw-users'] = [{ + 'email': arguments['new-email'], + 'name': arguments['new-name'], + 'role': arguments['new-role'], + 'pass': password, + 'course': arguments['new-course'], + 'course-role': arguments['new-course-role'], + 'course-lms-id': arguments['new-lms-id'], + }] + + arguments['send-emails'] = ~arguments['skip-emails'] + + result = autograder.api.users.upsert.send(arguments, exit_on_error = True) + + autograder.cli.common.list_user_op_results(result, table = arguments['table']) + return 0 + +def main(): + return run(_get_parser().parse_args()) + +def _get_parser(): + parser = autograder.api.users.upsert._get_parser() + + parser.add_argument('--skip-inserts', dest = 'skip-inserts', + action = 'store_true', default = False, + help = 'Skip inserts (default: %(default)s).') + + parser.add_argument('--skip-updates', dest = 'skip-updates', + action = 'store_true', default = False, + help = 'Skip updates (default: %(default)s).') + + parser.add_argument('--new-email', dest = 'new-email', + action = 'store', type = str, required = True, + help = 'The email of the user to upsert.') + + parser.add_argument('--new-name', dest = 'new-name', + action = 'store', type = str, default = '', + help = 'The name of the user to upsert.') + + parser.add_argument('--new-role', dest = 'new-role', + action = 'store', type = str, default = 'user', + choices = autograder.api.constants.SERVER_ROLES, + help = 'The role of the user to upsert (default: %(default)s).') + + parser.add_argument('--new-pass', dest = 'new-pass', + action = 'store', type = str, default = '', + help = 'The password of the user to upsert.' + + ' If empty, the server will generate and email a password.') + + parser.add_argument('--new-course', dest = 'new-course', + action = 'store', type = str, default = '', + help = 'The course of the user to upsert.') + + parser.add_argument('--new-course-role', dest = 'new-course-role', + action = 'store', type = str, default = 'student', + choices = autograder.api.constants.COURSE_ROLES, + help = 'The course role of the user to upsert (default: %(default)s).') + + parser.add_argument('--new-lms-id', dest = 'new-lms-id', + action = 'store', type = str, default = '', + help = 'The lms id of the user to upsert.') + + parser.add_argument('--table', dest = 'table', + action = 'store_true', default = False, + help = 'Output the results as a TSV table with a header (default: %(default)s).') + + return parser + +if (__name__ == '__main__'): + sys.exit(main()) From 961f9e6641edaf04d504f34671b2ceed976771d3 Mon Sep 17 00:00:00 2001 From: Lucas Ellenberger Date: Tue, 3 Sep 2024 16:00:01 -0700 Subject: [PATCH 02/12] Improved API params for upsert. --- autograder/api/users/upsert.py | 16 ++++++++++++++++ autograder/cli/users/upsert.py | 8 -------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/autograder/api/users/upsert.py b/autograder/api/users/upsert.py index fef32133..98b9f376 100644 --- a/autograder/api/users/upsert.py +++ b/autograder/api/users/upsert.py @@ -9,9 +9,25 @@ autograder.api.config.PARAM_DRY_RUN, autograder.api.config.PARAM_SKIP_EMAILS, + autograder.api.config.APIParam('skip-inserts', + 'Skip inserts (default: False).', + required = False, + parser_options = {'action': 'store_true', 'default': False} + ), + + autograder.api.config.APIParam('skip-updates', + 'Skip updates (default: False).', + required = False, + parser_options = {'action': 'store_true', 'default': False} + ), + autograder.api.config.APIParam('raw-users', 'A list of users to upsert.', required = True, cli_param = False), + + autograder.api.config.APIParam('send-emails', + 'Send any emails.', + required = True, cli_param = False), ] DESCRIPTION = 'Upsert one or more users to the server (Update if exists, otherwise insert).' diff --git a/autograder/cli/users/upsert.py b/autograder/cli/users/upsert.py index ca6769fc..95939315 100644 --- a/autograder/cli/users/upsert.py +++ b/autograder/cli/users/upsert.py @@ -34,14 +34,6 @@ def main(): def _get_parser(): parser = autograder.api.users.upsert._get_parser() - parser.add_argument('--skip-inserts', dest = 'skip-inserts', - action = 'store_true', default = False, - help = 'Skip inserts (default: %(default)s).') - - parser.add_argument('--skip-updates', dest = 'skip-updates', - action = 'store_true', default = False, - help = 'Skip updates (default: %(default)s).') - parser.add_argument('--new-email', dest = 'new-email', action = 'store', type = str, required = True, help = 'The email of the user to upsert.') From 9e00a4e19a314e9ece6cc5759289ec2ccdab494a Mon Sep 17 00:00:00 2001 From: Lucas Ellenberger Date: Wed, 4 Sep 2024 15:10:06 -0700 Subject: [PATCH 03/12] Added functionality to print user op results. --- autograder/api/constants.py | 1 - autograder/api/users/upsert.py | 1 - autograder/cli/common.py | 93 ++++++++++++++++++++++++++++++++-- autograder/cli/users/upsert.py | 9 +++- 4 files changed, 97 insertions(+), 7 deletions(-) diff --git a/autograder/api/constants.py b/autograder/api/constants.py index 3a01e2f4..7f243c8c 100644 --- a/autograder/api/constants.py +++ b/autograder/api/constants.py @@ -7,7 +7,6 @@ API_RESPONSE_KEY_CONTENT = API_REQUEST_JSON_KEY SERVER_ROLES = [ - 'unknown', 'user', 'creator', 'admin', diff --git a/autograder/api/users/upsert.py b/autograder/api/users/upsert.py index 98b9f376..c53f010e 100644 --- a/autograder/api/users/upsert.py +++ b/autograder/api/users/upsert.py @@ -7,7 +7,6 @@ autograder.api.config.PARAM_USER_PASS, autograder.api.config.PARAM_DRY_RUN, - autograder.api.config.PARAM_SKIP_EMAILS, autograder.api.config.APIParam('skip-inserts', 'Skip inserts (default: False).', diff --git a/autograder/cli/common.py b/autograder/cli/common.py index 3604d007..57f5df50 100644 --- a/autograder/cli/common.py +++ b/autograder/cli/common.py @@ -1,6 +1,8 @@ HEADERS = ['email', 'name', 'role', 'lms-id'] SYNC_HEADERS = HEADERS + ['operation'] +INDENT = ' ' + SYNC_USERS_KEYS = [ ('add-users', 'Added', 'add'), ('mod-users', 'Modified', 'mod'), @@ -8,6 +10,26 @@ ('skip-users', 'Skipped', 'skip'), ] +USER_OP_KEYS = [ + ('added', 'Added'), + ('modified', 'Modified'), + ('removed', 'Removed'), + ('skipped', 'Skipped'), + ('not-exists', 'Not Exists'), + ('emailed', 'Emailed'), + ('enrolled', 'Enrolled'), + ('dopped', 'Dropped'), +] + +USER_OP_ERROR_KEYS = [ + ('validation-errors', 'Validation Errors'), + ('system-errors', 'System Errors'), +] + +ALL_USER_OP_KEYS = [ + ('email', 'Email'), +] + USER_OP_KEYS + USER_OP_ERROR_KEYS + def list_users(users, table = False): if (table): _list_users_table(users) @@ -48,7 +70,7 @@ def _list_sync_users(sync_users): continue print("%s Users:" % (label)) - _list_users(users, indent = ' ') + _list_users(users, indent = INDENT) def _list_sync_users_table(sync_users): print("\t".join(SYNC_HEADERS)) @@ -70,5 +92,70 @@ def list_add_users(result, table = False): list_sync_users(result, table = table) -def list_user_op_results(result, table = False): - print(result) +def _list_user_op_results(results): + op_results = {} + for (key, label) in USER_OP_KEYS: + emails = [] + for result in results: + if (result.get(key, None) is not None): + emails.append(result['email']) + + if (len(emails) > 0): + op_results[label] = emails + + _print_user_op_results_from_dict(op_results) + + # Print all errors last so users can easily see them. + for result in results: + val_errors = result.get('validation-errors', None) + if ((val_errors is not None) and (len(val_errors) > 0)): + print("Encountered %d validation errors while operating on user: '%s'." % ( + len(val_errors), result['email'])) + + for i in range(len(val_errors)): + print(INDENT + "Index: %d, Message: '%s'." % (i, val_errors[i]['external-message'])) + + sys_errors = result.get('system-errors', None) + if ((sys_errors is not None) and (len(sys_errors) > 0)): + print("Encountered %d system errors while operating on user: '%s'." % ( + len(sys_errors), result['email'])) + + for i in range(len(sys_errors)): + print(INDENT + "Index: %d, Message: '%s'." % (i, sys_errors[i]['external-message'])) + +def _list_user_op_results_table(results, header = True, keys = ALL_USER_OP_KEYS): + rows = [] + for result in results: + # Clean the error messages into a better format. + for error_key, _ in USER_OP_ERROR_KEYS: + if (result.get(error_key, None) is not None): + result[error_key] = [error['external-message'] for error in result[error_key]] + + rows.append([result.get(key, '') for key, _ in keys]) + + _print_tsv(rows, header, [header_key for _, header_key in keys]) + +def list_user_op_results(results, table = False): + if (table): + _list_user_op_results_table(results) + else: + _list_user_op_results(results) + +def _print_user_op_results_from_dict(result): + lines = [] + for label, emails in result.items(): + lines.append(label) + for email in emails: + lines.append(INDENT + email) + + print("\n".join(lines)) + +def _print_tsv(rows, header, header_keys): + lines = [] + if (header): + lines.append("\t".join(header_keys)) + + for row in rows: + lines.append("\t".join([str(value) for value in row])) + + print("\n".join(lines)) diff --git a/autograder/cli/users/upsert.py b/autograder/cli/users/upsert.py index 95939315..3da824d3 100644 --- a/autograder/cli/users/upsert.py +++ b/autograder/cli/users/upsert.py @@ -21,11 +21,11 @@ def run(arguments): 'course-lms-id': arguments['new-lms-id'], }] - arguments['send-emails'] = ~arguments['skip-emails'] + arguments['send-emails'] = not arguments['skip-emails'] result = autograder.api.users.upsert.send(arguments, exit_on_error = True) - autograder.cli.common.list_user_op_results(result, table = arguments['table']) + autograder.cli.common.list_user_op_results(result['results'], table = arguments['table']) return 0 def main(): @@ -34,6 +34,11 @@ def main(): def _get_parser(): parser = autograder.api.users.upsert._get_parser() + parser.add_argument('--skip-emails', dest = 'skip-emails', + action = 'store_true', default = False, + help = 'Skip sending any emails. Be aware that this may result in inaccessible\ + information.') + parser.add_argument('--new-email', dest = 'new-email', action = 'store', type = str, required = True, help = 'The email of the user to upsert.') From 8f45d93495a2b70f50601e2800fb96d0d863d249 Mon Sep 17 00:00:00 2001 From: Lucas Ellenberger Date: Wed, 4 Sep 2024 15:54:29 -0700 Subject: [PATCH 04/12] Functional api tests. --- autograder/api/users/upsert.py | 8 ++--- tests/api/testdata/users_upsert_add.json | 28 +++++++++++++++++ tests/api/testdata/users_upsert_mod.json | 27 ++++++++++++++++ .../testdata/users_upsert_skip_inserts.json | 28 +++++++++++++++++ .../testdata/users_upsert_skip_updates.json | 28 +++++++++++++++++ .../api/testdata/users_upsert_val_error.json | 31 +++++++++++++++++++ 6 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 tests/api/testdata/users_upsert_add.json create mode 100644 tests/api/testdata/users_upsert_mod.json create mode 100644 tests/api/testdata/users_upsert_skip_inserts.json create mode 100644 tests/api/testdata/users_upsert_skip_updates.json create mode 100644 tests/api/testdata/users_upsert_val_error.json diff --git a/autograder/api/users/upsert.py b/autograder/api/users/upsert.py index c53f010e..f4caa11a 100644 --- a/autograder/api/users/upsert.py +++ b/autograder/api/users/upsert.py @@ -20,13 +20,13 @@ parser_options = {'action': 'store_true', 'default': False} ), - autograder.api.config.APIParam('raw-users', - 'A list of users to upsert.', - required = True, cli_param = False), - autograder.api.config.APIParam('send-emails', 'Send any emails.', required = True, cli_param = False), + + autograder.api.config.APIParam('raw-users', + 'A list of users to upsert.', + required = True, cli_param = False), ] DESCRIPTION = 'Upsert one or more users to the server (Update if exists, otherwise insert).' diff --git a/tests/api/testdata/users_upsert_add.json b/tests/api/testdata/users_upsert_add.json new file mode 100644 index 00000000..ea390c91 --- /dev/null +++ b/tests/api/testdata/users_upsert_add.json @@ -0,0 +1,28 @@ +{ + "module": "autograder.api.users.upsert", + "arguments": { + "user": "server-admin@test.edulinq.org", + "pass": "server-admin", + "send-emails": true, + "raw-users": [ + { + "email": "new-user@test.edulinq.org", + "name": "", + "role": "user", + "pass": "", + "course": "", + "course-role": "student", + "course-lms-id": "" + } + ] + }, + "output": { + "results": [ + { + "email": "new-user@test.edulinq.org", + "added": true, + "emailed": true + } + ] + } +} diff --git a/tests/api/testdata/users_upsert_mod.json b/tests/api/testdata/users_upsert_mod.json new file mode 100644 index 00000000..2f18c01d --- /dev/null +++ b/tests/api/testdata/users_upsert_mod.json @@ -0,0 +1,27 @@ +{ + "module": "autograder.api.users.upsert", + "arguments": { + "user": "server-admin@test.edulinq.org", + "pass": "server-admin", + "send-emails": true, + "raw-users": [ + { + "email": "server-user@test.edulinq.org", + "name": "", + "role": "creator", + "pass": "", + "course": "", + "course-role": "student", + "course-lms-id": "" + } + ] + }, + "output": { + "results": [ + { + "email": "server-user@test.edulinq.org", + "modified": true + } + ] + } +} diff --git a/tests/api/testdata/users_upsert_skip_inserts.json b/tests/api/testdata/users_upsert_skip_inserts.json new file mode 100644 index 00000000..9fdc46c6 --- /dev/null +++ b/tests/api/testdata/users_upsert_skip_inserts.json @@ -0,0 +1,28 @@ +{ + "module": "autograder.api.users.upsert", + "arguments": { + "user": "server-admin@test.edulinq.org", + "pass": "server-admin", + "send-emails": true, + "raw-users": [ + { + "email": "alt-new-user@test.edulinq.org", + "name": "", + "role": "user", + "pass": "", + "course": "", + "course-role": "student", + "course-lms-id": "" + } + ], + "skip-inserts": true + }, + "output": { + "results": [ + { + "email": "alt-new-user@test.edulinq.org", + "skipped": true + } + ] + } +} diff --git a/tests/api/testdata/users_upsert_skip_updates.json b/tests/api/testdata/users_upsert_skip_updates.json new file mode 100644 index 00000000..1ce13995 --- /dev/null +++ b/tests/api/testdata/users_upsert_skip_updates.json @@ -0,0 +1,28 @@ +{ + "module": "autograder.api.users.upsert", + "arguments": { + "user": "server-admin@test.edulinq.org", + "pass": "server-admin", + "send-emails": true, + "raw-users": [ + { + "email": "server-user@test.edulinq.org", + "name": "", + "role": "creator", + "pass": "", + "course": "", + "course-role": "student", + "course-lms-id": "" + } + ], + "skip-updates": true + }, + "output": { + "results": [ + { + "email": "server-user@test.edulinq.org", + "skipped": true + } + ] + } +} diff --git a/tests/api/testdata/users_upsert_val_error.json b/tests/api/testdata/users_upsert_val_error.json new file mode 100644 index 00000000..91e65d8d --- /dev/null +++ b/tests/api/testdata/users_upsert_val_error.json @@ -0,0 +1,31 @@ +{ + "module": "autograder.api.users.upsert", + "arguments": { + "user": "server-admin@test.edulinq.org", + "pass": "server-admin", + "send-emails": true, + "raw-users": [ + { + "email": "server-user@test.edulinq.org", + "name": "", + "role": "owner", + "pass": "", + "course": "", + "course-role": "student", + "course-lms-id": "" + } + ] + }, + "output": { + "results": [ + { + "email": "server-user@test.edulinq.org", + "validation-errors": [ + { + "external-message": "You have insufficient permissions for the requested operation." + } + ] + } + ] + } +} From 9b26b79778f7438c8576982fb39d943313e4838b Mon Sep 17 00:00:00 2001 From: Lucas Ellenberger Date: Wed, 4 Sep 2024 16:42:55 -0700 Subject: [PATCH 05/12] Init CLI tests with remote disconnected error. --- tests/cli/testdata/users/users_upsert_add.txt | 13 +++++++++++++ tests/cli/testdata/users/users_upsert_mod.txt | 12 ++++++++++++ .../testdata/users/users_upsert_skip_inserts.txt | 12 ++++++++++++ .../testdata/users/users_upsert_skip_updates.txt | 12 ++++++++++++ tests/cli/testdata/users/users_upsert_val_error.txt | 12 ++++++++++++ 5 files changed, 61 insertions(+) create mode 100644 tests/cli/testdata/users/users_upsert_add.txt create mode 100644 tests/cli/testdata/users/users_upsert_mod.txt create mode 100644 tests/cli/testdata/users/users_upsert_skip_inserts.txt create mode 100644 tests/cli/testdata/users/users_upsert_skip_updates.txt create mode 100644 tests/cli/testdata/users/users_upsert_val_error.txt diff --git a/tests/cli/testdata/users/users_upsert_add.txt b/tests/cli/testdata/users/users_upsert_add.txt new file mode 100644 index 00000000..4158b5ba --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_add.txt @@ -0,0 +1,13 @@ +{ + "cli": "autograder.cli.users.upsert", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "--new-email", "cli-new-user@test.edulinq.org" + ] +} +--- +Added + cli-new-user@test.edulinq.org +Emailed + cli-new-user@test.edulinq.org diff --git a/tests/cli/testdata/users/users_upsert_mod.txt b/tests/cli/testdata/users/users_upsert_mod.txt new file mode 100644 index 00000000..ab72ea57 --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_mod.txt @@ -0,0 +1,12 @@ +{ + "cli": "autograder.cli.users.upsert", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "--new-email", "server-user@test.edulinq.org", + "--new-role", "creator" + ] +} +--- +Modified + server-user@test.edulinq.org diff --git a/tests/cli/testdata/users/users_upsert_skip_inserts.txt b/tests/cli/testdata/users/users_upsert_skip_inserts.txt new file mode 100644 index 00000000..821686a7 --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_skip_inserts.txt @@ -0,0 +1,12 @@ +{ + "cli": "autograder.cli.users.upsert", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "--new-email", "alt-cli-user@test.edulinq.org", + "--skip-inserts" + ] +} +--- +Skipped + alt-cli-user@test.edulinq.org diff --git a/tests/cli/testdata/users/users_upsert_skip_updates.txt b/tests/cli/testdata/users/users_upsert_skip_updates.txt new file mode 100644 index 00000000..bdd66c89 --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_skip_updates.txt @@ -0,0 +1,12 @@ +{ + "cli": "autograder.cli.users.upsert", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "--new-email", "server-user@test.edulinq.org", + "--skip-updates" + ] +} +--- +Skipped + server-user@test.edulinq.org diff --git a/tests/cli/testdata/users/users_upsert_val_error.txt b/tests/cli/testdata/users/users_upsert_val_error.txt new file mode 100644 index 00000000..3ae1ccb0 --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_val_error.txt @@ -0,0 +1,12 @@ +{ + "cli": "autograder.cli.users.upsert", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "--new-email", "server-user@test.edulinq.org", + "--new-role", "owner" + ] +} +--- +Encountered 1 validation errors while operating on user: 'server-user@test.edulinq.org'. + Index: 0, Message: 'You have insufficient permissions for the requested operation.'. From 5722ef905dd8f76c4b20eb7cea2f086a78bd3ab1 Mon Sep 17 00:00:00 2001 From: Lucas Ellenberger Date: Thu, 5 Sep 2024 10:39:31 -0700 Subject: [PATCH 06/12] Trying to debug CLI tests. --- autograder/api/users/upsert.py | 6 ++++-- tests/api/testdata/users_upsert_mod.json | 1 - tests/api/testdata/users_upsert_skip_inserts.json | 1 - tests/api/testdata/users_upsert_skip_updates.json | 1 - tests/api/testdata/users_upsert_val_error.json | 1 - tests/cli/testdata/users/users_upsert_add.txt | 7 ++++--- tests/cli/testdata/users/users_upsert_skip_inserts.txt | 4 ++-- tests/cli/testdata/users/users_upsert_skip_updates.txt | 1 + 8 files changed, 11 insertions(+), 11 deletions(-) diff --git a/autograder/api/users/upsert.py b/autograder/api/users/upsert.py index f4caa11a..7c1c6916 100644 --- a/autograder/api/users/upsert.py +++ b/autograder/api/users/upsert.py @@ -22,11 +22,13 @@ autograder.api.config.APIParam('send-emails', 'Send any emails.', - required = True, cli_param = False), + required = False, cli_param = False + ), autograder.api.config.APIParam('raw-users', 'A list of users to upsert.', - required = True, cli_param = False), + required = True, cli_param = False + ), ] DESCRIPTION = 'Upsert one or more users to the server (Update if exists, otherwise insert).' diff --git a/tests/api/testdata/users_upsert_mod.json b/tests/api/testdata/users_upsert_mod.json index 2f18c01d..0306b88e 100644 --- a/tests/api/testdata/users_upsert_mod.json +++ b/tests/api/testdata/users_upsert_mod.json @@ -3,7 +3,6 @@ "arguments": { "user": "server-admin@test.edulinq.org", "pass": "server-admin", - "send-emails": true, "raw-users": [ { "email": "server-user@test.edulinq.org", diff --git a/tests/api/testdata/users_upsert_skip_inserts.json b/tests/api/testdata/users_upsert_skip_inserts.json index 9fdc46c6..cd8a0ab6 100644 --- a/tests/api/testdata/users_upsert_skip_inserts.json +++ b/tests/api/testdata/users_upsert_skip_inserts.json @@ -3,7 +3,6 @@ "arguments": { "user": "server-admin@test.edulinq.org", "pass": "server-admin", - "send-emails": true, "raw-users": [ { "email": "alt-new-user@test.edulinq.org", diff --git a/tests/api/testdata/users_upsert_skip_updates.json b/tests/api/testdata/users_upsert_skip_updates.json index 1ce13995..375ae58f 100644 --- a/tests/api/testdata/users_upsert_skip_updates.json +++ b/tests/api/testdata/users_upsert_skip_updates.json @@ -3,7 +3,6 @@ "arguments": { "user": "server-admin@test.edulinq.org", "pass": "server-admin", - "send-emails": true, "raw-users": [ { "email": "server-user@test.edulinq.org", diff --git a/tests/api/testdata/users_upsert_val_error.json b/tests/api/testdata/users_upsert_val_error.json index 91e65d8d..b77a8339 100644 --- a/tests/api/testdata/users_upsert_val_error.json +++ b/tests/api/testdata/users_upsert_val_error.json @@ -3,7 +3,6 @@ "arguments": { "user": "server-admin@test.edulinq.org", "pass": "server-admin", - "send-emails": true, "raw-users": [ { "email": "server-user@test.edulinq.org", diff --git a/tests/cli/testdata/users/users_upsert_add.txt b/tests/cli/testdata/users/users_upsert_add.txt index 4158b5ba..8d4ee1e0 100644 --- a/tests/cli/testdata/users/users_upsert_add.txt +++ b/tests/cli/testdata/users/users_upsert_add.txt @@ -3,11 +3,12 @@ "arguments": [ "--user", "server-admin@test.edulinq.org", "--pass", "server-admin", - "--new-email", "cli-new-user@test.edulinq.org" + "--new-email", "new-user@test.edulinq.org", + "--skip-emails", false ] } --- Added - cli-new-user@test.edulinq.org + new-user@test.edulinq.org Emailed - cli-new-user@test.edulinq.org + new-user@test.edulinq.org diff --git a/tests/cli/testdata/users/users_upsert_skip_inserts.txt b/tests/cli/testdata/users/users_upsert_skip_inserts.txt index 821686a7..61eef4d0 100644 --- a/tests/cli/testdata/users/users_upsert_skip_inserts.txt +++ b/tests/cli/testdata/users/users_upsert_skip_inserts.txt @@ -3,10 +3,10 @@ "arguments": [ "--user", "server-admin@test.edulinq.org", "--pass", "server-admin", - "--new-email", "alt-cli-user@test.edulinq.org", + "--new-email", "alt-new-user@test.edulinq.org", "--skip-inserts" ] } --- Skipped - alt-cli-user@test.edulinq.org + alt-new-user@test.edulinq.org diff --git a/tests/cli/testdata/users/users_upsert_skip_updates.txt b/tests/cli/testdata/users/users_upsert_skip_updates.txt index bdd66c89..573ec684 100644 --- a/tests/cli/testdata/users/users_upsert_skip_updates.txt +++ b/tests/cli/testdata/users/users_upsert_skip_updates.txt @@ -4,6 +4,7 @@ "--user", "server-admin@test.edulinq.org", "--pass", "server-admin", "--new-email", "server-user@test.edulinq.org", + "--new-role", "creator", "--skip-updates" ] } From f31a58a9c93a03bf24a14cf6e2db74de87992811 Mon Sep 17 00:00:00 2001 From: Lucas Ellenberger Date: Thu, 5 Sep 2024 13:25:55 -0700 Subject: [PATCH 07/12] Working CLI tests and bulk upsert. --- autograder/api/users/upsert.py | 12 +- autograder/cli/common.py | 6 +- autograder/cli/users/upsert-file.py | 114 ++++++++++++++++++ tests/api/testdata/users_upsert_add.json | 3 + tests/api/testdata/users_upsert_file_add.json | 60 +++++++++ .../users_upsert_file_add_complex.json | 66 ++++++++++ tests/api/testdata/users_upsert_file_mod.json | 44 +++++++ .../users_upsert_file_val_errors.json | 69 +++++++++++ tests/api/testdata/users_upsert_mod.json | 4 + .../testdata/users_upsert_skip_inserts.json | 7 +- .../testdata/users_upsert_skip_updates.json | 7 +- .../api/testdata/users_upsert_val_error.json | 4 + tests/cli/data/users_upsert_file_add.txt | 3 + .../data/users_upsert_file_add_complex.txt | 3 + tests/cli/data/users_upsert_file_mod.txt | 2 + .../cli/data/users_upsert_file_val_errors.txt | 3 + tests/cli/testdata/users/users_upsert_add.txt | 3 +- .../testdata/users/users_upsert_file_add.txt | 17 +++ .../users/users_upsert_file_add_complex.txt | 19 +++ .../testdata/users/users_upsert_file_mod.txt | 12 ++ .../users/users_upsert_file_val_errors.txt | 16 +++ .../testdata/users/users_upsert_val_error.txt | 1 + 22 files changed, 459 insertions(+), 16 deletions(-) create mode 100644 autograder/cli/users/upsert-file.py create mode 100644 tests/api/testdata/users_upsert_file_add.json create mode 100644 tests/api/testdata/users_upsert_file_add_complex.json create mode 100644 tests/api/testdata/users_upsert_file_mod.json create mode 100644 tests/api/testdata/users_upsert_file_val_errors.json create mode 100644 tests/cli/data/users_upsert_file_add.txt create mode 100644 tests/cli/data/users_upsert_file_add_complex.txt create mode 100644 tests/cli/data/users_upsert_file_mod.txt create mode 100644 tests/cli/data/users_upsert_file_val_errors.txt create mode 100644 tests/cli/testdata/users/users_upsert_file_add.txt create mode 100644 tests/cli/testdata/users/users_upsert_file_add_complex.txt create mode 100644 tests/cli/testdata/users/users_upsert_file_mod.txt create mode 100644 tests/cli/testdata/users/users_upsert_file_val_errors.txt diff --git a/autograder/api/users/upsert.py b/autograder/api/users/upsert.py index 7c1c6916..46a4f82c 100644 --- a/autograder/api/users/upsert.py +++ b/autograder/api/users/upsert.py @@ -11,24 +11,20 @@ autograder.api.config.APIParam('skip-inserts', 'Skip inserts (default: False).', required = False, - parser_options = {'action': 'store_true', 'default': False} - ), + parser_options = {'action': 'store_true', 'default': False}), autograder.api.config.APIParam('skip-updates', 'Skip updates (default: False).', required = False, - parser_options = {'action': 'store_true', 'default': False} - ), + parser_options = {'action': 'store_true', 'default': False}), autograder.api.config.APIParam('send-emails', 'Send any emails.', - required = False, cli_param = False - ), + required = True, cli_param = False), autograder.api.config.APIParam('raw-users', 'A list of users to upsert.', - required = True, cli_param = False - ), + required = True, cli_param = False), ] DESCRIPTION = 'Upsert one or more users to the server (Update if exists, otherwise insert).' diff --git a/autograder/cli/common.py b/autograder/cli/common.py index 57f5df50..6594d4e6 100644 --- a/autograder/cli/common.py +++ b/autograder/cli/common.py @@ -136,10 +136,12 @@ def _list_user_op_results_table(results, header = True, keys = ALL_USER_OP_KEYS) _print_tsv(rows, header, [header_key for _, header_key in keys]) def list_user_op_results(results, table = False): + sorted_results = sorted(results, key=lambda x: x["email"]) + if (table): - _list_user_op_results_table(results) + _list_user_op_results_table(sorted_results) else: - _list_user_op_results(results) + _list_user_op_results(sorted_results) def _print_user_op_results_from_dict(result): lines = [] diff --git a/autograder/cli/users/upsert-file.py b/autograder/cli/users/upsert-file.py new file mode 100644 index 00000000..48cabebd --- /dev/null +++ b/autograder/cli/users/upsert-file.py @@ -0,0 +1,114 @@ +import sys + +import autograder.api.constants +import autograder.api.users.upsert +import autograder.cli.common +import autograder.util.hash + +def run(arguments): + arguments = vars(arguments) + + arguments['raw-users'] = _load_users(arguments['path']) + arguments['send-emails'] = not arguments['skip-emails'] + + result = autograder.api.users.upsert.send(arguments, exit_on_error = True) + + autograder.cli.common.list_user_op_results(result['results'], table = arguments['table']) + return 0 + +def _load_users(path): + users = [] + + with open(path, 'r') as file: + lineno = 0 + for line in file: + lineno += 1 + + line = line.strip() + if (line == ""): + continue + + parts = line.split("\t") + if (len(parts) > 7): + raise ValueError( + "File ('%s') line (%d) has too many values. Max is 7, found %d." % ( + path, lineno, len(parts))) + + email = parts.pop(0) + + password = '' + if (len(parts) > 0): + password = parts.pop(0) + if (password != ''): + password = autograder.util.hash.sha256_hex(password) + + name = '' + if (len(parts) > 0): + name = parts.pop(0) + + role = 'user' + if (len(parts) > 0): + role = parts.pop(0) + + if (role not in autograder.api.constants.SERVER_ROLES): + raise ValueError( + "File ('%s') line (%d) has an invalid role '%s'." % ( + path, lineno, role)) + + course = '' + if (len(parts) > 0): + course = parts.pop(0) + + course_role = 'unknown' + if (len(parts) > 0): + course_role = parts.pop(0) + + if (course_role not in autograder.api.constants.COURSE_ROLES): + raise ValueError( + "File ('%s') line (%d) has an invalid course role '%s'." % ( + path, lineno, course_role)) + + course_lms_id = '' + if (len(parts) > 0): + course_lms_id = parts.pop(0) + + users.append({ + 'email': email, + 'pass': password, + 'name': name, + 'role': role, + 'course': course, + 'course-role': course_role, + 'course-lms-id': course_lms_id, + }) + + return users + +def main(): + return run(_get_parser().parse_args()) + +def _get_parser(): + parser = autograder.api.users.upsert._get_parser() + + parser.description = ('Upsert users to the course from a TSV file.' + + ' (Update if exists, otherwiese insert).') + + parser.add_argument('path', metavar = 'PATH', + action = 'store', type = str, + help = 'Path to a TSV file where each line contains up to seven columns:' + + ' [email, pass, name, role, course, course-role, lms-id].' + + ' Only the email is required.') + + parser.add_argument('--skip-emails', dest = 'skip-emails', + action = 'store_true', default = False, + help = 'Skip sending any emails. Be aware that this may result in inaccessible\ + information.') + + parser.add_argument('--table', dest = 'table', + action = 'store_true', default = False, + help = 'Output the results as a TSV table with a header (default: %(default)s).') + + return parser + +if (__name__ == '__main__'): + sys.exit(main()) diff --git a/tests/api/testdata/users_upsert_add.json b/tests/api/testdata/users_upsert_add.json index ea390c91..6ab8c83b 100644 --- a/tests/api/testdata/users_upsert_add.json +++ b/tests/api/testdata/users_upsert_add.json @@ -4,6 +4,9 @@ "user": "server-admin@test.edulinq.org", "pass": "server-admin", "send-emails": true, + "skip-inserts": false, + "skip-updates": false, + "dry-run": false, "raw-users": [ { "email": "new-user@test.edulinq.org", diff --git a/tests/api/testdata/users_upsert_file_add.json b/tests/api/testdata/users_upsert_file_add.json new file mode 100644 index 00000000..f0341f51 --- /dev/null +++ b/tests/api/testdata/users_upsert_file_add.json @@ -0,0 +1,60 @@ +{ + "module": "autograder.api.users.upsert", + "endpoint": "users/upsert-file", + "arguments": { + "user": "server-admin@test.edulinq.org", + "pass": "server-admin", + "send-emails": true, + "skip-inserts": false, + "skip-updates": false, + "dry-run": false, + "raw-users": [ + { + "email": "file-new-user-1@test.edulinq.org", + "name": "", + "role": "user", + "pass": "", + "course": "", + "course-role": "unknown", + "course-lms-id": "" + }, + { + "email": "file-new-user-2@test.edulinq.org", + "name": "", + "role": "user", + "pass": "", + "course": "", + "course-role": "unknown", + "course-lms-id": "" + }, + { + "email": "file-new-user-3@test.edulinq.org", + "name": "", + "role": "user", + "pass": "", + "course": "", + "course-role": "unknown", + "course-lms-id": "" + } + ] + }, + "output": { + "results": [ + { + "email": "file-new-user-1@test.edulinq.org", + "added": true, + "emailed": true + }, + { + "email": "file-new-user-2@test.edulinq.org", + "added": true, + "emailed": true + }, + { + "email": "file-new-user-3@test.edulinq.org", + "added": true, + "emailed": true + } + ] + } +} diff --git a/tests/api/testdata/users_upsert_file_add_complex.json b/tests/api/testdata/users_upsert_file_add_complex.json new file mode 100644 index 00000000..ce6a4f43 --- /dev/null +++ b/tests/api/testdata/users_upsert_file_add_complex.json @@ -0,0 +1,66 @@ +{ + "module": "autograder.api.users.upsert", + "endpoint": "users/upsert-file", + "arguments": { + "user": "server-admin@test.edulinq.org", + "pass": "server-admin", + "send-emails": true, + "skip-inserts": false, + "skip-updates": false, + "dry-run": false, + "raw-users": [ + { + "email": "first@test.edulinq.org", + "name": "first", + "role": "user", + "pass": "", + "course": "course101", + "course-role": "student", + "course-lms-id": "first-lms-id" + }, + { + "email": "second@test.edulinq.org", + "name": "second", + "role": "creator", + "pass": "", + "course": "", + "course-role": "unknown", + "course-lms-id": "" + }, + { + "email": "val-error@test.edulinq.org", + "name": "val-error", + "role": "owner", + "pass": "", + "course": "", + "course-role": "unknown", + "course-lms-id": "" + } + ] + }, + "output": { + "results": [ + { + "email": "first@test.edulinq.org", + "added": true, + "emailed": true, + "enrolled": [ + "course101" + ] + }, + { + "email": "second@test.edulinq.org", + "added": true, + "emailed": true + }, + { + "email": "val-error@test.edulinq.org", + "validation-errors": [ + { + "external-message": "You have insufficient permissions for the requested operation." + } + ] + } + ] + } +} diff --git a/tests/api/testdata/users_upsert_file_mod.json b/tests/api/testdata/users_upsert_file_mod.json new file mode 100644 index 00000000..7e3f3ded --- /dev/null +++ b/tests/api/testdata/users_upsert_file_mod.json @@ -0,0 +1,44 @@ +{ + "module": "autograder.api.users.upsert", + "endpoint": "users/upsert-file", + "arguments": { + "user": "server-admin@test.edulinq.org", + "pass": "server-admin", + "send-emails": true, + "skip-inserts": false, + "skip-updates": false, + "dry-run": false, + "raw-users": [ + { + "email": "server-user@test.edulinq.org", + "name": "sammy", + "role": "user", + "pass": "", + "course": "", + "course-role": "unknown", + "course-lms-id": "" + }, + { + "email": "server-creator@test.edulinq.org", + "name": "", + "role": "user", + "pass": "", + "course": "", + "course-role": "unknown", + "course-lms-id": "" + } + ] + }, + "output": { + "results": [ + { + "email": "server-user@test.edulinq.org", + "modified": true + }, + { + "email": "server-creator@test.edulinq.org", + "modified": true + } + ] + } +} diff --git a/tests/api/testdata/users_upsert_file_val_errors.json b/tests/api/testdata/users_upsert_file_val_errors.json new file mode 100644 index 00000000..98ce9abb --- /dev/null +++ b/tests/api/testdata/users_upsert_file_val_errors.json @@ -0,0 +1,69 @@ +{ + "module": "autograder.api.users.upsert", + "endpoint": "users/upsert-file", + "arguments": { + "user": "server-admin@test.edulinq.org", + "pass": "server-admin", + "send-emails": true, + "skip-inserts": false, + "skip-updates": false, + "dry-run": false, + "raw-users": [ + { + "email": "error@test.edulinq.org", + "name": "", + "role": "owner", + "pass": "", + "course": "", + "course-role": "unknown", + "course-lms-id": "" + }, + { + "email": "server-user@test.edulinq.org", + "name": "", + "role": "owner", + "pass": "", + "course": "", + "course-role": "unknown", + "course-lms-id": "" + }, + { + "email": "server-owner@test.edulinq.org", + "name": "", + "role": "user", + "pass": "", + "course": "", + "course-role": "unknown", + "course-lms-id": "" + } + ] + }, + "output": { + "results": [ + { + "email": "error@test.edulinq.org", + "validation-errors": [ + { + "external-message": "You have insufficient permissions for the requested operation." + } + ] + }, + { + "email": "server-user@test.edulinq.org", + "validation-errors": [ + { + "external-message": "You have insufficient permissions for the requested operation." + } + ] + }, + { + "email": "server-owner@test.edulinq.org", + "validation-errors": [ + { + "external-message": "You have insufficient permissions for the requested operation." + } + ] + } + ] + } +} diff --git a/tests/api/testdata/users_upsert_mod.json b/tests/api/testdata/users_upsert_mod.json index 0306b88e..ac53d82b 100644 --- a/tests/api/testdata/users_upsert_mod.json +++ b/tests/api/testdata/users_upsert_mod.json @@ -3,6 +3,10 @@ "arguments": { "user": "server-admin@test.edulinq.org", "pass": "server-admin", + "send-emails": true, + "skip-inserts": false, + "skip-updates": false, + "dry-run": false, "raw-users": [ { "email": "server-user@test.edulinq.org", diff --git a/tests/api/testdata/users_upsert_skip_inserts.json b/tests/api/testdata/users_upsert_skip_inserts.json index cd8a0ab6..9be449d7 100644 --- a/tests/api/testdata/users_upsert_skip_inserts.json +++ b/tests/api/testdata/users_upsert_skip_inserts.json @@ -3,6 +3,10 @@ "arguments": { "user": "server-admin@test.edulinq.org", "pass": "server-admin", + "send-emails": true, + "skip-inserts": true, + "skip-updates": false, + "dry-run": false, "raw-users": [ { "email": "alt-new-user@test.edulinq.org", @@ -13,8 +17,7 @@ "course-role": "student", "course-lms-id": "" } - ], - "skip-inserts": true + ] }, "output": { "results": [ diff --git a/tests/api/testdata/users_upsert_skip_updates.json b/tests/api/testdata/users_upsert_skip_updates.json index 375ae58f..25bbfc7c 100644 --- a/tests/api/testdata/users_upsert_skip_updates.json +++ b/tests/api/testdata/users_upsert_skip_updates.json @@ -3,6 +3,10 @@ "arguments": { "user": "server-admin@test.edulinq.org", "pass": "server-admin", + "send-emails": true, + "skip-inserts": false, + "skip-updates": true, + "dry-run": false, "raw-users": [ { "email": "server-user@test.edulinq.org", @@ -13,8 +17,7 @@ "course-role": "student", "course-lms-id": "" } - ], - "skip-updates": true + ] }, "output": { "results": [ diff --git a/tests/api/testdata/users_upsert_val_error.json b/tests/api/testdata/users_upsert_val_error.json index b77a8339..ca7ca0b1 100644 --- a/tests/api/testdata/users_upsert_val_error.json +++ b/tests/api/testdata/users_upsert_val_error.json @@ -3,6 +3,10 @@ "arguments": { "user": "server-admin@test.edulinq.org", "pass": "server-admin", + "send-emails": true, + "skip-inserts": false, + "skip-updates": false, + "dry-run": false, "raw-users": [ { "email": "server-user@test.edulinq.org", diff --git a/tests/cli/data/users_upsert_file_add.txt b/tests/cli/data/users_upsert_file_add.txt new file mode 100644 index 00000000..90903bbb --- /dev/null +++ b/tests/cli/data/users_upsert_file_add.txt @@ -0,0 +1,3 @@ +file-new-user-1@test.edulinq.org +file-new-user-2@test.edulinq.org +file-new-user-3@test.edulinq.org diff --git a/tests/cli/data/users_upsert_file_add_complex.txt b/tests/cli/data/users_upsert_file_add_complex.txt new file mode 100644 index 00000000..eefea37d --- /dev/null +++ b/tests/cli/data/users_upsert_file_add_complex.txt @@ -0,0 +1,3 @@ +first@test.edulinq.org first user course101 student first-lms-id +second@test.edulinq.org second creator +val-error@test.edulinq.org val-error owner diff --git a/tests/cli/data/users_upsert_file_mod.txt b/tests/cli/data/users_upsert_file_mod.txt new file mode 100644 index 00000000..3f9dbe24 --- /dev/null +++ b/tests/cli/data/users_upsert_file_mod.txt @@ -0,0 +1,2 @@ +server-user@test.edulinq.org sammy +server-creator@test.edulinq.org user diff --git a/tests/cli/data/users_upsert_file_val_errors.txt b/tests/cli/data/users_upsert_file_val_errors.txt new file mode 100644 index 00000000..2e050993 --- /dev/null +++ b/tests/cli/data/users_upsert_file_val_errors.txt @@ -0,0 +1,3 @@ +error@test.edulinq.org owner +server-user@test.edulinq.org owner +server-owner@test.edulinq.org user diff --git a/tests/cli/testdata/users/users_upsert_add.txt b/tests/cli/testdata/users/users_upsert_add.txt index 8d4ee1e0..eaf699fb 100644 --- a/tests/cli/testdata/users/users_upsert_add.txt +++ b/tests/cli/testdata/users/users_upsert_add.txt @@ -3,8 +3,7 @@ "arguments": [ "--user", "server-admin@test.edulinq.org", "--pass", "server-admin", - "--new-email", "new-user@test.edulinq.org", - "--skip-emails", false + "--new-email", "new-user@test.edulinq.org" ] } --- diff --git a/tests/cli/testdata/users/users_upsert_file_add.txt b/tests/cli/testdata/users/users_upsert_file_add.txt new file mode 100644 index 00000000..23e5ca38 --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_file_add.txt @@ -0,0 +1,17 @@ +{ + "cli": "autograder.cli.users.upsert-file", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "__DATA_DIR__(users_upsert_file_add.txt)" + ] +} +--- +Added + file-new-user-1@test.edulinq.org + file-new-user-2@test.edulinq.org + file-new-user-3@test.edulinq.org +Emailed + file-new-user-1@test.edulinq.org + file-new-user-2@test.edulinq.org + file-new-user-3@test.edulinq.org diff --git a/tests/cli/testdata/users/users_upsert_file_add_complex.txt b/tests/cli/testdata/users/users_upsert_file_add_complex.txt new file mode 100644 index 00000000..9d2f0d1c --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_file_add_complex.txt @@ -0,0 +1,19 @@ +{ + "cli": "autograder.cli.users.upsert-file", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "__DATA_DIR__(users_upsert_file_add_complex.txt)" + ] +} +--- +Added + first@test.edulinq.org + second@test.edulinq.org +Emailed + first@test.edulinq.org + second@test.edulinq.org +Enrolled + first@test.edulinq.org +Encountered 1 validation errors while operating on user: 'val-error@test.edulinq.org'. + Index: 0, Message: 'You have insufficient permissions for the requested operation.'. diff --git a/tests/cli/testdata/users/users_upsert_file_mod.txt b/tests/cli/testdata/users/users_upsert_file_mod.txt new file mode 100644 index 00000000..2af5f296 --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_file_mod.txt @@ -0,0 +1,12 @@ +{ + "cli": "autograder.cli.users.upsert-file", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "__DATA_DIR__(users_upsert_file_mod.txt)" + ] +} +--- +Modified + server-creator@test.edulinq.org + server-user@test.edulinq.org diff --git a/tests/cli/testdata/users/users_upsert_file_val_errors.txt b/tests/cli/testdata/users/users_upsert_file_val_errors.txt new file mode 100644 index 00000000..ed2442a7 --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_file_val_errors.txt @@ -0,0 +1,16 @@ +{ + "cli": "autograder.cli.users.upsert-file", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "__DATA_DIR__(users_upsert_file_val_errors.txt)" + ] +} +--- + +Encountered 1 validation errors while operating on user: 'error@test.edulinq.org'. + Index: 0, Message: 'You have insufficient permissions for the requested operation.'. +Encountered 1 validation errors while operating on user: 'server-owner@test.edulinq.org'. + Index: 0, Message: 'You have insufficient permissions for the requested operation.'. +Encountered 1 validation errors while operating on user: 'server-user@test.edulinq.org'. + Index: 0, Message: 'You have insufficient permissions for the requested operation.'. diff --git a/tests/cli/testdata/users/users_upsert_val_error.txt b/tests/cli/testdata/users/users_upsert_val_error.txt index 3ae1ccb0..5f4037d0 100644 --- a/tests/cli/testdata/users/users_upsert_val_error.txt +++ b/tests/cli/testdata/users/users_upsert_val_error.txt @@ -8,5 +8,6 @@ ] } --- + Encountered 1 validation errors while operating on user: 'server-user@test.edulinq.org'. Index: 0, Message: 'You have insufficient permissions for the requested operation.'. From 65cbf9166cfc2555f9fa2936a1b9ee2628a4aad3 Mon Sep 17 00:00:00 2001 From: Lucas Ellenberger Date: Fri, 6 Sep 2024 10:32:52 -0700 Subject: [PATCH 08/12] Changed expected to match API sorting. --- autograder/cli/common.py | 32 ++++++++----------- tests/api/testdata/users_upsert_file_mod.json | 4 +-- .../users_upsert_file_val_errors.json | 4 +-- 3 files changed, 17 insertions(+), 23 deletions(-) diff --git a/autograder/cli/common.py b/autograder/cli/common.py index 6594d4e6..a638d3c6 100644 --- a/autograder/cli/common.py +++ b/autograder/cli/common.py @@ -107,41 +107,35 @@ def _list_user_op_results(results): # Print all errors last so users can easily see them. for result in results: - val_errors = result.get('validation-errors', None) - if ((val_errors is not None) and (len(val_errors) > 0)): - print("Encountered %d validation errors while operating on user: '%s'." % ( - len(val_errors), result['email'])) - - for i in range(len(val_errors)): - print(INDENT + "Index: %d, Message: '%s'." % (i, val_errors[i]['external-message'])) + for error_key, label in USER_OP_ERROR_KEYS: + if (result.get(error_key, None) is not None): + _clean_result_error(result, error_key) - sys_errors = result.get('system-errors', None) - if ((sys_errors is not None) and (len(sys_errors) > 0)): - print("Encountered %d system errors while operating on user: '%s'." % ( - len(sys_errors), result['email'])) + print("Encountered %d %s while operating on user: '%s'." % ( + len(result[error_key]), label.lower(), result['email'])) - for i in range(len(sys_errors)): - print(INDENT + "Index: %d, Message: '%s'." % (i, sys_errors[i]['external-message'])) + for i in range(len(result[error_key])): + print(INDENT + "Index: %d, Message: '%s'." % (i, result[error_key][i])) def _list_user_op_results_table(results, header = True, keys = ALL_USER_OP_KEYS): rows = [] for result in results: - # Clean the error messages into a better format. for error_key, _ in USER_OP_ERROR_KEYS: if (result.get(error_key, None) is not None): - result[error_key] = [error['external-message'] for error in result[error_key]] + _clean_result_error(result, error_key) rows.append([result.get(key, '') for key, _ in keys]) _print_tsv(rows, header, [header_key for _, header_key in keys]) def list_user_op_results(results, table = False): - sorted_results = sorted(results, key=lambda x: x["email"]) - if (table): - _list_user_op_results_table(sorted_results) + _list_user_op_results_table(results) else: - _list_user_op_results(sorted_results) + _list_user_op_results(results) + +def _clean_result_error(result, key): + result[key] = [error['external-message'] for error in result[key]] def _print_user_op_results_from_dict(result): lines = [] diff --git a/tests/api/testdata/users_upsert_file_mod.json b/tests/api/testdata/users_upsert_file_mod.json index 7e3f3ded..95abd02e 100644 --- a/tests/api/testdata/users_upsert_file_mod.json +++ b/tests/api/testdata/users_upsert_file_mod.json @@ -32,11 +32,11 @@ "output": { "results": [ { - "email": "server-user@test.edulinq.org", + "email": "server-creator@test.edulinq.org", "modified": true }, { - "email": "server-creator@test.edulinq.org", + "email": "server-user@test.edulinq.org", "modified": true } ] diff --git a/tests/api/testdata/users_upsert_file_val_errors.json b/tests/api/testdata/users_upsert_file_val_errors.json index 98ce9abb..7c7be54e 100644 --- a/tests/api/testdata/users_upsert_file_val_errors.json +++ b/tests/api/testdata/users_upsert_file_val_errors.json @@ -49,7 +49,7 @@ ] }, { - "email": "server-user@test.edulinq.org", + "email": "server-owner@test.edulinq.org", "validation-errors": [ { "external-message": "You have insufficient permissions for the requested operation." @@ -57,7 +57,7 @@ ] }, { - "email": "server-owner@test.edulinq.org", + "email": "server-user@test.edulinq.org", "validation-errors": [ { "external-message": "You have insufficient permissions for the requested operation." From cebf6751e86672f649e3e6c9e2c0461083cfb56e Mon Sep 17 00:00:00 2001 From: Lucas Ellenberger Date: Fri, 6 Sep 2024 10:47:46 -0700 Subject: [PATCH 09/12] Removed unnecessary newline on empty result. --- autograder/cli/common.py | 3 ++- autograder/cli/users/upsert-file.py | 4 ++-- autograder/cli/users/upsert.py | 4 ++-- tests/cli/testdata/users/users_upsert_file_val_errors.txt | 1 - tests/cli/testdata/users/users_upsert_val_error.txt | 1 - 5 files changed, 6 insertions(+), 7 deletions(-) diff --git a/autograder/cli/common.py b/autograder/cli/common.py index a638d3c6..1a55181e 100644 --- a/autograder/cli/common.py +++ b/autograder/cli/common.py @@ -103,7 +103,8 @@ def _list_user_op_results(results): if (len(emails) > 0): op_results[label] = emails - _print_user_op_results_from_dict(op_results) + if (len(op_results) > 0): + _print_user_op_results_from_dict(op_results) # Print all errors last so users can easily see them. for result in results: diff --git a/autograder/cli/users/upsert-file.py b/autograder/cli/users/upsert-file.py index 48cabebd..2269a212 100644 --- a/autograder/cli/users/upsert-file.py +++ b/autograder/cli/users/upsert-file.py @@ -101,8 +101,8 @@ def _get_parser(): parser.add_argument('--skip-emails', dest = 'skip-emails', action = 'store_true', default = False, - help = 'Skip sending any emails. Be aware that this may result in inaccessible\ - information.') + help = 'Skip sending any emails. Be aware that this may result in inaccessible' + + ' information.') parser.add_argument('--table', dest = 'table', action = 'store_true', default = False, diff --git a/autograder/cli/users/upsert.py b/autograder/cli/users/upsert.py index 3da824d3..ba914d92 100644 --- a/autograder/cli/users/upsert.py +++ b/autograder/cli/users/upsert.py @@ -36,8 +36,8 @@ def _get_parser(): parser.add_argument('--skip-emails', dest = 'skip-emails', action = 'store_true', default = False, - help = 'Skip sending any emails. Be aware that this may result in inaccessible\ - information.') + help = 'Skip sending any emails. Be aware that this may result in inaccessible' + + ' information.') parser.add_argument('--new-email', dest = 'new-email', action = 'store', type = str, required = True, diff --git a/tests/cli/testdata/users/users_upsert_file_val_errors.txt b/tests/cli/testdata/users/users_upsert_file_val_errors.txt index ed2442a7..c55d9abe 100644 --- a/tests/cli/testdata/users/users_upsert_file_val_errors.txt +++ b/tests/cli/testdata/users/users_upsert_file_val_errors.txt @@ -7,7 +7,6 @@ ] } --- - Encountered 1 validation errors while operating on user: 'error@test.edulinq.org'. Index: 0, Message: 'You have insufficient permissions for the requested operation.'. Encountered 1 validation errors while operating on user: 'server-owner@test.edulinq.org'. diff --git a/tests/cli/testdata/users/users_upsert_val_error.txt b/tests/cli/testdata/users/users_upsert_val_error.txt index 5f4037d0..3ae1ccb0 100644 --- a/tests/cli/testdata/users/users_upsert_val_error.txt +++ b/tests/cli/testdata/users/users_upsert_val_error.txt @@ -8,6 +8,5 @@ ] } --- - Encountered 1 validation errors while operating on user: 'server-user@test.edulinq.org'. Index: 0, Message: 'You have insufficient permissions for the requested operation.'. From 58eba0403b9a1d685076055b002919fd48d2dd5f Mon Sep 17 00:00:00 2001 From: Lucas Ellenberger Date: Tue, 10 Sep 2024 16:07:34 -0700 Subject: [PATCH 10/12] Print by result, then operation, end with summary. --- autograder/cli/common.py | 67 +++++++------------ autograder/cli/users/upsert-file.py | 2 +- autograder/cli/users/upsert.py | 2 +- .../users_upsert_file_add_complex.json | 9 ++- .../users_upsert_file_val_errors.json | 27 ++++---- .../api/testdata/users_upsert_val_error.json | 9 ++- tests/cli/testdata/users/users_upsert_add.txt | 8 +-- .../testdata/users/users_upsert_file_add.txt | 18 ++--- .../users/users_upsert_file_add_complex.txt | 22 +++--- .../testdata/users/users_upsert_file_mod.txt | 8 ++- .../users/users_upsert_file_val_errors.txt | 16 +++-- tests/cli/testdata/users/users_upsert_mod.txt | 5 +- .../users/users_upsert_skip_inserts.txt | 5 +- .../users/users_upsert_skip_updates.txt | 5 +- .../testdata/users/users_upsert_val_error.txt | 6 +- 15 files changed, 100 insertions(+), 109 deletions(-) diff --git a/autograder/cli/common.py b/autograder/cli/common.py index 33e0b0bf..f038d4d1 100644 --- a/autograder/cli/common.py +++ b/autograder/cli/common.py @@ -22,8 +22,9 @@ ] USER_OP_ERROR_KEYS = [ - ('validation-errors', 'Validation Errors'), - ('system-errors', 'System Errors'), + ('validation-error', 'Validation Error'), + ('system-error', 'System Error'), + ('communication-error', 'System Error'), ] ALL_USER_OP_KEYS = [ @@ -91,67 +92,47 @@ def _list_sync_users_table(sync_users): def list_add_users(result, table = False): errors = result['errors'] if ((errors is not None) and (len(errors) > 0)): - print("Encounted %d errors." % (len(errors))) + print("Encountered %d errors." % (len(errors))) for error in errors: print(" Index: %d, Email: '%s', Message: '%s'." % ( error['index'], error['email'], error['message'])) list_sync_users(result, table = table) -def _list_user_op_results(results): - op_results = {} - for (key, label) in USER_OP_KEYS: - emails = [] - for result in results: - if (result.get(key, None) is not None): - emails.append(result['email']) - - if (len(emails) > 0): - op_results[label] = emails - - if (len(op_results) > 0): - _print_user_op_results_from_dict(op_results) - - # Print all errors last so users can easily see them. +def _list_user_op_responses(results): + error_count = 0 for result in results: + print(result['email']) + for op_key, label in USER_OP_KEYS: + if (result.get(op_key, None) is not None): + print(INDENT + label) + if (isinstance(result[op_key], list)): + for value in result[op_key]: + print(INDENT + INDENT + value) + for error_key, label in USER_OP_ERROR_KEYS: if (result.get(error_key, None) is not None): - _clean_result_error(result, error_key) - - print("Encountered %d %s while operating on user: '%s'." % ( - len(result[error_key]), label.lower(), result['email'])) + error_count += 1 + print(INDENT + label) + print(INDENT + INDENT + result[error_key]["message"]) - for i in range(len(result[error_key])): - print(INDENT + "Index: %d, Message: '%s'." % (i, result[error_key][i])) + print("Processed %d users. Encountered %d errors." % (len(results), error_count)) -def _list_user_op_results_table(results, header = True, keys = ALL_USER_OP_KEYS): +def _list_user_op_responses_table(results, header = True, keys = ALL_USER_OP_KEYS): rows = [] for result in results: - for error_key, _ in USER_OP_ERROR_KEYS: - if (result.get(error_key, None) is not None): - _clean_result_error(result, error_key) + for error_key in USER_OP_ERROR_KEYS: + result[error_key] = result[error_key]["message"] rows.append([result.get(key, '') for key, _ in keys]) _print_tsv(rows, header, [header_key for _, header_key in keys]) -def list_user_op_results(results, table = False): +def list_user_op_responses(results, table = False): if (table): - _list_user_op_results_table(results) + _list_user_op_responses_table(results) else: - _list_user_op_results(results) - -def _clean_result_error(result, key): - result[key] = [error['external-message'] for error in result[key]] - -def _print_user_op_results_from_dict(result): - lines = [] - for label, emails in result.items(): - lines.append(label) - for email in emails: - lines.append(INDENT + email) - - print("\n".join(lines)) + _list_user_op_responses(results) def _print_tsv(rows, header, header_keys): lines = [] diff --git a/autograder/cli/users/upsert-file.py b/autograder/cli/users/upsert-file.py index 2269a212..7a76fee4 100644 --- a/autograder/cli/users/upsert-file.py +++ b/autograder/cli/users/upsert-file.py @@ -13,7 +13,7 @@ def run(arguments): result = autograder.api.users.upsert.send(arguments, exit_on_error = True) - autograder.cli.common.list_user_op_results(result['results'], table = arguments['table']) + autograder.cli.common.list_user_op_responses(result['results'], table = arguments['table']) return 0 def _load_users(path): diff --git a/autograder/cli/users/upsert.py b/autograder/cli/users/upsert.py index ba914d92..f10725f5 100644 --- a/autograder/cli/users/upsert.py +++ b/autograder/cli/users/upsert.py @@ -25,7 +25,7 @@ def run(arguments): result = autograder.api.users.upsert.send(arguments, exit_on_error = True) - autograder.cli.common.list_user_op_results(result['results'], table = arguments['table']) + autograder.cli.common.list_user_op_responses(result['results'], table = arguments['table']) return 0 def main(): diff --git a/tests/api/testdata/users_upsert_file_add_complex.json b/tests/api/testdata/users_upsert_file_add_complex.json index ce6a4f43..712cff06 100644 --- a/tests/api/testdata/users_upsert_file_add_complex.json +++ b/tests/api/testdata/users_upsert_file_add_complex.json @@ -55,11 +55,10 @@ }, { "email": "val-error@test.edulinq.org", - "validation-errors": [ - { - "external-message": "You have insufficient permissions for the requested operation." - } - ] + "validation-error": { + "locator": "", + "message": "You have insufficient permissions for the requested operation." + } } ] } diff --git a/tests/api/testdata/users_upsert_file_val_errors.json b/tests/api/testdata/users_upsert_file_val_errors.json index 7c7be54e..a9eafe5b 100644 --- a/tests/api/testdata/users_upsert_file_val_errors.json +++ b/tests/api/testdata/users_upsert_file_val_errors.json @@ -42,27 +42,24 @@ "results": [ { "email": "error@test.edulinq.org", - "validation-errors": [ - { - "external-message": "You have insufficient permissions for the requested operation." - } - ] + "validation-error": { + "locator": "", + "message": "You have insufficient permissions for the requested operation." + } }, { "email": "server-owner@test.edulinq.org", - "validation-errors": [ - { - "external-message": "You have insufficient permissions for the requested operation." - } - ] + "validation-error": { + "locator": "", + "message": "You have insufficient permissions for the requested operation." + } }, { "email": "server-user@test.edulinq.org", - "validation-errors": [ - { - "external-message": "You have insufficient permissions for the requested operation." - } - ] + "validation-error": { + "locator": "", + "message": "You have insufficient permissions for the requested operation." + } } ] } diff --git a/tests/api/testdata/users_upsert_val_error.json b/tests/api/testdata/users_upsert_val_error.json index ca7ca0b1..438bc180 100644 --- a/tests/api/testdata/users_upsert_val_error.json +++ b/tests/api/testdata/users_upsert_val_error.json @@ -23,11 +23,10 @@ "results": [ { "email": "server-user@test.edulinq.org", - "validation-errors": [ - { - "external-message": "You have insufficient permissions for the requested operation." - } - ] + "validation-error": { + "locator": "", + "message": "You have insufficient permissions for the requested operation." + } } ] } diff --git a/tests/cli/testdata/users/users_upsert_add.txt b/tests/cli/testdata/users/users_upsert_add.txt index eaf699fb..291ff6c0 100644 --- a/tests/cli/testdata/users/users_upsert_add.txt +++ b/tests/cli/testdata/users/users_upsert_add.txt @@ -7,7 +7,7 @@ ] } --- -Added - new-user@test.edulinq.org -Emailed - new-user@test.edulinq.org +new-user@test.edulinq.org + Added + Emailed +Processed 1 users. Encountered 0 errors. diff --git a/tests/cli/testdata/users/users_upsert_file_add.txt b/tests/cli/testdata/users/users_upsert_file_add.txt index 23e5ca38..68846006 100644 --- a/tests/cli/testdata/users/users_upsert_file_add.txt +++ b/tests/cli/testdata/users/users_upsert_file_add.txt @@ -7,11 +7,13 @@ ] } --- -Added - file-new-user-1@test.edulinq.org - file-new-user-2@test.edulinq.org - file-new-user-3@test.edulinq.org -Emailed - file-new-user-1@test.edulinq.org - file-new-user-2@test.edulinq.org - file-new-user-3@test.edulinq.org +file-new-user-1@test.edulinq.org + Added + Emailed +file-new-user-2@test.edulinq.org + Added + Emailed +file-new-user-3@test.edulinq.org + Added + Emailed +Processed 3 users. Encountered 0 errors. diff --git a/tests/cli/testdata/users/users_upsert_file_add_complex.txt b/tests/cli/testdata/users/users_upsert_file_add_complex.txt index 9d2f0d1c..2ac6378e 100644 --- a/tests/cli/testdata/users/users_upsert_file_add_complex.txt +++ b/tests/cli/testdata/users/users_upsert_file_add_complex.txt @@ -7,13 +7,15 @@ ] } --- -Added - first@test.edulinq.org - second@test.edulinq.org -Emailed - first@test.edulinq.org - second@test.edulinq.org -Enrolled - first@test.edulinq.org -Encountered 1 validation errors while operating on user: 'val-error@test.edulinq.org'. - Index: 0, Message: 'You have insufficient permissions for the requested operation.'. +first@test.edulinq.org + Added + Emailed + Enrolled + course101 +second@test.edulinq.org + Added + Emailed +val-error@test.edulinq.org + Validation Error + You have insufficient permissions for the requested operation. +Processed 3 users. Encountered 1 errors. diff --git a/tests/cli/testdata/users/users_upsert_file_mod.txt b/tests/cli/testdata/users/users_upsert_file_mod.txt index 2af5f296..388a4ae8 100644 --- a/tests/cli/testdata/users/users_upsert_file_mod.txt +++ b/tests/cli/testdata/users/users_upsert_file_mod.txt @@ -7,6 +7,8 @@ ] } --- -Modified - server-creator@test.edulinq.org - server-user@test.edulinq.org +server-creator@test.edulinq.org + Modified +server-user@test.edulinq.org + Modified +Processed 2 users. Encountered 0 errors. diff --git a/tests/cli/testdata/users/users_upsert_file_val_errors.txt b/tests/cli/testdata/users/users_upsert_file_val_errors.txt index c55d9abe..ae740ade 100644 --- a/tests/cli/testdata/users/users_upsert_file_val_errors.txt +++ b/tests/cli/testdata/users/users_upsert_file_val_errors.txt @@ -7,9 +7,13 @@ ] } --- -Encountered 1 validation errors while operating on user: 'error@test.edulinq.org'. - Index: 0, Message: 'You have insufficient permissions for the requested operation.'. -Encountered 1 validation errors while operating on user: 'server-owner@test.edulinq.org'. - Index: 0, Message: 'You have insufficient permissions for the requested operation.'. -Encountered 1 validation errors while operating on user: 'server-user@test.edulinq.org'. - Index: 0, Message: 'You have insufficient permissions for the requested operation.'. +error@test.edulinq.org + Validation Error + You have insufficient permissions for the requested operation. +server-owner@test.edulinq.org + Validation Error + You have insufficient permissions for the requested operation. +server-user@test.edulinq.org + Validation Error + You have insufficient permissions for the requested operation. +Processed 3 users. Encountered 3 errors. diff --git a/tests/cli/testdata/users/users_upsert_mod.txt b/tests/cli/testdata/users/users_upsert_mod.txt index ab72ea57..ff2a18b0 100644 --- a/tests/cli/testdata/users/users_upsert_mod.txt +++ b/tests/cli/testdata/users/users_upsert_mod.txt @@ -8,5 +8,6 @@ ] } --- -Modified - server-user@test.edulinq.org +server-user@test.edulinq.org + Modified +Processed 1 users. Encountered 0 errors. diff --git a/tests/cli/testdata/users/users_upsert_skip_inserts.txt b/tests/cli/testdata/users/users_upsert_skip_inserts.txt index 61eef4d0..04f35f06 100644 --- a/tests/cli/testdata/users/users_upsert_skip_inserts.txt +++ b/tests/cli/testdata/users/users_upsert_skip_inserts.txt @@ -8,5 +8,6 @@ ] } --- -Skipped - alt-new-user@test.edulinq.org +alt-new-user@test.edulinq.org + Skipped +Processed 1 users. Encountered 0 errors. diff --git a/tests/cli/testdata/users/users_upsert_skip_updates.txt b/tests/cli/testdata/users/users_upsert_skip_updates.txt index 573ec684..3d320c31 100644 --- a/tests/cli/testdata/users/users_upsert_skip_updates.txt +++ b/tests/cli/testdata/users/users_upsert_skip_updates.txt @@ -9,5 +9,6 @@ ] } --- -Skipped - server-user@test.edulinq.org +server-user@test.edulinq.org + Skipped +Processed 1 users. Encountered 0 errors. diff --git a/tests/cli/testdata/users/users_upsert_val_error.txt b/tests/cli/testdata/users/users_upsert_val_error.txt index 3ae1ccb0..25103eb8 100644 --- a/tests/cli/testdata/users/users_upsert_val_error.txt +++ b/tests/cli/testdata/users/users_upsert_val_error.txt @@ -8,5 +8,7 @@ ] } --- -Encountered 1 validation errors while operating on user: 'server-user@test.edulinq.org'. - Index: 0, Message: 'You have insufficient permissions for the requested operation.'. +server-user@test.edulinq.org + Validation Error + You have insufficient permissions for the requested operation. +Processed 1 users. Encountered 1 errors. From 7a318b2426f4101362e8684bccc4274b58e9be71 Mon Sep 17 00:00:00 2001 From: Lucas Ellenberger Date: Wed, 11 Sep 2024 17:05:03 -0700 Subject: [PATCH 11/12] Clean upsert-file input. --- autograder/api/users/upsert.py | 2 +- autograder/cli/common.py | 1 + autograder/cli/users/upsert-file.py | 10 ++++++++-- tests/cli/testdata/users/users_upsert_add.txt | 1 + tests/cli/testdata/users/users_upsert_file_add.txt | 1 + .../testdata/users/users_upsert_file_add_complex.txt | 1 + tests/cli/testdata/users/users_upsert_file_mod.txt | 1 + .../testdata/users/users_upsert_file_val_errors.txt | 1 + tests/cli/testdata/users/users_upsert_mod.txt | 1 + tests/cli/testdata/users/users_upsert_skip_inserts.txt | 1 + tests/cli/testdata/users/users_upsert_skip_updates.txt | 1 + tests/cli/testdata/users/users_upsert_val_error.txt | 1 + 12 files changed, 19 insertions(+), 3 deletions(-) diff --git a/autograder/api/users/upsert.py b/autograder/api/users/upsert.py index 46a4f82c..251a4d36 100644 --- a/autograder/api/users/upsert.py +++ b/autograder/api/users/upsert.py @@ -27,7 +27,7 @@ required = True, cli_param = False), ] -DESCRIPTION = 'Upsert one or more users to the server (Update if exists, otherwise insert).' +DESCRIPTION = 'Upsert one or more users to the server (update if exists, insert otherwise).' def send(arguments, **kwargs): return autograder.api.common.handle_api_request(arguments, API_PARAMS, API_ENDPOINT, **kwargs) diff --git a/autograder/cli/common.py b/autograder/cli/common.py index f038d4d1..bdbb6c9d 100644 --- a/autograder/cli/common.py +++ b/autograder/cli/common.py @@ -116,6 +116,7 @@ def _list_user_op_responses(results): print(INDENT + label) print(INDENT + INDENT + result[error_key]["message"]) + print() print("Processed %d users. Encountered %d errors." % (len(results), error_count)) def _list_user_op_responses_table(results, header = True, keys = ALL_USER_OP_KEYS): diff --git a/autograder/cli/users/upsert-file.py b/autograder/cli/users/upsert-file.py index 7a76fee4..ed620a0e 100644 --- a/autograder/cli/users/upsert-file.py +++ b/autograder/cli/users/upsert-file.py @@ -29,6 +29,8 @@ def _load_users(path): continue parts = line.split("\t") + parts = [part.strip() for part in parts] + if (len(parts) > 7): raise ValueError( "File ('%s') line (%d) has too many values. Max is 7, found %d." % ( @@ -49,6 +51,7 @@ def _load_users(path): role = 'user' if (len(parts) > 0): role = parts.pop(0) + role = role.lower() if (role not in autograder.api.constants.SERVER_ROLES): raise ValueError( @@ -62,6 +65,7 @@ def _load_users(path): course_role = 'unknown' if (len(parts) > 0): course_role = parts.pop(0) + course_role = course_role.lower() if (course_role not in autograder.api.constants.COURSE_ROLES): raise ValueError( @@ -97,12 +101,14 @@ def _get_parser(): action = 'store', type = str, help = 'Path to a TSV file where each line contains up to seven columns:' + ' [email, pass, name, role, course, course-role, lms-id].' - + ' Only the email is required.') + + ' Only the email is required. Leading and trailing whitespace is stripped' + + ' from all fields, including pass. If pass is empty, a password will be' + + ' randomly generated and emailed to the user.') parser.add_argument('--skip-emails', dest = 'skip-emails', action = 'store_true', default = False, help = 'Skip sending any emails. Be aware that this may result in inaccessible' - + ' information.') + + ' information (default: %(default)s).') parser.add_argument('--table', dest = 'table', action = 'store_true', default = False, diff --git a/tests/cli/testdata/users/users_upsert_add.txt b/tests/cli/testdata/users/users_upsert_add.txt index 291ff6c0..7e4bbf1e 100644 --- a/tests/cli/testdata/users/users_upsert_add.txt +++ b/tests/cli/testdata/users/users_upsert_add.txt @@ -10,4 +10,5 @@ new-user@test.edulinq.org Added Emailed + Processed 1 users. Encountered 0 errors. diff --git a/tests/cli/testdata/users/users_upsert_file_add.txt b/tests/cli/testdata/users/users_upsert_file_add.txt index 68846006..aa5a5b35 100644 --- a/tests/cli/testdata/users/users_upsert_file_add.txt +++ b/tests/cli/testdata/users/users_upsert_file_add.txt @@ -16,4 +16,5 @@ file-new-user-2@test.edulinq.org file-new-user-3@test.edulinq.org Added Emailed + Processed 3 users. Encountered 0 errors. diff --git a/tests/cli/testdata/users/users_upsert_file_add_complex.txt b/tests/cli/testdata/users/users_upsert_file_add_complex.txt index 2ac6378e..878ff7b0 100644 --- a/tests/cli/testdata/users/users_upsert_file_add_complex.txt +++ b/tests/cli/testdata/users/users_upsert_file_add_complex.txt @@ -18,4 +18,5 @@ second@test.edulinq.org val-error@test.edulinq.org Validation Error You have insufficient permissions for the requested operation. + Processed 3 users. Encountered 1 errors. diff --git a/tests/cli/testdata/users/users_upsert_file_mod.txt b/tests/cli/testdata/users/users_upsert_file_mod.txt index 388a4ae8..24b52bb1 100644 --- a/tests/cli/testdata/users/users_upsert_file_mod.txt +++ b/tests/cli/testdata/users/users_upsert_file_mod.txt @@ -11,4 +11,5 @@ server-creator@test.edulinq.org Modified server-user@test.edulinq.org Modified + Processed 2 users. Encountered 0 errors. diff --git a/tests/cli/testdata/users/users_upsert_file_val_errors.txt b/tests/cli/testdata/users/users_upsert_file_val_errors.txt index ae740ade..f9a240d5 100644 --- a/tests/cli/testdata/users/users_upsert_file_val_errors.txt +++ b/tests/cli/testdata/users/users_upsert_file_val_errors.txt @@ -16,4 +16,5 @@ server-owner@test.edulinq.org server-user@test.edulinq.org Validation Error You have insufficient permissions for the requested operation. + Processed 3 users. Encountered 3 errors. diff --git a/tests/cli/testdata/users/users_upsert_mod.txt b/tests/cli/testdata/users/users_upsert_mod.txt index ff2a18b0..3b1a6248 100644 --- a/tests/cli/testdata/users/users_upsert_mod.txt +++ b/tests/cli/testdata/users/users_upsert_mod.txt @@ -10,4 +10,5 @@ --- server-user@test.edulinq.org Modified + Processed 1 users. Encountered 0 errors. diff --git a/tests/cli/testdata/users/users_upsert_skip_inserts.txt b/tests/cli/testdata/users/users_upsert_skip_inserts.txt index 04f35f06..4bee35ce 100644 --- a/tests/cli/testdata/users/users_upsert_skip_inserts.txt +++ b/tests/cli/testdata/users/users_upsert_skip_inserts.txt @@ -10,4 +10,5 @@ --- alt-new-user@test.edulinq.org Skipped + Processed 1 users. Encountered 0 errors. diff --git a/tests/cli/testdata/users/users_upsert_skip_updates.txt b/tests/cli/testdata/users/users_upsert_skip_updates.txt index 3d320c31..7cfdc6bb 100644 --- a/tests/cli/testdata/users/users_upsert_skip_updates.txt +++ b/tests/cli/testdata/users/users_upsert_skip_updates.txt @@ -11,4 +11,5 @@ --- server-user@test.edulinq.org Skipped + Processed 1 users. Encountered 0 errors. diff --git a/tests/cli/testdata/users/users_upsert_val_error.txt b/tests/cli/testdata/users/users_upsert_val_error.txt index 25103eb8..e2ad0373 100644 --- a/tests/cli/testdata/users/users_upsert_val_error.txt +++ b/tests/cli/testdata/users/users_upsert_val_error.txt @@ -11,4 +11,5 @@ server-user@test.edulinq.org Validation Error You have insufficient permissions for the requested operation. + Processed 1 users. Encountered 1 errors. From 9945237d4dc436d909a46e3b5edd277c43de21cd Mon Sep 17 00:00:00 2001 From: Lucas Ellenberger Date: Thu, 12 Sep 2024 12:00:43 -0700 Subject: [PATCH 12/12] Fixed table error and added table tests. --- autograder/cli/common.py | 9 +++++---- autograder/cli/users/upsert.py | 2 +- .../cli/testdata/users/users_upsert_add_table.txt | 12 ++++++++++++ .../users/users_upsert_file_add_complex_table.txt | 13 +++++++++++++ .../testdata/users/users_upsert_file_add_table.txt | 13 +++++++++++++ .../testdata/users/users_upsert_file_mod_table.txt | 12 ++++++++++++ .../users/users_upsert_file_val_errors_table.txt | 13 +++++++++++++ .../cli/testdata/users/users_upsert_mod_table.txt | 13 +++++++++++++ .../users/users_upsert_skip_inserts_table.txt | 13 +++++++++++++ .../users/users_upsert_skip_updates_table.txt | 14 ++++++++++++++ .../users/users_upsert_val_error_table.txt | 13 +++++++++++++ 11 files changed, 122 insertions(+), 5 deletions(-) create mode 100644 tests/cli/testdata/users/users_upsert_add_table.txt create mode 100644 tests/cli/testdata/users/users_upsert_file_add_complex_table.txt create mode 100644 tests/cli/testdata/users/users_upsert_file_add_table.txt create mode 100644 tests/cli/testdata/users/users_upsert_file_mod_table.txt create mode 100644 tests/cli/testdata/users/users_upsert_file_val_errors_table.txt create mode 100644 tests/cli/testdata/users/users_upsert_mod_table.txt create mode 100644 tests/cli/testdata/users/users_upsert_skip_inserts_table.txt create mode 100644 tests/cli/testdata/users/users_upsert_skip_updates_table.txt create mode 100644 tests/cli/testdata/users/users_upsert_val_error_table.txt diff --git a/autograder/cli/common.py b/autograder/cli/common.py index 398fcf2c..73681bc7 100644 --- a/autograder/cli/common.py +++ b/autograder/cli/common.py @@ -33,7 +33,7 @@ USER_OP_ERROR_KEYS = [ ('validation-error', 'Validation Error'), ('system-error', 'System Error'), - ('communication-error', 'System Error'), + ('communication-error', 'Communication Error'), ] ALL_USER_OP_KEYS = [ @@ -195,7 +195,7 @@ def _list_user_op_responses(results): if (result.get(error_key, None) is not None): error_count += 1 print(INDENT + label) - print(INDENT + INDENT + result[error_key]["message"]) + print(INDENT + INDENT + result[error_key]['message']) print() print("Processed %d users. Encountered %d errors." % (len(results), error_count)) @@ -203,8 +203,9 @@ def _list_user_op_responses(results): def _list_user_op_responses_table(results, header = True, keys = ALL_USER_OP_KEYS): rows = [] for result in results: - for error_key in USER_OP_ERROR_KEYS: - result[error_key] = result[error_key]["message"] + for error_key, _ in USER_OP_ERROR_KEYS: + if (result.get(error_key, None) is not None): + result[error_key] = result[error_key]['message'] rows.append([result.get(key, '') for key, _ in keys]) diff --git a/autograder/cli/users/upsert.py b/autograder/cli/users/upsert.py index f10725f5..fd415f2b 100644 --- a/autograder/cli/users/upsert.py +++ b/autograder/cli/users/upsert.py @@ -37,7 +37,7 @@ def _get_parser(): parser.add_argument('--skip-emails', dest = 'skip-emails', action = 'store_true', default = False, help = 'Skip sending any emails. Be aware that this may result in inaccessible' - + ' information.') + + ' information (default: %(default)s).') parser.add_argument('--new-email', dest = 'new-email', action = 'store', type = str, required = True, diff --git a/tests/cli/testdata/users/users_upsert_add_table.txt b/tests/cli/testdata/users/users_upsert_add_table.txt new file mode 100644 index 00000000..3b7cc305 --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_add_table.txt @@ -0,0 +1,12 @@ +{ + "cli": "autograder.cli.users.upsert", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "--new-email", "new-user@test.edulinq.org", + "--table" + ] +} +--- +Email Added Modified Removed Skipped Not Exists Emailed Enrolled Dropped Validation Error System Error Communication Error +new-user@test.edulinq.org True True diff --git a/tests/cli/testdata/users/users_upsert_file_add_complex_table.txt b/tests/cli/testdata/users/users_upsert_file_add_complex_table.txt new file mode 100644 index 00000000..42053042 --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_file_add_complex_table.txt @@ -0,0 +1,13 @@ +{ + "cli": "autograder.cli.users.upsert-file", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "--table", + "__DATA_DIR__(users_upsert_file_add_complex.txt)" + ]} +--- +Email Added Modified Removed Skipped Not Exists Emailed Enrolled Dropped Validation Error System Error Communication Error +first@test.edulinq.org True True ['course101'] +second@test.edulinq.org True True +val-error@test.edulinq.org You have insufficient permissions for the requested operation. diff --git a/tests/cli/testdata/users/users_upsert_file_add_table.txt b/tests/cli/testdata/users/users_upsert_file_add_table.txt new file mode 100644 index 00000000..4fc1f11a --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_file_add_table.txt @@ -0,0 +1,13 @@ +{ + "cli": "autograder.cli.users.upsert-file", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "--table", + "__DATA_DIR__(users_upsert_file_add.txt)" + ]} +--- +Email Added Modified Removed Skipped Not Exists Emailed Enrolled Dropped Validation Error System Error Communication Error +file-new-user-1@test.edulinq.org True True +file-new-user-2@test.edulinq.org True True +file-new-user-3@test.edulinq.org True True diff --git a/tests/cli/testdata/users/users_upsert_file_mod_table.txt b/tests/cli/testdata/users/users_upsert_file_mod_table.txt new file mode 100644 index 00000000..d854f165 --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_file_mod_table.txt @@ -0,0 +1,12 @@ +{ + "cli": "autograder.cli.users.upsert-file", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "--table", + "__DATA_DIR__(users_upsert_file_mod.txt)" + ]} +--- +Email Added Modified Removed Skipped Not Exists Emailed Enrolled Dropped Validation Error System Error Communication Error +server-creator@test.edulinq.org True +server-user@test.edulinq.org True diff --git a/tests/cli/testdata/users/users_upsert_file_val_errors_table.txt b/tests/cli/testdata/users/users_upsert_file_val_errors_table.txt new file mode 100644 index 00000000..75b634a5 --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_file_val_errors_table.txt @@ -0,0 +1,13 @@ +{ + "cli": "autograder.cli.users.upsert-file", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "--table", + "__DATA_DIR__(users_upsert_file_val_errors.txt)" + ]} +--- +Email Added Modified Removed Skipped Not Exists Emailed Enrolled Dropped Validation Error System Error Communication Error +error@test.edulinq.org You have insufficient permissions for the requested operation. +server-owner@test.edulinq.org You have insufficient permissions for the requested operation. +server-user@test.edulinq.org You have insufficient permissions for the requested operation. diff --git a/tests/cli/testdata/users/users_upsert_mod_table.txt b/tests/cli/testdata/users/users_upsert_mod_table.txt new file mode 100644 index 00000000..cf74045e --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_mod_table.txt @@ -0,0 +1,13 @@ +{ + "cli": "autograder.cli.users.upsert", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "--new-email", "server-user@test.edulinq.org", + "--new-role", "creator", + "--table" + ] +} +--- +Email Added Modified Removed Skipped Not Exists Emailed Enrolled Dropped Validation Error System Error Communication Error +server-user@test.edulinq.org True diff --git a/tests/cli/testdata/users/users_upsert_skip_inserts_table.txt b/tests/cli/testdata/users/users_upsert_skip_inserts_table.txt new file mode 100644 index 00000000..84897fec --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_skip_inserts_table.txt @@ -0,0 +1,13 @@ +{ + "cli": "autograder.cli.users.upsert", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "--new-email", "alt-new-user@test.edulinq.org", + "--skip-inserts", + "--table" + ] +} +--- +Email Added Modified Removed Skipped Not Exists Emailed Enrolled Dropped Validation Error System Error Communication Error +alt-new-user@test.edulinq.org True diff --git a/tests/cli/testdata/users/users_upsert_skip_updates_table.txt b/tests/cli/testdata/users/users_upsert_skip_updates_table.txt new file mode 100644 index 00000000..2cdb8bec --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_skip_updates_table.txt @@ -0,0 +1,14 @@ +{ + "cli": "autograder.cli.users.upsert", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "--new-email", "server-user@test.edulinq.org", + "--new-role", "creator", + "--skip-updates", + "--table" + ] +} +--- +Email Added Modified Removed Skipped Not Exists Emailed Enrolled Dropped Validation Error System Error Communication Error +server-user@test.edulinq.org True diff --git a/tests/cli/testdata/users/users_upsert_val_error_table.txt b/tests/cli/testdata/users/users_upsert_val_error_table.txt new file mode 100644 index 00000000..395a2eea --- /dev/null +++ b/tests/cli/testdata/users/users_upsert_val_error_table.txt @@ -0,0 +1,13 @@ +{ + "cli": "autograder.cli.users.upsert", + "arguments": [ + "--user", "server-admin@test.edulinq.org", + "--pass", "server-admin", + "--new-email", "server-user@test.edulinq.org", + "--new-role", "owner", + "--table" + ] +} +--- +Email Added Modified Removed Skipped Not Exists Emailed Enrolled Dropped Validation Error System Error Communication Error +server-user@test.edulinq.org You have insufficient permissions for the requested operation.