search

lduarte1991 / hxat

Contains the currently-in-development project by HarvardX to bring the annotation tool currently living in the edX platform to a more accessible LTI implementation.
12 stars 7 forks source link

Allow zero or more course admins #135

Closed arthurian closed 3 years ago

arthurian commented 3 years ago

This PR modifies LTICourse.course_admins so that it is no longer a required field when editing a course.

The motivation for this change is to allow the following workflow:

  1. Course support staff installs the LTI tool in the course and launches the tool to set things up, which creates the LTICourse and adds them as an admin by default.
  2. Course support staff finishes setting up the annotation assignment(s) and then removes himself/herself from the list of course admins and adds the real instructor to the list of pending admins, so that when that instructor logs in they become the course admin.

Previously, trying to remove yourself from the list of course admins would result in a 403 Forbidden, but this error should no longer occur when course_admins is empty. If an error does occur, the user should now see a form error message rather than a generic 403.

@nmaekawa

lduarte1991 commented 3 years ago

👍 on this change. This is leftover from when i had a rudimentary knowledge of how LTIs work. Originally, I added another level of verification at this point because I considered profiles to be in charge of that authorization instead of the LTI config/init (as it should rightfully be). Over time, the LTICourse.course_admins became only the vehicle used to marking someone as an "instructor" for the sidebar.

Changes look good to me and can be merged.