Bug 12001: Move GetMemberAccountBalance to Koha::Account->non_issues_charges
[srvgit] / C4 / Circulation.pm
index dd23734..9bd4e06 100644 (file)
@@ -22,36 +22,43 @@ package C4::Circulation;
 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;
 use C4::Debug;
-use C4::Branch; # GetBranches
 use C4::Log; # logaction
-use C4::Koha qw(
-    GetAuthorisedValueByCode
-    GetAuthValCode
-    GetKohaAuthorisedValueLib
-);
 use C4::Overdues qw(CalcFine UpdateFine get_chargeable_units);
 use C4::RotatingCollections qw(GetCollectionItemBranches);
 use Algorithm::CheckDigits;
 
 use Data::Dumper;
+use Koha::Account;
+use Koha::AuthorisedValues;
+use Koha::Biblioitems;
 use Koha::DateUtils;
 use Koha::Calendar;
-use Koha::Borrower::Debarments;
+use Koha::Checkouts;
+use Koha::IssuingRules;
+use Koha::Items;
+use Koha::Patrons;
+use Koha::Patron::Debarments;
 use Koha::Database;
+use Koha::Libraries;
+use Koha::Holds;
+use Koha::RefundLostItemFeeRule;
+use Koha::RefundLostItemFeeRules;
+use Koha::Account::Lines;
+use Koha::Account::Offsets;
 use Carp;
 use List::MoreUtils qw( uniq );
+use Scalar::Util qw( looks_like_number );
 use Date::Calc qw(
   Today
   Today_and_Now
@@ -61,11 +68,10 @@ use Date::Calc qw(
   Day_of_Week
   Add_Delta_Days
 );
-use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
+use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
 
 BEGIN {
        require Exporter;
-    $VERSION = 3.07.00.049;    # for version checking
        @ISA    = qw(Exporter);
 
        # FIXME subs that should probably be elsewhere
@@ -84,17 +90,15 @@ BEGIN {
                &AddRenewal
                &GetRenewCount
         &GetSoonestRenewDate
-               &GetItemIssue
-               &GetItemIssues
+        &GetLatestAutoRenewDate
                &GetIssuingCharges
-               &GetIssuingRule
         &GetBranchBorrowerCircRule
         &GetBranchItemRule
                &GetBiblioIssues
                &GetOpenIssue
-               &AnonymiseIssueHistory
         &CheckIfIssuedToPatron
         &IsItemIssued
+        GetTopIssues
        );
 
        # subs to deal with returns
@@ -138,7 +142,7 @@ use C4::Circulation;
 
 The functions in this module deal with circulation, issues, and
 returns, as well as general information about the library.
-Also deals with stocktaking.
+Also deals with inventory.
 
 =head1 FUNCTIONS
 
@@ -164,7 +168,7 @@ System Pref options.
 #
 sub barcodedecode {
     my ($barcode, $filter) = @_;
-    my $branch = C4::Branch::mybranch();
+    my $branch = C4::Context::mybranch();
     $filter = C4::Context->preference('itemBarcodeInputFilter') unless $filter;
     $filter or return $barcode;     # ensure filter is defined, else return untouched barcode
        if ($filter eq 'whitespace') {
@@ -274,10 +278,6 @@ is a reference-to-hash which may have any of the following keys:
 
 There is no item in the catalog with the given barcode. The value is C<$barcode>.
 
-=item C<IsPermanent>
-
-The item's home branch is permanent. This doesn't prevent the item from being transferred, though. The value is the code of the item's home branch.
-
 =item C<DestinationEqualsHolding>
 
 The item is already at the branch to which it is being transferred. The transfer is nonetheless considered to have failed. The value should be ignored.
@@ -304,38 +304,32 @@ sub transferbook {
     my ( $tbr, $barcode, $ignoreRs ) = @_;
     my $messages;
     my $dotransfer      = 1;
-    my $branches        = GetBranches();
-    my $itemnumber = GetItemnumberFromBarcode( $barcode );
-    my $issue      = GetItemIssue($itemnumber);
-    my $biblio = GetBiblioFromItemNumber($itemnumber);
+    my $item = Koha::Items->find( { barcode => $barcode } );
 
     # bad barcode..
-    if ( not $itemnumber ) {
+    unless ( $item ) {
         $messages->{'BadBarcode'} = $barcode;
         $dotransfer = 0;
     }
 
+    my $itemnumber = $item->itemnumber;
+    my $issue = GetOpenIssue($itemnumber);
     # get branches of book...
-    my $hbr = $biblio->{'homebranch'};
-    my $fbr = $biblio->{'holdingbranch'};
+    my $hbr = $item->homebranch;
+    my $fbr = $item->holdingbranch;
 
     # if using Branch Transfer Limits
     if ( C4::Context->preference("UseBranchTransferLimits") == 1 ) {
+        my $code = C4::Context->preference("BranchTransferLimitsType") eq 'ccode' ? $item->ccode : $item->biblio->biblioitem->itemtype; # BranchTransferLimitsType is 'ccode' or 'itemtype'
         if ( C4::Context->preference("item-level_itypes") && C4::Context->preference("BranchTransferLimitsType") eq 'itemtype' ) {
-            if ( ! IsBranchTransferAllowed( $tbr, $fbr, $biblio->{'itype'} ) ) {
-                $messages->{'NotAllowed'} = $tbr . "::" . $biblio->{'itype'};
+            if ( ! IsBranchTransferAllowed( $tbr, $fbr, $item->itype ) ) {
+                $messages->{'NotAllowed'} = $tbr . "::" . $item->itype;
                 $dotransfer = 0;
             }
-        } elsif ( ! IsBranchTransferAllowed( $tbr, $fbr, $biblio->{ C4::Context->preference("BranchTransferLimitsType") } ) ) {
-            $messages->{'NotAllowed'} = $tbr . "::" . $biblio->{ C4::Context->preference("BranchTransferLimitsType") };
+        } elsif ( ! IsBranchTransferAllowed( $tbr, $fbr, $code ) ) {
+            $messages->{'NotAllowed'} = $tbr . "::" . $code;
             $dotransfer = 0;
-       }
-    }
-
-    # if is permanent...
-    if ( $hbr && $branches->{$hbr}->{'PE'} ) {
-        $messages->{'IsPermanent'} = $hbr;
-        $dotransfer = 0;
+        }
     }
 
     # can't transfer book if is already there....
@@ -345,9 +339,9 @@ sub transferbook {
     }
 
     # check if it is still issued to someone, return it...
-    if ($issue->{borrowernumber}) {
+    if ( $issue ) {
         AddReturn( $barcode, $fbr );
-        $messages->{'WasReturned'} = $issue->{borrowernumber};
+        $messages->{'WasReturned'} = $issue->borrowernumber;
     }
 
     # find reserves.....
@@ -370,7 +364,7 @@ sub transferbook {
 
     }
     ModDateLastSeen( $itemnumber );
-    return ( $dotransfer, $messages, $biblio );
+    return ( $dotransfer, $messages );
 }
 
 
@@ -378,6 +372,9 @@ sub TooMany {
     my $borrower        = shift;
     my $biblionumber = shift;
        my $item                = shift;
+    my $params = shift;
+    my $onsite_checkout = $params->{onsite_checkout} || 0;
+    my $switch_onsite_checkout = $params->{switch_onsite_checkout} || 0;
     my $cat_borrower    = $borrower->{'categorycode'};
     my $dbh             = C4::Context->dbh;
        my $branch;
@@ -389,17 +386,26 @@ sub TooMany {
  
     # given branch, patron category, and item type, determine
     # applicable issuing rule
-    my $issuing_rule = GetIssuingRule($cat_borrower, $type, $branch);
+    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
+        {   categorycode => $cat_borrower,
+            itemtype     => $type,
+            branchcode   => $branch
+        }
+    );
+
 
     # if a rule is found and has a loan limit set, count
     # how many loans the patron already has that meet that
     # rule
-    if (defined($issuing_rule) and defined($issuing_rule->{'maxissueqty'})) {
+    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};
+        my $rule_itemtype = $issuing_rule->itemtype;
         if ($rule_itemtype eq "*") {
             # matching rule has the default item type, so count only
             # those existing loans that don't fall under a more
@@ -420,8 +426,8 @@ sub TooMany {
                                     AND   itemtype <> '*'
                                   ) ";
             }
-            push @bind_params, $issuing_rule->{branchcode};
-            push @bind_params, $issuing_rule->{categorycode};
+            push @bind_params, $issuing_rule->branchcode;
+            push @bind_params, $issuing_rule->categorycode;
             push @bind_params, $cat_borrower;
         } else {
             # rule has specific item type, so count loans of that
@@ -437,7 +443,7 @@ sub TooMany {
 
         $count_query .= " AND borrowernumber = ? ";
         push @bind_params, $borrower->{'borrowernumber'};
-        my $rule_branch = $issuing_rule->{branchcode};
+        my $rule_branch = $issuing_rule->branchcode;
         if ($rule_branch ne "*") {
             if (C4::Context->preference('CircControl') eq 'PickupLibrary') {
                 $count_query .= " AND issues.branchcode = ? ";
@@ -450,13 +456,37 @@ 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_loans_allowed = $issuing_rule->{'maxissueqty'};
-        if ($current_loan_count >= $max_loans_allowed) {
-            return ($current_loan_count, $max_loans_allowed);
+        my $max_checkouts_allowed = $issuing_rule->maxissueqty;
+        my $max_onsite_checkouts_allowed = $issuing_rule->maxonsiteissueqty;
+
+        if ( $onsite_checkout and defined $max_onsite_checkouts_allowed ) {
+            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') ) {
+            my $delta = $switch_onsite_checkout ? 1 : 0;
+            if ( $checkout_count >= $max_checkouts_allowed + $delta ) {
+                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,
+                };
+            }
         }
     }
 
@@ -464,9 +494,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') {
@@ -478,148 +511,78 @@ 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 $max_loans_allowed = $branch_borrower_circ_rule->{maxissueqty};
-        if ($current_loan_count >= $max_loans_allowed) {
-            return ($current_loan_count, $max_loans_allowed);
-        }
-    }
-
-    # OK, the patron can issue !!!
-    return;
-}
-
-=head2 itemissues
-
-  @issues = &itemissues($biblioitemnumber, $biblio);
-
-Looks up information about who has borrowed the bookZ<>(s) with the
-given biblioitemnumber.
-
-C<$biblio> is ignored.
+        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};
 
-C<&itemissues> returns an array of references-to-hash. The keys
-include the fields from the C<items> table in the Koha database.
-Additional keys include:
-
-=over 4
-
-=item C<date_due>
-
-If the item is currently on loan, this gives the due date.
-
-If the item is not on loan, then this is either "Available" or
-"Cancelled", if the item has been withdrawn.
-
-=item C<card>
-
-If the item is currently on loan, this gives the card number of the
-patron who currently has the item.
-
-=item C<timestamp0>, C<timestamp1>, C<timestamp2>
-
-These give the timestamp for the last three times the item was
-borrowed.
-
-=item C<card0>, C<card1>, C<card2>
-
-The card number of the last three patrons who borrowed this item.
-
-=item C<borrower0>, C<borrower1>, C<borrower2>
-
-The borrower number of the last three patrons who borrowed this item.
-
-=back
-
-=cut
-
-#'
-sub itemissues {
-    my ( $bibitem, $biblio ) = @_;
-    my $dbh = C4::Context->dbh;
-    my $sth =
-      $dbh->prepare("Select * from items where items.biblioitemnumber = ?")
-      || die $dbh->errstr;
-    my $i = 0;
-    my @results;
-
-    $sth->execute($bibitem) || die $sth->errstr;
-
-    while ( my $data = $sth->fetchrow_hashref ) {
-
-        # Find out who currently has this item.
-        # FIXME - Wouldn't it be better to do this as a left join of
-        # some sort? Currently, this code assumes that if
-        # fetchrow_hashref() fails, then the book is on the shelf.
-        # fetchrow_hashref() can fail for any number of reasons (e.g.,
-        # database server crash), not just because no items match the
-        # search criteria.
-        my $sth2 = $dbh->prepare(
-            "SELECT * FROM issues
-                LEFT JOIN borrowers ON issues.borrowernumber = borrowers.borrowernumber
-                WHERE itemnumber = ?
-            "
-        );
-
-        $sth2->execute( $data->{'itemnumber'} );
-        if ( my $data2 = $sth2->fetchrow_hashref ) {
-            $data->{'date_due'} = $data2->{'date_due'};
-            $data->{'card'}     = $data2->{'cardnumber'};
-            $data->{'borrower'} = $data2->{'borrowernumber'};
+        if ( $onsite_checkout and defined $max_onsite_checkouts_allowed ) {
+            if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
+                return {
+                    reason => 'TOO_MANY_ONSITE_CHECKOUTS',
+                    count => $onsite_checkout_count,
+                    max_allowed => $max_onsite_checkouts_allowed,
+                }
+            }
         }
-        else {
-            $data->{'date_due'} = ($data->{'withdrawn'} eq '1') ? 'Cancelled' : 'Available';
+        if ( C4::Context->preference('ConsiderOnSiteCheckoutsAsNormalCheckouts') ) {
+            my $delta = $switch_onsite_checkout ? 1 : 0;
+            if ( $checkout_count >= $max_checkouts_allowed + $delta ) {
+                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,
+                };
+            }
         }
+    }
 
-
-        # Find the last 3 people who borrowed this item.
-        $sth2 = $dbh->prepare(
-            "SELECT * FROM old_issues
-                LEFT JOIN borrowers ON  issues.borrowernumber = borrowers.borrowernumber
-                WHERE itemnumber = ?
-                ORDER BY returndate DESC,timestamp DESC"
-        );
-
-        $sth2->execute( $data->{'itemnumber'} );
-        for ( my $i2 = 0 ; $i2 < 2 ; $i2++ )
-        {    # FIXME : error if there is less than 3 pple borrowing this item
-            if ( my $data2 = $sth2->fetchrow_hashref ) {
-                $data->{"timestamp$i2"} = $data2->{'timestamp'};
-                $data->{"card$i2"}      = $data2->{'cardnumber'};
-                $data->{"borrower$i2"}  = $data2->{'borrowernumber'};
-            }    # if
-        }    # for
-
-        $results[$i] = $data;
-        $i++;
+    if ( not defined( $issuing_rule ) and not defined($branch_borrower_circ_rule->{maxissueqty}) ) {
+        return { reason => 'NO_RULE_DEFINED', max_allowed => 0 };
     }
 
-    return (@results);
+    # OK, the patron can issue !!!
+    return;
 }
 
 =head2 CanBookBeIssued
 
-  ( $issuingimpossible, $needsconfirmation ) =  CanBookBeIssued( $borrower, 
-                      $barcode, $duedatespec, $inprocess, $ignore_reserves );
+  ( $issuingimpossible, $needsconfirmation, [ $alerts ] ) =  CanBookBeIssued( $patron,
+                      $barcode, $duedate, $inprocess, $ignore_reserves, $params );
 
 Check if a book can be issued.
 
-C<$issuingimpossible> and C<$needsconfirmation> are some hashref.
+C<$issuingimpossible> and C<$needsconfirmation> are hashrefs.
+
+IMPORTANT: The assumption by users of this routine is that causes blocking
+the issue are keyed by uppercase labels and other returned
+data is keyed in lower case!
 
 =over 4
 
-=item C<$borrower> hash with borrower informations (from GetMember or GetMemberDetails)
+=item C<$patron> is a Koha::Patron
 
 =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
 
+=item C<$params> Hashref of additional parameters
+
+Available keys:
+    override_high_holds - Ignore high holds
+    onsite_checkout     - Checkout is an onsite checkout that will not leave the library
+
 =back
 
 Returns :
@@ -695,16 +658,21 @@ if the borrower borrows to much things
 =cut
 
 sub CanBookBeIssued {
-    my ( $borrower, $barcode, $duedate, $inprocess, $ignore_reserves ) = @_;
+    my ( $patron, $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 %messages;             # filled with information messages that should be displayed.
+
+    my $onsite_checkout     = $params->{onsite_checkout}     || 0;
+    my $override_high_holds = $params->{override_high_holds} || 0;
 
-    my $item = GetItem(GetItemnumberFromBarcode( $barcode ));
-    my $issue = GetItemIssue($item->{itemnumber});
+    my $item = GetItem(undef, $barcode );
+    my $issue = Koha::Checkouts->find( { itemnumber => $item->{itemnumber} } );
        my $biblioitem = GetBiblioItemData($item->{biblioitemnumber});
        $item->{'itemtype'}=$item->{'itype'}; 
     my $dbh             = C4::Context->dbh;
+    my $patron_unblessed = $patron->unblessed;
 
     # MANDATORY CHECKS - unless item exists, nothing else matters
     unless ( $item->{barcode} ) {
@@ -722,9 +690,9 @@ sub CanBookBeIssued {
     unless ( $duedate ) {
         my $issuedate = $now->clone();
 
-        my $branch = _GetCircControlBranch($item,$borrower);
+        my $branch = _GetCircControlBranch($item, $patron_unblessed);
         my $itype = ( C4::Context->preference('item-level_itypes') ) ? $item->{'itype'} : $biblioitem->{'itemtype'};
-        $duedate = CalcDateDue( $issuedate, $itype, $branch, $borrower );
+        $duedate = CalcDateDue( $issuedate, $itype, $branch, $patron_unblessed );
 
         # Offline circ calls AddIssue directly, doesn't run through here
         #  So issuingimpossible should be ok.
@@ -742,120 +710,195 @@ sub CanBookBeIssued {
     #
     # BORROWER STATUS
     #
-    if ( $borrower->{'category_type'} eq 'X' && (  $item->{barcode}  )) { 
+    if ( $patron->category->category_type eq 'X' && (  $item->{barcode}  )) {
        # stats only borrower -- add entry to statistics table, and return issuingimpossible{STATS} = 1  .
         &UpdateStats({
                      branch => C4::Context->userenv->{'branch'},
                      type => 'localuse',
                      itemnumber => $item->{'itemnumber'},
-                     itemtype => $item->{'itemtype'},
-                     borrowernumber => $borrower->{'borrowernumber'},
+                     itemtype => $item->{'itype'},
+                     borrowernumber => $patron->borrowernumber,
                      ccode => $item->{'ccode'}}
                     );
         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;
+
+    my $flags = C4::Members::patronflags( $patron_unblessed );
+    if ( ref $flags ) {
+        if ( $flags->{GNA} ) {
+            $issuingimpossible{GNA} = 1;
+        }
+        if ( $flags->{'LOST'} ) {
+            $issuingimpossible{CARD_LOST} = 1;
+        }
+        if ( $flags->{'DBARRED'} ) {
+            $issuingimpossible{DEBARRED} = 1;
+        }
     }
-    if ( !defined $borrower->{dateexpiry} || $borrower->{'dateexpiry'} eq '0000-00-00') {
+
+    if ( $patron->is_expired ) {
         $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");
-            $issuingimpossible{EXPIRED} = 1;
-        }
     }
+
     #
     # BORROWER STATUS
     #
 
     # DEBTS
-    my ($balance, $non_issue_charges, $other_charges) =
-      C4::Members::GetMemberAccountBalance( $borrower->{'borrowernumber'} );
+    my $account = $patron->account;
+    my $balance = $account->balance;
+    my $non_issues_charges = $account->non_issues_charges;
+    my $other_charges = $balance - $non_issues_charges;
+
     my $amountlimit = C4::Context->preference("noissuescharge");
     my $allowfineoverride = C4::Context->preference("AllowFineOverride");
     my $allfinesneedoverride = C4::Context->preference("AllFinesNeedOverride");
+
+    # Check the debt of this patrons guarantees
+    my $no_issues_charge_guarantees = C4::Context->preference("NoIssuesChargeGuarantees");
+    $no_issues_charge_guarantees = undef unless looks_like_number( $no_issues_charge_guarantees );
+    if ( defined $no_issues_charge_guarantees ) {
+        my @guarantees = $patron->guarantees();
+        my $guarantees_non_issues_charges;
+        foreach my $g ( @guarantees ) {
+            $guarantees_non_issues_charges += $g->account->non_issues_charges;
+        }
+
+        if ( $guarantees_non_issues_charges > $no_issues_charge_guarantees && !$inprocess && !$allowfineoverride) {
+            $issuingimpossible{DEBT_GUARANTEES} = $guarantees_non_issues_charges;
+        } elsif ( $guarantees_non_issues_charges > $no_issues_charge_guarantees && !$inprocess && $allowfineoverride) {
+            $needsconfirmation{DEBT_GUARANTEES} = $guarantees_non_issues_charges;
+        } elsif ( $allfinesneedoverride && $guarantees_non_issues_charges > 0 && $guarantees_non_issues_charges <= $no_issues_charge_guarantees && !$inprocess ) {
+            $needsconfirmation{DEBT_GUARANTEES} = $guarantees_non_issues_charges;
+        }
+    }
+
     if ( C4::Context->preference("IssuingInProcess") ) {
-        if ( $non_issue_charges > $amountlimit && !$inprocess && !$allowfineoverride) {
-            $issuingimpossible{DEBT} = sprintf( "%.2f", $non_issue_charges );
-        } elsif ( $non_issue_charges > $amountlimit && !$inprocess && $allowfineoverride) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_charges );
-        } elsif ( $allfinesneedoverride && $non_issue_charges > 0 && $non_issue_charges <= $amountlimit && !$inprocess ) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_charges );
+        if ( $non_issues_charges > $amountlimit && !$inprocess && !$allowfineoverride) {
+            $issuingimpossible{DEBT} = sprintf( "%.2f", $non_issues_charges );
+        } elsif ( $non_issues_charges > $amountlimit && !$inprocess && $allowfineoverride) {
+            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issues_charges );
+        } elsif ( $allfinesneedoverride && $non_issues_charges > 0 && $non_issues_charges <= $amountlimit && !$inprocess ) {
+            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issues_charges );
         }
     }
     else {
-        if ( $non_issue_charges > $amountlimit && $allowfineoverride ) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_charges );
-        } elsif ( $non_issue_charges > $amountlimit && !$allowfineoverride) {
-            $issuingimpossible{DEBT} = sprintf( "%.2f", $non_issue_charges );
-        } elsif ( $non_issue_charges > 0 && $allfinesneedoverride ) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_charges );
+        if ( $non_issues_charges > $amountlimit && $allowfineoverride ) {
+            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issues_charges );
+        } elsif ( $non_issues_charges > $amountlimit && !$allowfineoverride) {
+            $issuingimpossible{DEBT} = sprintf( "%.2f", $non_issues_charges );
+        } elsif ( $non_issues_charges > 0 && $allfinesneedoverride ) {
+            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issues_charges );
         }
     }
+
     if ($balance > 0 && $other_charges > 0) {
         $alerts{OTHER_CHARGES} = sprintf( "%.2f", $other_charges );
     }
 
-    my ($blocktype, $count) = C4::Members::IsMemberBlocked($borrower->{'borrowernumber'});
-    if ($blocktype == -1) {
-        ## patron has outstanding overdue loans
-           if ( C4::Context->preference("OverduesBlockCirc") eq 'block'){
-               $issuingimpossible{USERBLOCKEDOVERDUE} = $count;
-           }
-           elsif ( C4::Context->preference("OverduesBlockCirc") eq 'confirmation'){
-               $needsconfirmation{USERBLOCKEDOVERDUE} = $count;
-           }
-    } elsif($blocktype == 1) {
-        # patron has accrued fine days or has a restriction. $count is a date
-        if ($count eq '9999-12-31') {
-            $issuingimpossible{USERBLOCKEDNOENDDATE} = $count;
+    $patron = Koha::Patrons->find( $patron->borrowernumber ); # FIXME Refetch just in case, to avoid regressions. But must not be needed
+    $patron_unblessed = $patron->unblessed;
+
+    if ( my $debarred_date = $patron->is_debarred ) {
+         # patron has accrued fine days or has a restriction. $count is a date
+        if ($debarred_date eq '9999-12-31') {
+            $issuingimpossible{USERBLOCKEDNOENDDATE} = $debarred_date;
         }
         else {
-            $issuingimpossible{USERBLOCKEDWITHENDDATE} = $count;
+            $issuingimpossible{USERBLOCKEDWITHENDDATE} = $debarred_date;
+        }
+    } elsif ( my $num_overdues = $patron->has_overdues ) {
+        ## patron has outstanding overdue loans
+        if ( C4::Context->preference("OverduesBlockCirc") eq 'block'){
+            $issuingimpossible{USERBLOCKEDOVERDUE} = $num_overdues;
+        }
+        elsif ( C4::Context->preference("OverduesBlockCirc") eq 'confirmation'){
+            $needsconfirmation{USERBLOCKEDOVERDUE} = $num_overdues;
         }
     }
 
-#
-    # JB34 CHECKS IF BORROWERS DONT 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;
+    # CHECK IF BOOK ALREADY ISSUED TO THIS BORROWER
+    #
+    if ( $issue && $issue->borrowernumber eq $patron->borrowernumber ){
+
+        # Already issued to current borrower.
+        # If it is an on-site checkout if it can be switched to a normal checkout
+        # or ask whether the loan should be renewed
+
+        if ( $issue->onsite_checkout
+                and C4::Context->preference('SwitchOnSiteCheckouts') ) {
+            $messages{ONSITE_CHECKOUT_WILL_BE_SWITCHED} = 1;
+        } else {
+            my ($CanBookBeRenewed,$renewerror) = CanBookBeRenewed(
+                $patron->borrowernumber,
+                $item->{'itemnumber'},
+            );
+            if ( $CanBookBeRenewed == 0 ) {    # no more renewals allowed
+                if ( $renewerror eq 'onsite_checkout' ) {
+                    $issuingimpossible{NO_RENEWAL_FOR_ONSITE_CHECKOUTS} = 1;
+                }
+                else {
+                    $issuingimpossible{NO_MORE_RENEWALS} = 1;
+                }
+            }
+            else {
+                $needsconfirmation{RENEW_ISSUE} = 1;
             }
         }
     }
+    elsif ( $issue ) {
+
+        # issued to someone else
+
+        my $patron = Koha::Patrons->find( $issue->borrowernumber );
+
+        my ( $can_be_returned, $message ) = CanBookBeReturned( $item, C4::Context->userenv->{branch} );
+
+        unless ( $can_be_returned ) {
+            $issuingimpossible{RETURN_IMPOSSIBLE} = 1;
+            $issuingimpossible{branch_to_return} = $message;
+        } else {
+            $needsconfirmation{ISSUED_TO_ANOTHER} = 1;
+            $needsconfirmation{issued_firstname} = $patron->firstname;
+            $needsconfirmation{issued_surname} = $patron->surname;
+            $needsconfirmation{issued_cardnumber} = $patron->cardnumber;
+            $needsconfirmation{issued_borrowernumber} = $patron->borrowernumber;
+        }
+    }
+
+    # JB34 CHECKS IF BORROWERS DON'T HAVE ISSUE TOO MANY BOOKS
+    #
+    my $switch_onsite_checkout = (
+          C4::Context->preference('SwitchOnSiteCheckouts')
+      and $issue
+      and $issue->onsite_checkout
+      and $issue->borrowernumber == $patron->borrowernumber ? 1 : 0 );
+    my $toomany = TooMany( $patron_unblessed, $item->{biblionumber}, $item, { onsite_checkout => $onsite_checkout, switch_onsite_checkout => $switch_onsite_checkout, } );
+    # if TooMany max_allowed returns 0 the user doesn't have permission to check out this book
+    if ( $toomany && not exists $needsconfirmation{RENEW_ISSUE} ) {
+        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 {
+            $issuingimpossible{TOO_MANY} = $toomany->{reason};
+            $issuingimpossible{current_loan_count} = $toomany->{count};
+            $issuingimpossible{max_loans_allowed} = $toomany->{max_allowed};
+        }
+    }
+
+    #
+    # CHECKPREVCHECKOUT: CHECK IF ITEM HAS EVER BEEN LENT TO PATRON
+    #
+    $patron = Koha::Patrons->find( $patron->borrowernumber ); # FIXME Refetch just in case, to avoid regressions. But must not be needed
+    my $wants_check = $patron->wants_check_for_previous_checkout;
+    $needsconfirmation{PREVISSUE} = 1
+        if ($wants_check and $patron->do_check_for_previous_checkout($item));
 
     #
     # ITEM CHECKING
@@ -907,7 +950,8 @@ sub CanBookBeIssued {
         $issuingimpossible{RESTRICTED} = 1;
     }
     if ( $item->{'itemlost'} && C4::Context->preference("IssueLostItem") ne 'nothing' ) {
-        my $code = GetAuthorisedValueByCode( 'LOST', $item->{'itemlost'} );
+        my $av = Koha::AuthorisedValues->search({ category => 'LOST', authorised_value => $item->{itemlost} });
+        my $code = $av->count ? $av->next->lib : '';
         $needsconfirmation{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'confirm' );
         $alerts{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'alert' );
     }
@@ -918,8 +962,8 @@ sub CanBookBeIssued {
                 $issuingimpossible{ITEMNOTSAMEBRANCH} = 1;
                 $issuingimpossible{'itemhomebranch'} = $item->{C4::Context->preference("HomeOrHoldingBranch")};
             }
-            $needsconfirmation{BORRNOTSAMEBRANCH} = GetBranchName( $borrower->{'branchcode'} )
-              if ( $borrower->{'branchcode'} ne $userenv->{branch} );
+            $needsconfirmation{BORRNOTSAMEBRANCH} = $patron->branchcode
+              if ( $patron->branchcode ne $userenv->{branch} );
         }
     }
     #
@@ -928,73 +972,40 @@ sub CanBookBeIssued {
     my $rentalConfirmation = C4::Context->preference("RentalFeesCheckoutConfirmation");
 
     if ( $rentalConfirmation ){
-        my ($rentalCharge) = GetIssuingCharges( $item->{'itemnumber'}, $borrower->{'borrowernumber'} );
+        my ($rentalCharge) = GetIssuingCharges( $item->{'itemnumber'}, $patron->borrowernumber );
         if ( $rentalCharge > 0 ){
-            $rentalCharge = sprintf("%.02f", $rentalCharge);
             $needsconfirmation{RENTALCHARGE} = $rentalCharge;
         }
     }
 
-    #
-    # CHECK IF BOOK ALREADY ISSUED TO THIS BORROWER
-    #
-    if ( $issue->{borrowernumber} && $issue->{borrowernumber} eq $borrower->{'borrowernumber'} ){
-
-        # Already issued to current borrower. Ask whether the loan should
-        # be renewed.
-        my ($CanBookBeRenewed,$renewerror) = CanBookBeRenewed(
-            $borrower->{'borrowernumber'},
-            $item->{'itemnumber'}
-        );
-        if ( $CanBookBeRenewed == 0 ) {    # no more renewals allowed
-            $issuingimpossible{NO_MORE_RENEWALS} = 1;
-        }
-        else {
-            $needsconfirmation{RENEW_ISSUE} = 1;
-        }
-    }
-    elsif ($issue->{borrowernumber}) {
-
-        # issued to someone else
-        my $currborinfo =    C4::Members::GetMember( borrowernumber => $issue->{borrowernumber} );
-
-#        warn "=>.$currborinfo->{'firstname'} $currborinfo->{'surname'} ($currborinfo->{'cardnumber'})";
-        $needsconfirmation{ISSUED_TO_ANOTHER} = 1;
-        $needsconfirmation{issued_firstname} = $currborinfo->{'firstname'};
-        $needsconfirmation{issued_surname} = $currborinfo->{'surname'};
-        $needsconfirmation{issued_cardnumber} = $currborinfo->{'cardnumber'};
-        $needsconfirmation{issued_borrowernumber} = $currborinfo->{'borrowernumber'};
-    }
-
     unless ( $ignore_reserves ) {
         # See if the item is on reserve.
         my ( $restype, $res ) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
         if ($restype) {
             my $resbor = $res->{'borrowernumber'};
-            if ( $resbor ne $borrower->{'borrowernumber'} ) {
-                my ( $resborrower ) = C4::Members::GetMember( borrowernumber => $resbor );
-                my $branchname = GetBranchName( $res->{'branchcode'} );
+            if ( $resbor ne $patron->borrowernumber ) {
+                my $patron = Koha::Patrons->find( $resbor );
                 if ( $restype eq "Waiting" )
                 {
                     # The item is on reserve and waiting, but has been
                     # reserved by some other patron.
                     $needsconfirmation{RESERVE_WAITING} = 1;
-                    $needsconfirmation{'resfirstname'} = $resborrower->{'firstname'};
-                    $needsconfirmation{'ressurname'} = $resborrower->{'surname'};
-                    $needsconfirmation{'rescardnumber'} = $resborrower->{'cardnumber'};
-                    $needsconfirmation{'resborrowernumber'} = $resborrower->{'borrowernumber'};
-                    $needsconfirmation{'resbranchname'} = $branchname;
-                    $needsconfirmation{'reswaitingdate'} = format_date($res->{'waitingdate'});
+                    $needsconfirmation{'resfirstname'} = $patron->firstname;
+                    $needsconfirmation{'ressurname'} = $patron->surname;
+                    $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
+                    $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
+                    $needsconfirmation{'resbranchcode'} = $res->{branchcode};
+                    $needsconfirmation{'reswaitingdate'} = $res->{'waitingdate'};
                 }
                 elsif ( $restype eq "Reserved" ) {
                     # The item is on reserve for someone else.
                     $needsconfirmation{RESERVED} = 1;
-                    $needsconfirmation{'resfirstname'} = $resborrower->{'firstname'};
-                    $needsconfirmation{'ressurname'} = $resborrower->{'surname'};
-                    $needsconfirmation{'rescardnumber'} = $resborrower->{'cardnumber'};
-                    $needsconfirmation{'resborrowernumber'} = $resborrower->{'borrowernumber'};
-                    $needsconfirmation{'resbranchname'} = $branchname;
-                    $needsconfirmation{'resreservedate'} = format_date($res->{'reservedate'});
+                    $needsconfirmation{'resfirstname'} = $patron->firstname;
+                    $needsconfirmation{'ressurname'} = $patron->surname;
+                    $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
+                    $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
+                    $needsconfirmation{'resbranchcode'} = $patron->branchcode;
+                    $needsconfirmation{'resreservedate'} = $res->{reservedate};
                 }
             }
         }
@@ -1002,7 +1013,7 @@ sub CanBookBeIssued {
 
     ## CHECK AGE RESTRICTION
     my $agerestriction  = $biblioitem->{'agerestriction'};
-    my ($restriction_age, $daysToAgeRestriction) = GetAgeRestriction( $agerestriction, $borrower );
+    my ($restriction_age, $daysToAgeRestriction) = GetAgeRestriction( $agerestriction, $patron->unblessed );
     if ( $daysToAgeRestriction && $daysToAgeRestriction > 0 ) {
         if ( C4::Context->preference('AgeRestrictionOverride') ) {
             $needsconfirmation{AGE_RESTRICTION} = "$agerestriction";
@@ -1013,17 +1024,24 @@ sub CanBookBeIssued {
     }
 
     ## check for high holds decreasing loan period
-    my $decrease_loan = C4::Context->preference('decreaseLoanHighHolds');
-    if ( $decrease_loan && $decrease_loan == 1 ) {
-        my ( $reserved, $num, $duration, $returndate ) =
-          checkHighHolds( $item, $borrower );
-
-        if ( $num >= C4::Context->preference('decreaseLoanHighHoldsValue') ) {
-            $needsconfirmation{HIGHHOLDS} = {
-                num_holds  => $num,
-                duration   => $duration,
-                returndate => output_pref($returndate),
-            };
+    if ( C4::Context->preference('decreaseLoanHighHolds') ) {
+        my $check = checkHighHolds( $item, $patron_unblessed );
+
+        if ( $check->{exceeded} ) {
+            if ($override_high_holds) {
+                $alerts{HIGHHOLDS} = {
+                    num_holds  => $check->{outstanding},
+                    duration   => $check->{duration},
+                    returndate => output_pref( $check->{due_date} ),
+                };
+            }
+            else {
+                $needsconfirmation{HIGHHOLDS} = {
+                    num_holds  => $check->{outstanding},
+                    duration   => $check->{duration},
+                    returndate => output_pref( $check->{due_date} ),
+                };
+            }
         }
     }
 
@@ -1040,20 +1058,25 @@ sub CanBookBeIssued {
         require C4::Serials;
         my $is_a_subscription = C4::Serials::CountSubscriptionFromBiblionumber($biblionumber);
         unless ($is_a_subscription) {
-            my $issues = GetIssues( {
-                borrowernumber => $borrower->{borrowernumber},
-                biblionumber   => $biblionumber,
-            } );
-            my @issues = $issues ? @$issues : ();
+            # FIXME Should be $patron->checkouts($args);
+            my $checkouts = Koha::Checkouts->search(
+                {
+                    borrowernumber => $patron->borrowernumber,
+                    biblionumber   => $biblionumber,
+                },
+                {
+                    join => 'item',
+                }
+            );
             # if we get here, we don't already have a loan on this item,
             # so if there are any loans on this bib, ask for confirmation
-            if (scalar @issues > 0) {
+            if ( $checkouts->count ) {
                 $needsconfirmation{BIBLIO_ALREADY_ISSUED} = 1;
             }
         }
     }
 
-    return ( \%issuingimpossible, \%needsconfirmation, \%alerts );
+    return ( \%issuingimpossible, \%needsconfirmation, \%alerts, \%messages, );
 }
 
 =head2 CanBookBeReturned
@@ -1115,38 +1138,84 @@ sub CanBookBeReturned {
 
 sub checkHighHolds {
     my ( $item, $borrower ) = @_;
-    my $biblio = GetBiblioFromItemNumber( $item->{itemnumber} );
     my $branch = _GetCircControlBranch( $item, $borrower );
-    my $dbh    = C4::Context->dbh;
-    my $sth    = $dbh->prepare(
-'select count(borrowernumber) as num_holds from reserves where biblionumber=?'
-    );
-    $sth->execute( $item->{'biblionumber'} );
-    my ($holds) = $sth->fetchrow_array;
-    if ($holds) {
+    my $item_object = Koha::Items->find( $item->{itemnumber} );
+
+    my $return_data = {
+        exceeded    => 0,
+        outstanding => 0,
+        duration    => 0,
+        due_date    => undef,
+    };
+
+    my $holds = Koha::Holds->search( { biblionumber => $item->{'biblionumber'} } );
+
+    if ( $holds->count() ) {
+        $return_data->{outstanding} = $holds->count();
+
+        my $decreaseLoanHighHoldsControl        = C4::Context->preference('decreaseLoanHighHoldsControl');
+        my $decreaseLoanHighHoldsValue          = C4::Context->preference('decreaseLoanHighHoldsValue');
+        my $decreaseLoanHighHoldsIgnoreStatuses = C4::Context->preference('decreaseLoanHighHoldsIgnoreStatuses');
+
+        my @decreaseLoanHighHoldsIgnoreStatuses = split( /,/, $decreaseLoanHighHoldsIgnoreStatuses );
+
+        if ( $decreaseLoanHighHoldsControl eq 'static' ) {
+
+            # static means just more than a given number of holds on the record
+
+            # If the number of holds is less than the threshold, we can stop here
+            if ( $holds->count() < $decreaseLoanHighHoldsValue ) {
+                return $return_data;
+            }
+        }
+        elsif ( $decreaseLoanHighHoldsControl eq 'dynamic' ) {
+
+            # dynamic means X more than the number of holdable items on the record
+
+            # let's get the items
+            my @items = $holds->next()->biblio()->items();
+
+            # Remove any items with status defined to be ignored even if the would not make item unholdable
+            foreach my $status (@decreaseLoanHighHoldsIgnoreStatuses) {
+                @items = grep { !$_->$status } @items;
+            }
+
+            # Remove any items that are not holdable for this patron
+            @items = grep { CanItemBeReserved( $borrower->{borrowernumber}, $_->itemnumber ) eq 'OK' } @items;
+
+            my $items_count = scalar @items;
+
+            my $threshold = $items_count + $decreaseLoanHighHoldsValue;
+
+            # If the number of holds is less than the count of items we have
+            # plus the number of holds allowed above that count, we can stop here
+            if ( $holds->count() <= $threshold ) {
+                return $return_data;
+            }
+        }
+
         my $issuedate = DateTime->now( time_zone => C4::Context->tz() );
 
         my $calendar = Koha::Calendar->new( branchcode => $branch );
 
-        my $itype =
-          ( C4::Context->preference('item-level_itypes') )
-          ? $biblio->{'itype'}
-          : $biblio->{'itemtype'};
-        my $orig_due =
-          C4::Circulation::CalcDateDue( $issuedate, $itype, $branch,
-            $borrower );
+        my $itype = $item_object->effective_itemtype;
+        my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branch, $borrower );
+
+        my $decreaseLoanHighHoldsDuration = C4::Context->preference('decreaseLoanHighHoldsDuration');
 
-        my $reduced_datedue =
-          $calendar->addDate( $issuedate,
-            C4::Context->preference('decreaseLoanHighHoldsDuration') );
+        my $reduced_datedue = $calendar->addDate( $issuedate, $decreaseLoanHighHoldsDuration );
+        $reduced_datedue->set_hour($orig_due->hour);
+        $reduced_datedue->set_minute($orig_due->minute);
+        $reduced_datedue->truncate( to => 'minute' );
 
         if ( DateTime->compare( $reduced_datedue, $orig_due ) == -1 ) {
-            return ( 1, $holds,
-                C4::Context->preference('decreaseLoanHighHoldsDuration'),
-                $reduced_datedue );
+            $return_data->{exceeded} = 1;
+            $return_data->{duration} = $decreaseLoanHighHoldsDuration;
+            $return_data->{due_date} = $reduced_datedue;
         }
     }
-    return ( 0, 0, 0, undef );
+
+    return $return_data;
 }
 
 =head2 AddIssue
@@ -1157,17 +1226,17 @@ Issue a book. Does no check, they are done in CanBookBeIssued. If we reach this
 
 =over 4
 
-=item C<$borrower> is a hash with borrower informations (from GetMember or GetMemberDetails).
+=item C<$borrower> is a hash with borrower informations (from Koha::Patron->unblessed).
 
 =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 :
 
@@ -1189,172 +1258,199 @@ AddIssue does the following things :
 
 sub AddIssue {
     my ( $borrower, $barcode, $datedue, $cancelreserve, $issuedate, $sipmode, $params ) = @_;
+
     my $onsite_checkout = $params && $params->{onsite_checkout} ? 1 : 0;
+    my $switch_onsite_checkout = $params && $params->{switch_onsite_checkout};
     my $auto_renew = $params && $params->{auto_renew};
-    my $dbh = C4::Context->dbh;
-    my $barcodecheck=CheckValidBarcode($barcode);
+    my $dbh          = C4::Context->dbh;
+    my $barcodecheck = CheckValidBarcode($barcode);
 
     my $issue;
 
-    if ($datedue && ref $datedue ne 'DateTime') {
+    if ( $datedue && ref $datedue ne 'DateTime' ) {
         $datedue = dt_from_string($datedue);
     }
+
     # $issuedate defaults to today.
-    if ( ! defined $issuedate ) {
-        $issuedate = DateTime->now(time_zone => C4::Context->tz());
+    if ( !defined $issuedate ) {
+        $issuedate = DateTime->now( time_zone => C4::Context->tz() );
     }
     else {
-        if ( ref $issuedate ne 'DateTime') {
+        if ( ref $issuedate ne 'DateTime' ) {
             $issuedate = dt_from_string($issuedate);
 
         }
     }
-       if ($borrower and $barcode and $barcodecheck ne '0'){#??? wtf
-               # find which item we issue
-               my $item = GetItem('', $barcode) or return;     # if we don't get an Item, abort.
-               my $branch = _GetCircControlBranch($item,$borrower);
-               
-               # get actual issuing if there is one
-               my $actualissue = GetItemIssue( $item->{itemnumber});
-               
-               # get biblioinformation for this item
-               my $biblio = GetBiblioFromItemNumber($item->{itemnumber});
-               
-               #
-               # check if we just renew the issue.
-               #
-               if ($actualissue->{borrowernumber} eq $borrower->{'borrowernumber'}) {
-                   $datedue = AddRenewal(
-                       $borrower->{'borrowernumber'},
-                       $item->{'itemnumber'},
-                       $branch,
-                       $datedue,
-                       $issuedate, # here interpreted as the renewal date
-                       );
-               }
-               else {
-        # it's NOT a renewal
-                       if ( $actualissue->{borrowernumber}) {
-                               # This book is currently on loan, but not to the person
-                               # who wants to borrow it now. mark it returned before issuing to the new borrower
-                               AddReturn(
-                                       $item->{'barcode'},
-                                       C4::Context->userenv->{'branch'}
-                               );
-                       }
+
+    # Stop here if the patron or barcode doesn't exist
+    if ( $borrower && $barcode && $barcodecheck ) {
+        # find which item we issue
+        my $item = GetItem( '', $barcode )
+          or return;    # if we don't get an Item, abort.
+        my $item_object = Koha::Items->find( { barcode => $barcode } );
+
+        my $branch = _GetCircControlBranch( $item, $borrower );
+
+        # get actual issuing if there is one
+        my $actualissue = Koha::Checkouts->find( { itemnumber => $item->{itemnumber} } );
+
+        # check if we just renew the issue.
+        if ( $actualissue and $actualissue->borrowernumber eq $borrower->{'borrowernumber'}
+                and not $switch_onsite_checkout ) {
+            $datedue = AddRenewal(
+                $borrower->{'borrowernumber'},
+                $item->{'itemnumber'},
+                $branch,
+                $datedue,
+                $issuedate,    # here interpreted as the renewal date
+            );
+        }
+        else {
+            # it's NOT a renewal
+            if ( $actualissue and not $switch_onsite_checkout ) {
+                # This book is currently on loan, but not to the person
+                # who wants to borrow it now. mark it returned before issuing to the new borrower
+                my ( $allowed, $message ) = CanBookBeReturned( $item, C4::Context->userenv->{branch} );
+                return unless $allowed;
+                AddReturn( $item->{'barcode'}, C4::Context->userenv->{'branch'} );
+            }
 
             MoveReserve( $item->{'itemnumber'}, $borrower->{'borrowernumber'}, $cancelreserve );
-                       # Starting process for transfer job (checking transfert and validate it if we have one)
-            my ($datesent) = GetTransfers($item->{'itemnumber'});
+
+            # 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 visibility of this case (maybe for stats ....)
-                my $sth =
-                    $dbh->prepare(
+                # 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(),
                         tobranch = ?,
                         comments = 'Forced branchtransfer'
                     WHERE itemnumber= ? AND datearrived IS NULL"
-                    );
-                $sth->execute(C4::Context->userenv->{'branch'},$item->{'itemnumber'});
+                );
+                $sth->execute( C4::Context->userenv->{'branch'},
+                    $item->{'itemnumber'} );
             }
 
-        # If automatic renewal wasn't selected while issuing, set the value according to the issuing rule.
-        unless ($auto_renew) {
-            my $issuingrule = GetIssuingRule($borrower->{categorycode}, $item->{itype}, $branch);
-            $auto_renew = $issuingrule->{auto_renew};
-        }
+            # If automatic renewal wasn't selected while issuing, set the value according to the issuing rule.
+            unless ($auto_renew) {
+                my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
+                    {   categorycode => $borrower->{categorycode},
+                        itemtype     => $item->{itype},
+                        branchcode   => $branch
+                    }
+                );
 
-        # Record in the database the fact that the book was issued.
-        unless ($datedue) {
-            my $itype = ( C4::Context->preference('item-level_itypes') ) ? $biblio->{'itype'} : $biblio->{'itemtype'};
-            $datedue = CalcDateDue( $issuedate, $itype, $branch, $borrower );
+                $auto_renew = $issuing_rule->auto_renew if $issuing_rule;
+            }
 
-        }
-        $datedue->truncate( to => 'minute');
+            # Record in the database the fact that the book was issued.
+            unless ($datedue) {
+                my $itype = $item_object->effective_itemtype;
+                $datedue = CalcDateDue( $issuedate, $itype, $branch, $borrower );
 
-        $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
             }
-        );
+            $datedue->truncate( to => 'minute' );
 
-        if ( C4::Context->preference('ReturnToShelvingCart') ) { ## ReturnToShelvingCart is on, anything issued should be taken off the cart.
-          CartToShelf( $item->{'itemnumber'} );
-        }
-        $item->{'issues'}++;
-        if ( C4::Context->preference('UpdateTotalIssuesOnCirc') ) {
-            UpdateTotalIssues($item->{'biblionumber'}, 1);
-        }
+            $issue = Koha::Database->new()->schema()->resultset('Issue')->update_or_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 item was lost, it has now been found, reverse any list item charges if neccessary.
-        if ( $item->{'itemlost'} ) {
-            if ( C4::Context->preference('RefundLostItemFeeOnReturn' ) ) {
-                _FixAccountForLostAndReturned( $item->{'itemnumber'}, undef, $item->{'barcode'} );
+            if ( C4::Context->preference('ReturnToShelvingCart') ) {
+                # ReturnToShelvingCart is on, anything issued should be taken off the cart.
+                CartToShelf( $item->{'itemnumber'} );
+            }
+            $item->{'issues'}++;
+            if ( C4::Context->preference('UpdateTotalIssuesOnCirc') ) {
+                UpdateTotalIssues( $item->{'biblionumber'}, 1 );
             }
-        }
 
-        ModItem({ issues           => $item->{'issues'},
-                  holdingbranch    => C4::Context->userenv->{'branch'},
-                  itemlost         => 0,
-                  datelastborrowed => DateTime->now(time_zone => C4::Context->tz())->ymd(),
-                  onloan           => $datedue->ymd(),
-                }, $item->{'biblionumber'}, $item->{'itemnumber'});
-        ModDateLastSeen( $item->{'itemnumber'} );
+            ## If item was lost, it has now been found, reverse any list item charges if necessary.
+            if ( $item->{'itemlost'} ) {
+                if (
+                    Koha::RefundLostItemFeeRules->should_refund(
+                        {
+                            current_branch      => C4::Context->userenv->{branch},
+                            item_home_branch    => $item->{homebranch},
+                            item_holding_branch => $item->{holdingbranch}
+                        }
+                    )
+                  )
+                {
+                    _FixAccountForLostAndReturned( $item->{'itemnumber'}, undef,
+                        $item->{'barcode'} );
+                }
+            }
 
-        # If it costs to borrow this book, charge it to the patron's account.
-        my ( $charge, $itemtype ) = GetIssuingCharges(
-            $item->{'itemnumber'},
-            $borrower->{'borrowernumber'}
-        );
-        if ( $charge > 0 ) {
-            AddIssuingCharge(
-                $item->{'itemnumber'},
-                $borrower->{'borrowernumber'}, $charge
+            ModItem(
+                {
+                    issues        => $item->{'issues'},
+                    holdingbranch => C4::Context->userenv->{'branch'},
+                    itemlost      => 0,
+                    onloan        => $datedue->ymd(),
+                    datelastborrowed => DateTime->now( time_zone => C4::Context->tz() )->ymd(),
+                },
+                $item->{'biblionumber'},
+                $item->{'itemnumber'}
             );
-            $item->{'charge'} = $charge;
-        }
+            ModDateLastSeen( $item->{'itemnumber'} );
 
-        # Record the fact that this book was issued.
-        &UpdateStats({
-                      branch => C4::Context->userenv->{'branch'},
-                      type => ( $onsite_checkout ? 'onsite_checkout' : 'issue' ),
-                      amount => $charge,
-                      other => ($sipmode ? "SIP-$sipmode" : ''),
-                      itemnumber => $item->{'itemnumber'},
-                      itemtype => $item->{'itype'},
-                      borrowernumber => $borrower->{'borrowernumber'},
-                      ccode => $item->{'ccode'}}
-        );
+           # If it costs to borrow this book, charge it to the patron's account.
+            my ( $charge, $itemtype ) = GetIssuingCharges( $item->{'itemnumber'}, $borrower->{'borrowernumber'} );
+            if ( $charge > 0 ) {
+                AddIssuingCharge( $item->{'itemnumber'}, $borrower->{'borrowernumber'}, $charge );
+                $item->{'charge'} = $charge;
+            }
 
-        # 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,
-            });
+            # Record the fact that this book was issued.
+            &UpdateStats(
+                {
+                    branch => C4::Context->userenv->{'branch'},
+                    type => ( $onsite_checkout ? 'onsite_checkout' : 'issue' ),
+                    amount         => $charge,
+                    other          => ( $sipmode ? "SIP-$sipmode" : '' ),
+                    itemnumber     => $item->{'itemnumber'},
+                    itemtype       => $item->{'itype'},
+                    location       => $item->{location},
+                    borrowernumber => $borrower->{'borrowernumber'},
+                    ccode          => $item->{'ccode'}
+                }
+            );
+
+            # 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'},
+                $item->{'itemnumber'}
+            ) if C4::Context->preference("IssueLog");
         }
     }
-
-    logaction("CIRCULATION", "ISSUE", $borrower->{'borrowernumber'}, $biblio->{'itemnumber'})
-        if C4::Context->preference("IssueLog");
-  }
-  return $issue;
+    return $issue;
 }
 
 =head2 GetLoanLength
@@ -1383,47 +1479,47 @@ sub GetLoanLength {
     my $loanlength = $sth->fetchrow_hashref;
 
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
     $sth->execute( $borrowertype, '*', $branchcode );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
     $sth->execute( '*', $itemtype, $branchcode );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
     $sth->execute( '*', '*', $branchcode );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
     $sth->execute( $borrowertype, $itemtype, '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
     $sth->execute( $borrowertype, '*', '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
     $sth->execute( '*', $itemtype, '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
     $sth->execute( '*', '*', '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
-    # if no rule is set => 21 days (hardcoded)
+    # if no rule is set => 0 day (hardcoded)
     return {
-        issuelength => 21,
-        renewalperiod => 21,
+        issuelength => 0,
+        renewalperiod => 0,
         lengthunit => 'days',
     };
 
@@ -1441,72 +1537,23 @@ Get the Hard Due Date and it's comparison for an itemtype, a borrower type and a
 sub GetHardDueDate {
     my ( $borrowertype, $itemtype, $branchcode ) = @_;
 
-    my $rule = GetIssuingRule( $borrowertype, $itemtype, $branchcode );
+    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
+        {   categorycode => $borrowertype,
+            itemtype     => $itemtype,
+            branchcode   => $branchcode
+        }
+    );
+
 
-    if ( defined( $rule ) ) {
-        if ( $rule->{hardduedate} ) {
-            return (dt_from_string($rule->{hardduedate}, 'iso'),$rule->{hardduedatecompare});
+    if ( defined( $issuing_rule ) ) {
+        if ( $issuing_rule->hardduedate ) {
+            return (dt_from_string($issuing_rule->hardduedate, 'iso'),$issuing_rule->hardduedatecompare);
         } else {
             return (undef, undef);
         }
     }
 }
 
-=head2 GetIssuingRule
-
-  my $irule = &GetIssuingRule($borrowertype,$itemtype,branchcode)
-
-FIXME - This is a copy-paste of GetLoanLength
-as a stop-gap.  Do not wish to change API for GetLoanLength 
-this close to release.
-
-Get the issuing rule for an itemtype, a borrower type and a branch
-Returns a hashref from the issuingrules table.
-
-=cut
-
-sub GetIssuingRule {
-    my ( $borrowertype, $itemtype, $branchcode ) = @_;
-    my $dbh = C4::Context->dbh;
-    my $sth =  $dbh->prepare( "select * from issuingrules where categorycode=? and itemtype=? and branchcode=?"  );
-    my $irule;
-
-    $sth->execute( $borrowertype, $itemtype, $branchcode );
-    $irule = $sth->fetchrow_hashref;
-    return $irule if defined($irule) ;
-
-    $sth->execute( $borrowertype, "*", $branchcode );
-    $irule = $sth->fetchrow_hashref;
-    return $irule if defined($irule) ;
-
-    $sth->execute( "*", $itemtype, $branchcode );
-    $irule = $sth->fetchrow_hashref;
-    return $irule if defined($irule) ;
-
-    $sth->execute( "*", "*", $branchcode );
-    $irule = $sth->fetchrow_hashref;
-    return $irule if defined($irule) ;
-
-    $sth->execute( $borrowertype, $itemtype, "*" );
-    $irule = $sth->fetchrow_hashref;
-    return $irule if defined($irule) ;
-
-    $sth->execute( $borrowertype, "*", "*" );
-    $irule = $sth->fetchrow_hashref;
-    return $irule if defined($irule) ;
-
-    $sth->execute( "*", $itemtype, "*" );
-    $irule = $sth->fetchrow_hashref;
-    return $irule if defined($irule) ;
-
-    $sth->execute( "*", "*", "*" );
-    $irule = $sth->fetchrow_hashref;
-    return $irule if defined($irule) ;
-
-    # if no rule matches,
-    return;
-}
-
 =head2 GetBranchBorrowerCircRule
 
   my $branch_cat_rule = GetBranchBorrowerCircRule($branchcode, $categorycode);
@@ -1519,6 +1566,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 +1583,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 +1592,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 +1651,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:
 
@@ -1625,17 +1670,17 @@ sub GetBranchItemRule {
     my $result = {};
 
     my @attempts = (
-        ['SELECT holdallowed, returnbranch
+        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
             FROM branch_item_rules
             WHERE branchcode = ?
               AND itemtype = ?', $branchcode, $itemtype],
-        ['SELECT holdallowed, returnbranch
+        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
             FROM default_branch_circ_rules
             WHERE branchcode = ?', $branchcode],
-        ['SELECT holdallowed, returnbranch
+        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
             FROM default_branch_item_rules
             WHERE itemtype = ?', $itemtype],
-        ['SELECT holdallowed, returnbranch
+        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
             FROM default_circ_rules'],
     );
 
@@ -1648,11 +1693,13 @@ sub GetBranchItemRule {
         # defaults tables, we have to check that the key we want is set, not
         # just that a row was returned
         $result->{'holdallowed'}  = $search_result->{'holdallowed'}  unless ( defined $result->{'holdallowed'} );
+        $result->{'hold_fulfillment_policy'} = $search_result->{'hold_fulfillment_policy'} unless ( defined $result->{'hold_fulfillment_policy'} );
         $result->{'returnbranch'} = $search_result->{'returnbranch'} unless ( defined $result->{'returnbranch'} );
     }
     
     # built-in default circulation rule
     $result->{'holdallowed'} = 2 unless ( defined $result->{'holdallowed'} );
+    $result->{'hold_fulfillment_policy'} = 'any' unless ( defined $result->{'hold_fulfillment_policy'} );
     $result->{'returnbranch'} = 'homebranch' unless ( defined $result->{'returnbranch'} );
 
     return $result;
@@ -1702,12 +1749,6 @@ No item with this barcode exists. The value is C<$barcode>.
 
 The book is not currently on loan. The value is C<$barcode>.
 
-=item C<IsPermanent>
-
-The book's home branch is a permanent collection. If you have borrowed
-this book, you are not allowed to return it. The value is the code for
-the book's home branch.
-
 =item C<withdrawn>
 
 This book has been withdrawn/cancelled. The value should be ignored.
@@ -1725,6 +1766,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,29 +1787,31 @@ 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;
     }
     $branch = C4::Context->userenv->{'branch'} unless $branch;  # we trust userenv to be a safe fallback/default
     my $messages;
-    my $borrower;
-    my $biblio;
+    my $patron;
     my $doreturn       = 1;
     my $validTransfert = 0;
     my $stat_type = 'return';
 
     # get information on item
-    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";
+    my $item = GetItem( undef, $barcode );
+    unless ($item) {
+        return ( 0, { BadBarcode => $barcode } );    # no barcode means no item or borrower.  bail out.
+    }
+
+    my $itemnumber = $item->{ itemnumber };
+    my $itemtype = $item->{itype}; # GetItem called effective_itemtype
+
+    my $issue  = Koha::Checkouts->find( { itemnumber => $itemnumber } );
+    if ( $issue ) {
+        $patron = Koha::Patrons->find( $issue->borrowernumber )
+            or die "Data inconsistency: barcode $barcode (itemnumber:$itemnumber) claims to be issued to non-existent borrowernumber '" . $issue->borrowernumber . "'\n"
+                . Dumper($issue->unblessed) . "\n";
     } else {
         $messages->{'NotIssued'} = $barcode;
         # even though item is not on loan, it may still be transferred;  therefore, get current branch info
@@ -1773,8 +1824,6 @@ sub AddReturn {
         }
     }
 
-    my $item = GetItem($itemnumber) or die "GetItem($itemnumber) failed";
-
     if ( $item->{'location'} eq 'PROC' ) {
         if ( C4::Context->preference("InProcessingToShelvingCart") ) {
             $item->{'location'} = 'CART';
@@ -1787,13 +1836,13 @@ 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
+    my $borrowernumber = $patron ? $patron->borrowernumber : undef;    # we don't know if we had a borrower or not
+    my $patron_unblessed = $patron ? $patron->unblessed : {};
 
     my $yaml = C4::Context->preference('UpdateNotForLoanStatusOnCheckin');
     if ($yaml) {
@@ -1814,14 +1863,6 @@ 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 ) {
-        my $branches = GetBranches();    # a potentially expensive call for a non-feature.
-        $branches->{$hbr}->{PE} and $messages->{'IsPermanent'} = $hbr;
-    }
-
     # check if the return is allowed at this branch
     my ($returnallowed, $message) = CanBookBeReturned($item, $branch);
     unless ($returnallowed){
@@ -1830,7 +1871,7 @@ sub AddReturn {
             Rightbranch => $message
         };
         $doreturn = 0;
-        return ( $doreturn, $messages, $issue, $borrower );
+        return ( $doreturn, $messages, $issue, $patron_unblessed);
     }
 
     if ( $item->{'withdrawn'} ) { # book has been cancelled
@@ -1838,72 +1879,55 @@ sub AddReturn {
         $doreturn = 0 if C4::Context->preference("BlockReturnOfWithdrawnItems");
     }
 
+    if ( $item->{itemlost} and C4::Context->preference("BlockReturnOfLostItems") ) {
+        $doreturn = 0;
+    }
+
     # case of a return of document (deal with issues and holdingbranch)
     my $today = DateTime->now( time_zone => C4::Context->tz() );
 
     if ($doreturn) {
-        my $datedue = $issue->{date_due};
-        $borrower or warn "AddReturn without current borrower";
+        my $is_overdue;
+        die "The item is not issed and cannot be returned" unless $issue; # Just in case...
+        $patron or warn "AddReturn without current borrower";
                my $circControlBranch;
         if ($dropbox) {
             # 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;
-        }
-
-        if ($borrowernumber) {
-            if ( ( C4::Context->preference('CalculateFinesOnReturn') && $issue->{'overdue'} ) || $return_date ) {
-                # we only need to calculate and change the fines if we want to do that on return
-                # Should be on for hourly loans
-                my $control = C4::Context->preference('CircControl');
-                my $control_branchcode =
-                    ( $control eq 'ItemHomeLibrary' ) ? $item->{homebranch}
-                  : ( $control eq 'PatronLibrary' )   ? $borrower->{branchcode}
-                  :                                     $issue->{branchcode};
-
-                my $date_returned =
-                  $return_date ? dt_from_string($return_date) : $today;
-
-                my ( $amount, $type, $unitcounttotal ) =
-                  C4::Overdues::CalcFine( $item, $borrower->{categorycode},
-                    $control_branchcode, $datedue, $date_returned );
-
-                $type ||= q{};
-
-                if ( C4::Context->preference('finesMode') eq 'production' ) {
-                    if ( $amount > 0 ) {
-                        C4::Overdues::UpdateFine( $issue->{itemnumber},
-                            $issue->{borrowernumber},
-                            $amount, $type, 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
+            $circControlBranch = _GetCircControlBranch($item,$patron_unblessed);
+            $is_overdue = $issue->is_overdue( $dropboxdate );
+        } else {
+            $is_overdue = $issue->is_overdue;
+        }
 
-                        C4::Overdues::UpdateFine( $issue->{itemnumber},
-                            $issue->{borrowernumber},
-                            0, $type, output_pref($datedue) );
-                    }
+        if ($patron) {
+            eval {
+                MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
+                    $circControlBranch, $return_date, $patron->privacy );
+            };
+            unless ( $@ ) {
+                if ( ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue ) || $return_date ) {
+                    _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $patron_unblessed, return_date => $return_date } );
                 }
-            }
+            } else {
+                carp "The checkin for the following issue failed, Please go to the about page, section 'data corrupted' to know how to fix this problem ($@)" . Dumper( $issue->unblessed );
 
-            MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
-                $circControlBranch, $return_date, $borrower->{'privacy'} );
+                return ( 0, { WasReturned => 0, DataCorrupted => 1 }, $issue, $patron_unblessed );
+            }
 
             # FIXME is the "= 1" right?  This could be the borrower hash.
             $messages->{'WasReturned'} = 1;
 
         }
 
-        ModItem({ onloan => undef }, $issue->{'biblionumber'}, $item->{'itemnumber'});
+        ModItem({ onloan => undef }, $item->{biblionumber}, $item->{'itemnumber'});
     }
 
     # 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
+    my $item_holding_branch = $item->{ holdingbranch };
     if ($item->{'holdingbranch'} ne $branch) {
         UpdateHoldingbranch($branch, $item->{'itemnumber'});
         $item->{'holdingbranch'} = $branch; # update item data holdingbranch too
@@ -1936,10 +1960,21 @@ sub AddReturn {
     # fix up the accounts.....
     if ( $item->{'itemlost'} ) {
         $messages->{'WasLost'} = 1;
-
-        if ( C4::Context->preference('RefundLostItemFeeOnReturn' ) ) {
-            _FixAccountForLostAndReturned($item->{'itemnumber'}, $borrowernumber, $barcode);    # can tolerate undef $borrowernumber
-            $messages->{'LostItemFeeRefunded'} = 1;
+        unless ( C4::Context->preference("BlockReturnOfLostItems") ) {
+            if (
+                Koha::RefundLostItemFeeRules->should_refund(
+                    {
+                        current_branch      => C4::Context->userenv->{branch},
+                        item_home_branch    => $item->{homebranch},
+                        item_holding_branch => $item_holding_branch
+                    }
+                )
+              )
+            {
+                _FixAccountForLostAndReturned( $item->{'itemnumber'},
+                    $borrowernumber, $barcode );
+                $messages->{'LostItemFeeRefunded'} = 1;
+            }
         }
     }
 
@@ -1947,26 +1982,26 @@ sub AddReturn {
     if ($borrowernumber) {
         my $fix = _FixOverduesOnReturn($borrowernumber, $item->{itemnumber}, $exemptfine, $dropbox);
         defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $item->{itemnumber}...) failed!";  # zero is OK, check defined
-        
-        if ( $issue->{overdue} && $issue->{date_due} ) {
+
+        if ( $issue and $issue->is_overdue ) {
         # fix fine days
             $today = $dropboxdate if $dropbox;
-            my ($debardate,$reminder) = _debar_user_on_return( $borrower, $item, $issue->{date_due}, $today );
+            my ($debardate,$reminder) = _debar_user_on_return( $patron_unblessed, $item, dt_from_string($issue->date_due), $today );
             if ($reminder){
                 $messages->{'PrevDebarred'} = $debardate;
             } else {
                 $messages->{'Debarred'} = $debardate if $debardate;
             }
         # there's no overdue on the item but borrower had been previously debarred
-        } elsif ( $issue->{date_due} and $borrower->{'debarred'} ) {
-             if ( $borrower->{debarred} eq "9999-12-31") {
-                $messages->{'ForeverDebarred'} = $borrower->{'debarred'};
+        } elsif ( $issue->date_due and $patron->debarred ) {
+             if ( $patron->debarred eq "9999-12-31") {
+                $messages->{'ForeverDebarred'} = $patron->debarred;
              } else {
-                  my $borrower_debar_dt = dt_from_string( $borrower->{debarred} );
+                  my $borrower_debar_dt = dt_from_string( $patron->debarred );
                   $borrower_debar_dt->truncate(to => 'day');
                   my $today_dt = $today->clone()->truncate(to => 'day');
                   if ( DateTime->compare( $borrower_debar_dt, $today_dt ) != -1 ) {
-                      $messages->{'PrevDebarred'} = $borrower->{'debarred'};
+                      $messages->{'PrevDebarred'} = $patron->debarred;
                   }
              }
         }
@@ -1983,65 +2018,63 @@ sub AddReturn {
     }
 
     # Record the fact that this book was returned.
-    # FIXME itemtype should record item level type, not bibliolevel type
     UpdateStats({
-                branch => $branch,
-                type => $stat_type,
-                itemnumber => $item->{'itemnumber'},
-                itemtype => $biblio->{'itemtype'},
-                borrowernumber => $borrowernumber,
-                ccode => $item->{'ccode'}}
-    );
+        branch         => $branch,
+        type           => $stat_type,
+        itemnumber     => $itemnumber,
+        itemtype       => $itemtype,
+        borrowernumber => $borrowernumber,
+        ccode          => $item->{ ccode }
+    });
+
+    # Send a check-in slip. # NOTE: borrower may be undef. Do not try to send messages then.
+    if ( $patron ) {
+        my $circulation_alert = 'C4::ItemCirculationAlertPreference';
+        my %conditions = (
+            branchcode   => $branch,
+            categorycode => $patron->categorycode,
+            item_type    => $item->{itype},
+            notification => 'CHECKIN',
+        );
+        if ($doreturn && $circulation_alert->is_enabled_for(\%conditions)) {
+            SendCirculationAlert({
+                type     => 'CHECKIN',
+                item     => $item,
+                borrower => $patron->unblessed,
+                branch   => $branch,
+            });
+        }
+
+        logaction("CIRCULATION", "RETURN", $borrowernumber, $item->{'itemnumber'})
+            if C4::Context->preference("ReturnLog");
+        }
 
-    # 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,
-        });
-    }
-    
-    logaction("CIRCULATION", "RETURN", $borrowernumber, $item->{'itemnumber'})
-        if C4::Context->preference("ReturnLog");
-    
     # Remove any OVERDUES related debarment if the borrower has no overdues
     if ( $borrowernumber
-      && $borrower->{'debarred'}
+      && $patron->debarred
       && C4::Context->preference('AutoRemoveOverduesRestrictions')
-      && !C4::Members::HasOverdues( $borrowernumber )
+      && !Koha::Patrons->find( $borrowernumber )->has_overdues
       && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
     ) {
         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;
         }
     }
 
-    return ( $doreturn, $messages, $issue, $borrower );
+    return ( $doreturn, $messages, $issue, ( $patron ? $patron->unblessed : {} ));
 }
 
 =head2 MarkIssueReturned
@@ -2070,7 +2103,25 @@ routine in C<C4::Accounts>.
 sub MarkIssueReturned {
     my ( $borrowernumber, $itemnumber, $dropbox_branch, $returndate, $privacy ) = @_;
 
+
+    # Retrieve the issue
+    my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return;
+    my $issue_id = $issue->issue_id;
+
+    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 Koha::Patrons->find( $anonymouspatron );
+    }
+    my $database = Koha::Database->new();
+    my $schema   = $database->schema;
     my $dbh   = C4::Context->dbh;
+
     my $query = 'UPDATE issues SET returndate=';
     my @bind;
     if ($dropbox_branch) {
@@ -2084,32 +2135,39 @@ sub MarkIssueReturned {
     } else {
         $query .= ' now() ';
     }
-    $query .= ' WHERE  borrowernumber = ?  AND itemnumber = ?';
-    push @bind, $borrowernumber, $itemnumber;
-    # FIXME transaction
-    my $sth_upd  = $dbh->prepare($query);
-    $sth_upd->execute(@bind);
-    my $sth_copy = $dbh->prepare('INSERT INTO old_issues SELECT * FROM issues
-                                  WHERE borrowernumber = ?
-                                  AND itemnumber = ?');
-    $sth_copy->execute($borrowernumber, $itemnumber);
-    # 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 = ?");
-       $sth_ano->execute($anonymouspatron, $borrowernumber, $itemnumber);
-    }
-    my $sth_del  = $dbh->prepare("DELETE FROM issues
-                                  WHERE borrowernumber = ?
-                                  AND itemnumber = ?");
-    $sth_del->execute($borrowernumber, $itemnumber);
-
-    ModItem( { 'onloan' => undef }, undef, $itemnumber );
+    $query .= ' WHERE issue_id = ?';
+    push @bind, $issue_id;
+
+    # FIXME Improve the return value and handle it from callers
+    $schema->txn_do(sub {
+
+        # Update the returndate
+        $dbh->do( $query, undef, @bind );
+
+        # We just updated the returndate, so we need to refetch $issue
+        $issue->discard_changes;
+
+        # Create the old_issues entry
+        my $old_checkout = Koha::Old::Checkout->new($issue->unblessed)->store;
+
+        # anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber
+        if ( $privacy == 2) {
+            $dbh->do(q|UPDATE old_issues SET borrowernumber=? WHERE issue_id = ?|, undef, $anonymouspatron, $old_checkout->issue_id);
+        }
+
+        # And finally delete the issue
+        $issue->delete;
+
+        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 );
+        }
+    });
+
+    return $issue_id;
 }
 
 =head2 _debar_user_on_return
@@ -2122,26 +2180,30 @@ C<$item> item hashref
 
 C<$datedue> date due DateTime object
 
-C<$today> DateTime object representing the return time
+C<$return_date> DateTime object representing the return time
 
 Internal function, called only by AddReturn that calculates and updates
- the user fine days, and debars him if necessary.
+ the user fine days, and debars them if necessary.
 
 Should only be called for overdue returns
 
 =cut
 
 sub _debar_user_on_return {
-    my ( $borrower, $item, $dt_due, $dt_today ) = @_;
+    my ( $borrower, $item, $dt_due, $return_date ) = @_;
 
     my $branchcode = _GetCircControlBranch( $item, $borrower );
 
     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);
+    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
+        {   categorycode => $borrower->{categorycode},
+            itemtype     => $item->{itype},
+            branchcode   => $branchcode
+        }
+    );
+    my $finedays = $issuing_rule ? $issuing_rule->finedays : undef;
+    my $unit     = $issuing_rule ? $issuing_rule->lengthunit : undef;
+    my $chargeable_units = C4::Overdues::get_chargeable_units($unit, $dt_due, $return_date, $branchcode);
 
     if ($finedays) {
 
@@ -2151,7 +2213,7 @@ sub _debar_user_on_return {
 
         # grace period is measured in the same units as the loan
         my $grace =
-          DateTime::Duration->new( $unit => $issuingrule->{firstremind} );
+          DateTime::Duration->new( $unit => $issuing_rule->firstremind );
 
         my $deltadays = DateTime::Duration->new(
             days => $chargeable_units
@@ -2161,26 +2223,41 @@ sub _debar_user_on_return {
 
             # If the max suspension days is < than the suspension days
             # the suspension days is limited to this maximum period.
-            my $max_sd = $issuingrule->{maxsuspensiondays};
+            my $max_sd = $issuing_rule->maxsuspensiondays;
             if ( defined $max_sd ) {
                 $max_sd = DateTime::Duration->new( days => $max_sd );
                 $suspension_days = $max_sd
                   if DateTime::Duration->compare( $max_sd, $suspension_days ) < 0;
             }
 
+            my ( $has_been_extended, $is_a_reminder );
+            if ( C4::Context->preference('CumulativeRestrictionPeriods') and $borrower->{debarred} ) {
+                my $debarment = @{ GetDebarments( { borrowernumber => $borrower->{borrowernumber}, type => 'SUSPENSION' } ) }[0];
+                if ( $debarment ) {
+                    $return_date = dt_from_string( $debarment->{expiration}, 'sql' );
+                    $has_been_extended = 1;
+                }
+            }
+
             my $new_debar_dt =
-              $dt_today->clone()->add_duration( $suspension_days );
+              $return_date->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}) ) {
-                    return ($borrower->{debarred},1);
+            my $patron = Koha::Patrons->find( $borrower->{borrowernumber} );
+            my $new_debarment_str;
+            if ( $borrower->{debarred} eq $patron->is_debarred ) {
+                $is_a_reminder = 1;
+                $new_debarment_str = $borrower->{debarred};
+            } else {
+                $new_debarment_str = $new_debar_dt->ymd();
             }
-            return $new_debar_dt->ymd();
+            # FIXME Should return a DateTime object
+            return $new_debarment_str, $is_a_reminder;
         }
     }
     return;
@@ -2215,39 +2292,65 @@ sub _FixOverduesOnReturn {
     my $dbh = C4::Context->dbh;
 
     # check for overdue fine
-    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 = $sth->fetchrow_hashref;
-    return 0 unless $data;    # no warning, there's just nothing to fix
+    my $accountline = Koha::Account::Lines->search(
+        {
+            borrowernumber => $borrowernumber,
+            itemnumber     => $item,
+            -or            => [
+                accounttype => 'FU',
+                accounttype => 'O',
+            ],
+        }
+    )->next();
+    return 0 unless $accountline;    # no warning, there's just nothing to fix
 
     my $uquery;
-    my @bind = ($data->{'accountlines_id'});
     if ($exemptfine) {
-        $uquery = "update accountlines set accounttype='FFOR', amountoutstanding=0";
+        my $amountoutstanding = $accountline->amountoutstanding;
+
+        $accountline->accounttype('FFOR');
+        $accountline->amountoutstanding(0);
+
+        Koha::Account::Offset->new(
+            {
+                debit_id => $accountline->id,
+                type => 'Forgiven',
+                amount => $amountoutstanding * -1,
+            }
+        );
+
         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");
+    } elsif ($dropbox && $accountline->lastincrement) {
+        my $outstanding = $accountline->amountoutstanding - $accountline->lastincrement;
+        my $amt = $accountline->amount - $accountline->lastincrement;
+
+        Koha::Account::Offset->new(
+            {
+                debit_id => $accountline->id,
+                type => 'Dropbox',
+                amount => $accountline->lastincrement * -1,
+            }
+        );
+
+        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) ;
+
+        $accountline->accounttype('F');
+
+        if ( $outstanding >= 0 && $amt >= 0 ) {
+            $accountline->amount($amt);
+            $accountline->amountoutstanding($outstanding);
         }
+
     } else {
-        $uquery = "update accountlines set accounttype='F' ";
+        $accountline->accounttype('F');
     }
-    $uquery .= " where (accountlines_id = ?)";
-    my $usth = $dbh->prepare($uquery);
-    return $usth->execute(@bind);
+
+    return $accountline->store();
 }
 
 =head2 _FixAccountForLostAndReturned
@@ -2258,83 +2361,45 @@ 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.
-
 =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 IN ('L', 'Rep', 'W') ORDER BY date DESC, accountno DESC");
-    $sth->execute($itemnumber);
-    my $data = $sth->fetchrow_hashref;
-    $data or return;    # bail if there is nothing to do
-    $data->{accounttype} eq 'W' and return;    # Written off
-
-    # 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 (accountlines_id = ?)");
-    $usth->execute($data->{'accountlines_id'});      # 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->{'accountlines_id'};
-            # FIXME: move prepares outside while loop!
-            my $usth = $dbh->prepare("UPDATE accountlines SET amountoutstanding= ?
-                    WHERE (accountlines_id = ?)");
-            $usth->execute($newamtos,$thisacct);
-            $usth = $dbh->prepare("INSERT INTO accountoffsets
-                (borrowernumber, accountno, offsetaccount,  offsetamount)
-                VALUES
-                (?,?,?,?)");
-            $usth->execute($data->{'borrowernumber'},$accdata->{'accountno'},$nextaccntno,$newamtos);
-        }
-    }
-    $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;
+    my $accountline = Koha::Account::Lines->search(
+        {
+            itemnumber  => $itemnumber,
+            accounttype => { -in => [ 'L', 'Rep', 'W' ] },
+        },
+        {
+            order_by => { -desc => [ 'date', 'accountno' ] }
+        }
+    )->next();
+
+    return unless $accountline;
+    return if $accountline->accounttype eq 'W';    # Written off
+
+    $accountline->accounttype('LR');
+    $accountline->store();
+
+    my $account = Koha::Account->new( { patron_id => $accountline->borrowernumber } );
+    my $credit_id = $account->pay(
+        {
+            amount       => $accountline->amount,
+            description  => "Item Returned " . $item_id,
+            account_type => 'CR',
+            offset_type  => 'Lost Item Return',
+            accounlines  => [$accountline],
+
+        }
+    );
+
+    ModItem( { paidfor => '' }, undef, $itemnumber );
+
+    return $credit_id;
 }
 
 =head2 _GetCircControlBranch
@@ -2368,183 +2433,32 @@ sub _GetCircControlBranch {
         # 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
-
-  $issue = &GetItemIssue($itemnumber);
-
-Returns patron currently having a book, or undef if not checked out.
-
-C<$itemnumber> is the itemnumber.
-
-C<$issue> is a hashref of the row from the issues table.
-
-=cut
-
-sub GetItemIssue {
-    my ($itemnumber) = @_;
-    return unless $itemnumber;
-    my $sth = C4::Context->dbh->prepare(
-        "SELECT items.*, issues.*
-        FROM issues
-        LEFT JOIN items ON issues.itemnumber=items.itemnumber
-        WHERE issues.itemnumber=?");
-    $sth->execute($itemnumber);
-    my $data = $sth->fetchrow_hashref;
-    return unless $data;
-    $data->{issuedate} = dt_from_string($data->{issuedate}, 'sql');
-    $data->{issuedate}->truncate(to => 'minute');
-    $data->{date_due} = dt_from_string($data->{date_due}, 'sql');
-    $data->{date_due}->truncate(to => 'minute');
-    my $dt = DateTime->now( time_zone => C4::Context->tz)->truncate( to => 'minute');
-    $data->{'overdue'} = DateTime->compare($data->{'date_due'}, $dt ) == -1 ? 1 : 0;
-    return $data;
-}
-
-=head2 GetOpenIssue
-
-  $issue = GetOpenIssue( $itemnumber );
-
-Returns the row from the issues table if the item is currently issued, undef if the item is not currently issued
-
-C<$itemnumber> is the item's itemnumber
-
-Returns a hashref
-
-=cut
-
-sub GetOpenIssue {
-  my ( $itemnumber ) = @_;
-  return unless $itemnumber;
-  my $dbh = C4::Context->dbh;  
-  my $sth = $dbh->prepare( "SELECT * FROM issues WHERE itemnumber = ? AND returndate IS NULL" );
-  $sth->execute( $itemnumber );
-  return $sth->fetchrow_hashref();
-
-}
-
-=head2 GetIssues
-
-    $issues = GetIssues({});    # return all issues!
-    $issues = GetIssues({ borrowernumber => $borrowernumber, biblionumber => $biblionumber });
-
-Returns all pending issues that match given criteria.
-Returns a arrayref or undef if an error occurs.
-
-Allowed criteria are:
-
-=over 2
-
-=item * borrowernumber
-
-=item * biblionumber
-
-=item * itemnumber
-
-=back
-
-=cut
-
-sub GetIssues {
-    my ($criteria) = @_;
-
-    # Build filters
-    my @filters;
-    my @allowed = qw(borrowernumber biblionumber itemnumber);
-    foreach (@allowed) {
-        if (defined $criteria->{$_}) {
-            push @filters, {
-                field => $_,
-                value => $criteria->{$_},
-            };
+            $branch = $item->{homebranch};
         }
     }
-
-    # Do we need to join other tables ?
-    my %join;
-    if (defined $criteria->{biblionumber}) {
-        $join{items} = 1;
-    }
-
-    # Build SQL query
-    my $where = '';
-    if (@filters) {
-        $where = "WHERE " . join(' AND ', map { "$_->{field} = ?" } @filters);
-    }
-    my $query = q{
-        SELECT issues.*
-        FROM issues
-    };
-    if (defined $join{items}) {
-        $query .= q{
-            LEFT JOIN items ON (issues.itemnumber = items.itemnumber)
-        };
-    }
-    $query .= $where;
-
-    # Execute SQL query
-    my $dbh = C4::Context->dbh;
-    my $sth = $dbh->prepare($query);
-    my $rv = $sth->execute(map { $_->{value} } @filters);
-
-    return $rv ? $sth->fetchall_arrayref({}) : undef;
+    return $branch;
 }
 
-=head2 GetItemIssues
+=head2 GetOpenIssue
 
-  $issues = &GetItemIssues($itemnumber, $history);
+  $issue = GetOpenIssue( $itemnumber );
 
-Returns patrons that have issued a book
+Returns the row from the issues table if the item is currently issued, undef if the item is not currently issued
 
-C<$itemnumber> is the itemnumber
-C<$history> is false if you just want the current "issuer" (if any)
-and true if you want issues history from old_issues also.
+C<$itemnumber> is the item's itemnumber
 
-Returns reference to an array of hashes
+Returns a hashref
 
 =cut
 
-sub GetItemIssues {
-    my ( $itemnumber, $history ) = @_;
-    
-    my $today = DateTime->now( time_zome => C4::Context->tz);  # get today date
-    $today->truncate( to => 'minute' );
-    my $sql = "SELECT * FROM issues
-              JOIN borrowers USING (borrowernumber)
-              JOIN items     USING (itemnumber)
-              WHERE issues.itemnumber = ? ";
-    if ($history) {
-        $sql .= "UNION ALL
-                 SELECT * FROM old_issues
-                 LEFT JOIN borrowers USING (borrowernumber)
-                 JOIN items USING (itemnumber)
-                 WHERE old_issues.itemnumber = ? ";
-    }
-    $sql .= "ORDER BY date_due DESC";
-    my $sth = C4::Context->dbh->prepare($sql);
-    if ($history) {
-        $sth->execute($itemnumber, $itemnumber);
-    } else {
-        $sth->execute($itemnumber);
-    }
-    my $results = $sth->fetchall_arrayref({});
-    foreach (@$results) {
-        my $date_due = dt_from_string($_->{date_due},'sql');
-        $date_due->truncate( to => 'minute' );
+sub GetOpenIssue {
+  my ( $itemnumber ) = @_;
+  return unless $itemnumber;
+  my $dbh = C4::Context->dbh;  
+  my $sth = $dbh->prepare( "SELECT * FROM issues WHERE itemnumber = ? AND returndate IS NULL" );
+  $sth->execute( $itemnumber );
+  return $sth->fetchrow_hashref();
 
-        $_->{overdue} = (DateTime->compare($date_due, $today) == -1) ? 1 : 0;
-    }
-    return $results;
 }
 
 =head2 GetBiblioIssues
@@ -2554,7 +2468,7 @@ sub GetItemIssues {
 this function get all issues from a biblionumber.
 
 Return:
-C<$issues> is a reference to array which each value is ref-to-hash. This ref-to-hash containts all column from
+C<$issues> is a reference to array which each value is ref-to-hash. This ref-to-hash contains all column from
 tables issues and the firstname,surname & cardnumber from borrowers.
 
 =cut
@@ -2604,12 +2518,15 @@ sub GetUpcomingDueIssues {
     my $dbh = C4::Context->dbh;
 
     my $statement = <<END_SQL;
-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
-HAVING days_until_due >= 0 AND days_until_due <= ?
+SELECT *
+FROM (
+    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
+) tmp
+WHERE days_until_due >= 0 AND days_until_due <= ?
 END_SQL
 
     my @bind_parameters = ( $params->{'days_in_advance'} );
@@ -2652,10 +2569,11 @@ sub CanBookBeRenewed {
     my $renews = 1;
 
     my $item      = GetItem($itemnumber)      or return ( 0, 'no_item' );
-    my $itemissue = GetItemIssue($itemnumber) or return ( 0, 'no_checkout' );
+    my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return ( 0, 'no_checkout' );
+    return ( 0, 'onsite_checkout' ) if $issue->onsite_checkout;
 
-    $borrowernumber ||= $itemissue->{borrowernumber};
-    my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber )
+    $borrowernumber ||= $issue->borrowernumber;
+    my $patron = Koha::Patrons->find( $borrowernumber )
       or return;
 
     my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber);
@@ -2667,83 +2585,159 @@ 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;
                 }
             }
-        }
 
-        @reservable = uniq(@reservable);
-
-        if ( @reservable >= @borrowernumbers ) {
-            $resfound = 0;
+            # 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;
+            my %borrowers;
+            ITEM: foreach my $i (@itemnumbers) {
+                my $item = GetItem($i);
+                next if IsItemOnHoldAndFound($i);
+                for my $b (@borrowernumbers) {
+                    my $borr = $borrowers{$b} //= Koha::Patrons->find( $b )->unblessed;
+                    next unless IsAvailableForItemLevelRequest($item, $borr);
+                    next unless CanItemBeReserved($b,$i);
+
+                    push @reservable, $i;
+                    if (@reservable >= @borrowernumbers) {
+                        $resfound = 0;
+                        last ITEM;
+                    }
+                    last;
+                }
+            }
         }
     }
-
     return ( 0, "on_reserve" ) if $resfound;    # '' when no hold was found
 
     return ( 1, undef ) if $override_limit;
 
-    my $branchcode = _GetCircControlBranch( $item, $borrower );
-    my $issuingrule =
-      GetIssuingRule( $borrower->{categorycode}, $item->{itype}, $branchcode );
+    my $branchcode = _GetCircControlBranch( $item, $patron->unblessed );
+    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
+        {   categorycode => $patron->categorycode,
+            itemtype     => $item->{itype},
+            branchcode   => $branchcode
+        }
+    );
 
     return ( 0, "too_many" )
-      if $issuingrule->{renewalsallowed} <= $itemissue->{renewals};
+      if not $issuing_rule or $issuing_rule->renewalsallowed <= $issue->renewals;
+
+    my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing');
+    my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing');
+    $patron         = Koha::Patrons->find($borrowernumber); # FIXME Is this really useful?
+    my $restricted  = $patron->is_debarred;
+    my $hasoverdues = $patron->has_overdues;
+
+    if ( $restricted and $restrictionblockrenewing ) {
+        return ( 0, 'restriction');
+    } elsif ( ($hasoverdues and $overduesblockrenewing eq 'block') || ($issue->is_overdue and $overduesblockrenewing eq 'blockitem') ) {
+        return ( 0, 'overdue');
+    }
+
+    if ( $issue->auto_renew ) {
+
+        if ( $patron->category->effective_BlockExpiredPatronOpacActions and $patron->is_expired ) {
+            return ( 0, 'auto_account_expired' );
+        }
+
+        if ( defined $issuing_rule->no_auto_renewal_after
+                and $issuing_rule->no_auto_renewal_after ne "" ) {
+            # Get issue_date and add no_auto_renewal_after
+            # If this is greater than today, it's too late for renewal.
+            my $maximum_renewal_date = dt_from_string($issue->issuedate, 'sql');
+            $maximum_renewal_date->add(
+                $issuing_rule->lengthunit => $issuing_rule->no_auto_renewal_after
+            );
+            my $now = dt_from_string;
+            if ( $now >= $maximum_renewal_date ) {
+                return ( 0, "auto_too_late" );
+            }
+        }
+        if ( defined $issuing_rule->no_auto_renewal_after_hard_limit
+                      and $issuing_rule->no_auto_renewal_after_hard_limit ne "" ) {
+            # If no_auto_renewal_after_hard_limit is >= today, it's also too late for renewal
+            if ( dt_from_string >= dt_from_string( $issuing_rule->no_auto_renewal_after_hard_limit ) ) {
+                return ( 0, "auto_too_late" );
+            }
+        }
+
+        if ( C4::Context->preference('OPACFineNoRenewalsBlockAutoRenew') ) {
+            my $fine_no_renewals = C4::Context->preference("OPACFineNoRenewals");
+            my $amountoutstanding = $patron->account->balance;
+            if ( $amountoutstanding and $amountoutstanding > $fine_no_renewals ) {
+                return ( 0, "auto_too_much_oweing" );
+            }
+        }
+    }
+
+    if ( defined $issuing_rule->norenewalbefore
+        and $issuing_rule->norenewalbefore ne "" )
+    {
+
+        # Calculate soonest renewal by subtracting 'No renewal before' from due date
+        my $soonestrenewal = dt_from_string( $issue->date_due, 'sql' )->subtract(
+            $issuing_rule->lengthunit => $issuing_rule->norenewalbefore );
 
-    if ( $issuingrule->{norenewalbefore} ) {
+        # Depending on syspref reset the exact time, only check the date
+        if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date'
+            and $issuing_rule->lengthunit eq 'days' )
+        {
+            $soonestrenewal->truncate( to => 'day' );
+        }
 
-        # 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 ( $soonestrenewal > DateTime->now( time_zone => C4::Context->tz() ) )
         {
-            return ( 0, "auto_too_soon" ) if $itemissue->{auto_renew};
+            return ( 0, "auto_too_soon" ) if $issue->auto_renew;
             return ( 0, "too_soon" );
         }
+        elsif ( $issue->auto_renew ) {
+            return ( 0, "auto_renew" );
+        }
+    }
+
+    # Fallback for automatic renewals:
+    # If norenewalbefore is undef, don't renew before due date.
+    if ( $issue->auto_renew ) {
+        my $now = dt_from_string;
+        return ( 0, "auto_renew" )
+          if $now >= dt_from_string( $issue->date_due, 'sql' );
+        return ( 0, "auto_too_soon" );
     }
 
-    return ( 0, "auto_renew" ) if $itemissue->{auto_renew};
     return ( 1, undef );
 }
 
@@ -2761,7 +2755,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.
@@ -2779,43 +2773,47 @@ sub AddRenewal {
     my $lastreneweddate = shift || DateTime->now(time_zone => C4::Context->tz)->ymd();
 
     my $item   = GetItem($itemnumber) or return;
-    my $biblio = GetBiblioFromItemNumber($itemnumber) or return;
+    my $item_object = Koha::Items->find( $itemnumber ); # Should replace $item
+    my $biblio = $item_object->biblio;
 
     my $dbh = C4::Context->dbh;
 
     # Find the issues record for this book
-    my $sth =
-      $dbh->prepare("SELECT * FROM issues WHERE itemnumber = ?");
-    $sth->execute( $itemnumber );
-    my $issuedata = $sth->fetchrow_hashref;
+    my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } );
 
-    return unless ( $issuedata );
+    return unless $issue;
 
-    $borrowernumber ||= $issuedata->{borrowernumber};
+    $borrowernumber ||= $issue->borrowernumber;
 
     if ( defined $datedue && ref $datedue ne 'DateTime' ) {
         carp 'Invalid date passed to AddRenewal.';
         return;
     }
 
+    my $patron = Koha::Patrons->find( $borrowernumber ) or return; # FIXME Should do more than just return
+    my $patron_unblessed = $patron->unblessed;
+
+    if ( C4::Context->preference('CalculateFinesOnReturn') && $issue->is_overdue ) {
+        _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $patron_unblessed } );
+    }
+    _FixOverduesOnReturn( $borrowernumber, $itemnumber );
+
     # 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::GetMember( borrowernumber => $borrowernumber ) or return;
-        my $itemtype = (C4::Context->preference('item-level_itypes')) ? $biblio->{'itype'} : $biblio->{'itemtype'};
-
+        my $itemtype = $item_object->effective_itemtype;
         $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?
-                                        dt_from_string( $issuedata->{date_due} ) :
+                                        dt_from_string( $issue->date_due, 'sql' ) :
                                         DateTime->now( time_zone => C4::Context->tz());
-        $datedue =  CalcDateDue($datedue, $itemtype, $issuedata->{'branchcode'}, $borrower, 'is a renewal');
+        $datedue =  CalcDateDue($datedue, $itemtype, _GetCircControlBranch($item, $patron_unblessed), $patron_unblessed, 'is a renewal');
     }
 
     # 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 = ?
+    my $renews = $issue->renewals + 1;
+    my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ?
                             WHERE borrowernumber=? 
                             AND itemnumber=?"
     );
@@ -2823,14 +2821,13 @@ sub AddRenewal {
     $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $lastreneweddate, $borrowernumber, $itemnumber );
 
     # Update the renewal count on the item, and tell zebra to reindex
-    $renews = $biblio->{'renewals'} + 1;
-    ModItem({ renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $biblio->{'biblionumber'}, $itemnumber);
+    $renews = $item->{renewals} + 1;
+    ModItem({ renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item->{biblionumber}, $itemnumber);
 
     # Charge a new rental fee, if applicable?
     my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
     if ( $charge > 0 ) {
         my $accountno = getnextacctno( $borrowernumber );
-        my $item = GetBiblioFromItemNumber($itemnumber);
         my $manager_id = 0;
         $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv; 
         $sth = $dbh->prepare(
@@ -2840,50 +2837,61 @@ sub AddRenewal {
                     VALUES (now(),?,?,?,?,?,?,?,?)"
         );
         $sth->execute( $borrowernumber, $accountno, $charge, $manager_id,
-            "Renewal of Rental Item $item->{'title'} $item->{'barcode'}",
+            "Renewal of Rental Item " . $biblio->title . " $item->{'barcode'}",
             'Rent', $charge, $itemnumber );
     }
 
     # Send a renewal slip according to checkout alert preferencei
-    if ( C4::Context->preference('RenewalSendNotice') eq '1') {
-       my $borrower = C4::Members::GetMemberDetails( $borrowernumber, 0 );
-       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     => 'RENEWAL',
-                       item     => $item,
-               borrower => $borrower,
-               branch   => $branch,
-               });
-       }
+    if ( C4::Context->preference('RenewalSendNotice') eq '1' ) {
+        my $circulation_alert = 'C4::ItemCirculationAlertPreference';
+        my %conditions        = (
+            branchcode   => $branch,
+            categorycode => $patron->categorycode,
+            item_type    => $item->{itype},
+            notification => 'CHECKOUT',
+        );
+        if ( $circulation_alert->is_enabled_for( \%conditions ) ) {
+            SendCirculationAlert(
+                {
+                    type     => 'RENEWAL',
+                    item     => $item,
+                    borrower => $patron->unblessed,
+                    branch   => $branch,
+                }
+            );
+        }
     }
 
     # Remove any OVERDUES related debarment if the borrower has no overdues
-    my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber );
-    if ( $borrowernumber
-      && $borrower->{'debarred'}
-      && !C4::Members::HasOverdues( $borrowernumber )
+    if ( $patron
+      && $patron->is_debarred
+      && ! $patron->has_overdues
       && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
     ) {
         DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
     }
 
-    # Log the renewal
-    UpdateStats({branch => $branch,
-                type => 'renew',
-                amount => $charge,
-                itemnumber => $itemnumber,
-                itemtype => $item->{itype},
-                borrowernumber => $borrowernumber,
-                ccode => $item->{'ccode'}}
-                );
-       return $datedue;
+    unless ( C4::Context->interface eq 'opac' ) { #if from opac we are obeying OpacRenewalBranch as calculated in opac-renew.pl
+        $branch = C4::Context->userenv ? C4::Context->userenv->{branch} : $branch;
+    }
+
+    # Add the renewal to stats
+    UpdateStats(
+        {
+            branch         => $branch,
+            type           => 'renew',
+            amount         => $charge,
+            itemnumber     => $itemnumber,
+            itemtype       => $item->{itype},
+            location       => $item->{location},
+            borrowernumber => $borrowernumber,
+            ccode          => $item->{'ccode'}
+        }
+    );
+
+    #Log the renewal
+    logaction("CIRCULATION", "RENEWAL", $borrowernumber, $itemnumber) if C4::Context->preference("RenewalLog");
+    return $datedue;
 }
 
 sub GetRenewCount {
@@ -2894,8 +2902,10 @@ sub GetRenewCount {
     my $renewsallowed = 0;
     my $renewsleft    = 0;
 
-    my $borrower = C4::Members::GetMember( borrowernumber => $bornum);
-    my $item     = GetItem($itemno); 
+    my $patron = Koha::Patrons->find( $bornum );
+    my $item     = GetItem($itemno);
+
+    return (0, 0, 0) unless $patron or $item; # Wrong call, no renewal allowed
 
     # Look in the issues table for this item, lent to this borrower,
     # and not yet returned.
@@ -2910,11 +2920,16 @@ sub GetRenewCount {
     my $data = $sth->fetchrow_hashref;
     $renewcount = $data->{'renewals'} if $data->{'renewals'};
     # $item and $borrower should be calculated
-    my $branchcode = _GetCircControlBranch($item, $borrower);
-    
-    my $issuingrule = GetIssuingRule($borrower->{categorycode}, $item->{itype}, $branchcode);
-    
-    $renewsallowed = $issuingrule->{'renewalsallowed'};
+    my $branchcode = _GetCircControlBranch($item, $patron->unblessed);
+
+    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
+        {   categorycode => $patron->categorycode,
+            itemtype     => $item->{itype},
+            branchcode   => $branchcode
+        }
+    );
+
+    $renewsallowed = $issuing_rule ? $issuing_rule->renewalsallowed : 0;
     $renewsleft    = $renewsallowed - $renewcount;
     if($renewsleft < 0){ $renewsleft = 0; }
     return ( $renewcount, $renewsallowed, $renewsleft );
@@ -2945,29 +2960,102 @@ sub GetSoonestRenewDate {
     my $dbh = C4::Context->dbh;
 
     my $item      = GetItem($itemnumber)      or return;
-    my $itemissue = GetItemIssue($itemnumber) or return;
+    my $itemissue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return;
 
-    $borrowernumber ||= $itemissue->{borrowernumber};
-    my $borrower = C4::Members::GetMemberDetails($borrowernumber)
+    $borrowernumber ||= $itemissue->borrowernumber;
+    my $patron = Koha::Patrons->find( $borrowernumber )
       or return;
 
-    my $branchcode = _GetCircControlBranch( $item, $borrower );
-    my $issuingrule =
-      GetIssuingRule( $borrower->{categorycode}, $item->{itype}, $branchcode );
+    my $branchcode = _GetCircControlBranch( $item, $patron->unblessed );
+    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
+        {   categorycode => $patron->categorycode,
+            itemtype     => $item->{itype},
+            branchcode   => $branchcode
+        }
+    );
 
-    my $now = DateTime->now( time_zone => C4::Context->tz() );
+    my $now = dt_from_string;
+    return $now unless $issuing_rule;
 
-    if ( $issuingrule->{norenewalbefore} ) {
+    if ( defined $issuing_rule->norenewalbefore
+        and $issuing_rule->norenewalbefore ne "" )
+    {
         my $soonestrenewal =
-          $itemissue->{date_due}->subtract(
-            $issuingrule->{lengthunit} => $issuingrule->{norenewalbefore} );
+          dt_from_string( $itemissue->date_due )->subtract(
+            $issuing_rule->lengthunit => $issuing_rule->norenewalbefore );
 
-        $soonestrenewal = $now > $soonestrenewal ? $now : $soonestrenewal;
-        return $soonestrenewal;
+        if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date'
+            and $issuing_rule->lengthunit eq 'days' )
+        {
+            $soonestrenewal->truncate( to => 'day' );
+        }
+        return $soonestrenewal if $now < $soonestrenewal;
     }
     return $now;
 }
 
+=head2 GetLatestAutoRenewDate
+
+  $NoAutoRenewalAfterThisDate = &GetLatestAutoRenewDate($borrowernumber, $itemnumber);
+
+Find out the latest possible auto renew date of a borrowed item.
+
+C<$borrowernumber> is the borrower number of the patron who currently
+has the item on loan.
+
+C<$itemnumber> is the number of the item to renew.
+
+C<$GetLatestAutoRenewDate> returns the DateTime of the latest possible
+auto renew date, based on the value "No auto renewal after" and the "No auto
+renewal after (hard limit) of the applicable issuing rule.
+Returns undef if there is no date specify in the circ rules or if the patron, loan,
+or item cannot be found.
+
+=cut
+
+sub GetLatestAutoRenewDate {
+    my ( $borrowernumber, $itemnumber ) = @_;
+
+    my $dbh = C4::Context->dbh;
+
+    my $item      = GetItem($itemnumber)      or return;
+    my $itemissue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return;
+
+    $borrowernumber ||= $itemissue->borrowernumber;
+    my $patron = Koha::Patrons->find( $borrowernumber )
+      or return;
+
+    my $branchcode = _GetCircControlBranch( $item, $patron->unblessed );
+    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
+        {   categorycode => $patron->categorycode,
+            itemtype     => $item->{itype},
+            branchcode   => $branchcode
+        }
+    );
+
+    return unless $issuing_rule;
+    return
+      if ( not $issuing_rule->no_auto_renewal_after
+            or $issuing_rule->no_auto_renewal_after eq '' )
+      and ( not $issuing_rule->no_auto_renewal_after_hard_limit
+             or $issuing_rule->no_auto_renewal_after_hard_limit eq '' );
+
+    my $maximum_renewal_date;
+    if ( $issuing_rule->no_auto_renewal_after ) {
+        $maximum_renewal_date = dt_from_string($itemissue->issuedate);
+        $maximum_renewal_date->add(
+            $issuing_rule->lengthunit => $issuing_rule->no_auto_renewal_after
+        );
+    }
+
+    if ( $issuing_rule->no_auto_renewal_after_hard_limit ) {
+        my $dt = dt_from_string( $issuing_rule->no_auto_renewal_after_hard_limit );
+        $maximum_renewal_date = $dt if not $maximum_renewal_date or $maximum_renewal_date > $dt;
+    }
+    return $maximum_renewal_date;
+}
+
+
 =head2 GetIssuingCharges
 
   ($charge, $item_type) = &GetIssuingCharges($itemnumber, $borrowernumber);
@@ -3007,7 +3095,7 @@ sub GetIssuingCharges {
     if ( my $item_data = $sth->fetchrow_hashref ) {
         $item_type = $item_data->{itemtype};
         $charge    = $item_data->{rentalcharge};
-        my $branch = C4::Branch::mybranch();
+        my $branch = C4::Context::mybranch();
         my $discount_query = q|SELECT rentaldiscount,
             issuingrules.itemtype, issuingrules.branchcode
             FROM borrowers
@@ -3023,6 +3111,9 @@ sub GetIssuingCharges {
             my $discount = _get_discount_from_rule($discount_rules, $branch, $item_type);
             $charge = ( $charge * ( 100 - $discount ) ) / 100;
         }
+        if ($charge) {
+            $charge = sprintf '%.2f', $charge; # ensure no fractions of a penny returned
+        }
     }
 
     return ( $charge, $item_type );
@@ -3073,19 +3164,33 @@ sub _get_discount_from_rule {
 
 sub AddIssuingCharge {
     my ( $itemnumber, $borrowernumber, $charge ) = @_;
-    my $dbh = C4::Context->dbh;
-    my $nextaccntno = getnextacctno( $borrowernumber );
-    my $manager_id = 0;
+
+    my $nextaccntno = getnextacctno($borrowernumber);
+
+    my $manager_id  = 0;
     $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv;
-    my $query ="
-        INSERT INTO accountlines
-            (borrowernumber, itemnumber, accountno,
-            date, amount, description, accounttype,
-            amountoutstanding, manager_id)
-        VALUES (?, ?, ?,now(), ?, 'Rental', 'Rent',?,?)
-    ";
-    my $sth = $dbh->prepare($query);
-    $sth->execute( $borrowernumber, $itemnumber, $nextaccntno, $charge, $charge, $manager_id );
+
+    my $accountline = Koha::Account::Line->new(
+        {
+            borrowernumber    => $borrowernumber,
+            itemnumber        => $itemnumber,
+            accountno         => $nextaccntno,
+            amount            => $charge,
+            amountoutstanding => $charge,
+            manager_id        => $manager_id,
+            description       => 'Rental',
+            accounttype       => 'Rent',
+            date              => \'NOW()',
+        }
+    )->store();
+
+    Koha::Account::Offset->new(
+        {
+            debit_id => $accountline->id,
+            type     => 'Rental Fee',
+            amount   => $charge,
+        }
+    )->store();
 }
 
 =head2 GetTransfers
@@ -3102,7 +3207,8 @@ sub GetTransfers {
     my $query = '
         SELECT datesent,
                frombranch,
-               tobranch
+               tobranch,
+               branchtransfer_id
         FROM branchtransfers
         WHERE itemnumber = ?
           AND datearrived IS NULL
@@ -3126,7 +3232,7 @@ sub GetTransfersFromTo {
     return unless ( $frombranch && $tobranch );
     my $dbh   = C4::Context->dbh;
     my $query = "
-        SELECT itemnumber,datesent,frombranch
+        SELECT branchtransfer_id,itemnumber,datesent,frombranch
         FROM   branchtransfers
         WHERE  frombranch=?
           AND  tobranch=?
@@ -3160,48 +3266,6 @@ sub DeleteTransfer {
     return $sth->execute($itemnumber);
 }
 
-=head2 AnonymiseIssueHistory
-
-  ($rows,$err_history_not_deleted) = AnonymiseIssueHistory($date,$borrowernumber)
-
-This function write NULL instead of C<$borrowernumber> given on input arg into the table issues.
-if C<$borrowernumber> is not set, it will delete the issue history for all borrower older than C<$date>.
-
-If c<$borrowernumber> is set, it will delete issue history for only that borrower, regardless of their opac privacy
-setting (force delete).
-
-return the number of affected rows and a value that evaluates to true if an error occurred deleting the history.
-
-=cut
-
-sub AnonymiseIssueHistory {
-    my $date           = shift;
-    my $borrowernumber = shift;
-    my $dbh            = C4::Context->dbh;
-    my $query          = "
-        UPDATE old_issues
-        SET    borrowernumber = ?
-        WHERE  returndate < ?
-          AND borrowernumber IS NOT NULL
-    ";
-
-    # 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;
-    my @bind_params = ($anonymouspatron, $date);
-    if (defined $borrowernumber) {
-       $query .= " AND borrowernumber = ?";
-       push @bind_params, $borrowernumber;
-    } else {
-       $query .= " AND (SELECT privacy FROM borrowers WHERE borrowers.borrowernumber=old_issues.borrowernumber) <> 0";
-    }
-    my $sth = $dbh->prepare($query);
-    $sth->execute(@bind_params);
-    my $anonymisation_err = $dbh->err;
-    my $rows_affected = $sth->rows;  ### doublecheck row count return function
-    return ($rows_affected, $anonymisation_err);
-}
-
 =head2 SendCirculationAlert
 
 Send out a C<check-in> or C<checkout> alert using the messaging system.
@@ -3246,7 +3310,7 @@ sub SendCirculationAlert {
     my %message_name = (
         CHECKIN  => 'Item_Check_in',
         CHECKOUT => 'Item_Checkout',
-       RENEWAL  => 'Item_Checkout',
+        RENEWAL  => 'Item_Checkout',
     );
     my $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences({
         borrowernumber => $borrower->{borrowernumber},
@@ -3254,47 +3318,45 @@ sub SendCirculationAlert {
     });
     my $issues_table = ( $type eq 'CHECKOUT' || $type eq 'RENEWAL' ) ? 'issues' : 'old_issues';
 
+    my $schema = Koha::Database->new->schema;
     my @transports = keys %{ $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";
-            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, $_);
+
+    # From the MySQL doc:
+    # LOCK TABLES is not transaction-safe and implicitly commits any active transaction before attempting to lock the tables.
+    # If the LOCK/UNLOCK statements are executed from tests, the current transaction will be committed.
+    # To avoid that we need to guess if this code is execute from tests or not (yes it is a bit hacky)
+    my $do_not_lock = ( exists $ENV{_} && $ENV{_} =~ m|prove| ) || $ENV{KOHA_NO_TABLE_LOCKS};
+
+    for my $mtt (@transports) {
+        my $letter =  C4::Letters::GetPreparedLetter (
+            module => 'circulation',
+            letter_code => $type,
+            branchcode => $branch,
+            message_transport_type => $mtt,
+            lang => $borrower->{lang},
+            tables => {
+                $issues_table => $item->{itemnumber},
+                'items'       => $item->{itemnumber},
+                'biblio'      => $item->{biblionumber},
+                'biblioitems' => $item->{biblionumber},
+                'borrowers'   => $borrower,
+                'branches'    => $branch,
+            }
+        ) or next;
+
+        $schema->storage->txn_begin;
+        C4::Context->dbh->do(q|LOCK TABLE message_queue READ|) unless $do_not_lock;
+        C4::Context->dbh->do(q|LOCK TABLE message_queue WRITE|) unless $do_not_lock;
+        my $message = C4::Message->find_last_message($borrower, $type, $mtt);
+        unless ( $message ) {
+            C4::Context->dbh->do(q|UNLOCK TABLES|) unless $do_not_lock;
+            C4::Message->enqueue($letter, $borrower, $mtt);
         } 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;
         }
+        C4::Context->dbh->do(q|UNLOCK TABLES|) unless $do_not_lock;
+        $schema->storage->txn_commit;
     }
 
     return;
@@ -3344,7 +3406,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
@@ -3405,7 +3467,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
@@ -3427,10 +3489,20 @@ 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 );
+            }
+        }
+        if ( C4::Context->preference('useDaysMode') ne 'Days' ) {
+          my $calendar = Koha::Calendar->new( branchcode => $branch );
+          if ( $calendar->is_holiday($datedue) ) {
+              # Don't return on a closed day
+              $datedue = $calendar->prev_open_day( $datedue );
+          }
         }
     }
 
@@ -3438,92 +3510,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;
@@ -3606,12 +3592,12 @@ sub ReturnLostItem{
     my ( $borrowernumber, $itemnum ) = @_;
 
     MarkIssueReturned( $borrowernumber, $itemnum );
-    my $borrower = C4::Members::GetMember( 'borrowernumber'=>$borrowernumber );
+    my $patron = Koha::Patrons->find( $borrowernumber );
     my $item = C4::Items::GetItem( $itemnum );
     my $old_note = ($item->{'paidfor'} && ($item->{'paidfor'} ne q{})) ? $item->{'paidfor'}.' / ' : q{};
     my @datearr = localtime(time);
     my $date = ( 1900 + $datearr[5] ) . "-" . ( $datearr[4] + 1 ) . "-" . $datearr[3];
-    my $bor = "$borrower->{'firstname'} $borrower->{'surname'} $borrower->{'cardnumber'}";
+    my $bor = $patron->firstname . ' ' . $patron->surname . ' ' . $patron->cardnumber;
     ModItem({ paidfor =>  $old_note."Paid for by $bor $date" }, undef, $itemnum);
 }
 
@@ -3619,6 +3605,8 @@ sub ReturnLostItem{
 sub LostItem{
     my ($itemnumber, $mark_returned) = @_;
 
+    $mark_returned //= C4::Context->preference('MarkLostItemsAsReturned');
+
     my $dbh = C4::Context->dbh();
     my $sth=$dbh->prepare("SELECT issues.*,items.*,biblio.title 
                            FROM issues 
@@ -3630,7 +3618,7 @@ sub LostItem{
 
     # If a borrower lost the item, add a replacement cost to the their record
     if ( my $borrowernumber = $issues->{borrowernumber} ){
-        my $borrower = C4::Members::GetMemberDetails( $borrowernumber );
+        my $patron = Koha::Patrons->find( $borrowernumber );
 
         if (C4::Context->preference('WhenLostForgiveFine')){
             my $fix = _FixOverduesOnReturn($borrowernumber, $itemnumber, 1, 0); # 1, 0 = exemptfine, no-dropbox
@@ -3642,7 +3630,7 @@ sub LostItem{
             #warn " $issues->{'borrowernumber'}  /  $itemnumber ";
         }
 
-        MarkIssueReturned($borrowernumber,$itemnumber,undef,undef,$borrower->{'privacy'}) if $mark_returned;
+        MarkIssueReturned($borrowernumber,$itemnumber,undef,undef,$patron->privacy) if $mark_returned;
     }
 }
 
@@ -3698,9 +3686,10 @@ sub ProcessOfflineOperation {
 sub ProcessOfflineReturn {
     my $operation = shift;
 
-    my $itemnumber = C4::Items::GetItemnumberFromBarcode( $operation->{barcode} );
+    my $item = Koha::Items->find({barcode => $operation->{barcode}});
 
-    if ( $itemnumber ) {
+    if ( $item ) {
+        my $itemnumber = $item->itemnumber;
         my $issue = GetOpenIssue( $itemnumber );
         if ( $issue ) {
             MarkIssueReturned(
@@ -3726,16 +3715,17 @@ sub ProcessOfflineReturn {
 sub ProcessOfflineIssue {
     my $operation = shift;
 
-    my $borrower = C4::Members::GetMemberDetails( undef, $operation->{cardnumber} ); # Get borrower from operation cardnumber
+    my $patron = Koha::Patrons->find( { cardnumber => $operation->{cardnumber} } );
 
-    if ( $borrower->{borrowernumber} ) {
-        my $itemnumber = C4::Items::GetItemnumberFromBarcode( $operation->{barcode} );
-        unless ($itemnumber) {
+    if ( $patron ) {
+        my $item = Koha::Items->find({ barcode => $operation->{barcode} });
+        unless ($item) {
             return "Barcode not found.";
         }
+        my $itemnumber = $item->itemnumber;
         my $issue = GetOpenIssue( $itemnumber );
 
-        if ( $issue and ( $issue->{borrowernumber} ne $borrower->{borrowernumber} ) ) { # Item already issued to another borrower, mark it returned
+        if ( $issue and ( $issue->{borrowernumber} ne $patron->borrowernumber ) ) { # Item already issued to another patron mark it returned
             MarkIssueReturned(
                 $issue->{borrowernumber},
                 $itemnumber,
@@ -3744,7 +3734,7 @@ sub ProcessOfflineIssue {
             );
         }
         AddIssue(
-            $borrower,
+            $patron->unblessed,
             $operation->{'barcode'},
             undef,
             1,
@@ -3760,10 +3750,10 @@ sub ProcessOfflineIssue {
 sub ProcessOfflinePayment {
     my $operation = shift;
 
-    my $borrower = C4::Members::GetMemberDetails( undef, $operation->{cardnumber} ); # Get borrower from operation cardnumber
+    my $patron = Koha::Patrons->find( { cardnumber => $operation->{cardnumber} });
     my $amount = $operation->{amount};
 
-    recordpayment( $borrower->{borrowernumber}, $amount );
+    Koha::Account->new( { patron_id => $patron->id } )->pay( { amount => $amount } );
 
     return "Success."
 }
@@ -3783,8 +3773,6 @@ sub TransferSlip {
     my $item =  GetItem( $itemnumber, $barcode )
       or return;
 
-    my $pulldate = C4::Dates->new();
-
     return C4::Letters::GetPreparedLetter (
         module => 'circulation',
         letter_code => 'TRANSFERSLIP',
@@ -3845,7 +3833,7 @@ sub IsItemIssued {
   my ($ageRestriction, $daysToAgeRestriction) = GetAgeRestriction($record_restrictions, $borrower);
   my ($ageRestriction, $daysToAgeRestriction) = GetAgeRestriction($record_restrictions);
 
-  if($daysToAgeRestriction <= 0) { #Borrower is allowed to access this material, as he is older or as old as the agerestriction }
+  if($daysToAgeRestriction <= 0) { #Borrower is allowed to access this material, as they are older or as old as the agerestriction }
   if($daysToAgeRestriction > 0) { #Borrower is this many days from meeting the agerestriction }
 
 @PARAM1 the koha.biblioitems.agerestriction value, like K18, PEGI 13, ...
@@ -3863,7 +3851,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;
 
@@ -3902,7 +3890,8 @@ sub GetAgeRestriction {
             }
 
             #Get how many days the borrower has to reach the age restriction
-            my $daysToAgeRestriction = Date_to_Days(@alloweddate) - Date_to_Days(Today);
+            my @Today = split /-/, DateTime->today->ymd();
+            my $daysToAgeRestriction = Date_to_Days(@alloweddate) - Date_to_Days(@Today);
             #Negative days means the borrower went past the age restriction age
             return ($restriction_year, $daysToAgeRestriction);
         }
@@ -3911,7 +3900,6 @@ sub GetAgeRestriction {
     return ($restriction_year);
 }
 
-1;
 
 =head2 GetPendingOnSiteCheckouts
 
@@ -3944,6 +3932,128 @@ sub GetPendingOnSiteCheckouts {
     |, { 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 * FROM (
+        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, b.title, b.author, bi.itemtype, bi.publishercode,
+          bi.place, bi.publicationyear, b.copyrightdate, bi.pages, bi.size,
+          i.ccode
+        ORDER BY count DESC
+    };
+
+    $query .= q{ ) xxx WHERE count > 0 };
+    $count = int($count);
+    if ($count > 0) {
+        $query .= "LIMIT $count";
+    }
+
+    my $rows = $dbh->selectall_arrayref($query, { Slice => {} }, @where_args);
+
+    return @$rows;
+}
+
+sub _CalculateAndUpdateFine {
+    my ($params) = @_;
+
+    my $borrower    = $params->{borrower};
+    my $item        = $params->{item};
+    my $issue       = $params->{issue};
+    my $return_date = $params->{return_date};
+
+    unless ($borrower) { carp "No borrower passed in!" && return; }
+    unless ($item)     { carp "No item passed in!"     && return; }
+    unless ($issue)    { carp "No issue passed in!"    && return; }
+
+    my $datedue = dt_from_string( $issue->date_due );
+
+    # we only need to calculate and change the fines if we want to do that on return
+    # Should be on for hourly loans
+    my $control = C4::Context->preference('CircControl');
+    my $control_branchcode =
+        ( $control eq 'ItemHomeLibrary' ) ? $item->{homebranch}
+      : ( $control eq 'PatronLibrary' )   ? $borrower->{branchcode}
+      :                                     $issue->branchcode;
+
+    my $date_returned = $return_date ? dt_from_string($return_date) : dt_from_string();
+
+    my ( $amount, $type, $unitcounttotal ) =
+      C4::Overdues::CalcFine( $item, $borrower->{categorycode}, $control_branchcode, $datedue, $date_returned );
+
+    $type ||= q{};
+
+    if ( C4::Context->preference('finesMode') eq 'production' ) {
+        if ( $amount > 0 ) {
+            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_id       => $issue->issue_id,
+                itemnumber     => $issue->itemnumber,
+                borrowernumber => $issue->borrowernumber,
+                amount         => 0,
+                type           => $type,
+                due            => output_pref($datedue),
+            });
+        }
+    }
+}
+
+1;
+
 __END__
 
 =head1 AUTHOR
@@ -3951,4 +4061,3 @@ __END__
 Koha Development Team <http://koha-community.org/>
 
 =cut
-