From eb6444a1a6cb7200a077311dbd6ce85a6432f7fe Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Thu, 16 May 2013 14:02:24 -0400 Subject: [PATCH] [Signed off] 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 systems have branches that charge fines, and others that don't, so a patron could just return any overdue items 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. Signed-off-by: Mickey Coalwell Signed-off-by: George Williams --- C4/Circulation.pm | 37 ++++++++++++++++++++++++++----------- 1 files changed, 26 insertions(+), 11 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 78cbeab..3d3b699 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