Lp 1861319: Repair expire setting logic

Move the checks in Circulate.pm from mk_env to do_renew after the
circ_lib has been set for opac, auto, and desk renewals.

Only check for expired patrons in mk_env if we're not doing a renewal,
i.e. it's a regular checkout.

OPINION: That checks for patron validity in mk_env should be moved to
their own subroutine and called from do_checkout and do_renew.

Signed-off-by: Jason Stephenson <jason@sigio.com>
Signed-off-by: John Amundson <jamundson@cwmars.org>
Signed-off-by: Galen Charlton <gmc@equinoxinitiative.org>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm
index 0d36dae..7ac877b 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm
@@ -1019,24 +1019,15 @@ sub mk_env {
     
         $self->bail_on_events(OpenILS::Event->new('PATRON_CARD_INACTIVE'))
             unless $U->is_true($patron->card->active);
-    
-        my $expire = DateTime::Format::ISO8601->new->parse_datetime(
-            clean_ISO8601($patron->expire_date));
 
-        # An expired patron can renew with the assistance of an OUS.
-        if($self->opac_renewal or $self->auto_renewal) {
-            my $use_circ_lib = $self->editor->retrieve_config_global_flag('circ.opac_renewal.use_original_circ_lib');
-            if($use_circ_lib and $U->is_true($use_circ_lib->enabled)) {
-                $self->circ_lib($self->circ->circ_lib);
-            }
-            else {
-                $self->circ_lib($patron->home_ou);
-            }
-        }
+        # Expired patrons cannot check out.  Renewals for expired
+        # patrons depend on a setting and will be checked in the
+        # do_renew subroutine.
+        if ($self->is_checkout) {
+            my $expire = DateTime::Format::ISO8601->new->parse_datetime(
+                clean_ISO8601($patron->expire_date));
 
-        my $expire_setting = $U->ou_ancestor_setting_value($self->circ_lib, OILS_SETTING_ALLOW_RENEW_FOR_EXPIRED_PATRON);
-        unless ($self->is_renewal and $expire_setting) {
-            if(CORE::time > $expire->epoch) {
+            if (CORE::time > $expire->epoch) {
                 $self->bail_on_events(OpenILS::Event->new('PATRON_ACCOUNT_EXPIRED'))
             }
         }
@@ -4132,6 +4123,15 @@ sub do_renew {
         $self->circ_lib($circ->circ_lib) if($desk_renewal_use_circ_lib);
     }
 
+    # Check if expired patron is allowed to renew, and bail if not.
+    my $expire = DateTime::Format::ISO8601->new->parse_datetime(clean_ISO8601($self->patron->expire_date));
+    if (CORE::time > $expire->epoch) {
+        my $allow_renewal = $U->ou_ancestor_setting_value($self->circ_lib, OILS_SETTING_ALLOW_RENEW_FOR_EXPIRED_PATRON);
+        unless ($U->is_true($allow_renewal)) {
+            return $self->bail_on_events(OpenILS::Event->new('PATRON_ACCOUNT_EXPIRED'));
+        }
+    }
+
     # Run the fine generator against the old circ
     # XXX This seems unnecessary, given that handle_fines runs in do_checkin
     # a few lines down.  Commenting out, for now.