fix crash in backend grade calculator

This fixes a very specific corner case in the grade calculator. When
drop rules eliminated all pointed assignments in the drop highest
phase and ended up with only unpointed assignments being considered in
the dropped lowest phase the grade calculator would error out as it
attempted to subtract from nil. Since we can't do the drop math that
is expected in this phase, the grade calculator will act similar to
dropping for all unpointed assignments.

fixes GRADE-2209

test plan:
 - (For this test plan you'll need to look at the canvas logs)
 - Create an course with a student
 - In the default assignment group, create three assignments, 2 of
   which are 0 points possible and 1 with some number of points
   possible.
 - Adjust the assignment group drop rules to drop one highest
   and one lowest
 - Score the assignments
 - Confirm in the logs that none of the runs of the grade calculator
   crashed with the error "undefined method `+' for nil:NilClass"
 - Confirm the course score for the student is nil

Change-Id: I10ac06d5b99b5c328b0b509902268af57bfc0c37
Reviewed-on: https://gerrit.instructure.com/193580
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
QA-Review: Adrian Packel <apackel@instructure.com>
Tested-by: Jenkins
Product-Review: Keith Garner <kgarner@instructure.com>

lib/grade_calculator.rb

@@ -795,11 +795,11 @@ class GradeCalculator
   end
 
   def keep_highest(submissions, cant_drop, keep, max_total)
-    keep_helper(submissions, cant_drop, keep, max_total) { |*args| big_f_best(*args) }
+    keep_helper(submissions, cant_drop, keep, max_total, keep_mode: :highest) { |*args| big_f_best(*args) }
   end
 
   def keep_lowest(submissions, cant_drop, keep, max_total)
-    keep_helper(submissions, cant_drop, keep, max_total) { |*args| big_f_worst(*args) }
+    keep_helper(submissions, cant_drop, keep, max_total, keep_mode: :lowest) { |*args| big_f_worst(*args) }
   end
 
   # @submissions: set of droppable submissions
@@ -808,30 +808,51 @@ class GradeCalculator
   # @max_total: the highest number of points possible
   # @big_f_blk: sorting block for the big_f function
   # returns +keep+ +submissions+
-  def keep_helper(submissions, cant_drop, keep, max_total, &big_f_blk)
+  def keep_helper(submissions, cant_drop, keep, max_total, keep_mode: nil, &big_f_blk)
     return submissions if submissions.size <= keep
 
     unpointed, pointed = (submissions + cant_drop).partition { |s|
       s[:total].zero?
     }
-    grades = pointed.map { |s| s[:score].to_f / s[:total] }.sort
 
-    q_high = estimate_q_high(pointed, unpointed, grades)
-    q_low  = grades.first
-    q_mid  = (q_low + q_high) / 2
+    kept = nil
+    if pointed.empty? && keep_mode == :lowest
+      # this is a really dumb situation that we saw in the wild.  17
+      # assignments, 2 of them have points possible, the rest have 0
+      # points possible with drop rules of drop 8 lowest and drop 7
+      # highest.
+      #
+      # In drop_pointed above, the call to keep_highest that
+      # goes here ends up eliminating the pointed assignments, so when
+      # keep_lowest is called, we end up here with unpointed
+      # assignments which completely breaks math. estimate_q_high
+      # comes back as NaN and q_low is nil.  "(nil + NaN)/2" means
+      # you're gonna have a bad time.
+      #
+      # What we'll do instead is just sort by score like
+      # drop_unpointed above, and drop the unpointed
+      # ones up to keep.
+      kept = unpointed.sort_by { |s| s[:score].to_f }[-keep,keep]
+    else
+      grades = pointed.map { |s| s[:score].to_f / s[:total] }.sort
 
-    x, kept = big_f_blk.call(q_mid, submissions, cant_drop, keep)
-    threshold = 1 / (2 * keep * max_total**2)
-    until q_high - q_low < threshold
-      x < 0 ?
-        q_high = q_mid :
-        q_low  = q_mid
-      q_mid = (q_low + q_high) / 2
-
-      # bail if we can't can't ever satisfy the threshold (floats!)
-      break if q_mid == q_high || q_mid == q_low
+      q_high = estimate_q_high(pointed, unpointed, grades)
+      q_low  = grades.first
+      q_mid  = (q_low + q_high) / 2
 
       x, kept = big_f_blk.call(q_mid, submissions, cant_drop, keep)
+      threshold = 1 / (2 * keep * max_total**2)
+      until q_high - q_low < threshold
+        x < 0 ?
+          q_high = q_mid :
+          q_low  = q_mid
+        q_mid = (q_low + q_high) / 2
+
+        # bail if we can't can't ever satisfy the threshold (floats!)
+        break if q_mid == q_high || q_mid == q_low
+
+        x, kept = big_f_blk.call(q_mid, submissions, cant_drop, keep)
+      end
     end
 
     kept

spec/lib/grade_calculator_spec.rb

@@ -2059,6 +2059,12 @@ describe GradeCalculator do
       check_grades(100.0, 100.0)
     end
 
+    it "should work with drop rules that result in only unpointed assignments going to the drop lowest phase" do
+      set_grades([[9,0],[10,0],[2,2]])
+      @group.update_attribute :rules, "drop_lowest:1\ndrop_highest:1"
+      check_grades(nil, nil)
+    end
+
     it "should support never_drop" do
       set_default_grades
       rules = "drop_lowest:1\nnever_drop:#{@assignments[3].id}" # 3/38