From 990a5e538c9498e1755aeecc95be9afa1c2184d6 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi 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 --- 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.34.1