Skip to content

Patch bugs in full-text links logic #458

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.

Sign up for GitHub

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

Open
jazairi wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Patch bugs in full-text links logic #458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion app/javascript/controllers/content_loader_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
13 changes: 7 additions & 6 deletions app/models/normalize_primo_record.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions app/views/search/_result_primo.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'] } %>
Comment thread
jazairi marked this conversation as resolved.
<% end %>
<% end %>
<% end %>
Expand Down
17 changes: 14 additions & 3 deletions test/models/normalize_primo_record_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down