Summary: | fines calculation erroneous when a repeatable holiday is added. | ||
---|---|---|---|
Product: | Koha | Reporter: | Chris Cormack <chris> |
Component: | Circulation | Assignee: | 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: | ||
Circulation function: | |||
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
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. |