From 9f7800b15961e6c7122632d052514dc21b03a57a Mon Sep 17 00:00:00 2001 From: jazairi <16103405+jazairi@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:58:48 -0700 Subject: [PATCH] Patch bugs in full-text links logic Why these changes are being introduced: There are two uncaught edge cases in the full-text link logic: 1. Full-text options are removed when libkey links exist. 2. I a PNX record as an empty links object (rather than omitting it altogether), then full-text options will not display. Relevant ticket(s): - [USE-663](https://mitlibraries.atlassian.net/browse/USE-663) How this addresses that need: This addresses the edge cases noted above. Side effects of this change: This change was prompted by full-text links not appearing in staging, despite working properly in local development environments. It's unclear whether this change is what's required to fix staging. --- .../controllers/content_loader_controller.js | 2 +- app/models/normalize_primo_record.rb | 13 +++++++------ app/views/search/_result_primo.html.erb | 6 ++++-- test/models/normalize_primo_record_test.rb | 17 ++++++++++++++--- 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/app/javascript/controllers/content_loader_controller.js b/app/javascript/controllers/content_loader_controller.js index 02450a68..074adaf0 100644 --- a/app/javascript/controllers/content_loader_controller.js +++ b/app/javascript/controllers/content_loader_controller.js @@ -37,7 +37,7 @@ export default class extends Controller { if (parentElement.querySelector('.libkey-link')) { const resultGet = parentElement.closest('.result-get') if (resultGet) { - const primoLinks = resultGet.querySelectorAll('.primo-link') + const primoLinks = resultGet.querySelectorAll('.primo-link:not(.primo-link-preserve)') // removing instead of hiding to avoid layout issues when selecting which link to highlight primoLinks.forEach(link => link.remove()) } diff --git a/app/models/normalize_primo_record.rb b/app/models/normalize_primo_record.rb index 8c7a6cc5..04719a57 100644 --- a/app/models/normalize_primo_record.rb +++ b/app/models/normalize_primo_record.rb @@ -131,12 +131,13 @@ def links end end - # Add Full-text options if pnx['links'] is nil and record has Alma-E (electronic availability) - full_record_link = record_link - if @record.dig('pnx', 'links').nil? && - @record.dig('delivery', 'deliveryCategory')&.include?('Alma-E') && - full_record_link.present? - links << { 'url' => "#{full_record_link}#nui.getit.service_viewit", 'kind' => 'Full-text options' } + # Add Full-text options when Alma-E is present and Primo does not provide direct PDF/HTML link + # fields. + has_direct_fulfillment = @record.dig('pnx', 'links', 'linktopdf').present? || + @record.dig('pnx', 'links', 'linktohtml').present? + if !has_direct_fulfillment && + @record.dig('delivery', 'deliveryCategory')&.include?('Alma-E') && record_link.present? + links << { 'url' => "#{record_link}#nui.getit.service_viewit", 'kind' => 'Full-text options' } end # Return links if we found any diff --git a/app/views/search/_result_primo.html.erb b/app/views/search/_result_primo.html.erb index 7b8d47ac..9c6ab477 100644 --- a/app/views/search/_result_primo.html.erb +++ b/app/views/search/_result_primo.html.erb @@ -92,9 +92,11 @@ <%= link_to 'View full record', link['url'], class: 'button', data: { content_piece: 'View Full Record' } %> <% 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. %> + <%# We hide most links with the `primo-link` css class if LibKey returns results. Keep Full-text options visible when available. %> <% else %> - <%= link_to link['kind'], link['url'], class: 'button primo-link', data: { content_piece: link['kind'] } %> + <% link_classes = ['button', 'primo-link'] %> + <% link_classes << 'primo-link-preserve' if link['kind'] == 'Full-text options' %> + <%= link_to link['kind'], link['url'], class: link_classes.join(' '), data: { content_piece: link['kind'] } %> <% end %> <% end %> <% end %> diff --git a/test/models/normalize_primo_record_test.rb b/test/models/normalize_primo_record_test.rb index 35d106e0..875483a8 100644 --- a/test/models/normalize_primo_record_test.rb +++ b/test/models/normalize_primo_record_test.rb @@ -473,16 +473,27 @@ def cdi_record assert_match(/#nui\.getit\.service_viewit$/, full_text_link['url']) end - test 'excludes Full-text options link when pnx[links] is present' do + test 'excludes Full-text options link when direct Primo PDF or HTML links are present' do record = full_record.deep_dup - - # Add delivery category with electronic record['delivery']['deliveryCategory'] = %w[Alma-E] + normalized = NormalizePrimoRecord.new(record, 'test').normalize full_text_link = normalized[:links].find { |link| link['kind'] == 'Full-text options' } assert_nil full_text_link end + test 'includes Full-text options link when pnx[links] is present but has no usable links' do + record = alma_record.deep_dup + record['pnx']['links'] = {} + record['delivery']['deliveryCategory'] = %w[Alma-E] + + normalized = NormalizePrimoRecord.new(record, 'test').normalize + full_text_link = normalized[:links].find { |link| link['kind'] == 'Full-text options' } + assert_not_nil full_text_link + assert_match %r{/discovery/fulldisplay\?}, full_text_link['url'] + assert_match(/#nui\.getit\.service_viewit$/, full_text_link['url']) + end + test 'excludes Full-text options link when only Alma-P present' do record = alma_record.deep_dup record['pnx']['links'] = nil