Bug 30254 - New overdue fine applied to incorrectly when using "Refund lost item charge and charge new overdue fine" option in circ rules
Summary: New overdue fine applied to incorrectly when using "Refund lost item charge a...
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Nick Clemens
QA Contact:
URL:
Keywords:
Depends on: 23091
Blocks:
  Show dependency treegraph
 
Reported: 2022-03-09 12:58 UTC by Andrew Fuerste-Henry
Modified: 2022-11-23 10:06 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:


Attachments
Bug 30254: Unit tests (1.22 KB, patch)
2022-10-31 18:44 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 30254: Don't charge overdue fines unless some fine exists (5.26 KB, patch)
2022-10-31 18:44 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 30254: Unit tests (1.29 KB, patch)
2022-11-01 10:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30254: Don't charge overdue fines unless some fine exists (5.32 KB, patch)
2022-11-01 10:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30254: (QA follow-up) Remove warn from tests (933 bytes, patch)
2022-11-01 10:21 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 Andrew Fuerste-Henry 2022-03-09 12:58:20 UTC
I've only confirmed this on 21.05. I haven't been able to replicate on master.
When using the lost item fee refund feature to charge a new overdue fee when a lost item is found, Koha can end up creating an overdue fine on a patron who never actually had an overdue item.

To recreate:
- set Lost item fee refund on return policy to "Refund lost item charge and charge new overdue fine"
- have an itemtype / patron combo that charges an overdue fine
- check item out to patron and then right back in again
- confirm patron doesn't have a fine because the item was not late
- set the item to Missing
- wait until after the item's due date from your previous checkout
- check the item out to a new patron
- confirm your original patron now has a fine

When the item is checked out and Koha clears its missing status, it looks back for the most recent checkout in old_issues and generates an overdue fine as if that item had been checked out the whole time. Even though this item wasn't lost by a patron and the first patron never had an overdue fine on it.

Things get more confusing if the first patron is anonymizing their reading history, as then the fine ends up on the anonymous borrower.
Comment 1 Andrew Fuerste-Henry 2022-04-25 15:22:35 UTC
Confirmed in master. To recreate:
1 - set Lost item fee refund on return policy to "Refund lost item charge and charge new overdue fine", turn on FinesMode, make sure your circ rules charge fines
2 - have an itemtype / patron combo that charges an overdue fine
3 - check item out (with a due date in the future) and then right back in again
4 - confirm patron doesn't have a fine because the item was not late
5 - set the item to Lost
6 - in the database, edit the date_due of your checkout to a date in the past
7 - check the item in, it is marked found
8 - confirm your patron now has a fine
Comment 2 Sara Brown 2022-08-24 18:33:04 UTC
This remains an issue in 21.11.
Comment 3 Sara Brown 2022-10-28 17:50:53 UTC
Still applicable as of 22.05.05.
Comment 4 Nick Clemens 2022-10-31 18:44:33 UTC
Created attachment 142839 [details] [review]
Bug 30254: Unit tests
Comment 5 Nick Clemens 2022-10-31 18:44:37 UTC
Created attachment 142840 [details] [review]
Bug 30254: Don't charge overdue fines unless some fine exists

We need to determine if a book was lost by a patron, the clues we have
are previous charges. If we don't find any, we shouldn't charge a new fine

To test:
 1 - set Lost item fee refund on return policy to "Refund lost item charge and charge new overdue fine", turn on FinesMode, make sure your circ rules charge fines
 2 - have an itemtype / patron combo that charges an overdue fine
 3 - check item out (with a due date in the future) and then right back in again
 4 - confirm patron doesn't have a fine because the item was not late
 5 - set the item to Lost
 6 - in the database, edit the date_due of your checkout to a date in the past
 7 - check the item in, it is marked found
 8 - confirm your patron now has a fine
 9 - Apply patch
10 - Repeat with a new item and patron
11 - Confirm no charges
Comment 6 Martin Renvoize 2022-11-01 10:21:33 UTC
Created attachment 142851 [details] [review]
Bug 30254: Unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2022-11-01 10:21:37 UTC
Created attachment 142852 [details] [review]
Bug 30254: Don't charge overdue fines unless some fine exists

We need to determine if a book was lost by a patron, the clues we have
are previous charges. If we don't find any, we shouldn't charge a new fine

To test:
 1 - set Lost item fee refund on return policy to "Refund lost item charge and charge new overdue fine", turn on FinesMode, make sure your circ rules charge fines
 2 - have an itemtype / patron combo that charges an overdue fine
 3 - check item out (with a due date in the future) and then right back in again
 4 - confirm patron doesn't have a fine because the item was not late
 5 - set the item to Lost
 6 - in the database, edit the date_due of your checkout to a date in the past
 7 - check the item in, it is marked found
 8 - confirm your patron now has a fine
 9 - Apply patch
10 - Repeat with a new item and patron
11 - Confirm no charges

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2022-11-01 10:21:42 UTC
Created attachment 142853 [details] [review]
Bug 30254: (QA follow-up) Remove warn from tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2022-11-01 10:22:33 UTC
Thanks for beating me to this one Nick, much appreciated..

Simple fix, signing off.
Comment 10 Katrin Fischer 2022-11-03 11:06:09 UTC
I am sorry, looks good, passes QA tests and unit tests, but couldn't finish testing here (ran out of time).
Comment 11 Martin Renvoize 2022-11-23 10:06:22 UTC
I think this may have fallen off Katrin's radar... bump