From 630468bf08d9ebb62c9e07fb7de85824b64aef42 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Thu, 6 May 2021 16:31:39 +0100
Subject: [PATCH] Bug 27064: (QA follow-up) Pass 'replace' through to
 Koha::Item::Transfer->cancel
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

The `replace` option found in Koha::Item->request_transfer should be
passed through to Koha::Item::Transfer->cancel and prevent any reverse
transfers from being queued in such cases.

This prevents modItemTransfer from adding superflous reverse transfers
whenever it is used.

Test plan
1/ Run t/db_dependent/Koha/Item.t to prove the new tests pass

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 Koha/Item.pm               |  2 +-
 Koha/Item/Transfer.pm      |  3 ++-
 t/db_dependent/Koha/Item.t | 27 ++++++++++++++++++++++++---
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/Koha/Item.pm b/Koha/Item.pm
index 2bad020287..730a73bc8d 100644
--- a/Koha/Item.pm
+++ b/Koha/Item.pm
@@ -467,7 +467,7 @@ sub request_transfer {
         }
     )->store();
 
-    $request->cancel( { reason => $params->{reason}, force => 1 } )
+    $request->cancel( { reason => $params->{reason}, force => 1, replace => 1 } )
       if ( defined($request) && $params->{replace} );
 
     return $transfer;
diff --git a/Koha/Item/Transfer.pm b/Koha/Item/Transfer.pm
index 4112b3fab9..21d8a2938c 100644
--- a/Koha/Item/Transfer.pm
+++ b/Koha/Item/Transfer.pm
@@ -159,9 +159,10 @@ sub cancel {
       ->store;
 
     # Set up return transfer if transfer was force cancelled whilst in transit
+    # and we were not notified that the transfer is being replaced.
     # NOTE: We don't catch here, as we're happy to fail if there are already
     # other transfers in the queue.
-    if ($in_transit) {
+    if ($in_transit && !$params->{replace}) {
         try {
             $self->item->request_transfer(
                 { to => $self->from_library, reason => 'TransferCancellation' } );
diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t
index a894048b1e..161a13b575 100755
--- a/t/db_dependent/Koha/Item.t
+++ b/t/db_dependent/Koha/Item.t
@@ -459,7 +459,7 @@ subtest 'pickup_locations' => sub {
 };
 
 subtest 'request_transfer' => sub {
-    plan tests => 7;
+    plan tests => 13;
     $schema->storage->txn_begin;
 
     my $library1 = $builder->build_object( { class => 'Koha::Libraries' } );
@@ -485,6 +485,7 @@ subtest 'request_transfer' => sub {
     is( ref($transfer), 'Koha::Item::Transfer',
         'Koha::Item->request_transfer should return a Koha::Item::Transfer object'
     );
+    my $original_transfer = $transfer->get_from_storage;
 
     # Transfer already in progress
     throws_ok { $item->request_transfer( { to => $library2, reason => 'Manual' } ) }
@@ -496,7 +497,27 @@ subtest 'request_transfer' => sub {
         'Koha::Item::Transfer',
         'The exception contains the found Koha::Item::Transfer' );
 
-    $transfer->datearrived(dt_from_string)->store();
+    # Queue transfer
+    my $queued_transfer = $item->request_transfer(
+        { to => $library2, reason => 'Manual', enqueue => 1 } );
+    is( ref($queued_transfer), 'Koha::Item::Transfer',
+        'Koha::Item->request_transfer allowed when enqueue is set' );
+    my $transfers = $item->get_transfers;
+    is($transfers->count, 2, "There are now 2 live transfers in the queue");
+    $transfer = $transfer->get_from_storage;
+    is_deeply($transfer->unblessed, $original_transfer->unblessed, "Original transfer unchanged");
+    $queued_transfer->datearrived(dt_from_string)->store();
+
+    # Replace transfer
+    my $replaced_transfer = $item->request_transfer(
+        { to => $library2, reason => 'Manual', replace => 1 } );
+    is( ref($replaced_transfer), 'Koha::Item::Transfer',
+        'Koha::Item->request_transfer allowed when replace is set' );
+    $original_transfer->discard_changes;
+    ok($original_transfer->datecancelled, "Original transfer cancelled");
+    $transfers = $item->get_transfers;
+    is($transfers->count, 1, "There is only 1 live transfer in the queue");
+    $replaced_transfer->datearrived(dt_from_string)->store();
 
     # BranchTransferLimits
     t::lib::Mocks::mock_preference('UseBranchTransferLimits', 1);
@@ -513,7 +534,7 @@ subtest 'request_transfer' => sub {
 
     my $forced_transfer = $item->request_transfer( { to => $library1, reason => 'Manual', ignore_limits => 1 } );
     is( ref($forced_transfer), 'Koha::Item::Transfer',
-        'Koha::Item->request_transfer allowed when forced'
+        'Koha::Item->request_transfer allowed when ignore_limits is set'
     );
 
     $schema->storage->txn_rollback;
-- 
2.39.5