Merge pull request #390 from MITLibraries/use-556-skipDelivery

Safe Primo link parsing

Author: JPrevost

README.md

@@ -129,6 +129,7 @@ may have unexpected consequences if applied to other TIMDEX UI apps.
 - `ORIGINS`: sets origins for CORS (currently used only for TACOS API calls).
 - `PLATFORM_NAME`: The value set is added to the header after the MIT Libraries logo. The logic and CSS for this comes from our theme gem.
 - `PRIMO_NDE_VID`: The Primo view ID for NDE Only required if `FEATURE_PRIMO_NDE_LINKS` is enabled. Ask Enterprise Systems for value.
+- `PRIMO_SKIP_DELIVERY`: default is `true`. Determines if `skipDelivery=true` or `skipDelivery=false` is added to Primo API calls. This affects performance and possibly responses.
 - `PRIMO_TIMEOUT`: The number of seconds before a Primo request times out (default 6).
 - `REQUESTS_PER_PERIOD` - number of requests that can be made for general throttles per `REQUEST_PERIOD`
 - `REQUEST_PERIOD` - time in minutes used along with `REQUESTS_PER_PERIOD`

app/models/normalize_primo_record.rb

@@ -114,7 +114,7 @@ def links
 
       parsed_string = parse_link_string(@record['pnx']['links']['linktopdf'].first)
 
-      if parsed_string['U'].present?
+      if parsed_string&.dig('U').present?
         links << { 'url' => parsed_string['U'],
                    'kind' => 'Get PDF' }
       end
@@ -125,7 +125,7 @@ def links
 
       parsed_string = parse_link_string(@record['pnx']['links']['linktohtml'].first)
 
-      if parsed_string['U'].present?
+      if parsed_string&.dig('U').present?
         links << { 'url' => parsed_string['U'],
                    'kind' => 'Read online' }
       end
@@ -323,6 +323,7 @@ def availability
   end
 
   def other_availability
+    return unless @record['delivery']
     return unless @record['delivery']['bestlocation']
     return unless @record['delivery']['holding']
 

app/models/primo_search.rb

@@ -97,6 +97,13 @@ def primo_vid
     ENV.fetch('PRIMO_VID', nil)
   end
 
+  # skip_delivery determines whether to include the delivery parameter in Primo API calls, which affects performance
+  # and in theory results (but we have not observed differences yet). Setting it to true can speed up response times
+  # but may exclude some availability information.
+  def skip_delivery?
+    ENV.fetch('PRIMO_SKIP_DELIVERY', 'true').to_s.downcase == 'true'
+  end
+
   # clean_term performs search term sanitization to prepare the term for querying the Primo API
   #
   # We are using the built in `URI.encode_www_form_component`. Previous versions (including in bento) had us doing
@@ -121,7 +128,8 @@ def search_url(term, per_page, offset = 0)
       primo_scope,
       '&limit=',
       per_page,
-      '&pcAvailability=true'
+      '&pcAvailability=true',
+      "&skipDelivery=#{skip_delivery?}"
     ]
 
     # Add offset parameter for pagination (only if > 0)

test/models/normalize_primo_record_test.rb

@@ -84,6 +84,18 @@ def cdi_record
     assert_empty normalized[:links]
   end
 
+  test 'handles non-$$ formatted pdf and html link strings without raising' do
+    record = full_record.deep_dup
+    record['pnx']['links']['linktopdf'] = ['https://example.com/plain-url.pdf']
+    record['pnx']['links']['linktohtml'] = ['https://example.com/plain-url.html']
+
+    assert_nothing_raised { NormalizePrimoRecord.new(record, 'test').normalize }
+    normalized = NormalizePrimoRecord.new(record, 'test').normalize
+    link_kinds = normalized[:links].map { |l| l['kind'] }
+    assert_not_includes link_kinds, 'Get PDF'
+    assert_not_includes link_kinds, 'Read online'
+  end
+
   test 'parse_link_string creates expected data structure' do
     # Strings that don't start with $$ should not be processed
     link_string = 'https://example.com?param1=value1&param2=value2'