Bug 22377

Summary: Lost fines may not be refunded if another patron has a writeoff for the same item
Product: Koha Reporter: Nick Clemens <nick>
Component: Fines and feesAssignee: Bugs List <koha-bugs>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: cbrannon, jonathan.druart, josef.moravec, martin.renvoize, tomascohen
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22563
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 22563    
Bug Blocks:    
Attachments: Bug 22377: Add test case

Description Nick Clemens 2019-02-20 14:35:23 UTC
To recreate:
1 - Checkout an item to Patron A, backdate the due date and run fines to generate an overdue
2 - Return the item
3 - Checkout the item to Patron B, mark it lost and ensure a fine is generated
4 - In the DB set the date i=on the lost fine to yesterday
5 - Writeoff the fine for Patron A
6 - Return the item 
7 - It is no longer lost, but the lost fine is not refunded
Comment 1 Jonathan Druart 2019-03-05 17:08:20 UTC
Do you have the rule defined? It seems that it is not created on install.
Comment 2 Nick Clemens 2019-03-07 18:48:34 UTC
(In reply to Jonathan Druart from comment #1)
> Do you have the rule defined? It seems that it is not created on install.

Which rule? 

I believe the problem is in the code  for _FixAccountForLostAndReturned

    my $accountlines = Koha::Account::Lines->search(
        {
            itemnumber  => $itemnumber,
            accounttype => { -in => [ 'L', 'Rep', 'W' ] },
        },
        {
            order_by => { -desc => [ 'date', 'accountno' ] }
        }
    );

We order by date and accountno - so a more recent writeoff will be found insetad of the lost fine
Comment 3 Martin Renvoize 2019-04-01 15:48:28 UTC
Bug 22563 could be very helpful here.
Comment 4 Martin Renvoize 2019-04-01 16:33:24 UTC
On second thoughts.. why do we look for 'W' at all in that query?.. 'W' = Writeoff == Credit.. should we not be looking solely for the 'L' (and/or 'LR') debt line?
Comment 5 Martin Renvoize 2019-04-02 09:38:41 UTC
I'm working on a test for this at the tip of my accounts branch over here: https://github.com/PTFS-Europe/koha/compare/accounts_all...PTFS-Europe:bug_22377

Currently however I'm struggling to replicate the problem.
Comment 6 Martin Renvoize 2019-04-09 12:27:02 UTC
Created attachment 87605 [details] [review]
Bug 22377: Add test case
Comment 7 Martin Renvoize 2019-04-09 12:27:46 UTC
Attached my attempt to write a test for this.. anyone is welcome to take over as I couldn't replicate the issue.
Comment 8 Christopher Brannon 2019-07-03 18:11:12 UTC
Would like to see this move forward.  This is an active problem.
Comment 9 Martin Renvoize 2019-10-24 12:39:48 UTC
Test still applies and I still can't replicate the issue... 

Reducing severity for now until someone can come up with a failing test case.
Comment 10 Nick Clemens 2019-10-24 16:13:37 UTC
The tests are constructed correctly, however, the problematic code has changed.

The search as I describe has been removed, it now looks for a status of undef or '!= returned' AND specifically for lost fines

Additionally, a write-off no longer gets an itemnumber, so we can't accidentally get the wrong fines.

Fixed by bug 22563