Bug 4906 - Allow fine calculation to exclude grace period
Summary: Allow fine calculation to exclude grace period
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Target Milestone: ---
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-24 13:13 UTC by Katrin Fischer
Modified: 2014-05-26 21:04 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
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
Bug 4906 - Problem with grace period calculation and fine intervals >1 (4.49 KB, patch)
2012-08-09 14:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 4906 - Problem with grace period calculation and fine intervals >1 (4.49 KB, patch)
2012-08-09 14:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 4906 - Problem with grace period calculation and fine intervals >1 (4.52 KB, patch)
2012-11-19 13:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 4906 - Problem with grace period calculation and fine intervals >1 (4.52 KB, patch)
2012-11-20 22:17 UTC, Srikanth Dhondi
Details | Diff | Splinter Review
Bug 4906 - Problem with grace period calculation and fine intervals >1 - Followup (1.11 KB, patch)
2012-11-21 12:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 4906 - Problem with grace period calculation and fine intervals >1 (4.37 KB, patch)
2013-03-08 10:33 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 4906: Allow fine calculation to exclude grace period (4.49 KB, patch)
2013-03-08 10:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2010-06-24 13:13:01 UTC
With a fine interval > 1 there is currently no way to avoid a effective grace period. Fines can not start the day after the due date.

Configuration:
With grace period = 1
fine interval 5 (= weekly, the library is closed Sunday and Saturday)

Result:
The first fine ist set 7 days after the item was due.
Monday, checkout
Tuesday, item due
Wednesday
Thursday
Friday
Saturday, holiday
Sunday, holiday
Monday,
Tuesday, first fine

Grace period for fine intervals > 1 should describe the days between the due date and the day the first fine is set.

CalcFines() in Overdues.pm:
 266         my $days_minus_grace = $daystocharge - $data->{'firstremind'};
 267     if ($data->{'chargeperiod'} > 0 && $days_minus_grace > 0 ) { 
 268         $amount = int($daystocharge / $data->{'chargeperiod'}) * $data->{'fine'};
Comment 1 Katrin Fischer 2012-01-06 16:08:17 UTC
Still valid :(
Comment 2 Katrin Fischer 2012-08-06 18:16:00 UTC
Hi Kyle, is there a reason you removed me as assignee from this bug? 
I know it's an old bug - but I have some code that would have worked before hourly loans an I hoped to look into this again before end of August.
Comment 3 Kyle M Hall 2012-08-09 12:43:16 UTC
Katrin, I was trying to re-assign it to myself, but it looks like I only switched it to the default assignee. I was planning on taking a crack at this.
Comment 4 Kyle M Hall 2012-08-09 12:43:52 UTC
Katrin, I was trying to re-assign it to myself, but it looks like I only switched it to the default assignee. I was planning on taking a crack at this, but I have re-assigned it back to you in case you get to it first.
Comment 5 Kyle M Hall 2012-08-09 14:56:06 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2012-08-09 14:57:21 UTC Comment hidden (obsolete)
Comment 7 Srikanth Dhondi 2012-11-19 01:09:55 UTC
Applying: Bug 4906 - Problem with grace period calculation and fine intervals >1
/home/srikanth/Projects/Koha/koha/.git/rebase-apply/patch:17: trailing whitespace.
        if ( C4::Context->preference('FinesIncludeGracePeriod') ) {        
error: patch failed: C4/Overdues.pm:264
error: C4/Overdues.pm: patch does not apply
error: patch failed: installer/data/mysql/sysprefs.sql:374
error: installer/data/mysql/sysprefs.sql: patch does not apply
error: patch failed: installer/data/mysql/updatedatabase.pl:5635
error: installer/data/mysql/updatedatabase.pl: patch does not apply
Patch failed at 0001 Bug 4906 - Problem with grace period calculation and fine intervals >1
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Patch left in /tmp/Bug-4906---Problem-with-grace-period-calculation-a-w6Gl8z.patch
Comment 8 Kyle M Hall 2012-11-19 13:57:17 UTC Comment hidden (obsolete)
Comment 9 Srikanth Dhondi 2012-11-20 21:45:37 UTC
Applying: Bug 4906 - Problem with grace period calculation and fine intervals >1
error: patch failed: installer/data/mysql/updatedatabase.pl:6055
error: installer/data/mysql/updatedatabase.pl: patch does not apply
Patch failed at 0001 Bug 4906 - Problem with grace period calculation and fine intervals >1
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Patch left in /tmp/Bug-4906---Problem-with-grace-period-calculation-a-GyPe81.patch
Comment 10 Srikanth Dhondi 2012-11-20 22:17:47 UTC Comment hidden (obsolete)
Comment 11 Srikanth Dhondi 2012-11-21 01:10:57 UTC
I think the system preference is called "FinesForGracePeriod" not "FinesIncludeGracePeriod". Could you confirm that? 

(In reply to comment #8)
> Created attachment 13515 [details] [review]
> Bug 4906 - Problem with grace period calculation and fine intervals >1
> 
> This patch adds the system preference FinesIncludeGracePeriod which
> allows the library the choice to include the grace period when calculating
> a fine ( Koha's current behavior ) or to ingore the grace period for
> the purposes of fines calculation. It is set by default to preserve the
> current behavior.
Comment 12 Kyle M Hall 2012-11-21 12:18:19 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2012-11-21 12:19:11 UTC
You are correct. Thanks for catching that!

(In reply to comment #11)
> I think the system preference is called "FinesForGracePeriod" not
> "FinesIncludeGracePeriod". Could you confirm that?
Comment 14 Paul Poulain 2012-12-11 07:57:41 UTC
QA comment: the QA for this patch should not be a problem, because it's a small one. BUT I think it does not solve the problem katrin described in his initial description: if you set FinesIncludeGracePeriod, then there is no change in the behaviour, and you will still have the same result.

The problem Katrin pointed in his description is the fact that the fine is placed *at the end* of each fine interval, not at the beginning.

Your patch is related to another feature I think. Katrin, do you agree that this patch does not solve the problem you describe (it's a valuable ENH though, but maybe in another bug ?)
Comment 15 Jan Kissig 2012-12-23 12:27:14 UTC
I think what Paul stated is the bug Katrin described, the first fine should be charged after due-date + graceperiod. Now it is charged after due-date + grace + fine-interval.
Comment 16 Paul Poulain 2013-01-04 15:35:39 UTC
(In reply to comment #15)
> I think what Paul stated is the bug Katrin described, the first fine should
> be charged after due-date + graceperiod. Now it is charged after due-date +
> grace + fine-interval.

exactly !
Comment 17 Marcel de Rooy 2013-03-08 10:14:40 UTC
QA: Looking at this one now.
Comment 18 Marcel de Rooy 2013-03-08 10:33:30 UTC Comment hidden (obsolete)
Comment 19 Marcel de Rooy 2013-03-08 10:56:39 UTC
Changing the title of this report and opening a new bug for the original one.
Comment 20 Marcel de Rooy 2013-03-08 10:57:54 UTC
Created attachment 15970 [details] [review]
Bug 4906: Allow fine calculation to exclude grace period

This patch adds the system preference FinesIncludeGracePeriod which
allows the library the choice to include the grace period when calculating
a fine ( Koha's current behavior ) or to ignore the grace period for
the purposes of fines calculation. It is set by default to preserve the
current behavior.

Signed-off-by: Srikanth Dhondi <srikanth@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Rebased March 8, 2013. Included renaming pref.
Works as advertised. Renamed the bug title and opened a new report for the
original bug.
Comment 21 Marcel de Rooy 2013-03-08 11:07:44 UTC
QA Comment:
Kyle probably should have opened a new report for this development some time
ago. I did now the other way around. Bug 9768 is the new old one :)

This patch works as advertised. Code looks good to me.

Passed QA
Comment 22 Katrin Fischer 2013-03-08 11:25:15 UTC
Can someone who tested this explain in a few words where the difference is between Kyle's change and what the bug was intended to do? I am a bit confused at this point :)
Comment 23 Kyle M Hall 2013-03-08 13:46:38 UTC
I think Paul explains it in comment 14. 

(In reply to comment #22)
> Can someone who tested this explain in a few words where the difference is
> between Kyle's change and what the bug was intended to do? I am a bit
> confused at this point :)
Comment 24 Jared Camins-Esakov 2013-03-17 01:48:49 UTC
This patch has been pushed to master.