Bugzilla – Attachment 56126 Details for
Bug 14514
LocalHoldsPriority and the HoldsQueue conflict with each other
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 14514 - LocalHoldsPriority and the HoldsQueue conflict with each other
Bug-14514---LocalHoldsPriority-and-the-HoldsQueue-.patch (text/plain), 9.24 KB, created by
Danielle Elder
on 2016-10-11 07:51:00 UTC
(
hide
)
Description:
Bug 14514 - LocalHoldsPriority and the HoldsQueue conflict with each other
Filename:
MIME Type:
Creator:
Danielle Elder
Created:
2016-10-11 07:51:00 UTC
Size:
9.24 KB
patch
obsolete
>From f344e837075413896f16cc15dd54887e8c4a3f02 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >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 > >Signed-off-by: Barton Chittenden barton@bywatersolutions.com >Signed-off-by: Dani Elder <dani@bywatersolutions.com> >--- > 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 1a2de1d..e5f29c7 100755 >--- a/C4/HoldsQueue.pm >+++ b/C4/HoldsQueue.pm >@@ -388,6 +388,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 0416c24..43043d5 100755 >--- a/t/db_dependent/HoldsQueue.t >+++ b/t/db_dependent/HoldsQueue.t >@@ -8,7 +8,7 @@ > > use Modern::Perl; > >-use Test::More tests => 38; >+use Test::More tests => 42; > use Data::Dumper; > > use C4::Calendar; >@@ -62,6 +62,7 @@ my $itemtype = $builder->build({ source => 'Itemtype', value => { notforloan => > #Set up the stage > # Sysprefs and cost matrix > C4::Context->set_preference('HoldsQueueSkipClosed', 0); >+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'"); >@@ -332,6 +333,85 @@ $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice > is( scalar( @$holds_queue ), 2, "Holds not filled with items from closed libraries" ); > C4::Context->set_preference( 'HoldsQueueSkipClosed', 0 ); > >+## 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 = $builder->build({ source => 'Itemtype', value => { notforloan => 0 } })->{itemtype}; > $borrowernumber = $borrower3->{borrowernumber}; >-- >2.1.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 14514
:
47654
|
47655
|
49089
|
51595
|
53787
|
56126
|
56456