From 6ff69d55b0bb0f3b88a6d14f26d79e904e6513bc Mon Sep 17 00:00:00 2001 From: William Lavoie Date: Mon, 9 Mar 2026 08:10:24 -0400 Subject: [PATCH] Bug 33200: Preventing holds from creating transfers with IndependentBranchesTransfers enabled To test, you must have - IndependentBranches, IndependentBranchesPatronModifications and IndependentBranchesTransfers preferences set to Yes - canreservefromotherbranches must be set to Don't allow - Circulation rules that allow holds and checkouts - Two libraries (I used Midway (A) and Centerville (B)) - A record with two items, one at each library, with barcodes, check out the one from library A - Two non-superlibrarian staff user, one in each library, with at least catalogue and holds permissions, this is what I gave them - circulate (all) - catalogue - delete_borrowers - edit_borrowers - reserveforothers (all) 1. Log in to the staff interface with the non-superlibrarian staff user from library A 2. Go to the record with the two items, click 'Place hold' 3. Place a hold for a user, without changing anything (should be for 'next available item') 4. Log out and log back in with the non-superlibrarian staff user from library B 5. Check in the item from library B, which should be available --> Hold is confirmed for patron from library A, transfer is created 6. Click on 'Confirm hold and transfer' 7. Optionally, you can log out and log back in as staff from library A 8. Go to Circulation > Transfers to receive (should this even be available with IndependentBranchesTransfers turned on?) --> Item from other branch is there 9. Apply the patch 10. Create an other record with two items, one at each library, with barcodes, check out the one from library A 11. repeat step 1, 2, 3, 4, 5 --> The 'Hold found' popup is not shown, because there is no hold for this item in the user branch (library B) 12. repeat step 7,8 --> Item from other branch is not there --- C4/Reserves.pm | 30 +++++++++++++++++++++++++++++- t/db_dependent/Reserves.t | 35 +++++++++++++++++++++++++++++++---- 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index de50ce5c930..13cfc6df33d 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -90,6 +90,7 @@ use Koha::Library; use Koha::Patrons; use Koha::Plugins; use Koha::Policy::Holds; +use Data::Dumper; use List::MoreUtils qw( any ); @@ -811,7 +812,9 @@ sub CheckReserves { } # Find this item in the reserves - my @reserves = _Findgroupreserve( $item->biblionumber, $item->itemnumber, $lookahead_days, $ignore_borrowers ); + my @reserves = _Findgroupreserve( $item->biblionumber, $item->itemnumber, $lookahead_days, $ignore_borrowers); + # When IndependentBranchesTransfers is activate remove the reserve made from other branches + @reserves = _FilterHoldsForIndependentBranches( @reserves ); # $priority and $highest are used to find the most important item # in the list returned by &_Findgroupreserve. (The lower $priority, @@ -1871,6 +1874,31 @@ sub _Findgroupreserve { return @results; } +=head2 _FilterHoldsForIndependentBranches + + @reserves = &_FilterHoldsForIndependentBranches( @reserves ); + +Check transfers is allowed from system preference and remove the reserves made from other branches + +C<&_FilterHoldsForIndependentBranches> returns : +C<@results> is an array of references-to-hash whose keys are mostly +fields from the reserves table of the Koha database, plus + +=cut + +sub _FilterHoldsForIndependentBranches { + my ( @reserves) = @_; + if ( C4::Context->preference("IndependentBranchesTransfers") && scalar @reserves) + { + my @results; + foreach my $res (@reserves) { + push( @results, $res ) if ($res->{branchcode} eq C4::Context->userenv->{'branch'}); + } + return @results; + } + return @reserves; +} + =head2 _koha_notify_reserve _koha_notify_reserve( $hold->reserve_id ); diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 3b952ebb811..387e5a6e0f4 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,8 +17,7 @@ use Modern::Perl; -use Test::More tests => 69; -use Test::NoWarnings; +use Test::More tests => 73; use Test::MockModule; use Test::Warn; @@ -63,7 +62,9 @@ my $builder = t::lib::TestBuilder->new; my $frameworkcode = q//; -t::lib::Mocks::mock_preference( 'ReservesNeedReturns', 1 ); + +t::lib::Mocks::mock_preference('ReservesNeedReturns', 1); +t::lib::Mocks::mock_preference( 'IndependentBranchesTransfers', 0 ); # Somewhat arbitrary field chosen for age restriction unit tests. Must be added to db before the framework is cached $dbh->do( @@ -134,6 +135,32 @@ ok( exists( $reserve->{reserve_id} ), 'CheckReserves() include reserve_id in its ( $status, $reserve, $all_reserves ) = CheckReserves($item); is( $status, "Reserved", "CheckReserves Test 2" ); +# Set the preference 'IndependentBranchesTransfers' is set to 'yes' +# the userenv branch and the branche code are not the same holds should be filtered +t::lib::Mocks::mock_preference( 'IndependentBranchesTransfers', 1 ); +($status, $reserve, $all_reserves) = CheckReserves( $item ); +is( $status, "", 'Reserves is filtered'); + +# Set the userenv branchcode to be the same to the item branchcode. +t::lib::Mocks::mock_userenv({ branchcode => $branchcode }); +($status, $reserve, $all_reserves) = CheckReserves( $item ); +is( $status, "Reserved", 'Reserves should not be filtered'); + +t::lib::Mocks::mock_userenv({ branchcode => $branch_1 }); +t::lib::Mocks::mock_preference( 'IndependentBranchesTransfers', 0 ); +my $ReservesControlBranch = C4::Context->preference('ReservesControlBranch'); +t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'ItemHomeLibrary' ); +ok( + $item->homebranch eq Koha::Policy::Holds->holds_control_library( $item, $patron ), + "Koha::Policy::Holds->holds_control_library returns item home branch when set to ItemHomeLibrary" +); +t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' ); +ok( + $patron->branchcode eq Koha::Policy::Holds->holds_control_library( $item, $patron ), + "Koha::Policy::Holds->holds_control_library returns patron home branch when set to PatronLibrary" +); +t::lib::Mocks::mock_preference( 'ReservesControlBranch', $ReservesControlBranch ); + ### ### Regression test for bug 10272 ### @@ -1471,7 +1498,7 @@ sub count_hold_print_messages { q{ SELECT COUNT(*) FROM message_queue - WHERE letter_code = 'HOLD' + WHERE letter_code = 'HOLD' AND message_transport_type = 'print' } ); -- 2.43.0