diff --git a/app/controllers/payments_controller.rb b/app/controllers/payments_controller.rb index a44e996f..060e0e31 100644 --- a/app/controllers/payments_controller.rb +++ b/app/controllers/payments_controller.rb @@ -39,9 +39,8 @@ def create # # POST /payments/accept def accept - ils_client.pay_fines(user: cybersource_response.user, - amount: cybersource_response.amount, - session_id: cybersource_response.session_id) + ils_client.pay_fines(user_id: cybersource_response.user_id, + amount: cybersource_response.amount) redirect_to fines_path, flash: { success: (t 'mylibrary.fine_payment.accept_html', amount: cybersource_response.amount) @@ -67,11 +66,11 @@ def payments_json_response end def create_payment_params - params.permit(%I[reason billseq amount session_id user group]) + params.permit(%I[user_id amount]) end def cybersource_request - Cybersource::PaymentRequest.new(**params.permit(:user, :amount, :session_id).to_h.symbolize_keys).sign! + Cybersource::PaymentRequest.new(**create_payment_params.to_h.symbolize_keys).sign! end def cybersource_response diff --git a/app/models/cybersource/payment_request.rb b/app/models/cybersource/payment_request.rb index 2a477410..91ebc2a4 100644 --- a/app/models/cybersource/payment_request.rb +++ b/app/models/cybersource/payment_request.rb @@ -6,17 +6,16 @@ module Cybersource class PaymentRequest include Enumerable - attr_reader :user, :amount, :session_id, :signature, :signed_date_time + attr_reader :user_id, :amount, :signature, :signed_date_time # Set of fields we use to generate the signature and that Cybersource verifies REQUEST_SIGNED_FIELDS = %i[access_key profile_id transaction_uuid signed_date_time - locale transaction_type reference_number amount currency unsigned_field_names - signed_field_names merchant_defined_data1].freeze + locale transaction_type reference_number amount currency + unsigned_field_names signed_field_names].freeze - def initialize(user:, amount:, session_id:) - @user = user + def initialize(user_id:, amount:) + @user_id = user_id @amount = amount - @session_id = session_id @signed_fields = REQUEST_SIGNED_FIELDS @unsigned_fields = [] @signed_date_time, @signature = nil @@ -81,14 +80,9 @@ def transaction_uuid @transaction_uuid ||= SecureRandom.uuid end - # Used to check if the user has a payment pending by comparing to a cookie - def reference_number - @session_id - end - # Passed back to us by Cybersource and used to look up the user in the ILS - def merchant_defined_data1 - @user + def reference_number + @user_id end # Matches us with configuration on the Cybersource side; differs for dev/test/prod diff --git a/app/models/cybersource/payment_response.rb b/app/models/cybersource/payment_response.rb index f20f10a1..407857f7 100644 --- a/app/models/cybersource/payment_response.rb +++ b/app/models/cybersource/payment_response.rb @@ -41,16 +41,12 @@ def signed_data @signed_data = @signed_fields.index_with { |field| @fields[field] } end - def user - @fields['req_merchant_defined_data1'] + def user_id + @fields['req_reference_number'] end def amount @fields['req_amount'] end - - def session_id - @fields['req_reference_number'] - end end end diff --git a/app/models/folio/account.rb b/app/models/folio/account.rb index 7b057871..1e125a6b 100644 --- a/app/models/folio/account.rb +++ b/app/models/folio/account.rb @@ -22,12 +22,6 @@ def key record['id'] end - # TODO: remove; unused after migration off of Symphony - # Also update the pay all button template not to render it - def sequence - nil - end - def patron_key record.dig('loan', 'proxyUserId') || record['userId'] end diff --git a/app/services/folio_client.rb b/app/services/folio_client.rb index 6b60abe2..ec0ef336 100644 --- a/app/services/folio_client.rb +++ b/app/services/folio_client.rb @@ -221,14 +221,14 @@ def find_patron_by_barcode(barcode, patron_info: true) # Mark all of a user's fines (accounts) as having been paid # The payment will show as being made from the 'Online' service point # rubocop:disable Metrics/MethodLength - def pay_fines(user:, amount:, session_id:) - patron = find_patron_by_barcode(user) + def pay_fines(user_id:, amount:) + patron = Patron.find(user_id) payload = { accountIds: patron.fines.map(&:key), paymentMethod: 'Credit card', amount: amount, userName: 'libsys_admin', - transactionInfo: session_id, + transactionInfo: user_id, servicePointId: Settings.folio.online_service_point_id, notifyPatron: true } diff --git a/app/views/fines/_pay_all_button.html.erb b/app/views/fines/_pay_all_button.html.erb index bbd4da9c..2bd1ecfb 100644 --- a/app/views/fines/_pay_all_button.html.erb +++ b/app/views/fines/_pay_all_button.html.erb @@ -1,15 +1,10 @@ -<% fines = patron_or_group.fines %> -<% amount = fines.sum(&:owed) %> +<% amount = patron_or_group.fines.sum(&:owed) %> <% if amount.positive? %>
<% if patron_or_group.can_pay_fines? %> <%= form_tag payments_path, method: :post do %> - <%= hidden_field_tag :reason, fines.map(&:status).join(',') %> - <%= hidden_field_tag :billseq, fines.map(&:sequence).values_at(0, -1).join('-') %> + <%= hidden_field_tag :user_id, patron_or_group.key %> <%= hidden_field_tag :amount, format('%.2f', amount) %> - <%= hidden_field_tag :session_id, SecureRandom.hex %> - <%= hidden_field_tag :group, patron_or_group.is_a?(Symphony::Group) ? 'G' : '' %> - <%= hidden_field_tag :user, patron_or_group.barcode %> <%= button_tag class: 'btn btn-md btn-info', type: 'submit', data: { 'pay-button' => true } do %> <%= sul_icon :'sharp-payment-24px' %> Pay <%= number_to_currency(amount) %> now diff --git a/spec/controllers/fines_controller_spec.rb b/spec/controllers/fines_controller_spec.rb index 3e0a8275..a2d4c34a 100644 --- a/spec/controllers/fines_controller_spec.rb +++ b/spec/controllers/fines_controller_spec.rb @@ -3,11 +3,11 @@ require 'rails_helper' RSpec.describe FinesController do - let(:mock_patron) { instance_double(Folio::Patron, group?: false, barcode: '1234') } + let(:mock_patron) { instance_double(Folio::Patron, group?: false, key: '513a9054-5897-11ee-8c99-0242ac120002') } let(:fines) do [ - instance_double(Folio::Account, key: '1', sequence: '1', owed: '12', status: 'BADCHECK') + instance_double(Folio::Account, key: '1', owed: '12', status: 'BADCHECK') ] end diff --git a/spec/controllers/payments_controller_spec.rb b/spec/controllers/payments_controller_spec.rb index ea69b004..988c093a 100644 --- a/spec/controllers/payments_controller_spec.rb +++ b/spec/controllers/payments_controller_spec.rb @@ -4,7 +4,7 @@ RSpec.describe PaymentsController do let(:user) { { username: 'somesunetid', patron_key: '513a9054-5897-11ee-8c99-0242ac120002' } } - let(:mock_patron) { instance_double(Folio::Patron, group?: false, barcode: '1234', payments: payments) } + let(:mock_patron) { instance_double(Folio::Patron, group?: false, key: user[:patron_key], payments: payments) } let(:mock_client) { instance_double(FolioClient, ping: true, pay_fines: nil) } let(:mock_graphql_client_response) do [ @@ -55,7 +55,7 @@ describe '#create' do before do - post :create, params: { user: 'u', session_id: 's', group: 'g', amount: 'a' } + post :create, params: { user_id: '513a9054-5897-11ee-8c99-0242ac120002', amount: '10.00' } end it 'renders a form to send to cybersource' do @@ -65,9 +65,10 @@ describe '#accept' do let(:cybersource_response) do - instance_double(Cybersource::PaymentResponse, user: '123', amount: '10.00', - session_id: 'session_this_is_the_one', - valid?: true, payment_success?: true) + instance_double(Cybersource::PaymentResponse, user_id: '513a9054-5897-11ee-8c99-0242ac120002', + amount: '10.00', + valid?: true, + payment_success?: true) end before do @@ -77,7 +78,7 @@ it 'updates the payment in the ILS' do post :accept expect(mock_client).to have_received(:pay_fines) - .with(user: '123', amount: '10.00', session_id: 'session_this_is_the_one') + .with(user_id: '513a9054-5897-11ee-8c99-0242ac120002', amount: '10.00') end it 'redirects to fines page' do diff --git a/spec/models/cybersource/payment_request_spec.rb b/spec/models/cybersource/payment_request_spec.rb index b91a3780..e2bb1e90 100644 --- a/spec/models/cybersource/payment_request_spec.rb +++ b/spec/models/cybersource/payment_request_spec.rb @@ -4,23 +4,19 @@ RSpec.describe Cybersource::PaymentRequest do subject(:request_params) do - described_class.new(user: '1234567890', amount: '100.00', session_id: '3672f43945ec20cf2966525aeb7691e4').sign!.to_h + described_class.new(user_id: '0340214b-5492-472d-b634-c5c115639465', amount: '100.00').sign!.to_h end before do allow(Cybersource::Security).to receive(:secret_key).and_return('very_secret') end - it 'stores the user barcode as merchant defined data' do - expect(request_params[:merchant_defined_data1]).to eq('1234567890') - end - it 'stores the amount of total charges' do expect(request_params[:amount]).to eq('100.00') end - it 'stores the session id as the transaction reference number' do - expect(request_params[:reference_number]).to eq('3672f43945ec20cf2966525aeb7691e4') + it 'stores the user id as the transaction reference number' do + expect(request_params[:reference_number]).to eq('0340214b-5492-472d-b634-c5c115639465') end it 'signs the transaction parameters' do diff --git a/spec/models/cybersource/payment_response_spec.rb b/spec/models/cybersource/payment_response_spec.rb index 5cebc387..d9465251 100644 --- a/spec/models/cybersource/payment_response_spec.rb +++ b/spec/models/cybersource/payment_response_spec.rb @@ -8,35 +8,29 @@ let(:signature) do Cybersource::Security.generate_signature( { - req_merchant_defined_data1: '1234567890', req_amount: '100.00', - req_reference_number: '3672f43945ec20cf2966525aeb7691e4' + req_reference_number: '0340214b-5492-472d-b634-c5c115639465' } ) end let(:decision) { 'ACCEPT' } let(:params) do - ActionController::Parameters.new(req_merchant_defined_data1: '1234567890', - req_amount: '100.00', - req_reference_number: '3672f43945ec20cf2966525aeb7691e4', - signed_field_names: 'req_merchant_defined_data1,req_amount,req_reference_number', + ActionController::Parameters.new(req_amount: '100.00', + req_reference_number: '0340214b-5492-472d-b634-c5c115639465', + signed_field_names: 'req_amount,req_reference_number', unsigned_field_names: '', signature: signature, decision: decision) end - it 'parses the user barcode from the merchant defined data' do - expect(cybersource_response.user).to eq('1234567890') + it 'parses the user id from the merchant defined data' do + expect(cybersource_response.user_id).to eq('0340214b-5492-472d-b634-c5c115639465') end it 'parses the amount of total charges' do expect(cybersource_response.amount).to eq('100.00') end - it 'parses the session id from the transaction reference number' do - expect(cybersource_response.session_id).to eq('3672f43945ec20cf2966525aeb7691e4') - end - it 'validates that the transaction is signed and accepted' do expect { cybersource_response.validate! }.not_to raise_error end diff --git a/spec/views/fines/_pay_all_button.html.erb_spec.rb b/spec/views/fines/_pay_all_button.html.erb_spec.rb index f7fee107..76a3c993 100644 --- a/spec/views/fines/_pay_all_button.html.erb_spec.rb +++ b/spec/views/fines/_pay_all_button.html.erb_spec.rb @@ -5,8 +5,10 @@ RSpec.describe 'fines/_pay_all_button' do subject(:output) { Capybara.string(rendered) } - let(:patron) { instance_double(Symphony::Patron, barcode: '1', fines: fines, can_pay_fines?: true) } - let(:fines) { [instance_double(Symphony::Fine, owed: 3, status: 'A', sequence: '1')] } + let(:patron) do + instance_double(Folio::Patron, key: '513a9054-5897-11ee-8c99-0242ac120002', fines: fines, can_pay_fines?: true) + end + let(:fines) { [instance_double(Folio::Account, owed: 3)] } before do without_partial_double_verification do diff --git a/spec/views/fines/index.html.erb_spec.rb b/spec/views/fines/index.html.erb_spec.rb index 91c076d5..96d40013 100644 --- a/spec/views/fines/index.html.erb_spec.rb +++ b/spec/views/fines/index.html.erb_spec.rb @@ -3,196 +3,138 @@ require 'rails_helper' RSpec.describe 'fines/index' do - context 'with a Symphony Fine' do - let(:fine) do - instance_double(Symphony::Fine, owed: 3, status: 'A', sequence: '1', nice_status: 'Damaged', - bib?: false, key: 'abc', bill_date: Date.new, fee: 5, - library: 'Best Lib', barcode: '12345') - end - let(:fines) { [fine] } - let(:checkouts) { [] } - let(:patron) do - instance_double(Symphony::Patron, barcode: '1', fines: fines, can_pay_fines?: true, requests: [], - checkouts: checkouts, remaining_checkouts: nil, barred?: false, - status: 'OK', group?: false, sponsor?: false) - end + let(:fine) do + instance_double(Folio::Account, + owed: 3, + status: 'A', + nice_status: 'Damaged', + bib?: true, + key: 'abc', + bill_date: Date.new, + fee: 5, + library: 'Best Lib', + barcode: '12345', + author: 'Author 1', + title: 'Title', + shelf_key: 'AB 1234', + call_number: 'AB 1234', + catkey: '12345', + patron_key: 'patronkey123', + instance_of?: Folio::Account) + end + let(:fines) { [fine] } + let(:checkouts) do + [Folio::Checkout.new( + { 'id' => '31d15973-acb6-4a12-92c7-5e2d5f2470ed', + 'item' => { 'title' => 'Mental growth during the first three years' }, + 'overdue' => true, + 'details' => { 'feesAndFines' => { 'amountRemainingToPay' => 10 } } } + )] + end + let(:patron) do + instance_double(Folio::Patron, + key: '1', + fines: fines, + can_pay_fines?: true, + requests: [], + checkouts: [], + remaining_checkouts: nil, + barred?: false, + status: 'OK', + group?: false, + sponsor?: false, + display_name: 'Shea Sponsor', + instance_of?: Folio::Patron) + end + let(:group) do + instance_double(Folio::Group, + barred?: false, + blocked?: false, + status: 'OK', + requests: [], + checkouts: [], + member_name: 'Piper Proxy', + fines: [], + can_pay_fines?: true, + key: 'Sponsor1') + end + let(:patron_or_group) { patron } + context 'when the patron is not in a group' do before do assign(:fines, fines) assign(:checkouts, checkouts) - allow(fine).to receive(:to_partial_path).and_return('fines/fine') without_partial_double_verification do allow(view).to receive_messages(patron_or_group: patron, patron: patron) end + allow(fine).to receive(:to_partial_path).and_return('fines/fine') end - context 'when the patron has fines' do - it 'shows the shared computer payment alert' do - render - - expect(rendered).to have_text('Shared computer users: Due to computer security risks, you should not use a shared computer to make a fine payment.') # rubocop:disable Layout/LineLength - end + it 'shows the fined item author' do + render + expect(rendered).to have_text('Author 1') end - context 'when the patron has no fines' do - let(:patron) do - # create a patron with empty array for fines - instance_double(Symphony::Patron, - barcode: '1', - fines: [], - can_pay_fines?: true, - requests: [], - checkouts: [], - remaining_checkouts: nil, - barred?: false, - status: 'OK', - group?: false, - sponsor?: false) - end + it 'shows the Pay button' do + render + expect(rendered).to have_text('Pay $3.00 now') + end - it 'does not show the shared computer payment alert' do + context 'when the patron has accruing fines' do + it 'shows the accrued amount' do render - - expect(rendered).not_to have_text('Shared computer users: Due to computer security risks, you should not use a shared computer to make a fine payment.') # rubocop:disable Layout/LineLength + expect(rendered).to have_text('Accruing: $10.00') end end end - context 'with a FOLIO Fine' do - let(:fine) do - instance_double(Folio::Account, - owed: 3, - status: 'A', - sequence: '1', - nice_status: 'Damaged', - bib?: true, - key: 'abc', - bill_date: Date.new, - fee: 5, - library: 'Best Lib', - barcode: '12345', - author: 'Author 1', - title: 'Title', - shelf_key: 'AB 1234', - call_number: 'AB 1234', - catkey: '12345', - patron_key: 'patronkey123', - instance_of?: Folio::Account) - end - let(:fines) { [fine] } - let(:checkouts) do - [Folio::Checkout.new( - { 'id' => '31d15973-acb6-4a12-92c7-5e2d5f2470ed', - 'item' => { 'title' => 'Mental growth during the first three years' }, - 'overdue' => true, - 'details' => { 'feesAndFines' => { 'amountRemainingToPay' => 10 } } } - )] - end - let(:patron) do - instance_double(Folio::Patron, - barcode: '1', - fines: fines, - can_pay_fines?: true, - requests: [], - checkouts: [], - remaining_checkouts: nil, - barred?: false, - status: 'OK', - group?: false, - sponsor?: false, - display_name: 'Shea Sponsor', - instance_of?: Folio::Patron) - end - let(:group) do - instance_double(Folio::Group, - barred?: false, - blocked?: false, - status: 'OK', - requests: [], - checkouts: [], - member_name: 'Piper Proxy', - fines: [], - can_pay_fines?: true, - barcode: 'Sponsor1') - end - let(:patron_or_group) { patron } - - context 'when the patron is not in a group' do - before do - assign(:fines, fines) - assign(:checkouts, checkouts) - without_partial_double_verification do - allow(view).to receive_messages(patron_or_group: patron, patron: patron) - end - allow(fine).to receive(:to_partial_path).and_return('fines/fine') + context 'with a sponsor account' do + before do + assign(:fines, fines) + assign(:checkouts, checkouts) + assign(:patron_or_group, patron_or_group) + # make the patron a group sponsor + allow(patron).to receive_messages(sponsor?: true, group: group) + # allow the is_a? check and return true for Folio::Patron + allow(patron).to receive(:is_a?).and_return(false) + allow(patron).to receive(:is_a?).with(Folio::Patron).and_return(true) + allow(patron_or_group).to receive_messages(group?: true, proxy_borrower?: false) + without_partial_double_verification do + allow(view).to receive_messages(patron: patron, patron_or_group: patron_or_group) end + allow(fine).to receive(:to_partial_path).and_return('fines/fine') + end - it 'shows the fined item author' do + context 'when one of their proxies incurred a fine' do + it 'displays the proxy fine under the sponsor Self tab' do render - expect(rendered).to have_text('Author 1') + + expect(rendered).to include('Borrower:', 'Piper Proxy') end - it 'shows the Pay button' do + it 'allows the sponsor to pay the fine' do render + expect(rendered).to have_text('Pay $3.00 now') end - context 'when the patron has accruing fines' do - it 'shows the accrued amount' do - render - expect(rendered).to have_text('Accruing: $10.00') + it 'renders the correct message in the group tab' do + without_partial_double_verification do + allow(view).to receive(:params).and_return({ group: true }) end + render + expect(rendered).to have_text("Fines incurred by proxy borrowers appear in the list of fines under their sponsor's Self tab.") # rubocop:disable Layout/LineLength end end - context 'with a sponsor account' do + context 'with a sponsor/self fine' do before do - assign(:fines, fines) - assign(:checkouts, checkouts) - assign(:patron_or_group, patron_or_group) - # make the patron a group sponsor - allow(patron).to receive_messages(sponsor?: true, group: group) - # allow the is_a? check and return true for Folio::Patron - allow(patron).to receive(:is_a?).and_return(false) - allow(patron).to receive(:is_a?).with(Folio::Patron).and_return(true) - allow(patron_or_group).to receive_messages(group?: true, proxy_borrower?: false) - without_partial_double_verification do - allow(view).to receive_messages(patron: patron, patron_or_group: patron_or_group) - end - allow(fine).to receive(:to_partial_path).and_return('fines/fine') + allow(group).to receive(:member_name).and_return(nil) end - context 'when one of their proxies incurred a fine' do - it 'displays the proxy fine under the sponsor Self tab' do - render - - expect(rendered).to include('Borrower:', 'Piper Proxy') - end - - it 'allows the sponsor to pay the fine' do - render - - expect(rendered).to have_text('Pay $3.00 now') - end - - it 'renders the correct message in the group tab' do - without_partial_double_verification do - allow(view).to receive(:params).and_return({ group: true }) - end - render - expect(rendered).to have_text("Fines incurred by proxy borrowers appear in the list of fines under their sponsor's Self tab.") # rubocop:disable Layout/LineLength - end - end - - context 'with a sponsor/self fine' do - before do - allow(group).to receive(:member_name).and_return(nil) - end - - it 'displays the sponsor borrower name' do - render - expect(rendered).to include('Borrower:', 'Shea Sponsor') - end + it 'displays the sponsor borrower name' do + render + expect(rendered).to include('Borrower:', 'Shea Sponsor') end end end diff --git a/spec/views/summaries/summary.html.erb_spec.rb b/spec/views/summaries/summary.html.erb_spec.rb index 6d387c68..79b928b0 100644 --- a/spec/views/summaries/summary.html.erb_spec.rb +++ b/spec/views/summaries/summary.html.erb_spec.rb @@ -3,10 +3,10 @@ require 'rails_helper' RSpec.describe 'summaries/_summary' do - let(:fines) { [instance_double(Symphony::Fine, owed: 3, status: 'A', sequence: '1')] } + let(:fines) { [instance_double(Folio::Account, owed: 3)] } let(:patron) do - instance_double(Symphony::Patron, barcode: '1', fines: fines, can_pay_fines?: true, requests: [], checkouts: [], - remaining_checkouts: nil) + instance_double(Folio::Patron, key: '513a9054-5897-11ee-8c99-0242ac120002', fines: fines, can_pay_fines?: true, + requests: [], checkouts: [], remaining_checkouts: nil) end before do @@ -24,10 +24,7 @@ end context 'when the patron has no fines' do - let(:patron) do - instance_double(Symphony::Patron, barcode: '1', fines: [], can_pay_fines?: true, requests: [], checkouts: [], - remaining_checkouts: nil) - end + let(:fines) { [] } it 'does not show the shared computer payment alert' do render