diff --git a/README.md b/README.md index b4234f88..158b31b2 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,7 @@ See `Optional Environment Variables` for more information. - `MIT_PRIMO_URL`: The base URL for MIT Libraries' Primo instance (used to generate record links). - `PRIMO_API_KEY`: The Primo Search API key. - `PRIMO_API_URL`: The Primo Search API base URL. +- `PRIMO_INST_ID`: Our Ex Libris Instituion ID - `PRIMO_SCOPE`: The Primo Search API `scope` used in Primo UI links. Does not affect our API calls. Ask Enterprise Systems for value. - `PRIMO_TAB`: The Primo Search API `tab` used in Primo UI links. Does not affect our API calls. Ask Enterprise Systems for value. - `PRIMO_VID`: The Primo Search API `vid` (or 'view ID`) param. Used in both our API calls and Primo UI. Ask Enterprise Systems for value. diff --git a/app/javascript/controllers/content_loader_controller.js b/app/javascript/controllers/content_loader_controller.js index 510fa83c..f831d639 100644 --- a/app/javascript/controllers/content_loader_controller.js +++ b/app/javascript/controllers/content_loader_controller.js @@ -10,6 +10,16 @@ export default class extends Controller { load() { fetch(this.urlValue) .then(response => response.text()) - .then(html => this.element.innerHTML = html) + .then(html => { + this.element.innerHTML = html; + // Hide primo links if libkey link is present + if (this.element.querySelector('.libkey-link')) { + const resultGet = this.element.closest('.result-get'); + if (resultGet) { + const primoLinks = resultGet.querySelectorAll('.primo-link'); + primoLinks.forEach(link => link.style.display = 'none'); + } + } + }) } } diff --git a/app/models/libkey.rb b/app/models/libkey.rb index 201c29e9..e57d9daf 100644 --- a/app/models/libkey.rb +++ b/app/models/libkey.rb @@ -29,6 +29,7 @@ def self.lookup(type:, identifier:, libkey_client: nil) Sentry.set_tags('mitlib.libkeyurl': url) Sentry.set_tags('mitlib.libkeystatus': e.message) Sentry.capture_message('Unexpected Libkey response status') + Rails.logger.error("Unexpected Libkey response status: #{e.message}") nil rescue HTTP::Error Rails.logger.error('Libkey connection error') diff --git a/app/models/normalize_primo_record.rb b/app/models/normalize_primo_record.rb index bac9e9f0..35999f34 100644 --- a/app/models/normalize_primo_record.rb +++ b/app/models/normalize_primo_record.rb @@ -107,13 +107,55 @@ def links links << { 'url' => record_link, 'kind' => 'full record' } end - # Add openurl if available - links << { 'url' => openurl, 'kind' => 'openurl' } if openurl.present? + # If $$Topenurl_article + if @record['pnx']['links'] && @record['pnx']['links']['openurl'] && openurl.present? + links << { 'url' => openurl, 'kind' => 'Check Availability' } + end + + # Add PDF if available + if @record['pnx']['links'] && @record['pnx']['links']['linktopdf'] + + parsed_string = parse_link_string(@record['pnx']['links']['linktopdf'].first) + + if parsed_string['U'].present? + links << { 'url' => parsed_string['U'], + 'kind' => 'View PDF' } + end + end + + # Add HTML if available + if @record['pnx']['links'] && @record['pnx']['links']['linktohtml'] + + parsed_string = parse_link_string(@record['pnx']['links']['linktohtml'].first) + + if parsed_string['U'].present? + links << { 'url' => parsed_string['U'], + 'kind' => 'View HTML' } + end + end # Return links if we found any links.any? ? links : [] end + # Parses a link string into a hash of key-value pairs. + # The link string is a series of key-value pairs separated by $$, where each pair is prefixed by a single character. + # For example: "$$Uhttps://example.com$$TView PDF" would be parsed into { 'U' => 'https://example.com', 'T' => 'View PDF' } + def parse_link_string(link_string) + return unless link_string.start_with?('$$') + + parts = link_string.split('$$') + hash = {} + parts.each do |part| + next if part.empty? + + key = part[0] + value = part[1..-1] + hash[key] = value + end + hash + end + def citation # We don't want to include citations for Alma records at this time. If we include them in the future they need # to be cleaned up as they currently look like `Engineering village 2$$QEngineering village 2` @@ -263,6 +305,9 @@ def openurl def construct_primo_openurl return unless @record['delivery']['almaOpenurl'] + # Primo OpenURL links to some formats are not helpful + return if format == 'Journal' + return if format == 'Video' # Here we are converting the Alma link resolver URL provided by the Primo # Search API to redirect to the Primo UI. This is done for UX purposes, @@ -270,15 +315,16 @@ def construct_primo_openurl # disambiguation page. primo_openurl_base = [ENV.fetch('MIT_PRIMO_URL', nil), '/discovery/openurl?institution=', - ENV.fetch('EXL_INST_ID', nil), + ENV.fetch('PRIMO_INST_ID', nil), '&vid=', ENV.fetch('PRIMO_VID', nil), '&'].join primo_openurl = @record['delivery']['almaOpenurl'].gsub(ENV.fetch('ALMA_OPENURL', nil), primo_openurl_base) # The ctx params appear to break Primo openurls, so we need to remove them. - params = Rack::Utils.parse_nested_query(primo_openurl) - filtered = params.delete_if { |key, _value| key.starts_with?('ctx') } + # params = Rack::Utils.parse_nested_query(primo_openurl) + # filtered = params.delete_if { |key, _value| key.starts_with?('ctx') } + filtered = primo_openurl URI::DEFAULT_PARSER.unescape(filtered.to_param) end diff --git a/app/views/libkey/lookup.html.erb b/app/views/libkey/lookup.html.erb index eae36a64..847f74ea 100644 --- a/app/views/libkey/lookup.html.erb +++ b/app/views/libkey/lookup.html.erb @@ -1,3 +1,3 @@ <% if Libkey.enabled? && @libkey.present? %> - <%= link_to( @libkey[:text], @libkey[:link], class: 'button', style: 'border: none;' ) %> + <%= link_to( @libkey[:text], @libkey[:link], class: 'button libkey-link' ) %> <% end %> diff --git a/app/views/search/_result_primo.html.erb b/app/views/search/_result_primo.html.erb index 399c06cf..bdb96be3 100644 --- a/app/views/search/_result_primo.html.erb +++ b/app/views/search/_result_primo.html.erb @@ -13,7 +13,6 @@
-

<%= result[:format] %> <%= result[:year] %> @@ -63,19 +62,25 @@ <% if result[:links].present? %> <% result[:links].each do |link| %> <% if link['kind'].downcase == 'full record' %> + <%# Full record link button uses the same link as the title link %> <% if Feature.enabled?(:record_link) %> <%= link_to 'View full record', link['url'], class: 'button' %> <% end %> + <%# Primo supplies PDF and HTML links in addition to the OpenURL variant that may be useful. %> + <%# We hide links with the `primo-link` css class if LibKey returns results. %> <% else %> - <%= link_to link['kind'].titleize, link['url'], class: 'button' %> + <%= link_to link['kind'], link['url'], class: 'button primo-link' %> <% end %> <% end %> <% end %> + + <%# LibKey links are our preferred links so we call the API when possible to get these preferred links %> <% if Libkey.enabled? && result[:doi].present? %> <%= render(partial: 'trigger_libkey', locals: { kind: 'doi', identifier: result[:doi] }) %> <% elsif Libkey.enabled? && result[:pmid].present? %> <%= render(partial: 'trigger_libkey', locals: { kind: 'pmid', identifier: result[:pmid] }) %> <% end %> + <% if result[:availability].present? %> <%= availability(result[:availability], result[:location], result[:other_availability]) %> <% end %> diff --git a/test/fixtures/primo/full_record.json b/test/fixtures/primo/full_record.json index 24ad3425..0bd4a68d 100644 --- a/test/fixtures/primo/full_record.json +++ b/test/fixtures/primo/full_record.json @@ -60,6 +60,26 @@ "22110403" ] }, + "links": { + "openurl": [ + "$$Topenurl_article" + ], + "openurlfulltext": [ + "$$Topenurlfull_article" + ], + "backlink": [ + "$$Uhttp://pascal-francis.inist.fr/vibad/index.php?action=getRecordDetail&idt=21178602$$DView record in Pascal Francis$$Hfree_for_read" + ], + "thumbnail": [ + "$$Tsyndetics_thumb_exl" + ], + "linktopdf": [ + "$$Uhttps://libproxy.mit.edu/login?&url=https://www.jstor.org/stable/pdf/20464433$$EPDF$$P50$$Gjstor$$H" + ], + "linktohtml": [ + "$$Uhttps://libproxy.mit.edu/login?&url=https://www.jstor.org/stable/20464433$$EHTML$$P50$$Gjstor$$H" + ] + }, "facets": { "frbrtype": [ "5" diff --git a/test/models/normalize_primo_record_test.rb b/test/models/normalize_primo_record_test.rb index adfa03a5..d67e4b2e 100644 --- a/test/models/normalize_primo_record_test.rb +++ b/test/models/normalize_primo_record_test.rb @@ -73,7 +73,7 @@ def cdi_record assert_equal 'full record', normalized[:links].first['kind'] # Second link should be the Alma openurl - assert_equal 'openurl', normalized[:links].second['kind'] + assert_equal 'Check Availability', normalized[:links].second['kind'] end test 'handles missing links' do @@ -81,6 +81,21 @@ def cdi_record assert_empty normalized[:links] end + test 'parse_link_string creates expected data structure' do + # Strings that don't start with $$ should not be processed + link_string = 'https://example.com?param1=value1¶m2=value2' + assert_nil NormalizePrimoRecord.new(full_record, 'test').send(:parse_link_string, link_string) + + # Extract properly formatted links + link_string = '$$Uhttps://libproxy.mit.edu/login?&url=https://www.jstor.org/stable/pdf/20464433$$EPDF$$P50$$Gjstor$$H' + expected = { 'U' => 'https://libproxy.mit.edu/login?&url=https://www.jstor.org/stable/pdf/20464433', 'E' => 'PDF', 'P' => '50', 'G' => 'jstor', 'H' => '' } + assert_equal expected, NormalizePrimoRecord.new(full_record, 'test').send(:parse_link_string, link_string) + + link_string = '$$Uhttps://libproxy.mit.edu/login?&url=https://www.jstor.org/stable/20464433$$EHTML$$P50$$Gjstor$$H' + expected = { 'U' => 'https://libproxy.mit.edu/login?&url=https://www.jstor.org/stable/20464433', 'E' => 'HTML', 'P' => '50', 'G' => 'jstor', 'H' => '' } + assert_equal expected, NormalizePrimoRecord.new(full_record, 'test').send(:parse_link_string, link_string) + end + test 'normalizes citation for cdi records' do record = full_record.dup record['pnx']['control']['recordid'] = ['cdi_crossref_primary_10_1234_test_article'] @@ -286,8 +301,10 @@ def cdi_record link_kinds = normalized[:links].map { |link| link['kind'] } assert_includes link_kinds, 'full record' - assert_includes link_kinds, 'openurl' - assert_equal 2, normalized[:links].length # Only full record and openurl + assert_includes link_kinds, 'Check Availability' + assert_includes link_kinds, 'View PDF' + assert_includes link_kinds, 'View HTML' + assert_equal 4, normalized[:links].length end # Additional coverage tests for existing methods @@ -359,14 +376,14 @@ def cdi_record test 'handles openurl server validation' do # Test with matching server normalized = NormalizePrimoRecord.new(full_record, 'test').normalize - openurl_link = normalized[:links].find { |link| link['kind'] == 'openurl' } + openurl_link = normalized[:links].find { |link| link['kind'] == 'Check Availability' } assert_not_nil openurl_link # Test with mismatched server. Should log warning but still return URL record = full_record.deep_dup record['delivery']['almaOpenurl'] = 'https://different.server.com/openurl?param=value' normalized = NormalizePrimoRecord.new(record, 'test').normalize - openurl_link = normalized[:links].find { |link| link['kind'] == 'openurl' } + openurl_link = normalized[:links].find { |link| link['kind'] == 'Check Availability' } assert_not_nil openurl_link end