Bug 3341

Summary: fines calculation erroneous when a repeatable holiday is added.
Product: Koha Reporter: Chris Cormack <chris>
Component: CirculationAssignee: Galen Charlton <gmcharlt>
Status: CLOSED FIXED QA Contact:
Severity: major    
Priority: PATCH-Sent (DO NOT USE) CC: cookie.wolfrom, fcapovilla, koha.sekjal, m.de.rooy, mcoalwell, nengard, smoreland
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Comment one line in Overdues.pm that could cause "amountoutstanding > amount"
Comment one line in Overdues.pm that could cause "amountoutstanding > amount" (Correct patch formatting)
Signed patch

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.