From 616e85ff061360f9fb4c21e0ce3edec5f3ab4e63 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Fri, 17 May 2013 07:08:24 -0500 Subject: [PATCH] Bug 10272 - CheckReserves returns not respecting ReservesControlBranch CheckReserves is using the CircControl system preference to determine what patrons an item can fill a hold for. It should be using ReservesControlBranch instead. Test Plan: 1) Create an item at Library A, place holds for it for patrons at Library B, Library C, and Library A in that order, for pickup at the patrons home library. 2) Make sure the holds policy for Library A is set to Hold Policy = "From home library" and Return Policy = "Item returns home". Make sure the holds policies for the other libraries are set to Hold Policy = "From any library". 3) Check the item in at Library C, the hold for the patron at Library B should pop up, even though it's in violation of the circulation rules. Don't click the confirm button! 4) Apply this patch, and reload the page, now the hold listed should be for the last hold, the hold for the patron at Library A, which is correct. This patch adds the subroutine C4::Reserves::GetReservesControlBranch as an equivilent to C4::Circulation::_GetCircControlBranch. --- C4/Reserves.pm | 30 +++++++++++++++++++++++++++++- opac/opac-reserve.pl | 2 +- t/db_dependent/Reserves.t | 18 +++++++++++++++++- 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 5ff12e4..6200f15 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -130,6 +130,8 @@ BEGIN { &ReserveSlip &ToggleSuspend &SuspendAll + + &GetReservesControlBranch ); @EXPORT_OK = qw( MergeHolds ); } @@ -863,7 +865,7 @@ sub CheckReserves { if ( $res->{'priority'} && $res->{'priority'} < $priority ) { my $borrowerinfo=C4::Members::GetMember(borrowernumber => $res->{'borrowernumber'}); my $iteminfo=C4::Items::GetItem($itemnumber); - my $branch=C4::Circulation::_GetCircControlBranch($iteminfo,$borrowerinfo); + my $branch = GetReservesControlBranch( $iteminfo, $borrowerinfo ); my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$iteminfo->{'itype'}); next if ($branchitemrule->{'holdallowed'} == 0); next if (($branchitemrule->{'holdallowed'} == 1) && ($branch ne $borrowerinfo->{'branchcode'})); @@ -2168,6 +2170,32 @@ sub ReserveSlip { ); } +=head2 GetReservesControlBranch + + my $reserves_control_branch = GetReservesControlBranch($item, $borrower); + + Return the branchcode to be used to determine which reserves + policy applies to a transaction. + + C<$iteminfos> is a hashref for an item. Only 'homebranch' is used. + + C<$borrower> is a hashref to borrower. Only 'branchcode' is used. + +=cut + +sub GetReservesControlBranch { + my ( $item, $borrower ) = @_; + + my $reserves_control = C4::Context->preference('ReservesControlBranch'); + + my $branchcode = + ( $reserves_control eq 'ItemHomeLibrary' ) ? $item->{'homebranch'} + : ( $reserves_control eq 'PatronLibrary' ) ? $borrower->{'branchcode'} + : undef; + + return $branchcode; +} + =head1 AUTHOR Koha Development Team diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index be72e84..4bba9a6 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -490,7 +490,7 @@ foreach my $biblioNum (@biblionumbers) { # If there is no loan, return and transfer, we show a checkbox. $itemLoopIter->{notforloan} = $itemLoopIter->{notforloan} || 0; - my $branch = ( C4::Context->preference('ReservesControlBranch') eq 'ItemHomeLibrary' ) ? $itemInfo->{'homebranch'} : $borr->{'branchcode'}; + my $branch = GetReservesControlBranch( $itemInfo, $borr ); my $branchitemrule = GetBranchItemRule( $branch, $itemInfo->{'itype'} ); my $policy_holdallowed = 1; diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 1e20318..ddb9b60 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -4,7 +4,7 @@ use strict; use warnings; use C4::Branch; -use Test::More tests => 4; +use Test::More tests => 6; use MARC::Record; use C4::Biblio; use C4::Items; @@ -66,6 +66,22 @@ ok($status eq "Reserved", "CheckReserves Test 2"); ($status, $reserve, $all_reserves) = CheckReserves(undef, $barcode); ok($status eq "Reserved", "CheckReserves Test 3"); +my $ReservesControlBranch = C4::Context->preference('ReservesControlBranch'); +C4::Context->set_preference( 'ReservesControlBranch', 'ItemHomeLibrary' ); +ok( + 'ItemHomeLib' eq GetReservesControlBranch( + { homebranch => 'ItemHomeLib' }, + { branchcode => 'PatronHomeLib' } + ), "GetReservesControlBranch returns item home branch when set to ItemHomeLibrary" +); +C4::Context->set_preference( 'ReservesControlBranch', 'PatronLibrary' ); +ok( + 'PatronHomeLib' eq GetReservesControlBranch( + { homebranch => 'ItemHomeLib' }, + { branchcode => 'PatronHomeLib' } + ), "GetReservesControlBranch returns patron home branch when set to PatronLibrary" +); +C4::Context->set_preference( 'ReservesControlBranch', $ReservesControlBranch ); # Teardown Test--------------------- # Delete item. -- 1.7.10.4