Hold Targeter: Ensure old best still valid

Re-using a now invalid copy is a bad idea for various reasons.

Signed-off-by: Thomas Berezansky <tsbere@mvlc.org>
Signed-off-by: Mike Rylander <mrylander@gmail.com>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm
index 0f36c83..7c07e6f 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm
@@ -1338,11 +1338,13 @@ sub new_hold_copy_targeter {
                        $log->debug("\t".scalar(@good_copies)." (non-current) copies available for targeting...");
 
                        my $old_best = $hold->current_copy;
+                       my $old_best_still_valid = 0; # Assume no, but the next line says yes if it is still a potential.
+                       $old_best_still_valid = 1 if ( $old_best && grep { ''.$old_best->id eq ''.$_->id } @$all_copies );
                        $hold->update({ current_copy => undef }) if ($old_best);
        
                        if (!scalar(@good_copies)) {
                                $log->info("\tNo (non-current) copies eligible to fill the hold.");
-                               if ( $old_best && grep { ''.$old_best->id eq ''.$_->id } @$all_copies ) {
+                               if ( $old_best_still_valid ) {
                                        # the old copy is still available
                                        $log->debug("\tPushing current_copy back onto the targeting list");
                                        push @good_copies, $old_best;
@@ -1488,14 +1490,25 @@ sub new_hold_copy_targeter {
                                $hold->update( { current_copy => ''.$best->id, prev_check_time => 'now' } );
                                $log->debug("\tUpdating hold [".$hold->id."] with new 'current_copy' [".$best->id."] for hold fulfillment.");
                        } elsif (
-                               $old_best &&
+                               $old_best_still_valid &&
                                !action::hold_request
                                        ->search_where(
                                                { current_copy => $old_best->id,
                                                  fulfillment_time => undef,
                                                  cancel_time => undef,
                                                }       
-                                       )
+                                       ) &&
+                               ( OpenILS::Utils::PermitHold::permit_copy_hold(
+                                       { title => $old_best->call_number->record->to_fieldmapper,
+                                         title_descriptor => $old_best->call_number->record->record_descriptor->next->to_fieldmapper,
+                                         patron => $hold->usr->to_fieldmapper,
+                                         copy => $old_best->to_fieldmapper,
+                                         requestor => $hold->requestor->to_fieldmapper,
+                                         request_lib => $hold->request_lib->to_fieldmapper,
+                                         pickup_lib => $hold->pickup_lib->id,
+                                         retarget => 1
+                                       }
+                               ))
                        ) {     
                                $hold->update( { prev_check_time => 'now', current_copy => ''.$old_best->id } );
                                $log->debug( "\tRetargeting the previously targeted copy [".$old_best->id."]" );