ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

limit enrollments api by section visibilities fixes #8819 

test plan:
 * add two sections to a course
 * add a teacher or ta to a section, and limit them to that section
 * as that teacher, go to GB2
 * you should not be able to view any enrollments outside of that
   section

Change-Id: I3d7dd17828cb79aaf162b0947640e2fa7da7952d
Reviewed-on: https://gerrit.instructure.com/11705
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
This commit is contained in:
Cody Cutrer 2012-06-19 11:42:43 -06:00
parent dff3dec7e2
commit dea41f5213
9 changed files with 85 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
app/controllers/enrollments_api_controller.rb
View File

@ -241,9 +241,11 @@ class EnrollmentsApiController < ApplicationController
# 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) ?
@context.enrollments.scoped(scope_arguments) :
@context.current_enrollments.scoped(scope_arguments)
scope = @context.enrollments_visible_to(@current_user, :type => :all, :include_priors => true).scoped(scope_arguments)
unless scope_arguments[:conditions].include?(:workflow_state)
scope = scope.scoped(:conditions => ['enrollments.workflow_state NOT IN (?)', ['rejected', 'completed', 'deleted', 'inactive']])
end
scope
else
false
end

2
app/controllers/submissions_api_controller.rb
View File

@ -310,7 +310,7 @@ class SubmissionsApiController < ApplicationController
def visible_user_ids
scope = if @section
@context.enrollments_visible_to(@current_user, false, false, [@section.id])
@context.enrollments_visible_to(@current_user, :section_ids => [@section.id])
else
@context.enrollments_visible_to(@current_user)
end

4
app/controllers/users_controller.rb
View File

@ -1007,7 +1007,7 @@ class UsersController < ApplicationController
enrollments = student.student_enrollments.active.all(:include => :course)
enrollments.each do |enrollment|
should_include = enrollment.course.user_has_been_teacher?(@teacher) &&
enrollment.course.enrollments_visible_to(@teacher, true).find_by_id(enrollment.id) &&
enrollment.course.enrollments_visible_to(@teacher, :include_priors => true).find_by_id(enrollment.id) &&
enrollment.course.grants_right?(@current_user, :read_reports)
if should_include
Enrollment.recompute_final_score_if_stale(enrollment.course, student) { enrollment.reload }
@ -1027,7 +1027,7 @@ class UsersController < ApplicationController
redirect_to_referrer_or_default(root_url)
elsif authorized_action(course, @current_user, :read_reports)
Enrollment.recompute_final_score_if_stale(course)
@courses[course] = teacher_activity_report(@teacher, course, course.enrollments_visible_to(@teacher, true))
@courses[course] = teacher_activity_report(@teacher, course, course.enrollments_visible_to(@teacher, :include_priors => true))
end
end

37
app/models/course.rb
View File

@ -71,13 +71,13 @@ class Course < ActiveRecord::Base
has_many :course_sections
has_many :active_course_sections, :class_name => 'CourseSection', :conditions => {:workflow_state => 'active'}
has_many :enrollments, :include => [:user, :course], :conditions => ['enrollments.workflow_state != ?', 'deleted'], :dependent => :destroy
has_many :current_enrollments, :class_name => 'Enrollment', :conditions => ['enrollments.workflow_state != ? AND enrollments.workflow_state != ? AND enrollments.workflow_state != ? AND enrollments.workflow_state != ?', 'rejected', 'completed', 'deleted', 'inactive'], :include => :user
has_many :current_enrollments, :class_name => 'Enrollment', :conditions => "enrollments.workflow_state NOT IN ('rejected', 'completed', 'deleted', 'inactive')", :include => :user
has_many :prior_enrollments, :class_name => 'Enrollment', :include => [:user, :course], :conditions => "enrollments.workflow_state = 'completed'"
has_many :students, :through => :student_enrollments, :source => :user
has_many :all_students, :through => :all_student_enrollments, :source => :user
has_many :participating_students, :through => :enrollments, :source => :user, :conditions => "enrollments.type IN ('StudentEnrollment', 'StudentViewEnrollment') and enrollments.workflow_state = 'active'"
has_many :student_enrollments, :class_name => 'Enrollment', :conditions => ["enrollments.workflow_state != ? AND enrollments.workflow_state != ? AND enrollments.workflow_state != ? AND enrollments.workflow_state != ? AND enrollments.type IN ('StudentEnrollment', 'StudentViewEnrollment')", 'deleted', 'completed', 'rejected', 'inactive'], :include => :user
has_many :all_student_enrollments, :class_name => 'Enrollment', :conditions => ["enrollments.workflow_state != ? AND enrollments.type IN ('StudentEnrollment', 'StudentViewEnrollment')", 'deleted'], :include => :user
has_many :student_enrollments, :class_name => 'Enrollment', :conditions => "enrollments.workflow_state NOT IN ('rejected', 'completed', 'deleted', 'inactive') AND enrollments.type IN ('StudentEnrollment', 'StudentViewEnrollment')", :include => :user
has_many :all_student_enrollments, :class_name => 'Enrollment', :conditions => "enrollments.workflow_state != 'deleted' AND enrollments.type IN ('StudentEnrollment', 'StudentViewEnrollment')", :include => :user
has_many :all_real_students, :through => :all_real_student_enrollments, :source => :user
has_many :all_real_student_enrollments, :class_name => 'StudentEnrollment', :conditions => ["enrollments.workflow_state != ?", 'deleted'], :include => :user
has_many :detailed_enrollments, :class_name => 'Enrollment', :conditions => ['enrollments.workflow_state != ?', 'deleted'], :include => {:user => {:pseudonym => :communication_channel}}
@ -2338,17 +2338,32 @@ class Course < ActiveRecord::Base
# returns a scope, not an array of users/enrollments
def students_visible_to(user, include_priors=false)
enrollments_visible_to(user, include_priors, true)
enrollments_visible_to(user, :include_priors => include_priors, :return_users => true)
end
def enrollments_visible_to(user, include_priors=false, return_users=false, limit_to_section_ids=nil)
def enrollments_visible_to(user, opts = {})
visibilities = section_visibilities_for(user)
if return_users
scope = include_priors ? self.all_students : self.students
relation = []
relation << 'all' if opts[:include_priors]
if opts[:type] == :all
relation << 'user' if opts[:return_users]
else
scope = include_priors ? self.all_student_enrollments : self.student_enrollments
relation << (opts[:type].try(:to_s) || 'student')
end
if limit_to_section_ids
scope = scope.scoped(:conditions => { 'enrollments.course_section_id' => limit_to_section_ids.to_a })
if opts[:return_users]
relation.last << 's'
else
relation << 'enrollments'
end
relation = relation.join('_')
# our relations don't all follow the same pattern
relation = case relation
when 'all_enrollments'; 'enrollments'
when 'enrollments'; 'current_enrollments'
else; relation
end
scope = self.send(relation.to_sym)
if opts[:section_ids]
scope = scope.scoped(:conditions => { 'enrollments.course_section_id' => opts[:section_ids].to_a })
end
unless visibilities.any?{|v|v[:admin]}
scope = scope.scoped(:conditions => "enrollments.type != 'StudentViewEnrollment'")
@ -2356,7 +2371,7 @@ class Course < ActiveRecord::Base
# See also Users#messageable_users (same logic used to get users across multiple courses)
case enrollment_visibility_level_for(user, visibilities)
when :full then scope
when :sections then scope.scoped({:conditions => "enrollments.course_section_id IN (#{visibilities.map{|s| s[:course_section_id]}.join(",")})"})
when :sections then scope.scoped(:conditions => ["enrollments.course_section_id IN (?) OR (enrollments.limit_privileges_to_course_section=? AND enrollments.type IN ('TeacherEnrollment', 'TaEnrollment', 'DesignerEnrollment'))", visibilities.map{|s| s[:course_section_id]}, false])
when :restricted then scope.scoped({:conditions => "enrollments.user_id IN (#{(visibilities.map{|s| s[:associated_user_id]}.compact + [user.id]).join(",")})"})
else scope.scoped({:conditions => "FALSE"})
end

23
app/models/enrollment.rb
View File

@ -29,13 +29,14 @@ class Enrollment < ActiveRecord::Base
has_many :pseudonyms, :primary_key => :user_id, :foreign_key => :user_id
has_many :course_account_associations, :foreign_key => 'course_id', :primary_key => 'course_id'
validates_presence_of :user_id
validates_presence_of :course_id
validates_presence_of :user_id, :course_id
validates_inclusion_of :limit_privileges_to_course_section, :in => [true, false]
before_save :assign_uuid
before_save :assert_section
before_save :update_user_account_associations_if_necessary
before_save :audit_groups_for_deleted_enrollments
before_validation :infer_privileges
after_create :create_linked_enrollments
after_save :clear_email_caches
after_save :cancel_future_appointments
@ -355,6 +356,24 @@ class Enrollment < ActiveRecord::Base
self.root_account_id = self.course_section.root_account_id rescue nil
end
def infer_privileges
# limit_privileges_to_course_section affects whether this user can see
# users from other sections (for any purpose - messaging, roster, grading)
# admins (teacher, ta, designer) that have this flag are also visible TO
# users from any section (but not students/observers).
# currently, this flag is actually only configurable for teachers and
# TAs; designers are always course-wide, and so are students.
# In the future, we should probably allow configuring it for students,
# possibly section-wide (i.e. "Students in this section can see students
# from all other sections")
if self.is_a?(TeacherEnrollment) || self.is_a?(TaEnrollment)
self.limit_privileges_to_course_section = false if self.limit_privileges_to_course_section.nil?
else
self.limit_privileges_to_course_section = false
end
true
end
def course_name
self.course.name || t('#enrollment.default_course_name', "Course")
end

14
db/migrate/20120620190441_fix_default_limit_privileges_to_course_section.rb Normal file
View File

@ -0,0 +1,14 @@
class FixDefaultLimitPrivilegesToCourseSection < ActiveRecord::Migration
tag :predeploy, :postdeploy
self.transactional = false
def self.up
Enrollment.find_ids_in_ranges do |(start_id, end_id)|
Enrollment.update_all({ :limit_privileges_to_course_section => false }, ["type IN ('StudentEnrollment', 'ObserverEnrollment', 'StudentViewEnrollment', 'DesignerEnrollment') AND id>=? AND id <=?", start_id, end_id])
Enrollment.update_all({ :limit_privileges_to_course_section => false }, ["type IN ('TeacherEnrollment', 'TaEnrollment') AND limit_privileges_to_course_section IS NULL AND id>=? AND id <=?", start_id, end_id])
end
end
def self.down
end
end

6
spec/apis/v1/enrollments_api_spec.rb
View File

@ -49,7 +49,7 @@ describe EnrollmentsApiController, :type => :integration do
'id' => new_enrollment.id,
'user_id' => @unenrolled_user.id,
'course_section_id' => @section.id,
'limit_privileges_to_course_section' => true,
'limit_privileges_to_course_section' => false,
'enrollment_state' => 'active',
'course_id' => @course.id,
'type' => 'StudentEnrollment',
@ -65,7 +65,6 @@ describe EnrollmentsApiController, :type => :integration do
new_enrollment.root_account_id.should eql @course.account.id
new_enrollment.user_id.should eql @unenrolled_user.id
new_enrollment.course_section_id.should eql @section.id
new_enrollment.limit_privileges_to_course_section.should eql true
new_enrollment.workflow_state.should eql 'active'
new_enrollment.course_id.should eql @course.id
new_enrollment.should be_an_instance_of StudentEnrollment
@ -207,7 +206,7 @@ describe EnrollmentsApiController, :type => :integration do
'id' => new_enrollment.id,
'user_id' => @unenrolled_user.id,
'course_section_id' => @section.id,
'limit_privileges_to_course_section' => true,
'limit_privileges_to_course_section' => false,
'enrollment_state' => 'active',
'course_id' => @course.id,
'type' => 'StudentEnrollment',
@ -223,7 +222,6 @@ describe EnrollmentsApiController, :type => :integration do
new_enrollment.root_account_id.should eql @course.account.id
new_enrollment.user_id.should eql @unenrolled_user.id
new_enrollment.course_section_id.should eql @section.id
new_enrollment.limit_privileges_to_course_section.should eql true
new_enrollment.workflow_state.should eql 'active'
new_enrollment.course_id.should eql @course.id
new_enrollment.should be_an_instance_of StudentEnrollment

4
spec/models/course_spec.rb
View File

@ -2466,6 +2466,10 @@ describe Course, "section_visibility" do
it "should return user's sections" do
@course.sections_visible_to(@ta).should eql [@course.default_section]
end
it "should return non-limited admins from other sections" do
@course.enrollments_visible_to(@ta, :type => :teacher, :return_users => true).should eql [@teacher]
end
end
context "restricted" do

14
spec/models/user_spec.rb
View File

@ -815,7 +815,9 @@ describe User do
it "should not include users from other sections if visibility is limited to sections" do
set_up_course_with_users
@course.enroll_user(@student, 'StudentEnrollment', :enrollment_state => 'active', :limit_privileges_to_course_section => true)
enrollment = @course.enroll_user(@student, 'StudentEnrollment', :enrollment_state => 'active', :limit_privileges_to_course_section => true)
# we currently force limit_privileges_to_course_section to be false for students; override it in the db
Enrollment.update_all({ :limit_privileges_to_course_section => true }, :id => enrollment.id)
messageable_users = @student.messageable_users.map(&:id)
messageable_users.should include @this_section_user.id
messageable_users.should_not include @other_section_user.id
@ -873,7 +875,9 @@ describe User do
it "should respect section visibility when returning users for a specified group" do
set_up_course_with_users
@course.enroll_user(@student, 'StudentEnrollment', :enrollment_state => 'active', :limit_privileges_to_course_section => true)
enrollment = @course.enroll_user(@student, 'StudentEnrollment', :enrollment_state => 'active', :limit_privileges_to_course_section => true)
# we currently force limit_privileges_to_course_section to be false for students; override it in the db
Enrollment.update_all({ :limit_privileges_to_course_section => true }, :id => enrollment.id)
@group.users << @other_section_user
@ -1021,8 +1025,10 @@ describe User do
it "should return concluded enrollments in the group and section if they are still members" do
set_up_course_with_users
@course.enroll_user(@student, 'StudentEnrollment', :enrollment_state => 'active', :limit_privileges_to_course_section => true)
enrollment = @course.enroll_user(@student, 'StudentEnrollment', :enrollment_state => 'active', :limit_privileges_to_course_section => true)
# we currently force limit_privileges_to_course_section to be false for students; override it in the db
Enrollment.update_all({ :limit_privileges_to_course_section => true }, :id => enrollment.id)
@group.users << @other_section_user
@this_section_user_enrollment.conclude