From 8c0ac9c2d3d60bb1f01606f06740d07bae9fab8c Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Thu, 4 Feb 2016 19:41:57 +0000 Subject: [PATCH] Bug 14514 - LocalHoldsPriority and the HoldsQueue conflict with each other It appears that the LocalHoldsPriority feature and the Holds Queue are fundamentally at odds with each other. The problem appears to be that both are attempting to choose the best way to fill holds. When you are using the holds queue and you check in an item that has been selected by the holds queue builder, that part of Koha where the LocalHoldsPriority feature lives doesn't get to see all the holds in order to pick the best one. Instead only the hold selected by the holds queue builder is returned so to the LocalHoldsPriority feature, that is only one hold to pick from! Test Plan: 1) Apply this patch 2) prove t/db_dependent/HoldsQueue.t 3) All tests should pass --- C4/HoldsQueue.pm | 54 +++++++++++++++++++++++++++++ t/db_dependent/HoldsQueue.t | 82 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 135 insertions(+), 1 deletion(-) diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm index f5a9be7..30334f5 100755 --- a/C4/HoldsQueue.pm +++ b/C4/HoldsQueue.pm @@ -375,6 +375,60 @@ sub MapItemsToHoldRequests { # figure out which item-level requests can be filled my $num_items_remaining = scalar(@$available_items); + + # Look for Local Holds Priority matches first + if ( C4::Context->preference('LocalHoldsPriority') ) { + my $LocalHoldsPriorityPatronControl = + C4::Context->preference('LocalHoldsPriorityPatronControl'); + my $LocalHoldsPriorityItemControl = + C4::Context->preference('LocalHoldsPriorityItemControl'); + + foreach my $request (@$hold_requests) { + last if $num_items_remaining == 0; + + my $local_hold_match; + foreach my $item (@$available_items) { + next + if ( !$item->{holdallowed} ) + || ( $item->{holdallowed} == 1 + && $item->{homebranch} ne $request->{borrowerbranch} ); + + my $local_holds_priority_item_branchcode = + $item->{$LocalHoldsPriorityItemControl}; + + my $local_holds_priority_patron_branchcode = + ( $LocalHoldsPriorityPatronControl eq 'PickupLibrary' ) + ? $request->{branchcode} + : ( $LocalHoldsPriorityPatronControl eq 'HomeLibrary' ) + ? $request->{borrowerbranch} + : undef; + + $local_hold_match = + $local_holds_priority_item_branchcode eq + $local_holds_priority_patron_branchcode; + + if ($local_hold_match) { + if ( exists $items_by_itemnumber{ $item->{itemnumber} } + and not exists $allocated_items{ $item->{itemnumber} } ) + { + $item_map{ $item->{itemnumber} } = { + borrowernumber => $request->{borrowernumber}, + biblionumber => $request->{biblionumber}, + holdingbranch => $item->{holdingbranch}, + pickup_branch => $request->{branchcode} + || $request->{borrowerbranch}, + item_level => 0, + reservedate => $request->{reservedate}, + reservenotes => $request->{reservenotes}, + }; + $allocated_items{ $item->{itemnumber} }++; + $num_items_remaining--; + } + } + } + } + } + foreach my $request (@$hold_requests) { last if $num_items_remaining == 0; diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t index dd9f0b0..a60ea69 100755 --- a/t/db_dependent/HoldsQueue.t +++ b/t/db_dependent/HoldsQueue.t @@ -12,7 +12,7 @@ use C4::Context; use Data::Dumper; -use Test::More tests => 23; +use Test::More tests => 27; use C4::Branch; use C4::Members; @@ -66,6 +66,7 @@ $itemtype or BAIL_OUT("No adequate itemtype"); #FIXME Should be $itemtype = $ite #Set up the stage # Sysprefs and cost matrix +C4::Context->set_preference('LocalHoldsPriority', 0); $dbh->do("UPDATE systempreferences SET value = ? WHERE variable = 'StaticHoldsQueueWeight'", undef, join( ',', @other_branches, $borrower_branchcode, $least_cost_branch_code)); $dbh->do("UPDATE systempreferences SET value = '0' WHERE variable = 'RandomizeHoldsQueueWeight'"); @@ -307,6 +308,85 @@ $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice is( @$holds_queue, 3, "Holds queue filling correct number for holds for default holds policy 'from any library'" ); #warn "HOLDS QUEUE: " . Data::Dumper::Dumper( $holds_queue ); +## Test LocalHoldsPriority +C4::Context->set_preference('LocalHoldsPriority', 1); + +$dbh->do("DELETE FROM default_circ_rules"); +$dbh->do("INSERT INTO default_circ_rules ( holdallowed ) VALUES ( 2 )"); +$dbh->do("DELETE FROM issues"); + +# Test homebranch = patron branch +C4::Context->set_preference('LocalHoldsPriorityPatronControl', 'HomeLibrary'); +C4::Context->set_preference('LocalHoldsPriorityItemControl', 'homebranch'); +C4::Context->clear_syspref_cache(); +$dbh->do("DELETE FROM reserves"); +$sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); +$sth->execute( $borrower2->{borrowernumber}, $biblionumber, $branchcodes[0], 2 ); +$sth->execute( $borrower3->{borrowernumber}, $biblionumber, $branchcodes[0], 3 ); + +$dbh->do("DELETE FROM items"); +# barcode, homebranch, holdingbranch, itemtype +$items_insert_sth->execute( $barcode + 4, $branchcodes[2], $branchcodes[0] ); + +C4::HoldsQueue::CreateQueue(); +$holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} }); +is( $holds_queue->[0]->{cardnumber}, $borrower3->{cardnumber}, "Holds queue giving priority to patron who's home library matches item's home library"); + +# Test holdingbranch = patron branch +C4::Context->set_preference('LocalHoldsPriorityPatronControl', 'HomeLibrary'); +C4::Context->set_preference('LocalHoldsPriorityItemControl', 'holdingbranch'); +C4::Context->clear_syspref_cache(); +$dbh->do("DELETE FROM reserves"); +$sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); +$sth->execute( $borrower2->{borrowernumber}, $biblionumber, $branchcodes[0], 2 ); +$sth->execute( $borrower3->{borrowernumber}, $biblionumber, $branchcodes[0], 3 ); + +$dbh->do("DELETE FROM items"); +# barcode, homebranch, holdingbranch, itemtype +$items_insert_sth->execute( $barcode + 4, $branchcodes[0], $branchcodes[2] ); + +C4::HoldsQueue::CreateQueue(); +$holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} }); +is( $holds_queue->[0]->{cardnumber}, $borrower3->{cardnumber}, "Holds queue giving priority to patron who's home library matches item's holding library"); + +# Test holdingbranch = pickup branch +C4::Context->set_preference('LocalHoldsPriorityPatronControl', 'PickupLibrary'); +C4::Context->set_preference('LocalHoldsPriorityItemControl', 'holdingbranch'); +C4::Context->clear_syspref_cache(); +$dbh->do("DELETE FROM reserves"); +$sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); +$sth->execute( $borrower2->{borrowernumber}, $biblionumber, $branchcodes[0], 2 ); +$sth->execute( $borrower3->{borrowernumber}, $biblionumber, $branchcodes[2], 3 ); + +$dbh->do("DELETE FROM items"); +# barcode, homebranch, holdingbranch, itemtype +$items_insert_sth->execute( $barcode + 4, $branchcodes[0], $branchcodes[2] ); + +C4::HoldsQueue::CreateQueue(); +$holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} }); +is( $holds_queue->[0]->{cardnumber}, $borrower3->{cardnumber}, "Holds queue giving priority to patron who's home library matches item's holding library"); + +# Test homebranch = pickup branch +C4::Context->set_preference('LocalHoldsPriorityPatronControl', 'PickupLibrary'); +C4::Context->set_preference('LocalHoldsPriorityItemControl', 'homebranch'); +C4::Context->clear_syspref_cache(); +$dbh->do("DELETE FROM reserves"); +$sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); +$sth->execute( $borrower2->{borrowernumber}, $biblionumber, $branchcodes[0], 2 ); +$sth->execute( $borrower3->{borrowernumber}, $biblionumber, $branchcodes[2], 3 ); + +$dbh->do("DELETE FROM items"); +# barcode, homebranch, holdingbranch, itemtype +$items_insert_sth->execute( $barcode + 4, $branchcodes[2], $branchcodes[0] ); + +C4::HoldsQueue::CreateQueue(); +$holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} }); +is( $holds_queue->[0]->{cardnumber}, $borrower3->{cardnumber}, "Holds queue giving priority to patron who's home library matches item's holding library"); + +C4::Context->set_preference('LocalHoldsPriority', 0); +## End testing of LocalHoldsPriority + + # Bug 14297 $itemtype = Koha::ItemTypes->search->next->itemtype; $borrowernumber = $borrower3->{borrowernumber}; -- 2.1.4