From 8a73e8d767ceacd056ba886477646319ebac27fe Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Thu, 16 May 2013 14:02:24 -0400 Subject: [PATCH] Bug 10262 - fine calculation at checkin not respecting CircControl The fines.pl script uses the system preference CircControl to decide what branches circ rules to use for fine generation. Recently, code was added to the returns system to recalculate the fine at checkin time ( to support hourly loans ). The problem is that this code does not respect CircControl. Test Plan: 1) Set circ control to "the library you are logged in at" 2) Set different fines rules for two different librarys 3) Check an item out at library A, backdate the due date so it's overdue and will have fines. 4) Check the item in at library B 5) Observe that the fines should be generated based on library A's rules, but the fines will be based on library B's rules instead! 5) Apply the patch 6) Repeat steps 3 and 4. 7) Observe now that the fines should reflect the fines rules for Library A Note: it seems counter-intuitive for the fines system to behave this way based on the preference being set to "the library you are logged in at" but it does make sense. The rules used are from "the library you are logged in at" when the item is first checked out. If the fines system really did use the rules for the library the item was returned to, it would be easy to exploit the library system. Some Koha using system have branches that charge fines, and others that don't, so a patron could just return any overdue item's to a non-charging branch to avoid ever paying fines! Furthermore, it would mean that the fines.pl script would be using one set of rules to charge fines, and the returns system could possibly be using another. Since fines.pl has been around far longer, it makes sense to assume the fines.pl behavior is canonical. --- C4/Circulation.pm | 37 ++++++++++++++++++++++++++----------- 1 files changed, 26 insertions(+), 11 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 1de40e8..25017b9 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1785,19 +1785,34 @@ sub AddReturn { } if ($borrowernumber) { - if($issue->{'overdue'}){ - my ( $amount, $type, $unitcounttotal ) = C4::Overdues::CalcFine( $item, $borrower->{categorycode},$branch, $datedue, $today ); + if ( $issue->{'overdue'} ) { + my $control = C4::Context->preference('CircControl'); + my $control_branchcode = + ( $control eq 'ItemHomeLibrary' ) ? $item->{homebranch} + : ( $control eq 'PatronLibrary' ) ? $borrower->{branchcode} + : $issue->{branchcode}; + + my ( $amount, $type, $unitcounttotal ) = + C4::Overdues::CalcFine( $item, $borrower->{categorycode}, + $control_branchcode, $datedue, $today ); + $type ||= q{}; - if ( $amount > 0 && ( C4::Context->preference('finesMode') eq 'production' )) { - C4::Overdues::UpdateFine( - $issue->{itemnumber}, - $issue->{borrowernumber}, - $amount, $type, output_pref($datedue) - ); - } + + if ( $amount > 0 + && C4::Context->preference('finesMode') eq 'production' ) + { + C4::Overdues::UpdateFine( $issue->{itemnumber}, + $issue->{borrowernumber}, + $amount, $type, output_pref($datedue) ); + } } - MarkIssueReturned($borrowernumber, $item->{'itemnumber'}, $circControlBranch, '', $borrower->{'privacy'}); - $messages->{'WasReturned'} = 1; # FIXME is the "= 1" right? This could be the borrower hash. + + MarkIssueReturned( $borrowernumber, $item->{'itemnumber'}, + $circControlBranch, '', $borrower->{'privacy'} ); + + # FIXME is the "= 1" right? This could be the borrower hash. + $messages->{'WasReturned'} = 1; + } ModItem({ onloan => undef }, $issue->{'biblionumber'}, $item->{'itemnumber'}); -- 1.7.2.5