HYC-2048 - Fix rendering of department with multiple terms (#1149)

* Add test

* Treat term as an array so that we can deal with vocab entries that return a single or multiple terms

* Debug warning to debug

* Fix term delimiter

app/renderers/hyrax/renderers/person_attribute_renderer.rb

@@ -24,16 +24,17 @@ def render_dl_row
       def attribute_value_to_html(value)
         person = value.split('||')
         display_text = ''
-        display_text << "<li><span#{html_attributes(microdata_value_attributes(field.to_s.chomp('_display')))}>#{person[1]}</span>"
+        display_text << "<span#{html_attributes(microdata_value_attributes(field.to_s.chomp('_display')))}>#{person[1]}</span>"
         if person.length > 2
           display_text << '<ul>'
           person[2..-1].each do |attr|
-            display_text << "<li>#{format_attribute(attr)}</li>"
+            format_attribute(attr).each do |attr|
+              display_text << "<li>#{attr}</li>"
+            end
           end
           display_text << '</ul>'
         end
 
-        display_text << '</li>'
       rescue ArgumentError
         value
       end
@@ -47,13 +48,15 @@ def format_attribute(text)
           # Get the full hierarchy of terms, with correct short labels, for the affiliation id
           term = DepartmentsService.term(text.split(':').last.strip)
           if term.present?
-            text = term.split(';').map do |term_val|
-              DepartmentsService.short_label(term_val.strip)
-            end.join(', ')
+            text = Array(term).map { |t| t.split(';') }.map do |term_list|
+              term_list.map do |term_val|
+                DepartmentsService.short_label(term_val.strip) || term_val.strip
+              end.join(', ')
+            end
           end
         end
 
-        text
+        Array(text)
       end
     end
   end

app/services/departments_service.rb

@@ -15,7 +15,7 @@ def self.identifier(term)
     return nil if term.blank?
     authority.all.reject { |item| item['active'] == false }.find { |department| department['label'] == term }['id']
   rescue StandardError
-    Rails.logger.warn "DepartmentsService: cannot find identifier for '#{id}'"
+    Rails.logger.debug "DepartmentsService: cannot find identifier for '#{id}'"
     nil
   end
 
@@ -24,7 +24,7 @@ def self.term(id)
     return nil if id.blank?
     authority.find(id).fetch('term')
   rescue StandardError
-    Rails.logger.warn "DepartmentsService: cannot find term for '#{id}'"
+    Rails.logger.debug "DepartmentsService: cannot find term for '#{id}'"
     nil
   end
 
@@ -35,7 +35,7 @@ def self.short_label(id)
     return nil if id.blank?
     authority.find(id).fetch('short_label')
   rescue KeyError
-    Rails.logger.warn "DepartmentsService: cannot find short_label for '#{id}'"
+    Rails.logger.debug "DepartmentsService: cannot find short_label for '#{id}'"
     nil
   end
 

config/authorities/departments.yml

@@ -321,7 +321,7 @@ terms:
   active: true
   short_label: Curriculum in Peace, War, and Defense
 - id: Curriculum in Russian and East European Studies
-  term: College of Arts and Sciences; Center for Slavic; Eurasian; and East European Studies; Curriculum in Russian and East European Studies
+  term: College of Arts and Sciences; Center for Slavic, Eurasian, and East European Studies; Curriculum in Russian and East European Studies
   active: true
   short_label: Curriculum in Russian and East European Studies
 - id: Curriculum in Toxicology
@@ -1089,7 +1089,7 @@ terms:
   active: true
   short_label: Injury Prevention Research Center
 - id: IAM
-  term: Institute for Advanced Materials; Nanoscience and Technology
+  term: Institute for Advanced Materials, Nanoscience and Technology
   active: false
   short_label: IAM
 - id: Institute for Advanced Materials, Nanoscience and Technology

spec/renderers/hyrax/renderers/person_attribute_renderer_spec.rb

@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Hyrax::Renderers::PersonAttributeRenderer do
+  describe '#render_dl_row' do
+    subject { Nokogiri::HTML(renderer.render_dl_row) }
+
+    let(:expected) { Nokogiri::HTML(tr_content) }
+
+    context 'with department a single term' do
+      let(:field) { :creator_display }
+      let(:renderer) { described_class.new(field, ['index:1||art, person||Affiliation: Art History']) }
+      let(:tr_content) do
+        %(
+          <dt>Creator display</dt>
+          <dd>
+            <ul class="tabular">
+              <li itemprop="creator" itemtype="http://schema.org/Person" class="attribute attribute-creator_display">
+                <span>art, person</span>
+                <ul>
+                  <li>College of Arts and Sciences, Department of Art and Art History, Art History</li>
+                </ul>
+              </li>
+            </ul>
+          </dd>
+        )
+      end
+
+      it { expect(subject).to be_equivalent_to(expected) }
+    end
+
+    context 'with department containing multiple terms' do
+      let(:field) { :creator_display }
+      let(:renderer) { described_class.new(field, ['index:1||person||Affiliation: Joint Department of Biomedical Engineering']) }
+      let(:tr_content) do
+        %(
+          <dt>Creator display</dt>
+          <dd>
+            <ul class="tabular">
+              <li itemprop="creator" itemtype="http://schema.org/Person" class="attribute attribute-creator_display">
+                <span>person</span>
+                <ul>
+                  <li>School of Medicine, Joint Department of Biomedical Engineering</li>
+                  <li>North Carolina State University, Joint Department of Biomedical Engineering</li>
+                </ul>
+              </li>
+            </ul>
+          </dd>
+        )
+      end
+
+      it { expect(subject).to be_equivalent_to(expected) }
+    end
+  end
+end

spec/services/departments_service_spec.rb

@@ -55,9 +55,9 @@
     end
 
     it 'logs but does not raise error for non-existent terms' do
-      allow(Rails.logger).to receive(:warn)
+      allow(Rails.logger).to receive(:debug)
       expect(service.short_label('not-a-department')).to be_nil
-      expect(Rails.logger).to have_received(:warn)
+      expect(Rails.logger).to have_received(:debug)
     end
   end
 end