(bug #4043) fix checkoverdues sqlquery
[koha.git] / C4 / Overdues.pm
index 62b0fbd..f67f16a 100644 (file)
@@ -19,11 +19,13 @@ package C4::Overdues;
 # 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 C4::Debug;
 
 use vars qw($VERSION @ISA @EXPORT);
 
@@ -66,9 +68,12 @@ BEGIN {
 
        # subs to move to Circulation.pm
        push @EXPORT, qw(
-        &GetIssuingRules
         &GetIssuesIteminfo
        );
+    #
+       # &GetIssuingRules - delete.
+       # use C4::Circulation::GetIssuingRule instead.
+       
        # subs to move to Members.pm
        push @EXPORT, qw(
         &CheckBorrowerDebarred
@@ -96,11 +101,9 @@ overdue items. It is primarily used by the 'misc/fines2.pl' script.
 
 =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.
 
@@ -112,72 +115,72 @@ Koha database.
 
 #'
 sub Getoverdues {
+    my $params = shift;
     my $dbh = C4::Context->dbh;
-    my $sth =  (C4::context->preference('item-level_itypes')) ? 
-                               $dbh->prepare(
-                               "SELECT issues.*,items.itype as itemtype FROM issues 
-                       LEFT JOIN items USING (itemnumber)
-                       WHERE date_due < now() 
-                           AND returndate IS NULL ORDER BY borrowernumber " )
-                               :
-                               $dbh->prepare(
-                    "SELECT issues.*,biblioitems.itemtype,items.itype FROM issues 
-                    LEFT JOIN items USING (itemnumber)
-                    LEFT JOIN biblioitems USING (biblioitemnumber)
-                    WHERE date_due < now() 
-                        AND returndate IS 
-                        NULL 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
 
-( $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 {
-
-# 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
-         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  = ?
-                AND issues.returndate is NULL
-                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) =
-    &CalcFine($itemnumber, $borrowercode, $days_overdue);
+  ($amount, $chargename, $daycount, $daycounttotal) =
+    &CalcFine($item, $categorycode, $branch, $days_overdue, $description, $start_date, $end_date );
 
 Calculates the fine for a book.
 
@@ -187,78 +190,91 @@ standard fine for books might be $0.50, but $1.50 for DVDs, or staff
 members might get a longer grace period between the first and second
 reminders that a book is overdue).
 
-The fine is calculated as follows: if it is time for the first
-reminder, the fine is the value listed for the given (branch, item type,
-borrower code) combination. If it is time for the second reminder, the
-fine is doubled. Finally, if it is time to send the account to a
-collection agency, the fine is set to 5 local monetary units (a really
-good deal for the patron if the library is in Italy). Otherwise, the
-fine is 0.
-
-Note that the way this function is currently implemented, it only
-returns a nonzero value on the notable days listed above. That is, if
-the categoryitems entry says to send a first reminder 7 days after the
-book is due, then if you call C<&CalcFine> 7 days after the book is
-due, it will give a nonzero fine. If you call C<&CalcFine> the next
-day, however, it will say that the fine is 0.
 
-C<$itemnumber> is the book's item number.
+C<$item> is an item object (hashref).
 
-C<$borrowercode> is the borrower code of the patron who currently has
+C<$categorycode> is the category code (string) of the patron who currently has
 the book.
 
-C<$days_overdue> is the number of days elapsed since the book's due
-date.
+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<$start_date> & C<$end_date> are C4::Dates objects 
+defining the date range over which to determine the fine.
+Note that if these are defined, we ignore C<$difference> and C<$dues> , 
+but retain these for backwards-comptibility with extant fines scripts.
+
+Fines scripts should just supply the date range over which to calculate the fine.
 
-C<&CalcFine> returns a list of three 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<$message> is a text message, either "First Notice", "Second Notice",
-or "Final Notice".
+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: 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
 
-#'
 sub CalcFine {
-    my ( $item, $bortype, $difference , $dues  ) = @_;
+    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 $printout;
-    # calculate how many days the patron is late
-    my $countspecialday=&GetSpecialHolidays($dues,$item->{itemnumber});
-    my $countrepeatableday=&GetRepeatableHolidays($dues,$item->{itemnumber},$difference);    
-    my $countalldayclosed = $countspecialday + $countrepeatableday;
-    my $daycount = $difference - $countalldayclosed;
-    # get issuingrules (fines part will be used)
-    my $data = GetIssuingRules($item->{'itemtype'},$bortype);
-    my $daycounttotal = $daycount - $data->{'firstremind'};
-    if ($data->{'chargeperiod'} >0) { # if there is a rule for this bortype
-        if ($data->{'firstremind'} < $daycount)
-            {
-            $amount   = int($daycounttotal/$data->{'chargeperiod'})*$data->{'fine'};
-        }
+       my $daystocharge;
+       # get issuingrules (fines part will be used)
+    $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 ).
+           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') {
+                       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.
+       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 {
-        # get fines default rules
-        my $data = GetIssuingRules($item->{'itemtype'},'*');
-        $daycounttotal = $daycount - $data->{'firstremind'};
-        if ($data->{'firstremind'} < $daycount)
-            {
-                if ($data->{'chargeperiod'} >0) { # if there is a rule for this bortype
-                    $amount   = int($daycounttotal/$data->{'chargeperiod'})*$data->{'fine'};
-                }
-            }
+        # a zero (or null)  chargeperiod means no charge.
     }
-    
-    warn "Calc Fine for $item->{'itemnumber'}, $bortype, $difference , $dues = $amount / $daycount";
- return ( $amount, $data->{'chargename'}, $printout ,$daycounttotal ,$daycount );
+       $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);
 
@@ -271,47 +287,52 @@ C<$itemnumber> is the book's item number.
 =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=?
 |;
-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 );
+    }
 
-    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 --;
+    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--;
             }
         }
     }
 
-return $specialdaycount;
+    return $specialdaycount;
 }
 
-=item GetRepeatableHolidays
+=head2 GetRepeatableHolidays
 
 &GetRepeatableHolidays($date_dues, $itemnumber, $difference,);
 
@@ -325,31 +346,31 @@ C<$difference> numbers of between day date of the day and date due
 
 =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);
 
@@ -359,27 +380,25 @@ C<$itemnumber> is  item number.
 
 =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=?
 |;
-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);
 
@@ -389,21 +408,21 @@ C<$itemnumber> is  item number.
 
 =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=?
-|;
-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);
 
@@ -429,13 +448,17 @@ accountlines table of the Koha database.
 
 =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.
+#
+# Possible Answer: You might update a fine for a damaged item, *after* it is returned.
+#
 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
@@ -443,48 +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")
+       #       "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(
-        "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 ) {
 
-        # I think this if-clause deals with the case where we're updating
-        # an existing fine.
-        #    print "in accounts ...";
-    if ( $data->{'amount'} != $amount ) {
-           
-        #      print "updating";
+               # 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'};
+            $diff = 0 if ( $data->{amount} > $amount);
             my $out  = $data->{'amountoutstanding'} + $diff;
-            my $sth2 = $dbh->prepare(
-                "UPDATE accountlines SET date=now(), amount=?,
-      amountoutstanding=?,accounttype='FU' WHERE
-      borrowernumber=? AND itemnumber=?
-      AND (accounttype='FU' OR accounttype='O') AND description LIKE ?"
-            );
-            $sth2->execute( $amount, $out, $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'}"
         }
-    }
-    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 $title = $sth4->fetchrow_hashref;
-        $sth4->finish;
+        my $title = $sth4->fetchrow;
 
 #         #   print "not in account";
 #         my $sth3 = $dbh->prepare("Select max(accountno) from accountlines");
@@ -495,31 +531,25 @@ sub UpdateFine {
 #         $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,accountno) VALUES
-    (?,?,now(),?,?,'FU',?,?)"
-        );
-        $sth2->execute( $borrowernumber, $itemnum, $amount,
-            "$type $title->{'title'} $due",
-            $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(
-        C4::Context->userenv->{'number'},
         "FINES",
         $type,
         $borrowernumber,
         "due=".$due."  amount=".$amount." itemnumber=".$itemnum
         ) if C4::Context->preference("FinesLog");
-
-    $sth->finish;
 }
 
-=item BorType
+=head2 BorType
 
   $borrower = &BorType($borrowernumber);
 
@@ -537,17 +567,15 @@ sub BorType {
     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);
-    my $data = $sth->fetchrow_hashref;
-    $sth->finish;
-    return ($data);
+    return $sth->fetchrow_hashref;
 }
 
-=item ReplacementCost
+=head2 ReplacementCost
 
   $cost = &ReplacementCost($itemnumber);
 
@@ -563,13 +591,12 @@ sub ReplacementCost {
       $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;
-    $sth->finish;
     return ( $data->{'replacementprice'} );
 }
 
-=item GetFine
+=head2 GetFine
 
 $data->{'sum(amountoutstanding)'} = &GetFine($itemnum,$borrowernumber);
 
@@ -585,21 +612,20 @@ C<$borrowernumber> is the borrowernumber
 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();
-    $sth->finish();
-    $dbh->disconnect();
     return ( $data->{'sum(amountoutstanding)'} );
 }
 
 
+=head2 GetIssuingRules
 
-
-=item GetIssuingRules
+FIXME - This sub should be deprecated and removed.
+It ignores branch and defaults.
 
 $data = &GetIssuingRules($itemtype,$categorycode);
 
@@ -615,9 +641,10 @@ category he or she belongs to.
 =cut 
 
 sub GetIssuingRules {
+       warn "GetIssuingRules is deprecated: use GetIssuingRule from C4::Circulation instead.";
    my ($itemtype,$categorycode)=@_;
    my $dbh   = C4::Context->dbh();    
-   my $query=qq|SELECT * 
+   my $query=qq|SELECT *
         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 ($data) = $sth->fetchrow_hashref;
-   $sth->finish;
-return ($data);
-
+    return $sth->fetchrow_hashref;
 }
 
 
 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
@@ -644,13 +668,11 @@ sub ReplacementCost2 {
     my $sth = $dbh->prepare($query);
     $sth->execute( $itemnum, $borrowernumber );
     my $data = $sth->fetchrow_hashref();
-    $sth->finish();
-    $dbh->disconnect();
     return ( $data->{'amountoutstanding'} );
 }
 
 
-=item GetNextIdNotify
+=head2 GetNextIdNotify
 
 ($result) = &GetNextIdNotify($reference);
 
@@ -662,38 +684,34 @@ C<$reference> contains the beggining of file number
 
 =cut
 
-
-
 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%\"
          |;
-# 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;
 
-=item NumberNotifyId
+        if ( $count < 10 ) {
+            ( $count = "0" . $count );
+        }
+        $result = $reference . $count;
+    }
+    return $result;
+}
+
+=head2 NumberNotifyId
 
 (@notify) = &NumberNotifyId($borrowernumber);
 
@@ -711,18 +729,15 @@ sub NumberNotifyId{
             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);
-
 }
 
-=item AmountNotify
+=head2 AmountNotify
 
 ($totalnotify) = &AmountNotify($notifyid);
 
@@ -749,7 +764,7 @@ sub AmountNotify{
 }
 
 
-=item GetNotifyId
+=head2 GetNotifyId
 
 ($notify_id) = &GetNotifyId($borrowernumber,$itemnumber);
 
@@ -764,23 +779,22 @@ C<$notify_id> contains the file number for the borrower number nad item number
 
 =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')|;
- 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);
 
@@ -809,25 +823,31 @@ C<$notify_id> contains the file number
 
 C<$level> contains the file level
 
-
 =cut
 
- sub CreateItemAccountLine {
-  my ($borrowernumber,$itemnumber,$date,$amount,$description,$accounttype,$amountoutstanding,$timestamp,$notify_id,$level)=@_;
-  my $dbh = C4::Context->dbh;
-  my $nextaccntno = 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
              (?,?,?,?,?,?,?,?,?,?,?)";
-  
-  
-  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);
 
@@ -847,39 +867,30 @@ C<$borrowernumber> contains the borrowernumber
 =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')|;
-}else
-{
-    $query=qq|UPDATE accountlines
+    } else {
+        $query = qq|UPDATE accountlines
      SET notify_id=?, notify_level=?
-           WHERE borrowernumber=?
+   WHERE borrowernumber=?
     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);
 
@@ -892,20 +903,23 @@ C<$itemnumber> contains the borrower categorycode
 
 =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 {
-    my($itemnumber) = @_;
-    my $query=qq|SELECT *
+    my $itemnumber = shift or return;
+    my $query = qq|SELECT *
              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);
 
@@ -918,19 +932,49 @@ C<$categorycode> contains the borrower categorycode
 =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=?|;
-    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);
 }
 
-=item CheckAccountLineLevelInfo
+=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;
+}
+
+=head2 CheckAccountLineLevelInfo
 
 ($exist) = &CheckAccountLineLevelInfo($borrowernumber,$itemnumber,$accounttype,notify_level);
 
@@ -951,21 +995,20 @@ C<$notify_level> contains the accountline level
 =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=?|;
-    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);
 
@@ -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
-=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);
 
@@ -1005,26 +1047,22 @@ C<$borrowernumber> contains the borrower number
 
 =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);
 
@@ -1047,7 +1085,7 @@ sub UpdateBorrowerDebarred{
         return 1;
 }
 
-=item CheckExistantNotifyid
+=head2 CheckExistantNotifyid
 
   ($exist) = &CheckExistantNotifyid($borrowernumber,$itemnumber,$accounttype,$notify_id);
 
@@ -1063,26 +1101,18 @@ C<$date_due> contains the date of item return
 =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 = ?|;
-    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);
 
@@ -1102,19 +1132,18 @@ C<$notify_id> contains the file number
 =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 = ?|;
-    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
 
@@ -1124,17 +1153,17 @@ this function is not exported, only used with GetOverduesForBranch
 =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
@@ -1143,114 +1172,68 @@ Sql request for display all information for branchoverdues.pl
 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 ";
-    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 $itype_link =  (C4::Context->preference('item-level_itypes')) ?  " items.itype " :  " biblioitems.itemtype ";
+    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,
-                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 ( issues.returndate  is null)
-              AND ( 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++;
-        }
         }
-        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 ( issues.returndate  is null )
-             AND ( 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);
 }
 
 
@@ -1264,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 ) = @_;
+    my $dbh = C4::Context->dbh;
     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 );
-        $sth->finish;
     }
     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 );
-        $sth->finish;
     }
     return 1;
 }
@@ -1306,15 +1286,12 @@ sub RemoveNotifyLine {
             AND notify_date=?"
     );
     $sth->execute( $borrowernumber, $itemnumber, $notify_date );
-    $sth->finish;
     return 1;
 }
 
 1;
 __END__
 
-=back
-
 =head1 AUTHOR
 
 Koha Developement team <info@koha.org>