Skip to content

Commit

Permalink
Remove AccessPatron
Browse files Browse the repository at this point in the history
- No longer used

Closes #4330
  • Loading branch information
maxkadel committed Sep 13, 2024
1 parent c3151fe commit e2ab8d3
Show file tree
Hide file tree
Showing 20 changed files with 40 additions and 106 deletions.
16 changes: 2 additions & 14 deletions app/controllers/requests/form_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def aeon?
# will post and a JSON document of selected "requestable" objects with selection parameters and
# user information for further processing and distribution to various request endpoints.
def submit
@submission = Requests::Submission.new(sanitize_submission(params), Patron.new(user: current_or_guest_user, session:))
@submission = Requests::Submission.new(sanitize_submission(params), Patron.new(user: current_or_guest_user))
respond_to do |format|
format.js do
valid = @submission.valid?
Expand Down Expand Up @@ -104,23 +104,11 @@ def respond_to_validation_error(submission)
end

def authorize_patron(user)
patron = Patron.new(user:, session:)
patron = Patron.new(user:)
flash.now[:error] = patron.errors.join(", ") if patron.errors.present?
patron
end

def current_or_guest_user
user = super

# store guest user information in the session for later
# TODO: Do we still hit this code? How do the user_name and email end up in the request params?
if user.guest? && params[:request].present? && params[:request][:user_name].present?
session["user_name"] = params[:request][:user_name]
session["email"] = params[:request][:email]
end
user
end

def sanitize(str)
str.gsub(/[^A-Za-z0-9@\-_\.]/, '') if str.is_a? String
str
Expand Down
20 changes: 0 additions & 20 deletions app/models/requests/access_patron.rb

This file was deleted.

9 changes: 3 additions & 6 deletions app/models/requests/patron.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,14 @@

module Requests
class Patron
attr_reader :user, :session, :patron_hash, :errors
attr_reader :user, :patron_hash, :errors

delegate :guest?, :provider, :cas_provider?, :alma_provider?, to: :user

def initialize(user:, session: {}, patron_hash: nil)
def initialize(user:, patron_hash: nil)
@user = user
@session = session
@errors = []
# load the patron_hash from bibdata unless we are passing it in
# load the patron_hash from bibdata or alma unless we are passing it in
@patron_hash = patron_hash || load_patron(user:)
end

Expand Down Expand Up @@ -96,8 +95,6 @@ def load_patron(user:)
# Uncomment to fake being a non barcoded user
# patron_hash[:barcode] = nil
patron_hash || {}
elsif session["email"].present? && session["user_name"].present?
AccessPatron.new(session:).hash
else
{}
end
Expand Down
4 changes: 2 additions & 2 deletions spec/helpers/requests/application_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"university_id" => "9999999", "patron_group" => "staff", "patron_id" => "99999", "active_email" => "foo@princeton.edu" }.with_indifferent_access
end
let(:patron) do
Requests::Patron.new(user:, session: {}, patron_hash: valid_patron)
Requests::Patron.new(user:, patron_hash: valid_patron)
end

describe '#isbn_string' do
Expand Down Expand Up @@ -127,7 +127,7 @@

describe '#suppress_login' do
let(:unauthenticated_patron) { FactoryBot.build(:unauthenticated_patron) }
let(:patron) { Requests::Patron.new(user: unauthenticated_patron, session: {}) }
let(:patron) { Requests::Patron.new(user: unauthenticated_patron) }
let(:params) do
{
system_id: '9973529363506421',
Expand Down
4 changes: 2 additions & 2 deletions spec/mailers/requests/request_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@
let(:user_info) do
stub_request(:get, "#{Requests.config[:bibdata_base]}/patron/foo?ldap=true").to_return(status: 200, body: valid_patron_response, headers: {})
user = instance_double(User, guest?: false, uid: 'foo', alma_provider?: false)
Requests::Patron.new(user:, session: {})
Requests::Patron.new(user:)
end

let(:guest_user_info) do
user = instance_double(User, guest?: true, uid: 'foo')
Requests::Patron.new(user:, session: { "email" => "guest@foo.edu", 'user_name' => 'Guest Request' })
Requests::Patron.new(user:)
end

before { stub_delivery_locations }
Expand Down
2 changes: 1 addition & 1 deletion spec/models/concerns/requests/scsb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"patron_id" => "99999", "active_email" => "foo@princeton.edu" }.with_indifferent_access
end
let(:patron) do
Requests::Patron.new(user:, session: {}, patron_hash: valid_patron)
Requests::Patron.new(user:, patron_hash: valid_patron)
end
let(:scsb_no_format) { fixture('/SCSB-7935196.json') }
let(:location_code) { 'scsbnypl' }
Expand Down
14 changes: 0 additions & 14 deletions spec/models/requests/access_patron_spec.rb

This file was deleted.

4 changes: 2 additions & 2 deletions spec/models/requests/clancy_item_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"university_id" => "9999999", "patron_group" => "staff", "patron_id" => "99999", "active_email" => "foo@princeton.edu" }.with_indifferent_access
end
let(:patron) do
Requests::Patron.new(user:, session: {}, patron_hash: valid_patron)
Requests::Patron.new(user:, patron_hash: valid_patron)
end

before do
Expand Down Expand Up @@ -153,7 +153,7 @@
"university_id" => "9999999", "patron_group" => "staff", "patron_id" => "99999", "active_email" => "foo@princeton.edu" }.with_indifferent_access
end
let(:patron) do
Requests::Patron.new(user:, session: {}, patron_hash: valid_patron)
Requests::Patron.new(user:, patron_hash: valid_patron)
end

before do
Expand Down
4 changes: 2 additions & 2 deletions spec/models/requests/form_decorator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
ldap: }.with_indifferent_access
end
let(:campus_unauthorized_patron) { File.open('spec/fixtures/bibdata_patron_unauth_response.json') }
let(:patron) { Requests::Patron.new(user:, session: {}, patron_hash: test_patron) }
let(:patron) { Requests::Patron.new(user:, patron_hash: test_patron) }

let(:requestable) { instance_double(Requests::RequestableDecorator, stubbed_questions) }
let(:hidden_field_metadata) do
Expand Down Expand Up @@ -67,7 +67,7 @@

context 'user not yet logged in' do
let(:user) { FactoryBot.build(:user, guest: true, uid: nil) }
let(:patron) { Requests::Patron.new(user:, session: {}, patron_hash: HashWithIndifferentAccess.new) }
let(:patron) { Requests::Patron.new(user:, patron_hash: HashWithIndifferentAccess.new) }
it 'does not show a message' do
expect(decorator.patron_message).to eq ""
end
Expand Down
2 changes: 1 addition & 1 deletion spec/models/requests/form_no_vcr_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"patron_id" => "99999", "active_email" => "foo@princeton.edu" }.with_indifferent_access
end
let(:patron) do
Requests::Patron.new(user:, session: {}, patron_hash: valid_patron)
Requests::Patron.new(user:, patron_hash: valid_patron)
end
context "with an object with a LOT of items" do
let(:document_id) { '9933643713506421' }
Expand Down
2 changes: 1 addition & 1 deletion spec/models/requests/form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"patron_id" => "99999", "active_email" => "foo@princeton.edu" }.with_indifferent_access
end
let(:patron) do
Requests::Patron.new(user:, session: {}, patron_hash: valid_patron)
Requests::Patron.new(user:, patron_hash: valid_patron)
end

context "with a bad system_id" do
Expand Down
2 changes: 1 addition & 1 deletion spec/models/requests/illiad_patron_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
let(:ldap_data) { { uid: 'foo', department: 'Library - Information Technology', address: 'Firestone Library$Library Information Technology', telephone: '123-456-7890', surname: 'Doe', givenname: 'Joe', email: 'joe@abc.com', pustatus: 'fac', status: 'faculty' }.with_indifferent_access }
let(:user_info) do
user = instance_double(User, guest?: false, uid: 'foo')
Requests::Patron.new(user:, session: {}, patron_hash: valid_patron)
Requests::Patron.new(user:, patron_hash: valid_patron)
end

let(:illiad_patron) { described_class.new(user_info) }
Expand Down
2 changes: 1 addition & 1 deletion spec/models/requests/illiad_transaction_client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
let(:valid_patron) { { "netid" => "abc234", ldap: { status: "faculty", pustatus: "fac" } }.with_indifferent_access }
let(:user_info) do
user = instance_double(User, guest?: false, uid: 'foo')
Requests::Patron.new(user:, session: {}, patron_hash: valid_patron)
Requests::Patron.new(user:, patron_hash: valid_patron)
end
let(:requestable) do
[{ "selected" => "true", "bibid" => "10921934", "mfhd" => "22241110470006421", "call_number" => "HF1131 .B485",
Expand Down
43 changes: 13 additions & 30 deletions spec/models/requests/patron_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@
# rubocop:disable RSpec/MultipleExpectations
describe Requests::Patron, requests: true do
subject(:patron) do
described_class.new(user:, session:, patron_hash: patron_values)
end

let(:session) do
{}
described_class.new(user:, patron_hash: patron_values)
end
let(:first_name) { "Test" }
let(:patron_values) do
Expand Down Expand Up @@ -39,33 +35,20 @@
.to_return(status: 200, body: patron_with_multiple_barcodes, headers: { "Content-Type" => "application/json" })
end

it 'creates an access patron with the active barcode' do
patron = described_class.new(user:, session:)
it 'creates a patron with the active barcode' do
patron = described_class.new(user:)
expect(patron.barcode).to eq('77777777')
end
end
context 'When an access patron visits the site' do
describe '#access_patron' do
it 'creates an access patron with required access attributes' do
patron = described_class.new(user: instance_double(User, guest?: true),
session: { email: 'foo@bar.com', user_name: 'foobar' }.with_indifferent_access)
expect(patron).to be_truthy
expect(patron.active_email).to eq('foo@bar.com')
expect(patron.last_name).to eq('foobar')
expect(patron.barcode).to eq('ACCESS')
expect(patron.eligible_for_library_services?).to be_truthy
end
end
end

context 'A user with a valid princeton net id patron record' do
describe '#patron' do
before do
stub_request(:get, "#{Requests.config[:bibdata_base]}/patron/foo?ldap=true")
.to_return(status: 200, body: valid_patron_response, headers: {})
end
it 'Handles an authorized princeton net ID holder' do
patron = described_class.new(user:,
session: { email: 'foo@bar.com', user_name: 'foobar' }.with_indifferent_access)
patron = described_class.new(user:)
expect(patron).to be_truthy
expect(patron.active_email).to eq('a@b.com')
expect(patron.netid).to eq('jstudent')
Expand All @@ -84,7 +67,7 @@
.to_return(status: 200, body: valid_barcode_patron_response, headers: {})
end
it 'Handles an authorized princeton net ID holder' do
patron = described_class.new(user:, session: {})
patron = described_class.new(user:)
expect(patron).to be_truthy
expect(patron.active_email).to eq('a@b.com')
expect(patron.netid).to be_nil
Expand All @@ -99,7 +82,7 @@
.to_return(status: 404, body: invalid_patron_response, headers: {})
end
it 'Handles an authorized princeton net ID holder' do
patron = described_class.new(user:, session: {})
patron = described_class.new(user:)
expect(patron.errors).to eq(["A problem occurred looking up your library account."])
end
end
Expand All @@ -111,7 +94,7 @@
.to_return(status: 403, body: invalid_patron_response, headers: {})
end
it 'Handles an authorized princeton net ID holder' do
patron = described_class.new(user:, session: {})
patron = described_class.new(user:)
expect(patron.errors).to eq(["A problem occurred looking up your library account."])
end
end
Expand All @@ -123,7 +106,7 @@
.to_return(status: 500, body: invalid_patron_response, headers: {})
end
it 'cannot return a patron record' do
patron = described_class.new(user:, session: {})
patron = described_class.new(user:)
expect(patron.errors).to eq(["A problem occurred looking up your library account."])
end
end
Expand Down Expand Up @@ -154,7 +137,7 @@

it 'logs the error' do
allow(Rails.logger).to receive(:error)
described_class.new(user:, session: {})
described_class.new(user:)
expect(Rails.logger).to have_received(:error).with("Unable to connect to #{bibdata_uri}/patron/foo?ldap=true")
end
end
Expand All @@ -172,7 +155,7 @@
end
it 'logs the error' do
allow(Rails.logger).to receive(:error)
described_class.new(user:, session: {})
described_class.new(user:)
expect(Rails.logger).to have_received(:error).with("#{patron_url} returned an invalid patron response: <html><head<title>Request Rejected</title></head><html>")
end
end
Expand All @@ -189,13 +172,13 @@
end
it 'logs the error' do
allow(Rails.logger).to receive(:error)
described_class.new(user:, session: {})
described_class.new(user:)
expect(Rails.logger).to have_received(:error).with("#{bibdata_uri}/patron/foo?ldap=true returned an empty patron response")
end
end
context 'Passing in patron information instead of loading it from bibdata' do
it "does not call to bibdata" do
patron = described_class.new(user: instance_double(User, guest?: false, uid: 'foo'), session: {}, patron_hash: { barcode: "1234567890" })
patron = described_class.new(user: instance_double(User, guest?: false, uid: 'foo'), patron_hash: { barcode: "1234567890" })
expect(patron.barcode).to eq('1234567890')
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/models/requests/requestable_decorator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"patron_id" => "99999", "active_email" => "foo@princeton.edu",
ldap: }.with_indifferent_access
end
let(:patron) { Requests::Patron.new(user:, session: {}, patron_hash: valid_patron) }
let(:patron) { Requests::Patron.new(user:, patron_hash: valid_patron) }

let(:requestable) { instance_double(Requests::Requestable, stubbed_questions) }
let(:default_stubbed_questions) { { patron:, item_data?: true, circulates?: true, on_shelf?: false, recap?: false, annex?: false, holding_library_in_library_only?: false, scsb_in_library_use?: false, on_order?: false, in_process?: false, aeon?: false, ill_eligible?: false, clancy_available?: false, held_at_marquand_library?: false, item_at_clancy?: false, cul_avery?: false, eligible_for_library_services?: true } }
Expand Down
6 changes: 3 additions & 3 deletions spec/models/requests/requestable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@
"patron_id" => "99999", "active_email" => "foo@princeton.edu" }.with_indifferent_access
end
let(:patron) do
Requests::Patron.new(user:, session: {}, patron_hash: valid_patron)
Requests::Patron.new(user:, patron_hash: valid_patron)
end
let(:params) do
{
Expand Down Expand Up @@ -727,7 +727,7 @@
"patron_id" => "99999", "active_email" => "foo@princeton.edu" }.with_indifferent_access
end
let(:patron) do
Requests::Patron.new(user:, session: {}, patron_hash: valid_patron)
Requests::Patron.new(user:, patron_hash: valid_patron)
end
let(:params) do
{
Expand Down Expand Up @@ -798,7 +798,7 @@
let(:valid_patron_response) { '{"netid":"foo","first_name":"Foo","last_name":"Request","barcode":"22101007797777","university_id":"9999999","patron_group":"staff","patron_id":"99999","active_email":"foo@princeton.edu"}' }
let(:patron) do
stub_request(:get, "#{Requests.config[:bibdata_base]}/patron/foo?ldap=true").to_return(status: 200, body: valid_patron_response, headers: {})
Requests::Patron.new(user:, session: {})
Requests::Patron.new(user:)
end
let(:params) do
{
Expand Down
2 changes: 1 addition & 1 deletion spec/models/requests/router_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
let(:user) { FactoryBot.create(:user) }
let(:valid_patron) { { "netid" => "foo" }.with_indifferent_access }
let(:patron) do
Requests::Patron.new(user:, session: {}, patron_hash: valid_patron)
Requests::Patron.new(user:, patron_hash: valid_patron)
end

let(:scsb_single_holding_item) { fixture('/SCSB-2635660.json') }
Expand Down
2 changes: 1 addition & 1 deletion spec/models/requests/submission_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
end
let(:user_info) do
user = instance_double(User, guest?: false, uid: 'foo')
Requests::Patron.new(user:, session: {}, patron_hash: valid_patron)
Requests::Patron.new(user:, patron_hash: valid_patron)
end

context 'A valid submission' do
Expand Down
2 changes: 1 addition & 1 deletion spec/models/requests/submissions/hold_item_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
let(:valid_patron) { { "netid" => "foo", university_id: "99999999" }.with_indifferent_access }
let(:user_info) do
user = instance_double(User, guest?: false, uid: 'foo')
Requests::Patron.new(user:, session: {}, patron_hash: valid_patron)
Requests::Patron.new(user:, patron_hash: valid_patron)
end

let(:requestable) do
Expand Down
Loading

0 comments on commit e2ab8d3

Please sign in to comment.