From 96286d98063f52665d8b6b404cef246626957190 Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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. Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Nick Clemens --- 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.11.0