From cbc52c7fb34a984009c0e605d284213621bd8ebe 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..ca3a735 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); + + # Looks for Local Holds Priority matches first for item-level reqeuests + 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