limit module progression recomputation for adhoc overrides

only relock progressions for students affected by the change
(i.e. students added or removed from the assignment override)

closes #CNVS-32542

Change-Id: I814117bef9346bdb375f0a52e3c0814637d49e85
Reviewed-on: https://gerrit.instructure.com/92330
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Product-Review: James Williams  <jamesw@instructure.com>
QA-Review: James Williams  <jamesw@instructure.com>

app/models/assignment.rb

@@ -2155,10 +2155,10 @@ class Assignment < ActiveRecord::Base
       Lti::AppLaunchCollator.any?(context, [:post_grades])
   end
 
-  def run_if_overrides_changed!
+  def run_if_overrides_changed!(student_ids=nil)
     relocked_modules = []
-    self.relock_modules!(relocked_modules)
-    each_submission_type { |submission| submission.relock_modules!(relocked_modules) if submission }
+    self.relock_modules!(relocked_modules, student_ids)
+    each_submission_type { |submission| submission.relock_modules!(relocked_modules, student_ids) if submission }
 
     if only_visible_to_overrides?
       Rails.logger.info "GRADES: recalculating because assignment overrides on #{global_id} changed."
@@ -2166,7 +2166,11 @@ class Assignment < ActiveRecord::Base
     end
   end
 
-  def run_if_overrides_changed_later!
-    self.send_later_if_production_enqueue_args(:run_if_overrides_changed!, {:singleton => "assignment_overrides_changed_#{self.global_id}"})
+  def run_if_overrides_changed_later!(student_ids=nil)
+    if student_ids
+      self.send_later_if_production_enqueue_args(:run_if_overrides_changed!, {:strand => "assignment_overrides_changed_for_students_#{self.global_id}"}, student_ids)
+    else
+      self.send_later_if_production_enqueue_args(:run_if_overrides_changed!, {:singleton => "assignment_overrides_changed_#{self.global_id}"})
+    end
   end
 end

app/models/assignment_override.rb

@@ -24,7 +24,7 @@ class AssignmentOverride < ActiveRecord::Base
 
   attr_accessible
 
-  attr_accessor :dont_touch_assignment, :preloaded_student_ids
+  attr_accessor :dont_touch_assignment, :preloaded_student_ids, :changed_student_ids
 
   belongs_to :assignment
   belongs_to :quiz, class_name: 'Quizzes::Quiz'

app/models/context_module.rb

@@ -68,18 +68,19 @@ class ContextModule < ActiveRecord::Base
     @relock_warning
   end
 
-  def relock_progressions(relocked_modules=[])
+  def relock_progressions(relocked_modules=[], student_ids=nil)
     return if relocked_modules.include?(self)
     self.class.connection.after_transaction_commit do
       relocked_modules << self
-      if self.context_module_progressions.where(:current => true).where.not(:workflow_state => 'locked').
-          update_all(["workflow_state = 'locked', lock_version = lock_version + 1, current = ?", false]) > 0
+      progression_scope = self.context_module_progressions.where(:current => true).where.not(:workflow_state => 'locked')
+      progression_scope = progression_scope.where(:user_id => student_ids) if student_ids
 
+      if progression_scope.update_all(["workflow_state = 'locked', lock_version = lock_version + 1, current = ?", false]) > 0
         send_later_if_production_enqueue_args(:evaluate_all_progressions, {:strand => "module_reeval_#{self.global_context_id}"})
       end
 
       self.context.context_modules.each do |mod|
-        mod.relock_progressions(relocked_modules) if self.is_prerequisite_for?(mod)
+        mod.relock_progressions(relocked_modules, student_ids) if self.is_prerequisite_for?(mod)
       end
     end
   end

lib/api/v1/assignment_override.rb

@@ -267,6 +267,8 @@ module Api::V1::AssignmentOverride
         Set.new :
         override.assignment_override_students.map(&:user_id).to_set
 
+      override.changed_student_ids = Set.new
+
       override_data[:students].each do |student|
         if defunct_student_ids.include?(student.id)
           defunct_student_ids.delete(student.id)
@@ -275,10 +277,12 @@ module Api::V1::AssignmentOverride
           link = override.assignment_override_students.build
           link.assignment_override = override
           link.user = student
+          override.changed_student_ids << student.id
         end
       end
 
       unless defunct_student_ids.empty?
+        override.changed_student_ids.merge(defunct_student_ids)
         override.assignment_override_students.
           where(:user_id => defunct_student_ids.to_a).
           delete_all
@@ -313,7 +317,11 @@ module Api::V1::AssignmentOverride
       update_assignment_override_without_save(override, override_data)
       override.save!
     end
-    override.assignment.run_if_overrides_changed_later!
+    if override.set_type == 'ADHOC' && override.changed_student_ids.present?
+      override.assignment.run_if_overrides_changed_later!(override.changed_student_ids.to_a)
+    else
+      override.assignment.run_if_overrides_changed_later!
+    end
     return true
   rescue ActiveRecord::RecordInvalid
     return false

lib/has_content_tags.rb

@@ -56,9 +56,9 @@ module HasContentTags
     Rails.cache.delete locked_cache_key(user)
   end
 
-  def relock_modules!(relocked_modules=[])
+  def relock_modules!(relocked_modules=[], student_ids=nil)
     ContextModule.where(:id => ContentTag.where(:content_id => self, :content_type => self.class.to_s).not_deleted.select(:context_module_id)).each do |mod|
-      mod.relock_progressions(relocked_modules)
+      mod.relock_progressions(relocked_modules, student_ids)
     end
   end
 end

spec/apis/v1/assignment_overrides_spec.rb

@@ -749,6 +749,7 @@ describe AssignmentOverridesController, type: :request do
       end
 
       it "should relock modules when changing overrides" do
+        # but only for the students they affect
         @assignment.only_visible_to_overrides = true
         @assignment.save!
 
@@ -757,15 +758,26 @@ describe AssignmentOverridesController, type: :request do
         mod.completion_requirements = {tag.id => {:type => 'must_submit'}}
         mod.save!
 
+        @old_student = @student
+        @new_student = student_in_course(:course => @course).user
         @other_student = student_in_course(:course => @course).user
 
-        prog = mod.evaluate_for(@other_student)
-        expect(prog).to be_completed # since they can't see the assignment
+        prog = mod.evaluate_for(@old_student)
+        expect(prog).to be_unlocked # since they can see the assignment
 
-        api_update_override(@course, @assignment, @override, :assignment_override => { :student_ids => [@other_student.id] })
+        new_prog = mod.evaluate_for(@new_student)
+        expect(new_prog).to be_completed # since they can't see the assignment yet
+
+        other_prog = mod.evaluate_for(@other_student)
+        other_prog.any_instantiation.expects(:evaluate!).never
+
+        api_update_override(@course, @assignment, @override, :assignment_override => { :student_ids => [@new_student.id] })
 
         prog.reload
-        expect(prog).to be_unlocked # now they can
+        expect(prog).to be_completed # now they can't see it anymore
+
+        new_prog.reload
+        expect(new_prog).to be_unlocked # now they can
       end
 
       it "recomputes grades when changing overrides" do