diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 811228d948..5b6b2d2f9b 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2402,7 +2402,43 @@ sub AddReturn { } # Check availability - my $availability = $item->checkin_availability( { branch => $branch } ); + my $hbr = Koha::CirculationRules->get_return_branch_policy($item); + + # check if returnbranch and homebranch belong to the same float group + my $validate_float = + Koha::Libraries->find( $item->homebranch )->validate_float_sibling( { branchcode => $branch } ); + + # get the proper branch to which to return the item + my $returnbranch; + if ( $hbr eq 'noreturn' ) { + $returnbranch = $branch; + } elsif ( $hbr eq 'returnbylibrarygroup' ) { + + # if library isn't in same the float group, transfer item to homebranch + $hbr = 'homebranch'; + $returnbranch = $validate_float ? $branch : $item->$hbr; + } else { + $returnbranch = $item->$hbr; + } + + # if $hbr was "noreturn" or any other non-item table value, then it should 'float' (i.e. stay at this branch) + my $transfer_trigger = $hbr eq 'homebranch' ? 'ReturnToHome' : $hbr eq 'holdingbranch' ? 'ReturnToHolding' : undef; + + # check library float limits if enabled if the item isn't being transferred away + if ( ( $returnbranch eq $branch ) && C4::Context->preference('UseLibraryFloatLimits') ) { + my $effective_itemtype = $item->effective_itemtype; + my $limit = Koha::Library::FloatLimits->find( { itemtype => $effective_itemtype, branchcode => $branch } ); + if ($limit) { + my $transfer_library = + Koha::Library::FloatLimits->lowest_ratio_library( $item, $branch, $item->holdingbranch ); + if ( $transfer_library && $transfer_library->branchcode ne $branch ) { + $returnbranch = $transfer_library->branchcode; + $transfer_trigger = 'LibraryFloatLimit'; + } + } + } + + my $availability = $item->checkin_availability( { library => $branch, to_library => $returnbranch } ); # Extract context objects my $issue = $availability->context->{checkout}; @@ -2476,62 +2512,9 @@ sub AddReturn { $messages->{'withdrawn'} = 1; } - # full item data, but no borrowernumber or checkout info (no issue) - my $hbr = Koha::CirculationRules->get_return_branch_policy($item); - - # check if returnbranch and homebranch belong to the same float group - my $validate_float = - Koha::Libraries->find( $item->homebranch )->validate_float_sibling( { branchcode => $branch } ); - - # get the proper branch to which to return the item - my $returnbranch; - if ( $hbr eq 'noreturn' ) { - $returnbranch = $branch; - } elsif ( $hbr eq 'returnbylibrarygroup' ) { - - # if library isn't in same the float group, transfer item to homebranch - $hbr = 'homebranch'; - $returnbranch = $validate_float ? $branch : $item->$hbr; - } else { - $returnbranch = $item->$hbr; - } - - # if $hbr was "noreturn" or any other non-item table value, then it should 'float' (i.e. stay at this branch) - my $transfer_trigger = $hbr eq 'homebranch' ? 'ReturnToHome' : $hbr eq 'holdingbranch' ? 'ReturnToHolding' : undef; - - # check library float limits if enabled if the item isn't being transferred away - if ( ( $returnbranch eq $branch ) && C4::Context->preference('UseLibraryFloatLimits') ) { - my $effective_itemtype = $item->effective_itemtype; - my $limit = Koha::Library::FloatLimits->find( { itemtype => $effective_itemtype, branchcode => $branch } ); - if ($limit) { - my $transfer_library = - Koha::Library::FloatLimits->lowest_ratio_library( $item, $branch, $item->holdingbranch ); - if ( $transfer_library && $transfer_library->branchcode ne $branch ) { - $returnbranch = $transfer_library->branchcode; - $transfer_trigger = 'LibraryFloatLimit'; - } - } - } - my $borrowernumber = $patron ? $patron->borrowernumber : undef; # we don't know if we had a borrower or not my $patron_unblessed = $patron ? $patron->unblessed : {}; - # Check branch transfer limits (bug 7376) - if ( $doreturn && defined($returnbranch) ) { - my $from_library = Koha::Libraries->find($branch); - my $to_library = Koha::Libraries->find($returnbranch); - if ( !$item->can_be_transferred( { from => $from_library, to => $to_library } ) ) { - $messages->{'Wrongbranch'} = { - Wrongbranch => $branch, - Rightbranch => $returnbranch - }; - $doreturn = 0; - my $indexer = Koha::SearchEngine::Indexer->new( { index => $Koha::SearchEngine::BIBLIOS_INDEX } ); - $indexer->index_records( $item->biblionumber, "specialUpdate", "biblioserver" ); - return ( $doreturn, $messages, $issue, $patron_unblessed ); - } - } - # Update item location my $loc_messages = $item->location_update_trigger('checkin'); foreach my $loc_msg_key ( keys %$loc_messages ) { diff --git a/Koha/Item.pm b/Koha/Item.pm index 0ee59425c3..534e085f7e 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -809,7 +809,8 @@ sub check_booking { my $availability = $item->checkin_availability( { - branch => $branchcode, + library => $branchcode, + to_library => $destination_branchcode, } ); @@ -817,6 +818,11 @@ Returns check-in availability information for this item. This is a convenience method that calls Koha::Item::Checkin::Availability->check(). +Parameters: + library => branchcode where the return takes place (required) + to_library => branchcode where the item should be sent after return + (optional, for transfer limit validation) + Returns a Koha::Availability::Result object with methods: available() # Boolean: true if check-in allowed needs_confirmation()# Boolean: true if confirmation required diff --git a/Koha/Item/Checkin/Availability.pm b/Koha/Item/Checkin/Availability.pm index dde017af96..19315b32a6 100644 --- a/Koha/Item/Checkin/Availability.pm +++ b/Koha/Item/Checkin/Availability.pm @@ -21,6 +21,7 @@ use Modern::Perl; use C4::Context; use Koha::Availability::Result; +use Koha::Libraries; =head1 NAME @@ -30,12 +31,18 @@ Koha::Item::Checkin::Availability - Check-in availability validation for items my $availability = Koha::Item::Checkin::Availability->check( $item, { - branch => $branch, + library => $branchcode, + to_library => $destination_branchcode, } ); # Or via instance method - my $availability = $item->checkin_availability( { branch => $branch } ); + my $availability = $item->checkin_availability( + { + library => $branchcode, + to_library => $destination_branchcode, + } + ); =head1 DESCRIPTION @@ -55,7 +62,8 @@ The result categorizes conditions as: my $availability = Koha::Item::Checkin::Availability->check( $item, { - branch => $branch, + library => $branchcode, + to_library => $destination_branchcode, } ); @@ -64,7 +72,9 @@ Validates check-in availability for an item. Parameters: $item - Koha::Item object (required) $params - hashref with: - branch => $branchcode (required) + library => branchcode where the return takes place (required) + to_library => branchcode where the item should be sent after return + (optional, for transfer limit validation) Returns a Koha::Availability::Result object with: blockers => {} # Conditions that prevent check-in @@ -80,7 +90,8 @@ Returns a Koha::Availability::Result object with: sub check { my ( $class, $item, $params ) = @_; - my $branch = $params->{branch}; + my $library = $params->{library}; + my $to_library = $params->{to_library}; my $result = Koha::Availability::Result->new(); @@ -96,18 +107,33 @@ sub check { return $result; } - # Check branch check-in policy - my ( $returnallowed, $message ) = _can_book_be_returned( $item, $branch ); + # Check AllowReturnToBranch policy + my ( $returnallowed, $message ) = _check_return_policy( $item, $library ); unless ($returnallowed) { $result->add_blocker( Wrongbranch => { - Wrongbranch => $branch, + Wrongbranch => $library, Rightbranch => $message } ); return $result; } + # Check branch transfer limits (bug 7376) + if ( defined $to_library ) { + my $from = Koha::Libraries->find($library); + my $to = Koha::Libraries->find($to_library); + if ( !$item->can_be_transferred( { from => $from, to => $to } ) ) { + $result->add_blocker( + Wrongbranch => { + Wrongbranch => $library, + Rightbranch => $to_library + } + ); + return $result; + } + } + # Check if item is currently checked out my $checkout = $item->checkout; if ($checkout) { @@ -129,33 +155,31 @@ sub check { =head2 Internal methods -=head3 _can_book_be_returned +=head3 _check_return_policy - my ($allowed, $message) = _can_book_be_returned($item, $branch); + my ($allowed, $message) = _check_return_policy($item, $library); -Internal method to check if an item can be checked in to a specific branch -based on the AllowReturnToBranch system preference. - -This is extracted from C4::Circulation::CanBookBeReturned. +Check if an item can be checked in at the given library based on the +AllowReturnToBranch system preference. =cut -sub _can_book_be_returned { - my ( $item, $branch ) = @_; +sub _check_return_policy { + my ( $item, $library ) = @_; my $allowreturntobranch = C4::Context->preference("AllowReturnToBranch") || 'anywhere'; my $allowed = 1; my $message; - if ( $allowreturntobranch eq 'homebranch' && $branch ne $item->homebranch ) { + if ( $allowreturntobranch eq 'homebranch' && $library ne $item->homebranch ) { $allowed = 0; $message = $item->homebranch; - } elsif ( $allowreturntobranch eq 'holdingbranch' && $branch ne $item->holdingbranch ) { + } elsif ( $allowreturntobranch eq 'holdingbranch' && $library ne $item->holdingbranch ) { $allowed = 0; $message = $item->holdingbranch; } elsif ( $allowreturntobranch eq 'homeorholdingbranch' - && $branch ne $item->homebranch - && $branch ne $item->holdingbranch ) + && $library ne $item->homebranch + && $library ne $item->holdingbranch ) { $allowed = 0; $message = $item->homebranch; diff --git a/t/db_dependent/Koha/Item/Checkin/Availability.t b/t/db_dependent/Koha/Item/Checkin/Availability.t index e5b6b66943..99ee9b7c33 100755 --- a/t/db_dependent/Koha/Item/Checkin/Availability.t +++ b/t/db_dependent/Koha/Item/Checkin/Availability.t @@ -40,7 +40,7 @@ subtest 'check() - item exists' => sub { my $library = $builder->build_object( { class => 'Koha::Libraries' } ); my $item = $builder->build_object( { class => 'Koha::Items' } ); - my $result = $item->checkin_availability( { branch => $library->branchcode } ); + my $result = $item->checkin_availability( { library => $library->branchcode } ); isa_ok( $result, 'Koha::Availability::Result', 'Returns Result object' ); ok( $result->available, 'Item is available for check-in' ); @@ -64,7 +64,7 @@ subtest 'check() - BlockedWithdrawn blocker' => sub { } ); - my $result = $item->checkin_availability( { branch => $library->branchcode } ); + my $result = $item->checkin_availability( { library => $library->branchcode } ); is( $result->blockers->{BlockedWithdrawn}, 1, 'BlockedWithdrawn blocker set' ); is( keys %{ $result->confirmations }, 0, 'no confirmations when blocked' ); @@ -90,7 +90,7 @@ subtest 'check() - BlockedLost blocker' => sub { my $result = $item->checkin_availability( { - branch => $library->branchcode, + library => $library->branchcode, } ); @@ -125,7 +125,7 @@ subtest 'check() - Wrongbranch blocker' => sub { t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homebranch' ); my $result = $item->checkin_availability( { - branch => $wrongbranch->branchcode, + library => $wrongbranch->branchcode, } ); @@ -137,7 +137,7 @@ subtest 'check() - Wrongbranch blocker' => sub { t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'holdingbranch' ); $result = $item->checkin_availability( { - branch => $wrongbranch->branchcode, + library => $wrongbranch->branchcode, } ); @@ -154,7 +154,7 @@ subtest 'check() - Wrongbranch blocker' => sub { t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homeorholdingbranch' ); $result = $item->checkin_availability( { - branch => $wrongbranch->branchcode, + library => $wrongbranch->branchcode, } ); @@ -167,7 +167,7 @@ subtest 'check() - Wrongbranch blocker' => sub { t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); $result = $item->checkin_availability( { - branch => $wrongbranch->branchcode, + library => $wrongbranch->branchcode, } ); @@ -188,7 +188,7 @@ subtest 'check() - NotIssued confirmation' => sub { my $result = $item->checkin_availability( { - branch => $library->branchcode, + library => $library->branchcode, } ); @@ -221,7 +221,7 @@ subtest 'check() - checked out item (no confirmations)' => sub { my $result = $item->checkin_availability( { - branch => $library->branchcode, + library => $library->branchcode, } );