Bug 22377 - Lost fines may not be refunded if another patron has a writeoff for the same item
Summary: Lost fines may not be refunded if another patron has a writeoff for the same ...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 22563
Blocks:
  Show dependency treegraph
 
Reported: 2019-02-20 14:35 UTC by Nick Clemens
Modified: 2020-06-04 20:32 UTC (History)
5 users (show)

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


Attachments
Bug 22377: Add test case (6.98 KB, patch)
2019-04-09 12:27 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 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