Migrate Material authors and contributors from string array to has_many Person relationship

app/assets/javascripts/people.js

@@ -0,0 +1,39 @@
+var People = {
+    add: function (role) {
+        var templateId = '#person-' + role + '-template';
+        var listId = '#person-' + role + '-list';
+        var newForm = $(templateId).clone().html();
+
+        // Ensure the index of the new form is 1 greater than the current highest index, to prevent collisions
+        var index = 0;
+        $(listId + ' .person-form').each(function () {
+            var newIndex = parseInt($(this).data('index'));
+            if (newIndex > index) {
+                index = newIndex;
+            }
+        });
+
+        // Replace the placeholder index with the actual index
+        newForm = $(newForm.replace(/replace-me/g, index + 1));
+        newForm.appendTo(listId);
+
+        return false; // Stop form being submitted
+    },
+
+    // This is just cosmetic. The actual removal is done by rails,
+    //   by virtue of the hidden checkbox being checked when the label is clicked.
+    delete: function () {
+        $(this).parents('.person-form').fadeOut();
+    }
+};
+
+document.addEventListener("turbolinks:load", function() {
+
+    $('[id^="person-"]')
+        .on('click', '[id^="add-person-"]', function() {
+            var role = $(this).data('role');
+            People.add(role);
+            return false;
+        })
+        .on('change', '.delete-person-btn input.destroy-attribute', People.delete);
+});

app/controllers/materials_controller.rb

@@ -171,11 +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: [] }, { authors: [] }, { 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: [] },
+                                     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_orcid.rb

@@ -0,0 +1,18 @@
+module HasOrcid
+  extend ActiveSupport::Concern
+
+  included do
+    auto_strip_attributes :orcid
+    before_validation :normalize_orcid
+  end
+
+  def orcid_url
+    return nil if orcid.blank?
+    "#{OrcidValidator::ORCID_PREFIX}#{orcid}"
+  end
+
+  def normalize_orcid
+    return if orcid.blank?
+    self.orcid = orcid.strip.sub(OrcidValidator::ORCID_DOMAIN_REGEX, '')
+  end
+end

app/models/concerns/has_people.rb

@@ -0,0 +1,42 @@
+module HasPeople
+  extend ActiveSupport::Concern
+
+  included do
+    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(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|
+        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_key)
+    # Remove existing links for this role
+    people.where(role: role_key).destroy_all
+
+    Array(value).reject(&:blank?).map do |person_data|
+      if person_data.is_a?(String)
+        # Legacy format: store as full_name directly
+        people.build(full_name: person_data.strip, role: role_key)
+      elsif person_data.is_a?(Hash)
+        people.build(**person_data, role: role_key)
+      elsif person_data.is_a?(Person)
+        person_data.role = role_key
+        person_data
+      end
+    end
+  end
+end

app/models/material.rb

@@ -21,6 +21,7 @@ class Material < ApplicationRecord
   include HasDifficultyLevel
   include HasEdamTerms
   include InSpace
+  include HasPeople
 
   if TeSS::Config.solr_enabled
     # :nocov:
@@ -30,8 +31,12 @@ class Material < ApplicationRecord
       text :description
       text :contact
       text :doi
-      text :authors
-      text :contributors
+      text :authors do
+        authors.map(&:display_name)
+      end
+      text :contributors do
+        contributors.map(&:display_name)
+      end
       text :target_audience
       text :keywords
       text :resource_type
@@ -51,7 +56,9 @@ class Material < ApplicationRecord
       end
       # other fields
       string :title
-      string :authors, multiple: true
+      string :authors, multiple: true do
+        authors.map(&:display_name)
+      end
       string :scientific_topics, multiple: true do
         scientific_topics_and_synonyms
       end
@@ -62,7 +69,9 @@ class Material < ApplicationRecord
       string :keywords, multiple: true
       string :fields, multiple: true
       string :resource_type, multiple: true
-      string :contributors, multiple: true
+      string :contributors, multiple: true do
+        contributors.map(&:display_name)
+      end
       string :content_provider do
         content_provider.try(:title)
       end
@@ -102,6 +111,10 @@ class Material < ApplicationRecord
 
   has_many :stars, as: :resource, dependent: :destroy
 
+  # Use HasPeople concern for authors and contributors
+  has_person_role :authors, role_key: 'author'
+  has_person_role :contributors, role_key: 'contributor'
+
   # Remove trailing and squeezes (:squish option) white spaces inside the string (before_validation):
   # e.g. "James     Bond  " => "James Bond"
   auto_strip_attributes :title, :description, :url, squish: false
@@ -111,10 +124,10 @@ class Material < ApplicationRecord
   validates :other_types, presence: true, if: proc { |m| m.resource_type.include?('other') }
   validates :keywords, length: { maximum: 20 }
 
-  clean_array_fields(:keywords, :fields, :contributors, :authors,
+  clean_array_fields(:keywords, :fields,
                      :target_audience, :resource_type, :subsets)
 
-  update_suggestions(:keywords, :contributors, :authors, :target_audience,
+  update_suggestions(:keywords, :target_audience,
                      :resource_type)
 
   def description=(desc)
@@ -212,8 +225,8 @@ def to_oai_dc
              'xsi:schemaLocation' => 'http://www.openarchives.org/OAI/2.0/oai_dc/ http://www.openarchives.org/OAI/2.0/oai_dc.xsd') do
       xml.tag!('dc:title', title)
       xml.tag!('dc:description', description)
-      authors.each { |a| xml.tag!('dc:creator', a) }
-      contributors.each { |a| xml.tag!('dc:contributor', a) }
+      authors.each { |a| xml.tag!('dc:creator', a.display_name) }
+      contributors.each { |c| xml.tag!('dc:contributor', c.display_name) }
       xml.tag!('dc:publisher', content_provider.title) if content_provider
 
       xml.tag!('dc:format', 'text/html')

app/models/person.rb

@@ -0,0 +1,37 @@
+class Person < ApplicationRecord
+  include HasOrcid
+
+  belongs_to :profile, optional: true
+  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
+
+  # Automatically link to profile based on ORCID on save
+  before_save :link_to_profile_by_orcid
+
+  # Return the display name - full_name if present, otherwise construct from given_name and family_name
+  def display_name
+    full_name.presence || "#{given_name} #{family_name}".strip
+  end
+
+  private
+
+  def name_presence
+    if full_name.blank? && (given_name.blank? || family_name.blank?)
+      errors.add(:base, "Either full_name or both given_name and family_name must be present")
+    end
+  end
+
+  # Automatically link to a Profile if one exists with a matching ORCID
+  def link_to_profile_by_orcid
+    if orcid.blank?
+      self.profile = nil
+    else
+      matching_profile = Profile.find_by(orcid: orcid)
+      self.profile = matching_profile if matching_profile.present?
+    end
+  end
+end

app/models/profile.rb

@@ -1,7 +1,9 @@
 require 'uri'
 
 class Profile < ApplicationRecord
-  auto_strip_attributes :firstname, :surname, :website, :orcid, squish: false
+  include HasOrcid
+
+  auto_strip_attributes :firstname, :surname, :website, squish: false
   belongs_to :user, inverse_of: :profile
 
   before_validation :normalize_orcid
@@ -25,11 +27,6 @@ def full_name
     "#{firstname} #{surname}".strip
   end
 
-  def orcid_url
-    return nil if orcid.blank?
-    "#{OrcidValidator::ORCID_PREFIX}#{orcid}"
-  end
-
   def merge(*others)
     Profile.transaction do
       attrs = attributes
@@ -66,11 +63,6 @@ def authenticate_orcid(orcid)
 
   private
 
-  def normalize_orcid
-    return if orcid.blank?
-    self.orcid = orcid.strip.sub(OrcidValidator::ORCID_DOMAIN_REGEX, '')
-  end
-
   def check_public
     public ? self.type = 'Trainer' : self.type = 'Profile'
   end

app/serializers/application_serializer.rb

@@ -31,5 +31,9 @@ def ontology_terms(type)
     object.send(type).map { |t| { preferred_label: t.preferred_label, uri: t.uri } }
   end
 
+  def people(type)
+    object.send(type).map(&:display_name)
+  end
+
   link(:self) { polymorphic_path(object) }
 end

app/serializers/material_serializer.rb

@@ -18,4 +18,12 @@ class MaterialSerializer < ApplicationSerializer
   has_many :nodes
   has_many :collections
   has_many :events
+
+  def contributors
+    people(:contributors)
+  end
+
+  def authors
+    people(:authors)
+  end
 end

app/views/common/_extra_metadata.html.erb

@@ -59,8 +59,8 @@
   <%= display_attribute(resource, :sponsors) { |values| values.join(', ') } %>
 <% end %>
 
-<%= display_attribute(resource, :authors) { |values| values.join(', ') } if resource.respond_to?(:authors) %>
-<%= display_attribute(resource, :contributors) { |values| values.join(', ') } if resource.respond_to?(:contributors) %>
+<%= display_attribute(resource, :authors) { |values| values.map(&:display_name).join(', ') } if resource.respond_to?(:authors) %>
+<%= display_attribute(resource, :contributors) { |values| values.map(&:display_name).join(', ') } if resource.respond_to?(:contributors) %>
 <%= display_attribute(resource, :remote_created_date) if resource.respond_to?(:remote_created_date) %>
 <%= display_attribute(resource, :remote_updated_date) if resource.respond_to?(:remote_updated_date) %>
 <%= display_attribute(resource, :scientific_topics) { |values| values.map { |x| x.preferred_label }.join(', ') } %>

app/views/common/_person_form.html.erb

@@ -0,0 +1,30 @@
+<% 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}[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&.id) %>
+  <%= hidden_field_tag("#{field_name_prefix}[role]", role) %>
+
+  <div class="form-group">
+    <%= text_field_tag "#{field_name_prefix}[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}[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}[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}[orcid]", person&.orcid, :class => 'form-control person-orcid', :placeholder => 'ORCID (optional)' %>
+  </div>
+
+  <label class="ml-2 delete-person-btn">
+    <i class="icon icon-sm cross-icon icon-greyscale"></i>
+    <%= hidden_field_tag "#{field_name_prefix}[_destroy]", '0', :autocomplete => 'off' %>
+    <%= check_box_tag "#{field_name_prefix}[_destroy]", '1', false, :class => 'destroy-attribute', :autocomplete => 'off' %>
+  </label>
+</div>

app/views/materials/_form.html.erb

@@ -68,12 +68,40 @@
               visibility_toggle: TeSS::Config.feature['materials_disabled'] %>
 
   <!-- Field: Authors -->
-  <%= f.multi_input :authors, suggestions_url: people_autocomplete_suggestions_path, title: t('materials.hints.authors'),
-                    visibility_toggle: TeSS::Config.feature['materials_disabled'] %>
+  <div class="form-group" id="person-author">
+    <%= f.label :authors %>
+    <%= f.field_lock :authors %>
+
+    <div id="person-author-list">
+      <% @material.people.where(role: 'author').each_with_index do |person, index| %>
+        <%= render partial: 'common/person_form',
+                   locals: { form_name: 'material', index: index, person: person, role: 'author' } %>
+      <% end %>
+    </div>
+
+    <a href="#" id="add-person-author" data-role="author" class="btn btn-icon">
+      <i class="icon icon-h4 plus-icon"></i>
+    </a>
+    <span class="help-inline-block help-block">Add author information</span>
+  </div>
 
   <!-- Field: Contributors -->
-  <%= f.multi_input :contributors, suggestions_url: people_autocomplete_suggestions_path, title: t('materials.hints.contributors'),
-                    visibility_toggle: TeSS::Config.feature['materials_disabled'] %>
+  <div class="form-group" id="person-contributor">
+    <%= f.label :contributors %>
+    <%= f.field_lock :contributors %>
+
+    <div id="person-contributor-list">
+      <% @material.people.where(role: 'contributor').each_with_index do |person, index| %>
+        <%= render partial: 'common/person_form',
+                   locals: { form_name: 'material', index: "contributor_#{index}", person: person, role: 'contributor' } %>
+      <% end %>
+    </div>
+
+    <a href="#" id="add-person-contributor" data-role="contributor" class="btn btn-icon">
+      <i class="icon icon-h4 plus-icon"></i>
+    </a>
+    <span class="help-inline-block help-block">Add contributor information</span>
+  </div>
 
   <!-- Field: Target Audience -->
   <%= f.multi_input :target_audience, label: 'Target audiences', errors: @material.errors[:target_audience],
@@ -195,6 +223,15 @@
              locals: { form_name: 'material', external_resource: ExternalResource.new } %>
 </div>
 
+<div id="person-author-template" style="display: none">
+  <%= render partial: 'common/person_form',
+             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: nil, role: 'contributor' } %>
+</div>
 
 <script type="text/javascript">
     function removeSuggestion(suggestion) {