Bug 8939 - Fines Charged During Grace Period
Summary: Fines Charged During Grace Period
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: 3.8
Hardware: PC Linux
: P5 - low major (vote)
Target Milestone: ---
Assignee: Bugs List
QA Contact: Paul Poulain
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-17 23:33 UTC by Robert Williams
Modified: 2013-05-23 06:24 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Fine calculation condition update (1.09 KB, patch)
2012-10-17 23:58 UTC, Robert Williams
Details | Diff | Splinter Review
Bug 8939 : Fines being charged during the grace period (1.36 KB, patch)
2012-10-18 01:00 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Williams 2012-10-17 23:33:26 UTC
When grace period specified in a circulation rule is greater than one day (fines charged daily) and a user returns an item (matching the circ rule) within the grace period, the fine is charged rather than ignored (zero). In one example, with a 14-day grace period, the fines log produced nightly by fines.pl shows:

22903	STU	[snipped]	122222	2012-09-29 23:59:00	2	4
22951	STU	[snipped]	125741	2012-10-11 23:59:00	-10	1
22965	STU	[snipped]	77536	2012-10-14 23:59:00	-13	0.25

The negative numbers indicate how many days of grace period remain (-10 means the item is 4 days overdue); positive numbers shows how many days is overdue after the grace period. The negative values should result in no processing.

There appears to be an error in the if condition that determines whether to process/charge a fine on an overdue (negative values result in a TRUE as well as positive values):

    if ($data->{'chargeperiod'}  && $units_minus_grace  ) {

would work as expected if the second condition is changed:

    if ($data->{'chargeperiod'}  && ($units_minus_grace > 0)  ) {

I've tested on 3.8.3 (Debian package installation) and the change works for daily accruing fines. I have not yet tested using an hourly fine rule, but after a quick glance it seems it should work as well.

I'll try (I'm new to Git) to add/submit a patch next.
Comment 1 Robert Williams 2012-10-17 23:58:56 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2012-10-18 01:00:08 UTC
Created attachment 12894 [details] [review]
Bug 8939 : Fines being charged during the grace period

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 3 Chris Cormack 2012-10-18 01:01:06 UTC
Hi Robert

Next time, can you make your patch using git format-patch 

This one had no commit message, or authorship details, I fixed that when signing off.
Comment 4 Robert Williams 2012-10-18 14:43:15 UTC
Thanks, Chris. I think I see what I did wrong now. I'll give it a better go next time.
Comment 5 Paul Poulain 2012-10-26 15:32:04 UTC
QA comment: tiny patch, very good test plan, obviously fixing a problem.

passed QA

Robert, welcome as a contributor, you're 194th !
Comment 6 Paul Poulain 2012-10-26 15:32:20 UTC
Patch pushed to master
Comment 7 Chris Cormack 2012-10-27 01:31:09 UTC
Pushed to 3.8.x will be in 3.8.7