Attachment 83041 Details for Bug 21759 – Bug 21759: Avoid manually setting amountoutstanding in _FixAccountForLostAndReturned

[patch] Bug 21759: Avoid manually setting amountoutstanding in _FixAccountForLostAndReturned

Bug-21759-Avoid-manually-setting-amountoutstanding.patch (text/plain), 4.13 KB, created by Martin Renvoize (ashimema) on 2018-12-11 14:20:24 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2018-12-11 14:20:24 UTC

Size: 4.13 KB

patch

obsolete

>From 3276471c5d18f1fb1cbba7308087340dcd2a14fd Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Mon, 10 Dec 2018 16:45:00 -0300
>Subject: [PATCH] Bug 21759: Avoid manually setting amountoutstanding in
> _FixAccountForLostAndReturned
>
>This patch changes the behaviour in the _FixAccountForLostAndFound
>method.
>
>The method will now add the amountoutstanding value for the lost item
>fee to the CR credit to be generated. This means that:
>- If there's some remaining debt, the same amount  will be added to the
>  CR credit and used to cancel that debt. The final amountoutstanding
>  will be the same as before, but an offset will be generated as
>  required.
>- If the line was written off, the behaviour remains unchanged, so no
>  offset.
>- If the line was payed and/or written off in full only the payments are
>  refund, preserving the current behaviour.
>
>To test:
>- Apply the regression tests patch
>- Run:
>  $ kshell
> k$ prove t/db_dependent/Circulation.t
>=> FAIL: Tests fail because the behaviour is not correct
>- Apply this patch
>- Run:
> k$ prove t/db_dependent/Circulation.t
>=> SUCCESS: Tests now pass!
>- Sign off :-D
>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> C4/Circulation.pm | 46 ++++++++++++++++++++++++----------------------
> 1 file changed, 24 insertions(+), 22 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 0999a970fd..67a0e9a757 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -2404,12 +2404,14 @@ sub _FixAccountForLostAndReturned {
>     );
> 
>     return unless $accountlines->count > 0;
>-    my $accountline = $accountlines->next;
>+    my $accountline     = $accountlines->next;
>+    my $total_to_refund = 0;
>+    my $account = Koha::Patrons->find( $accountline->borrowernumber )->account;
> 
>     # Use cases
>     if ( $accountline->amount > $accountline->amountoutstanding ) {
>         # some amount has been cancelled. collect the offsets that are not writeoffs
>-        # this works because the only way to subtract from a debt is
>+        # this works because the only way to subtract from this kind of a debt is
>         # using the UI buttons 'Pay' and 'Write off'
>         my $credits_offsets = Koha::Account::Offsets->search({
>             debit_id  => $accountline->id,
>@@ -2418,30 +2420,30 @@ sub _FixAccountForLostAndReturned {
>             amount    => { '<'  => 0 } # credits are negative on the DB
>         });
> 
>-        my $total_to_refund = ( $credits_offsets->count > 0 )
>-                                ? $credits_offsets->total * -1 # credits are negative on the DB
>-                                : 0;
>+        $total_to_refund = ( $credits_offsets->count > 0 )
>+                            ? $credits_offsets->total * -1 # credits are negative on the DB
>+                            : 0;
>+    }
> 
>-        if ( $total_to_refund > 0 ) {
>-            my $account = Koha::Patrons->find( $accountline->borrowernumber )->account;
>-            $credit = $account->add_credit(
>-                {
>-                    amount      => $total_to_refund,
>-                    description => 'Item Returned ' . $item_id,
>-                    type        => 'lost_item_return'
>-                }
>-            );
>-        }
>+    my $credit_total = $accountline->amountoutstanding + $total_to_refund;
> 
>-        ModItem( { paidfor => '' }, undef, $itemnumber, { log_action => 0 } );
>+    if ( $credit_total > 0 ) {
>+        $credit = $account->add_credit(
>+            {   amount      => $credit_total,
>+                description => 'Item Returned ' . $item_id,
>+                type        => 'lost_item_return'
>+            }
>+        );
>+
>+        # TODO: ->apply should just accept the accountline
>+        $credit->apply( { debits => $accountlines->reset } );
>     }
>-    # else {
>-        # $accountline->amount == $accountline->amountoutstanding
>-    #}
> 
>-    $accountline->accounttype('LR');
>-    $accountline->amountoutstanding(0);
>-    $accountline->store();
>+    # Manually set the accounttype
>+    $accountline->discard_changes->accounttype('LR');
>+    $accountline->store;
>+
>+    ModItem( { paidfor => '' }, undef, $itemnumber, { log_action => 0 } );
> 
>     return ($credit) ? $credit->id : undef;
> }
>-- 
>2.19.2

Actions: View | Diff | Splinter Review

Attachments on bug 21759: 83029 | 83030 | 83038 | 83039 | 83040 | 83041 | 83042 | 83043