From 630f40ba1115e0b0b9ea9caf273429ddd4f9fdd9 Mon Sep 17 00:00:00 2001 From: Jeremy Prevost Date: Tue, 21 Jan 2025 14:37:53 -0500 Subject: [PATCH 1/2] Update Primo author link encoding Why are these changes being introduced: * Author linking does not encode the author names which sometimes made the links to Primo not work as expected Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/PW-132 How does this address that need: * Encodes the author component of the URL Document any side effects to this change: I didn't look at anything else as we so rarely touch this code I don't remember how a lot of it works and I didn't want to pull strings that caused much more work than we intended at this time. --- app/models/normalize_primo_common.rb | 10 +++++--- test/models/normalize_primo_test.rb | 34 ++++++++++++++-------------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/app/models/normalize_primo_common.rb b/app/models/normalize_primo_common.rb index cf56802e..1b06c247 100644 --- a/app/models/normalize_primo_common.rb +++ b/app/models/normalize_primo_common.rb @@ -53,9 +53,13 @@ def sanitize_authors(authors) end def author_link(author) - [ENV['MIT_PRIMO_URL'], '/discovery/search?query=creator,exact,', - author, '&tab=', ENV['PRIMO_MAIN_VIEW_TAB'], '&search_scope=all&vid=', - ENV['PRIMO_VID']].join + [ENV.fetch('MIT_PRIMO_URL', nil), '/discovery/search?query=creator,exact,', + clean_author(author), '&tab=', ENV.fetch('PRIMO_MAIN_VIEW_TAB', nil), '&search_scope=all&vid=', + ENV.fetch('PRIMO_VID', nil)].join + end + + def clean_author(author) + URI.encode_uri_component(author) end def year diff --git a/test/models/normalize_primo_test.rb b/test/models/normalize_primo_test.rb index 929363fe..86a90c99 100644 --- a/test/models/normalize_primo_test.rb +++ b/test/models/normalize_primo_test.rb @@ -5,8 +5,8 @@ def popcorn VCR.use_cassette('popcorn primo', allow_playback_repeats: true) do raw_query = SearchPrimo.new.search('popcorn', - ENV['PRIMO_BOOK_SCOPE'], 5) - NormalizePrimo.new.to_result(raw_query, ENV['PRIMO_BOOK_SCOPE'], + ENV.fetch('PRIMO_BOOK_SCOPE', nil), 5) + NormalizePrimo.new.to_result(raw_query, ENV.fetch('PRIMO_BOOK_SCOPE', nil), 'popcorn') end end @@ -15,8 +15,8 @@ def monkeys VCR.use_cassette('monkeys primo articles', allow_playback_repeats: true) do raw_query = SearchPrimo.new.search('monkeys', - ENV['PRIMO_ARTICLE_SCOPE'], 5) - NormalizePrimo.new.to_result(raw_query, ENV['PRIMO_ARTICLE_SCOPE'], + ENV.fetch('PRIMO_ARTICLE_SCOPE', nil), 5) + NormalizePrimo.new.to_result(raw_query, ENV.fetch('PRIMO_ARTICLE_SCOPE', nil), 'monkeys') end end @@ -28,8 +28,8 @@ def missing_fields VCR.use_cassette('missing fields primo', allow_playback_repeats: true) do raw_query = SearchPrimo.new.search('Chʻomsŭkʻi, kkŭt ŏmnŭn tojŏn', - ENV['PRIMO_BOOK_SCOPE'], 5) - NormalizePrimo.new.to_result(raw_query, ENV['PRIMO_BOOK_SCOPE'], + ENV.fetch('PRIMO_BOOK_SCOPE', nil), 5) + NormalizePrimo.new.to_result(raw_query, ENV.fetch('PRIMO_BOOK_SCOPE', nil), 'Chʻomsŭkʻi, kkŭt ŏmnŭn tojŏn') end end @@ -38,8 +38,8 @@ def missing_fields VCR.use_cassette('no results primo', allow_playback_repeats: true) do raw_query = SearchPrimo.new.search('popcornandorangejuice', - ENV['PRIMO_BOOK_SCOPE'], 5) - query = NormalizePrimo.new.to_result(raw_query, ENV['PRIMO_BOOK_SCOPE'], + ENV.fetch('PRIMO_BOOK_SCOPE', nil), 5) + query = NormalizePrimo.new.to_result(raw_query, ENV.fetch('PRIMO_BOOK_SCOPE', nil), 'popcornandorangejuice') assert_equal(0, query['total']) end @@ -79,7 +79,7 @@ def missing_fields result = popcorn['results'].first assert_not_equal 'Rudolph, J.$$QRudolph, J.', result.authors.first.first assert_equal ['Rudolph, J.', - 'https://mit.primo.exlibrisgroup.com/discovery/search?query=creator,exact,Rudolph, J.&tab=all&search_scope=all&vid=FAKE_PRIMO_VID'], + 'https://mit.primo.exlibrisgroup.com/discovery/search?query=creator,exact,Rudolph%2C%20J.&tab=all&search_scope=all&vid=FAKE_PRIMO_VID'], result.authors.first end @@ -87,39 +87,39 @@ def missing_fields result = monkeys['results'].second assert_not_equal ['Beran, Michael J ; Smith, J. David'], result.authors.first.first assert_equal ['Beran, Michael J', - 'https://mit.primo.exlibrisgroup.com/discovery/search?query=creator,exact,Beran, Michael J&tab=all&search_scope=all&vid=FAKE_PRIMO_VID'], + 'https://mit.primo.exlibrisgroup.com/discovery/search?query=creator,exact,Beran%2C%20Michael%20J&tab=all&search_scope=all&vid=FAKE_PRIMO_VID'], result.authors.first assert_equal ['Smith, J. David', - 'https://mit.primo.exlibrisgroup.com/discovery/search?query=creator,exact,Smith, J. David&tab=all&search_scope=all&vid=FAKE_PRIMO_VID'], + 'https://mit.primo.exlibrisgroup.com/discovery/search?query=creator,exact,Smith%2C%20J.%20David&tab=all&search_scope=all&vid=FAKE_PRIMO_VID'], result.authors.second end test 'cleans up single author data in the expected Alma format' do - normalizer = NormalizePrimoCommon.new('record', ENV['PRIMO_BOOK_SCOPE']) + normalizer = NormalizePrimoCommon.new('record', ENV.fetch('PRIMO_BOOK_SCOPE', nil)) authors = ['Evans, Bill$$QEvans, Bill'] assert_equal ['Evans, Bill'], normalizer.sanitize_authors(authors) end test 'cleans up multiple author data in the expected Alma format' do - normalizer = NormalizePrimoCommon.new('record', ENV['PRIMO_BOOK_SCOPE']) + normalizer = NormalizePrimoCommon.new('record', ENV.fetch('PRIMO_BOOK_SCOPE', nil)) authors = ['Blakey, Art$$QBlakey, Art', 'Shorter, Wayne$$QShorter, Wayne'] assert_equal ['Blakey, Art', 'Shorter, Wayne'], normalizer.sanitize_authors(authors) end test 'cleans up multiple author data in the expected CDI format' do - normalizer = NormalizePrimoCommon.new('record', ENV['PRIMO_ARTICLE_SCOPE']) + normalizer = NormalizePrimoCommon.new('record', ENV.fetch('PRIMO_ARTICLE_SCOPE', nil)) authors = ['Blakey, Art ; Shorter, Wayne'] assert_equal ['Blakey, Art', 'Shorter, Wayne'], normalizer.sanitize_authors(authors) end test 'does not attempt to clean up acceptable single author data' do - normalizer = NormalizePrimoCommon.new('record', ENV['PRIMO_BOOK_SCOPE']) + normalizer = NormalizePrimoCommon.new('record', ENV.fetch('PRIMO_BOOK_SCOPE', nil)) authors = ['Redman, Joshua'] assert_equal ['Redman, Joshua'], normalizer.sanitize_authors(authors) end test 'does not attempt to clean up acceptable multiple author data' do - normalizer = NormalizePrimoCommon.new('record', ENV['PRIMO_BOOK_SCOPE']) + normalizer = NormalizePrimoCommon.new('record', ENV.fetch('PRIMO_BOOK_SCOPE', nil)) authors = ['Redman, Joshua', 'Mehldau, Brad'] assert_equal ['Redman, Joshua', 'Mehldau, Brad'], normalizer.sanitize_authors(authors) end @@ -132,7 +132,7 @@ def missing_fields test 'can handle bad results' do assert_raises NormalizePrimo::InvalidResults do - NormalizePrimo.new.to_result('', ENV['PRIMO_BOOK_SCOPE'], 'popcorn') + NormalizePrimo.new.to_result('', ENV.fetch('PRIMO_BOOK_SCOPE', nil), 'popcorn') end end end From 9e51fd8702e36132d2f543f05f8cfe06386df283 Mon Sep 17 00:00:00 2001 From: Jeremy Prevost Date: Tue, 21 Jan 2025 15:17:34 -0500 Subject: [PATCH 2/2] Improve method name Code review identified two method names that were likely to be misleading in the future. This updates the newly introduced method to be less similar to the existing method. --- app/models/normalize_primo_common.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/models/normalize_primo_common.rb b/app/models/normalize_primo_common.rb index 1b06c247..552811ee 100644 --- a/app/models/normalize_primo_common.rb +++ b/app/models/normalize_primo_common.rb @@ -52,13 +52,15 @@ def sanitize_authors(authors) authors.map { |author| author.strip.gsub(/\$\$Q.*$/, '') } end + # author_link constructs a link to Primo as an exact creator search def author_link(author) [ENV.fetch('MIT_PRIMO_URL', nil), '/discovery/search?query=creator,exact,', - clean_author(author), '&tab=', ENV.fetch('PRIMO_MAIN_VIEW_TAB', nil), '&search_scope=all&vid=', + encode_author(author), '&tab=', ENV.fetch('PRIMO_MAIN_VIEW_TAB', nil), '&search_scope=all&vid=', ENV.fetch('PRIMO_VID', nil)].join end - def clean_author(author) + # encode_author ensures author components are URI encoded + def encode_author(author) URI.encode_uri_component(author) end