From 4355cc61ad4271f5aec7dbab371f3d46cbda0a33 Mon Sep 17 00:00:00 2001 From: Jason Etheridge Date: Fri, 5 Jun 2020 09:00:53 -0400 Subject: [PATCH] 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 Signed-off-by: Rogan Hamby Signed-off-by: John Amundson Signed-off-by: Jane Sandberg --- .../lib/OpenILS/Application/Circ/Circulate.pm | 57 ++++++++++++++----- 1 file changed, 42 insertions(+), 15 deletions(-) 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 d99d5259c4..66280f5999 100644 --- 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; -- 2.43.2