Bug 28432 - When refunding lost fees we should not include voided payments [20.11 and below]
Summary: When refunding lost fees we should not include voided payments [20.11 and below]
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: 20.11
Hardware: All All
: P5 - low major (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-24 10:56 UTC by Martin Renvoize
Modified: 2022-01-21 14:48 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 28432: Unit Tests (5.20 KB, patch)
2021-05-24 11:56 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2021-05-24 10:56:11 UTC
+++ This bug was initially created as a clone of Bug #28421 +++

To recreate:
1 - Mark an item lost
2 - Add a manual lost fee for that barcode to a patron 
    (or checkout before #1 and ensure a fine will be charged when marked lost)
3 - Pay the fee
4 - Void the fee
5 - Pay the fee
6 - Checkin the item
7 - The patron has a credit for twice the fee

The problematic code appears to be in _FixAccountForLostAndReturned:

        my $credits_offsets = Koha::Account::Offsets->search({
            debit_id  => $accountline->id,
            credit_id => { '!=' => undef }, # it is not the debit itself
            type      => { '!=' => 'Writeoff' },
            amount    => { '<'  => 0 } # credits are negative on the DB
        });

We count both the payment and the voided payment
Comment 1 Martin Renvoize 2021-05-24 11:56:40 UTC
Created attachment 121323 [details] [review]
Bug 28432: Unit Tests

This patch adds test test cases for both a Voided Payment and Voided
Writeoff. These cases need special handling in the refund process.

Test plan
1/ Run t/db_dependent/Koha/Items.t
2/ Prior to this commit the test should pass
3/ After this commit the test should fail
4/ The subsequent commit should make the test pass again.
Comment 2 Martin Renvoize 2021-05-24 11:59:01 UTC
Pretty confident in the test.. but no idea how to resolve the actual bug..

I've tried using joins to look at the credit line status.. but for some reason that breaks all sorts..

>  my $credits_offsets = Koha::Account::Offsets->search(
>      {
>          debit_id  => $lost_charge->id,
>          credit_id => { '!=' => undef },     # it is not the debit itself
>          'credit.credit_type_code' => { '!=' => 'Writeoff' },
>          'credit.status'           => { '!=' => 'VOID' },
>      },
>      { join => 'credit' }
>  );
Comment 3 Nick Clemens 2021-05-26 14:42:01 UTC
Is this 20.05 and earlier? 20.11 does not appear to have _FixAccountForLostAndReturned?