instructure / canvas-lms

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

Enrollments API: inactive enrollments returned by default #1723

Open putmanb opened 3 years ago

putmanb commented 3 years ago

Summary:

Per the API documentation for the state[] param of "List enrollments" (https://canvas.instructure.com/doc/api/enrollments.html#method.enrollments_api.index):

Filter by enrollment state. If omitted, 'active' and 'invited' enrollments are returned.

However, in my testing enrollments with enrollment_state = 'inactive' are being returned for a course, even when the state[] parameter is omitted.

Steps to reproduce:

  1. Create a course with at least 1 inactive enrollment.
  2. Use the API to list enrollments for that course, do not include a state[] param: /api/v1/courses/1234/enrollments
  3. Observe the returned results.

Expected behavior:

The inactive enrollment should NOT be included.

Actual behavior:

The inactive enrollment IS included.

Notes

The specific case I observed was for a custom role of base type of TeacherEnrollment. I haven't confirmed other cases. So the bug could be limited to custom roles only.

holapozy commented 3 years ago

Dear Instructure/Canvas-Lms Trust you are doing well and staying safe Please I want to set up canvas LMS for my students but I need assistance with materials to guide me through the configuration and troubleshooting Thank you

On Wed, 14 Oct 2020, 10:07 pm putmanb, notifications@github.com wrote:

Summary:

Per the API documentation for the state[] param of "List enrollments" ( https://canvas.instructure.com/doc/api/enrollments.html#method.enrollments_api.index ):

Filter by enrollment state. If omitted, 'active' and 'invited' enrollments are returned.

However, in my testing enrollments with enrollment_state = 'inactive' are being returned for a course, even when the state[] parameter is omitted. Steps to reproduce:

  1. Create a course with at least 1 inactive enrollment.
  2. Use the API to list enrollments for that course, do not include a state[] param: /api/v1/courses/1234/enrollments
  3. Observe the returned results.

Expected behavior:

The inactive enrollment should NOT be included. Actual behavior:

The inactive enrollment IS included. Notes

The specific case I observed was for a custom role of base type of TeacherEnrollment. I haven't confirmed other cases. So the bug could be limited to custom roles only.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/instructure/canvas-lms/issues/1723, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKCYTI5TNIGUJSQ5GF5E3VDSKYHI5ANCNFSM4SRD5QGQ .

spencerolson commented 3 years ago

Hi @putmanb thanks for reporting this. It looks like, when params[:state] is omitted, we return inactive enrollments if the calling user is an admin (the user has permissions to "read as admin" to be more specific): https://github.com/instructure/canvas-lms/blob/master/app/controllers/enrollments_api_controller.rb#L882

I'll check with a few folks to see if: 1) this is intended, in which case we'll update the API docs, OR 2) this is not intended, in which case we'll fix up the code and leave the API docs as-is.

Thanks again for reporting this!

Spencer