From f8d59a7b501ab200b8af659eec79c4c6cecc6dc3 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Date: Thu, 11 Jun 2026 17:23:11 +0100
Subject: [PATCH] Bug 42443: (QA follow-up) Prevent cancelling completed
 transfers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The delete endpoint was passing force => 1 to cancel() with no state
guards, allowing already-received and already-cancelled transfers to
be re-cancelled, corrupting datearrived/datecancelled audit fields.

Add explicit 400 responses for both cases and tests to cover them.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
---
 Koha/REST/V1/Transfers.pm         |  6 ++++++
 t/db_dependent/api/v1/transfers.t | 24 +++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/Koha/REST/V1/Transfers.pm b/Koha/REST/V1/Transfers.pm
index b9583c288a..d882543b75 100644
--- a/Koha/REST/V1/Transfers.pm
+++ b/Koha/REST/V1/Transfers.pm
@@ -60,6 +60,12 @@ sub delete {
     return $c->render_resource_not_found("Transfer")
         unless $transfer;
 
+    return $c->render( status => 400, openapi => { error => "Transfer has already been received" } )
+        if $transfer->datearrived;
+
+    return $c->render( status => 400, openapi => { error => "Transfer has already been cancelled" } )
+        if $transfer->datecancelled;
+
     return try {
         my $body = $c->req->json;
 
diff --git a/t/db_dependent/api/v1/transfers.t b/t/db_dependent/api/v1/transfers.t
index 4f480aef32..c12f9c0211 100755
--- a/t/db_dependent/api/v1/transfers.t
+++ b/t/db_dependent/api/v1/transfers.t
@@ -117,7 +117,7 @@ subtest 'list() tests' => sub {
 
 subtest 'delete() tests' => sub {
 
-    plan tests => 10;
+    plan tests => 14;
 
     $schema->storage->txn_begin;
 
@@ -165,6 +165,23 @@ subtest 'delete() tests' => sub {
     $t->delete_ok( "//$userid:$password@/api/v1/transfers/"
             . ( $transfer->id + 1000 ) => json => { cancellation_reason => 'Manual' } )->status_is(404);
 
+    # Cannot cancel an already-received transfer
+    my $arrived_item     = $builder->build_sample_item;
+    my $arrived_transfer = $builder->build_object(
+        {
+            class => 'Koha::Item::Transfers',
+            value => {
+                itemnumber    => $arrived_item->itemnumber,
+                datesent      => \'NOW()',
+                datearrived   => \'NOW()',
+                datecancelled => undef,
+            }
+        }
+    );
+    $t->delete_ok(
+        "//$userid:$password@/api/v1/transfers/" . $arrived_transfer->id => json =>
+            { cancellation_reason => 'Manual' } )->status_is(400);
+
     # Cancel an in-transit transfer with the supplied reason
     $t->delete_ok(
         "//$userid:$password@/api/v1/transfers/" . $transfer->id => json => { cancellation_reason => 'WrongTransfer' } )
@@ -174,5 +191,10 @@ subtest 'delete() tests' => sub {
     ok( defined $transfer->datecancelled, 'The transfer has been cancelled' );
     is( $transfer->cancellation_reason, 'WrongTransfer', 'The supplied cancellation reason is recorded' );
 
+    # Cannot cancel an already-cancelled transfer
+    $t->delete_ok(
+        "//$userid:$password@/api/v1/transfers/" . $transfer->id => json => { cancellation_reason => 'Manual' } )
+        ->status_is(400);
+
     $schema->storage->txn_rollback;
 };