From d26a10c1fedf55a7e85387e6555601e41db0e043 Mon Sep 17 00:00:00 2001 From: Andrew Isherwood Date: Tue, 11 Jun 2019 14:17:17 +0100 Subject: [PATCH] Bug 23051: Renew items when fines paid off When the RenewAccruingItemWhenPaid syspref is enabled and all the fines on an item that is accruing fines are paid, we automatically renew that item to prevent it from starting to accrue fines again. This patch adds an additional argument to C4::Circulation::AddRenewal which allows us to skip the calculation of fines upon renewal, which we don't want to do if the fines on that item have just been paid. Existing calls to AddRenewal have not been amended because there seems to be a convention of only passing undef when adding arguments that require their positioning to be maintained. Since the new argument is the last one, this is not the case with any existing call. Sponsored-by: Loughborough University Signed-off-by: Lucy Harrison Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize --- C4/Circulation.pm | 8 +++++- Koha/Account.pm | 61 +++++++++++++++++++++++++++++++++++++++++++- Koha/Account/Line.pm | 49 +++++++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 2 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 860d965cf1..0fb39e80b4 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2909,6 +2909,11 @@ C<$datedue> can be a DateTime object used to set the due date. C<$lastreneweddate> is an optional ISO-formatted date used to set issues.lastreneweddate. If this parameter is not supplied, lastreneweddate is set to the current date. +C<$skipfinecalc> is an optional boolean. There may be circumstances where, even if the +CalculateFinesOnReturn syspref is enabled, we don't want to calculate fines upon renew, +for example, when we're renewing as a result of a fine being paid (see RenewAccruingItemWhenPaid +syspref) + If C<$datedue> is the empty string, C<&AddRenewal> will calculate the due date automatically from the book's item type. @@ -2920,6 +2925,7 @@ sub AddRenewal { my $branch = shift; my $datedue = shift; my $lastreneweddate = shift || DateTime->now(time_zone => C4::Context->tz); + my $skipfinecalc = shift; my $item_object = Koha::Items->find($itemnumber) or return; my $biblio = $item_object->biblio; @@ -2945,7 +2951,7 @@ sub AddRenewal { my $schema = Koha::Database->schema; $schema->txn_do(sub{ - if ( C4::Context->preference('CalculateFinesOnReturn') ) { + if ( !skipfinecalc && C4::Context->preference('CalculateFinesOnReturn') ) { _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed } ); } _FixOverduesOnReturn( $borrowernumber, $itemnumber, undef, 'RENEWED' ); diff --git a/Koha/Account.pm b/Koha/Account.pm index f1bab9ff59..a4f73bbb8d 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -24,10 +24,11 @@ use Data::Dumper; use List::MoreUtils qw( uniq ); use Try::Tiny; -use C4::Circulation qw( ReturnLostItem ); +use C4::Circulation qw( ReturnLostItem CanBookBeRenewed AddRenewal ); use C4::Letters; use C4::Log qw( logaction ); use C4::Stats qw( UpdateStats ); +use C4::Overdues qw(GetFine); use Koha::Patrons; use Koha::Account::Lines; @@ -96,6 +97,10 @@ sub pay { && !defined($cash_register) ); my @fines_paid; # List of account lines paid on with this payment + # Item numbers that have had a fine paid where the line has a accounttype + # of OVERDUE and a status of UNRETURNED. We might want to try and renew + # these items. + my $overdue_unreturned = {}; my $balance_remaining = $amount; # Set it now so we can adjust the amount if necessary $balance_remaining ||= 0; @@ -114,6 +119,18 @@ sub pay { $fine->amountoutstanding($new_amountoutstanding)->store(); $balance_remaining = $balance_remaining - $amount_to_pay; + # If we need to make a note of the item associated with this line, + # in order that we can potentially renew it, do so. + if ( + $new_amountoutstanding == 0 && + $fine->accounttype && + $fine->accounttype eq 'OVERDUE' && + $fine->status && + $fine->status eq 'UNRETURNED' + ) { + $overdue_unreturned->{$fine->itemnumber} = $fine; + } + # Same logic exists in Koha::Account::Line::apply if ( $new_amountoutstanding == 0 && $fine->itemnumber @@ -174,6 +191,18 @@ sub pay { $fine->amountoutstanding( $old_amountoutstanding - $amount_to_pay ); $fine->store(); + # If we need to make a note of the item associated with this line, + # in order that we can potentially renew it, do so. + if ( + $old_amountoutstanding - $amount_to_pay == 0 && + $fine->accounttype && + $fine->accounttype eq 'OVERDUE' && + $fine->status && + $fine->status eq 'UNRETURNED' + ) { + $overdue_unreturned->{$fine->itemnumber} = $fine; + } + if ( $fine->amountoutstanding == 0 && $fine->itemnumber && $fine->debit_type_code @@ -254,6 +283,36 @@ sub pay { } ); + # If we have overdue unreturned items that have had payments made + # against them, check whether the balance on those items is now zero + # and, if the syspref is set, renew them + # Same logic exists in Koha::Account::Line::apply + if ( + C4::Context->preference('RenewAccruingItemWhenPaid') && + keys %{$overdue_unreturned} + ) { + foreach my $itemnumber (keys %{$overdue_unreturned}) { + # Only do something if this item has no fines left on it + my $fine = C4::Overdues::GetFine( $itemnumber, $self->{patron_id} ); + next if $fine && $fine > 0; + + my ( $renew_ok, $error ) = + C4::Circulation::CanBookBeRenewed( + $self->{patron_id}, $itemnumber + ); + if ( $renew_ok ) { + C4::Circulation::AddRenewal( + $self->{patron_id}, + $itemnumber, + $library_id, + undef, + undef, + 1 + ); + } + } + } + if ( C4::Context->preference("FinesLog") ) { logaction( "FINES", 'CREATE', diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm index 546fc8457a..51eed7ae23 100644 --- a/Koha/Account/Line.pm +++ b/Koha/Account/Line.pm @@ -21,6 +21,7 @@ use Carp; use Data::Dumper; use C4::Log qw(logaction); +use C4::Overdues qw(GetFine); use Koha::Account::CreditType; use Koha::Account::DebitType; @@ -452,6 +453,11 @@ sub apply { my $schema = Koha::Database->new->schema; + # Item numbers that have had a fine paid where the line has a accounttype + # of OVERDUE and a status of UNRETURNED. We might want to try and renew + # these items. + my $overdue_unreturned = {}; + $schema->txn_do( sub { for my $debit ( @{$debits} ) { @@ -484,6 +490,19 @@ sub apply { $self->amountoutstanding( $available_credit * -1 )->store; $debit->amountoutstanding( $owed - $amount_to_cancel )->store; + # If we need to make a note of the item associated with this line, + # in order that we can potentially renew it, do so. + # Same logic existing in Koha::Account::pay + if ( + $debit->amountoutstanding == 0 && + $debit->accounttype && + $debit->accounttype eq 'OVERDUE' && + $debit->status && + $debit->status eq 'UNRETURNED' + ) { + $overdue_unreturned->{$debit->itemnumber} = $debit; + } + # Same logic exists in Koha::Account::pay if ( $debit->amountoutstanding == 0 && $debit->itemnumber @@ -496,6 +515,36 @@ sub apply { } }); + # If we have overdue unreturned items that have had payments made + # against them, check whether the balance on those items is now zero + # and, if the syspref is set, renew them + # Same logic existing in Koha::Account::pay + if ( + C4::Context->preference('RenewAccruingItemWhenPaid') && + keys %{$overdue_unreturned} + ) { + foreach my $itemnumber (keys %{$overdue_unreturned}) { + # Only do something if this item has no fines left on it + my $fine = C4::Overdues::GetFine( $itemnumber, $self->borrowernumber ); + next if $fine && $fine > 0; + + my ( $renew_ok, $error ) = + C4::Circulation::CanBookBeRenewed( + $self->borrowernumber, $itemnumber + ); + if ( $renew_ok ) { + C4::Circulation::AddRenewal( + $self->borrowernumber, + $itemnumber, + $overdue_unreturned->{$itemnumber}->{branchcode}, + undef, + undef, + 1 + ); + } + } + } + return $available_credit; } -- 2.39.5