From 2448e200e2b520ce5bfe01f3256f1de0659d71c7 Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Fri, 17 May 2013 07:08:24 -0500
Subject: [PATCH] Bug 10272: make CheckReserves respect ReservesControlBranch

CheckReserves was 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) Set ReservesControlBranch to "item's home library".

2) 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.

3) 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".

4) 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!

5) 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.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Fixed POD so that arguments and explanation match (C<$item>).
Also tested opac-reserves.pl for regressions.
Passes all tests, QA script, and Reserves.t.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
(cherry picked from commit 78eba2f974ca9e8ff51f64d2356b004e7bfcecff)
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
---
 C4/Reserves.pm            | 30 +++++++++++++++++++++++++++++-
 opac/opac-reserve.pl      |  2 +-
 t/db_dependent/Reserves.t | 19 ++++++++++++++++++-
 3 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 9d9be05b74..7273802d56 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -130,6 +130,8 @@ BEGIN {
         &ReserveSlip
         &ToggleSuspend
         &SuspendAll
+
+        &GetReservesControlBranch
     );
     @EXPORT_OK = qw( MergeHolds );
 }    
@@ -874,7 +876,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'}));
@@ -2179,6 +2181,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<$item> 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 <http://koha-community.org/>
diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
index d0da2c5de6..57181aa26b 100755
--- a/opac/opac-reserve.pl
+++ b/opac/opac-reserve.pl
@@ -489,7 +489,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 0e0852aaf9..5f8bc9fe1e 100755
--- a/t/db_dependent/Reserves.t
+++ b/t/db_dependent/Reserves.t
@@ -2,7 +2,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 4;
+use Test::More tests => 6;
 use MARC::Record;
 
 use C4::Branch;
@@ -76,3 +76,20 @@ is($status, "Reserved", "CheckReserves Test 2");
 
 ($status, $reserve, $all_reserves) = CheckReserves(undef, $barcode);
 is($status, "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 );
-- 
2.39.5