From 6a680d64516abf557918666eb6f72a732b6918d9 Mon Sep 17 00:00:00 2001 From: Eduardo Escobar Date: Tue, 20 Apr 2021 14:44:35 -0500 Subject: [PATCH] improve the enrollments check in the missing policy applicator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The activerecord query in ‘recently_missing_submissions‘ now uses a join between courses and enrollments to only apply the condition to the right submissions. fixes EVAL-1547 flag=none test plan: - Create a course (A) with at least one student enrolled. - Create a course (B) with the same student enrolled. - Create an assignment for both courses (A) and (B) with a due date of one day before the current date. - Conclude the enrollment for the student to course (B). - Enable 'Automatically apply grade for missing submissions' late policy for both courses (A) and (B). - Wait 5 minutes to get the 'apply_missing_deductions' cronjob executed. - Notice that only the submission (A) is affected by the late policy. Change-Id: Ic8386f713117826f24dc414a6ff92f9565d09b1e Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/263274 Tested-by: Service Cloud Jenkins Reviewed-by: Spencer Olson Reviewed-by: Adrian Packel QA-Review: Kai Bjorkman Product-Review: Syed Hussain --- lib/missing_policy_applicator.rb | 15 ++++++------ spec/lib/missing_policy_applicator_spec.rb | 28 ++++++++++++++++++++++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/lib/missing_policy_applicator.rb b/lib/missing_policy_applicator.rb index fd2a2922829..3466b1ac3a9 100644 --- a/lib/missing_policy_applicator.rb +++ b/lib/missing_policy_applicator.rb @@ -35,13 +35,14 @@ class MissingPolicyApplicator def recently_missing_submissions now = Time.zone.now - Submission.active. - joins(assignment: {course: :late_policy}). - eager_load(:grading_period, assignment: [:post_policy, { course: [:late_policy, :default_post_policy] }]). - for_enrollments(Enrollment.all_active_or_pending). - merge(Assignment.published). - missing. - where(score: nil, grade: nil, cached_due_date: 1.day.ago(now)..now, + Submission.active + .joins(assignment: {course: [:late_policy, :enrollments]}) + .where("enrollments.user_id = submissions.user_id") + .eager_load(:grading_period, assignment: [:post_policy, { course: [:late_policy, :default_post_policy] }]) + .merge(Assignment.published) + .merge(Enrollment.all_active_or_pending) + .missing + .where(score: nil, grade: nil, cached_due_date: 1.day.ago(now)..now, late_policies: { missing_submission_deduction_enabled: true }) end diff --git a/spec/lib/missing_policy_applicator_spec.rb b/spec/lib/missing_policy_applicator_spec.rb index 162582436d5..dc0639fe680 100644 --- a/spec/lib/missing_policy_applicator_spec.rb +++ b/spec/lib/missing_policy_applicator_spec.rb @@ -267,6 +267,34 @@ describe MissingPolicyApplicator do expect { applicator.apply_missing_deductions }.not_to(change { submission.reload.score }) end + it 'does not change the grade on missing submissions for concluded enrollments in other courses' do + c1 = course_with_student(active_all: true).course + c2 = course_with_student(user: @student, active_all: true).course + c1.assignments.create!( + valid_assignment_attributes.merge(grading_type: 'letter_grade', due_at: 1.hour.ago(now)) + ) + c2.assignments.create!( + valid_assignment_attributes.merge(grading_type: 'letter_grade', due_at: 1.hour.ago(now)) + ) + c2.student_enrollments.find_by(user_id: @student).conclude + LatePolicy.create!( + course_id: c1.id, + missing_submission_deduction_enabled: true, + missing_submission_deduction: 75 + ) + LatePolicy.create!( + course_id: c2.id, + missing_submission_deduction_enabled: true, + missing_submission_deduction: 75 + ) + s1 = c1.submissions.find_by(user_id: @student) + s1.update_columns(score: nil, grade: nil) + + s2 = c2.submissions.find_by(user_id: @student) + s2.update_columns(score: nil, grade: nil) + expect { applicator.apply_missing_deductions }.not_to change { s2.reload.grade } + end + it 'does not change the grade on missing submissions for concluded students' do create_recent_assignment @course.student_enrollments.find_by(user_id: @student).conclude