From bfabfc68c7e421056fbc505d16238bd3fc0bb2b1 Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Wed, 19 Jun 2019 14:18:42 -0400
Subject: [PATCH] Bug 23165: Renewals for Self Checkout (SCO) module do not
 record branchcode in statistics

Koha considers SCO to be part of the OPAC, but the code related to OpacRenewalBranch is in opac-renew.pl. Because of this, the branchcode does not get set when a renewal takes place via the self check module. It would make more sense to move this logic into C4::Circulation::AddRenewal to centralize it and ensure that the statistics branch is set no matter where in the OPAC AddRenewal may be called.

Test Plan:
1) Using the SCO module, renew an item
2) View the statistics table row for the renewal, note that branch is empty
3) Apply this patch
4) Restart all the things!
5) Renew the item again
6) View the stats table row for the new renewal, the branch code should be set
   as per the system preference OpacRenewalBranch
---
 C4/Circulation.pm  | 26 ++++++++++++++++++++++++--
 opac/opac-renew.pl | 21 +--------------------
 2 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 84349d98cf..4931de58a9 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -2951,8 +2951,30 @@ sub AddRenewal {
         DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
     }
 
-    unless ( C4::Context->interface eq 'opac' ) { #if from opac we are obeying OpacRenewalBranch as calculated in opac-renew.pl
-        $branch = C4::Context->userenv ? C4::Context->userenv->{branch} : $branch;
+    # If from opac we are obeying OpacRenewalBranch as calculated in opac-renew.pl
+    if ( C4::Context->interface eq 'opac' ) {
+        my $renewalbranch = C4::Context->preference('OpacRenewalBranch');
+        if ( $renewalbranch eq 'itemhomebranch' ) {
+            my $item = Koha::Items->find($itemnumber);
+            $branch = $item->homebranch;
+        }
+        elsif ( $renewalbranch eq 'patronhomebranch' ) {
+            $branch = Koha::Patrons->find( $borrowernumber )->branchcode;
+        }
+        elsif ( $renewalbranch eq 'checkoutbranch' ) {
+            my $issue = GetOpenIssue($itemnumber); # FIXME Should not be $item->checkout?
+            $branch = $issue->{'branchcode'};
+        }
+        elsif ( $renewalbranch eq 'NULL' ) {
+            $branch = '';
+        }
+        else {
+            $branch = 'OPACRenew';
+        }
+    }
+    else {
+        $branch =
+          C4::Context->userenv ? C4::Context->userenv->{branch} : $branch;
     }
 
     # Add the renewal to stats
diff --git a/opac/opac-renew.pl b/opac/opac-renew.pl
index 85570d54ad..81c27689ef 100755
--- a/opac/opac-renew.pl
+++ b/opac/opac-renew.pl
@@ -63,26 +63,7 @@ else {
         my ( $status, $error ) =
           CanBookBeRenewed( $borrowernumber, $itemnumber );
         if ( $status == 1 && $opacrenew == 1 ) {
-            my $renewalbranch = C4::Context->preference('OpacRenewalBranch');
-            my $branchcode;
-            if ( $renewalbranch eq 'itemhomebranch' ) {
-                my $item = Koha::Items->find($itemnumber);
-                $branchcode = $item->homebranch;
-            }
-            elsif ( $renewalbranch eq 'patronhomebranch' ) {
-                $branchcode = Koha::Patrons->find( $borrowernumber )->branchcode;
-            }
-            elsif ( $renewalbranch eq 'checkoutbranch' ) {
-                my $issue = GetOpenIssue($itemnumber); # FIXME Should not be $item->checkout?
-                $branchcode = $issue->{'branchcode'};
-            }
-            elsif ( $renewalbranch eq 'NULL' ) {
-                $branchcode = '';
-            }
-            else {
-                $branchcode = 'OPACRenew';
-            }
-            AddRenewal( $borrowernumber, $itemnumber, $branchcode, undef, undef );
+            AddRenewal( $borrowernumber, $itemnumber );
             push( @renewed, $itemnumber );
         }
         else {
-- 
2.20.1 (Apple Git-117)