From 321d8f7ce677bf4feb6b3946ad652d6921ca6bea Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Fri, 8 Apr 2022 18:52:09 +0000
Subject: [PATCH] Bug 22042: [22.05.x] Block all return actions when
 BlockReturnOfWithdrawn items is set to block

Currently this syspref only bokcs the literal 'return' from a patron, i.e. the checkin

It still processes transfers, refunds lost items, updates NotForLoan status etc.

We should block all of these things

To test:
1 - Set BlockReturnOfWithdrawn to block
2 - Set an item as lost and withdrawn
3 - Check it in
4 - Item is found
5 - Apply patch
6 - Repeat 1-3
7 - Checkin is blocked, item still lost

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
---
 C4/Circulation.pm            | 15 ++++++++++-----
 t/db_dependent/Circulation.t | 15 ++++++++++++++-
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 7a7be63be9..7314ca07d1 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -2095,6 +2095,16 @@ sub AddReturn {
         }
     }
 
+    if ( $item->withdrawn ) { # book has been cancelled
+        $messages->{'withdrawn'} = 1;
+
+        # In the case where we block return of withdrawn, we should completely block the return
+        # without updating item statuses, so we exit early
+        return ( 0, $messages, $issue, ( $patron ? $patron->unblessed : {} ))
+            if C4::Context->preference("BlockReturnOfWithdrawnItems");
+    }
+
+
         # full item data, but no borrowernumber or checkout info (no issue)
     my $hbr = GetBranchItemRule($item->homebranch, $itemtype)->{'returnbranch'} || "homebranch";
         # get the proper branch to which to return the item
@@ -2164,11 +2174,6 @@ sub AddReturn {
         return ( $doreturn, $messages, $issue, $patron_unblessed);
     }
 
-    if ( $item->withdrawn ) { # book has been cancelled
-        $messages->{'withdrawn'} = 1;
-        $doreturn = 0 if C4::Context->preference("BlockReturnOfWithdrawnItems");
-    }
-
     if ( $item->itemlost and C4::Context->preference("BlockReturnOfLostItems") ) {
         $doreturn = 0;
     }
diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
index 3683d02df4..8f419ab19d 100755
--- a/t/db_dependent/Circulation.t
+++ b/t/db_dependent/Circulation.t
@@ -18,7 +18,7 @@
 use Modern::Perl;
 use utf8;
 
-use Test::More tests => 60;
+use Test::More tests => 61;
 use Test::Exception;
 use Test::MockModule;
 use Test::Deep qw( cmp_deeply );
@@ -5450,6 +5450,19 @@ subtest "GetSoonestRenewDate tests" => sub {
     );
 };
 
+subtest 'Tests for BlockReturnOfWithdrawnItems' => sub {
+
+    plan tests => 1;
+
+    t::lib::Mocks::mock_preference('BlockReturnOfWithdrawnItems', 1);
+    my $item = $builder->build_sample_item();
+    $item->withdrawn(1)->itemlost(1)->store;
+    my @return = AddReturn( $item->barcode, $item->homebranch, 0, undef );
+    is_deeply(
+        \@return,
+        [ 0, { NotIssued => $item->barcode, withdrawn => 1 }, undef, {} ], "Item returned as withdrawn, no other messages");
+};
+
 $schema->storage->txn_rollback;
 C4::Context->clear_syspref_cache();
 $branches = Koha::Libraries->search();
-- 
2.39.5