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

Allow SIS IDs #440

Open
4 tasks
Thetwam opened this issue Nov 16, 2020 · 6 comments · May be fixed by #522
Open
4 tasks

Allow SIS IDs #440

Thetwam opened this issue Nov 16, 2020 · 6 comments · May be fixed by #522

Comments

@Thetwam
Copy link
Member

Thetwam commented Nov 16, 2020

It is common for users to want to use SIS IDs in lieu of canvas IDs. Canvas provides a way to use these alternate IDs with the syntax <ID_TYPE>:<ID_VALUE>, e.g. sis_course_id:A1234.

In some cases, like Canvas.get_user() and Canvas.get_account(), we added special (optional) arguments like id_type and use_sis_id, respectively. These allow users to indicate that the ID being passed is something other than a Canvas ID.

It turns out that there are many more places this functionality would be useful. In particular, we received a request to add it to Course.enroll_user() for the user parameter. This is different from our previous implementations because those only modified the URL, not individual parameters.

Instead of adding optional arguments like id_type, use_sis_id, it's probably most straightforward to allow users to pass any properly-formatted string as an ID (e.g. "sis_course_id:A1234"). This is currently not possible with any ID that goes through the obj_or_id to check. We'll need to modify obj_or_id to allow SIS IDs. There are also "special IDs" that Canvas lists. We actually have some exceptions for self, but there are others.

This raises the question of what kind of checks to put into place regarding "valid" SIS IDs. Should we only allow specific ID types (from a list of all the Canvas-accepted ones), or allow any string that the user passes? Should we limit the types to only relevant ones (e.g. not allowing sis_course_id when the ID should be a user)?

Curious for others thoughts on the matter.

Some action items:

  • Confirm manually (curl/Postman/etc.) that the Enroll a user endpoint accepts SIS IDs for the enrollment[user_id] parameter
  • Determine which SIS ID and special ID types we're going to support
  • Update canvasapi.util.obj_or_id to accept SIS IDs (and possibly other special IDs)
  • Determine how to handle existing implementations (such as id_type in Canvas.get_user())
@eriko
Copy link

eriko commented Dec 2, 2020

From using pandarus I have never found a endpoint that did not accept an sis_*_id when that was a relevant. Honestly I did not realize that doing this was not supported yet in canvasapi.

@Thetwam
Copy link
Member Author

Thetwam commented Dec 2, 2020

Admittedly, we thought we had all the cases covered too! It turns out there's a need for a general-purpose solution, like in Course.enroll_user as described above.

@eriko
Copy link

eriko commented Dec 2, 2020

I am in the process of porting over my course creation and enrollment management tool from ruby to python for institutional reasons. Since I generally am dealing with our external ids (that become SIS id) for courses and humans instead of the canvas ones I have found the sis_*_id: syntax to be really handy.

@ndegroot
Copy link

One solution could be to replace the canvasapi.util.obj_or_id() check in enroll_user() with a variant canvasapi.util.obj_or_general_id as stated above.

I can confirm it works inside enroll_user() with 'sis_login_id:[valid sis login]'

Would you consider a pull request?

obj_or_general_id

ndegroot added a commit to ndegroot/canvasapi that referenced this issue Nov 1, 2021
…enroll_user to support 'sis_login_id:' syntax
@ndegroot
Copy link

ndegroot commented Nov 1, 2021

it's a PR now: #522

ndegroot added a commit to ndegroot/canvasapi that referenced this issue Nov 3, 2021
@bennettscience
Copy link
Contributor

I just ran into this today while creating pairing codes for students. We had a list of SIS IDs and I think I prefer the id_type kwarg because it allows me to just drop any old ID in there and then specify the type as needed without extra string parsing.

One thing that would help in the short term is linking to valid sis_*_id types in the docs. Any of those send well-formatted requests as is.

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

Successfully merging a pull request may close this issue.

4 participants