From e5826c6113204de1fbc2341264cdf2eb9f64572f Mon Sep 17 00:00:00 2001
From: Lari Taskula <lari.taskula@hypernova.fi>
Date: Fri, 30 Aug 2019 16:30:07 +0000
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)

Sponsored-by: National Library of Finland

Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
---
 C4/Circulation.pm | 65 ++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 40 insertions(+), 25 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 50c436ecba..96a0a06783 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -956,7 +956,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} );
 
         unless ( $can_be_returned ) {
             $issuingimpossible{RETURN_IMPOSSIBLE} = 1;
@@ -1208,7 +1208,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
 
@@ -1227,26 +1227,35 @@ Returns:
 =cut
 
 sub CanBookBeReturned {
-  my ($item, $branch) = @_;
-  my $allowreturntobranch = C4::Context->preference("AllowReturnToBranch") || 'anywhere';
-
-  # assume return is allowed to start
-  my $allowed = 1;
-  my $message;
-
-  # identify all cases where return is forbidden
-  if ($allowreturntobranch eq 'homebranch' && $branch ne $item->{'homebranch'}) {
-     $allowed = 0;
-     $message = $item->{'homebranch'};
-  } elsif ($allowreturntobranch eq 'holdingbranch' && $branch ne $item->{'holdingbranch'}) {
-     $allowed = 0;
-     $message = $item->{'holdingbranch'};
-  } elsif ($allowreturntobranch eq 'homeorholdingbranch' && $branch ne $item->{'homebranch'} && $branch ne $item->{'holdingbranch'}) {
-     $allowed = 0;
-     $message = $item->{'homebranch'}; # FIXME: choice of homebranch is arbitrary
-  }
-
-  return ($allowed, $message);
+    my ($item, $branch) = @_;
+    my $allowreturntobranch = C4::Context->preference("AllowReturnToBranch") || 'anywhere';
+
+    # assume return is allowed to start
+    my $allowed = 1;
+    my $to_branch = $branch;
+    my $message;
+
+    # identify all cases where return is forbidden
+    if ($allowreturntobranch eq 'homebranch' && $branch ne $item->homebranch) {
+        $allowed = 0;
+        $message = $to_branch = $item->homebranch;
+    } elsif ($allowreturntobranch eq 'holdingbranch' && $branch ne $item->holdingbranch) {
+        $allowed = 0;
+        $message = $to_branch = $item->holdingbranch;
+    } elsif ($allowreturntobranch eq 'homeorholdingbranch' && $branch ne $item->homebranch && $branch ne $item->holdingbranch) {
+        $allowed = 0;
+        $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);
 }
 
 =head2 CheckHighHolds
@@ -1470,7 +1479,7 @@ sub AddIssue {
             if ( $actualissue and not $switch_onsite_checkout ) {
                 # 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'} );
             }
@@ -1907,6 +1916,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<Transferlimit>
+
+A transfer limit exists between item's holding branch and the return branch.
+
 =item C<ResFound>
 
 The item was reserved. The value is a reference-to-hash whose keys are
@@ -2030,12 +2043,14 @@ 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.11.0