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
Hi nick,
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)
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.
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.
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.
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.
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
This will be for 21.11
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>
It works :) Amended test plan to show the interaction with bug 22435
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>
Pushed to master for 21.11, thanks to everybody involved!