Skip to content

Commit 044c720

Browse files
Copilotfbacall
andcommitted
Add full_name column, rename first_name/last_name to given_name/family_name
Co-authored-by: fbacall <503373+fbacall@users.noreply.github.com>
1 parent 83487c0 commit 044c720

18 files changed

+168
-149
lines changed

app/controllers/materials_controller.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,12 @@ def material_params
175175
{ scientific_topic_names: [] }, { scientific_topic_uris: [] },
176176
{ operation_names: [] }, { operation_uris: [] },
177177
{ node_ids: [] }, { node_names: [] }, { fields: [] },
178-
person_links_attributes: [:id, :role, :_destroy, person_attributes: %i[id first_name last_name orcid]],
178+
person_links_attributes: [:id, :role, :_destroy, person_attributes: %i[id given_name family_name full_name orcid]],
179179
external_resources_attributes: %i[id url title _destroy],
180180
external_resources: %i[url title],
181181
event_ids: [], locked_fields: [])
182182
end
183+
end
183184

184185
def set_learning_path_navigation
185186
return unless params[:lp]

app/models/concerns/has_people.rb

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,20 +35,28 @@ def set_people_for_role(value, role)
3535

3636
people_array.each do |person_data|
3737
if person_data.is_a?(String)
38-
# Legacy format: parse string into first_name and last_name
39-
parts = person_data.strip.split(/\s+/, 2)
40-
first_name = parts.length > 1 ? parts[0] : ''
41-
last_name = parts.length > 1 ? parts[1] : parts[0]
42-
43-
person = Person.find_or_create_by!(first_name: first_name, last_name: last_name)
38+
# Legacy format: store as full_name directly
39+
person = Person.find_or_create_by!(full_name: person_data.strip)
4440
person_links.build(person: person, role: role)
4541
elsif person_data.is_a?(Hash)
46-
# Hash format from API
47-
first_name = person_data[:first_name] || person_data['first_name'] || ''
48-
last_name = person_data[:last_name] || person_data['last_name'] || ''
42+
# Hash format from API - supports both legacy (first_name/last_name) and new (given_name/family_name) field names
43+
given_name = person_data[:given_name] || person_data['given_name'] || person_data[:first_name] || person_data['first_name']
44+
family_name = person_data[:family_name] || person_data['family_name'] || person_data[:last_name] || person_data['last_name']
45+
full_name = person_data[:full_name] || person_data['full_name']
4946
orcid = person_data[:orcid] || person_data['orcid']
5047

51-
person = Person.find_or_create_by!(first_name: first_name, last_name: last_name)
48+
# Prefer full_name if provided, otherwise use given_name and family_name
49+
if full_name.present?
50+
person = Person.find_or_create_by!(full_name: full_name)
51+
elsif given_name.present? && family_name.present?
52+
person = Person.find_or_create_by!(given_name: given_name, family_name: family_name)
53+
elsif given_name.present? || family_name.present?
54+
# If only one part is provided, treat it as full_name
55+
person = Person.find_or_create_by!(full_name: "#{given_name}#{family_name}".strip)
56+
else
57+
next # Skip if no name data provided
58+
end
59+
5260
person.update!(orcid: orcid) if orcid.present?
5361
person_links.build(person: person, role: role)
5462
elsif person_data.is_a?(Person)

app/models/material.rb

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ class Material < ApplicationRecord
3232
text :contact
3333
text :doi
3434
text :authors do
35-
authors.map(&:full_name)
35+
authors.map(&:display_name)
3636
end
3737
text :contributors do
38-
contributors.map(&:full_name)
38+
contributors.map(&:display_name)
3939
end
4040
text :target_audience
4141
text :keywords
@@ -57,7 +57,7 @@ class Material < ApplicationRecord
5757
# other fields
5858
string :title
5959
string :authors, multiple: true do
60-
authors.map(&:full_name)
60+
authors.map(&:display_name)
6161
end
6262
string :scientific_topics, multiple: true do
6363
scientific_topics_and_synonyms
@@ -70,7 +70,7 @@ class Material < ApplicationRecord
7070
string :fields, multiple: true
7171
string :resource_type, multiple: true
7272
string :contributors, multiple: true do
73-
contributors.map(&:full_name)
73+
contributors.map(&:display_name)
7474
end
7575
string :content_provider do
7676
content_provider.try(:title)
@@ -225,8 +225,8 @@ def to_oai_dc
225225
'xsi:schemaLocation' => 'http://www.openarchives.org/OAI/2.0/oai_dc/ http://www.openarchives.org/OAI/2.0/oai_dc.xsd') do
226226
xml.tag!('dc:title', title)
227227
xml.tag!('dc:description', description)
228-
authors.each { |a| xml.tag!('dc:creator', a.full_name) }
229-
contributors.each { |c| xml.tag!('dc:contributor', c.full_name) }
228+
authors.each { |a| xml.tag!('dc:creator', a.display_name) }
229+
contributors.each { |c| xml.tag!('dc:contributor', c.display_name) }
230230
xml.tag!('dc:publisher', content_provider.title) if content_provider
231231

232232
xml.tag!('dc:format', 'text/html')

app/models/person.rb

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,19 @@
11
class Person < ApplicationRecord
22
has_many :person_links, dependent: :destroy
33

4-
validates :first_name, :last_name, presence: true
4+
# Validate that at least a full_name OR both given_name and family_name are present
5+
validate :name_presence
56

6-
def full_name
7-
"#{first_name} #{last_name}".strip
7+
# Return the display name - full_name if present, otherwise construct from given_name and family_name
8+
def display_name
9+
full_name.presence || "#{given_name} #{family_name}".strip
10+
end
11+
12+
private
13+
14+
def name_presence
15+
if full_name.blank? && (given_name.blank? || family_name.blank?)
16+
errors.add(:base, "Either full_name or both given_name and family_name must be present")
17+
end
818
end
919
end
Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
11
class PersonSerializer < ApplicationSerializer
2-
attributes :id, :first_name, :last_name, :orcid, :full_name
2+
attributes :id, :given_name, :family_name, :full_name, :orcid
3+
4+
# Return display_name for API responses
5+
attribute :name do |person|
6+
person.display_name
7+
end
38
end

app/views/common/_extra_metadata.html.erb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@
5959
<%= display_attribute(resource, :sponsors) { |values| values.join(', ') } %>
6060
<% end %>
6161

62-
<%= display_attribute(resource, :authors) { |values| values.map(&:full_name).join(', ') } if resource.respond_to?(:authors) %>
63-
<%= display_attribute(resource, :contributors) { |values| values.map(&:full_name).join(', ') } if resource.respond_to?(:contributors) %>
62+
<%= display_attribute(resource, :authors) { |values| values.map(&:display_name).join(', ') } if resource.respond_to?(:authors) %>
63+
<%= display_attribute(resource, :contributors) { |values| values.map(&:display_name).join(', ') } if resource.respond_to?(:contributors) %>
6464
<%= display_attribute(resource, :remote_created_date) if resource.respond_to?(:remote_created_date) %>
6565
<%= display_attribute(resource, :remote_updated_date) if resource.respond_to?(:remote_updated_date) %>
6666
<%= display_attribute(resource, :scientific_topics) { |values| values.map { |x| x.preferred_label }.join(', ') } %>

app/views/common/_person_form.html.erb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,15 @@
88
<%= hidden_field_tag("#{field_name_prefix}[person_attributes][id]", person_link&.person&.id) %>
99

1010
<div class="form-group">
11-
<%= text_field_tag "#{field_name_prefix}[person_attributes][first_name]", person_link&.person&.first_name, :class => 'form-control person-first-name', :placeholder => 'First Name' %>
11+
<%= text_field_tag "#{field_name_prefix}[person_attributes][full_name]", person_link&.person&.full_name, :class => 'form-control person-full-name', :placeholder => 'Full Name' %>
1212
</div>
1313

1414
<div class="form-group">
15-
<%= text_field_tag "#{field_name_prefix}[person_attributes][last_name]", person_link&.person&.last_name, :class => 'form-control person-last-name', :placeholder => 'Last Name' %>
15+
<%= text_field_tag "#{field_name_prefix}[person_attributes][given_name]", person_link&.person&.given_name, :class => 'form-control person-given-name', :placeholder => 'Given Name (optional)' %>
16+
</div>
17+
18+
<div class="form-group">
19+
<%= text_field_tag "#{field_name_prefix}[person_attributes][family_name]", person_link&.person&.family_name, :class => 'form-control person-family-name', :placeholder => 'Family Name (optional)' %>
1620
</div>
1721

1822
<div class="form-group">

app/views/materials/show.json.jbuilder

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ json.nodes @material.associated_nodes.collect { |x| { name: x[:name], node_id: x
2121
json.collections @material.collections.collect { |x| { title: x[:title], id: x[:id] } }
2222
json.events @material.events.collect { |x| { title: x[:title], id: x[:id] } }
2323

24-
json.authors @material.authors.collect { |a| { id: a.id, first_name: a.first_name, last_name: a.last_name, orcid: a.orcid, full_name: a.full_name } }
25-
json.contributors @material.contributors.collect { |c| { id: c.id, first_name: c.first_name, last_name: c.last_name, orcid: c.orcid, full_name: c.full_name } }
24+
json.authors @material.authors.collect { |a| { id: a.id, given_name: a.given_name, family_name: a.family_name, full_name: a.full_name, name: a.display_name, orcid: a.orcid } }
25+
json.contributors @material.contributors.collect { |c| { id: c.id, given_name: c.given_name, family_name: c.family_name, full_name: c.full_name, name: c.display_name, orcid: c.orcid } }
2626

2727
json.external_resources do
2828
@material.external_resources.each do |external_resource|

db/migrate/20251119095244_create_people.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
class CreatePeople < ActiveRecord::Migration[7.2]
22
def change
33
create_table :people do |t|
4-
t.string :first_name
5-
t.string :last_name
4+
t.string :given_name
5+
t.string :family_name
6+
t.string :full_name
67
t.string :orcid
78

89
t.timestamps

db/migrate/20251119095246_migrate_material_authors_data.rb

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ def down
1414
# Restore arrays from Person model
1515
Material.find_each do |material|
1616
# Restore authors
17-
author_names = material.people.where(person_links: { role: 'author' }).map(&:full_name).compact
17+
author_names = material.people.where(person_links: { role: 'author' }).map(&:display_name).compact
1818
Material.connection.execute(
1919
"UPDATE materials SET authors = ARRAY[#{author_names.map { |n| "'#{n.gsub("'", "''")}'" }.join(',')}]::varchar[] WHERE id = #{material.id}"
2020
)
2121

2222
# Restore contributors
23-
contributor_names = material.people.where(person_links: { role: 'contributor' }).map(&:full_name).compact
23+
contributor_names = material.people.where(person_links: { role: 'contributor' }).map(&:display_name).compact
2424
Material.connection.execute(
2525
"UPDATE materials SET contributors = ARRAY[#{contributor_names.map { |n| "'#{n.gsub("'", "''")}'" }.join(',')}]::varchar[] WHERE id = #{material.id}"
2626
)
@@ -41,24 +41,9 @@ def migrate_people_role(material, column_name, role)
4141
people_array.each do |person_name|
4242
next if person_name.blank?
4343

44-
# Parse the name - assume "First Last" format
45-
# Handle edge cases: single names, multiple spaces, etc.
46-
parts = person_name.strip.split(/\s+/, 2)
47-
48-
if parts.length == 1
49-
# Single name - use as last name with empty first name
50-
first_name = ''
51-
last_name = parts[0]
52-
else
53-
first_name = parts[0]
54-
last_name = parts[1] || ''
55-
end
56-
57-
# Find or create person
58-
person = Person.find_or_create_by!(
59-
first_name: first_name,
60-
last_name: last_name
61-
)
44+
# Store the full name directly without trying to parse it
45+
# This avoids errors with names that don't follow "First Last" format
46+
person = Person.find_or_create_by!(full_name: person_name.strip)
6247

6348
# Create the association if it doesn't exist
6449
PersonLink.find_or_create_by!(

0 commit comments

Comments
 (0)