dropped assignments calculation respects ignore_muted
Fixes calculation of dropped assignments so that muted assignments are never considered for dropping when computing posted score. In addition, prevents score metadata for unposted grades from overwriting score metadata for posted grades. closes GRADE-763 Test Plan: * Prerequisites: Have a course with at least one student and one teacher. 1. Create two assignments in a course with the same point value and place them in the same assignment group. 2. Set up a grading rule to drop the lowest 1 score. 3. Mute one of the assignments. 4. Have the teacher enter a score of zero for the muted assignment. 5. Have the teacher enter a positive score for the second assignment. 6. Check the teacher interaction report and verify there is no page error. Change-Id: I5015ef73bb40bb0a45a21f166fa6b9ff146002da Reviewed-on: https://gerrit.instructure.com/137538 Reviewed-by: Keith T. Garner <kgarner@instructure.com> Tested-by: Jenkins QA-Review: Indira Pai <ipai@instructure.com> Product-Review: Keith T. Garner <kgarner@instructure.com>
This commit is contained in:
Spencer Olson
parent
556e698a5d
commit
ac4b5714eb
|
|
@ -442,6 +442,12 @@ class GradeCalculator
|
|||
end
|
||||
|
||||
def save_course_and_grading_period_metadata
|
||||
# We only save score metadata for posted grades. This means, if we're
|
||||
# calculating unposted grades (which means @ignore_muted is false),
|
||||
# we don't want to update the score metadata. TODO: start storing the
|
||||
# score metadata for unposted grades.
|
||||
return unless @ignore_muted
|
||||
|
||||
ScoreMetadata.connection.execute("
|
||||
UPDATE #{ScoreMetadata.quoted_table_name} metadata
|
||||
SET
|
||||
|
|
@ -519,35 +525,41 @@ class GradeCalculator
|
|||
sc.id IS NULL;
|
||||
")
|
||||
|
||||
ScoreMetadata.connection.execute("
|
||||
UPDATE #{ScoreMetadata.quoted_table_name} md
|
||||
SET
|
||||
calculation_details = CAST(val.calculation_details as json),
|
||||
updated_at = #{updated_at}
|
||||
FROM (VALUES #{dropped_values}) val
|
||||
(enrollment_id, assignment_group_id, calculation_details)
|
||||
INNER JOIN #{Score.quoted_table_name} scores ON
|
||||
scores.enrollment_id = val.enrollment_id AND
|
||||
scores.assignment_group_id = val.assignment_group_id
|
||||
WHERE
|
||||
scores.id = md.score_id;
|
||||
INSERT INTO #{ScoreMetadata.quoted_table_name}
|
||||
(score_id, calculation_details, created_at, updated_at)
|
||||
SELECT
|
||||
scores.id AS score_id,
|
||||
CAST(val.calculation_details as json) AS calculation_details,
|
||||
#{updated_at} AS created_at,
|
||||
#{updated_at} AS updated_at
|
||||
FROM (VALUES #{dropped_values}) val
|
||||
(enrollment_id, assignment_group_id, calculation_details)
|
||||
LEFT OUTER JOIN #{Score.quoted_table_name} scores ON
|
||||
scores.enrollment_id = val.enrollment_id AND
|
||||
scores.assignment_group_id = val.assignment_group_id
|
||||
LEFT OUTER JOIN #{ScoreMetadata.quoted_table_name} metadata ON
|
||||
metadata.score_id = scores.id
|
||||
WHERE
|
||||
metadata.id IS NULL;
|
||||
")
|
||||
# We only save score metadata for posted grades. This means, if we're
|
||||
# calculating unposted grades (which means @ignore_muted is false),
|
||||
# we don't want to update the score metadata. TODO: start storing the
|
||||
# score metadata for unposted grades.
|
||||
if @ignore_muted
|
||||
ScoreMetadata.connection.execute("
|
||||
UPDATE #{ScoreMetadata.quoted_table_name} md
|
||||
SET
|
||||
calculation_details = CAST(val.calculation_details as json),
|
||||
updated_at = #{updated_at}
|
||||
FROM (VALUES #{dropped_values}) val
|
||||
(enrollment_id, assignment_group_id, calculation_details)
|
||||
INNER JOIN #{Score.quoted_table_name} scores ON
|
||||
scores.enrollment_id = val.enrollment_id AND
|
||||
scores.assignment_group_id = val.assignment_group_id
|
||||
WHERE
|
||||
scores.id = md.score_id;
|
||||
INSERT INTO #{ScoreMetadata.quoted_table_name}
|
||||
(score_id, calculation_details, created_at, updated_at)
|
||||
SELECT
|
||||
scores.id AS score_id,
|
||||
CAST(val.calculation_details as json) AS calculation_details,
|
||||
#{updated_at} AS created_at,
|
||||
#{updated_at} AS updated_at
|
||||
FROM (VALUES #{dropped_values}) val
|
||||
(enrollment_id, assignment_group_id, calculation_details)
|
||||
LEFT OUTER JOIN #{Score.quoted_table_name} scores ON
|
||||
scores.enrollment_id = val.enrollment_id AND
|
||||
scores.assignment_group_id = val.assignment_group_id
|
||||
LEFT OUTER JOIN #{ScoreMetadata.quoted_table_name} metadata ON
|
||||
metadata.score_id = scores.id
|
||||
WHERE
|
||||
metadata.id IS NULL;
|
||||
")
|
||||
end
|
||||
end
|
||||
|
||||
# returns information about assignments groups in the form:
|
||||
|
|
@ -613,7 +625,7 @@ class GradeCalculator
|
|||
Rails.logger.info "GRADES: calculating... submissions=#{logged_submissions.inspect}"
|
||||
|
||||
kept = drop_assignments(group_submissions, group.rules_hash)
|
||||
dropped_submissions = (group_submissions - kept).map { |s| s[:submission].id }
|
||||
dropped_submissions = (group_submissions - kept).map { |s| s[:submission]&.id }.compact
|
||||
|
||||
score, possible = kept.reduce([0, 0]) { |(s_sum,p_sum),s|
|
||||
[s_sum + s[:score], p_sum + s[:total]]
|
||||
|
|
@ -643,10 +655,11 @@ class GradeCalculator
|
|||
Rails.logger.info "GRADES: dropping assignments! #{rules.inspect}"
|
||||
|
||||
cant_drop = []
|
||||
if never_drop_ids.present?
|
||||
cant_drop, submissions = submissions.partition { |s|
|
||||
never_drop_ids.include? s[:assignment].id
|
||||
}
|
||||
if never_drop_ids.present? || @ignore_muted
|
||||
cant_drop, submissions = submissions.partition do |submission|
|
||||
assignment = submission[:assignment]
|
||||
(@ignore_muted && assignment.muted?) || never_drop_ids.include?(assignment.id)
|
||||
end
|
||||
end
|
||||
|
||||
# fudge the drop rules if there aren't enough submissions
|
||||
|
|
|
|||
|
|
@ -1266,11 +1266,19 @@ describe GradeCalculator do
|
|||
end
|
||||
end
|
||||
|
||||
def check_grades(current, final)
|
||||
def check_grades(current, final, check_current: true, check_final: true)
|
||||
GradeCalculator.recompute_final_score(@student.id, @course.id)
|
||||
@enrollment.reload
|
||||
expect(@enrollment.computed_current_score).to eq current
|
||||
expect(@enrollment.computed_final_score).to eq final
|
||||
expect(@enrollment.computed_current_score).to eq current if check_current
|
||||
expect(@enrollment.computed_final_score).to eq final if check_final
|
||||
end
|
||||
|
||||
def check_current_grade(current)
|
||||
check_grades(current, nil, check_current: true, check_final: false)
|
||||
end
|
||||
|
||||
def check_final_grade(final)
|
||||
check_grades(nil, final, check_current: false, check_final: true)
|
||||
end
|
||||
|
||||
it "should work without assignments or submissions" do
|
||||
|
|
@ -1293,6 +1301,78 @@ describe GradeCalculator do
|
|||
check_grades(200.0, 150.0)
|
||||
end
|
||||
|
||||
it "muted assignments are not considered for the drop list when computing " \
|
||||
"current grade for students (they are just excluded from the computation entirely)" do
|
||||
set_grades([[4, 10], [3, 10], [9, 10]])
|
||||
@group.update_attribute(:rules, 'drop_lowest:1')
|
||||
@assignments.first.mute!
|
||||
# 4/10 is excluded from the computation because it's muted
|
||||
# 3/10 is dropped for being the lowest
|
||||
# 9/10 is included
|
||||
# 9/10 => 90.0%
|
||||
check_current_grade(90.0)
|
||||
end
|
||||
|
||||
it "ungraded assignments are not considered for the drop list when computing " \
|
||||
"current grade for students (they are just excluded from the computation entirely)" do
|
||||
set_grades([[nil, 20], [3, 10], [9, 10]])
|
||||
@group.update_attribute(:rules, 'drop_lowest:1')
|
||||
# nil/20 is excluded from the computation because it's not graded
|
||||
# 3/10 is dropped for being the lowest
|
||||
# 9/10 is included
|
||||
# 9/10 => 90.0%
|
||||
check_current_grade(90.0)
|
||||
end
|
||||
|
||||
it "ungraded + muted assignments are not considered for the drop list when " \
|
||||
"computing current grade for students (they are just excluded from the computation entirely)" do
|
||||
set_grades([[nil, 20], [4, 10], [3, 10], [9, 10]])
|
||||
@group.update_attribute(:rules, 'drop_lowest:1')
|
||||
@assignments.second.mute!
|
||||
# nil/20 is excluded from the computation because it's not graded
|
||||
# 4/10 is exclued from the computation because it's muted
|
||||
# 3/10 is dropped for being the lowest
|
||||
# 9/10 is included
|
||||
# 9/10 => 90.0%
|
||||
check_current_grade(90.0)
|
||||
end
|
||||
|
||||
it "ungraded assignments are treated as 0/points_possible for the drop list " \
|
||||
"when computing final grade for students" do
|
||||
set_grades([[nil, 20], [3, 10], [9, 10]])
|
||||
@group.update_attribute(:rules, 'drop_lowest:1')
|
||||
# nil/20 is treated as 0/20 because it's not graded
|
||||
# 3/10 is included
|
||||
# 9/10 is included
|
||||
# 12/20 => 60.0%
|
||||
check_final_grade(60.0)
|
||||
end
|
||||
|
||||
it "muted assignments are not considered for the drop list when computing " \
|
||||
"final grade for students (but they are included as 0/10 in the final computation)" do
|
||||
set_grades([[4, 10], [3, 10], [9, 10]])
|
||||
@group.update_attribute(:rules, 'drop_lowest:1')
|
||||
@assignments.first.mute!
|
||||
# 4/10 is ignored for drop rules because it is muted. it is included
|
||||
# 3/10 is dropped for being the lowest
|
||||
# 9/10 is included
|
||||
# (4/10 is treated as 0/10 because it is muted) + 9/10 = 9/20 => 45.0%
|
||||
check_final_grade(45.0)
|
||||
end
|
||||
|
||||
it "ungraded are treated as 0/points_possible for the drop list and muted " \
|
||||
"assignments are ignored for the drop list when computing final grade for students" do
|
||||
set_grades([[nil, 20], [4, 10], [3, 10], [9, 10]])
|
||||
@group.update_attribute(:rules, 'drop_lowest:1')
|
||||
@assignments.second.mute!
|
||||
# nil/20 is treated as 0/20 because it's not graded. it is dropped.
|
||||
# 4/10 is ignored for drop rules because it is muted. it is included.
|
||||
# 3/10 is included
|
||||
# 9/10 is included
|
||||
# (4/10 is treated as 0/10 because it is muted) + 3/10 + 9/10 = 12/30 => 40.0%
|
||||
check_final_grade(40.0)
|
||||
end
|
||||
|
||||
it 'should "work" when no submissions have points possible' do
|
||||
set_grades [[10,0], [5,0], [20,0], [0,0]]
|
||||
@group.update_attribute(:rules, 'drop_lowest:1')
|
||||
|
|
@ -1527,23 +1607,49 @@ describe GradeCalculator do
|
|||
it "saves dropped submission to group score metadata" do
|
||||
@group.update_attribute(:rules, "drop_lowest:2\nnever_drop:#{@overridden_lowest.id}")
|
||||
GradeCalculator.new(@user.id, @course.id).compute_and_save_scores
|
||||
enrollment = Enrollment.where(user_id: @user.id, course_id: @course.id).first
|
||||
enrollment = Enrollment.find_by(user_id: @user.id, course_id: @course.id)
|
||||
score = enrollment.find_score(assignment_group: @group)
|
||||
expect(score.score_metadata.calculation_details).to eq ({
|
||||
'current' => {'dropped' => [find_submission(@overridden_middle)]},
|
||||
'final' => {'dropped' => [find_submission(@overridden_middle)]}
|
||||
})
|
||||
end
|
||||
|
||||
it "does not include muted assignments in the dropped submission list in group score metadata" do
|
||||
@group.update_attribute(:rules, "drop_lowest:2\nnever_drop:#{@overridden_lowest.id}")
|
||||
@overridden_middle.mute!
|
||||
GradeCalculator.new(@user.id, @course.id).compute_and_save_scores
|
||||
enrollment = Enrollment.where(user_id: @user.id, course_id: @course.id).first
|
||||
score = enrollment.find_score(assignment_group: @group)
|
||||
expect(score.score_metadata.calculation_details).to eq({
|
||||
'current' => {'dropped' => []},
|
||||
'final' => {'dropped' => []}
|
||||
})
|
||||
end
|
||||
|
||||
it "saves dropped submissions to course score metadata" do
|
||||
@group.update_attribute(:rules, "drop_lowest:2\nnever_drop:#{@overridden_lowest.id}")
|
||||
GradeCalculator.new(@user.id, @course.id).compute_and_save_scores
|
||||
enrollment = Enrollment.where(user_id: @user.id, course_id: @course.id).first
|
||||
score = enrollment.find_score(course_score: true)
|
||||
expect(score.score_metadata.calculation_details).to eq ({
|
||||
expect(score.score_metadata.calculation_details).to eq({
|
||||
'current' => {'dropped' => [find_submission(@overridden_middle)]},
|
||||
'final' => {'dropped' => [find_submission(@overridden_middle)]}
|
||||
})
|
||||
end
|
||||
|
||||
it "does not include muted assignments in the dropped submission list in course score metadata" do
|
||||
@group.update_attribute(:rules, "drop_lowest:2\nnever_drop:#{@overridden_lowest.id}")
|
||||
@overridden_middle.mute!
|
||||
GradeCalculator.new(@user.id, @course.id).compute_and_save_scores
|
||||
enrollment = Enrollment.where(user_id: @user.id, course_id: @course.id).first
|
||||
score = enrollment.find_score(course_score: true)
|
||||
expect(score.score_metadata.calculation_details).to eq({
|
||||
'current' => {'dropped' => []},
|
||||
'final' => {'dropped' => []}
|
||||
})
|
||||
end
|
||||
|
||||
it "updates existing course score metadata" do
|
||||
@group.update_attribute(:rules, "drop_lowest:2\nnever_drop:#{@overridden_lowest.id}")
|
||||
GradeCalculator.new(@user.id, @course.id).compute_and_save_scores
|
||||
|
|
|
|||
Loading…
Reference in New Issue