From 6b610c6d7b67fe2a5bab28afb9a9edc6d381fb97 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Fri, 20 May 2022 11:48:28 -0300
Subject: [PATCH] Bug 30825: Remove GetReservesControlBranch in favour of
 Koha::Item->holds_control_library

This patch removes the GetReservesControlBranch method, and replaces its
uses with the newly introduced method.

To test:
1. Apply this patch
2. Verify that placing holds from the OPAC works
=> SUCCESS: Things work as expected
3. Run:
   $ kshell
  k$ prove t/db_dependent/Reserves* \
           t/db_dependent/Hold* \
           t/db_dependent/Koha/Hold* \
           t/db_dependent/Koha/Biblio.t
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
---
 C4/Reserves.pm            | 36 +++---------------------------------
 opac/opac-reserve.pl      |  5 ++---
 t/db_dependent/Reserves.t | 21 ++-------------------
 3 files changed, 7 insertions(+), 55 deletions(-)

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 5193a5454c..659f379bfd 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -134,8 +134,6 @@ BEGIN {
       ToggleSuspend
       SuspendAll
 
-      GetReservesControlBranch
-
       CalculatePriority
 
       IsItemOnHoldAndFound
@@ -922,7 +920,7 @@ sub CheckReserves {
                     $item ||= Koha::Items->find($itemnumber);
                     next if $res->{itemtype} && $res->{itemtype} ne $item->effective_itemtype;
                     $patron ||= Koha::Patrons->find( $res->{borrowernumber} );
-                    my $branch = GetReservesControlBranch( $item->unblessed, $patron->unblessed );
+                    my $branch = $item->holds_control_library( $patron );
                     my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$item->effective_itemtype);
                     next if ($branchitemrule->{'holdallowed'} eq 'not_allowed');
                     next if (($branchitemrule->{'holdallowed'} eq 'from_home_library') && ($item->homebranch ne $patron->branchcode));
@@ -1343,8 +1341,7 @@ sub IsAvailableForItemLevelRequest {
         return 0 unless $destination;
         return 0 unless $destination->pickup_location;
         return 0 unless $item->can_be_transferred( { to => $destination } );
-        my $reserves_control_branch =
-            GetReservesControlBranch( $item->unblessed(), $patron->unblessed() );
+        my $reserves_control_branch = $item->holds_control_library( $patron );
         my $branchitemrule =
             C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->itype );
         my $home_library = Koha::Libraries->find( {branchcode => $item->homebranch} );
@@ -1386,8 +1383,7 @@ sub ItemsAnyAvailableAndNotRestricted {
     my @items = Koha::Items->search( { biblionumber => $param->{biblionumber} } )->as_list;
 
     foreach my $i (@items) {
-        my $reserves_control_branch =
-            GetReservesControlBranch( $i->unblessed(), $param->{patron}->unblessed );
+        my $reserves_control_branch = $i->holds_control_library( $param->{patron} );
         my $branchitemrule =
             C4::Circulation::GetBranchItemRule( $reserves_control_branch, $i->itype );
         my $item_library = Koha::Libraries->find( { branchcode => $i->homebranch } );
@@ -2158,32 +2154,6 @@ 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;
-}
-
 =head2 CalculatePriority
 
     my $p = CalculatePriority($biblionumber, $resdate);
diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
index b8ffca4e44..ebe6c73645 100755
--- a/opac/opac-reserve.pl
+++ b/opac/opac-reserve.pl
@@ -24,7 +24,7 @@ use CGI qw ( -utf8 );
 use C4::Auth qw( get_template_and_user );
 use C4::Koha qw( getitemtypeimagelocation getitemtypeimagesrc );
 use C4::Circulation qw( GetBranchItemRule GetTransfers );
-use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve GetReservesControlBranch ItemsAnyAvailableAndNotRestricted IsAvailableForItemLevelRequest );
+use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve ItemsAnyAvailableAndNotRestricted IsAvailableForItemLevelRequest );
 use C4::Biblio qw( GetBiblioData GetFrameworkCode GetMarcBiblio );
 use C4::Items qw( GetHostItemsInfo GetItemsInfo );
 use C4::Output qw( output_html_with_http_headers );
@@ -559,8 +559,7 @@ foreach my $biblioNum (@biblionumbers) {
         # If there is no loan, return and transfer, we show a checkbox.
         $itemLoopIter->{notforloan} = $itemLoopIter->{notforloan} || 0;
 
-        my $patron_unblessed = $patron->unblessed;
-        my $branch = GetReservesControlBranch( $itemInfo, $patron_unblessed );
+        my $branch = $item->holds_control_library( $patron );
 
         my $policy_holdallowed = !$itemLoopIter->{already_reserved};
         # items_any_available defined outside of the current loop,
diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t
index 5bda45c4fa..ba3fe4d4ba 100755
--- a/t/db_dependent/Reserves.t
+++ b/t/db_dependent/Reserves.t
@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 71;
+use Test::More tests => 69;
 use Test::MockModule;
 use Test::Warn;
 
@@ -31,7 +31,7 @@ use C4::Circulation qw( AddReturn AddIssue );
 use C4::Items;
 use C4::Biblio qw( GetMarcBiblio GetMarcFromKohaField ModBiblio );
 use C4::Members;
-use C4::Reserves qw( AddReserve AlterPriority CheckReserves GetReservesControlBranch ModReserve ModReserveAffect ReserveSlip CalculatePriority CanReserveBeCanceledFromOpac CanBookBeReserved IsAvailableForItemLevelRequest MoveReserve ChargeReserveFee RevertWaitingStatus CanItemBeReserved MergeHolds );
+use C4::Reserves qw( AddReserve AlterPriority CheckReserves ModReserve ModReserveAffect ReserveSlip CalculatePriority CanReserveBeCanceledFromOpac CanBookBeReserved IsAvailableForItemLevelRequest MoveReserve ChargeReserveFee RevertWaitingStatus CanItemBeReserved MergeHolds );
 use Koha::ActionLogs;
 use Koha::Caches;
 use Koha::DateUtils qw( dt_from_string output_pref );
@@ -131,23 +131,6 @@ 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');
-t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'ItemHomeLibrary' );
-ok(
-    'ItemHomeLib' eq GetReservesControlBranch(
-        { homebranch => 'ItemHomeLib' },
-        { branchcode => 'PatronHomeLib' }
-    ), "GetReservesControlBranch returns item home branch when set to ItemHomeLibrary"
-);
-t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' );
-ok(
-    'PatronHomeLib' eq GetReservesControlBranch(
-        { homebranch => 'ItemHomeLib' },
-        { branchcode => 'PatronHomeLib' }
-    ), "GetReservesControlBranch returns patron home branch when set to PatronLibrary"
-);
-t::lib::Mocks::mock_preference( 'ReservesControlBranch', $ReservesControlBranch );
-
 ###
 ### Regression test for bug 10272
 ###
-- 
2.30.2