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 <L.M.Harrison@lboro.ac.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 860d965..0fb39e8 100644 (file)
--- 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 f1bab9f..a4f73bb 100644 (file)
--- 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 546fc84..51eed7a 100644 (file)
--- 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;
 }