revert DA grading to fix grade calc performance regression
this reverts part of b4840c75cc
fixes CNVS-14840
test plan:
- with differentiated assignments off, grading should work correctly
- in a course with many assignments, you should be able to download the
gradebook csv
- with differentiated assignments on, grading will not work correctly, this is
intentional.
Change-Id: Id910ff28763ba13c529a21d4c125a164e094aeac
Reviewed-on: https://gerrit.instructure.com/39436
Reviewed-by: Mike Nomitch <mnomitch@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: Caleb Guanzon <cguanzon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
This commit is contained in:
Simon Williams
parent
1413ae1dd4
commit
e241218d48
|
|
@ -136,7 +136,10 @@ class GradeSummaryPresenter
|
|||
:content_participations)
|
||||
.where("assignments.workflow_state != 'deleted'")
|
||||
.find_all_by_user_id(student)
|
||||
.select{ |submission| submission.assignment_visible_to_student?(student_id)}
|
||||
|
||||
if @context.feature_enabled?(:differentiated_assignments)
|
||||
ss = ss.select{ |submission| submission.assignment_visible_to_student?(student_id)}
|
||||
end
|
||||
|
||||
assignments_index = assignments.index_by(&:id)
|
||||
|
||||
|
|
|
|||
|
|
@ -53,7 +53,6 @@ class GradeCalculator
|
|||
submissions_by_user = @submissions.group_by(&:user_id)
|
||||
@user_ids.map do |user_id|
|
||||
user_submissions = submissions_by_user[user_id] || []
|
||||
user_submissions.select!{|submission| submission.assignment_visible_to_student?(user_id)}
|
||||
current, current_groups = calculate_current_score(user_id, user_submissions)
|
||||
final, final_groups = calculate_final_score(user_id, user_submissions)
|
||||
[[current, current_groups], [final, final_groups]]
|
||||
|
|
@ -90,7 +89,7 @@ class GradeCalculator
|
|||
end
|
||||
|
||||
def calculate_score(submissions, user_id, grade_updates, ignore_ungraded)
|
||||
group_sums = create_group_sums(submissions, ignore_ungraded, user_id)
|
||||
group_sums = create_group_sums(submissions, ignore_ungraded)
|
||||
info = calculate_total_from_group_scores(group_sums)
|
||||
grade_updates[user_id] = info[:grade]
|
||||
[info, group_sums.index_by { |s| s[:id] }]
|
||||
|
|
@ -106,8 +105,8 @@ class GradeCalculator
|
|||
# :weight => 50},
|
||||
# ...]
|
||||
# each group
|
||||
def create_group_sums(submissions, ignore_ungraded=true, user_id)
|
||||
assignments_by_group_id = assignments_for_user(user_id).group_by(&:assignment_group_id)
|
||||
def create_group_sums(submissions, ignore_ungraded=true)
|
||||
assignments_by_group_id = @assignments.group_by(&:assignment_group_id)
|
||||
submissions_by_assignment_id = Hash[
|
||||
submissions.map { |s| [s.assignment_id, s] }
|
||||
]
|
||||
|
|
@ -147,11 +146,6 @@ class GradeCalculator
|
|||
end
|
||||
end
|
||||
|
||||
def assignments_for_user(user_id)
|
||||
valid_ids = User.find(user_id).assignments_visibile_in_course(@course).pluck(:id)
|
||||
@assignments.select{|assig| valid_ids.include? assig.id}
|
||||
end
|
||||
|
||||
# see comments for dropAssignments in grade_calculator.coffee
|
||||
def drop_assignments(submissions, rules)
|
||||
drop_lowest = rules[:drop_lowest] || 0
|
||||
|
|
|
|||
|
|
@ -598,88 +598,5 @@ describe GradeCalculator do
|
|||
check_grades(grade, grade)
|
||||
end
|
||||
end
|
||||
|
||||
context "differentiated assignments grade calculation" do
|
||||
def set_up_course_for_differentiated_assignments(enabler_method)
|
||||
@course.send(enabler_method, :differentiated_assignments)
|
||||
set_grades [[5, 20], [15, 20], [10,20], [nil, 20], [20, 20], [10,20], [nil, 20]]
|
||||
@assignments.each do |a|
|
||||
a.only_visible_to_overrides = true
|
||||
a.save!
|
||||
end
|
||||
@overridden_lowest = @assignments[0]
|
||||
@overridden_highest = @assignments[1]
|
||||
@overridden_middle = @assignments[2]
|
||||
@non_overridden_lowest = @assignments[3]
|
||||
@non_overridden_highest = @assignments[4]
|
||||
@non_overridden_middle = @assignments[5]
|
||||
@not_graded = @assignments.last
|
||||
|
||||
@user.enrollments.each(&:destroy)
|
||||
@section = @course.course_sections.create!(name: "test section")
|
||||
student_in_section(@section, user: @user)
|
||||
|
||||
create_section_override_for_assignment(@overridden_lowest, course_section: @section)
|
||||
create_section_override_for_assignment(@overridden_highest, course_section: @section)
|
||||
create_section_override_for_assignment(@overridden_middle, course_section: @section)
|
||||
end
|
||||
|
||||
def get_user_points_and_course_total(user,course)
|
||||
calc = GradeCalculator.new [user.id], course.id
|
||||
final_grade_info = calc.compute_scores.first.last.first
|
||||
[final_grade_info[:total], final_grade_info[:possible]]
|
||||
end
|
||||
|
||||
context "DA enabled" do
|
||||
before do
|
||||
set_up_course_for_differentiated_assignments(:enable_feature!)
|
||||
end
|
||||
it "should calculate scores based on visible assignments only" do
|
||||
# 5 + 15 + 10 + 20 + 10
|
||||
get_user_points_and_course_total(@user,@course). should == [60,100]
|
||||
end
|
||||
it "should drop the lowest visible when that rule is in place" do
|
||||
@group.update_attribute(:rules, 'drop_lowest:1')
|
||||
# 5 + 15 + 10 + 20 + 10 - 5
|
||||
get_user_points_and_course_total(@user,@course). should == [55,80]
|
||||
end
|
||||
it "should drop the highest visible when that rule is in place" do
|
||||
@group.update_attribute(:rules, 'drop_highest:1')
|
||||
# 5 + 15 + 10 + 20 + 10 - 20
|
||||
get_user_points_and_course_total(@user,@course). should == [40,80]
|
||||
end
|
||||
it "shouldnt count an invisible assingment with never drop on" do
|
||||
@group.update_attribute(:rules, "drop_lowest:2\nnever_drop:#{@overridden_lowest.id}")
|
||||
# 5 + 15 + 10 + 20 + 10 - 10 - 10
|
||||
get_user_points_and_course_total(@user,@course). should == [40,60]
|
||||
end
|
||||
end
|
||||
|
||||
context "DA disabled" do
|
||||
before do
|
||||
set_up_course_for_differentiated_assignments(:disable_feature!)
|
||||
end
|
||||
it "should calculate scores based on all active assignments" do
|
||||
# 5 + 15 + 10 + 0 + 20 + 10
|
||||
get_user_points_and_course_total(@user,@course). should == [60,140]
|
||||
end
|
||||
it "should drop the lowest visible when that rule is in place" do
|
||||
@group.update_attribute(:rules, 'drop_lowest:1')
|
||||
# 5 + 15 + 10 + 0 + 20 + 10 - 0
|
||||
get_user_points_and_course_total(@user,@course). should == [60,120]
|
||||
end
|
||||
it "should drop the highest visible when that rule is in place" do
|
||||
@group.update_attribute(:rules, 'drop_highest:1')
|
||||
# 5 + 15 + 10 + 0 + 20 + 10 - 20
|
||||
get_user_points_and_course_total(@user,@course). should == [40,120]
|
||||
end
|
||||
it "shouldnt count an invisible assingment with never drop on" do
|
||||
@group.update_attribute(:rules, "drop_lowest:2\nnever_drop:#{@non_overridden_lowest.id}")
|
||||
# 5 + 15 + 10 + 0 + 20 + 10 - 5 - 0
|
||||
get_user_points_and_course_total(@user,@course). should == [55,100]
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
end
|
||||
end
|
||||
|
|
|
|||
Loading…
Reference in New Issue