From 2c413ef29afd84b79693beaf64f89a5fd1f2bb90 Mon Sep 17 00:00:00 2001 From: Josh Stompro Date: Sat, 31 Oct 2015 10:38:41 -0500 Subject: [PATCH] LP#1511828: sort proximities numerically when targeting holds The hold targeter had three instances of using lexical sorting rather than numerical sorting. Two of the instances affected hold targeting if proximity values were over 9. Proximity values were being sorted lexically. For instance, 1,2,5,11,15,100,120 was sorted as 1,100,11,120,15,2,5, causing interesting hold targeting results. Normally proximity doesn't go that high so it isn't a problem, but we were using proximity adjustments to strictly order locations so each location had a different proximity, driving the numbers above 9. One instance wasn't causing any current issue but could cause problems in the future if more best hold selection sort options are added to bring the total number of sort options over 9. Also included are some changes to reduce warnings in the logs suggested by Bill Erickson. The sorting problem was found by Galen Charlton, thanks Galen. Testing Notes - to trigger this issue you need to have adjusted proximities over 9. 1. Use proximity adjustment rules to add a +10 proximity adjustment to one item circ lib, Branch A and a +2 proximity adjustment to the item circ lib Branch B. 2. Find a title with a copy at Branch A and B. Place a hold with a pickup location of Branch B. 3. The copy at Branch A should get targeted since it has a proximity of 12-14 (10+normal proximity) and Branch B has a proximity of 2 (2+normal proximity). 12 gets sorted before 2 lexically. 4. After the fix the copy at Branch B should get targeted since 2 is sorted before 12. Signed-off-by: Josh Stompro Signed-off-by: Galen Charlton --- .../lib/OpenILS/Application/Storage/Publisher/action.pm | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 3d592d66d1..652acc1883 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm @@ -363,8 +363,8 @@ sub get_hold_sort_order { # Return only the keys of our hash, sorted by value, # keys for null values omitted. return [ - grep { defined $row->{$_} } ( - sort {$row->{$a} cmp $row->{$b}} keys %$row + sort { $row->{$a} <=> $row->{$b} } ( + grep { defined $row->{$_} } keys %$row ) ]; } @@ -1433,11 +1433,11 @@ sub new_hold_copy_targeter { $all_copies = [ grep { ''.$_->circ_lib ne $pu_lib && ( $_->status == 0 || $_->status == 7 ) } @good_copies ]; - my $min_prox = [ sort keys %$prox_list ]->[0]; + my $min_prox = [ sort {$a<=>$b} keys %$prox_list ]->[0]; my $best; if ($hold->hold_type eq 'R' || $hold->hold_type eq 'F') { # Recall/Force holds bypass hold rules. $best = $good_copies[0] if(scalar @good_copies); - } else { + } elsif (defined $min_prox) { $best = choose_nearest_copy($hold, { $min_prox => delete($$prox_list{$min_prox}) }); } @@ -1989,7 +1989,7 @@ sub choose_nearest_copy { my $hold = shift; my $prox_list = shift; - for my $p ( sort keys %$prox_list ) { + for my $p ( sort {$a<=>$b} keys %$prox_list ) { next unless (ref $$prox_list{$p}); my @capturable = @{ $$prox_list{$p} }; -- 2.11.0