projects / koha.git / blobdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | inline | side by side
(bug #4043) fix checkoverdues sqlquery
[koha.git] / C4 / Overdues.pm
diff --git a/C4/Overdues.pm b/C4/Overdues.pm
index 196c641..f67f16a 100644 (file)
--- a/C4/Overdues.pm
+++ b/C4/Overdues.pm
@@ -19,12 +19,13 @@ package C4::Overdues;
 # Suite 330, Boston, MA  02111-1307 USA
 
 use strict;
 # Suite 330, Boston, MA  02111-1307 USA
 
 use strict;
-use Date::Calc qw/Today/;
+use Date::Calc qw/Today Date_to_Days/;
 use Date::Manip qw/UnixDate/;
 use C4::Circulation;
 use C4::Context;
 use C4::Accounts;
 use C4::Log; # logaction
 use Date::Manip qw/UnixDate/;
 use C4::Circulation;
 use C4::Context;
 use C4::Accounts;
 use C4::Log; # logaction
+use C4::Debug;
 
 use vars qw($VERSION @ISA @EXPORT);
 
 
 use vars qw($VERSION @ISA @EXPORT);
 
@@ -100,11 +101,9 @@ overdue items. It is primarily used by the 'misc/fines2.pl' script.
 
 =head1 FUNCTIONS
 
 
 =head1 FUNCTIONS
 
-=over 2
+=head2 Getoverdues
 
 
-=item Getoverdues
-
-  ($overdues) = &Getoverdues();
+  $overdues = Getoverdues( { minimumdays => 1, maximumdays => 30 } );
 
 Returns the list of all overdue books, with their itemtype.
 
 
 Returns the list of all overdue books, with their itemtype.
 
@@ -116,70 +115,72 @@ Koha database.
 
 #'
 sub Getoverdues {
 
 #'
 sub Getoverdues {
+    my $params = shift;
     my $dbh = C4::Context->dbh;
     my $dbh = C4::Context->dbh;
-    my $sth =  (C4::Context->preference('item-level_itypes')) ? 
-                               $dbh->prepare(
-                               "SELECT issues.*,items.itype as itemtype, items.homebranch FROM issues 
-                        LEFT JOIN items USING (itemnumber)
-                        WHERE date_due < now() 
-                        ORDER BY borrowernumber " )
-                               :
-                               $dbh->prepare(
-                    "SELECT issues.*,biblioitems.itemtype,items.itype, items.homebranch  FROM issues 
-                     LEFT JOIN items USING (itemnumber)
-                     LEFT JOIN biblioitems USING (biblioitemnumber)
-                     WHERE date_due < now() 
-                     ORDER BY borrowernumber " );
-    $sth->execute;
-
-    my @results;
-    while ( my $data = $sth->fetchrow_hashref ) {
-        push @results, $data;
+    my $statement;
+    if ( C4::Context->preference('item-level_itypes') ) {
+        $statement = "
+   SELECT issues.*, items.itype as itemtype, items.homebranch, items.barcode
+     FROM issues 
+LEFT JOIN items       USING (itemnumber)
+    WHERE date_due < now() 
+";
+    } else {
+        $statement = "
+   SELECT issues.*, biblioitems.itemtype, items.itype, items.homebranch, items.barcode
+     FROM issues 
+LEFT JOIN items       USING (itemnumber)
+LEFT JOIN biblioitems USING (biblioitemnumber)
+    WHERE date_due < now() 
+";
     }
     }
-    $sth->finish;
 
 
-    return \@results;
+    my @bind_parameters;
+    if ( exists $params->{'minimumdays'} and exists $params->{'maximumdays'} ) {
+        $statement .= ' AND TO_DAYS( NOW() )-TO_DAYS( date_due ) BETWEEN ? and ? ';
+        push @bind_parameters, $params->{'minimumdays'}, $params->{'maximumdays'};
+    } elsif ( exists $params->{'minimumdays'} ) {
+        $statement .= ' AND ( TO_DAYS( NOW() )-TO_DAYS( date_due ) ) > ? ';
+        push @bind_parameters, $params->{'minimumdays'};
+    } elsif ( exists $params->{'maximumdays'} ) {
+        $statement .= ' AND ( TO_DAYS( NOW() )-TO_DAYS( date_due ) ) < ? ';
+        push @bind_parameters, $params->{'maximumdays'};
+    }
+    $statement .= 'ORDER BY borrowernumber';
+    my $sth = $dbh->prepare( $statement );
+    $sth->execute( @bind_parameters );
+    return $sth->fetchall_arrayref({});
 }
 
 }
 
+
 =head2 checkoverdues
 
 =head2 checkoverdues
 
-( $count, $overdueitems )=checkoverdues( $borrowernumber, $dbh );
+($count, $overdueitems) = checkoverdues($borrowernumber);
 
 
-Not exported
+Returns a count and a list of overdueitems for a given borrowernumber
 
 =cut
 
 sub checkoverdues {
 
 =cut
 
 sub checkoverdues {
-
-# From Main.pm, modified to return a list of overdueitems, in addition to a count
-#checks whether a borrower has overdue items
-    my ( $borrowernumber, $dbh ) = @_;
-    my @datearr = localtime;
-    my $today   =
-      ( $datearr[5] + 1900 ) . "-" . ( $datearr[4] + 1 ) . "-" . $datearr[3];
-    my @overdueitems;
-    my $count = 0;
-    my $sth   = $dbh->prepare(
+    my $borrowernumber = shift or return;
+    my $sth = C4::Context->dbh->prepare(
         "SELECT * FROM issues
         "SELECT * FROM issues
-         LEFT JOIN items ON issues.itemnumber      = items.itemnumber
-         LEFT JOIN biblio ON items.biblionumber=biblio.biblionumber
+         LEFT JOIN items       ON issues.itemnumber      = items.itemnumber
+         LEFT JOIN biblio      ON items.biblionumber     = biblio.biblionumber
          LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
             WHERE issues.borrowernumber  = ?
          LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
             WHERE issues.borrowernumber  = ?
-                AND issues.date_due < ?"
+            AND   issues.date_due < CURDATE()"
     );
     );
-    $sth->execute( $borrowernumber, $today );
-    while ( my $data = $sth->fetchrow_hashref ) {
-        push( @overdueitems, $data );
-        $count++;
-    }
-    $sth->finish;
-    return ( $count, \@overdueitems );
+    # FIXME: SELECT * across 4 tables?  do we really need the marc AND marcxml blobs??
+    $sth->execute($borrowernumber);
+    my $results = $sth->fetchall_arrayref({});
+    return ( scalar(@$results), $results);  # returning the count and the results is silly
 }
 
 }
 
-=item CalcFine
+=head2 CalcFine
 
 
-  ($amount, $chargename, $message, $daycounttotal, $daycount) =
-    &CalcFine($itemnumber, $categorycode, $branch, $days_overdue, $description, $start_date, $end_date );
+  ($amount, $chargename, $daycount, $daycounttotal) =
+    &CalcFine($item, $categorycode, $branch, $days_overdue, $description, $start_date, $end_date );
 
 Calculates the fine for a book.
 
 
 Calculates the fine for a book.
 
@@ -190,12 +191,12 @@ members might get a longer grace period between the first and second
 reminders that a book is overdue).
 
 
 reminders that a book is overdue).
 
 
-C<$itemnumber> is the book's item number.
+C<$item> is an item object (hashref).
 
 
-C<$categorycode> is the category code of the patron who currently has
+C<$categorycode> is the category code (string) of the patron who currently has
 the book.
 
 the book.
 
-C<$branchcode> is the library whose issuingrules govern this transaction.
+C<$branchcode> is the library (string) whose issuingrules govern this transaction.
 
 C<$days_overdue> is the number of days elapsed since the book's due date.
   NOTE: supplying days_overdue is deprecated.
 
 C<$days_overdue> is the number of days elapsed since the book's due date.
   NOTE: supplying days_overdue is deprecated.
@@ -207,58 +208,73 @@ but retain these for backwards-comptibility with extant fines scripts.
 
 Fines scripts should just supply the date range over which to calculate the fine.
 
 
 Fines scripts should just supply the date range over which to calculate the fine.
 
-C<&CalcFine> returns a list of four values:
+C<&CalcFine> returns four values:
 
 C<$amount> is the fine owed by the patron (see above).
 
 C<$chargename> is the chargename field from the applicable record in
 the categoryitem table, whatever that is.
 
 C<$amount> is the fine owed by the patron (see above).
 
 C<$chargename> is the chargename field from the applicable record in
 the categoryitem table, whatever that is.
+
+C<$daycount> is the number of days between start and end dates, Calendar adjusted (where needed), 
+minus any applicable grace period.
+
+C<$daycounttotal> is C<$daycount> without consideration of grace period.
+
 FIXME - What is chargename supposed to be ?
 
 FIXME - What is chargename supposed to be ?
 
-C<$message> is a text message, either "First Notice", "Second Notice",
-or "Final Notice".
+FIXME: previously attempted to return C<$message> as a text message, either "First Notice", "Second Notice",
+or "Final Notice".  But CalcFine never defined any value.
 
 =cut
 
 
 =cut
 
-#'
 sub CalcFine {
     my ( $item, $bortype, $branchcode, $difference ,$dues , $start_date, $end_date  ) = @_;
 sub CalcFine {
     my ( $item, $bortype, $branchcode, $difference ,$dues , $start_date, $end_date  ) = @_;
+       $debug and warn sprintf("CalcFine(%s, %s, %s, %s, %s, %s, %s)",
+                       ($item ? '{item}' : 'UNDEF'), 
+                       ($bortype    || 'UNDEF'), 
+                       ($branchcode || 'UNDEF'), 
+                       ($difference || 'UNDEF'), 
+                       ($dues       || 'UNDEF'), 
+                       ($start_date ? ($start_date->output('iso') || 'Not a C4::Dates object') : 'UNDEF'), 
+                       (  $end_date ? (  $end_date->output('iso') || 'Not a C4::Dates object') : 'UNDEF')
+       );
     my $dbh = C4::Context->dbh;
     my $amount = 0;
     my $dbh = C4::Context->dbh;
     my $amount = 0;
-    my $printout;
        my $daystocharge;
        # get issuingrules (fines part will be used)
        my $daystocharge;
        # get issuingrules (fines part will be used)
-    my $data = C4::Circulation::GetIssuingRule($bortype, $item->{'itemtype'},$branchcode);
+    $debug and warn sprintf("CalcFine calling GetIssuingRule(%s, %s, %s)", $bortype, $item->{'itemtype'}, $branchcode);
+    my $data = C4::Circulation::GetIssuingRule($bortype, $item->{'itemtype'}, $branchcode);
        if($difference) {
                # if $difference is supplied, the difference has already been calculated, but we still need to adjust for the calendar.
        # use copy-pasted functions from calendar module.  (deprecated -- these functions will be removed from C4::Overdues ).
        if($difference) {
                # if $difference is supplied, the difference has already been calculated, but we still need to adjust for the calendar.
        # use copy-pasted functions from calendar module.  (deprecated -- these functions will be removed from C4::Overdues ).
-           my $countspecialday=&GetSpecialHolidays($dues,$item->{itemnumber});
-           my $countrepeatableday=&GetRepeatableHolidays($dues,$item->{itemnumber},$difference);    
-           my $countalldayclosed = $countspecialday + $countrepeatableday;
+           my $countspecialday    =    &GetSpecialHolidays($dues,$item->{itemnumber});
+           my $countrepeatableday = &GetRepeatableHolidays($dues,$item->{itemnumber},$difference);    
+           my $countalldayclosed  = $countspecialday + $countrepeatableday;
            $daystocharge = $difference - $countalldayclosed;
        } else {
                # if $difference is not supplied, we have C4::Dates objects giving us the date range, and we use the calendar module.
                if(C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed') {
            $daystocharge = $difference - $countalldayclosed;
        } else {
                # if $difference is not supplied, we have C4::Dates objects giving us the date range, and we use the calendar module.
                if(C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed') {
-                       my $calendar = C4::Calendar->new(  branchcode => $branchcode );
+                       my $calendar = C4::Calendar->new( branchcode => $branchcode );
                        $daystocharge = $calendar->daysBetween( $start_date, $end_date );
                } else {
                        $daystocharge = Date_to_Days(split('-',$end_date->output('iso'))) - Date_to_Days(split('-',$start_date->output('iso')));
                }
        }
        # correct for grace period.
                        $daystocharge = $calendar->daysBetween( $start_date, $end_date );
                } else {
                        $daystocharge = Date_to_Days(split('-',$end_date->output('iso'))) - Date_to_Days(split('-',$start_date->output('iso')));
                }
        }
        # correct for grace period.
-       $daystocharge -= $data->{'firstremind'};
-    if ($data->{'chargeperiod'} > 0 && $daystocharge > 0 ) { 
-        $amount   = int($daystocharge / $data->{'chargeperiod'}) * $data->{'fine'};
+       my $days_minus_grace = $daystocharge - $data->{'firstremind'};
+    if ($data->{'chargeperiod'} > 0 && $days_minus_grace > 0 ) { 
+        $amount = int($days_minus_grace / $data->{'chargeperiod'}) * $data->{'fine'};
     } else {
         # a zero (or null)  chargeperiod means no charge.
     }
     } else {
         # a zero (or null)  chargeperiod means no charge.
     }
-    
-    #  warn "Calc Fine: " . join(", ", ($item->{'itemnumber'}, $bortype, $difference , $data->{'fine'} . " * " . $daycount . " days = \$ " . $amount , "desc: $dues")) ;
-    return ( $amount, $data->{'chargename'}, $printout ,$daystocharge , $daystocharge + $data->{'firstremind'} );
+       $amount = C4::Context->preference('maxFine') if(C4::Context->preference('maxFine') && ( $amount > C4::Context->preference('maxFine')));
+       $debug and warn sprintf("CalcFine returning (%s, %s, %s, %s)", $amount, $data->{'chargename'}, $days_minus_grace, $daystocharge);
+    return ($amount, $data->{'chargename'}, $days_minus_grace, $daystocharge);
+    # FIXME: chargename is NEVER populated anywhere.
 }
 
 
 }
 
 
-=item GetSpecialHolidays
+=head2 GetSpecialHolidays
 
 &GetSpecialHolidays($date_dues,$itemnumber);
 
 
 &GetSpecialHolidays($date_dues,$itemnumber);
 
@@ -271,47 +287,52 @@ C<$itemnumber> is the book's item number.
 =cut
 
 sub GetSpecialHolidays {
 =cut
 
 sub GetSpecialHolidays {
-my ($date_dues,$itemnumber) = @_;
-# calcul the today date
-my $today = join "-", &Today();
-
-# return the holdingbranch
-my $iteminfo=GetIssuesIteminfo($itemnumber);
-# use sql request to find all date between date_due and today
-my $dbh = C4::Context->dbh;
-my $query=qq|SELECT DATE_FORMAT(concat(year,'-',month,'-',day),'%Y-%m-%d')as date 
+    my ( $date_dues, $itemnumber ) = @_;
+
+    # calcul the today date
+    my $today = join "-", &Today();
+
+    # return the holdingbranch
+    my $iteminfo = GetIssuesIteminfo($itemnumber);
+
+    # use sql request to find all date between date_due and today
+    my $dbh = C4::Context->dbh;
+    my $query =
+      qq|SELECT DATE_FORMAT(concat(year,'-',month,'-',day),'%Y-%m-%d') as date
 FROM `special_holidays`
 WHERE DATE_FORMAT(concat(year,'-',month,'-',day),'%Y-%m-%d') >= ?
 AND   DATE_FORMAT(concat(year,'-',month,'-',day),'%Y-%m-%d') <= ?
 AND branchcode=?
 |;
 FROM `special_holidays`
 WHERE DATE_FORMAT(concat(year,'-',month,'-',day),'%Y-%m-%d') >= ?
 AND   DATE_FORMAT(concat(year,'-',month,'-',day),'%Y-%m-%d') <= ?
 AND branchcode=?
 |;
-my @result=GetWdayFromItemnumber($itemnumber);
-my @result_date;
-my $wday;
-my $dateinsec;
-my $sth = $dbh->prepare($query);
-$sth->execute($date_dues,$today,$iteminfo->{'branchcode'});
-
-while ( my $special_date=$sth->fetchrow_hashref){
-    push (@result_date,$special_date);
-}
+    my @result = GetWdayFromItemnumber($itemnumber);
+    my @result_date;
+    my $wday;
+    my $dateinsec;
+    my $sth = $dbh->prepare($query);
+    $sth->execute( $date_dues, $today, $iteminfo->{'branchcode'} )
+      ;    # FIXME: just use NOW() in SQL instead of passing in $today
 
 
-my $specialdaycount=scalar(@result_date);
+    while ( my $special_date = $sth->fetchrow_hashref ) {
+        push( @result_date, $special_date );
+    }
+
+    my $specialdaycount = scalar(@result_date);
 
 
-    for (my $i=0;$i<scalar(@result_date);$i++){
-        $dateinsec=UnixDate($result_date[$i]->{'date'},"%o");
-        (undef,undef,undef,undef,undef,undef,$wday,undef,undef) =localtime($dateinsec);
-        for (my $j=0;$j<scalar(@result);$j++){
-            if ($wday == ($result[$j]->{'weekday'})){
-            $specialdaycount --;
+    for ( my $i = 0 ; $i < scalar(@result_date) ; $i++ ) {
+        $dateinsec = UnixDate( $result_date[$i]->{'date'}, "%o" );
+        ( undef, undef, undef, undef, undef, undef, $wday, undef, undef ) =
+          localtime($dateinsec);
+        for ( my $j = 0 ; $j < scalar(@result) ; $j++ ) {
+            if ( $wday == ( $result[$j]->{'weekday'} ) ) {
+                $specialdaycount--;
             }
         }
     }
 
             }
         }
     }
 
-return $specialdaycount;
+    return $specialdaycount;
 }
 
 }
 
-=item GetRepeatableHolidays
+=head2 GetRepeatableHolidays
 
 &GetRepeatableHolidays($date_dues, $itemnumber, $difference,);
 
 
 &GetRepeatableHolidays($date_dues, $itemnumber, $difference,);
 
@@ -325,31 +346,31 @@ C<$difference> numbers of between day date of the day and date due
 
 =cut
 
 
 =cut
 
-sub GetRepeatableHolidays{
-my ($date_dues,$itemnumber,$difference) = @_;
-my $dateinsec=UnixDate($date_dues,"%o");
-my ($sec,$min,$hour,$mday,$mon,$year,$wday,$yday,$isdst) =localtime($dateinsec);
-my @result=GetWdayFromItemnumber($itemnumber);
-my @dayclosedcount;
-my $j;
-
-for (my $i=0;$i<scalar(@result);$i++){
-    my $k=$wday;
-
-        for ( $j=0;$j<$difference;$j++){
-            if ($result[$i]->{'weekday'} == $k)
-                    {
-                    push ( @dayclosedcount ,$k);
+sub GetRepeatableHolidays {
+    my ( $date_dues, $itemnumber, $difference ) = @_;
+    my $dateinsec = UnixDate( $date_dues, "%o" );
+    my ( $sec, $min, $hour, $mday, $mon, $year, $wday, $yday, $isdst ) =
+      localtime($dateinsec);
+    my @result = GetWdayFromItemnumber($itemnumber);
+    my @dayclosedcount;
+    my $j;
+
+    for ( my $i = 0 ; $i < scalar(@result) ; $i++ ) {
+        my $k = $wday;
+
+        for ( $j = 0 ; $j < $difference ; $j++ ) {
+            if ( $result[$i]->{'weekday'} == $k ) {
+                push( @dayclosedcount, $k );
             }
             }
-        $k++;
-        ($k=0) if($k eq 7);
+            $k++;
+            ( $k = 0 ) if ( $k eq 7 );
         }
     }
         }
     }
-return scalar(@dayclosedcount);
+    return scalar(@dayclosedcount);
 }
 
 
 }
 
 
-=item GetWayFromItemnumber
+=head2 GetWayFromItemnumber
 
 &Getwdayfromitemnumber($itemnumber);
 
 
 &Getwdayfromitemnumber($itemnumber);
 
@@ -359,27 +380,25 @@ C<$itemnumber> is  item number.
 
 =cut
 
 
 =cut
 
-sub GetWdayFromItemnumber{
-my($itemnumber)=@_;
-my $iteminfo=GetIssuesIteminfo($itemnumber);
-my @result;
-my $dbh = C4::Context->dbh;
-my $query = qq|SELECT weekday  
+sub GetWdayFromItemnumber {
+    my ($itemnumber) = @_;
+    my $iteminfo = GetIssuesIteminfo($itemnumber);
+    my @result;
+    my $query = qq|SELECT weekday
     FROM repeatable_holidays
     WHERE branchcode=?
 |;
     FROM repeatable_holidays
     WHERE branchcode=?
 |;
-my $sth = $dbh->prepare($query);
-    #  print $query;
+    my $sth = C4::Context->dbh->prepare($query);
 
 
-$sth->execute($iteminfo->{'branchcode'});
-while ( my $weekday=$sth->fetchrow_hashref){
-    push (@result,$weekday);
+    $sth->execute( $iteminfo->{'branchcode'} );
+    while ( my $weekday = $sth->fetchrow_hashref ) {
+        push( @result, $weekday );
     }
     }
-return @result;
+    return @result;
 }
 
 
 }
 
 
-=item GetIssuesIteminfo
+=head2 GetIssuesIteminfo
 
 &GetIssuesIteminfo($itemnumber);
 
 
 &GetIssuesIteminfo($itemnumber);
 
@@ -389,21 +408,21 @@ C<$itemnumber> is  item number.
 
 =cut
 
 
 =cut
 
-sub GetIssuesIteminfo{
-my($itemnumber)=@_;
-my $dbh = C4::Context->dbh;
-my $query = qq|SELECT *  
+sub GetIssuesIteminfo {
+    my ($itemnumber) = @_;
+    my $dbh          = C4::Context->dbh;
+    my $query        = qq|SELECT *
     FROM issues
     WHERE itemnumber=?
     FROM issues
     WHERE itemnumber=?
-|;
-my $sth = $dbh->prepare($query);
-$sth->execute($itemnumber);
-my ($issuesinfo)=$sth->fetchrow_hashref;
-return $issuesinfo;
+    |;
+    my $sth = $dbh->prepare($query);
+    $sth->execute($itemnumber);
+    my ($issuesinfo) = $sth->fetchrow_hashref;
+    return $issuesinfo;
 }
 
 
 }
 
 
-=item UpdateFine
+=head2 UpdateFine
 
   &UpdateFine($itemnumber, $borrowernumber, $amount, $type, $description);
 
 
   &UpdateFine($itemnumber, $borrowernumber, $amount, $type, $description);
 
@@ -429,13 +448,17 @@ accountlines table of the Koha database.
 
 =cut
 
 
 =cut
 
-#'
-# FIXME - This API doesn't look right: why should the caller have to
+#
+# Question: Why should the caller have to
 # specify both the item number and the borrower number? A book can't
 # be on loan to two different people, so the item number should be
 # sufficient.
 # specify both the item number and the borrower number? A book can't
 # be on loan to two different people, so the item number should be
 # sufficient.
+#
+# Possible Answer: You might update a fine for a damaged item, *after* it is returned.
+#
 sub UpdateFine {
     my ( $itemnum, $borrowernumber, $amount, $type, $due ) = @_;
 sub UpdateFine {
     my ( $itemnum, $borrowernumber, $amount, $type, $due ) = @_;
+       $debug and warn "UpdateFine($itemnum, $borrowernumber, $amount, " . ($type||'""') . ", $due) called";
     my $dbh = C4::Context->dbh;
     # FIXME - What exactly is this query supposed to do? It looks up an
     # entry in accountlines that matches the given item and borrower
     my $dbh = C4::Context->dbh;
     # FIXME - What exactly is this query supposed to do? It looks up an
     # entry in accountlines that matches the given item and borrower
@@ -443,46 +466,61 @@ sub UpdateFine {
     # account type has one of several values, but what does this _mean_?
     # Does it look up existing fines for this item?
     # FIXME - What are these various account types? ("FU", "O", "F", "M")
     # account type has one of several values, but what does this _mean_?
     # Does it look up existing fines for this item?
     # FIXME - What are these various account types? ("FU", "O", "F", "M")
+       #       "L"   is LOST item
+       #   "A"   is Account Management Fee
+       #   "N"   is New Card
+       #   "M"   is Sundry
+       #   "O"   is Overdue ??
+       #   "F"   is Fine ??
+       #   "FU"  is Fine UPDATE??
+       #       "Pay" is Payment
+       #   "REF" is Cash Refund
     my $sth = $dbh->prepare(
     my $sth = $dbh->prepare(
-        "Select * from accountlines where itemnumber=? and
-  borrowernumber=? and (accounttype='FU' or accounttype='O' or
-  accounttype='F' or accounttype='M') and description like ?"
+        "SELECT * FROM accountlines
+               WHERE itemnumber=?
+               AND   borrowernumber=?
+               AND   accounttype IN ('FU','O','F','M')
+               AND   description like ? "
     );
     $sth->execute( $itemnum, $borrowernumber, "%$due%" );
 
     if ( my $data = $sth->fetchrow_hashref ) {
 
     );
     $sth->execute( $itemnum, $borrowernumber, "%$due%" );
 
     if ( my $data = $sth->fetchrow_hashref ) {
 
-               # we're updating an existing fine.
-    if ( $data->{'amount'} != $amount ) {
-           
+               # we're updating an existing fine.  Only modify if we're adding to the charge.
+        # Note that in the current implementation, you cannot pay against an accruing fine
+        # (i.e. , of accounttype 'FU').  Doing so will break accrual.
+       if ( $data->{'amount'} != $amount ) {
             my $diff = $amount - $data->{'amount'};
             my $diff = $amount - $data->{'amount'};
+            $diff = 0 if ( $data->{amount} > $amount);
             my $out  = $data->{'amountoutstanding'} + $diff;
             my $out  = $data->{'amountoutstanding'} + $diff;
-                       my $increment = -1 * $diff;
-            my $sth2 = $dbh->prepare(
-                "UPDATE accountlines SET date=now(), amount=?,
-      amountoutstanding=?, lastincrement=? , accounttype='FU' WHERE
-      borrowernumber=? AND itemnumber=?
-      AND (accounttype='FU' OR accounttype='O') AND description LIKE ?"
-            );
-            $sth2->execute( $amount, $out, $increment, $data->{'borrowernumber'},
-                $data->{'itemnumber'}, "%$due%" );
-            $sth2->finish;
-        }
-        else {
-
+            my $query = "
+                UPDATE accountlines
+                               SET date=now(), amount=?, amountoutstanding=?,
+                                       lastincrement=?, accounttype='FU'
+                               WHERE borrowernumber=?
+                               AND   itemnumber=?
+                               AND   accounttype IN ('FU','O')
+                               AND   description LIKE ?
+                               LIMIT 1 ";
+            my $sth2 = $dbh->prepare($query);
+                       # FIXME: BOGUS query cannot ensure uniqueness w/ LIKE %x% !!!
+                       #               LIMIT 1 added to prevent multiple affected lines
+                       # FIXME: accountlines table needs unique key!! Possibly a combo of borrowernumber and accountline.  
+                       #               But actually, we should just have a regular autoincrementing PK and forget accountline,
+                       #               including the bogus getnextaccountno function (doesn't prevent conflict on simultaneous ops).
+                       # FIXME: Why only 2 account types here?
+                       $debug and print STDERR "UpdateFine query: $query\n" .
+                               "w/ args: $amount, $out, $diff, $data->{'borrowernumber'}, $data->{'itemnumber'}, \"\%$due\%\"\n";
+            $sth2->execute($amount, $out, $diff, $data->{'borrowernumber'}, $data->{'itemnumber'}, "%$due%");
+        } else {
             #      print "no update needed $data->{'amount'}"
         }
             #      print "no update needed $data->{'amount'}"
         }
-    }
-    else {
-
-        # I think this else-clause deals with the case where we're adding
-        # a new fine.
+    } else {
         my $sth4 = $dbh->prepare(
             "SELECT title FROM biblio LEFT JOIN items ON biblio.biblionumber=items.biblionumber WHERE items.itemnumber=?"
         );
         $sth4->execute($itemnum);
         my $sth4 = $dbh->prepare(
             "SELECT title FROM biblio LEFT JOIN items ON biblio.biblionumber=items.biblionumber WHERE items.itemnumber=?"
         );
         $sth4->execute($itemnum);
-        my $title = $sth4->fetchrow_hashref;
-        $sth4->finish;
+        my $title = $sth4->fetchrow;
 
 #         #   print "not in account";
 #         my $sth3 = $dbh->prepare("Select max(accountno) from accountlines");
 
 #         #   print "not in account";
 #         my $sth3 = $dbh->prepare("Select max(accountno) from accountlines");
@@ -493,17 +531,14 @@ sub UpdateFine {
 #         $sth3->finish;
 #         $accountno[0]++;
 # begin transaction
 #         $sth3->finish;
 #         $accountno[0]++;
 # begin transaction
-  my $nextaccntno = C4::Accounts::getnextacctno($borrowernumber);
-    my $sth2 = $dbh->prepare(
-            "INSERT INTO accountlines
-    (borrowernumber,itemnumber,date,amount,
-    description,accounttype,amountoutstanding,lastincrement,accountno) VALUES
-    (?,?,now(),?,?,'FU',?,?,?)"
-        );
-        $sth2->execute( $borrowernumber, $itemnum, $amount,
-            "$type $title->{'title'} $due",
-            $amount,$amount, $nextaccntno);
-        $sth2->finish;
+               my $nextaccntno = C4::Accounts::getnextacctno($borrowernumber);
+               my $desc = ($type ? "$type " : '') . "$title $due";     # FIXEDME, avoid whitespace prefix on empty $type
+               my $query = "INSERT INTO accountlines
+                   (borrowernumber,itemnumber,date,amount,description,accounttype,amountoutstanding,lastincrement,accountno)
+                           VALUES (?,?,now(),?,?,'FU',?,?,?)";
+               my $sth2 = $dbh->prepare($query);
+               $debug and print STDERR "UpdateFine query: $query\nw/ args: $borrowernumber, $itemnum, $amount, $desc, $amount, $amount, $nextaccntno\n";
+        $sth2->execute($borrowernumber, $itemnum, $amount, $desc, $amount, $amount, $nextaccntno);
     }
     # logging action
     &logaction(
     }
     # logging action
     &logaction(
@@ -512,11 +547,9 @@ sub UpdateFine {
         $borrowernumber,
         "due=".$due."  amount=".$amount." itemnumber=".$itemnum
         ) if C4::Context->preference("FinesLog");
         $borrowernumber,
         "due=".$due."  amount=".$amount." itemnumber=".$itemnum
         ) if C4::Context->preference("FinesLog");
-
-    $sth->finish;
 }
 
 }
 
-=item BorType
+=head2 BorType
 
   $borrower = &BorType($borrowernumber);
 
 
   $borrower = &BorType($borrowernumber);
 
@@ -534,17 +567,15 @@ sub BorType {
     my ($borrowernumber) = @_;
     my $dbh              = C4::Context->dbh;
     my $sth              = $dbh->prepare(
     my ($borrowernumber) = @_;
     my $dbh              = C4::Context->dbh;
     my $sth              = $dbh->prepare(
-        "SELECT * from borrowers 
+        "SELECT * from borrowers
       LEFT JOIN categories ON borrowers.categorycode=categories.categorycode 
       WHERE borrowernumber=?"
     );
     $sth->execute($borrowernumber);
       LEFT JOIN categories ON borrowers.categorycode=categories.categorycode 
       WHERE borrowernumber=?"
     );
     $sth->execute($borrowernumber);
-    my $data = $sth->fetchrow_hashref;
-    $sth->finish;
-    return ($data);
+    return $sth->fetchrow_hashref;
 }
 
 }
 
-=item ReplacementCost
+=head2 ReplacementCost
 
   $cost = &ReplacementCost($itemnumber);
 
 
   $cost = &ReplacementCost($itemnumber);
 
@@ -560,13 +591,12 @@ sub ReplacementCost {
       $dbh->prepare("Select replacementprice from items where itemnumber=?");
     $sth->execute($itemnum);
 
       $dbh->prepare("Select replacementprice from items where itemnumber=?");
     $sth->execute($itemnum);
 
-    # FIXME - Use fetchrow_array or something.
+    # FIXME - Use fetchrow_array or a slice.
     my $data = $sth->fetchrow_hashref;
     my $data = $sth->fetchrow_hashref;
-    $sth->finish;
     return ( $data->{'replacementprice'} );
 }
 
     return ( $data->{'replacementprice'} );
 }
 
-=item GetFine
+=head2 GetFine
 
 $data->{'sum(amountoutstanding)'} = &GetFine($itemnum,$borrowernumber);
 
 
 $data->{'sum(amountoutstanding)'} = &GetFine($itemnum,$borrowernumber);
 
@@ -582,21 +612,17 @@ C<$borrowernumber> is the borrowernumber
 sub GetFine {
     my ( $itemnum, $borrowernumber ) = @_;
     my $dbh   = C4::Context->dbh();
 sub GetFine {
     my ( $itemnum, $borrowernumber ) = @_;
     my $dbh   = C4::Context->dbh();
-    my $query = "SELECT sum(amountoutstanding) FROM accountlines 
+    my $query = "SELECT sum(amountoutstanding) FROM accountlines
     where accounttype like 'F%'  
   AND amountoutstanding > 0 AND itemnumber = ? AND borrowernumber=?";
     my $sth = $dbh->prepare($query);
     $sth->execute( $itemnum, $borrowernumber );
     my $data = $sth->fetchrow_hashref();
     where accounttype like 'F%'  
   AND amountoutstanding > 0 AND itemnumber = ? AND borrowernumber=?";
     my $sth = $dbh->prepare($query);
     $sth->execute( $itemnum, $borrowernumber );
     my $data = $sth->fetchrow_hashref();
-    $sth->finish();
-    $dbh->disconnect();
     return ( $data->{'sum(amountoutstanding)'} );
 }
 
 
     return ( $data->{'sum(amountoutstanding)'} );
 }
 
 
-
-
-=item GetIssuingRules
+=head2 GetIssuingRules
 
 FIXME - This sub should be deprecated and removed.
 It ignores branch and defaults.
 
 FIXME - This sub should be deprecated and removed.
 It ignores branch and defaults.
@@ -615,9 +641,10 @@ category he or she belongs to.
 =cut 
 
 sub GetIssuingRules {
 =cut 
 
 sub GetIssuingRules {
+       warn "GetIssuingRules is deprecated: use GetIssuingRule from C4::Circulation instead.";
    my ($itemtype,$categorycode)=@_;
    my $dbh   = C4::Context->dbh();    
    my ($itemtype,$categorycode)=@_;
    my $dbh   = C4::Context->dbh();    
-   my $query=qq|SELECT * 
+   my $query=qq|SELECT *
         FROM issuingrules
         WHERE issuingrules.itemtype=?
             AND issuingrules.categorycode=?
         FROM issuingrules
         WHERE issuingrules.itemtype=?
             AND issuingrules.categorycode=?
@@ -625,17 +652,14 @@ sub GetIssuingRules {
     my $sth = $dbh->prepare($query);
     #  print $query;
     $sth->execute($itemtype,$categorycode);
     my $sth = $dbh->prepare($query);
     #  print $query;
     $sth->execute($itemtype,$categorycode);
-    my ($data) = $sth->fetchrow_hashref;
-   $sth->finish;
-return ($data);
-
+    return $sth->fetchrow_hashref;
 }
 
 
 sub ReplacementCost2 {
     my ( $itemnum, $borrowernumber ) = @_;
     my $dbh   = C4::Context->dbh();
 }
 
 
 sub ReplacementCost2 {
     my ( $itemnum, $borrowernumber ) = @_;
     my $dbh   = C4::Context->dbh();
-    my $query = "SELECT amountoutstanding 
+    my $query = "SELECT amountoutstanding
          FROM accountlines
              WHERE accounttype like 'L'
          AND amountoutstanding > 0
          FROM accountlines
              WHERE accounttype like 'L'
          AND amountoutstanding > 0
@@ -644,13 +668,11 @@ sub ReplacementCost2 {
     my $sth = $dbh->prepare($query);
     $sth->execute( $itemnum, $borrowernumber );
     my $data = $sth->fetchrow_hashref();
     my $sth = $dbh->prepare($query);
     $sth->execute( $itemnum, $borrowernumber );
     my $data = $sth->fetchrow_hashref();
-    $sth->finish();
-    $dbh->disconnect();
     return ( $data->{'amountoutstanding'} );
 }
 
 
     return ( $data->{'amountoutstanding'} );
 }
 
 
-=item GetNextIdNotify
+=head2 GetNextIdNotify
 
 ($result) = &GetNextIdNotify($reference);
 
 
 ($result) = &GetNextIdNotify($reference);
 
@@ -662,38 +684,34 @@ C<$reference> contains the beggining of file number
 
 =cut
 
 
 =cut
 
-
-
 sub GetNextIdNotify {
 sub GetNextIdNotify {
-my ($reference)=@_;
-my $query=qq|SELECT max(notify_id) 
+    my ($reference) = @_;
+    my $query = qq|SELECT max(notify_id)
          FROM accountlines
          WHERE notify_id  like \"$reference%\"
          |;
          FROM accountlines
          WHERE notify_id  like \"$reference%\"
          |;
-# AND borrowernumber=?|;   
-my $dbh = C4::Context->dbh;
-my $sth=$dbh->prepare($query);
-$sth->execute();
-my $result=$sth->fetchrow;
-$sth->finish;
-my $count;
-    if ($result eq '')
-    {
-    ($result=$reference."01")  ;
-    }else
-    {
-    $count=substr($result,6)+1;
-     
-    if($count<10){
-     ($count = "0".$count);
-     }
-     $result=$reference.$count;
-     }
-return $result;
-}
 
 
+    # AND borrowernumber=?|;
+    my $dbh = C4::Context->dbh;
+    my $sth = $dbh->prepare($query);
+    $sth->execute();
+    my $result = $sth->fetchrow;
+    my $count;
+    if ( $result eq '' ) {
+        ( $result = $reference . "01" );
+    }
+    else {
+        $count = substr( $result, 6 ) + 1;
+
+        if ( $count < 10 ) {
+            ( $count = "0" . $count );
+        }
+        $result = $reference . $count;
+    }
+    return $result;
+}
 
 
-=item NumberNotifyId
+=head2 NumberNotifyId
 
 (@notify) = &NumberNotifyId($borrowernumber);
 
 
 (@notify) = &NumberNotifyId($borrowernumber);
 
@@ -711,18 +729,15 @@ sub NumberNotifyId{
             FROM accountlines
             WHERE borrowernumber=?|;
     my @notify;
             FROM accountlines
             WHERE borrowernumber=?|;
     my @notify;
-    my $sth=$dbh->prepare($query);
-        $sth->execute($borrowernumber);
-          while ( my ($numberofnotify)=$sth->fetchrow){
-    push (@notify,$numberofnotify);
+    my $sth = $dbh->prepare($query);
+    $sth->execute($borrowernumber);
+    while ( my ($numberofnotify) = $sth->fetchrow ) {
+        push( @notify, $numberofnotify );
     }
     }
-    $sth->finish;
-
     return (@notify);
     return (@notify);
-
 }
 
 }
 
-=item AmountNotify
+=head2 AmountNotify
 
 ($totalnotify) = &AmountNotify($notifyid);
 
 
 ($totalnotify) = &AmountNotify($notifyid);
 
@@ -749,7 +764,7 @@ sub AmountNotify{
 }
 
 
 }
 
 
-=item GetNotifyId
+=head2 GetNotifyId
 
 ($notify_id) = &GetNotifyId($borrowernumber,$itemnumber);
 
 
 ($notify_id) = &GetNotifyId($borrowernumber,$itemnumber);
 
@@ -764,23 +779,22 @@ C<$notify_id> contains the file number for the borrower number nad item number
 
 =cut
 
 
 =cut
 
- sub GetNotifyId {
- my ($borrowernumber,$itemnumber)=@_;
- my $query=qq|SELECT notify_id 
+sub GetNotifyId {
+    my ( $borrowernumber, $itemnumber ) = @_;
+    my $query = qq|SELECT notify_id
            FROM accountlines
            WHERE borrowernumber=?
           AND itemnumber=?
            AND (accounttype='FU' or accounttype='O')|;
            FROM accountlines
            WHERE borrowernumber=?
           AND itemnumber=?
            AND (accounttype='FU' or accounttype='O')|;
- my $dbh = C4::Context->dbh;
- my $sth=$dbh->prepare($query);
- $sth->execute($borrowernumber,$itemnumber);
- my ($notify_id)=$sth->fetchrow;
- $sth->finish;
- return ($notify_id);
-
- }
+    my $dbh = C4::Context->dbh;
+    my $sth = $dbh->prepare($query);
+    $sth->execute( $borrowernumber, $itemnumber );
+    my ($notify_id) = $sth->fetchrow;
+    $sth->finish;
+    return ($notify_id);
+}
 
 
-=item CreateItemAccountLine
+=head2 CreateItemAccountLine
 
 () = &CreateItemAccountLine($borrowernumber,$itemnumber,$date,$amount,$description,$accounttype,$amountoutstanding,$timestamp,$notify_id,$level);
 
 
 () = &CreateItemAccountLine($borrowernumber,$itemnumber,$date,$amount,$description,$accounttype,$amountoutstanding,$timestamp,$notify_id,$level);
 
@@ -809,25 +823,31 @@ C<$notify_id> contains the file number
 
 C<$level> contains the file level
 
 
 C<$level> contains the file level
 
-
 =cut
 
 =cut
 
- sub CreateItemAccountLine {
-  my ($borrowernumber,$itemnumber,$date,$amount,$description,$accounttype,$amountoutstanding,$timestamp,$notify_id,$level)=@_;
-  my $dbh = C4::Context->dbh;
-  my $nextaccntno = C4::Accounts::getnextacctno($borrowernumber);
-   my $query= "INSERT into accountlines  
+sub CreateItemAccountLine {
+    my (
+        $borrowernumber, $itemnumber,  $date,              $amount,
+        $description,    $accounttype, $amountoutstanding, $timestamp,
+        $notify_id,      $level
+    ) = @_;
+    my $dbh         = C4::Context->dbh;
+    my $nextaccntno = C4::Accounts::getnextacctno($borrowernumber);
+    my $query       = "INSERT into accountlines
          (borrowernumber,accountno,itemnumber,date,amount,description,accounttype,amountoutstanding,timestamp,notify_id,notify_level)
           VALUES
              (?,?,?,?,?,?,?,?,?,?,?)";
          (borrowernumber,accountno,itemnumber,date,amount,description,accounttype,amountoutstanding,timestamp,notify_id,notify_level)
           VALUES
              (?,?,?,?,?,?,?,?,?,?,?)";
-  
-  
-  my $sth=$dbh->prepare($query);
-  $sth->execute($borrowernumber,$nextaccntno,$itemnumber,$date,$amount,$description,$accounttype,$amountoutstanding,$timestamp,$notify_id,$level);
-  $sth->finish;
- }
 
 
-=item UpdateAccountLines
+    my $sth = $dbh->prepare($query);
+    $sth->execute(
+        $borrowernumber, $nextaccntno,       $itemnumber,
+        $date,           $amount,            $description,
+        $accounttype,    $amountoutstanding, $timestamp,
+        $notify_id,      $level
+    );
+}
+
+=head2 UpdateAccountLines
 
 () = &UpdateAccountLines($notify_id,$notify_level,$borrowernumber,$itemnumber);
 
 
 () = &UpdateAccountLines($notify_id,$notify_level,$borrowernumber,$itemnumber);
 
@@ -847,39 +867,30 @@ C<$borrowernumber> contains the borrowernumber
 =cut
 
 sub UpdateAccountLines {
 =cut
 
 sub UpdateAccountLines {
-my ($notify_id,$notify_level,$borrowernumber,$itemnumber)=@_;
-my $query;
-if ($notify_id eq '')
-{
-
-    $query=qq|UPDATE accountlines
+    my ( $notify_id, $notify_level, $borrowernumber, $itemnumber ) = @_;
+    my $query;
+    if ( $notify_id eq '' ) {
+        $query = qq|UPDATE accountlines
     SET  notify_level=?
     WHERE borrowernumber=? AND itemnumber=?
     AND (accounttype='FU' or accounttype='O')|;
     SET  notify_level=?
     WHERE borrowernumber=? AND itemnumber=?
     AND (accounttype='FU' or accounttype='O')|;
-}else
-{
-    $query=qq|UPDATE accountlines
+    } else {
+        $query = qq|UPDATE accountlines
      SET notify_id=?, notify_level=?
      SET notify_id=?, notify_level=?
-           WHERE borrowernumber=?
+   WHERE borrowernumber=?
     AND itemnumber=?
     AND itemnumber=?
-        AND (accounttype='FU' or accounttype='O')|;
-}
- my $dbh = C4::Context->dbh;
- my $sth=$dbh->prepare($query);
-
-if ($notify_id eq '')
-{
-    $sth->execute($notify_level,$borrowernumber,$itemnumber);
-}else
-{
-    $sth->execute($notify_id,$notify_level,$borrowernumber,$itemnumber);
-}
- $sth->finish;
+    AND (accounttype='FU' or accounttype='O')|;
+    }
 
 
+    my $sth = C4::Context->dbh->prepare($query);
+    if ( $notify_id eq '' ) {
+        $sth->execute( $notify_level, $borrowernumber, $itemnumber );
+    } else {
+        $sth->execute( $notify_id, $notify_level, $borrowernumber, $itemnumber );
+    }
 }
 
 }
 
-
-=item GetItems
+=head2 GetItems
 
 ($items) = &GetItems($itemnumber);
 
 
 ($items) = &GetItems($itemnumber);
 
@@ -892,20 +903,23 @@ C<$itemnumber> contains the borrower categorycode
 
 =cut
 
 
 =cut
 
+# FIXME: This is a bad function to have here.
+# Shouldn't it be in C4::Items?
+# Shouldn't it be called GetItem since you only get 1 row?
+# Shouldn't it be called GetItem since you give it only 1 itemnumber?
+
 sub GetItems {
 sub GetItems {
-    my($itemnumber) = @_;
-    my $query=qq|SELECT *
+    my $itemnumber = shift or return;
+    my $query = qq|SELECT *
              FROM items
               WHERE itemnumber=?|;
              FROM items
               WHERE itemnumber=?|;
-        my $dbh = C4::Context->dbh;
-        my $sth=$dbh->prepare($query);
-        $sth->execute($itemnumber);
-        my ($items)=$sth->fetchrow_hashref;
-        $sth->finish;
-    return($items);
+    my $sth = C4::Context->dbh->prepare($query);
+    $sth->execute($itemnumber);
+    my ($items) = $sth->fetchrow_hashref;
+    return ($items);
 }
 
 }
 
-=item GetOverdueDelays
+=head2 GetOverdueDelays
 
 (@delays) = &GetOverdueDelays($categorycode);
 
 
 (@delays) = &GetOverdueDelays($categorycode);
 
@@ -918,19 +932,49 @@ C<$categorycode> contains the borrower categorycode
 =cut
 
 sub GetOverdueDelays {
 =cut
 
 sub GetOverdueDelays {
-    my($category) = @_;
-    my $dbh = C4::Context->dbh;
-        my $query=qq|SELECT delay1,delay2,delay3
+    my ($category) = @_;
+    my $query      = qq|SELECT delay1,delay2,delay3
                 FROM overduerules
                 WHERE categorycode=?|;
                 FROM overduerules
                 WHERE categorycode=?|;
-    my $sth=$dbh->prepare($query);
-        $sth->execute($category);
-        my (@delays)=$sth->fetchrow_array;
-        $sth->finish;
-        return(@delays);
+    my $sth = C4::Context->dbh->prepare($query);
+    $sth->execute($category);
+    my (@delays) = $sth->fetchrow_array;
+    return (@delays);
+}
+
+=head2 GetBranchcodesWithOverdueRules
+
+=over 4
+
+my @branchcodes = C4::Overdues::GetBranchcodesWithOverdueRules()
+
+returns a list of branch codes for branches with overdue rules defined.
+
+=back
+
+=cut
+
+sub GetBranchcodesWithOverdueRules {
+    my $dbh               = C4::Context->dbh;
+    my $rqoverduebranches = $dbh->prepare("SELECT DISTINCT branchcode FROM overduerules WHERE delay1 IS NOT NULL AND branchcode <> ''");
+    my $availbranches = C4::Branch::GetBranches();
+    
+    $rqoverduebranches->execute;
+    my @branches = map { shift @$_ } @{ $rqoverduebranches->fetchall_arrayref };
+    
+    my $defaultbranches = $dbh->prepare("SELECT DISTINCT branchcode FROM overduerules WHERE delay1 IS NOT NULL AND branchcode = ''");
+    $defaultbranches->execute();
+    if($defaultbranches->rows > 0){
+        foreach my $branch (keys %$availbranches){
+            if(not grep{/^$branch$/} @branches){
+                push @branches, $branch;
+            }
+        }
+    }
+    return @branches;
 }
 
 }
 
-=item CheckAccountLineLevelInfo
+=head2 CheckAccountLineLevelInfo
 
 ($exist) = &CheckAccountLineLevelInfo($borrowernumber,$itemnumber,$accounttype,notify_level);
 
 
 ($exist) = &CheckAccountLineLevelInfo($borrowernumber,$itemnumber,$accounttype,notify_level);
 
@@ -951,21 +995,20 @@ C<$notify_level> contains the accountline level
 =cut
 
 sub CheckAccountLineLevelInfo {
 =cut
 
 sub CheckAccountLineLevelInfo {
-    my($borrowernumber,$itemnumber,$level) = @_;
-    my $dbh = C4::Context->dbh;
-        my $query=    qq|SELECT count(*)
+    my ( $borrowernumber, $itemnumber, $level ) = @_;
+    my $dbh   = C4::Context->dbh;
+    my $query = qq|SELECT count(*)
             FROM accountlines
             WHERE borrowernumber =?
             AND itemnumber = ?
             AND notify_level=?|;
             FROM accountlines
             WHERE borrowernumber =?
             AND itemnumber = ?
             AND notify_level=?|;
-    my $sth=$dbh->prepare($query);
-        $sth->execute($borrowernumber,$itemnumber,$level);
-        my ($exist)=$sth->fetchrow;
-        $sth->finish;
-        return($exist);
+    my $sth = $dbh->prepare($query);
+    $sth->execute( $borrowernumber, $itemnumber, $level );
+    my ($exist) = $sth->fetchrow;
+    return ($exist);
 }
 
 }
 
-=item GetOverduerules
+=head2 GetOverduerules
 
 ($overduerules) = &GetOverduerules($categorycode);
 
 
 ($overduerules) = &GetOverduerules($categorycode);
 
@@ -976,24 +1019,23 @@ C<$overduerules> return value of debbraed field in overduerules table
 C<$category> contains the borrower categorycode
 
 C<$notify_level> contains the notify level
 C<$category> contains the borrower categorycode
 
 C<$notify_level> contains the notify level
-=cut
 
 
+=cut
 
 
-sub GetOverduerules{
-    my($category,$notify_level) = @_;
-    my $dbh = C4::Context->dbh;
-        my $query=qq|SELECT debarred$notify_level
-             FROM overduerules
-             WHERE categorycode=?|;
-    my $sth=$dbh->prepare($query);
-        $sth->execute($category);
-        my ($overduerules)=$sth->fetchrow;
-        $sth->finish;
-        return($overduerules);
+sub GetOverduerules {
+    my ( $category, $notify_level ) = @_;
+    my $dbh   = C4::Context->dbh;
+    my $query = qq|SELECT debarred$notify_level
+                     FROM overduerules
+                    WHERE categorycode=?|;
+    my $sth = $dbh->prepare($query);
+    $sth->execute($category);
+    my ($overduerules) = $sth->fetchrow;
+    return ($overduerules);
 }
 
 
 }
 
 
-=item CheckBorrowerDebarred
+=head2 CheckBorrowerDebarred
 
 ($debarredstatus) = &CheckBorrowerDebarred($borrowernumber);
 
 
 ($debarredstatus) = &CheckBorrowerDebarred($borrowernumber);
 
@@ -1005,26 +1047,22 @@ C<$borrowernumber> contains the borrower number
 
 =cut
 
 
 =cut
 
-
-sub CheckBorrowerDebarred{
-    my($borrowernumber) = @_;
-    my $dbh = C4::Context->dbh;
-        my $query=qq|SELECT debarred
-              FROM borrowers
-             WHERE borrowernumber=?
-            |;
-    my $sth=$dbh->prepare($query);
-        $sth->execute($borrowernumber);
-        my ($debarredstatus)=$sth->fetchrow;
-        $sth->finish;
-        if ($debarredstatus eq '1'){
-    return(1);}
-    else{
-    return(0);
-    }
+# FIXME: Shouldn't this be in C4::Members?
+sub CheckBorrowerDebarred {
+    my ($borrowernumber) = @_;
+    my $dbh   = C4::Context->dbh;
+    my $query = qq|
+        SELECT debarred
+        FROM borrowers
+        WHERE borrowernumber=?
+    |;
+    my $sth = $dbh->prepare($query);
+    $sth->execute($borrowernumber);
+    my ($debarredstatus) = $sth->fetchrow;
+    return ( $debarredstatus eq '1' ? 1 : 0 );
 }
 
 }
 
-=item UpdateBorrowerDebarred
+=head2 UpdateBorrowerDebarred
 
 ($borrowerstatut) = &UpdateBorrowerDebarred($borrowernumber);
 
 
 ($borrowerstatut) = &UpdateBorrowerDebarred($borrowernumber);
 
@@ -1047,7 +1085,7 @@ sub UpdateBorrowerDebarred{
         return 1;
 }
 
         return 1;
 }
 
-=item CheckExistantNotifyid
+=head2 CheckExistantNotifyid
 
   ($exist) = &CheckExistantNotifyid($borrowernumber,$itemnumber,$accounttype,$notify_id);
 
 
   ($exist) = &CheckExistantNotifyid($borrowernumber,$itemnumber,$accounttype,$notify_id);
 
@@ -1063,26 +1101,18 @@ C<$date_due> contains the date of item return
 =cut
 
 sub CheckExistantNotifyid {
 =cut
 
 sub CheckExistantNotifyid {
-     my($borrowernumber,$date_due) = @_;
-     my $dbh = C4::Context->dbh;
-         my $query =  qq|SELECT notify_id FROM accountlines 
+    my ( $borrowernumber, $date_due ) = @_;
+    my $dbh   = C4::Context->dbh;
+    my $query = qq|SELECT notify_id FROM accountlines
              LEFT JOIN issues ON issues.itemnumber= accountlines.itemnumber
              WHERE accountlines.borrowernumber =?
               AND date_due = ?|;
              LEFT JOIN issues ON issues.itemnumber= accountlines.itemnumber
              WHERE accountlines.borrowernumber =?
               AND date_due = ?|;
-    my $sth=$dbh->prepare($query);
-         $sth->execute($borrowernumber,$date_due);
-         my ($exist)=$sth->fetchrow;
-         $sth->finish;
-         if ($exist eq '')
-    {
-    return(0);
-    }else
-        {
-    return($exist);
-    }
+    my $sth = $dbh->prepare($query);
+    $sth->execute( $borrowernumber, $date_due );
+    return $sth->fetchrow || 0;
 }
 
 }
 
-=item CheckAccountLineItemInfo
+=head2 CheckAccountLineItemInfo
 
   ($exist) = &CheckAccountLineItemInfo($borrowernumber,$itemnumber,$accounttype,$notify_id);
 
 
   ($exist) = &CheckAccountLineItemInfo($borrowernumber,$itemnumber,$accounttype,$notify_id);
 
@@ -1102,19 +1132,18 @@ C<$notify_id> contains the file number
 =cut
 
 sub CheckAccountLineItemInfo {
 =cut
 
 sub CheckAccountLineItemInfo {
-     my($borrowernumber,$itemnumber,$accounttype,$notify_id) = @_;
-     my $dbh = C4::Context->dbh;
-         my $query =  qq|SELECT count(*) FROM accountlines
+    my ( $borrowernumber, $itemnumber, $accounttype, $notify_id ) = @_;
+    my $dbh   = C4::Context->dbh;
+    my $query = qq|SELECT count(*) FROM accountlines
              WHERE borrowernumber =?
              AND itemnumber = ?
               AND accounttype= ?
             AND notify_id = ?|;
              WHERE borrowernumber =?
              AND itemnumber = ?
               AND accounttype= ?
             AND notify_id = ?|;
-    my $sth=$dbh->prepare($query);
-         $sth->execute($borrowernumber,$itemnumber,$accounttype,$notify_id);
-         my ($exist)=$sth->fetchrow;
-         $sth->finish;
-         return($exist);
- }
+    my $sth = $dbh->prepare($query);
+    $sth->execute( $borrowernumber, $itemnumber, $accounttype, $notify_id );
+    my ($exist) = $sth->fetchrow;
+    return ($exist);
+}
 
 =head2 CheckItemNotify
 
 
 =head2 CheckItemNotify
 
@@ -1124,17 +1153,17 @@ this function is not exported, only used with GetOverduesForBranch
 =cut
 
 sub CheckItemNotify {
 =cut
 
 sub CheckItemNotify {
-       my ($notify_id,$notify_level,$itemnumber) = @_;
-       my $dbh = C4::Context->dbh;
-       my $sth = $dbh->prepare("
-         SELECT COUNT(*) FROM notifys
- WHERE notify_id  = ?
- AND notify_level  = ? 
-  AND  itemnumber  =  ? ");
- $sth->execute($notify_id,$notify_level,$itemnumber);
-       my $notified = $sth->fetchrow;
-$sth->finish;
-return ($notified);
+    my ($notify_id,$notify_level,$itemnumber) = @_;
+    my $dbh = C4::Context->dbh;
+    my $sth = $dbh->prepare("
+    SELECT COUNT(*)
+     FROM notifys
+    WHERE notify_id    = ?
+     AND  notify_level = ? 
+     AND  itemnumber   = ? ");
+    $sth->execute($notify_id,$notify_level,$itemnumber);
+    my $notified = $sth->fetchrow;
+    return ($notified);
 }
 
 =head2 GetOverduesForBranch
 }
 
 =head2 GetOverduesForBranch
@@ -1143,112 +1172,68 @@ Sql request for display all information for branchoverdues.pl
 2 possibilities : with or without location .
 display is filtered by branch
 
 2 possibilities : with or without location .
 display is filtered by branch
 
+FIXME: This function should be renamed.
+
 =cut
 
 sub GetOverduesForBranch {
     my ( $branch, $location) = @_;
        my $itype_link =  (C4::Context->preference('item-level_itypes')) ?  " items.itype " :  " biblioitems.itemtype ";
 =cut
 
 sub GetOverduesForBranch {
     my ( $branch, $location) = @_;
        my $itype_link =  (C4::Context->preference('item-level_itypes')) ?  " items.itype " :  " biblioitems.itemtype ";
-    if ( not $location ) {
-        my $dbh = C4::Context->dbh;
-        my $sth = $dbh->prepare("
-            SELECT 
-                borrowers.surname,
-                borrowers.firstname,
-                biblio.title,
-                itemtypes.description,
-                issues.date_due,
-                issues.returndate,
-                branches.branchname,
+    my $dbh = C4::Context->dbh;
+    my $select = "
+    SELECT
+            borrowers.borrowernumber,
+            borrowers.surname,
+            borrowers.firstname,
+            borrowers.phone,
+            borrowers.email,
+               biblio.title,
+               biblio.biblionumber,
+               issues.date_due,
+               issues.returndate,
+               issues.branchcode,
+             branches.branchname,
                 items.barcode,
                 items.barcode,
-                borrowers.phone,
-                borrowers.email,
                 items.itemcallnumber,
                 items.itemcallnumber,
-                borrowers.borrowernumber,
-                items.itemnumber,
-                biblio.biblionumber,
-                issues.branchcode,
-                accountlines.notify_id,
-                accountlines.notify_level,
                 items.location,
                 items.location,
-                accountlines.amountoutstanding
-            FROM  accountlines
-            LEFT JOIN issues ON issues.itemnumber = accountlines.itemnumber AND issues.borrowernumber = accountlines.borrowernumber
-            LEFT JOIN borrowers ON borrowers.borrowernumber = accountlines.borrowernumber
-            LEFT JOIN items ON items.itemnumber = issues.itemnumber
-            LEFT JOIN biblio ON biblio.biblionumber = items.biblionumber
-            LEFT JOIN biblioitems ON biblioitems.biblioitemnumber=items.biblioitemnumber
-            LEFT JOIN itemtypes ON itemtypes.itemtype = $itype_link
-            LEFT JOIN branches ON branches.branchcode = issues.branchcode
-            WHERE ( accountlines.amountoutstanding  != '0.000000')
-              AND ( accountlines.accounttype  = 'FU')
-              AND (issues.branchcode = ?)
-              AND (issues.date_due <= NOW())
-            ORDER BY  borrowers.surname
-        ");
-       $sth->execute($branch);
-        my @getoverdues;
-        my $i = 0;
-        while ( my $data = $sth->fetchrow_hashref ) {
-       #check if the document has already been notified
-       my $countnotify = CheckItemNotify($data->{'notify_id'},$data->{'notify_level'},$data->{'itemnumber'});
-       if ($countnotify eq '0'){
+                items.itemnumber,
+            itemtypes.description,
+         accountlines.notify_id,
+         accountlines.notify_level,
+         accountlines.amountoutstanding
+    FROM  accountlines
+    LEFT JOIN issues      ON    issues.itemnumber     = accountlines.itemnumber
+                          AND   issues.borrowernumber = accountlines.borrowernumber
+    LEFT JOIN borrowers   ON borrowers.borrowernumber = accountlines.borrowernumber
+    LEFT JOIN items       ON     items.itemnumber     = issues.itemnumber
+    LEFT JOIN biblio      ON      biblio.biblionumber =  items.biblionumber
+    LEFT JOIN biblioitems ON biblioitems.biblioitemnumber = items.biblioitemnumber
+    LEFT JOIN itemtypes   ON itemtypes.itemtype       = $itype_link
+    LEFT JOIN branches    ON  branches.branchcode     = issues.branchcode
+    WHERE (accountlines.amountoutstanding  != '0.000000')
+      AND (accountlines.accounttype         = 'FU'      )
+      AND (issues.branchcode =  ?   )
+      AND (issues.date_due  <= NOW())
+    ";
+    my @getoverdues;
+    my $i = 0;
+    my $sth;
+    if ($location) {
+        $sth = $dbh->prepare("$select AND items.location = ? ORDER BY borrowers.surname, borrowers.firstname");
+        $sth->execute($branch, $location);
+    } else {
+        $sth = $dbh->prepare("$select ORDER BY borrowers.surname, borrowers.firstname");
+        $sth->execute($branch);
+    }
+    while ( my $data = $sth->fetchrow_hashref ) {
+    #check if the document has already been notified
+        my $countnotify = CheckItemNotify($data->{'notify_id'}, $data->{'notify_level'}, $data->{'itemnumber'});
+        if ($countnotify eq '0') {
             $getoverdues[$i] = $data;
             $i++;
             $getoverdues[$i] = $data;
             $i++;
-        }
-        }
-        return (@getoverdues);
-       $sth->finish;
-    }
-    else {
-        my $dbh = C4::Context->dbh;
-        my $sth = $dbh->prepare( "
-            SELECT  borrowers.surname,
-                    borrowers.firstname,
-                    biblio.title,
-                    itemtypes.description,
-                    issues.date_due,
-                    issues.returndate,
-                    branches.branchname,
-                    items.barcode,
-                    borrowers.phone,
-                    borrowers.email,
-                    items.itemcallnumber,
-                    borrowers.borrowernumber,
-                    items.itemnumber,
-                    biblio.biblionumber,
-                    issues.branchcode,
-                    accountlines.notify_id,
-                    accountlines.notify_level,
-                    items.location,
-                    accountlines.amountoutstanding
-            FROM  accountlines
-            LEFT JOIN issues ON issues.itemnumber = accountlines.itemnumber AND issues.borrowernumber = accountlines.borrowernumber
-            LEFT JOIN borrowers ON borrowers.borrowernumber = accountlines.borrowernumber
-            LEFT JOIN items ON items.itemnumber = issues.itemnumber
-            LEFT JOIN biblio ON biblio.biblionumber = items.biblionumber
-            LEFT JOIN biblioitems ON biblioitems.biblioitemnumber=items.biblioitemnumber
-            LEFT JOIN itemtypes ON itemtypes.itemtype = $itype_link
-            LEFT JOIN branches ON branches.branchcode = issues.branchcode
-           WHERE ( accountlines.amountoutstanding  != '0.000000')
-             AND ( accountlines.accounttype  = 'FU')
-             AND (issues.branchcode = ? AND items.location = ?)
-             AND (issues.date_due <= NOW())
-           ORDER BY  borrowers.surname
-        " );
-        $sth->execute( $branch, $location);
-        my @getoverdues;
-       my $i = 0;
-        while ( my $data = $sth->fetchrow_hashref ) {
-       #check if the document has already been notified
-         my $countnotify = CheckItemNotify($data->{'notify_id'},$data->{'notify_level'},$data->{'itemnumber'});
-         if ($countnotify eq '0'){                     
-               $getoverdues[$i] = $data;
-                $i++;
-        }
         }
         }
-        $sth->finish;
-        return (@getoverdues); 
     }
     }
+    return (@getoverdues);
 }
 
 
 }
 
 
@@ -1262,25 +1247,22 @@ Creat a line into notify, if the method is phone, the notification_send_date is
 
 sub AddNotifyLine {
     my ( $borrowernumber, $itemnumber, $overduelevel, $method, $notifyId ) = @_;
 
 sub AddNotifyLine {
     my ( $borrowernumber, $itemnumber, $overduelevel, $method, $notifyId ) = @_;
+    my $dbh = C4::Context->dbh;
     if ( $method eq "phone" ) {
     if ( $method eq "phone" ) {
-        my $dbh = C4::Context->dbh;
         my $sth = $dbh->prepare(
             "INSERT INTO notifys (borrowernumber,itemnumber,notify_date,notify_send_date,notify_level,method,notify_id)
         VALUES (?,?,now(),now(),?,?,?)"
         );
         $sth->execute( $borrowernumber, $itemnumber, $overduelevel, $method,
             $notifyId );
         my $sth = $dbh->prepare(
             "INSERT INTO notifys (borrowernumber,itemnumber,notify_date,notify_send_date,notify_level,method,notify_id)
         VALUES (?,?,now(),now(),?,?,?)"
         );
         $sth->execute( $borrowernumber, $itemnumber, $overduelevel, $method,
             $notifyId );
-        $sth->finish;
     }
     else {
     }
     else {
-        my $dbh = C4::Context->dbh;
         my $sth = $dbh->prepare(
             "INSERT INTO notifys (borrowernumber,itemnumber,notify_date,notify_level,method,notify_id)
         VALUES (?,?,now(),?,?,?)"
         );
         $sth->execute( $borrowernumber, $itemnumber, $overduelevel, $method,
             $notifyId );
         my $sth = $dbh->prepare(
             "INSERT INTO notifys (borrowernumber,itemnumber,notify_date,notify_level,method,notify_id)
         VALUES (?,?,now(),?,?,?)"
         );
         $sth->execute( $borrowernumber, $itemnumber, $overduelevel, $method,
             $notifyId );
-        $sth->finish;
     }
     return 1;
 }
     }
     return 1;
 }
@@ -1304,15 +1286,12 @@ sub RemoveNotifyLine {
             AND notify_date=?"
     );
     $sth->execute( $borrowernumber, $itemnumber, $notify_date );
             AND notify_date=?"
     );
     $sth->execute( $borrowernumber, $itemnumber, $notify_date );
-    $sth->finish;
     return 1;
 }
 
 1;
 __END__
 
     return 1;
 }
 
 1;
 __END__
 
-=back
-
 =head1 AUTHOR
 
 Koha Developement team <info@koha.org>
 =head1 AUTHOR
 
 Koha Developement team <info@koha.org>
Koha ILS @ FFZG - based on git://git.koha.org/pub/scm/koha.git