Bug 24161: Add the Koha::Acquisition::Order::Claim[s] classes
[koha-ffzg.git] / C4 / Circulation.pm
index e01ddd9..8db887d 100644 (file)
@@ -18,9 +18,7 @@ package C4::Circulation;
 # You should have received a copy of the GNU General Public License
 # along with Koha; if not, see <http://www.gnu.org/licenses>.
 
-
-use strict;
-#use warnings; FIXME - Bug 2505
+use Modern::Perl;
 use DateTime;
 use POSIX qw( floor );
 use Koha::DateUtils;
@@ -46,7 +44,7 @@ use Koha::Biblioitems;
 use Koha::DateUtils;
 use Koha::Calendar;
 use Koha::Checkouts;
-use Koha::IssuingRules;
+use Koha::Illrequests;
 use Koha::Items;
 use Koha::Patrons;
 use Koha::Patron::Debarments;
@@ -54,12 +52,13 @@ use Koha::Database;
 use Koha::Libraries;
 use Koha::Account::Lines;
 use Koha::Holds;
-use Koha::RefundLostItemFeeRule;
 use Koha::RefundLostItemFeeRules;
 use Koha::Account::Lines;
 use Koha::Account::Offsets;
 use Koha::Config::SysPrefs;
 use Koha::Charges::Fees;
+use Koha::Util::SystemPreferences;
+use Koha::Checkouts::ReturnClaims;
 use Carp;
 use List::MoreUtils qw( uniq any );
 use Scalar::Util qw( looks_like_number );
@@ -253,7 +252,7 @@ sub decode {
 =head2 transferbook
 
   ($dotransfer, $messages, $iteminformation) = &transferbook($newbranch, 
-                                            $barcode, $ignore_reserves);
+                                            $barcode, $ignore_reserves, $trigger);
 
 Transfers an item to a new branch. If the item is currently on loan, it is automatically returned before the actual transfer.
 
@@ -264,6 +263,8 @@ C<$barcode> is the barcode of the item to be transferred.
 If C<$ignore_reserves> is true, C<&transferbook> ignores reserves.
 Otherwise, if an item is reserved, the transfer fails.
 
+C<$trigger> is the enum value for what triggered the transfer.
+
 Returns three values:
 
 =over
@@ -305,7 +306,7 @@ The item was eligible to be transferred. Barring problems communicating with the
 =cut
 
 sub transferbook {
-    my ( $tbr, $barcode, $ignoreRs ) = @_;
+    my ( $tbr, $barcode, $ignoreRs, $trigger ) = @_;
     my $messages;
     my $dotransfer      = 1;
     my $item = Koha::Items->find( { barcode => $barcode } );
@@ -314,10 +315,10 @@ sub transferbook {
     unless ( $item ) {
         $messages->{'BadBarcode'} = $barcode;
         $dotransfer = 0;
+        return ( $dotransfer, $messages );
     }
 
     my $itemnumber = $item->itemnumber;
-    my $issue = GetOpenIssue($itemnumber);
     # get branches of book...
     my $hbr = $item->homebranch;
     my $fbr = $item->holdingbranch;
@@ -343,6 +344,7 @@ sub transferbook {
     }
 
     # check if it is still issued to someone, return it...
+    my $issue = Koha::Checkouts->find({ itemnumber => $itemnumber });
     if ( $issue ) {
         AddReturn( $barcode, $fbr );
         $messages->{'WasReturned'} = $issue->borrowernumber;
@@ -354,14 +356,13 @@ sub transferbook {
       CheckReserves( $itemnumber );
     if ( $resfound and not $ignoreRs ) {
         $resrec->{'ResFound'} = $resfound;
-
-        #         $messages->{'ResFound'} = $resrec;
+        $messages->{'ResFound'} = $resrec;
         $dotransfer = 1;
     }
 
     #actually do the transfer....
     if ($dotransfer) {
-        ModItemTransfer( $itemnumber, $fbr, $tbr );
+        ModItemTransfer( $itemnumber, $fbr, $tbr, $trigger );
 
         # don't need to update MARC anymore, we do it in batch now
         $messages->{'WasTransfered'} = 1;
@@ -374,8 +375,7 @@ sub transferbook {
 
 sub TooMany {
     my $borrower        = shift;
-    my $biblionumber = shift;
-       my $item                = shift;
+    my $item_object = shift;
     my $params = shift;
     my $onsite_checkout = $params->{onsite_checkout} || 0;
     my $switch_onsite_checkout = $params->{switch_onsite_checkout} || 0;
@@ -383,11 +383,9 @@ sub TooMany {
     my $dbh             = C4::Context->dbh;
        my $branch;
        # Get which branchcode we need
-       $branch = _GetCircControlBranch($item,$borrower);
-       my $type = (C4::Context->preference('item-level_itypes')) 
-                       ? $item->{'itype'}         # item-level
-                       : $item->{'itemtype'};     # biblio-level
+    $branch = _GetCircControlBranch($item_object->unblessed,$borrower);
+    my $type = $item_object->effective_itemtype;
+
     # given branch, patron category, and item type, determine
     # applicable issuing rule
     my $maxissueqty_rule = Koha::CirculationRules->get_effective_rule(
@@ -411,7 +409,7 @@ sub TooMany {
     # 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($maxissueqty_rule) and defined($maxissueqty_rule->rule_value)) {
+    if (defined($maxissueqty_rule) and $maxissueqty_rule->rule_value ne '') {
         my @bind_params;
         my $count_query = q|
             SELECT COUNT(*) AS total, COALESCE(SUM(onsite_checkout), 0) AS onsite_checkouts
@@ -426,18 +424,20 @@ sub TooMany {
             # specific rule
             if (C4::Context->preference('item-level_itypes')) {
                 $count_query .= " WHERE items.itype NOT IN (
-                                    SELECT itemtype FROM issuingrules
+                                    SELECT itemtype FROM circulation_rules
                                     WHERE branchcode = ?
                                     AND   (categorycode = ? OR categorycode = ?)
-                                    AND   itemtype <> '*'
+                                    AND   itemtype IS NOT NULL
+                                    AND   rule_name = 'maxissueqty'
                                   ) ";
-            } else { 
-                $count_query .= " JOIN  biblioitems USING (biblionumber) 
+            } else {
+                $count_query .= " JOIN  biblioitems USING (biblionumber)
                                   WHERE biblioitems.itemtype NOT IN (
-                                    SELECT itemtype FROM issuingrules
+                                    SELECT itemtype FROM circulation_rules
                                     WHERE branchcode = ?
                                     AND   (categorycode = ? OR categorycode = ?)
-                                    AND   itemtype <> '*'
+                                    AND   itemtype IS NOT NULL
+                                    AND   rule_name = 'maxissueqty'
                                   ) ";
             }
             push @bind_params, $maxissueqty_rule->branchcode;
@@ -458,15 +458,15 @@ sub TooMany {
         $count_query .= " AND borrowernumber = ? ";
         push @bind_params, $borrower->{'borrowernumber'};
         my $rule_branch = $maxissueqty_rule->branchcode;
-        unless ($rule_branch) {
+        if ($rule_branch) {
             if (C4::Context->preference('CircControl') eq 'PickupLibrary') {
                 $count_query .= " AND issues.branchcode = ? ";
-                push @bind_params, $branch;
+                push @bind_params, $rule_branch;
             } elsif (C4::Context->preference('CircControl') eq 'PatronLibrary') {
                 ; # if branch is the patron's home branch, then count all loans by patron
             } else {
                 $count_query .= " AND items.homebranch = ? ";
-                push @bind_params, $branch;
+                push @bind_params, $rule_branch;
             }
         }
 
@@ -475,7 +475,7 @@ sub TooMany {
         my $max_checkouts_allowed = $maxissueqty_rule ? $maxissueqty_rule->rule_value : undef;
         my $max_onsite_checkouts_allowed = $maxonsiteissueqty_rule ? $maxonsiteissueqty_rule->rule_value : undef;
 
-        if ( $onsite_checkout and defined $max_onsite_checkouts_allowed ) {
+        if ( $onsite_checkout and $max_onsite_checkouts_allowed ne '' ) {
             if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
                 return {
                     reason => 'TOO_MANY_ONSITE_CHECKOUTS',
@@ -705,12 +705,11 @@ sub CanBookBeIssued {
     if ($duedate && ref $duedate ne 'DateTime') {
         $duedate = dt_from_string($duedate);
     }
-    my $now = DateTime->now( time_zone => C4::Context->tz() );
+    my $now = dt_from_string();
     unless ( $duedate ) {
         my $issuedate = $now->clone();
 
-        my $branch = $circ_library;
-        $duedate = CalcDateDue( $issuedate, $effective_itemtype, $branch, $patron_unblessed );
+        $duedate = CalcDateDue( $issuedate, $effective_itemtype, $circ_library->branchcode, $patron_unblessed );
 
         # Offline circ calls AddIssue directly, doesn't run through here
         #  So issuingimpossible should be ok.
@@ -752,11 +751,11 @@ sub CanBookBeIssued {
         return( { STATS => 1 }, {});
     }
 
-    if ( $patron->gonenoaddress == 1 ) {
+    if ( $patron->gonenoaddress && $patron->gonenoaddress == 1 ) {
         $issuingimpossible{GNA} = 1;
     }
 
-    if ( $patron->lost == 1 ) {
+    if ( $patron->lost && $patron->lost == 1 ) {
         $issuingimpossible{CARD_LOST} = 1;
     }
     if ( $patron->is_debarred ) {
@@ -785,7 +784,7 @@ sub CanBookBeIssued {
     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 = map { $_->guarantee } $patron->guarantee_relationships();
         my $guarantees_non_issues_charges;
         foreach my $g ( @guarantees ) {
             $guarantees_non_issues_charges += $g->account->non_issues_charges;
@@ -886,11 +885,15 @@ sub CanBookBeIssued {
             $issuingimpossible{RETURN_IMPOSSIBLE} = 1;
             $issuingimpossible{branch_to_return} = $message;
         } else {
+            if ( C4::Context->preference('AutoReturnCheckedOutItems') ) {
+                $alerts{RETURNED_FROM_ANOTHER} = { patron => $patron };
+            } 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;
+            }
         }
     }
 
@@ -901,7 +904,7 @@ sub CanBookBeIssued {
       and $issue
       and $issue->onsite_checkout
       and $issue->borrowernumber == $patron->borrowernumber ? 1 : 0 );
-    my $toomany = TooMany( $patron_unblessed, $item_object->biblionumber, $item_unblessed, { onsite_checkout => $onsite_checkout, switch_onsite_checkout => $switch_onsite_checkout, } );
+    my $toomany = TooMany( $patron_unblessed, $item_object, { 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 ) {
@@ -958,7 +961,7 @@ sub CanBookBeIssued {
         }
         else {
             my $itemtype = Koha::ItemTypes->find($biblioitem->itemtype);
-            if ( $itemtype and $itemtype->notforloan == 1){
+            if ( $itemtype && defined $itemtype->notforloan && $itemtype->notforloan == 1){
                 if (!C4::Context->preference("AllowNotForLoanOverride")) {
                     $issuingimpossible{NOT_FOR_LOAN} = 1;
                     $issuingimpossible{itemtype_notforloan} = $effective_itemtype;
@@ -996,16 +999,23 @@ sub CanBookBeIssued {
               if ( $patron->branchcode ne $userenv->{branch} );
         }
     }
+
     #
     # CHECK IF THERE IS RENTAL CHARGES. RENTAL MUST BE CONFIRMED BY THE BORROWER
     #
     my $rentalConfirmation = C4::Context->preference("RentalFeesCheckoutConfirmation");
-
-    if ( $rentalConfirmation ){
+    if ($rentalConfirmation) {
         my ($rentalCharge) = GetIssuingCharges( $item_object->itemnumber, $patron->borrowernumber );
-        my $itemtype = Koha::ItemTypes->find( $item_object->itype ); # GetItem sets effective itemtype
-        $rentalCharge += $fees->accumulate_rentalcharge({ from => dt_from_string(), to => $duedate });
-        if ( $rentalCharge > 0 ){
+
+        my $itemtype_object = Koha::ItemTypes->find( $item_object->effective_itemtype );
+        if ($itemtype_object) {
+            my $accumulate_charge = $fees->accumulate_rentalcharge();
+            if ( $accumulate_charge > 0 ) {
+                $rentalCharge += $accumulate_charge;
+            }
+        }
+
+        if ( $rentalCharge > 0 ) {
             $needsconfirmation{RENTALCHARGE} = $rentalCharge;
         }
     }
@@ -1170,7 +1180,7 @@ sub CanBookBeReturned {
 
 sub checkHighHolds {
     my ( $item, $borrower ) = @_;
-    my $branch = _GetCircControlBranch( $item, $borrower );
+    my $branchcode = _GetCircControlBranch( $item, $borrower );
     my $item_object = Koha::Items->find( $item->{itemnumber} );
 
     my $return_data = {
@@ -1205,7 +1215,7 @@ sub checkHighHolds {
             # dynamic means X more than the number of holdable items on the record
 
             # let's get the items
-            my @items = $holds->next()->biblio()->items();
+            my @items = $holds->next()->biblio()->items()->as_list;
 
             # Remove any items with status defined to be ignored even if the would not make item unholdable
             foreach my $status (@decreaseLoanHighHoldsIgnoreStatuses) {
@@ -1226,12 +1236,12 @@ sub checkHighHolds {
             }
         }
 
-        my $issuedate = DateTime->now( time_zone => C4::Context->tz() );
+        my $issuedate = dt_from_string();
 
-        my $calendar = Koha::Calendar->new( branchcode => $branch );
+        my $calendar = Koha::Calendar->new( branchcode => $branchcode );
 
         my $itype = $item_object->effective_itemtype;
-        my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branch, $borrower );
+        my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branchcode, $borrower );
 
         my $decreaseLoanHighHoldsDuration = C4::Context->preference('decreaseLoanHighHoldsDuration');
 
@@ -1305,7 +1315,7 @@ sub AddIssue {
 
     # $issuedate defaults to today.
     if ( !defined $issuedate ) {
-        $issuedate = DateTime->now( time_zone => C4::Context->tz() );
+        $issuedate = dt_from_string();
     }
     else {
         if ( ref $issuedate ne 'DateTime' ) {
@@ -1321,7 +1331,7 @@ sub AddIssue {
           or return;    # if we don't get an Item, abort.
         my $item_unblessed = $item_object->unblessed;
 
-        my $branch = _GetCircControlBranch( $item_unblessed, $borrower );
+        my $branchcode = _GetCircControlBranch( $item_unblessed, $borrower );
 
         # get actual issuing if there is one
         my $actualissue = $item_object->checkout;
@@ -1332,7 +1342,7 @@ sub AddIssue {
             $datedue = AddRenewal(
                 $borrower->{'borrowernumber'},
                 $item_object->itemnumber,
-                $branch,
+                $branchcode,
                 $datedue,
                 $issuedate,    # here interpreted as the renewal date
             );
@@ -1340,13 +1350,13 @@ sub AddIssue {
         else {
             unless ($datedue) {
                 my $itype = $item_object->effective_itemtype;
-                $datedue = CalcDateDue( $issuedate, $itype, $branch, $borrower );
+                $datedue = CalcDateDue( $issuedate, $itype, $branchcode, $borrower );
 
             }
             $datedue->truncate( to => 'minute' );
 
             my $patron = Koha::Patrons->find( $borrower );
-            my $library = Koha::Libraries->find( $branch );
+            my $library = Koha::Libraries->find( $branchcode );
             my $fees = Koha::Charges::Fees->new(
                 {
                     patron    => $patron,
@@ -1384,20 +1394,22 @@ sub AddIssue {
 
             # 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},
+                my $rule = Koha::CirculationRules->get_effective_rule(
+                    {
+                        categorycode => $borrower->{categorycode},
                         itemtype     => $item_object->effective_itemtype,
-                        branchcode   => $branch
+                        branchcode   => $branchcode,
+                        rule_name    => 'auto_renew'
                     }
                 );
 
-                $auto_renew = $issuing_rule->auto_renew if $issuing_rule;
+                $auto_renew = $rule->rule_value if $rule;
             }
 
             # 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 );
+                $datedue = CalcDateDue( $issuedate, $itype, $branchcode, $borrower );
 
             }
             $datedue->truncate( to => 'minute' );
@@ -1423,9 +1435,9 @@ sub AddIssue {
                     }
                 )->store;
             }
-
-            if ( C4::Context->preference('ReturnToShelvingCart') ) {
-                # ReturnToShelvingCart is on, anything issued should be taken off the cart.
+            if ( $item_object->location && $item_object->location eq 'CART'
+                && ( !$item_object->permanent_location || $item_object->permanent_location ne 'CART' ) ) {
+            ## Item was moved to cart via UpdateItemLocationOnCheckin, anything issued should be taken off the cart.
                 CartToShelf( $item_object->itemnumber );
             }
 
@@ -1445,37 +1457,30 @@ sub AddIssue {
                     )
                   )
                 {
-                    _FixAccountForLostAndReturned( $item_object->itemnumber, undef,
+                    _FixAccountForLostAndFound( $item_object->itemnumber, undef,
                         $item_object->barcode );
                 }
             }
 
-            ModItem(
-                {
-                    issues        => $item_object->issues + 1,
-                    holdingbranch => C4::Context->userenv->{'branch'},
-                    itemlost      => 0,
-                    onloan        => $datedue->ymd(),
-                    datelastborrowed => DateTime->now( time_zone => C4::Context->tz() )->ymd(),
-                },
-                $item_object->biblionumber,
-                $item_object->itemnumber,
-                { log_action => 0 }
-            );
+            $item_object->issues( ( $item_object->issues || 0 ) + 1);
+            $item_object->holdingbranch(C4::Context->userenv->{'branch'});
+            $item_object->itemlost(0);
+            $item_object->onloan($datedue->ymd());
+            $item_object->datelastborrowed( dt_from_string()->ymd() );
+            $item_object->store({log_action => 0});
             ModDateLastSeen( $item_object->itemnumber );
 
             # If it costs to borrow this book, charge it to the patron's account.
             my ( $charge, $itemtype ) = GetIssuingCharges( $item_object->itemnumber, $borrower->{'borrowernumber'} );
-            if ( $charge > 0 ) {
-                my $description = "Rental";
-                AddIssuingCharge( $issue, $charge, $description );
+            if ( $charge && $charge > 0 ) {
+                AddIssuingCharge( $issue, $charge, 'RENT' );
             }
 
             my $itemtype_object = Koha::ItemTypes->find( $item_object->effective_itemtype );
             if ( $itemtype_object ) {
                 my $accumulate_charge = $fees->accumulate_rentalcharge();
                 if ( $accumulate_charge > 0 ) {
-                    AddIssuingCharge( $issue, $accumulate_charge, 'Daily rental' ) if $accumulate_charge > 0;
+                    AddIssuingCharge( $issue, $accumulate_charge, 'RENT_DAILY' );
                     $charge += $accumulate_charge;
                     $item_unblessed->{charge} = $charge;
                 }
@@ -1499,7 +1504,7 @@ sub AddIssue {
             # Send a checkout slip.
             my $circulation_alert = 'C4::ItemCirculationAlertPreference';
             my %conditions        = (
-                branchcode   => $branch,
+                branchcode   => $branchcode,
                 categorycode => $borrower->{categorycode},
                 item_type    => $item_object->effective_itemtype,
                 notification => 'CHECKOUT',
@@ -1510,7 +1515,7 @@ sub AddIssue {
                         type     => 'CHECKOUT',
                         item     => $item_object->unblessed,
                         borrower => $borrower,
-                        branch   => $branch,
+                        branch   => $branchcode,
                     }
                 );
             }
@@ -1533,67 +1538,77 @@ Get loan length for an itemtype, a borrower type and a branch
 =cut
 
 sub GetLoanLength {
-    my ( $borrowertype, $itemtype, $branchcode ) = @_;
-    my $dbh = C4::Context->dbh;
-    my $sth = $dbh->prepare(qq{
-        SELECT issuelength, lengthunit, renewalperiod
-        FROM issuingrules
-        WHERE   categorycode=?
-            AND itemtype=?
-            AND branchcode=?
-            AND issuelength IS NOT NULL
-    });
+    my ( $categorycode, $itemtype, $branchcode ) = @_;
+
+    # Set search precedences
+    my @params = (
+        {
+            categorycode => $categorycode,
+            itemtype     => $itemtype,
+            branchcode   => $branchcode,
+        },
+        {
+            categorycode => $categorycode,
+            itemtype     => undef,
+            branchcode   => $branchcode,
+        },
+        {
+            categorycode => undef,
+            itemtype     => $itemtype,
+            branchcode   => $branchcode,
+        },
+        {
+            categorycode => undef,
+            itemtype     => undef,
+            branchcode   => $branchcode,
+        },
+        {
+            categorycode => $categorycode,
+            itemtype     => $itemtype,
+            branchcode   => undef,
+        },
+        {
+            categorycode => $categorycode,
+            itemtype     => undef,
+            branchcode   => undef,
+        },
+        {
+            categorycode => undef,
+            itemtype     => $itemtype,
+            branchcode   => undef,
+        },
+        {
+            categorycode => undef,
+            itemtype     => undef,
+            branchcode   => undef,
+        },
+    );
 
-    # try to find issuelength & return the 1st available.
-    # check with borrowertype, itemtype and branchcode, then without one of those parameters
-    $sth->execute( $borrowertype, $itemtype, $branchcode );
-    my $loanlength = $sth->fetchrow_hashref;
-
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    $sth->execute( $borrowertype, '*', $branchcode );
-    $loanlength = $sth->fetchrow_hashref;
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    $sth->execute( '*', $itemtype, $branchcode );
-    $loanlength = $sth->fetchrow_hashref;
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    $sth->execute( '*', '*', $branchcode );
-    $loanlength = $sth->fetchrow_hashref;
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    $sth->execute( $borrowertype, $itemtype, '*' );
-    $loanlength = $sth->fetchrow_hashref;
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    $sth->execute( $borrowertype, '*', '*' );
-    $loanlength = $sth->fetchrow_hashref;
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    $sth->execute( '*', $itemtype, '*' );
-    $loanlength = $sth->fetchrow_hashref;
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    $sth->execute( '*', '*', '*' );
-    $loanlength = $sth->fetchrow_hashref;
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    # if no rule is set => 0 day (hardcoded)
-    return {
-        issuelength => 0,
+    # Initialize default values
+    my $rules = {
+        issuelength   => 0,
         renewalperiod => 0,
-        lengthunit => 'days',
+        lengthunit    => 'days',
     };
 
+    # Search for rules!
+    foreach my $rule_name (qw( issuelength renewalperiod lengthunit )) {
+        foreach my $params (@params) {
+            my $rule = Koha::CirculationRules->search(
+                {
+                    rule_name => $rule_name,
+                    %$params,
+                }
+            )->next();
+
+            if ($rule) {
+                $rules->{$rule_name} = $rule->rule_value;
+                last;
+            }
+        }
+    }
+
+    return $rules;
 }
 
 
@@ -1608,19 +1623,21 @@ Get the Hard Due Date and it's comparison for an itemtype, a borrower type and a
 sub GetHardDueDate {
     my ( $borrowertype, $itemtype, $branchcode ) = @_;
 
-    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
-        {   categorycode => $borrowertype,
+    my $rules = Koha::CirculationRules->get_effective_rules(
+        {
+            categorycode => $borrowertype,
             itemtype     => $itemtype,
-            branchcode   => $branchcode
+            branchcode   => $branchcode,
+            rules        => [ 'hardduedate', 'hardduedatecompare' ],
         }
     );
 
-
-    if ( defined( $issuing_rule ) ) {
-        if ( $issuing_rule->hardduedate ) {
-            return (dt_from_string($issuing_rule->hardduedate, 'iso'),$issuing_rule->hardduedatecompare);
-        else {
-            return (undef, undef);
+    if ( defined( $rules->{hardduedate} ) ) {
+        if ( $rules->{hardduedate} ) {
+            return ( dt_from_string( $rules->{hardduedate}, 'iso' ), $rules->{hardduedatecompare} );
+        }
+        else {
+            return ( undef, undef );
         }
     }
 }
@@ -1717,43 +1734,43 @@ Neither C<$branchcode> nor C<$itemtype> should be '*'.
 
 sub GetBranchItemRule {
     my ( $branchcode, $itemtype ) = @_;
-    my $dbh = C4::Context->dbh();
-    my $result = {};
-
-    my @attempts = (
-        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
-            FROM branch_item_rules
-            WHERE branchcode = ?
-              AND itemtype = ?', $branchcode, $itemtype],
-        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
-            FROM default_branch_circ_rules
-            WHERE branchcode = ?', $branchcode],
-        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
-            FROM default_branch_item_rules
-            WHERE itemtype = ?', $itemtype],
-        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
-            FROM default_circ_rules'],
+
+    # Search for rules!
+    my $holdallowed_rule = Koha::CirculationRules->get_effective_rule(
+        {
+            branchcode => $branchcode,
+            itemtype => $itemtype,
+            rule_name => 'holdallowed',
+        }
+    );
+    my $hold_fulfillment_policy_rule = Koha::CirculationRules->get_effective_rule(
+        {
+            branchcode => $branchcode,
+            itemtype => $itemtype,
+            rule_name => 'hold_fulfillment_policy',
+        }
+    );
+    my $returnbranch_rule = Koha::CirculationRules->get_effective_rule(
+        {
+            branchcode => $branchcode,
+            itemtype => $itemtype,
+            rule_name => 'returnbranch',
+        }
     );
 
-    foreach my $attempt (@attempts) {
-        my ($query, @bind_params) = @{$attempt};
-        my $search_result = $dbh->selectrow_hashref ( $query , {}, @bind_params )
-          or next;
-
-        # Since branch/category and branch/itemtype use the same per-branch
-        # defaults tables, we have to check that the key we want is set, not
-        # just that a row was returned
-        $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'} );
+    my $rules;
+    $rules->{holdallowed} = defined $holdallowed_rule
+        ? $holdallowed_rule->rule_value
+        : 2;
+    $rules->{hold_fulfillment_policy} = defined $hold_fulfillment_policy_rule
+        ? $hold_fulfillment_policy_rule->rule_value
+        : 'any';
+    $rules->{returnbranch} = defined $returnbranch_rule
+        ? $returnbranch_rule->rule_value
+        : 'homebranch';
 
-    return $result;
+    return $rules;
 }
 
 =head2 AddReturn
@@ -1837,6 +1854,7 @@ sub AddReturn {
         undef $branch;
     }
     $branch = C4::Context->userenv->{'branch'} unless $branch;  # we trust userenv to be a safe fallback/default
+    my $return_date_specified = !!$return_date;
     $return_date //= dt_from_string();
     my $messages;
     my $patron;
@@ -1860,7 +1878,8 @@ sub AddReturn {
                 . Dumper($issue->unblessed) . "\n";
     } else {
         $messages->{'NotIssued'} = $barcode;
-        ModItem({ onloan => undef }, $item->biblionumber, $item->itemnumber) if defined $item->onloan;
+        $item->onloan(undef)->store if defined $item->onloan;
+
         # even though item is not on loan, it may still be transferred;  therefore, get current branch info
         $doreturn = 0;
         # No issue, no borrowernumber.  ONLY if $doreturn, *might* you have a $borrower later.
@@ -1871,27 +1890,40 @@ sub AddReturn {
         }
     }
 
-    my $item_unblessed = $item->unblessed;
-    if ( $item->location eq 'PROC' ) {
-        if ( C4::Context->preference("InProcessingToShelvingCart") ) {
-            $item_unblessed->{location} = 'CART';
-        }
-        else {
-            $item_unblessed->{location} = $item->permanent_location;
-        }
-
-        ModItem( $item_unblessed, $item->biblionumber, $item->itemnumber, { log_action => 0 } );
-    }
-
         # full item data, but no borrowernumber or checkout info (no issue)
     my $hbr = GetBranchItemRule($item->homebranch, $itemtype)->{'returnbranch'} || "homebranch";
         # get the proper branch to which to return the item
     my $returnbranch = $hbr ne 'noreturn' ? $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 $transfer_trigger = $hbr eq 'homebranch' ? 'ReturnToHome' : $hbr eq 'holdingbranch' ? 'ReturnToHolding' : undef;
 
     my $borrowernumber = $patron ? $patron->borrowernumber : undef;    # we don't know if we had a borrower or not
     my $patron_unblessed = $patron ? $patron->unblessed : {};
 
+    my $update_loc_rules = get_yaml_pref_hash('UpdateItemLocationOnCheckin');
+    map { $update_loc_rules->{$_} = $update_loc_rules->{$_}[0] } keys %$update_loc_rules; #We can only move to one location so we flatten the arrays
+    if ($update_loc_rules) {
+        if (defined $update_loc_rules->{_ALL_}) {
+            if ($update_loc_rules->{_ALL_} eq '_PERM_') { $update_loc_rules->{_ALL_} = $item->permanent_location; }
+            if ($update_loc_rules->{_ALL_} eq '_BLANK_') { $update_loc_rules->{_ALL_} = ''; }
+            if ( $item->location ne $update_loc_rules->{_ALL_}) {
+                $messages->{'ItemLocationUpdated'} = { from => $item->location, to => $update_loc_rules->{_ALL_} };
+                $item->location($update_loc_rules->{_ALL_})->store;
+            }
+        }
+        else {
+            foreach my $key ( keys %$update_loc_rules ) {
+                if ( $update_loc_rules->{$key} eq '_PERM_' ) { $update_loc_rules->{$key} = $item->permanent_location; }
+                if ( $update_loc_rules->{$key} eq '_BLANK_') { $update_loc_rules->{$key} = '' ;}
+                if ( ($item->location eq $key && $item->location ne $update_loc_rules->{$key}) || ($key eq '_BLANK_' && $item->location eq '' && $update_loc_rules->{$key} ne '') ) {
+                    $messages->{'ItemLocationUpdated'} = { from => $item->location, to => $update_loc_rules->{$key} };
+                    $item->location($update_loc_rules->{$key})->store;
+                    last;
+                }
+            }
+        }
+    }
+
     my $yaml = C4::Context->preference('UpdateNotForLoanStatusOnCheckin');
     if ($yaml) {
         $yaml = "$yaml\n\n";  # YAML is anal on ending \n. Surplus does not hurt
@@ -1904,7 +1936,7 @@ sub AddReturn {
             foreach my $key ( keys %$rules ) {
                 if ( $item->notforloan eq $key ) {
                     $messages->{'NotForLoanStatusUpdated'} = { from => $item->notforloan, to => $rules->{$key} };
-                    ModItem( { notforloan => $rules->{$key} }, undef, $itemnumber, { log_action => 0 } );
+                    $item->notforloan($rules->{$key})->store({ log_action => 0 });
                     last;
                 }
             }
@@ -1912,7 +1944,7 @@ sub AddReturn {
     }
 
     # check if the return is allowed at this branch
-    my ($returnallowed, $message) = CanBookBeReturned($item_unblessed, $branch);
+    my ($returnallowed, $message) = CanBookBeReturned($item->unblessed, $branch);
     unless ($returnallowed){
         $messages->{'Wrongbranch'} = {
             Wrongbranch => $branch,
@@ -1933,18 +1965,23 @@ sub AddReturn {
 
     # case of a return of document (deal with issues and holdingbranch)
     if ($doreturn) {
-        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";
-        $is_overdue = $issue->is_overdue( $return_date );
 
         if ($patron) {
             eval {
                 MarkIssueReturned( $borrowernumber, $item->itemnumber, $return_date, $patron->privacy );
             };
             unless ( $@ ) {
-                if ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue ) {
-                    _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed, return_date => $return_date } );
+                if (
+                    (
+                        C4::Context->preference('CalculateFinesOnReturn')
+                        || ( $return_date_specified && C4::Context->preference('CalculateFinesOnBackdate') )
+                    )
+                    && !$item->itemlost
+                  )
+                {
+                    _CalculateAndUpdateFine( { issue => $issue, item => $item->unblessed, 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 );
@@ -1957,15 +1994,14 @@ sub AddReturn {
 
         }
 
-        ModItem( { onloan => undef }, $item->biblionumber, $item->itemnumber, { log_action => 0 } );
+        $item->onloan(undef)->store({ log_action => 0 });
     }
 
     # 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_unblessed->{'holdingbranch'} = $branch; # update item data holdingbranch too # FIXME I guess this is for the _debar_user_on_return call later
+        $item->holdingbranch($branch)->store;
     }
 
     my $leave_item_lost = C4::Context->preference("BlockReturnOfLostItems") ? 1 : 0;
@@ -1983,15 +2019,12 @@ sub AddReturn {
             );
             $sth->execute( $item->itemnumber );
             # if we have a reservation with valid transfer, we can set it's status to 'W'
-            ShelfToCart( $item->itemnumber ) if ( C4::Context->preference("ReturnToShelvingCart") );
             C4::Reserves::ModReserveStatus($item->itemnumber, 'W');
         } else {
             $messages->{'WrongTransfer'}     = $tobranch;
             $messages->{'WrongTransferItem'} = $item->itemnumber;
         }
         $validTransfert = 1;
-    } else {
-        ShelfToCart( $item->itemnumber ) if ( C4::Context->preference("ReturnToShelvingCart") );
     }
 
     # fix up the accounts.....
@@ -2008,7 +2041,7 @@ sub AddReturn {
                 )
               )
             {
-                _FixAccountForLostAndReturned( $item->itemnumber,
+                _FixAccountForLostAndFound( $item->itemnumber,
                     $borrowernumber, $barcode );
                 $messages->{'LostItemFeeRefunded'} = 1;
             }
@@ -2017,12 +2050,12 @@ sub AddReturn {
 
     # fix up the overdues in accounts...
     if ($borrowernumber) {
-        my $fix = _FixOverduesOnReturn( $borrowernumber, $item->itemnumber, $exemptfine );
+        my $fix = _FixOverduesOnReturn( $borrowernumber, $item->itemnumber, $exemptfine, 'RETURNED' );
         defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $item->itemnumber...) failed!";  # zero is OK, check defined
 
         if ( $issue and $issue->is_overdue ) {
         # fix fine days
-            my ($debardate,$reminder) = _debar_user_on_return( $patron_unblessed, $item_unblessed, dt_from_string($issue->date_due), $return_date );
+            my ($debardate,$reminder) = _debar_user_on_return( $patron_unblessed, $item->unblessed, dt_from_string($issue->date_due), $return_date );
             if ($reminder){
                 $messages->{'PrevDebarred'} = $debardate;
             } else {
@@ -2044,10 +2077,16 @@ sub AddReturn {
     }
 
     # find reserves.....
-    # if we don't have a reserve with the status W, we launch the Checkreserves routine
+    # launch the Checkreserves routine to find any holds
     my ($resfound, $resrec);
     my $lookahead= C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds
     ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->itemnumber, undef, $lookahead ) unless ( $item->withdrawn );
+    # if a hold is found and is waiting at another branch, change the priority back to 1 and trigger the hold (this will trigger a transfer and update the hold status properly)
+    if ( $resfound eq "Waiting" and $branch ne $resrec->{branchcode} ) {
+        my $hold = C4::Reserves::RevertWaitingStatus( { itemnumber => $item->itemnumber } );
+        $resfound = 'Reserved';
+        $resrec = $hold->unblessed;
+    }
     if ($resfound) {
           $resrec->{'ResFound'} = $resfound;
         $messages->{'ResFound'} = $resrec;
@@ -2075,7 +2114,7 @@ sub AddReturn {
         if ($doreturn && $circulation_alert->is_enabled_for(\%conditions)) {
             SendCirculationAlert({
                 type     => 'CHECKIN',
-                item     => $item_unblessed,
+                item     => $item->unblessed,
                 borrower => $patron->unblessed,
                 branch   => $branch,
             });
@@ -2095,19 +2134,42 @@ sub AddReturn {
         DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
     }
 
+    # Check if this item belongs to a biblio record that is attached to an
+    # ILL request, if it is we need to update the ILL request's status
+    if (C4::Context->preference('CirculateILL')) {
+        my $request = Koha::Illrequests->find(
+            { biblio_id => $item->biblio->biblionumber }
+        );
+        $request->status('RET') if $request;
+    }
+
     # 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'}){
-        my $BranchTransferLimitsType = C4::Context->preference("BranchTransferLimitsType");
+        my $BranchTransferLimitsType = C4::Context->preference("BranchTransferLimitsType") eq 'itemtype' ? 'effective_itemtype' : 'ccode';
         if  (C4::Context->preference("AutomaticItemReturn"    ) or
             (C4::Context->preference("UseBranchTransferLimits") and
              ! IsBranchTransferAllowed($branch, $returnbranch, $item->$BranchTransferLimitsType )
            )) {
-            $debug and warn sprintf "about to call ModItemTransfer(%s, %s, %s)", $item->itemnumber,$branch, $returnbranch;
-            $debug and warn "item: " . Dumper($item_unblessed);
-            ModItemTransfer($item->itemnumber, $branch, $returnbranch);
+            $debug and warn sprintf "about to call ModItemTransfer(%s, %s, %s, %s)", $item->itemnumber,$branch, $returnbranch, $transfer_trigger;
+            $debug and warn "item: " . Dumper($item->unblessed);
+            ModItemTransfer($item->itemnumber, $branch, $returnbranch, $transfer_trigger);
             $messages->{'WasTransfered'} = 1;
         } else {
             $messages->{'NeedsTransfer'} = $returnbranch;
+            $messages->{'TransferTrigger'} = $transfer_trigger;
+        }
+    }
+
+    if ( C4::Context->preference('ClaimReturnedLostValue') ) {
+        my $claims = Koha::Checkouts::ReturnClaims->search(
+           {
+               itemnumber => $item->id,
+               resolution => undef,
+           }
+        );
+
+        if ( $claims->count ) {
+            $messages->{ReturnClaims} = $claims;
         }
     }
 
@@ -2139,10 +2201,13 @@ sub MarkIssueReturned {
 
     # Retrieve the issue
     my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return;
+
+    return unless $issue->borrowernumber == $borrowernumber; # If the item is checked out to another patron we do not return it
+
     my $issue_id = $issue->issue_id;
 
     my $anonymouspatron;
-    if ( $privacy == 2 ) {
+    if ( $privacy && $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
@@ -2169,14 +2234,14 @@ sub MarkIssueReturned {
         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) {
+        if ( $privacy && $privacy == 2) {
             $old_checkout->borrowernumber($anonymouspatron)->store;
         }
 
         # And finally delete the issue
         $issue->delete;
 
-        ModItem( { 'onloan' => undef }, undef, $itemnumber, { log_action => 0 } );
+        $issue->item->onloan(undef)->store({ log_action => 0 });
 
         if ( C4::Context->preference('StoreLastBorrower') ) {
             my $item = Koha::Items->find( $itemnumber );
@@ -2205,103 +2270,122 @@ Internal function, called only by AddReturn that calculates and updates
 
 Should only be called for overdue returns
 
+Calculation of the debarment date has been moved to a separate subroutine _calculate_new_debar_dt
+to ease testing.
+
 =cut
 
-sub _debar_user_on_return {
+sub _calculate_new_debar_dt {
     my ( $borrower, $item, $dt_due, $return_date ) = @_;
 
     my $branchcode = _GetCircControlBranch( $item, $borrower );
-    $return_date //= dt_from_string();
-
     my $circcontrol = C4::Context->preference('CircControl');
-    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
+    my $issuing_rule = Koha::CirculationRules->get_effective_rules(
         {   categorycode => $borrower->{categorycode},
             itemtype     => $item->{itype},
-            branchcode   => $branchcode
+            branchcode   => $branchcode,
+            rules => [
+                'finedays',
+                'lengthunit',
+                'firstremind',
+                'maxsuspensiondays',
+                'suspension_chargeperiod',
+            ]
         }
     );
-    my $finedays = $issuing_rule ? $issuing_rule->finedays : undef;
-    my $unit     = $issuing_rule ? $issuing_rule->lengthunit : undef;
+    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) {
+    return unless $finedays;
 
-        # finedays is in days, so hourly loans must multiply by 24
-        # thus 1 hour late equals 1 day suspension * finedays rate
-        $finedays = $finedays * 24 if ( $unit eq 'hours' );
+    # finedays is in days, so hourly loans must multiply by 24
+    # thus 1 hour late equals 1 day suspension * finedays rate
+    $finedays = $finedays * 24 if ( $unit eq 'hours' );
 
-        # grace period is measured in the same units as the loan
-        my $grace =
-          DateTime::Duration->new( $unit => $issuing_rule->firstremind );
+    # grace period is measured in the same units as the loan
+    my $grace =
+      DateTime::Duration->new( $unit => $issuing_rule->{firstremind} );
 
-        my $deltadays = DateTime::Duration->new(
-            days => $chargeable_units
-        );
-        if ( $deltadays->subtract($grace)->is_positive() ) {
-            my $suspension_days = $deltadays * $finedays;
-
-            # If the max suspension days is < than the suspension days
-            # the suspension days is limited to this maximum period.
-            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 $deltadays = DateTime::Duration->new(
+        days => $chargeable_units
+    );
 
-            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;
-                }
-            }
+    if ( $deltadays->subtract($grace)->is_positive() ) {
+        my $suspension_days = $deltadays * $finedays;
 
-            if ( $issuing_rule->suspension_chargeperiod > 1 ) {
-                # No need to / 1 and do not consider / 0
-                $suspension_days = DateTime::Duration->new(
-                    days => floor( $suspension_days->in_units('days') / $issuing_rule->suspension_chargeperiod )
-                );
-            }
+        if ( defined $issuing_rule->{suspension_chargeperiod} && $issuing_rule->{suspension_chargeperiod} > 1 ) {
+            # No need to / 1 and do not consider / 0
+            $suspension_days = DateTime::Duration->new(
+                days => floor( $suspension_days->in_units('days') / $issuing_rule->{suspension_chargeperiod} )
+            );
+        }
 
-            my $new_debar_dt;
-            # Use the calendar or not to calculate the debarment date
-            if ( C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed' ) {
-                my $calendar = Koha::Calendar->new(
-                    branchcode => $branchcode,
-                    days_mode  => 'Calendar'
-                );
-                $new_debar_dt = $calendar->addDate( $return_date, $suspension_days );
-            }
-            else {
-                $new_debar_dt = $return_date->clone()->add_duration($suspension_days);
-            }
+        # If the max suspension days is < than the suspension days
+        # the suspension days is limited to this maximum period.
+        my $max_sd = $issuing_rule->{maxsuspensiondays};
+        if ( defined $max_sd && $max_sd ne '' ) {
+            $max_sd = DateTime::Duration->new( days => $max_sd );
+            $suspension_days = $max_sd
+              if DateTime::Duration->compare( $max_sd, $suspension_days ) < 0;
+        }
 
-            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
-            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();
+        my ( $has_been_extended );
+        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;
             }
-            # FIXME Should return a DateTime object
-            return $new_debarment_str, $is_a_reminder;
         }
+
+        my $new_debar_dt;
+        # Use the calendar or not to calculate the debarment date
+        if ( C4::Context->preference('SuspensionsCalendar') eq 'noSuspensionsWhenClosed' ) {
+            my $calendar = Koha::Calendar->new(
+                branchcode => $branchcode,
+                days_mode  => 'Calendar'
+            );
+            $new_debar_dt = $calendar->addDate( $return_date, $suspension_days );
+        }
+        else {
+            $new_debar_dt = $return_date->clone()->add_duration($suspension_days);
+        }
+        return $new_debar_dt;
     }
     return;
 }
 
+sub _debar_user_on_return {
+    my ( $borrower, $item, $dt_due, $return_date ) = @_;
+
+    $return_date //= dt_from_string();
+
+    my $new_debar_dt = _calculate_new_debar_dt ($borrower, $item, $dt_due, $return_date);
+
+    return unless $new_debar_dt;
+
+    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
+    my $patron = Koha::Patrons->find( $borrower->{borrowernumber} );
+    my ($new_debarment_str, $is_a_reminder);
+    if ( $borrower->{debarred} && $borrower->{debarred} eq $patron->is_debarred ) {
+        $is_a_reminder = 1;
+        $new_debarment_str = $borrower->{debarred};
+    } else {
+        $new_debarment_str = $new_debar_dt->ymd();
+    }
+    # FIXME Should return a DateTime object
+    return $new_debarment_str, $is_a_reminder;
+}
+
 =head2 _FixOverduesOnReturn
 
-   &_FixOverduesOnReturn($borrowernumber, $itemnumber, $exemptfine);
+   &_FixOverduesOnReturn($borrowernumber, $itemnumber, $exemptfine, $status);
 
 C<$borrowernumber> borrowernumber
 
@@ -2309,12 +2393,14 @@ C<$itemnumber> itemnumber
 
 C<$exemptfine> BOOL -- remove overdue charge associated with this issue. 
 
+C<$status> ENUM -- reason for fix [ RETURNED, RENEWED, LOST, FORGIVEN ]
+
 Internal function
 
 =cut
 
 sub _FixOverduesOnReturn {
-    my ( $borrowernumber, $item, $exemptfine ) = @_;
+    my ( $borrowernumber, $item, $exemptfine, $status ) = @_;
     unless( $borrowernumber ) {
         warn "_FixOverduesOnReturn() not supplied valid borrowernumber";
         return;
@@ -2323,55 +2409,73 @@ sub _FixOverduesOnReturn {
         warn "_FixOverduesOnReturn() not supplied valid itemnumber";
         return;
     }
+    unless( $status ) {
+        warn "_FixOverduesOnReturn() not supplied valid status";
+        return;
+    }
 
-    # check for overdue fine
-    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 $schema = Koha::Database->schema;
 
-    if ($exemptfine) {
-        my $amountoutstanding = $accountline->amountoutstanding;
+    my $result = $schema->txn_do(
+        sub {
+            # check for overdue fine
+            my $accountlines = Koha::Account::Lines->search(
+                {
+                    borrowernumber  => $borrowernumber,
+                    itemnumber      => $item,
+                    debit_type_code => 'OVERDUE',
+                    status          => 'UNRETURNED'
+                }
+            );
+            return 0 unless $accountlines->count; # no warning, there's just nothing to fix
 
-        $accountline->accounttype('FFOR');
-        $accountline->amountoutstanding(0);
+            my $accountline = $accountlines->next;
+            if ($exemptfine) {
+                my $amountoutstanding = $accountline->amountoutstanding;
 
-        Koha::Account::Offset->new(
-            {
-                debit_id => $accountline->id,
-                type => 'Forgiven',
-                amount => $amountoutstanding * -1,
+                my $account = Koha::Account->new({patron_id => $borrowernumber});
+                my $credit = $account->add_credit(
+                    {
+                        amount     => $amountoutstanding,
+                        user_id    => C4::Context->userenv ? C4::Context->userenv->{'number'} : undef,
+                        library_id => C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef,
+                        interface  => C4::Context->interface,
+                        type       => 'FORGIVEN',
+                        item_id    => $item
+                    }
+                );
+
+                $credit->apply({ debits => [ $accountline ], offset_type => 'Forgiven' });
+
+                $accountline->status('FORGIVEN');
+
+                if (C4::Context->preference("FinesLog")) {
+                    &logaction("FINES", 'MODIFY',$borrowernumber,"Overdue forgiven: item $item");
+                }
+            } else {
+                $accountline->status($status);
             }
-        )->store();
 
-        if (C4::Context->preference("FinesLog")) {
-            &logaction("FINES", 'MODIFY',$borrowernumber,"Overdue forgiven: item $item");
+            return $accountline->store();
         }
-    } else {
-        $accountline->accounttype('F');
-    }
+    );
 
-    return $accountline->store();
+    return $result;
 }
 
-=head2 _FixAccountForLostAndReturned
+=head2 _FixAccountForLostAndFound
 
-  &_FixAccountForLostAndReturned($itemnumber, [$borrowernumber, $barcode]);
+  &_FixAccountForLostAndFound($itemnumber, [$borrowernumber, $barcode]);
 
-Calculates the charge for a book lost and returned.
+Finds the most recent lost item charge for this item and refunds the borrower
+appropriatly, taking into account any payments or writeoffs already applied
+against the charge.
 
 Internal function, not exported, called only by AddReturn.
 
 =cut
 
-sub _FixAccountForLostAndReturned {
+sub _FixAccountForLostAndFound {
     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
@@ -2381,18 +2485,24 @@ sub _FixAccountForLostAndReturned {
     # check for charge made for lost book
     my $accountlines = Koha::Account::Lines->search(
         {
-            itemnumber  => $itemnumber,
-            accounttype => { -in => [ 'L', 'Rep', 'W' ] },
+            itemnumber      => $itemnumber,
+            debit_type_code => 'LOST',
+            status          => [ undef, { '<>' => 'FOUND' } ]
         },
         {
-            order_by => { -desc => [ 'date' ] }
+            order_by => { -desc => [ 'date', 'accountlines_id' ] }
         }
     );
 
     return unless $accountlines->count > 0;
     my $accountline     = $accountlines->next;
     my $total_to_refund = 0;
-    my $account = Koha::Patrons->find( $accountline->borrowernumber )->account;
+
+    return unless $accountline->borrowernumber;
+    my $patron = Koha::Patrons->find( $accountline->borrowernumber );
+    return unless $patron; # Patron has been deleted, nobody to credit the return to
+
+    my $account = $patron->account;
 
     # Use cases
     if ( $accountline->amount > $accountline->amountoutstanding ) {
@@ -2416,22 +2526,24 @@ sub _FixAccountForLostAndReturned {
     if ( $credit_total > 0 ) {
         my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;
         $credit = $account->add_credit(
-            {   amount      => $credit_total,
-                description => 'Item Returned ' . $item_id,
-                type        => 'lost_item_return',
-                library_id  => $branchcode
+            {
+                amount      => $credit_total,
+                description => 'Item found ' . $item_id,
+                type        => 'LOST_FOUND',
+                interface   => C4::Context->interface,
+                library_id  => $branchcode,
+                item_id     => $itemnumber
             }
         );
 
-        # TODO: ->apply should just accept the accountline
-        $credit->apply( { debits => $accountlines->reset } );
+        $credit->apply( { debits => [ $accountline ] } );
     }
 
-    # Manually set the accounttype
-    $accountline->discard_changes->accounttype('LR');
+    # Update the account status
+    $accountline->discard_changes->status('FOUND');
     $accountline->store;
 
-    ModItem( { paidfor => '' }, undef, $itemnumber, { log_action => 0 } );
+    $accountline->item->paidfor('')->store({ log_action => 0 });
 
     if ( defined $account and C4::Context->preference('AccountAutoReconcile') ) {
         $account->reconcile_balance;
@@ -2605,6 +2717,7 @@ sub CanBookBeRenewed {
 
     my $dbh    = C4::Context->dbh;
     my $renews = 1;
+    my $auto_renew = 0;
 
     my $item      = Koha::Items->find($itemnumber)      or return ( 0, 'no_item' );
     my $issue = $item->checkout or return ( 0, 'no_checkout' );
@@ -2613,6 +2726,115 @@ sub CanBookBeRenewed {
 
     my $patron = $issue->patron or return;
 
+    # override_limit will override anything else except on_reserve
+    unless ( $override_limit ){
+        my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed );
+        my $issuing_rule = Koha::CirculationRules->get_effective_rules(
+            {
+                categorycode => $patron->categorycode,
+                itemtype     => $item->effective_itemtype,
+                branchcode   => $branchcode,
+                rules => [
+                    'renewalsallowed',
+                    'no_auto_renewal_after',
+                    'no_auto_renewal_after_hard_limit',
+                    'lengthunit',
+                    'norenewalbefore',
+                ]
+            }
+        );
+
+        return ( 0, "too_many" )
+          if not $issuing_rule->{renewalsallowed} 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 && $patron->autorenew_checkouts ) {
+
+            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 =
+                  C4::Context->preference("OPACFineNoRenewalsIncludeCredit")
+                  ? $patron->account->balance
+                  : $patron->account->outstanding_debits->total_outstanding;
+                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} );
+
+            # 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' );
+            }
+
+            if ( $soonestrenewal > dt_from_string() )
+            {
+                return ( 0, "auto_too_soon" ) if $issue->auto_renew && $patron->autorenew_checkouts;
+                return ( 0, "too_soon" );
+            }
+            elsif ( $issue->auto_renew && $patron->autorenew_checkouts ) {
+                $auto_renew = 1;
+            }
+        }
+
+        # Fallback for automatic renewals:
+        # If norenewalbefore is undef, don't renew before due date.
+        if ( $issue->auto_renew && !$auto_renew && $patron->autorenew_checkouts ) {
+            my $now = dt_from_string;
+            if ( $now >= dt_from_string( $issue->date_due, 'sql' ) ){
+                $auto_renew = 1;
+            } else {
+                return ( 0, "auto_too_soon" );
+            }
+        }
+    }
+
     my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber);
 
     # This item can fill one or more unfilled reserve, can those unfilled reserves
@@ -2659,16 +2881,16 @@ sub CanBookBeRenewed {
             # 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 = Koha::Items->find($i)->unblessed;
-                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;
+            my %patrons;
+            ITEM: foreach my $itemnumber (@itemnumbers) {
+                my $item = Koha::Items->find( $itemnumber );
+                next if IsItemOnHoldAndFound( $itemnumber );
+                for my $borrowernumber (@borrowernumbers) {
+                    my $patron = $patrons{$borrowernumber} //= Koha::Patrons->find( $borrowernumber );
+                    next unless IsAvailableForItemLevelRequest($item, $patron);
+                    next unless CanItemBeReserved($borrowernumber,$itemnumber);
+
+                    push @reservable, $itemnumber;
                     if (@reservable >= @borrowernumbers) {
                         $resfound = 0;
                         last ITEM;
@@ -2679,101 +2901,7 @@ sub CanBookBeRenewed {
         }
     }
     return ( 0, "on_reserve" ) if $resfound;    # '' when no hold was found
-
-    return ( 1, undef ) if $override_limit;
-
-    my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed );
-    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
-        {   categorycode => $patron->categorycode,
-            itemtype     => $item->effective_itemtype,
-            branchcode   => $branchcode
-        }
-    );
-
-    return ( 0, "too_many" )
-      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 );
-
-        # 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' );
-        }
-
-        if ( $soonestrenewal > DateTime->now( time_zone => C4::Context->tz() ) )
-        {
-            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 $auto_renew && !$override_limit; # 0 if auto-renewal should not succeed
 
     return ( 1, undef );
 }
@@ -2797,6 +2925,11 @@ 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.
 
+C<$skipfinecalc> is an optional boolean. There may be circumstances where, even if the
+CalculateFinesOnReturn syspref is enabled, we don't want to calculate fines upon renew,
+for example, when we're renewing as a result of a fine being paid (see RenewAccruingItemWhenPaid
+syspref)
+
 If C<$datedue> is the empty string, C<&AddRenewal> will calculate the due date automatically
 from the book's item type.
 
@@ -2807,7 +2940,8 @@ sub AddRenewal {
     my $itemnumber      = shift or return;
     my $branch          = shift;
     my $datedue         = shift;
-    my $lastreneweddate = shift || DateTime->now(time_zone => C4::Context->tz);
+    my $lastreneweddate = shift || dt_from_string();
+    my $skipfinecalc    = shift;
 
     my $item_object   = Koha::Items->find($itemnumber) or return;
     my $biblio = $item_object->biblio;
@@ -2830,115 +2964,116 @@ sub AddRenewal {
 
     my $circ_library = Koha::Libraries->find( _GetCircControlBranch($item_unblessed, $patron_unblessed) );
 
-    if ( C4::Context->preference('CalculateFinesOnReturn') && $issue->is_overdue ) {
-        _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, 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.
-    my $itemtype = $item_object->effective_itemtype;
-    unless ($datedue) {
-
-        $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?
-                                        dt_from_string( $issue->date_due, 'sql' ) :
-                                        DateTime->now( time_zone => C4::Context->tz());
-        $datedue =  CalcDateDue($datedue, $itemtype, $circ_library, $patron_unblessed, 'is a renewal');
-    }
+    my $schema = Koha::Database->schema;
+    $schema->txn_do(sub{
 
-    my $fees = Koha::Charges::Fees->new(
-        {
-            patron    => $patron,
-            library   => $circ_library,
-            item      => $item_object,
-            from_date => dt_from_string( $issue->date_due, 'sql' ),
-            to_date   => dt_from_string($datedue),
+        if ( !$skipfinecalc && C4::Context->preference('CalculateFinesOnReturn') ) {
+            _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed } );
+        }
+        _FixOverduesOnReturn( $borrowernumber, $itemnumber, undef, 'RENEWED' );
+
+        # 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.
+        my $itemtype = $item_object->effective_itemtype;
+        unless ($datedue) {
+
+            $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?
+                                            dt_from_string( $issue->date_due, 'sql' ) :
+                                            dt_from_string();
+            $datedue =  CalcDateDue($datedue, $itemtype, $circ_library->branchcode, $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 = $issue->renewals + 1;
-    my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ?
-                            WHERE borrowernumber=? 
-                            AND itemnumber=?"
-    );
+        my $fees = Koha::Charges::Fees->new(
+            {
+                patron    => $patron,
+                library   => $circ_library,
+                item      => $item_object,
+                from_date => dt_from_string( $issue->date_due, 'sql' ),
+                to_date   => dt_from_string($datedue),
+            }
+        );
 
-    $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $lastreneweddate, $borrowernumber, $itemnumber );
+        # Update the issues record to have the new due date, and a new count
+        # of how many times it has been renewed.
+        my $renews = ( $issue->renewals || 0 ) + 1;
+        my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ?
+                                WHERE borrowernumber=?
+                                AND itemnumber=?"
+        );
 
-    # Update the renewal count on the item, and tell zebra to reindex
-    $renews = $item_object->renewals + 1;
-    ModItem( { renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item_object->biblionumber, $itemnumber, { log_action => 0 } );
+        $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $lastreneweddate, $borrowernumber, $itemnumber );
 
-    # Charge a new rental fee, if applicable
-    my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
-    if ( $charge > 0 ) {
-        my $description = "Renewal of Rental Item " . $biblio->title . " " .$item_object->barcode;
-        AddIssuingCharge($issue, $charge, $description);
-    }
+        # Update the renewal count on the item, and tell zebra to reindex
+        $renews = ( $item_object->renewals || 0 ) + 1;
+        $item_object->renewals($renews);
+        $item_object->onloan($datedue);
+        $item_object->store({ log_action => 0 });
 
-    # Charge a new accumulate rental fee, if applicable
-    my $itemtype_object = Koha::ItemTypes->find( $itemtype );
-    if ( $itemtype_object ) {
-        my $accumulate_charge = $fees->accumulate_rentalcharge();
-        if ( $accumulate_charge > 0 ) {
-            my $type_desc = "Renewal of Daily Rental Item " . $biblio->title . " $item_unblessed->{'barcode'}";
-            AddIssuingCharge( $issue, $accumulate_charge, $type_desc )
+        # Charge a new rental fee, if applicable
+        my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
+        if ( $charge > 0 ) {
+            AddIssuingCharge($issue, $charge, 'RENT_RENEW');
         }
-        $charge += $accumulate_charge;
-    }
 
-    # Send a renewal slip according to checkout alert preferencei
-    if ( C4::Context->preference('RenewalSendNotice') eq '1' ) {
-        my $circulation_alert = 'C4::ItemCirculationAlertPreference';
-        my %conditions        = (
-            branchcode   => $branch,
-            categorycode => $patron->categorycode,
-            item_type    => $itemtype,
-            notification => 'CHECKOUT',
-        );
-        if ( $circulation_alert->is_enabled_for( \%conditions ) ) {
-            SendCirculationAlert(
-                {
-                    type     => 'RENEWAL',
-                    item     => $item_unblessed,
-                    borrower => $patron->unblessed,
-                    branch   => $branch,
-                }
+        # Charge a new accumulate rental fee, if applicable
+        my $itemtype_object = Koha::ItemTypes->find( $itemtype );
+        if ( $itemtype_object ) {
+            my $accumulate_charge = $fees->accumulate_rentalcharge();
+            if ( $accumulate_charge > 0 ) {
+                AddIssuingCharge( $issue, $accumulate_charge, 'RENT_DAILY_RENEW' )
+            }
+            $charge += $accumulate_charge;
+        }
+
+        # Send a renewal slip according to checkout alert preferencei
+        if ( C4::Context->preference('RenewalSendNotice') eq '1' ) {
+            my $circulation_alert = 'C4::ItemCirculationAlertPreference';
+            my %conditions        = (
+                branchcode   => $branch,
+                categorycode => $patron->categorycode,
+                item_type    => $itemtype,
+                notification => 'CHECKOUT',
             );
+            if ( $circulation_alert->is_enabled_for( \%conditions ) ) {
+                SendCirculationAlert(
+                    {
+                        type     => 'RENEWAL',
+                        item     => $item_unblessed,
+                        borrower => $patron->unblessed,
+                        branch   => $branch,
+                    }
+                );
+            }
         }
-    }
 
-    # Remove any OVERDUES related debarment if the borrower has no overdues
-    if ( $patron
-      && $patron->is_debarred
-      && ! $patron->has_overdues
-      && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
-    ) {
-        DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
-    }
+        # Remove any OVERDUES related debarment if the borrower has no overdues
+        if ( $patron
+          && $patron->is_debarred
+          && ! $patron->has_overdues
+          && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
+        ) {
+            DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
+        }
 
-    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         => $item_object->renewal_branchcode({branch => $branch}),
+                type           => 'renew',
+                amount         => $charge,
+                itemnumber     => $itemnumber,
+                itemtype       => $itemtype,
+                location       => $item_object->location,
+                borrowernumber => $borrowernumber,
+                ccode          => $item_object->ccode,
+            }
+        );
 
-    # Add the renewal to stats
-    UpdateStats(
-        {
-            branch         => $branch,
-            type           => 'renew',
-            amount         => $charge,
-            itemnumber     => $itemnumber,
-            itemtype       => $itemtype,
-            location       => $item_object->location,
-            borrowernumber => $borrowernumber,
-            ccode          => $item_object->ccode,
-        }
-    );
+        #Log the renewal
+        logaction("CIRCULATION", "RENEWAL", $borrowernumber, $itemnumber) if C4::Context->preference("RenewalLog");
+    });
 
-    #Log the renewal
-    logaction("CIRCULATION", "RENEWAL", $borrowernumber, $itemnumber) if C4::Context->preference("RenewalLog");
     return $datedue;
 }
 
@@ -2970,14 +3105,16 @@ sub GetRenewCount {
     # $item and $borrower should be calculated
     my $branchcode = _GetCircControlBranch($item->unblessed, $patron->unblessed);
 
-    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
-        {   categorycode => $patron->categorycode,
+    my $rule = Koha::CirculationRules->get_effective_rule(
+        {
+            categorycode => $patron->categorycode,
             itemtype     => $item->effective_itemtype,
-            branchcode   => $branchcode
+            branchcode   => $branchcode,
+            rule_name    => 'renewalsallowed',
         }
     );
 
-    $renewsallowed = $issuing_rule ? $issuing_rule->renewalsallowed : 0;
+    $renewsallowed = $rule ? $rule->rule_value : 0;
     $renewsleft    = $renewsallowed - $renewcount;
     if($renewsleft < 0){ $renewsleft = 0; }
     return ( $renewcount, $renewsallowed, $renewsleft );
@@ -3015,25 +3152,29 @@ sub GetSoonestRenewDate {
       or return;
 
     my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed );
-    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
+    my $issuing_rule = Koha::CirculationRules->get_effective_rules(
         {   categorycode => $patron->categorycode,
             itemtype     => $item->effective_itemtype,
-            branchcode   => $branchcode
+            branchcode   => $branchcode,
+            rules => [
+                'norenewalbefore',
+                'lengthunit',
+            ]
         }
     );
 
     my $now = dt_from_string;
     return $now unless $issuing_rule;
 
-    if ( defined $issuing_rule->norenewalbefore
-        and $issuing_rule->norenewalbefore ne "" )
+    if ( defined $issuing_rule->{norenewalbefore}
+        and $issuing_rule->{norenewalbefore} ne "" )
     {
         my $soonestrenewal =
           dt_from_string( $itemissue->date_due )->subtract(
-            $issuing_rule->lengthunit => $issuing_rule->norenewalbefore );
+            $issuing_rule->{lengthunit} => $issuing_rule->{norenewalbefore} );
 
         if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date'
-            and $issuing_rule->lengthunit eq 'days' )
+            and $issuing_rule->{lengthunit} eq 'days' )
         {
             $soonestrenewal->truncate( to => 'day' );
         }
@@ -3074,30 +3215,36 @@ sub GetLatestAutoRenewDate {
       or return;
 
     my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed );
-    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
-        {   categorycode => $patron->categorycode,
+    my $circulation_rules = Koha::CirculationRules->get_effective_rules(
+        {
+            categorycode => $patron->categorycode,
             itemtype     => $item->effective_itemtype,
-            branchcode   => $branchcode
+            branchcode   => $branchcode,
+            rules => [
+                'no_auto_renewal_after',
+                'no_auto_renewal_after_hard_limit',
+                'lengthunit',
+            ]
         }
     );
 
-    return unless $issuing_rule;
+    return unless $circulation_rules;
     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 '' );
+      if ( not $circulation_rules->{no_auto_renewal_after}
+            or $circulation_rules->{no_auto_renewal_after} eq '' )
+      and ( not $circulation_rules->{no_auto_renewal_after_hard_limit}
+             or $circulation_rules->{no_auto_renewal_after_hard_limit} eq '' );
 
     my $maximum_renewal_date;
-    if ( $issuing_rule->no_auto_renewal_after ) {
+    if ( $circulation_rules->{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
+            $circulation_rules->{lengthunit} => $circulation_rules->{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 );
+    if ( $circulation_rules->{no_auto_renewal_after_hard_limit} ) {
+        my $dt = dt_from_string( $circulation_rules->{no_auto_renewal_after_hard_limit} );
         $maximum_renewal_date = $dt if not $maximum_renewal_date or $maximum_renewal_date > $dt;
     }
     return $maximum_renewal_date;
@@ -3144,19 +3291,10 @@ sub GetIssuingCharges {
         $item_type = $item_data->{itemtype};
         $charge    = $item_data->{rentalcharge};
         my $branch = C4::Context::mybranch();
-        my $discount_query = q|SELECT rentaldiscount,
-            issuingrules.itemtype, issuingrules.branchcode
-            FROM borrowers
-            LEFT JOIN issuingrules ON borrowers.categorycode = issuingrules.categorycode
-            WHERE borrowers.borrowernumber = ?
-            AND (issuingrules.itemtype = ? OR issuingrules.itemtype = '*')
-            AND (issuingrules.branchcode = ? OR issuingrules.branchcode = '*')|;
-        my $discount_sth = $dbh->prepare($discount_query);
-        $discount_sth->execute( $borrowernumber, $item_type, $branch );
-        my $discount_rules = $discount_sth->fetchall_arrayref({});
-        if (@{$discount_rules}) {
+        my $patron = Koha::Patrons->find( $borrowernumber );
+        my $discount = _get_discount_from_rule($patron->categorycode, $branch, $item_type);
+        if ($discount) {
             # We may have multiple rules so get the most specific
-            my $discount = _get_discount_from_rule($discount_rules, $branch, $item_type);
             $charge = ( $charge * ( 100 - $discount ) ) / 100;
         }
         if ($charge) {
@@ -3169,49 +3307,55 @@ sub GetIssuingCharges {
 
 # Select most appropriate discount rule from those returned
 sub _get_discount_from_rule {
-    my ($rules_ref, $branch, $itemtype) = @_;
-    my $discount;
+    my ($categorycode, $branchcode, $itemtype) = @_;
 
-    if (@{$rules_ref} == 1) { # only 1 applicable rule use it
-        $discount = $rules_ref->[0]->{rentaldiscount};
-        return (defined $discount) ? $discount : 0;
-    }
-    # could have up to 4 does one match $branch and $itemtype
-    my @d = grep { $_->{branchcode} eq $branch && $_->{itemtype} eq $itemtype } @{$rules_ref};
-    if (@d) {
-        $discount = $d[0]->{rentaldiscount};
-        return (defined $discount) ? $discount : 0;
-    }
-    # do we have item type + all branches
-    @d = grep { $_->{branchcode} eq q{*} && $_->{itemtype} eq $itemtype } @{$rules_ref};
-    if (@d) {
-        $discount = $d[0]->{rentaldiscount};
-        return (defined $discount) ? $discount : 0;
-    }
-    # do we all item types + this branch
-    @d = grep { $_->{branchcode} eq $branch && $_->{itemtype} eq q{*} } @{$rules_ref};
-    if (@d) {
-        $discount = $d[0]->{rentaldiscount};
-        return (defined $discount) ? $discount : 0;
-    }
-    # so all and all (surely we wont get here)
-    @d = grep { $_->{branchcode} eq q{*} && $_->{itemtype} eq q{*} } @{$rules_ref};
-    if (@d) {
-        $discount = $d[0]->{rentaldiscount};
-        return (defined $discount) ? $discount : 0;
+    # Set search precedences
+    my @params = (
+        {
+            branchcode   => $branchcode,
+            itemtype     => $itemtype,
+            categorycode => $categorycode,
+        },
+        {
+            branchcode   => undef,
+            categorycode => $categorycode,
+            itemtype     => $itemtype,
+        },
+        {
+            branchcode   => $branchcode,
+            categorycode => $categorycode,
+            itemtype     => undef,
+        },
+        {
+            branchcode   => undef,
+            categorycode => $categorycode,
+            itemtype     => undef,
+        },
+    );
+
+    foreach my $params (@params) {
+        my $rule = Koha::CirculationRules->search(
+            {
+                rule_name => 'rentaldiscount',
+                %$params,
+            }
+        )->next();
+
+        return $rule->rule_value if $rule;
     }
+
     # none of the above
     return 0;
 }
 
 =head2 AddIssuingCharge
 
-  &AddIssuingCharge( $checkout, $charge, [$description] )
+  &AddIssuingCharge( $checkout, $charge, $type )
 
 =cut
 
 sub AddIssuingCharge {
-    my ( $checkout, $charge, $description ) = @_;
+    my ( $checkout, $charge, $type ) = @_;
 
     # FIXME What if checkout does not exist?
 
@@ -3219,11 +3363,11 @@ sub AddIssuingCharge {
     my $accountline = $account->add_debit(
         {
             amount      => $charge,
-            description => $description,
             note        => undef,
-            user_id     => C4::Context->userenv ? C4::Context->userenv->{'number'} : 0,
+            user_id     => C4::Context->userenv ? C4::Context->userenv->{'number'} : undef,
             library_id  => C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef,
-            type        => 'rent',
+            interface   => C4::Context->interface,
+            type        => $type,
             item_id     => $checkout->itemnumber,
             issue_id    => $checkout->issue_id,
         }
@@ -3362,7 +3506,7 @@ sub SendCirculationAlert {
     # 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};
+    my $do_not_lock = ( exists $ENV{_} && $ENV{_} =~ m|prove| ) || $ENV{KOHA_TESTING};
 
     for my $mtt (@transports) {
         my $letter =  C4::Letters::GetPreparedLetter (
@@ -3421,20 +3565,7 @@ sub updateWrongTransfer {
        ModItemTransfer($itemNumber, $FromLibrary, $waitingAtLibrary);
 
 #third step changing holdingbranch of item
-       UpdateHoldingbranch($FromLibrary,$itemNumber);
-}
-
-=head2 UpdateHoldingbranch
-
-  $items = UpdateHoldingbranch($branch,$itmenumber);
-
-Simple methode for updating hodlingbranch in items BDD line
-
-=cut
-
-sub UpdateHoldingbranch {
-       my ( $branch,$itemnumber ) = @_;
-    ModItem({ holdingbranch => $branch }, undef, $itemnumber);
+    my $item = Koha::Items->find($itemNumber)->holdingbranch($FromLibrary)->store;
 }
 
 =head2 CalcDateDue
@@ -3472,9 +3603,7 @@ sub CalcDateDue {
             $datedue = $startdate->clone;
         }
     } else {
-        $datedue =
-          DateTime->now( time_zone => C4::Context->tz() )
-          ->truncate( to => 'minute' );
+        $datedue = dt_from_string()->truncate( to => 'minute' );
     }
 
 
@@ -3538,7 +3667,7 @@ sub CalcDateDue {
           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 );
+              $datedue = $calendar->prev_open_days( $datedue, 1 );
           }
         }
     }
@@ -3636,15 +3765,7 @@ sub DeleteBranchTransferLimits {
 
 sub ReturnLostItem{
     my ( $borrowernumber, $itemnum ) = @_;
-
     MarkIssueReturned( $borrowernumber, $itemnum );
-    my $patron = Koha::Patrons->find( $borrowernumber );
-    my $item = Koha::Items->find($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 = $patron->firstname . ' ' . $patron->surname . ' ' . $patron->cardnumber;
-    ModItem({ paidfor =>  $old_note."Paid for by $bor $date" }, undef, $itemnum);
 }
 
 
@@ -3677,11 +3798,20 @@ sub LostItem{
     if ( my $borrowernumber = $issues->{borrowernumber} ){
         my $patron = Koha::Patrons->find( $borrowernumber );
 
-        my $fix = _FixOverduesOnReturn($borrowernumber, $itemnumber, C4::Context->preference('WhenLostForgiveFine'), 0); # 1, 0 = exemptfine, no-dropbox
+        my $fix = _FixOverduesOnReturn($borrowernumber, $itemnumber, C4::Context->preference('WhenLostForgiveFine'), 'LOST');
         defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $itemnumber...) failed!";  # zero is OK, check defined
 
         if (C4::Context->preference('WhenLostChargeReplacementFee')){
-            C4::Accounts::chargelostitem($borrowernumber, $itemnumber, $issues->{'replacementprice'}, "Lost Item $issues->{'title'} $issues->{'barcode'} $issues->{'itemcallnumber'}");
+            C4::Accounts::chargelostitem(
+                $borrowernumber,
+                $itemnumber,
+                $issues->{'replacementprice'},
+                sprintf( "%s %s %s",
+                    $issues->{'title'}          || q{},
+                    $issues->{'barcode'}        || q{},
+                    $issues->{'itemcallnumber'} || q{},
+                ),
+            );
             #FIXME : Should probably have a way to distinguish this from an item that really was returned.
             #warn " $issues->{'borrowernumber'}  /  $itemnumber ";
         }
@@ -3691,7 +3821,7 @@ sub LostItem{
 
     #When item is marked lost automatically cancel its outstanding transfers and set items holdingbranch to the transfer source branch (frombranch)
     if (my ( $datesent,$frombranch,$tobranch ) = GetTransfers($itemnumber)) {
-        ModItem({holdingbranch => $frombranch}, undef, $itemnumber);
+        Koha::Items->find($itemnumber)->holdingbranch($frombranch)->store;
     }
     my $transferdeleted = DeleteTransfer($itemnumber);
 }
@@ -3754,17 +3884,16 @@ sub ProcessOfflineReturn {
         my $itemnumber = $item->itemnumber;
         my $issue = GetOpenIssue( $itemnumber );
         if ( $issue ) {
+            my $leave_item_lost = C4::Context->preference("BlockReturnOfLostItems") ? 1 : 0;
+            ModDateLastSeen( $itemnumber, $leave_item_lost );
             MarkIssueReturned(
                 $issue->{borrowernumber},
                 $itemnumber,
                 $operation->{timestamp},
             );
-            ModItem(
-                { renewals => 0, onloan => undef },
-                $issue->{'biblionumber'},
-                $itemnumber,
-                { log_action => 0 }
-            );
+            $item->renewals(0);
+            $item->onloan(undef);
+            $item->store({ log_action => 0 });
             return "Success.";
         } else {
             return "Item not issued.";
@@ -3813,7 +3942,13 @@ sub ProcessOfflinePayment {
 
     my $patron = Koha::Patrons->find({ cardnumber => $operation->{cardnumber} });
 
-    $patron->account->pay({ amount => $operation->{amount}, library_id => $operation->{branchcode} });
+    $patron->account->pay(
+        {
+            amount     => $operation->{amount},
+            library_id => $operation->{branchcode},
+            interface  => 'koc'
+        }
+    );
 
     return "Success.";
 }
@@ -3910,6 +4045,7 @@ sub GetAgeRestriction {
     my ($record_restrictions, $borrower) = @_;
     my $markers = C4::Context->preference('AgeRestrictionMarker');
 
+    return unless $record_restrictions;
     # Split $record_restrictions to something like FSK 16 or PEGI 6
     my @values = split ' ', uc($record_restrictions);
     return unless @values;
@@ -3953,7 +4089,7 @@ sub GetAgeRestriction {
             }
 
             #Get how many days the borrower has to reach the age restriction
-            my @Today = split /-/, DateTime->today->ymd();
+            my @Today = split /-/, dt_from_string()->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);