fix some N+1s in the due date cacher

closes TALLY-777

test plan:
 - specs pass

Change-Id: I4f7b4a250534ba74f01c0d8dd908fd049b33be93
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/231710
Reviewed-by: Gary Mei <gmei@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
QA-Review: Adrian Packel <apackel@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>

lib/due_date_cacher.rb

@@ -82,7 +82,7 @@ class DueDateCacher
   def self.recompute(assignment, update_grades: false, executing_user: nil)
     current_caller = caller(1..1).first
     Rails.logger.debug "DDC.recompute(#{assignment&.id}) - #{current_caller}"
-    return unless assignment.active?
+    return unless assignment.persisted? && assignment.active?
     # We use a strand here instead of a singleton because a bunch of
     # assignment updates with upgrade_grades could end up causing
     # score table fights.
@@ -168,13 +168,14 @@ class DueDateCacher
     # in a transaction on the correct shard:
     @course.shard.activate do
       values = []
+
+      assignments_by_id = Assignment.find(@assignment_ids).index_by(&:id)
+
       effective_due_dates.to_hash.each do |assignment_id, student_due_dates|
         students_without_priors = student_due_dates.keys - enrollment_counts.prior_student_ids
-        existing_anonymous_ids = Submission.where.not(user: nil).
-          where(user: students_without_priors).
-          anonymous_ids_for(assignment_id)
+        existing_anonymous_ids = existing_anonymous_ids_by_assignment_id[assignment_id]
 
-        create_moderation_selections_for_assignment(assignment_id, student_due_dates.keys, @user_ids)
+        create_moderation_selections_for_assignment(assignments_by_id[assignment_id], student_due_dates.keys, @user_ids)
 
         quiz_lti = quiz_lti_assignments.include?(assignment_id)
 
@@ -190,25 +191,29 @@ class DueDateCacher
         end
       end
 
+      assignments_to_delete_all_submissions_for = []
       # Delete submissions for students who don't have visibility to this assignment anymore
       @assignment_ids.each do |assignment_id|
         assigned_student_ids = effective_due_dates.find_effective_due_dates_for_assignment(assignment_id).keys
-        submission_scope = Submission.active.where(assignment_id: assignment_id)
 
         if @user_ids.blank? && assigned_student_ids.blank? && enrollment_counts.prior_student_ids.blank?
-          submission_scope.in_batches.update_all(workflow_state: :deleted, updated_at: Time.zone.now)
+          assignments_to_delete_all_submissions_for << assignment_id
         else
           # Delete the users we KNOW we need to delete in batches (it makes the database happier this way)
           deletable_student_ids =
             enrollment_counts.accepted_student_ids - assigned_student_ids - enrollment_counts.prior_student_ids
           deletable_student_ids.each_slice(1000) do |deletable_student_ids_chunk|
             # using this approach instead of using .in_batches because we want to limit the IDs in the IN clause to 1k
-            submission_scope.where(user_id: deletable_student_ids_chunk).
+            Submission.active.where(assignment_id: assignment_id, user_id: deletable_student_ids_chunk).
               update_all(workflow_state: :deleted, updated_at: Time.zone.now)
           end
           User.clear_cache_keys(deletable_student_ids, :submissions)
         end
       end
+      unless assignments_to_delete_all_submissions_for.empty?
+        Submission.active.where(assignment_id: assignments_to_delete_all_submissions_for).
+          in_batches.update_all(workflow_state: :deleted, updated_at: Time.zone.now)
+      end
 
       # Get any stragglers that might have had their enrollment removed from the course
       # 100 students at a time for 10 assignments each == slice of up to 1K submissions
@@ -412,4 +417,13 @@ class DueDateCacher
         where(context_type: 'Assignment', context_id: @assignment_ids).
         where.not(workflow_state: 'deleted').distinct.pluck(:context_id).to_set
   end
+
+  def existing_anonymous_ids_by_assignment_id
+    @existing_anonymous_ids_by_assignment_id ||=
+      Submission.
+        anonymized.
+        for_assignment(effective_due_dates.to_hash.keys).
+        pluck(:assignment_id, :anonymous_id).
+        each_with_object(Hash.new { |h,k| h[k] = [] }) { |data, h| h[data.first] << data.last }
+  end
 end

lib/moderation.rb

@@ -16,17 +16,16 @@
 # with this program. If not, see <http://www.gnu.org/licenses/>.
 
 module Moderation
-  def create_moderation_selections_for_assignment(assignment_id, student_ids, student_context)
-    assignment = Assignment.find(assignment_id)
+  def create_moderation_selections_for_assignment(assignment, student_ids, student_context)
+    assignment = Assignment.find(assignment) unless assignment.is_a?(Assignment)
     return unless assignment.moderated_grading
 
     # Add selections for students in Student IDs
     already_moderated_ids = assignment.moderated_grading_selections.pluck(:student_id)
     to_add_ids = student_ids - already_moderated_ids
-    to_add = User.where(id: to_add_ids)
 
-    to_add.each do |student|
-      assignment.moderated_grading_selections.create! student: student
+    to_add_ids.each do |student_id|
+      assignment.moderated_grading_selections.create! student_id: student_id
     end
 
     # Delete selections not in Student IDs but in the context of students to be considered
@@ -34,4 +33,4 @@ module Moderation
     extra_moderated_ids &= student_context if student_context.any?
     assignment.moderated_grading_selections.where(student_id: extra_moderated_ids).destroy_all
   end
-end
+end

spec/lib/due_date_cacher_spec.rb

@@ -29,6 +29,13 @@ describe DueDateCacher do
       @instance = double('instance', :recompute => nil)
     end
 
+    it "doesn't call self.recompute_course if the assignment passed in hasn't been persisted" do
+      expect(DueDateCacher).not_to receive(:recompute_course)
+
+      assignment = Assignment.new(course: @course, workflow_state: :published)
+      DueDateCacher.recompute(assignment)
+    end
+
     it "wraps assignment in an array" do
       expect(DueDateCacher).to receive(:new).with(@course, [@assignment.id], hash_including(update_grades: false)).
         and_return(@instance)