From 92deb9cd8aa31f49ab406f776d7a9fcc318d58a1 Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Wed, 24 Aug 2016 11:41:57 -0400 Subject: [PATCH] LP#1205492: fix crash when attempting to renew deposit & rental loans in OPAC This patch fixes a bug where attempting to renew a loan on an item that requires a deposit or a rental fee can cause an Apache internal server error. In particular, this patch supplies descriptions for the following events: ITEM_DEPOSIT_REQUIRED ITEM_RENTAL_FEE_REQUIRED ITEM_DEPOSIT_PAID It also normalizes how 'fail_part' is set in the renewal response, as the payload of an event returned when a renewal fails can be either a hash or an acp Fieldmapper object depending on the type of event. In the former case, attempting to access an nonexistent ->fail_part method is what causes the crash. To test ------- [1] Create an item that requires a deposit or a rental fee and check it out. [2] Attempt to renew the loan in the public catalog. Note that an internal server error is returned. [3] Apply the patch and attept step 2 again. This time, the public catalog should display a notification to the patron that the renewal cannot take place, rather than crashing. [4] Set up other situations where a renewal should fail for reasons other than a deposit/rental fee being required. Verify that appropriate error messages are displayed in the public catalog. Signed-off-by: Galen Charlton Signed-off-by: Mike Rylander --- Open-ILS/src/extras/ils_events.xml | 6 +++--- .../src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm | 9 +++++++++ Open-ILS/src/templates/opac/myopac/circs.tt2 | 4 ++-- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/Open-ILS/src/extras/ils_events.xml b/Open-ILS/src/extras/ils_events.xml index 2ecd618bae..b6f060369c 100644 --- a/Open-ILS/src/extras/ils_events.xml +++ b/Open-ILS/src/extras/ils_events.xml @@ -159,13 +159,13 @@ The selected bib record has volumes attached - + Payment of an item deposit is required. - + Payment of an item rental fee is required. - + An item deposit was paid. While you were trying to make payments, this account's transaction history changed. Please go back and try again. diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm index 1219b7046c..d2be12d789 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm @@ -1566,6 +1566,15 @@ sub load_myopac_circs { if($resp) { my $evt = ref($resp->{evt}) eq 'ARRAY' ? $resp->{evt}->[0] : $resp->{evt}; + + # extract the fail_part, if present, from the event payload; + # since # the payload is an acp object in some cases, + # blindly looking for a # 'fail_part' key in the template can + # break things + $evt->{fail_part} = (ref($evt->{payload}) eq 'HASH' && exists $evt->{payload}->{fail_part}) ? + $evt->{payload}->{fail_part} : + ''; + $data->{renewal_response} = $evt; $success_renewals++ if $evt->{textcode} eq 'SUCCESS'; $failed_renewals++ if $evt->{textcode} ne 'SUCCESS'; diff --git a/Open-ILS/src/templates/opac/myopac/circs.tt2 b/Open-ILS/src/templates/opac/myopac/circs.tt2 index c78c153632..4715925ed4 100644 --- a/Open-ILS/src/templates/opac/myopac/circs.tt2 +++ b/Open-ILS/src/templates/opac/myopac/circs.tt2 @@ -179,7 +179,7 @@ circ.renewal_response.textcode != 'SUCCESS' %] [%# XXX colspan="0" does not work in IE %] - + [% renew_fail_msg = ''; IF circ.renewal_response.textcode == 'TOTAL_HOLD_COPY_RATIO_EXCEEDED' OR @@ -188,7 +188,7 @@ renew_fail_msg = l('Item is needed for a hold'); ELSE; renew_fail_msg = circ.renewal_response.desc || - circ.renewal_response.payload.fail_part || + circ.renewal_response.fail_part || circ.renewal_response.textcode; END; renew_fail_msg | html; -- 2.43.2