Bug 15548: Move new patron related code to Patron*
[koha.git] / C4 / Circulation.pm
index 80e5dc4..9b4edf8 100644 (file)
@@ -5,31 +5,30 @@ package C4::Circulation;
 #
 # This file is part of Koha.
 #
-# Koha is free software; you can redistribute it and/or modify it under the
-# terms of the GNU General Public License as published by the Free Software
-# Foundation; either version 2 of the License, or (at your option) any later
-# version.
+# Koha is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
 #
-# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
-# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
-# A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
+# Koha is distributed in the hope that it will be useful, but
+# WITHOUT ANY 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.,
-# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+# You should have received a copy of the GNU General Public License
+# along with Koha; if not, see <http://www.gnu.org/licenses>.
 
 
 use strict;
 #use warnings; FIXME - Bug 2505
 use DateTime;
+use Koha::DateUtils;
 use C4::Context;
 use C4::Stats;
 use C4::Reserves;
 use C4::Biblio;
 use C4::Items;
 use C4::Members;
-use C4::Dates;
-use C4::Dates qw(format_date);
 use C4::Accounts;
 use C4::ItemCirculationAlertPreference;
 use C4::Message;
@@ -41,15 +40,18 @@ use C4::Koha qw(
     GetAuthValCode
     GetKohaAuthorisedValueLib
 );
-use C4::Overdues qw(CalcFine UpdateFine);
+use C4::Overdues qw(CalcFine UpdateFine get_chargeable_units);
 use C4::RotatingCollections qw(GetCollectionItemBranches);
 use Algorithm::CheckDigits;
 
 use Data::Dumper;
 use Koha::DateUtils;
 use Koha::Calendar;
-use Koha::Borrower::Debarments;
+use Koha::Items;
+use Koha::Patrons;
+use Koha::Patron::Debarments;
 use Koha::Database;
+use Koha::Libraries;
 use Carp;
 use List::MoreUtils qw( uniq );
 use Date::Calc qw(
@@ -73,6 +75,7 @@ BEGIN {
                &barcodedecode
         &LostItem
         &ReturnLostItem
+        &GetPendingOnSiteCheckouts
        );
 
        # subs to deal with issuing a book
@@ -94,6 +97,7 @@ BEGIN {
                &AnonymiseIssueHistory
         &CheckIfIssuedToPatron
         &IsItemIssued
+        GetTopIssues
        );
 
        # subs to deal with returns
@@ -377,6 +381,8 @@ sub TooMany {
     my $borrower        = shift;
     my $biblionumber = shift;
        my $item                = shift;
+    my $params = shift;
+    my $onsite_checkout = $params->{onsite_checkout} || 0;
     my $cat_borrower    = $borrower->{'categorycode'};
     my $dbh             = C4::Context->dbh;
        my $branch;
@@ -395,8 +401,11 @@ sub TooMany {
     # rule
     if (defined($issuing_rule) and defined($issuing_rule->{'maxissueqty'})) {
         my @bind_params;
-        my $count_query = "SELECT COUNT(*) FROM issues
-                           JOIN items USING (itemnumber) ";
+        my $count_query = q|
+            SELECT COUNT(*) AS total, COALESCE(SUM(onsite_checkout), 0) AS onsite_checkouts
+            FROM issues
+            JOIN items USING (itemnumber)
+        |;
 
         my $rule_itemtype = $issuing_rule->{itemtype};
         if ($rule_itemtype eq "*") {
@@ -449,13 +458,36 @@ sub TooMany {
             }
         }
 
-        my $count_sth = $dbh->prepare($count_query);
-        $count_sth->execute(@bind_params);
-        my ($current_loan_count) = $count_sth->fetchrow_array;
+        my ( $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $count_query, {}, @bind_params );
+
+        my $max_checkouts_allowed = $issuing_rule->{maxissueqty};
+        my $max_onsite_checkouts_allowed = $issuing_rule->{maxonsiteissueqty};
 
-        my $max_loans_allowed = $issuing_rule->{'maxissueqty'};
-        if ($current_loan_count >= $max_loans_allowed) {
-            return ($current_loan_count, $max_loans_allowed);
+        if ( $onsite_checkout ) {
+            if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
+                return {
+                    reason => 'TOO_MANY_ONSITE_CHECKOUTS',
+                    count => $onsite_checkout_count,
+                    max_allowed => $max_onsite_checkouts_allowed,
+                }
+            }
+        }
+        if ( C4::Context->preference('ConsiderOnSiteCheckoutsAsNormalCheckouts') ) {
+            if ( $checkout_count >= $max_checkouts_allowed ) {
+                return {
+                    reason => 'TOO_MANY_CHECKOUTS',
+                    count => $checkout_count,
+                    max_allowed => $max_checkouts_allowed,
+                };
+            }
+        } elsif ( not $onsite_checkout ) {
+            if ( $checkout_count - $onsite_checkout_count >= $max_checkouts_allowed )  {
+                return {
+                    reason => 'TOO_MANY_CHECKOUTS',
+                    count => $checkout_count - $onsite_checkout_count,
+                    max_allowed => $max_checkouts_allowed,
+                };
+            }
         }
     }
 
@@ -463,9 +495,12 @@ sub TooMany {
     my $branch_borrower_circ_rule = GetBranchBorrowerCircRule($branch, $cat_borrower);
     if (defined($branch_borrower_circ_rule->{maxissueqty})) {
         my @bind_params = ();
-        my $branch_count_query = "SELECT COUNT(*) FROM issues
-                                  JOIN items USING (itemnumber)
-                                  WHERE borrowernumber = ? ";
+        my $branch_count_query = q|
+            SELECT COUNT(*) AS total, COALESCE(SUM(onsite_checkout), 0) AS onsite_checkouts
+            FROM issues
+            JOIN items USING (itemnumber)
+            WHERE borrowernumber = ?
+        |;
         push @bind_params, $borrower->{borrowernumber};
 
         if (C4::Context->preference('CircControl') eq 'PickupLibrary') {
@@ -477,13 +512,35 @@ sub TooMany {
             $branch_count_query .= " AND items.homebranch = ? ";
             push @bind_params, $branch;
         }
-        my $branch_count_sth = $dbh->prepare($branch_count_query);
-        $branch_count_sth->execute(@bind_params);
-        my ($current_loan_count) = $branch_count_sth->fetchrow_array;
+        my ( $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $branch_count_query, {}, @bind_params );
+        my $max_checkouts_allowed = $branch_borrower_circ_rule->{maxissueqty};
+        my $max_onsite_checkouts_allowed = $branch_borrower_circ_rule->{maxonsiteissueqty};
 
-        my $max_loans_allowed = $branch_borrower_circ_rule->{maxissueqty};
-        if ($current_loan_count >= $max_loans_allowed) {
-            return ($current_loan_count, $max_loans_allowed);
+        if ( $onsite_checkout ) {
+            if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
+                return {
+                    reason => 'TOO_MANY_ONSITE_CHECKOUTS',
+                    count => $onsite_checkout_count,
+                    max_allowed => $max_onsite_checkouts_allowed,
+                }
+            }
+        }
+        if ( C4::Context->preference('ConsiderOnSiteCheckoutsAsNormalCheckouts') ) {
+            if ( $checkout_count >= $max_checkouts_allowed ) {
+                return {
+                    reason => 'TOO_MANY_CHECKOUTS',
+                    count => $checkout_count,
+                    max_allowed => $max_checkouts_allowed,
+                };
+            }
+        } elsif ( not $onsite_checkout ) {
+            if ( $checkout_count - $onsite_checkout_count >= $max_checkouts_allowed )  {
+                return {
+                    reason => 'TOO_MANY_CHECKOUTS',
+                    count => $checkout_count - $onsite_checkout_count,
+                    max_allowed => $max_checkouts_allowed,
+                };
+            }
         }
     }
 
@@ -602,7 +659,7 @@ sub itemissues {
 =head2 CanBookBeIssued
 
   ( $issuingimpossible, $needsconfirmation ) =  CanBookBeIssued( $borrower, 
-                      $barcode, $duedatespec, $inprocess, $ignore_reserves );
+                      $barcode, $duedate, $inprocess, $ignore_reserves );
 
 Check if a book can be issued.
 
@@ -614,9 +671,10 @@ C<$issuingimpossible> and C<$needsconfirmation> are some hashref.
 
 =item C<$barcode> is the bar code of the book being issued.
 
-=item C<$duedatespec> is a C4::Dates object.
+=item C<$duedates> is a DateTime object.
 
 =item C<$inprocess> boolean switch
+
 =item C<$ignore_reserves> boolean switch
 
 =back
@@ -694,11 +752,13 @@ if the borrower borrows to much things
 =cut
 
 sub CanBookBeIssued {
-    my ( $borrower, $barcode, $duedate, $inprocess, $ignore_reserves ) = @_;
+    my ( $borrower, $barcode, $duedate, $inprocess, $ignore_reserves, $params ) = @_;
     my %needsconfirmation;    # filled with problems that needs confirmations
     my %issuingimpossible;    # filled with problems that causes the issue to be IMPOSSIBLE
     my %alerts;               # filled with messages that shouldn't stop issuing, but the librarian should be aware of.
 
+    my $onsite_checkout = $params->{onsite_checkout} || 0;
+
     my $item = GetItem(GetItemnumberFromBarcode( $barcode ));
     my $issue = GetItemIssue($item->{itemnumber});
        my $biblioitem = GetBiblioItemData($item->{biblioitemnumber});
@@ -754,36 +814,29 @@ sub CanBookBeIssued {
         ModDateLastSeen( $item->{'itemnumber'} );
         return( { STATS => 1 }, {});
     }
-    if ( $borrower->{flags}->{GNA} ) {
-        $issuingimpossible{GNA} = 1;
-    }
-    if ( $borrower->{flags}->{'LOST'} ) {
-        $issuingimpossible{CARD_LOST} = 1;
-    }
-    if ( $borrower->{flags}->{'DBARRED'} ) {
-        $issuingimpossible{DEBARRED} = 1;
+    if ( ref $borrower->{flags} ) {
+        if ( $borrower->{flags}->{GNA} ) {
+            $issuingimpossible{GNA} = 1;
+        }
+        if ( $borrower->{flags}->{'LOST'} ) {
+            $issuingimpossible{CARD_LOST} = 1;
+        }
+        if ( $borrower->{flags}->{'DBARRED'} ) {
+            $issuingimpossible{DEBARRED} = 1;
+        }
     }
     if ( !defined $borrower->{dateexpiry} || $borrower->{'dateexpiry'} eq '0000-00-00') {
         $issuingimpossible{EXPIRED} = 1;
     } else {
-        my ($y, $m, $d) =  split /-/,$borrower->{'dateexpiry'};
-        if ($y && $m && $d) { # are we really writing oinvalid dates to borrs
-            my $expiry_dt = DateTime->new(
-                year => $y,
-                month => $m,
-                day   => $d,
-                time_zone => C4::Context->tz,
-            );
-            $expiry_dt->truncate( to => 'day');
-            my $today = $now->clone()->truncate(to => 'day');
-            if (DateTime->compare($today, $expiry_dt) == 1) {
-                $issuingimpossible{EXPIRED} = 1;
-            }
-        } else {
-            carp("Invalid expity date in borr");
+        my $expiry_dt = dt_from_string( $borrower->{dateexpiry}, 'sql', 'floating' );
+        $expiry_dt->truncate( to => 'day');
+        my $today = $now->clone()->truncate(to => 'day');
+        $today->set_time_zone( 'floating' );
+        if ( DateTime->compare($today, $expiry_dt) == 1 ) {
             $issuingimpossible{EXPIRED} = 1;
         }
     }
+
     #
     # BORROWER STATUS
     #
@@ -836,23 +889,22 @@ sub CanBookBeIssued {
     }
 
 #
-    # JB34 CHECKS IF BORROWERS DONT HAVE ISSUE TOO MANY BOOKS
+    # JB34 CHECKS IF BORROWERS DON'T HAVE ISSUE TOO MANY BOOKS
     #
-       my ($current_loan_count, $max_loans_allowed) = TooMany( $borrower, $item->{biblionumber}, $item );
-    # if TooMany max_loans_allowed returns 0 the user doesn't have permission to check out this book
-    if (defined $max_loans_allowed && $max_loans_allowed == 0) {
-        $needsconfirmation{PATRON_CANT} = 1;
-    } else {
-        if($max_loans_allowed){
-            if ( C4::Context->preference("AllowTooManyOverride") ) {
-                $needsconfirmation{TOO_MANY} = 1;
-                $needsconfirmation{current_loan_count} = $current_loan_count;
-                $needsconfirmation{max_loans_allowed} = $max_loans_allowed;
-            } else {
-                $issuingimpossible{TOO_MANY} = 1;
-                $issuingimpossible{current_loan_count} = $current_loan_count;
-                $issuingimpossible{max_loans_allowed} = $max_loans_allowed;
-            }
+    my $toomany = TooMany( $borrower, $item->{biblionumber}, $item, { onsite_checkout => $onsite_checkout } );
+    # if TooMany max_allowed returns 0 the user doesn't have permission to check out this book
+    if ( $toomany ) {
+        if ( $toomany->{max_allowed} == 0 ) {
+            $needsconfirmation{PATRON_CANT} = 1;
+        }
+        if ( C4::Context->preference("AllowTooManyOverride") ) {
+            $needsconfirmation{TOO_MANY} = $toomany->{reason};
+            $needsconfirmation{current_loan_count} = $toomany->{count};
+            $needsconfirmation{max_loans_allowed} = $toomany->{max_allowed};
+        } else {
+            $needsconfirmation{TOO_MANY} = $toomany->{reason};
+            $issuingimpossible{current_loan_count} = $toomany->{count};
+            $issuingimpossible{max_loans_allowed} = $toomany->{max_allowed};
         }
     }
 
@@ -946,7 +998,12 @@ sub CanBookBeIssued {
             $item->{'itemnumber'}
         );
         if ( $CanBookBeRenewed == 0 ) {    # no more renewals allowed
-            $issuingimpossible{NO_MORE_RENEWALS} = 1;
+            if ( $renewerror eq 'onsite_checkout' ) {
+                $issuingimpossible{NO_RENEWAL_FOR_ONSITE_CHECKOUTS} = 1;
+            }
+            else {
+                $issuingimpossible{NO_MORE_RENEWALS} = 1;
+            }
         }
         else {
             $needsconfirmation{RENEW_ISSUE} = 1;
@@ -983,7 +1040,7 @@ sub CanBookBeIssued {
                     $needsconfirmation{'rescardnumber'} = $resborrower->{'cardnumber'};
                     $needsconfirmation{'resborrowernumber'} = $resborrower->{'borrowernumber'};
                     $needsconfirmation{'resbranchname'} = $branchname;
-                    $needsconfirmation{'reswaitingdate'} = format_date($res->{'waitingdate'});
+                    $needsconfirmation{'reswaitingdate'} = $res->{'waitingdate'};
                 }
                 elsif ( $restype eq "Reserved" ) {
                     # The item is on reserve for someone else.
@@ -993,7 +1050,7 @@ sub CanBookBeIssued {
                     $needsconfirmation{'rescardnumber'} = $resborrower->{'cardnumber'};
                     $needsconfirmation{'resborrowernumber'} = $resborrower->{'borrowernumber'};
                     $needsconfirmation{'resbranchname'} = $branchname;
-                    $needsconfirmation{'resreservedate'} = format_date($res->{'reservedate'});
+                    $needsconfirmation{'resreservedate'} = $res->{'reservedate'};
                 }
             }
         }
@@ -1160,13 +1217,13 @@ Issue a book. Does no check, they are done in CanBookBeIssued. If we reach this
 
 =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).
+=item C<$datedue> is a DateTime object for the max date of return, i.e. the date due (optional).
 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.
+Defaults to today.  Unlike C<$datedue>, NOT a DateTime object, unfortunately.
 
 AddIssue does the following things :
 
@@ -1193,6 +1250,8 @@ sub AddIssue {
     my $dbh = C4::Context->dbh;
     my $barcodecheck=CheckValidBarcode($barcode);
 
+    my $issue;
+
     if ($datedue && ref $datedue ne 'DateTime') {
         $datedue = dt_from_string($datedue);
     }
@@ -1263,12 +1322,6 @@ sub AddIssue {
         }
 
         # Record in the database the fact that the book was issued.
-        my $sth =
-          $dbh->prepare(
-                "INSERT INTO issues
-                    (borrowernumber, itemnumber,issuedate, date_due, branchcode, onsite_checkout, auto_renew)
-                VALUES (?,?,?,?,?,?,?)"
-          );
         unless ($datedue) {
             my $itype = ( C4::Context->preference('item-level_itypes') ) ? $biblio->{'itype'} : $biblio->{'itemtype'};
             $datedue = CalcDateDue( $issuedate, $itype, $branch, $borrower );
@@ -1276,15 +1329,18 @@ sub AddIssue {
         }
         $datedue->truncate( to => 'minute');
 
-        $sth->execute(
-            $borrower->{'borrowernumber'},      # borrowernumber
-            $item->{'itemnumber'},              # itemnumber
-            $issuedate->strftime('%Y-%m-%d %H:%M:%S'), # issuedate
-            $datedue->strftime('%Y-%m-%d %H:%M:%S'),   # date_due
-            C4::Context->userenv->{'branch'},   # branchcode
-            $onsite_checkout,
-            $auto_renew ? 1 : 0                 # automatic renewal
+        $issue = Koha::Database->new()->schema()->resultset('Issue')->create(
+            {
+                borrowernumber  => $borrower->{'borrowernumber'},
+                itemnumber      => $item->{'itemnumber'},
+                issuedate       => $issuedate->strftime('%Y-%m-%d %H:%M:%S'),
+                date_due        => $datedue->strftime('%Y-%m-%d %H:%M:%S'),
+                branchcode      => C4::Context->userenv->{'branch'},
+                onsite_checkout => $onsite_checkout,
+                auto_renew      => $auto_renew ? 1 : 0
+            }
         );
+
         if ( C4::Context->preference('ReturnToShelvingCart') ) { ## ReturnToShelvingCart is on, anything issued should be taken off the cart.
           CartToShelf( $item->{'itemnumber'} );
         }
@@ -1293,7 +1349,7 @@ sub AddIssue {
             UpdateTotalIssues($item->{'biblionumber'}, 1);
         }
 
-        ## If item was lost, it has now been found, reverse any list item charges if neccessary.
+        ## If item was lost, it has now been found, reverse any list item charges if necessary.
         if ( $item->{'itemlost'} ) {
             if ( C4::Context->preference('RefundLostItemFeeOnReturn' ) ) {
                 _FixAccountForLostAndReturned( $item->{'itemnumber'}, undef, $item->{'barcode'} );
@@ -1354,7 +1410,7 @@ sub AddIssue {
     logaction("CIRCULATION", "ISSUE", $borrower->{'borrowernumber'}, $biblio->{'itemnumber'})
         if C4::Context->preference("IssueLog");
   }
-  return ($datedue);   # not necessarily the same as when it came in!
+  return $issue;
 }
 
 =head2 GetLoanLength
@@ -1519,6 +1575,10 @@ maxissueqty - maximum number of loans that a
 patron of the given category can have at the given
 branch.  If the value is undef, no limit.
 
+maxonsiteissueqty - maximum of on-site checkouts that a
+patron of the given category can have at the given
+branch.  If the value is undef, no limit.
+
 This will first check for a specific branch and
 category match from branch_borrower_circ_rules. 
 
@@ -1532,6 +1592,7 @@ If no rule has been found in the database, it will default to
 the buillt in rule:
 
 maxissueqty - undef
+maxonsiteissueqty - undef
 
 C<$branchcode> and C<$categorycode> should contain the
 literal branch code and patron category code, respectively - no
@@ -1540,53 +1601,45 @@ wildcards.
 =cut
 
 sub GetBranchBorrowerCircRule {
-    my $branchcode = shift;
-    my $categorycode = shift;
+    my ( $branchcode, $categorycode ) = @_;
 
-    my $branch_cat_query = "SELECT maxissueqty
-                            FROM branch_borrower_circ_rules
-                            WHERE branchcode = ?
-                            AND   categorycode = ?";
+    my $rules;
     my $dbh = C4::Context->dbh();
-    my $sth = $dbh->prepare($branch_cat_query);
-    $sth->execute($branchcode, $categorycode);
-    my $result;
-    if ($result = $sth->fetchrow_hashref()) {
-        return $result;
-    }
+    $rules = $dbh->selectrow_hashref( q|
+        SELECT maxissueqty, maxonsiteissueqty
+        FROM branch_borrower_circ_rules
+        WHERE branchcode = ?
+        AND   categorycode = ?
+    |, {}, $branchcode, $categorycode ) ;
+    return $rules if $rules;
 
     # try same branch, default borrower category
-    my $branch_query = "SELECT maxissueqty
-                        FROM default_branch_circ_rules
-                        WHERE branchcode = ?";
-    $sth = $dbh->prepare($branch_query);
-    $sth->execute($branchcode);
-    if ($result = $sth->fetchrow_hashref()) {
-        return $result;
-    }
+    $rules = $dbh->selectrow_hashref( q|
+        SELECT maxissueqty, maxonsiteissueqty
+        FROM default_branch_circ_rules
+        WHERE branchcode = ?
+    |, {}, $branchcode ) ;
+    return $rules if $rules;
 
     # try default branch, same borrower category
-    my $category_query = "SELECT maxissueqty
-                          FROM default_borrower_circ_rules
-                          WHERE categorycode = ?";
-    $sth = $dbh->prepare($category_query);
-    $sth->execute($categorycode);
-    if ($result = $sth->fetchrow_hashref()) {
-        return $result;
-    }
-  
+    $rules = $dbh->selectrow_hashref( q|
+        SELECT maxissueqty, maxonsiteissueqty
+        FROM default_borrower_circ_rules
+        WHERE categorycode = ?
+    |, {}, $categorycode ) ;
+    return $rules if $rules;
+
     # try default branch, default borrower category
-    my $default_query = "SELECT maxissueqty
-                          FROM default_circ_rules";
-    $sth = $dbh->prepare($default_query);
-    $sth->execute();
-    if ($result = $sth->fetchrow_hashref()) {
-        return $result;
-    }
-    
+    $rules = $dbh->selectrow_hashref( q|
+        SELECT maxissueqty, maxonsiteissueqty
+        FROM default_circ_rules
+    |, {} );
+    return $rules if $rules;
+
     # built-in default circulation rule
     return {
         maxissueqty => undef,
+        maxonsiteissueqty => undef,
     };
 }
 
@@ -1607,6 +1660,7 @@ holdallowed => Hold policy for this branch and itemtype. Possible values:
 returnbranch => branch to which to return item.  Possible values:
   noreturn: do not return, let item remain where checked in (floating collections)
   homebranch: return to item's home branch
+  holdingbranch: return to issuer branch
 
 This searches branchitemrules in the following order:
 
@@ -1725,6 +1779,14 @@ fields from the reserves table of the Koha database, and
 C<biblioitemnumber>. It also has the key C<ResFound>, whose value is
 either C<Waiting>, C<Reserved>, or 0.
 
+=item C<WasReturned>
+
+Value 1 if return is successful.
+
+=item C<NeedsTransfer>
+
+If AutomaticItemReturn is disabled, return branch is given as value of NeedsTransfer.
+
 =back
 
 C<$iteminformation> is a reference-to-hash, giving information about the
@@ -1738,7 +1800,7 @@ patron who last borrowed the book.
 sub AddReturn {
     my ( $barcode, $branch, $exemptfine, $dropbox, $return_date, $dropboxdate ) = @_;
 
-    if ($branch and not GetBranchDetail($branch)) {
+    if ($branch and not Koha::Libraries->find($branch)) {
         warn "AddReturn error: branch '$branch' not found.  Reverting to " . C4::Context->userenv->{'branch'};
         undef $branch;
     }
@@ -1756,10 +1818,9 @@ sub AddReturn {
         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"
+            or die "Data inconsistency: barcode $barcode (itemnumber:$itemnumber) claims to be issued to non-existent borrowernumber '$issue->{borrowernumber}'\n"
                 . Dumper($issue) . "\n";
     } else {
         $messages->{'NotIssued'} = $barcode;
@@ -1788,9 +1849,9 @@ sub AddReturn {
 
         # full item data, but no borrowernumber or checkout info (no issue)
         # we know GetItem should work because GetItemnumberFromBarcode worked
-    my $hbr      = GetBranchItemRule($item->{'homebranch'}, $item->{'itype'})->{'returnbranch'} || "homebranch";
+    my $hbr = GetBranchItemRule($item->{'homebranch'}, $item->{'itype'})->{'returnbranch'} || "homebranch";
         # get the proper branch to which to return the item
-    $hbr = $item->{$hbr} || $branch ;
+    my $returnbranch = $item->{$hbr} || $branch ;
         # if $hbr was "noreturn" or any other non-item table value, then it should 'float' (i.e. stay at this branch)
 
     my $borrowernumber = $borrower->{'borrowernumber'} || undef;    # we don't know if we had a borrower or not
@@ -1817,9 +1878,9 @@ sub AddReturn {
 
     # 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 ) {
+    if ( $returnbranch ) {
         my $branches = GetBranches();    # a potentially expensive call for a non-feature.
-        $branches->{$hbr}->{PE} and $messages->{'IsPermanent'} = $hbr;
+        $branches->{$returnbranch}->{PE} and $messages->{'IsPermanent'} = $returnbranch;
     }
 
     # check if the return is allowed at this branch
@@ -1849,7 +1910,7 @@ sub AddReturn {
             # define circControlBranch only if dropbox mode is set
             # don't allow dropbox mode to create an invalid entry in issues (issuedate > today)
             # FIXME: check issuedate > returndate, factoring in holidays
-            #$circControlBranch = _GetCircControlBranch($item,$borrower) unless ( $item->{'issuedate'} eq C4::Dates->today('iso') );;
+
             $circControlBranch = _GetCircControlBranch($item,$borrower);
             $issue->{'overdue'} = DateTime->compare($issue->{'date_due'}, $dropboxdate ) == -1 ? 1 : 0;
         }
@@ -1875,24 +1936,48 @@ sub AddReturn {
 
                 if ( C4::Context->preference('finesMode') eq 'production' ) {
                     if ( $amount > 0 ) {
-                        C4::Overdues::UpdateFine( $issue->{itemnumber},
-                            $issue->{borrowernumber},
-                            $amount, $type, output_pref($datedue) );
+                        C4::Overdues::UpdateFine(
+                            {
+                                issue_id       => $issue->{issue_id},
+                                itemnumber     => $issue->{itemnumber},
+                                borrowernumber => $issue->{borrowernumber},
+                                amount         => $amount,
+                                type           => $type,
+                                due            => output_pref($datedue),
+                            }
+                        );
                     }
                     elsif ($return_date) {
 
-                       # Backdated returns may have fines that shouldn't exist,
-                       # so in this case, we need to drop those fines to 0
-
-                        C4::Overdues::UpdateFine( $issue->{itemnumber},
-                            $issue->{borrowernumber},
-                            0, $type, output_pref($datedue) );
+                        # Backdated returns may have fines that shouldn't exist,
+                        # so in this case, we need to drop those fines to 0
+
+                        C4::Overdues::UpdateFine(
+                            {
+                                issue_id       => $issue->{issue_id},
+                                itemnumber     => $issue->{itemnumber},
+                                borrowernumber => $issue->{borrowernumber},
+                                amount         => 0,
+                                type           => $type,
+                                due            => output_pref($datedue),
+                            }
+                        );
                     }
                 }
             }
 
-            MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
-                $circControlBranch, $return_date, $borrower->{'privacy'} );
+            eval {
+                MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
+                    $circControlBranch, $return_date, $borrower->{'privacy'} );
+            };
+            if ( $@ ) {
+                $messages->{'Wrongbranch'} = {
+                    Wrongbranch => $branch,
+                    Rightbranch => $message
+                };
+                carp $@;
+                return ( 0, { WasReturned => 0 }, $issue, $borrower );
+            }
 
             # FIXME is the "= 1" right?  This could be the borrower hash.
             $messages->{'WasReturned'} = 1;
@@ -2023,21 +2108,18 @@ sub AddReturn {
         DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
     }
 
-    # 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 ( !$is_in_rotating_collection && ($doreturn or $messages->{'NotIssued'}) and !$resfound and ($branch ne $hbr) and not $messages->{'WrongTransfer'}){
-        if ( C4::Context->preference("AutomaticItemReturn"    ) or
+    # Transfer to returnbranch if Automatic transfer set or append message NeedsTransfer
+    if (!$is_in_rotating_collection && ($doreturn or $messages->{'NotIssued'}) and !$resfound and ($branch ne $returnbranch) and not $messages->{'WrongTransfer'}){
+        if  (C4::Context->preference("AutomaticItemReturn"    ) or
             (C4::Context->preference("UseBranchTransferLimits") and
-             ! IsBranchTransferAllowed($branch, $hbr, $item->{C4::Context->preference("BranchTransferLimitsType")} )
+             ! IsBranchTransferAllowed($branch, $returnbranch, $item->{C4::Context->preference("BranchTransferLimitsType")} )
            )) {
-            $debug and warn sprintf "about to call ModItemTransfer(%s, %s, %s)", $item->{'itemnumber'},$branch, $hbr;
+            $debug and warn sprintf "about to call ModItemTransfer(%s, %s, %s)", $item->{'itemnumber'},$branch, $returnbranch;
             $debug and warn "item: " . Dumper($item);
-            ModItemTransfer($item->{'itemnumber'}, $branch, $hbr);
+            ModItemTransfer($item->{'itemnumber'}, $branch, $returnbranch);
             $messages->{'WasTransfered'} = 1;
         } else {
-            $messages->{'NeedsTransfer'} = 1;   # TODO: instead of 1, specify branchcode that the transfer SHOULD go to, $item->{homebranch}
+            $messages->{'NeedsTransfer'} = $returnbranch;
         }
     }
 
@@ -2070,6 +2152,16 @@ routine in C<C4::Accounts>.
 sub MarkIssueReturned {
     my ( $borrowernumber, $itemnumber, $dropbox_branch, $returndate, $privacy ) = @_;
 
+    my $anonymouspatron;
+    if ( $privacy == 2 ) {
+        # The default of 0 will not work due to foreign key constraints
+        # The anonymisation will fail if AnonymousPatron is not a valid entry
+        # We need to check if the anonymous patron exist, Koha will fail loudly if it does not
+        # Note that a warning should appear on the about page (System information tab).
+        $anonymouspatron = C4::Context->preference('AnonymousPatron');
+        die "Fatal error: the patron ($borrowernumber) has requested their circulation history be anonymized on check-in, but the AnonymousPatron system preference is empty or not set correctly."
+            unless C4::Members::GetMember( borrowernumber => $anonymouspatron );
+    }
     my $dbh   = C4::Context->dbh;
     my $query = 'UPDATE issues SET returndate=';
     my @bind;
@@ -2095,10 +2187,6 @@ sub MarkIssueReturned {
     $sth_copy->execute($borrowernumber, $itemnumber);
     # anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber
     if ( $privacy == 2) {
-        # The default of 0 does not work due to foreign key constraints
-        # The anonymisation will fail quietly if AnonymousPatron is not a valid entry
-        # FIXME the above is unacceptable - bug 9942 relates
-        my $anonymouspatron = (C4::Context->preference('AnonymousPatron')) ? C4::Context->preference('AnonymousPatron') : 0;
         my $sth_ano = $dbh->prepare("UPDATE old_issues SET borrowernumber=?
                                   WHERE borrowernumber = ?
                                   AND itemnumber = ?");
@@ -2110,6 +2198,12 @@ sub MarkIssueReturned {
     $sth_del->execute($borrowernumber, $itemnumber);
 
     ModItem( { 'onloan' => undef }, undef, $itemnumber );
+
+    if ( C4::Context->preference('StoreLastBorrower') ) {
+        my $item = Koha::Items->find( $itemnumber );
+        my $patron = Koha::Patrons->find( $borrowernumber );
+        $item->last_returned_by( $patron );
+    }
 }
 
 =head2 _debar_user_on_return
@@ -2135,16 +2229,13 @@ sub _debar_user_on_return {
     my ( $borrower, $item, $dt_due, $dt_today ) = @_;
 
     my $branchcode = _GetCircControlBranch( $item, $borrower );
-    my $calendar = Koha::Calendar->new( branchcode => $branchcode );
-
-    # $deltadays is a DateTime::Duration object
-    my $deltadays = $calendar->days_between( $dt_due, $dt_today );
 
     my $circcontrol = C4::Context->preference('CircControl');
     my $issuingrule =
       GetIssuingRule( $borrower->{categorycode}, $item->{itype}, $branchcode );
     my $finedays = $issuingrule->{finedays};
     my $unit     = $issuingrule->{lengthunit};
+    my $chargeable_units = C4::Overdues::get_chargeable_units($unit, $dt_due, $dt_today, $branchcode);
 
     if ($finedays) {
 
@@ -2156,6 +2247,9 @@ sub _debar_user_on_return {
         my $grace =
           DateTime::Duration->new( $unit => $issuingrule->{firstremind} );
 
+        my $deltadays = DateTime::Duration->new(
+            days => $chargeable_units
+        );
         if ( $deltadays->subtract($grace)->is_positive() ) {
             my $suspension_days = $deltadays * $finedays;
 
@@ -2171,13 +2265,13 @@ sub _debar_user_on_return {
             my $new_debar_dt =
               $dt_today->clone()->add_duration( $suspension_days );
 
-            Koha::Borrower::Debarments::AddUniqueDebarment({
+            Koha::Patron::Debarments::AddUniqueDebarment({
                 borrowernumber => $borrower->{borrowernumber},
                 expiration     => $new_debar_dt->ymd(),
                 type           => 'SUSPENSION',
             });
             # if borrower was already debarred but does not get an extra debarment
-            if ( $borrower->{debarred} eq Koha::Borrower::Debarments::IsDebarred($borrower->{borrowernumber}) ) {
+            if ( $borrower->{debarred} eq Koha::Patron::Debarments::IsDebarred($borrower->{borrowernumber}) ) {
                     return ($borrower->{debarred},1);
             }
             return $new_debar_dt->ymd();
@@ -2402,6 +2496,8 @@ sub GetItemIssue {
     $sth->execute($itemnumber);
     my $data = $sth->fetchrow_hashref;
     return unless $data;
+    $data->{issuedate_sql} = $data->{issuedate};
+    $data->{date_due_sql} = $data->{date_due};
     $data->{issuedate} = dt_from_string($data->{issuedate}, 'sql');
     $data->{issuedate}->truncate(to => 'minute');
     $data->{date_due} = dt_from_string($data->{date_due}, 'sql');
@@ -2604,13 +2700,12 @@ sub GetUpcomingDueIssues {
     my $dbh = C4::Context->dbh;
 
     my $statement = <<END_SQL;
-SELECT issues.*, items.itype as itemtype, items.homebranch, DATE( date_due )-DATE( NOW() ) as days_until_due, branches.branchemail
+SELECT issues.*, items.itype as itemtype, items.homebranch, TO_DAYS( date_due )-TO_DAYS( NOW() ) as days_until_due, branches.branchemail
 FROM issues 
 LEFT JOIN items USING (itemnumber)
 LEFT OUTER JOIN branches USING (branchcode)
 WHERE returndate is NULL
-AND (DATE( date_due )-DATE( NOW() )) >= 0
-AND (DATE( date_due )-DATE( NOW() )) <= ?
+HAVING days_until_due >= 0 AND days_until_due <= ?
 END_SQL
 
     my @bind_parameters = ( $params->{'days_in_advance'} );
@@ -2654,6 +2749,7 @@ sub CanBookBeRenewed {
 
     my $item      = GetItem($itemnumber)      or return ( 0, 'no_item' );
     my $itemissue = GetItemIssue($itemnumber) or return ( 0, 'no_checkout' );
+    return ( 0, 'onsite_checkout' ) if $itemissue->{onsite_checkout};
 
     $borrowernumber ||= $itemissue->{borrowernumber};
     my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber )
@@ -2668,56 +2764,63 @@ sub CanBookBeRenewed {
     {
         my $schema = Koha::Database->new()->schema();
 
-        # Get all other items that could possibly fill reserves
-        my @itemnumbers = $schema->resultset('Item')->search(
-            {
-                biblionumber => $resrec->{biblionumber},
-                onloan       => undef,
-                -not         => { itemnumber => $itemnumber }
-            },
-            { columns => 'itemnumber' }
-        )->get_column('itemnumber')->all();
-
-        # Get all other reserves that could have been filled by this item
-        my @borrowernumbers;
-        while (1) {
-            my ( $reserve_found, $reserve, undef ) =
-              C4::Reserves::CheckReserves( $itemnumber, undef, undef,
-                \@borrowernumbers );
-
-            if ($reserve_found) {
-                push( @borrowernumbers, $reserve->{borrowernumber} );
-            }
-            else {
-                last;
-            }
+        my $item_holds = $schema->resultset('Reserve')->search( { itemnumber => $itemnumber, found => undef } )->count();
+        if ($item_holds) {
+            # There is an item level hold on this item, no other item can fill the hold
+            $resfound = 1;
         }
+        else {
 
-        # If the count of the union of the lists of reservable items for each borrower
-        # is equal or greater than the number of borrowers, we know that all reserves
-        # can be filled with available items. We can get the union of the sets simply
-        # by pushing all the elements onto an array and removing the duplicates.
-        my @reservable;
-        foreach my $b (@borrowernumbers) {
-            my ( $borr ) = C4::Members::GetMemberDetails( $b );
-            foreach my $i (@itemnumbers) {
-                my $item   = GetItem($i);
-                if (   IsAvailableForItemLevelRequest($item, $borr)
-                    && CanItemBeReserved( $b, $i )
-                    && !IsItemOnHoldAndFound($i) )
+            # Get all other items that could possibly fill reserves
+            my @itemnumbers = $schema->resultset('Item')->search(
                 {
-                    push( @reservable, $i );
+                    biblionumber => $resrec->{biblionumber},
+                    onloan       => undef,
+                    notforloan   => 0,
+                    -not         => { itemnumber => $itemnumber }
+                },
+                { columns => 'itemnumber' }
+            )->get_column('itemnumber')->all();
+
+            # Get all other reserves that could have been filled by this item
+            my @borrowernumbers;
+            while (1) {
+                my ( $reserve_found, $reserve, undef ) =
+                  C4::Reserves::CheckReserves( $itemnumber, undef, undef, \@borrowernumbers );
+
+                if ($reserve_found) {
+                    push( @borrowernumbers, $reserve->{borrowernumber} );
+                }
+                else {
+                    last;
+                }
+            }
+
+            # If the count of the union of the lists of reservable items for each borrower
+            # is equal or greater than the number of borrowers, we know that all reserves
+            # can be filled with available items. We can get the union of the sets simply
+            # by pushing all the elements onto an array and removing the duplicates.
+            my @reservable;
+            foreach my $b (@borrowernumbers) {
+                my ($borr) = C4::Members::GetMemberDetails($b);
+                foreach my $i (@itemnumbers) {
+                    my $item = GetItem($i);
+                    if (   IsAvailableForItemLevelRequest( $item, $borr )
+                        && CanItemBeReserved( $b, $i )
+                        && !IsItemOnHoldAndFound($i) )
+                    {
+                        push( @reservable, $i );
+                    }
                 }
             }
-        }
 
-        @reservable = uniq(@reservable);
+            @reservable = uniq(@reservable);
 
-        if ( @reservable >= @borrowernumbers ) {
-            $resfound = 0;
+            if ( @reservable >= @borrowernumbers ) {
+                $resfound = 0;
+            }
         }
     }
-
     return ( 0, "on_reserve" ) if $resfound;    # '' when no hold was found
 
     return ( 1, undef ) if $override_limit;
@@ -2729,22 +2832,53 @@ sub CanBookBeRenewed {
     return ( 0, "too_many" )
       if $issuingrule->{renewalsallowed} <= $itemissue->{renewals};
 
-    if ( $issuingrule->{norenewalbefore} ) {
+    my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing');
+    my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing');
+    my $restricted = Koha::Patron::Debarments::IsDebarred($borrowernumber);
+    my $hasoverdues = C4::Members::HasOverdues($borrowernumber);
 
-        # Get current time and add norenewalbefore.
-        # If this is smaller than date_due, it's too soon for renewal.
-        if (
-            DateTime->now( time_zone => C4::Context->tz() )->add(
-                $issuingrule->{lengthunit} => $issuingrule->{norenewalbefore}
-            ) < $itemissue->{date_due}
-          )
+    if ( $restricted and $restrictionblockrenewing ) {
+        return ( 0, 'restriction');
+    } elsif ( ($hasoverdues and $overduesblockrenewing eq 'block') || ($itemissue->{overdue} and $overduesblockrenewing eq 'blockitem') ) {
+        return ( 0, 'overdue');
+    }
+
+    if ( defined $issuingrule->{norenewalbefore}
+        and $issuingrule->{norenewalbefore} ne "" )
+    {
+
+        # Calculate soonest renewal by subtracting 'No renewal before' from due date
+        my $soonestrenewal =
+          $itemissue->{date_due}->clone()
+          ->subtract(
+            $issuingrule->{lengthunit} => $issuingrule->{norenewalbefore} );
+
+        # Depending on syspref reset the exact time, only check the date
+        if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date'
+            and $issuingrule->{lengthunit} eq 'days' )
+        {
+            $soonestrenewal->truncate( to => 'day' );
+        }
+
+        if ( $soonestrenewal > DateTime->now( time_zone => C4::Context->tz() ) )
         {
             return ( 0, "auto_too_soon" ) if $itemissue->{auto_renew};
             return ( 0, "too_soon" );
         }
+        elsif ( $itemissue->{auto_renew} ) {
+            return ( 0, "auto_renew" );
+        }
+    }
+
+    # Fallback for automatic renewals:
+    # If norenewalbefore is undef, don't renew before due date.
+    elsif ( $itemissue->{auto_renew} ) {
+        my $now = dt_from_string;
+        return ( 0, "auto_renew" )
+          if $now >= $itemissue->{date_due};
+        return ( 0, "auto_too_soon" );
     }
 
-    return ( 0, "auto_renew" ) if $itemissue->{auto_renew};
     return ( 1, undef );
 }
 
@@ -2762,7 +2896,7 @@ C<$itemnumber> is the number of the item to renew.
 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 DateTime 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.
@@ -2956,15 +3090,22 @@ sub GetSoonestRenewDate {
     my $issuingrule =
       GetIssuingRule( $borrower->{categorycode}, $item->{itype}, $branchcode );
 
-    my $now = DateTime->now( time_zone => C4::Context->tz() );
+    my $now = dt_from_string;
 
-    if ( $issuingrule->{norenewalbefore} ) {
+    if ( defined $issuingrule->{norenewalbefore}
+        and $issuingrule->{norenewalbefore} ne "" )
+    {
         my $soonestrenewal =
-          $itemissue->{date_due}->subtract(
+          $itemissue->{date_due}->clone()
+          ->subtract(
             $issuingrule->{lengthunit} => $issuingrule->{norenewalbefore} );
 
-        $soonestrenewal = $now > $soonestrenewal ? $now : $soonestrenewal;
-        return $soonestrenewal;
+        if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date'
+            and $issuingrule->{lengthunit} eq 'days' )
+        {
+            $soonestrenewal->truncate( to => 'day' );
+        }
+        return $soonestrenewal if $now < $soonestrenewal;
     }
     return $now;
 }
@@ -3187,8 +3328,9 @@ sub AnonymiseIssueHistory {
     ";
 
     # The default of 0 does not work due to foreign key constraints
-    # The anonymisation will fail quietly if AnonymousPatron is not a valid entry
-    my $anonymouspatron = (C4::Context->preference('AnonymousPatron')) ? C4::Context->preference('AnonymousPatron') : 0;
+    # The anonymisation should not fail quietly if AnonymousPatron is not a valid entry
+    # Set it to undef (NULL)
+    my $anonymouspatron = C4::Context->preference('AnonymousPatron') || undef;
     my @bind_params = ($anonymouspatron, $date);
     if (defined $borrowernumber) {
        $query .= " AND borrowernumber = ?";
@@ -3254,19 +3396,6 @@ sub SendCirculationAlert {
         message_name   => $message_name{$type},
     });
     my $issues_table = ( $type eq 'CHECKOUT' || $type eq 'RENEWAL' ) ? 'issues' : 'old_issues';
-    my $letter =  C4::Letters::GetPreparedLetter (
-        module => 'circulation',
-        letter_code => $type,
-        branchcode => $branch,
-        tables => {
-            $issues_table => $item->{itemnumber},
-            'items'       => $item->{itemnumber},
-            'biblio'      => $item->{biblionumber},
-            'biblioitems' => $item->{biblionumber},
-            'borrowers'   => $borrower,
-            'branches'    => $branch,
-        }
-    ) or return;
 
     my @transports = keys %{ $borrower_preferences->{transports} };
     # warn "no transports" unless @transports;
@@ -3275,15 +3404,43 @@ sub SendCirculationAlert {
         my $message = C4::Message->find_last_message($borrower, $type, $_);
         if (!$message) {
             #warn "create new message";
+            my $letter =  C4::Letters::GetPreparedLetter (
+                module => 'circulation',
+                letter_code => $type,
+                branchcode => $branch,
+                message_transport_type => $_,
+                tables => {
+                    $issues_table => $item->{itemnumber},
+                    'items'       => $item->{itemnumber},
+                    'biblio'      => $item->{biblionumber},
+                    'biblioitems' => $item->{biblionumber},
+                    'borrowers'   => $borrower,
+                    'branches'    => $branch,
+                }
+            ) or next;
             C4::Message->enqueue($letter, $borrower, $_);
         } else {
             #warn "append to old message";
+            my $letter =  C4::Letters::GetPreparedLetter (
+                module => 'circulation',
+                letter_code => $type,
+                branchcode => $branch,
+                message_transport_type => $_,
+                tables => {
+                    $issues_table => $item->{itemnumber},
+                    'items'       => $item->{itemnumber},
+                    'biblio'      => $item->{biblionumber},
+                    'biblioitems' => $item->{biblionumber},
+                    'borrowers'   => $borrower,
+                    'branches'    => $branch,
+                }
+            ) or next;
             $message->append($letter);
             $message->update;
         }
     }
 
-    return $letter;
+    return;
 }
 
 =head2 updateWrongTransfer
@@ -3330,7 +3487,7 @@ $newdatedue = CalcDateDue($startdate,$itemtype,$branchcode,$borrower);
 
 this function calculates the due date given the start date and configured circulation rules,
 checking against the holidays calendar as per the 'useDaysMode' syspref.
-C<$startdate>   = C4::Dates object representing start date of loan period (assumed to be today)
+C<$startdate>   = DateTime object representing start date of loan period (assumed to be today)
 C<$itemtype>  = itemtype code of item in question
 C<$branch>  = location whose calendar to use
 C<$borrower> = Borrower object
@@ -3391,7 +3548,7 @@ sub CalcDateDue {
         }
     }
 
-    # if Hard Due Dates are used, retreive them and apply as necessary
+    # if Hard Due Dates are used, retrieve them and apply as necessary
     my ( $hardduedate, $hardduedatecompare ) =
       GetHardDueDate( $borrower->{'categorycode'}, $itemtype, $branch );
     if ($hardduedate) {    # hardduedates are currently dates
@@ -3413,10 +3570,13 @@ sub CalcDateDue {
 
     # if ReturnBeforeExpiry ON the datedue can't be after borrower expirydate
     if ( C4::Context->preference('ReturnBeforeExpiry') ) {
-        my $expiry_dt = dt_from_string( $borrower->{dateexpiry}, 'iso' );
-        $expiry_dt->set( hour => 23, minute => 59);
-        if ( DateTime->compare( $datedue, $expiry_dt ) == 1 ) {
-            $datedue = $expiry_dt->clone;
+        my $expiry_dt = dt_from_string( $borrower->{dateexpiry}, 'iso', 'floating');
+        if( $expiry_dt ) { #skip empty expiry date..
+            $expiry_dt->set( hour => 23, minute => 59);
+            my $d1= $datedue->clone->set_time_zone('floating');
+            if ( DateTime->compare( $d1, $expiry_dt ) == 1 ) {
+                $datedue = $expiry_dt->clone->set_time_zone( C4::Context->tz );
+            }
         }
     }
 
@@ -3424,92 +3584,6 @@ sub CalcDateDue {
 }
 
 
-=head2 CheckRepeatableHolidays
-
-  $countrepeatable = CheckRepeatableHoliday($itemnumber,$week_day,$branchcode);
-
-This function checks if the date due is a repeatable holiday
-
-C<$date_due>   = returndate calculate with no day check
-C<$itemnumber>  = itemnumber
-C<$branchcode>  = localisation of issue 
-
-=cut
-
-sub CheckRepeatableHolidays{
-my($itemnumber,$week_day,$branchcode)=@_;
-my $dbh = C4::Context->dbh;
-my $query = qq|SELECT count(*)  
-       FROM repeatable_holidays 
-       WHERE branchcode=?
-       AND weekday=?|;
-my $sth = $dbh->prepare($query);
-$sth->execute($branchcode,$week_day);
-my $result=$sth->fetchrow;
-return $result;
-}
-
-
-=head2 CheckSpecialHolidays
-
-  $countspecial = CheckSpecialHolidays($years,$month,$day,$itemnumber,$branchcode);
-
-This function check if the date is a special holiday
-
-C<$years>   = the years of datedue
-C<$month>   = the month of datedue
-C<$day>     = the day of datedue
-C<$itemnumber>  = itemnumber
-C<$branchcode>  = localisation of issue 
-
-=cut
-
-sub CheckSpecialHolidays{
-my ($years,$month,$day,$itemnumber,$branchcode) = @_;
-my $dbh = C4::Context->dbh;
-my $query=qq|SELECT count(*) 
-            FROM `special_holidays`
-            WHERE year=?
-            AND month=?
-            AND day=?
-             AND branchcode=?
-           |;
-my $sth = $dbh->prepare($query);
-$sth->execute($years,$month,$day,$branchcode);
-my $countspecial=$sth->fetchrow ;
-return $countspecial;
-}
-
-=head2 CheckRepeatableSpecialHolidays
-
-  $countspecial = CheckRepeatableSpecialHolidays($month,$day,$itemnumber,$branchcode);
-
-This function check if the date is a repeatble special holidays
-
-C<$month>   = the month of datedue
-C<$day>     = the day of datedue
-C<$itemnumber>  = itemnumber
-C<$branchcode>  = localisation of issue 
-
-=cut
-
-sub CheckRepeatableSpecialHolidays{
-my ($month,$day,$itemnumber,$branchcode) = @_;
-my $dbh = C4::Context->dbh;
-my $query=qq|SELECT count(*) 
-            FROM `repeatable_holidays`
-            WHERE month=?
-            AND day=?
-             AND branchcode=?
-           |;
-my $sth = $dbh->prepare($query);
-$sth->execute($month,$day,$branchcode);
-my $countspecial=$sth->fetchrow ;
-return $countspecial;
-}
-
-
-
 sub CheckValidBarcode{
 my ($barcode) = @_;
 my $dbh = C4::Context->dbh;
@@ -3769,8 +3843,6 @@ sub TransferSlip {
     my $item =  GetItem( $itemnumber, $barcode )
       or return;
 
-    my $pulldate = C4::Dates->new();
-
     return C4::Letters::GetPreparedLetter (
         module => 'circulation',
         letter_code => 'TRANSFERSLIP',
@@ -3794,12 +3866,15 @@ sub TransferSlip {
 sub CheckIfIssuedToPatron {
     my ($borrowernumber, $biblionumber) = @_;
 
-    my $items = GetItemsByBiblioitemnumber($biblionumber);
-
-    foreach my $item (@{$items}) {
-        return 1 if ($item->{borrowernumber} && $item->{borrowernumber} eq $borrowernumber);
-    }
-
+    my $dbh = C4::Context->dbh;
+    my $query = q|
+        SELECT COUNT(*) FROM issues
+        LEFT JOIN items ON items.itemnumber = issues.itemnumber
+        WHERE items.biblionumber = ?
+        AND issues.borrowernumber = ?
+    |;
+    my $is_issued = $dbh->selectrow_array($query, {}, $biblionumber, $borrowernumber );
+    return 1 if $is_issued;
     return;
 }
 
@@ -3846,7 +3921,7 @@ sub GetAgeRestriction {
     my @values = split ' ', uc($record_restrictions);
     return unless @values;
 
-    # Search first occurence of one of the markers
+    # Search first occurrence of one of the markers
     my @markers = split /\|/, uc($markers);
     return unless @markers;
 
@@ -3894,8 +3969,99 @@ sub GetAgeRestriction {
     return ($restriction_year);
 }
 
-1;
 
+=head2 GetPendingOnSiteCheckouts
+
+=cut
+
+sub GetPendingOnSiteCheckouts {
+    my $dbh = C4::Context->dbh;
+    return $dbh->selectall_arrayref(q|
+        SELECT
+          items.barcode,
+          items.biblionumber,
+          items.itemnumber,
+          items.itemnotes,
+          items.itemcallnumber,
+          items.location,
+          issues.date_due,
+          issues.branchcode,
+          issues.date_due < NOW() AS is_overdue,
+          biblio.author,
+          biblio.title,
+          borrowers.firstname,
+          borrowers.surname,
+          borrowers.cardnumber,
+          borrowers.borrowernumber
+        FROM items
+        LEFT JOIN issues ON items.itemnumber = issues.itemnumber
+        LEFT JOIN biblio ON items.biblionumber = biblio.biblionumber
+        LEFT JOIN borrowers ON issues.borrowernumber = borrowers.borrowernumber
+        WHERE issues.onsite_checkout = 1
+    |, { Slice => {} } );
+}
+
+sub GetTopIssues {
+    my ($params) = @_;
+
+    my ($count, $branch, $itemtype, $ccode, $newness)
+        = @$params{qw(count branch itemtype ccode newness)};
+
+    my $dbh = C4::Context->dbh;
+    my $query = q{
+        SELECT b.biblionumber, b.title, b.author, bi.itemtype, bi.publishercode,
+          bi.place, bi.publicationyear, b.copyrightdate, bi.pages, bi.size,
+          i.ccode, SUM(i.issues) AS count
+        FROM biblio b
+        LEFT JOIN items i ON (i.biblionumber = b.biblionumber)
+        LEFT JOIN biblioitems bi ON (bi.biblionumber = b.biblionumber)
+    };
+
+    my (@where_strs, @where_args);
+
+    if ($branch) {
+        push @where_strs, 'i.homebranch = ?';
+        push @where_args, $branch;
+    }
+    if ($itemtype) {
+        if (C4::Context->preference('item-level_itypes')){
+            push @where_strs, 'i.itype = ?';
+            push @where_args, $itemtype;
+        } else {
+            push @where_strs, 'bi.itemtype = ?';
+            push @where_args, $itemtype;
+        }
+    }
+    if ($ccode) {
+        push @where_strs, 'i.ccode = ?';
+        push @where_args, $ccode;
+    }
+    if ($newness) {
+        push @where_strs, 'TO_DAYS(NOW()) - TO_DAYS(b.datecreated) <= ?';
+        push @where_args, $newness;
+    }
+
+    if (@where_strs) {
+        $query .= 'WHERE ' . join(' AND ', @where_strs);
+    }
+
+    $query .= q{
+        GROUP BY b.biblionumber
+        HAVING count > 0
+        ORDER BY count DESC
+    };
+
+    $count = int($count);
+    if ($count > 0) {
+        $query .= "LIMIT $count";
+    }
+
+    my $rows = $dbh->selectall_arrayref($query, { Slice => {} }, @where_args);
+
+    return @$rows;
+}
+
+1;
 __END__
 
 =head1 AUTHOR