Bug 29234: Transit on checking
[koha-ffzg.git] / C4 / Circulation.pm
index bc1da9d..237181f 100644 (file)
@@ -24,7 +24,6 @@ use POSIX qw( floor );
 use YAML::XS;
 use Encode;
 
-use Koha::DateUtils qw( dt_from_string output_pref );
 use C4::Context;
 use C4::Stats qw( UpdateStats );
 use C4::Reserves qw( CheckReserves CanItemBeReserved MoveReserve ModReserve ModReserveMinusPriority RevertWaitingStatus IsItemOnHoldAndFound IsAvailableForItemLevelRequest ItemsAnyAvailableAndNotRestricted );
@@ -43,13 +42,13 @@ use Koha::Account;
 use Koha::AuthorisedValues;
 use Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue;
 use Koha::Biblioitems;
-use Koha::DateUtils qw( dt_from_string output_pref );
+use Koha::DateUtils qw( dt_from_string );
 use Koha::Calendar;
 use Koha::Checkouts;
 use Koha::Illrequests;
 use Koha::Items;
 use Koha::Patrons;
-use Koha::Patron::Debarments qw( DelUniqueDebarment GetDebarments AddUniqueDebarment );
+use Koha::Patron::Debarments qw( DelUniqueDebarment AddUniqueDebarment );
 use Koha::Database;
 use Koha::Libraries;
 use Koha::Account::Lines;
@@ -96,7 +95,6 @@ BEGIN {
       GetBranchBorrowerCircRule
       GetBranchItemRule
       GetBiblioIssues
-      GetOpenIssue
       GetUpcomingDueIssues
       CheckIfIssuedToPatron
       IsItemIssued
@@ -108,7 +106,6 @@ BEGIN {
 
       transferbook
       TooMany
-      GetTransfers
       GetTransfersFromTo
       updateWrongTransfer
       CalcDateDue
@@ -124,6 +121,7 @@ BEGIN {
       DeleteOfflineOperation
       ProcessOfflineOperation
       ProcessOfflinePayment
+      ProcessOfflineIssue
     );
     push @EXPORT_OK, '_GetCircControlBranch';    # This is wrong!
 }
@@ -480,8 +478,9 @@ sub TooMany {
             } elsif (C4::Context->preference('CircControl') eq 'PatronLibrary') {
                 $checkouts = $patron->checkouts; # if branch is the patron's home branch, then count all loans by patron
             } else {
+                my $branch_type = C4::Context->preference('HomeOrHoldingBranch') || 'homebranch';
                 $checkouts = $patron->checkouts->search(
-                    { 'item.homebranch' => $maxissueqty_rule->branchcode } );
+                    { "item.$branch_type" => $maxissueqty_rule->branchcode } );
             }
         } else {
             $checkouts = $patron->checkouts; # if rule is not branch specific then count all loans by patron
@@ -576,9 +575,10 @@ sub TooMany {
         } elsif (C4::Context->preference('CircControl') eq 'PatronLibrary') {
             $checkouts = $patron->checkouts; # if branch is the patron's home branch, then count all loans by patron
         } else {
+            my $branch_type = C4::Context->preference('HomeOrHoldingBranch') || 'homebranch';
             $checkouts = $patron->checkouts->search(
-                { 'item.homebranch' => $branch},
-                { prefetch          => 'item' } );
+                { "item.$branch_type" => $branch},
+                { prefetch            => 'item' } );
         }
 
         my $checkout_count = $checkouts->count;
@@ -794,20 +794,11 @@ sub CanBookBeIssued {
     my $patron_unblessed = $patron->unblessed;
 
     my $circ_library = Koha::Libraries->find( _GetCircControlBranch($item_unblessed, $patron_unblessed) );
-    #
-    # DUE DATE is OK ? -- should already have checked.
-    #
-    if ($duedate && ref $duedate ne 'DateTime') {
-        $duedate = dt_from_string($duedate);
-    }
-    my $now = dt_from_string();
-    unless ( $duedate ) {
-        my $issuedate = $now->clone();
 
-        $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 $now = dt_from_string();
+    $duedate ||= CalcDateDue( $now, $effective_itemtype, $circ_library->branchcode, $patron_unblessed );
+    if (DateTime->compare($duedate,$now) == -1 ) { # duedate cannot be before now
+         $needsconfirmation{INVALID_DATE} = $duedate;
     }
 
     my $fees = Koha::Charges::Fees->new(
@@ -819,29 +810,23 @@ sub CanBookBeIssued {
         }
     );
 
-    if ($duedate) {
-        my $today = $now->clone();
-        $today->truncate( to => 'minute');
-        if (DateTime->compare($duedate,$today) == -1 ) { # duedate cannot be before now
-            $needsconfirmation{INVALID_DATE} = output_pref($duedate);
-        }
-    } else {
-            $issuingimpossible{INVALID_DATE} = output_pref($duedate);
-    }
-
     #
     # BORROWER STATUS
     #
     if ( $patron->category->category_type eq 'X' && (  $item_object->barcode  )) {
        # stats only borrower -- add entry to statistics table, and return issuingimpossible{STATS} = 1  .
-        C4::Stats::UpdateStats({
-                     branch => C4::Context->userenv->{'branch'},
-                     type => 'localuse',
-                     itemnumber => $item_object->itemnumber,
-                     itemtype => $effective_itemtype,
-                     borrowernumber => $patron->borrowernumber,
-                     ccode => $item_object->ccode}
-                    );
+        C4::Stats::UpdateStats(
+            {
+                branch         => C4::Context->userenv->{'branch'},
+                type           => 'localuse',
+                itemnumber     => $item_object->itemnumber,
+                itemtype       => $effective_itemtype,
+                borrowernumber => $patron->borrowernumber,
+                ccode          => $item_object->ccode,
+                categorycode   => $patron->categorycode,
+                location       => $item_object->location,
+            }
+        );
         ModDateLastSeen( $item_object->itemnumber ); # FIXME Move to Koha::Item
         return( { STATS => 1 }, {});
     }
@@ -1207,7 +1192,7 @@ sub CanBookBeIssued {
                     $needsconfirmation{'ressurname'} = $patron->surname;
                     $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
                     $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
-                    $needsconfirmation{'resbranchcode'} = $patron->branchcode;
+                    $needsconfirmation{'resbranchcode'} = $res->{branchcode};
                     $needsconfirmation{'resreservedate'} = $res->{reservedate};
                     $needsconfirmation{'reserve_id'} = $res->{reserve_id};
                 }
@@ -1218,7 +1203,7 @@ sub CanBookBeIssued {
                     $needsconfirmation{'ressurname'} = $patron->surname;
                     $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
                     $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
-                    $needsconfirmation{'resbranchcode'} = $patron->branchcode;
+                    $needsconfirmation{'resbranchcode'} = $res->{branchcode};
                     $needsconfirmation{'resreservedate'} = $res->{reservedate};
                     $needsconfirmation{'reserve_id'} = $res->{reserve_id};
                 }
@@ -1229,7 +1214,7 @@ sub CanBookBeIssued {
                     $needsconfirmation{'ressurname'} = $patron->surname;
                     $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
                     $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
-                    $needsconfirmation{'resbranchcode'} = $patron->branchcode;
+                    $needsconfirmation{'resbranchcode'} = $res->{branchcode};
                     $needsconfirmation{'resreservedate'} = $res->{reservedate};
                     $needsconfirmation{'reserve_id'} = $res->{reserve_id};
                 }
@@ -1254,19 +1239,16 @@ sub CanBookBeIssued {
         my $check = checkHighHolds( $item_object, $patron );
 
         if ( $check->{exceeded} ) {
+            my $highholds = {
+                num_holds  => $check->{outstanding},
+                duration   => $check->{duration},
+                returndate => $check->{due_date},
+            };
             if ($override_high_holds) {
-                $alerts{HIGHHOLDS} = {
-                    num_holds  => $check->{outstanding},
-                    duration   => $check->{duration},
-                    returndate => output_pref( { dt => dt_from_string($check->{due_date}), dateformat => 'iso', timeformat => '24hr' }),
-                };
+                $alerts{HIGHHOLDS} = $highholds;
             }
             else {
-                $needsconfirmation{HIGHHOLDS} = {
-                    num_holds  => $check->{outstanding},
-                    duration   => $check->{duration},
-                    returndate => output_pref( { dt => dt_from_string($check->{due_date}), dateformat => 'iso', timeformat => '24hr' }),
-                };
+                $needsconfirmation{HIGHHOLDS} = $highholds;
             }
         }
     }
@@ -1373,7 +1355,12 @@ sub checkHighHolds {
         due_date    => undef,
     };
 
-    my $holds = Koha::Holds->search( { biblionumber => $item->biblionumber } );
+
+    # Count holds on this record, ignoring the borrowers own holds as they would be filled by the checkout
+    my $holds = Koha::Holds->search({
+        biblionumber => $item->biblionumber,
+        borrowernumber => { '!=' => $patron->borrowernumber }
+    });
 
     if ( $holds->count() ) {
         $return_data->{outstanding} = $holds->count();
@@ -1388,8 +1375,8 @@ sub checkHighHolds {
 
             # static means just more than a given number of holds on the record
 
-            # If the number of holds is less than the threshold, we can stop here
-            if ( $holds->count() < $decreaseLoanHighHoldsValue ) {
+            # If the number of holds is not above the threshold, we can stop here
+            if ( $holds->count() <= $decreaseLoanHighHoldsValue ) {
                 return $return_data;
             }
         }
@@ -1406,7 +1393,9 @@ sub checkHighHolds {
             }
 
             # Remove any items that are not holdable for this patron
-            @items = grep { CanItemBeReserved( $patron , $_, undef, { ignore_found_holds => 1 } )->{status} eq 'OK' } @items;
+            # We need to ignore hold counts as the borrower's own hold that will be filled by the checkout
+            # could prevent them from placing further holds
+            @items = grep { CanItemBeReserved( $patron, $_, undef, { ignore_hold_counts => 1 } )->{status} eq 'OK' } @items;
 
             my $items_count = scalar @items;
 
@@ -1433,7 +1422,7 @@ sub checkHighHolds {
 
         my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branchcode, $patron->unblessed );
 
-        my $rule = Koha::CirculationRules->get_effective_rule(
+        my $rule = Koha::CirculationRules->get_effective_rule_value(
             {
                 categorycode => $patron->categorycode,
                 itemtype     => $item->effective_itemtype,
@@ -1443,9 +1432,9 @@ sub checkHighHolds {
         );
 
         my $duration;
-        if ( defined($rule) && $rule->rule_value ne '' ){
+        if ( defined($rule) && $rule ne '' ){
             # overrides decreaseLoanHighHoldsDuration syspref
-            $duration = $rule->rule_value;
+            $duration = $rule;
         } else {
             $duration = C4::Context->preference('decreaseLoanHighHoldsDuration');
         }
@@ -1481,8 +1470,8 @@ Calculated if empty.
 
 =item C<$cancelreserve> is 1 to override and cancel any pending reserves for the item (optional).
 
-=item C<$issuedate> is the date to issue the item in iso (YYYY-MM-DD) format (optional).
-Defaults to today.  Unlike C<$datedue>, NOT a DateTime object, unfortunately.
+=item C<$issuedate> is a DateTime object for the date to issue the item (optional).
+Defaults to today.
 
 AddIssue does the following things :
 
@@ -1558,11 +1547,31 @@ sub AddIssue {
             );
         }
         else {
+
             unless ($datedue) {
                 my $itype = $item_object->effective_itemtype;
                 $datedue = CalcDateDue( $issuedate, $itype, $branchcode, $borrower );
 
             }
+
+            # Check if we need to use an exact due date set by the ILL module
+            if ( C4::Context->preference('ILLModule') ) {
+                # Check if there is an ILL connected with the biblio of the item we are issuing
+                my $ill_request = Koha::Illrequests->search({
+                    biblio_id => $item_object->biblionumber,
+                    borrowernumber => $borrower->{'borrowernumber'},
+                    completed => undef,
+                    due_date => { '!=', undef },
+                })->next;
+
+                if ( $ill_request and length( $ill_request->due_date ) > 0 ) {
+                    my $ill_dt = dt_from_string( $ill_request->due_date );
+                    $ill_dt->set_hour(23);
+                    $ill_dt->set_minute(59);
+                    $datedue = $ill_dt;
+                }
+            }
+
             $datedue->truncate( to => 'minute' );
 
             my $patron = Koha::Patrons->find( $borrower );
@@ -1621,7 +1630,7 @@ sub AddIssue {
 
             # If automatic renewal wasn't selected while issuing, set the value according to the issuing rule.
             unless ($auto_renew) {
-                my $rule = Koha::CirculationRules->get_effective_rule(
+                my $rule = Koha::CirculationRules->get_effective_rule_value(
                     {
                         categorycode => $borrower->{categorycode},
                         itemtype     => $item_object->effective_itemtype,
@@ -1630,13 +1639,13 @@ sub AddIssue {
                     }
                 );
 
-                $auto_renew = $rule->rule_value if $rule;
+                $auto_renew = $rule if defined $rule && $rule ne '';
             }
 
             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'),
+                issuedate       => $issuedate,
+                date_due        => $datedue,
                 branchcode      => C4::Context->userenv->{'branch'},
                 onsite_checkout => $onsite_checkout,
                 auto_renew      => $auto_renew ? 1 : 0,
@@ -1745,6 +1754,7 @@ sub AddIssue {
                     location       => $item_object->location,
                     borrowernumber => $borrower->{'borrowernumber'},
                     ccode          => $item_object->ccode,
+                    categorycode   => $borrower->{'categorycode'}
                 }
             );
 
@@ -1933,11 +1943,6 @@ holdallowed => Hold policy for this branch and itemtype. Possible values:
   from_any_library:      Holds allowed from any patron.
   from_local_hold_group: Holds allowed from libraries in hold group
 
-returnbranch => branch to which to return item.  Possible values:
-  noreturn: do not return, let item remain where checked in (floating collections)
-  homebranch: return to item's home branch
-  holdingbranch: return to issuer branch
-
 This searches branchitemrules in the following order:
 
   * Same branchcode and itemtype
@@ -1956,13 +1961,12 @@ sub GetBranchItemRule {
     my $rules = Koha::CirculationRules->get_effective_rules({
         branchcode => $branchcode,
         itemtype => $itemtype,
-        rules => ['holdallowed', 'hold_fulfillment_policy', 'returnbranch']
+        rules => ['holdallowed', 'hold_fulfillment_policy']
     });
 
     # built-in default circulation rule
     $rules->{holdallowed} //= 'from_any_library';
     $rules->{hold_fulfillment_policy} //= 'any';
-    $rules->{returnbranch} //= 'homebranch';
 
     return $rules;
 }
@@ -2094,8 +2098,18 @@ sub AddReturn {
         }
     }
 
+    if ( $item->withdrawn ) { # book has been cancelled
+        $messages->{'withdrawn'} = 1;
+
+        # In the case where we block return of withdrawn, we should completely block the return
+        # without updating item statuses, so we exit early
+        return ( 0, $messages, $issue, ( $patron ? $patron->unblessed : {} ))
+            if C4::Context->preference("BlockReturnOfWithdrawnItems");
+    }
+
+
         # full item data, but no borrowernumber or checkout info (no issue)
-    my $hbr = GetBranchItemRule($item->homebranch, $itemtype)->{'returnbranch'} || "homebranch";
+    my $hbr = Koha::CirculationRules->get_return_branch_policy($item);
         # get the proper branch to which to return the item
     my $returnbranch = $hbr ne 'noreturn' ? $item->$hbr : $branch;
         # if $hbr was "noreturn" or any other non-item table value, then it should 'float' (i.e. stay at this branch)
@@ -2143,7 +2157,7 @@ sub AddReturn {
             foreach my $key ( keys %$rules ) {
                 if ( $item->notforloan eq $key ) {
                     $messages->{'NotForLoanStatusUpdated'} = { from => $item->notforloan, to => $rules->{$key} };
-                    $item->notforloan($rules->{$key})->store({ log_action => 0, skip_record_index => 1, skip_holds_queue => 1 });
+                    $item->notforloan($rules->{$key})->store({ log_action => 0, skip_record_index => 1, skip_holds_queue => 1 }) unless $rules->{$key} eq 'ONLYMESSAGE';
                     last;
                 }
             }
@@ -2163,11 +2177,6 @@ sub AddReturn {
         return ( $doreturn, $messages, $issue, $patron_unblessed);
     }
 
-    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") ) {
         $doreturn = 0;
     }
@@ -2227,6 +2236,8 @@ sub AddReturn {
             for my $message (@object_messages) {
                 $messages->{'LostItemFeeRefunded'} = 1
                   if $message->message eq 'lost_refunded';
+                $messages->{'ProcessingFeeRefunded'} = 1
+                  if $message->message eq 'processing_refunded';
                 $messages->{'LostItemFeeRestored'} = 1
                   if $message->message eq 'lost_restored';
 
@@ -2282,8 +2293,15 @@ sub AddReturn {
                 $validTransfer = 1 if $transfer->reason eq 'Reserve';
             }
             else {
-                $messages->{'WasTransfered'}   = $transfer->tobranch;
                 $messages->{'TransferTrigger'} = $transfer->reason;
+                if ( $transfer->frombranch eq $branch ) {
+                    $transfer->transit;
+                    $messages->{'WasTransfered'}   = $transfer->tobranch;
+                }
+                else {
+                    $messages->{'WrongTransfer'}     = $transfer->tobranch;
+                    $messages->{'WrongTransferItem'} = $item->itemnumber;
+                }
             }
         }
     }
@@ -2296,10 +2314,23 @@ sub AddReturn {
         if ( $issue and $issue->is_overdue($return_date) ) {
         # fix fine days
             my ($debardate,$reminder) = _debar_user_on_return( $patron_unblessed, $item->unblessed, dt_from_string($issue->date_due), $return_date );
-            if ($reminder){
-                $messages->{'PrevDebarred'} = $debardate;
-            } else {
-                $messages->{'Debarred'} = $debardate if $debardate;
+            if ($debardate and $debardate ne "9999-12-31") {
+                if ($reminder){
+                    $messages->{'PrevDebarred'} = $debardate;
+                } else {
+                    $messages->{'Debarred'} = $debardate;
+                }
+            } elsif ($patron->debarred) {
+                if ( $patron->debarred eq "9999-12-31") {
+                    $messages->{'ForeverDebarred'} = $patron->debarred;
+                } else {
+                    my $borrower_debar_dt = dt_from_string( $patron->debarred );
+                    $borrower_debar_dt->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;
+                    }
+                }
             }
         # there's no overdue on the item but borrower had been previously debarred
         } elsif ( $issue->date_due and $patron->debarred ) {
@@ -2346,6 +2377,7 @@ sub AddReturn {
     }
 
     # Record the fact that this book was returned.
+    my $categorycode = $patron_unblessed ? $patron_unblessed->{categorycode} : undef;
     C4::Stats::UpdateStats({
         branch         => $branch,
         type           => $stat_type,
@@ -2354,6 +2386,7 @@ sub AddReturn {
         location       => $item->location,
         borrowernumber => $borrowernumber,
         ccode          => $item->ccode,
+        categorycode   => $categorycode,
     });
 
     # Send a check-in slip. # NOTE: borrower may be undef. Do not try to send messages then.
@@ -2433,6 +2466,12 @@ sub AddReturn {
         }
     }
 
+    # Check for bundle status
+    if ( $item->in_bundle ) {
+        my $host = $item->bundle_host;
+        $messages->{InBundle} = $host;
+    }
+
     my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX });
     $indexer->index_records( $item->biblionumber, "specialUpdate", "biblioserver" );
 
@@ -2527,10 +2566,11 @@ sub MarkIssueReturned {
         }
 
         # Remove any OVERDUES related debarment if the borrower has no overdues
+        my $overdue_restrictions = $patron->restrictions->search({ type => 'OVERDUES' });
         if ( C4::Context->preference('AutoRemoveOverduesRestrictions')
           && $patron->debarred
           && !$patron->has_overdues
-          && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
+          && $overdue_restrictions->count
         ) {
             DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
         }
@@ -2619,9 +2659,10 @@ sub _calculate_new_debar_dt {
 
         my ( $has_been_extended );
         if ( C4::Context->preference('CumulativeRestrictionPeriods') and $borrower->{debarred} ) {
-            my $debarment = @{ GetDebarments( { borrowernumber => $borrower->{borrowernumber}, type => 'SUSPENSION' } ) }[0];
+            my $patron = Koha::Patrons->find($borrower->{borrowernumber});
+            my $debarment = $patron->restrictions->search({type => 'SUSPENSION' },{rows => 1})->single;
             if ( $debarment ) {
-                $return_date = dt_from_string( $debarment->{expiration}, 'sql' );
+                $return_date = dt_from_string( $debarment->expiration, 'sql' );
                 $has_been_extended = 1;
             }
         }
@@ -2788,28 +2829,6 @@ sub _GetCircControlBranch {
     return $branch;
 }
 
-=head2 GetOpenIssue
-
-  $issue = GetOpenIssue( $itemnumber );
-
-Returns the row from the issues table if the item is currently issued, undef if the item is not currently issued
-
-C<$itemnumber> is the item's itemnumber
-
-Returns a hashref
-
-=cut
-
-sub GetOpenIssue {
-  my ( $itemnumber ) = @_;
-  return unless $itemnumber;
-  my $dbh = C4::Context->dbh;  
-  my $sth = $dbh->prepare( "SELECT * FROM issues WHERE itemnumber = ? AND returndate IS NULL" );
-  $sth->execute( $itemnumber );
-  return $sth->fetchrow_hashref();
-
-}
-
 =head2 GetUpcomingDueIssues
 
   my $upcoming_dues = GetUpcomingDueIssues( { days_in_advance => 4 } );
@@ -2875,7 +2894,7 @@ sub CanBookBeRenewed {
     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 });
+    return ( 0, 'item_denied_renewal') if $item->is_denied_renewal;
 
     my $patron = $issue->patron or return;
 
@@ -2896,11 +2915,11 @@ sub CanBookBeRenewed {
         );
 
         return ( 0, "too_many" )
-          if not $issuing_rule->{renewalsallowed} or $issuing_rule->{renewalsallowed} <= $issue->renewals;
+          if not $issuing_rule->{renewalsallowed} or $issuing_rule->{renewalsallowed} <= $issue->renewals_count;
 
         return ( 0, "too_unseen" )
           if C4::Context->preference('UnseenRenewals') &&
-            $issuing_rule->{unseen_renewals_allowed} &&
+            looks_like_number($issuing_rule->{unseen_renewals_allowed}) &&
             $issuing_rule->{unseen_renewals_allowed} <= $issue->unseen_renewals;
 
         my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing');
@@ -2943,89 +2962,65 @@ sub CanBookBeRenewed {
         }
     }
 
-    # Note: possible_reserves will contain all title level holds on this bib and item level
-    # holds on the checked out item
-    my ( $resfound, $resrec, $possible_reserves ) = C4::Reserves::CheckReserves($itemnumber);
-
-    # If next hold is non priority, then check if any hold with priority (non_priority = 0) exists for the same biblionumber.
-    if ( $resfound && $resrec->{non_priority} ) {
-        $resfound = Koha::Holds->search(
-            { biblionumber => $resrec->{biblionumber}, non_priority => 0 } )
-          ->count > 0;
-    }
-
+    # There is an item level hold on this item, no other item can fill the hold
+    return ( 0, "on_reserve" )
+      if ( $item->current_holds->search( { non_priority => 0 } )->count );
 
-
-    # This item can fill one or more unfilled reserve, can those unfilled reserves
-    # all be filled by other available items?
-    if ( $resfound
-        && C4::Context->preference('AllowRenewalIfOtherItemsAvailable') )
-    {
-        my $item_holds = Koha::Holds->search( { itemnumber => $itemnumber, found => undef } )->count();
-        if ($item_holds) {
-            # There is an item level hold on this item, no other item can fill the hold
-            $resfound = 1;
-        }
-        else {
+    my $fillable_holds = Koha::Holds->search(
+        {
+            biblionumber => $item->biblionumber,
+            non_priority => 0,
+            found        => undef,
+            reservedate  => { '<=' => \'NOW()' },
+            suspend      => 0,
+        },
+        { prefetch => 'patron' }
+    );
+    if ( $fillable_holds->count ) {
+        if ( C4::Context->preference('AllowRenewalIfOtherItemsAvailable') ) {
+            my @possible_holds = $fillable_holds->as_list;
 
             # Get all other items that could possibly fill reserves
             # FIXME We could join reserves (or more tables) here to eliminate some checks later
-            my $items = Koha::Items->search({
-                biblionumber => $resrec->{biblionumber},
+            my @other_items = Koha::Items->search({
+                biblionumber => $item->biblionumber,
                 onloan       => undef,
                 notforloan   => 0,
-                -not         => { itemnumber => $itemnumber }
-            });
-            my $item_count = $items->count();
-
-            # Get all other reserves that could have been filled by this item
-            my @borrowernumbers = map { $_->{borrowernumber} } @$possible_reserves;
-            # Note: fetching the patrons in this manner means that a patron with 2 holds will
-            # not block renewal if one reserve can be satisfied i.e. each patron is checked once
-            my $patrons = Koha::Patrons->search({
-                borrowernumber => { -in => \@borrowernumbers }
-            });
-            my $patron_count = $patrons->count();
+                -not         => { itemnumber => $itemnumber } })->as_list;
 
-            return ( 0, "on_reserve" ) if ($patron_count > $item_count);
-            # We cannot possibly fill all reserves if we don't have enough items
+            return ( 0, "on_reserve" ) if @possible_holds && (scalar @other_items < scalar @possible_holds);
 
-            # If we can fill each hold that has been found with the available items on the record
-            # then the patron can renew. If we cannot, they cannot renew.
-            # FIXME This code does not check whether the item we are renewing can fill
-            # any of the existing reserves.
-            my $reservable = 0;
             my %matched_items;
-            my $seen = 0;
-            PATRON: while ( my $patron = $patrons->next ) {
-                # If there is a reserve that cannot be filled we are done
-                return ( 0, "on_reserve" ) if ( $seen > $reservable );
-                my $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron });
-                while ( my $other_item = $items->next ) {
-                    next if defined $matched_items{$other_item->itemnumber};
-                    next if IsItemOnHoldAndFound( $other_item->itemnumber );
-                    next unless IsAvailableForItemLevelRequest($other_item, $patron, undef, $items_any_available);
-                    next unless CanItemBeReserved($patron,$other_item,undef,{ignore_hold_counts=>1})->{status} eq 'OK';
-                    # NOTE: At checkin we call 'CheckReserves' which checks hold 'policy'
-                    # CanItemBeReserved checks 'rules' and 'policies' which means
-                    # items will fill holds at checkin that are rejected here
-                    $reservable++;
-                    if ($reservable >= $patron_count) {
-                        $resfound = 0;
-                        last PATRON;
-                    }
-                    $matched_items{$other_item->itemnumber} = 1;
-                    last;
+            foreach my $possible_hold (@possible_holds) {
+                my $fillable = 0;
+                my $patron_with_reserve = Koha::Patrons->find($possible_hold->borrowernumber);
+                my $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron_with_reserve });
+
+                # FIXME: We are not checking whether the item we are renewing can fill the hold
+
+                foreach my $other_item (@other_items) {
+                  next if defined $matched_items{$other_item->itemnumber};
+                  next if IsItemOnHoldAndFound( $other_item->itemnumber );
+                  next unless IsAvailableForItemLevelRequest($other_item, $patron_with_reserve, undef, $items_any_available);
+                  next unless CanItemBeReserved($patron_with_reserve,$other_item,undef,{ignore_hold_counts=>1})->{status} eq 'OK';
+                  # NOTE: At checkin we call 'CheckReserves' which checks hold 'policy'
+                  # CanItemBeReserved checks 'rules' and 'policies' which means
+                  # items will fill holds at checkin that are rejected here
+                  $fillable = 1;
+                  $matched_items{$other_item->itemnumber} = 1;
+                  last;
                 }
-                $items->reset;
-                $seen++;
+                return ( 0, "on_reserve" ) unless $fillable;
             }
+
+        } else {
+            my ($status, $matched_reserve, $possible_reserves) = CheckReserves($itemnumber);
+            return ( 0, "on_reserve" ) if $status;
         }
     }
 
-    return ( 0, "on_reserve" ) if $resfound;    # '' when no hold was found
     return ( 0, $auto_renew, { soonest_renew_date => $soonest } ) if $auto_renew =~ 'too_soon';#$auto_renew ne "no" && $auto_renew ne "ok";
-    $soonest = GetSoonestRenewDate($borrowernumber, $itemnumber);
+    $soonest = GetSoonestRenewDate($issue);
     if ( $soonest > dt_from_string() ){
         return (0, "too_soon", { soonest_renew_date => $soonest } ) unless $override_limit;
     }
@@ -3037,7 +3032,7 @@ sub CanBookBeRenewed {
 
 =head2 AddRenewal
 
-  &AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate], [$seen]);
+  &AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate], [$seen], [$automatic]);
 
 Renews a loan.
 
@@ -3066,6 +3061,8 @@ C<$seen> is a boolean flag indicating if the item was seen or not during the ren
 informs the incrementing of the unseen_renewals column. If this flag is not supplied, we
 fallback to a true value
 
+C<$automatic> is a boolean flag indicating the renewal was triggered automatically and not by a person ( librarian or patron )
+
 =cut
 
 sub AddRenewal {
@@ -3076,6 +3073,7 @@ sub AddRenewal {
     my $lastreneweddate = shift || dt_from_string();
     my $skipfinecalc    = shift;
     my $seen            = shift;
+    my $automatic       = shift;
 
     # Fallback on a 'seen' renewal
     $seen = defined $seen && $seen == 0 ? 0 : 1;
@@ -3085,6 +3083,8 @@ sub AddRenewal {
     my $issue  = $item_object->checkout;
     my $item_unblessed = $item_object->unblessed;
 
+    my $renewal_type = $automatic ? "Automatic" : "Manual";
+
     my $dbh = C4::Context->dbh;
 
     return unless $issue;
@@ -3092,8 +3092,7 @@ sub AddRenewal {
     $borrowernumber ||= $issue->borrowernumber;
 
     if ( defined $datedue && ref $datedue ne 'DateTime' ) {
-        carp 'Invalid date passed to AddRenewal.';
-        return;
+        $datedue = dt_from_string($datedue, 'sql');
     }
 
     my $patron = Koha::Patrons->find( $borrowernumber ) or return; # FIXME Should do more than just return
@@ -3143,7 +3142,7 @@ sub AddRenewal {
                     rule_name    => 'unseen_renewals_allowed'
                 }
             );
-            if (!$seen && $rule && $rule->rule_value) {
+            if (!$seen && $rule && looks_like_number($rule->rule_value)) {
                 $unseen_renewals++;
             } else {
                 # If the renewal is seen, unseen should revert to 0
@@ -3153,8 +3152,8 @@ sub AddRenewal {
 
         # 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 = ?, unseen_renewals = ?, lastreneweddate = ? WHERE issue_id = ?");
+        my $renews = ( $issue->renewals_count || 0 ) + 1;
+        my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals_count = ?, unseen_renewals = ?, lastreneweddate = ? WHERE issue_id = ?");
 
         eval{
             $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $unseen_renewals, $lastreneweddate, $issue->issue_id );
@@ -3169,7 +3168,8 @@ sub AddRenewal {
         $renews = ( $item_object->renewals || 0 ) + 1;
         $item_object->renewals($renews);
         $item_object->onloan($datedue);
-        $item_object->store({ log_action => 0 });
+        # Don't index as we are in a transaction
+        $item_object->store({ log_action => 0, skip_record_index => 1 });
 
         # Charge a new rental fee, if applicable
         my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
@@ -3209,14 +3209,26 @@ sub AddRenewal {
         }
 
         # Remove any OVERDUES related debarment if the borrower has no overdues
+        my $overdue_restrictions = $patron->restrictions->search({ type => 'OVERDUES' });
         if ( $patron
           && $patron->is_debarred
           && ! $patron->has_overdues
-          && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
+          && $overdue_restrictions->count
         ) {
             DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
         }
 
+        # Add renewal record
+        my $renewal = Koha::Checkouts::Renewal->new(
+            {
+                checkout_id  => $issue->issue_id,
+                interface    => C4::Context->interface,
+                renewal_type => $renewal_type,
+                renewer_id   => C4::Context->userenv ? C4::Context->userenv->{'number'} : undef,
+                seen         => $seen,
+            }
+        )->store();
+
         # Add the renewal to stats
         C4::Stats::UpdateStats(
             {
@@ -3228,6 +3240,7 @@ sub AddRenewal {
                 location       => $item_object->location,
                 borrowernumber => $borrowernumber,
                 ccode          => $item_object->ccode,
+                categorycode   => $patron->categorycode,
             }
         );
 
@@ -3241,6 +3254,9 @@ sub AddRenewal {
             }
         });
     });
+    # We index now, after the transaction is committed
+    my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX });
+    $indexer->index_records( $item_object->biblionumber, "specialUpdate", "biblioserver" );
 
     return $datedue;
 }
@@ -3272,7 +3288,7 @@ sub GetRenewCount {
     );
     $sth->execute( $bornum, $itemno );
     my $data = $sth->fetchrow_hashref;
-    $renewcount = $data->{'renewals'} if $data->{'renewals'};
+    $renewcount = $data->{'renewals_count'} if $data->{'renewals_count'};
     $unseencount = $data->{'unseen_renewals'} if $data->{'unseen_renewals'};
     # $item and $borrower should be calculated
     my $branchcode = _GetCircControlBranch($item->unblessed, $patron->unblessed);
@@ -3305,34 +3321,25 @@ sub GetRenewCount {
 
 =head2 GetSoonestRenewDate
 
-  $NoRenewalBeforeThisDate = &GetSoonestRenewDate($borrowernumber, $itemnumber);
+  $NoRenewalBeforeThisDate = &GetSoonestRenewDate($checkout);
 
 Find out the soonest possible renew date of a borrowed item.
 
-C<$borrowernumber> is the borrower number of the patron who currently
-has the item on loan.
-
-C<$itemnumber> is the number of the item to renew.
+C<$checkout> is the checkout object to renew.
 
 C<$GetSoonestRenewDate> returns the DateTime of the soonest possible
 renew date, based on the value "No renewal before" of the applicable
 issuing rule. Returns the current date if the item can already be
-renewed, and returns undefined if the borrower, loan, or item
+renewed, and returns undefined if the patron, item, or checkout
 cannot be found.
 
 =cut
 
 sub GetSoonestRenewDate {
-    my ( $borrowernumber, $itemnumber ) = @_;
-
-    my $dbh = C4::Context->dbh;
+    my ( $checkout ) = @_;
 
-    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 $item   = $checkout->item or return;
+    my $patron = $checkout->patron or return;
 
     my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed );
     my $issuing_rule = Koha::CirculationRules->get_effective_rules(
@@ -3352,7 +3359,7 @@ sub GetSoonestRenewDate {
         and $issuing_rule->{norenewalbefore} ne "" )
     {
         my $soonestrenewal =
-          dt_from_string( $itemissue->date_due )->subtract(
+          dt_from_string( $checkout->date_due )->subtract(
             $issuing_rule->{lengthunit} => $issuing_rule->{norenewalbefore} );
 
         if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date'
@@ -3361,9 +3368,9 @@ sub GetSoonestRenewDate {
             $soonestrenewal->truncate( to => 'day' );
         }
         return $soonestrenewal if $now < $soonestrenewal;
-    } elsif ( $itemissue->auto_renew && $patron->autorenew_checkouts ) {
+    } elsif ( $checkout->auto_renew && $patron->autorenew_checkouts ) {
         # Checkouts with auto-renewing fall back to due date
-        my $soonestrenewal = dt_from_string( $itemissue->date_due );
+        my $soonestrenewal = dt_from_string( $checkout->date_due );
         if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date'
             and $issuing_rule->{lengthunit} eq 'days' )
         {
@@ -3527,35 +3534,6 @@ sub AddIssuingCharge {
     );
 }
 
-=head2 GetTransfers
-
-  GetTransfers($itemnumber);
-
-=cut
-
-sub GetTransfers {
-    my ($itemnumber) = @_;
-
-    my $dbh = C4::Context->dbh;
-
-    my $query = '
-        SELECT datesent,
-               frombranch,
-               tobranch,
-               branchtransfer_id,
-               daterequested,
-               reason
-        FROM branchtransfers
-        WHERE itemnumber = ?
-          AND datearrived IS NULL
-          AND datecancelled IS NULL
-        ';
-    my $sth = $dbh->prepare($query);
-    $sth->execute($itemnumber);
-    my @row = $sth->fetchrow_array();
-    return @row;
-}
-
 =head2 GetTransfersFromTo
 
   @results = GetTransfersFromTo($frombranch,$tobranch);
@@ -3743,7 +3721,7 @@ sub CalcDateDue {
     my $loanlength =
             GetLoanLength( $borrower->{'categorycode'}, $itemtype, $branch );
 
-    my $length_key = ( $isrenewal and defined $loanlength->{renewalperiod} )
+    my $length_key = ( $isrenewal and defined $loanlength->{renewalperiod} and $loanlength->{renewalperiod} ne q{} )
             ? qq{renewalperiod}
             : qq{issuelength};
 
@@ -4057,16 +4035,16 @@ sub ProcessOfflineReturn {
 
     if ( $item ) {
         my $itemnumber = $item->itemnumber;
-        my $issue = GetOpenIssue( $itemnumber );
+        my $issue = $item->checkout;
         if ( $issue ) {
             my $leave_item_lost = C4::Context->preference("BlockReturnOfLostItems") ? 1 : 0;
             ModDateLastSeen( $itemnumber, $leave_item_lost );
             MarkIssueReturned(
-                $issue->{borrowernumber},
+                $issue->borrowernumber,
                 $itemnumber,
                 $operation->{timestamp},
             );
-            $item->renewals(0);
+            $item->renewals_count(0);
             $item->onloan(undef);
             $item->store({ log_action => 0 });
             return "Success.";
@@ -4089,11 +4067,11 @@ sub ProcessOfflineIssue {
             return "Barcode not found.";
         }
         my $itemnumber = $item->itemnumber;
-        my $issue = GetOpenIssue( $itemnumber );
+        my $issue = $item->checkout;
 
-        if ( $issue and ( $issue->{borrowernumber} ne $patron->borrowernumber ) ) { # Item already issued to another patron mark it returned
+        if ( $issue and ( $issue->borrowernumber ne $patron->borrowernumber ) ) { # Item already issued to another patron mark it returned
             MarkIssueReturned(
-                $issue->{borrowernumber},
+                $issue->borrowernumber,
                 $itemnumber,
                 $operation->{timestamp},
             );
@@ -4390,8 +4368,9 @@ sub _CalculateAndUpdateFine {
     # we only need to calculate and change the fines if we want to do that on return
     # Should be on for hourly loans
     my $control = C4::Context->preference('CircControl');
+    my $branch_type = C4::Context->preference('HomeOrHoldingBranch') || 'homebranch';
     my $control_branchcode =
-        ( $control eq 'ItemHomeLibrary' ) ? $item->{homebranch}
+        ( $control eq 'ItemHomeLibrary' ) ? $item->{$branch_type}
       : ( $control eq 'PatronLibrary' )   ? $borrower->{branchcode}
       :                                     $issue->branchcode;
 
@@ -4407,7 +4386,7 @@ sub _CalculateAndUpdateFine {
                 itemnumber     => $issue->itemnumber,
                 borrowernumber => $issue->borrowernumber,
                 amount         => $amount,
-                due            => output_pref($datedue),
+                due            => $datedue,
             });
         }
         elsif ($return_date) {
@@ -4420,7 +4399,7 @@ sub _CalculateAndUpdateFine {
                 itemnumber     => $issue->itemnumber,
                 borrowernumber => $issue->borrowernumber,
                 amount         => 0,
-                due            => output_pref($datedue),
+                due            => $datedue,
             });
         }
     }
@@ -4485,7 +4464,7 @@ sub _CanBookBeAutoRenewed {
         }
     }
 
-    my $soonest = GetSoonestRenewDate($patron->id, $item->id);
+    my $soonest = GetSoonestRenewDate($issue);
     if ( $soonest > dt_from_string() )
     {
         return ( "auto_too_soon", $soonest );
@@ -4494,28 +4473,6 @@ sub _CanBookBeAutoRenewed {
     return "ok";
 }
 
-sub _item_denied_renewal {
-    my ($params) = @_;
-
-    my $item = $params->{item};
-    return unless $item;
-
-    my $denyingrules = Koha::Config::SysPrefs->find('ItemsDeniedRenewal')->get_yaml_pref_hash();
-    return unless $denyingrules;
-    foreach my $field (keys %$denyingrules) {
-        my $val = $item->$field;
-        if( !defined $val) {
-            if ( any { !defined $_ }  @{$denyingrules->{$field}} ){
-                return 1;
-            }
-        } elsif (any { defined($_) && $val eq $_ } @{$denyingrules->{$field}}) {
-           # If the results matches the values in the syspref
-           # We return true if match found
-            return 1;
-        }
-    }
-    return 0;
-}
 
 1;