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

Automatically fix course-copied Canvas Files assignments whenever possible #2764

Closed
seanh opened this issue Jun 1, 2021 · 0 comments · Fixed by #2927
Closed

Automatically fix course-copied Canvas Files assignments whenever possible #2764

seanh opened this issue Jun 1, 2021 · 0 comments · Fixed by #2927
Assignees
Labels
Backend Canvas Files Issues with our Canvas Files integration Canvas Issues specific to Instructure's Canvas LMS Course Copy

Comments

@seanh
Copy link
Collaborator

seanh commented Jun 1, 2021

Problem

Go to a Canvas course that contains Canvas Files assignments (e.g. Developer Test Course with Sections Enabled) and copy the course: go to SettingsCopy this course. Now go to the new course and launch one of the Canvas Files assignments: you should see an error message.

Instead of copying a course you can also just use the pre-created copied course. You first have to log in using either the eng+canvasteacher@hypothes.is account (teacher) or the eng+coursecopy account (student) (both are in 1Password) then go to Copy of Developer Test Course with Sections Enabled.

Note: If you log in as a student who is also a member of the original course that was copied from you won't run into the error message: the copied assignments actually work fine for students who're members of the original course. You need to log in as a teacher (member of the original course or not) or as a student who is not a member of the original course.

When a new Canvas course is created by copying an existing course any Canvas Files assignments in the new course are broken: students and teachers run into error messages when trying to launch the Canvas Files assignments. It would be much better if the copied Canvas Files assignments could just work instead of showing an error message.

Students see this error message:

Screenshot from 2020-11-23 13-41-55

Teachers see this error message:

Screenshot from 2020-11-23 13-44-44

What's actually going wrong

Canvas file IDs are global not per-course. The "public inline preview url" API that we use to get a Via-compatible URL for a Canvas file takes no course ID only a global file ID: GET /api/v1/files/:id/public_url

When you copy a Canvas course it creates new copies of all the original course's files in the new course, with new file IDs.

Canvas also copies the assignments from the original course into the new course but it does not update the file IDs in the assignments. The "external tool URL" that's stored in Canvas for a Canvas Files assignment looks like this: https://lms.hypothes.is/lti_launches?canvas_file=true&file_id=<ID>. Where <ID> is the ID of the file in the original course. These launch URLs get copied unchanged to the new assignments in the new course.

So the copied Canvas Files assignments in the new course contain IDs of files from the original course. Only users who are members of the original course are allowed to access these original course files. Any users who're only members of the new course won't be allowed to access the files. They'll get a permission error from the Canvas API.

How this is currently handled in the code

When a student tries to launch a copied Canvas Files assignment they get a permission error from the Canvas API when we try to get the download URL for the file. The backend detects this permissions error from Canvas and raises CanvasAPIPermissionError. An exception view catches this error and turns it into a JSON API error response. The frontend special-cases this particular error response and renders the error dialog shown above.

Note that this permissions error doesn't necessarily mean that the assignment has been copied: it can also happen because the file is marked as unpublished (not visible to students) in Canvas and there's no way for us to tell the difference.

Also note that if the student is a member of the original course they won't see this error, the assignment will launch fine for them.

if status_code == 401:
return CanvasAPIPermissionError

class CanvasAPIPermissionError(CanvasAPIError):
"""
A Canvas API permissions error.
This happens when the user's access token is fine but they don't have
permission to carry out the requested action. For example a user might not
have permission to get a public URL for a file if they don't have
permission to read that file.
"""
error_code = "canvas_api_permission_error"

@exception_view_config(context=CanvasAPIPermissionError)
@exception_view_config(context=CanvasFileNotFoundInCourse)
def canvas_api_permission_error(self):
return self.error_response(
error_code=self.context.error_code, details=self.context.details
)

When a teacher launches a copied Canvas Files assignment they probably won't run into the permissions error because the teacher probably is a member of the original course and probably does have access to the original file. Instead, when getting the download URL for the file, we check whether the assignment's file_id is the id of one of the files in the course and if it's not we raise CanvasFileNotFoundInCourse. CanvasFileNotFoundInCourse is similarly caught by an exception view and turned into a JSON API response that's special-cased by the frontend.

Note that the file ID being missing from the current course doesn't necessarily mean that a course copy has happened. It could be because the assignment's file was deleted from the course, and there's no way for us to tell the difference.

We wouldn't want to do this check on student launches because it's an expensive check (it involves calling the Canvas API to get the list of all the files in the course) so we don't want to do it on every single launch (only on teacher launches is fine), and it wouldn't work because students might not have access to all the files in the course (teachers can mark files within a course as "unpublished" meaning students can't see them).

if self.request.lti_user.is_instructor:
self.canvas_api_client.check_file_in_course(file_id, course_id)

class CanvasFileNotFoundInCourse(ServiceError):
"""A Canvas file ID wasn't found in the current course."""
error_code = "canvas_file_not_found_in_course"
def __init__(self, file_id):
self.details = {"file_id": file_id}
super().__init__(self.details)

def check_file_in_course(self, file_id, course_id):
"""
Raise if the file with ID file_id isn't in the course with ID course_id.
:raise CanvasFileNotFoundInCourse: If the file isn't in the course
"""
if not any(
str(file_["id"]) == str(file_id) for file_ in self.list_files(course_id)
):
raise CanvasFileNotFoundInCourse(file_id)

Solution

I don't think there's any real way for us to discover the ID of the new copy of the file, or even to detect when an assignment is a copied one. Previous discussions with Instructure confirmed this.

Instead I think we can look for a file in the new (current) course that has the same name as the old file did and assume that it is the new copy of the file and update the assignment with its ID, and just accept that false positives will happen. We can do this "auto fixing" whenever either a student or a teacher launches an assignment.

This may sometimes result in us selecting the wrong file in edge cases where files have been renamed or deleted, or when multiple files have the same name. That's fine.

This may also trigger in situations other than course copy. E.g. when a student or teacher can't access a file because its been deleted. Or when a student can't access a file because its been marked as unpublished. If there's another file in the course with the same name that hasn't been deleted or marked as unpublished we may "fix" the assignment to use that file instead. This is also fine.

The devil is in the details

Fixing this is much harder than it sounds.

  • We can't tell when a course/assignment/file has been copied from another.

    For example there's nothing in the LTI launch params or in the file metadata from the Canvas API that indicates that the course/assignment/file was copied or tells us what the original course/assignment/file's ID is. I confirmed this with Instructure developers back in 2020. It might be worth re-checking in case things have changed.

    When a student or teacher launches an assignment we can detect when they get a permissions error for the file URL from the Canvas API. This could have happened due to a course copy or it could have happened because the file is unpublished in the course (students don't have access to unpublished files). Also it might not happen at all: if a course has been copied and the user happens to be a member of the original course they can get the file URL from the Canvas API just fine.

    It's very inefficient but when a teacher launches an assignment we can call the Canvas API to get a list of all the files in the course and we can tell that the assignment's file ID is not in the course. This could have happened due to a course copy or it could have happened because the assignment's file was deleted.

  • How can we get the filename of the original file? The user doesn't have permission to access the original file, so we can't make a Canvas API call to fetch the original file's metadata. (Some users will have permission if they happen to be members of the original course as well, but we can't rely on this.)

    We probably need to start recording Canvas files with their IDs and metadata in the DB so that we can look them up. (E.g. whenever a teacher is creating an assignment and we get the list of all the course's files from the Canvas API, also upsert them all into our DB.)

  • How can we update an assignment, changing its file ID? Canvas assignment configurations are stored as query parameters on launch URLs that're stored in Canvas, not in our DB. We can't update them. We're probably going to have to add support for saving Canvas assignment configurations in our DB, and having the DB config override the query params config when present.

    • What about editing assignments?

      You can edit an assignment in Canvas and change it to use a different Canvas File, or change it from a Canvas Files assignment to an HTML one etc. Currently when you do this it submits a new LTI launch URL to Canvas with new assignment config query params. If we've saved the assignment's config in our DB, and DB configs override URL ones, then editing assignments will be broken (they will update the assignment's launch URL in Canvas but not the overriding assignment config in the DB).

      We may need to refactor how Canvas assignments work so that they store the assignment configs in the DB like we do for non-Canvas assignments. There is already a separate issue for this: Move Canvas assignments into the DB #2527.

      However I think it's probably best to avoid this: instead of storing assignment configs in the DB we could store info about Canvas files in courses and update that info instead.)

  • What exactly do we use to decide whether one file looks the same as another? What metadata fields about files do we get back from the Canvas API? Filename? Path?

  • We need to avoid an assignment going back and forth between two different files.

    It would obviously be confusing if an assignment's document keeps changing between two different files (that actually have different contents). For example if each time the teacher launches an assignment we "fix" its Canvas ID to be file 1, whereas each time a student launches it we "fix" it to 2 again.

    This is especially bad if users might have made annotations on the assignment's file. If we change what file the assignment is using the annotations (which are associated with the PDF file's fingerprint in h) will disappear.

    This sort of thing can potentially happen because different users can see different files in Canvas. Some users may get a permissions error when trying to download a file, others won't. When getting the list of files in the course one user may get a different list than the next. Both things can happen because teachers have different permissions than students. Both things can also happen because different teachers have different permissions than each other and different students have different permissions than each other (e.g. depending on whether they belong to the original course).

    I think an invariant that we may want to ensure is this:

    If any user might have annotated the assignment's file that locks it in: we will never change the assignment's file ID. In practice this means that if we have every done a successful launch of an assignment (the assignment's file did exist in the current course and the user did have permission to access the file) then we will never change that assignment's file ID in future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend Canvas Files Issues with our Canvas Files integration Canvas Issues specific to Instructure's Canvas LMS Course Copy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants