From 36489396781426d0188fa12e543da4a75c9993af Mon Sep 17 00:00:00 2001 From: Gary Mei Date: Wed, 14 Oct 2020 11:26:26 -0500 Subject: [PATCH] fix missing policy not adding grade change records fixes EVAL-279 flag=fix_missing_policy_grade_change_records Test Plan - Enable the Fix Missing Policy Grade Change Records feature flag as site admin. - Set up a missing policy. - Create an assignment due soon. - After the due date/time passes, check Gradebook History. This may require another 5 minutes as MissingPolicyApplicator runs on intervals. Verify that the grade assigned by the missing policy appears in Gradebook History. - Disable the feature flag and verify that the above no longer happens. Change-Id: I6fa8319d9d98b9013d2938e61eb22542ea176058 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/250112 Tested-by: Service Cloud Jenkins QA-Review: Gary Mei Product-Review: Jody Sailor Reviewed-by: Adrian Packel Reviewed-by: Spencer Olson --- config/feature_flags/apogee_release_flags.yml | 5 ++ lib/missing_policy_applicator.rb | 8 ++- spec/lib/missing_policy_applicator_spec.rb | 59 ++++++++++++++++++- 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/config/feature_flags/apogee_release_flags.yml b/config/feature_flags/apogee_release_flags.yml index fe0cbfb140f..7f6f5501cc0 100644 --- a/config/feature_flags/apogee_release_flags.yml +++ b/config/feature_flags/apogee_release_flags.yml @@ -69,3 +69,8 @@ view_ungraded_as_zero: display_name: View Ungraded as Zero View in Gradebook description: The Gradebook will factor in ungraded submissions as if they were given a score of zero for calculations. This is just a view for the teacher, and does not affect actual scoring. +fix_missing_policy_grade_change_records: + state: hidden + applies_to: SiteAdmin + display_name: Fix Missing Policy Grade Change Records + description: Insert a grade change record when the periodic MissingPolicyApplicator job runs on a submission. diff --git a/lib/missing_policy_applicator.rb b/lib/missing_policy_applicator.rb index ed83eadbdbd..6b3a418d677 100644 --- a/lib/missing_policy_applicator.rb +++ b/lib/missing_policy_applicator.rb @@ -50,7 +50,9 @@ class MissingPolicyApplicator now = Time.zone.now GuardRail.activate(:primary) do - Submission.active.where(id: submissions).update_all( + submissions = Submission.active.where(id: submissions) + + submissions.update_all( score: score, grade: grade, graded_at: now, @@ -62,6 +64,10 @@ class MissingPolicyApplicator workflow_state: "graded" ) + if Account.site_admin.feature_enabled?(:fix_missing_policy_grade_change_records) + submissions.reload.each { |sub| sub.grade_change_audit(force_audit: true) } + end + if assignment.course.root_account.feature_enabled?(:missing_policy_applicator_emits_live_events) Canvas::LiveEvents.send_later_if_production(:submissions_bulk_updated, submissions) end diff --git a/spec/lib/missing_policy_applicator_spec.rb b/spec/lib/missing_policy_applicator_spec.rb index 29dc2da4b91..b8c700e52dc 100644 --- a/spec/lib/missing_policy_applicator_spec.rb +++ b/spec/lib/missing_policy_applicator_spec.rb @@ -297,6 +297,60 @@ describe MissingPolicyApplicator do expect(submission.reload.grade_matches_current_submission).to be true end + describe "grade change events" do + before(:each) do + allow(Auditors).to receive(:config).and_return({'write_paths' => ['active_record'], 'read_path' => 'active_record'}) + late_policy_missing_enabled + create_recent_assignment + @assignment = @course.assignments.last + @submission = @assignment.submissions.first + # The act of creating an assignment due in the past applies the missing + # policy on the submissions separately from MissingPolicyApplicator, so + # in order to test that MissingPolicyApplicator inserts grade change + # events, we have to delete any existing ones first, otherwise we may + # just be picking up on the ones generated by assignment creation. + Auditors::GradeChange. + for_assignment(@assignment).paginate(per_page: 10). + select { |gc| gc.submission_id == @submission.id }. + each { |gc| gc.destroy! } + end + + context "when fix_missing_policy_grade_change_records flag is enabled" do + before(:each) do + Account.site_admin.enable_feature!(:fix_missing_policy_grade_change_records) + end + + it "inserts a grade change for affected submissions" do + @submission.update_columns(score: nil, grade: nil) + applicator.apply_missing_deductions + grade_changes = Auditors::GradeChange.for_assignment(@assignment).paginate(per_page: 10) + expect(grade_changes.find { |gc| gc.submission_id == @submission.id }).not_to be_nil + end + + it "the inserted grade change contains the correct score before/after values" do + @submission.update_columns(score: nil, grade: nil) + applicator.apply_missing_deductions + grade_changes = Auditors::GradeChange.for_assignment(@assignment).paginate(per_page: 10) + submission_event = grade_changes.find { |gc| gc.submission_id == @submission.id } + expect(submission_event.score_before).to be_nil + expect(submission_event.score_after).to be 0.375 + end + end + + context "when fix_missing_policy_grade_change_records flag is not enabled" do + before(:each) do + Account.site_admin.disable_feature!(:fix_missing_policy_grade_change_records) + end + + it "does not insert a grade change for affected submissions" do + @submission.update_columns(score: nil, grade: nil) + applicator.apply_missing_deductions + grade_changes = Auditors::GradeChange.for_assignment(@assignment).paginate(per_page: 10) + expect(grade_changes.find { |gc| gc.submission_id == @submission.id }).to be_nil + end + end + end + describe "posting submissions" do let(:assignment) { @course.assignments.first } let(:submission) { assignment.submissions.first } @@ -308,13 +362,14 @@ describe MissingPolicyApplicator do end it "posts affected submissions if the assignment is automatically posted" do + submission.update_column(:posted_at, nil) applicator.apply_missing_deductions expect(submission.reload).to be_posted end - it "sets posted_at to nil for submissions if the assignment is manually posted" do - submission.update!(posted_at: Time.zone.now) + it "does not post affected submissions if the assignment is manually posted" do assignment.post_policy.update!(post_manually: true) + submission.update_column(:posted_at, nil) applicator.apply_missing_deductions expect(submission.reload).not_to be_posted end