---- 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
Created attachment 2708 [details] [review] Comment one line in Overdues.pm that could cause "amountoutstanding > amount" Commenting line 496 in Overdues.pm seems to fix the "amountoutstanding > amount" problem for me : my $diff = $amount - $data->{'amount'}; #$diff = 0 if ( $data->{amount} > $amount); my $out = $data->{'amountoutstanding'} + $diff; That line prevented the amountoutstanding from being reduced when it normally should have been. I don't know exactly why that line exists, so I'm looking for a second opinion before declaring the problem fixed.
Created attachment 3076 [details] [review] Comment one line in Overdues.pm that could cause "amountoutstanding > amount" (Correct patch formatting) Corrected the patch formatting. Sorry for the inconvenience.
Tested and signed. If amount goes up or down, outstanding should follow in both directions.
Created attachment 3174 [details] [review] Signed patch
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?
(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..
Pushed, please test and mark resolved, bonus points for this fix would have been adding a unit test to demonstrate it works.
Tested and closed. No unit test, but there was a "test plan"; many reports do not even include such information.