DA - speed grader shows right students and sections

fixes CNVS-13810

test plan:
  - with DA on create multiple assignments visible to
    select sections
  - go to speed grader for said assignments
    - only students with visibility should show up
    - only sections with visibility should show up
  - turn DA off and everything still works
  - note: group assingments not done yet

Change-Id: Ia065a82a24db40cc6ba46586114167e2ea8e75c1
Reviewed-on: https://gerrit.instructure.com/36981
Reviewed-by: Cameron Sutter <csutter@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: Amber Taniuchi <amber@instructure.com>
Product-Review: Hilary Scharton <hilary@instructure.com>
This commit is contained in:
Michael Nomitch 2014-06-27 10:46:56 -05:00 committed by Mike Nomitch
parent 6e31c3c2c0
commit 535a4b47ff
2 changed files with 103 additions and 9 deletions

View File

@ -1264,7 +1264,7 @@ class Assignment < ActiveRecord::Base
:methods => avatar_methods, :methods => avatar_methods,
:only => [:name, :id]) :only => [:name, :id])
} }
res[:context][:active_course_sections] = context.sections_visible_to(user). res[:context][:active_course_sections] = context.sections_visible_to(user, self.sections_with_visibility(user)).
map{|s| s.as_json(:include_root => false, :only => [:id, :name]) } map{|s| s.as_json(:include_root => false, :only => [:id, :name]) }
res[:context][:enrollments] = enrollments. res[:context][:enrollments] = enrollments.
map{|s| s.as_json(:include_root => false, :only => [:user_id, :course_section_id]) } map{|s| s.as_json(:include_root => false, :only => [:user_id, :course_section_id]) }
@ -1338,6 +1338,14 @@ class Assignment < ActiveRecord::Base
Attachment.skip_thumbnails = nil Attachment.skip_thumbnails = nil
end end
def sections_with_visibility(user)
return context.active_course_sections unless self.differentiated_assignments_applies?
visible_student_ids = visible_students_for_speed_grader(user).map(&:id)
context.active_course_sections.joins(:student_enrollments).
where(:enrollments => {:user_id => visible_student_ids, :type => "StudentEnrollment"}).uniq.reorder("name")
end
# quiz submission versions are too expensive to de-serialize so we have to # quiz submission versions are too expensive to de-serialize so we have to
# cap the number we will do # cap the number we will do
def too_many_qs_versions?(student_submissions) def too_many_qs_versions?(student_submissions)
@ -1374,12 +1382,6 @@ class Assignment < ActiveRecord::Base
# group's submission. the students name will be changed to the group's # group's submission. the students name will be changed to the group's
# name. for non-group assignments this just returns all visible users # name. for non-group assignments this just returns all visible users
def representatives(user) def representatives(user)
visible_students = (
user ?
context.students_visible_to(user) :
context.participating_students
).order_by_sortable_name.uniq.to_a
if grade_as_group? if grade_as_group?
submissions = self.submissions.includes(:user) submissions = self.submissions.includes(:user)
users_with_submissions = submissions users_with_submissions = submissions
@ -1395,7 +1397,7 @@ class Assignment < ActiveRecord::Base
group_category.groups.includes(:group_memberships => :user).map { |g| group_category.groups.includes(:group_memberships => :user).map { |g|
[g.name, g.users] [g.name, g.users]
}.map { |group_name, group_students| }.map { |group_name, group_students|
visible_group_students = group_students & visible_students visible_group_students = group_students & visible_students_for_speed_grader(user)
representative = (visible_group_students & users_with_turnitin_data).first representative = (visible_group_students & users_with_turnitin_data).first
representative ||= (visible_group_students & users_with_submissions).first representative ||= (visible_group_students & users_with_submissions).first
representative ||= visible_group_students.first representative ||= visible_group_students.first
@ -1412,10 +1414,23 @@ class Assignment < ActiveRecord::Base
representative representative
}.compact }.compact
else else
visible_students visible_students_for_speed_grader(user)
end end
end end
# using this method instead of students_with_visibility so we
# can add the includes and students_visible_to/participating_students scopes
def visible_students_for_speed_grader(user)
@visible_students_for_speed_grader ||= {}
@visible_students_for_speed_grader[user.global_id] ||= (
student_scope = user ? context.students_visible_to(user) : context.participating_students
if self.differentiated_assignments_applies?
student_scope = student_scope.able_to_see_assignment_in_course_with_da(self.id, context.id)
end
student_scope
).order_by_sortable_name.uniq.to_a
end
def visible_rubric_assessments_for(user) def visible_rubric_assessments_for(user)
if self.rubric_association if self.rubric_association
self.rubric_association.rubric_assessments.select{|a| a.grants_right?(user, :read)}.sort_by{|a| [a.assessment_type == 'grading' ? CanvasSort::First : CanvasSort::Last, Canvas::ICU.collation_key(a.assessor_name)] } self.rubric_association.rubric_assessments.select{|a| a.grants_right?(user, :read)}.sort_by{|a| [a.assessment_type == 'grading' ? CanvasSort::First : CanvasSort::Last, Canvas::ICU.collation_key(a.assessor_name)] }

View File

@ -1854,6 +1854,39 @@ describe Assignment do
end end
end end
describe "sections_with_visibility" do
before(:once) do
course_with_teacher(:active_all => true)
@course.enable_feature!(:draft_state)
@section = @course.course_sections.create!
@student = student_in_section(@section, opts={})
@assignment, @assignment2, @assignment3 = (1..3).map{|a|@course.assignments.create!}
@assignment.only_visible_to_overrides = true
create_section_override_for_assignment(@assignment, course_section: @section)
@assignment2.only_visible_to_overrides = true
@assignment3.only_visible_to_overrides = false
create_section_override_for_assignment(@assignment3, course_section: @section)
[@assignment, @assignment2, @assignment3].each(&:save!)
end
it "returns active_course_sections with differentiated assignments off" do
@course.disable_feature!(:differentiated_assignments)
@assignment.sections_with_visibility(@teacher).should == @course.course_sections
@assignment2.sections_with_visibility(@teacher).should == @course.course_sections
@assignment3.sections_with_visibility(@teacher).should == @course.course_sections
end
it "returns only sections with overrides with differentiated assignments on" do
@course.enable_feature!(:differentiated_assignments)
@assignment.sections_with_visibility(@teacher).should == [@section]
@assignment2.sections_with_visibility(@teacher).should == []
@assignment3.sections_with_visibility(@teacher).should == @course.course_sections
end
end
context "modules" do context "modules" do
it "should be locked when part of a locked module" do it "should be locked when part of a locked module" do
ag = @course.assignment_groups.create! ag = @course.assignment_groups.create!
@ -2321,6 +2354,52 @@ describe Assignment do
json[:submissions].first[:submission_comments].first[:created_at].to_i.should eql @comment.created_at.to_i json[:submissions].first[:submission_comments].first[:created_at].to_i.should eql @comment.created_at.to_i
end end
context "students and active course sections" do
before(:once) do
@course = course(:active_course => true)
@teacher, @student1, @student2 = (1..3).map{User.create}
@assignment = Assignment.create!(title: "title", context: @course, only_visible_to_overrides: true)
@course.enroll_teacher(@teacher)
@course.enroll_student(@student2, :enrollment_state => 'active')
@section1 = @course.course_sections.create!(name: "test section 1")
@section2 = @course.course_sections.create!(name: "test section 2")
student_in_section(@section1, user: @student1)
create_section_override_for_assignment(@assignment, {course_section: @section1})
end
it "should include all students and sections with DA off" do
@course.disable_feature!(:differentiated_assignments)
json = @assignment.speed_grader_json(@teacher)
json[:context][:students].map{|s| s[:id]}.include?(@student1.id).should be_true
json[:context][:students].map{|s| s[:id]}.include?(@student2.id).should be_true
json[:context][:active_course_sections].map{|cs| cs[:id]}.include?(@section1.id).should be_true
json[:context][:active_course_sections].map{|cs| cs[:id]}.include?(@section2.id).should be_true
end
it "should include only students and sections with overrides when DA is on" do
@course.enable_feature!(:differentiated_assignments)
json = @assignment.speed_grader_json(@teacher)
json[:context][:students].map{|s| s[:id]}.include?(@student1.id).should be_true
json[:context][:students].map{|s| s[:id]}.include?(@student2.id).should be_false
json[:context][:active_course_sections].map{|cs| cs[:id]}.include?(@section1.id).should be_true
json[:context][:active_course_sections].map{|cs| cs[:id]}.include?(@section2.id).should be_false
end
it "should include all students when is only_visible_to_overrides false" do
@course.enable_feature!(:differentiated_assignments)
@assignment.only_visible_to_overrides = false
@assignment.save!
json = @assignment.speed_grader_json(@teacher)
json[:context][:students].map{|s| s[:id]}.include?(@student1.id).should be_true
json[:context][:students].map{|s| s[:id]}.include?(@student2.id).should be_true
json[:context][:active_course_sections].map{|cs| cs[:id]}.include?(@section1.id).should be_true
json[:context][:active_course_sections].map{|cs| cs[:id]}.include?(@section2.id).should be_true
end
end
it "should return submission lateness" do it "should return submission lateness" do
# Set up # Set up
section_1 = @course.course_sections.create!(:name => 'Section one') section_1 = @course.course_sections.create!(:name => 'Section one')