diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 6d2cb761b32..2d186b4a2d9 100644 --- a/app/models/assignment.rb +++ b/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 diff --git a/app/models/assignment_override.rb b/app/models/assignment_override.rb index 6efcf04a4f6..56fca09d9b0 100644 --- a/app/models/assignment_override.rb +++ b/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' diff --git a/app/models/context_module.rb b/app/models/context_module.rb index 603b6d61430..2d1e4211309 100644 --- a/app/models/context_module.rb +++ b/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 diff --git a/lib/api/v1/assignment_override.rb b/lib/api/v1/assignment_override.rb index a8099fdc7c0..7036ab461d7 100644 --- a/lib/api/v1/assignment_override.rb +++ b/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 diff --git a/lib/has_content_tags.rb b/lib/has_content_tags.rb index 580677f77d9..b6b520aeefb 100644 --- a/lib/has_content_tags.rb +++ b/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 \ No newline at end of file diff --git a/spec/apis/v1/assignment_overrides_spec.rb b/spec/apis/v1/assignment_overrides_spec.rb index a25ec20b755..41e1c800721 100644 --- a/spec/apis/v1/assignment_overrides_spec.rb +++ b/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