projects / koha.git / blobdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | inline | side by side
Bug 18736: (follow-up) Fix missing rounding and bad formatting
[koha.git] / C4 / Circulation.pm
diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 7d04d3f..b74fdff 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -681,7 +681,7 @@ sub CanBookBeIssued {
     my $onsite_checkout     = $params->{onsite_checkout}     || 0;
     my $override_high_holds = $params->{override_high_holds} || 0;
 
     my $onsite_checkout     = $params->{onsite_checkout}     || 0;
     my $override_high_holds = $params->{override_high_holds} || 0;
 
-    my $item = Koha::Items->find({barcode => $barcode });
+    my $item_object = Koha::Items->find({barcode => $barcode });
 
     # MANDATORY CHECKS - unless item exists, nothing else matters
     unless ( $item_object ) {
 
     # MANDATORY CHECKS - unless item exists, nothing else matters
     unless ( $item_object ) {
@@ -689,16 +689,16 @@ sub CanBookBeIssued {
     }
     return ( \%issuingimpossible, \%needsconfirmation ) if %issuingimpossible;
 
     }
     return ( \%issuingimpossible, \%needsconfirmation ) if %issuingimpossible;
 
-    my $item_unblessed = $item->unblessed; # Transition...
-    my $issue = $item->checkout;
-    my $biblio = $item->biblio;
+    my $item_unblessed = $item_object->unblessed; # Transition...
+    my $issue = $item_object->checkout;
+    my $biblio = $item_object->biblio;
 
     my $biblioitem = $biblio->biblioitem;
 
     my $biblioitem = $biblio->biblioitem;
-    my $effective_itemtype = $item->effective_itemtype;
+    my $effective_itemtype = $item_object->effective_itemtype;
     my $dbh             = C4::Context->dbh;
     my $patron_unblessed = $patron->unblessed;
 
     my $dbh             = C4::Context->dbh;
     my $patron_unblessed = $patron->unblessed;
 
-    my $library = Koha::Libraries->find( _GetCircControlBranch($item, $patron_unblessed) );
+    my $circ_library = Koha::Libraries->find( _GetCircControlBranch($item_unblessed, $patron_unblessed) );
     #
     # DUE DATE is OK ? -- should already have checked.
     #
     #
     # DUE DATE is OK ? -- should already have checked.
     #
@@ -709,7 +709,7 @@ sub CanBookBeIssued {
     unless ( $duedate ) {
         my $issuedate = $now->clone();
 
     unless ( $duedate ) {
         my $issuedate = $now->clone();
 
-        my $branch = _GetCircControlBranch($item_unblessed, $patron_unblessed);
+        my $branch = $circ_library;
         $duedate = CalcDateDue( $issuedate, $effective_itemtype, $branch, $patron_unblessed );
 
         # Offline circ calls AddIssue directly, doesn't run through here
         $duedate = CalcDateDue( $issuedate, $effective_itemtype, $branch, $patron_unblessed );
 
         # Offline circ calls AddIssue directly, doesn't run through here
@@ -719,7 +719,7 @@ sub CanBookBeIssued {
     my $fees = Koha::Charges::Fees->new(
         {
             patron    => $patron,
     my $fees = Koha::Charges::Fees->new(
         {
             patron    => $patron,
-            library   => $library,
+            library   => $circ_library,
             item      => $item_object,
             to_date   => $duedate,
         }
             item      => $item_object,
             to_date   => $duedate,
         }
@@ -738,17 +738,17 @@ sub CanBookBeIssued {
     #
     # BORROWER STATUS
     #
     #
     # 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',
        # 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,
                      itemtype => $effective_itemtype,
                      borrowernumber => $patron->borrowernumber,
-                     ccode => $item->ccode}
+                     ccode => $item_object->ccode}
                     );
                     );
-        ModDateLastSeen( $item->itemnumber ); # FIXME Move to Koha::Item
+        ModDateLastSeen( $item_object->itemnumber ); # FIXME Move to Koha::Item
         return( { STATS => 1 }, {});
     }
 
         return( { STATS => 1 }, {});
     }
 
@@ -859,7 +859,7 @@ sub CanBookBeIssued {
         } else {
             my ($CanBookBeRenewed,$renewerror) = CanBookBeRenewed(
                 $patron->borrowernumber,
         } else {
             my ($CanBookBeRenewed,$renewerror) = CanBookBeRenewed(
                 $patron->borrowernumber,
-                $item->itemnumber,
+                $item_object->itemnumber,
             );
             if ( $CanBookBeRenewed == 0 ) {    # no more renewals allowed
                 if ( $renewerror eq 'onsite_checkout' ) {
             );
             if ( $CanBookBeRenewed == 0 ) {    # no more renewals allowed
                 if ( $renewerror eq 'onsite_checkout' ) {
@@ -901,7 +901,7 @@ sub CanBookBeIssued {
       and $issue
       and $issue->onsite_checkout
       and $issue->borrowernumber == $patron->borrowernumber ? 1 : 0 );
       and $issue
       and $issue->onsite_checkout
       and $issue->borrowernumber == $patron->borrowernumber ? 1 : 0 );
-    my $toomany = TooMany( $patron_unblessed, $item->biblionumber, $item_unblessed, { onsite_checkout => $onsite_checkout, switch_onsite_checkout => $switch_onsite_checkout, } );
+    my $toomany = TooMany( $patron_unblessed, $item_object->biblionumber, $item_unblessed, { 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 ) {
     # 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 ) {
@@ -929,14 +929,14 @@ sub CanBookBeIssued {
     #
     # ITEM CHECKING
     #
     #
     # ITEM CHECKING
     #
-    if ( $item->notforloan )
+    if ( $item_object->notforloan )
     {
         if(!C4::Context->preference("AllowNotForLoanOverride")){
             $issuingimpossible{NOT_FOR_LOAN} = 1;
     {
         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;
         }else{
             $needsconfirmation{NOT_FOR_LOAN_FORCING} = 1;
-            $needsconfirmation{item_notforloan} = $item->notforloan;
+            $needsconfirmation{item_notforloan} = $item_object->notforloan;
         }
     }
     else {
         }
     }
     else {
@@ -969,17 +969,17 @@ sub CanBookBeIssued {
             }
         }
     }
             }
         }
     }
-    if ( $item->withdrawn && $item->withdrawn > 0 )
+    if ( $item_object->withdrawn && $item_object->withdrawn > 0 )
     {
         $issuingimpossible{WTHDRAWN} = 1;
     }
     {
         $issuingimpossible{WTHDRAWN} = 1;
     }
-    if (   $item->restricted
-        && $item->restricted == 1 )
+    if (   $item_object->restricted
+        && $item_object->restricted == 1 )
     {
         $issuingimpossible{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' );
         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' );
@@ -988,9 +988,9 @@ sub CanBookBeIssued {
         my $userenv = C4::Context->userenv;
         unless ( C4::Context->IsSuperLibrarian() ) {
             my $HomeOrHoldingBranch = C4::Context->preference("HomeOrHoldingBranch");
         my $userenv = C4::Context->userenv;
         unless ( C4::Context->IsSuperLibrarian() ) {
             my $HomeOrHoldingBranch = C4::Context->preference("HomeOrHoldingBranch");
-            if ( $item->$HomeOrHoldingBranch ne $userenv->{branch} ){
+            if ( $item_object->$HomeOrHoldingBranch ne $userenv->{branch} ){
                 $issuingimpossible{ITEMNOTSAMEBRANCH} = 1;
                 $issuingimpossible{ITEMNOTSAMEBRANCH} = 1;
-                $issuingimpossible{'itemhomebranch'} = $item->$HomeOrHoldingBranch;
+                $issuingimpossible{'itemhomebranch'} = $item_object->$HomeOrHoldingBranch;
             }
             $needsconfirmation{BORRNOTSAMEBRANCH} = $patron->branchcode
               if ( $patron->branchcode ne $userenv->{branch} );
             }
             $needsconfirmation{BORRNOTSAMEBRANCH} = $patron->branchcode
               if ( $patron->branchcode ne $userenv->{branch} );
@@ -1002,8 +1002,8 @@ sub CanBookBeIssued {
     my $rentalConfirmation = C4::Context->preference("RentalFeesCheckoutConfirmation");
 
     if ( $rentalConfirmation ){
     my $rentalConfirmation = C4::Context->preference("RentalFeesCheckoutConfirmation");
 
     if ( $rentalConfirmation ){
-        my ($rentalCharge) = GetIssuingCharges( $item->itemnumber, $patron->borrowernumber );
-        my $itemtype = Koha::ItemTypes->find( $item->itype ); # GetItem sets effective itemtype
+        my ($rentalCharge) = GetIssuingCharges( $item_object->itemnumber, $patron->borrowernumber );
+        my $itemtype = Koha::ItemTypes->find( $item_object->itype ); # GetItem sets effective itemtype
         $rentalCharge += $fees->accumulate_rentalcharge({ from => dt_from_string(), to => $duedate });
         if ( $rentalCharge > 0 ){
             $needsconfirmation{RENTALCHARGE} = $rentalCharge;
         $rentalCharge += $fees->accumulate_rentalcharge({ from => dt_from_string(), to => $duedate });
         if ( $rentalCharge > 0 ){
             $needsconfirmation{RENTALCHARGE} = $rentalCharge;
@@ -1012,7 +1012,7 @@ sub CanBookBeIssued {
 
     unless ( $ignore_reserves ) {
         # See if the item is on reserve.
 
     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 ) {
         if ($restype) {
             my $resbor = $res->{'borrowernumber'};
             if ( $resbor ne $patron->borrowernumber ) {
@@ -1086,7 +1086,7 @@ sub CanBookBeIssued {
     ) {
         # Check if borrower has already issued an item from the same biblio
         # Only if it's not a subscription
     ) {
         # 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) {
         require C4::Serials;
         my $is_a_subscription = C4::Serials::CountSubscriptionFromBiblionumber($biblionumber);
         unless ($is_a_subscription) {
@@ -1317,21 +1317,21 @@ sub AddIssue {
     # Stop here if the patron or barcode doesn't exist
     if ( $borrower && $barcode && $barcodecheck ) {
         # find which item we issue
     # Stop here if the patron or barcode doesn't exist
     if ( $borrower && $barcode && $barcodecheck ) {
         # find which item we issue
-        my $item = Koha::Items->find({ barcode => $barcode })
+        my $item_object = Koha::Items->find({ barcode => $barcode })
           or return;    # if we don't get an Item, abort.
           or return;    # if we don't get an Item, abort.
-        my $item_unblessed = $item->unblessed;
+        my $item_unblessed = $item_object->unblessed;
 
         my $branch = _GetCircControlBranch( $item_unblessed, $borrower );
 
         # get actual issuing if there is one
 
         my $branch = _GetCircControlBranch( $item_unblessed, $borrower );
 
         # get actual issuing if there is one
-        my $actualissue = $item->checkout;
+        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'},
 
         # 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,
+                $item_object->itemnumber,
                 $branch,
                 $datedue,
                 $issuedate,    # here interpreted as the renewal date
                 $branch,
                 $datedue,
                 $issuedate,    # here interpreted as the renewal date
@@ -1362,13 +1362,13 @@ sub AddIssue {
                 # who wants to borrow it now. mark it returned before issuing to the new borrower
                 my ( $allowed, $message ) = CanBookBeReturned( $item_unblessed, C4::Context->userenv->{branch} );
                 return unless $allowed;
                 # who wants to borrow it now. mark it returned before issuing to the new borrower
                 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)
 
             # 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(
             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(
@@ -1379,14 +1379,14 @@ sub AddIssue {
                     WHERE itemnumber= ? AND datearrived IS NULL"
                 );
                 $sth->execute( C4::Context->userenv->{'branch'},
                     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},
             }
 
             # 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->effective_itemtype,
+                        itemtype     => $item_object->effective_itemtype,
                         branchcode   => $branch
                     }
                 );
                         branchcode   => $branch
                     }
                 );
@@ -1396,7 +1396,7 @@ sub AddIssue {
 
             # Record in the database the fact that the book was issued.
             unless ($datedue) {
 
             # Record in the database the fact that the book was issued.
             unless ($datedue) {
-                my $itype = $item->effective_itemtype;
+                my $itype = $item_object->effective_itemtype;
                 $datedue = CalcDateDue( $issuedate, $itype, $branch, $borrower );
 
             }
                 $datedue = CalcDateDue( $issuedate, $itype, $branch, $borrower );
 
             }
@@ -1411,14 +1411,14 @@ sub AddIssue {
                 auto_renew      => $auto_renew ? 1 : 0,
             };
 
                 auto_renew      => $auto_renew ? 1 : 0,
             };
 
-            $issue = Koha::Checkouts->find( { itemnumber => $item->itemnumber } );
+            $issue = Koha::Checkouts->find( { itemnumber => $item_object->itemnumber } );
             if ($issue) {
                 $issue->set($issue_attributes)->store;
             }
             else {
                 $issue = Koha::Checkout->new(
                     {
             if ($issue) {
                 $issue->set($issue_attributes)->store;
             }
             else {
                 $issue = Koha::Checkout->new(
                     {
-                        itemnumber => $item->itemnumber,
+                        itemnumber => $item_object->itemnumber,
                         %$issue_attributes,
                     }
                 )->store;
                         %$issue_attributes,
                     }
                 )->store;
@@ -1426,58 +1426,58 @@ sub AddIssue {
 
             if ( C4::Context->preference('ReturnToShelvingCart') ) {
                 # ReturnToShelvingCart is on, anything issued should be taken off the cart.
 
             if ( C4::Context->preference('ReturnToShelvingCart') ) {
                 # ReturnToShelvingCart is on, anything issued should be taken off the cart.
-                CartToShelf( $item->itemnumber );
+                CartToShelf( $item_object->itemnumber );
             }
 
             if ( C4::Context->preference('UpdateTotalIssuesOnCirc') ) {
             }
 
             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 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},
                 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(
                 {
                 }
             }
 
             ModItem(
                 {
-                    issues        => $item->issues + 1,
+                    issues        => $item_object->issues + 1,
                     holdingbranch => C4::Context->userenv->{'branch'},
                     itemlost      => 0,
                     onloan        => $datedue->ymd(),
                     datelastborrowed => DateTime->now( time_zone => C4::Context->tz() )->ymd(),
                 },
                     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 }
             );
                 { log_action => 0 }
             );
-            ModDateLastSeen( $item->itemnumber );
+            ModDateLastSeen( $item_object->itemnumber );
 
             # If it costs to borrow this book, charge it to the patron's account.
 
             # If it costs to borrow this book, charge it to the patron's account.
-            my ( $charge, $itemtype ) = GetIssuingCharges( $item->itemnumber, $borrower->{'borrowernumber'} );
+            my ( $charge, $itemtype ) = GetIssuingCharges( $item_object->itemnumber, $borrower->{'borrowernumber'} );
             if ( $charge > 0 ) {
                 my $description = "Rental";
                 AddIssuingCharge( $issue, $charge, $description );
             }
 
             if ( $charge > 0 ) {
                 my $description = "Rental";
                 AddIssuingCharge( $issue, $charge, $description );
             }
 
-            my $itemtype = Koha::ItemTypes->find( $item_object->effective_itemtype );
-            if ( $itemtype ) {
+            my $itemtype_object = Koha::ItemTypes->find( $item_object->effective_itemtype );
+            if ( $itemtype_object ) {
                 my $accumulate_charge = $fees->accumulate_rentalcharge();
                 if ( $accumulate_charge > 0 ) {
                     AddIssuingCharge( $issue, $accumulate_charge, 'Daily rental' ) if $accumulate_charge > 0;
                     $charge += $accumulate_charge;
                 my $accumulate_charge = $fees->accumulate_rentalcharge();
                 if ( $accumulate_charge > 0 ) {
                     AddIssuingCharge( $issue, $accumulate_charge, 'Daily rental' ) if $accumulate_charge > 0;
                     $charge += $accumulate_charge;
-                    $item->{charge} = $charge;
+                    $item_unblessed->{charge} = $charge;
                 }
             }
 
                 }
             }
 
@@ -1488,11 +1488,11 @@ sub AddIssue {
                     type => ( $onsite_checkout ? 'onsite_checkout' : 'issue' ),
                     amount         => $charge,
                     other          => ( $sipmode ? "SIP-$sipmode" : '' ),
                     type => ( $onsite_checkout ? 'onsite_checkout' : 'issue' ),
                     amount         => $charge,
                     other          => ( $sipmode ? "SIP-$sipmode" : '' ),
-                    itemnumber     => $item->itemnumber,
-                    itemtype       => $item->effective_itemtype,
-                    location       => $item->location,
+                    itemnumber     => $item_object->itemnumber,
+                    itemtype       => $item_object->effective_itemtype,
+                    location       => $item_object->location,
                     borrowernumber => $borrower->{'borrowernumber'},
                     borrowernumber => $borrower->{'borrowernumber'},
-                    ccode          => $item->ccode,
+                    ccode          => $item_object->ccode,
                 }
             );
 
                 }
             );
 
@@ -1501,14 +1501,14 @@ sub AddIssue {
             my %conditions        = (
                 branchcode   => $branch,
                 categorycode => $borrower->{categorycode},
             my %conditions        = (
                 branchcode   => $branch,
                 categorycode => $borrower->{categorycode},
-                item_type    => $item->effective_itemtype,
+                item_type    => $item_object->effective_itemtype,
                 notification => 'CHECKOUT',
             );
             if ( $circulation_alert->is_enabled_for( \%conditions ) ) {
                 SendCirculationAlert(
                     {
                         type     => 'CHECKOUT',
                 notification => 'CHECKOUT',
             );
             if ( $circulation_alert->is_enabled_for( \%conditions ) ) {
                 SendCirculationAlert(
                     {
                         type     => 'CHECKOUT',
-                        item     => $item->unblessed,
+                        item     => $item_object->unblessed,
                         borrower => $borrower,
                         branch   => $branch,
                     }
                         borrower => $borrower,
                         branch   => $branch,
                     }
@@ -1517,7 +1517,7 @@ sub AddIssue {
             logaction(
                 "CIRCULATION", "ISSUE",
                 $borrower->{'borrowernumber'},
             logaction(
                 "CIRCULATION", "ISSUE",
                 $borrower->{'borrowernumber'},
-                $item->itemnumber,
+                $item_object->itemnumber,
             ) if C4::Context->preference("IssueLog");
         }
     }
             ) if C4::Context->preference("IssueLog");
         }
     }
@@ -1759,7 +1759,7 @@ sub GetBranchItemRule {
 =head2 AddReturn
 
   ($doreturn, $messages, $iteminformation, $borrower) =
 =head2 AddReturn
 
   ($doreturn, $messages, $iteminformation, $borrower) =
-      &AddReturn( $barcode, $branch [,$exemptfine] [,$dropbox] [,$returndate] );
+      &AddReturn( $barcode, $branch [,$exemptfine] [,$returndate] );
 
 Returns a book.
 
 
 Returns a book.
 
@@ -1772,12 +1772,6 @@ Returns a book.
 =item C<$exemptfine> indicates that overdue charges for the item will be
 removed. Optional.
 
 =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.
 
 =item C<$return_date> allows the default return date to be overridden
 by the given return date. Optional.
 
@@ -1836,13 +1830,14 @@ patron who last borrowed the book.
 =cut
 
 sub AddReturn {
 =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
 
     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;
     my $messages;
     my $patron;
     my $doreturn       = 1;
@@ -1937,31 +1932,18 @@ sub AddReturn {
     }
 
     # case of a return of document (deal with issues and holdingbranch)
     }
 
     # 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";
     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";
-        if ($dropbox) {
-            $is_overdue = $issue->is_overdue( $dropboxdate );
-        } else {
-            $is_overdue = $issue->is_overdue;
-        }
+        $is_overdue = $issue->is_overdue( $return_date );
 
         if ($patron) {
             eval {
 
         if ($patron) {
             eval {
-                if ( $dropbox ) {
-                    MarkIssueReturned( $borrowernumber, $item->itemnumber,
-                        $dropboxdate, $patron->privacy );
-                }
-                else {
-                    MarkIssueReturned( $borrowernumber, $item->itemnumber,
-                        $return_date, $patron->privacy );
-                }
+                MarkIssueReturned( $borrowernumber, $item->itemnumber, $return_date, $patron->privacy );
             };
             unless ( $@ ) {
             };
             unless ( $@ ) {
-                if ( ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue ) || $return_date ) {
+                if ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue ) {
                     _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed, return_date => $return_date } );
                 }
             } else {
                     _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed, return_date => $return_date } );
                 }
             } else {
@@ -2035,14 +2017,12 @@ sub AddReturn {
 
     # fix up the overdues in accounts...
     if ($borrowernumber) {
 
     # fix up the overdues in accounts...
     if ($borrowernumber) {
-        my $fix = _FixOverduesOnReturn($borrowernumber, $item->itemnumber, $exemptfine, $dropbox);
+        my $fix = _FixOverduesOnReturn( $borrowernumber, $item->itemnumber, $exemptfine );
         defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $item->itemnumber...) failed!";  # zero is OK, check defined
 
         if ( $issue and $issue->is_overdue ) {
         # fix fine days
         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_unblessed, 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 {
             if ($reminder){
                 $messages->{'PrevDebarred'} = $debardate;
             } else {
@@ -2055,7 +2035,7 @@ sub AddReturn {
              } else {
                   my $borrower_debar_dt = dt_from_string( $patron->debarred );
                   $borrower_debar_dt->truncate(to => 'day');
              } 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;
                   }
                   if ( DateTime->compare( $borrower_debar_dt, $today_dt ) != -1 ) {
                       $messages->{'PrevDebarred'} = $patron->debarred;
                   }
@@ -2210,7 +2190,7 @@ sub MarkIssueReturned {
 
 =head2 _debar_user_on_return
 
 
 =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
 
 
 C<$borrower> borrower hashref
 
@@ -2218,7 +2198,7 @@ C<$item> item hashref
 
 C<$datedue> date due DateTime object
 
 
 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.
 
 Internal function, called only by AddReturn that calculates and updates
  the user fine days, and debars them if necessary.
@@ -2231,6 +2211,7 @@ sub _debar_user_on_return {
     my ( $borrower, $item, $dt_due, $return_date ) = @_;
 
     my $branchcode = _GetCircControlBranch( $item, $borrower );
     my ( $borrower, $item, $dt_due, $return_date ) = @_;
 
     my $branchcode = _GetCircControlBranch( $item, $borrower );
+    $return_date //= dt_from_string();
 
     my $circcontrol = C4::Context->preference('CircControl');
     my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
 
     my $circcontrol = C4::Context->preference('CircControl');
     my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
@@ -2320,21 +2301,20 @@ sub _debar_user_on_return {
 
 =head2 _FixOverduesOnReturn
 
 
 =head2 _FixOverduesOnReturn
 
-   &_FixOverduesOnReturn($brn,$itm, $exemptfine, $dropboxmode);
+   &_FixOverduesOnReturn($borrowernumber, $itemnumber, $exemptfine);
 
 
-C<$brn> borrowernumber
+C<$borrowernumber> borrowernumber
 
 
-C<$itm> itemnumber
+C<$itemnumber> itemnumber
 
 C<$exemptfine> BOOL -- remove overdue charge associated with this issue. 
 
 C<$exemptfine> BOOL -- remove overdue charge associated with this issue. 
-C<$dropboxmode> BOOL -- remove lastincrement on overdue charge associated with this issue.
 
 Internal function
 
 =cut
 
 sub _FixOverduesOnReturn {
 
 Internal function
 
 =cut
 
 sub _FixOverduesOnReturn {
-    my ($borrowernumber, $item, $exemptfine, $dropbox ) = @_;
+    my ( $borrowernumber, $item, $exemptfine ) = @_;
     unless( $borrowernumber ) {
         warn "_FixOverduesOnReturn() not supplied valid borrowernumber";
         return;
     unless( $borrowernumber ) {
         warn "_FixOverduesOnReturn() not supplied valid borrowernumber";
         return;
@@ -2374,30 +2354,6 @@ sub _FixOverduesOnReturn {
         if (C4::Context->preference("FinesLog")) {
             &logaction("FINES", 'MODIFY',$borrowernumber,"Overdue forgiven: item $item");
         }
         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;
-
-        Koha::Account::Offset->new(
-            {
-                debit_id => $accountline->id,
-                type => 'Dropbox',
-                amount => $accountline->lastincrement * -1,
-            }
-        )->store();
-
-        if ( C4::Context->preference("FinesLog") ) {
-            &logaction( "FINES", 'MODIFY', $borrowernumber,
-                "Dropbox adjustment $amt, item $item" );
-        }
-
-        $accountline->accounttype('F');
-
-        if ( $outstanding >= 0 && $amt >= 0 ) {
-            $accountline->amount($amt);
-            $accountline->amountoutstanding($outstanding);
-        }
-
     } else {
         $accountline->accounttype('F');
     }
     } else {
         $accountline->accounttype('F');
     }
@@ -2853,10 +2809,10 @@ sub AddRenewal {
     my $datedue         = shift;
     my $lastreneweddate = shift || DateTime->now(time_zone => C4::Context->tz);
 
     my $datedue         = shift;
     my $lastreneweddate = shift || DateTime->now(time_zone => C4::Context->tz);
 
-    my $item   = Koha::Items->find($itemnumber) or return;
-    my $biblio = $item->biblio;
-    my $issue  = $item->checkout;
-    my $item_unblessed = $item->unblessed;
+    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;
 
 
     my $dbh = C4::Context->dbh;
 
@@ -2872,7 +2828,7 @@ sub AddRenewal {
     my $patron = Koha::Patrons->find( $borrowernumber ) or return; # FIXME Should do more than just return
     my $patron_unblessed = $patron->unblessed;
 
     my $patron = Koha::Patrons->find( $borrowernumber ) or return; # FIXME Should do more than just return
     my $patron_unblessed = $patron->unblessed;
 
-    my $circ_library = Koha::Libraries->find( _GetCircControlBranch($item, $patron_unblessed) );
+    my $circ_library = Koha::Libraries->find( _GetCircControlBranch($item_unblessed, $patron_unblessed) );
 
     if ( C4::Context->preference('CalculateFinesOnReturn') && $issue->is_overdue ) {
         _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed } );
 
     if ( C4::Context->preference('CalculateFinesOnReturn') && $issue->is_overdue ) {
         _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed } );
@@ -2882,13 +2838,13 @@ sub AddRenewal {
     # 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.
     # 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) {
 
     unless ($datedue) {
 
-        my $itemtype = $item->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 = (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_unblessed, $patron_unblessed), $patron_unblessed, 'is a renewal');
+        $datedue =  CalcDateDue($datedue, $itemtype, $circ_library, $patron_unblessed, 'is a renewal');
     }
 
     my $fees = Koha::Charges::Fees->new(
     }
 
     my $fees = Koha::Charges::Fees->new(
@@ -2896,7 +2852,8 @@ sub AddRenewal {
             patron    => $patron,
             library   => $circ_library,
             item      => $item_object,
             patron    => $patron,
             library   => $circ_library,
             item      => $item_object,
-            to_date   => dt_from_string( $datedue ),
+            from_date => dt_from_string( $issue->date_due, 'sql' ),
+            to_date   => dt_from_string($datedue),
         }
     );
 
         }
     );
 
@@ -2911,22 +2868,22 @@ sub AddRenewal {
     $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $lastreneweddate, $borrowernumber, $itemnumber );
 
     # Update the renewal count on the item, and tell zebra to reindex
     $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->renewals + 1;
-    ModItem( { renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item->biblionumber, $itemnumber, { log_action => 0 } );
+    $renews = $item_object->renewals + 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 ) {
 
     # Charge a new rental fee, if applicable
     my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
     if ( $charge > 0 ) {
-        my $description = "Renewal of Rental Item " . $biblio->title . " " .$item->barcode;
+        my $description = "Renewal of Rental Item " . $biblio->title . " " .$item_object->barcode;
         AddIssuingCharge($issue, $charge, $description);
     }
 
     # Charge a new accumulate rental fee, if applicable
         AddIssuingCharge($issue, $charge, $description);
     }
 
     # Charge a new accumulate rental fee, if applicable
-    my $itemtype = Koha::ItemTypes->find( $item_object->effective_itemtype );
-    if ( $itemtype ) {
+    my $itemtype_object = Koha::ItemTypes->find( $itemtype );
+    if ( $itemtype_object ) {
         my $accumulate_charge = $fees->accumulate_rentalcharge();
         if ( $accumulate_charge > 0 ) {
         my $accumulate_charge = $fees->accumulate_rentalcharge();
         if ( $accumulate_charge > 0 ) {
-            my $type_desc = "Renewal of Daily Rental Item " . $biblio->title . " $item->{'barcode'}";
+            my $type_desc = "Renewal of Daily Rental Item " . $biblio->title . " $item_unblessed->{'barcode'}";
             AddIssuingCharge( $issue, $accumulate_charge, $type_desc )
         }
         $charge += $accumulate_charge;
             AddIssuingCharge( $issue, $accumulate_charge, $type_desc )
         }
         $charge += $accumulate_charge;
@@ -2938,7 +2895,7 @@ sub AddRenewal {
         my %conditions        = (
             branchcode   => $branch,
             categorycode => $patron->categorycode,
         my %conditions        = (
             branchcode   => $branch,
             categorycode => $patron->categorycode,
-            item_type    => $item->effective_itemtype,
+            item_type    => $itemtype,
             notification => 'CHECKOUT',
         );
         if ( $circulation_alert->is_enabled_for( \%conditions ) ) {
             notification => 'CHECKOUT',
         );
         if ( $circulation_alert->is_enabled_for( \%conditions ) ) {
@@ -2973,10 +2930,10 @@ sub AddRenewal {
             type           => 'renew',
             amount         => $charge,
             itemnumber     => $itemnumber,
             type           => 'renew',
             amount         => $charge,
             itemnumber     => $itemnumber,
-            itemtype       => $item->effective_itemtype,
-            location       => $item->location,
+            itemtype       => $itemtype,
+            location       => $item_object->location,
             borrowernumber => $borrowernumber,
             borrowernumber => $borrowernumber,
-            ccode          => $item->ccode,
+            ccode          => $item_object->ccode,
         }
     );
 
         }
     );
 
@@ -4123,7 +4080,7 @@ sub _CalculateAndUpdateFine {
       : ( $control eq 'PatronLibrary' )   ? $borrower->{branchcode}
       :                                     $issue->branchcode;
 
       : ( $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 );
 
     my ( $amount, $unitcounttotal, $unitcount  ) =
       C4::Overdues::CalcFine( $item, $borrower->{categorycode}, $control_branchcode, $datedue, $date_returned );
Koha ILS @ FFZG - based on git://git.koha.org/pub/scm/koha.git