Skip to content

Commit

Permalink
Merge pull request #1049 from sul-dlss/item-key
Browse files Browse the repository at this point in the history
Rename item_key to item_id to align with FOLIO naming
  • Loading branch information
jcoyne authored Feb 13, 2024
2 parents 1223b77 + 2b49f4b commit 79d1bbd
Show file tree
Hide file tree
Showing 10 changed files with 26 additions and 26 deletions.
4 changes: 2 additions & 2 deletions app/controllers/renewals_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ def bulk_renewal_error_flash(response)
end

def renew_item_params
params.require(%I[item_key])
params.require(%I[item_id])
end

# Make sure the checkout belongs to the user trying to do the renewal
# and make sure the item is renewable
def authorize_update!
checkout = patron_or_group.checkouts.find { |request| request.item_key == params.require(:item_key) }
checkout = patron_or_group.checkouts.find { |request| request.item_id == params.require(:item_id) }

raise CheckoutException, 'Error' if checkout.nil? || checkout.item_category_non_renewable?
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/concerns/folio/folio_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def barcode
item['barcode']
end

def item_key
def item_id
bib['itemId']
end

Expand Down
2 changes: 1 addition & 1 deletion app/services/folio_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def ping

def renew_items(checkouts)
checkouts.each_with_object(success: [], error: []) do |checkout, status|
response = renew_item_by_id(checkout.patron_key, checkout.item_key)
response = renew_item_by_id(checkout.patron_key, checkout.item_id)

case response.status
when 200
Expand Down
2 changes: 1 addition & 1 deletion app/views/checkouts/_checkout.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
<div class="row">
<div class="col-11 offset-1 col-md-10 offset-md-2">
<%= form_tag renewals_path, method: :post do %>
<%= hidden_field_tag :item_key, checkout.item_key %>
<%= hidden_field_tag :item_id, checkout.item_id %>
<%= hidden_field_tag :title, checkout.title %>
<%= hidden_field_tag :group, params[:group] if params[:group] %>
<%= button_tag class: 'btn btn-link btn-renewable-submit btn-icon-prefix', type: 'submit' do %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/checkouts/_checkout.json.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ json.attributes do
json.renewable checkout.renewable?
json.renewable_at checkout.renewable_at
json.renew_from_period checkout.renew_from_period
json.item_key checkout.item_key
json.item_id checkout.item_id
json.overdue checkout.overdue?
json.accrued checkout.accrued
json.days_remaining checkout.days_remaining
Expand Down
28 changes: 14 additions & 14 deletions spec/controllers/renewals_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
let(:mock_patron) do
instance_double(Folio::Patron, checkouts:, key: '513a9054-5897-11ee-8c99-0242ac120002')
end
let(:checkouts) { [instance_double(Folio::Checkout, item_key: '123', item_category_non_renewable?: false)] }
let(:checkouts) { [instance_double(Folio::Checkout, item_id: '123', item_category_non_renewable?: false)] }

before do
warden.set_user(user)
Expand All @@ -23,19 +23,19 @@
end

describe '#create' do
it 'requires item_key params' do
it 'requires item_id params' do
expect { post :create, params: {} }.to raise_error(ActionController::ParameterMissing)
end

context 'when everything is good' do
it 'renews the item and sets flash messages' do
post :create, params: { item_key: '123' }
post :create, params: { item_id: '123' }

expect(flash[:success]).to match(/Success!/)
end

it 'renews the item and redirects to checkouts_path' do
post :create, params: { item_key: '123' }
post :create, params: { item_id: '123' }

expect(response).to redirect_to checkouts_path
end
Expand All @@ -45,45 +45,45 @@
let(:api_response) { instance_double('Response', status: 401, content_type: :json) }

it 'does not renew the item and sets flash messages' do
post :create, params: { item_key: '123' }
post :create, params: { item_id: '123' }

expect(flash[:error]).to match(/Sorry!/)
end

it 'does not renew the item and redirects to checkouts_path' do
post :create, params: { item_key: '123' }
post :create, params: { item_id: '123' }

expect(response).to redirect_to checkouts_path
end
end

context 'with a group request' do
it 'renews the item and redirects to checkouts_path' do
post :create, params: { item_key: '123', group: true }
post :create, params: { item_id: '123', group: true }

expect(response).to redirect_to checkouts_path(group: true)
end
end

context 'when the requested item is not checked out to the patron' do
it 'does not renew the item and sets flash messages' do
post :create, params: { item_key: 'some_made_up_item_key' }
post :create, params: { item_id: 'some_made_up_item_id' }

expect(flash[:error]).to match('An unexpected error has occurred')
end

it 'does not renew the item and redirects to checkouts_path' do
post :create, params: { item_key: 'some_made_up_item_key' }
post :create, params: { item_id: 'some_made_up_item_id' }

expect(response).to redirect_to checkouts_path
end
end

context 'when the requested item is not eligible even though Folio does not stop us' do
let(:checkouts) { [instance_double(Folio::Checkout, item_key: '123', item_category_non_renewable?: true)] }
let(:checkouts) { [instance_double(Folio::Checkout, item_id: '123', item_category_non_renewable?: true)] }

it 'does not renew the item and sets flash messages' do
post :create, params: { item_key: '123' }
post :create, params: { item_id: '123' }

expect(flash[:error]).to match('An unexpected error has occurred')
end
Expand All @@ -98,11 +98,11 @@
let(:mock_patron) { instance_double(Folio::Patron, checkouts:) }
let(:checkouts) do
[
instance_double(Folio::Checkout, key: '1', renewable?: true, item_key: '123', title: 'ABC'),
instance_double(Folio::Checkout, key: '2', renewable?: true, item_key: '456',
instance_double(Folio::Checkout, key: '1', renewable?: true, item_id: '123', title: 'ABC'),
instance_double(Folio::Checkout, key: '2', renewable?: true, item_id: '456',
title: 'Principles of optics : electromagnetic theory of ' \
'propagation, interference and diffraction of light'),
instance_double(Folio::Checkout, key: '3', renewable?: false, item_key: '789', title: 'Not')
instance_double(Folio::Checkout, key: '3', renewable?: false, item_id: '789', title: 'Not')
]
end

Expand Down
2 changes: 1 addition & 1 deletion spec/features/renew_item_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

let(:api_response) { instance_double('Response', status: 201, content_type: :json) }
let(:bulk_renew_response) do
{ success: [instance_double(Folio::Checkout, key: '1', renewable?: true, item_key: '123', title: 'ABC')] }
{ success: [instance_double(Folio::Checkout, key: '1', renewable?: true, item_id: '123', title: 'ABC')] }
end

before do
Expand Down
4 changes: 2 additions & 2 deletions spec/models/concerns/folio/folio_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
it { expect(model.barcode).to eq '36105110374977' }
end

describe '#item_key' do
it { expect(model.item_key).to eq '6d9a4f99-d144-51cf-92d7-3edbfc588abe' }
describe '#item_id' do
it { expect(model.item_id).to eq '6d9a4f99-d144-51cf-92d7-3edbfc588abe' }
end
end
4 changes: 2 additions & 2 deletions spec/services/folio_client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@
# rubocop:disable RSpec/MultipleExpectations
it 'groups the successful and failed renewal responses' do
expect(renew_items[:success].count).to eq 1
expect(renew_items[:success].first.item_key).to eq '6d9a4f99-d144-51cf-92d7-3edbfc588abe'
expect(renew_items[:success].first.item_id).to eq '6d9a4f99-d144-51cf-92d7-3edbfc588abe'
expect(renew_items[:error].count).to eq 1
expect(renew_items[:error].first.item_key).to eq 'cc3d8728-a6b9-45c4-ad0c-432873c3ae47'
expect(renew_items[:error].first.item_id).to eq 'cc3d8728-a6b9-45c4-ad0c-432873c3ae47'
end
# rubocop:enable RSpec/MultipleExpectations
end
Expand Down
2 changes: 1 addition & 1 deletion spec/views/checkouts/_checkout.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
days_remaining: 120,
due_date: 120.days.from_now,
from_ill?: false,
item_key: nil,
item_id: nil,
key: 'abc123',
library_name: 'Green Library',
lost?: false,
Expand Down

0 comments on commit 79d1bbd

Please sign in to comment.