ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

apply deductions for assignments missing submissions 

closes CNVS-36867

Test Plan
  1. Create a course with "New Gradebook" enabled
  2. Create three students in the course
     a. Alice always submits assignments early
     b. Bob always submits assignments late
     c. Chad never submits anything
  3. Make sure all users and courses are set to the same time
     zone as your Mac.  If your Mac is on Mountain Time this
     will probably happen by default anyway)
  4. Create a late policy for the course
     a. deducts 10% every hour late, down to 50% score
     b. grants 30% score for missing submissions
  5. Create assignments in the course, due in a few minutes
     A. 10 points, grade as complete/incomplete
     B. 10 points, grade as points, submit on paper
     C. 10 points, grade as points, online text
     D. 10 points, online quiz
  7. Immediately enter Alice's submissions
     a. Masquerade as Alice, take quiz D (100%), submit C
     b. Mark Alice complete on A, 10 points on B and C
  8. Wait until 6 minutes after the assignments' due time
     a. Masquerade as Bob, take quiz D (100%), submit C
     b. Mark Bob complete on A, 10 points on B and C
  9. Verify grades and status indicators on the assignments
     Alice: A(OK) B(10) C(10) D(10)
     Bob:   B(OK) B(9)  C(9)  D(9) [late]
     Chad:  C(X)  B(3)  C(3)  D(3)  [missing]
 10. Change the due time on all assignments to 2 hours earlier
     than it was. Refresh the gradebook and verify
     Alice: A(OK) B(8)  C(8)  D(8) [late]
     Bob:   B(OK) B(7)  C(7)  D(7) [late]
     Chad:  C(X)  B(3)  C(3)  D(3)  [missing]

Change-Id: I7b81c907b678997ebecab7342012ba4678f7c94f
Reviewed-on: https://gerrit.instructure.com/113912
Reviewed-by: Shahbaz Javeed <sjaveed@instructure.com>
Reviewed-by: Derek Bender <djbender@instructure.com>
QA-Review: Anju Reddy <areddy@instructure.com>
Tested-by: Jenkins
Product-Review: Christi Wruck
This commit is contained in:
Matt Taylor 2017-06-01 13:59:39 -05:00
parent aedb44a59a
commit 2ce599d587
9 changed files with 99 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
app/models/late_policy.rb
View File

@ -42,14 +42,14 @@ class LatePolicy < ActiveRecord::Base
possible * [late_percent_deduct, maximum_deduct].min / 100
end
def missing_points_deducted(assignment)
return assignment.points_possible.to_f if assignment.grading_type == 'pass_fail'
assignment.points_possible.to_f * missing_submission_deduction.to_f / 100
def missing_points_deducted(points_possible, grading_type)
return points_possible.to_f if grading_type == 'pass_fail'
points_possible.to_f * missing_submission_deduction.to_f / 100
end
def points_for_missing(assignment)
return 0 if assignment.grading_type == 'pass_fail'
assignment.points_possible.to_f * (100 - missing_submission_deduction.to_f) / 100
def points_for_missing(points_possible, grading_type)
return 0 if grading_type == 'pass_fail'
points_possible.to_f * (100 - missing_submission_deduction.to_f) / 100
end
private

21
app/models/submission.rb
View File

@ -1325,15 +1325,30 @@ class Submission < ActiveRecord::Base
end
# apply_late_policy is called directly by bulk update processes, or indirectly by before_save on a Submission
def apply_late_policy(late_policy=nil, points_possible=nil)
return if score.nil? || points_deducted_changed?
def apply_late_policy(late_policy=nil, points_possible=nil, grading_type=nil)
return if points_deducted_changed?
late_policy ||= assignment.course.late_policy
points_possible ||= assignment.points_possible
grading_type ||= assignment.grading_type
return score_missing(late_policy, points_possible, grading_type) if missing?
score_late_or_none(late_policy, points_possible) if score
end
def score_missing(late_policy, points_possible, grading_type)
return unless late_policy&.missing_submission_deduction_enabled
self.points_deducted = nil
self.score = late_policy.points_for_missing(points_possible, grading_type)
end
private :score_missing
def score_late_or_none(late_policy, points_possible)
raw_score = score_changed? ? score : originally_entered_score
deducted = late_points_deducted(raw_score, late_policy, points_possible)
new_score = raw_score - deducted
self.points_deducted = deducted
self.score = raw_score - deducted
self.score = new_score
end
private :score_late_or_none
def originally_entered_score
score + (points_deducted || 0)

6
lib/late_policy_applicator.rb
View File

@ -66,7 +66,7 @@ class LatePolicyApplicator
private
def process_submission(late_policy, assignment, submission)
submission.apply_late_policy(late_policy, assignment.points_possible)
submission.apply_late_policy(late_policy, assignment.points_possible, assignment.grading_type)
if submission.changed?
submission.skip_grade_calc = true
return submission.save!
@ -76,7 +76,9 @@ class LatePolicyApplicator
end
def relevant_submissions(assignment)
@relevant_submissions[assignment.id] ||= assignment.submissions.late.where.not(score: nil).
@relevant_submissions[assignment.id] ||= assignment.submissions.late.
where.not(score: nil).
union(assignment.submissions.missing).
where(user_id: relevant_student_ids(assignment))
end

2
lib/missing_policy_applicator.rb
View File

@ -41,7 +41,7 @@ class MissingPolicyApplicator
# Given submissions must all be for the same assignment
def apply_missing_deduction(assignment, submissions)
score = assignment.course.late_policy.points_for_missing(assignment)
score = assignment.course.late_policy.points_for_missing(assignment.points_possible, assignment.grading_type)
grade = assignment.score_to_grade(score)
# We are using update_all here for performance reasons

5
spec/factories/late_policy_factory.rb
View File

@ -29,7 +29,7 @@ module Factories
private
def late_policy_params(course: nil, deduct: 0, every: :hour, down_to: 0, missing_submission_deduction: 100)
def late_policy_params(course: nil, deduct: 0, every: :hour, down_to: 0, missing: 100)
{
course_id: course && course[:id],
late_submission_deduction_enabled: deduct.positive?,
@ -37,7 +37,8 @@ module Factories
late_submission_interval: every,
late_submission_minimum_percent_enabled: down_to.positive?,
late_submission_minimum_percent: down_to,
missing_submission_deduction: missing_submission_deduction
missing_submission_deduction_enabled: !!missing,
missing_submission_deduction: missing
}
end
end

52
spec/lib/late_policy_applicator_spec.rb
View File

@ -22,7 +22,7 @@ describe LatePolicyApplicator do
before :once do
course_factory(active_all: true)
@published_assignment = @course.assignments.create(workflow_state: 'published')
@published_assignment = @course.assignments.create!(workflow_state: 'published')
end
it 'instantiates an applicator for the course' do
@ -65,7 +65,7 @@ describe LatePolicyApplicator do
before :once do
course_factory(active_all: true)
@published_assignment = @course.assignments.create(workflow_state: 'published', points_possible: 20)
@published_assignment = @course.assignments.create!(workflow_state: 'published', points_possible: 20)
end
it 'instantiates an applicator for the assignment' do
@ -130,21 +130,18 @@ describe LatePolicyApplicator do
@now = Time.zone.now
course_factory(active_all: true, grading_periods: [:old, :current])
@late_policy = LatePolicy.new(
course: @course,
late_submission_deduction_enabled: true,
late_submission_deduction: 50.0
)
@late_policy = late_policy_factory(course: @course, deduct: 50.0, every: :day, missing: 95.0)
@course.late_policy = @late_policy
@course.save!
@students = Array.new(2) do
user = User.create
@students = Array.new(3) do
user = User.create!
@course.enroll_student(user, enrollment_state: 'active')
user
end
@assignment_in_closed_gp = @course.assignments.create(
@assignment_in_closed_gp = @course.assignments.create!(
points_possible: 20, due_at: @now - 3.months, submission_types: 'online_text_entry'
)
@ -168,7 +165,15 @@ describe LatePolicyApplicator do
submission_type: 'online_text_entry'
)
@assignment_in_open_gp = @course.assignments.create(
@missing_submission1 = @assignment_in_closed_gp.submissions.find_by(user: @students[2])
Submission.where(id: @missing_submission1).
update_all(
submitted_at: nil,
cached_due_date: 1.month.ago(@now),
score: nil
)
@assignment_in_open_gp = @course.assignments.create!(
points_possible: 20, due_at: @now - 1.month, submission_types: 'online_text_entry'
)
@ -189,6 +194,14 @@ describe LatePolicyApplicator do
score: 20,
submission_type: 'online_text_entry'
)
@missing_submission2 = @assignment_in_open_gp.submissions.find_by(user: @students[2])
Submission.where(id: @missing_submission2).
update_all(
submitted_at: nil,
cached_due_date: @now - 1.month,
score: nil
)
# rubocop:enable Rails/SkipsModelValidations
end
@ -211,6 +224,12 @@ describe LatePolicyApplicator do
expect { @late_policy_applicator.process }.to change { @late_submission2.reload.score }.by(-10)
end
it 'applies the missing policy to missing submissions in the open grading period' do
@late_policy_applicator = LatePolicyApplicator.new(@course)
expect { @late_policy_applicator.process }.to change { @missing_submission2.reload.score }.to(1)
end
it 'does not apply the late policy to timely submissions in the open grading period' do
@late_policy_applicator = LatePolicyApplicator.new(@course)
@ -223,6 +242,12 @@ describe LatePolicyApplicator do
expect { @late_policy_applicator.process }.not_to change { @late_submission1.reload.score }
end
it 'does not apply the late policy to missing submissions in the closed grading period' do
@late_policy_applicator = LatePolicyApplicator.new(@course)
expect { @late_policy_applicator.process }.not_to change { @missing_submission1.reload.score }
end
it 'does not apply the late policy to timely submissions in the closed grading period' do
@late_policy_applicator = LatePolicyApplicator.new(@course)
@ -231,8 +256,11 @@ describe LatePolicyApplicator do
it 'calls re-calculates grades in bulk after processing all submissions' do
@late_policy_applicator = LatePolicyApplicator.new(@course)
student_ids = [0,2].map { |i| @students[i].id }
expect(@course).to receive(:recompute_student_scores).with([@students[0].id])
expect(@course).to receive(:recompute_student_scores).
with(array_including(student_ids)).
with(Array.new(2, kind_of(Integer)))
@late_policy_applicator.process
end

4
spec/lib/missing_policy_applicator_spec.rb
View File

@ -104,8 +104,8 @@ describe MissingPolicyApplicator do
end
it 'does not apply deductions to assignments that went missing over 24 hours ago' do
late_policy_missing_enabled
assignment_old
late_policy_missing_enabled
applicator.apply_missing_deductions
submission = @course.submissions.first
@ -148,8 +148,8 @@ describe MissingPolicyApplicator do
end
it 'recomputes student scores for affected students' do
late_policy_missing_enabled
create_recent_assignment
late_policy_missing_enabled
enrollment = student.enrollments.find_by(course_id: @course.id)
enrollment.scores.first_or_create.update(grading_period_id: nil, final_score: 100, current_score: 100)

27
spec/models/late_policy_spec.rb
View File

@ -147,39 +147,24 @@ describe LatePolicy do
describe '#points_for_missing' do
it 'returns 0 when assignment grading_type is pass_fail' do
policy = late_policy_model
assignment = instance_double('Assignment')
allow(assignment).to receive(:grading_type).and_return('pass_fail')
expect(policy.points_for_missing(assignment)).to eq(0)
expect(policy.points_for_missing(100, 'pass_fail')).to eq(0)
end
it 'computes expected value' do
policy = late_policy_model(missing_submission_deduction: 60)
assignment = instance_double('Assignment')
allow(assignment).to receive(:grading_type).and_return('foo')
allow(assignment).to receive(:points_possible).and_return(100)
expect(policy.points_for_missing(assignment)).to eq(40)
policy = late_policy_model(missing: 60)
expect(policy.points_for_missing(100, 'foo')).to eq(40)
end
end
describe '#missing_points_deducted' do
it 'returns points_possible when assignment grading_type is pass_fail' do
policy = late_policy_model
assignment = instance_double('Assignment')
allow(assignment).to receive(:grading_type).and_return('pass_fail')
allow(assignment).to receive(:points_possible).and_return(100)
expect(policy.missing_points_deducted(assignment)).to eq(100)
expect(policy.missing_points_deducted(100, 'pass_fail')).to eq(100)
end
it 'computes expected value' do
policy = late_policy_model(missing_submission_deduction: 60)
assignment = instance_double('Assignment')
allow(assignment).to receive(:grading_type).and_return('foo')
allow(assignment).to receive(:points_possible).and_return(100)
expect(policy.missing_points_deducted(assignment)).to eq(60)
policy = late_policy_model(missing: 60)
expect(policy.missing_points_deducted(100, 'foo')).to eq(60)
end
end

20
spec/models/submission_spec.rb
View File

@ -529,7 +529,7 @@ describe Submission do
before(:once) do
@date = Time.zone.local(2017, 1, 15, 12)
@assignment.update!(due_at: 3.hours.ago(@date), points_possible: 1000, submission_types: "online_text_entry")
@late_policy = late_policy_model(deduct: 10.0, every: :hour)
@late_policy = late_policy_model(deduct: 10.0, every: :hour, missing: 80.0)
end
let(:submission) { @assignment.submissions.find_by(user_id: @student) }
@ -554,6 +554,16 @@ describe Submission do
end
end
it "does not round decimal places in the score" do
Timecop.freeze(2.days.ago(@date)) do
@assignment.submit_homework(@student, body: "a body")
original_score = 1.3800000000000001
submission.score = original_score
submission.apply_late_policy(@late_policy, 1000)
expect(submission.score).to eq original_score
end
end
it "deducts only once even if called twice" do
Timecop.freeze(@date) do
@assignment.submit_homework(@student, body: "a body")
@ -566,6 +576,14 @@ describe Submission do
expect(submission.points_deducted).to eq 300
end
end
it "applies missing policy if submission is missing" do
Timecop.freeze(1.day.from_now(@date)) do
submission.score = nil
submission.apply_late_policy(@late_policy, 1000)
expect(submission.score).to eq 200
end
end
end
describe "#apply_late_policy_before_save" do