diff --git a/app/controllers/alma_controller.rb b/app/controllers/alma_controller.rb
index ba81d555..7ce551fb 100644
--- a/app/controllers/alma_controller.rb
+++ b/app/controllers/alma_controller.rb
@@ -4,7 +4,9 @@ class AlmaController < ApplicationController
def sru
return unless AlmaSru.enabled? && expected_params?
- @availability = AlmaSru.lookup(params[:doc_id])
+ result = AlmaSru.lookup(params[:doc_id])
+ @availability = result[:availability]
+ @alma_e = result[:alma_e]
end
private
diff --git a/app/javascript/controllers/content_loader_controller.js b/app/javascript/controllers/content_loader_controller.js
index 02450a68..e3af9f05 100644
--- a/app/javascript/controllers/content_loader_controller.js
+++ b/app/javascript/controllers/content_loader_controller.js
@@ -33,6 +33,16 @@ export default class extends Controller {
// Replace the entire element with the fetched HTML, or remove if empty
if (html.trim()) {
this.element.outerHTML = html
+
+ // Keep Alma availability in `.result-content` but place “Full-text options”
+ // with fulfillment links in the descendant `.result-get` container.
+ const resultContent = parentElement.closest('.result-content') || parentElement
+ const almaFulltextOptions = resultContent.querySelector('.alma-fulltext-options')
+ const resultGet = resultContent.querySelector('.result-get')
+ if (almaFulltextOptions && resultGet) {
+ resultGet.appendChild(almaFulltextOptions)
+ }
+
// Hide primo links if libkey link is present
if (parentElement.querySelector('.libkey-link')) {
const resultGet = parentElement.closest('.result-get')
diff --git a/app/models/alma_sru.rb b/app/models/alma_sru.rb
index 7777928a..fa570aa0 100644
--- a/app/models/alma_sru.rb
+++ b/app/models/alma_sru.rb
@@ -20,11 +20,15 @@ class InvalidAlmaId < StandardError; end
# lookup is the primary method of interacting with this model.
#
- # It will receive an Alma ID, validate it, look it up in the Alma SRU, and return a formatted result.
+ # It will receive an Alma ID, validate it, look it up in the Alma SRU, and return availability info and Alma-E status.
#
# It accepts an "alma_client" argument for use when testing, but this is not used in normal operations.
+ #
+ # Returns a hash with:
+ # - :availability => formatted availability statements array
+ # - :alma_e => boolean indicating if record is Alma-E
def self.lookup(raw_identifier, alma_client: nil)
- return [] unless enabled?
+ return { availability: [], alma_e: false } unless enabled?
# Validate the raw identifier received. This will raise an InvalidAlmaId if validation fails.
raise InvalidAlmaId unless valid_alma_id?(raw_identifier)
@@ -41,24 +45,22 @@ def self.lookup(raw_identifier, alma_client: nil)
parse_response(alma_http.timeout(6).get(url), identifier)
rescue InvalidAlmaId
Rails.logger.debug("Invalid Alma ID: #{raw_identifier}")
-
- []
+ { availability: [], alma_e: false }
rescue LookupFailure => e
Rails.logger.debug("Alma lookup failure: #{e}")
-
- []
+ { availability: [], alma_e: false }
rescue HTTP::Error
Sentry.capture_message('Alma SRU connection failure')
Rails.logger.error('Alma SRU connection error')
-
- []
+ { availability: [], alma_e: false }
end
# parse_response receives the raw response from the Alma SRU endpoint.
#
# For any non-200 response, it raises a LookupFailure.
#
- # Other responses (in XML format) are parsed by Nokogiri, and we pluck content with an `AVA` tag.
+ # Other responses (in XML format) are parsed by Nokogiri to extract both AVA (holdings) and AVE (electronic) data.
+ # Returns a hash with :availability and :alma_e keys.
def self.parse_response(raw_response, reference_identifier)
raise LookupFailure, raw_response.status unless raw_response.status == 200
@@ -76,7 +78,15 @@ def self.parse_response(raw_response, reference_identifier)
# Reduce list to a single item if multiples exist
results[0] += ' and other locations' if results.length > 1
- results.first(1)
+ availability = results.first(1)
+
+ # Check for AVE tags to determine if record is Alma-E
+ alma_e = alma_e?(parsed)
+
+ {
+ availability: availability,
+ alma_e: alma_e
+ }
end
# ava_to_hash takes an XML element that represents a single availability record
@@ -112,6 +122,26 @@ def self.fetch_controlfield(parsed_xml)
parsed_xml.xpath("//holding:controlfield[@tag='001']", NAMESPACE)&.text
end
+ # alma_e? receives a parsed XML document (Nokogiri::XML::Document) and returns true if the record
+ # contains AVE (electronic) datafields. We fall back on the 959 subfield b for legacy records.
+ def self.alma_e?(parsed_xml)
+ return true if parsed_xml.xpath("//holding:datafield[@tag='AVE']", NAMESPACE).any?
+
+ legacy_net_access?(parsed_xml)
+ end
+
+ # Some records omit AVE but still indicate electronic access in local 959$b=NET. After consulting
+ # with Metadata and Enterprise Systems, we learned that this a deprecated practice from before the
+ # Alma migration.
+ #
+ # It is still unclear whether Primo is determining electronic access from this subfield, but it's
+ # the only electronic access indicator we can find in the record other than AVE. We can revisit
+ # this approach if it proves ineffective.
+ def self.legacy_net_access?(parsed_xml)
+ parsed_xml.xpath("//holding:datafield[@tag='959']/holding:subfield[@code='b']", NAMESPACE)
+ .any? { |node| node.text.to_s.strip.casecmp('NET').zero? }
+ end
+
# format_availability receives a hash representing a single availability
# statement, and formats it for human readability. Values for "e" and "q" are
# required, while "c" and "d" are optional.
diff --git a/app/models/normalize_primo_record.rb b/app/models/normalize_primo_record.rb
index 8c7a6cc5..dcfe7d52 100644
--- a/app/models/normalize_primo_record.rb
+++ b/app/models/normalize_primo_record.rb
@@ -131,14 +131,6 @@ 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' }
- end
-
# Return links if we found any
links.any? ? links : []
end
diff --git a/app/views/alma/sru.html.erb b/app/views/alma/sru.html.erb
index 10c50e5c..4da03e48 100644
--- a/app/views/alma/sru.html.erb
+++ b/app/views/alma/sru.html.erb
@@ -1,9 +1,15 @@
-<% if AlmaSru.enabled? && @availability.present? %>
-
- <% @availability.each do |statement| %>
-
<%= link_to(sanitize(statement, tags: %w[i strong], attributes: %w[class aria-hidden]),
- "#{PrimoLinkBuilder.new(record_id: params[:doc_id], context: 'L').full_record_link}#getit_link1_0",
- data: {content_piece: 'Availability Link' }) %>
- <% end %>
-
+ <% @availability.each do |statement| %>
+
<%= link_to(sanitize(statement, tags: %w[i strong], attributes: %w[class aria-hidden]),
+ "#{PrimoLinkBuilder.new(record_id: params[:doc_id], context: 'L').full_record_link}#getit_link1_0",
+ data: {content_piece: 'Availability Link' }) %>
+ <% end %>
+
<% if result[:links].present? %>
<% result[:links].each do |link| %>
diff --git a/test/controllers/alma_controller_test.rb b/test/controllers/alma_controller_test.rb
index b6d9c00c..5f1d8da6 100644
--- a/test/controllers/alma_controller_test.rb
+++ b/test/controllers/alma_controller_test.rb
@@ -16,17 +16,27 @@ class AlmaControllerTest < ActionDispatch::IntegrationTest
assert response.body.blank?
end
- test 'alma sru route returns nothing if lookup returns content with no AVA' do
+ test 'alma sru route returns full-text options when Alma-E is true' do
VCR.use_cassette('alma sru no availability') do
needle = 'alma9935053423706761'
get almasru_path(doc_id: needle)
+ assert_response :success
+ assert_select 'a.button', { count: 1, text: 'Full-text options' }
+ end
+ end
+
+ test 'alma sru route returns nothing when lookup has no AVA and no Alma-E' do
+ VCR.use_cassette('alma sru nonexistent record') do
+ needle = 'alma9900000000006761'
+ get almasru_path(doc_id: needle)
+
assert_response :success
assert response.body.blank?
end
end
- test 'alma sru route returns HTML for successful lookup' do
+ test 'alma sru route returns availability for successful lookup' do
VCR.use_cassette('alma sru single record') do
needle = 'alma990014651640106761'
get almasru_path(doc_id: needle)
diff --git a/test/models/alma_sru_test.rb b/test/models/alma_sru_test.rb
index d2eec48a..cfbb65af 100644
--- a/test/models/alma_sru_test.rb
+++ b/test/models/alma_sru_test.rb
@@ -43,8 +43,9 @@ class AlmaSruTest < ActiveSupport::TestCase
assert_equal(
[" Available in Rotch Library Stacks (NA680.C25 2007)"],
- result
+ result[:availability]
)
+ assert_equal false, result[:alma_e]
end
end
@@ -54,79 +55,154 @@ class AlmaSruTest < ActiveSupport::TestCase
result = AlmaSru.lookup(needle)
- assert_equal(1, result.length)
- assert_includes result[0], 'and other locations'
+ assert_equal(1, result[:availability].length)
+ assert_includes result[:availability][0], 'and other locations'
+ assert_equal false, result[:alma_e]
end
end
- test 'lookup returns empty list if no availability' do
+ test 'lookup returns empty availability if no AVA' do
VCR.use_cassette('alma sru no availability') do
needle = 'alma9935053423706761'
result = AlmaSru.lookup(needle)
- assert_equal([], result)
+ assert_equal([], result[:availability])
end
end
+ test 'lookup returns true Alma-E if AVE is present' do
+ # This cassette was generated to demonstrate a record with no AVA, but it has an AVE
+ VCR.use_cassette('alma sru no availability') do
+ needle = 'alma9935053423706761'
+
+ result = AlmaSru.lookup(needle)
+
+ assert_equal true, result[:alma_e]
+ end
+ end
+
+ test 'lookup returns false Alma-E when AVE is absent' do
+ VCR.use_cassette('alma sru single record') do
+ needle = 'alma990014651640106761'
+
+ result = AlmaSru.lookup(needle)
+
+ assert_equal false, result[:alma_e]
+ end
+ end
+
+ test 'alma_e? returns true when AVE is absent but 959 subfield b is NET' do
+ xml_content = <<~XML
+
+
+
+
+
+ 990027661060106761
+
+ MIT Access Only
+ n-mit
+ NET
+ **See URL(s)
+
+
+
+
+
+
+ XML
+
+ parsed = Nokogiri::XML(xml_content)
+ assert_equal true, AlmaSru.alma_e?(parsed)
+ end
+
+ test 'alma_e? returns false when AVE is absent and 959 subfield b is not NET' do
+ xml_content = <<~XML
+
+
+
+
+
+ 990002941700106761
+
+ LSA
+ JRNAL
+
+
+
+
+
+
+ XML
+
+ parsed = Nokogiri::XML(xml_content)
+ assert_equal false, AlmaSru.alma_e?(parsed)
+ end
+
test 'lookup returns empty list for non-existent records' do
VCR.use_cassette('alma sru nonexistent record') do
needle = 'alma9900000000006761'
result = AlmaSru.lookup(needle)
- assert_equal([], result)
+ assert_equal([], result[:availability])
+ assert_equal false, result[:alma_e]
end
end
- test 'lookup returns empty list if alma URL not set' do
+ test 'lookup returns empty availability if alma URL not set' do
needle = 'alma990014651640106761'
VCR.use_cassette('alma sru single record') do
- assert_equal(1, AlmaSru.lookup(needle).length)
+ result = AlmaSru.lookup(needle)
+ assert_equal(1, result[:availability].length)
end
ClimateControl.modify(MIT_ALMA_URL: nil) do
- assert_equal([], AlmaSru.lookup(needle))
+ result = AlmaSru.lookup(needle)
+ assert_equal({ availability: [], alma_e: false }, result)
end
end
- test 'lookup returns empty list if exl_inst_id not set' do
+ test 'lookup returns empty availability if exl_inst_id not set' do
needle = 'alma990014651640106761'
VCR.use_cassette('alma sru single record') do
- assert_equal(1, AlmaSru.lookup(needle).length)
+ result = AlmaSru.lookup(needle)
+ assert_equal(1, result[:availability].length)
end
ClimateControl.modify(EXL_INST_ID: nil) do
AlmaSru.remove_instance_variable(:@enabled)
- assert_equal([], AlmaSru.lookup(needle))
+ result = AlmaSru.lookup(needle)
+ assert_equal({ availability: [], alma_e: false }, result)
end
end
- test 'lookup returns empty list with non-complying ID' do
+ test 'lookup returns empty hash for non-complying ID' do
needle = 'foo'
result = AlmaSru.lookup(needle)
- assert_equal([], result)
+ assert_equal({ availability: [], alma_e: false }, result)
end
- test 'lookup returns empty list with empty string' do
+ test 'lookup returns empty hash with empty string' do
needle = ''
result = AlmaSru.lookup(needle)
- assert_equal([], result)
+ assert_equal({ availability: [], alma_e: false }, result)
end
- test 'lookup returns empty list with nil input' do
+ test 'lookup returns empty hash with nil input' do
needle = nil
result = AlmaSru.lookup(needle)
- assert_equal([], result)
+ assert_equal({ availability: [], alma_e: false }, result)
end
test 'lookup survives failing to connect to Alma SRU' do
@@ -137,7 +213,7 @@ class AlmaSruTest < ActiveSupport::TestCase
assert_nothing_raised do
result = AlmaSru.lookup(needle, alma_client: alma_client)
- assert_equal([], result)
+ assert_equal({ availability: [], alma_e: false }, result)
end
end
@@ -149,7 +225,7 @@ class AlmaSruTest < ActiveSupport::TestCase
assert_nothing_raised do
result = AlmaSru.lookup(needle, alma_client: alma_client)
- assert_equal([], result)
+ assert_equal({ availability: [], alma_e: false }, result)
end
end
diff --git a/test/models/normalize_primo_record_test.rb b/test/models/normalize_primo_record_test.rb
index 35d106e0..8b630bab 100644
--- a/test/models/normalize_primo_record_test.rb
+++ b/test/models/normalize_primo_record_test.rb
@@ -458,20 +458,6 @@ def cdi_record
assert_not normalized[:dedup_record]
end
- test 'includes Full-text options link when pnx[links] is nil and both Alma-P and Alma-E present' do
- record = alma_record.deep_dup
-
- # Ensure no direct links
- record['pnx']['links'] = nil
-
- # Add delivery category with both physical and electronic
- record['delivery']['deliveryCategory'] = %w[Alma-P 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 pnx[links] is present' do
record = full_record.deep_dup
@@ -492,26 +478,6 @@ def cdi_record
assert_nil full_text_link
end
- test 'includes Full-text options link when only Alma-E present' do
- record = alma_record.deep_dup
- record['pnx']['links'] = nil
- record['delivery']['deliveryCategory'] = ['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 no delivery category present' do
- record = alma_record.deep_dup
- record['pnx']['links'] = nil
- record['delivery']['deliveryCategory'] = nil
- 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 'dedup_url requires both frbrized and alma_record conditions' do
# CDI record that is frbrized - should return nil
normalizer = NormalizePrimoRecord.new(cdi_record, 'test')