BUGFIX issuingrules : total for all itemtype was not properly calculated
[koha.git] / C4 / Circulation.pm
index 362b29a..91a7b77 100644 (file)
@@ -25,8 +25,11 @@ use C4::Stats;
 use C4::Reserves;
 use C4::Koha;
 use C4::Biblio;
+use C4::Items;
 use C4::Members;
-use C4::Date;
+use C4::Dates;
+use C4::Calendar;
+use C4::Accounts;
 use Date::Calc qw(
   Today
   Today_and_Now
@@ -40,10 +43,52 @@ use POSIX qw(strftime);
 use C4::Branch; # GetBranches
 use C4::Log; # logaction
 
-our ($VERSION,@ISA,@EXPORT,@EXPORT_OK,%EXPORT_TAGS);
-
-# set the version for version checking
-$VERSION = 3.00;
+use Data::Dumper;
+
+use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
+
+BEGIN {
+       # set the version for version checking
+       $VERSION = 3.01;
+       @ISA    = qw(Exporter);
+
+       # FIXME subs that should probably be elsewhere
+       push @EXPORT, qw(
+               &FixOverduesOnReturn
+               &barcodedecode
+       );
+
+       # subs to deal with issuing a book
+       push @EXPORT, qw(
+               &CanBookBeIssued
+               &CanBookBeRenewed
+               &AddIssue
+               &AddRenewal
+               &GetRenewCount
+               &GetItemIssue
+               &GetItemIssues
+               &GetBorrowerIssues
+               &GetIssuingCharges
+               &GetIssuingRule
+               &GetBiblioIssues
+               &AnonymiseIssueHistory
+       );
+
+       # subs to deal with returns
+       push @EXPORT, qw(
+               &AddReturn
+        &MarkIssueReturned
+       );
+
+       # subs to deal with transfers
+       push @EXPORT, qw(
+               &transferbook
+               &GetTransfers
+               &GetTransfersFromTo
+               &updateWrongTransfer
+               &DeleteTransfer
+       );
+}
 
 =head1 NAME
 
@@ -61,70 +106,52 @@ Also deals with stocktaking.
 
 =head1 FUNCTIONS
 
-=cut
-
-@ISA    = qw(Exporter);
+=head2 barcodedecode
 
-# FIXME subs that should probably be elsewhere
-push @EXPORT, qw(
-  &FixOverduesOnReturn
-  &cuecatbarcodedecode
-);
-
-# subs to deal with issuing a book
-push @EXPORT, qw(
-  &CanBookBeIssued
-  &CanBookBeRenewed
-  &AddIssue
-  &AddRenewal
-  &GetItemIssue
-  &GetItemIssues
-  &GetBorrowerIssues
-  &GetIssuingCharges
-  &GetBiblioIssues
-  &AnonymiseIssueHistory
-);
-# subs to deal with returns
-push @EXPORT, qw(
-  &AddReturn
-);
-
-# subs to deal with transfers
-push @EXPORT, qw(
-  &transferbook
-  &GetTransfers
-  &GetTransfersFromTo
-  &updateWrongTransfer
-  &DeleteTransfer
-);
-
-# FIXME - At least, I'm pretty sure this is for decoding CueCat stuff.
-# FIXME From Paul : i don't understand what this sub does & why it has to be called on every circ. Speak of this with chris maybe ?
-
-=head2 decode
-
-=head3 $str = &decode($chunk);
+=head3 $str = &barcodedecode($barcode);
 
 =over 4
 
-=item Decodes a segment of a string emitted by a CueCat barcode scanner and
-returns it.
+=item Generic filter function for barcode string.
+Called on every circ if the System Pref itemBarcodeInputFilter is set.
+Will do some manipulation of the barcode for systems that deliver a barcode
+to circulation.pl that differs from the barcode stored for the item.
+For proper functioning of this filter, calling the function on the 
+correct barcode string (items.barcode) should return an unaltered barcode.
 
 =back
 
 =cut
 
-sub cuecatbarcodedecode {
+# FIXME -- the &decode fcn below should be wrapped into this one.
+# FIXME -- these plugins should be moved out of Circulation.pm
+#
+sub barcodedecode {
     my ($barcode) = @_;
-    chomp($barcode);
-    my @fields = split( /\./, $barcode );
-    my @results = map( decode($_), @fields[ 1 .. $#fields ] );
-    if ( $#results == 2 ) {
-        return $results[2];
-    }
-    else {
-        return $barcode;
-    }
+    my $filter = C4::Context->preference('itemBarcodeInputFilter');
+       if($filter eq 'whitespace') {
+               $barcode =~ s/\s//g;
+               return $barcode;
+       } elsif($filter eq 'cuecat') {
+               chomp($barcode);
+           my @fields = split( /\./, $barcode );
+           my @results = map( decode($_), @fields[ 1 .. $#fields ] );
+           if ( $#results == 2 ) {
+               return $results[2];
+           }
+           else {
+               return $barcode;
+           }
+       } elsif($filter eq 'T-prefix') {
+               if ( $barcode =~ /^[Tt]/) {
+                       if (substr($barcode,1,1) eq '0') {
+                               return $barcode;
+                       } else {
+                               $barcode = substr($barcode,2) + 0 ;
+                       }
+               }
+               return sprintf( "T%07d",$barcode);
+       }
 }
 
 =head2 decode
@@ -372,45 +399,52 @@ sub TooMany {
     my $biblionumber = shift;
        my $item                = shift;
     my $cat_borrower    = $borrower->{'categorycode'};
-    my $branch_borrower = $borrower->{'branchcode'};
     my $dbh             = C4::Context->dbh;
-
- my $branch_issuer = C4::Context->userenv->{'branchcode'};
-#TODO : specify issuer or borrower for circrule.
-  my $type = (C4::Context->preference('item-level_itypes')) 
+       my $branch;
+       # Get which branchcode we need
+       if (C4::Context->preference('CircControl') eq 'PickupLibrary'){
+               $branch = C4::Context->userenv->{'branch'}; 
+       }
+       elsif (C4::Context->preference('CircControl') eq 'PatronLibrary'){
+        $branch = $borrower->{'branchcode'}; 
+       }
+       else {
+               # items home library
+               $branch = $item->{'homebranch'};
+       }
+       my $type = (C4::Context->preference('item-level_itypes')) 
                        ? $item->{'itype'}         # item-level
                        : $item->{'itemtype'};     # biblio-level
   
-  my $sth =
+       my $sth =
       $dbh->prepare(
                 'SELECT * FROM issuingrules 
                         WHERE categorycode = ? 
-                            AND branchcode = ?
-                            AND itemtype = ? '
+                            AND itemtype = ? 
+                            AND branchcode = ?'
       );
 
     my $query2 = "SELECT  COUNT(*) FROM issues i, biblioitems s1, items s2 
                 WHERE i.borrowernumber = ? 
-                    AND i.returndate IS NULL 
                     AND i.itemnumber = s2.itemnumber 
-                    AND s1.biblioitemnumber = s2.biblioitemnumber"
-                               . (C4::Context->preference('item-level_itypes'))
-                               ? " AND s2.itype=? "
-                : " AND s1.itemtype= ? ";
+                    AND s1.biblioitemnumber = s2.biblioitemnumber";
+    if (C4::Context->preference('item-level_itypes')){
+          $query2.=" AND s2.itype=? ";
+    } else { 
+          $query2.=" AND s1.itemtype= ? ";
+    }
     my $sth2=  $dbh->prepare($query2);
     my $sth3 =
       $dbh->prepare(
             'SELECT COUNT(*) FROM issues
-                WHERE borrowernumber = ?
-                    AND returndate IS NULL'
+                WHERE borrowernumber = ?'
             );
     my $alreadyissued;
 
     # check the 3 parameters (branch / itemtype / category code
-    $sth->execute( $cat_borrower, $type, $branch_borrower );
+    $sth->execute( $cat_borrower, $type, $branch );
     my $result = $sth->fetchrow_hashref;
-#     warn "$cat_borrower, $type, $branch_borrower = ".Data::Dumper::Dumper($result);
-
+#     warn "$cat_borrower, $type, $branch = ".Data::Dumper::Dumper($result);
     if ( $result->{maxissueqty} ne '' ) {
 #         warn "checking on everything set";
         $sth2->execute( $borrower->{'borrowernumber'}, $type );
@@ -419,11 +453,11 @@ sub TooMany {
             return ( "$alreadyissued / ".( $result->{maxissueqty} + 0 )." (rule on branch/category/itemtype failed)" );
         }
         # now checking for total
-        $sth->execute( $cat_borrower, '', $branch_borrower );
+        $sth->execute( $cat_borrower, '*', $branch );
         my $result = $sth->fetchrow_hashref;
-        if ( $result->{maxissueqty} ne '*' ) {
-            $sth2->execute( $borrower->{'borrowernumber'}, $type );
-            my $alreadyissued = $sth2->fetchrow;
+        if ( $result->{maxissueqty} ne '' ) {
+            $sth3->execute( $borrower->{'borrowernumber'} );
+            my $alreadyissued = $sth3->fetchrow;
             if ( $result->{'maxissueqty'} <= $alreadyissued ) {
                 return ( "$alreadyissued / ".( $result->{maxissueqty} + 0 )." (rule on branch/category/total failed)"  );
             }
@@ -431,9 +465,9 @@ sub TooMany {
     }
 
     # check the 2 parameters (branch / itemtype / default categorycode
-    $sth->execute( '*', $type, $branch_borrower );
-    my $result = $sth->fetchrow_hashref;
-#     warn "*, $type, $branch_borrower = ".Data::Dumper::Dumper($result);
+    $sth->execute( '*', $type, $branch );
+    $result = $sth->fetchrow_hashref;
+#     warn "*, $type, $branch = ".Data::Dumper::Dumper($result);
 
     if ( $result->{maxissueqty} ne '' ) {
 #         warn "checking on 2 parameters (default categorycode)";
@@ -443,11 +477,11 @@ sub TooMany {
             return ( "$alreadyissued / ".( $result->{maxissueqty} + 0 )." (rule on branch / default category / itemtype failed)"  );
         }
         # now checking for total
-        $sth->execute( '*', '*', $branch_borrower );
+        $sth->execute( '*', '*', $branch );
         my $result = $sth->fetchrow_hashref;
         if ( $result->{maxissueqty} ne '' ) {
-            $sth2->execute( $borrower->{'borrowernumber'}, $type );
-            my $alreadyissued = $sth2->fetchrow;
+            $sth3->execute( $borrower->{'borrowernumber'} );
+            my $alreadyissued = $sth3->fetchrow;
             if ( $result->{'maxissueqty'} <= $alreadyissued ) {
                 return ( "$alreadyissued / ".( $result->{maxissueqty} + 0 )." (rule on branch / default category / total failed)" );
             }
@@ -456,7 +490,7 @@ sub TooMany {
     
     # check the 1 parameters (default branch / itemtype / categorycode
     $sth->execute( $cat_borrower, $type, '*' );
-    my $result = $sth->fetchrow_hashref;
+    $result = $sth->fetchrow_hashref;
 #     warn "$cat_borrower, $type, * = ".Data::Dumper::Dumper($result);
     
     if ( $result->{maxissueqty} ne '' ) {
@@ -470,8 +504,8 @@ sub TooMany {
         $sth->execute( $cat_borrower, '*', '*' );
         my $result = $sth->fetchrow_hashref;
         if ( $result->{maxissueqty} ne '' ) {
-            $sth2->execute( $borrower->{'borrowernumber'}, $type );
-            my $alreadyissued = $sth2->fetchrow;
+            $sth3->execute( $borrower->{'borrowernumber'} );
+            my $alreadyissued = $sth3->fetchrow;
             if ( $result->{'maxissueqty'} <= $alreadyissued ) {
                 return ( "$alreadyissued / ".( $result->{maxissueqty} + 0 )." (rule on default branch / category / total failed)"  );
             }
@@ -480,7 +514,7 @@ sub TooMany {
 
     # check the 0 parameters (default branch / itemtype / default categorycode
     $sth->execute( '*', $type, '*' );
-    my $result = $sth->fetchrow_hashref;
+    $result = $sth->fetchrow_hashref;
 #     warn "*, $type, * = ".Data::Dumper::Dumper($result);
 
     if ( $result->{maxissueqty} ne '' ) {
@@ -490,19 +524,20 @@ sub TooMany {
         if ( $result->{'maxissueqty'} <= $alreadyissued ) {
             return ( "$alreadyissued / ".( $result->{maxissueqty} + 0 )." (rule on default branch / default category / itemtype failed)"  );
         }
-        # now checking for total
-        $sth->execute( '*', '*', '*' );
-        my $result = $sth->fetchrow_hashref;
-        if ( $result->{maxissueqty} ne '' ) {
-            $sth2->execute( $borrower->{'borrowernumber'}, $type );
-            my $alreadyissued = $sth2->fetchrow;
-            if ( $result->{'maxissueqty'} <= $alreadyissued ) {
-                return ( "$alreadyissued / ".( $result->{maxissueqty} + 0 )." (rule on default branch / default category / total failed)"  );
-            }
-        }
-    }
+       }
+    # now checking for total
+    $sth->execute( '*', '*', '*' );
+    $result = $sth->fetchrow_hashref;
+    if ( $result->{maxissueqty} ne '' ) {
+               warn "checking total";
+               $sth2->execute( $borrower->{'borrowernumber'}, $type );
+               my $alreadyissued = $sth2->fetchrow;
+               if ( $result->{'maxissueqty'} <= $alreadyissued ) {
+                       return ( "$alreadyissued / ".( $result->{maxissueqty} + 0 )." (rule on default branch / default category / total failed)"  );
+               }
+       }
 
-    #OK, the patron can issue !!!
+    # OK, the patron can issue !!!
     return;
 }
 
@@ -554,12 +589,6 @@ The borrower number of the last three patrons who borrowed this item.
 sub itemissues {
     my ( $bibitem, $biblio ) = @_;
     my $dbh = C4::Context->dbh;
-
-    # FIXME - If this function die()s, the script will abort, and the
-    # user won't get anything; depending on how far the script has
-    # gotten, the user might get a blank page. It would be much better
-    # to at least print an error message. The easiest way to do this
-    # is to set $SIG{__DIE__}.
     my $sth =
       $dbh->prepare("Select * from items where items.biblioitemnumber = ?")
       || die $dbh->errstr;
@@ -581,7 +610,6 @@ sub itemissues {
             "SELECT * FROM issues
                 LEFT JOIN borrowers ON issues.borrowernumber = borrowers.borrowernumber
                 WHERE itemnumber = ?
-                    AND returndate IS NULL
             "
         );
 
@@ -592,22 +620,16 @@ sub itemissues {
             $data->{'borrower'} = $data2->{'borrowernumber'};
         }
         else {
-            if ( $data->{'wthdrawn'} eq '1' ) {
-                $data->{'date_due'} = 'Cancelled';
-            }
-            else {
-                $data->{'date_due'} = 'Available';
-            }    # else
-        }    # else
+            $data->{'date_due'} = ($data->{'wthdrawn'} eq '1') ? 'Cancelled' : 'Available';
+        }
 
         $sth2->finish;
 
         # Find the last 3 people who borrowed this item.
         $sth2 = $dbh->prepare(
-            "SELECT * FROM issues
+            "SELECT * FROM old_issues
                 LEFT JOIN borrowers ON  issues.borrowernumber = borrowers.borrowernumber
                 WHERE itemnumber = ?
-                AND returndate IS NOT NULL
                 ORDER BY returndate DESC,timestamp DESC"
         );
 
@@ -632,30 +654,36 @@ sub itemissues {
 
 =head2 CanBookBeIssued
 
-$issuingimpossible, $needsconfirmation = 
-        CanBookBeIssued( $borrower, $barcode, $year, $month, $day, $inprocess );
-
+( $issuingimpossible, $needsconfirmation ) = 
+        CanBookBeIssued( $borrower, $barcode, $duedatespec, $inprocess );
+C<$duedatespec> is a C4::Dates object.
 C<$issuingimpossible> and C<$needsconfirmation> are some hashref.
 
 =cut
 
 sub CanBookBeIssued {
-    my ( $borrower, $barcode, $year, $month, $day, $inprocess ) = @_;
+    my ( $borrower, $barcode, $duedate, $inprocess ) = @_;
     my %needsconfirmation;    # filled with problems that needs confirmations
     my %issuingimpossible;    # filled with problems that causes the issue to be IMPOSSIBLE
     my $item = GetItem(GetItemnumberFromBarcode( $barcode ));
     my $issue = GetItemIssue($item->{itemnumber});
+       my $biblioitem = GetBiblioItemData($item->{biblioitemnumber});
+       $item->{'itemtype'}=$item->{'itype'}; 
     my $dbh             = C4::Context->dbh;
 
     #
-    # DUE DATE is OK ?
+    # DUE DATE is OK ? -- should already have checked.
     #
-    my ( $duedate, $invalidduedate ) = fixdate( $year, $month, $day );
-    $issuingimpossible{INVALID_DATE} = 1 if ($invalidduedate);
+    #$issuingimpossible{INVALID_DATE} = 1 unless ($duedate);
 
     #
     # BORROWER STATUS
     #
+    if ( $borrower->{'category_type'} eq 'X' && (  $item->{barcode}  )) { 
+       # stats only borrower -- add entry to statistics table, and return issuingimpossible{STATS} = 1  .
+        &UpdateStats(C4::Context->userenv->{'branch'},'localuse','','',$item->{'itemnumber'},$item->{'itemtype'},$borrower->{'borrowernumber'});
+        return( { STATS => 1 }, {});
+    }
     if ( $borrower->{flags}->{GNA} ) {
         $issuingimpossible{GNA} = 1;
     }
@@ -680,7 +708,7 @@ sub CanBookBeIssued {
 
     # DEBTS
     my ($amount) =
-      C4::Members::GetMemberAccountRecords( $borrower->{'borrowernumber'}, $duedate );
+      C4::Members::GetMemberAccountRecords( $borrower->{'borrowernumber'}, '' && $duedate->output('iso') );
     if ( C4::Context->preference("IssuingInProcess") ) {
         my $amountlimit = C4::Context->preference("noissuescharge");
         if ( $amount > $amountlimit && !$inprocess ) {
@@ -699,7 +727,6 @@ sub CanBookBeIssued {
     #
     # JB34 CHECKS IF BORROWERS DONT HAVE ISSUE TOO MANY BOOKS
     #
-    
        my $toomany = TooMany( $borrower, $item->{biblionumber}, $item );
     $needsconfirmation{TOO_MANY} = $toomany if $toomany;
 
@@ -714,6 +741,22 @@ sub CanBookBeIssued {
     {
         $issuingimpossible{NOT_FOR_LOAN} = 1;
     }
+       elsif ( !$item->{'notforloan'} ){
+               # we have to check itemtypes.notforloan also
+               if (C4::Context->preference('item-level_itypes')){
+                       # this should probably be a subroutine
+                       my $sth = $dbh->prepare("SELECT notforloan FROM itemtypes WHERE itemtype = ?");
+                       $sth->execute($item->{'itemtype'});
+                       my $notforloan=$sth->fetchrow_hashref();
+                       $sth->finish();
+                       if ($notforloan->{'notforloan'} == 1){
+                               $issuingimpossible{NOT_FOR_LOAN} = 1;                           
+                       }
+               }
+               elsif ($biblioitem->{'notforloan'} == 1){
+                       $issuingimpossible{NOT_FOR_LOAN} = 1;
+               }
+       }
     if ( $item->{'wthdrawn'} && $item->{'wthdrawn'} == 1 )
     {
         $issuingimpossible{WTHDRAWN} = 1;
@@ -727,7 +770,7 @@ sub CanBookBeIssued {
         my $userenv = C4::Context->userenv;
         if ( ($userenv) && ( $userenv->{flags} != 1 ) ) {
             $issuingimpossible{NOTSAMEBRANCH} = 1
-              if ( $item->{C4::Context->preference("HomeOrHoldingbranch")} ne $userenv->{branch} );
+              if ( $item->{C4::Context->preference("HomeOrHoldingBranch")} ne $userenv->{branch} );
         }
     }
 
@@ -739,7 +782,7 @@ sub CanBookBeIssued {
 
         # Already issued to current borrower. Ask whether the loan should
         # be renewed.
-        my ($CanBookBeRenewed) = CanBookBeRenewed(
+        my ($CanBookBeRenewed,$renewerror) = CanBookBeRenewed(
             $borrower->{'borrowernumber'},
             $item->{'itemnumber'}
         );
@@ -763,44 +806,30 @@ sub CanBookBeIssued {
     # See if the item is on reserve.
     my ( $restype, $res ) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
     if ($restype) {
-        my $resbor = $res->{'borrowernumber'};
+               my $resbor = $res->{'borrowernumber'};
+               my ( $resborrower, $flags ) = GetMemberDetails( $resbor, 0 );
+               my $branches  = GetBranches();
+               my $branchname = $branches->{ $res->{'branchcode'} }->{'branchname'};
         if ( $resbor ne $borrower->{'borrowernumber'} && $restype eq "Waiting" )
         {
-
             # The item is on reserve and waiting, but has been
             # reserved by some other patron.
-            my ( $resborrower, $flags ) =
-              GetMemberDetails( $resbor, 0 );
-            my $branches   = GetBranches();
-            my $branchname =
-              $branches->{ $res->{'branchcode'} }->{'branchname'};
             $needsconfirmation{RESERVE_WAITING} =
 "$resborrower->{'firstname'} $resborrower->{'surname'} ($resborrower->{'cardnumber'}, $branchname)";
-
-# CancelReserve(0, $res->{'itemnumber'}, $res->{'borrowernumber'}); Doesn't belong in a checking subroutine.
         }
         elsif ( $restype eq "Reserved" ) {
-
             # The item is on reserve for someone else.
-            my ( $resborrower, $flags ) =
-              GetMemberDetails( $resbor, 0 );
-            my $branches   = GetBranches();
-            my $branchname =
-              $branches->{ $res->{'branchcode'} }->{'branchname'};
             $needsconfirmation{RESERVED} =
 "$res->{'reservedate'} : $resborrower->{'firstname'} $resborrower->{'surname'} ($resborrower->{'cardnumber'})";
         }
     }
     if ( C4::Context->preference("LibraryName") eq "Horowhenua Library Trust" ) {
         if ( $borrower->{'categorycode'} eq 'W' ) {
-            my %issuingimpossible;
-            return ( \%issuingimpossible, \%needsconfirmation );
-        } else {
-            return ( \%issuingimpossible, \%needsconfirmation );
+            my %emptyhash;
+            return ( \%emptyhash, \%needsconfirmation );
         }
-    } else {
-        return ( \%issuingimpossible, \%needsconfirmation );
-    }
+       }
+       return ( \%issuingimpossible, \%needsconfirmation );
 }
 
 =head2 AddIssue
@@ -837,90 +866,102 @@ AddIssue does the following things :
 sub AddIssue {
     my ( $borrower, $barcode, $date, $cancelreserve ) = @_;
     my $dbh = C4::Context->dbh;
-my $barcodecheck=CheckValidBarcode($barcode);
-if ($borrower and $barcode and $barcodecheck ne '0'){
-#   my ($borrower, $flags) = &GetMemberDetails($borrowernumber, 0);
-    # find which item we issue
-    my $item = GetItem('', $barcode);
-       
-       my $datedue;
-    
-    # get actual issuing if there is one
-    my $actualissue = GetItemIssue( $item->{itemnumber});
-    
-    # get biblioinformation for this item
-    my $biblio = GetBiblioFromItemNumber($item->{itemnumber});
-
-    #
-    # check if we just renew the issue.
-    #
-    if ( $actualissue->{borrowernumber} eq $borrower->{'borrowernumber'} ) {
-        AddRenewal(
-            $borrower->{'borrowernumber'},
-            $item->{'itemnumber'}
-        );
-
-    }
-    else {# it's NOT a renewal
-        if ( $actualissue->{borrowernumber}) {
-            # This book is currently on loan, but not to the person
-            # who wants to borrow it now. mark it returned before issuing to the new borrower
-            AddReturn(
-                $item->{'barcode'},
-                C4::Context->userenv->{'branch'}
-            );
-        }
+       my $barcodecheck=CheckValidBarcode($barcode);
+       if ($borrower and $barcode and $barcodecheck ne '0'){
+               # find which item we issue
+               my $item = GetItem('', $barcode);
+               my $datedue; 
+               
+               my $branch;
+               # Get which branchcode we need
+               if (C4::Context->preference('CircControl') eq 'PickupLibrary'){
+                       $branch = C4::Context->userenv->{'branch'}; 
+               }
+               elsif (C4::Context->preference('CircControl') eq 'PatronLibrary'){
+                       $branch = $borrower->{'branchcode'}; 
+               }
+               else {
+                       # items home library
+                       $branch = $item->{'homebranch'};
+               }
+               
+               # get actual issuing if there is one
+               my $actualissue = GetItemIssue( $item->{itemnumber});
+               
+               # get biblioinformation for this item
+               my $biblio = GetBiblioFromItemNumber($item->{itemnumber});
+               
+               #
+               # check if we just renew the issue.
+               #
+               if ( $actualissue->{borrowernumber} eq $borrower->{'borrowernumber'} ) {
+                       AddRenewal(
+                               $borrower->{'borrowernumber'},
+                               $item->{'itemnumber'},
+                               $branch,
+                               $date
+                       );
 
-        # See if the item is on reserve.
-        my ( $restype, $res ) =
-          C4::Reserves::CheckReserves( $item->{'itemnumber'} );
-        if ($restype) {
-            my $resbor = $res->{'borrowernumber'};
-            if ( $resbor eq $borrower->{'borrowernumber'} ) {
+               }
+               else {
+        # it's NOT a renewal
+                       if ( $actualissue->{borrowernumber}) {
+                               # This book is currently on loan, but not to the person
+                               # who wants to borrow it now. mark it returned before issuing to the new borrower
+                               AddReturn(
+                                       $item->{'barcode'},
+                                       C4::Context->userenv->{'branch'}
+                               );
+                       }
 
-                # The item is reserved by the current patron
-                ModReserveFill($res);
-            }
-            elsif ( $restype eq "Waiting" ) {
-
-                # warn "Waiting";
-                # The item is on reserve and waiting, but has been
-                # reserved by some other patron.
-                my ( $resborrower, $flags ) = GetMemberDetails( $resbor, 0 );
-                my $branches   = GetBranches();
-                my $branchname =
-                  $branches->{ $res->{'branchcode'} }->{'branchname'};
-            }
-            elsif ( $restype eq "Reserved" ) {
-
-                # warn "Reserved";
-                # The item is reserved by someone else.
-                my ( $resborrower, $flags ) =
-                  GetMemberDetails( $resbor, 0 );
-                my $branches   = GetBranches();
-                my $branchname =
-                  $branches->{ $res->{'branchcode'} }->{'branchname'};
-                if ($cancelreserve) { # cancel reserves on this item
-                    CancelReserve( 0, $res->{'itemnumber'},
-                        $res->{'borrowernumber'} );
-                }
-            }
-            if ($cancelreserve) {
-                CancelReserve( $res->{'biblionumber'}, 0,
+                       # See if the item is on reserve.
+                       my ( $restype, $res ) =
+                         C4::Reserves::CheckReserves( $item->{'itemnumber'} );
+                       if ($restype) {
+                               my $resbor = $res->{'borrowernumber'};
+                               if ( $resbor eq $borrower->{'borrowernumber'} ) {
+
+                                       # The item is reserved by the current patron
+                                       ModReserveFill($res);
+                               }
+                               elsif ( $restype eq "Waiting" ) {
+
+                                       # warn "Waiting";
+                                       # The item is on reserve and waiting, but has been
+                                       # reserved by some other patron.
+                                       my ( $resborrower, $flags ) = GetMemberDetails( $resbor, 0 );
+                                       my $branches   = GetBranches();
+                                       my $branchname =
+                                         $branches->{ $res->{'branchcode'} }->{'branchname'};
+                               }
+                               elsif ( $restype eq "Reserved" ) {
+
+                                       # warn "Reserved";
+                                       # The item is reserved by someone else.
+                                       my ( $resborrower, $flags ) =
+                                         GetMemberDetails( $resbor, 0 );
+                                       my $branches   = GetBranches();
+                                       my $branchname =  $branches->{ $res->{'branchcode'} }->{'branchname'};
+                                       if ($cancelreserve) { # cancel reserves on this item
+                                               CancelReserve( 0, $res->{'itemnumber'},
+                                                       $res->{'borrowernumber'} );
+                                       }
+                               }
+                               if ($cancelreserve) {
+                                       CancelReserve( $res->{'biblionumber'}, 0,
                     $res->{'borrowernumber'} );
-            }
-            else {
-    # set waiting reserve to first in reserve queue as book isn't waiting now
-                ModReserve(
-                    1,
-                    $res->{'biblionumber'},
-                    $res->{'borrowernumber'},
-                    $res->{'branchcode'}
-                );
-            }
-        }
+                               }
+                               else {
+                                       # set waiting reserve to first in reserve queue as book isn't waiting now
+                                       ModReserve(1,
+                                               $res->{'biblionumber'},
+                                               $res->{'borrowernumber'},
+                                               $res->{'branchcode'}
+                                       );
+                               }
+                       }
 
-        # Starting process for transfer job (checking transfert and validate it if we have one)
+                       # Starting process for transfer job (checking transfert and validate it if we have one)
             my ($datesent) = GetTransfers($item->{'itemnumber'});
             if ($datesent) {
         #      updating line of branchtranfert to finish it, and changing the to branch value, implement a comment for lisibility of this case (maybe for stats ....)
@@ -929,7 +970,7 @@ if ($borrower and $barcode and $barcodecheck ne '0'){
                     "UPDATE branchtransfers 
                         SET datearrived = now(),
                         tobranch = ?,
-                        comments = 'Forced branchtransfert'
+                        comments = 'Forced branchtransfer'
                     WHERE itemnumber= ? AND datearrived IS NULL"
                     );
                     $sth->execute(C4::Context->userenv->{'branch'},$item->{'itemnumber'});
@@ -943,43 +984,37 @@ if ($borrower and $barcode and $barcodecheck ne '0'){
                     (borrowernumber, itemnumber,issuedate, date_due, branchcode)
                 VALUES (?,?,?,?,?)"
           );
-               my $itype=(C4::Context->preference('item-level_itypes')) ?  $biblio->{'itype'} : $biblio->{'itemtype'} ;
-        my $loanlength = GetLoanLength(
-            $borrower->{'categorycode'},
-            $itype,
-            $borrower->{'branchcode'}
-        );
-        $datedue  = time + ($loanlength) * 86400;
-        my @datearr  = localtime($datedue);
-        my $dateduef =
-            sprintf("%04d-%02d-%02d", 1900 + $datearr[5], $datearr[4] + 1, $datearr[3]);
+               my $dateduef;
         if ($date) {
             $dateduef = $date;
-        }
-       $dateduef=CheckValidDatedue($dateduef,$item->{'itemnumber'},C4::Context->userenv->{'branch'});
-       # if ReturnBeforeExpiry ON the datedue can't be after borrower expirydate
-        if ( C4::Context->preference('ReturnBeforeExpiry')
-            && $dateduef gt $borrower->{dateexpiry} )
-        {
-            $dateduef = $borrower->{dateexpiry};
-        }
-        $sth->execute(
+        } else {
+                       my $itype=(C4::Context->preference('item-level_itypes')) ?  $biblio->{'itype'} : $biblio->{'itemtype'} ;
+               my $loanlength = GetLoanLength(
+                   $borrower->{'categorycode'},
+                   $itype,
+                $branch
+               );
+                       $dateduef = CalcDateDue(C4::Dates->new(),$loanlength,$branch);
+               # if ReturnBeforeExpiry ON the datedue can't be after borrower expirydate
+               if ( C4::Context->preference('ReturnBeforeExpiry') && $dateduef->output('iso') gt $borrower->{dateexpiry} ) {
+                   $dateduef = C4::Dates->new($borrower->{dateexpiry},'iso');
+               }
+        };
+               $sth->execute(
             $borrower->{'borrowernumber'},
             $item->{'itemnumber'},
-            strftime( "%Y-%m-%d", localtime ),$dateduef, C4::Context->userenv->{'branch'}
+            strftime( "%Y-%m-%d", localtime ),$dateduef->output('iso'), C4::Context->userenv->{'branch'}
         );
         $sth->finish;
         $item->{'issues'}++;
-        $sth =
-          $dbh->prepare(
-            "UPDATE items SET issues=?, holdingbranch=?, itemlost=0, datelastborrowed  = now() WHERE itemnumber=?");
-        $sth->execute(
-            $item->{'issues'},
-            C4::Context->userenv->{'branch'},
-            $item->{'itemnumber'}
-        );
-        $sth->finish;
-        &ModDateLastSeen( $item->{'itemnumber'} );
+        ModItem({ issues           => $item->{'issues'},
+                  holdingbranch    => C4::Context->userenv->{'branch'},
+                  itemlost         => 0,
+                  datelastborrowed => C4::Dates->new()->output('iso'),
+                  onloan           => $dateduef->output('iso'),
+                }, $item->{'biblionumber'}, $item->{'itemnumber'});
+        ModDateLastSeen( $item->{'itemnumber'} );
+        
         # If it costs to borrow this book, charge it to the patron's account.
         my ( $charge, $itemtype ) = GetIssuingCharges(
             $item->{'itemnumber'},
@@ -998,14 +1033,14 @@ if ($borrower and $barcode and $barcodecheck ne '0'){
             C4::Context->userenv->{'branch'},
             'issue',                        $charge,
             '',                             $item->{'itemnumber'},
-            $item->{'itemtype'}, $borrower->{'borrowernumber'}
+            $item->{'itype'}, $borrower->{'borrowernumber'}
         );
     }
     
-    &logaction(C4::Context->userenv->{'number'},"CIRCULATION","ISSUE",$borrower->{'borrowernumber'},$biblio->{'biblionumber'}) 
+    logaction("CIRCULATION", "ISSUE", $borrower->{'borrowernumber'}, $biblio->{'biblionumber'}) 
         if C4::Context->preference("IssueLog");
     return ($datedue);
-  }  
+  }
 }
 
 =head2 GetLoanLength
@@ -1023,7 +1058,7 @@ sub GetLoanLength {
       $dbh->prepare(
 "select issuelength from issuingrules where categorycode=? and itemtype=? and branchcode=? and issuelength is not null"
       );
-
+# warn "in get loan lenght $borrowertype $itemtype $branchcode ";
 # try to find issuelength & return the 1st available.
 # check with borrowertype, itemtype and branchcode, then without one of those parameters
     $sth->execute( $borrowertype, $itemtype, $branchcode );
@@ -1070,15 +1105,75 @@ sub GetLoanLength {
     return 21;
 }
 
+=head2 GetIssuingRule
+
+FIXME - This is a copy-paste of GetLoanLength 
+as a stop-gap.  Do not wish to change API for GetLoanLength 
+this close to release, however, Overdues::GetIssuingRules is broken.
+
+Get the issuing rule for an itemtype, a borrower type and a branch
+Returns a hashref from the issuingrules table.
+
+my $irule = &GetIssuingRule($borrowertype,$itemtype,branchcode)
+
+=cut
+
+sub GetIssuingRule {
+    my ( $borrowertype, $itemtype, $branchcode ) = @_;
+    my $dbh = C4::Context->dbh;
+    my $sth =  $dbh->prepare( "select * from issuingrules where categorycode=? and itemtype=? and branchcode=? and issuelength is not null"  );
+    my $irule;
+
+       $sth->execute( $borrowertype, $itemtype, $branchcode );
+    $irule = $sth->fetchrow_hashref;
+    return $irule if defined($irule) ;
+
+    $sth->execute( $borrowertype, $itemtype, "*" );
+    $irule = $sth->fetchrow_hashref;
+    return $irule if defined($irule) ;
+
+    $sth->execute( $borrowertype, "*", $branchcode );
+    $irule = $sth->fetchrow_hashref;
+    return $irule if defined($irule) ;
+
+    $sth->execute( "*", $itemtype, $branchcode );
+    $irule = $sth->fetchrow_hashref;
+    return $irule if defined($irule) ;
+
+    $sth->execute( $borrowertype, "*", "*" );
+    $irule = $sth->fetchrow_hashref;
+    return $irule if defined($irule) ;
+
+    $sth->execute( "*", "*", $branchcode );
+    $irule = $sth->fetchrow_hashref;
+    return $irule if defined($irule) ;
+
+    $sth->execute( "*", $itemtype, "*" );
+    $irule = $sth->fetchrow_hashref;
+    return $irule if defined($irule) ;
+
+    $sth->execute( "*", "*", "*" );
+    $irule = $sth->fetchrow_hashref;
+    return $irule if defined($irule) ;
+
+    # if no rule matches,
+    return undef;
+}
+
 =head2 AddReturn
 
 ($doreturn, $messages, $iteminformation, $borrower) =
-    &AddReturn($barcode, $branch);
+    &AddReturn($barcode, $branch, $exemptfine, $dropbox);
 
 Returns a book.
 
 C<$barcode> is the bar code of the book being returned. C<$branch> is
-the code of the branch where the book is being returned.
+the code of the branch where the book is being returned.  C<$exemptfine>
+indicates that overdue charges for the item will be removed.  C<$dropbox>
+indicates that the check-in date is assumed to be yesterday, or the last
+non-holiday as defined in C4::Calendar .  If overdue
+charges are applied and C<$dropbox> is true, the last charge will be removed.
+This assumes that the fines accrual script has run for _today_.
 
 C<&AddReturn> returns a list of four items:
 
@@ -1121,7 +1216,7 @@ patron who last borrowed the book.
 =cut
 
 sub AddReturn {
-    my ( $barcode, $branch ) = @_;
+    my ( $barcode, $branch, $exemptfine, $dropbox ) = @_;
     my $dbh      = C4::Context->dbh;
     my $messages;
     my $doreturn = 1;
@@ -1131,6 +1226,8 @@ sub AddReturn {
     
     # get information on item
     my $iteminformation = GetItemIssue( GetItemnumberFromBarcode($barcode));
+    my $biblio = GetBiblioItemData($iteminformation->{'biblioitemnumber'});
+#     use Data::Dumper;warn Data::Dumper::Dumper($iteminformation);  
     unless ($iteminformation->{'itemnumber'} ) {
         $messages->{'BadBarcode'} = $barcode;
         $doreturn = 0;
@@ -1138,16 +1235,28 @@ sub AddReturn {
         # find the borrower
         if ( ( not $iteminformation->{borrowernumber} ) && $doreturn ) {
             $messages->{'NotIssued'} = $barcode;
+            # even though item is not on loan, it may still
+            # be transferred; therefore, get current branch information
+            my $curr_iteminfo = GetItem($iteminformation->{'itemnumber'});
+            $iteminformation->{'homebranch'} = $curr_iteminfo->{'homebranch'};
+            $iteminformation->{'holdingbranch'} = $curr_iteminfo->{'holdingbranch'};
             $doreturn = 0;
         }
     
         # check if the book is in a permanent collection....
-        my $hbr      = $iteminformation->{'homebranch'};
+        my $hbr      = $iteminformation->{C4::Context->preference("HomeOrHoldingBranch")};
         my $branches = GetBranches();
+               # FIXME -- This 'PE' attribute is largely undocumented.  afaict, there's no user interface that reflects this functionality.
         if ( $hbr && $branches->{$hbr}->{'PE'} ) {
             $messages->{'IsPermanent'} = $hbr;
         }
-    
+               
+                   # if independent branches are on and returning to different branch, refuse the return
+        if ($hbr ne C4::Context->userenv->{'branch'} && C4::Context->preference("IndependantBranches")){
+                         $messages->{'Wrongbranch'} = 1;
+                         $doreturn=0;
+                   }
+                       
         # check that the book has been cancelled
         if ( $iteminformation->{'wthdrawn'} ) {
             $messages->{'wthdrawn'} = 1;
@@ -1162,27 +1271,37 @@ sub AddReturn {
     # case of a return of document (deal with issues and holdingbranch)
     
         if ($doreturn) {
-            my $sth =
-            $dbh->prepare(
-    "UPDATE issues SET returndate = now() WHERE (borrowernumber = ?) AND (itemnumber = ?) AND (returndate IS NULL)"
-            );
-            $sth->execute( $borrower->{'borrowernumber'},
-                $iteminformation->{'itemnumber'} );
+                       my $circControlBranch;
+                       if($dropbox) {
+                               # don't allow dropbox mode to create an invalid entry in issues ( issuedate > returndate)
+                               undef($dropbox) if ( $iteminformation->{'issuedate'} eq C4::Dates->today('iso') );
+                               if (C4::Context->preference('CircControl') eq 'ItemHomeBranch' ) {
+                                       $circControlBranch = $iteminformation->{homebranch};
+                               } elsif ( C4::Context->preference('CircControl') eq 'PatronLibrary') {
+                                       $circControlBranch = $borrower->{branchcode};
+                               } else { # CircControl must be PickupLibrary.
+                                       $circControlBranch = $iteminformation->{holdingbranch};
+                                       # FIXME - is this right ? are we sure that the holdingbranch is still the pickup branch?
+                               }
+                       }
+            MarkIssueReturned($borrower->{'borrowernumber'}, $iteminformation->{'itemnumber'},$circControlBranch);
             $messages->{'WasReturned'} = 1;    # FIXME is the "= 1" right?
         }
     
     # continue to deal with returns cases, but not only if we have an issue
     
-    # the holdingbranch is updated if the document is returned in an other location .
-    if ( $iteminformation->{'holdingbranch'} ne C4::Context->userenv->{'branch'} )
-            {
-                    UpdateHoldingbranch(C4::Context->userenv->{'branch'},$iteminformation->{'itemnumber'});    
-    #          reload iteminformation holdingbranch with the userenv value
-                    $iteminformation->{'holdingbranch'} = C4::Context->userenv->{'branch'};
-            }
+        # the holdingbranch is updated if the document is returned in an other location .
+        if ( $iteminformation->{'holdingbranch'} ne C4::Context->userenv->{'branch'} ) {
+                       UpdateHoldingbranch(C4::Context->userenv->{'branch'},$iteminformation->{'itemnumber'}); 
+                       #               reload iteminformation holdingbranch with the userenv value
+                       $iteminformation->{'holdingbranch'} = C4::Context->userenv->{'branch'};
+        }
         ModDateLastSeen( $iteminformation->{'itemnumber'} );
-        ($borrower) = C4::Members::GetMemberDetails( $iteminformation->{borrowernumber}, 0 );
-        
+        ModItem({ onloan => undef }, $biblio->{'biblionumber'}, $iteminformation->{'itemnumber'});
+                   
+                   if ($iteminformation->{borrowernumber}){
+                         ($borrower) = C4::Members::GetMemberDetails( $iteminformation->{borrowernumber}, 0 );
+        }       
         # fix up the accounts.....
         if ( $iteminformation->{'itemlost'} ) {
             $messages->{'WasLost'} = 1;
@@ -1219,7 +1338,7 @@ sub AddReturn {
         }
         # fix up the overdues in accounts...
         FixOverduesOnReturn( $borrower->{'borrowernumber'},
-            $iteminformation->{'itemnumber'} );
+            $iteminformation->{'itemnumber'}, $exemptfine, $dropbox );
     
     # find reserves.....
     #     if we don't have a reserve with the status W, we launch the Checkreserves routine
@@ -1236,41 +1355,94 @@ sub AddReturn {
         UpdateStats(
             $branch, 'return', '0', '',
             $iteminformation->{'itemnumber'},
-            $iteminformation->{'itemtype'},
+            $biblio->{'itemtype'},
             $borrower->{'borrowernumber'}
         );
         
-        &logaction(C4::Context->userenv->{'number'},"CIRCULATION","RETURN",$iteminformation->{borrowernumber},$iteminformation->{'biblionumber'}) 
+        logaction("CIRCULATION", "RETURN", $iteminformation->{borrowernumber}, $iteminformation->{'biblionumber'}) 
             if C4::Context->preference("ReturnLog");
         
         #adding message if holdingbranch is non equal a userenv branch to return the document to homebranch
         #we check, if we don't have reserv or transfert for this document, if not, return it to homebranch .
         
         if ( ($iteminformation->{'holdingbranch'} ne $iteminformation->{'homebranch'}) and not $messages->{'WrongTransfer'} and ($validTransfert ne 1) and ($reserveDone ne 1) ){
-                    if (C4::Context->preference("AutomaticItemReturn") == 1) {
-                    ModItemTransfer($iteminformation->{'itemnumber'}, C4::Context->userenv->{'branch'}, $iteminformation->{'homebranch'});
-                    $messages->{'WasTransfered'} = 1;
-                    warn "was transfered";
-                    }
+                       if (C4::Context->preference("AutomaticItemReturn") == 1) {
+                               ModItemTransfer($iteminformation->{'itemnumber'}, C4::Context->userenv->{'branch'}, $iteminformation->{'homebranch'});
+                               $messages->{'WasTransfered'} = 1;
+                       }
+                       else {
+                               $messages->{'NeedsTransfer'} = 1;
+                       }
         }
     }
     return ( $doreturn, $messages, $iteminformation, $borrower );
 }
 
+=head2 MarkIssueReturned
+
+=over 4
+
+MarkIssueReturned($borrowernumber, $itemnumber, $dropbox_branch);
+
+=back
+
+Unconditionally marks an issue as being returned by
+moving the C<issues> row to C<old_issues> and
+setting C<returndate> to the current date, or
+the last non-holiday date of the branccode specified in
+C<dropbox> .  Assumes you've already checked that 
+it's safe to do this, i.e. last non-holiday > issuedate.
+
+Ideally, this function would be internal to C<C4::Circulation>,
+not exported, but it is currently needed by one 
+routine in C<C4::Accounts>.
+
+=cut
+
+sub MarkIssueReturned {
+    my ($borrowernumber, $itemnumber, $dropbox_branch ) = @_;
+       my $dbh = C4::Context->dbh;
+       my $query = "UPDATE issues SET returndate=";
+       my @bind = ($borrowernumber,$itemnumber);
+       if($dropbox_branch) {
+               my $calendar = C4::Calendar->new(  branchcode => $dropbox_branch );
+               my $dropboxdate = $calendar->addDate(C4::Dates->new(), -1 );
+               unshift @bind, $dropboxdate->output('iso') ;
+               $query .= " ? "
+       } else {
+               $query .= " now() ";
+       }
+       $query .=  " WHERE  borrowernumber = ?  AND itemnumber = ?";
+    # FIXME transaction
+    my $sth_upd  = $dbh->prepare($query);
+    $sth_upd->execute(@bind);
+    my $sth_copy = $dbh->prepare("INSERT INTO old_issues SELECT * FROM issues 
+                                  WHERE borrowernumber = ?
+                                  AND itemnumber = ?");
+    $sth_copy->execute($borrowernumber, $itemnumber);
+    my $sth_del  = $dbh->prepare("DELETE FROM issues
+                                  WHERE borrowernumber = ?
+                                  AND itemnumber = ?");
+    $sth_del->execute($borrowernumber, $itemnumber);
+}
+
 =head2 FixOverduesOnReturn
 
-    &FixOverduesOnReturn($brn,$itm);
+    &FixOverduesOnReturn($brn,$itm, $exemptfine, $dropboxmode);
 
 C<$brn> borrowernumber
 
 C<$itm> itemnumber
 
+C<$exemptfine> BOOL -- remove overdue charge associated with this issue. 
+C<$dropboxmode> BOOL -- remove lastincrement on overdue charge associated with this issue.
+
 internal function, called only by AddReturn
 
 =cut
 
 sub FixOverduesOnReturn {
-    my ( $borrowernumber, $item ) = @_;
+    my ( $borrowernumber, $item, $exemptfine, $dropbox ) = @_;
     my $dbh = C4::Context->dbh;
 
     # check for overdue fine
@@ -1281,14 +1453,35 @@ sub FixOverduesOnReturn {
     $sth->execute( $borrowernumber, $item );
 
     # alter fine to show that the book has been returned
-    if ( my $data = $sth->fetchrow_hashref ) {
-        my $usth =
-          $dbh->prepare(
-"UPDATE accountlines SET accounttype='F' WHERE (borrowernumber = ?) AND (itemnumber = ?) AND (accountno = ?)"
-          );
-        $usth->execute( $borrowernumber, $item, $data->{'accountno'} );
+   my $data; 
+       if ($data = $sth->fetchrow_hashref) {
+        my $uquery;
+               my @bind = ($borrowernumber,$item ,$data->{'accountno'});
+               if ($exemptfine) {
+                       $uquery = "update accountlines set accounttype='FFOR', amountoutstanding=0";
+                       if (C4::Context->preference("FinesLog")) {
+                       &logaction("FINES", 'MODIFY',$borrowernumber,"Overdue forgiven: item $item");
+                       }
+               } elsif ($dropbox && $data->{lastincrement}) {
+                       my $outstanding = $data->{amountoutstanding} - $data->{lastincrement} ;
+                       my $amt = $data->{amount} - $data->{lastincrement} ;
+                       if (C4::Context->preference("FinesLog")) {
+                       &logaction("FINES", 'MODIFY',$borrowernumber,"Dropbox adjustment $amt, item $item");
+                       }
+                        $uquery = "update accountlines set accounttype='F' ";
+                        if($outstanding  >= 0 && $amt >=0) {
+                               $uquery .= ", amount = ? , amountoutstanding=? ";
+                               unshift @bind, ($amt, $outstanding) ;
+                       }
+               } else {
+                       $uquery = "update accountlines set accounttype='F' ";
+               }
+               $uquery .= " where (borrowernumber = ?) and (itemnumber = ?) and (accountno = ?)";
+        my $usth = $dbh->prepare($uquery);
+        $usth->execute(@bind);
         $usth->finish();
     }
+
     $sth->finish();
     return;
 }
@@ -1309,7 +1502,6 @@ Internal function, called by AddReturn
 
 sub FixAccountForLostAndReturned {
        my ($iteminfo, $borrower) = @_;
-       my %env;
        my $dbh = C4::Context->dbh;
        my $itm = $iteminfo->{'itemnumber'};
        # check for charge made for lost book
@@ -1334,7 +1526,7 @@ sub FixAccountForLostAndReturned {
                $usth->execute($data->{'borrowernumber'},$itm,$acctno);
                $usth->finish;
        #check if any credit is left if so writeoff other accounts
-               my $nextaccntno = getnextacctno(\%env,$data->{'borrowernumber'},$dbh);
+               my $nextaccntno = getnextacctno($data->{'borrowernumber'});
                if ($amountleft < 0){
                $amountleft*=-1;
                }
@@ -1371,7 +1563,7 @@ sub FixAccountForLostAndReturned {
                if ($amountleft > 0){
                        $amountleft*=-1;
                }
-               my $desc="Book Returned ".$iteminfo->{'barcode'};
+               my $desc="Item Returned ".$iteminfo->{'barcode'};
                $usth = $dbh->prepare("INSERT INTO accountlines
                        (borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding)
                        VALUES (?,?,now(),?,?,'CR',?)");
@@ -1382,9 +1574,7 @@ sub FixAccountForLostAndReturned {
                        VALUES (?,?,?,?)");
                $usth->execute($borrower->{'borrowernumber'},$data->{'accountno'},$nextaccntno,$offset);
                $usth->finish;
-               $usth = $dbh->prepare("UPDATE items SET paidfor='' WHERE itemnumber=?");
-               $usth->execute($itm);
-               $usth->finish;
+        ModItem({ paidfor => '' }, undef, $itm);
        }
        $sth->finish;
        return;
@@ -1399,6 +1589,7 @@ Returns patrons currently having a book. nothing if item is not issued atm
 C<$itemnumber> is the itemnumber
 
 Returns an array of hashes
+
 =cut
 
 sub GetItemIssue {
@@ -1414,7 +1605,7 @@ sub GetItemIssue {
         "SELECT * FROM issues 
         LEFT JOIN items ON issues.itemnumber=items.itemnumber
     WHERE
-    issues.itemnumber=?  AND returndate IS NULL ");
+    issues.itemnumber=?");
     $sth->execute($itemnumber);
     my $data = $sth->fetchrow_hashref;
     my $datedue = $data->{'date_due'};
@@ -1437,6 +1628,7 @@ C<$itemnumber> is the itemnumber
 C<$history> is 0 if you want actuel "issuer" (if it exist) and 1 if you want issues history
 
 Returns an array of hashes
+
 =cut
 
 sub GetItemIssues {
@@ -1447,15 +1639,24 @@ sub GetItemIssues {
     # get today date
     my $today = POSIX::strftime("%Y%m%d", localtime);
 
-    my $sth = $dbh->prepare(
-        "SELECT * FROM issues 
-        LEFT JOIN borrowers ON borrowers.borrowernumber 
-        LEFT JOIN items ON items.itemnumber=issues.itemnumber 
-    WHERE
-    issues.itemnumber=?".($history?"":" AND returndate IS NULL ").
-    "ORDER BY issues.date_due DESC"
-    );
-    $sth->execute($itemnumber);
+    my $sql = "SELECT * FROM issues 
+              JOIN borrowers USING (borrowernumber)
+              JOIN items USING (itemnumber)
+              WHERE issues.itemnumber = ? ";
+    if ($history) {
+        $sql .= "UNION ALL
+                 SELECT * FROM old_issues 
+                 LEFT JOIN borrowers USING (borrowernumber)
+                 JOIN items USING (itemnumber)
+                 WHERE old_issues.itemnumber = ? ";
+    }
+    $sql .= "ORDER BY date_due DESC";
+    my $sth = $dbh->prepare($sql);
+    if ($history) {
+        $sth->execute($itemnumber, $itemnumber);
+    } else {
+        $sth->execute($itemnumber);
+    }
     while ( my $data = $sth->fetchrow_hashref ) {
         my $datedue = $data->{'date_due'};
         $datedue =~ s/-//g;
@@ -1493,10 +1694,18 @@ sub GetBiblioIssues {
             LEFT JOIN biblioitems ON items.itemnumber = biblioitems.biblioitemnumber
             LEFT JOIN biblio ON biblio.biblionumber = items.biblioitemnumber
         WHERE biblio.biblionumber = ?
-        ORDER BY issues.timestamp
+        UNION ALL
+        SELECT old_issues.*,items.barcode,biblio.biblionumber,biblio.title, biblio.author,borrowers.cardnumber,borrowers.surname,borrowers.firstname
+        FROM old_issues
+            LEFT JOIN borrowers ON borrowers.borrowernumber = old_issues.borrowernumber
+            LEFT JOIN items ON old_issues.itemnumber = items.itemnumber
+            LEFT JOIN biblioitems ON items.itemnumber = biblioitems.biblioitemnumber
+            LEFT JOIN biblio ON biblio.biblionumber = items.biblioitemnumber
+        WHERE biblio.biblionumber = ?
+        ORDER BY timestamp
     ";
     my $sth = $dbh->prepare($query);
-    $sth->execute($biblionumber);
+    $sth->execute($biblionumber, $biblionumber);
 
     my @issues;
     while ( my $data = $sth->fetchrow_hashref ) {
@@ -1507,7 +1716,7 @@ sub GetBiblioIssues {
 
 =head2 CanBookBeRenewed
 
-$ok = &CanBookBeRenewed($borrowernumber, $itemnumber);
+($ok,$error) = &CanBookBeRenewed($borrowernumber, $itemnumber);
 
 Find out whether a borrowed item may be renewed.
 
@@ -1521,7 +1730,7 @@ C<$itemnumber> is the number of the item to renew.
 C<$CanBookBeRenewed> returns a true value iff the item may be renewed. The
 item must currently be on loan to the specified borrower; renewals
 must be allowed for the item's type; and the borrower must not have
-already renewed the loan.
+already renewed the loan. $error will contain the reason the renewal can not proceed
 
 =cut
 
@@ -1532,6 +1741,7 @@ sub CanBookBeRenewed {
     my $dbh       = C4::Context->dbh;
     my $renews    = 1;
     my $renewokay = 0;
+       my $error;
 
     # Look in the issues table for this item, lent to this borrower,
     # and not yet returned.
@@ -1540,8 +1750,7 @@ sub CanBookBeRenewed {
     my $sth1 = $dbh->prepare(
         "SELECT * FROM issues
             WHERE borrowernumber = ?
-            AND itemnumber = ?
-            AND returndate IS NULL"
+            AND itemnumber = ?"
     );
     $sth1->execute( $borrowernumber, $itemnumber );
     if ( my $data1 = $sth1->fetchrow_hashref ) {
@@ -1552,34 +1761,38 @@ sub CanBookBeRenewed {
         # because it's a bit messy: given the item number, we need to find
         # the biblioitem, which gives us the itemtype, which tells us
         # whether it may be renewed.
-        my $sth2 = $dbh->prepare(
-            "SELECT renewalsallowed FROM items
-                LEFT JOIN biblioitems on items.biblioitemnumber = biblioitems.biblioitemnumber
-                LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype
-                WHERE items.itemnumber = ?
-                "
-        );
+        my $query = "SELECT renewalsallowed FROM items ";
+        $query .= (C4::Context->preference('item-level_itypes'))
+                    ? "LEFT JOIN itemtypes ON items.itype = itemtypes.itemtype "
+                    : "LEFT JOIN biblioitems on items.biblioitemnumber = biblioitems.biblioitemnumber
+                       LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype ";
+        $query .= "WHERE items.itemnumber = ?";
+        my $sth2 = $dbh->prepare($query);
         $sth2->execute($itemnumber);
         if ( my $data2 = $sth2->fetchrow_hashref ) {
             $renews = $data2->{'renewalsallowed'};
         }
-        if ( $renews && $renews >= $data1->{'renewals'} ) {
+        if ( $renews && $renews > $data1->{'renewals'} ) {
             $renewokay = 1;
         }
+        else {
+                       $error="too_many";
+               }
         $sth2->finish;
         my ( $resfound, $resrec ) = C4::Reserves::CheckReserves($itemnumber);
         if ($resfound) {
             $renewokay = 0;
+                       $error="on_reserve"
         }
 
     }
     $sth1->finish;
-    return ($renewokay);
+    return ($renewokay,$error);
 }
 
 =head2 AddRenewal
 
-&AddRenewal($borrowernumber, $itemnumber, $datedue);
+&AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue]);
 
 Renews a loan.
 
@@ -1588,43 +1801,43 @@ has the item.
 
 C<$itemnumber> is the number of the item to renew.
 
-C<$datedue> can be used to set the due date. If C<$datedue> is the
-empty string, C<&AddRenewal> will calculate the due date automatically
-from the book's item type. If you wish to set the due date manually,
-C<$datedue> should be in the form YYYY-MM-DD.
+C<$branch> is the library branch.  Defaults to the homebranch of the ITEM.
 
-=cut
+C<$datedue> can be a C4::Dates object used to set the due date.
 
-sub AddRenewal {
+If C<$datedue> is the empty string, C<&AddRenewal> will calculate the due date automatically
+from the book's item type.
 
-    my ( $borrowernumber, $itemnumber, $branch ,$datedue ) = @_;
-    my $dbh = C4::Context->dbh;
+=cut
 
+sub AddRenewal {
+       my $borrowernumber = shift or return undef;
+       my     $itemnumber = shift or return undef;
+    my $item   = GetItem($itemnumber) or return undef;
+    my $biblio = GetBiblioFromItemNumber($itemnumber) or return undef;
+    my $branch  = (@_) ? shift : $item->{homebranch};  # opac-renew doesn't send branch
+    my $datedue;
     # If the due date wasn't specified, calculate it by adding the
     # book's loan length to today's date.
-    if ( $datedue eq "" ) {
+    unless (@_ and $datedue = shift and $datedue->output('iso')) {
 
-        my $biblio = GetBiblioFromItemNumber($itemnumber);
-        my $borrower = C4::Members::GetMemberDetails( $borrowernumber, 0 );
+        my $borrower = C4::Members::GetMemberDetails( $borrowernumber, 0 ) or return undef;
         my $loanlength = GetLoanLength(
             $borrower->{'categorycode'},
              (C4::Context->preference('item-level_itypes')) ? $biblio->{'itype'} : $biblio->{'itemtype'} ,
-                       $borrower->{'branchcode'}
+                       $item->{homebranch}                     # item's homebranch determines loanlength OR do we want the branch specified by the AddRenewal argument?
         );
-               #FIXME --  choose issuer or borrower branch.
-               #FIXME -- where's the calendar ?
-        my ( $due_year, $due_month, $due_day ) =
-          Add_Delta_DHMS( Today_and_Now(), $loanlength, 0, 0, 0 );
-        $datedue = "$due_year-$due_month-$due_day";
-       $datedue=CheckValidDatedue($datedue,$itemnumber,$branch);
+               #FIXME -- use circControl?
+               $datedue =  CalcDateDue(C4::Dates->new(),$loanlength,$branch);  # this branch is the transactional branch.
+                                                               # The question of whether to use item's homebranch calendar is open.
     }
 
+    my $dbh = C4::Context->dbh;
     # Find the issues record for this book
     my $sth =
       $dbh->prepare("SELECT * FROM issues
                         WHERE borrowernumber=? 
-                        AND itemnumber=? 
-                        AND returndate IS NULL"
+                        AND itemnumber=?"
       );
     $sth->execute( $borrowernumber, $itemnumber );
     my $issuedata = $sth->fetchrow_hashref;
@@ -1635,14 +1848,14 @@ sub AddRenewal {
     my $renews = $issuedata->{'renewals'} + 1;
     $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?
                             WHERE borrowernumber=? 
-                            AND itemnumber=? 
-                            AND returndate IS NULL"
+                            AND itemnumber=?"
     );
-    $sth->execute( $datedue, $renews, $borrowernumber, $itemnumber );
+    $sth->execute( $datedue->output('iso'), $renews, $borrowernumber, $itemnumber );
     $sth->finish;
 
-    # Log the renewal
-
+    # Update the renewal count on the item, and tell zebra to reindex
+    $renews = $biblio->{'renewals'} + 1;
+    ModItem({ renewals => $renews }, $biblio->{'biblionumber'}, $itemnumber);
 
     # Charge a new rental fee, if applicable?
     my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
@@ -1651,17 +1864,52 @@ sub AddRenewal {
         my $item = GetBiblioFromItemNumber($itemnumber);
         $sth = $dbh->prepare(
                 "INSERT INTO accountlines
-                    (borrowernumber,accountno,date,amount,
-                        description,accounttype,amountoutstanding,
-                    itemnumber)
-                    VALUES (?,?,now(),?,?,?,?,?)"
+                    (date,
+                                       borrowernumber, accountno, amount,
+                    description,
+                                       accounttype, amountoutstanding, itemnumber
+                                       )
+                    VALUES (now(),?,?,?,?,?,?,?)"
         );
         $sth->execute( $borrowernumber, $accountno, $charge,
             "Renewal of Rental Item $item->{'title'} $item->{'barcode'}",
             'Rent', $charge, $itemnumber );
         $sth->finish;
     }
-    UpdateStats( C4::Context->userenv->{'branchcode'}, 'renew', $charge, '', $itemnumber );
+    # Log the renewal
+    UpdateStats( $branch, 'renew', $charge, '', $itemnumber, $item->{itype}, $borrowernumber);
+}
+
+sub GetRenewCount {
+    # check renewal status
+    my ($bornum,$itemno)=@_;
+    my $dbh = C4::Context->dbh;
+    my $renewcount = 0;
+        my $renewsallowed = 0;
+        my $renewsleft = 0;
+    # Look in the issues table for this item, lent to this borrower,
+    # and not yet returned.
+
+    # FIXME - I think this function could be redone to use only one SQL call.
+    my $sth = $dbh->prepare("select * from issues
+                                where (borrowernumber = ?)
+                                and (itemnumber = ?)");
+    $sth->execute($bornum,$itemno);
+    my $data = $sth->fetchrow_hashref;
+    $renewcount = $data->{'renewals'} if $data->{'renewals'};
+    $sth->finish;
+    my $query = "SELECT renewalsallowed FROM items ";
+    $query .= (C4::Context->preference('item-level_itypes'))
+                ? "LEFT JOIN itemtypes ON items.itype = itemtypes.itemtype "
+                : "LEFT JOIN biblioitems on items.biblioitemnumber = biblioitems.biblioitemnumber
+                   LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype ";
+    $query .= "WHERE items.itemnumber = ?";
+    my $sth2 = $dbh->prepare($query);
+    $sth2->execute($itemno);
+    my $data2 = $sth2->fetchrow_hashref();
+    $renewsallowed = $data2->{'renewalsallowed'};
+    $renewsleft = $renewsallowed - $renewcount;
+    return ($renewcount,$renewsallowed,$renewsleft);
 }
 
 =head2 GetIssuingCharges
@@ -1836,7 +2084,7 @@ sub AnonymiseIssueHistory {
     my $borrowernumber = shift;
     my $dbh            = C4::Context->dbh;
     my $query          = "
-        UPDATE issues
+        UPDATE old_issues
         SET    borrowernumber = NULL
         WHERE  returndate < '".$date."'
           AND borrowernumber IS NOT NULL
@@ -1876,54 +2124,78 @@ sub updateWrongTransfer {
 
 $items = UpdateHoldingbranch($branch,$itmenumber);
 Simple methode for updating hodlingbranch in items BDD line
+
 =cut
 
 sub UpdateHoldingbranch {
-       my ( $branch,$itmenumber ) = @_;
-       my $dbh = C4::Context->dbh;     
-# first step validate the actual line of transfert .
-       my $sth =
-               $dbh->prepare(
-                       "update items set holdingbranch = ? where itemnumber= ?"
-               );
-               $sth->execute($branch,$itmenumber);
-               $sth->finish;
-        
-       
+       my ( $branch,$itemnumber ) = @_;
+    ModItem({ holdingbranch => $branch }, undef, $itemnumber);
+}
+
+=head2 CalcDateDue
+
+$newdatedue = CalcDateDue($startdate,$loanlength,$branchcode);
+this function calculates the due date given the loan length ,
+checking against the holidays calendar as per the 'useDaysMode' syspref.
+C<$startdate>   = C4::Dates object representing start date of loan period (assumed to be today)
+C<$branch>  = location whose calendar to use
+C<$loanlength>  = loan length prior to adjustment
+=cut
+
+sub CalcDateDue { 
+       my ($startdate,$loanlength,$branch) = @_;
+       if(C4::Context->preference('useDaysMode') eq 'Days') {  # ignoring calendar
+               my $datedue = time + ($loanlength) * 86400;
+       #FIXME - assumes now even though we take a startdate 
+               my @datearr  = localtime($datedue);
+               return C4::Dates->new( sprintf("%04d-%02d-%02d", 1900 + $datearr[5], $datearr[4] + 1, $datearr[3]), 'iso');
+       } else {
+               my $calendar = C4::Calendar->new(  branchcode => $branch );
+               my $datedue = $calendar->addDate($startdate, $loanlength);
+               return $datedue;
+       }
 }
+
 =head2 CheckValidDatedue
+       This function does not account for holiday exceptions nor does it handle the 'useDaysMode' syspref .
+       To be replaced by CalcDateDue() once C4::Calendar use is tested.
 
 $newdatedue = CheckValidDatedue($date_due,$itemnumber,$branchcode);
-this function return a new date due after checked if it's a repeatable or special holiday
+this function validates the loan length against the holidays calendar, and adjusts the due date as per the 'useDaysMode' syspref.
 C<$date_due>   = returndate calculate with no day check
 C<$itemnumber>  = itemnumber
-C<$branchcode>  = localisation of issue 
+C<$branchcode>  = location of issue (affected by 'CircControl' syspref)
+C<$loanlength>  = loan length prior to adjustment
 =cut
-sub CheckValidDatedue{
+
+sub CheckValidDatedue {
 my ($date_due,$itemnumber,$branchcode)=@_;
-my @datedue=split('-',$date_due);
+my @datedue=split('-',$date_due->output('iso'));
 my $years=$datedue[0];
 my $month=$datedue[1];
 my $day=$datedue[2];
+# die "Item# $itemnumber ($branchcode) due: " . ${date_due}->output() . "\n(Y,M,D) = ($years,$month,$day)":
 my $dow;
 for (my $i=0;$i<2;$i++){
-       $dow=Day_of_Week($years,$month,$day);
-       ($dow=0) if ($dow>6);
-       my $result=CheckRepeatableHolidays($itemnumber,$dow,$branchcode);
-       my $countspecial=CheckSpecialHolidays($years,$month,$day,$itemnumber,$branchcode);
-       my $countspecialrepeatable=CheckRepeatableSpecialHolidays($month,$day,$itemnumber,$branchcode);
-               if (($result ne '0') or ($countspecial ne '0') or ($countspecialrepeatable ne '0') ){
-               $i=0;
-               (($years,$month,$day) = Add_Delta_Days($years,$month,$day, 1))if ($i ne '1');
-               }
-       }
-my $newdatedue=$years."-".$month."-".$day;
+    $dow=Day_of_Week($years,$month,$day);
+    ($dow=0) if ($dow>6);
+    my $result=CheckRepeatableHolidays($itemnumber,$dow,$branchcode);
+    my $countspecial=CheckSpecialHolidays($years,$month,$day,$itemnumber,$branchcode);
+    my $countspecialrepeatable=CheckRepeatableSpecialHolidays($month,$day,$itemnumber,$branchcode);
+        if (($result ne '0') or ($countspecial ne '0') or ($countspecialrepeatable ne '0') ){
+        $i=0;
+        (($years,$month,$day) = Add_Delta_Days($years,$month,$day, 1))if ($i ne '1');
+        }
+    }
+    my $newdatedue=C4::Dates->new(sprintf("%04d-%02d-%02d",$years,$month,$day),'iso');
 return $newdatedue;
 }
+
+
 =head2 CheckRepeatableHolidays
 
 $countrepeatable = CheckRepeatableHoliday($itemnumber,$week_day,$branchcode);
-this function check if the date due is a repeatable holiday
+this function checks if the date due is a repeatable holiday
 C<$date_due>   = returndate calculate with no day check
 C<$itemnumber>  = itemnumber
 C<$branchcode>  = localisation of issue 
@@ -1954,7 +2226,9 @@ C<$month>   = the month of datedue
 C<$day>     = the day of datedue
 C<$itemnumber>  = itemnumber
 C<$branchcode>  = localisation of issue 
+
 =cut
+
 sub CheckSpecialHolidays{
 my ($years,$month,$day,$itemnumber,$branchcode) = @_;
 my $dbh = C4::Context->dbh;
@@ -1980,7 +2254,9 @@ C<$month>   = the month of datedue
 C<$day>     = the day of datedue
 C<$itemnumber>  = itemnumber
 C<$branchcode>  = localisation of issue 
+
 =cut
+
 sub CheckRepeatableSpecialHolidays{
 my ($month,$day,$itemnumber,$branchcode) = @_;
 my $dbh = C4::Context->dbh;