From b6b06c15a3ba1c275e9185223aaff8ae552eda9b Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Fri, 22 Jan 2021 11:11:53 +0000
Subject: [PATCH] Bug 24446: (QA follow-up) Correction to datecancelled for
 ModItemTransfer
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ModItemTransfer always replaces any existing transfers, including those
in transit.. so we needed to add a 'force' option to
Koha::Item::Transfer->cancel();

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
---
 Koha/Item.pm                           |  2 +-
 Koha/Item/Transfer.pm                  | 10 +++++-----
 t/db_dependent/Circulation/transfers.t |  6 +++---
 t/db_dependent/Koha/Item/Transfer.t    | 16 +++++++++++-----
 4 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/Koha/Item.pm b/Koha/Item.pm
index d894fb517d..0794974b1a 100644
--- a/Koha/Item.pm
+++ b/Koha/Item.pm
@@ -447,7 +447,7 @@ sub request_transfer {
     Koha::Exceptions::Item::Transfer::Found->throw( transfer => $request )
       if ( $request && !$params->{enqueue} && !$params->{replace} );
 
-    $request->cancel( $params->{reason} )
+    $request->cancel( { reason => $params->{reason}, force => 1 } )
       if ( defined($request) && $params->{replace} );
 
     my $transfer = Koha::Item::Transfer->new(
diff --git a/Koha/Item/Transfer.pm b/Koha/Item/Transfer.pm
index 9c8d7abb0b..c9e3206b53 100644
--- a/Koha/Item/Transfer.pm
+++ b/Koha/Item/Transfer.pm
@@ -118,25 +118,25 @@ sub receive {
 
 =head3 cancel
 
-  $transfer->cancel($reason);
+  $transfer->cancel({ reason => $reason, [force => 1]});
 
 Cancel the transfer by setting the datecancelled time and recording the reason.
 
 =cut
 
 sub cancel {
-    my ( $self, $reason ) = @_;
+    my ( $self, $params ) = @_;
 
     Koha::Exceptions::MissingParameter->throw(
         error => "The 'reason' parameter is mandatory" )
-      unless defined($reason);
+      unless defined($params->{reason});
 
     # Throw exception if item is in transit already
-    Koha::Exceptions::Item::Transfer::Transit->throw() if ( $self->in_transit );
+    Koha::Exceptions::Item::Transfer::Transit->throw() if ( !$params->{force} && $self->in_transit );
 
     # Update the cancelled date
     $self->set(
-        { datecancelled => dt_from_string, cancellation_reason => $reason } )
+        { datecancelled => dt_from_string, cancellation_reason => $params->{reason} } )
       ->store;
 
     return $self;
diff --git a/t/db_dependent/Circulation/transfers.t b/t/db_dependent/Circulation/transfers.t
index 7ad80f31d6..c22992d381 100755
--- a/t/db_dependent/Circulation/transfers.t
+++ b/t/db_dependent/Circulation/transfers.t
@@ -173,7 +173,7 @@ is(CreateBranchTransferLimit(undef,$branchcode_2),undef,
 my @transfers = GetTransfers($item_id1);
 cmp_deeply(
     \@transfers,
-    [ re('^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$'), $branchcode_1, $branchcode_2, re('[0-9]*'), re('^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$'), undef ],
+    [ re('^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$'), $branchcode_1, $branchcode_2, re('[0-9]*'), re('^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$'), 'Manual' ],
     "Transfers of the item1"
 );    #NOTE: Only the first transfer is returned
 @transfers = GetTransfers;
@@ -255,8 +255,8 @@ ModItemTransfer(
     $trigger
 );
 $transfer->{_result}->discard_changes;
-ok( $transfer->datearrived, 'Date arrived is set when new transfer is initiated' );
-is( $transfer->comments, "Canceled, new transfer from $branchcode_1 to $branchcode_2 created", 'Transfer comment is set as expected when new transfer is initiated' );
+ok( $transfer->datecancelled, 'Date cancelled is set when new transfer is initiated' );
+is( $transfer->cancellation_reason, "Manual", 'Cancellation reason is set correctly when new transfer is initiated' );
 
 $schema->storage->txn_rollback;
 
diff --git a/t/db_dependent/Koha/Item/Transfer.t b/t/db_dependent/Koha/Item/Transfer.t
index 1bc6d7fa9f..ac782e28fb 100755
--- a/t/db_dependent/Koha/Item/Transfer.t
+++ b/t/db_dependent/Koha/Item/Transfer.t
@@ -208,7 +208,7 @@ subtest 'in_transit tests' => sub {
 };
 
 subtest 'cancel tests' => sub {
-    plan tests => 5;
+    plan tests => 7;
 
     $schema->storage->txn_begin;
 
@@ -245,15 +245,21 @@ subtest 'cancel tests' => sub {
       'Exception thrown if a reason is not passed to cancel';
 
     # Item in transit should result in failure
-    throws_ok { $transfer->cancel($cancellation_reason) }
+    throws_ok { $transfer->cancel({ reason => $cancellation_reason }) }
     'Koha::Exceptions::Item::Transfer::Transit',
       'Exception thrown if item is in transit';
 
-    $transfer->datesent(undef)->store();
+    $transfer->cancel({ reason => $cancellation_reason, force => 1});
+    ok( $transfer->datecancelled, 'Forced cancellation, cancellation date set' );
+    is( $transfer->cancellation_reason, 'Manual', 'Forced cancellation, cancellation reason is set');
+
+    $transfer->datecancelled(undef);
+    $transfer->cancellation_reason(undef);
+    $transfer->datesent(undef);
 
     # Transit state unset
-    $transfer->discard_changes;
-    $transfer->cancel($cancellation_reason);
+    $transfer->store()->discard_changes;
+    $transfer->cancel({ reason => $cancellation_reason });
     ok( $transfer->datecancelled, 'Cancellation date set upon call to cancel' );
     is( $transfer->cancellation_reason, 'Manual', 'Cancellation reason is set');
 
-- 
2.20.1