Attachment 119192 Details for Bug 25508 – Bug 25508: Only return renewal outcomes to the controller

[patch] Bug 25508: Only return renewal outcomes to the controller

Bug-25508-Only-return-renewal-outcomes-to-the-cont.patch (text/plain), 3.06 KB, created by Martin Renvoize (ashimema) on 2021-04-06 07:52:04 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2021-04-06 07:52:04 UTC

Size: 3.06 KB

patch

obsolete

>From 29eed35aa08f7bf8bbd2eda2cdf89d13528e16da 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

Actions: View | Diff | Splinter Review

Attachments on bug 25508: 118765 | 118776 | 118777 | 118778 | 119191 | 119192 | 119213 | 119214 | 119639 | 119640 | 119641 | 119702 | 146001 | 146002 | 146003 | 146004