Bug 24394: Typo when adding a new cash register
[koha-ffzg.git] / C4 / Circulation.pm
index 971268c..d250627 100644 (file)
@@ -18,9 +18,7 @@ package C4::Circulation;
 # You should have received a copy of the GNU General Public License
 # along with Koha; if not, see <http://www.gnu.org/licenses>.
 
-
-use strict;
-#use warnings; FIXME - Bug 2505
+use Modern::Perl;
 use DateTime;
 use POSIX qw( floor );
 use Koha::DateUtils;
@@ -54,11 +52,13 @@ use Koha::Database;
 use Koha::Libraries;
 use Koha::Account::Lines;
 use Koha::Holds;
-use Koha::RefundLostItemFeeRule;
 use Koha::RefundLostItemFeeRules;
 use Koha::Account::Lines;
 use Koha::Account::Offsets;
 use Koha::Config::SysPrefs;
+use Koha::Charges::Fees;
+use Koha::Util::SystemPreferences;
+use Koha::Checkouts::ReturnClaims;
 use Carp;
 use List::MoreUtils qw( uniq any );
 use Scalar::Util qw( looks_like_number );
@@ -313,10 +313,10 @@ sub transferbook {
     unless ( $item ) {
         $messages->{'BadBarcode'} = $barcode;
         $dotransfer = 0;
+        return ( $dotransfer, $messages );
     }
 
     my $itemnumber = $item->itemnumber;
-    my $issue = GetOpenIssue($itemnumber);
     # get branches of book...
     my $hbr = $item->homebranch;
     my $fbr = $item->holdingbranch;
@@ -342,6 +342,7 @@ sub transferbook {
     }
 
     # check if it is still issued to someone, return it...
+    my $issue = Koha::Checkouts->find({ itemnumber => $itemnumber });
     if ( $issue ) {
         AddReturn( $barcode, $fbr );
         $messages->{'WasReturned'} = $issue->borrowernumber;
@@ -373,8 +374,7 @@ sub transferbook {
 
 sub TooMany {
     my $borrower        = shift;
-    my $biblionumber = shift;
-       my $item                = shift;
+    my $item_object = shift;
     my $params = shift;
     my $onsite_checkout = $params->{onsite_checkout} || 0;
     my $switch_onsite_checkout = $params->{switch_onsite_checkout} || 0;
@@ -382,17 +382,25 @@ sub TooMany {
     my $dbh             = C4::Context->dbh;
        my $branch;
        # Get which branchcode we need
-       $branch = _GetCircControlBranch($item,$borrower);
-       my $type = (C4::Context->preference('item-level_itypes')) 
-                       ? $item->{'itype'}         # item-level
-                       : $item->{'itemtype'};     # biblio-level
+    $branch = _GetCircControlBranch($item_object->unblessed,$borrower);
+    my $type = $item_object->effective_itemtype;
+
     # given branch, patron category, and item type, determine
     # applicable issuing rule
-    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
-        {   categorycode => $cat_borrower,
+    my $maxissueqty_rule = Koha::CirculationRules->get_effective_rule(
+        {
+            categorycode => $cat_borrower,
+            itemtype     => $type,
+            branchcode   => $branch,
+            rule_name    => 'maxissueqty',
+        }
+    );
+    my $maxonsiteissueqty_rule = Koha::CirculationRules->get_effective_rule(
+        {
+            categorycode => $cat_borrower,
             itemtype     => $type,
-            branchcode   => $branch
+            branchcode   => $branch,
+            rule_name    => 'maxonsiteissueqty',
         }
     );
 
@@ -400,7 +408,7 @@ sub TooMany {
     # if a rule is found and has a loan limit set, count
     # how many loans the patron already has that meet that
     # rule
-    if (defined($issuing_rule) and defined($issuing_rule->maxissueqty)) {
+    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
@@ -408,8 +416,8 @@ sub TooMany {
             JOIN items USING (itemnumber)
         |;
 
-        my $rule_itemtype = $issuing_rule->itemtype;
-        if ($rule_itemtype eq "*") {
+        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
@@ -429,8 +437,8 @@ sub TooMany {
                                     AND   itemtype <> '*'
                                   ) ";
             }
-            push @bind_params, $issuing_rule->branchcode;
-            push @bind_params, $issuing_rule->categorycode;
+            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
@@ -446,23 +454,23 @@ sub TooMany {
 
         $count_query .= " AND borrowernumber = ? ";
         push @bind_params, $borrower->{'borrowernumber'};
-        my $rule_branch = $issuing_rule->branchcode;
-        if ($rule_branch ne "*") {
+        my $rule_branch = $maxissueqty_rule->branchcode;
+        if ($rule_branch) {
             if (C4::Context->preference('CircControl') eq 'PickupLibrary') {
                 $count_query .= " AND issues.branchcode = ? ";
-                push @bind_params, $branch;
+                push @bind_params, $rule_branch;
             } elsif (C4::Context->preference('CircControl') eq 'PatronLibrary') {
                 ; # if branch is the patron's home branch, then count all loans by patron
             } else {
                 $count_query .= " AND items.homebranch = ? ";
-                push @bind_params, $branch;
+                push @bind_params, $rule_branch;
             }
         }
 
         my ( $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $count_query, {}, @bind_params );
 
-        my $max_checkouts_allowed = $issuing_rule->maxissueqty;
-        my $max_onsite_checkouts_allowed = $issuing_rule->maxonsiteissueqty;
+        my $max_checkouts_allowed = $maxissueqty_rule ? $maxissueqty_rule->rule_value : undef;
+        my $max_onsite_checkouts_allowed = $maxonsiteissueqty_rule ? $maxonsiteissueqty_rule->rule_value : undef;
 
         if ( $onsite_checkout and defined $max_onsite_checkouts_allowed ) {
             if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
@@ -495,7 +503,7 @@ sub TooMany {
 
     # 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->{maxissueqty})) {
+    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
@@ -515,10 +523,10 @@ sub TooMany {
             push @bind_params, $branch;
         }
         my ( $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $branch_count_query, {}, @bind_params );
-        my $max_checkouts_allowed = $branch_borrower_circ_rule->{maxissueqty};
-        my $max_onsite_checkouts_allowed = $branch_borrower_circ_rule->{maxonsiteissueqty};
+        my $max_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxissueqty};
+        my $max_onsite_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxonsiteissueqty};
 
-        if ( $onsite_checkout and defined $max_onsite_checkouts_allowed ) {
+        if ( $onsite_checkout and $max_onsite_checkouts_allowed ne '' ) {
             if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
                 return {
                     reason => 'TOO_MANY_ONSITE_CHECKOUTS',
@@ -547,7 +555,7 @@ sub TooMany {
         }
     }
 
-    if ( not defined( $issuing_rule ) and not defined($branch_borrower_circ_rule->{maxissueqty}) ) {
+    if ( not defined( $maxissueqty_rule ) and not defined($branch_borrower_circ_rule->{patron_maxissueqty}) ) {
         return { reason => 'NO_RULE_DEFINED', max_allowed => 0 };
     }
 
@@ -670,20 +678,24 @@ sub CanBookBeIssued {
     my $onsite_checkout     = $params->{onsite_checkout}     || 0;
     my $override_high_holds = $params->{override_high_holds} || 0;
 
-    my $item = GetItem(undef, $barcode );
+    my $item_object = Koha::Items->find({barcode => $barcode });
+
     # MANDATORY CHECKS - unless item exists, nothing else matters
-    unless ( $item ) {
+    unless ( $item_object ) {
         $issuingimpossible{UNKNOWN_BARCODE} = 1;
     }
     return ( \%issuingimpossible, \%needsconfirmation ) if %issuingimpossible;
 
-    my $issue = Koha::Checkouts->find( { itemnumber => $item->{itemnumber} } );
-    my $biblio = Koha::Biblios->find( $item->{biblionumber} );
+    my $item_unblessed = $item_object->unblessed; # Transition...
+    my $issue = $item_object->checkout;
+    my $biblio = $item_object->biblio;
+
     my $biblioitem = $biblio->biblioitem;
-    my $effective_itemtype = $item->{itype}; # GetItem deals with that
+    my $effective_itemtype = $item_object->effective_itemtype;
     my $dbh             = C4::Context->dbh;
     my $patron_unblessed = $patron->unblessed;
 
+    my $circ_library = Koha::Libraries->find( _GetCircControlBranch($item_unblessed, $patron_unblessed) );
     #
     # DUE DATE is OK ? -- should already have checked.
     #
@@ -694,12 +706,21 @@ sub CanBookBeIssued {
     unless ( $duedate ) {
         my $issuedate = $now->clone();
 
-        my $branch = _GetCircControlBranch($item, $patron_unblessed);
-        $duedate = CalcDateDue( $issuedate, $effective_itemtype, $branch, $patron_unblessed );
+        $duedate = CalcDateDue( $issuedate, $effective_itemtype, $circ_library->branchcode, $patron_unblessed );
 
         # Offline circ calls AddIssue directly, doesn't run through here
         #  So issuingimpossible should be ok.
     }
+
+    my $fees = Koha::Charges::Fees->new(
+        {
+            patron    => $patron,
+            library   => $circ_library,
+            item      => $item_object,
+            to_date   => $duedate,
+        }
+    );
+
     if ($duedate) {
         my $today = $now->clone();
         $today->truncate( to => 'minute');
@@ -713,25 +734,25 @@ sub CanBookBeIssued {
     #
     # BORROWER STATUS
     #
-    if ( $patron->category->category_type eq 'X' && (  $item->{barcode}  )) {
+    if ( $patron->category->category_type eq 'X' && (  $item_object->barcode  )) {
        # stats only borrower -- add entry to statistics table, and return issuingimpossible{STATS} = 1  .
         &UpdateStats({
                      branch => C4::Context->userenv->{'branch'},
                      type => 'localuse',
-                     itemnumber => $item->{'itemnumber'},
+                     itemnumber => $item_object->itemnumber,
                      itemtype => $effective_itemtype,
                      borrowernumber => $patron->borrowernumber,
-                     ccode => $item->{'ccode'}}
+                     ccode => $item_object->ccode}
                     );
-        ModDateLastSeen( $item->{'itemnumber'} );
+        ModDateLastSeen( $item_object->itemnumber ); # FIXME Move to Koha::Item
         return( { STATS => 1 }, {});
     }
 
-    if ( $patron->gonenoaddress == 1 ) {
+    if ( $patron->gonenoaddress && $patron->gonenoaddress == 1 ) {
         $issuingimpossible{GNA} = 1;
     }
 
-    if ( $patron->lost == 1 ) {
+    if ( $patron->lost && $patron->lost == 1 ) {
         $issuingimpossible{CARD_LOST} = 1;
     }
     if ( $patron->is_debarred ) {
@@ -760,7 +781,7 @@ sub CanBookBeIssued {
     my $no_issues_charge_guarantees = C4::Context->preference("NoIssuesChargeGuarantees");
     $no_issues_charge_guarantees = undef unless looks_like_number( $no_issues_charge_guarantees );
     if ( defined $no_issues_charge_guarantees ) {
-        my @guarantees = $patron->guarantees();
+        my @guarantees = map { $_->guarantee } $patron->guarantee_relationships();
         my $guarantees_non_issues_charges;
         foreach my $g ( @guarantees ) {
             $guarantees_non_issues_charges += $g->account->non_issues_charges;
@@ -834,7 +855,7 @@ sub CanBookBeIssued {
         } else {
             my ($CanBookBeRenewed,$renewerror) = CanBookBeRenewed(
                 $patron->borrowernumber,
-                $item->{'itemnumber'},
+                $item_object->itemnumber,
             );
             if ( $CanBookBeRenewed == 0 ) {    # no more renewals allowed
                 if ( $renewerror eq 'onsite_checkout' ) {
@@ -855,17 +876,21 @@ sub CanBookBeIssued {
 
         my $patron = Koha::Patrons->find( $issue->borrowernumber );
 
-        my ( $can_be_returned, $message ) = CanBookBeReturned( $item, C4::Context->userenv->{branch} );
+        my ( $can_be_returned, $message ) = CanBookBeReturned( $item_unblessed, C4::Context->userenv->{branch} );
 
         unless ( $can_be_returned ) {
             $issuingimpossible{RETURN_IMPOSSIBLE} = 1;
             $issuingimpossible{branch_to_return} = $message;
         } else {
+            if ( C4::Context->preference('AutoReturnCheckedOutItems') ) {
+                $alerts{RETURNED_FROM_ANOTHER} = { patron => $patron };
+            } else {
             $needsconfirmation{ISSUED_TO_ANOTHER} = 1;
             $needsconfirmation{issued_firstname} = $patron->firstname;
             $needsconfirmation{issued_surname} = $patron->surname;
             $needsconfirmation{issued_cardnumber} = $patron->cardnumber;
             $needsconfirmation{issued_borrowernumber} = $patron->borrowernumber;
+            }
         }
     }
 
@@ -876,7 +901,7 @@ sub CanBookBeIssued {
       and $issue
       and $issue->onsite_checkout
       and $issue->borrowernumber == $patron->borrowernumber ? 1 : 0 );
-    my $toomany = TooMany( $patron_unblessed, $item->{biblionumber}, $item, { onsite_checkout => $onsite_checkout, switch_onsite_checkout => $switch_onsite_checkout, } );
+    my $toomany = TooMany( $patron_unblessed, $item_object, { onsite_checkout => $onsite_checkout, switch_onsite_checkout => $switch_onsite_checkout, } );
     # if TooMany max_allowed returns 0 the user doesn't have permission to check out this book
     if ( $toomany && not exists $needsconfirmation{RENEW_ISSUE} ) {
         if ( $toomany->{max_allowed} == 0 ) {
@@ -899,19 +924,19 @@ sub CanBookBeIssued {
     $patron = Koha::Patrons->find( $patron->borrowernumber ); # FIXME Refetch just in case, to avoid regressions. But must not be needed
     my $wants_check = $patron->wants_check_for_previous_checkout;
     $needsconfirmation{PREVISSUE} = 1
-        if ($wants_check and $patron->do_check_for_previous_checkout($item));
+        if ($wants_check and $patron->do_check_for_previous_checkout($item_unblessed));
 
     #
     # ITEM CHECKING
     #
-    if ( $item->{'notforloan'} )
+    if ( $item_object->notforloan )
     {
         if(!C4::Context->preference("AllowNotForLoanOverride")){
             $issuingimpossible{NOT_FOR_LOAN} = 1;
-            $issuingimpossible{item_notforloan} = $item->{'notforloan'};
+            $issuingimpossible{item_notforloan} = $item_object->notforloan;
         }else{
             $needsconfirmation{NOT_FOR_LOAN_FORCING} = 1;
-            $needsconfirmation{item_notforloan} = $item->{'notforloan'};
+            $needsconfirmation{item_notforloan} = $item_object->notforloan;
         }
     }
     else {
@@ -944,17 +969,17 @@ sub CanBookBeIssued {
             }
         }
     }
-    if ( $item->{'withdrawn'} && $item->{'withdrawn'} > 0 )
+    if ( $item_object->withdrawn && $item_object->withdrawn > 0 )
     {
         $issuingimpossible{WTHDRAWN} = 1;
     }
-    if (   $item->{'restricted'}
-        && $item->{'restricted'} == 1 )
+    if (   $item_object->restricted
+        && $item_object->restricted == 1 )
     {
         $issuingimpossible{RESTRICTED} = 1;
     }
-    if ( $item->{'itemlost'} && C4::Context->preference("IssueLostItem") ne 'nothing' ) {
-        my $av = Koha::AuthorisedValues->search({ category => 'LOST', authorised_value => $item->{itemlost} });
+    if ( $item_object->itemlost && C4::Context->preference("IssueLostItem") ne 'nothing' ) {
+        my $av = Koha::AuthorisedValues->search({ category => 'LOST', authorised_value => $item_object->itemlost });
         my $code = $av->count ? $av->next->lib : '';
         $needsconfirmation{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'confirm' );
         $alerts{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'alert' );
@@ -962,29 +987,39 @@ sub CanBookBeIssued {
     if ( C4::Context->preference("IndependentBranches") ) {
         my $userenv = C4::Context->userenv;
         unless ( C4::Context->IsSuperLibrarian() ) {
-            if ( $item->{C4::Context->preference("HomeOrHoldingBranch")} ne $userenv->{branch} ){
+            my $HomeOrHoldingBranch = C4::Context->preference("HomeOrHoldingBranch");
+            if ( $item_object->$HomeOrHoldingBranch ne $userenv->{branch} ){
                 $issuingimpossible{ITEMNOTSAMEBRANCH} = 1;
-                $issuingimpossible{'itemhomebranch'} = $item->{C4::Context->preference("HomeOrHoldingBranch")};
+                $issuingimpossible{'itemhomebranch'} = $item_object->$HomeOrHoldingBranch;
             }
             $needsconfirmation{BORRNOTSAMEBRANCH} = $patron->branchcode
               if ( $patron->branchcode ne $userenv->{branch} );
         }
     }
+
     #
     # CHECK IF THERE IS RENTAL CHARGES. RENTAL MUST BE CONFIRMED BY THE BORROWER
     #
     my $rentalConfirmation = C4::Context->preference("RentalFeesCheckoutConfirmation");
+    if ($rentalConfirmation) {
+        my ($rentalCharge) = GetIssuingCharges( $item_object->itemnumber, $patron->borrowernumber );
+
+        my $itemtype_object = Koha::ItemTypes->find( $item_object->effective_itemtype );
+        if ($itemtype_object) {
+            my $accumulate_charge = $fees->accumulate_rentalcharge();
+            if ( $accumulate_charge > 0 ) {
+                $rentalCharge += $accumulate_charge;
+            }
+        }
 
-    if ( $rentalConfirmation ){
-        my ($rentalCharge) = GetIssuingCharges( $item->{'itemnumber'}, $patron->borrowernumber );
-        if ( $rentalCharge > 0 ){
+        if ( $rentalCharge > 0 ) {
             $needsconfirmation{RENTALCHARGE} = $rentalCharge;
         }
     }
 
     unless ( $ignore_reserves ) {
         # See if the item is on reserve.
-        my ( $restype, $res ) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
+        my ( $restype, $res ) = C4::Reserves::CheckReserves( $item_object->itemnumber );
         if ($restype) {
             my $resbor = $res->{'borrowernumber'};
             if ( $resbor ne $patron->borrowernumber ) {
@@ -1029,7 +1064,7 @@ sub CanBookBeIssued {
 
     ## check for high holds decreasing loan period
     if ( C4::Context->preference('decreaseLoanHighHolds') ) {
-        my $check = checkHighHolds( $item, $patron_unblessed );
+        my $check = checkHighHolds( $item_unblessed, $patron_unblessed );
 
         if ( $check->{exceeded} ) {
             if ($override_high_holds) {
@@ -1058,7 +1093,7 @@ sub CanBookBeIssued {
     ) {
         # Check if borrower has already issued an item from the same biblio
         # Only if it's not a subscription
-        my $biblionumber = $item->{biblionumber};
+        my $biblionumber = $item_object->biblionumber;
         require C4::Serials;
         my $is_a_subscription = C4::Serials::CountSubscriptionFromBiblionumber($biblionumber);
         unless ($is_a_subscription) {
@@ -1091,7 +1126,7 @@ Check whether the item can be returned to the provided branch
 
 =over 4
 
-=item C<$item> is a hash of item information as returned from GetItem
+=item C<$item> is a hash of item information as returned Koha::Items->find->unblessed (Temporary, should be a Koha::Item instead)
 
 =item C<$branch> is the branchcode where the return is taking place
 
@@ -1142,7 +1177,7 @@ sub CanBookBeReturned {
 
 sub checkHighHolds {
     my ( $item, $borrower ) = @_;
-    my $branch = _GetCircControlBranch( $item, $borrower );
+    my $branchcode = _GetCircControlBranch( $item, $borrower );
     my $item_object = Koha::Items->find( $item->{itemnumber} );
 
     my $return_data = {
@@ -1177,7 +1212,7 @@ sub checkHighHolds {
             # dynamic means X more than the number of holdable items on the record
 
             # let's get the items
-            my @items = $holds->next()->biblio()->items();
+            my @items = $holds->next()->biblio()->items()->as_list;
 
             # Remove any items with status defined to be ignored even if the would not make item unholdable
             foreach my $status (@decreaseLoanHighHoldsIgnoreStatuses) {
@@ -1200,10 +1235,10 @@ sub checkHighHolds {
 
         my $issuedate = DateTime->now( time_zone => C4::Context->tz() );
 
-        my $calendar = Koha::Calendar->new( branchcode => $branch );
+        my $calendar = Koha::Calendar->new( branchcode => $branchcode );
 
         my $itype = $item_object->effective_itemtype;
-        my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branch, $borrower );
+        my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branchcode, $borrower );
 
         my $decreaseLoanHighHoldsDuration = C4::Context->preference('decreaseLoanHighHoldsDuration');
 
@@ -1289,40 +1324,58 @@ sub AddIssue {
     # Stop here if the patron or barcode doesn't exist
     if ( $borrower && $barcode && $barcodecheck ) {
         # find which item we issue
-        my $item = GetItem( '', $barcode )
+        my $item_object = Koha::Items->find({ barcode => $barcode })
           or return;    # if we don't get an Item, abort.
-        my $item_object = Koha::Items->find( { barcode => $barcode } );
+        my $item_unblessed = $item_object->unblessed;
 
-        my $branch = _GetCircControlBranch( $item, $borrower );
+        my $branchcode = _GetCircControlBranch( $item_unblessed, $borrower );
 
         # get actual issuing if there is one
-        my $actualissue = Koha::Checkouts->find( { itemnumber => $item->{itemnumber} } );
+        my $actualissue = $item_object->checkout;
 
         # check if we just renew the issue.
         if ( $actualissue and $actualissue->borrowernumber eq $borrower->{'borrowernumber'}
                 and not $switch_onsite_checkout ) {
             $datedue = AddRenewal(
                 $borrower->{'borrowernumber'},
-                $item->{'itemnumber'},
-                $branch,
+                $item_object->itemnumber,
+                $branchcode,
                 $datedue,
                 $issuedate,    # here interpreted as the renewal date
             );
         }
         else {
+            unless ($datedue) {
+                my $itype = $item_object->effective_itemtype;
+                $datedue = CalcDateDue( $issuedate, $itype, $branchcode, $borrower );
+
+            }
+            $datedue->truncate( to => 'minute' );
+
+            my $patron = Koha::Patrons->find( $borrower );
+            my $library = Koha::Libraries->find( $branchcode );
+            my $fees = Koha::Charges::Fees->new(
+                {
+                    patron    => $patron,
+                    library   => $library,
+                    item      => $item_object,
+                    to_date   => $datedue,
+                }
+            );
+
             # it's NOT a renewal
             if ( $actualissue and not $switch_onsite_checkout ) {
                 # This book is currently on loan, but not to the person
                 # who wants to borrow it now. mark it returned before issuing to the new borrower
-                my ( $allowed, $message ) = CanBookBeReturned( $item, C4::Context->userenv->{branch} );
+                my ( $allowed, $message ) = CanBookBeReturned( $item_unblessed, C4::Context->userenv->{branch} );
                 return unless $allowed;
-                AddReturn( $item->{'barcode'}, C4::Context->userenv->{'branch'} );
+                AddReturn( $item_object->barcode, C4::Context->userenv->{'branch'} );
             }
 
-            C4::Reserves::MoveReserve( $item->{'itemnumber'}, $borrower->{'borrowernumber'}, $cancelreserve );
+            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->{'itemnumber'} );
+            my ($datesent) = GetTransfers( $item_object->itemnumber );
             if ($datesent) {
                 # updating line of branchtranfert to finish it, and changing the to branch value, implement a comment for visibility of this case (maybe for stats ....)
                 my $sth = $dbh->prepare(
@@ -1333,15 +1386,15 @@ sub AddIssue {
                     WHERE itemnumber= ? AND datearrived IS NULL"
                 );
                 $sth->execute( C4::Context->userenv->{'branch'},
-                    $item->{'itemnumber'} );
+                    $item_object->itemnumber );
             }
 
             # If automatic renewal wasn't selected while issuing, set the value according to the issuing rule.
             unless ($auto_renew) {
                 my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
                     {   categorycode => $borrower->{categorycode},
-                        itemtype     => $item->{itype},
-                        branchcode   => $branch
+                        itemtype     => $item_object->effective_itemtype,
+                        branchcode   => $branchcode
                     }
                 );
 
@@ -1351,68 +1404,87 @@ sub AddIssue {
             # Record in the database the fact that the book was issued.
             unless ($datedue) {
                 my $itype = $item_object->effective_itemtype;
-                $datedue = CalcDateDue( $issuedate, $itype, $branch, $borrower );
+                $datedue = CalcDateDue( $issuedate, $itype, $branchcode, $borrower );
 
             }
             $datedue->truncate( to => 'minute' );
 
-            $issue = Koha::Database->new()->schema()->resultset('Issue')->update_or_create(
-                {
-                    borrowernumber => $borrower->{'borrowernumber'},
-                    itemnumber     => $item->{'itemnumber'},
-                    issuedate      => $issuedate->strftime('%Y-%m-%d %H:%M:%S'),
-                    date_due       => $datedue->strftime('%Y-%m-%d %H:%M:%S'),
-                    branchcode     => C4::Context->userenv->{'branch'},
-                    onsite_checkout => $onsite_checkout,
-                    auto_renew      => $auto_renew ? 1 : 0
-                }
-              );
+            my $issue_attributes = {
+                borrowernumber  => $borrower->{'borrowernumber'},
+                issuedate       => $issuedate->strftime('%Y-%m-%d %H:%M:%S'),
+                date_due        => $datedue->strftime('%Y-%m-%d %H:%M:%S'),
+                branchcode      => C4::Context->userenv->{'branch'},
+                onsite_checkout => $onsite_checkout,
+                auto_renew      => $auto_renew ? 1 : 0,
+            };
 
-            if ( C4::Context->preference('ReturnToShelvingCart') ) {
-                # ReturnToShelvingCart is on, anything issued should be taken off the cart.
-                CartToShelf( $item->{'itemnumber'} );
+            $issue = Koha::Checkouts->find( { itemnumber => $item_object->itemnumber } );
+            if ($issue) {
+                $issue->set($issue_attributes)->store;
+            }
+            else {
+                $issue = Koha::Checkout->new(
+                    {
+                        itemnumber => $item_object->itemnumber,
+                        %$issue_attributes,
+                    }
+                )->store;
             }
-            $item->{'issues'}++;
+            if ( $item_object->location && $item_object->location eq 'CART'
+                && ( !$item_object->permanent_location || $item_object->permanent_location ne 'CART' ) ) {
+            ## Item was moved to cart via UpdateItemLocationOnCheckin, anything issued should be taken off the cart.
+                CartToShelf( $item_object->itemnumber );
+            }
+
             if ( C4::Context->preference('UpdateTotalIssuesOnCirc') ) {
-                UpdateTotalIssues( $item->{'biblionumber'}, 1 );
+                UpdateTotalIssues( $item_object->biblionumber, 1 );
             }
 
             ## If item was lost, it has now been found, reverse any list item charges if necessary.
-            if ( $item->{'itemlost'} ) {
+            if ( $item_object->itemlost ) {
                 if (
                     Koha::RefundLostItemFeeRules->should_refund(
                         {
                             current_branch      => C4::Context->userenv->{branch},
-                            item_home_branch    => $item->{homebranch},
-                            item_holding_branch => $item->{holdingbranch}
+                            item_home_branch    => $item_object->homebranch,
+                            item_holding_branch => $item_object->holdingbranch,
                         }
                     )
                   )
                 {
-                    _FixAccountForLostAndReturned( $item->{'itemnumber'}, undef,
-                        $item->{'barcode'} );
+                    _FixAccountForLostAndReturned( $item_object->itemnumber, undef,
+                        $item_object->barcode );
                 }
             }
 
             ModItem(
                 {
-                    issues        => $item->{'issues'},
+                    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->{'biblionumber'},
-                $item->{'itemnumber'},
+                $item_object->biblionumber,
+                $item_object->itemnumber,
                 { log_action => 0 }
             );
-            ModDateLastSeen( $item->{'itemnumber'} );
+            ModDateLastSeen( $item_object->itemnumber );
 
-           # If it costs to borrow this book, charge it to the patron's account.
-            my ( $charge, $itemtype ) = GetIssuingCharges( $item->{'itemnumber'}, $borrower->{'borrowernumber'} );
-            if ( $charge > 0 ) {
-                AddIssuingCharge( $issue, $charge );
-                $item->{'charge'} = $charge;
+            # 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 ) {
+                AddIssuingCharge( $issue, $charge, 'RENT' );
+            }
+
+            my $itemtype_object = Koha::ItemTypes->find( $item_object->effective_itemtype );
+            if ( $itemtype_object ) {
+                my $accumulate_charge = $fees->accumulate_rentalcharge();
+                if ( $accumulate_charge > 0 ) {
+                    AddIssuingCharge( $issue, $accumulate_charge, 'RENT_DAILY' );
+                    $charge += $accumulate_charge;
+                    $item_unblessed->{charge} = $charge;
+                }
             }
 
             # Record the fact that this book was issued.
@@ -1422,36 +1494,36 @@ sub AddIssue {
                     type => ( $onsite_checkout ? 'onsite_checkout' : 'issue' ),
                     amount         => $charge,
                     other          => ( $sipmode ? "SIP-$sipmode" : '' ),
-                    itemnumber     => $item->{'itemnumber'},
-                    itemtype       => $item->{'itype'},
-                    location       => $item->{location},
+                    itemnumber     => $item_object->itemnumber,
+                    itemtype       => $item_object->effective_itemtype,
+                    location       => $item_object->location,
                     borrowernumber => $borrower->{'borrowernumber'},
-                    ccode          => $item->{'ccode'}
+                    ccode          => $item_object->ccode,
                 }
             );
 
             # Send a checkout slip.
             my $circulation_alert = 'C4::ItemCirculationAlertPreference';
             my %conditions        = (
-                branchcode   => $branch,
+                branchcode   => $branchcode,
                 categorycode => $borrower->{categorycode},
-                item_type    => $item->{itype},
+                item_type    => $item_object->effective_itemtype,
                 notification => 'CHECKOUT',
             );
             if ( $circulation_alert->is_enabled_for( \%conditions ) ) {
                 SendCirculationAlert(
                     {
                         type     => 'CHECKOUT',
-                        item     => $item,
+                        item     => $item_object->unblessed,
                         borrower => $borrower,
-                        branch   => $branch,
+                        branch   => $branchcode,
                     }
                 );
             }
             logaction(
                 "CIRCULATION", "ISSUE",
                 $borrower->{'borrowernumber'},
-                $item->{'itemnumber'}
+                $item_object->itemnumber,
             ) if C4::Context->preference("IssueLog");
         }
     }
@@ -1567,28 +1639,25 @@ Retrieves circulation rule attributes that apply to the given
 branch and patron category, regardless of item type.  
 The return value is a hashref containing the following key:
 
-maxissueqty - maximum number of loans that a
+patron_maxissueqty - maximum number of loans that a
 patron of the given category can have at the given
 branch.  If the value is undef, no limit.
 
-maxonsiteissueqty - maximum of on-site checkouts that a
+patron_maxonsiteissueqty - maximum of on-site checkouts that a
 patron of the given category can have at the given
 branch.  If the value is undef, no limit.
 
-This will first check for a specific branch and
-category match from branch_borrower_circ_rules. 
-
-If no rule is found, it will then check default_branch_circ_rules
-(same branch, default category).  If no rule is found,
-it will then check default_borrower_circ_rules (default 
-branch, same category), then failing that, default_circ_rules
-(default branch, default category).
+This will check for different branch/category combinations in the following order:
+branch and category
+branch only
+category only
+default branch and category
 
 If no rule has been found in the database, it will default to
 the buillt in rule:
 
-maxissueqty - undef
-maxonsiteissueqty - undef
+patron_maxissueqty - undef
+patron_maxonsiteissueqty - undef
 
 C<$branchcode> and C<$categorycode> should contain the
 literal branch code and patron category code, respectively - no
@@ -1599,44 +1668,27 @@ wildcards.
 sub GetBranchBorrowerCircRule {
     my ( $branchcode, $categorycode ) = @_;
 
-    my $rules;
-    my $dbh = C4::Context->dbh();
-    $rules = $dbh->selectrow_hashref( q|
-        SELECT maxissueqty, maxonsiteissueqty
-        FROM branch_borrower_circ_rules
-        WHERE branchcode = ?
-        AND   categorycode = ?
-    |, {}, $branchcode, $categorycode ) ;
-    return $rules if $rules;
-
-    # try same branch, default borrower category
-    $rules = $dbh->selectrow_hashref( q|
-        SELECT maxissueqty, maxonsiteissueqty
-        FROM default_branch_circ_rules
-        WHERE branchcode = ?
-    |, {}, $branchcode ) ;
-    return $rules if $rules;
-
-    # try default branch, same borrower category
-    $rules = $dbh->selectrow_hashref( q|
-        SELECT maxissueqty, maxonsiteissueqty
-        FROM default_borrower_circ_rules
-        WHERE categorycode = ?
-    |, {}, $categorycode ) ;
-    return $rules if $rules;
-
-    # try default branch, default borrower category
-    $rules = $dbh->selectrow_hashref( q|
-        SELECT maxissueqty, maxonsiteissueqty
-        FROM default_circ_rules
-    |, {} );
-    return $rules if $rules;
-
-    # built-in default circulation rule
-    return {
-        maxissueqty => undef,
-        maxonsiteissueqty => undef,
+    # Initialize default values
+    my $rules = {
+        patron_maxissueqty       => undef,
+        patron_maxonsiteissueqty => undef,
     };
+
+    # Search for rules!
+    foreach my $rule_name (qw( patron_maxissueqty patron_maxonsiteissueqty )) {
+        my $rule = Koha::CirculationRules->get_effective_rule(
+            {
+                categorycode => $categorycode,
+                itemtype     => undef,
+                branchcode   => $branchcode,
+                rule_name    => $rule_name,
+            }
+        );
+
+        $rules->{$rule_name} = $rule->rule_value if defined $rule;
+    }
+
+    return $rules;
 }
 
 =head2 GetBranchItemRule
@@ -1671,49 +1723,49 @@ Neither C<$branchcode> nor C<$itemtype> should be '*'.
 
 sub GetBranchItemRule {
     my ( $branchcode, $itemtype ) = @_;
-    my $dbh = C4::Context->dbh();
-    my $result = {};
-
-    my @attempts = (
-        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
-            FROM branch_item_rules
-            WHERE branchcode = ?
-              AND itemtype = ?', $branchcode, $itemtype],
-        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
-            FROM default_branch_circ_rules
-            WHERE branchcode = ?', $branchcode],
-        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
-            FROM default_branch_item_rules
-            WHERE itemtype = ?', $itemtype],
-        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
-            FROM default_circ_rules'],
+
+    # Search for rules!
+    my $holdallowed_rule = Koha::CirculationRules->get_effective_rule(
+        {
+            branchcode => $branchcode,
+            itemtype => $itemtype,
+            rule_name => 'holdallowed',
+        }
+    );
+    my $hold_fulfillment_policy_rule = Koha::CirculationRules->get_effective_rule(
+        {
+            branchcode => $branchcode,
+            itemtype => $itemtype,
+            rule_name => 'hold_fulfillment_policy',
+        }
+    );
+    my $returnbranch_rule = Koha::CirculationRules->get_effective_rule(
+        {
+            branchcode => $branchcode,
+            itemtype => $itemtype,
+            rule_name => 'returnbranch',
+        }
     );
 
-    foreach my $attempt (@attempts) {
-        my ($query, @bind_params) = @{$attempt};
-        my $search_result = $dbh->selectrow_hashref ( $query , {}, @bind_params )
-          or next;
-
-        # Since branch/category and branch/itemtype use the same per-branch
-        # defaults tables, we have to check that the key we want is set, not
-        # just that a row was returned
-        $result->{'holdallowed'}  = $search_result->{'holdallowed'}  unless ( defined $result->{'holdallowed'} );
-        $result->{'hold_fulfillment_policy'} = $search_result->{'hold_fulfillment_policy'} unless ( defined $result->{'hold_fulfillment_policy'} );
-        $result->{'returnbranch'} = $search_result->{'returnbranch'} unless ( defined $result->{'returnbranch'} );
-    }
-    
     # built-in default circulation rule
-    $result->{'holdallowed'} = 2 unless ( defined $result->{'holdallowed'} );
-    $result->{'hold_fulfillment_policy'} = 'any' unless ( defined $result->{'hold_fulfillment_policy'} );
-    $result->{'returnbranch'} = 'homebranch' unless ( defined $result->{'returnbranch'} );
-
-    return $result;
+    my $rules;
+    $rules->{holdallowed} = defined $holdallowed_rule
+        ? $holdallowed_rule->rule_value
+        : 2;
+    $rules->{hold_fulfillment_policy} = defined $hold_fulfillment_policy_rule
+        ? $hold_fulfillment_policy_rule->rule_value
+        : 'any';
+    $rules->{returnbranch} = defined $returnbranch_rule
+        ? $returnbranch_rule->rule_value
+        : 'homebranch';
+
+    return $rules;
 }
 
 =head2 AddReturn
 
   ($doreturn, $messages, $iteminformation, $borrower) =
-      &AddReturn( $barcode, $branch [,$exemptfine] [,$dropbox] [,$returndate] );
+      &AddReturn( $barcode, $branch [,$exemptfine] [,$returndate] );
 
 Returns a book.
 
@@ -1726,12 +1778,6 @@ Returns a book.
 =item C<$exemptfine> indicates that overdue charges for the item will be
 removed. Optional.
 
-=item C<$dropbox> indicates that the check-in date is assumed to be
-yesterday, or the last non-holiday as defined in C4::Calendar .  If
-overdue charges are applied and C<$dropbox> is true, the last charge
-will be removed.  This assumes that the fines accrual script has run
-for _today_. Optional.
-
 =item C<$return_date> allows the default return date to be overridden
 by the given return date. Optional.
 
@@ -1790,13 +1836,14 @@ patron who last borrowed the book.
 =cut
 
 sub AddReturn {
-    my ( $barcode, $branch, $exemptfine, $dropbox, $return_date, $dropboxdate ) = @_;
+    my ( $barcode, $branch, $exemptfine, $return_date ) = @_;
 
     if ($branch and not Koha::Libraries->find($branch)) {
         warn "AddReturn error: branch '$branch' not found.  Reverting to " . C4::Context->userenv->{'branch'};
         undef $branch;
     }
     $branch = C4::Context->userenv->{'branch'} unless $branch;  # we trust userenv to be a safe fallback/default
+    $return_date //= dt_from_string();
     my $messages;
     my $patron;
     my $doreturn       = 1;
@@ -1804,22 +1851,22 @@ sub AddReturn {
     my $stat_type = 'return';
 
     # get information on item
-    my $item = GetItem( undef, $barcode );
+    my $item = Koha::Items->find({ barcode => $barcode });
     unless ($item) {
         return ( 0, { BadBarcode => $barcode } );    # no barcode means no item or borrower.  bail out.
     }
 
-    my $itemnumber = $item->{ itemnumber };
-    my $itemtype = $item->{itype}; # GetItem called effective_itemtype
+    my $itemnumber = $item->itemnumber;
+    my $itemtype = $item->effective_itemtype;
 
-    my $issue  = Koha::Checkouts->find( { itemnumber => $itemnumber } );
+    my $issue  = $item->checkout;
     if ( $issue ) {
-        $patron = Koha::Patrons->find( $issue->borrowernumber )
+        $patron = $issue->patron
             or die "Data inconsistency: barcode $barcode (itemnumber:$itemnumber) claims to be issued to non-existent borrowernumber '" . $issue->borrowernumber . "'\n"
                 . Dumper($issue->unblessed) . "\n";
     } else {
         $messages->{'NotIssued'} = $barcode;
-        ModItem({ onloan => undef }, $item->{biblionumber}, $item->{itemnumber}) if defined $item->{onloan};
+        ModItem({ onloan => undef }, $item->biblionumber, $item->itemnumber) 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.
@@ -1830,26 +1877,40 @@ sub AddReturn {
         }
     }
 
-    if ( $item->{'location'} eq 'PROC' ) {
-        if ( C4::Context->preference("InProcessingToShelvingCart") ) {
-            $item->{'location'} = 'CART';
-        }
-        else {
-            $item->{location} = $item->{permanent_location};
-        }
-
-        ModItem( $item, $item->{'biblionumber'}, $item->{'itemnumber'}, { log_action => 0 } );
-    }
-
+    my $item_unblessed = $item->unblessed;
         # full item data, but no borrowernumber or checkout info (no issue)
-    my $hbr = GetBranchItemRule($item->{'homebranch'}, $item->{'itype'})->{'returnbranch'} || "homebranch";
+    my $hbr = GetBranchItemRule($item->homebranch, $itemtype)->{'returnbranch'} || "homebranch";
         # get the proper branch to which to return the item
-    my $returnbranch = $item->{$hbr} || $branch ;
+    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 $borrowernumber = $patron ? $patron->borrowernumber : undef;    # we don't know if we had a borrower or not
     my $patron_unblessed = $patron ? $patron->unblessed : {};
 
+    my $update_loc_rules = get_yaml_pref_hash('UpdateItemLocationOnCheckin');
+    map { $update_loc_rules->{$_} = $update_loc_rules->{$_}[0] } keys %$update_loc_rules; #We can only move to one location so we flatten the arrays
+    if ($update_loc_rules) {
+        if (defined $update_loc_rules->{_ALL_}) {
+            if ($update_loc_rules->{_ALL_} eq '_PERM_') { $update_loc_rules->{_ALL_} = $item->permanent_location; }
+            if ($update_loc_rules->{_ALL_} eq '_BLANK_') { $update_loc_rules->{_ALL_} = ''; }
+            if ( $item->location ne $update_loc_rules->{_ALL_}) {
+                $messages->{'ItemLocationUpdated'} = { from => $item->location, to => $update_loc_rules->{_ALL_} };
+                ModItem( { location => $update_loc_rules->{_ALL_} }, undef, $itemnumber );
+            }
+        }
+        else {
+            foreach my $key ( keys %$update_loc_rules ) {
+                if ( $update_loc_rules->{$key} eq '_PERM_' ) { $update_loc_rules->{$key} = $item->permanent_location; }
+                if ( $update_loc_rules->{$key} eq '_BLANK_') { $update_loc_rules->{$key} = '' ;}
+                if ( ($item->location eq $key && $item->location ne $update_loc_rules->{$key}) || ($key eq '_BLANK_' && $item->location eq '' && $update_loc_rules->{$key} ne '') ) {
+                    $messages->{'ItemLocationUpdated'} = { from => $item->location, to => $update_loc_rules->{$key} };
+                    ModItem( { location => $update_loc_rules->{$key} }, undef, $itemnumber );
+                    last;
+                }
+            }
+        }
+    }
+
     my $yaml = C4::Context->preference('UpdateNotForLoanStatusOnCheckin');
     if ($yaml) {
         $yaml = "$yaml\n\n";  # YAML is anal on ending \n. Surplus does not hurt
@@ -1860,8 +1921,8 @@ sub AddReturn {
         }
         else {
             foreach my $key ( keys %$rules ) {
-                if ( $item->{notforloan} eq $key ) {
-                    $messages->{'NotForLoanStatusUpdated'} = { from => $item->{notforloan}, to => $rules->{$key} };
+                if ( $item->notforloan eq $key ) {
+                    $messages->{'NotForLoanStatusUpdated'} = { from => $item->notforloan, to => $rules->{$key} };
                     ModItem( { notforloan => $rules->{$key} }, undef, $itemnumber, { log_action => 0 } );
                     last;
                 }
@@ -1870,7 +1931,7 @@ sub AddReturn {
     }
 
     # check if the return is allowed at this branch
-    my ($returnallowed, $message) = CanBookBeReturned($item, $branch);
+    my ($returnallowed, $message) = CanBookBeReturned($item_unblessed, $branch);
     unless ($returnallowed){
         $messages->{'Wrongbranch'} = {
             Wrongbranch => $branch,
@@ -1880,42 +1941,27 @@ sub AddReturn {
         return ( $doreturn, $messages, $issue, $patron_unblessed);
     }
 
-    if ( $item->{'withdrawn'} ) { # book has been cancelled
+    if ( $item->withdrawn ) { # book has been cancelled
         $messages->{'withdrawn'} = 1;
         $doreturn = 0 if C4::Context->preference("BlockReturnOfWithdrawnItems");
     }
 
-    if ( $item->{itemlost} and C4::Context->preference("BlockReturnOfLostItems") ) {
+    if ( $item->itemlost and C4::Context->preference("BlockReturnOfLostItems") ) {
         $doreturn = 0;
     }
 
     # case of a return of document (deal with issues and holdingbranch)
-    my $today = DateTime->now( time_zone => C4::Context->tz() );
-
     if ($doreturn) {
-        my $is_overdue;
         die "The item is not issed and cannot be returned" unless $issue; # Just in case...
         $patron or warn "AddReturn without current borrower";
-               my $circControlBranch;
-        if ($dropbox) {
-            # define circControlBranch only if dropbox mode is set
-            # don't allow dropbox mode to create an invalid entry in issues (issuedate > today)
-            # FIXME: check issuedate > returndate, factoring in holidays
-
-            $circControlBranch = _GetCircControlBranch($item,$patron_unblessed);
-            $is_overdue = $issue->is_overdue( $dropboxdate );
-        } else {
-            $is_overdue = $issue->is_overdue;
-        }
 
         if ($patron) {
             eval {
-                MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
-                    $circControlBranch, $return_date, $patron->privacy );
+                MarkIssueReturned( $borrowernumber, $item->itemnumber, $return_date, $patron->privacy );
             };
             unless ( $@ ) {
-                if ( ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue ) || $return_date ) {
-                    _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $patron_unblessed, return_date => $return_date } );
+                if ( C4::Context->preference('CalculateFinesOnReturn') && !$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 );
@@ -1928,58 +1974,55 @@ sub AddReturn {
 
         }
 
-        ModItem( { onloan => undef }, $item->{biblionumber}, $item->{itemnumber}, { log_action => 0 } );
+        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->{'holdingbranch'} = $branch; # update item data holdingbranch too
+    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
     }
 
     my $leave_item_lost = C4::Context->preference("BlockReturnOfLostItems") ? 1 : 0;
-    ModDateLastSeen( $item->{itemnumber}, $leave_item_lost );
+    ModDateLastSeen( $item->itemnumber, $leave_item_lost );
 
     # check if we have a transfer for this document
-    my ($datesent,$frombranch,$tobranch) = GetTransfers( $item->{'itemnumber'} );
+    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'} );
+    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'} );
+            $sth->execute( $item->itemnumber );
             # if we have a reservation with valid transfer, we can set it's status to 'W'
-            ShelfToCart( $item->{'itemnumber'} ) if ( C4::Context->preference("ReturnToShelvingCart") );
-            C4::Reserves::ModReserveStatus($item->{'itemnumber'}, 'W');
+            C4::Reserves::ModReserveStatus($item->itemnumber, 'W');
         } else {
             $messages->{'WrongTransfer'}     = $tobranch;
-            $messages->{'WrongTransferItem'} = $item->{'itemnumber'};
+            $messages->{'WrongTransferItem'} = $item->itemnumber;
         }
         $validTransfert = 1;
-    } else {
-        ShelfToCart( $item->{'itemnumber'} ) if ( C4::Context->preference("ReturnToShelvingCart") );
     }
 
     # fix up the accounts.....
-    if ( $item->{'itemlost'} ) {
+    if ( $item->itemlost ) {
         $messages->{'WasLost'} = 1;
         unless ( C4::Context->preference("BlockReturnOfLostItems") ) {
             if (
                 Koha::RefundLostItemFeeRules->should_refund(
                     {
                         current_branch      => C4::Context->userenv->{branch},
-                        item_home_branch    => $item->{homebranch},
+                        item_home_branch    => $item->homebranch,
                         item_holding_branch => $item_holding_branch
                     }
                 )
               )
             {
-                _FixAccountForLostAndReturned( $item->{'itemnumber'},
+                _FixAccountForLostAndReturned( $item->itemnumber,
                     $borrowernumber, $barcode );
                 $messages->{'LostItemFeeRefunded'} = 1;
             }
@@ -1988,14 +2031,12 @@ sub AddReturn {
 
     # fix up the overdues in accounts...
     if ($borrowernumber) {
-        my $fix = _FixOverduesOnReturn($borrowernumber, $item->{itemnumber}, $exemptfine, $dropbox);
-        defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $item->{itemnumber}...) failed!";  # zero is OK, check defined
+        my $fix = _FixOverduesOnReturn( $borrowernumber, $item->itemnumber, $exemptfine, 'RETURNED' );
+        defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $item->itemnumber...) failed!";  # zero is OK, check defined
 
         if ( $issue and $issue->is_overdue ) {
         # fix fine days
-            $today = dt_from_string($return_date) if $return_date;
-            $today = $dropboxdate if $dropbox;
-            my ($debardate,$reminder) = _debar_user_on_return( $patron_unblessed, $item, dt_from_string($issue->date_due), $today );
+            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 {
@@ -2008,7 +2049,7 @@ sub AddReturn {
              } else {
                   my $borrower_debar_dt = dt_from_string( $patron->debarred );
                   $borrower_debar_dt->truncate(to => 'day');
-                  my $today_dt = $today->clone()->truncate(to => 'day');
+                  my $today_dt = $return_date->clone()->truncate(to => 'day');
                   if ( DateTime->compare( $borrower_debar_dt, $today_dt ) != -1 ) {
                       $messages->{'PrevDebarred'} = $patron->debarred;
                   }
@@ -2020,7 +2061,7 @@ sub AddReturn {
     # if we don't have a reserve with the status W, we launch the Checkreserves routine
     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'} );
+    ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->itemnumber, undef, $lookahead ) unless ( $item->withdrawn );
     if ($resfound) {
           $resrec->{'ResFound'} = $resfound;
         $messages->{'ResFound'} = $resrec;
@@ -2033,7 +2074,7 @@ sub AddReturn {
         itemnumber     => $itemnumber,
         itemtype       => $itemtype,
         borrowernumber => $borrowernumber,
-        ccode          => $item->{ ccode }
+        ccode          => $item->ccode,
     });
 
     # Send a check-in slip. # NOTE: borrower may be undef. Do not try to send messages then.
@@ -2042,19 +2083,19 @@ sub AddReturn {
         my %conditions = (
             branchcode   => $branch,
             categorycode => $patron->categorycode,
-            item_type    => $item->{itype},
+            item_type    => $itemtype,
             notification => 'CHECKIN',
         );
         if ($doreturn && $circulation_alert->is_enabled_for(\%conditions)) {
             SendCirculationAlert({
                 type     => 'CHECKIN',
-                item     => $item,
+                item     => $item_unblessed,
                 borrower => $patron->unblessed,
                 branch   => $branch,
             });
         }
 
-        logaction("CIRCULATION", "RETURN", $borrowernumber, $item->{'itemnumber'})
+        logaction("CIRCULATION", "RETURN", $borrowernumber, $item->itemnumber)
             if C4::Context->preference("ReturnLog");
         }
 
@@ -2070,55 +2111,65 @@ sub AddReturn {
 
     # Transfer to returnbranch if Automatic transfer set or append message NeedsTransfer
     if (!$is_in_rotating_collection && ($doreturn or $messages->{'NotIssued'}) and !$resfound and ($branch ne $returnbranch) and not $messages->{'WrongTransfer'}){
+        my $BranchTransferLimitsType = C4::Context->preference("BranchTransferLimitsType");
         if  (C4::Context->preference("AutomaticItemReturn"    ) or
             (C4::Context->preference("UseBranchTransferLimits") and
-             ! IsBranchTransferAllowed($branch, $returnbranch, $item->{C4::Context->preference("BranchTransferLimitsType")} )
+             ! 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);
-            ModItemTransfer($item->{'itemnumber'}, $branch, $returnbranch);
+            $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;
         } else {
             $messages->{'NeedsTransfer'} = $returnbranch;
         }
     }
 
+    if ( C4::Context->preference('ClaimReturnedLostValue') ) {
+        my $claims = Koha::Checkouts::ReturnClaims->search(
+           {
+               itemnumber => $item->id,
+               resolution => undef,
+           }
+        );
+
+        if ( $claims->count ) {
+            $messages->{ReturnClaims} = $claims;
+        }
+    }
+
     return ( $doreturn, $messages, $issue, ( $patron ? $patron->unblessed : {} ));
 }
 
 =head2 MarkIssueReturned
 
-  MarkIssueReturned($borrowernumber, $itemnumber, $dropbox_branch, $returndate, $privacy);
+  MarkIssueReturned($borrowernumber, $itemnumber, $returndate, $privacy);
 
 Unconditionally marks an issue as being returned by
 moving the C<issues> row to C<old_issues> and
-setting C<returndate> to the current date, or
-the last non-holiday date of the branccode specified in
-C<dropbox_branch> .  Assumes you've already checked that 
-it's safe to do this, i.e. last non-holiday > issuedate.
+setting C<returndate> to the current date.
 
 if C<$returndate> is specified (in iso format), it is used as the date
-of the return. It is ignored when a dropbox_branch is passed in.
+of the return.
 
 C<$privacy> contains the privacy parameter. If the patron has set privacy to 2,
 the old_issue is immediately anonymised
 
 Ideally, this function would be internal to C<C4::Circulation>,
-not exported, but it is currently needed by one 
-routine in C<C4::Accounts>.
+not exported, but it is currently used in misc/cronjobs/longoverdue.pl
+and offline_circ/process_koc.pl.
 
 =cut
 
 sub MarkIssueReturned {
-    my ( $borrowernumber, $itemnumber, $dropbox_branch, $returndate, $privacy ) = @_;
-
+    my ( $borrowernumber, $itemnumber, $returndate, $privacy ) = @_;
 
     # Retrieve the issue
     my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return;
     my $issue_id = $issue->issue_id;
 
     my $anonymouspatron;
-    if ( $privacy == 2 ) {
+    if ( $privacy && $privacy == 2 ) {
         # The default of 0 will not work due to foreign key constraints
         # The anonymisation will fail if AnonymousPatron is not a valid entry
         # We need to check if the anonymous patron exist, Koha will fail loudly if it does not
@@ -2127,41 +2178,26 @@ sub MarkIssueReturned {
         die "Fatal error: the patron ($borrowernumber) has requested their circulation history be anonymized on check-in, but the AnonymousPatron system preference is empty or not set correctly."
             unless Koha::Patrons->find( $anonymouspatron );
     }
-    my $database = Koha::Database->new();
-    my $schema   = $database->schema;
-    my $dbh   = C4::Context->dbh;
 
-    my $query = 'UPDATE issues SET returndate=';
-    my @bind;
-    if ($dropbox_branch) {
-        my $calendar = Koha::Calendar->new( branchcode => $dropbox_branch );
-        my $dropboxdate = $calendar->addDate( DateTime->now( time_zone => C4::Context->tz), -1 );
-        $query .= ' ? ';
-        push @bind, $dropboxdate->strftime('%Y-%m-%d %H:%M');
-    } elsif ($returndate) {
-        $query .= ' ? ';
-        push @bind, $returndate;
-    } else {
-        $query .= ' now() ';
-    }
-    $query .= ' WHERE issue_id = ?';
-    push @bind, $issue_id;
+    my $schema = Koha::Database->schema;
 
     # FIXME Improve the return value and handle it from callers
     $schema->txn_do(sub {
 
-        # Update the returndate
-        $dbh->do( $query, undef, @bind );
-
-        # We just updated the returndate, so we need to refetch $issue
-        $issue->discard_changes;
+        # Update the returndate value
+        if ( $returndate ) {
+            $issue->returndate( $returndate )->store->discard_changes; # update and refetch
+        }
+        else {
+            $issue->returndate( \'NOW()' )->store->discard_changes; # update and refetch
+        }
 
         # Create the old_issues entry
         my $old_checkout = Koha::Old::Checkout->new($issue->unblessed)->store;
 
         # anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber
-        if ( $privacy == 2) {
-            $dbh->do(q|UPDATE old_issues SET borrowernumber=? WHERE issue_id = ?|, undef, $anonymouspatron, $old_checkout->issue_id);
+        if ( $privacy && $privacy == 2) {
+            $old_checkout->borrowernumber($anonymouspatron)->store;
         }
 
         # And finally delete the issue
@@ -2181,7 +2217,7 @@ sub MarkIssueReturned {
 
 =head2 _debar_user_on_return
 
-    _debar_user_on_return($borrower, $item, $datedue, today);
+    _debar_user_on_return($borrower, $item, $datedue, $returndate);
 
 C<$borrower> borrower hashref
 
@@ -2189,20 +2225,22 @@ C<$item> item hashref
 
 C<$datedue> date due DateTime object
 
-C<$return_date> DateTime object representing the return time
+C<$returndate> DateTime object representing the return time
 
 Internal function, called only by AddReturn that calculates and updates
  the user fine days, and debars them if necessary.
 
 Should only be called for overdue returns
 
+Calculation of the debarment date has been moved to a separate subroutine _calculate_new_debar_dt
+to ease testing.
+
 =cut
 
-sub _debar_user_on_return {
+sub _calculate_new_debar_dt {
     my ( $borrower, $item, $dt_due, $return_date ) = @_;
 
     my $branchcode = _GetCircControlBranch( $item, $borrower );
-
     my $circcontrol = C4::Context->preference('CircControl');
     my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
         {   categorycode => $borrower->{categorycode},
@@ -2214,98 +2252,110 @@ sub _debar_user_on_return {
     my $unit     = $issuing_rule ? $issuing_rule->lengthunit : undef;
     my $chargeable_units = C4::Overdues::get_chargeable_units($unit, $dt_due, $return_date, $branchcode);
 
-    if ($finedays) {
+    return unless $finedays;
 
-        # finedays is in days, so hourly loans must multiply by 24
-        # thus 1 hour late equals 1 day suspension * finedays rate
-        $finedays = $finedays * 24 if ( $unit eq 'hours' );
+    # finedays is in days, so hourly loans must multiply by 24
+    # thus 1 hour late equals 1 day suspension * finedays rate
+    $finedays = $finedays * 24 if ( $unit eq 'hours' );
 
-        # grace period is measured in the same units as the loan
-        my $grace =
-          DateTime::Duration->new( $unit => $issuing_rule->firstremind );
+    # grace period is measured in the same units as the loan
+    my $grace =
+      DateTime::Duration->new( $unit => $issuing_rule->firstremind );
 
-        my $deltadays = DateTime::Duration->new(
-            days => $chargeable_units
-        );
-        if ( $deltadays->subtract($grace)->is_positive() ) {
-            my $suspension_days = $deltadays * $finedays;
-
-            # If the max suspension days is < than the suspension days
-            # the suspension days is limited to this maximum period.
-            my $max_sd = $issuing_rule->maxsuspensiondays;
-            if ( defined $max_sd ) {
-                $max_sd = DateTime::Duration->new( days => $max_sd );
-                $suspension_days = $max_sd
-                  if DateTime::Duration->compare( $max_sd, $suspension_days ) < 0;
-            }
+    my $deltadays = DateTime::Duration->new(
+        days => $chargeable_units
+    );
 
-            my ( $has_been_extended, $is_a_reminder );
-            if ( C4::Context->preference('CumulativeRestrictionPeriods') and $borrower->{debarred} ) {
-                my $debarment = @{ GetDebarments( { borrowernumber => $borrower->{borrowernumber}, type => 'SUSPENSION' } ) }[0];
-                if ( $debarment ) {
-                    $return_date = dt_from_string( $debarment->{expiration}, 'sql' );
-                    $has_been_extended = 1;
-                }
-            }
+    if ( $deltadays->subtract($grace)->is_positive() ) {
+        my $suspension_days = $deltadays * $finedays;
 
-            if ( $issuing_rule->suspension_chargeperiod > 1 ) {
-                # No need to / 1 and do not consider / 0
-                $suspension_days = DateTime::Duration->new(
-                    days => floor( $suspension_days->in_units('days') / $issuing_rule->suspension_chargeperiod )
-                );
-            }
+        if ( $issuing_rule->suspension_chargeperiod > 1 ) {
+            # No need to / 1 and do not consider / 0
+            $suspension_days = DateTime::Duration->new(
+                days => floor( $suspension_days->in_units('days') / $issuing_rule->suspension_chargeperiod )
+            );
+        }
 
-            my $new_debar_dt;
-            # Use the calendar or not to calculate the debarment date
-            if ( C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed' ) {
-                my $calendar = Koha::Calendar->new(
-                    branchcode => $branchcode,
-                    days_mode  => 'Calendar'
-                );
-                $new_debar_dt = $calendar->addDate( $return_date, $suspension_days );
-            }
-            else {
-                $new_debar_dt = $return_date->clone()->add_duration($suspension_days);
-            }
+        # If the max suspension days is < than the suspension days
+        # the suspension days is limited to this maximum period.
+        my $max_sd = $issuing_rule->maxsuspensiondays;
+        if ( defined $max_sd ) {
+            $max_sd = DateTime::Duration->new( days => $max_sd );
+            $suspension_days = $max_sd
+              if DateTime::Duration->compare( $max_sd, $suspension_days ) < 0;
+        }
 
-            Koha::Patron::Debarments::AddUniqueDebarment({
-                borrowernumber => $borrower->{borrowernumber},
-                expiration     => $new_debar_dt->ymd(),
-                type           => 'SUSPENSION',
-            });
-            # if borrower was already debarred but does not get an extra debarment
-            my $patron = Koha::Patrons->find( $borrower->{borrowernumber} );
-            my $new_debarment_str;
-            if ( $borrower->{debarred} eq $patron->is_debarred ) {
-                $is_a_reminder = 1;
-                $new_debarment_str = $borrower->{debarred};
-            } else {
-                $new_debarment_str = $new_debar_dt->ymd();
+        my ( $has_been_extended );
+        if ( C4::Context->preference('CumulativeRestrictionPeriods') and $borrower->{debarred} ) {
+            my $debarment = @{ GetDebarments( { borrowernumber => $borrower->{borrowernumber}, type => 'SUSPENSION' } ) }[0];
+            if ( $debarment ) {
+                $return_date = dt_from_string( $debarment->{expiration}, 'sql' );
+                $has_been_extended = 1;
             }
-            # FIXME Should return a DateTime object
-            return $new_debarment_str, $is_a_reminder;
         }
+
+        my $new_debar_dt;
+        # Use the calendar or not to calculate the debarment date
+        if ( C4::Context->preference('SuspensionsCalendar') eq 'noSuspensionsWhenClosed' ) {
+            my $calendar = Koha::Calendar->new(
+                branchcode => $branchcode,
+                days_mode  => 'Calendar'
+            );
+            $new_debar_dt = $calendar->addDate( $return_date, $suspension_days );
+        }
+        else {
+            $new_debar_dt = $return_date->clone()->add_duration($suspension_days);
+        }
+        return $new_debar_dt;
     }
     return;
 }
 
+sub _debar_user_on_return {
+    my ( $borrower, $item, $dt_due, $return_date ) = @_;
+
+    $return_date //= dt_from_string();
+
+    my $new_debar_dt = _calculate_new_debar_dt ($borrower, $item, $dt_due, $return_date);
+
+    return unless $new_debar_dt;
+
+    Koha::Patron::Debarments::AddUniqueDebarment({
+        borrowernumber => $borrower->{borrowernumber},
+        expiration     => $new_debar_dt->ymd(),
+        type           => 'SUSPENSION',
+    });
+    # if borrower was already debarred but does not get an extra debarment
+    my $patron = Koha::Patrons->find( $borrower->{borrowernumber} );
+    my ($new_debarment_str, $is_a_reminder);
+    if ( $borrower->{debarred} && $borrower->{debarred} eq $patron->is_debarred ) {
+        $is_a_reminder = 1;
+        $new_debarment_str = $borrower->{debarred};
+    } else {
+        $new_debarment_str = $new_debar_dt->ymd();
+    }
+    # FIXME Should return a DateTime object
+    return $new_debarment_str, $is_a_reminder;
+}
+
 =head2 _FixOverduesOnReturn
 
-   &_FixOverduesOnReturn($brn,$itm, $exemptfine, $dropboxmode);
+   &_FixOverduesOnReturn($borrowernumber, $itemnumber, $exemptfine, $status);
 
-C<$brn> borrowernumber
+C<$borrowernumber> borrowernumber
 
-C<$itm> itemnumber
+C<$itemnumber> itemnumber
 
 C<$exemptfine> BOOL -- remove overdue charge associated with this issue. 
-C<$dropboxmode> BOOL -- remove lastincrement on overdue charge associated with this issue.
+
+C<$status> ENUM -- reason for fix [ RETURNED, RENEWED, LOST, FORGIVEN ]
 
 Internal function
 
 =cut
 
 sub _FixOverduesOnReturn {
-    my ($borrowernumber, $item, $exemptfine, $dropbox ) = @_;
+    my ( $borrowernumber, $item, $exemptfine, $status ) = @_;
     unless( $borrowernumber ) {
         warn "_FixOverduesOnReturn() not supplied valid borrowernumber";
         return;
@@ -2314,73 +2364,67 @@ sub _FixOverduesOnReturn {
         warn "_FixOverduesOnReturn() not supplied valid itemnumber";
         return;
     }
+    unless( $status ) {
+        warn "_FixOverduesOnReturn() not supplied valid status";
+        return;
+    }
 
-    # check for overdue fine
-    my $accountline = Koha::Account::Lines->search(
-        {
-            borrowernumber => $borrowernumber,
-            itemnumber     => $item,
-            -or            => [
-                accounttype => 'FU',
-                accounttype => 'O',
-            ],
-        }
-    )->next();
-    return 0 unless $accountline;    # no warning, there's just nothing to fix
-
-    if ($exemptfine) {
-        my $amountoutstanding = $accountline->amountoutstanding;
+    my $schema = Koha::Database->schema;
 
-        $accountline->accounttype('FFOR');
-        $accountline->amountoutstanding(0);
+    my $result = $schema->txn_do(
+        sub {
+            # check for overdue fine
+            my $accountlines = Koha::Account::Lines->search(
+                {
+                    borrowernumber  => $borrowernumber,
+                    itemnumber      => $item,
+                    debit_type_code => 'OVERDUE',
+                    status          => 'UNRETURNED'
+                }
+            );
+            return 0 unless $accountlines->count; # no warning, there's just nothing to fix
 
-        Koha::Account::Offset->new(
-            {
-                debit_id => $accountline->id,
-                type => 'Forgiven',
-                amount => $amountoutstanding * -1,
-            }
-        )->store();
+            my $accountline = $accountlines->next;
+            if ($exemptfine) {
+                my $amountoutstanding = $accountline->amountoutstanding;
 
-        if (C4::Context->preference("FinesLog")) {
-            &logaction("FINES", 'MODIFY',$borrowernumber,"Overdue forgiven: item $item");
-        }
-    } elsif ($dropbox && $accountline->lastincrement) {
-        my $outstanding = $accountline->amountoutstanding - $accountline->lastincrement;
-        my $amt = $accountline->amount - $accountline->lastincrement;
+                my $account = Koha::Account->new({patron_id => $borrowernumber});
+                my $credit = $account->add_credit(
+                    {
+                        amount     => $amountoutstanding,
+                        user_id    => C4::Context->userenv ? C4::Context->userenv->{'number'} : undef,
+                        library_id => C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef,
+                        interface  => C4::Context->interface,
+                        type       => 'FORGIVEN',
+                        item_id    => $item
+                    }
+                );
 
-        Koha::Account::Offset->new(
-            {
-                debit_id => $accountline->id,
-                type => 'Dropbox',
-                amount => $accountline->lastincrement * -1,
-            }
-        )->store();
+                $credit->apply({ debits => [ $accountline ], offset_type => 'Forgiven' });
 
-        if ( C4::Context->preference("FinesLog") ) {
-            &logaction( "FINES", 'MODIFY', $borrowernumber,
-                "Dropbox adjustment $amt, item $item" );
-        }
+                $accountline->status('FORGIVEN');
 
-        $accountline->accounttype('F');
+                if (C4::Context->preference("FinesLog")) {
+                    &logaction("FINES", 'MODIFY',$borrowernumber,"Overdue forgiven: item $item");
+                }
+            } else {
+                $accountline->status($status);
+            }
 
-        if ( $outstanding >= 0 && $amt >= 0 ) {
-            $accountline->amount($amt);
-            $accountline->amountoutstanding($outstanding);
+            return $accountline->store();
         }
+    );
 
-    } else {
-        $accountline->accounttype('F');
-    }
-
-    return $accountline->store();
+    return $result;
 }
 
 =head2 _FixAccountForLostAndReturned
 
   &_FixAccountForLostAndReturned($itemnumber, [$borrowernumber, $barcode]);
 
-Calculates the charge for a book lost and returned.
+Finds the most recent lost item charge for this item and refunds the borrower
+appropriatly, taking into account any payments or writeoffs already applied
+against the charge.
 
 Internal function, not exported, called only by AddReturn.
 
@@ -2396,18 +2440,24 @@ sub _FixAccountForLostAndReturned {
     # check for charge made for lost book
     my $accountlines = Koha::Account::Lines->search(
         {
-            itemnumber  => $itemnumber,
-            accounttype => { -in => [ 'L', 'Rep', 'W' ] },
+            itemnumber      => $itemnumber,
+            debit_type_code => 'LOST',
+            status          => [ undef, { '<>' => 'RETURNED' } ]
         },
         {
-            order_by => { -desc => [ 'date', 'accountno' ] }
+            order_by => { -desc => [ 'date', 'accountlines_id' ] }
         }
     );
 
     return unless $accountlines->count > 0;
     my $accountline     = $accountlines->next;
     my $total_to_refund = 0;
-    my $account = Koha::Patrons->find( $accountline->borrowernumber )->account;
+
+    return unless $accountline->borrowernumber;
+    my $patron = Koha::Patrons->find( $accountline->borrowernumber );
+    return unless $patron; # Patron has been deleted, nobody to credit the return to
+
+    my $account = $patron->account;
 
     # Use cases
     if ( $accountline->amount > $accountline->amountoutstanding ) {
@@ -2433,21 +2483,19 @@ sub _FixAccountForLostAndReturned {
         $credit = $account->add_credit(
             {   amount      => $credit_total,
                 description => 'Item Returned ' . $item_id,
-                type        => 'lost_item_return',
+                type        => 'LOST_RETURN',
+                interface   => C4::Context->interface,
                 library_id  => $branchcode
             }
         );
 
-        # TODO: ->apply should just accept the accountline
-        $credit->apply( { debits => $accountlines->reset } );
+        $credit->apply( { debits => [ $accountline ] } );
     }
 
-    # Manually set the accounttype
-    $accountline->discard_changes->accounttype('LR');
+    # Update the account status
+    $accountline->discard_changes->status('RETURNED');
     $accountline->store;
 
-    ModItem( { paidfor => '' }, undef, $itemnumber, { log_action => 0 } );
-
     if ( defined $account and C4::Context->preference('AccountAutoReconcile') ) {
         $account->reconcile_balance;
     }
@@ -2621,15 +2669,12 @@ sub CanBookBeRenewed {
     my $dbh    = C4::Context->dbh;
     my $renews = 1;
 
-    my $item      = GetItem($itemnumber)      or return ( 0, 'no_item' );
-    my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return ( 0, 'no_checkout' );
+    my $item      = Koha::Items->find($itemnumber)      or return ( 0, 'no_item' );
+    my $issue = $item->checkout or return ( 0, 'no_checkout' );
     return ( 0, 'onsite_checkout' ) if $issue->onsite_checkout;
     return ( 0, 'item_denied_renewal') if _item_denied_renewal({ item => $item });
 
-
-    $borrowernumber ||= $issue->borrowernumber;
-    my $patron = Koha::Patrons->find( $borrowernumber )
-      or return;
+    my $patron = $issue->patron or return;
 
     my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber);
 
@@ -2677,16 +2722,16 @@ sub CanBookBeRenewed {
             # can be filled with available items. We can get the union of the sets simply
             # by pushing all the elements onto an array and removing the duplicates.
             my @reservable;
-            my %borrowers;
-            ITEM: foreach my $i (@itemnumbers) {
-                my $item = GetItem($i);
-                next if IsItemOnHoldAndFound($i);
-                for my $b (@borrowernumbers) {
-                    my $borr = $borrowers{$b} //= Koha::Patrons->find( $b )->unblessed;
-                    next unless IsAvailableForItemLevelRequest($item, $borr);
-                    next unless CanItemBeReserved($b,$i);
-
-                    push @reservable, $i;
+            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;
@@ -2700,10 +2745,10 @@ sub CanBookBeRenewed {
 
     return ( 1, undef ) if $override_limit;
 
-    my $branchcode = _GetCircControlBranch( $item, $patron->unblessed );
+    my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed );
     my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
         {   categorycode => $patron->categorycode,
-            itemtype     => $item->{itype},
+            itemtype     => $item->effective_itemtype,
             branchcode   => $branchcode
         }
     );
@@ -2752,7 +2797,10 @@ sub CanBookBeRenewed {
 
         if ( C4::Context->preference('OPACFineNoRenewalsBlockAutoRenew') ) {
             my $fine_no_renewals = C4::Context->preference("OPACFineNoRenewals");
-            my $amountoutstanding = $patron->account->balance;
+            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" );
             }
@@ -2825,17 +2873,15 @@ sub AddRenewal {
     my $itemnumber      = shift or return;
     my $branch          = shift;
     my $datedue         = shift;
-    my $lastreneweddate = shift || DateTime->now(time_zone => C4::Context->tz)->ymd();
+    my $lastreneweddate = shift || DateTime->now(time_zone => C4::Context->tz);
 
-    my $item   = GetItem($itemnumber) or return;
-    my $item_object = Koha::Items->find( $itemnumber ); # Should replace $item
+    my $item_object   = Koha::Items->find($itemnumber) or return;
     my $biblio = $item_object->biblio;
+    my $issue  = $item_object->checkout;
+    my $item_unblessed = $item_object->unblessed;
 
     my $dbh = C4::Context->dbh;
 
-    # Find the issues record for this book
-    my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } );
-
     return unless $issue;
 
     $borrowernumber ||= $issue->borrowernumber;
@@ -2848,112 +2894,120 @@ sub AddRenewal {
     my $patron = Koha::Patrons->find( $borrowernumber ) or return; # FIXME Should do more than just return
     my $patron_unblessed = $patron->unblessed;
 
-    if ( C4::Context->preference('CalculateFinesOnReturn') && $issue->is_overdue ) {
-        _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $patron_unblessed } );
-    }
-    _FixOverduesOnReturn( $borrowernumber, $itemnumber );
-
-    # If the due date wasn't specified, calculate it by adding the
-    # book's loan length to today's date or the current due date
-    # based on the value of the RenewalPeriodBase syspref.
-    unless ($datedue) {
+    my $circ_library = Koha::Libraries->find( _GetCircControlBranch($item_unblessed, $patron_unblessed) );
 
-        my $itemtype = $item_object->effective_itemtype;
-        $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?
-                                        dt_from_string( $issue->date_due, 'sql' ) :
-                                        DateTime->now( time_zone => C4::Context->tz());
-        $datedue =  CalcDateDue($datedue, $itemtype, _GetCircControlBranch($item, $patron_unblessed), $patron_unblessed, 'is a renewal');
-    }
+    my $schema = Koha::Database->schema;
+    $schema->txn_do(sub{
 
-    # Update the issues record to have the new due date, and a new count
-    # of how many times it has been renewed.
-    my $renews = $issue->renewals + 1;
-    my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ?
-                            WHERE borrowernumber=? 
-                            AND itemnumber=?"
-    );
+        if ( C4::Context->preference('CalculateFinesOnReturn') ) {
+            _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed } );
+        }
+        _FixOverduesOnReturn( $borrowernumber, $itemnumber, undef, 'RENEWED' );
 
-    $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $lastreneweddate, $borrowernumber, $itemnumber );
+        # If the due date wasn't specified, calculate it by adding the
+        # book's loan length to today's date or the current due date
+        # based on the value of the RenewalPeriodBase syspref.
+        my $itemtype = $item_object->effective_itemtype;
+        unless ($datedue) {
 
-    # Update the renewal count on the item, and tell zebra to reindex
-    $renews = $item->{renewals} + 1;
-    ModItem( { renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item->{biblionumber}, $itemnumber, { log_action => 0 } );
+            $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?
+                                            dt_from_string( $issue->date_due, 'sql' ) :
+                                            DateTime->now( time_zone => C4::Context->tz());
+            $datedue =  CalcDateDue($datedue, $itemtype, $circ_library->branchcode, $patron_unblessed, 'is a renewal');
+        }
 
-    # Charge a new rental fee, if applicable?
-    my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
-    if ( $charge > 0 ) {
-        my $accountno = C4::Accounts::getnextacctno( $borrowernumber );
-        my $manager_id = 0;
-        $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv; 
-        my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;
-        Koha::Account::Line->new(
+        my $fees = Koha::Charges::Fees->new(
             {
-                date              => dt_from_string(),
-                borrowernumber    => $borrowernumber,
-                accountno         => $accountno,
-                amount            => $charge,
-                manager_id        => $manager_id,
-                accounttype       => 'Rent',
-                amountoutstanding => $charge,
-                itemnumber        => $itemnumber,
-                branchcode        => $branchcode,
-                description       => 'Renewal of Rental Item '
-                  . $biblio->title
-                  . " $item->{'barcode'}",
+                patron    => $patron,
+                library   => $circ_library,
+                item      => $item_object,
+                from_date => dt_from_string( $issue->date_due, 'sql' ),
+                to_date   => dt_from_string($datedue),
             }
-        )->store();
-    }
+        );
 
-    # Send a renewal slip according to checkout alert preferencei
-    if ( C4::Context->preference('RenewalSendNotice') eq '1' ) {
-        my $circulation_alert = 'C4::ItemCirculationAlertPreference';
-        my %conditions        = (
-            branchcode   => $branch,
-            categorycode => $patron->categorycode,
-            item_type    => $item->{itype},
-            notification => 'CHECKOUT',
+        # 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=?"
         );
-        if ( $circulation_alert->is_enabled_for( \%conditions ) ) {
-            SendCirculationAlert(
-                {
-                    type     => 'RENEWAL',
-                    item     => $item,
-                    borrower => $patron->unblessed,
-                    branch   => $branch,
-                }
-            );
+
+        $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $lastreneweddate, $borrowernumber, $itemnumber );
+
+        # Update the renewal count on the item, and tell zebra to reindex
+        $renews = ( $item_object->renewals || 0 ) + 1;
+        ModItem( { renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item_object->biblionumber, $itemnumber, { log_action => 0 } );
+
+        # Charge a new rental fee, if applicable
+        my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
+        if ( $charge > 0 ) {
+            AddIssuingCharge($issue, $charge, 'RENT_RENEW');
         }
-    }
 
-    # Remove any OVERDUES related debarment if the borrower has no overdues
-    if ( $patron
-      && $patron->is_debarred
-      && ! $patron->has_overdues
-      && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
-    ) {
-        DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
-    }
+        # Charge a new accumulate rental fee, if applicable
+        my $itemtype_object = Koha::ItemTypes->find( $itemtype );
+        if ( $itemtype_object ) {
+            my $accumulate_charge = $fees->accumulate_rentalcharge();
+            if ( $accumulate_charge > 0 ) {
+                AddIssuingCharge( $issue, $accumulate_charge, 'RENT_DAILY_RENEW' )
+            }
+            $charge += $accumulate_charge;
+        }
 
-    unless ( C4::Context->interface eq 'opac' ) { #if from opac we are obeying OpacRenewalBranch as calculated in opac-renew.pl
-        $branch = C4::Context->userenv ? C4::Context->userenv->{branch} : $branch;
-    }
+        # Send a renewal slip according to checkout alert preferencei
+        if ( C4::Context->preference('RenewalSendNotice') eq '1' ) {
+            my $circulation_alert = 'C4::ItemCirculationAlertPreference';
+            my %conditions        = (
+                branchcode   => $branch,
+                categorycode => $patron->categorycode,
+                item_type    => $itemtype,
+                notification => 'CHECKOUT',
+            );
+            if ( $circulation_alert->is_enabled_for( \%conditions ) ) {
+                SendCirculationAlert(
+                    {
+                        type     => 'RENEWAL',
+                        item     => $item_unblessed,
+                        borrower => $patron->unblessed,
+                        branch   => $branch,
+                    }
+                );
+            }
+        }
 
-    # Add the renewal to stats
-    UpdateStats(
-        {
-            branch         => $branch,
-            type           => 'renew',
-            amount         => $charge,
-            itemnumber     => $itemnumber,
-            itemtype       => $item->{itype},
-            location       => $item->{location},
-            borrowernumber => $borrowernumber,
-            ccode          => $item->{'ccode'}
+        # Remove any OVERDUES related debarment if the borrower has no overdues
+        if ( $patron
+          && $patron->is_debarred
+          && ! $patron->has_overdues
+          && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
+        ) {
+            DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
         }
-    );
 
-    #Log the renewal
-    logaction("CIRCULATION", "RENEWAL", $borrowernumber, $itemnumber) if C4::Context->preference("RenewalLog");
+        unless ( C4::Context->interface eq 'opac' ) { #if from opac we are obeying OpacRenewalBranch as calculated in opac-renew.pl
+            $branch = C4::Context->userenv ? C4::Context->userenv->{branch} : $branch;
+        }
+
+        # Add the renewal to stats
+        UpdateStats(
+            {
+                branch         => $branch,
+                type           => 'renew',
+                amount         => $charge,
+                itemnumber     => $itemnumber,
+                itemtype       => $itemtype,
+                location       => $item_object->location,
+                borrowernumber => $borrowernumber,
+                ccode          => $item_object->ccode,
+            }
+        );
+
+        #Log the renewal
+        logaction("CIRCULATION", "RENEWAL", $borrowernumber, $itemnumber) if C4::Context->preference("RenewalLog");
+    });
+
     return $datedue;
 }
 
@@ -2966,7 +3020,7 @@ sub GetRenewCount {
     my $renewsleft    = 0;
 
     my $patron = Koha::Patrons->find( $bornum );
-    my $item     = GetItem($itemno);
+    my $item   = Koha::Items->find($itemno);
 
     return (0, 0, 0) unless $patron or $item; # Wrong call, no renewal allowed
 
@@ -2983,11 +3037,11 @@ sub GetRenewCount {
     my $data = $sth->fetchrow_hashref;
     $renewcount = $data->{'renewals'} if $data->{'renewals'};
     # $item and $borrower should be calculated
-    my $branchcode = _GetCircControlBranch($item, $patron->unblessed);
+    my $branchcode = _GetCircControlBranch($item->unblessed, $patron->unblessed);
 
     my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
         {   categorycode => $patron->categorycode,
-            itemtype     => $item->{itype},
+            itemtype     => $item->effective_itemtype,
             branchcode   => $branchcode
         }
     );
@@ -3022,17 +3076,17 @@ sub GetSoonestRenewDate {
 
     my $dbh = C4::Context->dbh;
 
-    my $item      = GetItem($itemnumber)      or return;
-    my $itemissue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return;
+    my $item      = Koha::Items->find($itemnumber)      or return;
+    my $itemissue = $item->checkout or return;
 
     $borrowernumber ||= $itemissue->borrowernumber;
     my $patron = Koha::Patrons->find( $borrowernumber )
       or return;
 
-    my $branchcode = _GetCircControlBranch( $item, $patron->unblessed );
+    my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed );
     my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
         {   categorycode => $patron->categorycode,
-            itemtype     => $item->{itype},
+            itemtype     => $item->effective_itemtype,
             branchcode   => $branchcode
         }
     );
@@ -3081,17 +3135,17 @@ sub GetLatestAutoRenewDate {
 
     my $dbh = C4::Context->dbh;
 
-    my $item      = GetItem($itemnumber)      or return;
-    my $itemissue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return;
+    my $item      = Koha::Items->find($itemnumber)  or return;
+    my $itemissue = $item->checkout                 or return;
 
     $borrowernumber ||= $itemissue->borrowernumber;
     my $patron = Koha::Patrons->find( $borrowernumber )
       or return;
 
-    my $branchcode = _GetCircControlBranch( $item, $patron->unblessed );
+    my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed );
     my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
         {   categorycode => $patron->categorycode,
-            itemtype     => $item->{itype},
+            itemtype     => $item->effective_itemtype,
             branchcode   => $branchcode
         }
     );
@@ -3221,45 +3275,28 @@ sub _get_discount_from_rule {
 
 =head2 AddIssuingCharge
 
-  &AddIssuingCharge( $checkout, $charge )
+  &AddIssuingCharge( $checkout, $charge, $type )
 
 =cut
 
 sub AddIssuingCharge {
-    my ( $checkout, $charge ) = @_;
+    my ( $checkout, $charge, $type ) = @_;
 
     # FIXME What if checkout does not exist?
 
-    my $nextaccntno = C4::Accounts::getnextacctno( $checkout->borrowernumber );
-
-    my $manager_id  = 0;
-    $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv;
-
-    my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;
-
-    my $accountline = Koha::Account::Line->new(
-        {
-            borrowernumber    => $checkout->borrowernumber,
-            itemnumber        => $checkout->itemnumber,
-            issue_id          => $checkout->issue_id,
-            accountno         => $nextaccntno,
-            amount            => $charge,
-            amountoutstanding => $charge,
-            manager_id        => $manager_id,
-            branchcode        => $branchcode,
-            description       => 'Rental',
-            accounttype       => 'Rent',
-            date              => \'NOW()',
-        }
-    )->store();
-
-    Koha::Account::Offset->new(
+    my $account = Koha::Account->new({ patron_id => $checkout->borrowernumber });
+    my $accountline = $account->add_debit(
         {
-            debit_id => $accountline->id,
-            type     => 'Rental Fee',
-            amount   => $charge,
+            amount      => $charge,
+            note        => undef,
+            user_id     => C4::Context->userenv ? C4::Context->userenv->{'number'} : undef,
+            library_id  => C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef,
+            interface   => C4::Context->interface,
+            type        => $type,
+            item_id     => $checkout->itemnumber,
+            issue_id    => $checkout->issue_id,
         }
-    )->store();
+    );
 }
 
 =head2 GetTransfers
@@ -3570,7 +3607,7 @@ sub CalcDateDue {
           my $calendar = Koha::Calendar->new( branchcode => $branch );
           if ( $calendar->is_holiday($datedue) ) {
               # Don't return on a closed day
-              $datedue = $calendar->prev_open_day( $datedue );
+              $datedue = $calendar->prev_open_days( $datedue, 1 );
           }
         }
     }
@@ -3668,15 +3705,7 @@ sub DeleteBranchTransferLimits {
 
 sub ReturnLostItem{
     my ( $borrowernumber, $itemnum ) = @_;
-
     MarkIssueReturned( $borrowernumber, $itemnum );
-    my $patron = Koha::Patrons->find( $borrowernumber );
-    my $item = C4::Items::GetItem( $itemnum );
-    my $old_note = ($item->{'paidfor'} && ($item->{'paidfor'} ne q{})) ? $item->{'paidfor'}.' / ' : q{};
-    my @datearr = localtime(time);
-    my $date = ( 1900 + $datearr[5] ) . "-" . ( $datearr[4] + 1 ) . "-" . $datearr[3];
-    my $bor = $patron->firstname . ' ' . $patron->surname . ' ' . $patron->cardnumber;
-    ModItem({ paidfor =>  $old_note."Paid for by $bor $date" }, undef, $itemnum);
 }
 
 
@@ -3709,16 +3738,16 @@ sub LostItem{
     if ( my $borrowernumber = $issues->{borrowernumber} ){
         my $patron = Koha::Patrons->find( $borrowernumber );
 
-        my $fix = _FixOverduesOnReturn($borrowernumber, $itemnumber, C4::Context->preference('WhenLostForgiveFine'), 0); # 1, 0 = exemptfine, no-dropbox
+        my $fix = _FixOverduesOnReturn($borrowernumber, $itemnumber, C4::Context->preference('WhenLostForgiveFine'), 'LOST');
         defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $itemnumber...) failed!";  # zero is OK, check defined
 
         if (C4::Context->preference('WhenLostChargeReplacementFee')){
-            C4::Accounts::chargelostitem($borrowernumber, $itemnumber, $issues->{'replacementprice'}, "Lost Item $issues->{'title'} $issues->{'barcode'} $issues->{'itemcallnumber'}");
+            C4::Accounts::chargelostitem($borrowernumber, $itemnumber, $issues->{'replacementprice'}, "$issues->{'title'} $issues->{'barcode'} $issues->{'itemcallnumber'}");
             #FIXME : Should probably have a way to distinguish this from an item that really was returned.
             #warn " $issues->{'borrowernumber'}  /  $itemnumber ";
         }
 
-        MarkIssueReturned($borrowernumber,$itemnumber,undef,undef,$patron->privacy) if $mark_returned;
+        MarkIssueReturned($borrowernumber,$itemnumber,undef,$patron->privacy) if $mark_returned;
     }
 
     #When item is marked lost automatically cancel its outstanding transfers and set items holdingbranch to the transfer source branch (frombranch)
@@ -3789,7 +3818,6 @@ sub ProcessOfflineReturn {
             MarkIssueReturned(
                 $issue->{borrowernumber},
                 $itemnumber,
-                undef,
                 $operation->{timestamp},
             );
             ModItem(
@@ -3824,7 +3852,6 @@ sub ProcessOfflineIssue {
             MarkIssueReturned(
                 $issue->{borrowernumber},
                 $itemnumber,
-                undef,
                 $operation->{timestamp},
             );
         }
@@ -3847,7 +3874,13 @@ sub ProcessOfflinePayment {
 
     my $patron = Koha::Patrons->find({ cardnumber => $operation->{cardnumber} });
 
-    $patron->account->pay({ amount => $operation->{amount}, library_id => $operation->{branchcode} });
+    $patron->account->pay(
+        {
+            amount     => $operation->{amount},
+            library_id => $operation->{branchcode},
+            interface  => 'koc'
+        }
+    );
 
     return "Success.";
 }
@@ -3863,8 +3896,12 @@ sub ProcessOfflinePayment {
 sub TransferSlip {
     my ($branch, $itemnumber, $barcode, $to_branch) = @_;
 
-    my $item =  GetItem( $itemnumber, $barcode )
-      or return;
+    my $item =
+      $itemnumber
+      ? Koha::Items->find($itemnumber)
+      : Koha::Items->find( { barcode => $barcode } );
+
+    $item or return;
 
     return C4::Letters::GetPreparedLetter (
         module => 'circulation',
@@ -3872,8 +3909,8 @@ sub TransferSlip {
         branchcode => $branch,
         tables => {
             'branches'    => $to_branch,
-            'biblio'      => $item->{biblionumber},
-            'items'       => $item,
+            'biblio'      => $item->biblionumber,
+            'items'       => $item->unblessed,
         },
     );
 }
@@ -4110,7 +4147,7 @@ sub _CalculateAndUpdateFine {
       : ( $control eq 'PatronLibrary' )   ? $borrower->{branchcode}
       :                                     $issue->branchcode;
 
-    my $date_returned = $return_date ? dt_from_string($return_date) : dt_from_string();
+    my $date_returned = $return_date ? $return_date : dt_from_string();
 
     my ( $amount, $unitcounttotal, $unitcount  ) =
       C4::Overdues::CalcFine( $item, $borrower->{categorycode}, $control_branchcode, $datedue, $date_returned );
@@ -4150,7 +4187,7 @@ sub _item_denied_renewal {
     my $denyingrules = Koha::Config::SysPrefs->find('ItemsDeniedRenewal')->get_yaml_pref_hash();
     return unless $denyingrules;
     foreach my $field (keys %$denyingrules) {
-        my $val = $item->{$field};
+        my $val = $item->$field;
         if( !defined $val) {
             if ( any { !defined $_ }  @{$denyingrules->{$field}} ){
                 return 1;