From 0b390e1542280055f4d8ac87015699be2bb3ee09 Mon Sep 17 00:00:00 2001 From: jazairi <16103405+jazairi@users.noreply.github.com> Date: Wed, 17 Jun 2026 14:06:33 -0700 Subject: [PATCH] Add 'Full text options' fulfillment link. Why these changes are being introduced: UXWS has requested for journal records with Browzine links should also include a 'Full-text options' button that links to the full record. Relevant ticket(s): - [USE-614](https://mitlibraries.atlassian.net/browse/USE-614) How this addresses that need: This adds the 'Full-text options' link. Side effects of this change: - Button class has been removed from certain LibKey links for styling purposes. - New link is rendered outside of libkey-links div for styling purposes. - Added comments clarifying the Browzine/LibKey partials. --- app/controllers/thirdiron_controller.rb | 19 +++++++++++ app/helpers/results_helper.rb | 10 ++++++ app/views/search/_result_primo.html.erb | 6 ++-- app/views/search/_trigger_browzine.html.erb | 4 ++- app/views/thirdiron/browzine.html.erb | 17 +++++++--- app/views/thirdiron/libkey.html.erb | 8 ++++- test/controllers/thirdiron_controller_test.rb | 32 +++++++++++++++-- test/helpers/results_helper_test.rb | 34 +++++++++++++++++++ 8 files changed, 118 insertions(+), 12 deletions(-) diff --git a/app/controllers/thirdiron_controller.rb b/app/controllers/thirdiron_controller.rb index 11f8c18f..8ceccad6 100644 --- a/app/controllers/thirdiron_controller.rb +++ b/app/controllers/thirdiron_controller.rb @@ -1,3 +1,5 @@ +require 'uri' + class ThirdironController < ApplicationController layout false @@ -14,6 +16,7 @@ def browzine return unless ThirdIron.enabled? && params[:issn].present? @browzine = Browzine.lookup(issn: params[:issn]) + @full_record_url = safe_full_record_url(params[:full_record_url]) end private @@ -21,4 +24,20 @@ def browzine def expected_params? params[:type].present? && params[:identifier].present? end + + def safe_full_record_url(url) + return nil unless url.is_a?(String) + + url = url.strip + return nil if url.blank? + + parsed = URI.parse(url) + return nil unless parsed.is_a?(URI::HTTP) + return nil if parsed.host.blank? + return nil if parsed.userinfo.present? + + parsed.to_s + rescue URI::InvalidURIError, ArgumentError + nil + end end diff --git a/app/helpers/results_helper.rb b/app/helpers/results_helper.rb index c9e1e3c0..13aef52b 100644 --- a/app/helpers/results_helper.rb +++ b/app/helpers/results_helper.rb @@ -81,6 +81,16 @@ def article?(format) format.match?(/\barticle\b/i) end + # Extracts the full record URL from a result's links + # + # @param result [Hash] A normalized result hash with :links + # @return [String, nil] The URL of the 'full record' link, or nil if not found + def full_record_url(result) + return unless result.is_a?(Hash) + + result[:links]&.find { |link| link['kind'] == 'full record' }&.dig('url') + end + # Determines if a result has any fulfillment links to render # # @param result [Hash] A normalized Primo result hash diff --git a/app/views/search/_result_primo.html.erb b/app/views/search/_result_primo.html.erb index c824ccde..2d4bc0b4 100644 --- a/app/views/search/_result_primo.html.erb +++ b/app/views/search/_result_primo.html.erb @@ -3,8 +3,8 @@
<%# NOTE: the order of the two items in this lockup are being swapped visually with CSS. The markup is ordered to make the screen reading experience more logical %>

- <% if result[:links]&.find { |link| link['kind'] == 'full record' } %> - <%= link_to(result[:title], result[:links].find { |link| link['kind'] == 'full record' }['url'], data: { content_piece: 'Result Title' }) %> + <% if full_record_url(result) %> + <%= link_to(result[:title], full_record_url(result), data: { content_piece: 'Result Title' }) %> <% else %> <%= result[:title] %> <% end %> @@ -122,7 +122,7 @@ <%# Trigger BrowZine lookup (render inside result-get so injected HTML is part of the flex `.result-get` area and receives expected styles) %> <% if ThirdIron.enabled? && result[:format].downcase == 'journal' && result[:issn].present? %> - <%= render(partial: 'trigger_browzine', locals: { issn: result[:issn] }) %> + <%= render(partial: 'trigger_browzine', locals: { issn: result[:issn], full_record_url: full_record_url(result) }) %> <% end %>

<% end %> diff --git a/app/views/search/_trigger_browzine.html.erb b/app/views/search/_trigger_browzine.html.erb index 5d457174..1a796111 100644 --- a/app/views/search/_trigger_browzine.html.erb +++ b/app/views/search/_trigger_browzine.html.erb @@ -1,6 +1,8 @@ <% return unless ThirdIron.enabled? %> -<% data_url = "/browzine?issn=#{issn}" %> +<% query_params = { issn: issn } %> +<% query_params[:full_record_url] = full_record_url if full_record_url.present? %> +<% data_url = "/browzine?#{query_params.to_query}" %> - <% if @browzine[:browzine_link].present? %> -
- <%= link_to @browzine[:browzine_link][:text], @browzine[:browzine_link][:link], class: 'button libkey-link', data: { matomo_seen: "Results, Browzine Link Seen, Tab: {{getActiveTabName}}", matomo_click: "Results, Browzine Link Engaged, Link: {{getElementText}}", content_piece: @browzine[:browzine_link][:text] } %> -
+<%# Dedicated Browzine endpoint response for journal records. + Rendered when: A Primo journal result with ISSN triggers a Browzine API lookup via /browzine?issn=X&full_record_url=Y + Use case: Primary fulfillment source for journal articles (via trigger_browzine partial from _result_primo.html.erb) + Returns: Browzine link + optional "Full-text options" button (links to Primo record) +%> +<% if ThirdIron.enabled? && @browzine.present? && @browzine[:browzine_link].present? %> + <% if @full_record_url.present? %> + <%= link_to 'Full-text options', @full_record_url, class: 'button libkey-link', data: { matomo_seen: "Results, Full-text Options Link Seen, Tab: {{getActiveTabName}}", matomo_click: "Results, Full-text Options Link Engaged, Link: {{getElementText}}", content_piece: 'Full-text options' } %> <% end %> + +
+ <%= link_to @browzine[:browzine_link][:text], @browzine[:browzine_link][:link], class: 'libkey-link', data: { matomo_seen: "Results, Browzine Link Seen, Tab: {{getActiveTabName}}", matomo_click: "Results, Browzine Link Engaged, Link: {{getElementText}}", content_piece: @browzine[:browzine_link][:text] } %> +
<% end %> diff --git a/app/views/thirdiron/libkey.html.erb b/app/views/thirdiron/libkey.html.erb index 557ccaee..4317686c 100644 --- a/app/views/thirdiron/libkey.html.erb +++ b/app/views/thirdiron/libkey.html.erb @@ -13,10 +13,16 @@ <%= link_to( @libkey[:best_integrator_link][:text], @libkey[:best_integrator_link][:link], class: 'button libkey-link', data: { matomo_seen: "Results, LibKey Link Seen, Tab: {{getActiveTabName}}", matomo_click: "Results, LibKey Link Engaged, Link: {{getElementText}}", content_piece: @libkey[:best_integrator_link][:text] } ) %> <% end %> + <%# Browzine link returned as part of LibKey fulfillment response. + Rendered when: LibKey API includes a browzine_link in its response + Use case: Secondary fulfillment option alongside PDF, HTML, or other direct links from LibKey + Note: Different from dedicated browzine.html.erb which is called directly for journals. + LibKey Browzine link appears when looking up DOI/PMID fulfillment (articles, etc.) + %> <%# Display browzine link if available. This should always display if we have it regardless of other links. %> <% if @libkey[:browzine_link].present? %>
- <%= link_to @libkey[:browzine_link][:text], @libkey[:browzine_link][:link], class: 'button libkey-link', data: { matomo_seen: "Results, Browzine Link Seen, Tab: {{getActiveTabName}}", matomo_click: "Results, Browzine Link Engaged, Link: {{getElementText}}", content_piece: @libkey[:browzine_link][:text] } %> + <%= link_to @libkey[:browzine_link][:text], @libkey[:browzine_link][:link], class: 'libkey-link', data: { matomo_seen: "Results, Browzine Link Seen, Tab: {{getActiveTabName}}", matomo_click: "Results, Browzine Link Engaged, Link: {{getElementText}}", content_piece: @libkey[:browzine_link][:text] } %>
<% end %> diff --git a/test/controllers/thirdiron_controller_test.rb b/test/controllers/thirdiron_controller_test.rb index b51f935a..28a3d1d1 100644 --- a/test/controllers/thirdiron_controller_test.rb +++ b/test/controllers/thirdiron_controller_test.rb @@ -27,14 +27,14 @@ class ThirdironControllerTest < ActionDispatch::IntegrationTest get '/libkey?type=doi&identifier=10.1038/s41567-023-02305-y' assert_response :success - assert_select 'a.button', { count: 3 } + assert_select 'a.libkey-link', { count: 3 } end VCR.use_cassette('libkey pmid') do get '/libkey?type=pmid&identifier=22110403' assert_response :success - assert_select 'a.button', { count: 3 } + assert_select 'a.libkey-link', { count: 3 } end end @@ -111,7 +111,35 @@ class ThirdironControllerTest < ActionDispatch::IntegrationTest get '/browzine?issn=1546170X' assert_response :success + + # Only browzine link, no button since no full_record_url provided + assert_select 'a.button', { count: 0 } + assert_select 'a.libkey-link', { count: 1 } + end + end + + test 'browzine route with full_record_url returns both links' do + VCR.use_cassette('browzine issn') do + get '/browzine?issn=1546170X&full_record_url=https://example.com/full-record' + + assert_response :success + + # Button (Full-text options) and browzine link both have libkey-link class assert_select 'a.button', { count: 1 } + assert_select 'a.button[href="https://example.com/full-record"]' + assert_select 'a.libkey-link', { count: 2 } + end + end + + test 'browzine route ignores unsafe full_record_url values' do + VCR.use_cassette('browzine issn') do + get '/browzine?issn=1546170X&full_record_url=javascript:alert(1)' + + assert_response :success + + # Unsafe URL should not produce a "Full-text options" button + assert_select 'a.button', { count: 0 } + assert_select 'a.libkey-link', { count: 1 } end end diff --git a/test/helpers/results_helper_test.rb b/test/helpers/results_helper_test.rb index 94ab08b7..b82d520a 100644 --- a/test/helpers/results_helper_test.rb +++ b/test/helpers/results_helper_test.rb @@ -191,4 +191,38 @@ class ResultsHelperTest < ActionView::TestCase }) end end + + test 'full_record_url returns the URL when result has a full record link' do + result = { + links: [ + { 'kind' => 'PDF', 'url' => 'https://pdf.example.com' }, + { 'kind' => 'full record', 'url' => 'https://primo.example.com/record' } + ] + } + assert_equal 'https://primo.example.com/record', full_record_url(result) + end + + test 'full_record_url returns nil when result has no links' do + result = {} + assert_nil full_record_url(result) + end + + test 'full_record_url returns nil when result has links but no full record link' do + result = { + links: [ + { 'kind' => 'PDF', 'url' => 'https://pdf.example.com' }, + { 'kind' => 'HTML', 'url' => 'https://html.example.com' } + ] + } + assert_nil full_record_url(result) + end + + test 'full_record_url returns nil when result is nil' do + assert_nil full_record_url(nil) + end + + test 'full_record_url returns nil when result[:links] is nil' do + result = { links: nil } + assert_nil full_record_url(result) + end end