LP#? Hold targeter refactoring and optimization.

Signed-off-by: Bill Erickson <berickxx@gmail.com>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm
index 93f7cf0..d598057 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm
@@ -681,19 +681,30 @@ sub inspect_previous_target {
     # See if the previous copy is in our list of valid copies.
     my ($prev) = grep {$_->{id} eq $prev_id} @copies;
 
-    # previous target is no longer valid.
+    # exit if previous target is no longer valid.
     return 1 unless $prev;
 
     if ($self->{skip_viable}) {
-        # In skip_viable mode, leave the hold as-is if
-        # the existing current_copy is still permitted.
+        # In skip_viable mode, leave the hold as-is if the existing
+        # current_copy is still permitted.
+        # Note: viability checking is done this late in the process
+        # (specifically after other potential copies have been fetched)
+        # because we first need to confirm the current_copy is a valid
+        # potential copy (e.g. it's holdable, non-deleted, etc.), which
+        # copy_is_permitted, which only checks hold matrix policies,
+        # does not check.
 
         return $self->exit_targeter("Skipping with viable target = $prev_id")
             if $self->copy_is_permitted($prev);
-    }
 
-    # Previous copy is targetable.  Keep it around for later.
-    $self->{valid_previous_copy} = $prev;
+        # Previous copy is now confirmed non-viable.
+
+    } else {
+
+        # Previous copy may be targetable.  Keep it around for later
+        # in case we need to confirm its viability and re-use it.
+        $self->{valid_previous_copy} = $prev;
+    }
 
     # Remove the previous copy from the working set of potential copies.
     # It will be revisited later if needed.