Bug 20819: Add consent to self-registration process
[koha.git] / C4 / Circulation.pm
index 8bd8d85..dce77ea 100644 (file)
@@ -22,6 +22,7 @@ package C4::Circulation;
 use strict;
 #use warnings; FIXME - Bug 2505
 use DateTime;
+use POSIX qw( floor );
 use Koha::DateUtils;
 use C4::Context;
 use C4::Stats;
@@ -278,10 +279,6 @@ is a reference-to-hash which may have any of the following keys:
 
 There is no item in the catalog with the given barcode. The value is C<$barcode>.
 
-=item C<IsPermanent>
-
-The item's home branch is permanent. This doesn't prevent the item from being transferred, though. The value is the code of the item's home branch.
-
 =item C<DestinationEqualsHolding>
 
 The item is already at the branch to which it is being transferred. The transfer is nonetheless considered to have failed. The value should be ignored.
@@ -336,15 +333,6 @@ sub transferbook {
         }
     }
 
-    # if is permanent...
-    # FIXME Is this still used by someone?
-    # See other FIXME in AddReturn
-    my $library = Koha::Libraries->find($hbr);
-    if ( $library and $library->get_categories->search({'me.categorycode' => 'PE'})->count ) {
-        $messages->{'IsPermanent'} = $hbr;
-        $dotransfer = 0;
-    }
-
     # can't transfer book if is already there....
     if ( $fbr eq $tbr ) {
         $messages->{'DestinationEqualsHolding'} = 1;
@@ -474,7 +462,7 @@ sub TooMany {
         my $max_checkouts_allowed = $issuing_rule->maxissueqty;
         my $max_onsite_checkouts_allowed = $issuing_rule->maxonsiteissueqty;
 
-        if ( $onsite_checkout ) {
+        if ( $onsite_checkout and defined $max_onsite_checkouts_allowed ) {
             if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
                 return {
                     reason => 'TOO_MANY_ONSITE_CHECKOUTS',
@@ -528,7 +516,7 @@ sub TooMany {
         my $max_checkouts_allowed = $branch_borrower_circ_rule->{maxissueqty};
         my $max_onsite_checkouts_allowed = $branch_borrower_circ_rule->{maxonsiteissueqty};
 
-        if ( $onsite_checkout ) {
+        if ( $onsite_checkout and defined $max_onsite_checkouts_allowed ) {
             if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
                 return {
                     reason => 'TOO_MANY_ONSITE_CHECKOUTS',
@@ -567,7 +555,7 @@ sub TooMany {
 
 =head2 CanBookBeIssued
 
-  ( $issuingimpossible, $needsconfirmation, [ $alerts ] ) =  CanBookBeIssued( $borrower,
+  ( $issuingimpossible, $needsconfirmation, [ $alerts ] ) =  CanBookBeIssued( $patron,
                       $barcode, $duedate, $inprocess, $ignore_reserves, $params );
 
 Check if a book can be issued.
@@ -580,7 +568,7 @@ data is keyed in lower case!
 
 =over 4
 
-=item C<$borrower> hash with borrower informations (from Koha::Patron->unblessed)
+=item C<$patron> is a Koha::Patron
 
 =item C<$barcode> is the bar code of the book being issued.
 
@@ -671,7 +659,7 @@ if the borrower borrows to much things
 =cut
 
 sub CanBookBeIssued {
-    my ( $borrower, $barcode, $duedate, $inprocess, $ignore_reserves, $params ) = @_;
+    my ( $patron, $barcode, $duedate, $inprocess, $ignore_reserves, $params ) = @_;
     my %needsconfirmation;    # filled with problems that needs confirmations
     my %issuingimpossible;    # filled with problems that causes the issue to be IMPOSSIBLE
     my %alerts;               # filled with messages that shouldn't stop issuing, but the librarian should be aware of.
@@ -681,16 +669,18 @@ sub CanBookBeIssued {
     my $override_high_holds = $params->{override_high_holds} || 0;
 
     my $item = GetItem(undef, $barcode );
-    my $issue = Koha::Checkouts->find( { itemnumber => $item->{itemnumber} } );
-       my $biblioitem = GetBiblioItemData($item->{biblioitemnumber});
-       $item->{'itemtype'}=$item->{'itype'}; 
-    my $dbh             = C4::Context->dbh;
-
     # MANDATORY CHECKS - unless item exists, nothing else matters
-    unless ( $item->{barcode} ) {
+    unless ( $item ) {
         $issuingimpossible{UNKNOWN_BARCODE} = 1;
     }
-       return ( \%issuingimpossible, \%needsconfirmation ) if %issuingimpossible;
+    return ( \%issuingimpossible, \%needsconfirmation ) if %issuingimpossible;
+
+    my $issue = Koha::Checkouts->find( { itemnumber => $item->{itemnumber} } );
+    my $biblio = Koha::Biblios->find( $item->{biblionumber} );
+    my $biblioitem = $biblio->biblioitem;
+    my $effective_itemtype = $item->{itype}; # GetItem deals with that
+    my $dbh             = C4::Context->dbh;
+    my $patron_unblessed = $patron->unblessed;
 
     #
     # DUE DATE is OK ? -- should already have checked.
@@ -702,9 +692,8 @@ sub CanBookBeIssued {
     unless ( $duedate ) {
         my $issuedate = $now->clone();
 
-        my $branch = _GetCircControlBranch($item,$borrower);
-        my $itype = ( C4::Context->preference('item-level_itypes') ) ? $item->{'itype'} : $biblioitem->{'itemtype'};
-        $duedate = CalcDateDue( $issuedate, $itype, $branch, $borrower );
+        my $branch = _GetCircControlBranch($item, $patron_unblessed);
+        $duedate = CalcDateDue( $issuedate, $effective_itemtype, $branch, $patron_unblessed );
 
         # Offline circ calls AddIssue directly, doesn't run through here
         #  So issuingimpossible should be ok.
@@ -722,43 +711,33 @@ sub CanBookBeIssued {
     #
     # BORROWER STATUS
     #
-    my $patron = Koha::Patrons->find( $borrower->{borrowernumber} );
     if ( $patron->category->category_type eq 'X' && (  $item->{barcode}  )) {
        # stats only borrower -- add entry to statistics table, and return issuingimpossible{STATS} = 1  .
         &UpdateStats({
                      branch => C4::Context->userenv->{'branch'},
                      type => 'localuse',
                      itemnumber => $item->{'itemnumber'},
-                     itemtype => $item->{'itype'},
-                     borrowernumber => $borrower->{'borrowernumber'},
+                     itemtype => $effective_itemtype,
+                     borrowernumber => $patron->borrowernumber,
                      ccode => $item->{'ccode'}}
                     );
         ModDateLastSeen( $item->{'itemnumber'} );
         return( { STATS => 1 }, {});
     }
 
-    my $flags = C4::Members::patronflags( $borrower );
-    if ( ref $flags ) {
-        if ( $flags->{GNA} ) {
-            $issuingimpossible{GNA} = 1;
-        }
-        if ( $flags->{'LOST'} ) {
-            $issuingimpossible{CARD_LOST} = 1;
-        }
-        if ( $flags->{'DBARRED'} ) {
-            $issuingimpossible{DEBARRED} = 1;
-        }
+    if ( $patron->gonenoaddress == 1 ) {
+        $issuingimpossible{GNA} = 1;
     }
-    if ( !defined $borrower->{dateexpiry} || $borrower->{'dateexpiry'} eq '0000-00-00') {
+
+    if ( $patron->lost == 1 ) {
+        $issuingimpossible{CARD_LOST} = 1;
+    }
+    if ( $patron->is_debarred ) {
+        $issuingimpossible{DEBARRED} = 1;
+    }
+
+    if ( $patron->is_expired ) {
         $issuingimpossible{EXPIRED} = 1;
-    } else {
-        my $expiry_dt = dt_from_string( $borrower->{dateexpiry}, 'sql', 'floating' );
-        $expiry_dt->truncate( to => 'day');
-        my $today = $now->clone()->truncate(to => 'day');
-        $today->set_time_zone( 'floating' );
-        if ( DateTime->compare($today, $expiry_dt) == 1 ) {
-            $issuingimpossible{EXPIRED} = 1;
-        }
     }
 
     #
@@ -766,8 +745,10 @@ sub CanBookBeIssued {
     #
 
     # DEBTS
-    my ($balance, $non_issue_charges, $other_charges) =
-      C4::Members::GetMemberAccountBalance( $borrower->{'borrowernumber'} );
+    my $account = $patron->account;
+    my $balance = $account->balance;
+    my $non_issues_charges = $account->non_issues_charges;
+    my $other_charges = $balance - $non_issues_charges;
 
     my $amountlimit = C4::Context->preference("noissuescharge");
     my $allowfineoverride = C4::Context->preference("AllowFineOverride");
@@ -777,12 +758,10 @@ 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 $p = Koha::Patrons->find( $borrower->{borrowernumber} );
-        my @guarantees = $p->guarantees();
+        my @guarantees = $patron->guarantees();
         my $guarantees_non_issues_charges;
         foreach my $g ( @guarantees ) {
-            my ( $b, $n, $o ) = C4::Members::GetMemberAccountBalance( $g->id );
-            $guarantees_non_issues_charges += $n;
+            $guarantees_non_issues_charges += $g->account->non_issues_charges;
         }
 
         if ( $guarantees_non_issues_charges > $no_issues_charge_guarantees && !$inprocess && !$allowfineoverride) {
@@ -795,21 +774,21 @@ sub CanBookBeIssued {
     }
 
     if ( C4::Context->preference("IssuingInProcess") ) {
-        if ( $non_issue_charges > $amountlimit && !$inprocess && !$allowfineoverride) {
-            $issuingimpossible{DEBT} = sprintf( "%.2f", $non_issue_charges );
-        } elsif ( $non_issue_charges > $amountlimit && !$inprocess && $allowfineoverride) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_charges );
-        } elsif ( $allfinesneedoverride && $non_issue_charges > 0 && $non_issue_charges <= $amountlimit && !$inprocess ) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_charges );
+        if ( $non_issues_charges > $amountlimit && !$inprocess && !$allowfineoverride) {
+            $issuingimpossible{DEBT} = $non_issues_charges;
+        } elsif ( $non_issues_charges > $amountlimit && !$inprocess && $allowfineoverride) {
+            $needsconfirmation{DEBT} = $non_issues_charges;
+        } elsif ( $allfinesneedoverride && $non_issues_charges > 0 && $non_issues_charges <= $amountlimit && !$inprocess ) {
+            $needsconfirmation{DEBT} = $non_issues_charges;
         }
     }
     else {
-        if ( $non_issue_charges > $amountlimit && $allowfineoverride ) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_charges );
-        } elsif ( $non_issue_charges > $amountlimit && !$allowfineoverride) {
-            $issuingimpossible{DEBT} = sprintf( "%.2f", $non_issue_charges );
-        } elsif ( $non_issue_charges > 0 && $allfinesneedoverride ) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_charges );
+        if ( $non_issues_charges > $amountlimit && $allowfineoverride ) {
+            $needsconfirmation{DEBT} = $non_issues_charges;
+        } elsif ( $non_issues_charges > $amountlimit && !$allowfineoverride) {
+            $issuingimpossible{DEBT} = $non_issues_charges;
+        } elsif ( $non_issues_charges > 0 && $allfinesneedoverride ) {
+            $needsconfirmation{DEBT} = $non_issues_charges;
         }
     }
 
@@ -817,7 +796,9 @@ sub CanBookBeIssued {
         $alerts{OTHER_CHARGES} = sprintf( "%.2f", $other_charges );
     }
 
-    $patron = Koha::Patrons->find( $borrower->{borrowernumber} );
+    $patron = Koha::Patrons->find( $patron->borrowernumber ); # FIXME Refetch just in case, to avoid regressions. But must not be needed
+    $patron_unblessed = $patron->unblessed;
+
     if ( my $debarred_date = $patron->is_debarred ) {
          # patron has accrued fine days or has a restriction. $count is a date
         if ($debarred_date eq '9999-12-31') {
@@ -839,7 +820,7 @@ sub CanBookBeIssued {
     #
     # CHECK IF BOOK ALREADY ISSUED TO THIS BORROWER
     #
-    if ( $issue && $issue->borrowernumber eq $borrower->{'borrowernumber'} ){
+    if ( $issue && $issue->borrowernumber eq $patron->borrowernumber ){
 
         # Already issued to current borrower.
         # If it is an on-site checkout if it can be switched to a normal checkout
@@ -850,7 +831,7 @@ sub CanBookBeIssued {
             $messages{ONSITE_CHECKOUT_WILL_BE_SWITCHED} = 1;
         } else {
             my ($CanBookBeRenewed,$renewerror) = CanBookBeRenewed(
-                $borrower->{'borrowernumber'},
+                $patron->borrowernumber,
                 $item->{'itemnumber'},
             );
             if ( $CanBookBeRenewed == 0 ) {    # no more renewals allowed
@@ -892,8 +873,8 @@ sub CanBookBeIssued {
           C4::Context->preference('SwitchOnSiteCheckouts')
       and $issue
       and $issue->onsite_checkout
-      and $issue->borrowernumber == $borrower->{'borrowernumber'} ? 1 : 0 );
-    my $toomany = TooMany( $borrower, $item->{biblionumber}, $item, { onsite_checkout => $onsite_checkout, switch_onsite_checkout => $switch_onsite_checkout, } );
+      and $issue->borrowernumber == $patron->borrowernumber ? 1 : 0 );
+    my $toomany = TooMany( $patron_unblessed, $item->{biblionumber}, $item, { onsite_checkout => $onsite_checkout, switch_onsite_checkout => $switch_onsite_checkout, } );
     # if TooMany max_allowed returns 0 the user doesn't have permission to check out this book
     if ( $toomany && not exists $needsconfirmation{RENEW_ISSUE} ) {
         if ( $toomany->{max_allowed} == 0 ) {
@@ -913,7 +894,7 @@ sub CanBookBeIssued {
     #
     # CHECKPREVCHECKOUT: CHECK IF ITEM HAS EVER BEEN LENT TO PATRON
     #
-    $patron = Koha::Patrons->find($borrower->{borrowernumber});
+    $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));
@@ -936,25 +917,28 @@ sub CanBookBeIssued {
         if (C4::Context->preference('item-level_itypes')){
             # this should probably be a subroutine
             my $sth = $dbh->prepare("SELECT notforloan FROM itemtypes WHERE itemtype = ?");
-            $sth->execute($item->{'itemtype'});
+            $sth->execute($effective_itemtype);
             my $notforloan=$sth->fetchrow_hashref();
             if ($notforloan->{'notforloan'}) {
                 if (!C4::Context->preference("AllowNotForLoanOverride")) {
                     $issuingimpossible{NOT_FOR_LOAN} = 1;
-                    $issuingimpossible{itemtype_notforloan} = $item->{'itype'};
+                    $issuingimpossible{itemtype_notforloan} = $effective_itemtype;
                 } else {
                     $needsconfirmation{NOT_FOR_LOAN_FORCING} = 1;
-                    $needsconfirmation{itemtype_notforloan} = $item->{'itype'};
+                    $needsconfirmation{itemtype_notforloan} = $effective_itemtype;
                 }
             }
         }
-        elsif ($biblioitem->{'notforloan'} == 1){
-            if (!C4::Context->preference("AllowNotForLoanOverride")) {
-                $issuingimpossible{NOT_FOR_LOAN} = 1;
-                $issuingimpossible{itemtype_notforloan} = $biblioitem->{'itemtype'};
-            } else {
-                $needsconfirmation{NOT_FOR_LOAN_FORCING} = 1;
-                $needsconfirmation{itemtype_notforloan} = $biblioitem->{'itemtype'};
+        else {
+            my $itemtype = Koha::ItemTypes->find($biblioitem->itemtype);
+            if ( $itemtype and $itemtype->notforloan == 1){
+                if (!C4::Context->preference("AllowNotForLoanOverride")) {
+                    $issuingimpossible{NOT_FOR_LOAN} = 1;
+                    $issuingimpossible{itemtype_notforloan} = $effective_itemtype;
+                } else {
+                    $needsconfirmation{NOT_FOR_LOAN_FORCING} = 1;
+                    $needsconfirmation{itemtype_notforloan} = $effective_itemtype;
+                }
             }
         }
     }
@@ -980,8 +964,8 @@ sub CanBookBeIssued {
                 $issuingimpossible{ITEMNOTSAMEBRANCH} = 1;
                 $issuingimpossible{'itemhomebranch'} = $item->{C4::Context->preference("HomeOrHoldingBranch")};
             }
-            $needsconfirmation{BORRNOTSAMEBRANCH} = $borrower->{'branchcode'}
-              if ( $borrower->{'branchcode'} ne $userenv->{branch} );
+            $needsconfirmation{BORRNOTSAMEBRANCH} = $patron->branchcode
+              if ( $patron->branchcode ne $userenv->{branch} );
         }
     }
     #
@@ -990,7 +974,7 @@ sub CanBookBeIssued {
     my $rentalConfirmation = C4::Context->preference("RentalFeesCheckoutConfirmation");
 
     if ( $rentalConfirmation ){
-        my ($rentalCharge) = GetIssuingCharges( $item->{'itemnumber'}, $borrower->{'borrowernumber'} );
+        my ($rentalCharge) = GetIssuingCharges( $item->{'itemnumber'}, $patron->borrowernumber );
         if ( $rentalCharge > 0 ){
             $needsconfirmation{RENTALCHARGE} = $rentalCharge;
         }
@@ -1001,7 +985,7 @@ sub CanBookBeIssued {
         my ( $restype, $res ) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
         if ($restype) {
             my $resbor = $res->{'borrowernumber'};
-            if ( $resbor ne $borrower->{'borrowernumber'} ) {
+            if ( $resbor ne $patron->borrowernumber ) {
                 my $patron = Koha::Patrons->find( $resbor );
                 if ( $restype eq "Waiting" )
                 {
@@ -1030,8 +1014,8 @@ sub CanBookBeIssued {
     }
 
     ## CHECK AGE RESTRICTION
-    my $agerestriction  = $biblioitem->{'agerestriction'};
-    my ($restriction_age, $daysToAgeRestriction) = GetAgeRestriction( $agerestriction, $borrower );
+    my $agerestriction  = $biblioitem->agerestriction;
+    my ($restriction_age, $daysToAgeRestriction) = GetAgeRestriction( $agerestriction, $patron->unblessed );
     if ( $daysToAgeRestriction && $daysToAgeRestriction > 0 ) {
         if ( C4::Context->preference('AgeRestrictionOverride') ) {
             $needsconfirmation{AGE_RESTRICTION} = "$agerestriction";
@@ -1043,7 +1027,7 @@ sub CanBookBeIssued {
 
     ## check for high holds decreasing loan period
     if ( C4::Context->preference('decreaseLoanHighHolds') ) {
-        my $check = checkHighHolds( $item, $borrower );
+        my $check = checkHighHolds( $item, $patron_unblessed );
 
         if ( $check->{exceeded} ) {
             if ($override_high_holds) {
@@ -1076,9 +1060,10 @@ sub CanBookBeIssued {
         require C4::Serials;
         my $is_a_subscription = C4::Serials::CountSubscriptionFromBiblionumber($biblionumber);
         unless ($is_a_subscription) {
+            # FIXME Should be $patron->checkouts($args);
             my $checkouts = Koha::Checkouts->search(
                 {
-                    borrowernumber => $borrower->{borrowernumber},
+                    borrowernumber => $patron->borrowernumber,
                     biblionumber   => $biblionumber,
                 },
                 {
@@ -1198,7 +1183,7 @@ sub checkHighHolds {
             }
 
             # Remove any items that are not holdable for this patron
-            @items = grep { CanItemBeReserved( $borrower->{borrowernumber}, $_->itemnumber ) eq 'OK' } @items;
+            @items = grep { CanItemBeReserved( $borrower->{borrowernumber}, $_->itemnumber )->{status} eq 'OK' } @items;
 
             my $items_count = scalar @items;
 
@@ -1332,7 +1317,7 @@ sub AddIssue {
                 AddReturn( $item->{'barcode'}, C4::Context->userenv->{'branch'} );
             }
 
-            MoveReserve( $item->{'itemnumber'}, $borrower->{'borrowernumber'}, $cancelreserve );
+            C4::Reserves::MoveReserve( $item->{'itemnumber'}, $borrower->{'borrowernumber'}, $cancelreserve );
 
             # Starting process for transfer job (checking transfert and validate it if we have one)
             my ($datesent) = GetTransfers( $item->{'itemnumber'} );
@@ -1416,14 +1401,15 @@ sub AddIssue {
                     datelastborrowed => DateTime->now( time_zone => C4::Context->tz() )->ymd(),
                 },
                 $item->{'biblionumber'},
-                $item->{'itemnumber'}
+                $item->{'itemnumber'},
+                { log_action => 0 }
             );
             ModDateLastSeen( $item->{'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( $item->{'itemnumber'}, $borrower->{'borrowernumber'}, $charge );
+                AddIssuingCharge( $issue, $charge );
                 $item->{'charge'} = $charge;
             }
 
@@ -1436,6 +1422,7 @@ sub AddIssue {
                     other          => ( $sipmode ? "SIP-$sipmode" : '' ),
                     itemnumber     => $item->{'itemnumber'},
                     itemtype       => $item->{'itype'},
+                    location       => $item->{location},
                     borrowernumber => $borrower->{'borrowernumber'},
                     ccode          => $item->{'ccode'}
                 }
@@ -1765,12 +1752,6 @@ No item with this barcode exists. The value is C<$barcode>.
 
 The book is not currently on loan. The value is C<$barcode>.
 
-=item C<IsPermanent>
-
-The book's home branch is a permanent collection. If you have borrowed
-this book, you are not allowed to return it. The value is the code for
-the book's home branch.
-
 =item C<withdrawn>
 
 This book has been withdrawn/cancelled. The value should be ignored.
@@ -1836,6 +1817,7 @@ sub AddReturn {
                 . Dumper($issue->unblessed) . "\n";
     } else {
         $messages->{'NotIssued'} = $barcode;
+        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.
@@ -1854,7 +1836,7 @@ sub AddReturn {
             $item->{location} = $item->{permanent_location};
         }
 
-        ModItem( $item, $item->{'biblionumber'}, $item->{'itemnumber'} );
+        ModItem( $item, $item->{'biblionumber'}, $item->{'itemnumber'}, { log_action => 0 } );
     }
 
         # full item data, but no borrowernumber or checkout info (no issue)
@@ -1878,23 +1860,13 @@ sub AddReturn {
             foreach my $key ( keys %$rules ) {
                 if ( $item->{notforloan} eq $key ) {
                     $messages->{'NotForLoanStatusUpdated'} = { from => $item->{notforloan}, to => $rules->{$key} };
-                    ModItem( { notforloan => $rules->{$key} }, undef, $itemnumber );
+                    ModItem( { notforloan => $rules->{$key} }, undef, $itemnumber, { log_action => 0 } );
                     last;
                 }
             }
         }
     }
 
-
-    # check if the book is in a permanent collection....
-    # FIXME -- This 'PE' attribute is largely undocumented.  afaict, there's no user interface that reflects this functionality.
-    if ( $returnbranch ) {
-        my $library = Koha::Libraries->find($returnbranch);
-        if ( $library and $library->get_categories->search({'me.categorycode' => 'PE'})->count ) {
-            $messages->{'IsPermanent'} = $returnbranch;
-        }
-    }
-
     # check if the return is allowed at this branch
     my ($returnallowed, $message) = CanBookBeReturned($item, $branch);
     unless ($returnallowed){
@@ -1911,6 +1883,10 @@ sub AddReturn {
         $doreturn = 0 if C4::Context->preference("BlockReturnOfWithdrawnItems");
     }
 
+    if ( $item->{itemlost} and C4::Context->preference("BlockReturnOfLostItems") ) {
+        $doreturn = 0;
+    }
+
     # case of a return of document (deal with issues and holdingbranch)
     my $today = DateTime->now( time_zone => C4::Context->tz() );
 
@@ -1950,7 +1926,7 @@ sub AddReturn {
 
         }
 
-        ModItem({ onloan => undef }, $item->{biblionumber}, $item->{'itemnumber'});
+        ModItem( { onloan => undef }, $item->{biblionumber}, $item->{itemnumber}, { log_action => 0 } );
     }
 
     # the holdingbranch is updated if the document is returned to another location.
@@ -1960,7 +1936,9 @@ sub AddReturn {
         UpdateHoldingbranch($branch, $item->{'itemnumber'});
         $item->{'holdingbranch'} = $branch; # update item data holdingbranch too
     }
-    ModDateLastSeen( $item->{'itemnumber'} );
+
+    my $leave_item_lost = C4::Context->preference("BlockReturnOfLostItems") ? 1 : 0;
+    ModDateLastSeen( $item->{itemnumber}, $leave_item_lost );
 
     # check if we have a transfer for this document
     my ($datesent,$frombranch,$tobranch) = GetTransfers( $item->{'itemnumber'} );
@@ -1988,8 +1966,7 @@ sub AddReturn {
     # fix up the accounts.....
     if ( $item->{'itemlost'} ) {
         $messages->{'WasLost'} = 1;
-
-        if ( $item->{'itemlost'} ) {
+        unless ( C4::Context->preference("BlockReturnOfLostItems") ) {
             if (
                 Koha::RefundLostItemFeeRules->should_refund(
                     {
@@ -2000,7 +1977,8 @@ sub AddReturn {
                 )
               )
             {
-                _FixAccountForLostAndReturned( $item->{'itemnumber'}, $borrowernumber, $barcode );
+                _FixAccountForLostAndReturned( $item->{'itemnumber'},
+                    $borrowernumber, $barcode );
                 $messages->{'LostItemFeeRefunded'} = 1;
             }
         }
@@ -2013,6 +1991,7 @@ sub AddReturn {
 
         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 );
             if ($reminder){
@@ -2186,7 +2165,7 @@ sub MarkIssueReturned {
         # And finally delete the issue
         $issue->delete;
 
-        ModItem( { 'onloan' => undef }, undef, $itemnumber );
+        ModItem( { 'onloan' => undef }, undef, $itemnumber, { log_action => 0 } );
 
         if ( C4::Context->preference('StoreLastBorrower') ) {
             my $item = Koha::Items->find( $itemnumber );
@@ -2267,8 +2246,25 @@ sub _debar_user_on_return {
                 }
             }
 
-            my $new_debar_dt =
-              $return_date->clone()->add_duration( $suspension_days );
+            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);
+            }
 
             Koha::Patron::Debarments::AddUniqueDebarment({
                 borrowernumber => $borrower->{borrowernumber},
@@ -2420,12 +2416,12 @@ sub _FixAccountForLostAndReturned {
             description  => "Item Returned " . $item_id,
             account_type => 'CR',
             offset_type  => 'Lost Item Return',
-            accounlines  => [$accountline],
+            lines        => [$accountline],
 
         }
     );
 
-    ModItem( { paidfor => '' }, undef, $itemnumber );
+    ModItem( { paidfor => '' }, undef, $itemnumber, { log_action => 0 } );
 
     return $credit_id;
 }
@@ -2546,12 +2542,15 @@ sub GetUpcomingDueIssues {
     my $dbh = C4::Context->dbh;
 
     my $statement = <<END_SQL;
-SELECT issues.*, items.itype as itemtype, items.homebranch, TO_DAYS( date_due )-TO_DAYS( NOW() ) as days_until_due, branches.branchemail
-FROM issues 
-LEFT JOIN items USING (itemnumber)
-LEFT OUTER JOIN branches USING (branchcode)
-WHERE returndate is NULL
-HAVING days_until_due >= 0 AND days_until_due <= ?
+SELECT *
+FROM (
+    SELECT issues.*, items.itype as itemtype, items.homebranch, TO_DAYS( date_due )-TO_DAYS( NOW() ) as days_until_due, branches.branchemail
+    FROM issues
+    LEFT JOIN items USING (itemnumber)
+    LEFT OUTER JOIN branches USING (branchcode)
+    WHERE returndate is NULL
+) tmp
+WHERE days_until_due >= 0 AND days_until_due <= ?
 END_SQL
 
     my @bind_parameters = ( $params->{'days_in_advance'} );
@@ -2694,6 +2693,11 @@ sub CanBookBeRenewed {
     }
 
     if ( $issue->auto_renew ) {
+
+        if ( $patron->category->effective_BlockExpiredPatronOpacActions and $patron->is_expired ) {
+            return ( 0, 'auto_account_expired' );
+        }
+
         if ( defined $issuing_rule->no_auto_renewal_after
                 and $issuing_rule->no_auto_renewal_after ne "" ) {
             # Get issue_date and add no_auto_renewal_after
@@ -2717,7 +2721,7 @@ sub CanBookBeRenewed {
 
         if ( C4::Context->preference('OPACFineNoRenewalsBlockAutoRenew') ) {
             my $fine_no_renewals = C4::Context->preference("OPACFineNoRenewals");
-            my ( $amountoutstanding ) = C4::Members::GetMemberAccountRecords($patron->borrowernumber);
+            my $amountoutstanding = $patron->account->balance;
             if ( $amountoutstanding and $amountoutstanding > $fine_no_renewals ) {
                 return ( 0, "auto_too_much_oweing" );
             }
@@ -2842,7 +2846,7 @@ sub AddRenewal {
 
     # 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);
+    ModItem( { renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item->{biblionumber}, $itemnumber, { log_action => 0 } );
 
     # Charge a new rental fee, if applicable?
     my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
@@ -2903,6 +2907,7 @@ sub AddRenewal {
             amount         => $charge,
             itemnumber     => $itemnumber,
             itemtype       => $item->{itype},
+            location       => $item->{location},
             borrowernumber => $borrowernumber,
             ccode          => $item->{'ccode'}
         }
@@ -3177,22 +3182,25 @@ sub _get_discount_from_rule {
 
 =head2 AddIssuingCharge
 
-  &AddIssuingCharge( $itemno, $borrowernumber, $charge )
+  &AddIssuingCharge( $checkout, $charge )
 
 =cut
 
 sub AddIssuingCharge {
-    my ( $itemnumber, $borrowernumber, $charge ) = @_;
+    my ( $checkout, $charge ) = @_;
 
-    my $nextaccntno = getnextacctno($borrowernumber);
+    # FIXME What if checkout does not exist?
+
+    my $nextaccntno = getnextacctno($checkout->borrowernumber);
 
     my $manager_id  = 0;
     $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv;
 
     my $accountline = Koha::Account::Line->new(
         {
-            borrowernumber    => $borrowernumber,
-            itemnumber        => $itemnumber,
+            borrowernumber    => $checkout->borrowernumber,
+            itemnumber        => $checkout->itemnumber,
+            issue_id          => $checkout->issue_id,
             accountno         => $nextaccntno,
             amount            => $charge,
             amountoutstanding => $charge,
@@ -3548,6 +3556,9 @@ return $exist;
 
 Code is either an itemtype or collection doe depending on the pref BranchTransferLimitsType
 
+Deprecated in favor of Koha::Item::Transfer::Limits->find/search and
+Koha::Item->can_be_transferred.
+
 =cut
 
 sub IsBranchTransferAllowed {
@@ -3576,6 +3587,8 @@ sub IsBranchTransferAllowed {
 
 $code is either itemtype or collection code depending on what the pref BranchTransferLimitsType is set to.
 
+Deprecated in favor of Koha::Item::Transfer::Limit->new.
+
 =cut
 
 sub CreateBranchTransferLimit {
@@ -3597,6 +3610,10 @@ Deletes all the library transfer limits for one library.  Returns the
 number of limits deleted, 0e0 if no limits were deleted, or undef if
 no arguments are supplied.
 
+Deprecated in favor of Koha::Item::Transfer::Limits->search({
+    fromBranch => $fromBranch
+    })->delete.
+
 =cut
 
 sub DeleteBranchTransferLimits {
@@ -3622,9 +3639,20 @@ sub ReturnLostItem{
 
 
 sub LostItem{
-    my ($itemnumber, $mark_returned) = @_;
+    my ($itemnumber, $mark_lost_from, $force_mark_returned) = @_;
+
+    unless ( $mark_lost_from ) {
+        # Temporary check to avoid regressions
+        die q|LostItem called without $mark_lost_from, check the API.|;
+    }
 
-    $mark_returned //= C4::Context->preference('MarkLostItemsAsReturned');
+    my $mark_returned;
+    if ( $force_mark_returned ) {
+        $mark_returned = 1;
+    } else {
+        my $pref = C4::Context->preference('MarkLostItemsAsReturned') // q{};
+        $mark_returned = ( $pref =~ m|$mark_lost_from| );
+    }
 
     my $dbh = C4::Context->dbh();
     my $sth=$dbh->prepare("SELECT issues.*,items.*,biblio.title 
@@ -3720,7 +3748,8 @@ sub ProcessOfflineReturn {
             ModItem(
                 { renewals => 0, onloan => undef },
                 $issue->{'biblionumber'},
-                $itemnumber
+                $itemnumber,
+                { log_action => 0 }
             );
             return "Success.";
         } else {
@@ -3959,6 +3988,7 @@ sub GetTopIssues {
 
     my $dbh = C4::Context->dbh;
     my $query = q{
+        SELECT * FROM (
         SELECT b.biblionumber, b.title, b.author, bi.itemtype, bi.publishercode,
           bi.place, bi.publicationyear, b.copyrightdate, bi.pages, bi.size,
           i.ccode, SUM(i.issues) AS count
@@ -3996,11 +4026,13 @@ sub GetTopIssues {
     }
 
     $query .= q{
-        GROUP BY b.biblionumber
-        HAVING count > 0
+        GROUP BY b.biblionumber, b.title, b.author, bi.itemtype, bi.publishercode,
+          bi.place, bi.publicationyear, b.copyrightdate, bi.pages, bi.size,
+          i.ccode
         ORDER BY count DESC
     };
 
+    $query .= q{ ) xxx WHERE count > 0 };
     $count = int($count);
     if ($count > 0) {
         $query .= "LIMIT $count";