LP1819542 Hanging transits can cause checkins to fail

So two bits of defensive programming for do_checkin:

    sub fix_broken_transit_status
    sub cancel_transit_if_circ_exists

I don't know if the first one does anything useful, but the idea is that it'll
at least temporarily set the copy status to In Transit for any status checks
within the do_checkin method that test for that.  It doesn't actually repair
the status permanently (at least in the case of, say, an existing transit being
re-used for a ROUTE_ITEM event).  We may want to do that.

The second one will abort an associated transit (including retargeting a hold
for a hold transit) if both an active transit and an active circulation exist
for the item.  This handles the situation I've been using to test the bug:

    1) transit an item (CONC90000436 in Concerto)
    2) artificially change its status directly in the database (for example, to
       Available)
    3) check it out to a patron (99999376864 in Concerto), noting
       that the Cancel Transit prompt does not get triggered
    4) check in the item

Signed-off-by: Jason Etheridge <jason@EquinoxInitiative.org>
Signed-off-by: Rogan Hamby <rogan.hamby@gmail.com>
Signed-off-by: John Amundson <jamundson@cwmars.org>
Signed-off-by: Jane Sandberg <sandbej@linnbenton.edu>

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 d99d525..66280f5 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm
@@ -2518,6 +2518,46 @@ sub checkout_noncat {
    }
 }
 
+# if an item is in transit but the status doesn't agree, then we need to fix things.
+# The next two subs will hopefully do that
+sub fix_broken_transit_status {
+    my $self = shift;
+
+    # Capture the transit so we don't have to fetch it again later during checkin
+    # This used to live in sub check_transit_checkin_interval and later again in
+    # do_checkin
+    $self->transit(
+        $self->editor->search_action_transit_copy(
+            {target_copy => $self->copy->id, dest_recv_time => undef, cancel_time => undef}
+        )->[0]
+    );
+
+    if ($self->transit && $U->copy_status($self->copy->status)->id != OILS_COPY_STATUS_IN_TRANSIT) {
+        $logger->warn("circulator: we have a copy ".$self->copy->barcode.
+            " that is in-transit but without the In Transit status... fixing");
+        $self->copy->status(OILS_COPY_STATUS_IN_TRANSIT);
+        # FIXME - do we want to make this permanent if the checkin bails?
+        $self->update_copy;
+    }
+
+}
+sub cancel_transit_if_circ_exists {
+    my $self = shift;
+    if ($self->circ && $self->transit) {
+        $logger->warn("circulator: we have a copy ".$self->copy->barcode.
+            " that is in-transit AND circulating... aborting the transit");
+        my $circ_ses = create OpenSRF::AppSession("open-ils.circ");
+        my $result = $circ_ses->request(
+            "open-ils.circ.transit.abort",
+            $self->editor->authtoken,
+            { 'transitid' => $self->transit->id }
+        )->gather(1);
+        $logger->warn("circulator: transit abort result: ".$result);
+        $circ_ses->disconnect;
+        $self->transit(undef);
+    }
+}
+
 # If a copy goes into transit and is then checked in before the transit checkin 
 # interval has expired, push an event onto the overridable events list.
 sub check_transit_checkin_interval {
@@ -2530,13 +2570,6 @@ sub check_transit_checkin_interval {
     my $interval = $U->ou_ancestor_setting_value($self->circ_lib, 'circ.transit.min_checkin_interval');
     return unless $interval;
 
-    # capture the transit so we don't have to fetch it again later during checkin
-    $self->transit(
-        $self->editor->search_action_transit_copy(
-            {target_copy => $self->copy->id, dest_recv_time => undef, cancel_time => undef}
-        )->[0]
-    ); 
-
     # transit from X to X for whatever reason has no min interval
     return if $self->transit->source == $self->transit->dest;
 
@@ -2648,6 +2681,7 @@ sub do_checkin {
         OpenILS::Event->new('ASSET_COPY_NOT_FOUND')) 
         unless $self->copy;
 
+    $self->fix_broken_transit_status; # if applicable
     $self->check_transit_checkin_interval;
     $self->checkin_retarget;
 
@@ -2663,6 +2697,7 @@ sub do_checkin {
         $logger->warn("circulator: we have ".scalar(@$circs).
             " open circs for copy " .$self->copy->id."!!") if @$circs > 1;
     }
+    $self->cancel_transit_if_circ_exists; # if applicable
 
     my $stat = $U->copy_status($self->copy->status)->id;
 
@@ -2734,14 +2769,6 @@ sub do_checkin {
     $self->override_events unless $self->is_renewal;
     return if $self->bail_out;
     
-    if( $self->copy and !$self->transit ) {
-        $self->transit(
-            $self->editor->search_action_transit_copy(
-                { target_copy => $self->copy->id, dest_recv_time => undef, cancel_time => undef }
-            )->[0]
-        ); 
-    }
-
     if( $self->circ ) {
         $self->checkin_handle_circ_start;
         return if $self->bail_out;