From 1424aa20280f90cb4bbbff044ee4fe742fbf87bc Mon Sep 17 00:00:00 2001 From: David Zuckerman Date: Mon, 23 Mar 2026 14:11:49 -0700 Subject: [PATCH] Created custom alma_record_not_found_error, raising 403 if Alma patron record is not active or missing --- app/lib/error/alma_record_not_found_error.rb | 6 ++++++ app/models/alma/user.rb | 4 ++++ app/services/alma_services.rb | 16 ++++++++-------- .../patron_not_found_error.html.erb | 2 +- spec/forms_helper.rb | 14 +++++++++++++- spec/models/user_spec.rb | 19 +++++++++++++++++++ 6 files changed, 51 insertions(+), 10 deletions(-) create mode 100644 app/lib/error/alma_record_not_found_error.rb diff --git a/app/lib/error/alma_record_not_found_error.rb b/app/lib/error/alma_record_not_found_error.rb new file mode 100644 index 00000000..05a68df6 --- /dev/null +++ b/app/lib/error/alma_record_not_found_error.rb @@ -0,0 +1,6 @@ +module Error + # Raised when Alma returns a not-found style response. + # Subclassing ActiveRecord::RecordNotFound preserves existing 404 handling. + class AlmaRecordNotFoundError < ActiveRecord::RecordNotFound + end +end diff --git a/app/models/alma/user.rb b/app/models/alma/user.rb index 8412de53..d5e29604 100644 --- a/app/models/alma/user.rb +++ b/app/models/alma/user.rb @@ -42,6 +42,10 @@ def find_if_active(id) find_if_exists(id).tap do |rec| return nil unless rec && rec.active? end + rescue Error::AlmaRecordNotFoundError => e + return if e.message.include?('Alma query failed with response: 404') + + raise end end diff --git a/app/services/alma_services.rb b/app/services/alma_services.rb index aab58c30..422b51a5 100644 --- a/app/services/alma_services.rb +++ b/app/services/alma_services.rb @@ -50,14 +50,14 @@ def authenticate_alma_patron?(alma_user_id, alma_password) def get_user(alma_user_id) params = { view: 'full', expand: 'fees' } connection.get(user_uri_for(alma_user_id), params).tap do |res| - raise ActiveRecord::RecordNotFound, "Alma query failed with response: #{res.status}" unless res.status == 200 + raise Error::AlmaRecordNotFoundError, "Alma query failed with response: #{res.status}" unless res.status == 200 end end def save(alma_user_id, user) # noinspection RubyArgCount connection.put(user_uri_for(alma_user_id), user.to_json, { 'Content-Type' => 'application/json' }).tap do |res| - raise ActiveRecord::RecordNotFound, 'Failed to save.' unless res.status == 200 + raise Error::AlmaRecordNotFoundError, 'Failed to save.' unless res.status == 200 end 'Saved user.' # TODO: what is this for? @@ -85,7 +85,7 @@ def fee_uri_for(alma_id, fee_id) def fetch_all(alma_user_id) res = connection.get(fees_uri_for(alma_user_id)) - raise ActiveRecord::RecordNotFound, 'No fees could be found.' unless res.status == 200 + raise Error::AlmaRecordNotFoundError, 'No fees could be found.' unless res.status == 200 JSON.parse(res.body) end @@ -97,7 +97,7 @@ def credit(alma_user_id, pp_ref_number, fee) payment_uri = URIs.append(fee_uri_for(alma_user_id, fee.id), '?', URI.encode_www_form(params)) connection.post(payment_uri).tap do |res| - raise ActiveRecord::RecordNotFound, "Alma query failed with response: #{res.status}" unless res.status == 200 + raise Error::AlmaRecordNotFoundError, "Alma query failed with response: #{res.status}" unless res.status == 200 end end end @@ -119,14 +119,14 @@ def fetch_sets(env, offset = 0) } res = connection(env).get(URIs.append(alma_api_url, 'conf/sets'), params) - raise ActiveRecord::RecordNotFound, 'No item sets could be found..' unless res.status == 200 + raise Error::AlmaRecordNotFoundError, 'No item sets could be found..' unless res.status == 200 JSON.parse(res.body) end def fetch_set(env, id) res = connection(env).get(URIs.append(alma_api_url, "conf/sets/#{id}")) - raise ActiveRecord::RecordNotFound, "No set with ID #{id} found..." unless res.status == 200 + raise Error::AlmaRecordNotFoundError, "No set with ID #{id} found..." unless res.status == 200 JSON.parse(res.body) end @@ -137,7 +137,7 @@ def fetch_members(set_id, env, offset = 0) limit: 100 } res = connection(env).get(URIs.append(alma_api_url, "conf/sets/#{set_id}/members"), params) - raise ActiveRecord::RecordNotFound, 'No item sets could be found.' unless res.status == 200 + raise Error::AlmaRecordNotFoundError, 'No item sets could be found.' unless res.status == 200 JSON.parse(res.body) end @@ -145,7 +145,7 @@ def fetch_members(set_id, env, offset = 0) def fetch_item(env, mms_id, holding_id, item_pid) uri = URIs.append(alma_api_url, "bibs/#{mms_id}/holdings/#{holding_id}/items/#{item_pid}") res = connection(env).get(uri) - raise ActiveRecord::RecordNotFound, 'Item could be found.' unless res.status == 200 + raise Error::AlmaRecordNotFoundError, 'Item could be found.' unless res.status == 200 JSON.parse(res.body) end diff --git a/app/views/application/patron_not_found_error.html.erb b/app/views/application/patron_not_found_error.html.erb index de70159b..d57f98bc 100644 --- a/app/views/application/patron_not_found_error.html.erb +++ b/app/views/application/patron_not_found_error.html.erb @@ -1,2 +1,2 @@

Forbidden

-

Your patron record cannot be found. Please contact the Privileges Desk.

+

This form is only available to active library patrons. Please contact the Privileges Desk.

diff --git a/spec/forms_helper.rb b/spec/forms_helper.rb index 503ee9c2..a707d89c 100644 --- a/spec/forms_helper.rb +++ b/spec/forms_helper.rb @@ -56,11 +56,23 @@ def show_path(id) with_patron_login(patron_id) do get new_form_path expect(response).to have_http_status(:forbidden) - expected_msg = /Your patron record cannot be found/ + expected_msg = /This form is only available to active library patrons/ expect(response.body).to match(expected_msg) end end + it 'forbids users when active patron lookup returns Alma 404' do + patron_id = Alma::Type.sample_id_for(allowed_patron_types.first) + allow(AlmaServices::Patron).to receive(:get_user).with(patron_id) + .and_raise(Error::AlmaRecordNotFoundError, + 'Alma query failed with response: 404') + + with_patron_login(patron_id) do + get new_form_path + expect(response).to have_http_status(:forbidden) + end + end + describe 'with a valid user' do attr_reader :patron_id attr_reader :patron diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 768d4f49..e3144568 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -235,4 +235,23 @@ end end + describe :primary_patron_record do + it 'returns nil when active patron lookup returns nil' do + user = User.new(uid: 'does_not_exist') + allow(Alma::User).to receive(:find_if_active).with('does_not_exist') + .and_return(nil) + + expect(user.primary_patron_record).to be_nil + end + + it 're-raises active patron lookup errors' do + user = User.new(uid: 'fake_uid') + allow(Alma::User).to receive(:find_if_active).with('fake_uid') + .and_raise(Error::AlmaRecordNotFoundError, + 'Alma query failed with response: 500') + + expect { user.primary_patron_record }.to raise_error(Error::AlmaRecordNotFoundError) + end + end + end