search

instructure / canvas-lms

The open LMS by Instructure, Inc.
https://github.com/instructure/canvas-lms/wiki
GNU Affero General Public License v3.0
5.42k stars 2.42k forks source link

Canvas Enrollments API is no longer returning inactive enrollments by default #2190

Closed MakotoTheKnight closed 1 year ago

MakotoTheKnight commented 1 year ago

Summary:

When making an API call to pull back enrollments through the Enrollments API, my team and I are observing that enrollments that are inactive are not being returned.

We believe that this is the contract that is being violated when attempting to reach out to Canvas:

Enrollments scoped to a course context will include inactive states by default if the caller has account admin authorization and the state[] parameter is omitted.

Note: This is only occurring in the Beta environment for Canvas, but we observed that this is slated for a future release sometime in early May.

Steps to reproduce:

  1. Create separate account with admin privileges
  2. Generate an access token for said account
  3. Invoke /api/v1/sections/<sis_id>/enrollments?sis_user_id[]=<sis_id>, ensuring that the aforementioned account has a few inactive enrollments

Expected behavior:

All enrollments, including inactive ones, are returned

Actual behavior:

Only active enrollments are returned.

Additional notes:

If the change to the API does mandate that it not bring back inactive records by default, then the filters introduced by this commit are overly restrictive and should be loosened up somehow.

jstanley0 commented 1 year ago

I am not able to reproduce the issue with those steps. /api/v1/sections/<sis_id>/enrollments returns active and inactive enrollments. The code you highlighted isn't in a code path that this API call would hit; that code handles the sis_user_id parameter specifically.

MakotoTheKnight commented 1 year ago

@jstanley0 - apologies for the miscommunication! I just double-checked the conversation and it is the case we are using the sis_user_id parameter. I'll update the original post to reflect that.

augiethornton commented 1 year ago

@MakotoTheKnight We now default to returning the old behavior of inactive enrollments if the state param is not included in the request. You should see the change reflected in the Beta environment.

MakotoTheKnight commented 1 year ago

Thank you @augiethornton! I'm able to confirm that the behavior is now consistent with what we originally expected.

I'll go ahead and close this out now. Thanks again!