An item can be marked as lost by longoverdue.pl, but left checked out to the patron. In this case, the item will continue to accrue fines.
Created attachment 25720 [details] [review] Bug 11872 - Lost overdue items should not generate fines An item can be marked as lost by longoverdue.pl, but left checked out to the patron. In this case, the item will continue to accrue fines. Test Plan: 1) Check out an item and back date it so it is overdue and should generate fines. 2) Mark the item as lost by either using longoverdue.pl, or just by setting itemlost to 1 by directly accessing the database 3) Run fines.pl 4) Note the overdue generated a fine 5) Repeat steps 1-2 6) Apply this patch 7) Run fines.pl 8) Note a fine was not generated
*** Bug 9559 has been marked as a duplicate of this bug. ***
Created attachment 25830 [details] [review] [SIGNED-OFF] Bug 11872 - Lost overdue items should not generate fines 1-Checked out item backdated 5 months. 2-Ran longoverdue.pl --lost 90=2 --confirm to get the itemlost flag set. 3-Ran fines.pl -v to get fines accrued. (So at this point in UI I saw $146 FU fine, and $5.00 Rental.) 4-Payed fines and checked in item. 5-Applied patch. 6 Did steps 1-3 At this point only saw $5.00 Rental, but no accrued fines. Correct.
At least one unit test is required for this change. Marked as Failed QA.
Created attachment 29228 [details] [review] Bug 11872 [QA Followup] - Unit Tests
Hi Kyle, I am wondering if the 2 routines checkoverdues and GetOverduesForBranch should not be changed too. If you consider that lost items are not overdues, both routines should be changed too, but I am not sure this is right. Otherwise, a routine named "Getoverdues" should return all overdues (even lost items). In this case you could remove the lost items in the 2 scripts calling this routine.
I think Jonathan raises a good point here - we need to decide if lost items count as overdues and then treat them consistently. I tend to say they are overdue AND lost, so should still show up with your overdue items. If we want the logic in the routine, maybe it could be a parameter.
Kyle, could you please take a look at Jonathan's and my comments?
Created attachment 30294 [details] [review] Bug 11872 - Lost overdue items should not generate fines An item can be marked as lost by longoverdue.pl, but left checked out to the patron. In this case, the item will continue to accrue fines. Test Plan: 1) Check out an item and back date it so it is overdue and should generate fines. 2) Mark the item as lost by either using longoverdue.pl, or just by setting itemlost to 1 by directly accessing the database 3) Run fines.pl 4) Note the overdue generated a fine 5) Repeat steps 1-2 6) Apply this patch 7) Run fines.pl 8) Note a fine was not generated
(In reply to Jonathan Druart from comment #6) > Otherwise, a routine named "Getoverdues" should return all overdues (even > lost items). In this case you could remove the lost items in the 2 scripts > calling this routine. I agree, I've submitted an alternative patch that uses this path to resolution.
I'm not saying that the patch is wrong, but can someone explain reported numbers from those scripts please. They seem to be out of sync.
(In reply to Srdjan Jankovic from comment #11) > I'm not saying that the patch is wrong, but can someone explain reported > numbers from those scripts please. They seem to be out of sync. Srdjan, Sorry but I don't understand your question. What do you mean by "reported numbers"?
Created attachment 33913 [details] [review] Bug 11872 - Lost overdue items should not generate fines An item can be marked as lost by longoverdue.pl, but left checked out to the patron. In this case, the item will continue to accrue fines. Test Plan: 1) Check out an item and back date it so it is overdue and should generate fines. 2) Mark the item as lost by either using longoverdue.pl, or just by setting itemlost to 1 by directly accessing the database 3) Run fines.pl 4) Note the overdue generated a fine 5) Repeat steps 1-2 6) Apply this patch 7) Run fines.pl 8) Note a fine was not generated Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
This patch fixes an issue, but to pass the QA step, we need tests on C4::Overdues::Getoverdues. Maybe additional documentations (doc + help online) could be great.
(In reply to Jonathan Druart from comment #12) > Srdjan, > Sorry but I don't understand your question. > What do you mean by "reported numbers"? I do apologise, I cannot remember myself any more. I probably meant "what is the difference between fines.pl and staticfines.p, they report different numbers". But on the other hand, why would there be two scripts if they did the same thing...
Created attachment 33980 [details] [review] [PASSED QA] Bug 11872 - Lost overdue items should not generate fines An item can be marked as lost by longoverdue.pl, but left checked out to the patron. In this case, the item will continue to accrue fines. Test Plan: 1) Check out an item and back date it so it is overdue and should generate fines. 2) Mark the item as lost by either using longoverdue.pl, or just by setting itemlost to 1 by directly accessing the database 3) Run fines.pl 4) Note the overdue generated a fine 5) Repeat steps 1-2 6) Apply this patch 7) Run fines.pl 8) Note a fine was not generated Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Works as expected, super simple patch, passes qa scripts.. Passing QA
Patch pushed to master. Thanks Kyle!
Pushed to 3.18.x will be in 3.18.3
Pushed to 3.16.x, will be in 3.16.7
Pushed to 3.14, will be in 3.14.14