From 758eff07ada0bafac0b8b54847421c14cc269eaf Mon Sep 17 00:00:00 2001
From: James Van Mil <james.vanmil@gmail.com>
Date: Thu, 28 May 2015 13:51:02 -0400
Subject: [PATCH 1/4] Adds "Firstname, Lastname" support for Work creation.

This includes modifying Person object and behavior of associated
Profiles, Accounts, and Users to accomdate seperate Firstname and
Lastname fields.

The original Name field is retained for migration purposes.
---
 .../javascripts/curate/link_users.js.coffee   |  2 +-
 .../javascripts/curate/proxy_rights.js.coffee |  2 +-
 app/controllers/concerns/curate_controller.rb |  2 +-
 app/controllers/curate/people_controller.rb   |  2 +-
 .../generic_works_controller.rb               |  2 +-
 app/helpers/blacklight_helper.rb              |  5 ++++
 app/helpers/curate/collections_helper.rb      |  2 +-
 app/models/account.rb                         |  2 +-
 app/models/curate/user_behavior/base.rb       | 10 +++++++-
 .../person_metadata_datastream.rb             |  8 +++++++
 app/repository_models/person.rb               | 24 +++++++++----------
 app/views/curate/people/show.html.erb         |  2 +-
 .../_form_required_information.html.erb       |  3 +--
 .../_form_required_information.html.erb       |  3 +--
 .../_form_required_information.html.erb       |  3 +--
 .../_form_required_information.html.erb       |  3 +--
 .../registrations/_form_attributes.html.erb   |  3 ++-
 config/locales/sufia.en.yml                   |  2 +-
 ..._add_family_name_and_given_name_to_user.rb |  6 +++++
 .../curate/people_controller_spec.rb          | 12 +++++-----
 spec/curate/internal/factories.rb             |  3 ++-
 spec/features/generic_work_spec.rb            |  5 ++--
 .../features/manager_profile_workflow_spec.rb |  8 ++++---
 spec/features/person_profile_spec.rb          | 21 +++++++++++-----
 spec/features/user_profile_workflow_spec.rb   |  8 ++++---
 spec/models/account_spec.rb                   | 22 ++++++++---------
 .../shared/_site_actions.html.erb_spec.rb     |  4 ++--
 27 files changed, 103 insertions(+), 66 deletions(-)
 create mode 100644 db/migrate/20150423153556_add_family_name_and_given_name_to_user.rb

diff --git a/app/assets/javascripts/curate/link_users.js.coffee b/app/assets/javascripts/curate/link_users.js.coffee
index 9237c811..7bb3f7c4 100644
--- a/app/assets/javascripts/curate/link_users.js.coffee
+++ b/app/assets/javascripts/curate/link_users.js.coffee
@@ -104,7 +104,7 @@
           $.getJSON $targetElement.data('url'), { q: request.term + "*" }, ( data, status, xhr ) ->
             matches = []
             $.each data.response.docs, (idx, val) ->
-              matches.push {label: val['desc_metadata__name_tesim'][0], value: val['id']}
+              matches.push {label: (val['desc_metadata__first_name_tesim'] + " " + val['desc_metadata__last_name_tesim']), value: val['id']}
             response( matches )
         minLength: 2
         focus: ( event, ui ) ->
diff --git a/app/assets/javascripts/curate/proxy_rights.js.coffee b/app/assets/javascripts/curate/proxy_rights.js.coffee
index a880b7b5..a2ede137 100644
--- a/app/assets/javascripts/curate/proxy_rights.js.coffee
+++ b/app/assets/javascripts/curate/proxy_rights.js.coffee
@@ -73,7 +73,7 @@
           $.getJSON $targetElement.data('url'), { q: request.term, user: true}, ( data, status, xhr ) ->
             matches = []
             $.each data.response.docs, (idx, val) ->
-              matches.push {label: val['desc_metadata__name_tesim'][0], value: val['id']}
+              matches.push {label: (val['desc_metadata__first_name_tesim'] + " " + val['desc_metadata__last_name_tesim']), value: val['id']}
             response( matches )
         minLength: 2
         focus: ( event, ui ) ->
diff --git a/app/controllers/concerns/curate_controller.rb b/app/controllers/concerns/curate_controller.rb
index 6efac5aa..f9ff087b 100644
--- a/app/controllers/concerns/curate_controller.rb
+++ b/app/controllers/concerns/curate_controller.rb
@@ -85,7 +85,7 @@ def render_response_for_error(wrapper)
 
   def configure_permitted_parameters
     full_list = [:email, :password, :password_confirmation, :current_password,
-                 :name, :email, :alternate_email,
+                 :name, :last_name, :first_name, :email, :alternate_email,
                  :date_of_birth, :gender, :title,
                  :campus_phone_number, :alternate_phone_number,
                  :personal_webpage, :blog, :files]
diff --git a/app/controllers/curate/people_controller.rb b/app/controllers/curate/people_controller.rb
index 98daf660..99939c2e 100644
--- a/app/controllers/curate/people_controller.rb
+++ b/app/controllers/curate/people_controller.rb
@@ -18,7 +18,7 @@ def self.search_config
      initialized_config = Curate.configuration.search_config['people']
      # If the hash is empty, set reasonable defaults for this search type
      if initialized_config.nil?
-        Hash['qf' => 'desc_metadata__name_tesim','fl' => 'desc_metadata__name_tesim id','qt' => 'search','rows' => 10]
+        Hash['qf' => 'desc_metadata__first_name_tesim desc_metadata__last_name_tesim','fl' => 'desc_metadata__first_name_tesim desc_metadata__last_name_tesim id','qt' => 'search','rows' => 10]
      else
         initialized_config
      end
diff --git a/app/controllers/curation_concern/generic_works_controller.rb b/app/controllers/curation_concern/generic_works_controller.rb
index cc6d8950..f95f2ede 100644
--- a/app/controllers/curation_concern/generic_works_controller.rb
+++ b/app/controllers/curation_concern/generic_works_controller.rb
@@ -56,7 +56,7 @@ def after_create_response
   # Override setup_form in concrete controllers to get the form ready for display
   def setup_form
     if curation_concern.respond_to?(:creator)
-      curation_concern.creator << current_user.name if curation_concern.creator.empty? && !current_user.can_make_deposits_for.any?
+      curation_concern.creator << current_user.inverted_name if curation_concern.creator.empty? && !current_user.can_make_deposits_for.any?
     end
 
     curation_concern.editors << current_user.person if curation_concern.editors.blank?
diff --git a/app/helpers/blacklight_helper.rb b/app/helpers/blacklight_helper.rb
index fcb201e2..fb890804 100644
--- a/app/helpers/blacklight_helper.rb
+++ b/app/helpers/blacklight_helper.rb
@@ -16,4 +16,9 @@ module BlacklightHelper
   def application_name
     t('sufia.product_name')
   end
+
+  ## Override the default seperator used to display multivalue fields on the index view
+  def field_value_separator
+    tag(:br)
+  end
 end
diff --git a/app/helpers/curate/collections_helper.rb b/app/helpers/curate/collections_helper.rb
index d34dcb12..656c2702 100644
--- a/app/helpers/curate/collections_helper.rb
+++ b/app/helpers/curate/collections_helper.rb
@@ -121,7 +121,7 @@ def collection_line_item(collection, terminate, options={})
 
   def creators(work)
     if work.respond_to?(:creator)
-      "(#{work.creator.to_a.join(', ')})"
+      "(#{work.creator.to_a.join('; ')})"
     else
       ''
     end
diff --git a/app/models/account.rb b/app/models/account.rb
index 6adea5e7..d524edbe 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -215,7 +215,7 @@ def profile_title
   end
 
   def sync_profile_title_to_name(update_attributes)
-    return true unless update_attributes[:name]
+    return true unless update_attributes[:last_name] or update_attributes[:first_name]
     profile.update(title: profile_title)
   end
 
diff --git a/app/models/curate/user_behavior/base.rb b/app/models/curate/user_behavior/base.rb
index 7b25d35d..f351ef74 100644
--- a/app/models/curate/user_behavior/base.rb
+++ b/app/models/curate/user_behavior/base.rb
@@ -32,7 +32,15 @@ def manager_usernames
       end
 
       def name
-        read_attribute(:name) || user_key
+        name = "#{read_attribute(:first_name)} #{read_attribute(:last_name)}" 
+        return name unless name.blank?
+        user_key
+      end
+
+      def inverted_name
+        name = "#{read_attribute(:last_name)}, #{read_attribute(:first_name)}"
+        return name unless read_attribute(:last_name).blank? or read_attribute(:first_name).blank?
+        ""
       end
 
       def groups
diff --git a/app/repository_datastreams/person_metadata_datastream.rb b/app/repository_datastreams/person_metadata_datastream.rb
index 82cfe28c..3b8019f6 100644
--- a/app/repository_datastreams/person_metadata_datastream.rb
+++ b/app/repository_datastreams/person_metadata_datastream.rb
@@ -5,6 +5,14 @@ class PersonMetadataDatastream < ActiveFedora::NtriplesRDFDatastream
       index.as :stored_searchable
     end
 
+    map.first_name(to: "firstName", in: RDF::FOAF) do |index|
+      index.as :stored_searchable
+    end
+
+    map.last_name(to: "lastName", in: RDF::FOAF) do |index|
+      index.as :stored_searchable
+    end
+
     map.title(to: "title", in: RDF::FOAF) do |index|
       index.as :stored_searchable
     end
diff --git a/app/repository_models/person.rb b/app/repository_models/person.rb
index dbf6ff2e..8fb22eb5 100644
--- a/app/repository_models/person.rb
+++ b/app/repository_models/person.rb
@@ -23,6 +23,12 @@ class Person < ActiveFedora::Base
     datastream: :descMetadata, multiple: false,
     label: "Name"
 
+  attribute :first_name,
+    datastream: :descMetadata, multiple: false
+
+  attribute :last_name,
+    datastream: :descMetadata, multiple: false
+
   attribute :email,
     datastream: :descMetadata, multiple: false
 
@@ -53,6 +59,12 @@ class Person < ActiveFedora::Base
   attribute :gender,
     datastream: :descMetadata, multiple: false
 
+  def name
+    name = "#{self.first_name} #{self.last_name}"
+    return name unless name.blank? or self.first_name.blank? or self.last_name.blank?
+    user_key
+  end
+
   def validate_work(work)
     !work.is_a?(Person) && !work.is_a?(Collection) && work.is_a?(CurationConcern::Work)
   end
@@ -88,18 +100,6 @@ def date_uploaded
     Time.new(create_date).strftime("%Y-%m-%d")
   end
 
-  def first_name
-    name_parser.given
-  end
-
-  def last_name
-    name_parser.family
-  end
-
-  def name_parser
-    Namae.parse(self.name).first
-  end
-
   def user_key
     if user
       user.user_key
diff --git a/app/views/curate/people/show.html.erb b/app/views/curate/people/show.html.erb
index 88ee994b..801185a3 100644
--- a/app/views/curate/people/show.html.erb
+++ b/app/views/curate/people/show.html.erb
@@ -49,7 +49,7 @@
 
     <dl class="person-attribtues <%= dom_class(@person) %>">
 
-      <% @person.terms_for_display.reject{|attribute| (attribute == :name) || (attribute == :title)}.each do |attribute_name| %>
+      <% @person.terms_for_display.reject{|attribute| (attribute == :last_name) || (attribute == :first_name) || (attribute == :name) || (attribute == :title)}.each do |attribute_name| %>
         <% if @person.send(attribute_name).present? %>
           <dt class="attribute"><%= derived_label_for( @person, attribute_name) %>:</dt>
           <% [@person.send(attribute_name)].flatten.compact.each do |value| %>
diff --git a/app/views/curation_concern/articles/_form_required_information.html.erb b/app/views/curation_concern/articles/_form_required_information.html.erb
index a3f075b4..bea0ab29 100644
--- a/app/views/curation_concern/articles/_form_required_information.html.erb
+++ b/app/views/curation_concern/articles/_form_required_information.html.erb
@@ -22,8 +22,7 @@
           <%= f.input :creator, 
                       as: :multi_value, 
                       label: 'Author',
-                      required: true, 
-                      input_html: { value: "@current_user.name" } %>
+                      required: true %>
         </div>
         <%= render 'form_input_help_block', help_text: I18n.t('sufia.work.input_field.creator.help', work_type: curation_concern.human_readable_type) %>
       </div>
diff --git a/app/views/curation_concern/datasets/_form_required_information.html.erb b/app/views/curation_concern/datasets/_form_required_information.html.erb
index b1f9ddd7..ba27f2d6 100644
--- a/app/views/curation_concern/datasets/_form_required_information.html.erb
+++ b/app/views/curation_concern/datasets/_form_required_information.html.erb
@@ -21,8 +21,7 @@
         <div class="span6">
           <%= f.input :creator, 
                       as: :multi_value, 
-                      required: true, 
-                      input_html: { value: "@current_user.name" } %>
+                      required: true %>
         </div>
         <%= render 'form_input_help_block', help_text: I18n.t('sufia.work.input_field.creator.help', work_type: curation_concern.human_readable_type) %>
       </div>
diff --git a/app/views/curation_concern/documents/_form_required_information.html.erb b/app/views/curation_concern/documents/_form_required_information.html.erb
index 666be583..52e8b3c0 100644
--- a/app/views/curation_concern/documents/_form_required_information.html.erb
+++ b/app/views/curation_concern/documents/_form_required_information.html.erb
@@ -21,8 +21,7 @@
         <div class="span6">
           <%= f.input :creator, 
                       as: :multi_value, 
-                      required: true, 
-                      input_html: { value: "@current_user.name" } %>
+                      required: true %>
         </div>
         <%= render 'form_input_help_block', help_text: I18n.t('sufia.work.input_field.creator.help', work_type: curation_concern.human_readable_type) %>
       </div>
diff --git a/app/views/curation_concern/images/_form_required_information.html.erb b/app/views/curation_concern/images/_form_required_information.html.erb
index 666be583..52e8b3c0 100644
--- a/app/views/curation_concern/images/_form_required_information.html.erb
+++ b/app/views/curation_concern/images/_form_required_information.html.erb
@@ -21,8 +21,7 @@
         <div class="span6">
           <%= f.input :creator, 
                       as: :multi_value, 
-                      required: true, 
-                      input_html: { value: "@current_user.name" } %>
+                      required: true %>
         </div>
         <%= render 'form_input_help_block', help_text: I18n.t('sufia.work.input_field.creator.help', work_type: curation_concern.human_readable_type) %>
       </div>
diff --git a/app/views/registrations/_form_attributes.html.erb b/app/views/registrations/_form_attributes.html.erb
index 0fa59b43..26a0bf1b 100644
--- a/app/views/registrations/_form_attributes.html.erb
+++ b/app/views/registrations/_form_attributes.html.erb
@@ -3,7 +3,8 @@
   <fieldset class="span6">
     <legend>Personal Information</legend>
 
-    <%= f.input :name %>
+    <%= f.input :first_name, hint: 'Please include any middle names or initials here, if desired', required: true  %>
+    <%= f.input :last_name, required: true %>
     <%= f.input :title, label: 'Job Title' %>
     <%= f.input :personal_webpage %>
     <%= f.input :blog %>
diff --git a/config/locales/sufia.en.yml b/config/locales/sufia.en.yml
index 9e9c35a3..f92e2163 100644
--- a/config/locales/sufia.en.yml
+++ b/config/locales/sufia.en.yml
@@ -38,7 +38,7 @@ en:
         coverage_temporal:
           help:         "<p>Enter the period or date associated with the subject of your %{work_type}.</p><p>Examples:</p><li>19th century</li><li>Middle Ages</li><li>Jurassic Period</li>"
         creator:
-          help:         "<p>Enter the names of creators of the %{work_type}. This could include important authors, co-authors, or other significant contributors.</p>"
+          help:         "<p>Enter the names of creators of the %{work_type}, in <i>LastName, FirstName</i> format. These could include important authors, co-authors, or other significant contributors.</p>"
         cultural_context:
           help:         "<p>Enter the name of culture, people, or adjectival form of country associated with the subject of your %{work_type}.</p><p>Examples:</p><li>American</li><li>Vietnamese</li><li>Inuit</li>"
         date_created:
diff --git a/db/migrate/20150423153556_add_family_name_and_given_name_to_user.rb b/db/migrate/20150423153556_add_family_name_and_given_name_to_user.rb
new file mode 100644
index 00000000..f0a3a9e9
--- /dev/null
+++ b/db/migrate/20150423153556_add_family_name_and_given_name_to_user.rb
@@ -0,0 +1,6 @@
+class AddFamilyNameAndGivenNameToUser < ActiveRecord::Migration
+  def change
+    add_column User.table_name, :first_name, :string
+    add_column User.table_name, :last_name, :string
+  end
+end
diff --git a/spec/controllers/curate/people_controller_spec.rb b/spec/controllers/curate/people_controller_spec.rb
index 67610eca..93a5736c 100644
--- a/spec/controllers/curate/people_controller_spec.rb
+++ b/spec/controllers/curate/people_controller_spec.rb
@@ -26,28 +26,28 @@
 
   describe "searching via json" do
     before(:each) do
-      @katie = FactoryGirl.create(:person, name: 'Katie F. White-Kopp')
-      @alvin = FactoryGirl.create(:person, name: 'A. S. Mitchell')
-      @john = FactoryGirl.create(:person_with_user, name: 'John Corcoran III')
+      @katie = FactoryGirl.create(:person, first_name: 'Katie F.', last_name: 'White-Kopp')
+      @alvin = FactoryGirl.create(:person, first_name: 'A. S.', last_name: 'Mitchell')
+      @john = FactoryGirl.create(:person_with_user, first_name: 'John', last_name: 'Corcoran III')
     end
 
     it "should return results on full first name match" do
       get :index, q: 'Katie', format: :json
       json = JSON.parse(response.body)
-      json['response']['docs'].should == [{"id"=>@katie.pid, "desc_metadata__name_tesim"=>["Katie F. White-Kopp"]}]
+      json['response']['docs'].should == [{"id"=>@katie.pid, "desc_metadata__first_name_tesim"=>["Katie F."], "desc_metadata__last_name_tesim"=>["White-Kopp"]}]
     end
 
     it "should return results on full last name match" do
       get :index, q: 'Mitchell', format: :json
       json = JSON.parse(response.body)
-      json['response']['docs'].should == [{"id"=>@alvin.pid, "desc_metadata__name_tesim"=>["A. S. Mitchell"]}]
+      json['response']['docs'].should == [{"id"=>@alvin.pid, "desc_metadata__first_name_tesim"=>["A. S."], "desc_metadata__last_name_tesim"=>["Mitchell"]}]
     end
 
     describe "when constrained to users" do
       it "should return users" do
         get :index, q: '', user: true, format: :json
         json = JSON.parse(response.body)
-        json['response']['docs'].should == [{"id"=>@john.pid, "desc_metadata__name_tesim"=>["John Corcoran III"]}]
+        json['response']['docs'].should == [{"id"=>@john.pid, "desc_metadata__first_name_tesim"=>["John"], "desc_metadata__last_name_tesim"=>["Corcoran III"]}]
       end
     end
 
diff --git a/spec/curate/internal/factories.rb b/spec/curate/internal/factories.rb
index 73eb3846..33364b37 100644
--- a/spec/curate/internal/factories.rb
+++ b/spec/curate/internal/factories.rb
@@ -3,8 +3,9 @@
 FactoryGirl.define do
   factory :user do
     sequence(:email) {|n| "email-#{srand}@test.com" }
-    sequence(:name) {|n| "User Named #{n}" }
     waived_welcome_page true
+    sequence(:first_name) {|n| "First #{n}" }
+    sequence(:last_name) {|n| "Last #{n}" }
     user_does_not_require_profile_update true
     password 'a password'
     password_confirmation 'a password'
diff --git a/spec/features/generic_work_spec.rb b/spec/features/generic_work_spec.rb
index 7eae3a01..4ec3daa6 100644
--- a/spec/features/generic_work_spec.rb
+++ b/spec/features/generic_work_spec.rb
@@ -139,14 +139,13 @@
 end
 
 describe 'When I click on the link to create a work: ' do
-  let(:account) { FactoryGirl.create(:account, name: 'Iron Man') }
+  let(:account) { FactoryGirl.create(:account, first_name: 'Iron', last_name: 'Man') }
   let(:user) { account.user }
   let(:person) { account.person }
   before { login_as(user) }
   it 'should have my name set in the creator/contributor list' do
     visit new_curation_concern_generic_work_path
-    page.should have_css("a[href$='people/#{person.to_param}']")
-    page.should have_tag("a[href$='people/#{person.to_param}']", text: "Iron Man")
+    page.should have_tag("input", value: "Man, Iron")
   end
 end
 
diff --git a/spec/features/manager_profile_workflow_spec.rb b/spec/features/manager_profile_workflow_spec.rb
index e6378ae6..130ba4cb 100644
--- a/spec/features/manager_profile_workflow_spec.rb
+++ b/spec/features/manager_profile_workflow_spec.rb
@@ -14,7 +14,8 @@
 
       visit edit_user_path(user.id)
 
-      new_name = 'Frodo Baggins'
+      new_first_name = 'Frodo'
+      new_last_name = 'Baggins'
       new_pref = 'pref@example.com'
       new_alt = 'alt@example.com'
       new_title = 'student'
@@ -24,7 +25,8 @@
       new_blog = 'blog.example.com'
 
       within('form.edit_user') do
-        fill_in("user[name]", with: new_name)
+        fill_in("user[first_name]", with: new_first_name)
+        fill_in("user[last_name]", with: new_last_name)
         fill_in("user[email]", with: new_pref)
         fill_in("user[alternate_email]", with: new_alt)
         fill_in("user[title]", with: new_title)
@@ -43,7 +45,7 @@
       user.person.reload
 
       # Verify that everything got updated
-      user.name.should == new_name
+      user.name.should == "#{new_first_name} #{new_last_name}"
       user.email.should == new_pref
       user.alternate_email.should == new_alt
       user.title.should == new_title
diff --git a/spec/features/person_profile_spec.rb b/spec/features/person_profile_spec.rb
index 6fc5beb3..ec445ab0 100644
--- a/spec/features/person_profile_spec.rb
+++ b/spec/features/person_profile_spec.rb
@@ -4,7 +4,7 @@
 
   context 'logged in user' do
     let(:password) { FactoryGirl.attributes_for(:user).fetch(:password) }
-    let(:account) { FactoryGirl.create(:account, name: 'Iron Man') }
+    let(:account) { FactoryGirl.create(:account, first_name: 'Iron', last_name: "Man") }
     let(:user) { account.user }
     let(:person) { account.person }
     before { login_as(user) }
@@ -19,7 +19,8 @@
       visit catalog_index_path
       click_link 'My Profile'
       click_link 'Update Personal Information'
-      expect(page).to have_field('Name', with: 'Iron Man')
+      expect(page).to have_field('First name', with: 'Iron')
+      expect(page).to have_field('Last name', with: 'Man')
     end
 
     it 'should update their name and see the updated value' do
@@ -28,7 +29,8 @@
       click_link 'Update Personal Information'
       page.should have_link("Cancel", href: person_path(person))
       within('form.edit_user') do
-        fill_in("user[name]", with: 'Spider Man')
+        fill_in("user[first_name]", with: 'Spider')
+        fill_in("user[last_name]", with: 'Man')
         fill_in("user[current_password]", with: password)
         click_button "Update Account"
       end
@@ -69,14 +71,21 @@
         expect(page).to_not have_link('Marguerite Scypion') #title
       end
     end
+  end
+
+  context "searching" do
+    let!(:account) { FactoryGirl.create(:account, first_name: 'Bruce', last_name: 'Banner') }
+    let!(:user) { account.user }
+    let!(:person) { account.person }
+    before { login_as(user) }
     it 'with edit access is displayed in the results' do
       login_as(user)
       create_work
       visit catalog_index_path
-      fill_in 'Search Curate', with: 'Hulk'
+      fill_in 'Search Curate', with: 'Bruce'
       click_button 'keyword-search-submit'
       within('#documents') do
-        expect(page).to have_link('The Hulk') #title
+        expect(page).to have_link('Bruce Banner') #title
       end
     end
     it 'should not show repository managers in search results' do
@@ -113,7 +122,7 @@
 
   context 'A person when logged in' do
     let(:password) { FactoryGirl.attributes_for(:user).fetch(:password) }
-    let(:account) { FactoryGirl.create(:account, name: 'Iron Man') }
+    let(:account) { FactoryGirl.create(:account, first_name: 'Iron', last_name: 'Man') }
     let(:user) { account.user }
     let(:person) { account.person }
     let(:image_file){ Rails.root.join('../fixtures/files/image.png') }
diff --git a/spec/features/user_profile_workflow_spec.rb b/spec/features/user_profile_workflow_spec.rb
index bb374968..b450cad2 100644
--- a/spec/features/user_profile_workflow_spec.rb
+++ b/spec/features/user_profile_workflow_spec.rb
@@ -12,7 +12,8 @@
 
       visit edit_user_registration_path
 
-      new_name = 'Frodo Baggins'
+      new_first_name = 'Frodo'
+      new_last_name = 'Baggins'
       new_pref = 'pref@example.com'
       new_alt = 'alt@example.com'
       new_title = 'student'
@@ -23,7 +24,8 @@
 
       within('form.edit_user') do
         fill_in("user[current_password]", with: password)
-        fill_in("user[name]", with: new_name)
+        fill_in("user[first_name]", with: new_first_name)
+        fill_in("user[last_name]", with: new_last_name)
         fill_in("user[email]", with: new_pref)
         fill_in("user[alternate_email]", with: new_alt)
         fill_in("user[title]", with: new_title)
@@ -42,7 +44,7 @@
       user.person.reload
 
       # Verify that everything got updated
-      user.name.should == new_name
+      user.name.should == "#{new_first_name} #{new_last_name}"
       user.email.should == new_pref
       user.alternate_email.should == new_alt
       user.title.should == new_title
diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb
index 0593882c..f9c0d352 100644
--- a/spec/models/account_spec.rb
+++ b/spec/models/account_spec.rb
@@ -70,18 +70,19 @@
       end
 
       it 'if the person name changes, it keeps profile title in sync' do
-        new_name = 'Meriadoc Brandybuck'
-        subject.update_with_password(current_password: password, name: new_name)
-        person.name.should == new_name
+        new_first_name = 'Meriadoc'
+        new_last_name = 'Brandybuck'
+        subject.update_with_password(current_password: password, first_name: new_first_name, last_name: new_last_name)
         person.profile.reload
-        person.profile.title.should == new_name
+        person.profile.title.should == "#{new_first_name} #{new_last_name}"
       end
 
       it 'if the person name changes, it keeps the user.name in sync' do
-        new_name = 'Meriadoc Brandybuck'
-        subject.update_with_password(current_password: password, name: new_name)
-        person.name.should == new_name
-        person.user.name.should == new_name
+        new_first_name = 'Meriadoc'
+        new_last_name = 'Brandybuck'
+        subject.update_with_password(current_password: password, first_name: new_first_name, last_name: new_last_name)
+        person.name.should == "#{new_first_name} #{new_last_name}"
+        person.user.name.should == "#{new_first_name} #{new_last_name}"
       end
     end
 
@@ -100,7 +101,7 @@
 
   describe '#save via .new_with_session' do
     let(:expected_name) { 'Robert Frost' }
-    let(:attributes) { FactoryGirl.attributes_for(:user, name: expected_name) }
+    let(:attributes) { FactoryGirl.attributes_for(:user, first_name: 'Robert', last_name: 'Frost') }
     let(:session) { {} }
 
     subject { Account.new_with_session(attributes, session) }
@@ -155,8 +156,7 @@
     subject { Account.new(user) }
 
     it 'has a default title for the profile' do
-      subject.name.should == user.name
-      subject.profile.title.should == user.name
+      subject.profile.title.should == "Profile"
     end
   end
 
diff --git a/spec/views/shared/_site_actions.html.erb_spec.rb b/spec/views/shared/_site_actions.html.erb_spec.rb
index 5b069b40..e46727f4 100644
--- a/spec/views/shared/_site_actions.html.erb_spec.rb
+++ b/spec/views/shared/_site_actions.html.erb_spec.rb
@@ -21,8 +21,8 @@ def have_my_actions_section(&block)
 
   context 'logged in' do
     let(:person) { double(pid: 'test:1234') }
-    let(:name) { 'My Display Name' }
-    let(:current_user) { User.new(name: name, person: person).tap {|u| u.stub(groups: ['registered'])} }
+    let(:name) { 'Display Name' }
+    let(:current_user) { User.new(first_name: 'Display', last_name: 'Name', person: person).tap {|u| u.stub(groups: ['registered'])} }
     it 'renders a link to create a new user session' do
       expect(rendered).to_not have_login_section
       expect(rendered).to have_add_content_section do

From e0f7048782f6764c1bb3e70b4318c980ae16126e Mon Sep 17 00:00:00 2001
From: Glen Horton <hortongn@ucmail.uc.edu>
Date: Mon, 10 Aug 2015 16:06:08 -0400
Subject: [PATCH 2/4] Fix existing spec test for firstname/lastname change

---
 spec/features/person_profile_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/features/person_profile_spec.rb b/spec/features/person_profile_spec.rb
index ec445ab0..b59bdd18 100644
--- a/spec/features/person_profile_spec.rb
+++ b/spec/features/person_profile_spec.rb
@@ -103,7 +103,7 @@
     let(:email) { 'manager2@example.com' }
     let(:manager_user) { FactoryGirl.create(:user, email: email) }
 
-    let(:manager_2) {FactoryGirl.create(:account, email: 'manager@example.com', name: 'Stan Theman')}
+    let(:manager_2) {FactoryGirl.create(:account, email: 'manager@example.com', first_name: 'Stan', last_name: 'Theman')}
     let!(:user) {manager_2.user}
     let!(:person) {manager_2.person}
 

From 1c4a245a1031b5d5d375864e977b9b15f7c1f16c Mon Sep 17 00:00:00 2001
From: Thomas Scherz <thomas.scherz@uc.edu>
Date: Wed, 26 Aug 2015 00:02:37 -0400
Subject: [PATCH 3/4] Adds javascript:alert check for catalog parameters.

---
 app/controllers/catalog_controller.rb |  1 +
 app/helpers/params_helper.rb          | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/app/controllers/catalog_controller.rb b/app/controllers/catalog_controller.rb
index 1de04536..d6f2fef6 100644
--- a/app/controllers/catalog_controller.rb
+++ b/app/controllers/catalog_controller.rb
@@ -26,6 +26,7 @@ class CatalogController < ApplicationController
   CatalogController.solr_search_params_logic += [:hide_managers]
 
   before_filter :check_parameters?
+  before_filter :check_java_script_parameters?
 
   skip_before_filter :default_html_head
 
diff --git a/app/helpers/params_helper.rb b/app/helpers/params_helper.rb
index aa06f57d..219c2a74 100644
--- a/app/helpers/params_helper.rb
+++ b/app/helpers/params_helper.rb
@@ -100,6 +100,30 @@ def check_blind_sql_parameters_loop?()
     end
   end
 
+  def check_java_script_parameters?()
+    params.clone.each do |key, value|
+        if value.is_a?(Hash)
+          value.clone.each do |k,v|
+            unless defined?(v) == nil
+              if v.to_s.include?('javascript:alert')
+                render(:file => File.join(Rails.root, 'public/404.html'), :status => 404)
+                return false
+                break
+              end
+            end
+          end
+        else
+          unless defined?(value) == nil
+            if value.to_s.include?('javascript:alert')
+              render(:file => File.join(Rails.root, 'public/404.html'), :status => 404)
+              return false
+              break
+            end
+          end
+        end
+    end
+  end
+
   protected
 
     def limit_param_length(parameter, length_limit)

From 4b70cdacd7a812a9e17006473d167262f9450110 Mon Sep 17 00:00:00 2001
From: Glen Horton <hortongn@ucmail.uc.edu>
Date: Wed, 26 Aug 2015 10:09:51 -0400
Subject: [PATCH 4/4] Have 404 pages render without layout

---
 app/helpers/params_helper.rb | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/app/helpers/params_helper.rb b/app/helpers/params_helper.rb
index 219c2a74..8aa2c00b 100644
--- a/app/helpers/params_helper.rb
+++ b/app/helpers/params_helper.rb
@@ -40,13 +40,13 @@ def scrub_params(params)
 
   def check_parameters?(params_to_check=[:page, :per_page])
 
-    render(:file => File.join(Rails.root, 'public/404.html'), :status => 404) unless params[:page].to_i.to_s == params[:page] or params[:page].nil?
-    render(:file => File.join(Rails.root, 'public/404.html'), :status => 404) unless params[:page].to_i < 1000
-    render(:file => File.join(Rails.root, 'public/404.html'), :status => 404) if params[:page] && params[:page].to_i < 1
+    render(:file => 'public/404.html', :status => 404, :layout => false) unless params[:page].to_i.to_s == params[:page] or params[:page].nil?
+    render(:file => 'public/404.html', :status => 404, :layout => false) unless params[:page].to_i < 1000
+    render(:file => 'public/404.html', :status => 404, :layout => false) if params[:page] && params[:page].to_i < 1
 
-    render(:file => File.join(Rails.root, 'public/404.html'), :status => 404) unless params[:per_page].to_i.to_s == params[:per_page] or params[:per_page].nil?
-    render(:file => File.join(Rails.root, 'public/404.html'), :status => 404) unless params[:per_page].to_i < 1000
-    render(:file => File.join(Rails.root, 'public/404.html'), :status => 404) if params[:per_page] && params[:per_page].to_i < 1
+    render(:file => 'public/404.html', :status => 404, :layout => false) unless params[:per_page].to_i.to_s == params[:per_page] or params[:per_page].nil?
+    render(:file => 'public/404.html', :status => 404, :layout => false) unless params[:per_page].to_i < 1000
+    render(:file => 'public/404.html', :status => 404, :layout => false) if params[:per_page] && params[:per_page].to_i < 1
 
     limit_param_length(params[:q], 1000) unless defined?(params[:q]) == nil
     limit_param_length(params["f"]["desc_metadata__creator_sim"], 1000) unless defined?(params["f"]["desc_metadata__creator_sim"]) == nil
@@ -82,7 +82,7 @@ def check_blind_sql_parameters_loop?()
           value.clone.each do |k,v|
             unless defined?(v) == nil
               if v.to_s.include?('waitfor delay') || v.to_s.include?('DBMS_LOCK.SLEEP') || v.to_s.include?('SLEEP(5)') || v.to_s.include?('SLEEP(10)')
-                render(:file => File.join(Rails.root, 'public/404.html'), :status => 404)
+                render(:file => 'public/404.html', :status => 404, :layout => false)
                 return false
                 break
               end
@@ -91,7 +91,7 @@ def check_blind_sql_parameters_loop?()
         else
           unless defined?(value) == nil
             if value.to_s.include?('waitfor delay') || value.to_s.include?('DBMS_LOCK.SLEEP') || value.to_s.include?('SLEEP(5)') || value.to_s.include?('SLEEP(10)')
-              render(:file => File.join(Rails.root, 'public/404.html'), :status => 404)
+              render(:file => 'public/404.html', :status => 404, :layout => false)
               return false
               break
             end
@@ -106,7 +106,7 @@ def check_java_script_parameters?()
           value.clone.each do |k,v|
             unless defined?(v) == nil
               if v.to_s.include?('javascript:alert')
-                render(:file => File.join(Rails.root, 'public/404.html'), :status => 404)
+                render(:file => 'public/404.html', :status => 404, :layout => false)
                 return false
                 break
               end
@@ -115,7 +115,7 @@ def check_java_script_parameters?()
         else
           unless defined?(value) == nil
             if value.to_s.include?('javascript:alert')
-              render(:file => File.join(Rails.root, 'public/404.html'), :status => 404)
+              render(:file => 'public/404.html', :status => 404, :layout => false)
               return false
               break
             end
@@ -127,7 +127,7 @@ def check_java_script_parameters?()
   protected
 
     def limit_param_length(parameter, length_limit)
-      render(:file => File.join(Rails.root, 'public/404.html'), :status => 404) unless parameter.to_s.length < length_limit
+      render(:file => 'public/404.html', :status => 404, :layout => false) unless parameter.to_s.length < length_limit
     end
 
 end