From b689dd07cc46d695909a4b99c6fb08a0ae4adc41 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Date: Tue, 14 Apr 2026 17:04:27 +0100
Subject: [PATCH] Bug 41728: (QA follow-up) Restore original execution order
 for Wrongbranch
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The refactoring changed the relative order of the Wrongbranch early
return and several status-update operations. In the original AddReturn,
the execution order was:

  1. checkout/patron lookup
  2. not-issued handling + RecordLocalUseOnReturn
  3. withdrawn check (early return)
  4. location_update_trigger
  5. UpdateNotForLoanStatusOnCheckin
  6. CanBookBeReturned / Wrongbranch check (early return)
  7. BlockReturnOfLostItems

After the refactoring, the Wrongbranch blocker returned early before
steps 2, 4 and 5, causing three regressions:

  - RecordLocalUseOnReturn was not recorded for non-issued items
    scanned at the wrong branch
  - location_update_trigger was skipped for Wrongbranch returns
  - UpdateNotForLoanStatusOnCheckin was skipped for Wrongbranch returns

Fix by splitting blocker handling into two phases:

  1. BlockedWithdrawn: earliest early return, before status updates
     (matches original withdrawn check position)
  2. Wrongbranch and BlockedLost: after RecordLocalUseOnReturn,
     location_update_trigger, and UpdateNotForLoanStatusOnCheckin
     (matches original CanBookBeReturned position)

Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
---
 C4/Circulation.pm | 55 ++++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 3e735ba30a..a321a9d276 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -2461,33 +2461,20 @@ sub AddReturn {
         $doreturn = 0;
     }
 
-    # Handle blockers
-    if ( !$availability->available ) {
-        my $blockers = $availability->blockers;
+    # Handle BlockedWithdrawn blocker first - this was the earliest early
+    # return in the original code, before any status updates ran
+    if ( !$availability->available && $availability->blockers->{BlockedWithdrawn} ) {
+        $messages->{'withdrawn'} = 1;
 
-        if ( $blockers->{BlockedWithdrawn} ) {
-            $messages->{'withdrawn'} = 1;
+        # Record local use even when blocked, if preference is on
+        if ( !$issue && C4::Context->preference("RecordLocalUseOnReturn") ) {
+            my $localuse_count = $item->localuse || 0;
+            $localuse_count++;
+            $item->localuse($localuse_count)->store;
+            $messages->{'LocalUse'} = 1;
+        }
 
-            # Record local use even when blocked, if preference is on
-            if ( !$issue && C4::Context->preference("RecordLocalUseOnReturn") ) {
-                my $localuse_count = $item->localuse || 0;
-                $localuse_count++;
-                $item->localuse($localuse_count)->store;
-                $messages->{'LocalUse'} = 1;
-            }
-
-            return ( 0, $messages, $issue, ( $patron ? $patron->unblessed : {} ) );
-        }
-        if ( $blockers->{BlockedLost} ) {
-            $doreturn = 0;
-        }
-        if ( $blockers->{Wrongbranch} ) {
-            $messages->{'Wrongbranch'} = $blockers->{Wrongbranch};
-            $doreturn = 0;
-            my $indexer = Koha::SearchEngine::Indexer->new( { index => $Koha::SearchEngine::BIBLIOS_INDEX } );
-            $indexer->index_records( $item->biblionumber, "specialUpdate", "biblioserver" );
-            return ( $doreturn, $messages, $issue, ( $patron ? $patron->unblessed : {} ) );
-        }
+        return ( 0, $messages, $issue, ( $patron ? $patron->unblessed : {} ) );
     }
 
     my $itemnumber     = $item->itemnumber;
@@ -2558,6 +2545,24 @@ sub AddReturn {
         }
     }
 
+    # Handle remaining blockers after status updates - in the original code
+    # CanBookBeReturned and BlockReturnOfLostItems ran after
+    # location_update_trigger and UpdateNotForLoanStatusOnCheckin
+    if ( !$availability->available ) {
+        my $blockers = $availability->blockers;
+
+        if ( $blockers->{Wrongbranch} ) {
+            $messages->{'Wrongbranch'} = $blockers->{Wrongbranch};
+            $doreturn = 0;
+            my $indexer = Koha::SearchEngine::Indexer->new( { index => $Koha::SearchEngine::BIBLIOS_INDEX } );
+            $indexer->index_records( $item->biblionumber, "specialUpdate", "biblioserver" );
+            return ( $doreturn, $messages, $issue, $patron_unblessed );
+        }
+        if ( $blockers->{BlockedLost} ) {
+            $doreturn = 0;
+        }
+    }
+
     # case of a return of document (deal with issues and holdingbranch)
     if ($doreturn) {
         die "The item is not issed and cannot be returned" unless $issue;    # Just in case...