Remove the indirection - resources manage their own Person objects*

Remove the data migration for now, needs to be safer/better.

* This saves having to centrally manage People and provide ways of correcting metadata etc.

Author: fbacall

app/controllers/materials_controller.rb

@@ -171,12 +171,12 @@ def material_params
                                      :content_provider_id, :difficulty_level, :version, :status,
                                      :date_created, :date_modified, :date_published, :other_types,
                                      :prerequisites, :syllabus, :visible, :learning_objectives, { subsets: [] },
-                                     { contributors: [] }, { target_audience: [] },
+                                     { authors: [] }, { contributors: [] }, { target_audience: [] },
                                      { collection_ids: [] }, { keywords: [] }, { resource_type: [] },
                                      { scientific_topic_names: [] }, { scientific_topic_uris: [] },
                                      { operation_names: [] }, { operation_uris: [] },
                                      { node_ids: [] }, { node_names: [] }, { fields: [] },
-                                     person_links_attributes: [:id, :role, :_destroy, person_attributes: %i[id given_name family_name full_name first_name last_name orcid]],
+                                     people_attributes: %i[id role _destroy given_name family_name full_name first_name last_name orcid],
                                      external_resources_attributes: %i[id url title _destroy],
                                      external_resources: %i[url title],
                                      event_ids: [], locked_fields: [])

app/models/concerns/has_people.rb

@@ -2,66 +2,40 @@ module HasPeople
   extend ActiveSupport::Concern
 
   included do
-    has_many :person_links, as: :resource, dependent: :destroy
-    has_many :people, through: :person_links
-    accepts_nested_attributes_for :person_links, allow_destroy: true, reject_if: :all_blank
+    has_many :people, as: :resource, dependent: :destroy, inverse_of: :resource
+    accepts_nested_attributes_for :people, allow_destroy: true, reject_if: :all_blank
   end
 
   class_methods do
     # Define a person role association (e.g., :authors, :contributors)
     # This creates the association and a custom setter that accepts strings, hashes, or Person objects
     def has_person_role(role_name, role_key: role_name.to_s.singularize)
       # Define the association
-      has_many role_name, -> { where(person_links: { role: role_key }) }, through: :person_links, source: :person
+      has_many role_name, -> { where(role: role_key) }, class_name: 'Person', as: :resource, inverse_of: :resource
 
       # Define custom setter that accepts strings (legacy), hashes, or Person objects
       define_method("#{role_name}=") do |value|
-        set_people_for_role(value, role_key)
+        super(set_people_for_role(value, role_key))
       end
     end
   end
 
   private
 
   # Set people for a specific role, accepting various input formats
-  def set_people_for_role(value, role)
-    return if value.nil?
-
-    # Convert to array if needed
-    people_array = Array(value).reject(&:blank?)
-
+  def set_people_for_role(value, role_key)
     # Remove existing links for this role
-    person_links.where(role: role).destroy_all
+    people.where(role: role_key).destroy_all
 
-    people_array.each do |person_data|
+    Array(value).reject(&:blank?).map do |person_data|
       if person_data.is_a?(String)
         # Legacy format: store as full_name directly
-        person = Person.find_or_create_by!(full_name: person_data.strip)
-        person_links.build(person: person, role: role)
+        people.build(full_name: person_data.strip, role: role_key)
       elsif person_data.is_a?(Hash)
-        # Hash format from API - supports both legacy (first_name/last_name) and new (given_name/family_name) field names
-        given_name = person_data[:given_name] || person_data['given_name'] || person_data[:first_name] || person_data['first_name']
-        family_name = person_data[:family_name] || person_data['family_name'] || person_data[:last_name] || person_data['last_name']
-        full_name = person_data[:full_name] || person_data['full_name']
-        orcid = person_data[:orcid] || person_data['orcid']
-
-        # Prefer full_name if provided, otherwise use given_name and family_name
-        if full_name.present?
-          person = Person.find_or_create_by!(full_name: full_name)
-        elsif given_name.present? && family_name.present?
-          person = Person.find_or_create_by!(given_name: given_name, family_name: family_name)
-        elsif given_name.present? || family_name.present?
-          # If only one part is provided, treat it as full_name
-          person = Person.find_or_create_by!(full_name: "#{given_name}#{family_name}".strip)
-        else
-          next # Skip if no name data provided
-        end
-
-        person.update!(orcid: orcid) if orcid.present?
-        person_links.build(person: person, role: role)
+        people.build(**person_data, role: role_key)
       elsif person_data.is_a?(Person)
-        # Person object
-        person_links.build(person: person_data, role: role)
+        person_data.role = role_key
+        person_data
       end
     end
   end

app/models/person.rb

@@ -2,7 +2,9 @@ class Person < ApplicationRecord
   include HasOrcid
 
   belongs_to :profile, optional: true
-  has_many :person_links, dependent: :destroy
+  belongs_to :resource, polymorphic: true
+
+  validates :resource, :role, presence: true
 
   # Validate that at least a full_name OR both given_name and family_name are present
   validate :name_presence

app/models/person_link.rb

@@ -1,26 +1,25 @@
 <% index ||= 'replace-me' %> <%# This is so we can render a blank version of this sub-form in the page, %>
                              <%# which can be dynamically cloned using JavaScript to add more People to the main form %>
-<% field_name_prefix = "#{form_name}[person_links_attributes][#{index}]" %> <%# This format is dictated by "accepts_nested_attributes_for" %>
+<% field_name_prefix = "#{form_name}[people_attributes][#{index}]" %> <%# This format is dictated by "accepts_nested_attributes_for" %>
 
 <div class="form-inline person-form" data-index="<%= index -%>" data-role="<%= role -%>">
-  <%= hidden_field_tag("#{field_name_prefix}[id]", person_link&.id) %>
+  <%= hidden_field_tag("#{field_name_prefix}[id]", person&.id) %>
   <%= hidden_field_tag("#{field_name_prefix}[role]", role) %>
-  <%= hidden_field_tag("#{field_name_prefix}[person_attributes][id]", person_link&.person&.id) %>
 
   <div class="form-group">
-    <%= text_field_tag "#{field_name_prefix}[person_attributes][full_name]", person_link&.person&.full_name, :class => 'form-control person-full-name', :placeholder => 'Full Name' %>
+    <%= text_field_tag "#{field_name_prefix}[person_attributes][full_name]", person&.full_name, :class => 'form-control person-full-name', :placeholder => 'Full Name' %>
   </div>
 
   <div class="form-group">
-    <%= 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)' %>
+    <%= text_field_tag "#{field_name_prefix}[person_attributes][given_name]", person&.given_name, :class => 'form-control person-given-name', :placeholder => 'Given Name (optional)' %>
   </div>
 
   <div class="form-group">
-    <%= 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)' %>
+    <%= text_field_tag "#{field_name_prefix}[person_attributes][family_name]", person&.family_name, :class => 'form-control person-family-name', :placeholder => 'Family Name (optional)' %>
   </div>
 
   <div class="form-group">
-    <%= text_field_tag "#{field_name_prefix}[person_attributes][orcid]", person_link&.person&.orcid, :class => 'form-control person-orcid', :placeholder => 'ORCID (optional)' %>
+    <%= text_field_tag "#{field_name_prefix}[person_attributes][orcid]", person&.orcid, :class => 'form-control person-orcid', :placeholder => 'ORCID (optional)' %>
   </div>
 
   <label class="ml-2 delete-person-btn">

app/views/common/_person_form.html.erb

@@ -73,9 +73,9 @@
     <%= f.field_lock :authors %>
 
     <div id="person-author-list">
-      <% @material.person_links.where(role: 'author').each_with_index do |person_link, index| %>
+      <% @material.people.where(role: 'author').each_with_index do |person, index| %>
         <%= render partial: 'common/person_form',
-                   locals: { form_name: 'material', index: index, person_link: person_link, role: 'author' } %>
+                   locals: { form_name: 'material', index: index, person: person, role: 'author' } %>
       <% end %>
     </div>
 
@@ -91,9 +91,9 @@
     <%= f.field_lock :contributors %>
 
     <div id="person-contributor-list">
-      <% @material.person_links.where(role: 'contributor').each_with_index do |person_link, index| %>
+      <% @material.people.where(role: 'contributor').each_with_index do |person, index| %>
         <%= render partial: 'common/person_form',
-                   locals: { form_name: 'material', index: "contributor_#{index}", person_link: person_link, role: 'contributor' } %>
+                   locals: { form_name: 'material', index: "contributor_#{index}", person: person, role: 'contributor' } %>
       <% end %>
     </div>
 
@@ -225,12 +225,12 @@
 
 <div id="person-author-template" style="display: none">
   <%= render partial: 'common/person_form',
-             locals: { form_name: 'material', person_link: nil, role: 'author' } %>
+             locals: { form_name: 'material', person: nil, role: 'author' } %>
 </div>
 
 <div id="person-contributor-template" style="display: none">
   <%= render partial: 'common/person_form',
-             locals: { form_name: 'material', person_link: nil, role: 'contributor' } %>
+             locals: { form_name: 'material', person: nil, role: 'contributor' } %>
 </div>
 
 <script type="text/javascript">

app/views/materials/_form.html.erb

@@ -5,11 +5,14 @@ def change
       t.string :family_name
       t.string :full_name
       t.string :orcid
+      t.string :role, null: false
+      t.references :resource, polymorphic: true, null: false
       t.references :profile, null: true, foreign_key: true
 
       t.timestamps
     end
 
     add_index :people, :orcid
+    add_index :people, [:resource_type, :resource_id, :role]
   end
 end