diff --git a/app/controllers/holds_controller.rb b/app/controllers/holds_controller.rb
index a3039c0b..7ac30fd3 100644
--- a/app/controllers/holds_controller.rb
+++ b/app/controllers/holds_controller.rb
@@ -4,6 +4,7 @@ class HoldsController < ApplicationController
before_action :set_cache_headers
before_action :authenticate_user!
before_action :check_for_blanks!, only: :create
+ rescue_from NewHoldException, with: :deny_new
rescue_from HoldCreateException, with: :deny_create
rescue_from HoldException, with: :past_date
@@ -42,10 +43,14 @@ def batch_update
#
# GET /holds/new
def new
+ raise NewHoldException, 'Error' if params[:catkey].blank?
+
form_builder = PlaceHoldForm::Builder.new(catkey: params[:catkey],
user_token: current_user.session_token,
client: symphony_client)
@place_hold_form_params = form_builder.generate
+
+ raise NewHoldException, 'Error' if @place_hold_form_params.blank?
end
# Handles placing holds
@@ -88,6 +93,16 @@ def check_for_blanks!
raise HoldCreateException, 'Error'
end
+ def deny_new
+ flash[:error] = if params['catkey'].blank?
+ t 'myaccount.hold.new_hold.catkey_missing'
+ else
+ t 'myaccount.hold.new_hold.error_html'
+ end
+
+ redirect_to summaries_path
+ end
+
def deny_create
flash[:error] = if barcodes.blank?
t 'myaccount.hold.place_hold.select_volumes'
diff --git a/app/models/bib.rb b/app/models/bib.rb
index 24ba77b8..d068b596 100644
--- a/app/models/bib.rb
+++ b/app/models/bib.rb
@@ -10,7 +10,7 @@ def initialize(record)
end
def call_list
- bib.dig 'callList'
+ bib&.dig 'callList'
end
private
diff --git a/app/models/new_hold_exception.rb b/app/models/new_hold_exception.rb
new file mode 100644
index 00000000..f4ecd802
--- /dev/null
+++ b/app/models/new_hold_exception.rb
@@ -0,0 +1,3 @@
+# frozen_string_literal: true
+
+class NewHoldException < RuntimeError; end
diff --git a/app/services/place_hold_form/builder.rb b/app/services/place_hold_form/builder.rb
index 410b5edd..6e1bcb69 100644
--- a/app/services/place_hold_form/builder.rb
+++ b/app/services/place_hold_form/builder.rb
@@ -11,7 +11,8 @@ def initialize(catkey:, user_token:, client:)
def generate
bib_info = Bib.new(SymphonyClientParser::parsed_response(@client, :get_bib_info, @catkey, @user_token))
- @call_list = bib_info.call_list.map { |call| Call.new record: call }
+ return {} unless holdables_present?(bib_info)
+
process_volumetric_calls
{
@@ -19,18 +20,29 @@ def generate
title: bib_info.title,
author: bib_info.author,
volumetric_calls: @volumetric_calls,
- barcode: @volumetric_calls.present? ? nil : @call_list&.sample&.items&.sample&.barcode
+ barcode: find_barcode
}
end
private
+ # Find out what Items are holdable inside Calls. Set Calls to be just those that contain holdable items.
+ #
+ # * Each potential holdable must have a current location that is holdable.
+ def holdables_present?(bib_info)
+ return false if bib_info.call_list.blank?
+
+ @call_list = bib_info.call_list.map { |call| Call.new record: call }
+ filter_holdables if @call_list.count > 1
+
+ @call_list.present?
+ end
+
# Take the @call_list (Array of Calls) and check for volumetrics and process if present.
#
- # First: find out what Items are holdable inside Calls. Order is important.
+ # First: determine holdable volumetrics.
#
# * Must be more than 1 overall potential holdable Call.
- # * Each potential holdable must have a current location that is holdable.
# * To be a holdable set:
# * Must be at least one volumetric Call.
# * Must be more than one Call with items that are holdable.
@@ -44,7 +56,6 @@ def generate
#
# Fourth: Only pass along the volumetric calls that have unique Call#call_number
def process_volumetric_calls
- filter_holdables if @call_list.count > 1
@volumetric_calls = @call_list.dup if volumetric? && @call_list.count > 1
volumetric_natural_sort
compact_non_volumetric_calls if @volumetric_calls.select { |call| call.volumetric.nil? }.count > 1
@@ -95,4 +106,8 @@ def naturalize(value)
def volumetric?
@call_list&.any? { |call| call.volumetric.present? }
end
+
+ def find_barcode
+ @volumetric_calls.present? ? nil : @call_list&.sample&.items&.sample&.barcode
+ end
end
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 6098ac3e..1a6cab40 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -68,4 +68,7 @@ en:
select_date: To place a hold, please choose a not needed after date.
select_volumes: To place a hold, please select a volume.
success_html: For placed hold(s), you will receive a library notice once items are available for pick up. On-shelf items generally arrive within 2-3 weekdays. Items checked out to other borrowers will be recalled. Recalled items take a minimum of 10 days to arrive at your pickup location.
- error_html: We could not process your request. Please visit our website for more information under "Why did my hold fail?"
\ No newline at end of file
+ error_html: We could not process your request. Please visit our website for more information under "Why did my hold fail?"
+ new_hold:
+ catkey_missing: There was a problem processing your hold request, please contact your library if you need assistance.
+ error_html: Sorry! You cannot place a hold on this item. Contact your library if you need assistance.
diff --git a/spec/controllers/holds_controller_spec.rb b/spec/controllers/holds_controller_spec.rb
index aaa5cf9e..1f7b206e 100644
--- a/spec/controllers/holds_controller_spec.rb
+++ b/spec/controllers/holds_controller_spec.rb
@@ -83,10 +83,40 @@
end
it 'sends the form parameters to the view' do
- get :new
+ get :new, params: { catkey: 1 }
expect(assigns(:place_hold_form_params)).to eq(form_params)
end
+
+ context 'when catkey param is missing' do
+ it 'sets a flash error message' do
+ get :new, params: {}
+
+ expect(flash[:error]).to eq I18n.t('myaccount.hold.new_hold.catkey_missing')
+ end
+
+ it 'redirects to the summaries' do
+ get :new, params: {}
+
+ expect(response).to redirect_to summaries_path
+ end
+ end
+
+ context 'when user tries with a non holdable record' do
+ let(:form_params) {}
+
+ it 'sets a flash error message' do
+ get :new, params: { catkey: 1 }
+
+ expect(flash[:error]).to eq I18n.t('myaccount.hold.new_hold.error_html')
+ end
+
+ it 'redirects to the summaries' do
+ get :new, params: { catkey: 1 }
+
+ expect(response).to redirect_to summaries_path
+ end
+ end
end
describe '#create' do
diff --git a/spec/factories/bibs.rb b/spec/factories/bibs.rb
index 0dcb935b..e9037373 100644
--- a/spec/factories/bibs.rb
+++ b/spec/factories/bibs.rb
@@ -269,6 +269,196 @@
}
end
+ factory :bib_with_no_holdable_locations do
+ initialize_with {
+ new(body)
+ }
+
+ body {
+ {
+ 'resource' => '/catalog/bib',
+ 'key' => '26141494',
+ 'fields' => {
+ 'shadowed' => false,
+ 'author' => 'Diamond, Jared M. author.',
+ 'titleControlNumber' => 'l2018952825',
+ 'catalogDate' => '2019-05-28',
+ 'catalogFormat' => {
+ 'resource' => '/policy/catalogFormat', 'key' => 'MARC'
+ },
+ 'modifiedDate' => '2019-12-08',
+ 'systemModifiedDate' => '2020-02-04T15:11:00-05:00',
+ 'title' => 'Upheaval : turning points for nations in crisis',
+ 'callList' => [{
+ 'resource' => '/catalog/call',
+ 'key' => '26141494:4',
+ 'fields' => {
+ 'library' => {
+ 'resource' => '/policy/library', 'key' => 'UP-PAT'
+ },
+ 'callNumber' => 'HN13.D52 2019',
+ 'shadowed' => false,
+ 'volumetric' => nil,
+ 'dispCallNumber' => 'HN13.D52 2019',
+ 'bib' => {
+ 'resource' => '/catalog/bib', 'key' => '26141494'
+ },
+ 'itemList' => [{
+ 'resource' => '/catalog/item',
+ 'key' => '26141494:4:2',
+ 'fields' => {
+ 'call' => {
+ 'resource' => '/catalog/call', 'key' => '26141494:4'
+ },
+ 'mediaDesk' => nil,
+ 'itemType' => {
+ 'resource' => '/policy/itemType', 'key' => 'BOOK'
+ },
+ 'library' => {
+ 'resource' => '/policy/library', 'key' => 'UP-PAT'
+ },
+ 'shadowed' => false,
+ 'homeLocation' => {
+ 'resource' => '/policy/location', 'key' => 'PATERNO-2'
+ },
+ 'copyNumber' => 2,
+ 'bib' => {
+ 'resource' => '/catalog/bib', 'key' => '26141494'
+ },
+ 'barcode' => '000081297085',
+ 'circulate' => false,
+ 'currentLocation' => {
+ 'resource' => '/policy/location', 'key' => 'ILLEND'
+ }
+ }
+ }],
+ 'classification' => {
+ 'resource' => '/policy/classification', 'key' => 'LC'
+ }
+ }
+ },
+ {
+ 'resource' => '/catalog/call',
+ 'key' => '26141494:6',
+ 'fields' => {
+ 'library' => {
+ 'resource' => '/policy/library', 'key' => 'ALTOONA'
+ },
+ 'callNumber' => 'HN13.D52 2019',
+ 'shadowed' => false,
+ 'volumetric' => nil,
+ 'dispCallNumber' => 'HN13.D52 2019',
+ 'bib' => {
+ 'resource' => '/catalog/bib', 'key' => '26141494'
+ },
+ 'itemList' => [{
+ 'resource' => '/catalog/item',
+ 'key' => '26141494:6:1',
+ 'fields' => {
+ 'call' => {
+ 'resource' => '/catalog/call', 'key' => '26141494:6'
+ },
+ 'mediaDesk' => nil,
+ 'itemType' => {
+ 'resource' => '/policy/itemType', 'key' => 'BOOKFLOAT'
+ },
+ 'library' => {
+ 'resource' => '/policy/library', 'key' => 'ALTOONA'
+ },
+ 'shadowed' => false,
+ 'homeLocation' => {
+ 'resource' => '/policy/location', 'key' => 'STACKS-AA'
+ },
+ 'copyNumber' => 1,
+ 'bib' => {
+ 'resource' => '/catalog/bib', 'key' => '26141494'
+ },
+ 'barcode' => '000081321605',
+ 'circulate' => false,
+ 'currentLocation' => {
+ 'resource' => '/policy/location', 'key' => 'ILLEND'
+ }
+ }
+ }],
+ 'classification' => {
+ 'resource' => '/policy/classification', 'key' => 'LC'
+ }
+ }
+ },
+ {
+ 'resource' => '/catalog/call',
+ 'key' => '26141494:3',
+ 'fields' => {
+ 'library' => {
+ 'resource' => '/policy/library', 'key' => 'BEHREND'
+ },
+ 'callNumber' => 'HN13.D52 2019',
+ 'shadowed' => false,
+ 'volumetric' => nil,
+ 'dispCallNumber' => 'HN13.D52 2019',
+ 'bib' => {
+ 'resource' => '/catalog/bib', 'key' => '26141494'
+ },
+ 'itemList' => [{
+ 'resource' => '/catalog/item',
+ 'key' => '26141494:3:1',
+ 'fields' => {
+ 'call' => {
+ 'resource' => '/catalog/call', 'key' => '26141494:3'
+ },
+ 'mediaDesk' => nil,
+ 'itemType' => {
+ 'resource' => '/policy/itemType', 'key' => 'BOOKFLOAT'
+ },
+ 'library' => {
+ 'resource' => '/policy/library', 'key' => 'BEHREND'
+ },
+ 'shadowed' => false,
+ 'homeLocation' => {
+ 'resource' => '/policy/location', 'key' => 'STACKS-BD'
+ },
+ 'copyNumber' => 1,
+ 'bib' => {
+ 'resource' => '/catalog/bib', 'key' => '26141494'
+ },
+ 'barcode' => '000081287932',
+ 'circulate' => false,
+ 'currentLocation' => {
+ 'resource' => '/policy/location', 'key' => 'ILLEND'
+ }
+ }
+ }],
+ 'classification' => {
+ 'resource' => '/policy/classification', 'key' => 'LC'
+ }
+ }
+ },
+ {
+ 'resource' => '/catalog/call',
+ 'key' => '26141494:1',
+ 'fields' => {
+ 'library' => {
+ 'resource' => '/policy/library', 'key' => 'WSCRANTON'
+ },
+ 'callNumber' => 'SN15687372',
+ 'shadowed' => false,
+ 'volumetric' => nil,
+ 'dispCallNumber' => 'SN15687372',
+ 'bib' => {
+ 'resource' => '/catalog/bib', 'key' => '26141494'
+ },
+ 'itemList' => [],
+ 'classification' => {
+ 'resource' => '/policy/classification', 'key' => 'LC'
+ }
+ }
+ }],
+ 'createDate' => '2019-03-11'
+ }
+ }
+ }
+ end
+
factory :bib_with_holdables do
initialize_with {
new(body)
diff --git a/spec/services/place_hold_form/builder_spec.rb b/spec/services/place_hold_form/builder_spec.rb
index 018f1850..1aa161bd 100644
--- a/spec/services/place_hold_form/builder_spec.rb
+++ b/spec/services/place_hold_form/builder_spec.rb
@@ -36,6 +36,24 @@
expect(form_params[:author]).to eq 'Hill Street blues (Television program)'
end
+ context 'when response from Sirsi is an error, meaning no callList set' do
+ before do
+ bib_info.record['fields']['callList'] = ''
+ end
+
+ it 'will return an empty hash' do
+ expect(form_params).to be_empty
+ end
+ end
+
+ context 'when bib response has no holdable items' do
+ let(:bib_info) { build(:bib_with_no_holdable_locations) }
+
+ it 'will return an empty hash' do
+ expect(form_params).to be_empty
+ end
+ end
+
context 'when there are volumetric calls to present to the user' do
it 'will generate holdables when supplied with a body that has a callList' do
expect(form_params[:volumetric_calls].count).to be 8