clear whitespace on sis csv imports

Also clear old whitespace that was missed before this fix

closes #5448

Change-Id: I6096685223c43bfec9fd0c669c737af422687e10
Reviewed-on: https://gerrit.instructure.com/5354
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: JT Olds <jt@instructure.com>
This commit is contained in:
Bracken Mosbacker 2011-08-30 16:48:20 -06:00
parent 9712b2d13b
commit 795d10776f
15 changed files with 75 additions and 24 deletions

View File

@ -0,0 +1,24 @@
class ClearSisWhitespace < ActiveRecord::Migration
def self.clear(table, *cols)
cols = cols.map{|col|" #{col.to_s} = TRIM(#{col.to_s})"}.join(',')
execute("UPDATE #{table.to_s} SET #{cols}")
end
def self.up
clear(:pseudonyms, :unique_id, :sis_source_id, :sis_user_id)
clear(:users, :name, :sis_name)
clear(:enrollment_terms, :name, :sis_name, :sis_source_id)
clear(:course_sections, :name, :sis_name, :sis_source_id)
clear(:groups, :name, :sis_name, :sis_source_id)
clear(:courses, :name, :sis_name, :sis_source_id, :course_code, :sis_course_code)
clear(:abstract_courses, :name, :sis_name, :sis_source_id, :short_name, :sis_course_code)
clear(:course_sections, :name, :sis_name, :sis_source_id)
clear(:enrollments, :sis_source_id)
clear(:accounts, :name, :sis_name, :sis_source_id)
end
def self.down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -27,7 +27,7 @@ module SIS
def verify(csv, verify)
abstract_course_ids = (verify[:abstract_course_ids] ||= {})
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
abstract_course_id = row['abstract_course_id']
add_error(csv, "Duplicate abstract course id #{abstract_course_id}") if abstract_course_ids[abstract_course_id]
abstract_course_ids[abstract_course_id] = true
@ -43,7 +43,7 @@ module SIS
def process(csv)
start = Time.now
abstract_courses_to_update_sis_batch_id = []
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
update_progress
logger.debug("Processing AbstractCourse #{row.inspect}")

View File

@ -27,7 +27,7 @@ module SIS
def verify(csv, verify)
account_ids = (verify[:account_ids] ||= {})
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
account_id = row['account_id']
add_error(csv, "Duplicate account id #{account_id}") if account_ids[account_id]
account_ids[account_id] = true
@ -41,7 +41,7 @@ module SIS
start = Time.now
accounts_cache = {}
Account.skip_callback(:update_account_associations_if_changed) do
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
update_progress
logger.debug("Processing Account #{row.inspect}")

View File

@ -27,7 +27,7 @@ module SIS
def verify(csv, verify)
course_ids = (verify[:course_ids] ||= {})
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
course_id = row['course_id']
add_error(csv, "Duplicate course id #{course_id}") if course_ids[course_id]
course_ids[course_id] = true
@ -46,7 +46,7 @@ module SIS
course_ids_to_update_associations = [].to_set
Course.skip_callback(:update_enrollments_later) do
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
update_progress
Course.skip_updating_account_associations do

View File

@ -27,7 +27,7 @@ module SIS
end
def verify(csv, verify)
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
add_error(csv, "No course_id or section_id given for an enrollment") if row['course_id'].blank? && row['section_id'].blank?
add_error(csv, "No user_id given for an enrollment") if row['user_id'].blank?
add_error(csv, "Improper role \"#{row['role']}\" for an enrollment") unless row['role'] =~ /\Astudent|\Ateacher|\Ata|\Aobserver|\Adesigner/i
@ -49,7 +49,7 @@ module SIS
Enrollment.skip_callback(:belongs_to_touch_after_save_or_destroy_for_course) do
User.skip_updating_account_associations do
FasterCSV.open(csv[:fullpath], "rb", :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |csv_object|
FasterCSV.open(csv[:fullpath], "rb", PARSE_ARGS) do |csv_object|
row = csv_object.shift
count = 0

View File

@ -25,7 +25,7 @@ module SIS
def verify(csv, verify)
enrollment_ids = (verify[:enrollment_ids] ||= {})
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
enrollment_id = row['enrollment_id']
add_error(csv, "Duplicate enrollment id #{enrollment_id}") if enrollment_ids[enrollment_id]
enrollment_ids[enrollment_id] = true
@ -39,7 +39,7 @@ module SIS
# enrollment_id,grade_publishing_status
def process(csv)
start = Time.now
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
update_progress
logger.debug("Processing Enrollment #{row.inspect}")

View File

@ -25,7 +25,7 @@ module SIS
def verify(csv, verify)
group_ids = (verify[:group_ids] ||= {})
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
group_id = row['group_id']
add_error(csv, "Duplicate group id #{group_id}") if group_ids[group_id]
group_ids[group_id] = true
@ -39,7 +39,7 @@ module SIS
start = Time.now
accounts_cache = {}
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
update_progress
logger.debug("Processing Group #{row.inspect}")

View File

@ -23,7 +23,7 @@ module SIS
end
def verify(csv, verify)
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
add_error(csv, "No group_id given for a group user") if row['group_id'].blank?
add_error(csv, "No user_id given for a group user") if row['user_id'].blank?
add_error(csv, "Improper status \"#{row['status']}\" for a group user") unless row['status'] =~ /\A(accepted|deleted)/i
@ -36,7 +36,7 @@ module SIS
start = Time.now
groups_cache = {}
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
update_progress
logger.debug("Processing Group User #{row.inspect}")

View File

@ -28,7 +28,7 @@ module SIS
def verify(csv, verify)
# section ids must be unique across the account
section_ids = (verify[:sections_id] ||= {})
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
section_id = row['section_id']
course_id = row['course_id']
add_error(csv, "Duplicate section id #{section_id}") if section_ids[section_id]
@ -47,7 +47,7 @@ module SIS
sections_to_update_sis_batch_ids = []
course_ids_to_update_associations = [].to_set
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
update_progress
Course.skip_updating_account_associations do

View File

@ -322,7 +322,7 @@ module SIS
Attachment.skip_scribd_submits
@csvs[importer].each do |csv|
remaining_in_batch = 0
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
FasterCSV.foreach(csv[:fullpath], SisImporter::PARSE_ARGS) do |row|
if remaining_in_batch == 0
temp_file += 1
if out_csv
@ -368,7 +368,7 @@ module SIS
def process_file(base, file)
csv = { :base => base, :file => file, :fullpath => File.join(base, file) }
if File.file?(csv[:fullpath]) && File.extname(csv[:fullpath]).downcase == '.csv'
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
FasterCSV.foreach(csv[:fullpath], SisImporter::PARSE_ARGS) do |row|
importer = IMPORTERS.index do |importer|
if SIS.const_get(importer.to_s.camelcase + 'Importer').send('is_' + importer.to_s + '_csv?', row)
@csvs[importer] << csv

View File

@ -18,6 +18,11 @@
module SIS
class SisImporter
PARSE_ARGS = {:headers => :first_row,
:skip_blanks => true,
:header_converters => :downcase,
:converters => lambda{|field|field ? field.strip : field}
}
def initialize(sis_csv)
@sis = sis_csv
@root_account = @sis.root_account
@ -47,5 +52,11 @@ module SIS
def update_progress(count = 1)
@sis.update_progress(count)
end
def csv_rows(csv)
FasterCSV.foreach(csv[:fullpath], PARSE_ARGS) do |row|
yield row
end
end
end
end

View File

@ -28,7 +28,7 @@ module SIS
def verify(csv, verify)
term_ids = (verify[:terms_id] ||= {})
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
term_id = row['term_id']
add_error(csv, "Duplicate term id #{term_id}") if term_ids[term_id]
term_ids[term_id] = true
@ -42,7 +42,7 @@ module SIS
# account_id,parent_account_id,name,status
def process(csv)
start = Time.now
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
update_progress
logger.debug("Processing Term #{row.inspect}")

View File

@ -26,7 +26,7 @@ module SIS
def verify(csv, verify)
user_ids = (verify[:user_ids] ||= {})
identical_row_checker = (verify[:user_rows] ||= {})
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
user_id = row['user_id']
if user_ids[user_id]
if identical_row_checker[user_id] != row
@ -53,7 +53,7 @@ module SIS
pseudos_to_set_sis_batch_ids = []
User.skip_updating_account_associations do
FasterCSV.open(csv[:fullpath], "rb", :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |csv_object|
FasterCSV.open(csv[:fullpath], "rb", PARSE_ARGS) do |csv_object|
row = csv_object.shift
count = 0
until row.nil?

View File

@ -26,7 +26,7 @@ module SIS
end
def verify(csv, verify)
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
add_error(csv, "No xlist_course_id given for a cross-listing") if row['xlist_course_id'].blank?
add_error(csv, "No section_id given for a cross-listing") if row['section_id'].blank?
add_error(csv, "Improper status \"#{row['status']}\" for a cross-listing") unless row['status'] =~ /\A(active|deleted)\z/i
@ -41,7 +41,7 @@ module SIS
course_ids_to_update_associations = [].to_set
Course.skip_callback(:update_enrollments_later) do
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
csv_rows(csv) do |row|
update_progress
logger.debug("Processing CrossListing #{row.inspect}")

View File

@ -818,6 +818,22 @@ describe SIS::SisCsv do
Pseudonym.count.should == (p_count + 2)
p.sis_user_id.should == "user_2"
end
it "should strip white space on fields" do
process_csv_data(
"user_id,login_id,first_name,last_name,email,status",
"user_1 ,user1 ,User ,Uno ,user@example.com ,active ",
" user_2, user2, User, Dos, user2@example.com, active"
)
user = User.find_by_email('user@example.com')
user.should_not be_nil
p = user.pseudonyms.first
p.unique_id.should == "user1"
user = User.find_by_email('user2@example.com')
user.should_not be_nil
p = user.pseudonyms.first
p.unique_id.should == "user2"
end
end