Bugzilla – Attachment 18393 Details for
Bug 10262
fine calculation at checkin not respecting CircControl
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 10262 - fine calculation at checkin not respecting CircControl
Bug-10262---fine-calculation-at-checkin-not-respec.patch (text/plain), 4.59 KB, created by
Chris Cormack
on 2013-05-24 20:45:25 UTC
(
hide
)
Description:
Bug 10262 - fine calculation at checkin not respecting CircControl
Filename:
MIME Type:
Creator:
Chris Cormack
Created:
2013-05-24 20:45:25 UTC
Size:
4.59 KB
patch
obsolete
>From 463ab8855a0283be43f4a6bfe74662d69830e549 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >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 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 <mcoalwell@nekls.org> >Signed-off-by: George Williams <georgew@latahlibrary.org> >Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> >--- > C4/Circulation.pm | 37 ++++++++++++++++++++++++++----------- > 1 file changed, 26 insertions(+), 11 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 93bf8e1..30cbe2f 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1785,21 +1785,36 @@ sub AddReturn { > } > > if ($borrowernumber) { >- if( C4::Context->preference('CalculateFinesOnReturn') && $issue->{'overdue'}){ >+ if( C4::Context->preference('CalculateFinesOnReturn') && $issue->{'overdue'}){ > # we only need to calculate and change the fines if we want to do that on return > # Should be on for hourly loans >- my ( $amount, $type, $unitcounttotal ) = C4::Overdues::CalcFine( $item, $borrower->{categorycode},$branch, $datedue, $today ); >+ 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.10.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 10262
:
18172
|
18173
|
18175
|
18322
| 18393