X-Git-Url: http://koha-dev.rot13.org:8081/gitweb/?a=blobdiff_plain;f=C4%2FCirculation.pm;h=77650a0fab6ffce677ad4e1a96d45cf39d15a84a;hb=a326217179cbace605a1b28e886b5709eff566d7;hp=5b2043f692187cf18bdb8be8c2ebea16f0e5d467;hpb=38cf1fd318fc856dd4c167d00382570f946add07;p=koha_gimpoz

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 5b2043f692..77650a0fab 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -13,13 +13,13 @@ package C4::Circulation;
 # WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
 # A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
 #
-# You should have received a copy of the GNU General Public License along with
-# Koha; if not, write to the Free Software Foundation, Inc., 59 Temple Place,
-# Suite 330, Boston, MA  02111-1307 USA
+# You should have received a copy of the GNU General Public License along
+# with Koha; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
 
 use strict;
-require Exporter;
+#use warnings; FIXME - Bug 2505
 use C4::Context;
 use C4::Stats;
 use C4::Reserves;
@@ -30,6 +30,9 @@ use C4::Members;
 use C4::Dates;
 use C4::Calendar;
 use C4::Accounts;
+use C4::ItemCirculationAlertPreference;
+use C4::Message;
+use C4::Debug;
 use Date::Calc qw(
   Today
   Today_and_Now
@@ -48,8 +51,8 @@ use Data::Dumper;
 use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
 
 BEGIN {
-	# set the version for version checking
-	$VERSION = 3.01;
+	require Exporter;
+	$VERSION = 3.02;	# for version checking
 	@ISA    = qw(Exporter);
 
 	# FIXME subs that should probably be elsewhere
@@ -63,9 +66,7 @@ BEGIN {
 		&CanBookBeIssued
 		&CanBookBeRenewed
 		&AddIssue
-                &ForceIssue
 		&AddRenewal
-                &ForceRenewal
 		&GetRenewCount
 		&GetItemIssue
                 &GetOpenIssue
@@ -74,6 +75,7 @@ BEGIN {
 		&GetIssuingCharges
 		&GetIssuingRule
         &GetBranchBorrowerCircRule
+        &GetBranchItemRule
 		&GetBiblioIssues
 		&AnonymiseIssueHistory
 	);
@@ -81,7 +83,6 @@ BEGIN {
 	# subs to deal with returns
 	push @EXPORT, qw(
 		&AddReturn
-                &ForceReturn
         &MarkIssueReturned
 	);
 
@@ -92,6 +93,9 @@ BEGIN {
 		&GetTransfersFromTo
 		&updateWrongTransfer
 		&DeleteTransfer
+                &IsBranchTransferAllowed
+                &CreateBranchTransferLimit
+                &DeleteBranchTransferLimits
 	);
 }
 
@@ -113,7 +117,7 @@ Also deals with stocktaking.
 
 =head2 barcodedecode
 
-=head3 $str = &barcodedecode($barcode);
+=head3 $str = &barcodedecode($barcode, [$filter]);
 
 =over 4
 
@@ -124,6 +128,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.
 
+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
@@ -132,31 +140,27 @@ correct barcode string (items.barcode) should return an unaltered barcode.
 # 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;
-		return $barcode;
-	} elsif($filter eq 'cuecat') {
+	} 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 ;
-			}
+	    ($#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
@@ -168,6 +172,9 @@ sub barcodedecode {
 =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
@@ -180,7 +187,7 @@ sub decode {
     my $l = ( $#s + 1 ) % 4;
     if ($l) {
         if ( $l == 1 ) {
-            warn "Error!";
+            # warn "Error: Cuecat decode parsing failed!";
             return;
         }
         $l = 4 - $l;
@@ -271,9 +278,23 @@ sub transferbook {
     my $hbr = $biblio->{'homebranch'};
     my $fbr = $biblio->{'holdingbranch'};
 
+    # if using Branch Transfer Limits
+    if ( C4::Context->preference("UseBranchTransferLimits") == 1 ) {
+        if ( C4::Context->preference("item-level_itypes") && C4::Context->preference("BranchTransferLimitsType") eq 'itemtype' ) {
+            if ( ! IsBranchTransferAllowed( $tbr, $fbr, $biblio->{'itype'} ) ) {
+                $messages->{'NotAllowed'} = $tbr . "::" . $biblio->{'itype'};
+                $dotransfer = 0;
+            }
+        } elsif ( ! IsBranchTransferAllowed( $tbr, $fbr, $biblio->{ C4::Context->preference("BranchTransferLimitsType") } ) ) {
+            $messages->{'NotAllowed'} = $tbr . "::" . $biblio->{ C4::Context->preference("BranchTransferLimitsType") };
+            $dotransfer = 0;
+    	}
+    }
+
     # if is permanent...
     if ( $hbr && $branches->{$hbr}->{'PE'} ) {
         $messages->{'IsPermanent'} = $hbr;
+        $dotransfer = 0;
     }
 
     # can't transfer book if is already there....
@@ -305,99 +326,12 @@ sub transferbook {
 
         # don't need to update MARC anymore, we do it in batch now
         $messages->{'WasTransfered'} = 1;
-		ModDateLastSeen( $itemnumber );
+
     }
+    ModDateLastSeen( $itemnumber );
     return ( $dotransfer, $messages, $biblio );
 }
 
-=head2 CanBookBeIssued
-
-Check if a book can be issued.
-
-my ($issuingimpossible,$needsconfirmation) = CanBookBeIssued($borrower,$barcode,$year,$month,$day);
-
-=over 4
-
-=item C<$borrower> hash with borrower informations (from GetMemberDetails)
-
-=item C<$barcode> is the bar code of the book being issued.
-
-=item C<$year> C<$month> C<$day> contains the date of the return (in case it's forced by "stickyduedate".
-
-=back
-
-Returns :
-
-=over 4
-
-=item C<$issuingimpossible> a reference to a hash. It contains reasons why issuing is impossible.
-Possible values are :
-
-=back
-
-=head3 INVALID_DATE 
-
-sticky due date is invalid
-
-=head3 GNA
-
-borrower gone with no address
-
-=head3 CARD_LOST
-
-borrower declared it's card lost
-
-=head3 DEBARRED
-
-borrower debarred
-
-=head3 UNKNOWN_BARCODE
-
-barcode unknown
-
-=head3 NOT_FOR_LOAN
-
-item is not for loan
-
-=head3 WTHDRAWN
-
-item withdrawn.
-
-=head3 RESTRICTED
-
-item is restricted (set by ??)
-
-C<$issuingimpossible> a reference to a hash. It contains reasons why issuing is impossible.
-Possible values are :
-
-=head3 DEBT
-
-borrower has debts.
-
-=head3 RENEW_ISSUE
-
-renewing, not issuing
-
-=head3 ISSUED_TO_ANOTHER
-
-issued to someone else.
-
-=head3 RESERVED
-
-reserved for someone else.
-
-=head3 INVALID_DATE
-
-sticky due date is invalid
-
-=head3 TOO_MANY
-
-if the borrower borrows to much things
-
-=cut
-
-# check if a book can be issued.
-
 
 sub TooMany {
     my $borrower        = shift;
@@ -407,16 +341,7 @@ sub TooMany {
     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
@@ -608,7 +533,6 @@ sub itemissues {
             $data->{'date_due'} = ($data->{'wthdrawn'} eq '1') ? 'Cancelled' : 'Available';
         }
 
-        $sth2->finish;
 
         # Find the last 3 people who borrowed this item.
         $sth2 = $dbh->prepare(
@@ -628,22 +552,103 @@ sub itemissues {
             }    # if
         }    # for
 
-        $sth2->finish;
         $results[$i] = $data;
         $i++;
     }
 
-    $sth->finish;
     return (@results);
 }
 
 =head2 CanBookBeIssued
 
-( $issuingimpossible, $needsconfirmation ) = 
-        CanBookBeIssued( $borrower, $barcode, $duedatespec, $inprocess );
-C<$duedatespec> is a C4::Dates object.
+Check if a book can be issued.
+
+( $issuingimpossible, $needsconfirmation ) =  CanBookBeIssued( $borrower, $barcode, $duedatespec, $inprocess );
+
 C<$issuingimpossible> and C<$needsconfirmation> are some hashref.
 
+=over 4
+
+=item C<$borrower> hash with borrower informations (from GetMemberDetails)
+
+=item C<$barcode> is the bar code of the book being issued.
+
+=item C<$duedatespec> is a C4::Dates object.
+
+=item C<$inprocess>
+
+=back
+
+Returns :
+
+=over 4
+
+=item C<$issuingimpossible> a reference to a hash. It contains reasons why issuing is impossible.
+Possible values are :
+
+=back
+
+=head3 INVALID_DATE 
+
+sticky due date is invalid
+
+=head3 GNA
+
+borrower gone with no address
+
+=head3 CARD_LOST
+
+borrower declared it's card lost
+
+=head3 DEBARRED
+
+borrower debarred
+
+=head3 UNKNOWN_BARCODE
+
+barcode unknown
+
+=head3 NOT_FOR_LOAN
+
+item is not for loan
+
+=head3 WTHDRAWN
+
+item withdrawn.
+
+=head3 RESTRICTED
+
+item is restricted (set by ??)
+
+C<$needsconfirmation> a reference to a hash. It contains reasons why the loan could be prevented, 
+but ones that can be overriden by the operator.
+
+Possible values are :
+
+=head3 DEBT
+
+borrower has debts.
+
+=head3 RENEW_ISSUE
+
+renewing, not issuing
+
+=head3 ISSUED_TO_ANOTHER
+
+issued to someone else.
+
+=head3 RESERVED
+
+reserved for someone else.
+
+=head3 INVALID_DATE
+
+sticky due date is invalid
+
+=head3 TOO_MANY
+
+if the borrower borrows to much things
+
 =cut
 
 sub CanBookBeIssued {
@@ -656,10 +661,27 @@ sub CanBookBeIssued {
 	$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.
     #
-    #$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
@@ -699,7 +721,7 @@ sub CanBookBeIssued {
         if ( $amount > $amountlimit && !$inprocess ) {
             $issuingimpossible{DEBT} = sprintf( "%.2f", $amount );
         }
-        elsif ( $amount <= $amountlimit && !$inprocess ) {
+        elsif ( $amount > 0 && $amount <= $amountlimit && !$inprocess ) {
             $needsconfirmation{DEBT} = sprintf( "%.2f", $amount );
         }
     }
@@ -709,39 +731,62 @@ sub CanBookBeIssued {
         }
     }
 
-    #
+    my ($blocktype, $count) = C4::Members::IsMemberBlocked($borrower->{'borrowernumber'});
+    if($blocktype == -1){
+        ## remaining overdue documents
+        $issuingimpossible{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 );
-    $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
     #
-    unless ( $item->{barcode} ) {
-        $issuingimpossible{UNKNOWN_BARCODE} = 1;
-    }
     if (   $item->{'notforloan'}
         && $item->{'notforloan'} > 0 )
     {
-        $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(!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'}) {
+                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;
+            }
+        }
+    }
     if ( $item->{'wthdrawn'} && $item->{'wthdrawn'} == 1 )
     {
         $issuingimpossible{WTHDRAWN} = 1;
@@ -753,7 +798,7 @@ sub CanBookBeIssued {
     }
     if ( C4::Context->preference("IndependantBranches") ) {
         my $userenv = C4::Context->userenv;
-        if ( ($userenv) && ( $userenv->{flags} != 1 ) ) {
+        if ( ($userenv) && ( $userenv->{flags} % 2 != 1 ) ) {
             $issuingimpossible{NOTSAMEBRANCH} = 1
               if ( $item->{C4::Context->preference("HomeOrHoldingBranch")} ne $userenv->{branch} );
         }
@@ -781,7 +826,7 @@ sub CanBookBeIssued {
     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} =
@@ -792,7 +837,7 @@ sub CanBookBeIssued {
     my ( $restype, $res ) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
     if ($restype) {
 		my $resbor = $res->{'borrowernumber'};
-		my ( $resborrower ) = GetMemberDetails( $resbor, 0 );
+		my ( $resborrower ) = C4::Members::GetMemberDetails( $resbor, 0 );
 		my $branches  = GetBranches();
 		my $branchname = $branches->{ $res->{'branchcode'} }->{'branchname'};
         if ( $resbor ne $borrower->{'borrowernumber'} && $restype eq "Waiting" )
@@ -808,12 +853,6 @@ sub CanBookBeIssued {
 "$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 );
 }
 
@@ -821,53 +860,55 @@ sub CanBookBeIssued {
 
 Issue a book. Does no check, they are done in CanBookBeIssued. If we reach this sub, it means the user confirmed if needed.
 
-&AddIssue($borrower,$barcode,$date)
+&AddIssue($borrower, $barcode, [$datedue], [$cancelreserve], [$issuedate])
 
 =over 4
 
-=item C<$borrower> hash with borrower informations (from GetMemberDetails)
+=item C<$borrower> is a hash with borrower informations (from GetMemberDetails).
 
-=item C<$barcode> is the bar code of the book being issued.
+=item C<$barcode> is the barcode of the item being issued.
+
+=item C<$datedue> is a C4::Dates object for the max date of return, i.e. the date due (optional).
+Calculated if empty.
 
-=item C<$date> contains the max date of return. calculated if empty.
+=item C<$cancelreserve> is 1 to override and cancel any pending reserves for the item (optional).
+
+=item C<$issuedate> is the date to issue the item in iso (YYYY-MM-DD) format (optional).
+Defaults to today.  Unlike C<$datedue>, NOT a C4::Dates object, unfortunately.
 
 AddIssue does the following things :
-- step 01: check that there is a borrowernumber & a barcode provided
-- check for RENEWAL (book issued & being issued to the same patron)
-    - renewal YES = Calculate Charge & renew
-    - renewal NO  = 
-        * BOOK ACTUALLY ISSUED ? do a return if book is actually issued (but to someone else)
-        * RESERVE PLACED ?
-            - fill reserve if reserve to this patron
-            - cancel reserve or not, otherwise
-        * TRANSFERT PENDING ?
-            - complete the transfert
-        * ISSUE THE BOOK
+
+  - step 01: check that there is a borrowernumber & a barcode provided
+  - check for RENEWAL (book issued & being issued to the same patron)
+      - renewal YES = Calculate Charge & renew
+      - renewal NO  =
+          * BOOK ACTUALLY ISSUED ? do a return if book is actually issued (but to someone else)
+          * RESERVE PLACED ?
+              - fill reserve if reserve to this patron
+              - cancel reserve or not, otherwise
+          * TRANSFERT PENDING ?
+              - complete the transfert
+          * ISSUE THE BOOK
 
 =back
 
 =cut
 
 sub AddIssue {
-    my ( $borrower, $barcode, $date, $cancelreserve ) = @_;
+    my ( $borrower, $barcode, $datedue, $cancelreserve, $issuedate, $sipmode) = @_;
     my $dbh = C4::Context->dbh;
 	my $barcodecheck=CheckValidBarcode($barcode);
+
+    # $issuedate defaults to today.
+    if ( ! defined $issuedate ) {
+        $issuedate = strftime( "%Y-%m-%d", localtime );
+        # TODO: for hourly circ, this will need to be a C4::Dates object
+        # and all calls to AddIssue including issuedate will need to pass a Dates object.
+    }
 	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 $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'};
-		}
+		my $branch = _GetCircControlBranch($item,$borrower);
 		
 		# get actual issuing if there is one
 		my $actualissue = GetItemIssue( $item->{itemnumber});
@@ -878,14 +919,14 @@ sub AddIssue {
 		#
 		# check if we just renew the issue.
 		#
-		if ( $actualissue->{borrowernumber} eq $borrower->{'borrowernumber'} ) {
-			AddRenewal(
+		if ($actualissue->{borrowernumber} eq $borrower->{'borrowernumber'}) {
+			$datedue = AddRenewal(
 				$borrower->{'borrowernumber'},
 				$item->{'itemnumber'},
 				$branch,
-				$date
+				$datedue,
+                $issuedate, # here interpreted as the renewal date
 			);
-
 		}
 		else {
         # it's NOT a renewal
@@ -904,28 +945,23 @@ sub AddIssue {
 			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.
 				}
 				elsif ( $restype eq "Reserved" ) {
-
 					# warn "Reserved";
 					# The item is reserved by someone else.
 					if ($cancelreserve) { # cancel reserves on this item
-						CancelReserve( 0, $res->{'itemnumber'},
-							$res->{'borrowernumber'} );
+						CancelReserve(0, $res->{'itemnumber'}, $res->{'borrowernumber'});
 					}
 				}
 				if ($cancelreserve) {
-					CancelReserve( $res->{'biblionumber'}, 0,
-                    $res->{'borrowernumber'} );
+					CancelReserve($res->{'biblionumber'}, 0, $res->{'borrowernumber'});
 				}
 				else {
 					# set waiting reserve to first in reserve queue as book isn't waiting now
@@ -940,8 +976,8 @@ sub AddIssue {
 			# 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 ....)
-            my $sth =
+        # 	updating line of branchtranfert to finish it, and changing the to branch value, implement a comment for visibility of this case (maybe for stats ....)
+                my $sth =
                     $dbh->prepare(
                     "UPDATE branchtransfers 
                         SET datearrived = now(),
@@ -949,8 +985,7 @@ sub AddIssue {
                         comments = 'Forced branchtransfer'
                     WHERE itemnumber= ? AND datearrived IS NULL"
                     );
-                    $sth->execute(C4::Context->userenv->{'branch'},$item->{'itemnumber'});
-                    $sth->finish;
+                $sth->execute(C4::Context->userenv->{'branch'},$item->{'itemnumber'});
             }
 
         # Record in the database the fact that the book was issued.
@@ -960,37 +995,32 @@ sub AddIssue {
                     (borrowernumber, itemnumber,issuedate, date_due, branchcode)
                 VALUES (?,?,?,?,?)"
           );
-		my $dateduef;
-        if ($date) {
-            $dateduef = $date;
-        } 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->output('iso'), C4::Context->userenv->{'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, $borrower );
+
+        }
+        $sth->execute(
+            $borrower->{'borrowernumber'},      # borrowernumber
+            $item->{'itemnumber'},              # itemnumber
+            $issuedate,                         # issuedate
+            $datedue->output('iso'),            # date_due
+            C4::Context->userenv->{'branch'}    # branchcode
         );
         $sth->finish;
+        if ( C4::Context->preference('ReturnToShelvingCart') ) { ## ReturnToShelvingCart is on, anything issued should be taken off the cart.
+          CartToShelf( $item->{'itemnumber'} );
+        }
         $item->{'issues'}++;
         ModItem({ issues           => $item->{'issues'},
                   holdingbranch    => C4::Context->userenv->{'branch'},
                   itemlost         => 0,
                   datelastborrowed => C4::Dates->new()->output('iso'),
-                  onloan           => $dateduef->output('iso'),
+                  onloan           => $datedue->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'},
@@ -1007,41 +1037,35 @@ sub AddIssue {
         # Record the fact that this book was issued.
         &UpdateStats(
             C4::Context->userenv->{'branch'},
-            'issue',                        $charge,
-            '',                             $item->{'itemnumber'},
+            'issue', $charge,
+            ($sipmode ? "SIP-$sipmode" : ''), $item->{'itemnumber'},
             $item->{'itype'}, $borrower->{'borrowernumber'}
         );
+
+        # Send a checkout slip.
+        my $circulation_alert = 'C4::ItemCirculationAlertPreference';
+        my %conditions = (
+            branchcode   => $branch,
+            categorycode => $borrower->{categorycode},
+            item_type    => $item->{itype},
+            notification => 'CHECKOUT',
+        );
+        if ($circulation_alert->is_enabled_for(\%conditions)) {
+            SendCirculationAlert({
+                type     => 'CHECKOUT',
+                item     => $item,
+                borrower => $borrower,
+                branch   => $branch,
+            });
+        }
     }
-    
-    logaction("CIRCULATION", "ISSUE", $borrower->{'borrowernumber'}, $biblio->{'biblionumber'}) 
+
+    logaction("CIRCULATION", "ISSUE", $borrower->{'borrowernumber'}, $biblio->{'biblionumber'})
         if C4::Context->preference("IssueLog");
-    return ($datedue);
   }
+  return ($datedue);	# not necessarily the same as when it came in!
 }
 
-=head2 ForceIssue
-
-ForceIssue()
-
-Issues an item to a member, ignoring any problems that would normally dissallow the issue.
-
-=cut
-
-sub ForceIssue {
-  my ( $borrowernumber, $itemnumber, $date_due, $branchcode, $date ) = @_;
-warn "ForceIssue( $borrowernumber, $itemnumber, $date_due, $branchcode, $date );";
-  my $dbh = C4::Context->dbh;
-  my $sth = $dbh->prepare( "INSERT INTO `issues` ( `borrowernumber`, `itemnumber`, `date_due`, `branchcode`, `issuingbranch`, `returndate`, `lastreneweddate`, `return`,  `renewals`, `timestamp`, `issuedate` )
-                            VALUES ( ?, ?, ?, ?, ?, NULL, NULL, NULL, NULL, NOW(), ? )" );
-  $sth->execute( $borrowernumber, $itemnumber, $date_due, $branchcode, $branchcode, $date );
-  $sth->finish();
-
-  my $item = GetBiblioFromItemNumber( $itemnumber );
-
-  UpdateStats( $branchcode, 'issue', undef, undef, $itemnumber, $item->{ 'itemtype' }, $borrowernumber );
-}
-
-
 =head2 GetLoanLength
 
 Get loan length for an itemtype, a borrower type and a branch
@@ -1106,7 +1130,7 @@ sub GetLoanLength {
 
 =head2 GetIssuingRule
 
-FIXME - This is a copy-paste of GetLoanLength 
+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.
 
@@ -1246,26 +1270,100 @@ sub GetBranchBorrowerCircRule {
     };
 }
 
-=head2 AddReturn
+=head2 GetBranchItemRule
 
-($doreturn, $messages, $iteminformation, $borrower) =
-    &AddReturn($barcode, $branch, $exemptfine, $dropbox);
+=over 4
 
-Returns a book.
+my $branch_item_rule = GetBranchItemRule($branchcode, $itemtype);
 
-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.  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_.
+=back
 
-C<&AddReturn> returns a list of four items:
+Retrieves circulation rule attributes that apply to the given
+branch and item type, regardless of patron category.
+
+The return value is a hashref containing the following key:
+
+holdallowed => Hold policy for this branch and itemtype. Possible values:
+  0: No holds allowed.
+  1: Holds allowed only by patrons that have the same homebranch as the item.
+  2: Holds allowed from any patron.
+
+This searches branchitemrules in the following order:
+
+  * Same branchcode and itemtype
+  * Same branchcode, itemtype '*'
+  * branchcode '*', same itemtype
+  * branchcode and itemtype '*'
+
+Neither C<$branchcode> nor C<$categorycode> should be '*'.
+
+=cut
+
+sub GetBranchItemRule {
+    my ( $branchcode, $itemtype ) = @_;
+    my $dbh = C4::Context->dbh();
+    my $result = {};
+
+    my @attempts = (
+        ['SELECT holdallowed
+            FROM branch_item_rules
+            WHERE branchcode = ?
+              AND itemtype = ?', $branchcode, $itemtype],
+        ['SELECT holdallowed
+            FROM default_branch_circ_rules
+            WHERE branchcode = ?', $branchcode],
+        ['SELECT holdallowed
+            FROM default_branch_item_rules
+            WHERE itemtype = ?', $itemtype],
+        ['SELECT holdallowed
+            FROM default_circ_rules'],
+    );
+
+    foreach my $attempt (@attempts) {
+        my ($query, @bind_params) = @{$attempt};
+
+        # Since branch/category and branch/itemtype use the same per-branch
+        # defaults tables, we have to check that the key we want is set, not
+        # just that a row was returned
+        return $result if ( defined( $result->{'holdallowed'} = $dbh->selectrow_array( $query, {}, @bind_params ) ) );
+    }
+    
+    # built-in default circulation rule
+    return {
+        holdallowed => 2,
+    };
+}
+
+=head2 AddReturn
+
+($doreturn, $messages, $iteminformation, $borrower) =
+    &AddReturn($barcode, $branch, $exemptfine, $dropbox);
+
+Returns a book.
+
+=over 4
+
+=item C<$barcode> is the bar code of the book being returned.
+
+=item C<$branch> is the code of the branch where the book is being returned.
+
+=item C<$exemptfine> indicates that overdue charges for the item will be
+removed.
+
+=item 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_.
+
+=back
+
+C<&AddReturn> returns a list of four items:
 
 C<$doreturn> is true iff the return succeeded.
 
-C<$messages> is a reference-to-hash giving the reason for failure:
+C<$messages> is a reference-to-hash giving feedback on the operation.
+The keys of the hash are:
 
 =over 4
 
@@ -1287,6 +1385,12 @@ the book's home branch.
 
 This book has been withdrawn/cancelled. The value should be ignored.
 
+=item C<Wrongbranch>
+
+This book has was returned to the wrong branch.  The value is a hashref
+so that C<$messages->{Wrongbranch}->{Wrongbranch}> and C<$messages->{Wrongbranch}->{Rightbranch}>
+contain the branchcode of the incorrect and correct return library, respectively.
+
 =item C<ResFound>
 
 The item was reserved. The value is a reference-to-hash whose keys are
@@ -1296,6 +1400,9 @@ either C<Waiting>, C<Reserved>, or 0.
 
 =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.
 
@@ -1303,217 +1410,188 @@ patron who last borrowed the book.
 
 sub AddReturn {
     my ( $barcode, $branch, $exemptfine, $dropbox ) = @_;
-    my $dbh      = C4::Context->dbh;
+    if ($branch and not GetBranchDetail($branch)) {
+        warn "AddReturn error: branch '$branch' not found.  Reverting to " . C4::Context->userenv->{'branch'};
+        undef $branch;
+    }
+    $branch = C4::Context->userenv->{'branch'} unless $branch;  # we trust userenv to be a safe fallback/default
     my $messages;
-    my $doreturn = 1;
     my $borrower;
+    my $biblio;
+    my $doreturn       = 1;
     my $validTransfert = 0;
-    my $reserveDone = 0;
     
     # 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;
+    my $itemnumber = GetItemnumberFromBarcode( $barcode );
+    unless ($itemnumber) {
+        return (0, { BadBarcode => $barcode }); # no barcode means no item or borrower.  bail out.
+    }
+    my $issue  = GetItemIssue($itemnumber);
+#   warn Dumper($iteminformation);
+    if ($issue and $issue->{borrowernumber}) {
+        $borrower = C4::Members::GetMemberDetails($issue->{borrowernumber})
+            or die "Data inconsistency: barcode $barcode (itemnumber:$itemnumber) claims to be issued to non-existant borrowernumber '$issue->{borrowernumber}'\n"
+                . Dumper($issue) . "\n";
     } else {
-        # 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->{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;
-            $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
-        $borrower = C4::Members::GetMemberDetails( $iteminformation->{borrowernumber}, 0 );
-    
+        $messages->{'NotIssued'} = $barcode;
+        # even though item is not on loan, it may still be transferred;  therefore, get current branch info
+        $doreturn = 0;
+        # No issue, no borrowernumber.  ONLY if $doreturn, *might* you have a $borrower later.
+    }
+
+    my $item = GetItem($itemnumber) or die "GetItem($itemnumber) failed";
+        # full item data, but no borrowernumber or checkout info (no issue)
+        # we know GetItem should work because GetItemnumberFromBarcode worked
+    my $hbr = $item->{C4::Context->preference("HomeOrHoldingBranch")} || '';
+        # item must be from items table -- issues table has branchcode and issuingbranch, not homebranch nor holdingbranch
+
+    my $borrowernumber = $borrower->{'borrowernumber'} || undef;    # we don't know if we had a borrower or not
+
+    # check if the book is in a permanent collection....
+    # FIXME -- This 'PE' attribute is largely undocumented.  afaict, there's no user interface that reflects this functionality.
+    if ( $hbr ) {
+        my $branches = GetBranches();    # a potentially expensive call for a non-feature.
+        $branches->{$hbr}->{PE} and $messages->{'IsPermanent'} = $hbr;
+    }
+
+    # if indy branches and returning to different branch, refuse the return
+    if ($hbr ne $branch && C4::Context->preference("IndependantBranches")){
+        $messages->{'Wrongbranch'} = {
+            Wrongbranch => $branch,
+            Rightbranch => $hbr,
+        };
+        $doreturn = 0;
+        # bailing out here - in this case, current desired behavior
+        # is to act as if no return ever happened at all.
+        # FIXME - even in an indy branches situation, there should
+        # still be an option for the library to accept the item
+        # and transfer it to its owning library.
+        return ( $doreturn, $messages, $issue, $borrower );
+    }
+
+    if ( $item->{'wthdrawn'} ) { # book has been cancelled
+        $messages->{'wthdrawn'} = 1;
+        $doreturn = 0;
+    }
+
     # case of a return of document (deal with issues and holdingbranch)
-    
-        if ($doreturn) {
-			my $circControlBranch;
-			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
-    
-        # 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'} );
-        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;
-        }
-    
-    # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
-    #     check if we have a transfer for this document
-        my ($datesent,$frombranch,$tobranch) = GetTransfers( $iteminformation->{'itemnumber'} );
-    
-    #     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'} ) {
-                    my $sth =
-                    $dbh->prepare(
-                            "UPDATE branchtransfers SET datearrived = now() WHERE itemnumber= ? AND datearrived IS NULL"
-                    );
-                    $sth->execute( $iteminformation->{'itemnumber'} );
-                    $sth->finish;
-    #         now we check if there is a reservation with the validate of transfer if we have one, we can         set it with the status 'W'
-            C4::Reserves::ModReserveStatus( $iteminformation->{'itemnumber'},'W' );
-            }
-        else {
-            $messages->{'WrongTransfer'} = $tobranch;
-            $messages->{'WrongTransferItem'} = $iteminformation->{'itemnumber'};
-        }
-        $validTransfert = 1;
-        }
-    
-    # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # 
-        # fix up the accounts.....
-        if ($iteminformation->{'itemlost'}) {
-                FixAccountForLostAndReturned($iteminformation, $borrower);
-                $messages->{'WasLost'} = 1;
+    if ($doreturn) {
+        $borrower or warn "AddReturn without current borrower";
+		my $circControlBranch = _GetCircControlBranch($item,$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 ( $item->{'issuedate'} eq C4::Dates->today('iso') );
         }
-        # fix up the overdues in accounts...
-        FixOverduesOnReturn( $borrower->{'borrowernumber'},
-            $iteminformation->{'itemnumber'}, $exemptfine, $dropbox );
-    
-    # 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'} );
-        if ($resfound) {
-            $resrec->{'ResFound'}   = $resfound;
-            $messages->{'ResFound'} = $resrec;
-            $reserveDone = 1;
-        }
-    
-        # update stats?
-        # Record the fact that this book was returned.
-        UpdateStats(
-            $branch, 'return', '0', '',
-            $iteminformation->{'itemnumber'},
-            $biblio->{'itemtype'},
-            $borrower->{'borrowernumber'}
-        );
-        
-        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;
-			}
-			else {
-				$messages->{'NeedsTransfer'} = 1;
-			}
+
+        if ($borrowernumber) {
+            MarkIssueReturned($borrowernumber, $item->{'itemnumber'}, $circControlBranch);
+            $messages->{'WasReturned'} = 1;    # FIXME is the "= 1" right?  This could be the borrower hash.
         }
-    }
-    return ( $doreturn, $messages, $iteminformation, $borrower );
-}
 
-=head2 ForceReturn
+        ModItem({ onloan => undef }, $issue->{'biblionumber'}, $item->{'itemnumber'});
+    }
 
-ForceReturn( $barcode, $date, $branchcode );
+    # the holdingbranch is updated if the document is returned to another location.
+    # this is always done regardless of whether the item was on loan or not
+    if ($item->{'holdingbranch'} ne $branch) {
+        UpdateHoldingbranch($branch, $item->{'itemnumber'});
+        $item->{'holdingbranch'} = $branch; # update item data holdingbranch too
+    }
+    ModDateLastSeen( $item->{'itemnumber'} );
 
-Returns an item is if it were returned on C<$date>.
+    # check if we have a transfer for this document
+    my ($datesent,$frombranch,$tobranch) = GetTransfers( $item->{'itemnumber'} );
 
-This function is non-interactive and does not check for reserves.
+    # if we have a transfer to do, we update the line of transfers with the datearrived
+    if ($datesent) {
+        if ( $tobranch eq $branch ) {
+            my $sth = C4::Context->dbh->prepare(
+                "UPDATE branchtransfers SET datearrived = now() WHERE itemnumber= ? AND datearrived IS NULL"
+            );
+            $sth->execute( $item->{'itemnumber'} );
+            # if we have a reservation with valid transfer, we can set it's status to 'W'
+            C4::Reserves::ModReserveStatus($item->{'itemnumber'}, 'W');
+        } else {
+            $messages->{'WrongTransfer'}     = $tobranch;
+            $messages->{'WrongTransferItem'} = $item->{'itemnumber'};
+        }
+        $validTransfert = 1;
+    }
 
-C<$barcode> is the barcode of the item being returned.
+    # fix up the accounts.....
+    if ($item->{'itemlost'}) {
+        _FixAccountForLostAndReturned($item->{'itemnumber'}, $borrowernumber, $barcode);    # can tolerate undef $borrowernumber
+        $messages->{'WasLost'} = 1;
+    }
 
-C<$date> is the date of the actual return, in the format YYYY-MM-DD.
+    # fix up the overdues in accounts...
+    if ($borrowernumber) {
+        my $fix = _FixOverduesOnReturn($borrowernumber, $item->{itemnumber}, $exemptfine, $dropbox);
+        defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $item->{itemnumber}...) failed!";  # zero is OK, check defined
+    }
 
-C<$branchcode> is the branchcode for the library the item was returned to.
+    # find reserves.....
+    # if we don't have a reserve with the status W, we launch the Checkreserves routine
+    my ($resfound, $resrec) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
+    if ($resfound) {
+          $resrec->{'ResFound'} = $resfound;
+        $messages->{'ResFound'} = $resrec;
+    }
 
-=cut
+    # update stats?
+    # Record the fact that this book was returned.
+    UpdateStats(
+        $branch, 'return', '0', '',
+        $item->{'itemnumber'},
+        $biblio->{'itemtype'},
+        $borrowernumber
+    );
 
-sub ForceReturn {
-  my ( $barcode, $date, $branchcode ) = @_;
-  my $dbh = C4::Context->dbh;
+    # Send a check-in slip. # NOTE: borrower may be undef.  probably shouldn't try to send messages then.
+    my $circulation_alert = 'C4::ItemCirculationAlertPreference';
+    my %conditions = (
+        branchcode   => $branch,
+        categorycode => $borrower->{categorycode},
+        item_type    => $item->{itype},
+        notification => 'CHECKIN',
+    );
+    if ($doreturn && $circulation_alert->is_enabled_for(\%conditions)) {
+        SendCirculationAlert({
+            type     => 'CHECKIN',
+            item     => $item,
+            borrower => $borrower,
+            branch   => $branch,
+        });
+    }
     
-  my $item = GetBiblioFromItemNumber( undef, $barcode );
-      
-  ## FIXME: Is there a way to get the borrower of an item through the Koha API?
-  my $sth=$dbh->prepare( "SELECT borrowernumber FROM issues WHERE itemnumber = ? AND returndate IS NULL");
-  $sth->execute( $item->{'itemnumber'} );
-  my ( $borrowernumber ) = $sth->fetchrow;
-  $sth->finish();
-                
-  ## Move the issue from issues to old_issues
-  $sth = $dbh->prepare( "INSERT INTO old_issues ( SELECT * FROM issues WHERE itemnumber = ? AND returndate IS NULL )" );
-  $sth->execute( $item->{'itemnumber'} );
-  $sth->finish();
-  ## Delete the row in issues
-  $sth = $dbh->prepare( "DELETE FROM issues WHERE itemnumber = ? AND returndate IS NULL" );
-  $sth->execute( $item->{'itemnumber'} );
-  $sth->finish();
-  ## Now set the returndate
-  $sth = $dbh->prepare( 'UPDATE old_issues SET returndate = ? WHERE itemnumber = ? AND returndate IS NULL' );
-  $sth->execute( $date, $item->{'itemnumber'} );
-  $sth->finish();
-                                          
-  UpdateStats( $branchcode, 'return', my $amount, my $other, $item->{ 'itemnumber' }, $item->{ 'itemtype' }, $borrowernumber );
+    logaction("CIRCULATION", "RETURN", $borrowernumber, $item->{'biblionumber'})
+        if C4::Context->preference("ReturnLog");
+    
+    # FIXME: make this comment intelligible.
+    #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 ($branch ne $hbr) and not $messages->{'WrongTransfer'} and ($validTransfert ne 1) ){
+        if ( C4::Context->preference("AutomaticItemReturn"    ) or
+            (C4::Context->preference("UseBranchTransferLimits") and
+             ! IsBranchTransferAllowed($branch, $hbr, $item->{C4::Context->preference("BranchTransferLimitsType")} )
+           )) {
+            $debug and warn sprintf "about to call ModItemTransfer(%s, %s, %s)", $item->{'itemnumber'},$branch, $hbr;
+            $debug and warn "item: " . Dumper($item);
+            ModItemTransfer($item->{'itemnumber'}, $branch, $hbr);
+            $messages->{'WasTransfered'} = 1;
+        } else {
+            $messages->{'NeedsTransfer'} = 1;   # TODO: instead of 1, specify branchcode that the transfer SHOULD go to, $item->{homebranch}
+        }
+    }
+    return ( $doreturn, $messages, $issue, $borrower );
 }
 
-
 =head2 MarkIssueReturned
 
 =over 4
 
-MarkIssueReturned($borrowernumber, $itemnumber, $dropbox_branch);
+MarkIssueReturned($borrowernumber, $itemnumber, $dropbox_branch, $returndate);
 
 =back
 
@@ -1521,9 +1599,12 @@ 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 
+C<dropbox_branch> .  Assumes you've already checked that 
 it's safe to do this, i.e. last non-holiday > issuedate.
 
+if C<$returndate> is specified (in iso format), it is used as the date
+of the return. It is ignored when a dropbox_branch is passed in.
+
 Ideally, this function would be internal to C<C4::Circulation>,
 not exported, but it is currently needed by one 
 routine in C<C4::Accounts>.
@@ -1531,19 +1612,23 @@ 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 = ?";
+    my ( $borrowernumber, $itemnumber, $dropbox_branch, $returndate ) = @_;
+    my $dbh   = C4::Context->dbh;
+    my $query = "UPDATE issues SET returndate=";
+    my @bind;
+    if ($dropbox_branch) {
+        my $calendar = C4::Calendar->new( branchcode => $dropbox_branch );
+        my $dropboxdate = $calendar->addDate( C4::Dates->new(), -1 );
+        $query .= " ? ";
+        push @bind, $dropboxdate->output('iso');
+    } elsif ($returndate) {
+        $query .= " ? ";
+        push @bind, $returndate;
+    } else {
+        $query .= " now() ";
+    }
+    $query .= " WHERE  borrowernumber = ?  AND itemnumber = ?";
+    push @bind, $borrowernumber, $itemnumber;
     # FIXME transaction
     my $sth_upd  = $dbh->prepare($query);
     $sth_upd->execute(@bind);
@@ -1557,9 +1642,9 @@ sub MarkIssueReturned {
     $sth_del->execute($borrowernumber, $itemnumber);
 }
 
-=head2 FixOverduesOnReturn
+=head2 _FixOverduesOnReturn
 
-    &FixOverduesOnReturn($brn,$itm, $exemptfine, $dropboxmode);
+    &_FixOverduesOnReturn($brn,$itm, $exemptfine, $dropboxmode);
 
 C<$brn> borrowernumber
 
@@ -1568,184 +1653,214 @@ 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
+Internal function, called only by AddReturn
 
 =cut
 
-sub FixOverduesOnReturn {
-    my ( $borrowernumber, $item, $exemptfine, $dropbox ) = @_;
+sub _FixOverduesOnReturn {
+    my ($borrowernumber, $item);
+    unless ($borrowernumber = shift) {
+        warn "_FixOverduesOnReturn() not supplied valid borrowernumber";
+        return;
+    }
+    unless ($item = shift) {
+        warn "_FixOverduesOnReturn() not supplied valid itemnumber";
+        return;
+    }
+    my ($exemptfine, $dropbox) = @_;
     my $dbh = C4::Context->dbh;
 
     # check for overdue fine
-    my $sth =
-      $dbh->prepare(
+    my $sth = $dbh->prepare(
 "SELECT * FROM accountlines WHERE (borrowernumber = ?) AND (itemnumber = ?) AND (accounttype='FU' OR accounttype='O')"
-      );
+    );
     $sth->execute( $borrowernumber, $item );
 
     # alter fine to show that the book has been returned
-   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();
+    my $data = $sth->fetchrow_hashref;
+    return 0 unless $data;    # no warning, there's just nothing to fix
+
+    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);
+    return $usth->execute(@bind);
+}
+
+=head2 _FixAccountForLostAndReturned
+
+	&_FixAccountForLostAndReturned($itemnumber, [$borrowernumber, $barcode]);
+
+Calculates the charge for a book lost and returned.
+
+Internal function, not exported, called only by AddReturn.
+
+FIXME: This function reflects how inscrutable fines logic is.  Fix both.
+FIXME: Give a positive return value on success.  It might be the $borrowernumber who received credit, or the amount forgiven.
 
-    $sth->finish();
+=cut
+
+sub _FixAccountForLostAndReturned {
+    my $itemnumber     = shift or return;
+    my $borrowernumber = @_ ? shift : undef;
+    my $item_id        = @_ ? shift : $itemnumber;  # Send the barcode if you want that logged in the description
+    my $dbh = C4::Context->dbh;
+    # check for charge made for lost book
+    my $sth = $dbh->prepare("SELECT * FROM accountlines WHERE (itemnumber = ?) AND (accounttype='L' OR accounttype='Rep') ORDER BY date DESC");
+    $sth->execute($itemnumber);
+    my $data = $sth->fetchrow_hashref;
+    $data or return;    # bail if there is nothing to do
+
+    # writeoff this amount
+    my $offset;
+    my $amount = $data->{'amount'};
+    my $acctno = $data->{'accountno'};
+    my $amountleft;                                             # Starts off undef/zero.
+    if ($data->{'amountoutstanding'} == $amount) {
+        $offset     = $data->{'amount'};
+        $amountleft = 0;                                        # Hey, it's zero here, too.
+    } else {
+        $offset     = $amount - $data->{'amountoutstanding'};   # Um, isn't this the same as ZERO?  We just tested those two things are ==
+        $amountleft = $data->{'amountoutstanding'} - $amount;   # Um, isn't this the same as ZERO?  We just tested those two things are ==
+    }
+    my $usth = $dbh->prepare("UPDATE accountlines SET accounttype = 'LR',amountoutstanding='0'
+        WHERE (borrowernumber = ?)
+        AND (itemnumber = ?) AND (accountno = ?) ");
+    $usth->execute($data->{'borrowernumber'},$itemnumber,$acctno);      # We might be adjusting an account for some OTHER borrowernumber now.  Not the one we passed in.  
+    #check if any credit is left if so writeoff other accounts
+    my $nextaccntno = getnextacctno($data->{'borrowernumber'});
+    $amountleft *= -1 if ($amountleft < 0);
+    if ($amountleft > 0) {
+        my $msth = $dbh->prepare("SELECT * FROM accountlines WHERE (borrowernumber = ?)
+                            AND (amountoutstanding >0) ORDER BY date");     # might want to order by amountoustanding ASC (pay smallest first)
+        $msth->execute($data->{'borrowernumber'});
+        # offset transactions
+        my $newamtos;
+        my $accdata;
+        while (($accdata=$msth->fetchrow_hashref) and ($amountleft>0)){
+            if ($accdata->{'amountoutstanding'} < $amountleft) {
+                $newamtos = 0;
+                $amountleft -= $accdata->{'amountoutstanding'};
+            }  else {
+                $newamtos = $accdata->{'amountoutstanding'} - $amountleft;
+                $amountleft = 0;
+            }
+            my $thisacct = $accdata->{'accountno'};
+            # FIXME: move prepares outside while loop!
+            my $usth = $dbh->prepare("UPDATE accountlines SET amountoutstanding= ?
+                    WHERE (borrowernumber = ?)
+                    AND (accountno=?)");
+            $usth->execute($newamtos,$data->{'borrowernumber'},'$thisacct');    # FIXME: '$thisacct' is a string literal!
+            $usth = $dbh->prepare("INSERT INTO accountoffsets
+                (borrowernumber, accountno, offsetaccount,  offsetamount)
+                VALUES
+                (?,?,?,?)");
+            $usth->execute($data->{'borrowernumber'},$accdata->{'accountno'},$nextaccntno,$newamtos);
+        }
+        $msth->finish;  # $msth might actually have data left
+    }
+    $amountleft *= -1 if ($amountleft > 0);
+    my $desc = "Item Returned " . $item_id;
+    $usth = $dbh->prepare("INSERT INTO accountlines
+        (borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding)
+        VALUES (?,?,now(),?,?,'CR',?)");
+    $usth->execute($data->{'borrowernumber'},$nextaccntno,0-$amount,$desc,$amountleft);
+    if ($borrowernumber) {
+        # FIXME: same as query above.  use 1 sth for both
+        $usth = $dbh->prepare("INSERT INTO accountoffsets
+            (borrowernumber, accountno, offsetaccount,  offsetamount)
+            VALUES (?,?,?,?)");
+        $usth->execute($borrowernumber, $data->{'accountno'}, $nextaccntno, $offset);
+    }
+    ModItem({ paidfor => '' }, undef, $itemnumber);
     return;
 }
 
-=head2 FixAccountForLostAndReturned
+=head2 _GetCircControlBranch
 
-	&FixAccountForLostAndReturned($iteminfo,$borrower);
+   my $circ_control_branch = _GetCircControlBranch($iteminfos, $borrower);
 
-Calculates the charge for a book lost and returned (Not exported & used only once)
+Internal function : 
 
-C<$iteminfo> is a hashref to iteminfo. Only {itemnumber} is used.
+Return the library code to be used to determine which circulation
+policy applies to a transaction.  Looks up the CircControl and
+HomeOrHoldingBranch system preferences.
 
-C<$borrower> is a hashref to borrower. Only {borrowernumber is used.
+C<$iteminfos> is a hashref to iteminfo. Only {homebranch or holdingbranch} is used.
 
-Internal function, called by AddReturn
+C<$borrower> is a hashref to borrower. Only {branchcode} is used.
 
 =cut
 
-sub FixAccountForLostAndReturned {
-	my ($iteminfo, $borrower) = @_;
-	my $dbh = C4::Context->dbh;
-	my $itm = $iteminfo->{'itemnumber'};
-	# check for charge made for lost book
-	my $sth = $dbh->prepare("SELECT * FROM accountlines WHERE (itemnumber = ?) AND (accounttype='L' OR accounttype='Rep') ORDER BY date DESC");
-	$sth->execute($itm);
-	if (my $data = $sth->fetchrow_hashref) {
-	# writeoff this amount
-		my $offset;
-		my $amount = $data->{'amount'};
-		my $acctno = $data->{'accountno'};
-		my $amountleft;
-		if ($data->{'amountoutstanding'} == $amount) {
-		$offset = $data->{'amount'};
-		$amountleft = 0;
-		} else {
-		$offset = $amount - $data->{'amountoutstanding'};
-		$amountleft = $data->{'amountoutstanding'} - $amount;
-		}
-		my $usth = $dbh->prepare("UPDATE accountlines SET accounttype = 'LR',amountoutstanding='0'
-			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){
-		$amountleft*=-1;
-		}
-		if ($amountleft > 0){
-		my $msth = $dbh->prepare("SELECT * FROM accountlines WHERE (borrowernumber = ?)
-							AND (amountoutstanding >0) ORDER BY date");
-		$msth->execute($data->{'borrowernumber'});
-	# offset transactions
-		my $newamtos;
-		my $accdata;
-		while (($accdata=$msth->fetchrow_hashref) and ($amountleft>0)){
-			if ($accdata->{'amountoutstanding'} < $amountleft) {
-			$newamtos = 0;
-			$amountleft -= $accdata->{'amountoutstanding'};
-			}  else {
-			$newamtos = $accdata->{'amountoutstanding'} - $amountleft;
-			$amountleft = 0;
-			}
-			my $thisacct = $accdata->{'accountno'};
-			my $usth = $dbh->prepare("UPDATE accountlines SET amountoutstanding= ?
-					WHERE (borrowernumber = ?)
-					AND (accountno=?)");
-			$usth->execute($newamtos,$data->{'borrowernumber'},'$thisacct');
-			$usth->finish;
-			$usth = $dbh->prepare("INSERT INTO accountoffsets
-				(borrowernumber, accountno, offsetaccount,  offsetamount)
-				VALUES
-				(?,?,?,?)");
-			$usth->execute($data->{'borrowernumber'},$accdata->{'accountno'},$nextaccntno,$newamtos);
-			$usth->finish;
-		}
-		$msth->finish;
-		}
-		if ($amountleft > 0){
-			$amountleft*=-1;
-		}
-		my $desc="Item Returned ".$iteminfo->{'barcode'};
-		$usth = $dbh->prepare("INSERT INTO accountlines
-			(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->finish;
-        ModItem({ paidfor => '' }, undef, $itm);
-	}
-	$sth->finish;
-	return;
+sub _GetCircControlBranch {
+    my ($item, $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 = $item->{$branchfield};
+        # default to item home branch if holdingbranch is used
+        # and is not defined
+        if (!defined($branch) && $branchfield eq 'holdingbranch') {
+            $branch = $item->{homebranch};
+        }
+    }
+    return $branch;
 }
 
+
+
+
+
+
 =head2 GetItemIssue
 
-$issues = &GetItemIssue($itemnumber);
+$issue = &GetItemIssue($itemnumber);
 
-Returns patrons currently having a book. nothing if item is not issued atm
+Returns patron currently having a book, or undef if not checked out.
 
-C<$itemnumber> is the itemnumber
+C<$itemnumber> is the itemnumber.
 
-Returns an array of hashes
+C<$issue> is a hashref of the row from the issues table.
 
 =cut
 
 sub GetItemIssue {
-    my ( $itemnumber) = @_;
+    my ($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
-    WHERE
-    issues.itemnumber=?");
+        WHERE issues.itemnumber=?");
     $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;
     return ($data);
 }
 
@@ -1778,23 +1893,20 @@ $issues = &GetItemIssues($itemnumber, $history);
 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 {
-    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)
-              JOIN items USING (itemnumber)
+              JOIN items     USING (itemnumber)
               WHERE issues.itemnumber = ? ";
     if ($history) {
         $sql .= "UNION ALL
@@ -1804,23 +1916,17 @@ sub GetItemIssues {
                  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);
     }
-    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
@@ -1845,7 +1951,7 @@ sub GetBiblioIssues {
             LEFT JOIN borrowers ON borrowers.borrowernumber = issues.borrowernumber
             LEFT JOIN items ON issues.itemnumber = items.itemnumber
             LEFT JOIN biblioitems ON items.itemnumber = biblioitems.biblioitemnumber
-            LEFT JOIN biblio ON biblio.biblionumber = items.biblioitemnumber
+            LEFT JOIN biblio ON biblio.biblionumber = items.biblionumber
         WHERE biblio.biblionumber = ?
         UNION ALL
         SELECT old_issues.*,items.barcode,biblio.biblionumber,biblio.title, biblio.author,borrowers.cardnumber,borrowers.surname,borrowers.firstname
@@ -1853,7 +1959,7 @@ sub GetBiblioIssues {
             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
+            LEFT JOIN biblio ON biblio.biblionumber = items.biblionumber
         WHERE biblio.biblionumber = ?
         ORDER BY timestamp
     ";
@@ -1903,7 +2009,7 @@ END_SQL
 
 =head2 CanBookBeRenewed
 
-($ok,$error) = &CanBookBeRenewed($borrowernumber, $itemnumber);
+($ok,$error) = &CanBookBeRenewed($borrowernumber, $itemnumber[, $override_limit]);
 
 Find out whether a borrowed item may be renewed.
 
@@ -1914,6 +2020,10 @@ has the item on loan.
 
 C<$itemnumber> is the number of the item to renew.
 
+C<$override_limit>, if supplied with a true value, causes
+the limit on the number of times that the loan can be renewed
+(as controlled by the item type) to be ignored.
+
 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
@@ -1924,7 +2034,7 @@ already renewed the loan. $error will contain the reason the renewal can not pro
 sub CanBookBeRenewed {
 
     # check renewal status
-    my ( $borrowernumber, $itemnumber ) = @_;
+    my ( $borrowernumber, $itemnumber, $override_limit ) = @_;
     my $dbh       = C4::Context->dbh;
     my $renews    = 1;
     my $renewokay = 0;
@@ -1933,39 +2043,52 @@ sub CanBookBeRenewed {
     # 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 $sth1 = $dbh->prepare(
-        "SELECT * FROM issues
-            WHERE borrowernumber = ?
-            AND itemnumber = ?"
-    );
-    $sth1->execute( $borrowernumber, $itemnumber );
-    if ( my $data1 = $sth1->fetchrow_hashref ) {
-
-        # Found a matching item
-
-        # See if this item may be renewed. This query is convoluted
-        # 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 $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'} ) {
+    # Look in the issues table for this item, lent to this borrower,
+    # and not yet returned.
+    my %branch = (
+            'ItemHomeLibrary' => 'items.homebranch',
+            'PickupLibrary'   => 'items.holdingbranch',
+            'PatronLibrary'   => 'borrowers.branchcode'
+            );
+    my $controlbranch = $branch{C4::Context->preference('CircControl')};
+    my $itype         = C4::Context->preference('item-level_itypes') ? 'items.itype' : 'biblioitems.itemtype';
+    
+    my $sthcount = $dbh->prepare("
+                   SELECT 
+                    borrowers.categorycode, biblioitems.itemtype, issues.renewals, renewalsallowed, $controlbranch
+                   FROM  issuingrules, 
+                   issues 
+                   LEFT JOIN items USING (itemnumber) 
+                   LEFT JOIN borrowers USING (borrowernumber) 
+                   LEFT JOIN biblioitems USING (biblioitemnumber)
+                   
+                   WHERE
+                    issuingrules.categorycode = borrowers.categorycode
+                   AND
+                    issuingrules.itemtype = $itype
+                   AND
+                    (issuingrules.branchcode = $controlbranch OR issuingrules.branchcode = '*') 
+                   AND 
+                    borrowernumber = ? 
+                   AND
+                    itemnumber = ?
+                   ORDER BY
+                    issuingrules.categorycode desc,
+                    issuingrules.itemtype desc,
+                    issuingrules.branchcode desc
+                   LIMIT 1;
+                  ");
+
+    $sthcount->execute( $borrowernumber, $itemnumber );
+    if ( my $data1 = $sthcount->fetchrow_hashref ) {
+        
+        if ( ( $data1->{renewalsallowed} && $data1->{renewalsallowed} > $data1->{renewals} ) || $override_limit ) {
             $renewokay = 1;
         }
         else {
 			$error="too_many";
 		}
-        $sth2->finish;
+		
         my ( $resfound, $resrec ) = C4::Reserves::CheckReserves($itemnumber);
         if ($resfound) {
             $renewokay = 0;
@@ -1973,13 +2096,12 @@ sub CanBookBeRenewed {
         }
 
     }
-    $sth1->finish;
     return ($renewokay,$error);
 }
 
 =head2 AddRenewal
 
-&AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue]);
+&AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate]);
 
 Renews a loan.
 
@@ -1988,36 +2110,27 @@ has the item.
 
 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<$lastreneweddate> is an optional ISO-formatted date used to set issues.lastreneweddate.  If
+this parameter is not supplied, lastreneweddate is set to the current date.
+
 If C<$datedue> is the empty string, C<&AddRenewal> will calculate the due date automatically
 from the book's item type.
 
 =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 $branch          = shift;
+    my $datedue         = shift;
+    my $lastreneweddate = shift || C4::Dates->new()->output('iso');
     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.
-    unless (@_ and $datedue = shift and $datedue->output('iso')) {
-
-        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'} ,
-			$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.
-								# The question of whether to use item's homebranch calendar is open.
-    }
 
     my $dbh = C4::Context->dbh;
     # Find the issues record for this book
@@ -2029,20 +2142,40 @@ sub AddRenewal {
     $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.
     my $renews = $issuedata->{'renewals'} + 1;
-    $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = CURRENT_DATE
+    $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ?
                             WHERE borrowernumber=? 
                             AND itemnumber=?"
     );
-    $sth->execute( $datedue->output('iso'), $renews, $borrowernumber, $itemnumber );
+    $sth->execute( $datedue->output('iso'), $renews, $lastreneweddate, $borrowernumber, $itemnumber );
     $sth->finish;
 
     # 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 );
@@ -2065,42 +2198,9 @@ sub AddRenewal {
     }
     # Log the renewal
     UpdateStats( $branch, 'renew', $charge, '', $itemnumber, $item->{itype}, $borrowernumber);
+	return $datedue;
 }
 
-
-=head2 ForceRenewal
-
-ForRenewal( $itemnumber, $date, $date_due );
-
-Renews an item for the given date. This function should only be used to update renewals that have occurred in the past.
-
-C<$itemnumber> is the itemnumber of the item being renewed.
-
-C<$date> is the date the renewal took place, in the format YYYY-MM-DD
-
-C<$date_due> is the date the item is now due to be returned, in the format YYYY-MM-DD
-
-=cut
-
-sub ForceRenewal {
-  my ( $itemnumber, $date, $date_due ) = @_;
-  my $dbh = C4::Context->dbh;
-
-  my $sth = $dbh->prepare("SELECT * FROM issues WHERE itemnumber = ? AND returndate IS NULL");
-  $sth->execute( $itemnumber );
-  my $issue = $sth->fetchrow_hashref();
-  $sth->finish();
-  
-
-  $sth = $dbh->prepare('UPDATE issues SET renewals = ?, lastreneweddate = ?, date_due = ? WHERE itemnumber = ? AND returndate IS NULL');
-  $sth->execute( $issue->{'renewals'} + 1, $date, $date_due, $itemnumber );
-  $sth->finish();
-  
-  my $item = GetBiblioFromItemNumber( $itemnumber );
-  UpdateStats( $issue->{'branchcode'}, 'renew', undef, undef, $itemnumber, $item->{ 'itemtype' }, $issue->{'borrowernumber'} );
-}
-
-
 sub GetRenewCount {
     # check renewal status
     my ($bornum,$itemno)=@_;
@@ -2240,7 +2340,6 @@ sub GetTransfers {
     return @row;
 }
 
-
 =head2 GetTransfersFromTo
 
 @results = GetTransfersFromTo($frombranch,$tobranch);
@@ -2315,6 +2414,77 @@ sub AnonymiseIssueHistory {
     return $rows_affected;
 }
 
+=head2 SendCirculationAlert
+
+Send out a C<check-in> or C<checkout> alert using the messaging system.
+
+B<Parameters>:
+
+=over 4
+
+=item type
+
+Valid values for this parameter are: C<CHECKIN> and C<CHECKOUT>.
+
+=item item
+
+Hashref of information about the item being checked in or out.
+
+=item borrower
+
+Hashref of information about the borrower of the item.
+
+=item branch
+
+The branchcode from where the checkout or check-in took place.
+
+=back
+
+B<Example>:
+
+    SendCirculationAlert({
+        type     => 'CHECKOUT',
+        item     => $item,
+        borrower => $borrower,
+        branch   => $branch,
+    });
+
+=cut
+
+sub SendCirculationAlert {
+    my ($opts) = @_;
+    my ($type, $item, $borrower, $branch) =
+        ($opts->{type}, $opts->{item}, $opts->{borrower}, $opts->{branch});
+    my %message_name = (
+        CHECKIN  => 'Item Check-in',
+        CHECKOUT => 'Item Checkout',
+    );
+    my $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences({
+        borrowernumber => $borrower->{borrowernumber},
+        message_name   => $message_name{$type},
+    });
+    my $letter = C4::Letters::getletter('circulation', $type);
+    C4::Letters::parseletter($letter, 'biblio',      $item->{biblionumber});
+    C4::Letters::parseletter($letter, 'biblioitems', $item->{biblionumber});
+    C4::Letters::parseletter($letter, 'borrowers',   $borrower->{borrowernumber});
+    C4::Letters::parseletter($letter, 'branches',    $branch);
+    my @transports = @{ $borrower_preferences->{transports} };
+    # warn "no transports" unless @transports;
+    for (@transports) {
+        # warn "transport: $_";
+        my $message = C4::Message->find_last_message($borrower, $type, $_);
+        if (!$message) {
+            #warn "create new message";
+            C4::Message->enqueue($letter, $borrower, $_);
+        } else {
+            #warn "append to old message";
+            $message->append($letter);
+            $message->update;
+        }
+    }
+    $letter;
+}
+
 =head2 updateWrongTransfer
 
 $items = updateWrongTransfer($itemNumber,$borrowernumber,$waitingAtLibrary,$FromLibrary);
@@ -2364,17 +2534,34 @@ C<$loanlength>  = loan length prior to adjustment
 =cut
 
 sub CalcDateDue { 
-	my ($startdate,$loanlength,$branch) = @_;
+	my ($startdate,$loanlength,$branch,$borrower) = @_;
+	my $datedue;
+
 	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 
-		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 );
-		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') ) ) {
+            my $ceilingDate = C4::Dates->new( C4::Context->preference('ceilingDueDate') );
+            if ( $datedue->output( 'iso' ) gt $ceilingDate->output( 'iso' ) ) {
+                $datedue = $ceilingDate;
+            }
+	}
+
+	return $datedue;
 }
 
 =head2 CheckValidDatedue
@@ -2510,7 +2697,67 @@ $sth->finish;
 return $exist;
 }
 
-1;
+=head2 IsBranchTransferAllowed
+
+$allowed = IsBranchTransferAllowed( $toBranch, $fromBranch, $code );
+
+Code is either an itemtype or collection doe depending on the pref BranchTransferLimitsType
+
+=cut
+
+sub IsBranchTransferAllowed {
+	my ( $toBranch, $fromBranch, $code ) = @_;
+
+	if ( $toBranch eq $fromBranch ) { return 1; } ## Short circuit for speed.
+        
+	my $limitType = C4::Context->preference("BranchTransferLimitsType");   
+	my $dbh = C4::Context->dbh;
+            
+	my $sth = $dbh->prepare("SELECT * FROM branch_transfer_limits WHERE toBranch = ? AND fromBranch = ? AND $limitType = ?");
+	$sth->execute( $toBranch, $fromBranch, $code );
+	my $limit = $sth->fetchrow_hashref();
+                        
+	## If a row is found, then that combination is not allowed, if no matching row is found, then the combination *is allowed*
+	if ( $limit->{'limitId'} ) {
+		return 0;
+	} else {
+		return 1;
+	}
+}                                                        
+
+=head2 CreateBranchTransferLimit
+
+CreateBranchTransferLimit( $toBranch, $fromBranch, $code );
+
+$code is either itemtype or collection code depending on what the pref BranchTransferLimitsType is set to.
+
+=cut
+
+sub CreateBranchTransferLimit {
+   my ( $toBranch, $fromBranch, $code ) = @_;
+
+   my $limitType = C4::Context->preference("BranchTransferLimitsType");
+   
+   my $dbh = C4::Context->dbh;
+   
+   my $sth = $dbh->prepare("INSERT INTO branch_transfer_limits ( $limitType, toBranch, fromBranch ) VALUES ( ?, ?, ? )");
+   $sth->execute( $code, $toBranch, $fromBranch );
+}
+
+=head2 DeleteBranchTransferLimits
+
+DeleteBranchTransferLimits();
+
+=cut
+
+sub DeleteBranchTransferLimits {
+   my $dbh = C4::Context->dbh;
+   my $sth = $dbh->prepare("TRUNCATE TABLE branch_transfer_limits");
+   $sth->execute();
+}
+
+
+  1;
 
 __END__