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 <gmc@esilibrary.com>
Signed-off-by: Mike Rylander <mrylander@gmail.com>

diff --git a/Open-ILS/src/extras/ils_events.xml b/Open-ILS/src/extras/ils_events.xml
index 2ecd618..b6f0603 100644 (file)
--- a/Open-ILS/src/extras/ils_events.xml
+++ b/Open-ILS/src/extras/ils_events.xml
@@ -159,13 +159,13 @@
                <desc xml:lang="en-US">The selected bib record has volumes attached</desc>
        </event>
        <event code='1232' textcode='ITEM_DEPOSIT_REQUIRED'>
-               <desc xml:lang="en-US"></desc>
+               <desc xml:lang="en-US">Payment of an item deposit is required.</desc>
        </event>
        <event code='1233' textcode='ITEM_RENTAL_FEE_REQUIRED'>
-               <desc xml:lang="en-US"></desc>
+               <desc xml:lang="en-US">Payment of an item rental fee is required.</desc>
        </event>
        <event code='1234' textcode='ITEM_DEPOSIT_PAID'>
-               <desc xml:lang="en-US"></desc>
+               <desc xml:lang="en-US">An item deposit was paid.</desc>
        </event>
        <event code='1235' textcode='INVALID_USER_XACT_ID'>
         <desc xml:lang="en-US">While you were trying to make payments, this account's transaction history changed.  Please go back and try again.</desc>

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 1219b70..d2be12d 100644 (file)
--- 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 c78c153..4715925 100644 (file)
--- 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' %]
                     <tr>
                         <td colspan="6">[%# XXX colspan="0" does not work in IE %]
-                            <span class="failure-text" title="[% circ.renewal_response.textcode | html %] / [% circ.renewal_response.payload.fail_part | html %]">
+                            <span class="failure-text" title="[% circ.renewal_response.textcode | html %] / [% circ.renewal_response.fail_part | html %]">
                                 [%
                                     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;