Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#440: added util.py:obj_or_id_or_sis_str, use it in course.py:enroll… #522

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

ndegroot
Copy link

@ndegroot ndegroot commented Nov 1, 2021

Change course.py:enroll_user to also support 'sis_login_id:' syntax

Proposed Changes

  • in util.py add new function 'obj_or_id_or_sis_str'
  • in courses.py import this new function and change function 'enroll_user' to use this function instead of 'obj_or_id'

Fixes #440 Allow using SIS ids like sis_login_id:[loginname].

Extra important for scripts running under subadmin accounts: in that case the scripts can't use the get_user() function for new users, not enrolled yet in a sub-admin related course because the function is reserved for admins. So subadmin scripts really need this format in course.enroll_user() to be able to enroll a user programmaticly.

Notes:

  1. An alternative approach could be to expand function 'obj_or_id' to accept 'sis_login_id:xxxx' and variants. But not changing the function name is confusing. Changing all occurrances of function name 'obj_or_id' thoughout the code I didn't dare to do.
  2. in section.py there also an enroll_user function, maybe it could be changed also.

…enroll_user to support 'sis_login_id:' syntax
@ndegroot ndegroot mentioned this pull request Nov 1, 2021
4 tasks
@ndegroot
Copy link
Author

I've just checked my code again, run the tests and coverage on Python 3.11 against current state. Still working ok. Please give me feedback why this is still pending. Happy to rework it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow SIS IDs
1 participant