Bug 31060 - Fine duplicates when returning item with accountline status marked as lost
Summary: Fine duplicates when returning item with accountline status marked as lost
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor
Assignee: Shi Yao Wang
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-06-28 19:36 UTC by Shi Yao Wang
Modified: 2023-03-27 10:12 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:
Circulation function:


Attachments
Bug 31060: Fine duplicates when returning item with accountline status marked as lost (2.41 KB, patch)
2022-06-28 19:59 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 31060: Fine duplicates when returning item with accountline status marked as lost (3.59 KB, patch)
2022-07-07 14:05 UTC, Shi Yao Wang
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Shi Yao Wang 2022-06-28 19:36:32 UTC
To recreate:
1- System preferences:
    finesMode:                    Calculate and charge
    CalculateFinesOnReturn:       Do
    WhenLostChargeReplacementFee: Don't charge
    MarkLostItemsAsReturned:      uncheck from the items tab of the catalog module
2- Have a circulation rule with defined non-zero Fine amount and Fine charging interval. (Will have to use the patron category and item type associated to this circulation rule to test)
3- Checkout a document from the patron with backdated due date (backdated enough depending on the Fine charging interval set)
4- Run ./misc/cronjobs/fines.pl
5- There should be a Fine(Accruing) in accounting tab of the patron
6- Go to the checked out item detail and set Lost status: Long overdue (Lost) and click Set status
7- Now, Fine(Accruing) changed to Fine(Lost) in the accounting tab of the patron
8- In item detail, set Lost status: Choose and click Set status
9- Notice Fine(Lost) did not switch back to Fine(Accruing) in the accounting tab
10- Check in the item
11- Notice there is a duplicate fee Fine(Returned) and Fine(Lost) is still there
Comment 1 Shi Yao Wang 2022-06-28 19:59:36 UTC Comment hidden (obsolete)
Comment 2 Shi Yao Wang 2022-06-28 21:18:51 UTC
Actually, unit tests doesn't seem happy with this: 

prove t/db_dependent/Circulation.t 
>t/db_dependent/Circulation.t .. 17/61     # No tests run!
>t/db_dependent/Circulation.t .. 18/61 
>#   Failed test 'No tests run for subtest "GetUpcomingDueIssues"'
>#   at t/db_dependent/Circulation.t line 1594.
>Exception 'Koha::Exceptions::Account::UnrecognisedType' thrown 'Update type not >allowed on this debit_type'
># Looks like your test exited with 255 just after 18.
>t/db_dependent/Circulation.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
>Failed 44/61 subtests

Maybe a better solution is to make accountlines with status=LOST to switch back to UNRETURNED at step 9.
Comment 3 Shi Yao Wang 2022-07-07 14:05:18 UTC
Created attachment 137312 [details] [review]
Bug 31060: Fine duplicates when returning item with accountline status marked as lost

Make unpaid accountline switch from 'LOST' to 'UNRETURNED' when unsetting lost
status from an item.

To test:
1- System preferences:
    finesMode:                    Calculate and charge
    CalculateFinesOnReturn:       Do
    WhenLostChargeReplacementFee: Don't charge
    MarkLostItemsAsReturned:      uncheck from the items tab of the catalog module
2- Have a circulation rule with defined non-zero Fine amount and Fine charging interval. (Will have to use the patron category and item type associated to this circulation rule to test)
3- Checkout a document from the patron with backdated due date (backdated enough depending on the Fine charging interval set)
4- Run ./misc/cronjobs/fines.pl
5- There should be a Fine(Accruing) in accounting tab of the patron
6- Go to the checked out item detail and set Lost status: Long overdue (Lost) and click Set status
7- Now, Fine(Accruing) changed to Fine(Lost) in the accounting tab of the patron
8- In item detail, set Lost status: Choose and click Set status
9- Notice Fine(Lost) did not switch back to Fine(Accruing) in the accounting tab
10- Check in the item
11- Notice there is a duplicate fee Fine(Returned) and Fine(Lost) is still there
12- Apply the patch
13- Write off all fines of the patron
14- Repeat step 3 to 10 and notice step 9 is not true anymore
15- Notice there is only Fine(Returned) and no duplicate
Comment 4 Martin Renvoize (ashimema) 2022-07-15 15:35:32 UTC
I'm pretty sure this is by design actually.

It's not a duplicate fine.. it's a new fine... there are more preferences and circulation rules that play into this.

I would take a look at the 'Lost item fee refund on return policy' section of the circulation rules.  We describe what to do with these fines and lost fees there.. i.e should we restore them, or charge a new fee.. perhaps there needs to be further options there?

I don't think we should just undo the LOST status of the fine however.. that could trigger refunds should the user have already paid for example.
Comment 5 Magnus Enger 2023-03-27 10:12:24 UTC
Looks like this needs some more discussions/considerations.