From 8ffa434a403171e215ebaa8eaca609be90c979d8 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Wed, 24 Mar 2021 16:38:32 +0000
Subject: [PATCH] Bug 25508: Only return renewal outcomes to the controller

There are a few cases where the `renew_item` method in
Koha::Account::Line will return `undef`. For these cases, we should not
pass the error up the chain to the controllers as it leads to malformed
error messages in the UI.

Test plan
1 - Make sure FinesMode is on, RenewAccruingItemWhenPaid is off
2 - Checkout an item to a patron and make it overdue (can backdate the
    checkout)
3 - Make sure the itemtype has fines that will be charged
4 - Charge the fines:
    Set finesMode = production
    perl misc/cronjobs/fines.pl -v
5 - Check the fine appears on the patrons account
6 - Pay off the fine
7 - Receive alert after payment that reads: "The fines on
    the following items were paid off, renewal results are
    displayed below:
        No title ( ): Not renewed - Unknown error"
8 - Apply the patch
9 - Repeat steps 1 through 6 and note that you no longer trigger the
    error message.
---
 Koha/Account.pm      |  4 ++--
 Koha/Account/Line.pm | 13 +++++--------
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/Koha/Account.pm b/Koha/Account.pm
index b4838bcdf2..55ee594ff8 100644
--- a/Koha/Account.pm
+++ b/Koha/Account.pm
@@ -203,9 +203,9 @@ sub 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.
         my $amt = $old_amountoutstanding - $amount_to_pay;
-        if ($fine->renewable) {
+        if ( $fine->renewable ) {
             my $outcome = $fine->renew_item;
-            push @{$renew_outcomes}, $outcome;
+            push @{$renew_outcomes}, $outcome if $outcome;
         }
 
         if ( C4::Context->preference('MarkLostItemsAsReturned') =~ m|onpayment|
diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm
index 2cc81993ef..185c753772 100644
--- a/Koha/Account/Line.pm
+++ b/Koha/Account/Line.pm
@@ -901,7 +901,9 @@ sub renewable {
         $self->debit_type_code &&
         $self->debit_type_code eq 'OVERDUE' &&
         $self->status &&
-        $self->status eq 'UNRETURNED'
+        $self->status eq 'UNRETURNED' &&
+        $self->item &&
+        $self->patron
     ) ? 1 : 0;
 }
 
@@ -919,19 +921,14 @@ sub renew_item {
 
     my $outcome = {};
 
-    # We want to reject the call to renew if any of these apply:
+    # We want to reject the call to renew if:
     # - The RenewAccruingItemWhenPaid syspref is off
-    # - The line item doesn't have an item attached to it
-    # - The line item doesn't have a patron attached to it
-    #
+    # OR
     # - The RenewAccruingItemInOpac syspref is off
-    # AND
     # - There is an interface param passed and it's value is 'opac'
 
     if (
         !C4::Context->preference('RenewAccruingItemWhenPaid') ||
-        !$self->item ||
-        !$self->patron ||
         (
             !C4::Context->preference('RenewAccruingItemInOpac') &&
             $params->{interface} &&
-- 
2.20.1