From 796b5dd75d2c58d56cecffaed7241a512d8a543e Mon Sep 17 00:00:00 2001 From: jazairi <16103405+jazairi@users.noreply.github.com> Date: Fri, 15 May 2026 12:57:53 -0700 Subject: [PATCH] Trigger OpenAlex lookups on articles only Why these changes are being introduced: OpenAlex lookups currently triggers on non-article content types (e.g., Research Databases, Journals, eBooks). This can result in incorrect or misleading fulfillment links. Relevant ticket(s): - [USE-503](https://mitlibraries.atlassian.net/browse/USE-503) How this addresses that need: - Adds article? helper to check if format matches 'article' (catches Article, Magazine Article, Newsletter Article, etc.) - Passes format parameter through LibKey request/view chain Side effects of this change: We'll see fewer OpenAlex links, but hopefully more accurate ones. --- app/controllers/thirdiron_controller.rb | 1 + app/helpers/results_helper.rb | 10 ++++++ app/views/search/_result_primo.html.erb | 7 ++-- app/views/search/_trigger_libkey.html.erb | 4 ++- app/views/thirdiron/libkey.html.erb | 3 +- test/controllers/thirdiron_controller_test.rb | 14 ++++++-- test/helpers/results_helper_test.rb | 33 +++++++++++++++++++ 7 files changed, 65 insertions(+), 7 deletions(-) diff --git a/app/controllers/thirdiron_controller.rb b/app/controllers/thirdiron_controller.rb index 6d46a0fb..11f8c18f 100644 --- a/app/controllers/thirdiron_controller.rb +++ b/app/controllers/thirdiron_controller.rb @@ -7,6 +7,7 @@ def libkey @libkey = Libkey.lookup(type: params[:type], identifier: params[:identifier]) @doi = params[:type] == 'doi' ? params[:identifier] : nil @pmid = params[:type] == 'pmid' ? params[:identifier] : nil + @format = params[:format] end def browzine diff --git a/app/helpers/results_helper.rb b/app/helpers/results_helper.rb index 1d1f07f9..e5108c67 100644 --- a/app/helpers/results_helper.rb +++ b/app/helpers/results_helper.rb @@ -58,4 +58,14 @@ def search_worldcat_link 'https://mit.on.worldcat.org/search?' + worldcat_params end + + # Determines if a format value represents an article type + # + # @param format [String] The format value to check (e.g., 'Article', 'Magazine Article') + # @return [Boolean] True if format contains 'article' as a whole word (case-insensitive), false otherwise + def article?(format) + return false if format.blank? + + format.match?(/\barticle\b/i) + end end diff --git a/app/views/search/_result_primo.html.erb b/app/views/search/_result_primo.html.erb index 328e85f1..b51d2618 100644 --- a/app/views/search/_result_primo.html.erb +++ b/app/views/search/_result_primo.html.erb @@ -86,13 +86,14 @@ <%# LibKey links are our preferred links so we call the API when possible to get these preferred links %> <% if ThirdIron.enabled? && result[:doi].present? %> - <%= render(partial: 'trigger_libkey', locals: { kind: 'doi', identifier: result[:doi] }) %> + <%= render(partial: 'trigger_libkey', locals: { kind: 'doi', identifier: result[:doi], format: result[:format] }) %> <% elsif ThirdIron.enabled? && result[:pmid].present? %> - <%= render(partial: 'trigger_libkey', locals: { kind: 'pmid', identifier: result[:pmid] }) %> + <%= render(partial: 'trigger_libkey', locals: { kind: 'pmid', identifier: result[:pmid], format: result[:format] }) %> <% end %> <%# OpenAccess Links: If OA_ALWAYS is enabled, include them when available. If not we'll trigger when we have a DOI/PMID but LibKey does not return data. See app/views/thirdiron/libkey.html.erb %> - <% if Feature.enabled?(:oa_always) %> + <%# Only show OpenAccess links for articles %> + <% if Feature.enabled?(:oa_always) && article?(result[:format]) %> <%= render(partial: 'trigger_openalex_setup', locals: { doi: result[:doi], pmid: result[:pmid] }) %> <% end %> diff --git a/app/views/search/_trigger_libkey.html.erb b/app/views/search/_trigger_libkey.html.erb index c2ed071f..5ae30edc 100644 --- a/app/views/search/_trigger_libkey.html.erb +++ b/app/views/search/_trigger_libkey.html.erb @@ -1,6 +1,8 @@ <% return unless ThirdIron.enabled? %> -<% data_url = "/libkey?type=#{kind}&identifier=#{identifier}" %> +<% query_params = { type: kind, identifier: identifier } %> +<% query_params[:format] = format if format.present? %> +<% data_url = "/libkey?#{query_params.to_query}" %> <%# If we didn't get data back from LibKey, we try OpenAlex unless OA_ALWAYS is enabled at which point we would have already requested data from OpenAlex in parallel to LibKey %> -<% elsif @libkey.blank? && !Feature.enabled?(:oa_always) %> +<%# Only show OpenAlex links for articles %> +<% elsif @libkey.blank? && !Feature.enabled?(:oa_always) && article?(@format) %> <%= render(partial: 'search/trigger_openalex_setup', locals: { doi: @doi, pmid: @pmid }) %> <% end %> diff --git a/test/controllers/thirdiron_controller_test.rb b/test/controllers/thirdiron_controller_test.rb index 6f71db24..b51f935a 100644 --- a/test/controllers/thirdiron_controller_test.rb +++ b/test/controllers/thirdiron_controller_test.rb @@ -41,7 +41,7 @@ class ThirdironControllerTest < ActionDispatch::IntegrationTest test 'libkey for non-existent identifier inserts openalex setup when oa_always is false' do # Libkey responds here, so we have a cassette - but the response is empty VCR.use_cassette('libkey nonexistent') do - get '/libkey?type=doi&identifier=foobar' + get '/libkey?type=doi&identifier=foobar&format=Article' assert_response :success assert_select '.openalex-container', { count: 1 } @@ -52,7 +52,7 @@ class ThirdironControllerTest < ActionDispatch::IntegrationTest ClimateControl.modify(FEATURE_OA_ALWAYS: 'true') do # Libkey responds here, so we have a cassette - but the response is empty VCR.use_cassette('libkey nonexistent') do - get '/libkey?type=doi&identifier=foobar' + get '/libkey?type=doi&identifier=foobar&format=Article' assert_response :success assert response.body.blank? @@ -60,6 +60,16 @@ class ThirdironControllerTest < ActionDispatch::IntegrationTest end end + test 'libkey for non-existent identifier does not insert OpenAlex setup for non-article formats' do + # OpenAlex should only appear for articles + VCR.use_cassette('libkey nonexistent') do + get '/libkey?type=doi&identifier=foobar&format=Book' + + assert_response :success + assert_select '.openalex-container', { count: 0 } + end + end + test 'libkey no response when either env var is not set' do # No cassette because this never results in traffic to Libkey ClimateControl.modify(THIRDIRON_ID: nil) do diff --git a/test/helpers/results_helper_test.rb b/test/helpers/results_helper_test.rb index 1d0dcc1e..1d493294 100644 --- a/test/helpers/results_helper_test.rb +++ b/test/helpers/results_helper_test.rb @@ -90,4 +90,37 @@ class ResultsHelperTest < ActionView::TestCase assert link.start_with?('https://mit.on.worldcat.org/search?') assert_includes link, 'queryString=climate+change' end + + test 'article? returns true for Article format' do + assert article?('Article') + assert article?('Journal Article') + assert article?('Newspaper Article') + end + + test 'article? returns true for lowercase article formats' do + assert article?('article') + assert article?('journal article') + assert article?('newspaper article') + end + + test 'article? returns true for mixed case article formats' do + assert article?('ARTICLE') + assert article?('Article') + assert article?('JoUrNaL ArTiClE') + end + + test 'article? returns false for non-article formats' do + assert_not article?('Journal') + assert_not article?('eBook') + assert_not article?('Book Chapter') + assert_not article?('Conference Proceeding') + assert_not article?('Reference Entry') + assert_not article?('Research Database') + assert_not article?('Dataset') + end + + test 'article? returns false for blank or nil format' do + assert_not article?(nil) + assert_not article?('') + end end