From df8fba7cd78f31852b68fa3471cb0f0916b88309 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 e346e2dfd6..64979765ae 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 8894f817b9..1c75b257a4 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; @@ -88,6 +89,10 @@ sub pay { my $interface = $params ? ( $params->{interface} || C4::Context->interface ) : C4::Context->interface; 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; @@ -106,6 +111,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 @@ -166,6 +183,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 @@ -245,6 +274,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 f4f9498ee1..d72fe264a6 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 { while ( my $debit = $debits->next ) { @@ -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