Merge pull request #5875 from avalonmediasystem/may_i_see_your_id_please

Display Bibliographic ID distinctly from Other Identifiers and make searchable

app/models/iiif_manifest_presenter.rb

@@ -107,11 +107,16 @@ def combined_display_date(media_object)
 
   def display_other_identifiers(media_object)
     # bibliographic_id has form [:type,"value"], other_identifier has form [[:type,"value],[:type,"value"],...]
-    ids = media_object.bibliographic_id.present? ? [media_object.bibliographic_id] : []
-    ids += Array(media_object.other_identifier)
+    ids = Array(media_object.other_identifier) - [media_object.bibliographic_id]
+    return nil unless ids.present?
     ids.uniq.collect { |i| "#{ModsDocument::IDENTIFIER_TYPES[i[:source]]}: #{i[:id]}" }
   end
 
+  def display_bibliographic_id(media_object)
+    return nil unless media_object.bibliographic_id.present?
+    "#{ModsDocument::IDENTIFIER_TYPES[media_object.bibliographic_id[:source]]}: #{media_object.bibliographic_id[:id]}"
+  end
+
   def note_fields(media_object)
     fields = []
     note_types = ModsDocument::NOTE_TYPES.clone
@@ -197,6 +202,7 @@ def iiif_metadata_fields
       metadata_field('Lending Period', display_lending_period(media_object))
     ]
     fields += note_fields(media_object)
+    fields += [metadata_field('Bibliographic ID', display_bibliographic_id(media_object))]
     fields += [metadata_field('Other Identifier', display_other_identifiers(media_object))]
     fields
   end

app/models/media_object.rb

@@ -353,6 +353,7 @@ def to_solr(include_child_fields: false)
       all_text_values << solr_doc["notes_sim"]
       all_text_values << solr_doc["table_of_contents_ssim"]
       all_text_values << solr_doc["other_identifier_sim"]
+      all_text_values << solr_doc["bibliographic_id_ssi"]
       solr_doc["all_text_timv"] = all_text_values.flatten
       solr_doc.each_pair { |k,v| solr_doc[k] = v.is_a?(Array) ? v.select { |e| e =~ /\S/ } : v }
     end

spec/controllers/catalog_controller_spec.rb

@@ -172,7 +172,7 @@
 
     describe "search fields" do
       let(:media_object) { FactoryBot.create(:fully_searchable_media_object) }
-      ["title_tesi", "creator_ssim", "contributor_ssim", "unit_ssim", "collection_ssim", "abstract_ssi", "publisher_ssim", "topical_subject_ssim", "geographic_subject_ssim", "temporal_subject_ssim", "genre_ssim", "physical_description_ssim", "language_ssim", "date_sim", "notes_sim", "table_of_contents_ssim", "other_identifier_sim", "series_ssim" ].each do |field|
+      ["title_tesi", "creator_ssim", "contributor_ssim", "unit_ssim", "collection_ssim", "abstract_ssi", "publisher_ssim", "topical_subject_ssim", "geographic_subject_ssim", "temporal_subject_ssim", "genre_ssim", "physical_description_ssim", "language_ssim", "date_sim", "notes_sim", "table_of_contents_ssim", "other_identifier_sim", "series_ssim", "bibliographic_id_ssi" ].each do |field|
         it "should find results based upon #{field}" do
           query = Array(media_object.to_solr[field]).first
           #split on ' ' and only search on the first word of a multiword field value

spec/models/iiif_manifest_presenter_spec.rb

@@ -50,20 +50,40 @@
       allow_any_instance_of(IiifManifestPresenter).to receive(:lending_enabled).and_return(false)
 
       ['Title', 'Date', 'Main contributor', 'Summary', 'Contributor', 'Publisher', 'Genre', 'Subject', 'Time period',
-       'Location', 'Collection', 'Unit', 'Language', 'Rights Statement', 'Terms of Use', 'Physical Description',
-       'Series', 'Related Item', 'Notes', 'Table of Contents', 'Local Note', 'Other Identifiers', 'Access Restrictions'].each do |field|
+       'Location', 'Collection', 'Unit', 'Language', 'Rights Statement', 'Terms of Use', 'Physical Description', 'Series',
+       'Related Item', 'Notes', 'Table of Contents', 'Local Note', 'Other Identifier', 'Access Restrictions', 'Bibliographic ID'
+      ].each do |field|
         expect(subject).to include(field)
       end
       expect(subject).to_not include('Lending Period')
     end
 
+    context 'with duplicate identifiers' do
+      let(:media_object) do
+        FactoryBot.build(:fully_searchable_media_object).tap do |mo|
+          mo.other_identifier += mo.other_identifier
+          mo.other_identifier += [mo.bibliographic_id]
+          mo
+        end
+      end
+
+      it 'de-dupes other identifiers' do
+        allow_any_instance_of(IiifManifestPresenter).to receive(:lending_enabled).and_return(false)
+        expect(subject['Bibliographic ID'].first).to include media_object.bibliographic_id[:id]
+        expect(subject['Other Identifier'].size).to eq 1
+        expect(subject['Other Identifier'].first).not_to include media_object.bibliographic_id[:id]
+        expect(subject['Other Identifier'].first).to include media_object.other_identifier.first[:id]
+      end
+    end
+
     context 'when controlled digital lending is enabled' do
       it 'provides metadata' do
         allow_any_instance_of(IiifManifestPresenter).to receive(:lending_enabled).and_return(true)
 
         ['Title', 'Date', 'Main contributor', 'Summary', 'Contributor', 'Publisher', 'Genre', 'Subject', 'Time period',
-         'Location', 'Collection', 'Unit', 'Language', 'Rights Statement', 'Terms of Use', 'Physical Description',
-         'Series', 'Related Item', 'Notes', 'Table of Contents', 'Local Note', 'Other Identifiers', 'Access Restrictions', 'Lending Period'].each do |field|
+         'Location', 'Collection', 'Unit', 'Language', 'Rights Statement', 'Terms of Use', 'Physical Description', 'Series',
+         'Related Item', 'Notes', 'Table of Contents', 'Local Note', 'Other Identifier', 'Access Restrictions', 'Bibliographic ID', 'Lending Period'
+        ].each do |field|
           expect(subject).to include(field)
         end
       end