From bfabfc68c7e421056fbc505d16238bd3fc0bb2b1 Mon Sep 17 00:00:00 2001 From: Kyle M Hall 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)