BugFixing : 3306

[koha.git] / C4 / Circulation.pm

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 1535037..7c9fe89 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -109,7 +109,7 @@ Also deals with stocktaking.
 
 =head2 barcodedecode
 
 
 =head2 barcodedecode
 

-=head3 $str = &barcodedecode($barcode);
+=head3 $str = &barcodedecode($barcode, [$filter]);

 
 =over 4
 
 
 =over 4
 


@@ -120,6 +120,10 @@ 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.
 
 For proper functioning of this filter, calling the function on the 
 correct barcode string (items.barcode) should return an unaltered barcode.
 

+The optional $filter argument is to allow for testing or explicit 
+behavior that ignores the System Pref.  Valid values are the same as the 
+System Pref options.
+

 =back
 
 =cut
 =back
 
 =cut


@@ -128,31 +132,27 @@ correct barcode string (items.barcode) should return an unaltered barcode.
 # FIXME -- these plugins should be moved out of Circulation.pm
 #
 sub barcodedecode {
 # FIXME -- these plugins should be moved out of Circulation.pm
 #
 sub barcodedecode {

-    my ($barcode) = @_;
-    my $filter = C4::Context->preference('itemBarcodeInputFilter');
-       if($filter eq 'whitespace') {
+    my ($barcode, $filter) = @_;
+    $filter = C4::Context->preference('itemBarcodeInputFilter') unless $filter;
+    $filter or return $barcode;     # ensure filter is defined, else return untouched barcode
+       if ($filter eq 'whitespace') {

                $barcode =~ s/\s//g;
                $barcode =~ s/\s//g;

-               return $barcode;
-       } elsif($filter eq 'cuecat') {
+       } elsif ($filter eq 'cuecat') {

                chomp($barcode);
            my @fields = split( /\./, $barcode );
            my @results = map( decode($_), @fields[ 1 .. $#fields ] );
                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 ;
-                       }
+           ($#results == 2) and return $results[2];
+       } elsif ($filter eq 'T-prefix') {
+               if ($barcode =~ /^[Tt](\d)/) {
+                       (defined($1) and $1 eq '0') and return $barcode;
+            $barcode = substr($barcode, 2) + 0;     # FIXME: probably should be substr($barcode, 1)

                }
                }

-               return sprintf( "T%07d",$barcode);
+        return sprintf("T%07d", $barcode);
+        # FIXME: $barcode could be "T1", causing warning: substr outside of string
+        # Why drop the nonzero digit after the T?
+        # Why pass non-digits (or empty string) to "T%07d"?

        }
        }

+    return $barcode;    # return barcode, modified or not

 }
 
 =head2 decode
 }
 
 =head2 decode


@@ -164,6 +164,9 @@ sub barcodedecode {
 =item Decodes a segment of a string emitted by a CueCat barcode scanner and
 returns it.
 
 =item Decodes a segment of a string emitted by a CueCat barcode scanner and
 returns it.
 

+FIXME: Should be replaced with Barcode::Cuecat from CPAN
+or Javascript based decoding on the client side.
+

 =back
 
 =cut
 =back
 
 =cut


@@ -176,7 +179,7 @@ sub decode {
     my $l = ( $#s + 1 ) % 4;
     if ($l) {
         if ( $l == 1 ) {
     my $l = ( $#s + 1 ) % 4;
     if ($l) {
         if ( $l == 1 ) {

-            warn "Error!";
+            # warn "Error: Cuecat decode parsing failed!";

             return;
         }
         $l = 4 - $l;
             return;
         }
         $l = 4 - $l;


@@ -315,16 +318,7 @@ sub TooMany {
     my $dbh             = C4::Context->dbh;
        my $branch;
        # Get which branchcode we need
     my $dbh             = C4::Context->dbh;
        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'};
-       }
+       $branch = _GetCircControlBranch($item,$borrower);

        my $type = (C4::Context->preference('item-level_itypes')) 
                        ? $item->{'itype'}         # item-level
                        : $item->{'itemtype'};     # biblio-level
        my $type = (C4::Context->preference('item-level_itypes')) 
                        ? $item->{'itype'}         # item-level
                        : $item->{'itemtype'};     # biblio-level


@@ -645,10 +639,27 @@ sub CanBookBeIssued {
        $item->{'itemtype'}=$item->{'itype'}; 
     my $dbh             = C4::Context->dbh;
 
        $item->{'itemtype'}=$item->{'itype'}; 
     my $dbh             = C4::Context->dbh;
 

+    # MANDATORY CHECKS - unless item exists, nothing else matters
+    unless ( $item->{barcode} ) {
+        $issuingimpossible{UNKNOWN_BARCODE} = 1;
+    }
+       return ( \%issuingimpossible, \%needsconfirmation ) if %issuingimpossible;
+

     #
     # DUE DATE is OK ? -- should already have checked.
     #
     #
     # DUE DATE is OK ? -- should already have checked.
     #

-    #$issuingimpossible{INVALID_DATE} = 1 unless ($duedate);
+    unless ( $duedate ) {
+        my $issuedate = strftime( "%Y-%m-%d", localtime );
+
+        my $branch = _GetCircControlBranch($item,$borrower);
+        my $itype = ( C4::Context->preference('item-level_itypes') ) ? $item->{'itype'} : $biblioitem->{'itemtype'};
+        my $loanlength = GetLoanLength( $borrower->{'categorycode'}, $itype, $branch );
+        $duedate = CalcDateDue( C4::Dates->new( $issuedate, 'iso' ), $loanlength, $branch, $borrower );
+
+        # Offline circ calls AddIssue directly, doesn't run through here
+        #  So issuingimpossible should be ok.
+    }
+    $issuingimpossible{INVALID_DATE} = $duedate->output('syspref') unless ( $duedate && $duedate->output('iso') ge C4::Dates->today('iso') );

 
     #
     # BORROWER STATUS
 
     #
     # BORROWER STATUS


@@ -698,11 +709,25 @@ sub CanBookBeIssued {
         }
     }
 
         }
     }
 

+    my ($blocktype, $count) = C4::Members::IsMemberBlocked($borrower->{'borrowernumber'});
+    if($blocktype == -1){
+        ## remaining overdue documents
+        $needsconfirmation{USERBLOCKEDREMAINING} = $count;
+    }elsif($blocktype == 1){
+        ## blocked because of overdue return
+        $issuingimpossible{USERBLOCKEDOVERDUE} = $count;
+    }
+

     #
     # JB34 CHECKS IF BORROWERS DONT HAVE ISSUE TOO MANY BOOKS
     #
        my $toomany = TooMany( $borrower, $item->{biblionumber}, $item );
     #
     # JB34 CHECKS IF BORROWERS DONT HAVE ISSUE TOO MANY BOOKS
     #
        my $toomany = TooMany( $borrower, $item->{biblionumber}, $item );

-    $needsconfirmation{TOO_MANY} = $toomany if $toomany;
+    # if TooMany return / 0, then the user has no permission to check out this book
+    if ($toomany =~ /\/ 0/) {
+        $needsconfirmation{PATRON_CANT} = 1;
+    } else {
+        $needsconfirmation{TOO_MANY} = $toomany if $toomany;
+    }

 
     #
     # ITEM CHECKING
 
     #
     # ITEM CHECKING


@@ -713,24 +738,36 @@ sub CanBookBeIssued {
     if (   $item->{'notforloan'}
         && $item->{'notforloan'} > 0 )
     {
     if (   $item->{'notforloan'}
         && $item->{'notforloan'} > 0 )
     {

-        $issuingimpossible{NOT_FOR_LOAN} = 1;
+        if(C4::Context->preference("AllowNotForLoanOverride")){
+           $needsconfirmation{NOT_FOR_LOAN_FORCING} = 1;
+        }else{
+            $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'}) {
+                if (!C4::Context->preference("AllowNotForLoanOverride")) {
+                    $issuingimpossible{NOT_FOR_LOAN} = 1;
+                } else {
+                    $needsconfirmation{NOT_FOR_LOAN_FORCING} = 1;
+                }
+            }
+        }
+        elsif ($biblioitem->{'notforloan'} == 1){
+            if (!C4::Context->preference("AllowNotForLoanOverride")) {
+                $issuingimpossible{NOT_FOR_LOAN} = 1;
+            } else {
+                $needsconfirmation{NOT_FOR_LOAN_FORCING} = 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;
     if ( $item->{'wthdrawn'} && $item->{'wthdrawn'} == 1 )
     {
         $issuingimpossible{WTHDRAWN} = 1;


@@ -770,7 +807,7 @@ sub CanBookBeIssued {
     elsif ($issue->{borrowernumber}) {
 
         # issued to someone else
     elsif ($issue->{borrowernumber}) {
 
         # issued to someone else

-        my $currborinfo = GetMemberDetails( $issue->{borrowernumber} );
+        my $currborinfo =    C4::Members::GetMemberDetails( $issue->{borrowernumber} );

 
 #        warn "=>.$currborinfo->{'firstname'} $currborinfo->{'surname'} ($currborinfo->{'cardnumber'})";
         $needsconfirmation{ISSUED_TO_ANOTHER} =
 
 #        warn "=>.$currborinfo->{'firstname'} $currborinfo->{'surname'} ($currborinfo->{'cardnumber'})";
         $needsconfirmation{ISSUED_TO_ANOTHER} =


@@ -797,12 +834,6 @@ sub CanBookBeIssued {
 "$res->{'reservedate'} : $resborrower->{'firstname'} $resborrower->{'surname'} ($resborrower->{'cardnumber'})";
         }
     }
 "$res->{'reservedate'} : $resborrower->{'firstname'} $resborrower->{'surname'} ($resborrower->{'cardnumber'})";
         }
     }

-    if ( C4::Context->preference("LibraryName") eq "Horowhenua Library Trust" ) {
-        if ( $borrower->{'categorycode'} eq 'W' ) {
-            my %emptyhash;
-            return ( \%emptyhash, \%needsconfirmation );
-        }
-       }

        return ( \%issuingimpossible, \%needsconfirmation );
 }
 
        return ( \%issuingimpossible, \%needsconfirmation );
 }
 


@@ -857,9 +888,8 @@ sub AddIssue {
        if ($borrower and $barcode and $barcodecheck ne '0'){
                # find which item we issue
                my $item = GetItem('', $barcode) or return undef;       # if we don't get an Item, abort.
        if ($borrower and $barcode and $barcodecheck ne '0'){
                # find which item we issue
                my $item = GetItem('', $barcode) or return undef;       # if we don't get an Item, abort.

-               my $branch = (C4::Context->preference('CircControl') eq 'PickupLibrary') ? C4::Context->userenv->{'branch'} :
-                     (C4::Context->preference('CircControl') eq 'PatronLibrary') ? $borrower->{'branchcode'}        : 
-                     $item->{'homebranch'};     # fallback to item's homebranch
+               my $hbr = C4::Context->preference("HomeOrHoldingBranch")||"homebranch";
+               my $branch = _GetCircControlBranch($item,$borrower);

                
                # get actual issuing if there is one
                my $actualissue = GetItemIssue( $item->{itemnumber});
                
                # get actual issuing if there is one
                my $actualissue = GetItemIssue( $item->{itemnumber});


@@ -949,7 +979,7 @@ sub AddIssue {
         unless ($datedue) {
             my $itype = ( C4::Context->preference('item-level_itypes') ) ? $biblio->{'itype'} : $biblio->{'itemtype'};
             my $loanlength = GetLoanLength( $borrower->{'categorycode'}, $itype, $branch );
         unless ($datedue) {
             my $itype = ( C4::Context->preference('item-level_itypes') ) ? $biblio->{'itype'} : $biblio->{'itemtype'};
             my $loanlength = GetLoanLength( $borrower->{'categorycode'}, $itype, $branch );

-            $datedue = CalcDateDue( C4::Dates->new( $issuedate, 'iso' ), $loanlength, $branch );
+            $datedue = CalcDateDue( C4::Dates->new( $issuedate, 'iso' ), $loanlength, $branch, $borrower );

 
             # if ReturnBeforeExpiry ON the datedue can't be after borrower expirydate
             if ( C4::Context->preference('ReturnBeforeExpiry') && $datedue->output('iso') gt $borrower->{dateexpiry} ) {
 
             # if ReturnBeforeExpiry ON the datedue can't be after borrower expirydate
             if ( C4::Context->preference('ReturnBeforeExpiry') && $datedue->output('iso') gt $borrower->{dateexpiry} ) {


@@ -961,12 +991,11 @@ sub AddIssue {
             $item->{'itemnumber'},              # itemnumber
             $issuedate,                         # issuedate
             $datedue->output('iso'),            # date_due
             $item->{'itemnumber'},              # itemnumber
             $issuedate,                         # issuedate
             $datedue->output('iso'),            # date_due

-            C4::Context->userenv->{'branch'}    # branchcode
+            $branch                                                            # branchcode

         );
         );

-        $sth->finish;

         $item->{'issues'}++;
         ModItem({ issues           => $item->{'issues'},
         $item->{'issues'}++;
         ModItem({ issues           => $item->{'issues'},

-                  holdingbranch    => C4::Context->userenv->{'branch'},
+                  holdingbranch    => C4::Context->userenv->{branch},

                   itemlost         => 0,
                   datelastborrowed => C4::Dates->new()->output('iso'),
                   onloan           => $datedue->output('iso'),
                   itemlost         => 0,
                   datelastborrowed => C4::Dates->new()->output('iso'),
                   onloan           => $datedue->output('iso'),


@@ -1264,6 +1293,9 @@ either C<Waiting>, C<Reserved>, or 0.
 
 =back
 
 
 =back
 

+C<$iteminformation> is a reference-to-hash, giving information about the
+returned item from the issues table.
+

 C<$borrower> is a reference-to-hash, giving information about the
 patron who last borrowed the book.
 
 C<$borrower> is a reference-to-hash, giving information about the
 patron who last borrowed the book.
 


@@ -1277,26 +1309,30 @@ sub AddReturn {
     my $borrower;
     my $validTransfert = 0;
     my $reserveDone = 0;
     my $borrower;
     my $validTransfert = 0;
     my $reserveDone = 0;

+       $branch ||=C4::Context->userenv->{'branch'};

     
     # get information on item
     
     # get information on item

-    my $iteminformation = GetItemIssue( GetItemnumberFromBarcode($barcode));
+    my $itemnumber = GetItemnumberFromBarcode($barcode);
+    my $iteminformation = GetItemIssue( $itemnumber );

     my $biblio = GetBiblioItemData($iteminformation->{'biblioitemnumber'});
 #     use Data::Dumper;warn Data::Dumper::Dumper($iteminformation);  
     my $biblio = GetBiblioItemData($iteminformation->{'biblioitemnumber'});
 #     use Data::Dumper;warn Data::Dumper::Dumper($iteminformation);  

-    unless ($iteminformation->{'itemnumber'} ) {
+    unless ( $iteminformation->{'itemnumber'} or $itemnumber) {

         $messages->{'BadBarcode'} = $barcode;
         $doreturn = 0;
     } else {
         # find the borrower
         $messages->{'BadBarcode'} = $barcode;
         $doreturn = 0;
     } else {
         # find the borrower

-        if ( ( not $iteminformation->{borrowernumber} ) && $doreturn ) {
+        if ( not $iteminformation->{borrowernumber} ) {

             $messages->{'NotIssued'} = $barcode;
             $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;
         }
             $doreturn = 0;
         }

-    
+        
+        # even though item is not on loan, it may still
+        # be transferred; therefore, get current branch information
+        my $curr_iteminfo = GetItem($itemnumber);
+        $iteminformation->{'homebranch'} = $curr_iteminfo->{'homebranch'};
+        $iteminformation->{'holdingbranch'} = $curr_iteminfo->{'holdingbranch'};
+        $iteminformation->{'itemlost'} = $curr_iteminfo->{'itemlost'};
+        

         # check if the book is in a permanent collection....
         my $hbr      = $iteminformation->{C4::Context->preference("HomeOrHoldingBranch")};
         my $branches = GetBranches();
         # check if the book is in a permanent collection....
         my $hbr      = $iteminformation->{C4::Context->preference("HomeOrHoldingBranch")};
         my $branches = GetBranches();


@@ -1306,7 +1342,7 @@ sub AddReturn {
         }
                
                    # if independent branches are on and returning to different branch, refuse the return
         }
                
                    # if independent branches are on and returning to different branch, refuse the return

-        if ($hbr ne C4::Context->userenv->{'branch'} && C4::Context->preference("IndependantBranches")){
+        if ($hbr ne $branch && C4::Context->preference("IndependantBranches") && $iteminformation->{borrowernumber}){

                          $messages->{'Wrongbranch'} = 1;
                          $doreturn=0;
                    }
                          $messages->{'Wrongbranch'} = 1;
                          $doreturn=0;
                    }


@@ -1317,6 +1353,7 @@ sub AddReturn {
             $doreturn = 0;
         }
     
             $doreturn = 0;
         }
     

+

     #     new op dev : if the book returned in an other branch update the holding branch
     
     # update issues, thereby returning book (should push this out into another subroutine
     #     new op dev : if the book returned in an other branch update the holding branch
     
     # update issues, thereby returning book (should push this out into another subroutine


@@ -1325,39 +1362,27 @@ sub AddReturn {
     # case of a return of document (deal with issues and holdingbranch)
     
         if ($doreturn) {
     # case of a return of document (deal with issues and holdingbranch)
     
         if ($doreturn) {

-                       my $circControlBranch;
+                       my $circControlBranch = _GetCircControlBranch($iteminformation,$borrower);

                        if($dropbox) {
                                # don't allow dropbox mode to create an invalid entry in issues (issuedate > returndate) FIXME: actually checks eq, not gt
                                undef($dropbox) if ( $iteminformation->{'issuedate'} eq C4::Dates->today('iso') );
                        if($dropbox) {
                                # don't allow dropbox mode to create an invalid entry in issues (issuedate > returndate) FIXME: actually checks eq, not gt
                                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
                        }
             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'};
-            }
+            
+            
+            # We update the holdingbranch from circControlBranch variable
+            UpdateHoldingbranch($branch,$iteminformation->{'itemnumber'});
+            $iteminformation->{'holdingbranch'} = $branch;
+        
+            

             ModDateLastSeen( $iteminformation->{'itemnumber'} );
             ModItem({ onloan => undef }, $biblio->{'biblionumber'}, $iteminformation->{'itemnumber'});
 
             if ($iteminformation->{borrowernumber}){
               ($borrower) = C4::Members::GetMemberDetails( $iteminformation->{borrowernumber}, 0 );
             }
             ModDateLastSeen( $iteminformation->{'itemnumber'} );
             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;

         }
     
     # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
         }
     
     # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #


@@ -1366,7 +1391,7 @@ sub AddReturn {
     
     #     if we have a transfer to do, we update the line of transfers with the datearrived
         if ($datesent) {
     
     #     if we have a transfer to do, we update the line of transfers with the datearrived
         if ($datesent) {

-            if ( $tobranch eq C4::Context->userenv->{'branch'} ) {
+            if ( $tobranch eq $branch ) {

                     my $sth =
                     $dbh->prepare(
                             "UPDATE branchtransfers SET datearrived = now() WHERE itemnumber= ? AND datearrived IS NULL"
                     my $sth =
                     $dbh->prepare(
                             "UPDATE branchtransfers SET datearrived = now() WHERE itemnumber= ? AND datearrived IS NULL"


@@ -1387,6 +1412,7 @@ sub AddReturn {
         # fix up the accounts.....
         if ($iteminformation->{'itemlost'}) {
                 FixAccountForLostAndReturned($iteminformation, $borrower);
         # fix up the accounts.....
         if ($iteminformation->{'itemlost'}) {
                 FixAccountForLostAndReturned($iteminformation, $borrower);

+                ModItem({ itemlost => '0' }, $biblio->{'biblionumber'}, $iteminformation->{'itemnumber'});

                 $messages->{'WasLost'} = 1;
         }
         # fix up the overdues in accounts...
                 $messages->{'WasLost'} = 1;
         }
         # fix up the overdues in accounts...


@@ -1395,8 +1421,8 @@ sub AddReturn {
     
     # find reserves.....
     #     if we don't have a reserve with the status W, we launch the Checkreserves routine
     
     # find reserves.....
     #     if we don't have a reserve with the status W, we launch the Checkreserves routine

-        my ( $resfound, $resrec ) =
-        C4::Reserves::CheckReserves( $iteminformation->{'itemnumber'} );
+        my ( $resfound, $resrec ) = 
+        C4::Reserves::CheckReserves( $itemnumber, $barcode );

         if ($resfound) {
             $resrec->{'ResFound'}   = $resfound;
             $messages->{'ResFound'} = $resrec;
         if ($resfound) {
             $resrec->{'ResFound'}   = $resfound;
             $messages->{'ResFound'} = $resrec;


@@ -1418,9 +1444,9 @@ sub AddReturn {
         #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 .
         
         #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 ($doreturn and ($iteminformation->{'holdingbranch'} ne $iteminformation->{'homebranch'}) and not $messages->{'WrongTransfer'} and ($validTransfert ne 1) and ($reserveDone ne 1) ){
+        if (($doreturn or $messages->{'NotIssued'}) and ($branch ne $iteminformation->{$hbr}) and not $messages->{'WrongTransfer'} and ($validTransfert ne 1) and ($reserveDone ne 1) ){

                        if (C4::Context->preference("AutomaticItemReturn") == 1) {
                        if (C4::Context->preference("AutomaticItemReturn") == 1) {

-                               ModItemTransfer($iteminformation->{'itemnumber'}, C4::Context->userenv->{'branch'}, $iteminformation->{'homebranch'});
+                               ModItemTransfer($iteminformation->{'itemnumber'}, $branch, $iteminformation->{$hbr});

                                $messages->{'WasTransfered'} = 1;
                        }
                        else {
                                $messages->{'WasTransfered'} = 1;
                        }
                        else {


@@ -1584,7 +1610,6 @@ sub FixAccountForLostAndReturned {
                        WHERE (borrowernumber = ?)
                        AND (itemnumber = ?) AND (accountno = ?) ");
                $usth->execute($data->{'borrowernumber'},$itm,$acctno);
                        WHERE (borrowernumber = ?)
                        AND (itemnumber = ?) AND (accountno = ?) ");
                $usth->execute($data->{'borrowernumber'},$itm,$acctno);

-               $usth->finish;

        #check if any credit is left if so writeoff other accounts
                my $nextaccntno = getnextacctno($data->{'borrowernumber'});
                if ($amountleft < 0){
        #check if any credit is left if so writeoff other accounts
                my $nextaccntno = getnextacctno($data->{'borrowernumber'});
                if ($amountleft < 0){


@@ -1616,9 +1641,8 @@ sub FixAccountForLostAndReturned {
                                VALUES
                                (?,?,?,?)");
                        $usth->execute($data->{'borrowernumber'},$accdata->{'accountno'},$nextaccntno,$newamtos);
                                VALUES
                                (?,?,?,?)");
                        $usth->execute($data->{'borrowernumber'},$accdata->{'accountno'},$nextaccntno,$newamtos);

-                       $usth->finish;

                }
                }

-               $msth->finish;
+               $msth->finish;  # $msth might actually have data left

                }
                if ($amountleft > 0){
                        $amountleft*=-1;
                }
                if ($amountleft > 0){
                        $amountleft*=-1;


@@ -1628,62 +1652,74 @@ sub FixAccountForLostAndReturned {
                        (borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding)
                        VALUES (?,?,now(),?,?,'CR',?)");
                $usth->execute($data->{'borrowernumber'},$nextaccntno,0-$amount,$desc,$amountleft);
                        (borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding)
                        VALUES (?,?,now(),?,?,'CR',?)");
                $usth->execute($data->{'borrowernumber'},$nextaccntno,0-$amount,$desc,$amountleft);

-               $usth->finish;

                $usth = $dbh->prepare("INSERT INTO accountoffsets
                        (borrowernumber, accountno, offsetaccount,  offsetamount)
                        VALUES (?,?,?,?)");
                $usth->execute($borrower->{'borrowernumber'},$data->{'accountno'},$nextaccntno,$offset);
                $usth = $dbh->prepare("INSERT INTO accountoffsets
                        (borrowernumber, accountno, offsetaccount,  offsetamount)
                        VALUES (?,?,?,?)");
                $usth->execute($borrower->{'borrowernumber'},$data->{'accountno'},$nextaccntno,$offset);

-               $usth->finish;

         ModItem({ paidfor => '' }, undef, $itm);
        }
        $sth->finish;
        return;
 }
 
         ModItem({ paidfor => '' }, undef, $itm);
        }
        $sth->finish;
        return;
 }
 

-=head2 GetItemIssue
+=head2 _GetCircControlBranch

 
 

-$issues = &GetItemIssue($itemnumber);
+   my $circ_control_branch = _GetCircControlBranch($iteminfos, $borrower);

 
 

-Returns patrons currently having a book. nothing if item is not issued atm
+Internal function : 

 
 

-C<$itemnumber> is the itemnumber
+Return the library code to be used to determine which circulation
+policy applies to a transaction.  Looks up the CircControl and
+HomeOrHoldingBranch system preferences.

 
 

-Returns an array of hashes
+C<$iteminfos> is a hashref to iteminfo. Only {itemnumber} is used.

 
 

-FIXME: Though the above says that this function returns nothing if the
-item is not issued, this actually returns a hasref that looks like
-this:
-    {
-      itemnumber => 1,
-      overdue    => 1
+C<$borrower> is a hashref to borrower. Only {borrowernumber is used.
+
+=cut
+
+sub _GetCircControlBranch {
+    my ($iteminfos, $borrower) = @_;
+    my $circcontrol = C4::Context->preference('CircControl');
+    my $branch;
+
+    if ($circcontrol eq 'PickupLibrary') {
+        $branch= C4::Context->userenv->{'branch'};
+    } elsif ($circcontrol eq 'PatronLibrary') {
+        $branch=$borrower->{branchcode};
+    } else {
+        my $branchfield = C4::Context->preference('HomeOrHoldingBranch') || 'homebranch';
+        $branch = $iteminfos->{$branchfield};

     }
     }

+    return $branch;
+}
+
+=head2 GetItemIssue
+
+$issues = &GetItemIssue($itemnumber);
+
+Returns patron currently having a book, or undef if not checked out.
+
+C<$itemnumber> is the itemnumber

 
 

+C<$issues> is an array of hashes.

 
 =cut
 
 sub GetItemIssue {
 
 =cut
 
 sub GetItemIssue {

-    my ( $itemnumber) = @_;
+    my ($itemnumber) = @_;

     return unless $itemnumber;
     return unless $itemnumber;

-    my $dbh = C4::Context->dbh;
-    my @GetItemIssues;
-    
-    # get today date
-    my $today = POSIX::strftime("%Y%m%d", localtime);
-
-    my $sth = $dbh->prepare(
-        "SELECT * FROM issues 
+    my $sth = C4::Context->dbh->prepare(
+        "SELECT *
+        FROM issues 

         LEFT JOIN items ON issues.itemnumber=items.itemnumber
         LEFT JOIN items ON issues.itemnumber=items.itemnumber

-    WHERE
-    issues.itemnumber=?");
+        WHERE issues.itemnumber=?");

     $sth->execute($itemnumber);
     my $data = $sth->fetchrow_hashref;
     $sth->execute($itemnumber);
     my $data = $sth->fetchrow_hashref;

-    my $datedue = $data->{'date_due'};
-    $datedue =~ s/-//g;
-    if ( $datedue < $today ) {
-        $data->{'overdue'} = 1;
-    }
-    $data->{'itemnumber'} = $itemnumber; # fill itemnumber, in case item is not on issue
-    $sth->finish;
+    return unless $data;
+    $data->{'overdue'} = ($data->{'date_due'} lt C4::Dates->today('iso')) ? 1 : 0;
+    $data->{'itemnumber'} = $itemnumber; # fill itemnumber, in case item is not on issue.
+    # FIXME: that would mean issues.itemnumber IS NULL and we didn't really match it.

     return ($data);
 }
 
     return ($data);
 }
 


@@ -1694,23 +1730,20 @@ $issues = &GetItemIssues($itemnumber, $history);
 Returns patrons that have issued a book
 
 C<$itemnumber> is the itemnumber
 Returns patrons that have issued a book
 
 C<$itemnumber> is the itemnumber

-C<$history> is 0 if you want actuel "issuer" (if it exist) and 1 if you want issues history
+C<$history> is false if you just want the current "issuer" (if any)
+and true if you want issues history from old_issues also.

 
 

-Returns an array of hashes
+Returns reference to an array of hashes

 
 =cut
 
 sub GetItemIssues {
 
 =cut
 
 sub GetItemIssues {

-    my ( $itemnumber,$history ) = @_;
-    my $dbh = C4::Context->dbh;
-    my @GetItemIssues;
+    my ( $itemnumber, $history ) = @_;

     
     

-    # get today date
-    my $today = POSIX::strftime("%Y%m%d", localtime);
-
+    my $today = C4::Dates->today('iso');  # get today date

     my $sql = "SELECT * FROM issues 
               JOIN borrowers USING (borrowernumber)
     my $sql = "SELECT * FROM issues 
               JOIN borrowers USING (borrowernumber)

-              JOIN items USING (itemnumber)
+              JOIN items     USING (itemnumber)

               WHERE issues.itemnumber = ? ";
     if ($history) {
         $sql .= "UNION ALL
               WHERE issues.itemnumber = ? ";
     if ($history) {
         $sql .= "UNION ALL


@@ -1720,23 +1753,17 @@ sub GetItemIssues {
                  WHERE old_issues.itemnumber = ? ";
     }
     $sql .= "ORDER BY date_due DESC";
                  WHERE old_issues.itemnumber = ? ";
     }
     $sql .= "ORDER BY date_due DESC";

-    my $sth = $dbh->prepare($sql);
+    my $sth = C4::Context->dbh->prepare($sql);

     if ($history) {
         $sth->execute($itemnumber, $itemnumber);
     } else {
         $sth->execute($itemnumber);
     }
     if ($history) {
         $sth->execute($itemnumber, $itemnumber);
     } else {
         $sth->execute($itemnumber);
     }

-    while ( my $data = $sth->fetchrow_hashref ) {
-        my $datedue = $data->{'date_due'};
-        $datedue =~ s/-//g;
-        if ( $datedue < $today ) {
-            $data->{'overdue'} = 1;
-        }
-        my $itemnumber = $data->{'itemnumber'};
-        push @GetItemIssues, $data;
+    my $results = $sth->fetchall_arrayref({});
+    foreach (@$results) {
+        $_->{'overdue'} = ($_->{'date_due'} lt $today) ? 1 : 0;

     }
     }

-    $sth->finish;
-    return ( \@GetItemIssues );
+    return $results;

 }
 
 =head2 GetBiblioIssues
 }
 
 =head2 GetBiblioIssues


@@ -1908,7 +1935,8 @@ has the item.
 
 C<$itemnumber> is the number of the item to renew.
 
 
 C<$itemnumber> is the number of the item to renew.
 

-C<$branch> is the library branch.  Defaults to the homebranch of the ITEM.
+C<$branch> is the library where the renewal took place (if any).
+           The library that controls the circ policies for the renewal is retrieved from the issues record.

 
 C<$datedue> can be a C4::Dates object used to set the due date.
 
 
 C<$datedue> can be a C4::Dates object used to set the due date.
 


@@ -1921,13 +1949,16 @@ from the book's item type.
 =cut
 
 sub AddRenewal {
 =cut
 
 sub AddRenewal {

-       my $borrowernumber = shift or return undef;
-       my     $itemnumber = shift or return undef;
+    
+    my $borrowernumber  = shift or return undef;
+    my $itemnumber      = shift or return undef;

     my $item   = GetItem($itemnumber) 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 $branch  = (@_) ? shift : $item->{homebranch};  # opac-renew doesn't send branch

-    my $datedue = shift;
-    my $lastreneweddate = shift;
+    my $datedue         = shift;
+    my $lastreneweddate = shift || C4::Dates->new()->output('iso');
+
+
+    my $biblio = GetBiblioFromItemNumber($itemnumber) or return undef;

 
     # If the due date wasn't specified, calculate it by adding the
     # book's loan length to today's date.
 
     # If the due date wasn't specified, calculate it by adding the
     # book's loan length to today's date.


@@ -1940,7 +1971,7 @@ sub AddRenewal {
                        $item->{homebranch}                     # item's homebranch determines loanlength OR do we want the branch specified by the AddRenewal argument?
         );
                #FIXME -- use circControl?
                        $item->{homebranch}                     # item's homebranch determines loanlength OR do we want the branch specified by the AddRenewal argument?
         );
                #FIXME -- use circControl?

-               $datedue =  CalcDateDue(C4::Dates->new(),$loanlength,$branch);  # this branch is the transactional branch.
+               $datedue =  CalcDateDue(C4::Dates->new(),$loanlength,$branch,$borrower);        # this branch is the transactional branch.

                                                                # The question of whether to use item's homebranch calendar is open.
     }
 
                                                                # The question of whether to use item's homebranch calendar is open.
     }
 


@@ -1959,6 +1990,26 @@ sub AddRenewal {
     $sth->execute( $borrowernumber, $itemnumber );
     my $issuedata = $sth->fetchrow_hashref;
     $sth->finish;
     $sth->execute( $borrowernumber, $itemnumber );
     my $issuedata = $sth->fetchrow_hashref;
     $sth->finish;

+    if($datedue && ! $datedue->output('iso')){
+        warn "Invalid date passed to AddRenewal.";
+        return undef;
+    }
+    # If the due date wasn't specified, calculate it by adding the
+    # book's loan length to today's date or the current due date
+    # based on the value of the RenewalPeriodBase syspref.
+    unless ($datedue) {
+
+        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'} ,
+                               $issuedata->{'branchcode'}  );   # that's the circ control branch.
+
+        $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?
+                                        C4::Dates->new($issuedata->{date_due}, 'iso') :
+                                        C4::Dates->new();
+        $datedue =  CalcDateDue($datedue,$loanlength,$issuedata->{'branchcode'},$borrower);
+    }

 
     # Update the issues record to have the new due date, and a new count
     # of how many times it has been renewed.
 
     # Update the issues record to have the new due date, and a new count
     # of how many times it has been renewed.


@@ -1972,7 +2023,7 @@ sub AddRenewal {
 
     # Update the renewal count on the item, and tell zebra to reindex
     $renews = $biblio->{'renewals'} + 1;
 
     # Update the renewal count on the item, and tell zebra to reindex
     $renews = $biblio->{'renewals'} + 1;

-    ModItem({ renewals => $renews }, $biblio->{'biblionumber'}, $itemnumber);
+    ModItem({ renewals => $renews, onloan => $datedue->output('iso') }, $biblio->{'biblionumber'}, $itemnumber);

 
     # Charge a new rental fee, if applicable?
     my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
 
     # Charge a new rental fee, if applicable?
     my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );


@@ -2261,17 +2312,32 @@ C<$loanlength>  = loan length prior to adjustment
 =cut
 
 sub CalcDateDue { 
 =cut
 
 sub CalcDateDue { 

-       my ($startdate,$loanlength,$branch) = @_;
+       my ($startdate,$loanlength,$branch,$borrower) = @_;
+       my $datedue;
+

        if(C4::Context->preference('useDaysMode') eq 'Days') {  # ignoring calendar
        if(C4::Context->preference('useDaysMode') eq 'Days') {  # ignoring calendar

-               my $datedue = time + ($loanlength) * 86400;
+               my $timedue = time + ($loanlength) * 86400;

        #FIXME - assumes now even though we take a startdate 
        #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');
+               my @datearr  = localtime($timedue);
+               $datedue = C4::Dates->new( sprintf("%04d-%02d-%02d", 1900 + $datearr[5], $datearr[4] + 1, $datearr[3]), 'iso');

        } else {
                my $calendar = C4::Calendar->new(  branchcode => $branch );
        } else {
                my $calendar = C4::Calendar->new(  branchcode => $branch );

-               my $datedue = $calendar->addDate($startdate, $loanlength);
-               return $datedue;
+               $datedue = $calendar->addDate($startdate, $loanlength);

        }
        }

+
+       # if ReturnBeforeExpiry ON the datedue can't be after borrower expirydate
+       if ( C4::Context->preference('ReturnBeforeExpiry') && $datedue->output('iso') gt $borrower->{dateexpiry} ) {
+           $datedue = C4::Dates->new( $borrower->{dateexpiry}, 'iso' );
+       }
+
+       # if ceilingDueDate ON the datedue can't be after the ceiling date
+       if ( C4::Context->preference('ceilingDueDate')
+            && ( C4::Context->preference('ceilingDueDate') =~ C4::Dates->regexp('syspref') )
+            && $datedue->output gt C4::Context->preference('ceilingDueDate') ) {
+           $datedue = C4::Dates->new( C4::Context->preference('ceilingDueDate') );
+       }
+
+       return $datedue;

 }
 
 =head2 CheckValidDatedue
 }
 
 =head2 CheckValidDatedue