Bug 3341 - fines calculation erroneous when a repeatable holiday is added.
Summary: fines calculation erroneous when a repeatable holiday is added.
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: PATCH-Sent (DO NOT USE) major (vote)
Assignee: Galen Charlton
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-19 12:28 UTC by Chris Cormack
Modified: 2012-10-25 23:04 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Comment one line in Overdues.pm that could cause "amountoutstanding > amount" (661 bytes, patch)
2010-10-28 17:44 UTC, Frédérick Capovilla
Details | Diff | Splinter Review
Comment one line in Overdues.pm that could cause "amountoutstanding > amount" (Correct patch formatting) (982 bytes, patch)
2011-02-07 14:38 UTC, Frédérick Capovilla
Details | Diff | Splinter Review
Signed patch (1.31 KB, patch)
2011-02-17 12:42 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 Chris Cormack 2010-05-21 01:09:20 UTC


---- Reported by rch@liblime.com 2009-06-19 00:28:48 ----

Adding a repeatable holiday creates holidays retroactively for Koha's calendar module.  fines.pl balks when it sees an overdue fine calculation where amount != amountoutstanding (it assumes a payment was made, so tries to adjust the amountoutstanding appropriately).  This fails if the calendar module reports a smaller number of fine-inducing days on one run of fines.pl than it did on the previous.

Symptoms:
Overdue charges appear on patron accounts with Amount Outstanding > Amount.

Steps to reproduce:
  * run fines.pl with some overdue items.
  * set calendaring sysprefs to ensure fines calculations take the calendar into account
  * add a repeatable holiday that will fall within the overdue period of your overdue items
  * run fines.pl again
  * the new Amount is the correct fine.  Amount Outstanding is now incorrect, as it includes the fines on the days that are now holidays.

amountoutstanding > amount is nonsensical from Koha's standpoint, and should not occur.



---- Additional Comments From mcoalwell@nekls.org 2009-07-08 14:44:32 ----

Note that this erroneous fine calculation disregards the maxfine syspref and cheerfully adds fine amounts exceeding the maximum allowable. 



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:09 UTC  ---

This bug was previously known as _bug_ 3341 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=3341

Actual time not defined. Setting to 0.0
The original reporter of this bug does not have
   an account here. Reassigning to the person who moved
   it here: chris@bigballofwax.co.nz.
   Previous reporter was rch@liblime.com.
CC member bchurch@ptfs.com does not have an account here

Comment 1 Frédérick Capovilla 2010-10-28 17:44:42 UTC Comment hidden (obsolete)
Comment 2 Frédérick Capovilla 2011-02-07 14:38:14 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2011-02-17 12:41:03 UTC
Tested and signed.
If amount goes up or down, outstanding should follow in both directions.
Comment 4 Marcel de Rooy 2011-02-17 12:42:16 UTC
Created attachment 3174 [details] [review]
Signed patch
Comment 5 Chris Cormack 2011-02-17 20:58:09 UTC
This patch will certainly mean the amount can go up and down

BUT 

Have you tested with the scenario outlined in the bug
IE

Steps to reproduce:
  * run fines.pl with some overdue items.
  * set calendaring sysprefs to ensure fines calculations take the calendar
into account
  * add a repeatable holiday that will fall within the overdue period of your
overdue items
  * run fines.pl again
  * the new Amount is the correct fine.  Amount Outstanding is now incorrect,
as it includes the fines on the days that are now holidays.

Have you tried that, and is it now fixed?
Comment 6 Marcel de Rooy 2011-02-18 13:05:37 UTC
(In reply to comment #5)
> This patch will certainly mean the amount can go up and down
> BUT 
> Have you tested with the scenario outlined in the bug
> IE
> Steps to reproduce:
>   * run fines.pl with some overdue items.
>   * set calendaring sysprefs to ensure fines calculations take the calendar
> into account
>   * add a repeatable holiday that will fall within the overdue period of your
> overdue items
>   * run fines.pl again
>   * the new Amount is the correct fine.  Amount Outstanding is now incorrect,
> as it includes the fines on the days that are now holidays.
> Have you tried that, and is it now fixed?

Yes, I tested it. Would not dare to sign without testing ;) Maybe I should have written: outstanding now goes up and down too. My Dutch seems to be better..
Comment 7 Chris Cormack 2011-03-05 19:17:37 UTC
Pushed, please test and mark resolved, bonus points for this fix would have been adding a unit test to demonstrate it works.
Comment 8 Marcel de Rooy 2011-03-07 08:02:37 UTC
Tested and closed.
No unit test, but there was a "test plan"; many reports do not even include such information.