properly filter user enrollments. fixes #8650
when filtering user enrollments by type, return an empty array instead of a 401 error if the filter results in 0 enrollments being returned. test plan: * create User A with a student enrollment; * as User A or another user with permissions, make API call to return User A's enrollments, filtering with type[]=TeacherEnrollment (even though User A has no teacher enrollments); * verify that an empty array and not a 401 error is returned. Change-Id: I72ecbb339ba7245151626272c2cc4efd12292bc0 Reviewed-on: https://gerrit.instructure.com/10815 Tested-by: Jenkins <jenkins@instructure.com> Reviewed-by: Brian Palmer <brianp@instructure.com>
This commit is contained in:
parent
b55af018ca
commit
1dcbf23917
|
@ -220,6 +220,13 @@ class EnrollmentsApiController < ApplicationController
|
|||
end
|
||||
|
||||
protected
|
||||
# Internal: Collect course enrollments that @current_user has permissions to
|
||||
# read.
|
||||
#
|
||||
# scope_arguments - A hash to be passed as :conditions to an AR scope.
|
||||
# Allowed keys are any keys allowed in :conditions.
|
||||
#
|
||||
# Returns an ActiveRecord scope of enrollments on success, false on failure.
|
||||
def course_index_enrollments(scope_arguments)
|
||||
if authorized_action(@context, @current_user, :read_roster)
|
||||
scope_arguments[:conditions].include?(:workflow_state) ?
|
||||
|
@ -230,11 +237,18 @@ class EnrollmentsApiController < ApplicationController
|
|||
end
|
||||
end
|
||||
|
||||
# Internal: Collect user enrollments that @current_user has permissions to
|
||||
# read.
|
||||
#
|
||||
# scope_arguments - A hash to be passed as :conditions to an AR scope.
|
||||
# Allowed keys are any keys allowed in :conditions.
|
||||
#
|
||||
# Returns an ActiveRecord scope of enrollments on success, false on failure.
|
||||
def user_index_enrollments(scope_arguments)
|
||||
user = api_find(User, params[:user_id])
|
||||
# if user is requesting for themselves, just return all of their
|
||||
# enrollments without any extra checking.
|
||||
return user.current_enrollments if user == @current_user
|
||||
return user.current_enrollments.scoped(scope_arguments) if user == @current_user
|
||||
|
||||
# otherwise check for read_roster rights on all of the requested
|
||||
# user's accounts
|
||||
|
@ -243,16 +257,15 @@ class EnrollmentsApiController < ApplicationController
|
|||
accounts
|
||||
end
|
||||
|
||||
# if there aren't any ids in approved_accounts, then the user doesn't have
|
||||
# permissions.
|
||||
render_unauthorized_action(@user) and return false if approved_accounts.empty?
|
||||
|
||||
scope_arguments[:conditions].merge!({ 'enrollments.root_account_id' => approved_accounts })
|
||||
enrollments = scope_arguments[:conditions].include?(:workflow_state) ?
|
||||
user.enrollments.scoped(scope_arguments) :
|
||||
user.current_and_invited_enrollments.scoped(scope_arguments)
|
||||
|
||||
if enrollments.count == 0 && user.current_enrollments.count != 0
|
||||
render_unauthorized_action(@user)
|
||||
return false
|
||||
else
|
||||
return enrollments
|
||||
end
|
||||
enrollments
|
||||
end
|
||||
end
|
||||
|
|
|
@ -304,7 +304,7 @@ describe EnrollmentsApiController, :type => :integration do
|
|||
enrollment = @student.enrollments.first
|
||||
enrollment.course_section = @section
|
||||
enrollment.save!
|
||||
|
||||
|
||||
@path = "/api/v1/sections/#{@section.id}/enrollments"
|
||||
@params = { :controller => "enrollments_api", :action => "index", :section_id => @section.id.to_param, :format => "json" }
|
||||
json = api_call(:get, @path, @params)
|
||||
|
@ -642,6 +642,15 @@ describe EnrollmentsApiController, :type => :integration do
|
|||
h
|
||||
}
|
||||
end
|
||||
|
||||
it "should return an empty array when no user enrollments match a filter" do
|
||||
site_admin_user(:active_all => true)
|
||||
|
||||
json = api_call(:get, "#{@user_path}?type[]=TeacherEnrollment",
|
||||
@user_params.merge(:type => %w{TeacherEnrollment}))
|
||||
|
||||
json.should be_empty
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue