From ccb82110c421990b7f00ccbe02c7d00bac71bff8 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 Signed-off-by: Martin Renvoize Signed-off-by: Kyle M Hall Signed-off-by: David Nind --- C4/Reserves.pm | 36 +++--------------------------------- opac/opac-reserve.pl | 4 ++-- t/db_dependent/Reserves.t | 21 ++------------------- 3 files changed, 7 insertions(+), 54 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index b771a37083..a28e3dcba3 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -134,8 +134,6 @@ BEGIN { ToggleSuspend SuspendAll - GetReservesControlBranch - CalculatePriority IsItemOnHoldAndFound @@ -939,7 +937,7 @@ sub CheckReserves { next if $res->{item_group_id} && ( !$item->item_group || $item->item_group->id != $res->{item_group_id} ); 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)); @@ -1359,8 +1357,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} ); @@ -1409,8 +1406,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 } ); @@ -2185,32 +2181,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 118a63d1a7..3e1878e9cc 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -25,7 +25,7 @@ use CGI qw ( -utf8 ); use C4::Auth qw( get_template_and_user ); use C4::Koha qw( getitemtypeimagelocation getitemtypeimagesrc ); use C4::Circulation qw( GetBranchItemRule ); -use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve GetReservesControlBranch IsAvailableForItemLevelRequest GetReserveFee ); +use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve IsAvailableForItemLevelRequest GetReserveFee ); use C4::Biblio qw( GetBiblioData GetFrameworkCode ); use C4::Output qw( output_html_with_http_headers ); use C4::Context; @@ -477,7 +477,7 @@ foreach my $biblioNum (@biblionumbers) { $item_info->{hosttitle} = Koha::Biblios->find( $item_info->{biblionumber} )->title; } - my $branch = GetReservesControlBranch( $item_info, $patron_unblessed ); + my $branch = $item->holds_control_library( $patron ); my $policy_holdallowed = CanItemBeReserved( $patron, $item, undef, { get_from_cache => 1 } )->{status} eq 'OK' && diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 2a6ed48120..dda75c6cef 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 78; +use Test::More tests => 76; use Test::MockModule; use Test::Warn; @@ -32,7 +32,7 @@ use C4::Items; use C4::Biblio qw( GetMarcFromKohaField ModBiblio ); use C4::HoldsQueue; 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::Biblios; use Koha::Caches; @@ -129,23 +129,6 @@ ok(exists($reserve->{reserve_id}), 'CheckReserves() include reserve_id in its re ($status, $reserve, $all_reserves) = CheckReserves( $item ); is($status, "Reserved", "CheckReserves Test 2"); -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