Attachment 94365 Details for Bug 23165 – Bug 23165: Renewals for Self Checkout (SCO) module do not record branchcode in statistics

[patch] Bug 23165: Renewals for Self Checkout (SCO) module do not record branchcode in statistics

Bug-23165-Renewals-for-Self-Checkout-SCO-module-do.patch (text/plain), 4.00 KB, created by Magnus Enger on 2019-10-17 09:01:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Magnus Enger

Created: 2019-10-17 09:01:22 UTC

Size: 4.00 KB

patch

obsolete

>From c5db38ddaee0560f82bf203960b0376fafae9613 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
>
>Signed-off-by: Magnus Enger <magnus@libriotech.no>
>---
> 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 014c018628..fb6292dbf3 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.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 23165: 90815 | 90997 | 91016 | 91227 | 94364 | 94365