hypothesis / product-backlog

Where new feature ideas and current bugs for the Hypothesis product live
118 stars 7 forks source link

User's Canvas instance is configured to not send User - View List, which we depend on for groups functionality #1427

Open mkdir-washington-edu opened 1 year ago

mkdir-washington-edu commented 1 year ago

Bug report form

Edit

  1. Part of the confusion initially came from the fact that our fallback Canvas API error is the "unpublished file" error. That issue is now noted here: https://github.com/hypothesis/support/issues/41.
  2. The real issue below is noted by Marcos here: https://github.com/hypothesis/product-backlog/issues/1427#issuecomment-1461832444. The reasoning for why we should fix this is here: https://github.com/hypothesis/product-backlog/issues/1427#issuecomment-1622112150.

Steps to reproduce

Unable to reproduce in our system, but:

Video reproducing the issue

HAR file 1 - Files assignment

HAR file 2 - Sharepoint assignment (same error)

Expected behaviour

I suspect that we may be seeing a Sections or Groups error, but we're showing a Files error instead. Either way students should not see a Files error over a Sharepoint PDF.

The Files error is due to the fact that our Files error message is our fallback API error message for Canvas, and we're getting an API error we haven't anticipated before.

Actual behaviour

Students see:

image
mkdir-washington-edu commented 1 year ago

Support next steps: I'm going to ask the LMS admin if they're scoping the API key and, if so, if maybe they missed a scope. Edit: enforce scopes is off for this API key.

robertknight commented 1 year ago

Related Slack thread: https://hypothes-is.slack.com/archives/C4K6M7P5E/p1678100198045419

It looks like the error message the frontend and backend have a mismatch in specificity of error conditions. If the backend returns a canvas_api_permission_error error, the frontend renders the error message in the screenshot above.

mkdir-washington-edu commented 1 year ago

In this specific case I'm remembering this seems to be a Groups issue, as the error message happens only when the Groups integration is turned on.

mkdir-washington-edu commented 1 year ago

Per our Slack conversation, more details should be added to the error message above.

marcospri commented 1 year ago

The underling problem in the school might be related to the:

Users - view list permission for students in their canvas instance.

We also have a problem where this error is reported using an unrelated copy in the dialog. The solution for this could be emit a new error code for 403 related to files and use the current copy for that and use the existing canvas_api_permission_error with some other copy more broadly referring to permission issues.

mkdir-washington-edu commented 1 year ago

Ran into another issue here: https://app.hubspot.com/contacts/6291320/record/0-5/1714366257

Problem - API Key was deleted Error message was the Unpublished Files error in all non-group cases, and the group error when trying to retrieve the group sets.

mkdir-washington-edu commented 1 year ago

Another instance where we should probably be getting a non-Files error, but the Files error seems to be the fallback for an unknown error:

mkdir-washington-edu commented 1 year ago

This issue has really become two issues, which is my fault.

Problem number 1 is that we fall back to the Canvas Files message as our default error message, which is just confusing and causes us to misdiagnose problems. This is now noted here: https://github.com/hypothesis/support/issues/41. It should not be prioritized as part of this issue.

Problem number 2, which was the original problem here, is that we're expecting the "Users - view list" to be allowed in Canvas, and in at least one institution it's explicitly not allowed. While we haven't seen this elsewhere, I could anticipate other running into this again. Here's a possible scenario, based on partner setups we have seen. A. Partner uses a single Canvas Course to administer multiple real-life courses, with each real-life course getting its own Canvas Section. B. Students in these real-life courses are not allowed to see each others' data, including whether or not they are even enrolled in their real-life course that shares this single Canvas Course. C. The institution determines that students can tell that other students are enrolled using the Canvas "People" tab, and so they disable the "Users - view list" parameter.

We absolutely have multiple partners doing steps A and B currently, step C is a guess on my part.

leedenison commented 1 year ago

@mkdir-washington-edu - Can you tell how often this is happening? Is it for every Sharepoint PDF?

mkdir-washington-edu commented 1 year ago

@leedenison Sorry, I never know when I should vs shouldn't change the title on an issue. The original title is no longer a concern, and the underlying issue (and the reason it was showing up on Sharepoint docs) is now captured here: https://github.com/hypothesis/support/issues/41.

What I subsequently tried to steer this issue towards, and the second problem the school was having, is captured under "problem #2) here: https://github.com/hypothesis/product-backlog/issues/1427#issuecomment-1622112150