DA - remove filter from assignment_stats

fixes CNVS-17748

test plan:
  - with DA on and off
    - go to the grade summary page as a teacher
      - it should load assignment max/min/ave
    - go to it as a student
      - it should load assignment max/min/ave

Change-Id: I8996495eaf9c1427be6a3bbf3883b5c78e9ee571
Reviewed-on: https://gerrit.instructure.com/46521
Reviewed-by: Simon Williams <simon@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: Adam Stone <astone@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
This commit is contained in:
Michael Nomitch 2015-01-05 12:12:42 -06:00 committed by Mike Nomitch
parent 44d4a9fa7f
commit af3104a9b0
2 changed files with 58 additions and 141 deletions

View File

@ -168,12 +168,9 @@ class GradeSummaryPresenter
def assignment_stats
@stats ||= begin
chain = @context.assignments.active.except(:order)
if @context.feature_enabled?(:differentiated_assignments)
# if DA is on, further restrict the assignments to those visible to each student
# note: adding the context_id avoids a seq scan
chain = chain.joins(:assignment_student_visibilities).
where("assignment_student_visibilities.course_id = ?", @context.id)
end
# note: because a score is needed for max/min/ave we are not filtering
# by assignment_student_visibilities, if a stat is added that doesn't
# require score then add a filter when the DA feature is on
chain.joins(:submissions)
.where("submissions.user_id in (?)", real_and_active_student_ids)
.group("assignments.id")

View File

@ -59,149 +59,69 @@ describe GradeSummaryPresenter do
end
describe '#assignment_stats' do
context "course with DA disabled" do
before(:each) do
teacher_in_course
@course.disable_feature!(:differentiated_assignments)
end
it 'works' do
s1, s2, s3, s4 = n_students_in_course(4)
a = @course.assignments.create! points_possible: 10
a.grade_student s1, grade: 0
a.grade_student s2, grade: 5
a.grade_student s3, grade: 10
before(:each) do
teacher_in_course
@course.disable_feature!(:differentiated_assignments)
end
it 'works' do
s1, s2, s3, s4 = n_students_in_course(4)
a = @course.assignments.create! points_possible: 10
a.grade_student s1, grade: 0
a.grade_student s2, grade: 5
a.grade_student s3, grade: 10
# this student should be ignored
a.grade_student s4, grade: 99
s4.enrollments.each &:destroy
p = GradeSummaryPresenter.new(@course, @teacher, nil)
stats = p.assignment_stats
assignment_stats = stats[a.id]
expect(assignment_stats.max.to_f).to eq 10
expect(assignment_stats.min.to_f).to eq 0
expect(assignment_stats.avg.to_f).to eq 5
end
it 'filters out test students and inactive enrollments' do
s1, s2, s3, removed_student = n_students_in_course(4, {:course => @course})
fake_student = course_with_user('StudentViewEnrollment', {:course => @course}).user
fake_student.preferences[:fake_student] = true
a = @course.assignments.create! points_possible: 10
a.grade_student s1, grade: 0
a.grade_student s2, grade: 5
a.grade_student s3, grade: 10
a.grade_student removed_student, grade: 20
a.grade_student fake_student, grade: 100
removed_student.enrollments.each do |enrollment|
enrollment.workflow_state = 'inactive'
enrollment.save!
end
p = GradeSummaryPresenter.new(@course, @teacher, nil)
stats = p.assignment_stats
assignment_stats = stats[a.id]
expect(assignment_stats.max.to_f).to eq 10
expect(assignment_stats.min.to_f).to eq 0
expect(assignment_stats.avg.to_f).to eq 5
end
# this student should be ignored
a.grade_student s4, grade: 99
s4.enrollments.each &:destroy
p = GradeSummaryPresenter.new(@course, @teacher, nil)
stats = p.assignment_stats
assignment_stats = stats[a.id]
expect(assignment_stats.max.to_f).to eq 10
expect(assignment_stats.min.to_f).to eq 0
expect(assignment_stats.avg.to_f).to eq 5
end
context "course with DA enabled" do
before(:each) do
teacher_in_course
@course.enable_feature!(:differentiated_assignments)
it 'filters out test students and inactive enrollments' do
s1, s2, s3, removed_student = n_students_in_course(4, {:course => @course})
fake_student = course_with_user('StudentViewEnrollment', {:course => @course}).user
fake_student.preferences[:fake_student] = true
a = @course.assignments.create! points_possible: 10
a.grade_student s1, grade: 0
a.grade_student s2, grade: 5
a.grade_student s3, grade: 10
a.grade_student removed_student, grade: 20
a.grade_student fake_student, grade: 100
removed_student.enrollments.each do |enrollment|
enrollment.workflow_state = 'inactive'
enrollment.save!
end
it 'filters out test students and inactive enrollments' do
s1, s2, s3, removed_student = n_students_in_course(4, {:course => @course})
p = GradeSummaryPresenter.new(@course, @teacher, nil)
stats = p.assignment_stats
assignment_stats = stats[a.id]
expect(assignment_stats.max.to_f).to eq 10
expect(assignment_stats.min.to_f).to eq 0
expect(assignment_stats.avg.to_f).to eq 5
end
fake_student = course_with_user('StudentViewEnrollment', {:course => @course}).user
fake_student.preferences[:fake_student] = true
it 'doesnt factor nil grades into the average or min' do
s1, s2, s3, s4 = n_students_in_course(4)
a = @course.assignments.create! points_possible: 10
a.grade_student s1, grade: 2
a.grade_student s2, grade: 6
a.grade_student s3, grade: 10
a.grade_student s4, grade: nil
a = @course.assignments.create! points_possible: 10, only_visible_to_overrides: false
a.grade_student s1, grade: 0
a.grade_student s2, grade: 5
a.grade_student s3, grade: 10
a.grade_student removed_student, grade: 20
a.grade_student fake_student, grade: 100
removed_student.enrollments.each do |enrollment|
enrollment.workflow_state = 'inactive'
enrollment.save!
end
p = GradeSummaryPresenter.new(@course, @teacher, nil)
stats = p.assignment_stats
assignment_stats = stats[a.id]
expect(assignment_stats.max.to_f).to eq 10
expect(assignment_stats.min.to_f).to eq 0
expect(assignment_stats.avg.to_f).to eq 5
end
it 'filters out assignments that arent visible to students' do
s1, s2, s3 = n_students_in_course(3)
a = @course.assignments.create! points_possible: 10, only_visible_to_overrides: true
a.grade_student s1, grade: nil
a.grade_student s2, grade: nil
a.grade_student s3, grade: nil
p = GradeSummaryPresenter.new(@course, @teacher, nil)
stats = p.assignment_stats
assignment_stats = stats[a.id]
expect(assignment_stats).to be_nil
end
it 'gets the right submissions when only_visible_to_overrides is true' do
s1, s2 = n_students_in_course(2)
s3 = User.create!
@section = @course.course_sections.create!(name: "test section")
student_in_section(@section, user: s3)
a = @course.assignments.create! points_possible: 10, only_visible_to_overrides: true
create_section_override_for_assignment(a, course_section: @section)
a.grade_student s1, grade: nil
a.grade_student s2, grade: 8
a.grade_student s3, grade: 10
p = GradeSummaryPresenter.new(@course, @teacher, nil)
stats = p.assignment_stats
assignment_stats = stats[a.id]
# ensure s1 is filtered and s2 and s3 are not
expect(assignment_stats.max.to_f).to eq 10
expect(assignment_stats.min.to_f).to eq 8
expect(assignment_stats.avg.to_f).to eq 9
end
it 'does not filter out submissions that would be invisibile when only_visible_to_overrides is true (but is really false or nil)' do
s1, s2 = n_students_in_course(2)
s3 = User.create!
@section = @course.course_sections.create!(name: "test section")
student_in_section(@section, user: s3)
a1 = @course.assignments.create! points_possible: 10, only_visible_to_overrides: false
a2 = @course.assignments.create! points_possible: 10, only_visible_to_overrides: nil
create_section_override_for_assignment(a1, course_section: @section)
create_section_override_for_assignment(a2, course_section: @section)
[a1,a2].each do |a|
a.grade_student s1, grade: 0
a.grade_student s2, grade: 5
a.grade_student s3, grade: 10
end
p = GradeSummaryPresenter.new(@course, @teacher, nil)
stats = p.assignment_stats
[a1,a2].each do |a|
assignment_1_stats = stats[a.id]
expect(assignment_1_stats.max.to_f).to eq 10
expect(assignment_1_stats.min.to_f).to eq 0
expect(assignment_1_stats.avg.to_f).to eq 5
end
end
p = GradeSummaryPresenter.new(@course, @teacher, nil)
stats = p.assignment_stats
assignment_stats = stats[a.id]
expect(assignment_stats.max.to_f).to eq 10
expect(assignment_stats.min.to_f).to eq 2
expect(assignment_stats.avg.to_f).to eq 6
end
end