The fines cronjob currently skips updating fines if the date it is run is a holiday for the applicable library. This is problematic when an item is marked lost the day following a holiday as we can end up missing a day of fines. Additionally it confuses things for libraries who do not expect the calendar to affect fines. This seems to have been done when dropbox mode used calculations on the 'last update' of a fine to determine how much to refund. Now that dropbox mode simply calculates the fine with a different return date this code is no longer needed.
Created attachment 114287 [details] [review] Bug 27180: Update fines on holidays This patch removes code that prevents fines from being update on holidays. The fines are already being calculated, and that takes the calendar into account if needed, so fines won't be assessed on holidays if they shouldn't. To test: 1 - set finesCalendar to 'ignore' 2 - Checkout an item due yesterday 3 - Ensure circ rules have a fine amount set 4 - Make today a holiday 5 - run fines.pl 6 - No fine assessed 7 - Apply patch 8 - run fines.pl 9 - Fine assessed! 10 - set finesCalendar to 'use' 11 - checkout an item, due yesterday 12 - run fines.pl 13 - no fine assessed
Created attachment 114404 [details] [review] Bug 27180: Update fines on holidays This patch removes code that prevents fines from being update on holidays. The fines are already being calculated, and that takes the calendar into account if needed, so fines won't be assessed on holidays if they shouldn't. To test: 1 - set finesCalendar to 'ignore' 2 - Checkout an item due yesterday 3 - Ensure circ rules have a fine amount set 4 - Make today a holiday 5 - run fines.pl 6 - No fine assessed 7 - Apply patch 8 - run fines.pl 9 - Fine assessed! 10 - set finesCalendar to 'use' 11 - checkout an item, due yesterday 12 - run fines.pl 13 - no fine assessed Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This makes allot of sense and cleans up the code... Works as expected, signing off.
Created attachment 114706 [details] [review] Bug 27180: (follow-up) Remove unused %is_holiday
I think you could probably set this to PQA Victor.. I'd call your followup a 'QA follow-up) as it's trivial and works well..
Created attachment 115228 [details] [review] Bug 27180: Update fines on holidays This patch removes code that prevents fines from being update on holidays. The fines are already being calculated, and that takes the calendar into account if needed, so fines won't be assessed on holidays if they shouldn't. To test: 1 - set finesCalendar to 'ignore' 2 - Checkout an item due yesterday 3 - Ensure circ rules have a fine amount set 4 - Make today a holiday 5 - run fines.pl 6 - No fine assessed 7 - Apply patch 8 - run fines.pl 9 - Fine assessed! 10 - set finesCalendar to 'use' 11 - checkout an item, due yesterday 12 - run fines.pl 13 - no fine assessed Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 115229 [details] [review] Bug 27180: (follow-up) Remove unused %is_holiday Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 21.05, thanks to everybody involved!
Pushed to 20.11.x for 20.11.02
Pushed to 20.05.x for 20.05.08
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.
Nick, please fill the release notes.
It appears like now we are charging on closed days with this bug... see bug 27835. For our use case that is definitely unwanted behaviour and we need to fix it asap. I know I QA'd that... but I was under the assumtpion it would only affect finescalendar = ignore... which it doesn't.