stop course touch on enrollment creation
test plan: * specs closes #LS-1221 Change-Id: I9ec8d166730681ce648c30c5e8fb0d29d25ae343 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/242819 Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com> Reviewed-by: Jeremy Stanley <jeremy@instructure.com> Reviewed-by: Cody Cutrer <cody@instructure.com> QA-Review: Cody Cutrer <cody@instructure.com> Product-Review: James Williams <jamesw@instructure.com>
This commit is contained in:
parent
698a8cecce
commit
0e96dbad90
|
@ -2114,16 +2114,10 @@ class Course < ActiveRecord::Base
|
||||||
e.sis_pseudonym_id = opts[:sis_pseudonym_id]
|
e.sis_pseudonym_id = opts[:sis_pseudonym_id]
|
||||||
if e.changed?
|
if e.changed?
|
||||||
e.need_touch_user = true if opts[:skip_touch_user]
|
e.need_touch_user = true if opts[:skip_touch_user]
|
||||||
transaction do
|
if opts[:no_notify]
|
||||||
# without this, inserting/updating on enrollments will share lock the course, but then
|
e.save_without_broadcasting
|
||||||
# it tries to touch the course, which will deadlock with another transaction doing the
|
else
|
||||||
# same thing.
|
e.save
|
||||||
self.lock!(:no_key_update)
|
|
||||||
if opts[:no_notify]
|
|
||||||
e.save_without_broadcasting
|
|
||||||
else
|
|
||||||
e.save
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
e.user = user
|
e.user = user
|
||||||
|
|
|
@ -30,7 +30,7 @@ class Enrollment < ActiveRecord::Base
|
||||||
|
|
||||||
include Workflow
|
include Workflow
|
||||||
|
|
||||||
belongs_to :course, touch: true, inverse_of: :enrollments
|
belongs_to :course, inverse_of: :enrollments
|
||||||
belongs_to :course_section, inverse_of: :enrollments
|
belongs_to :course_section, inverse_of: :enrollments
|
||||||
belongs_to :root_account, class_name: 'Account', inverse_of: :enrollments
|
belongs_to :root_account, class_name: 'Account', inverse_of: :enrollments
|
||||||
belongs_to :user, inverse_of: :enrollments
|
belongs_to :user, inverse_of: :enrollments
|
||||||
|
|
|
@ -24,15 +24,13 @@ module SIS
|
||||||
def process(messages)
|
def process(messages)
|
||||||
i = Work.new(@batch, @root_account, @logger, messages)
|
i = Work.new(@batch, @root_account, @logger, messages)
|
||||||
|
|
||||||
Enrollment.skip_touch_callbacks(:course) do
|
Enrollment.suspend_callbacks(:set_update_cached_due_dates, :add_to_favorites_later,
|
||||||
Enrollment.suspend_callbacks(:set_update_cached_due_dates, :add_to_favorites_later,
|
:recache_course_grade_distribution, :update_user_account_associations_if_necessary) do
|
||||||
:recache_course_grade_distribution, :update_user_account_associations_if_necessary) do
|
User.skip_updating_account_associations do
|
||||||
User.skip_updating_account_associations do
|
Enrollment.process_as_sis(@sis_options) do
|
||||||
Enrollment.process_as_sis(@sis_options) do
|
yield i
|
||||||
yield i
|
while i.any_left_to_process?
|
||||||
while i.any_left_to_process?
|
i.process_batch
|
||||||
i.process_batch
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -22,11 +22,9 @@ module SIS
|
||||||
def process
|
def process
|
||||||
importer = Work.new(@batch, @root_account, @logger)
|
importer = Work.new(@batch, @root_account, @logger)
|
||||||
|
|
||||||
Enrollment.skip_touch_callbacks(:course) do
|
Enrollment.suspend_callbacks(:set_update_cached_due_dates) do
|
||||||
Enrollment.suspend_callbacks(:set_update_cached_due_dates) do
|
User.skip_updating_account_associations do
|
||||||
User.skip_updating_account_associations do
|
yield importer
|
||||||
yield importer
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -349,6 +349,7 @@ describe LatePolicyApplicator do
|
||||||
override.assignment_override_students.create!(user: @students[0])
|
override.assignment_override_students.create!(user: @students[0])
|
||||||
|
|
||||||
# turn on the late policy without calling callbacks
|
# turn on the late policy without calling callbacks
|
||||||
|
@late_policy = late_policy_factory(course: @course, deduct: 50.0, every: :day, missing: 95.0)
|
||||||
@course.update_attribute(:late_policy, @late_policy)
|
@course.update_attribute(:late_policy, @late_policy)
|
||||||
|
|
||||||
late_policy_applicator = LatePolicyApplicator.new(@course, [assignment_to_override])
|
late_policy_applicator = LatePolicyApplicator.new(@course, [assignment_to_override])
|
||||||
|
|
|
@ -139,27 +139,5 @@ module SIS
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'should skip touching courses' do
|
|
||||||
Timecop.freeze(2.days.ago) do
|
|
||||||
@c = course_model(sis_source_id: 'C001')
|
|
||||||
u = user_with_managed_pseudonym(sis_user_id: 'U001')
|
|
||||||
@e = @c.enroll_user(u)
|
|
||||||
@time = @c.updated_at
|
|
||||||
end
|
|
||||||
|
|
||||||
Enrollment.skip_touch_callbacks(:course) do
|
|
||||||
@e.updated_at = 2.seconds.from_now
|
|
||||||
@e.save!
|
|
||||||
end
|
|
||||||
@c.reload
|
|
||||||
expect(@c.updated_at).to eq @time
|
|
||||||
|
|
||||||
@e.updated_at = 5.seconds.from_now
|
|
||||||
@e.save!
|
|
||||||
@c.reload
|
|
||||||
expect(@c.updated_at).not_to eq @time
|
|
||||||
end
|
|
||||||
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -504,6 +504,7 @@ describe DiscussionTopic do
|
||||||
section2 = @course.course_sections.create!
|
section2 = @course.course_sections.create!
|
||||||
student1 = create_enrolled_user(@course, section1, :name => 'student 1', :enrollment_type => 'StudentEnrollment')
|
student1 = create_enrolled_user(@course, section1, :name => 'student 1', :enrollment_type => 'StudentEnrollment')
|
||||||
student2 = create_enrolled_user(@course, section2, :name => 'student 2', :enrollment_type => 'StudentEnrollment')
|
student2 = create_enrolled_user(@course, section2, :name => 'student 2', :enrollment_type => 'StudentEnrollment')
|
||||||
|
@course.reload
|
||||||
add_section_to_topic(topic, section2)
|
add_section_to_topic(topic, section2)
|
||||||
topic.save!
|
topic.save!
|
||||||
topic.publish!
|
topic.publish!
|
||||||
|
|
Loading…
Reference in New Issue