Revert "add distinct name part fields refs #5317"

This reverts commit 995110f.

Change-Id: Ic00e7ced20ca9d912587e3440862ceb50e601d15
Reviewed-on: https://gerrit.instructure.com/6511
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>

Author: ccutrer

app/models/account.rb

@@ -298,7 +298,7 @@ def fast_all_users(limit=nil)
   end
 
   def paginate_users_not_in_groups(groups, page, per_page = 15)
-    User.paginate_by_sql(["SELECT u.id, u.name, u.title, u.given_name, u.surname, u.suffix
+    User.paginate_by_sql(["SELECT u.id, u.name 
                              FROM users u
                             INNER JOIN user_account_associations uaa on uaa.user_id = u.id
                             WHERE uaa.account_id = ? AND u.workflow_state != 'deleted'

app/models/course.rb

@@ -467,7 +467,7 @@ def self.default_name
   end
 
   def paginate_users_not_in_groups(groups, page, per_page = 15)
-    User.paginate_by_sql(["SELECT u.id, u.name, u.title, u.given_name, u.surname, u.suffix
+    User.paginate_by_sql(["SELECT u.id, u.name
                              FROM users u
                             INNER JOIN enrollments e ON e.user_id = u.id
                             WHERE e.course_id = ? AND e.workflow_state NOT IN ('rejected', 'completed', 'deleted') AND e.type = 'StudentEnrollment'

app/models/user.rb

@@ -111,7 +111,7 @@ class User < ActiveRecord::Base
 
 
   include StickySisFields
-  are_sis_sticky :name, :title, :given_name, :surname, :suffix
+  are_sis_sticky :name
 
   def conversations
     all_conversations.visible # i.e. exclude any where the user has deleted all the messages
@@ -436,10 +436,7 @@ def participants
   end
 
   def last_name_first
-    _title, _given, _surname, _suffix = title, given_name, surname, suffix
-    _title, _given, _surname, _suffix = User.name_parts(read_attribute(:name)) if !_title && !_given && !_surname && !_suffix
-    _given = [_title, _given, _suffix].compact.join(' ')
-    _surname ? "#{_surname}, #{_given}" : _given
+    User.last_name_first(self.name)
   end
 
   def last_name_first_or_unnamed
@@ -449,68 +446,28 @@ def last_name_first_or_unnamed
   end
 
   def first_name
-    return given_name if given_name
-    User.name_parts(read_attribute(:name))[1] || ''
+    (self.name || "").split(/\s/)[0]
   end
 
   def last_name
-    return surname if surname
-    User.name_parts(read_attribute(:name))[2] || ''
+    (self.name || "").split(/\s/)[1..-1].join(" ")
   end
-
-  # Feel free to add, but the "authoritative" list (http://en.wikipedia.org/wiki/Title_(name)) is quite large
-  SUFFIXES = /^(Sn?r\.?|Senior|Jn?r\.?|Junior|II|III|IV|V|VI|Esq\.?|Esquire)$/i
-  TITLES = /^(Mrs?\.?|Ms\.?|Miss|Dr\.?|Doctor)$/
-
-  def self.name_parts(name)
-    return [nil, nil, nil, nil] unless name
-    surname, given, suffix = name.strip.split(/\s*,\s*/, 3)
-
-    # Doe, John, Sr.
-    # Otherwise change Ho, Chi, Min to Ho, Chi Min
-    if suffix && !(suffix =~ SUFFIXES)
-      given = "#{given} #{suffix}"
-      suffix = nil
-    end
-
-    if given
-      # John Doe, Sr.
-      if !suffix && given =~ SUFFIXES
-        suffix = given
-        given = surname
-        surname = nil
+  
+  def self.last_name_first(name)
+    name_groups = []
+    if name
+      comma_separated = name.split(",")
+      comma_separated.each do |clump|
+        names = clump.split
+        if name.match(/\s/)
+          names = clump.split.map{|c| c.split(".").join(". ").split }.flatten
+        end
+        name = names.pop
+        name += ", " + names.join(" ") if !names.empty?
+        name_groups << name
       end
-    else
-      # John Doe
-      given = name.strip
-      surname = nil
-    end
-
-    given_parts = given.split
-    # John Doe Sr.
-    if !suffix && given_parts.length > 1 && given_parts.last =~ SUFFIXES
-      suffix = given_parts.pop
-    end
-    # Dr. John Doe
-    if given_parts.length > 1 && given_parts.first =~ TITLES
-      title = given_parts.shift
-    end
-    surname = given_parts.pop if !surname && given_parts.length > 1
-
-    [ title, given_parts.empty? ? nil : given_parts.join(' '), surname, suffix ]
-  end
-
-  def name
-    return (read_attribute(:name) || '') if !title && !given_name && !surname && !suffix
-    [title, given_name, surname, suffix].compact.join(' ')
-  end
-
-  def name=(name)
-    # name != self.name prevents "John St. Clair" blowing away [nil, "John", "St. Clair", nil]
-    if name != self.name
-      self.title, self.given_name, self.surname, self.suffix = User.name_parts(name)
-      write_attribute(:name, name)
     end
+    name_groups.join ", "
   end
 
   def self.user_lookup_cache_key(id)
@@ -525,8 +482,8 @@ def self.invalidate_cache(id)
 
   def infer_defaults
     self.name = nil if self.name == "User"
-    self.name = self.email || t(:default_user_name, "User") if self.name.blank?
-    [:short_name, :title, :given_name, :surname, :suffix].each { |attr| write_attribute(attr, nil) if read_attribute(attr) == '' }
+    self.name ||= self.email || t(:default_user_name, "User")
+    self.short_name = nil if self.short_name == ""
     self.short_name ||= self.name
     self.sortable_name = self.last_name_first.downcase
     self.reminder_time_for_due_dates ||= 48.hours.to_i
@@ -1846,7 +1803,7 @@ def group_membership_visibility
   end
   memoize :group_membership_visibility
 
-  MESSAGEABLE_USER_COLUMNS = ['id', 'short_name', 'name', 'title', 'given_name', 'surname', 'suffix', 'avatar_image_url', 'avatar_image_source'].map{|col|"users.#{col}"}
+  MESSAGEABLE_USER_COLUMNS = ['id', 'short_name', 'name', 'avatar_image_url', 'avatar_image_source'].map{|col|"users.#{col}"}
   MESSAGEABLE_USER_COLUMN_SQL = MESSAGEABLE_USER_COLUMNS.join(", ")
   MESSAGEABLE_USER_CONTEXT_REGEX = /\A(course|section|group)_(\d+)(_([a-z]+))?\z/
   def messageable_users(options = {})

app/models/wimba_conference.rb

@@ -118,16 +118,17 @@ def send_request(action, opts={})
   end
 
   def touch_user(user)
+    names = user.last_name_first.split(/, /, 2)
     send_request('modifyUser', {
       'target' => wimba_id(user.uuid),
       'password_type' => 'A',
-      'first_name' => user.first_name,
-      'last_name' => user.last_name}) ||
+      'first_name' => names[1],
+      'last_name' => names[0]}) ||
     send_request('createUser', {
       'target' => wimba_id(user.uuid),
       'password_type' => 'A',
-      'first_name' => user.first_name,
-      'last_name' => user.last_name})
+      'first_name' => names[1],
+      'last_name' => names[0]})
   end
 
   def add_user_to_conference(user, role=:participant)

db/migrate/20111019163904_add_name_details_to_users.rb

@@ -59,8 +59,9 @@ def self.generate(url, tool, user, context, link_code, return_url)
     hash['user_id'] = user.opaque_identifier(:asset_string)
     hash['roles'] = user.lti_role_types(context).join(',') # AccountAdmin, Student, Faculty or Observer
     if tool.include_name?
-      hash['lis_person_name_given'] = user.first_name.present? ? user.first_name : nil
-      hash['lis_person_name_family'] = user.last_name.present? ? user.last_name : nil
+      last, other = user.last_name_first.split(/,/, 2)
+      hash['lis_person_name_given'] = other
+      hash['lis_person_name_family'] = last
       hash['lis_person_name_full'] = user.name
     end
     if tool.include_email?

lib/basic_lti.rb

@@ -103,16 +103,11 @@ def process_batch
               end
 
               user = pseudo.user
-              if ([:name, :title, :given_name, :surname, :suffix] & user.stuck_sis_fields.to_a).empty?
-                # clears out name, title, and suffix
-                user.name = nil
-                user.given_name = first_name.blank? ? nil : first_name
-                user.surname = last_name.blank? ? nil : last_name
-              end
+              user.name = "#{first_name} #{last_name}" unless user.stuck_sis_fields.include?(:name)
+
             else
               user = User.new
-              user.given_name = first_name.blank? ? nil : first_name
-              user.surname = last_name.blank? ? nil : last_name
+              user.name = "#{first_name} #{last_name}"
             end
 
             if status =~ /active/i

lib/sis/user_importer.rb

@@ -167,8 +167,8 @@
       @tool.include_name?.should eql(true)
       @tool.include_email?.should eql(false)
       hash = BasicLTI.generate('http://www.yahoo.com', @tool, @user, @course, '123456', 'http://www.yahoo.com')
-      hash['lis_person_name_given'].should == 'User'
-      hash['lis_person_name_family'].should == nil
+      hash['lis_person_name_given'].should == nil
+      hash['lis_person_name_family'].should == 'User'
       hash['lis_person_name_full'].should == @user.name
       hash['lis_person_contact_email_primary'].should be_nil
     end
@@ -179,8 +179,8 @@
       @tool.include_name?.should eql(true)
       @tool.include_email?.should eql(true)
       hash = BasicLTI.generate('http://www.yahoo.com', @tool, @user, @course, '123456', 'http://www.yahoo.com')
-      hash['lis_person_name_given'].should == 'User'
-      hash['lis_person_name_family'].should == nil
+      hash['lis_person_name_given'].should == nil
+      hash['lis_person_name_family'].should == 'User'
       hash['lis_person_name_full'].should == @user.name
       hash['lis_person_contact_email_primary'] = @user.email
     end

spec/lib/basic_lti_spec.rb

@@ -975,18 +975,6 @@ def gen_ssha_password(password)
       user.name.should eql("My Awesome Name")
     end
 
-    it "should preserve first name/last name split" do
-      process_csv_data_cleanly(
-        "user_id,login_id,password,first_name,last_name,email,status,ssha_password",
-        "user_1,user1,badpassword,John,St. Clair,user@example.com,active,"
-      )
-      user = Pseudonym.find_by_unique_id('user1').user
-      user.first_name.should == 'John'
-      user.last_name.should == 'St. Clair'
-      user.name.should == 'John St. Clair'
-      user.last_name_first.should == 'St. Clair, John'
-    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",

spec/lib/sis_csv_importer_spec.rb

@@ -790,112 +790,4 @@ def set_up_course_with_users
     course_with_student
     @student.section_for_course(@course).should == @course.default_section
   end
-
-  context "names" do
-    it "should infer name parts" do
-      User.name_parts('Cody Cutrer').should == [nil, 'Cody', 'Cutrer', nil]
-      User.name_parts('  Cody  Cutrer   ').should == [nil, 'Cody', 'Cutrer', nil]
-      User.name_parts('Cutrer, Cody').should == [nil, 'Cody', 'Cutrer', nil]
-      User.name_parts('Cutrer, Cody Houston').should == [nil, 'Cody Houston', 'Cutrer', nil]
-      User.name_parts('St. Clair, John').should == [nil, 'John', 'St. Clair', nil]
-      # sorry, can't figure this out
-      User.name_parts('John St. Clair').should == [nil, 'John St.', 'Clair', nil]
-      User.name_parts('Jefferson Thomas Cutrer IV').should == [nil, 'Jefferson Thomas', 'Cutrer', 'IV']
-      User.name_parts('Jefferson Thomas Cutrer, IV').should == [nil, 'Jefferson Thomas', 'Cutrer', 'IV']
-      User.name_parts('Cutrer, Jefferson, IV').should == [nil, 'Jefferson', 'Cutrer', 'IV']
-      User.name_parts('Cutrer, Jefferson IV').should == [nil, 'Jefferson', 'Cutrer', 'IV']
-      User.name_parts(nil).should == [nil, nil, nil, nil]
-      User.name_parts('Bob').should == [nil, 'Bob', nil, nil]
-      User.name_parts('Doctor').should == [nil, 'Doctor', nil, nil]
-      # sorry, have to choose one or the other
-      User.name_parts('Doctor John').should == ['Doctor', 'John', nil, nil]
-      User.name_parts('Doctor Kevorkian').should == ['Doctor', 'Kevorkian', nil, nil]
-      User.name_parts('Ho, Chi, Min').should == [nil, 'Chi Min', 'Ho', nil]
-      # sorry, don't understand cultures that put the surname first
-      # (maybe pass in the user's locale when parsing?)
-      User.name_parts('Ho Chi Min').should == [nil, 'Ho Chi', 'Min', nil]
-      User.name_parts('Mr. Cody Cutrer').should == ['Mr.', 'Cody', 'Cutrer', nil]
-      User.name_parts('').should == [nil, nil, nil, nil]
-      User.name_parts('John Doe').should == [nil, 'John', 'Doe', nil]
-      User.name_parts('Junior').should == [nil, 'Junior', nil, nil]
-    end
-
-    it "should return properly formatted names" do
-      u = User.new
-      u.name.should == ''
-      u.first_name.should == ''
-      u.last_name.should == ''
-      u.last_name_first.should == ''
-
-      u.given_name = 'Cody'
-      u.name.should == 'Cody'
-      u.first_name.should == 'Cody'
-      u.last_name.should == ''
-      u.last_name_first.should == 'Cody'
-
-      u.surname = 'Cutrer'
-      u.name.should == 'Cody Cutrer'
-      u.first_name.should == 'Cody'
-      u.last_name.should == 'Cutrer'
-      u.last_name_first.should == 'Cutrer, Cody'
-
-      u.title = 'Mr.'
-      u.name.should == 'Mr. Cody Cutrer'
-      u.first_name.should == 'Cody'
-      u.last_name.should == 'Cutrer'
-      u.last_name_first.should == 'Cutrer, Mr. Cody'
-
-      u.suffix = 'Esquire'
-      u.name.should == 'Mr. Cody Cutrer Esquire'
-      u.first_name.should == 'Cody'
-      u.last_name.should == 'Cutrer'
-      u.last_name_first.should == 'Cutrer, Mr. Cody Esquire'
-    end
-
-    # name attribute is set, but others are not
-    # name is returned raw, other fields are returned as parsed by name_parts
-    it "should return properly formatted names (backcompat)" do
-      u = User.new
-      u.write_attribute(:name, '')
-      u.name.should == ''
-      u.first_name.should == ''
-      u.last_name.should == ''
-      u.last_name_first.should == ''
-
-      u.write_attribute(:name, 'Cody')
-      u.name.should == 'Cody'
-      u.first_name.should == 'Cody'
-      u.last_name.should == ''
-      u.last_name_first.should == 'Cody'
-
-      u.write_attribute(:name, 'Cody Cutrer')
-      u.name.should == 'Cody Cutrer'
-      u.first_name.should == 'Cody'
-      u.last_name.should == 'Cutrer'
-      u.last_name_first.should == 'Cutrer, Cody'
-
-      u.write_attribute(:name, 'Cutrer, Cody')
-      u.name.should == 'Cutrer, Cody'
-      u.first_name.should == 'Cody'
-      u.last_name.should == 'Cutrer'
-      u.last_name_first.should == 'Cutrer, Cody'
-
-      u.write_attribute(:name, 'Mr. Jefferson Thomas Cutrer IV')
-      u.name.should == 'Mr. Jefferson Thomas Cutrer IV'
-      u.first_name.should == 'Jefferson Thomas'
-      u.last_name.should == 'Cutrer'
-      u.last_name_first.should == 'Cutrer, Mr. Jefferson Thomas IV'
-    end
-
-    it "should not blow away name details when assigning the same name via backcompat method" do
-      u = User.new
-      u.name = "St. Clair, John"
-      u.name.should == "John St. Clair"
-      u.last_name_first.should == 'St. Clair, John'
-      u.name = "John St. Clair"
-      u.last_name_first.should == 'St. Clair, John'
-      # The original assignment is still preserved
-      u.read_attribute(:name).should == 'St. Clair, John'
-    end
-  end
 end