Bug 28421 - Add tests for Voided Payment and Voided Writeoff.
Summary: Add tests for Voided Payment and Voided Writeoff.
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on: 22435 27971
Blocks:
  Show dependency treegraph
 
Reported: 2021-05-21 15:59 UTC by Nick Clemens
Modified: 2022-06-06 20:25 UTC (History)
4 users (show)

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


Attachments
Bug 28421: Unit Tests (5.20 KB, patch)
2021-05-24 09:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28421: Unit Tests (5.00 KB, patch)
2021-05-24 10:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28421: Unit Tests (5.10 KB, patch)
2021-06-16 00:02 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 28421: Unit Tests (5.16 KB, patch)
2021-06-18 15:22 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2021-05-21 15:59:06 UTC
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 09:09:49 UTC
Hi nick,
Comment 2 Martin Renvoize 2021-05-24 09:12:09 UTC
Hi Nick, 

_FixAccountForLostAndReturned has gone away in master.. what version were you testing against?

I'm working my way through writing a test and can confirm so far this does indeed exist in master, but I think the fix will be very different for versions prior to 21.05 release (we use double-entry accounting for VOID in the coming release)
Comment 3 Martin Renvoize 2021-05-24 09:59:43 UTC
Created attachment 121316 [details] [review]
Bug 28421: 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 4 Martin Renvoize 2021-05-24 10:14:15 UTC
Created attachment 121317 [details] [review]
Bug 28421: 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 5 Martin Renvoize 2021-05-24 10:17:54 UTC
OK, For master, this is actually fixed with a combination of bug 22435 and bug 27971.. nice little side effect fix there.

I've attached a test that I think should be added anyway.
Comment 6 Victor Grousset/tuxayo 2021-05-26 10:08:56 UTC
Depends on bug 22435, I couldn't apply it on 20.11.x due to a conflict I didn't know how to solve.

Tried to not apply the dependency:

error: sha1 information is lacking or useless (t/db_dependent/Koha/Items.t)

So not testable for now.
Comment 7 Martin Renvoize 2021-05-26 10:30:11 UTC
Oop.. it was meant to be 'rel_21_05_candidate'.. I mixed up the tags..

The 20.11 and below version is in it's own bug.. and I'm struggling to come up with a solution for it.. see bug 28432
Comment 8 Jonathan Druart 2021-05-26 11:10:11 UTC
This will be for 21.11
Comment 9 Victor Grousset/tuxayo 2021-06-16 00:02:10 UTC
Created attachment 122017 [details] [review]
Bug 28421: 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
0/ Don't apply
1/ Run t/db_dependent/Koha/Items.t
2/ Prior to this commit the test should pass
3/ Apply this commit but not the dependency(bug 22435)
3/ The test should fail
4/ Bug 22435 should make the test pass again.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 10 Victor Grousset/tuxayo 2021-06-16 00:02:56 UTC
It works :)

Amended test plan to show the interaction with bug 22435
Comment 11 Nick Clemens 2021-06-18 15:22:42 UTC
Created attachment 122162 [details] [review]
Bug 28421: 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
0/ Don't apply
1/ Run t/db_dependent/Koha/Items.t
2/ Prior to this commit the test should pass
3/ Apply this commit but not the dependency(bug 22435)
3/ The test should fail
4/ Bug 22435 should make the test pass again.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 12 Jonathan Druart 2021-08-04 12:06:13 UTC
Pushed to master for 21.11, thanks to everybody involved!