Bug 17600: Standardize our EXPORT_OK
[srvgit] / C4 / Circulation.pm
index 09389ec..0549230 100644 (file)
@@ -21,115 +21,108 @@ package C4::Circulation;
 use Modern::Perl;
 use DateTime;
 use POSIX qw( floor );
-use Koha::DateUtils;
+use YAML::XS;
+use Encode;
+
+use Koha::DateUtils qw( dt_from_string output_pref );
 use C4::Context;
-use C4::Stats;
-use C4::Reserves;
-use C4::Biblio;
-use C4::Items;
-use C4::Members;
+use C4::Stats qw( UpdateStats );
+use C4::Reserves qw( CheckReserves CanItemBeReserved MoveReserve ModReserve ModReserveMinusPriority RevertWaitingStatus IsItemOnHoldAndFound IsAvailableForItemLevelRequest );
+use C4::Biblio qw( UpdateTotalIssues );
+use C4::Items qw( ModItemTransfer ModDateLastSeen CartToShelf );
 use C4::Accounts;
 use C4::ItemCirculationAlertPreference;
 use C4::Message;
-use C4::Debug;
-use C4::Log; # logaction
-use C4::Overdues qw(CalcFine UpdateFine get_chargeable_units);
+use C4::Log qw( logaction ); # logaction
+use C4::Overdues;
 use C4::RotatingCollections qw(GetCollectionItemBranches);
-use Algorithm::CheckDigits;
+use Algorithm::CheckDigits qw( CheckDigits );
 
-use Data::Dumper;
+use Data::Dumper qw( Dumper );
 use Koha::Account;
 use Koha::AuthorisedValues;
 use Koha::Biblioitems;
-use Koha::DateUtils;
+use Koha::DateUtils qw( dt_from_string output_pref );
 use Koha::Calendar;
 use Koha::Checkouts;
+use Koha::Illrequests;
 use Koha::Items;
 use Koha::Patrons;
-use Koha::Patron::Debarments;
+use Koha::Patron::Debarments qw( DelUniqueDebarment GetDebarments );
 use Koha::Database;
 use Koha::Libraries;
 use Koha::Account::Lines;
 use Koha::Holds;
-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::Config::SysPref;
 use Koha::Checkouts::ReturnClaims;
-use Carp;
-use List::MoreUtils qw( uniq any );
+use Koha::SearchEngine::Indexer;
+use Koha::Exceptions::Checkout;
+use Carp qw( carp );
+use List::MoreUtils qw( any );
 use Scalar::Util qw( looks_like_number );
-use Date::Calc qw(
-  Today
-  Today_and_Now
-  Add_Delta_YM
-  Add_Delta_DHMS
-  Date_to_Days
-  Day_of_Week
-  Add_Delta_Days
-);
-use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
-
+use Date::Calc qw( Date_to_Days );
+our (@ISA, @EXPORT_OK);
 BEGIN {
-       require Exporter;
-       @ISA    = qw(Exporter);
-
-       # FIXME subs that should probably be elsewhere
-       push @EXPORT, qw(
-               &barcodedecode
-        &LostItem
-        &ReturnLostItem
-        &GetPendingOnSiteCheckouts
-       );
-
-       # subs to deal with issuing a book
-       push @EXPORT, qw(
-               &CanBookBeIssued
-               &CanBookBeRenewed
-               &AddIssue
-               &AddRenewal
-               &GetRenewCount
-        &GetSoonestRenewDate
-        &GetLatestAutoRenewDate
-               &GetIssuingCharges
-        &GetBranchBorrowerCircRule
-        &GetBranchItemRule
-               &GetBiblioIssues
-               &GetOpenIssue
-        &CheckIfIssuedToPatron
-        &IsItemIssued
-        GetTopIssues
-       );
-
-       # subs to deal with returns
-       push @EXPORT, qw(
-               &AddReturn
-        &MarkIssueReturned
-       );
-
-       # subs to deal with transfers
-       push @EXPORT, qw(
-               &transferbook
-               &GetTransfers
-               &GetTransfersFromTo
-               &updateWrongTransfer
-               &DeleteTransfer
-                &IsBranchTransferAllowed
-                &CreateBranchTransferLimit
-                &DeleteBranchTransferLimits
-        &TransferSlip
-       );
-
-    # subs to deal with offline circulation
-    push @EXPORT, qw(
-      &GetOfflineOperations
-      &GetOfflineOperation
-      &AddOfflineOperation
-      &DeleteOfflineOperation
-      &ProcessOfflineOperation
+
+    require Exporter;
+    @ISA = qw(Exporter);
+
+    # FIXME subs that should probably be elsewhere
+    push @EXPORT_OK, qw(
+      barcodedecode
+      LostItem
+      ReturnLostItem
+      GetPendingOnSiteCheckouts
+
+      CanBookBeIssued
+      checkHighHolds
+      CanBookBeRenewed
+      AddIssue
+      GetLoanLength
+      GetHardDueDate
+      AddRenewal
+      GetRenewCount
+      GetSoonestRenewDate
+      GetLatestAutoRenewDate
+      GetIssuingCharges
+      AddIssuingCharge
+      GetBranchBorrowerCircRule
+      GetBranchItemRule
+      GetBiblioIssues
+      GetOpenIssue
+      GetUpcomingDueIssues
+      CheckIfIssuedToPatron
+      IsItemIssued
+      GetAgeRestriction
+      GetTopIssues
+
+      AddReturn
+      MarkIssueReturned
+
+      transferbook
+      TooMany
+      GetTransfers
+      GetTransfersFromTo
+      updateWrongTransfer
+      CalcDateDue
+      CheckValidBarcode
+      IsBranchTransferAllowed
+      CreateBranchTransferLimit
+      DeleteBranchTransferLimits
+      TransferSlip
+
+      GetOfflineOperations
+      GetOfflineOperation
+      AddOfflineOperation
+      DeleteOfflineOperation
+      ProcessOfflineOperation
+      ProcessOfflinePayment
     );
+    push @EXPORT_OK, '_GetCircControlBranch';    # This is wrong!
 }
 
 =head1 NAME
@@ -178,7 +171,7 @@ sub barcodedecode {
        } elsif ($filter eq 'cuecat') {
                chomp($barcode);
            my @fields = split( /\./, $barcode );
-           my @results = map( decode($_), @fields[ 1 .. $#fields ] );
+           my @results = map( C4::Circulation::_decode($_), @fields[ 1 .. $#fields ] );
            ($#results == 2) and return $results[2];
        } elsif ($filter eq 'T-prefix') {
                if ($barcode =~ /^[Tt](\d)/) {
@@ -209,9 +202,9 @@ sub barcodedecode {
     return $barcode;    # return barcode, modified or not
 }
 
-=head2 decode
+=head2 _decode
 
-  $str = &decode($chunk);
+  $str = &_decode($chunk);
 
 Decodes a segment of a string emitted by a CueCat barcode scanner and
 returns it.
@@ -221,7 +214,7 @@ or Javascript based decoding on the client side.
 
 =cut
 
-sub decode {
+sub _decode {
     my ($encoded) = @_;
     my $seq =
       'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789+-';
@@ -250,18 +243,26 @@ sub decode {
 
 =head2 transferbook
 
-  ($dotransfer, $messages, $iteminformation) = &transferbook($newbranch, 
-                                            $barcode, $ignore_reserves);
+  ($dotransfer, $messages, $iteminformation) = &transferbook({
+                                                   from_branch => $frombranch
+                                                   to_branch => $tobranch,
+                                                   barcode => $barcode,
+                                                   ignore_reserves => $ignore_reserves,
+                                                   trigger => $trigger
+                                                });
 
 Transfers an item to a new branch. If the item is currently on loan, it is automatically returned before the actual transfer.
 
-C<$newbranch> is the code for the branch to which the item should be transferred.
+C<$fbr> is the code for the branch initiating the transfer.
+C<$tbr> is the code for the branch to which the item should be transferred.
 
 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
@@ -303,11 +304,24 @@ The item was eligible to be transferred. Barring problems communicating with the
 =cut
 
 sub transferbook {
-    my ( $tbr, $barcode, $ignoreRs ) = @_;
+    my $params = shift;
+    my $tbr      = $params->{to_branch};
+    my $fbr      = $params->{from_branch};
+    my $ignoreRs = $params->{ignore_reserves};
+    my $barcode  = $params->{barcode};
+    my $trigger  = $params->{trigger};
     my $messages;
     my $dotransfer      = 1;
     my $item = Koha::Items->find( { barcode => $barcode } );
 
+    Koha::Exceptions::MissingParameter->throw(
+        "Missing mandatory parameter: from_branch")
+      unless $fbr;
+
+    Koha::Exceptions::MissingParameter->throw(
+        "Missing mandatory parameter: to_branch")
+      unless $tbr;
+
     # bad barcode..
     unless ( $item ) {
         $messages->{'BadBarcode'} = $barcode;
@@ -318,7 +332,6 @@ sub transferbook {
     my $itemnumber = $item->itemnumber;
     # get branches of book...
     my $hbr = $item->homebranch;
-    my $fbr = $item->holdingbranch;
 
     # if using Branch Transfer Limits
     if ( C4::Context->preference("UseBranchTransferLimits") == 1 ) {
@@ -351,19 +364,18 @@ sub transferbook {
     # That'll save a database query.
     my ( $resfound, $resrec, undef ) =
       CheckReserves( $itemnumber );
-    if ( $resfound and not $ignoreRs ) {
+    if ( $resfound ) {
         $resrec->{'ResFound'} = $resfound;
-
-        #         $messages->{'ResFound'} = $resrec;
-        $dotransfer = 1;
+        $messages->{'ResFound'} = $resrec;
+        $dotransfer = 0 unless $ignoreRs;
     }
 
     #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;
+        $messages->{'WasTransfered'} = $tbr;
 
     }
     ModDateLastSeen( $itemnumber );
@@ -379,13 +391,30 @@ sub TooMany {
     my $switch_onsite_checkout = $params->{switch_onsite_checkout} || 0;
     my $cat_borrower    = $borrower->{'categorycode'};
     my $dbh             = C4::Context->dbh;
-       my $branch;
-       # Get which branchcode we need
-    $branch = _GetCircControlBranch($item_object->unblessed,$borrower);
+    # Get which branchcode we need
+    my $branch = _GetCircControlBranch($item_object->unblessed,$borrower);
     my $type = $item_object->effective_itemtype;
 
+    my ($type_object, $parent_type, $parent_maxissueqty_rule);
+    $type_object = Koha::ItemTypes->find( $type );
+    $parent_type = $type_object->parent_type if $type_object;
+    my $child_types = Koha::ItemTypes->search({ parent_type => $type });
+    # Find any children if we are a parent_type;
+
     # given branch, patron category, and item type, determine
     # applicable issuing rule
+
+    $parent_maxissueqty_rule = Koha::CirculationRules->get_effective_rule(
+        {
+            categorycode => $cat_borrower,
+            itemtype     => $parent_type,
+            branchcode   => $branch,
+            rule_name    => 'maxissueqty',
+        }
+    ) if $parent_type;
+    # If the parent rule is for default type we discount it
+    $parent_maxissueqty_rule = undef if $parent_maxissueqty_rule && !defined $parent_maxissueqty_rule->itemtype;
+
     my $maxissueqty_rule = Koha::CirculationRules->get_effective_rule(
         {
             categorycode => $cat_borrower,
@@ -394,6 +423,7 @@ sub TooMany {
             rule_name    => 'maxissueqty',
         }
     );
+
     my $maxonsiteissueqty_rule = Koha::CirculationRules->get_effective_rule(
         {
             categorycode => $cat_borrower,
@@ -404,156 +434,129 @@ sub TooMany {
     );
 
 
+    my $patron = Koha::Patrons->find($borrower->{borrowernumber});
     # 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 $maxissueqty_rule->rule_value ne '') {
-        my @bind_params;
-        my $count_query = q|
-            SELECT COUNT(*) AS total, COALESCE(SUM(onsite_checkout), 0) AS onsite_checkouts
-            FROM issues
-            JOIN items USING (itemnumber)
-        |;
+    if (defined($maxissueqty_rule) and $maxissueqty_rule->rule_value ne "") {
 
-        my $rule_itemtype = $maxissueqty_rule->itemtype;
-        unless ($rule_itemtype) {
-            # matching rule has the default item type, so count only
-            # those existing loans that don't fall under a more
-            # specific rule
-            if (C4::Context->preference('item-level_itypes')) {
-                $count_query .= " WHERE items.itype NOT IN (
-                                    SELECT itemtype FROM circulation_rules
-                                    WHERE branchcode = ?
-                                    AND   (categorycode = ? OR categorycode = ?)
-                                    AND   itemtype IS NOT NULL
-                                    AND   rule_name = 'maxissueqty'
-                                  ) ";
+        my $checkouts;
+        if ( $maxissueqty_rule->branchcode ) {
+            if ( C4::Context->preference('CircControl') eq 'PickupLibrary' ) {
+                $checkouts = $patron->checkouts->search(
+                    { 'me.branchcode' => $maxissueqty_rule->branchcode } );
+            } elsif (C4::Context->preference('CircControl') eq 'PatronLibrary') {
+                $checkouts = $patron->checkouts; # if branch is the patron's home branch, then count all loans by patron
             } else {
-                $count_query .= " JOIN  biblioitems USING (biblionumber)
-                                  WHERE biblioitems.itemtype NOT IN (
-                                    SELECT itemtype FROM circulation_rules
-                                    WHERE branchcode = ?
-                                    AND   (categorycode = ? OR categorycode = ?)
-                                    AND   itemtype IS NOT NULL
-                                    AND   rule_name = 'maxissueqty'
-                                  ) ";
+                $checkouts = $patron->checkouts->search(
+                    { 'item.homebranch' => $maxissueqty_rule->branchcode },
+                    { prefetch          => 'item' } );
             }
-            push @bind_params, $maxissueqty_rule->branchcode;
-            push @bind_params, $maxissueqty_rule->categorycode;
-            push @bind_params, $cat_borrower;
         } else {
-            # rule has specific item type, so count loans of that
-            # specific item type
-            if (C4::Context->preference('item-level_itypes')) {
-                $count_query .= " WHERE items.itype = ? ";
-            } else { 
-                $count_query .= " JOIN  biblioitems USING (biblionumber) 
-                                  WHERE biblioitems.itemtype= ? ";
-            }
-            push @bind_params, $type;
+            $checkouts = $patron->checkouts; # if rule is not branch specific then count all loans by patron
         }
+        my $sum_checkouts;
+        my $rule_itemtype = $maxissueqty_rule->itemtype;
+        while ( my $c = $checkouts->next ) {
+            my $itemtype = $c->item->effective_itemtype;
+            my @types;
+            unless ( $rule_itemtype ) {
+                # matching rule has the default item type, so count only
+                # those existing loans that don't fall under a more
+                # specific rule
+                @types = Koha::CirculationRules->search(
+                    {
+                        branchcode => $maxissueqty_rule->branchcode,
+                        categorycode => [ $maxissueqty_rule->categorycode, $cat_borrower ],
+                        itemtype  => { '!=' => undef },
+                        rule_name => 'maxissueqty'
+                    }
+                )->get_column('itemtype');
 
-        $count_query .= " AND borrowernumber = ? ";
-        push @bind_params, $borrower->{'borrowernumber'};
-        my $rule_branch = $maxissueqty_rule->branchcode;
-        if ($rule_branch) {
-            if (C4::Context->preference('CircControl') eq 'PickupLibrary') {
-                $count_query .= " AND issues.branchcode = ? ";
-                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
+                next if grep {$_ eq $itemtype} @types;
             } else {
-                $count_query .= " AND items.homebranch = ? ";
-                push @bind_params, $rule_branch;
+                my @types;
+                if ( $parent_maxissueqty_rule ) {
+                # if we have a parent item type then we count loans of the
+                # specific item type or its siblings or parent
+                    my $children = Koha::ItemTypes->search({ parent_type => $parent_type });
+                    @types = $children->get_column('itemtype');
+                    push @types, $parent_type;
+                } elsif ( $child_types ) {
+                # If we are a parent type, we need to count all child types and our own type
+                    @types = $child_types->get_column('itemtype');
+                    push @types, $type; # And don't forget to count our own types
+                } else { push @types, $type; } # Otherwise only count the specific itemtype
+
+                next unless grep {$_ eq $itemtype} @types;
             }
+            $sum_checkouts->{total}++;
+            $sum_checkouts->{onsite_checkouts}++ if $c->onsite_checkout;
+            $sum_checkouts->{itemtype}->{$itemtype}++;
         }
 
-        my ( $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $count_query, {}, @bind_params );
-
-        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 $max_onsite_checkouts_allowed ne '' ) {
-            if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
-                return {
-                    reason => 'TOO_MANY_ONSITE_CHECKOUTS',
-                    count => $onsite_checkout_count,
-                    max_allowed => $max_onsite_checkouts_allowed,
-                }
-            }
-        }
-        if ( C4::Context->preference('ConsiderOnSiteCheckoutsAsNormalCheckouts') ) {
-            my $delta = $switch_onsite_checkout ? 1 : 0;
-            if ( $checkout_count >= $max_checkouts_allowed + $delta ) {
-                return {
-                    reason => 'TOO_MANY_CHECKOUTS',
-                    count => $checkout_count,
-                    max_allowed => $max_checkouts_allowed,
-                };
-            }
-        } elsif ( not $onsite_checkout ) {
-            if ( $checkout_count - $onsite_checkout_count >= $max_checkouts_allowed )  {
-                return {
-                    reason => 'TOO_MANY_CHECKOUTS',
-                    count => $checkout_count - $onsite_checkout_count,
-                    max_allowed => $max_checkouts_allowed,
-                };
+        my $checkout_count_type = $sum_checkouts->{itemtype}->{$type} || 0;
+        my $checkout_count = $sum_checkouts->{total} || 0;
+        my $onsite_checkout_count = $sum_checkouts->{onsite_checkouts} || 0;
+
+        my $checkout_rules = {
+            checkout_count               => $checkout_count,
+            onsite_checkout_count        => $onsite_checkout_count,
+            onsite_checkout              => $onsite_checkout,
+            max_checkouts_allowed        => $maxissueqty_rule ? $maxissueqty_rule->rule_value : undef,
+            max_onsite_checkouts_allowed => $maxonsiteissueqty_rule ? $maxonsiteissueqty_rule->rule_value : undef,
+            switch_onsite_checkout       => $switch_onsite_checkout,
+        };
+        # If parent rules exists
+        if ( defined($parent_maxissueqty_rule) and defined($parent_maxissueqty_rule->rule_value) ){
+            $checkout_rules->{max_checkouts_allowed} = $parent_maxissueqty_rule ? $parent_maxissueqty_rule->rule_value : undef;
+            my $qty_over = _check_max_qty($checkout_rules);
+            return $qty_over if defined $qty_over;
+
+            # If the parent rule is less than or equal to the child, we only need check the parent
+            if( $maxissueqty_rule->rule_value < $parent_maxissueqty_rule->rule_value && defined($maxissueqty_rule->itemtype) ) {
+                $checkout_rules->{checkout_count} = $checkout_count_type;
+                $checkout_rules->{max_checkouts_allowed} = $maxissueqty_rule ? $maxissueqty_rule->rule_value : undef;
+                my $qty_over = _check_max_qty($checkout_rules);
+                return $qty_over if defined $qty_over;
             }
+        } else {
+            my $qty_over = _check_max_qty($checkout_rules);
+            return $qty_over if defined $qty_over;
         }
     }
 
     # Now count total loans against the limit for the branch
     my $branch_borrower_circ_rule = GetBranchBorrowerCircRule($branch, $cat_borrower);
     if (defined($branch_borrower_circ_rule->{patron_maxissueqty}) and $branch_borrower_circ_rule->{patron_maxissueqty} ne '') {
-        my @bind_params = ();
-        my $branch_count_query = q|
-            SELECT COUNT(*) AS total, COALESCE(SUM(onsite_checkout), 0) AS onsite_checkouts
-            FROM issues
-            JOIN items USING (itemnumber)
-            WHERE borrowernumber = ?
-        |;
-        push @bind_params, $borrower->{borrowernumber};
-
-        if (C4::Context->preference('CircControl') eq 'PickupLibrary') {
-            $branch_count_query .= " AND issues.branchcode = ? ";
-            push @bind_params, $branch;
+        my $checkouts;
+        if ( C4::Context->preference('CircControl') eq 'PickupLibrary' ) {
+            $checkouts = $patron->checkouts->search(
+                { 'me.branchcode' => $branch} );
         } elsif (C4::Context->preference('CircControl') eq 'PatronLibrary') {
-            ; # if branch is the patron's home branch, then count all loans by patron
+            $checkouts = $patron->checkouts; # if branch is the patron's home branch, then count all loans by patron
         } else {
-            $branch_count_query .= " AND items.homebranch = ? ";
-            push @bind_params, $branch;
+            $checkouts = $patron->checkouts->search(
+                { 'item.homebranch' => $branch},
+                { prefetch          => 'item' } );
         }
-        my ( $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $branch_count_query, {}, @bind_params );
+
+        my $checkout_count = $checkouts->count;
+        my $onsite_checkout_count = $checkouts->search({ onsite_checkout => 1 })->count;
         my $max_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxissueqty};
-        my $max_onsite_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxonsiteissueqty};
-
-        if ( $onsite_checkout and $max_onsite_checkouts_allowed ne '' ) {
-            if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
-                return {
-                    reason => 'TOO_MANY_ONSITE_CHECKOUTS',
-                    count => $onsite_checkout_count,
-                    max_allowed => $max_onsite_checkouts_allowed,
-                }
-            }
-        }
-        if ( C4::Context->preference('ConsiderOnSiteCheckoutsAsNormalCheckouts') ) {
-            my $delta = $switch_onsite_checkout ? 1 : 0;
-            if ( $checkout_count >= $max_checkouts_allowed + $delta ) {
-                return {
-                    reason => 'TOO_MANY_CHECKOUTS',
-                    count => $checkout_count,
-                    max_allowed => $max_checkouts_allowed,
-                };
-            }
-        } elsif ( not $onsite_checkout ) {
-            if ( $checkout_count - $onsite_checkout_count >= $max_checkouts_allowed )  {
-                return {
-                    reason => 'TOO_MANY_CHECKOUTS',
-                    count => $checkout_count - $onsite_checkout_count,
-                    max_allowed => $max_checkouts_allowed,
-                };
+        my $max_onsite_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxonsiteissueqty} || undef;
+
+        my $qty_over = _check_max_qty(
+            {
+                checkout_count               => $checkout_count,
+                onsite_checkout_count        => $onsite_checkout_count,
+                onsite_checkout              => $onsite_checkout,
+                max_checkouts_allowed        => $max_checkouts_allowed,
+                max_onsite_checkouts_allowed => $max_onsite_checkouts_allowed,
+                switch_onsite_checkout       => $switch_onsite_checkout
             }
-        }
+        );
+        return $qty_over if defined $qty_over;
     }
 
     if ( not defined( $maxissueqty_rule ) and not defined($branch_borrower_circ_rule->{patron_maxissueqty}) ) {
@@ -564,6 +567,52 @@ sub TooMany {
     return;
 }
 
+sub _check_max_qty {
+    my $params                       = shift;
+    my $checkout_count               = $params->{checkout_count};
+    my $onsite_checkout_count        = $params->{onsite_checkout_count};
+    my $onsite_checkout              = $params->{onsite_checkout};
+    my $max_checkouts_allowed        = $params->{max_checkouts_allowed};
+    my $max_onsite_checkouts_allowed = $params->{max_onsite_checkouts_allowed};
+    my $switch_onsite_checkout       = $params->{switch_onsite_checkout};
+
+    if ( $onsite_checkout and defined $max_onsite_checkouts_allowed ) {
+        if ( $max_onsite_checkouts_allowed eq '' ) { return; }
+        if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed ) {
+            return {
+                reason      => 'TOO_MANY_ONSITE_CHECKOUTS',
+                count       => $onsite_checkout_count,
+                max_allowed => $max_onsite_checkouts_allowed,
+            };
+        }
+    }
+    if ( C4::Context->preference('ConsiderOnSiteCheckoutsAsNormalCheckouts') ) {
+        if ( $max_checkouts_allowed eq '' ) { return; }
+        my $delta = $switch_onsite_checkout ? 1 : 0;
+        if ( $checkout_count >= $max_checkouts_allowed + $delta ) {
+            return {
+                reason      => 'TOO_MANY_CHECKOUTS',
+                count       => $checkout_count,
+                max_allowed => $max_checkouts_allowed,
+            };
+        }
+    }
+    elsif ( not $onsite_checkout ) {
+        if ( $max_checkouts_allowed eq '' ) { return; }
+        if (
+            $checkout_count - $onsite_checkout_count >= $max_checkouts_allowed )
+        {
+            return {
+                reason      => 'TOO_MANY_CHECKOUTS',
+                count       => $checkout_count - $onsite_checkout_count,
+                max_allowed => $max_checkouts_allowed,
+            };
+        }
+    }
+
+    return;
+}
+
 =head2 CanBookBeIssued
 
   ( $issuingimpossible, $needsconfirmation, [ $alerts ] ) =  CanBookBeIssued( $patron,
@@ -659,6 +708,10 @@ issued to someone else.
 
 reserved for someone else.
 
+=head3 TRANSFERRED
+
+reserved and being transferred for someone else.
+
 =head3 INVALID_DATE
 
 sticky due date is invalid or due date in the past
@@ -703,7 +756,7 @@ 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();
 
@@ -783,7 +836,7 @@ sub CanBookBeIssued {
     $no_issues_charge_guarantees = undef unless looks_like_number( $no_issues_charge_guarantees );
     if ( defined $no_issues_charge_guarantees ) {
         my @guarantees = map { $_->guarantee } $patron->guarantee_relationships();
-        my $guarantees_non_issues_charges;
+        my $guarantees_non_issues_charges = 0;
         foreach my $g ( @guarantees ) {
             $guarantees_non_issues_charges += $g->account->non_issues_charges;
         }
@@ -797,6 +850,21 @@ sub CanBookBeIssued {
         }
     }
 
+    # Check the debt of this patrons guarantors *and* the guarantees of those guarantors
+    my $no_issues_charge_guarantors = C4::Context->preference("NoIssuesChargeGuarantorsWithGuarantees");
+    $no_issues_charge_guarantors = undef unless looks_like_number( $no_issues_charge_guarantors );
+    if ( defined $no_issues_charge_guarantors ) {
+        my $guarantors_non_issues_charges += $patron->relationships_debt({ include_guarantors => 1, only_this_guarantor => 0, include_this_patron => 1 });
+
+        if ( $guarantors_non_issues_charges > $no_issues_charge_guarantors && !$inprocess && !$allowfineoverride) {
+            $issuingimpossible{DEBT_GUARANTORS} = $guarantors_non_issues_charges;
+        } elsif ( $guarantors_non_issues_charges > $no_issues_charge_guarantors && !$inprocess && $allowfineoverride) {
+            $needsconfirmation{DEBT_GUARANTORS} = $guarantors_non_issues_charges;
+        } elsif ( $allfinesneedoverride && $guarantors_non_issues_charges > 0 && $guarantors_non_issues_charges <= $no_issues_charge_guarantors && !$inprocess ) {
+            $needsconfirmation{DEBT_GUARANTORS} = $guarantors_non_issues_charges;
+        }
+    }
+
     if ( C4::Context->preference("IssuingInProcess") ) {
         if ( $non_issues_charges > $amountlimit && !$inprocess && !$allowfineoverride) {
             $issuingimpossible{DEBT} = $non_issues_charges;
@@ -841,6 +909,13 @@ sub CanBookBeIssued {
         }
     }
 
+    # Additional Materials Check
+    if ( C4::Context->preference("CircConfirmItemParts")
+        && $item_object->materials )
+    {
+        $needsconfirmation{ADDITIONAL_MATERIALS} = $item_object->materials;
+    }
+
     #
     # CHECK IF BOOK ALREADY ISSUED TO THIS BORROWER
     #
@@ -1036,6 +1111,7 @@ sub CanBookBeIssued {
                     $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
                     $needsconfirmation{'resbranchcode'} = $res->{branchcode};
                     $needsconfirmation{'reswaitingdate'} = $res->{'waitingdate'};
+                    $needsconfirmation{'reserve_id'} = $res->{reserve_id};
                 }
                 elsif ( $restype eq "Reserved" ) {
                     # The item is on reserve for someone else.
@@ -1046,6 +1122,29 @@ sub CanBookBeIssued {
                     $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
                     $needsconfirmation{'resbranchcode'} = $patron->branchcode;
                     $needsconfirmation{'resreservedate'} = $res->{reservedate};
+                    $needsconfirmation{'reserve_id'} = $res->{reserve_id};
+                }
+                elsif ( $restype eq "Transferred" ) {
+                    # The item is determined hold being transferred for someone else.
+                    $needsconfirmation{TRANSFERRED} = 1;
+                    $needsconfirmation{'resfirstname'} = $patron->firstname;
+                    $needsconfirmation{'ressurname'} = $patron->surname;
+                    $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
+                    $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
+                    $needsconfirmation{'resbranchcode'} = $patron->branchcode;
+                    $needsconfirmation{'resreservedate'} = $res->{reservedate};
+                    $needsconfirmation{'reserve_id'} = $res->{reserve_id};
+                }
+                elsif ( $restype eq "Processing" ) {
+                    # The item is determined hold being processed for someone else.
+                    $needsconfirmation{PROCESSING} = 1;
+                    $needsconfirmation{'resfirstname'} = $patron->firstname;
+                    $needsconfirmation{'ressurname'} = $patron->surname;
+                    $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
+                    $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
+                    $needsconfirmation{'resbranchcode'} = $patron->branchcode;
+                    $needsconfirmation{'resreservedate'} = $res->{reservedate};
+                    $needsconfirmation{'reserve_id'} = $res->{reserve_id};
                 }
             }
         }
@@ -1221,7 +1320,7 @@ sub checkHighHolds {
             }
 
             # Remove any items that are not holdable for this patron
-            @items = grep { CanItemBeReserved( $borrower->{borrowernumber}, $_->itemnumber )->{status} eq 'OK' } @items;
+            @items = grep { CanItemBeReserved( $borrower->{borrowernumber}, $_->itemnumber, undef, { ignore_found_holds => 1 } )->{status} eq 'OK' } @items;
 
             my $items_count = scalar @items;
 
@@ -1234,23 +1333,44 @@ sub checkHighHolds {
             }
         }
 
-        my $issuedate = DateTime->now( time_zone => C4::Context->tz() );
-
-        my $calendar = Koha::Calendar->new( branchcode => $branchcode );
+        my $issuedate = dt_from_string();
 
         my $itype = $item_object->effective_itemtype;
+        my $daysmode = Koha::CirculationRules->get_effective_daysmode(
+            {
+                categorycode => $borrower->{categorycode},
+                itemtype     => $itype,
+                branchcode   => $branchcode,
+            }
+        );
+        my $calendar = Koha::Calendar->new( branchcode => $branchcode, days_mode => $daysmode );
+
         my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branchcode, $borrower );
 
-        my $decreaseLoanHighHoldsDuration = C4::Context->preference('decreaseLoanHighHoldsDuration');
+        my $rule = Koha::CirculationRules->get_effective_rule(
+            {
+                categorycode => $borrower->{categorycode},
+                itemtype     => $item_object->effective_itemtype,
+                branchcode   => $branchcode,
+                rule_name    => 'decreaseloanholds',
+            }
+        );
 
-        my $reduced_datedue = $calendar->addDate( $issuedate, $decreaseLoanHighHoldsDuration );
+        my $duration;
+        if ( defined($rule) && $rule->rule_value ne '' ){
+            # overrides decreaseLoanHighHoldsDuration syspref
+            $duration = $rule->rule_value;
+        } else {
+            $duration = C4::Context->preference('decreaseLoanHighHoldsDuration');
+        }
+        my $reduced_datedue = $calendar->addDuration( $issuedate, $duration );
         $reduced_datedue->set_hour($orig_due->hour);
         $reduced_datedue->set_minute($orig_due->minute);
         $reduced_datedue->truncate( to => 'minute' );
 
         if ( DateTime->compare( $reduced_datedue, $orig_due ) == -1 ) {
             $return_data->{exceeded} = 1;
-            $return_data->{duration} = $decreaseLoanHighHoldsDuration;
+            $return_data->{duration} = $duration;
             $return_data->{due_date} = $reduced_datedue;
         }
     }
@@ -1313,7 +1433,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' ) {
@@ -1371,23 +1491,29 @@ sub AddIssue {
                 my ( $allowed, $message ) = CanBookBeReturned( $item_unblessed, C4::Context->userenv->{branch} );
                 return unless $allowed;
                 AddReturn( $item_object->barcode, C4::Context->userenv->{'branch'} );
+                # AddReturn certainly has side-effects, like onloan => undef
+                $item_object->discard_changes;
             }
 
             C4::Reserves::MoveReserve( $item_object->itemnumber, $borrower->{'borrowernumber'}, $cancelreserve );
 
             # Starting process for transfer job (checking transfert and validate it if we have one)
-            my ($datesent) = GetTransfers( $item_object->itemnumber );
-            if ($datesent) {
+            if ( my $transfer = $item_object->get_transfer ) {
                 # updating line of branchtranfert to finish it, and changing the to branch value, implement a comment for visibility of this case (maybe for stats ....)
-                my $sth = $dbh->prepare(
-                    "UPDATE branchtransfers 
-                        SET datearrived = now(),
-                        tobranch = ?,
-                        comments = 'Forced branchtransfer'
-                    WHERE itemnumber= ? AND datearrived IS NULL"
-                );
-                $sth->execute( C4::Context->userenv->{'branch'},
-                    $item_object->itemnumber );
+                $transfer->set(
+                    {
+                        datearrived => dt_from_string,
+                        tobranch    => C4::Context->userenv->{branch},
+                        comments    => 'Forced branchtransfer'
+                    }
+                )->store;
+                if ( $transfer->reason && $transfer->reason eq 'Reserve' ) {
+                    my $hold = $item_object->holds->search( { found => 'T' } )->next;
+                    if ( $hold ) { # Is this really needed?
+                        $hold->set( { found => undef } )->store;
+                        C4::Reserves::ModReserveMinusPriority($item_object->itemnumber, $hold->reserve_id);
+                    }
+                }
             }
 
             # If automatic renewal wasn't selected while issuing, set the value according to the issuing rule.
@@ -1404,14 +1530,6 @@ sub AddIssue {
                 $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, $branchcode, $borrower );
-
-            }
-            $datedue->truncate( to => 'minute' );
-
             my $issue_attributes = {
                 borrowernumber  => $borrower->{'borrowernumber'},
                 issuedate       => $issuedate->strftime('%Y-%m-%d %H:%M:%S'),
@@ -1421,6 +1539,19 @@ sub AddIssue {
                 auto_renew      => $auto_renew ? 1 : 0,
             };
 
+            # Get ID of logged in user.  if called from a batch job,
+            # no user session exists and C4::Context->userenv() returns
+            # the scalar '0'. Only do this if the syspref says so
+            if ( C4::Context->preference('RecordStaffUserOnCheckout') ) {
+                my $userenv = C4::Context->userenv();
+                my $usernumber = (ref($userenv) eq 'HASH') ? $userenv->{'number'} : undef;
+                if ($usernumber) {
+                    $issue_attributes->{issuer_id} = $usernumber;
+                }
+            }
+
+            # In the case that the borrower has an on-site checkout
+            # and SwitchOnSiteCheckouts is enabled this converts it to a regular checkout
             $issue = Koha::Checkouts->find( { itemnumber => $item_object->itemnumber } );
             if ($issue) {
                 $issue->set($issue_attributes)->store;
@@ -1433,6 +1564,8 @@ sub AddIssue {
                     }
                 )->store;
             }
+            $issue->discard_changes;
+            C4::Auth::track_login_daily( $borrower->{userid} );
             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.
@@ -1443,37 +1576,44 @@ sub AddIssue {
                 UpdateTotalIssues( $item_object->biblionumber, 1 );
             }
 
-            ## If item was lost, it has now been found, reverse any list item charges if necessary.
-            if ( $item_object->itemlost ) {
-                if (
-                    Koha::RefundLostItemFeeRules->should_refund(
+            # Record if item was lost
+            my $was_lost = $item_object->itemlost;
+
+            $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->datelastseen( dt_from_string()->ymd() );
+            $item_object->store({log_action => 0});
+
+            # If the item was lost, it has now been found, charge the overdue if necessary
+            if ($was_lost) {
+                if ( $item_object->{_charge} ) {
+                    $actualissue //= Koha::Old::Checkouts->search(
+                        { itemnumber => $item_unblessed->{itemnumber} },
                         {
-                            current_branch      => C4::Context->userenv->{branch},
-                            item_home_branch    => $item_object->homebranch,
-                            item_holding_branch => $item_object->holdingbranch,
+                            order_by => { '-desc' => 'returndate' },
+                            rows     => 1
                         }
-                    )
-                  )
-                {
-                    _FixAccountForLostAndReturned( $item_object->itemnumber, undef,
-                        $item_object->barcode );
+                    )->single;
+                    unless ( exists( $borrower->{branchcode} ) ) {
+                        my $patron = $actualissue->patron;
+                        $borrower = $patron->unblessed;
+                    }
+                    _CalculateAndUpdateFine(
+                        {
+                            issue       => $actualissue,
+                            item        => $item_unblessed,
+                            borrower    => $borrower,
+                            return_date => $issuedate
+                        }
+                    );
+                    _FixOverduesOnReturn( $borrower->{borrowernumber},
+                        $item_object->itemnumber, undef, 'RENEWED' );
                 }
             }
 
-            ModItem(
-                {
-                    issues        => ( $item_object->issues || 0 ) + 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 }
-            );
-            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 && $charge > 0 ) {
@@ -1528,6 +1668,14 @@ sub AddIssue {
                 $borrower->{'borrowernumber'},
                 $item_object->itemnumber,
             ) if C4::Context->preference("IssueLog");
+
+            Koha::Plugins->call('after_circ_action', {
+                action  => 'checkout',
+                payload => {
+                    type     => ( $onsite_checkout ? 'onsite_checkout' : 'issue' ),
+                    checkout => $issue->get_from_storage
+                }
+            });
         }
     }
     return $issue;
@@ -1544,50 +1692,6 @@ Get loan length for an itemtype, a borrower type and a branch
 sub GetLoanLength {
     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,
-        },
-    );
-
     # Initialize default values
     my $rules = {
         issuelength   => 0,
@@ -1595,21 +1699,20 @@ sub GetLoanLength {
         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();
+    my $found = Koha::CirculationRules->get_effective_rules( {
+        branchcode => $branchcode,
+        categorycode => $categorycode,
+        itemtype => $itemtype,
+        rules => [
+            'issuelength',
+            'renewalperiod',
+            'lengthunit'
+        ],
+    } );
 
-            if ($rule) {
-                $rules->{$rule_name} = $rule->rule_value;
-                last;
-            }
-        }
+    # Search for rules!
+    foreach my $rule_name (keys %$found) {
+        $rules->{$rule_name} = $found->{$rule_name};
     }
 
     return $rules;
@@ -1716,9 +1819,10 @@ branch and item type, regardless of patron category.
 The return value is a hashref containing the following keys:
 
 holdallowed => Hold policy for this branch and itemtype. Possible values:
-  0: No holds allowed.
-  1: Holds allowed only by patrons that have the same homebranch as the item.
-  2: Holds allowed from any patron.
+  not_allowed:           No holds allowed.
+  from_home_library:     Holds allowed only by patrons that have the same homebranch as the item.
+  from_any_library:      Holds allowed from any patron.
+  from_local_hold_group: Holds allowed from libraries in hold group
 
 returnbranch => branch to which to return item.  Possible values:
   noreturn: do not return, let item remain where checked in (floating collections)
@@ -1743,22 +1847,22 @@ sub GetBranchItemRule {
     my $holdallowed_rule = Koha::CirculationRules->get_effective_rule(
         {
             branchcode => $branchcode,
-            itemtype => $itemtype,
-            rule_name => 'holdallowed',
+            itemtype   => $itemtype,
+            rule_name  => 'holdallowed',
         }
     );
     my $hold_fulfillment_policy_rule = Koha::CirculationRules->get_effective_rule(
         {
             branchcode => $branchcode,
-            itemtype => $itemtype,
-            rule_name => 'hold_fulfillment_policy',
+            itemtype   => $itemtype,
+            rule_name  => 'hold_fulfillment_policy',
         }
     );
     my $returnbranch_rule = Koha::CirculationRules->get_effective_rule(
         {
             branchcode => $branchcode,
-            itemtype => $itemtype,
-            rule_name => 'returnbranch',
+            itemtype   => $itemtype,
+            rule_name  => 'returnbranch',
         }
     );
 
@@ -1766,7 +1870,7 @@ sub GetBranchItemRule {
     my $rules;
     $rules->{holdallowed} = defined $holdallowed_rule
         ? $holdallowed_rule->rule_value
-        : 2;
+        : 'from_any_library';
     $rules->{hold_fulfillment_policy} = defined $hold_fulfillment_policy_rule
         ? $hold_fulfillment_policy_rule->rule_value
         : 'any';
@@ -1858,11 +1962,12 @@ 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;
     my $doreturn       = 1;
-    my $validTransfert = 0;
+    my $validTransfer = 1;
     my $stat_type = 'return';
 
     # get information on item
@@ -1881,7 +1986,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({skip_record_index=>1}) 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.
@@ -1892,25 +1998,25 @@ sub AddReturn {
         }
     }
 
-    my $item_unblessed = $item->unblessed;
         # 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');
+    my $update_loc_rules = Koha::Config::SysPrefs->find('UpdateItemLocationOnCheckin')->get_yaml_pref_hash();
     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_}) {
+            if ( defined $item->location && $item->location ne $update_loc_rules->{_ALL_}) {
                 $messages->{'ItemLocationUpdated'} = { from => $item->location, to => $update_loc_rules->{_ALL_} };
-                ModItem( { location => $update_loc_rules->{_ALL_} }, undef, $itemnumber );
+                $item->location($update_loc_rules->{_ALL_})->store({skip_record_index=>1});
             }
         }
         else {
@@ -1919,7 +2025,7 @@ sub AddReturn {
                 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} };
-                    ModItem( { location => $update_loc_rules->{$key} }, undef, $itemnumber );
+                    $item->location($update_loc_rules->{$key})->store({skip_record_index=>1});
                     last;
                 }
             }
@@ -1930,7 +2036,7 @@ sub AddReturn {
     if ($yaml) {
         $yaml = "$yaml\n\n";  # YAML is anal on ending \n. Surplus does not hurt
         my $rules;
-        eval { $rules = YAML::Load($yaml); };
+        eval { $rules = YAML::XS::Load(Encode::encode_utf8($yaml)); };
         if ($@) {
             warn "Unable to parse UpdateNotForLoanStatusOnCheckin syspref : $@";
         }
@@ -1938,7 +2044,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, skip_record_index => 1 });
                     last;
                 }
             }
@@ -1946,13 +2052,15 @@ 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,
             Rightbranch => $message
         };
         $doreturn = 0;
+        my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX });
+        $indexer->index_records( $item->biblionumber, "specialUpdate", "biblioserver" );
         return ( $doreturn, $messages, $issue, $patron_unblessed);
     }
 
@@ -1972,14 +2080,24 @@ sub AddReturn {
 
         if ($patron) {
             eval {
-                MarkIssueReturned( $borrowernumber, $item->itemnumber, $return_date, $patron->privacy );
+                MarkIssueReturned( $borrowernumber, $item->itemnumber, $return_date, $patron->privacy, { skip_record_index => 1} );
             };
             unless ( $@ ) {
-                if ( C4::Context->preference('CalculateFinesOnReturn') && !$item->itemlost ) {
-                    _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 );
+                carp "The checkin for the following issue failed, Please go to the about page and check all messages on the 'System information' to see if there are configuration / data issues ($@)" . Dumper( $issue->unblessed );
+
+                my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX });
+                $indexer->index_records( $item->biblionumber, "specialUpdate", "biblioserver" );
 
                 return ( 0, { WasReturned => 0, DataCorrupted => 1 }, $issue, $patron_unblessed );
             }
@@ -1987,59 +2105,81 @@ sub AddReturn {
             # FIXME is the "= 1" right?  This could be the borrower hash.
             $messages->{'WasReturned'} = 1;
 
+        } else {
+            $item->onloan(undef)->store({ log_action => 0 , skip_record_index => 1 });
         }
-
-        ModItem( { onloan => undef }, $item->biblionumber, $item->itemnumber, { 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({ skip_record_index => 1 });
     }
 
+    my $item_was_lost = $item->itemlost;
     my $leave_item_lost = C4::Context->preference("BlockReturnOfLostItems") ? 1 : 0;
-    ModDateLastSeen( $item->itemnumber, $leave_item_lost );
-
-    # check if we have a transfer for this document
-    my ($datesent,$frombranch,$tobranch) = GetTransfers( $item->itemnumber );
-
-    # if we have a transfer to do, we update the line of transfers with the datearrived
-    my $is_in_rotating_collection = C4::RotatingCollections::isItemInAnyCollection( $item->itemnumber );
-    if ($datesent) {
-        if ( $tobranch eq $branch ) {
-            my $sth = C4::Context->dbh->prepare(
-                "UPDATE branchtransfers SET datearrived = now() WHERE itemnumber= ? AND datearrived IS NULL"
-            );
-            $sth->execute( $item->itemnumber );
-            # if we have a reservation with valid transfer, we can set it's status to 'W'
-            C4::Reserves::ModReserveStatus($item->itemnumber, 'W');
-        } else {
-            $messages->{'WrongTransfer'}     = $tobranch;
-            $messages->{'WrongTransferItem'} = $item->itemnumber;
-        }
-        $validTransfert = 1;
-    }
+    my $updated_item = ModDateLastSeen( $item->itemnumber, $leave_item_lost, { skip_record_index => 1 } ); # will unset itemlost if needed
 
     # fix up the accounts.....
-    if ( $item->itemlost ) {
+    if ($item_was_lost) {
         $messages->{'WasLost'} = 1;
         unless ( C4::Context->preference("BlockReturnOfLostItems") ) {
-            if (
-                Koha::RefundLostItemFeeRules->should_refund(
+            $messages->{'LostItemFeeRefunded'} = $updated_item->{_refunded};
+            $messages->{'LostItemFeeRestored'} = $updated_item->{_restored};
+
+            if ( $updated_item->{_charge} ) {
+                $issue //= Koha::Old::Checkouts->search(
+                    { itemnumber => $item->itemnumber },
+                    { order_by   => { '-desc' => 'returndate' }, rows => 1 } )
+                  ->single;
+                unless ( exists( $patron_unblessed->{branchcode} ) ) {
+                    my $patron = $issue->patron;
+                    $patron_unblessed = $patron->unblessed;
+                }
+                _CalculateAndUpdateFine(
                     {
-                        current_branch      => C4::Context->userenv->{branch},
-                        item_home_branch    => $item->homebranch,
-                        item_holding_branch => $item_holding_branch
+                        issue       => $issue,
+                        item        => $item->unblessed,
+                        borrower    => $patron_unblessed,
+                        return_date => $return_date
                     }
-                )
-              )
-            {
-                _FixAccountForLostAndReturned( $item->itemnumber,
-                    $borrowernumber, $barcode );
-                $messages->{'LostItemFeeRefunded'} = 1;
+                );
+                _FixOverduesOnReturn( $patron_unblessed->{borrowernumber},
+                    $item->itemnumber, undef, 'RETURNED' );
+                $messages->{'LostItemFeeCharged'} = 1;
+            }
+        }
+    }
+
+    # check if we have a transfer for this document
+    my $transfer = $item->get_transfer;
+
+    # if we have a transfer to complete, we update the line of transfers with the datearrived
+    if ($transfer) {
+        $validTransfer = 0;
+        if ( $transfer->in_transit ) {
+            if ( $transfer->tobranch eq $branch ) {
+                $transfer->receive;
+                $messages->{'TransferArrived'} = $transfer->frombranch;
+                # validTransfer=1 allows us returning the item back if the reserve is cancelled
+                $validTransfer = 1 if $transfer->reason eq 'Reserve';
+            }
+            else {
+                $messages->{'WrongTransfer'}     = $transfer->tobranch;
+                $messages->{'WrongTransferItem'} = $item->itemnumber;
+                $messages->{'TransferTrigger'}   = $transfer->reason;
+            }
+        }
+        else {
+            if ( $transfer->tobranch eq $branch ) {
+                $transfer->receive;
+                $messages->{'TransferArrived'} = $transfer->frombranch;
+                # validTransfer=1 allows us returning the item back if the reserve is cancelled
+                $validTransfer = 1 if $transfer->reason eq 'Reserve';
+            }
+            else {
+                $messages->{'WasTransfered'}   = $transfer->tobranch;
+                $messages->{'TransferTrigger'} = $transfer->reason;
             }
         }
     }
@@ -2047,11 +2187,11 @@ sub AddReturn {
     # fix up the overdues in accounts...
     if ($borrowernumber) {
         my $fix = _FixOverduesOnReturn( $borrowernumber, $item->itemnumber, $exemptfine, 'RETURNED' );
-        defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $item->itemnumber...) failed!";  # zero is OK, check defined
+        defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, ".$item->itemnumber."...) failed!";  # zero is OK, check defined
 
-        if ( $issue and $issue->is_overdue ) {
+        if ( $issue and $issue->is_overdue($return_date) ) {
         # 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 {
@@ -2073,10 +2213,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 and $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;
@@ -2088,6 +2234,7 @@ sub AddReturn {
         type           => $stat_type,
         itemnumber     => $itemnumber,
         itemtype       => $itemtype,
+        location       => $item->location,
         borrowernumber => $borrowernumber,
         ccode          => $item->ccode,
     });
@@ -2104,7 +2251,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,
             });
@@ -2114,29 +2261,34 @@ sub AddReturn {
             if C4::Context->preference("ReturnLog");
         }
 
-    # Remove any OVERDUES related debarment if the borrower has no overdues
-    if ( $borrowernumber
-      && $patron->debarred
-      && C4::Context->preference('AutoRemoveOverduesRestrictions')
-      && !Koha::Patrons->find( $borrowernumber )->has_overdues
-      && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
-    ) {
-        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 ( $doreturn and 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'}){
+    if ( $validTransfer && !C4::RotatingCollections::isItemInAnyCollection( $item->itemnumber )
+        && ( $doreturn or $messages->{'NotIssued'} )
+        and !$resfound
+        and ( $branch ne $returnbranch )
+        and not $messages->{'WrongTransfer'}
+        and not $messages->{'WasTransfered'} )
+    {
         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);
-            $messages->{'WasTransfered'} = 1;
+            ModItemTransfer($item->itemnumber, $branch, $returnbranch, $transfer_trigger, { skip_record_index => 1 });
+            $messages->{'WasTransfered'} = $returnbranch;
+            $messages->{'TransferTrigger'} = $transfer_trigger;
         } else {
             $messages->{'NeedsTransfer'} = $returnbranch;
+            $messages->{'TransferTrigger'} = $transfer_trigger;
         }
     }
 
@@ -2153,12 +2305,26 @@ sub AddReturn {
         }
     }
 
+    my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX });
+    $indexer->index_records( $item->biblionumber, "specialUpdate", "biblioserver" );
+
+    if ( $doreturn and $issue ) {
+        my $checkin = Koha::Old::Checkouts->find($issue->id);
+
+        Koha::Plugins->call('after_circ_action', {
+            action  => 'checkin',
+            payload => {
+                checkout=> $checkin
+            }
+        });
+    }
+
     return ( $doreturn, $messages, $issue, ( $patron ? $patron->unblessed : {} ));
 }
 
 =head2 MarkIssueReturned
 
-  MarkIssueReturned($borrowernumber, $itemnumber, $returndate, $privacy);
+  MarkIssueReturned($borrowernumber, $itemnumber, $returndate, $privacy, [$params] );
 
 Unconditionally marks an issue as being returned by
 moving the C<issues> row to C<old_issues> and
@@ -2174,10 +2340,12 @@ Ideally, this function would be internal to C<C4::Circulation>,
 not exported, but it is currently used in misc/cronjobs/longoverdue.pl
 and offline_circ/process_koc.pl.
 
+The last optional parameter allos passing skip_record_index to the item store call.
+
 =cut
 
 sub MarkIssueReturned {
-    my ( $borrowernumber, $itemnumber, $returndate, $privacy ) = @_;
+    my ( $borrowernumber, $itemnumber, $returndate, $privacy, $params ) = @_;
 
     # Retrieve the issue
     my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return;
@@ -2202,6 +2370,8 @@ sub MarkIssueReturned {
     # FIXME Improve the return value and handle it from callers
     $schema->txn_do(sub {
 
+        my $patron = Koha::Patrons->find( $borrowernumber );
+
         # Update the returndate value
         if ( $returndate ) {
             $issue->returndate( $returndate )->store->discard_changes; # update and refetch
@@ -2221,13 +2391,22 @@ sub MarkIssueReturned {
         # And finally delete the issue
         $issue->delete;
 
-        ModItem( { 'onloan' => undef }, undef, $itemnumber, { log_action => 0 } );
+        $issue->item->onloan(undef)->store({ log_action => 0, skip_record_index => $params->{skip_record_index} });
 
         if ( C4::Context->preference('StoreLastBorrower') ) {
             my $item = Koha::Items->find( $itemnumber );
-            my $patron = Koha::Patrons->find( $borrowernumber );
             $item->last_returned_by( $patron );
         }
+
+        # Remove any OVERDUES related debarment if the borrower has no overdues
+        if ( C4::Context->preference('AutoRemoveOverduesRestrictions')
+          && $patron->debarred
+          && !$patron->has_overdues
+          && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
+        ) {
+            DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
+        }
+
     });
 
     return $issue_id;
@@ -2285,7 +2464,7 @@ sub _calculate_new_debar_dt {
 
     # grace period is measured in the same units as the loan
     my $grace =
-      DateTime::Duration->new( $unit => $issuing_rule->{firstremind} );
+      DateTime::Duration->new( $unit => $issuing_rule->{firstremind} // 0);
 
     my $deltadays = DateTime::Duration->new(
         days => $chargeable_units
@@ -2326,7 +2505,7 @@ sub _calculate_new_debar_dt {
                 branchcode => $branchcode,
                 days_mode  => 'Calendar'
             );
-            $new_debar_dt = $calendar->addDate( $return_date, $suspension_days );
+            $new_debar_dt = $calendar->addDuration( $return_date, $suspension_days );
         }
         else {
             $new_debar_dt = $return_date->clone()->add_duration($suspension_days);
@@ -2410,9 +2589,13 @@ sub _FixOverduesOnReturn {
             return 0 unless $accountlines->count; # no warning, there's just nothing to fix
 
             my $accountline = $accountlines->next;
-            if ($exemptfine) {
-                my $amountoutstanding = $accountline->amountoutstanding;
+            my $payments = $accountline->credits;
 
+            my $amountoutstanding = $accountline->amountoutstanding;
+            if ( $accountline->amount == 0 && $payments->count == 0 ) {
+                $accountline->delete;
+                return 0; # no warning, we've just removed a zero value fine (backdated return)
+            } elsif ($exemptfine && ($amountoutstanding != 0)) {
                 my $account = Koha::Account->new({patron_id => $borrowernumber});
                 my $credit = $account->add_credit(
                     {
@@ -2427,15 +2610,12 @@ sub _FixOverduesOnReturn {
 
                 $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);
             }
 
+            $accountline->status($status);
             return $accountline->store();
         }
     );
@@ -2443,91 +2623,6 @@ sub _FixOverduesOnReturn {
     return $result;
 }
 
-=head2 _FixAccountForLostAndReturned
-
-  &_FixAccountForLostAndReturned($itemnumber, [$borrowernumber, $barcode]);
-
-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 {
-    my $itemnumber     = shift or return;
-    my $borrowernumber = @_ ? shift : undef;
-    my $item_id        = @_ ? shift : $itemnumber;  # Send the barcode if you want that logged in the description
-
-    my $credit;
-
-    # check for charge made for lost book
-    my $accountlines = Koha::Account::Lines->search(
-        {
-            itemnumber      => $itemnumber,
-            debit_type_code => 'LOST',
-            status          => [ undef, { '<>' => 'RETURNED' } ]
-        },
-        {
-            order_by => { -desc => [ 'date', 'accountlines_id' ] }
-        }
-    );
-
-    return unless $accountlines->count > 0;
-    my $accountline     = $accountlines->next;
-    my $total_to_refund = 0;
-
-    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 ) {
-        # some amount has been cancelled. collect the offsets that are not writeoffs
-        # this works because the only way to subtract from this kind of a debt is
-        # using the UI buttons 'Pay' and 'Write off'
-        my $credits_offsets = Koha::Account::Offsets->search({
-            debit_id  => $accountline->id,
-            credit_id => { '!=' => undef }, # it is not the debit itself
-            type      => { '!=' => 'Writeoff' },
-            amount    => { '<'  => 0 } # credits are negative on the DB
-        });
-
-        $total_to_refund = ( $credits_offsets->count > 0 )
-                            ? $credits_offsets->total * -1 # credits are negative on the DB
-                            : 0;
-    }
-
-    my $credit_total = $accountline->amountoutstanding + $total_to_refund;
-
-    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_RETURN',
-                interface   => C4::Context->interface,
-                library_id  => $branchcode
-            }
-        );
-
-        $credit->apply( { debits => [ $accountline ] } );
-    }
-
-    # Update the account status
-    $accountline->discard_changes->status('RETURNED');
-    $accountline->store;
-
-    if ( defined $account and C4::Context->preference('AccountAutoReconcile') ) {
-        $account->reconcile_balance;
-    }
-
-    return ($credit) ? $credit->id : undef;
-}
-
 =head2 _GetCircControlBranch
 
    my $circ_control_branch = _GetCircControlBranch($iteminfos, $borrower);
@@ -2587,50 +2682,6 @@ sub GetOpenIssue {
 
 }
 
-=head2 GetBiblioIssues
-
-  $issues = GetBiblioIssues($biblionumber);
-
-this function get all issues from a biblionumber.
-
-Return:
-C<$issues> is a reference to array which each value is ref-to-hash. This ref-to-hash contains all column from
-tables issues and the firstname,surname & cardnumber from borrowers.
-
-=cut
-
-sub GetBiblioIssues {
-    my $biblionumber = shift;
-    return unless $biblionumber;
-    my $dbh   = C4::Context->dbh;
-    my $query = "
-        SELECT issues.*,items.barcode,biblio.biblionumber,biblio.title, biblio.author,borrowers.cardnumber,borrowers.surname,borrowers.firstname
-        FROM issues
-            LEFT JOIN borrowers ON borrowers.borrowernumber = issues.borrowernumber
-            LEFT JOIN items ON issues.itemnumber = items.itemnumber
-            LEFT JOIN biblioitems ON items.itemnumber = biblioitems.biblioitemnumber
-            LEFT JOIN biblio ON biblio.biblionumber = items.biblionumber
-        WHERE biblio.biblionumber = ?
-        UNION ALL
-        SELECT old_issues.*,items.barcode,biblio.biblionumber,biblio.title, biblio.author,borrowers.cardnumber,borrowers.surname,borrowers.firstname
-        FROM old_issues
-            LEFT JOIN borrowers ON borrowers.borrowernumber = old_issues.borrowernumber
-            LEFT JOIN items ON old_issues.itemnumber = items.itemnumber
-            LEFT JOIN biblioitems ON items.itemnumber = biblioitems.biblioitemnumber
-            LEFT JOIN biblio ON biblio.biblionumber = items.biblionumber
-        WHERE biblio.biblionumber = ?
-        ORDER BY timestamp
-    ";
-    my $sth = $dbh->prepare($query);
-    $sth->execute($biblionumber, $biblionumber);
-
-    my @issues;
-    while ( my $data = $sth->fetchrow_hashref ) {
-        push @issues, $data;
-    }
-    return \@issues;
-}
-
 =head2 GetUpcomingDueIssues
 
   my $upcoming_dues = GetUpcomingDueIssues( { days_in_advance => 4 } );
@@ -2642,19 +2693,15 @@ sub GetUpcomingDueIssues {
 
     $params->{'days_in_advance'} = 7 unless exists $params->{'days_in_advance'};
     my $dbh = C4::Context->dbh;
-
-    my $statement = <<END_SQL;
-SELECT *
-FROM (
-    SELECT issues.*, items.itype as itemtype, items.homebranch, TO_DAYS( date_due )-TO_DAYS( NOW() ) as days_until_due, branches.branchemail
-    FROM issues
-    LEFT JOIN items USING (itemnumber)
-    LEFT OUTER JOIN branches USING (branchcode)
-    WHERE returndate is NULL
-) tmp
-WHERE days_until_due >= 0 AND days_until_due <= ?
-END_SQL
-
+    my $statement;
+    $statement = q{
+        SELECT issues.*, items.itype as itemtype, items.homebranch, TO_DAYS( date_due )-TO_DAYS( NOW() ) as days_until_due, branches.branchemail
+        FROM issues
+        LEFT JOIN items USING (itemnumber)
+        LEFT JOIN branches ON branches.branchcode =
+    };
+    $statement .= $params->{'owning_library'} ? " items.homebranch " : " issues.branchcode ";
+    $statement .= " WHERE returndate is NULL AND TO_DAYS( date_due )-TO_DAYS( NOW() ) BETWEEN 0 AND ?";
     my @bind_parameters = ( $params->{'days_in_advance'} );
     
     my $sth = $dbh->prepare( $statement );
@@ -2689,10 +2736,11 @@ already renewed the loan. $error will contain the reason the renewal can not pro
 =cut
 
 sub CanBookBeRenewed {
-    my ( $borrowernumber, $itemnumber, $override_limit ) = @_;
+    my ( $borrowernumber, $itemnumber, $override_limit, $cron ) = @_;
 
     my $dbh    = C4::Context->dbh;
     my $renews = 1;
+    my $auto_renew = "no";
 
     my $item      = Koha::Items->find($itemnumber)      or return ( 0, 'no_item' );
     my $issue = $item->checkout or return ( 0, 'no_checkout' );
@@ -2701,185 +2749,194 @@ sub CanBookBeRenewed {
 
     my $patron = $issue->patron or return;
 
-    my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber);
+    # 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',
+                    'unseen_renewals_allowed'
+                ]
+            }
+        );
+
+        return ( 0, "too_many" )
+          if not $issuing_rule->{renewalsallowed} or $issuing_rule->{renewalsallowed} <= $issue->renewals;
 
-    # This item can fill one or more unfilled reserve, can those unfilled reserves
-    # all be filled by other available items?
-    if ( $resfound
-        && C4::Context->preference('AllowRenewalIfOtherItemsAvailable') )
-    {
-        my $schema = Koha::Database->new()->schema();
+        return ( 0, "too_unseen" )
+          if C4::Context->preference('UnseenRenewals') &&
+            $issuing_rule->{unseen_renewals_allowed} &&
+            $issuing_rule->{unseen_renewals_allowed} <= $issue->unseen_renewals;
 
-        my $item_holds = $schema->resultset('Reserve')->search( { itemnumber => $itemnumber, found => undef } )->count();
-        if ($item_holds) {
-            # There is an item level hold on this item, no other item can fill the hold
-            $resfound = 1;
+        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');
         }
-        else {
 
-            # Get all other items that could possibly fill reserves
-            my @itemnumbers = $schema->resultset('Item')->search(
-                {
-                    biblionumber => $resrec->{biblionumber},
-                    onloan       => undef,
-                    notforloan   => 0,
-                    -not         => { itemnumber => $itemnumber }
-                },
-                { columns => 'itemnumber' }
-            )->get_column('itemnumber')->all();
+        if ( $issue->auto_renew && $patron->autorenew_checkouts ) {
 
-            # Get all other reserves that could have been filled by this item
-            my @borrowernumbers;
-            while (1) {
-                my ( $reserve_found, $reserve, undef ) =
-                  C4::Reserves::CheckReserves( $itemnumber, undef, undef, \@borrowernumbers );
+            if ( $patron->category->effective_BlockExpiredPatronOpacActions and $patron->is_expired ) {
+                return ( 0, 'auto_account_expired' );
+            }
 
-                if ($reserve_found) {
-                    push( @borrowernumbers, $reserve->{borrowernumber} );
+            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" );
                 }
-                else {
-                    last;
+            }
+            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 the count of the union of the lists of reservable items for each borrower
-            # is equal or greater than the number of borrowers, we know that all reserves
-            # can be filled with available items. We can get the union of the sets simply
-            # by pushing all the elements onto an array and removing the duplicates.
-            my @reservable;
-            my %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;
-                    }
-                    last;
+            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" );
                 }
             }
         }
-    }
-    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::CirculationRules->get_effective_rules(
+        if ( defined $issuing_rule->{norenewalbefore}
+            and $issuing_rule->{norenewalbefore} ne "" )
         {
-            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 ) {
+            # Calculate soonest renewal by subtracting 'No renewal before' from due date
+            my $soonestrenewal = dt_from_string( $issue->date_due, 'sql' )->subtract(
+                $issuing_rule->{lengthunit} => $issuing_rule->{norenewalbefore} );
 
-        if ( $patron->category->effective_BlockExpiredPatronOpacActions and $patron->is_expired ) {
-            return ( 0, 'auto_account_expired' );
-        }
+            # 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 ( 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 ( $soonestrenewal > dt_from_string() )
+            {
+                $auto_renew = ($issue->auto_renew && $patron->autorenew_checkouts) ? "auto_too_soon" : "too_soon";
             }
-        }
-        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" );
+            elsif ( $issue->auto_renew && $patron->autorenew_checkouts ) {
+                $auto_renew = "ok";
             }
         }
 
-        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" );
+        # Fallback for automatic renewals:
+        # If norenewalbefore is undef, don't renew before due date.
+        if ( $issue->auto_renew && $auto_renew eq "no" && $patron->autorenew_checkouts ) {
+            my $now = dt_from_string;
+            if ( $now >= dt_from_string( $issue->date_due, 'sql' ) ){
+                $auto_renew = "ok";
+            } else {
+                $auto_renew = "auto_too_soon";
             }
         }
     }
 
-    if ( defined $issuing_rule->{norenewalbefore}
-        and $issuing_rule->{norenewalbefore} ne "" )
-    {
+    my ( $resfound, $resrec, $possible_reserves ) = C4::Reserves::CheckReserves($itemnumber);
 
-        # Calculate soonest renewal by subtracting 'No renewal before' from due date
-        my $soonestrenewal = dt_from_string( $issue->date_due, 'sql' )->subtract(
-            $issuing_rule->{lengthunit} => $issuing_rule->{norenewalbefore} );
+    # If next hold is non priority, then check if any hold with priority (non_priority = 0) exists for the same biblionumber.
+    if ( $resfound && $resrec->{non_priority} ) {
+        $resfound = Koha::Holds->search(
+            { biblionumber => $resrec->{biblionumber}, non_priority => 0 } )
+          ->count > 0;
+    }
 
-        # 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" );
+
+    # This item can fill one or more unfilled reserve, can those unfilled reserves
+    # all be filled by other available items?
+    if ( $resfound
+        && C4::Context->preference('AllowRenewalIfOtherItemsAvailable') )
+    {
+        my $item_holds = Koha::Holds->search( { itemnumber => $itemnumber, found => undef } )->count();
+        if ($item_holds) {
+            # There is an item level hold on this item, no other item can fill the hold
+            $resfound = 1;
         }
-        elsif ( $issue->auto_renew ) {
-            return ( 0, "auto_renew" );
+        else {
+
+            # Get all other items that could possibly fill reserves
+            my $items = Koha::Items->search({
+                biblionumber => $resrec->{biblionumber},
+                onloan       => undef,
+                notforloan   => 0,
+                -not         => { itemnumber => $itemnumber }
+            });
+
+            # Get all other reserves that could have been filled by this item
+            my @borrowernumbers = map { $_->{borrowernumber} } @$possible_reserves;
+            my $patrons = Koha::Patrons->search({
+                borrowernumber => { -in => \@borrowernumbers }
+            });
+
+            # If the count of the union of the lists of reservable items for each borrower
+            # is equal or greater than the number of borrowers, we know that all reserves
+            # can be filled with available items. We can get the union of the sets simply
+            # by pushing all the elements onto an array and removing the duplicates.
+            my @reservable;
+            ITEM: while ( my $item = $items->next ) {
+                next if IsItemOnHoldAndFound( $item->itemnumber );
+                while ( my $patron = $patrons->next ) {
+                    next unless IsAvailableForItemLevelRequest($item, $patron);
+                    next unless CanItemBeReserved($patron->borrowernumber,$item->itemnumber,undef,{ignore_hold_counts=>1})->{status} eq 'OK';
+                    push @reservable, $item->itemnumber;
+                    if (@reservable >= @borrowernumbers) {
+                        $resfound = 0;
+                        last ITEM;
+                    }
+                    last;
+                }
+                $patrons->reset;
+            }
         }
     }
-
-    # 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" );
+    if( $cron ) { #The cron wants to return 'too_soon' over 'on_reserve'
+        return ( 0, $auto_renew  ) if $auto_renew =~ 'too_soon';#$auto_renew ne "no" && $auto_renew ne "ok";
+        return ( 0, "on_reserve" ) if $resfound;    # '' when no hold was found
+    } else { # For other purposes we want 'on_reserve' before 'too_soon'
+        return ( 0, "on_reserve" ) if $resfound;    # '' when no hold was found
+        return ( 0, $auto_renew  ) if $auto_renew =~ 'too_soon';#$auto_renew ne "no" && $auto_renew ne "ok";
     }
 
+    return ( 0, "auto_renew" ) if $auto_renew eq "ok" && !$override_limit; # 0 if auto-renewal should not succeed
+
     return ( 1, undef );
 }
 
 =head2 AddRenewal
 
-  &AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate]);
+  &AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate], [$seen]);
 
 Renews a loan.
 
@@ -2896,9 +2953,18 @@ 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.
 
+C<$seen> is a boolean flag indicating if the item was seen or not during the renewal. This
+informs the incrementing of the unseen_renewals column. If this flag is not supplied, we
+fallback to a true value
+
 =cut
 
 sub AddRenewal {
@@ -2906,7 +2972,12 @@ 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 $seen            = shift;
+
+    # Fallback on a 'seen' renewal
+    $seen = defined $seen && $seen == 0 ? 0 : 1;
 
     my $item_object   = Koha::Items->find($itemnumber) or return;
     my $biblio = $item_object->biblio;
@@ -2932,7 +3003,7 @@ sub AddRenewal {
     my $schema = Koha::Database->schema;
     $schema->txn_do(sub{
 
-        if ( C4::Context->preference('CalculateFinesOnReturn') ) {
+        if ( !$skipfinecalc && C4::Context->preference('CalculateFinesOnReturn') ) {
             _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed } );
         }
         _FixOverduesOnReturn( $borrowernumber, $itemnumber, undef, 'RENEWED' );
@@ -2945,7 +3016,7 @@ sub AddRenewal {
 
             $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?
                                             dt_from_string( $issue->date_due, 'sql' ) :
-                                            DateTime->now( time_zone => C4::Context->tz());
+                                            dt_from_string();
             $datedue =  CalcDateDue($datedue, $itemtype, $circ_library->branchcode, $patron_unblessed, 'is a renewal');
         }
 
@@ -2959,19 +3030,45 @@ sub AddRenewal {
             }
         );
 
+        # Increment the unseen renewals, if appropriate
+        # We only do so if the syspref is enabled and
+        # a maximum value has been set in the circ rules
+        my $unseen_renewals = $issue->unseen_renewals;
+        if (C4::Context->preference('UnseenRenewals')) {
+            my $rule = Koha::CirculationRules->get_effective_rule(
+                {   categorycode => $patron->categorycode,
+                    itemtype     => $item_object->effective_itemtype,
+                    branchcode   => $circ_library->branchcode,
+                    rule_name    => 'unseen_renewals_allowed'
+                }
+            );
+            if (!$seen && $rule && $rule->rule_value) {
+                $unseen_renewals++;
+            } else {
+                # If the renewal is seen, unseen should revert to 0
+                $unseen_renewals = 0;
+            }
+        }
+
         # 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=?"
-        );
+        my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, unseen_renewals = ?, lastreneweddate = ? WHERE issue_id = ?");
 
-        $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $lastreneweddate, $borrowernumber, $itemnumber );
+        eval{
+            $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $unseen_renewals, $lastreneweddate, $issue->issue_id );
+        };
+        if( $sth->err ){
+            Koha::Exceptions::Checkout::FailedRenewal->throw(
+                error => 'Update of issue# ' . $issue->issue_id . ' failed with error: ' . $sth->errstr
+            );
+        }
 
         # Update the renewal count on the item, and tell zebra to reindex
         $renews = ( $item_object->renewals || 0 ) + 1;
-        ModItem( { renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item_object->biblionumber, $itemnumber, { log_action => 0 } );
+        $item_object->renewals($renews);
+        $item_object->onloan($datedue);
+        $item_object->store({ log_action => 0 });
 
         # Charge a new rental fee, if applicable
         my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
@@ -3019,14 +3116,10 @@ sub AddRenewal {
             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 && defined C4::Context->userenv->{branch} ) ? C4::Context->userenv->{branch} : $branch;
-        }
-
         # Add the renewal to stats
         UpdateStats(
             {
-                branch         => $branch,
+                branch         => $item_object->renewal_branchcode({branch => $branch}),
                 type           => 'renew',
                 amount         => $charge,
                 itemnumber     => $itemnumber,
@@ -3039,6 +3132,13 @@ sub AddRenewal {
 
         #Log the renewal
         logaction("CIRCULATION", "RENEWAL", $borrowernumber, $itemnumber) if C4::Context->preference("RenewalLog");
+
+        Koha::Plugins->call('after_circ_action', {
+            action  => 'renewal',
+            payload => {
+                checkout  => $issue->get_from_storage
+            }
+        });
     });
 
     return $datedue;
@@ -3049,13 +3149,16 @@ sub GetRenewCount {
     my ( $bornum, $itemno ) = @_;
     my $dbh           = C4::Context->dbh;
     my $renewcount    = 0;
+    my $unseencount    = 0;
     my $renewsallowed = 0;
+    my $unseenallowed = 0;
     my $renewsleft    = 0;
+    my $unseenleft    = 0;
 
     my $patron = Koha::Patrons->find( $bornum );
     my $item   = Koha::Items->find($itemno);
 
-    return (0, 0, 0) unless $patron or $item; # Wrong call, no renewal allowed
+    return (0, 0, 0, 0, 0, 0) unless $patron or $item; # Wrong call, no renewal allowed
 
     # Look in the issues table for this item, lent to this borrower,
     # and not yet returned.
@@ -3069,22 +3172,34 @@ sub GetRenewCount {
     $sth->execute( $bornum, $itemno );
     my $data = $sth->fetchrow_hashref;
     $renewcount = $data->{'renewals'} if $data->{'renewals'};
+    $unseencount = $data->{'unseen_renewals'} if $data->{'unseen_renewals'};
     # $item and $borrower should be calculated
     my $branchcode = _GetCircControlBranch($item->unblessed, $patron->unblessed);
 
-    my $rule = Koha::CirculationRules->get_effective_rule(
+    my $rules = Koha::CirculationRules->get_effective_rules(
         {
             categorycode => $patron->categorycode,
             itemtype     => $item->effective_itemtype,
             branchcode   => $branchcode,
-            rule_name    => 'renewalsallowed',
+            rules        => [ 'renewalsallowed', 'unseen_renewals_allowed' ]
         }
     );
-
-    $renewsallowed = $rule ? $rule->rule_value : 0;
+    $renewsallowed = $rules ? $rules->{renewalsallowed} : 0;
+    $unseenallowed = $rules->{unseen_renewals_allowed} ?
+        $rules->{unseen_renewals_allowed} :
+        0;
     $renewsleft    = $renewsallowed - $renewcount;
+    $unseenleft    = $unseenallowed - $unseencount;
     if($renewsleft < 0){ $renewsleft = 0; }
-    return ( $renewcount, $renewsallowed, $renewsleft );
+    if($unseenleft < 0){ $unseenleft = 0; }
+    return (
+        $renewcount,
+        $renewsallowed,
+        $renewsleft,
+        $unseencount,
+        $unseenallowed,
+        $unseenleft
+    );
 }
 
 =head2 GetSoonestRenewDate
@@ -3257,12 +3372,17 @@ sub GetIssuingCharges {
     if ( my $item_data = $sth->fetchrow_hashref ) {
         $item_type = $item_data->{itemtype};
         $charge    = $item_data->{rentalcharge};
+        # FIXME This should follow CircControl
         my $branch = C4::Context::mybranch();
         my $patron = Koha::Patrons->find( $borrowernumber );
-        my $discount = _get_discount_from_rule($patron->categorycode, $branch, $item_type);
+        my $discount = Koha::CirculationRules->get_effective_rule({
+            categorycode => $patron->categorycode,
+            branchcode   => $branch,
+            itemtype     => $item_type,
+            rule_name    => 'rentaldiscount'
+        });
         if ($discount) {
-            # We may have multiple rules so get the most specific
-            $charge = ( $charge * ( 100 - $discount ) ) / 100;
+            $charge = ( $charge * ( 100 - $discount->rule_value ) ) / 100;
         }
         if ($charge) {
             $charge = sprintf '%.2f', $charge; # ensure no fractions of a penny returned
@@ -3272,49 +3392,6 @@ sub GetIssuingCharges {
     return ( $charge, $item_type );
 }
 
-# Select most appropriate discount rule from those returned
-sub _get_discount_from_rule {
-    my ($categorycode, $branchcode, $itemtype) = @_;
-
-    # 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, $type )
@@ -3356,10 +3433,13 @@ sub GetTransfers {
         SELECT datesent,
                frombranch,
                tobranch,
-               branchtransfer_id
+               branchtransfer_id,
+               daterequested,
+               reason
         FROM branchtransfers
         WHERE itemnumber = ?
           AND datearrived IS NULL
+          AND datecancelled IS NULL
         ';
     my $sth = $dbh->prepare($query);
     $sth->execute($itemnumber);
@@ -3384,6 +3464,8 @@ sub GetTransfersFromTo {
         FROM   branchtransfers
         WHERE  frombranch=?
           AND  tobranch=?
+          AND datecancelled IS NULL
+          AND datesent IS NOT NULL
           AND datearrived IS NULL
     ";
     my $sth = $dbh->prepare($query);
@@ -3396,24 +3478,6 @@ sub GetTransfersFromTo {
     return (@gettransfers);
 }
 
-=head2 DeleteTransfer
-
-  &DeleteTransfer($itemnumber);
-
-=cut
-
-sub DeleteTransfer {
-    my ($itemnumber) = @_;
-    return unless $itemnumber;
-    my $dbh          = C4::Context->dbh;
-    my $sth          = $dbh->prepare(
-        "DELETE FROM branchtransfers
-         WHERE itemnumber=?
-         AND datearrived IS NULL "
-    );
-    return $sth->execute($itemnumber);
-}
-
 =head2 SendCirculationAlert
 
 Send out a C<check-in> or C<checkout> alert using the messaging system.
@@ -3473,7 +3537,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 (
@@ -3492,7 +3556,6 @@ sub SendCirculationAlert {
             }
         ) or next;
 
-        $schema->storage->txn_begin;
         C4::Context->dbh->do(q|LOCK TABLE message_queue READ|) unless $do_not_lock;
         C4::Context->dbh->do(q|LOCK TABLE message_queue WRITE|) unless $do_not_lock;
         my $message = C4::Message->find_last_message($borrower, $type, $mtt);
@@ -3504,7 +3567,6 @@ sub SendCirculationAlert {
             $message->update;
         }
         C4::Context->dbh->do(q|UNLOCK TABLES|) unless $do_not_lock;
-        $schema->storage->txn_commit;
     }
 
     return;
@@ -3520,32 +3582,24 @@ This function validate the line of brachtransfer but with the wrong destination
 
 sub updateWrongTransfer {
        my ( $itemNumber,$waitingAtLibrary,$FromLibrary ) = @_;
-       my $dbh = C4::Context->dbh;     
-# first step validate the actual line of transfert .
-       my $sth =
-               $dbh->prepare(
-                       "update branchtransfers set datearrived = now(),tobranch=?,comments='wrongtransfer' where itemnumber= ? AND datearrived IS NULL"
-               );
-               $sth->execute($FromLibrary,$itemNumber);
-
-# second step create a new line of branchtransfer to the right location .
-       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
+    # first step: cancel the original transfer
+    my $item = Koha::Items->find($itemNumber);
+    my $transfer = $item->get_transfer;
+    $transfer->set({ datecancelled => dt_from_string, cancellation_reason => 'WrongTransfer' })->store();
 
-=cut
+    # second step: create a new transfer to the right location
+    my $new_transfer = $item->request_transfer(
+        {
+            to            => $transfer->to_library,
+            reason        => $transfer->reason,
+            comment       => $transfer->comments,
+            ignore_limits => 1,
+            enqueue       => 1
+        }
+    );
 
-sub UpdateHoldingbranch {
-       my ( $branch,$itemnumber ) = @_;
-    ModItem({ holdingbranch => $branch }, undef, $itemnumber);
+    return $new_transfer;
 }
 
 =head2 CalcDateDue
@@ -3553,7 +3607,7 @@ sub UpdateHoldingbranch {
 $newdatedue = CalcDateDue($startdate,$itemtype,$branchcode,$borrower);
 
 this function calculates the due date given the start date and configured circulation rules,
-checking against the holidays calendar as per the 'useDaysMode' syspref.
+checking against the holidays calendar as per the daysmode circulation rule.
 C<$startdate>   = DateTime object representing start date of loan period (assumed to be today)
 C<$itemtype>  = itemtype code of item in question
 C<$branch>  = location whose calendar to use
@@ -3583,14 +3637,20 @@ sub CalcDateDue {
             $datedue = $startdate->clone;
         }
     } else {
-        $datedue =
-          DateTime->now( time_zone => C4::Context->tz() )
-          ->truncate( to => 'minute' );
+        $datedue = dt_from_string()->truncate( to => 'minute' );
     }
 
 
+    my $daysmode = Koha::CirculationRules->get_effective_daysmode(
+        {
+            categorycode => $borrower->{categorycode},
+            itemtype     => $itemtype,
+            branchcode   => $branch,
+        }
+    );
+
     # calculate the datedue as normal
-    if ( C4::Context->preference('useDaysMode') eq 'Days' )
+    if ( $daysmode eq 'Days' )
     {    # ignoring calendar
         if ( $loanlength->{lengthunit} eq 'hours' ) {
             $datedue->add( hours => $loanlength->{$length_key} );
@@ -3607,8 +3667,8 @@ sub CalcDateDue {
         else { # days
             $dur = DateTime::Duration->new( days => $loanlength->{$length_key});
         }
-        my $calendar = Koha::Calendar->new( branchcode => $branch );
-        $datedue = $calendar->addDate( $datedue, $dur, $loanlength->{lengthunit} );
+        my $calendar = Koha::Calendar->new( branchcode => $branch, days_mode => $daysmode );
+        $datedue = $calendar->addDuration( $datedue, $dur, $loanlength->{lengthunit} );
         if ($loanlength->{lengthunit} eq 'days') {
             $datedue->set_hour(23);
             $datedue->set_minute(59);
@@ -3645,8 +3705,8 @@ sub CalcDateDue {
                 $datedue = $expiry_dt->clone->set_time_zone( C4::Context->tz );
             }
         }
-        if ( C4::Context->preference('useDaysMode') ne 'Days' ) {
-          my $calendar = Koha::Calendar->new( branchcode => $branch );
+        if ( $daysmode ne 'Days' ) {
+          my $calendar = Koha::Calendar->new( branchcode => $branch, days_mode => $daysmode );
           if ( $calendar->is_holiday($datedue) ) {
               # Don't return on a closed day
               $datedue = $calendar->prev_open_days( $datedue, 1 );
@@ -3750,9 +3810,23 @@ sub ReturnLostItem{
     MarkIssueReturned( $borrowernumber, $itemnum );
 }
 
+=head2 LostItem
+
+  LostItem( $itemnumber, $mark_lost_from, $force_mark_returned, [$params] );
+
+The final optional parameter, C<$params>, expected to contain
+'skip_record_index' key, which relayed down to Koha::Item/store,
+there it prevents calling of ModZebra index_records,
+which takes most of the time in batch adds/deletes: index_records better
+to be called later in C<additem.pl> after the whole loop.
+
+$params:
+    skip_record_index => 1|0
+
+=cut
 
 sub LostItem{
-    my ($itemnumber, $mark_lost_from, $force_mark_returned) = @_;
+    my ($itemnumber, $mark_lost_from, $force_mark_returned, $params) = @_;
 
     unless ( $mark_lost_from ) {
         # Temporary check to avoid regressions
@@ -3798,14 +3872,15 @@ sub LostItem{
             #warn " $issues->{'borrowernumber'}  /  $itemnumber ";
         }
 
-        MarkIssueReturned($borrowernumber,$itemnumber,undef,$patron->privacy) if $mark_returned;
+        MarkIssueReturned($borrowernumber,$itemnumber,undef,$patron->privacy,$params) if $mark_returned;
     }
 
-    #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);
+    # When an item is marked as lost, we should automatically cancel its outstanding transfers.
+    my $item = Koha::Items->find($itemnumber);
+    my $transfers = $item->get_transfers;
+    while (my $transfer = $transfers->next) {
+        $transfer->cancel({ reason => 'ItemLost', force => 1 });
     }
-    my $transferdeleted = DeleteTransfer($itemnumber);
 }
 
 sub GetOfflineOperations {
@@ -3866,17 +3941,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.";
@@ -4072,7 +4146,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);
@@ -4177,6 +4251,10 @@ sub GetTopIssues {
     return @$rows;
 }
 
+=head2 Internal methods
+
+=cut
+
 sub _CalculateAndUpdateFine {
     my ($params) = @_;
 
@@ -4253,7 +4331,6 @@ sub _item_denied_renewal {
     return 0;
 }
 
-
 1;
 
 __END__