From 468958423d7d49c69fc1ec51dd0240090f5bd226 Mon Sep 17 00:00:00 2001 From: cjcolvar Date: Mon, 17 Jun 2024 11:54:29 -0400 Subject: [PATCH] Display Bibliographic ID distinctly from Other Identifiers and make searchable --- app/models/iiif_manifest_presenter.rb | 10 ++++++-- app/models/media_object.rb | 1 + spec/controllers/catalog_controller_spec.rb | 2 +- spec/models/iiif_manifest_presenter_spec.rb | 28 ++++++++++++++++++--- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/app/models/iiif_manifest_presenter.rb b/app/models/iiif_manifest_presenter.rb index daa78fe450..4dab4f39b2 100644 --- a/app/models/iiif_manifest_presenter.rb +++ b/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 diff --git a/app/models/media_object.rb b/app/models/media_object.rb index 84767ea07b..e2030cd738 100644 --- a/app/models/media_object.rb +++ b/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 diff --git a/spec/controllers/catalog_controller_spec.rb b/spec/controllers/catalog_controller_spec.rb index 1d8a748e79..296d584b24 100644 --- a/spec/controllers/catalog_controller_spec.rb +++ b/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 diff --git a/spec/models/iiif_manifest_presenter_spec.rb b/spec/models/iiif_manifest_presenter_spec.rb index 4ae010dd84..686a3bb148 100644 --- a/spec/models/iiif_manifest_presenter_spec.rb +++ b/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