Attachment 92689 Details for Bug 23051 – Bug 23051: Renew items when fines paid off

[patch] Bug 23051: Renew items when fines paid off

Bug-23051-Renew-items-when-fines-paid-off.patch (text/plain), 9.29 KB, created by PTFS Europe Sandboxes on 2019-09-10 11:13:39 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: PTFS Europe Sandboxes

Created: 2019-09-10 11:13:39 UTC

Size: 9.29 KB

patch

obsolete

>From df8fba7cd78f31852b68fa3471cb0f0916b88309 Mon Sep 17 00:00:00 2001
>From: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
>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 <L.M.Harrison@lboro.ac.uk>
>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

Actions: View | Diff | Splinter Review

Attachments on bug 23051: 90559 | 90560 | 90561 | 91058 | 91059 | 91060 | 91792 | 91793 | 91794 | 91795 | 92684 | 92685 | 92686 | 92687 | 92688 | 92689 | 93922 | 93923 | 93924 | 93925 | 94020 | 94021 | 94022 | 94023 | 94024 | 94402 | 94434 | 94453 | 94525 | 94604 | 94605 | 94606 | 94607 | 94608 | 94609 | 94610 | 94611 | 94629 | 94649 | 95557 | 95558 | 95559 | 95560 | 100127 | 100128 | 100129 | 100130 | 100131 | 100132 | 100133 | 100134 | 100135 | 100138 | 100172 | 100182 | 100183 | 100184 | 100185 | 100186 | 100187 | 100188 | 100271