From 974622fc87ba89d66d1930e81471a902a67762ed 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. Signed-off-by: Lucy Harrison Sponsored-by: Loughborough University --- C4/Circulation.pm | 12 ++++++++++- Koha/Account.pm | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++- Koha/Account/Line.pm | 49 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 2 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 014c018628..57f5ea0928 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2831,6 +2831,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. @@ -2842,6 +2847,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; @@ -2864,7 +2870,11 @@ sub AddRenewal { my $circ_library = Koha::Libraries->find( _GetCircControlBranch($item_unblessed, $patron_unblessed) ); - if ( C4::Context->preference('CalculateFinesOnReturn') && $issue->is_overdue ) { + if ( + !$skipfinecalc && + C4::Context->preference('CalculateFinesOnReturn') && + $issue->is_overdue + ) { _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 afff235822..b576f0c5e3 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -23,10 +23,11 @@ use Carp; use Data::Dumper; use List::MoreUtils qw( uniq ); -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; @@ -94,6 +95,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; @@ -112,6 +117,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 @@ -172,6 +189,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->accounttype @@ -252,6 +281,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 124aa29153..19351a0c19 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::Offsets; use Koha::Database; @@ -203,6 +204,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} ) { @@ -235,6 +241,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 @@ -247,6 +266,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.11.0