Skip to content

Commit 148e66a

Browse files
jtoliojtolio
committed
supporting designer/observer imports and make course_id optional
now one of course_id/section_id is required for enrollments, but not both Change-Id: I4ebd19ac05988bf3eeffcb846d8bc1fafce73b40 Reviewed-on: https://gerrit.instructure.com/2316 Tested-by: Hudson <hudson@instructure.com> Reviewed-by: Bracken Mosbacker <bracken@instructure.com> Reviewed-by: Brian Whitmer <brian@instructure.com>
1 parent e288d16 commit 148e66a

4 files changed

Lines changed: 70 additions & 19 deletions

File tree

‎app/models/course.rb‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ class Course < ActiveRecord::Base
4545
has_many :teacher_enrollments, :class_name => 'TeacherEnrollment', :conditions => ['enrollments.workflow_state != ?', 'deleted'], :include => :user
4646
has_many :tas, :through => :ta_enrollments, :source => :user
4747
has_many :ta_enrollments, :class_name => 'TaEnrollment', :conditions => ['enrollments.workflow_state != ?', 'deleted'], :include => :user
48+
has_many :designers, :through => :designer_enrollments, :source => :user
49+
has_many :designer_enrollments, :class_name => 'DesignerEnrollment', :conditions => ['enrollments.workflow_state != ?', 'deleted'], :include => :user
4850
has_many :observers, :through => :observer_enrollments, :source => :user
4951
has_many :observer_enrollments, :class_name => 'ObserverEnrollment', :conditions => ['enrollments.workflow_state != ?', 'deleted'], :include => :user
5052
has_many :admins, :through => :enrollments, :source => :user, :conditions => "enrollments.type = 'TaEnrollment' or enrollments.type = 'TeacherEnrollment'"

‎lib/sis/enrollment_importer.rb‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ def self.is_enrollment_csv?(row)
2525

2626
def verify(csv, verify)
2727
FasterCSV.foreach(csv[:fullpath], :headers => :first_row, :skip_blanks => true, :header_converters => :downcase) do |row|
28-
add_error(csv, "No course_id given for an enrollment") if row['course_id'].blank?
28+
add_error(csv, "No course_id or section_id given for an enrollment") if row['course_id'].blank? && row['section_id'].blank?
2929
add_error(csv, "No user_id given for an enrollment") if row['user_id'].blank?
30-
add_error(csv, "Improper role \"#{row['role']}\" for an enrollment") unless row['role'] =~ /\Astudent|\Ateacher|\Ata|\Aobserver/i
30+
add_error(csv, "Improper role \"#{row['role']}\" for an enrollment") unless row['role'] =~ /\Astudent|\Ateacher|\Ata|\Aobserver|\Adesigner/i
3131
add_error(csv, "Improper status \"#{row['status']}\" for an enrollment") unless row['status'] =~ /\Aactive|\Adeleted|\Acompleted/i
3232
end
3333
end
@@ -42,18 +42,35 @@ def process(csv)
4242
pseudo = Pseudonym.find_by_account_id_and_sis_user_id(@root_account.id, row['user_id'])
4343
user = pseudo.user rescue nil
4444
course = Course.find_by_root_account_id_and_sis_source_id(@root_account.id, row['course_id'])
45-
unless user && course
46-
add_warning csv, "Course #{row['course_id']} didn't exist for user enrollment" unless course
47-
add_warning csv, "User #{row['user_id']} didn't exist for user enrollment" unless user
45+
section = CourseSection.find_by_root_account_id_and_sis_source_id(@root_account.id, row['section_id'])
46+
unless user && (course || section)
47+
add_warning csv, "Neither course #{row['course_id']} nor section #{row['section_id']} existed for user enrollment" unless (course || section)
48+
add_warning csv, "User #{row['user_id']} didn't exist for user enrollment" unless user
4849
next
4950
end
5051

51-
section = course.course_sections.find_by_sis_source_id(row['section_id'])
52-
if !section && row['section_id']
52+
if row['section_id'] && !section
5353
add_warning csv, "An enrollment referenced a non-existent section #{row['section_id']}"
54+
next
55+
end
56+
57+
if row['course_id'] && !course
58+
add_warning csv, "An enrollment referenced a non-existent course #{row['course_id']}"
59+
next
60+
end
61+
62+
unless section
63+
section = course.course_sections.find_by_sis_source_id(row['section_id'])
64+
section ||= course.default_section
5465
end
55-
section ||= course.default_section
56-
66+
67+
course ||= section.course
68+
69+
if course != section.course
70+
add_warning csv, "An enrollment listed a section and a course that are unrelated"
71+
next
72+
end
73+
5774
enrollment = section.enrollments.find_by_user_id(user.id)
5875
unless enrollment
5976
enrollment = Enrollment.new
@@ -79,6 +96,8 @@ def process(csv)
7996
associated_enrollment = pseudo && course.student_enrollments.find_by_user_id(pseudo.user_id)
8097
enrollment.associated_user_id = associated_enrollment && associated_enrollment.user_id
8198
end
99+
elsif row['role'] =~ /\Adesigner\z/i
100+
enrollment.type = 'DesignerEnrollment'
82101
end
83102

84103
if row['status']=~ /active/i

‎public/resources/CanvasSISImport.pdf‎

1.33 KB
Binary file not shown.

‎spec/lib/sis_csv_importer_spec.rb‎

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -290,20 +290,45 @@ def process_csv_data(*lines)
290290
before_count = Enrollment.count
291291
importer = process_csv_data(
292292
"course_id,user_id,role,section_id,status",
293-
",U001,student,1B,active",
293+
",U001,student,,active",
294294
"C001,,student,1B,active",
295295
"C001,U001,cheater,1B,active",
296296
"C001,U001,student,1B,semi-active"
297297
)
298298
Enrollment.count.should == before_count
299299

300300
errors = importer.errors.map { |r| r.last }
301-
errors.should == ["No course_id given for an enrollment",
302-
"No user_id given for an enrollment",
303-
"Improper role \"cheater\" for an enrollment",
301+
errors.should == ["No course_id or section_id given for an enrollment",
302+
"No user_id given for an enrollment",
303+
"Improper role \"cheater\" for an enrollment",
304304
"Improper status \"semi-active\" for an enrollment"]
305305
end
306-
306+
307+
it 'should warn about inconsistent data' do
308+
process_csv_data(
309+
"course_id,short_name,long_name,account_id,term_id,status",
310+
"C001,TC 101,Test Course 101,,,active",
311+
"C002,TC 102,Test Course 102,,,active"
312+
)
313+
process_csv_data(
314+
"section_id,course_id,name,start_date,end_date,status",
315+
"1B,C001,Sec1,2011-1-05 00:00:00,2011-4-14 00:00:00,active"
316+
)
317+
process_csv_data(
318+
"user_id,login_id,first_name,last_name,email,status",
319+
"U001,user1,User,Uno,user@example.com,active"
320+
)
321+
importer = process_csv_data(
322+
"course_id,user_id,role,section_id,status",
323+
"NONEXISTENT,U001,student,1B,active",
324+
"C001,U001,student,NONEXISTENT,active",
325+
"C002,U001,student,1B,active")
326+
warnings = importer.warnings.map { |r| r.last }
327+
warnings.should == ["An enrollment referenced a non-existent course NONEXISTENT",
328+
"An enrollment referenced a non-existent section NONEXISTENT",
329+
"An enrollment listed a section and a course that are unrelated"]
330+
end
331+
307332
it "should enroll users" do
308333
#create course, users, and sections
309334
process_csv_data(
@@ -315,27 +340,32 @@ def process_csv_data(*lines)
315340
"user_1,user1,User,Uno,user@example.com,active",
316341
"user_2,user2,User,Dos,user2@example.com,active",
317342
"user_3,user4,User,Tres,user3@example.com,active",
318-
"user_5,user5,User,Quatro,user5@example.com,active"
343+
"user_5,user5,User,Quatro,user5@example.com,active",
344+
"user_6,user6,User,Cinco,user6@example.com,active"
319345
)
320346
process_csv_data(
321347
"section_id,course_id,name,status,start_date,end_date",
322348
"S001,test_1,Sec1,active,,"
323349
)
324350
# the enrollments
325351
process_csv_data(
326-
"course_id,user_id,role,section_name,status,associated_user_id",
327-
"test_1,user_1,teacher,S001,active,",
328-
"test_1,user_2,student,S001,active,",
352+
"course_id,user_id,role,section_id,status,associated_user_id",
353+
"test_1,user_1,teacher,,active,",
354+
",user_2,student,S001,active,",
329355
"test_1,user_3,ta,S001,active,",
330-
"test_1,user_5,observer,S001,active,user_2"
356+
"test_1,user_5,observer,S001,active,user_2",
357+
"test_1,user_6,designer,S001,active,"
331358
)
332359
course = @account.courses.find_by_sis_source_id("test_1")
333360
course.teachers.first.name.should == "User Uno"
334361
course.students.first.name.should == "User Dos"
335362
course.tas.first.name.should == "User Tres"
336363
course.observers.first.name.should == "User Quatro"
337364
course.observer_enrollments.first.associated_user_id.should == course.students.first.id
365+
course.designers.first.name.should == "User Cinco"
338366
end
367+
368+
339369
end
340370

341371
context 'account importing' do

0 commit comments

Comments
 (0)