diff --git a/app/models/late_policy.rb b/app/models/late_policy.rb index 5c88317a815..6f4c7f9b2fa 100644 --- a/app/models/late_policy.rb +++ b/app/models/late_policy.rb @@ -42,6 +42,16 @@ 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 + 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 + end + private def interval_seconds diff --git a/config/initializers/periodic_jobs.rb b/config/initializers/periodic_jobs.rb index 3c232e66bf6..b9fda34823a 100644 --- a/config/initializers/periodic_jobs.rb +++ b/config/initializers/periodic_jobs.rb @@ -123,11 +123,11 @@ Rails.configuration.after_initialize do end end - Delayed::Periodic.cron 'Alerts::DelayedAlertSender.process', '30 11 * * *', :priority => Delayed::LOW_PRIORITY do + Delayed::Periodic.cron 'Alerts::DelayedAlertSender.process', '30 11 * * *', priority: Delayed::LOW_PRIORITY do with_each_shard_by_database(Alerts::DelayedAlertSender, :process) end - Delayed::Periodic.cron 'Attachment.do_notifications', '*/10 * * * *', :priority => Delayed::LOW_PRIORITY do + Delayed::Periodic.cron 'Attachment.do_notifications', '*/10 * * * *', priority: Delayed::LOW_PRIORITY do with_each_shard_by_database(Attachment, :do_notifications) end @@ -176,7 +176,11 @@ Rails.configuration.after_initialize do end end - Delayed::Periodic.cron 'EnrollmentState.recalculate_expired_states', '*/5 * * * *', :priority => Delayed::LOW_PRIORITY do + Delayed::Periodic.cron 'EnrollmentState.recalculate_expired_states', '*/5 * * * *', priority: Delayed::LOW_PRIORITY do with_each_shard_by_database(EnrollmentState, :recalculate_expired_states) end + + Delayed::Periodic.cron 'MissingPolicyApplicator.apply_missing_deductions', '*/5 * * * *', priority: Delayed::LOW_PRIORITY do + with_each_shard_by_database(MissingPolicyApplicator, :apply_missing_deductions) + end end diff --git a/lib/missing_policy_applicator.rb b/lib/missing_policy_applicator.rb new file mode 100644 index 00000000000..18575322dbb --- /dev/null +++ b/lib/missing_policy_applicator.rb @@ -0,0 +1,59 @@ +# +# Copyright (C) 2017 - present 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 . + +class MissingPolicyApplicator + def self.apply_missing_deductions + MissingPolicyApplicator.new.apply_missing_deductions + end + + def apply_missing_deductions + recently_missing_submissions.find_in_batches do |submissions| + filtered_submissions = submissions.reject { |s| s.grading_period&.closed? } + filtered_submissions.group_by(&:assignment).each(&method(:apply_missing_deduction)) + end + end + + private + + def recently_missing_submissions + now = Time.zone.now + Submission + .joins(assignment: {course: :late_policy}) + .eager_load(:grading_period, assignment: { course: :late_policy }) + .missing + .where(score: nil, grade: nil, cached_due_date: 1.day.ago(now)..now, + late_policies: { missing_submission_deduction_enabled: true }) + end + + # Given submissions must all be for the same assignment + def apply_missing_deduction(assignment, submissions) + score = assignment.course.late_policy.points_for_missing(assignment) + grade = assignment.score_to_grade(score) + + # We are using update_all here for performance reasons + # rubocop:disable SkipsModelValidations + Submission.where(id: submissions).update_all( + score: score, + grade: grade, + published_score: score, + published_grade: grade + ) + # rubocop:enable SkipsModelValidations + + assignment.course.recompute_student_scores(submissions.map(&:user_id).uniq) + end +end diff --git a/spec/factories/late_policy_factory.rb b/spec/factories/late_policy_factory.rb index 294815ed866..2eb661fc9b1 100644 --- a/spec/factories/late_policy_factory.rb +++ b/spec/factories/late_policy_factory.rb @@ -29,14 +29,15 @@ module Factories private - def late_policy_params(course: nil, deduct: 0, every: :hour, down_to: 0) + def late_policy_params(course: nil, deduct: 0, every: :hour, down_to: 0, missing_submission_deduction: 100) { course_id: course && course[:id], late_submission_deduction_enabled: deduct.positive?, late_submission_deduction: deduct, late_submission_interval: every, late_submission_minimum_percent_enabled: down_to.positive?, - late_submission_minimum_percent: down_to + late_submission_minimum_percent: down_to, + missing_submission_deduction: missing_submission_deduction } end end diff --git a/spec/lib/missing_policy_applicator_spec.rb b/spec/lib/missing_policy_applicator_spec.rb new file mode 100644 index 00000000000..6852d7099af --- /dev/null +++ b/spec/lib/missing_policy_applicator_spec.rb @@ -0,0 +1,160 @@ +# +# Copyright (C) 2017 - present 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 . + +require 'spec_helper' + +describe MissingPolicyApplicator do + describe '.apply_missing_deductions' do + it 'invokes #apply_missing_deductions' do + dbl = instance_double('MissingPolicyApplicator') + allow(described_class).to receive(:new).and_return(dbl) + expect(dbl).to receive(:apply_missing_deductions) + + described_class.apply_missing_deductions + end + end + + describe '#apply_missing_deductions' do + let(:now) { Time.zone.now } + let :late_policy_missing_enabled do + LatePolicy.create!( + course_id: @course.id, + missing_submission_deduction_enabled: true, + missing_submission_deduction: 75 + ) + end + let :late_policy_missing_disabled do + LatePolicy.create!( + course_id: @course.id, + missing_submission_deduction_enabled: false, + missing_submission_deduction: 75 + ) + end + let :create_recent_assignment do + @course.assignments.create!( + assignment_valid_attributes.merge(grading_type: 'letter_grade', due_at: 1.hour.ago(now)) + ) + end + let :assignment_old do + @course.assignments.create!( + assignment_valid_attributes.merge(grading_type: 'letter_grade', due_at: 25.hours.ago(now)) + ) + end + let :create_pass_fail_assignment do + @course.assignments.create!( + assignment_valid_attributes.merge(grading_type: 'pass_fail', due_at: 1.hour.ago(now)) + ) + end + let(:grading_period_group) do + group = @course.account.grading_period_groups.create!(title: "A Group") + term = @course.enrollment_term + group.enrollment_terms << term + group + end + let(:grading_period_closed) do + grading_period_group.grading_periods.create!( + title: 'A Grading Period', + start_date: 10.days.ago(now), + end_date: 30.minutes.ago(now), + close_date: 30.minutes.ago(now) + ) + end + + let(:applicator) { described_class.new } + let!(:student) { student_in_course(active_all: true, user_name: 'a student').user } + + before(:once) do + course_with_teacher(active_all: true) + end + + it 'applies deductions to assignments in a course with a LatePolicy with missing submission deductions enabled' do + late_policy_missing_enabled + create_recent_assignment + applicator.apply_missing_deductions + + submission = @course.submissions.first + + expect(submission.score).to be 0.375 + expect(submission.grade).to eql 'F' + end + + it 'does not apply deductions to assignments in a course with missing submission deductions disabled' do + late_policy_missing_disabled + create_recent_assignment + applicator.apply_missing_deductions + + submission = @course.submissions.first + + expect(submission.score).to be nil + expect(submission.grade).to be nil + end + + it 'does not apply deductions to assignments that went missing over 24 hours ago' do + late_policy_missing_enabled + assignment_old + applicator.apply_missing_deductions + + submission = @course.submissions.first + + expect(submission.score).to be nil + expect(submission.grade).to be nil + end + + it 'does not apply deductions to assignments in a course without a LatePolicy' do + create_recent_assignment + applicator.apply_missing_deductions + + submission = @course.submissions.first + + expect(submission.score).to be nil + expect(submission.grade).to be nil + end + + it 'assigns a score of zero to Complete / Incomplete assignments' do + late_policy_missing_enabled + create_pass_fail_assignment + applicator.apply_missing_deductions + + submission = @course.submissions.first + + expect(submission.score).to be 0.0 + expect(submission.grade).to eql 'incomplete' + end + + it 'does not apply deductions to submission in closed grading periods' do + grading_period_closed + late_policy_missing_enabled + create_recent_assignment + applicator.apply_missing_deductions + + submission = @course.submissions.first + + expect(submission.score).to be nil + expect(submission.grade).to be nil + end + + it 'recomputes student scores for affected students' do + late_policy_missing_enabled + create_recent_assignment + + 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) + + expect { applicator.apply_missing_deductions }.to change(enrollment, :computed_final_score) + end + end +end diff --git a/spec/models/late_policy_spec.rb b/spec/models/late_policy_spec.rb index f1e02293a15..1bd1ac5dd27 100644 --- a/spec/models/late_policy_spec.rb +++ b/spec/models/late_policy_spec.rb @@ -76,37 +76,37 @@ describe LatePolicy do describe 'rounding' do it 'only keeps 2 digits after the decimal for late_submission_minimum_percent' do - policy = LatePolicy.new(late_submission_minimum_percent: 100.223) - expect(policy.late_submission_minimum_percent).to eql BigDecimal.new('100.22') + policy = LatePolicy.new(late_submission_minimum_percent: 25.223) + expect(policy.late_submission_minimum_percent).to eql BigDecimal.new('25.22') end it 'rounds late_submission_minimum_percent' do - policy = LatePolicy.new(late_submission_minimum_percent: 100.225) - expect(policy.late_submission_minimum_percent).to eql BigDecimal.new('100.23') + policy = LatePolicy.new(late_submission_minimum_percent: 25.225) + expect(policy.late_submission_minimum_percent).to eql BigDecimal.new('25.23') end it 'only keeps 2 digits after the decimal for missing_submission_deduction' do - policy = LatePolicy.new(missing_submission_deduction: 100.223) - expect(policy.missing_submission_deduction).to eql BigDecimal.new('100.22') + policy = LatePolicy.new(missing_submission_deduction: 25.223) + expect(policy.missing_submission_deduction).to eql BigDecimal.new('25.22') end it 'rounds missing_submission_deduction' do - policy = LatePolicy.new(missing_submission_deduction: 100.225) - expect(policy.missing_submission_deduction).to eql BigDecimal.new('100.23') + policy = LatePolicy.new(missing_submission_deduction: 25.225) + expect(policy.missing_submission_deduction).to eql BigDecimal.new('25.23') end it 'only keeps 2 digits after the decimal for late_submission_deduction' do - policy = LatePolicy.new(late_submission_deduction: 100.223) - expect(policy.late_submission_deduction).to eql BigDecimal.new('100.22') + policy = LatePolicy.new(late_submission_deduction: 25.223) + expect(policy.late_submission_deduction).to eql BigDecimal.new('25.22') end it 'rounds late_submission_deduction' do - policy = LatePolicy.new(late_submission_deduction: 100.225) - expect(policy.late_submission_deduction).to eql BigDecimal.new('100.23') + policy = LatePolicy.new(late_submission_deduction: 25.225) + expect(policy.late_submission_deduction).to eql BigDecimal.new('25.23') end end - describe 'deduction' do + describe '#points_deducted' do it 'ignores disabled late submission deduction' do expect(late_policy_model.points_deducted(score: 500, possible: 1000, late_for: 1.day)).to eq 0 end @@ -144,6 +144,45 @@ describe LatePolicy do end end + 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) + 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) + 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) + 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) + end + end + describe '#update_late_submissions' do before :once do @course = Course.create(name: 'Late Policy Course')