fix assignment override calcs to ignore deleted student overrides

This patchset fixes the assignment override applicator and effective
due dates to only consider active AssignmentOverrideStudent
definitions. Without this fix, if a deleted AssignmentOverrideStudent
appeared first in the database based on id order, its potentially out
of date information would be used.

fixes GRADE-916

test plan:
 - Have a class with a student
 - Create and save an assignment.
 - Edit the assignment to add an override for the student and remove
   the everyone else due date.  Save the assignment.
 - Note the due date shows up on the student grades page
 - Edit the assignment to remove the student due date and add an
 everyone else due date.  Save the assignment.
 - Note the due date shows up on the student grades page
 - Edit the assignment to add an override for the student and remove
   the everyone else due date.  Save the assignment.
 - Note the due date shows up on the student grades page

Change-Id: I4ff90a7843fe1de0f391642b0a8b4ae09be72432
Reviewed-on: https://gerrit.instructure.com/142342
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
Reviewed-by: Shahbaz Javeed <sjaveed@instructure.com>
Tested-by: Jenkins
QA-Review: Indira Pai <ipai@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>

lib/assignment_override_applicator.rb

@@ -145,7 +145,7 @@ module AssignmentOverrideApplicator
   def self.preload_student_ids_for_adhoc_overrides(adhoc_overrides, visible_user_ids)
     if adhoc_overrides.any?
       override_ids_to_student_ids = {}
-      scope = AssignmentOverrideStudent.where(assignment_override_id: adhoc_overrides)
+      scope = AssignmentOverrideStudent.where(assignment_override_id: adhoc_overrides).active
       scope = if ActiveRecord::Relation === visible_user_ids
           return adhoc_overrides if visible_user_ids.is_a?(ActiveRecord::NullRelation)
           scope.
@@ -177,7 +177,7 @@ module AssignmentOverrideApplicator
       overrides.first
     else
       key = assignment_or_quiz.is_a?(Quizzes::Quiz) ? :quiz_id : :assignment_id
-      AssignmentOverrideStudent.where(key => assignment_or_quiz, :user_id => user).first
+      AssignmentOverrideStudent.where(key => assignment_or_quiz, :user_id => user).active.first
     end
   end
 
@@ -198,8 +198,6 @@ module AssignmentOverrideApplicator
       else
         assignment_or_quiz.assignment_overrides.where(:set_type => 'Group', :set_id => group.id).first
       end
-    else
-      nil
     end
   end
 
@@ -410,7 +408,8 @@ module AssignmentOverrideApplicator
     ActiveRecord::Associations::Preloader.new.preload(assignments, [:quiz, :assignment_overrides])
 
     if assignments.any?
-      override_students = AssignmentOverrideStudent.where(:assignment_id => assignments, :user_id => user).index_by(&:assignment_id)
+      override_students =
+        AssignmentOverrideStudent.where(assignment_id: assignments, user_id: user).active.index_by(&:assignment_id)
       assignments.each do |a|
         a.preloaded_override_students ||= {}
         a.preloaded_override_students[user.id] = Array(override_students[a.id])
@@ -423,7 +422,7 @@ module AssignmentOverrideApplicator
     ActiveRecord::Associations::Preloader.new.preload(quizzes, :assignment_overrides)
 
     if quizzes.any?
-      override_students = AssignmentOverrideStudent.where(:quiz_id => quizzes, :user_id => user).index_by(&:quiz_id)
+      override_students = AssignmentOverrideStudent.where(quiz_id: quizzes, user_id: user).active.index_by(&:quiz_id)
       quizzes.each do |q|
         q.preloaded_override_students ||= {}
         q.preloaded_override_students[user.id] = Array(override_students[q.id])

lib/effective_due_dates.rb

@@ -216,7 +216,8 @@ class EffectiveDueDates
               1 AS priority
             FROM
               overrides o
-            INNER JOIN #{AssignmentOverrideStudent.quoted_table_name} os ON os.assignment_override_id = o.id
+            INNER JOIN #{AssignmentOverrideStudent.quoted_table_name} os ON os.assignment_override_id = o.id AND
+              os.workflow_state = 'active'
             WHERE
               o.set_type = 'ADHOC'
               #{filter_students_sql('os')}

spec/lib/assignment_override_applicator_spec.rb

@@ -99,6 +99,23 @@ describe AssignmentOverrideApplicator do
       @reoverridden_assignment = AssignmentOverrideApplicator.assignment_overridden_for(@overridden_assignment, @student)
     end
 
+    it "ignores soft deleted Assignment Override Students" do
+      now = Time.zone.now.change(usec: 0)
+      adhoc_override = assignment_override_model(:assignment => @assignment)
+      override_student = adhoc_override.assignment_override_students.create!(user: @student)
+      adhoc_override.override_due_at(7.days.from_now(now))
+      adhoc_override.save!
+      override_student.update!(workflow_state: 'deleted')
+
+      adhoc_override = assignment_override_model(:assignment => @assignment)
+      adhoc_override.assignment_override_students.create!(user: @student)
+      adhoc_override.override_due_at(2.days.from_now(now))
+      adhoc_override.save!
+
+      overriden_assignment = AssignmentOverrideApplicator.assignment_overridden_for(@assignment, @student)
+      expect(overriden_assignment.due_at).to eq(adhoc_override.due_at)
+    end
+
     context "give teachers the more lenient of override.due_at or assignment.due_at" do
       before do
         teacher_in_course

spec/lib/effective_due_dates_spec.rb

@@ -492,6 +492,30 @@ describe Course do
         expect(result).to eq expected
       end
 
+      it 'ignores soft-deleted adhoc overrides' do
+        override = @assignment1.assignment_overrides.create!(due_at: 7.days.from_now(@now), due_at_overridden: true)
+        override_student = override.assignment_override_students.create!(user: @student1)
+        override_student.update!(workflow_state: 'deleted')
+
+        override = @assignment1.assignment_overrides.create!(due_at: 3.days.from_now(@now), due_at_overridden: true)
+        override.assignment_override_students.create!(user: @student1)
+
+        edd = EffectiveDueDates.for_course(@test_course, @assignment1)
+        result = edd.to_hash
+        expected = {
+          @assignment1.id => {
+            @student1.id => {
+              due_at: 3.days.from_now(@now),
+              grading_period_id: nil,
+              in_closed_grading_period: false,
+              override_id: override.id,
+              override_source: 'ADHOC'
+            }
+          }
+        }
+        expect(result).to eq(expected)
+      end
+
       it 'correctly matches adhoc overrides for different assignments' do
         @assignment2.only_visible_to_overrides = true
         @assignment2.save!