From b978767e130179a2482f3e282cb6e888366a76b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= <joonas.kylmala@helsinki.fi>
Date: Tue, 8 Jun 2021 11:14:20 +0300
Subject: [PATCH] Bug 28520: Allow creating a transfer back automatically if a
 hold is canceled during transit

This fixes regression caused by "Bug 12362: Cancel transfer with hold
cancelation" where cancelled hold's transfer didn't show up in
intranet and opac because it create a new transfer that was not yet
put in in-transit state. The original idea of bug 12362 was to be able
to trigger transfer back home if a hold was cancelled (a regression
caused by bug 26078). However, we can do it more simply by setting the
$validTransfer variable true in the item check-in code when we are
dealing with Reserve transfers. More down in the AddReturn() code
there is also a check "and !$resfound" to make sure we only try to
trigger the transfer back home automatically if there is no hold
waiting at the current location the item arrived in.

It should be noted however that now we only display generic message
for the automatic transfer reason. Bug 12362 made the return display
as the reason "Transfer was cancelled whilst in transit". However,
since this fixes the original regressions caused by bug 26078 and
restores similar behaviour to that I think giving a more descriptive
message for example regarding a hold being cancelled can be considered
a further enhancement.

To test:
 1) Apply patch
 1) Have biblio with item in branch A
 2) Create a new hold with a pickup library to branch B
 3) Check-in the item at branch A and confirm the hold and transfer
 4) Cancel the hold
 5) Check-in the hold at branch B and notice it prompt to return it to
 branch A
---
 C4/Circulation.pm                   |  4 ++++
 t/db_dependent/Circulation/Branch.t | 10 ++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 10db0d226a..5ce6d443a8 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -2171,6 +2171,8 @@ sub AddReturn {
             if ( $transfer->tobranch eq $branch ) {
                 $transfer->receive;
                 $messages->{'TransferArrived'} = $transfer->frombranch;
+                # validTransfer=1 allows us returning the item back if the reserve is cancelled
+                $validTransfer = 1 if $transfer->reason eq 'Reserve';
             }
             else {
                 $messages->{'WrongTransfer'}     = $transfer->tobranch;
@@ -2181,6 +2183,8 @@ sub AddReturn {
             if ( $transfer->tobranch eq $branch ) {
                 $transfer->receive;
                 $messages->{'TransferArrived'} = $transfer->frombranch;
+                # validTransfer=1 allows us returning the item back if the reserve is cancelled
+                $validTransfer = 1 if $transfer->reason eq 'Reserve';
             }
             else {
                 $messages->{'WasTransfered'}   = $transfer->tobranch;
diff --git a/t/db_dependent/Circulation/Branch.t b/t/db_dependent/Circulation/Branch.t
index 1a66ad1efb..041c000449 100755
--- a/t/db_dependent/Circulation/Branch.t
+++ b/t/db_dependent/Circulation/Branch.t
@@ -25,7 +25,7 @@ use Koha::CirculationRules;
 
 use Koha::Patrons;
 
-use Test::More tests => 15;
+use Test::More tests => 16;
 use t::lib::Mocks;
 use t::lib::TestBuilder;
 
@@ -307,7 +307,13 @@ is( $messages->{NeedsTransfer}, $samplebranch1->{branchcode}, "AddReturn respect
 ModItemTransfer($item_id2, $samplebranch2->{branchcode}, $samplebranch1->{branchcode}, "ReturnToHolding");
 # Fulfill it
 ($doreturn, $messages, $iteminformation, $borrower) = AddReturn('barcode_2',$samplebranch1->{branchcode});
-is( $messages->{NeedsTransfer}, undef, "AddReturn does not generate a new transfer for return policy when resolving an existing transfer" );
+is( $messages->{NeedsTransfer}, undef, "AddReturn does not generate a new transfer for return policy when resolving an existing non-Reserve transfer" );
+
+# Generate a hold caused transfer which doesn't have a hold i.e. is the hold is cancelled
+ModItemTransfer($item_id2, $samplebranch2->{branchcode}, $samplebranch1->{branchcode}, "Reserve");
+# Fulfill it
+($doreturn, $messages, $iteminformation, $borrower) = AddReturn('barcode_2',$samplebranch1->{branchcode});
+is( $messages->{NeedsTransfer}, $samplebranch2->{branchcode}, "AddReturn generates a new transfer for hold transfer if the hold was cancelled" );
 
 # item3 should not trigger transfer - floating collection
 $query =
-- 
2.25.1