update grading period scores when student is graded
When a student is graded, the score for their grading period will now
be updated in addition to their total course score. Previously, when a
student was graded, their grading period score was not updated.
closes CNVS-26709
test plan:
1) In a course with multiple grading periods enabled and grading
periods set up, grade students for assignments that fall in grading
periods and make sure the scores get updated for the grading period
and for the course total.
* To make sure the scores get updated for the grading period, open a
rails console and find the grading period scores (we'll assume the
student's enrollment ID is 5, and the ID of the grading period
that the assignment falls in is 8).
$ Enrollment.find(5).scores.where(grading_period_id: 8)
Then, give a different grade to the student for the assignment
that falls in the grading period and run the command above again.
You should see that the current_score and final_score have
changed.
* To make sure the scores get updated for the course total, open a
rails console and find the course total scores (we'll assume the
student's enrollment ID is 5).
$ Enrollment.find(5).scores.where(grading_period_id: nil)
Then, give a different grade to the student for the assignment
that falls in the grading period and run the command above again.
You should see that the current_score and final_score have
changed.
Edge case: make sure that grading period scores continue to get updated
even if MGP is turned off. To verify this, turn off MGP in a course
that originally had MGP turned on. Then, grade students in assignments
that fell in a grading period when MGP was enabled. After grading, the
grading period scores should be updated.
Change-Id: I29e4e22a0b73d1062a93c6ad420d876c2f5d040a
Reviewed-on: https://gerrit.instructure.com/98603
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Derek Bender <djbender@instructure.com>
Tested-by: Jenkins
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
This commit is contained in:
Spencer Olson
parent
43e64d7c52
commit
37fa769aef
|
|
@ -1083,10 +1083,10 @@ class Course < ActiveRecord::Base
|
|||
end
|
||||
end
|
||||
|
||||
def recompute_student_scores(student_ids = nil)
|
||||
def recompute_student_scores(student_ids = nil, grading_period_id: nil)
|
||||
student_ids ||= self.student_ids
|
||||
Rails.logger.info "GRADES: recomputing scores in course=#{global_id} students=#{student_ids.inspect}"
|
||||
Enrollment.recompute_final_score(student_ids, self.id)
|
||||
Enrollment.recompute_final_score(student_ids, self.id, grading_period_id: grading_period_id)
|
||||
end
|
||||
handle_asynchronously_if_production :recompute_student_scores,
|
||||
:singleton => proc { |c| "recompute_student_scores:#{ c.global_id }" }
|
||||
|
|
|
|||
|
|
@ -984,39 +984,45 @@ class Enrollment < ActiveRecord::Base
|
|||
# please add appropriate calls to this so that the cached values don't get
|
||||
# stale! And once you've added the call, add the condition to the comment
|
||||
# here for future enlightenment.
|
||||
def self.recompute_final_score(user_ids, course_id)
|
||||
GradeCalculator.recompute_final_score(user_ids, course_id)
|
||||
|
||||
def self.recompute_final_score(*args)
|
||||
GradeCalculator.recompute_final_score(*args)
|
||||
end
|
||||
|
||||
def self.recompute_final_score_if_stale(course, user=nil)
|
||||
Rails.cache.fetch(['recompute_final_scores', course.id, user].cache_key, :expires_in => Setting.get('recompute_grades_window', 600).to_i.seconds) do
|
||||
recompute_final_score user ? user.id : course.student_enrollments.except(:preload).distinct.pluck(:user_id), course.id
|
||||
def self.recompute_final_score_if_stale(course, user=nil, compute_score_opts = {})
|
||||
Rails.cache.fetch(
|
||||
['recompute_final_scores', course.id, user, compute_score_opts[:grading_period_id]].cache_key,
|
||||
expires_in: Setting.get('recompute_grades_window', 600).to_i.seconds
|
||||
) do
|
||||
user_id = user ? user.id : course.student_enrollments.except(:preload).distinct.pluck(:user_id)
|
||||
recompute_final_score(user_id, course.id, compute_score_opts)
|
||||
yield if block_given?
|
||||
true
|
||||
end
|
||||
end
|
||||
|
||||
def computed_current_grade
|
||||
cached_score_or_grade(:current, :grade)
|
||||
def computed_current_grade(grading_period_id: nil)
|
||||
cached_score_or_grade(:current, :grade, grading_period_id: grading_period_id)
|
||||
end
|
||||
|
||||
def computed_final_grade
|
||||
cached_score_or_grade(:final, :grade)
|
||||
def computed_final_grade(grading_period_id: nil)
|
||||
cached_score_or_grade(:final, :grade, grading_period_id: grading_period_id)
|
||||
end
|
||||
|
||||
def computed_current_score
|
||||
cached_score_or_grade(:current, :score)
|
||||
def computed_current_score(grading_period_id: nil)
|
||||
cached_score_or_grade(:current, :score, grading_period_id: grading_period_id)
|
||||
end
|
||||
|
||||
def computed_final_score
|
||||
cached_score_or_grade(:final, :score)
|
||||
def computed_final_score(grading_period_id: nil)
|
||||
cached_score_or_grade(:final, :score, grading_period_id: grading_period_id)
|
||||
end
|
||||
|
||||
def cached_score_or_grade(current_or_final, score_or_grade)
|
||||
score = find_score
|
||||
def cached_score_or_grade(current_or_final, score_or_grade, grading_period_id: nil)
|
||||
score = find_score(grading_period_id: grading_period_id)
|
||||
if score.present?
|
||||
score.send("#{current_or_final}_#{score_or_grade}")
|
||||
else
|
||||
return nil if grading_period_id.present?
|
||||
# TODO: drop the computed_current_score / computed_final_score columns
|
||||
# after the data fixup to populate the scores table completes
|
||||
score = read_attribute("computed_#{current_or_final}_score")
|
||||
|
|
|
|||
|
|
@ -32,6 +32,8 @@ class GradingPeriod < ActiveRecord::Base
|
|||
before_validation :adjust_close_date_for_course_period
|
||||
before_validation :ensure_close_date
|
||||
|
||||
after_save :recompute_scores, if: :dates_changed?
|
||||
|
||||
scope :current, -> do
|
||||
period_table = GradingPeriod.arel_table
|
||||
now = Time.zone.now
|
||||
|
|
@ -208,4 +210,19 @@ class GradingPeriod < ActiveRecord::Base
|
|||
errors.add(:close_date, t('must be on or after end date'))
|
||||
end
|
||||
end
|
||||
|
||||
def recompute_scores
|
||||
if course_group?
|
||||
courses = [grading_period_group.course]
|
||||
else
|
||||
term_ids = grading_period_group.enrollment_terms.pluck(:id)
|
||||
courses = Course.where(enrollment_term_id: term_ids)
|
||||
end
|
||||
# Course#recompute_student_scores is asynchronous
|
||||
courses.each { |course| course.recompute_student_scores(grading_period_id: self.id) }
|
||||
end
|
||||
|
||||
def dates_changed?
|
||||
start_date_changed? || end_date_changed?
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -398,7 +398,19 @@ class Submission < ActiveRecord::Base
|
|||
else
|
||||
Rails.logger.info "GRADES: submission #{global_id} score changed. recomputing grade for course #{context.global_id} user #{user_id}."
|
||||
self.class.connection.after_transaction_commit do
|
||||
Enrollment.send_later_if_production_enqueue_args(:recompute_final_score, { run_at: 3.seconds.from_now }, self.user_id, self.context.id)
|
||||
effective_due_dates = EffectiveDueDates.new(self.context, self.assignment_id)
|
||||
grading_period_id = effective_due_dates.grading_period_id_for(
|
||||
student_id: self.user_id,
|
||||
assignment_id: self.assignment_id
|
||||
)
|
||||
Enrollment.send_later_if_production_enqueue_args(
|
||||
:recompute_final_score,
|
||||
{ run_at: 3.seconds.from_now },
|
||||
self.user_id,
|
||||
self.context.id,
|
||||
grading_period_id: grading_period_id,
|
||||
update_all_grading_period_scores: false
|
||||
)
|
||||
end
|
||||
end
|
||||
self.assignment.send_later_if_production(:multiple_module_actions, [self.user_id], :scored, self.score) if self.assignment
|
||||
|
|
|
|||
|
|
@ -78,8 +78,20 @@ class EffectiveDueDates
|
|||
end
|
||||
end
|
||||
|
||||
def grading_period_id_for(student_id:, assignment_id:)
|
||||
find_effective_due_date(student_id, assignment_id).fetch(:grading_period_id, nil)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def find_effective_due_date(student_id, assignment_id)
|
||||
find_effective_due_dates_for_assignment(assignment_id).fetch(student_id, {})
|
||||
end
|
||||
|
||||
def find_effective_due_dates_for_assignment(assignment_id)
|
||||
to_hash.fetch(assignment_id, {})
|
||||
end
|
||||
|
||||
def usable_student_id?(student_id)
|
||||
return false unless student_id.present?
|
||||
return false unless student_id.to_i.present?
|
||||
|
|
|
|||
|
|
@ -20,7 +20,11 @@ class GradeCalculator
|
|||
attr_accessor :submissions, :assignments, :groups
|
||||
|
||||
def initialize(user_ids, course, opts = {})
|
||||
opts = opts.reverse_merge(:ignore_muted => true)
|
||||
opts = opts.reverse_merge(
|
||||
ignore_muted: true,
|
||||
update_all_grading_period_scores: true,
|
||||
update_course_score: true
|
||||
)
|
||||
|
||||
Rails.logger.info("GRADES: calc args: user_ids=#{user_ids.inspect}")
|
||||
Rails.logger.info("GRADES: calc args: course=#{course.inspect}")
|
||||
|
|
@ -29,6 +33,12 @@ class GradeCalculator
|
|||
@course = course.is_a?(Course) ? course : Course.find(course)
|
||||
@groups = @course.assignment_groups.active
|
||||
@grading_period = opts[:grading_period]
|
||||
# if we're updating an overall course score (no grading period specified), we
|
||||
# want to first update all grading period scores for the users
|
||||
@update_all_grading_period_scores = @grading_period.nil? && opts[:update_all_grading_period_scores]
|
||||
# if we're updating a grading period score, we also need to update the
|
||||
# overall course score
|
||||
@update_course_score = @grading_period.present? && opts[:update_course_score]
|
||||
@assignments = @course.assignments.published.gradeable.to_a
|
||||
@user_ids = Array(user_ids).map { |id| Shard.relative_id_for(id, Shard.current, @course.shard) }
|
||||
@current_updates = {}
|
||||
|
|
@ -37,12 +47,19 @@ class GradeCalculator
|
|||
end
|
||||
|
||||
# recomputes the scores and saves them to each user's Enrollment
|
||||
def self.recompute_final_score(user_ids, course_id)
|
||||
def self.recompute_final_score(user_ids, course_id, compute_score_opts = {})
|
||||
user_ids = Array(user_ids).uniq.map(&:to_i)
|
||||
return if user_ids.empty?
|
||||
course = Course.active.find(course_id)
|
||||
grading_period = GradingPeriod.for(course).find_by(
|
||||
id: compute_score_opts.delete(:grading_period_id)
|
||||
)
|
||||
user_ids.in_groups_of(1000, false) do |user_ids_group|
|
||||
calc = GradeCalculator.new user_ids_group, course_id
|
||||
calc.compute_and_save_scores
|
||||
GradeCalculator.new(
|
||||
user_ids_group,
|
||||
course_id,
|
||||
compute_score_opts.merge(grading_period: grading_period)
|
||||
).compute_and_save_scores
|
||||
end
|
||||
end
|
||||
|
||||
|
|
@ -78,12 +95,40 @@ class GradeCalculator
|
|||
end
|
||||
|
||||
def compute_and_save_scores
|
||||
calculate_grading_period_scores if @update_all_grading_period_scores
|
||||
compute_scores
|
||||
save_scores
|
||||
calculate_course_score if @update_course_score
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def calculate_grading_period_scores
|
||||
GradingPeriod.for(@course).each do |grading_period|
|
||||
# update this grading period score, and do not
|
||||
# update any other scores (grading period or course)
|
||||
# after this one
|
||||
GradeCalculator.new(
|
||||
@user_ids,
|
||||
@course,
|
||||
update_all_grading_period_scores: false,
|
||||
update_course_score: false,
|
||||
grading_period: grading_period
|
||||
).compute_and_save_scores
|
||||
end
|
||||
end
|
||||
|
||||
def calculate_course_score
|
||||
# update the overall course score now that we've finished
|
||||
# updating the grading period score
|
||||
GradeCalculator.new(
|
||||
@user_ids,
|
||||
@course,
|
||||
update_all_grading_period_scores: false,
|
||||
update_course_score: false
|
||||
).compute_and_save_scores
|
||||
end
|
||||
|
||||
def enrollments
|
||||
@enrollments ||= Enrollment.shard(@course).active.
|
||||
where(user_id: @user_ids, course_id: @course.id).
|
||||
|
|
|
|||
|
|
@ -52,19 +52,17 @@ describe DataFixup::PopulateScoresTable do
|
|||
@student2_enrollment.scores.create!(grading_period_id: nil, final_score: 29.3)
|
||||
create_grading_periods_for(@course2)
|
||||
gp = @course2.grading_periods.first
|
||||
@student3_enrollment.scores.create!(grading_period: gp, final_score: 100.3)
|
||||
|
||||
expect { DataFixup::PopulateScoresTable.run }.to change { Score.count }.from(2).to(5)
|
||||
.and change { @student1_enrollment.scores.count }.from(0).to(1)
|
||||
@student3_enrollment.scores.where(grading_period: gp).first.update(final_score: 100.3)
|
||||
@student3_enrollment.scores.where(grading_period_id: nil).first.delete
|
||||
expect { DataFixup::PopulateScoresTable.run }.to change { Score.count }.from(4).to(6)
|
||||
.and change { @student3_enrollment.scores.count }.from(1).to(2)
|
||||
.and change { @student4_enrollment.scores.count }.from(0).to(1)
|
||||
expect(Score.where(grading_period: nil).pluck(:current_score, :final_score)).to match_array(
|
||||
[[76.3, 24], [nil, 29.3], [56.5, nil], [nil, nil]]
|
||||
)
|
||||
expect(Score.pluck(:grading_period_id)).to match_array([nil, nil, gp.id, nil, nil])
|
||||
expect(Score.pluck(:grading_period_id)).to match_array([nil, nil, gp.id, gp.id, nil, nil])
|
||||
end
|
||||
|
||||
it "creates Score object for each grading period" do
|
||||
it "creates Score object for each grading period, if one doesn't already exist" do
|
||||
grader1 = teacher_in_course(course: @course1, active_all: true).user
|
||||
grader2 = teacher_in_course(course: @course2, active_all: true).user
|
||||
group1 = @course1.grading_period_groups.create!
|
||||
|
|
@ -83,6 +81,7 @@ describe DataFixup::PopulateScoresTable do
|
|||
assignment3.grade_student(@student4_enrollment.user, grader: grader2, score: 20)
|
||||
assignment4 = @course2.assignments.create!(due_at: 6.months.ago, points_possible: 100)
|
||||
assignment4.grade_student(@student3_enrollment.user, grader: grader2, score: 30)
|
||||
Score.where.not(grading_period_id: nil).delete_all
|
||||
|
||||
expect { DataFixup::PopulateScoresTable.run }.to change { Score.count }.from(4).to(10)
|
||||
.and change { @student1_enrollment.scores.count }.from(1).to(3)
|
||||
|
|
|
|||
|
|
@ -1276,7 +1276,7 @@ describe Course do
|
|||
end
|
||||
end
|
||||
|
||||
describe '#any_in_closed_grading_period?' do
|
||||
context 'grading periods' do
|
||||
before(:once) do
|
||||
@now = Time.zone.now
|
||||
@test_course = Course.create!
|
||||
|
|
@ -1286,141 +1286,147 @@ describe Course do
|
|||
@assignment2 = @test_course.assignments.create!
|
||||
@gp_group = Factories::GradingPeriodGroupHelper.new.create_for_account(@test_course.account)
|
||||
@gp_group.enrollment_terms << @test_course.enrollment_term
|
||||
end
|
||||
|
||||
it 'returns false if multiple grading periods is disabled' do
|
||||
Factories::GradingPeriodHelper.new.create_for_group(@gp_group, {
|
||||
@grading_period = Factories::GradingPeriodHelper.new.create_for_group(
|
||||
@gp_group,
|
||||
start_date: 20.days.ago(@now),
|
||||
end_date: 15.days.ago(@now),
|
||||
close_date: 10.days.ago(@now)
|
||||
})
|
||||
@assignment2.due_at = 17.days.ago(@now)
|
||||
@assignment2.only_visible_to_overrides = false
|
||||
@assignment2.save!
|
||||
|
||||
@test_course.disable_feature! :multiple_grading_periods
|
||||
edd = EffectiveDueDates.for_course(@test_course)
|
||||
edd.expects(:to_hash).never
|
||||
expect(edd.any_in_closed_grading_period?).to eq(false)
|
||||
)
|
||||
end
|
||||
|
||||
context 'with multiple grading periods' do
|
||||
before(:once) do
|
||||
@test_course.enable_feature! :multiple_grading_periods
|
||||
end
|
||||
|
||||
it 'returns true if any students in any assignments have a due date in a closed grading period' do
|
||||
Factories::GradingPeriodHelper.new.create_for_group(@gp_group, {
|
||||
start_date: 20.days.ago(@now),
|
||||
end_date: 15.days.ago(@now),
|
||||
close_date: 10.days.ago(@now)
|
||||
})
|
||||
@assignment2.due_at = 1.day.ago(@now)
|
||||
describe '#any_in_closed_grading_period?' do
|
||||
it 'returns false if multiple grading periods is disabled' do
|
||||
@assignment2.due_at = 17.days.ago(@now)
|
||||
@assignment2.only_visible_to_overrides = false
|
||||
@assignment2.save!
|
||||
override = @assignment2.assignment_overrides.create!(due_at: 19.days.ago(@now), due_at_overridden: true)
|
||||
override.assignment_override_students.create!(user: @student2)
|
||||
|
||||
edd = EffectiveDueDates.for_course(@test_course)
|
||||
expect(edd.any_in_closed_grading_period?).to eq(true)
|
||||
end
|
||||
|
||||
it 'returns false if no student in any assignments has a due date in a closed grading period' do
|
||||
Factories::GradingPeriodHelper.new.create_for_group(@gp_group, {
|
||||
start_date: 20.days.ago(@now),
|
||||
end_date: 15.days.ago(@now),
|
||||
close_date: 10.days.ago(@now)
|
||||
})
|
||||
@assignment2.due_at = 1.day.ago(@now)
|
||||
@assignment2.only_visible_to_overrides = false
|
||||
@assignment2.save!
|
||||
override = @assignment2.assignment_overrides.create!(due_at: 2.days.ago(@now), due_at_overridden: true)
|
||||
override.assignment_override_students.create!(user: @student2)
|
||||
|
||||
@test_course.disable_feature! :multiple_grading_periods
|
||||
edd = EffectiveDueDates.for_course(@test_course)
|
||||
edd.expects(:to_hash).never
|
||||
expect(edd.any_in_closed_grading_period?).to eq(false)
|
||||
end
|
||||
|
||||
it 'memoizes the result' do
|
||||
edd = EffectiveDueDates.for_course(@test_course)
|
||||
edd.expects(:to_hash).once.returns({})
|
||||
2.times { edd.any_in_closed_grading_period? }
|
||||
context 'with multiple grading periods' do
|
||||
before(:once) do
|
||||
@test_course.enable_feature! :multiple_grading_periods
|
||||
end
|
||||
|
||||
it 'returns true if any students in any assignments have a due date in a closed grading period' do
|
||||
@assignment2.due_at = 1.day.ago(@now)
|
||||
@assignment2.only_visible_to_overrides = false
|
||||
@assignment2.save!
|
||||
override = @assignment2.assignment_overrides.create!(due_at: 19.days.ago(@now), due_at_overridden: true)
|
||||
override.assignment_override_students.create!(user: @student2)
|
||||
|
||||
edd = EffectiveDueDates.for_course(@test_course)
|
||||
expect(edd.any_in_closed_grading_period?).to eq(true)
|
||||
end
|
||||
|
||||
it 'returns false if no student in any assignments has a due date in a closed grading period' do
|
||||
@assignment2.due_at = 1.day.ago(@now)
|
||||
@assignment2.only_visible_to_overrides = false
|
||||
@assignment2.save!
|
||||
override = @assignment2.assignment_overrides.create!(due_at: 2.days.ago(@now), due_at_overridden: true)
|
||||
override.assignment_override_students.create!(user: @student2)
|
||||
|
||||
edd = EffectiveDueDates.for_course(@test_course)
|
||||
expect(edd.any_in_closed_grading_period?).to eq(false)
|
||||
end
|
||||
|
||||
it 'memoizes the result' do
|
||||
edd = EffectiveDueDates.for_course(@test_course)
|
||||
edd.expects(:to_hash).once.returns({})
|
||||
2.times { edd.any_in_closed_grading_period? }
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#in_closed_grading_period?' do
|
||||
before(:once) do
|
||||
@now = Time.zone.now
|
||||
@test_course = Course.create!
|
||||
@student1 = student_in_course(course: @test_course, active_all: true).user
|
||||
@student2 = student_in_course(course: @test_course, active_all: true).user
|
||||
@assignment1 = @test_course.assignments.create!(due_at: 2.weeks.from_now(@now))
|
||||
@assignment2 = @test_course.assignments.create!
|
||||
@gp_group = Factories::GradingPeriodGroupHelper.new.create_for_account(@test_course.account)
|
||||
@gp_group.enrollment_terms << @test_course.enrollment_term
|
||||
end
|
||||
|
||||
it 'returns false if multiple grading periods is disabled' do
|
||||
Factories::GradingPeriodHelper.new.create_for_group(@gp_group, {
|
||||
start_date: 20.days.ago(@now),
|
||||
end_date: 15.days.ago(@now),
|
||||
close_date: 10.days.ago(@now)
|
||||
})
|
||||
@assignment2.due_at = 17.days.ago(@now)
|
||||
@assignment2.only_visible_to_overrides = false
|
||||
@assignment2.save!
|
||||
|
||||
@test_course.disable_feature! :multiple_grading_periods
|
||||
edd = EffectiveDueDates.for_course(@test_course)
|
||||
edd.expects(:to_hash).never
|
||||
expect(edd.in_closed_grading_period?(@assignment2)).to eq(false)
|
||||
end
|
||||
|
||||
it 'returns false if assignment id is nil' do
|
||||
edd = EffectiveDueDates.for_course(@test_course, @assignment1)
|
||||
edd.expects(:to_hash).never
|
||||
expect(edd.in_closed_grading_period?(nil)).to eq(false)
|
||||
end
|
||||
|
||||
context 'with multiple grading periods' do
|
||||
before(:once) do
|
||||
@test_course.enable_feature! :multiple_grading_periods
|
||||
describe '#grading_period_id_for' do
|
||||
it 'returns the grading_period_id for the given student and assignment' do
|
||||
@assignment1.update!(due_at: 2.days.from_now(@grading_period.start_date))
|
||||
effective_due_dates = EffectiveDueDates.new(@test_course, @assignment1.id)
|
||||
grading_period_id = effective_due_dates.grading_period_id_for(
|
||||
student_id: @student1.id,
|
||||
assignment_id: @assignment1.id
|
||||
)
|
||||
expect(grading_period_id).to eq(@grading_period.id)
|
||||
end
|
||||
|
||||
before(:each) do
|
||||
Factories::GradingPeriodHelper.new.create_for_group(@gp_group, {
|
||||
start_date: 20.days.ago(@now),
|
||||
end_date: 15.days.ago(@now),
|
||||
close_date: 10.days.ago(@now)
|
||||
})
|
||||
@assignment2.due_at = 1.day.ago(@now)
|
||||
it 'returns nil if there if the given student and assignment do not fall in a grading period' do
|
||||
effective_due_dates = EffectiveDueDates.new(@test_course, @assignment1.id)
|
||||
grading_period_id = effective_due_dates.grading_period_id_for(
|
||||
student_id: @student1.id,
|
||||
assignment_id: @assignment1.id
|
||||
)
|
||||
expect(grading_period_id).to be_nil
|
||||
end
|
||||
|
||||
it 'returns nil if the assignment is not assigned to the student' do
|
||||
@assignment1.update!(
|
||||
due_at: 2.days.from_now(@grading_period.start_date),
|
||||
only_visible_to_overrides: true
|
||||
)
|
||||
effective_due_dates = EffectiveDueDates.new(@test_course, @assignment1.id)
|
||||
grading_period_id = effective_due_dates.grading_period_id_for(
|
||||
student_id: @student1.id,
|
||||
assignment_id: @assignment1.id
|
||||
)
|
||||
expect(grading_period_id).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
describe '#in_closed_grading_period?' do
|
||||
it 'returns false if multiple grading periods is disabled' do
|
||||
@assignment2.due_at = 17.days.ago(@now)
|
||||
@assignment2.only_visible_to_overrides = false
|
||||
@assignment2.save!
|
||||
override = @assignment2.assignment_overrides.create!(due_at: 19.days.ago(@now), due_at_overridden: true)
|
||||
override.assignment_override_students.create!(user: @student2)
|
||||
|
||||
@edd = EffectiveDueDates.for_course(@test_course)
|
||||
@test_course.disable_feature! :multiple_grading_periods
|
||||
edd = EffectiveDueDates.for_course(@test_course)
|
||||
edd.expects(:to_hash).never
|
||||
expect(edd.in_closed_grading_period?(@assignment2)).to eq(false)
|
||||
end
|
||||
|
||||
it 'returns true if any students in the given assignment have a due date in a closed grading period' do
|
||||
expect(@edd.in_closed_grading_period?(@assignment2)).to eq(true)
|
||||
it 'returns false if assignment id is nil' do
|
||||
edd = EffectiveDueDates.for_course(@test_course, @assignment1)
|
||||
edd.expects(:to_hash).never
|
||||
expect(edd.in_closed_grading_period?(nil)).to eq(false)
|
||||
end
|
||||
|
||||
it 'accepts assignment id as the argument' do
|
||||
expect(@edd.in_closed_grading_period?(@assignment2.id)).to eq(true)
|
||||
end
|
||||
context 'with multiple grading periods' do
|
||||
before(:once) do
|
||||
@test_course.enable_feature! :multiple_grading_periods
|
||||
end
|
||||
|
||||
it 'returns false if no student in the given assignment has a due date in a closed grading period' do
|
||||
expect(@edd.in_closed_grading_period?(@assignment1)).to eq(false)
|
||||
end
|
||||
before(:each) do
|
||||
@assignment2.due_at = 1.day.ago(@now)
|
||||
@assignment2.only_visible_to_overrides = false
|
||||
@assignment2.save!
|
||||
override = @assignment2.assignment_overrides.create!(due_at: 19.days.ago(@now), due_at_overridden: true)
|
||||
override.assignment_override_students.create!(user: @student2)
|
||||
|
||||
it 'returns false if the specified student has a due date in an open grading period' do
|
||||
override = @assignment2.assignment_overrides.create!(due_at: 1.day.from_now(@now), due_at_overridden: true)
|
||||
override.assignment_override_students.create!(user: @student1)
|
||||
@edd = EffectiveDueDates.for_course(@test_course)
|
||||
end
|
||||
|
||||
expect(@edd.in_closed_grading_period?(@assignment2, @student1)).to be_falsey
|
||||
expect(@edd.in_closed_grading_period?(@assignment2, @student1.id)).to be_falsey
|
||||
it 'returns true if any students in the given assignment have a due date in a closed grading period' do
|
||||
expect(@edd.in_closed_grading_period?(@assignment2)).to eq(true)
|
||||
end
|
||||
|
||||
it 'accepts assignment id as the argument' do
|
||||
expect(@edd.in_closed_grading_period?(@assignment2.id)).to eq(true)
|
||||
end
|
||||
|
||||
it 'returns false if no student in the given assignment has a due date in a closed grading period' do
|
||||
expect(@edd.in_closed_grading_period?(@assignment1)).to eq(false)
|
||||
end
|
||||
|
||||
it 'returns false if the specified student has a due date in an open grading period' do
|
||||
override = @assignment2.assignment_overrides.create!(due_at: 1.day.from_now(@now), due_at_overridden: true)
|
||||
override.assignment_override_students.create!(user: @student1)
|
||||
|
||||
expect(@edd.in_closed_grading_period?(@assignment2, @student1)).to be_falsey
|
||||
expect(@edd.in_closed_grading_period?(@assignment2, @student1.id)).to be_falsey
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -456,6 +456,71 @@ describe GradeCalculator do
|
|||
end
|
||||
end
|
||||
|
||||
describe '#compute_and_save_scores' do
|
||||
before(:once) do
|
||||
@first_period, @second_period = grading_periods(count: 2)
|
||||
first_assignment = @course.assignments.create!(
|
||||
due_at: 1.day.from_now(@first_period.start_date),
|
||||
points_possible: 100
|
||||
)
|
||||
second_assignment = @course.assignments.create!(
|
||||
due_at: 1.day.from_now(@second_period.start_date),
|
||||
points_possible: 100
|
||||
)
|
||||
first_assignment.grade_student(@student, grade: 25, grader: @teacher)
|
||||
second_assignment.grade_student(@student, grade: 75, grader: @teacher)
|
||||
# update_column to avoid callbacks on submission that would trigger score updates
|
||||
Submission.where(user: @student, assignment: first_assignment).first.update_column(:score, 100.0)
|
||||
Submission.where(user: @student, assignment: second_assignment).first.update_column(:score, 95.0)
|
||||
end
|
||||
|
||||
let(:scores) { @student.enrollments.first.scores }
|
||||
let(:overall_course_score) { scores.where(grading_period_id: nil).first }
|
||||
|
||||
it 'updates the overall course score' do
|
||||
GradeCalculator.new(@student.id, @course).compute_and_save_scores
|
||||
expect(overall_course_score.current_score).to eq(97.5)
|
||||
end
|
||||
|
||||
it 'updates all grading period scores' do
|
||||
GradeCalculator.new(@student.id, @course).compute_and_save_scores
|
||||
grading_period_scores = scores.where.not(grading_period_id: nil).order(:current_score).pluck(:current_score)
|
||||
expect(grading_period_scores).to match_array([100.0, 95.0])
|
||||
end
|
||||
|
||||
it 'does not update grading period scores if update_all_grading_period_scores is false' do
|
||||
GradeCalculator.new(@student.id, @course, update_all_grading_period_scores: false).compute_and_save_scores
|
||||
grading_period_scores = scores.where.not(grading_period_id: nil).order(:current_score).pluck(:current_score)
|
||||
expect(grading_period_scores).to match_array([25.0, 75.0])
|
||||
end
|
||||
|
||||
context 'grading period is provided' do
|
||||
it 'updates the grading period score' do
|
||||
GradeCalculator.new(@student.id, @course, grading_period: @first_period).compute_and_save_scores
|
||||
score = scores.where(grading_period_id: @first_period).first
|
||||
expect(score.current_score).to eq(100.0)
|
||||
end
|
||||
|
||||
it 'updates the overall course score' do
|
||||
GradeCalculator.new(@student.id, @course, grading_period: @first_period).compute_and_save_scores
|
||||
expect(overall_course_score.current_score).to eq(97.5)
|
||||
end
|
||||
|
||||
it 'does not update scores for other grading periods' do
|
||||
GradeCalculator.new(@student.id, @course, grading_period: @first_period).compute_and_save_scores
|
||||
score = scores.where(grading_period_id: @second_period).first
|
||||
expect(score.current_score).to eq(75.0)
|
||||
end
|
||||
|
||||
it 'does not update the overall course score if update_course_score is false' do
|
||||
GradeCalculator.new(
|
||||
@student.id, @course, grading_period: @first_period, update_course_score: false
|
||||
).compute_and_save_scores
|
||||
expect(overall_course_score.current_score).to eq(50.0)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
it "should return grades in the order they are requested" do
|
||||
@student1 = @student
|
||||
student_in_course
|
||||
|
|
|
|||
|
|
@ -134,7 +134,7 @@ describe Enrollment do
|
|||
expect(@enrollment.computed_current_score).to eq 95.5
|
||||
end
|
||||
|
||||
it 'ignores grading period scores' do
|
||||
it 'ignores grading period scores when passed no arguments' do
|
||||
@enrollment.scores.create!(current_score: 80.3, grading_period: period)
|
||||
expect(@enrollment.computed_current_score).to eq 95.5
|
||||
end
|
||||
|
|
@ -144,11 +144,23 @@ describe Enrollment do
|
|||
score.destroy
|
||||
expect(@enrollment.computed_current_score).to eq 95.5
|
||||
end
|
||||
|
||||
it 'computes current score for a given grading period id' do
|
||||
@enrollment.scores.create!(current_score: 80.3)
|
||||
@enrollment.scores.create!(current_score: 70.6, grading_period: period)
|
||||
current_score = @enrollment.computed_current_score(grading_period_id: period.id)
|
||||
expect(current_score).to eq 70.6
|
||||
end
|
||||
|
||||
it 'returns nil if a grading period score is requested and does not exist' do
|
||||
current_score = @enrollment.computed_current_score(grading_period_id: period.id)
|
||||
expect(current_score).to eq nil
|
||||
end
|
||||
end
|
||||
|
||||
describe '#computed_current_grade' do
|
||||
before(:each) do
|
||||
Course.any_instance.expects(:grading_standard_enabled?).at_least_once.returns(true)
|
||||
Course.any_instance.stubs(:grading_standard_enabled?).returns(true)
|
||||
end
|
||||
|
||||
it 'uses the value from the associated score object, if one exists' do
|
||||
|
|
@ -160,7 +172,7 @@ describe Enrollment do
|
|||
expect(@enrollment.computed_current_grade).to eq 'A'
|
||||
end
|
||||
|
||||
it 'ignores grading period grades' do
|
||||
it 'ignores grading period grades when passed no arguments' do
|
||||
@enrollment.scores.create!(current_score: 80.3, grading_period: period)
|
||||
expect(@enrollment.computed_current_grade).to eq 'A'
|
||||
end
|
||||
|
|
@ -170,6 +182,18 @@ describe Enrollment do
|
|||
score.destroy
|
||||
expect(@enrollment.computed_current_grade).to eq 'A'
|
||||
end
|
||||
|
||||
it 'computes current grade for a given grading period id' do
|
||||
@enrollment.scores.create!(current_score: 80.3)
|
||||
@enrollment.scores.create!(current_score: 70.6, grading_period: period)
|
||||
current_grade = @enrollment.computed_current_grade(grading_period_id: period.id)
|
||||
expect(current_grade).to eq 'C-'
|
||||
end
|
||||
|
||||
it 'returns nil if a grading period grade is requested and does not exist' do
|
||||
current_grade = @enrollment.computed_current_grade(grading_period_id: period.id)
|
||||
expect(current_grade).to eq nil
|
||||
end
|
||||
end
|
||||
|
||||
describe '#graded_at' do
|
||||
|
|
@ -226,7 +250,7 @@ describe Enrollment do
|
|||
expect(@enrollment.computed_final_score).to eq 95.5
|
||||
end
|
||||
|
||||
it 'ignores grading period scores' do
|
||||
it 'ignores grading period scores when passed no arguments' do
|
||||
@enrollment.scores.create!(final_score: 80.3, grading_period: period)
|
||||
expect(@enrollment.computed_final_score).to eq 95.5
|
||||
end
|
||||
|
|
@ -236,11 +260,23 @@ describe Enrollment do
|
|||
score.destroy
|
||||
expect(@enrollment.computed_final_score).to eq 95.5
|
||||
end
|
||||
|
||||
it 'computes final score for a given grading period id' do
|
||||
@enrollment.scores.create!(final_score: 80.3)
|
||||
@enrollment.scores.create!(final_score: 70.6, grading_period: period)
|
||||
final_score = @enrollment.computed_final_score(grading_period_id: period.id)
|
||||
expect(final_score).to eq 70.6
|
||||
end
|
||||
|
||||
it 'returns nil if a grading period score is requested and does not exist' do
|
||||
final_score = @enrollment.computed_final_score(grading_period_id: period.id)
|
||||
expect(final_score).to eq nil
|
||||
end
|
||||
end
|
||||
|
||||
describe '#computed_final_grade' do
|
||||
before(:each) do
|
||||
Course.any_instance.expects(:grading_standard_enabled?).at_least_once.returns(true)
|
||||
Course.any_instance.stubs(:grading_standard_enabled?).returns(true)
|
||||
end
|
||||
|
||||
it 'uses the value from the associated score object, if one exists' do
|
||||
|
|
@ -252,7 +288,7 @@ describe Enrollment do
|
|||
expect(@enrollment.computed_final_grade).to eq 'A'
|
||||
end
|
||||
|
||||
it 'ignores grading period grades' do
|
||||
it 'ignores grading period grades when passed no arguments' do
|
||||
@enrollment.scores.create!(final_score: 80.3, grading_period: period)
|
||||
expect(@enrollment.computed_final_grade).to eq 'A'
|
||||
end
|
||||
|
|
@ -262,6 +298,18 @@ describe Enrollment do
|
|||
score.destroy
|
||||
expect(@enrollment.computed_final_grade).to eq 'A'
|
||||
end
|
||||
|
||||
it 'computes final grade for a given grading period id' do
|
||||
@enrollment.scores.create!(final_score: 80.3)
|
||||
@enrollment.scores.create!(final_score: 70.6, grading_period: period)
|
||||
final_grade = @enrollment.computed_final_grade(grading_period_id: period.id)
|
||||
expect(final_grade).to eq 'C-'
|
||||
end
|
||||
|
||||
it 'returns nil if a grading period grade is requested and does not exist' do
|
||||
final_grade = @enrollment.computed_final_grade(grading_period_id: period.id)
|
||||
expect(final_grade).to eq nil
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -23,14 +23,14 @@ describe GradingPeriod do
|
|||
|
||||
let(:group_helper) { Factories::GradingPeriodGroupHelper.new }
|
||||
let(:account) { Account.create! }
|
||||
let(:course) { Course.create! }
|
||||
let(:course) { account.courses.create! }
|
||||
let(:grading_period_group) do
|
||||
group = account.grading_period_groups.create!(title: "A Group")
|
||||
group.enrollment_terms << course.enrollment_term
|
||||
term = course.enrollment_term
|
||||
group.enrollment_terms << term
|
||||
group
|
||||
end
|
||||
let(:now) { Time.zone.now.change(usec: 0) }
|
||||
|
||||
let(:params) do
|
||||
{
|
||||
title: 'A Grading Period',
|
||||
|
|
@ -194,7 +194,6 @@ describe GradingPeriod do
|
|||
end
|
||||
end
|
||||
|
||||
|
||||
describe "#as_json_with_user_permissions" do
|
||||
it "includes the close_date in the returned object" do
|
||||
json = grading_period.as_json_with_user_permissions(User.new)
|
||||
|
|
@ -691,4 +690,39 @@ describe GradingPeriod do
|
|||
expect(subject.reload.weight).to eql 1.5
|
||||
end
|
||||
end
|
||||
|
||||
describe 'grading period scores' do
|
||||
before do
|
||||
student_in_course(course: course, active_all: true)
|
||||
teacher_in_course(course: course, active_all: true)
|
||||
@assignment = course.assignments.create!(due_at: 10.days.from_now(now), points_possible: 10)
|
||||
@assignment.grade_student(@student, grade: 8, grader: @teacher)
|
||||
end
|
||||
|
||||
it 'creates scores for the grading period upon its creation' do
|
||||
expect{ grading_period.save! }.to change{ Score.count }.from(1).to(2)
|
||||
end
|
||||
|
||||
it 'updates grading period scores when the grading period end date is changed' do
|
||||
grading_period.save!
|
||||
expect do
|
||||
day_after_assignment_is_due = 1.day.from_now(@assignment.due_at)
|
||||
grading_period.update!(
|
||||
end_date: day_after_assignment_is_due,
|
||||
close_date: day_after_assignment_is_due
|
||||
)
|
||||
end.to change{
|
||||
Score.where(grading_period_id: grading_period).first.current_score
|
||||
}.from(nil).to(80.0)
|
||||
end
|
||||
|
||||
it 'updates grading period scores when the grading period start date is changed' do
|
||||
day_before_grading_period_starts = 1.day.ago(grading_period.start_date)
|
||||
@assignment.update!(due_at: day_before_grading_period_starts)
|
||||
grading_period.save!
|
||||
expect{ grading_period.update!(start_date: 1.day.ago(@assignment.due_at)) }.to change{
|
||||
Score.where(grading_period_id: grading_period).first.current_score
|
||||
}.from(nil).to(80.0)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -619,6 +619,74 @@ describe Submission do
|
|||
end
|
||||
end
|
||||
|
||||
describe 'computation of scores' do
|
||||
before(:once) do
|
||||
@assignment.update!(points_possible: 10)
|
||||
@submission = Submission.create!(@valid_attributes)
|
||||
end
|
||||
|
||||
let(:scores) do
|
||||
enrollment = Enrollment.where(user_id: @submission.user_id, course_id: @submission.context).first
|
||||
enrollment.scores.order(:grading_period_id)
|
||||
end
|
||||
|
||||
let(:grading_period_scores) do
|
||||
scores.where.not(grading_period_id: nil)
|
||||
end
|
||||
|
||||
it 'recomputes course scores when the submission score changes' do
|
||||
expect { @submission.update!(score: 5) }.to change {
|
||||
scores.pluck(:current_score)
|
||||
}.from([nil]).to([50.0])
|
||||
end
|
||||
|
||||
context 'Multiple Grading Periods' do
|
||||
before(:once) do
|
||||
@now = Time.zone.now
|
||||
course = @submission.context
|
||||
assignment_outside_of_period = course.assignments.create!(
|
||||
due_at: 10.days.from_now(@now),
|
||||
points_possible: 10
|
||||
)
|
||||
assignment_outside_of_period.grade_student(@user, grade: 8, grader: @teacher)
|
||||
@assignment.update!(due_at: @now)
|
||||
@root_account = course.root_account
|
||||
@root_account.enable_feature!(:multiple_grading_periods)
|
||||
group = @root_account.grading_period_groups.create!
|
||||
group.enrollment_terms << course.enrollment_term
|
||||
@grading_period = group.grading_periods.create!(
|
||||
title: 'Current Grading Period',
|
||||
start_date: 5.days.ago(@now),
|
||||
end_date: 5.days.from_now(@now)
|
||||
)
|
||||
end
|
||||
|
||||
it 'updates the course score and grading period score if a submission ' \
|
||||
'in a grading period is graded' do
|
||||
expect { @submission.update!(score: 5) }.to change {
|
||||
scores.pluck(:current_score)
|
||||
}.from([nil, 80.0]).to([50.0, 65.0])
|
||||
end
|
||||
|
||||
it 'keeps grading period scores updated even if the feature flag is disabled' do
|
||||
@submission.update!(score: 10)
|
||||
@root_account.disable_feature!(:multiple_grading_periods)
|
||||
expect { @submission.update!(score: 5) }.to change {
|
||||
grading_period_scores.pluck(:current_score)
|
||||
}.from([100.0]).to([50.0])
|
||||
end
|
||||
|
||||
it 'only updates the course score (not the grading period score) if a submission ' \
|
||||
'not in a grading period is graded' do
|
||||
day_after_grading_period_ends = 1.day.from_now(@grading_period.end_date)
|
||||
@assignment.update!(due_at: day_after_grading_period_ends)
|
||||
expect { @submission.update!(score: 5) }.to change {
|
||||
scores.pluck(:current_score)
|
||||
}.from([nil, 80.0]).to([nil, 65.0])
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#can_grade?' do
|
||||
before(:each) do
|
||||
@account = Account.new
|
||||
|
|
|
|||
Loading…
Reference in New Issue