Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display Bibliographic ID distinctly from Other Identifiers and make searchable #5875

Merged
merged 1 commit into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions app/models/iiif_manifest_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions app/models/media_object.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/catalog_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 24 additions & 4 deletions spec/models/iiif_manifest_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down