diff --git a/doc/api/sis_csv.md b/doc/api/sis_csv.md index ae3968e3984..dd8e13f74f6 100644 --- a/doc/api/sis_csv.md +++ b/doc/api/sis_csv.md @@ -79,6 +79,11 @@ otherwise) it will not be overwritten

Last name of the user. +short_name +text +Display name of the user. + + email text The email address of the user. This might be the same as login_id, but should @@ -99,10 +104,10 @@ student to be able to log in but just not participate, leave the student Sample:
-user_id,login_id,password,first_name,last_name,email,status
-01103,bsmith01,,Bob,Smith,bob.smith@myschool.edu,active
-13834,jdoe03,,John,Doe,john.doe@myschool.edu,active
-13aa3,psue01,,Peggy,Sue,peggy.sue@myschool.edu,active
+user_id,login_id,password,first_name,last_name,short_name,email,status
+01103,bsmith01,,Bob,Smith,Bobby Smith,bob.smith@myschool.edu,active
+13834,jdoe03,,John,Doe,,john.doe@myschool.edu,active
+13aa3,psue01,,Peggy,Sue,,peggy.sue@myschool.edu,active
 
accounts.csv diff --git a/lib/sis/csv/user_importer.rb b/lib/sis/csv/user_importer.rb index 47a0775ad07..f8ba6d5b049 100644 --- a/lib/sis/csv/user_importer.rb +++ b/lib/sis/csv/user_importer.rb @@ -33,7 +33,7 @@ module SIS update_progress begin - importer.add_user(row['user_id'], row['login_id'], row['status'], row['first_name'], row['last_name'], row['email'], row['password'], row['ssha_password']) + importer.add_user(row['user_id'], row['login_id'], row['status'], row['first_name'], row['last_name'], row['email'], row['password'], row['ssha_password'], nil, row['short_name']) rescue ImportError => e messages << "#{e}" end diff --git a/lib/sis/user_importer.rb b/lib/sis/user_importer.rb index a7aebfee73b..d0b38a07690 100644 --- a/lib/sis/user_importer.rb +++ b/lib/sis/user_importer.rb @@ -62,14 +62,14 @@ module SIS @users_to_update_account_associations = [] end - def add_user(user_id, login_id, status, first_name, last_name, email=nil, password=nil, ssha_password=nil, integration_id=nil) - @logger.debug("Processing User #{[user_id, login_id, status, first_name, last_name, email, password, ssha_password, integration_id].inspect}") + def add_user(user_id, login_id, status, first_name, last_name, email=nil, password=nil, ssha_password=nil, integration_id=nil, short_name=nil) + @logger.debug("Processing User #{[user_id, login_id, status, first_name, last_name, email, password, ssha_password, integration_id, short_name].inspect}") raise ImportError, "No user_id given for a user" if user_id.blank? raise ImportError, "No login_id given for user #{user_id}" if login_id.blank? raise ImportError, "Improper status for user #{user_id}" unless status =~ /\A(active|deleted)/i - @batched_users << [user_id.to_s, login_id, status, first_name, last_name, email, password, ssha_password, integration_id] + @batched_users << [user_id.to_s, login_id, status, first_name, last_name, email, password, ssha_password, integration_id, short_name] process_batch if @batched_users.size >= @updates_every end @@ -86,7 +86,7 @@ module SIS while !@batched_users.empty? && tx_end_time > Time.now user_row = @batched_users.shift @logger.debug("Processing User #{user_row.inspect}") - user_id, login_id, status, first_name, last_name, email, password, ssha_password, integration_id = user_row + user_id, login_id, status, first_name, last_name, email, password, ssha_password, integration_id, short_name = user_row pseudo = @root_account.pseudonyms.find_by_sis_user_id(user_id.to_s) pseudo_by_login = @root_account.pseudonyms.active.by_unique_id(login_id).first @@ -110,10 +110,14 @@ module SIS unless user.stuck_sis_fields.include?(:sortable_name) user.sortable_name = last_name.present? && first_name.present? ? "#{last_name}, #{first_name}" : "#{first_name}#{last_name}" end + unless user.stuck_sis_fields.include?(:short_name) + user.short_name = short_name if short_name.present? + end else user = User.new user.name = "#{first_name} #{last_name}" user.sortable_name = last_name.present? && first_name.present? ? "#{last_name}, #{first_name}" : "#{first_name}#{last_name}" + user.short_name = short_name if short_name.present? end # we just leave all users registered now diff --git a/spec/lib/sis/csv/user_importer_spec.rb b/spec/lib/sis/csv/user_importer_spec.rb index 60cc1b0a481..9788521fbed 100644 --- a/spec/lib/sis/csv/user_importer_spec.rb +++ b/spec/lib/sis/csv/user_importer_spec.rb @@ -35,6 +35,7 @@ describe SIS::CSV::UserImporter do user = CommunicationChannel.find_by_path('user@example.com').user user.account.should eql(@account) user.name.should eql("User Uno") + user.short_name.should eql("User Uno") user.pseudonyms.count.should eql(1) pseudonym = user.pseudonyms.first @@ -51,6 +52,7 @@ describe SIS::CSV::UserImporter do user = CommunicationChannel.find_by_path('user@example.com').user user.account.should eql(@account) user.name.should eql("User Uno 2") + user.short_name.should eql("User Uno 2") user.pseudonyms.count.should eql(1) pseudonym = user.pseudonyms.first @@ -68,6 +70,44 @@ describe SIS::CSV::UserImporter do user = CommunicationChannel.find_by_path('user@example.com').user user.account.should eql(@account) user.name.should eql("My Awesome Name") + user.short_name.should eql("My Awesome Name") + end + + it "should create new users with display name" do + process_csv_data_cleanly( + "user_id,login_id,first_name,last_name,short_name,email,status", + "user_1,user1,User,Uno,The Uno,user@example.com,active" + ) + user = CommunicationChannel.find_by_path('user@example.com').user + user.account.should eql(@account) + user.name.should eql("User Uno") + user.short_name.should eql("The Uno") + + user.pseudonyms.count.should eql(1) + pseudonym = user.pseudonyms.first + pseudonym.unique_id.should eql('user1') + + user.communication_channels.count.should eql(1) + cc = user.communication_channels.first + cc.path.should eql("user@example.com") + + # Field order shouldn't matter + process_csv_data_cleanly( + "user_id,login_id,first_name,last_name,email,status,short_name", + "user_2,user2,User,Dos,user2@example.com,active,The Dos" + ) + user = CommunicationChannel.find_by_path('user2@example.com').user + user.account.should eql(@account) + user.name.should eql("User Dos") + user.short_name.should eql("The Dos") + + user.pseudonyms.count.should eql(1) + pseudonym = user.pseudonyms.first + pseudonym.unique_id.should eql('user2') + + user.communication_channels.count.should eql(1) + cc = user.communication_channels.first + cc.path.should eql("user2@example.com") end it "should preserve first name/last name split" do @@ -82,6 +122,22 @@ describe SIS::CSV::UserImporter do user.last_name.should == 'St. Clair' end + it "should tolerate blank first and last names" do + process_csv_data_cleanly( + "user_id,login_id,first_name,last_name,email,status", + "user_1,user1,,,user@example.com,active" + ) + user = CommunicationChannel.find_by_path('user@example.com').user + user.name.should eql(" ") + + process_csv_data_cleanly( + "user_id,login_id,email,status", + "user_2,user2,user2@example.com,active" + ) + user = CommunicationChannel.find_by_path('user2@example.com').user + user.name.should eql(" ") + end + it "should set passwords and not overwrite current passwords" do process_csv_data_cleanly( "user_id,login_id,password,first_name,last_name,email,status,ssha_password", @@ -320,6 +376,20 @@ describe SIS::CSV::UserImporter do Pseudonym.find_by_account_id_and_sis_user_id(@account.id, "user_1").user.last_name.should == "Uno-Dos" end + it "should allow a user to update display name specifically" do + process_csv_data_cleanly( + "user_id,login_id,first_name,last_name,short_name,email,status", + "user_1,user1,User,Uno,The Uno,user1@example.com,active" + ) + + process_csv_data_cleanly( + "user_id,login_id,first_name,last_name,short_name,email,status", + "user_1,user1,User,Uno,The Uno-Dos,user1@example.com,active" + ) + + Pseudonym.find_by_account_id_and_sis_user_id(@account.id, "user_1").user.short_name.should == "The Uno-Dos" + end + it "should allow a user to update emails specifically" do enable_cache do now = Time.now @@ -707,6 +777,29 @@ describe SIS::CSV::UserImporter do Pseudonym.find_by_unique_id('user5').should be_nil end + it "should handle display name stickiness" do + process_csv_data_cleanly( + "user_id,login_id,first_name,last_name,short_name,email,status", + "user_1,user1,User,Uno,The Uno,,active" + ) + user = Pseudonym.find_by_unique_id('user1').user + user.short_name = 'The Amazing Uno' + user.save! + process_csv_data_cleanly( + "user_id,login_id,first_name,last_name,short_name,email,status", + "user_1,user1,User,Uno,The Uno-Dos,,active" + ) + user.reload + user.short_name.should == 'The Amazing Uno' + process_csv_data_cleanly( + "user_id,login_id,first_name,last_name,short_name,email,status", + "user_1,user1,User,Uno,The Uno-Dos,,active", + {:override_sis_stickiness => true} + ) + user.reload + user.short_name.should == 'The Uno-Dos' + end + it 'should leave users around always' do process_csv_data_cleanly( "user_id,login_id,first_name,last_name,email,status",