don't include muted assignments in cached grade calculations
Test Plan: * Have canvas and delayed jobs running * Create a course with some assignments * Give scores to some students * Before muting an assignment download the CSV and make sure it's same as UI * Mute an assignment * Download CSV again and the final/current scores should be different than UI * In gradebook UI remove grades from muted assignment (so it's as if they weren't graded) * The UI grades should now be the same as the CSV grades closes #6578 Change-Id: I5774ecb899318278a02e1bc12caaa9d02fa3dac0 Reviewed-on: https://gerrit.instructure.com/7379 Tested-by: Hudson <hudson@instructure.com> Reviewed-by: Brian Palmer <brianp@instructure.com>
This commit is contained in:
parent
e68f1beecc
commit
58c5ab27a7
|
@ -170,7 +170,7 @@ class Assignment < ActiveRecord::Base
|
|||
end
|
||||
|
||||
def update_grades_if_details_changed
|
||||
if @points_possible_was != self.points_possible || @grades_affected
|
||||
if @points_possible_was != self.points_possible || @grades_affected || @muted_was != self.muted
|
||||
begin
|
||||
self.context.recompute_student_scores
|
||||
rescue
|
||||
|
@ -246,6 +246,7 @@ class Assignment < ActiveRecord::Base
|
|||
self.anonymous_peer_reviews = true if self.peer_reviews
|
||||
@workflow_state_was = self.workflow_state_was
|
||||
@points_possible_was = self.points_possible_was
|
||||
@muted_was = self.muted_was
|
||||
@submission_types_was = self.submission_types_was
|
||||
@due_at_was = self.due_at_was
|
||||
self.points_possible = nil if self.submission_types == 'not_graded'
|
||||
|
|
|
@ -0,0 +1,17 @@
|
|||
class RecalculateMutedAssignments < ActiveRecord::Migration
|
||||
def self.up
|
||||
|
||||
course_ids = Assignment.find(:all,
|
||||
:conditions => ['muted = ? AND context_type = ?', true, 'Course'],
|
||||
:select => 'distinct context_id').map(&:context_id)
|
||||
course_ids.each do |id|
|
||||
c = Course.find id
|
||||
c.recompute_student_scores
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
def self.down
|
||||
raise ActiveRecord::IrreversibleMigration
|
||||
end
|
||||
end
|
|
@ -79,8 +79,10 @@ class GradeCalculator
|
|||
:submission_count => 0}
|
||||
|
||||
# collect submissions for this user for all the assignments
|
||||
# if an assignment is muted it will be treated as if there is no submission
|
||||
group_assignments.each do |assignment|
|
||||
submission = submissions.detect { |s| s.assignment_id == assignment.id }
|
||||
submission = nil if assignment.muted
|
||||
submission ||= OpenStruct.new(:assignment_id=>assignment.id, :score=>0) unless ignore_ungraded
|
||||
assignment_submissions << {:assignment => assignment, :submission => submission}
|
||||
end
|
||||
|
|
|
@ -64,57 +64,65 @@ describe GradeCalculator do
|
|||
@user.enrollments.first.computed_current_score.should eql(100.0)
|
||||
@user.enrollments.first.computed_final_score.should eql(60.0)
|
||||
end
|
||||
|
||||
it "should ignore groups with no grades for current grade but not final grade" do
|
||||
|
||||
def two_groups_two_assignments(g1_weight, a1_possible, g2_weight, a2_possible)
|
||||
course_with_student
|
||||
@group = @course.assignment_groups.create!(:name => "some group", :group_weight => 50)
|
||||
@assignment = @group.assignments.build(:title => "some assignments", :points_possible => 10)
|
||||
@group = @course.assignment_groups.create!(:name => "some group", :group_weight => g1_weight)
|
||||
@assignment = @group.assignments.build(:title => "some assignments", :points_possible => a1_possible)
|
||||
@assignment.context = @course
|
||||
@assignment.save!
|
||||
@group2 = @course.assignment_groups.create!(:name => "some other group", :group_weight => 50)
|
||||
@assignment2 = @group2.assignments.build(:title => "some assignments", :points_possible => 10)
|
||||
@group2 = @course.assignment_groups.create!(:name => "some other group", :group_weight => g2_weight)
|
||||
@assignment2 = @group2.assignments.build(:title => "some assignments", :points_possible => a2_possible)
|
||||
@assignment2.context = @course
|
||||
@assignment2.save!
|
||||
@user.enrollments.first.computed_current_score.should eql(nil)
|
||||
@user.enrollments.first.computed_final_score.should eql(0.0)
|
||||
@submission = @assignment.grade_student(@user, :grade => "5")
|
||||
@submission[0].score.should eql(5.0)
|
||||
@user.reload
|
||||
@user.enrollments.first.computed_current_score.should eql(50.0)
|
||||
@user.enrollments.first.computed_final_score.should eql(25.0)
|
||||
end
|
||||
|
||||
it "should ignore groups with no grades for current grade calculation, even when weighted" do
|
||||
course_with_student
|
||||
@group = @course.assignment_groups.create!(:name => "some group", :group_weight => 50)
|
||||
@assignment = @group.assignments.build(:title => "some assignments", :points_possible => 10)
|
||||
@assignment.context = @course
|
||||
@assignment.save!
|
||||
@group2 = @course.assignment_groups.create!(:name => "some other group", :group_weight => 50)
|
||||
@assignment2 = @group2.assignments.build(:title => "some assignments", :points_possible => 10)
|
||||
@assignment2.context = @course
|
||||
@assignment2.save!
|
||||
@user.enrollments.first.computed_current_score.should eql(nil)
|
||||
@user.enrollments.first.computed_final_score.should eql(0.0)
|
||||
@submission = @assignment.grade_student(@user, :grade => "5")
|
||||
@submission[0].score.should eql(5.0)
|
||||
@course.group_weighting_scheme = "percent"
|
||||
@course.save!
|
||||
@user.reload
|
||||
@user.enrollments.first.computed_current_score.should eql(50.0)
|
||||
@user.enrollments.first.computed_final_score.should eql(25.0)
|
||||
describe "group with no grade or muted grade" do
|
||||
before(:each) do
|
||||
two_groups_two_assignments(50, 10, 50, 10)
|
||||
@user.enrollments.first.computed_current_score.should eql(nil)
|
||||
@user.enrollments.first.computed_final_score.should eql(0.0)
|
||||
@submission = @assignment.grade_student(@user, :grade => "5")
|
||||
@submission[0].score.should eql(5.0)
|
||||
end
|
||||
|
||||
it "should ignore no grade for current grade but not final grade" do
|
||||
@user.reload
|
||||
@user.enrollments.first.computed_current_score.should eql(50.0)
|
||||
@user.enrollments.first.computed_final_score.should eql(25.0)
|
||||
end
|
||||
|
||||
it "should ignore muted grade for current grade but not final grade" do
|
||||
# should have same scores as previous spec despite having a grade
|
||||
@assignment2.mute!
|
||||
@assignment2.grade_student(@user, :grade => "500")
|
||||
@user.reload
|
||||
@user.enrollments.first.computed_current_score.should eql(50.0)
|
||||
@user.enrollments.first.computed_final_score.should eql(25.0)
|
||||
end
|
||||
|
||||
it "should ignore no grade for current grade calculation, even when weighted" do
|
||||
@course.group_weighting_scheme = "percent"
|
||||
@course.save!
|
||||
@user.reload
|
||||
@user.enrollments.first.computed_current_score.should eql(50.0)
|
||||
@user.enrollments.first.computed_final_score.should eql(25.0)
|
||||
end
|
||||
|
||||
it "should ignore muted grade for current grade calculation, even when weighted" do
|
||||
# should have same scores as previous spec despite having a grade
|
||||
@assignment2.mute!
|
||||
@assignment2.grade_student(@user, :grade => "500")
|
||||
@course.group_weighting_scheme = "percent"
|
||||
@course.save!
|
||||
@user.reload
|
||||
@user.enrollments.first.computed_current_score.should eql(50.0)
|
||||
@user.enrollments.first.computed_final_score.should eql(25.0)
|
||||
end
|
||||
end
|
||||
|
||||
it "should compute a weighted grade when specified" do
|
||||
course_with_student
|
||||
@group = @course.assignment_groups.create!(:name => "some group", :group_weight => 50)
|
||||
@assignment = @group.assignments.build(:title => "some assignments", :points_possible => 10)
|
||||
@assignment.context = @course
|
||||
@assignment.save!
|
||||
@group2 = @course.assignment_groups.create!(:name => "some other group", :group_weight => 50)
|
||||
@assignment2 = @group2.assignments.build(:title => "some assignments", :points_possible => 40)
|
||||
@assignment2.context = @course
|
||||
@assignment2.save!
|
||||
two_groups_two_assignments(50, 10, 50, 40)
|
||||
@user.enrollments.first.computed_current_score.should eql(nil)
|
||||
@user.enrollments.first.computed_final_score.should eql(0.0)
|
||||
@submission = @assignment.grade_student(@user, :grade => "9")
|
||||
|
@ -140,15 +148,7 @@ describe GradeCalculator do
|
|||
end
|
||||
|
||||
it "should incorporate extra credit when the weighted total is more than 100%" do
|
||||
course_with_student
|
||||
@group = @course.assignment_groups.create!(:name => "some group", :group_weight => 50)
|
||||
@assignment = @group.assignments.build(:title => "some assignments", :points_possible => 10)
|
||||
@assignment.context = @course
|
||||
@assignment.save!
|
||||
@group2 = @course.assignment_groups.create!(:name => "some other group", :group_weight => 60)
|
||||
@assignment2 = @group2.assignments.build(:title => "some assignments", :points_possible => 40)
|
||||
@assignment2.context = @course
|
||||
@assignment2.save!
|
||||
two_groups_two_assignments(50, 10, 60, 40)
|
||||
@user.enrollments.first.computed_current_score.should eql(nil)
|
||||
@user.enrollments.first.computed_final_score.should eql(0.0)
|
||||
@submission = @assignment.grade_student(@user, :grade => "10")
|
||||
|
@ -174,15 +174,7 @@ describe GradeCalculator do
|
|||
end
|
||||
|
||||
it "should incorporate extra credit when the total is more than the possible" do
|
||||
course_with_student
|
||||
@group = @course.assignment_groups.create!(:name => "some group", :group_weight => 50)
|
||||
@assignment = @group.assignments.build(:title => "some assignments", :points_possible => 10)
|
||||
@assignment.context = @course
|
||||
@assignment.save!
|
||||
@group2 = @course.assignment_groups.create!(:name => "some other group", :group_weight => 60)
|
||||
@assignment2 = @group2.assignments.build(:title => "some assignments", :points_possible => 40)
|
||||
@assignment2.context = @course
|
||||
@assignment2.save!
|
||||
two_groups_two_assignments(50, 10, 60, 40)
|
||||
@user.enrollments.first.computed_current_score.should eql(nil)
|
||||
@user.enrollments.first.computed_final_score.should eql(0.0)
|
||||
@submission = @assignment.grade_student(@user, :grade => "11")
|
||||
|
@ -208,15 +200,7 @@ describe GradeCalculator do
|
|||
end
|
||||
|
||||
it "should properly calculate the grade when total weight is less than 100%" do
|
||||
course_with_student
|
||||
@group = @course.assignment_groups.create!(:name => "some group", :group_weight => 50)
|
||||
@assignment = @group.assignments.build(:title => "some assignments", :points_possible => 10)
|
||||
@assignment.context = @course
|
||||
@assignment.save!
|
||||
@group2 = @course.assignment_groups.create!(:name => "some other group", :group_weight => 40)
|
||||
@assignment2 = @group2.assignments.build(:title => "some assignments", :points_possible => 40)
|
||||
@assignment2.context = @course
|
||||
@assignment2.save!
|
||||
two_groups_two_assignments(50, 10, 40, 40)
|
||||
@submission = @assignment.grade_student(@user, :grade => "10")
|
||||
@course.group_weighting_scheme = "percent"
|
||||
@course.save!
|
||||
|
@ -231,42 +215,7 @@ describe GradeCalculator do
|
|||
@user.enrollments.first.computed_current_score.should eql(90.0)
|
||||
@user.enrollments.first.computed_final_score.should eql(90.0)
|
||||
end
|
||||
|
||||
it "should properly handle submissions with no score" do
|
||||
course_with_student
|
||||
@group = @course.assignment_groups.create!(:name => "group2", :group_weight => 50)
|
||||
@assignment_1 = @group.assignments.build(:title => "some assignments", :points_possible => 10)
|
||||
@assignment_1.context = @course
|
||||
@assignment_1.save!
|
||||
@assignment_2 = @group.assignments.build(:title => "some assignments", :points_possible => 4)
|
||||
@assignment_2.context = @course
|
||||
@assignment_2.save!
|
||||
@group2 = @course.assignment_groups.create!(:name => "assignments", :group_weight => 40)
|
||||
@assignment2_1 = @group2.assignments.build(:title => "some assignments", :points_possible => 40)
|
||||
@assignment2_1.context = @course
|
||||
@assignment2_1.save!
|
||||
|
||||
@assignment_1.grade_student(@user, :grade => nil)
|
||||
@assignment_2.grade_student(@user, :grade => "1")
|
||||
@assignment2_1.grade_student(@user, :grade => "40")
|
||||
|
||||
|
||||
@course.group_weighting_scheme = "percent"
|
||||
@course.save!
|
||||
@user.reload
|
||||
|
||||
@user.enrollments.first.computed_current_score.should eql(52.5)
|
||||
@user.enrollments.first.computed_final_score.should eql(43.6)
|
||||
|
||||
@course.group_weighting_scheme = nil
|
||||
@course.save!
|
||||
@user.reload
|
||||
|
||||
@user.enrollments.first.computed_current_score.should eql(93.2)
|
||||
@user.enrollments.first.computed_final_score.should eql(75.9)
|
||||
|
||||
end
|
||||
|
||||
|
||||
it "should properly calculate the grade when there are 'not graded' assignments with scores" do
|
||||
course_with_student
|
||||
@group = @course.assignment_groups.create!(:name => "some group")
|
||||
|
@ -285,7 +234,7 @@ describe GradeCalculator do
|
|||
@user.enrollments.first.computed_final_score.should eql(90.0)
|
||||
end
|
||||
|
||||
it "should recalculate all cached grades when an assignment is deleted/restored" do
|
||||
def two_graded_assignments
|
||||
course_with_student
|
||||
@group = @course.assignment_groups.create!(:name => "some group")
|
||||
@assignment = @group.assignments.build(:title => "some assignments", :points_possible => 5)
|
||||
|
@ -298,13 +247,16 @@ describe GradeCalculator do
|
|||
@submission2 = @assignment2.grade_student(@user, :grade => "4")
|
||||
@course.save!
|
||||
@user.reload
|
||||
|
||||
|
||||
@user.enrollments.first.computed_current_score.should eql(60.0)
|
||||
@user.enrollments.first.computed_final_score.should eql(60.0)
|
||||
|
||||
end
|
||||
|
||||
it "should recalculate all cached grades when an assignment is deleted/restored" do
|
||||
two_graded_assignments
|
||||
@assignment2.destroy
|
||||
@user.reload
|
||||
@user.enrollments.first.computed_current_score.should eql(40.0) # only the 2/5 should be there
|
||||
@user.enrollments.first.computed_current_score.should eql(40.0) # 2/5
|
||||
@user.enrollments.first.computed_final_score.should eql(40.0)
|
||||
|
||||
@assignment2.restore
|
||||
|
@ -312,6 +264,72 @@ describe GradeCalculator do
|
|||
@user.enrollments.first.computed_current_score.should eql(60.0)
|
||||
@user.enrollments.first.computed_final_score.should eql(60.0)
|
||||
end
|
||||
|
||||
it "should recalculate all cached grades when an assignment is muted/unmuted" do
|
||||
two_graded_assignments
|
||||
@assignment2.mute!
|
||||
@user.reload
|
||||
@user.enrollments.first.computed_current_score.should eql(40.0) # 2/5
|
||||
@user.enrollments.first.computed_final_score.should eql(20.0) # 2/10
|
||||
|
||||
@assignment2.unmute!
|
||||
@user.reload
|
||||
@user.enrollments.first.computed_current_score.should eql(60.0)
|
||||
@user.enrollments.first.computed_final_score.should eql(60.0)
|
||||
end
|
||||
|
||||
def nil_graded_assignment
|
||||
course_with_student
|
||||
@group = @course.assignment_groups.create!(:name => "group2", :group_weight => 50)
|
||||
@assignment_1 = @group.assignments.build(:title => "some assignments", :points_possible => 10)
|
||||
@assignment_1.context = @course
|
||||
@assignment_1.save!
|
||||
@assignment_2 = @group.assignments.build(:title => "some assignments", :points_possible => 4)
|
||||
@assignment_2.context = @course
|
||||
@assignment_2.save!
|
||||
@group2 = @course.assignment_groups.create!(:name => "assignments", :group_weight => 40)
|
||||
@assignment2_1 = @group2.assignments.build(:title => "some assignments", :points_possible => 40)
|
||||
@assignment2_1.context = @course
|
||||
@assignment2_1.save!
|
||||
|
||||
@assignment_1.grade_student(@user, :grade => nil)
|
||||
@assignment_2.grade_student(@user, :grade => "1")
|
||||
@assignment2_1.grade_student(@user, :grade => "40")
|
||||
end
|
||||
|
||||
it "should properly handle submissions with no score" do
|
||||
nil_graded_assignment
|
||||
|
||||
@user.reload
|
||||
@user.enrollments.first.computed_current_score.should eql(93.2)
|
||||
@user.enrollments.first.computed_final_score.should eql(75.9)
|
||||
|
||||
@course.group_weighting_scheme = "percent"
|
||||
@course.save!
|
||||
|
||||
@user.reload
|
||||
@user.enrollments.first.computed_current_score.should eql(52.5)
|
||||
@user.enrollments.first.computed_final_score.should eql(43.6)
|
||||
end
|
||||
|
||||
it "should treat muted assignments as if there is no submission" do
|
||||
# should have same scores as previous spec despite having a grade
|
||||
nil_graded_assignment()
|
||||
|
||||
@assignment_1.mute!
|
||||
@assignment_1.grade_student(@user, :grade => 500)
|
||||
|
||||
@user.reload
|
||||
@user.enrollments.first.computed_current_score.should eql(93.2)
|
||||
@user.enrollments.first.computed_final_score.should eql(75.9)
|
||||
|
||||
@course.group_weighting_scheme = "percent"
|
||||
@course.save!
|
||||
|
||||
@user.reload
|
||||
@user.enrollments.first.computed_current_score.should eql(52.5)
|
||||
@user.enrollments.first.computed_final_score.should eql(43.6)
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
|
|
|
@ -0,0 +1,40 @@
|
|||
#
|
||||
# Copyright (C) 2011 Instructure, Inc.
|
||||
#
|
||||
# This file is part of Canvas.
|
||||
#
|
||||
# Canvas is free software: you can redistribute it and/or modify it under
|
||||
# the terms of the GNU Affero General Public License as published by the Free
|
||||
# Software Foundation, version 3 of the License.
|
||||
#
|
||||
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
|
||||
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
|
||||
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
|
||||
# details.
|
||||
#
|
||||
# You should have received a copy of the GNU Affero General Public License along
|
||||
# with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
#
|
||||
|
||||
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
|
||||
require 'db/migrate/20111209171640_recalculate_muted_assignments.rb'
|
||||
|
||||
describe RecalculateMutedAssignments do
|
||||
describe "up" do
|
||||
it "should work" do
|
||||
c1 = course
|
||||
a1 = c1.assignments.create!(:title => "Test Assignment")
|
||||
c1.any_instantiation
|
||||
c1.expects(:recompute_student_scores).never
|
||||
|
||||
c2 = course
|
||||
a2 = c2.assignments.create!(:title => "Test Assignment2")
|
||||
a2.mute!
|
||||
c2.any_instantiation
|
||||
c2.expects :recompute_student_scores
|
||||
|
||||
RecalculateMutedAssignments.up
|
||||
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue