From 7dcebccf2f6e0bcb35666973b4b030c0ccd8df04 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Mon, 16 Dec 2024 10:48:17 +0100 Subject: [PATCH] Bug 7376: Transfer limits should be checked at check-in Test case: * UseBranchTransferLimits must be set * define your branch transfer limit. Refuse transfers from libraryA to libraryB * checkout a book owned by libraryB, from libraryB, with a librarian located at libraryB * move the librarian to libraryA ("Set Library" link top/right) * check-in the book => it's possible whatever your setup After the patch, the behaviour respect the branch transfer limit parameter: you can check-in if you accept transfers, you can't if you refuse them. (Note: IndependantBranches must be OFF, otherwise it's not possible to do the checkin whatever the branch transfer limits) TEST PLAN: Note: older signoffs did not validate this test plan Considering you use a ktd-like database. Log in at centerville Allow transfers between centerville, Midway and Franklin, don't allow transfers between Fairfield, Liberty and all other libraries. In addition, allow the transfer between Liberty and Midway Transfer as much books as you can from Centerville to Midway. Log in as Midway, check out as many books from Centerville as you can to Henry Acevedo. If you don't have enough books during the test plan, feel free to check out books again. During the whole test, without further precision, items checkout out in such conditions should be checked in. You will also need 6 checked out floating books at midway Note: '-> ok' means the checkin is allowed '-> not ok' means the checkin is not allowed Apply patch I - Test AllowReturnToBranch During this part of the test plan, you want to have all following configuration : 'CircControlReturnsBranch' -> 'The library the item is checked in at' In CIRC rules: 'Default holds policy and return policy' -> 'Item returns home' (make sure you have no other holds policy on any library) 'AllowReturnToBranch' -> as follows 1 - Set to "Either at the library the item is from or at the library the item was checked out from" X a - Check in to Centerville -> ok X b - Check in to Midway -> ok X c - Check in to Franklin -> not ok X d - Check in to Fairfield -> not ok 2 - Set to "Only at the library the item was checked out from" X a - Check in to Centerville -> not ok X b - Check in to Midway -> ok X c - Check in to Franklin -> not ok X d - Check in to Fairfield -> not ok 3 - Set to "Only at the library the item is from" X a - Check in to Centerville -> ok X b - Check in to Midway -> not ok X c - Check in to Franklin -> not ok X d - Check in to Fairfield -> not ok 4 - Set to "Any library" X a - Check in to Centerville -> ok X b - Check in to Midway -> ok X c - Check in to Franklin -> ok X d - Check in to Fairfield -> not ok II - Transfer and check out 6 floating items at midway. 1 - X a - Check in to Centerville -> ok X b - Check in to Midway -> ok X c - Check in to Franklin -> ok X d - Check in to Fairfield -> ok 2 - Set 'AllowReturnToBranch' to 'home library' X a - Check in to Centerville -> ok X b - Check in to Midway -> not ok III - Pathological & asymetrical cases. During this part of the test plan, you want to have all following configuration : 'CircControlReturnsBranch' -> 'The library the item is checked in at' 'Default holds policy and return policy' -> 'Item returns home' (make sure you have no other holds policy on any library) 'AllowReturnToBranch' -> 'Any library' FOR THIS PART OF THE TESTS ONLY (until IV): Don't allow tranfers from midway to centerville, but allow from centerville to midway X 1 - Check in a book at midway -> not ok Set 'Default holds policy and return policy' -> 'Item returns to issuing library' X 2 - Check in a book at centerville -> ok FOR THIS PART OF THE TESTS ONLY (until IV): Don't allow tranfers from centerville to midway X 3 - Check in a book at centerville -> not ok X 4 - Set 'AllowReturnToBranch' to "Only at the library the item was checked out from" X a - check in to Centerville -> not ok X b - check in to Midway -> not ok IV - Test 'CircControlReturnsBranch' (remove the precedent transfer restriction) During this part of the test plan, you want to have all following configuration : 'Default holds policy and return policy' -> 'Item returns home' (unchanged) 'Default checkout, hold and return policy for Midway' -> 'Item returns to issuing library' 'Default checkout, hold and return policy for Franklin' -> 'Item floats' 'Default checkout, hold and return policy for Fairfield' -> 'Item returns to issuing library' 'Default checkout, hold and return policy for Liberty' -> 'Item returns home' 'AllowReturnToBranch' -> "Any library" (unchanged) 'CircControlReturnsBranch' -> as follows 1 - 'The library the item is checked in at' X a - Check in to Centerville -> ok X b - Check in to Midway -> ok X c - Check in to Franklin -> ok X d - Check in to Fairfield -> not ok X e - Check in to Liberty -> not ok 2 - 'The library the current item is held by X a - Check in to Centerville -> ok X b - Check in to Midway -> ok X c - Check in to Franklin -> ok X d - Check in to Fairfield -> not ok X e - Check in to Liberty -> ok 3 - 'the library the item is owned by' X a - Check in to Centerville -> ok X b - Check in to Midway -> ok X c - Check in to Franklin -> ok X d - Check in to Fairfield -> not ok X e - Check in to Liberty -> not ok Sponsored-by: National Library of Finland Signed-off-by: Arthur Suzuki Signed-off-by: Julian Maurice Signed-off-by: Baptiste Wojtkowski --- C4/Circulation.pm | 40 +++++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 31794b7c02..3572117a8e 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1006,7 +1006,7 @@ sub CanBookBeIssued { my $patron = Koha::Patrons->find( $issue->borrowernumber ); - my ( $can_be_returned, $message ) = CanBookBeReturned( $item_unblessed, C4::Context->userenv->{branch} ); + my ( $can_be_returned, $message ) = CanBookBeReturned( $item_object, C4::Context->userenv->{branch} ); if ( !$can_be_returned ) { $issuingimpossible{RETURN_IMPOSSIBLE} = 1; @@ -1364,7 +1364,7 @@ Check whether the item can be returned to the provided branch =over 4 -=item C<$item> is a hash of item information as returned Koha::Items->find->unblessed (Temporary, should be a Koha::Item instead) +=item C<$item> is a Koha::Item object =item C<$branch> is the branchcode where the return is taking place @@ -1387,22 +1387,31 @@ sub CanBookBeReturned { my $allowreturntobranch = C4::Context->preference("AllowReturnToBranch") || 'anywhere'; # assume return is allowed to start - my $allowed = 1; + my $allowed = 1; + my $to_branch = $branch; my $message; # identify all cases where return is forbidden - if ( $allowreturntobranch eq 'homebranch' && $branch ne $item->{'homebranch'} ) { + if ( $allowreturntobranch eq 'homebranch' && $branch ne $item->homebranch ) { $allowed = 0; - $message = $item->{'homebranch'}; - } elsif ( $allowreturntobranch eq 'holdingbranch' && $branch ne $item->{'holdingbranch'} ) { + $message = $to_branch = $item->homebranch; + } elsif ( $allowreturntobranch eq 'holdingbranch' && $branch ne $item->holdingbranch ) { $allowed = 0; - $message = $item->{'holdingbranch'}; + $message = $to_branch = $item->holdingbranch; } elsif ( $allowreturntobranch eq 'homeorholdingbranch' - && $branch ne $item->{'homebranch'} - && $branch ne $item->{'holdingbranch'} ) + && $branch ne $item->homebranch + && $branch ne $item->holdingbranch ) { $allowed = 0; - $message = $item->{'homebranch'}; # FIXME: choice of homebranch is arbitrary + $message = $to_branch = $item->homebranch; # FIXME: choice of homebranch is arbitrary + } + + # Make sure there are no branch transfer limits between item's current + # branch (holdinbranch) and the return branch + my $to_library = Koha::Libraries->find($to_branch); + if ( !$item->can_be_transferred( { to => $to_library } ) ) { + $allowed = 0; + $message = $item->homebranch; } return ( $allowed, $message ); @@ -1671,7 +1680,7 @@ sub AddIssue { # This book is currently on loan, but not to the person # who wants to borrow it now. mark it returned before issuing to the new borrower - my ( $allowed, $message ) = CanBookBeReturned( $item_unblessed, C4::Context->userenv->{branch} ); + my ( $allowed, $message ) = CanBookBeReturned( $item_object, C4::Context->userenv->{branch} ); return unless $allowed; AddReturn( $item_object->barcode, C4::Context->userenv->{'branch'} ); @@ -2130,6 +2139,10 @@ This book has was returned to the wrong branch. The value is a hashref so that C<$messages->{Wrongbranch}->{Wrongbranch}> and C<$messages->{Wrongbranch}->{Rightbranch}> contain the branchcode of the incorrect and correct return library, respectively. +=item C + +A transfer limit exists between item's holding branch and the return branch. + =item C The item was reserved. The value is a reference-to-hash whose keys are @@ -2284,12 +2297,13 @@ sub AddReturn { } # check if the return is allowed at this branch - my ( $returnallowed, $message ) = CanBookBeReturned( $item->unblessed, $branch ); + my ( $returnallowed, $message ) = CanBookBeReturned( $item, $branch ); + unless ($returnallowed) { $messages->{'Wrongbranch'} = { Wrongbranch => $branch, Rightbranch => $message - }; + } if $message; $doreturn = 0; my $indexer = Koha::SearchEngine::Indexer->new( { index => $Koha::SearchEngine::BIBLIOS_INDEX } ); $indexer->index_records( $item->biblionumber, "specialUpdate", "biblioserver" ); -- 2.39.5