diff --git a/app/assets/javascripts/people.js b/app/assets/javascripts/people.js new file mode 100644 index 000000000..b364a80cd --- /dev/null +++ b/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); +}); diff --git a/app/controllers/materials_controller.rb b/app/controllers/materials_controller.rb index 4d9a54723..de9b8d25c 100644 --- a/app/controllers/materials_controller.rb +++ b/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: [] }, + { 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]], external_resources_attributes: %i[id url title _destroy], external_resources: %i[url title], event_ids: [], locked_fields: []) diff --git a/app/models/concerns/has_orcid.rb b/app/models/concerns/has_orcid.rb new file mode 100644 index 000000000..eabde600a --- /dev/null +++ b/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 diff --git a/app/models/concerns/has_people.rb b/app/models/concerns/has_people.rb new file mode 100644 index 000000000..960e8f17c --- /dev/null +++ b/app/models/concerns/has_people.rb @@ -0,0 +1,68 @@ +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 + 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 + + # Define custom setter that accepts strings (legacy), hashes, or Person objects + define_method("#{role_name}=") do |value| + 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?) + + # Remove existing links for this role + person_links.where(role: role).destroy_all + + people_array.each 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) + 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) + elsif person_data.is_a?(Person) + # Person object + person_links.build(person: person_data, role: role) + end + end + end +end diff --git a/app/models/material.rb b/app/models/material.rb index 9e1726c08..fef1da4d1 100644 --- a/app/models/material.rb +++ b/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') diff --git a/app/models/person.rb b/app/models/person.rb new file mode 100644 index 000000000..a04b88607 --- /dev/null +++ b/app/models/person.rb @@ -0,0 +1,35 @@ +class Person < ApplicationRecord + include HasOrcid + + belongs_to :profile, optional: true + has_many :person_links, dependent: :destroy + + # 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 diff --git a/app/models/person_link.rb b/app/models/person_link.rb new file mode 100644 index 000000000..80494a22a --- /dev/null +++ b/app/models/person_link.rb @@ -0,0 +1,7 @@ +class PersonLink < ApplicationRecord + belongs_to :resource, polymorphic: true + belongs_to :person + accepts_nested_attributes_for :person, reject_if: :all_blank + + validates :resource, :person, :role, presence: true +end diff --git a/app/models/profile.rb b/app/models/profile.rb index 9935bcca9..85ef53903 100644 --- a/app/models/profile.rb +++ b/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 diff --git a/app/serializers/material_serializer.rb b/app/serializers/material_serializer.rb index 4500fe153..25b467e18 100644 --- a/app/serializers/material_serializer.rb +++ b/app/serializers/material_serializer.rb @@ -5,7 +5,7 @@ class MaterialSerializer < ApplicationSerializer :doi, :licence, :version, :status, - :contact, :contributors, :authors, + :contact, :difficulty_level, :target_audience, :prerequisites, :syllabus, :learning_objectives, :subsets, @@ -18,4 +18,6 @@ class MaterialSerializer < ApplicationSerializer has_many :nodes has_many :collections has_many :events + has_many :authors, serializer: PersonSerializer + has_many :contributors, serializer: PersonSerializer end diff --git a/app/serializers/person_serializer.rb b/app/serializers/person_serializer.rb new file mode 100644 index 000000000..4d797d3db --- /dev/null +++ b/app/serializers/person_serializer.rb @@ -0,0 +1,8 @@ +class PersonSerializer < ApplicationSerializer + attributes :id, :given_name, :family_name, :full_name, :orcid, :profile_id + + # Return display_name for API responses + attribute :name do |person| + person.display_name + end +end diff --git a/app/views/common/_extra_metadata.html.erb b/app/views/common/_extra_metadata.html.erb index d32155ece..231ad4d81 100644 --- a/app/views/common/_extra_metadata.html.erb +++ b/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(', ') } %> diff --git a/app/views/common/_person_form.html.erb b/app/views/common/_person_form.html.erb new file mode 100644 index 000000000..f090bebec --- /dev/null +++ b/app/views/common/_person_form.html.erb @@ -0,0 +1,31 @@ +<% 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" %> + +
+ <%= hidden_field_tag("#{field_name_prefix}[id]", person_link&.id) %> + <%= hidden_field_tag("#{field_name_prefix}[role]", role) %> + <%= hidden_field_tag("#{field_name_prefix}[person_attributes][id]", person_link&.person&.id) %> + +
+ <%= 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][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][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][orcid]", person_link&.person&.orcid, :class => 'form-control person-orcid', :placeholder => 'ORCID (optional)' %> +
+ + +
diff --git a/app/views/materials/_form.html.erb b/app/views/materials/_form.html.erb index b71aac73a..0707befcf 100644 --- a/app/views/materials/_form.html.erb +++ b/app/views/materials/_form.html.erb @@ -68,12 +68,40 @@ visibility_toggle: TeSS::Config.feature['materials_disabled'] %> - <%= f.multi_input :authors, suggestions_url: people_autocomplete_suggestions_path, title: t('materials.hints.authors'), - visibility_toggle: TeSS::Config.feature['materials_disabled'] %> +
+ <%= f.label :authors %> + <%= f.field_lock :authors %> + +
+ <% @material.person_links.where(role: 'author').each_with_index do |person_link, index| %> + <%= render partial: 'common/person_form', + locals: { form_name: 'material', index: index, person_link: person_link, role: 'author' } %> + <% end %> +
+ + + + + Add author information +
- <%= f.multi_input :contributors, suggestions_url: people_autocomplete_suggestions_path, title: t('materials.hints.contributors'), - visibility_toggle: TeSS::Config.feature['materials_disabled'] %> +
+ <%= f.label :contributors %> + <%= f.field_lock :contributors %> + +
+ <% @material.person_links.where(role: 'contributor').each_with_index do |person_link, index| %> + <%= render partial: 'common/person_form', + locals: { form_name: 'material', index: "contributor_#{index}", person_link: person_link, role: 'contributor' } %> + <% end %> +
+ + + + + Add contributor information +
<%= 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 } %> + + +