fix update_account_associations job getting created for every imported user

Change-Id: Ia7dc8ca3912510114d88035e264e59ebb54fe086
Reviewed-on: https://gerrit.instructure.com/3446
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
This commit is contained in:
Zach Wily 2011-05-03 13:21:18 -06:00
parent 7ba5d5e3b2
commit 7beaf4d764
6 changed files with 171 additions and 160 deletions

View File

@ -117,7 +117,7 @@ class AccountsController < ApplicationController
@pseudonym.assert_communication_channel(true)
message_sent = notify && new_login
@pseudonym.send_registration_notification! if notify && (new_login || @user.pre_registered?)
@user.send_later(:update_account_associations)
@user.update_account_associations_later
@user.reload
data = OpenObject.new(:user => @user, :pseudonym => @pseudonym, :channel => @user.communication_channel, :new_login => new_login, :new_user => new_user, :message_sent => message_sent)
respond_to do |format|

View File

@ -38,8 +38,6 @@ class Enrollment < ActiveRecord::Base
after_save :touch_user
before_save :update_user_account_associations_if_necessary
attr_accessor :skip_updating_user_account_associations
trigger.after(:insert).where("NEW.workflow_state = 'active'") do
<<-SQL
UPDATE assignments
@ -141,9 +139,7 @@ class Enrollment < ActiveRecord::Base
end
def update_user_account_associations_if_necessary
if !@skip_updating_user_account_associations && should_update_user_account_association?
self.user.send_later(:update_account_associations)
end
self.user.update_account_associations_later if should_update_user_account_association?
end
protected :update_user_account_associations_if_necessary

View File

@ -79,7 +79,7 @@ class Pseudonym < ActiveRecord::Base
end
def update_account_associations_if_account_changed
update_user_account_associations if @should_update_user_account_associations
self.user.update_account_associations_later if self.user && @should_update_user_account_associations
end
def send_registration_notification!
@ -102,10 +102,6 @@ class Pseudonym < ActiveRecord::Base
end
end
def update_user_account_associations
self.user.send_later_if_production(:update_account_associations) if self.user
end
def set_password_changed
@password_changed = self.password && self.password_confirmation == self.password
end

View File

@ -193,6 +193,20 @@ class User < ActiveRecord::Base
end
memoize :page_views_by_day
def self.skip_updating_user_account_associations(&block)
@skip_updating_user_account_associations = true
block.call
ensure
@skip_updating_user_account_associations = false
end
def self.skip_updating_user_account_associations?
!!@skip_updating_user_account_associations
end
def update_account_associations_later
self.send_later_if_production(:update_account_associations) unless self.class.skip_updating_user_account_associations?
end
def self.update_account_associations(all_user_ids)
all_user_ids.uniq.compact.each_slice(100) do |user_ids|
User.find_all_by_id(user_ids).each do |user|

View File

@ -42,91 +42,94 @@ module SIS
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
logger.debug("Processing Enrollment #{row.inspect}")
update_progress
pseudo = Pseudonym.find_by_account_id_and_sis_user_id(@root_account.id, row['user_id'])
user = pseudo.user rescue nil
course = Course.find_by_root_account_id_and_sis_source_id(@root_account.id, row['course_id']) unless row['course_id'].blank?
section = CourseSection.find_by_root_account_id_and_sis_source_id(@root_account.id, row['section_id']) unless row['section_id'].blank?
unless user && (course || section)
add_warning csv, "Neither course #{row['course_id']} nor section #{row['section_id']} existed for user enrollment" unless (course || section)
add_warning csv, "User #{row['user_id']} didn't exist for user enrollment" unless user
next
end
if row['section_id'] && !section
add_warning csv, "An enrollment referenced a non-existent section #{row['section_id']}"
next
end
if row['course_id'] && !course
add_warning csv, "An enrollment referenced a non-existent course #{row['course_id']}"
next
end
unless section
section = course.course_sections.find_by_sis_source_id(row['section_id'])
section ||= course.default_section
end
course ||= section.course
if course != section.course
add_warning csv, "An enrollment listed a section and a course that are unrelated"
next
end
enrollment = section.enrollments.find_by_user_id(user.id)
unless enrollment
enrollment = Enrollment.new
enrollment.course_id = course.id
enrollment.user_id = user.id
enrollment.root_account_id = @root_account.id
end
enrollment.sis_batch_id = @batch.id if @batch
enrollment.sis_source_id = [row['course_id'], row['user_id'], row['role'], section.name].compact.join(":")
enrollment.course_id = course.id
enrollment.course_section_id = section.id
if row['role'] =~ /\Ateacher\z/i
enrollment.type = 'TeacherEnrollment'
elsif row['role'] =~ /student/i
enrollment.type = 'StudentEnrollment'
elsif row['role'] =~ /\Ata\z|assistant/i
enrollment.type = 'TaEnrollment'
elsif row['role'] =~ /\Aobserver\z/i
enrollment.type = 'ObserverEnrollment'
if row['associated_user_id']
pseudo = Pseudonym.find_by_account_id_and_sis_user_id(@root_account.id, row['associated_user_id'])
associated_enrollment = pseudo && course.student_enrollments.find_by_user_id(pseudo.user_id)
enrollment.associated_user_id = associated_enrollment && associated_enrollment.user_id
end
elsif row['role'] =~ /\Adesigner\z/i
enrollment.type = 'DesignerEnrollment'
end
# special-case status that bases the enrollment state
# off of availability dates instead of explicitly setting it.
if row['status']=~ /active_if_available/i
row['status'] = course.enrollment_state_based_on_date(enrollment)
end
if row['status']=~ /active/i
if user.workflow_state != 'deleted'
enrollment.workflow_state = 'active'
else
enrollment.workflow_state = 'deleted'
add_warning csv, "Attempted enrolling of deleted user #{row['user_id']} in course #{row['course_id']}"
User.skip_updating_user_account_associations do
pseudo = Pseudonym.find_by_account_id_and_sis_user_id(@root_account.id, row['user_id'])
user = pseudo.user rescue nil
course = Course.find_by_root_account_id_and_sis_source_id(@root_account.id, row['course_id']) unless row['course_id'].blank?
section = CourseSection.find_by_root_account_id_and_sis_source_id(@root_account.id, row['section_id']) unless row['section_id'].blank?
unless user && (course || section)
add_warning csv, "Neither course #{row['course_id']} nor section #{row['section_id']} existed for user enrollment" unless (course || section)
add_warning csv, "User #{row['user_id']} didn't exist for user enrollment" unless user
next
end
elsif row['status']=~ /deleted/i
enrollment.workflow_state = 'deleted'
elsif row['status']=~ /completed/i
enrollment.workflow_state = 'completed'
elsif row['status']=~ /inactive/i
enrollment.workflow_state = 'inactive'
end
update_account_association_user_ids.add(user.id) if enrollment.should_update_user_account_association?
enrollment.skip_updating_user_account_associations = true
enrollment.save_without_broadcasting
if row['section_id'] && !section
add_warning csv, "An enrollment referenced a non-existent section #{row['section_id']}"
next
end
if row['course_id'] && !course
add_warning csv, "An enrollment referenced a non-existent course #{row['course_id']}"
next
end
unless section
section = course.course_sections.find_by_sis_source_id(row['section_id'])
section ||= course.default_section
end
course ||= section.course
if course != section.course
add_warning csv, "An enrollment listed a section and a course that are unrelated"
next
end
enrollment = section.enrollments.find_by_user_id(user.id)
unless enrollment
enrollment = Enrollment.new
enrollment.course_id = course.id
enrollment.user_id = user.id
enrollment.root_account_id = @root_account.id
end
enrollment.sis_batch_id = @batch.id if @batch
enrollment.sis_source_id = [row['course_id'], row['user_id'], row['role'], section.name].compact.join(":")
enrollment.course_id = course.id
enrollment.course_section_id = section.id
if row['role'] =~ /\Ateacher\z/i
enrollment.type = 'TeacherEnrollment'
elsif row['role'] =~ /student/i
enrollment.type = 'StudentEnrollment'
elsif row['role'] =~ /\Ata\z|assistant/i
enrollment.type = 'TaEnrollment'
elsif row['role'] =~ /\Aobserver\z/i
enrollment.type = 'ObserverEnrollment'
if row['associated_user_id']
pseudo = Pseudonym.find_by_account_id_and_sis_user_id(@root_account.id, row['associated_user_id'])
associated_enrollment = pseudo && course.student_enrollments.find_by_user_id(pseudo.user_id)
enrollment.associated_user_id = associated_enrollment && associated_enrollment.user_id
end
elsif row['role'] =~ /\Adesigner\z/i
enrollment.type = 'DesignerEnrollment'
end
# special-case status that bases the enrollment state
# off of availability dates instead of explicitly setting it.
if row['status']=~ /active_if_available/i
row['status'] = course.enrollment_state_based_on_date(enrollment)
end
if row['status']=~ /active/i
if user.workflow_state != 'deleted'
enrollment.workflow_state = 'active'
else
enrollment.workflow_state = 'deleted'
add_warning csv, "Attempted enrolling of deleted user #{row['user_id']} in course #{row['course_id']}"
end
elsif row['status']=~ /deleted/i
enrollment.workflow_state = 'deleted'
elsif row['status']=~ /completed/i
enrollment.workflow_state = 'completed'
elsif row['status']=~ /inactive/i
enrollment.workflow_state = 'inactive'
end
update_account_association_user_ids.add(user.id) if enrollment.should_update_user_account_association?
enrollment.save_without_broadcasting
end
@sis.counts[:enrollments] += 1
end
# We batch these up at the end because normally a user would get several enrollments, and there's no reason

View File

@ -52,84 +52,86 @@ module SIS
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
logger.debug("Processing User #{row.inspect}")
update_account_association = false
update_progress
pseudo = Pseudonym.find_by_account_id_and_sis_user_id(@root_account.id, row['user_id'])
pseudo_by_login = Pseudonym.find_by_unique_id_and_account_id(row['login_id'], @root_account.id)
pseudo ||= pseudo_by_login
pseudo ||= Pseudonym.find_by_unique_id_and_account_id(row['email'], @root_account.id) if row['email'].present?
if pseudo
if pseudo.sis_user_id.present? && pseudo.sis_user_id != row['user_id']
add_warning(csv, "user #{pseudo.sis_user_id} has already claimed #{row['user_id']}'s requested login information, skipping")
@sis.counts[:users] += 1
next
end
if !pseudo_by_login.nil? && pseudo.unique_id != row['login_id']
add_warning(csv, "user #{pseudo_by_login.sis_user_id} has already claimed #{row['user_id']}'s requested login information, skipping")
@sis.counts[:users] += 1
next
User.skip_updating_user_account_associations do
update_account_association = false
pseudo = Pseudonym.find_by_account_id_and_sis_user_id(@root_account.id, row['user_id'])
pseudo_by_login = Pseudonym.find_by_unique_id_and_account_id(row['login_id'], @root_account.id)
pseudo ||= pseudo_by_login
pseudo ||= Pseudonym.find_by_unique_id_and_account_id(row['email'], @root_account.id) if row['email'].present?
if pseudo
if pseudo.sis_user_id.present? && pseudo.sis_user_id != row['user_id']
add_warning(csv, "user #{pseudo.sis_user_id} has already claimed #{row['user_id']}'s requested login information, skipping")
@sis.counts[:users] += 1
next
end
if !pseudo_by_login.nil? && pseudo.unique_id != row['login_id']
add_warning(csv, "user #{pseudo_by_login.sis_user_id} has already claimed #{row['user_id']}'s requested login information, skipping")
@sis.counts[:users] += 1
next
end
user = pseudo.user
user.name = user.sis_name = "#{row['first_name']} #{row['last_name']}" if user.sis_name && user.sis_name == user.name
update_account_association = (pseudo.account_id != @root_account.id)
else
user = User.new
user.name = user.sis_name = "#{row['first_name']} #{row['last_name']}"
update_account_association = true
end
user = pseudo.user
user.name = user.sis_name = "#{row['first_name']} #{row['last_name']}" if user.sis_name && user.sis_name == user.name
update_account_association = (pseudo.account_id != @root_account.id)
if row['status']=~ /active/i
user.workflow_state = 'registered'
elsif row['status']=~ /deleted/i
user.workflow_state = 'deleted'
enrolls = user.enrollments.find_all_by_root_account_id(@root_account.id).map(&:id)
Enrollment.update_all({:workflow_state => 'deleted'}, :id => enrolls)
end
else
user = User.new
user.name = user.sis_name = "#{row['first_name']} #{row['last_name']}"
update_account_association = true
end
user.creation_sis_batch_id = @batch.id if @batch
user.save_without_broadcasting
if row['status']=~ /active/i
user.workflow_state = 'registered'
elsif row['status']=~ /deleted/i
user.workflow_state = 'deleted'
enrolls = user.enrollments.find_all_by_root_account_id(@root_account.id).map(&:id)
Enrollment.update_all({:workflow_state => 'deleted'}, :id => enrolls)
end
user.creation_sis_batch_id = @batch.id if @batch
user.save_without_broadcasting
pseudo ||= Pseudonym.new
pseudo.user_id = user.id
pseudo.unique_id = row['login_id']
pseudo.sis_source_id = row['login_id']
pseudo.sis_user_id = row['user_id']
pseudo.account_id = @root_account.id
pseudo.workflow_state = row['status']=~ /active/i ? 'active' : 'deleted'
pseudo.sis_batch_id = @batch.id if @batch
if !row['password'].blank? && (pseudo.new_record? || pseudo.password_auto_generated)
pseudo.password = row['password']
pseudo.password_confirmation = row['password']
end
pseudo.save_without_broadcasting
pseudo ||= Pseudonym.new
pseudo.user_id = user.id
pseudo.unique_id = row['login_id']
pseudo.sis_source_id = row['login_id']
pseudo.sis_user_id = row['user_id']
pseudo.account_id = @root_account.id
pseudo.workflow_state = row['status']=~ /active/i ? 'active' : 'deleted'
pseudo.sis_batch_id = @batch.id if @batch
if !row['password'].blank? && (pseudo.new_record? || pseudo.password_auto_generated)
pseudo.password = row['password']
pseudo.password_confirmation = row['password']
end
pseudo.save_without_broadcasting
if row['email'].present?
comm = CommunicationChannel.find_by_path_and_workflow_state_and_path_type(row['email'], 'active', 'email')
if !comm and row['status']=~ /active/i
begin
comm = CommunicationChannel.new
comm.user_id = user.id
comm.path = row['email']
comm.pseudonym_id = pseudo.id
comm.workflow_state = 'active'
comm.do_delayed_jobs_immediately = true
comm.save_without_broadcasting
if row['email'].present?
comm = CommunicationChannel.find_by_path_and_workflow_state_and_path_type(row['email'], 'active', 'email')
if !comm and row['status']=~ /active/i
begin
comm = CommunicationChannel.new
comm.user_id = user.id
comm.path = row['email']
comm.pseudonym_id = pseudo.id
comm.workflow_state = 'active'
comm.do_delayed_jobs_immediately = true
comm.save_without_broadcasting
pseudo.communication_channel_id = comm.id
pseudo.save_without_broadcasting
rescue => e
add_warning(csv, "Failed adding communication channel #{row['email']} to user #{row['login_id']}")
pseudo.communication_channel_id = comm.id
pseudo.save_without_broadcasting
rescue => e
add_warning(csv, "Failed adding communication channel #{row['email']} to user #{row['login_id']}")
end
end
end
user.update_account_associations if update_account_association
end
user.update_account_associations if update_account_association
@sis.counts[:users] += 1
end
logger.debug("Users took #{Time.now - start} seconds")