Bug 27064: (QA follow-up) Do not create transfer prior to accepting it
[srvgit] / C4 / Circulation.pm
index 18c4dac..f0e39b7 100644 (file)
@@ -21,6 +21,9 @@ package C4::Circulation;
 use Modern::Perl;
 use DateTime;
 use POSIX qw( floor );
+use YAML::XS;
+use Encode;
+
 use Koha::DateUtils;
 use C4::Context;
 use C4::Stats;
@@ -56,9 +59,10 @@ use Koha::Account::Lines;
 use Koha::Account::Offsets;
 use Koha::Config::SysPrefs;
 use Koha::Charges::Fees;
-use Koha::Util::SystemPreferences;
+use Koha::Config::SysPref;
 use Koha::Checkouts::ReturnClaims;
 use Koha::SearchEngine::Indexer;
+use Koha::Exceptions::Checkout;
 use Carp;
 use List::MoreUtils qw( uniq any );
 use Scalar::Util qw( looks_like_number );
@@ -98,7 +102,6 @@ BEGIN {
                &GetIssuingCharges
         &GetBranchBorrowerCircRule
         &GetBranchItemRule
-               &GetBiblioIssues
                &GetOpenIssue
         &CheckIfIssuedToPatron
         &IsItemIssued
@@ -117,7 +120,6 @@ BEGIN {
                &GetTransfers
                &GetTransfersFromTo
                &updateWrongTransfer
-               &DeleteTransfer
                 &IsBranchTransferAllowed
                 &CreateBranchTransferLimit
                 &DeleteBranchTransferLimits
@@ -180,7 +182,7 @@ sub barcodedecode {
        } elsif ($filter eq 'cuecat') {
                chomp($barcode);
            my @fields = split( /\./, $barcode );
-           my @results = map( decode($_), @fields[ 1 .. $#fields ] );
+           my @results = map( C4::Circulation::_decode($_), @fields[ 1 .. $#fields ] );
            ($#results == 2) and return $results[2];
        } elsif ($filter eq 'T-prefix') {
                if ($barcode =~ /^[Tt](\d)/) {
@@ -211,9 +213,9 @@ sub barcodedecode {
     return $barcode;    # return barcode, modified or not
 }
 
-=head2 decode
+=head2 _decode
 
-  $str = &decode($chunk);
+  $str = &_decode($chunk);
 
 Decodes a segment of a string emitted by a CueCat barcode scanner and
 returns it.
@@ -223,7 +225,7 @@ or Javascript based decoding on the client side.
 
 =cut
 
-sub decode {
+sub _decode {
     my ($encoded) = @_;
     my $seq =
       'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789+-';
@@ -373,10 +375,10 @@ sub transferbook {
     # That'll save a database query.
     my ( $resfound, $resrec, undef ) =
       CheckReserves( $itemnumber );
-    if ( $resfound and not $ignoreRs ) {
+    if ( $resfound ) {
         $resrec->{'ResFound'} = $resfound;
         $messages->{'ResFound'} = $resrec;
-        $dotransfer = 1;
+        $dotransfer = 0 unless $ignoreRs;
     }
 
     #actually do the transfer....
@@ -384,7 +386,7 @@ sub transferbook {
         ModItemTransfer( $itemnumber, $fbr, $tbr, $trigger );
 
         # don't need to update MARC anymore, we do it in batch now
-        $messages->{'WasTransfered'} = 1;
+        $messages->{'WasTransfered'} = $tbr;
 
     }
     ModDateLastSeen( $itemnumber );
@@ -543,10 +545,11 @@ sub TooMany {
             $checkouts = $patron->checkouts->search(
                 { 'me.branchcode' => $branch} );
         } elsif (C4::Context->preference('CircControl') eq 'PatronLibrary') {
-            ; # if branch is the patron's home branch, then count all loans by patron
+            $checkouts = $patron->checkouts; # if branch is the patron's home branch, then count all loans by patron
         } else {
             $checkouts = $patron->checkouts->search(
-                { 'item.homebranch' => $branch} );
+                { 'item.homebranch' => $branch},
+                { prefetch          => 'item' } );
         }
 
         my $checkout_count = $checkouts->count;
@@ -716,6 +719,10 @@ issued to someone else.
 
 reserved for someone else.
 
+=head3 TRANSFERRED
+
+reserved and being transferred for someone else.
+
 =head3 INVALID_DATE
 
 sticky due date is invalid or due date in the past
@@ -840,7 +847,7 @@ sub CanBookBeIssued {
     $no_issues_charge_guarantees = undef unless looks_like_number( $no_issues_charge_guarantees );
     if ( defined $no_issues_charge_guarantees ) {
         my @guarantees = map { $_->guarantee } $patron->guarantee_relationships();
-        my $guarantees_non_issues_charges;
+        my $guarantees_non_issues_charges = 0;
         foreach my $g ( @guarantees ) {
             $guarantees_non_issues_charges += $g->account->non_issues_charges;
         }
@@ -1128,6 +1135,28 @@ sub CanBookBeIssued {
                     $needsconfirmation{'resreservedate'} = $res->{reservedate};
                     $needsconfirmation{'reserve_id'} = $res->{reserve_id};
                 }
+                elsif ( $restype eq "Transferred" ) {
+                    # The item is determined hold being transferred for someone else.
+                    $needsconfirmation{TRANSFERRED} = 1;
+                    $needsconfirmation{'resfirstname'} = $patron->firstname;
+                    $needsconfirmation{'ressurname'} = $patron->surname;
+                    $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
+                    $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
+                    $needsconfirmation{'resbranchcode'} = $patron->branchcode;
+                    $needsconfirmation{'resreservedate'} = $res->{reservedate};
+                    $needsconfirmation{'reserve_id'} = $res->{reserve_id};
+                }
+                elsif ( $restype eq "Processing" ) {
+                    # The item is determined hold being processed for someone else.
+                    $needsconfirmation{PROCESSING} = 1;
+                    $needsconfirmation{'resfirstname'} = $patron->firstname;
+                    $needsconfirmation{'ressurname'} = $patron->surname;
+                    $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
+                    $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
+                    $needsconfirmation{'resbranchcode'} = $patron->branchcode;
+                    $needsconfirmation{'resreservedate'} = $res->{reservedate};
+                    $needsconfirmation{'reserve_id'} = $res->{reserve_id};
+                }
             }
         }
     }
@@ -1329,16 +1358,30 @@ sub checkHighHolds {
 
         my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branchcode, $borrower );
 
-        my $decreaseLoanHighHoldsDuration = C4::Context->preference('decreaseLoanHighHoldsDuration');
+        my $rule = Koha::CirculationRules->get_effective_rule(
+            {
+                categorycode => $borrower->{categorycode},
+                itemtype     => $item_object->effective_itemtype,
+                branchcode   => $branchcode,
+                rule_name    => 'decreaseloanholds',
+            }
+        );
 
-        my $reduced_datedue = $calendar->addDate( $issuedate, $decreaseLoanHighHoldsDuration );
+        my $duration;
+        if ( defined($rule) && $rule->rule_value ne '' ){
+            # overrides decreaseLoanHighHoldsDuration syspref
+            $duration = $rule->rule_value;
+        } else {
+            $duration = C4::Context->preference('decreaseLoanHighHoldsDuration');
+        }
+        my $reduced_datedue = $calendar->addDuration( $issuedate, $duration );
         $reduced_datedue->set_hour($orig_due->hour);
         $reduced_datedue->set_minute($orig_due->minute);
         $reduced_datedue->truncate( to => 'minute' );
 
         if ( DateTime->compare( $reduced_datedue, $orig_due ) == -1 ) {
             $return_data->{exceeded} = 1;
-            $return_data->{duration} = $decreaseLoanHighHoldsDuration;
+            $return_data->{duration} = $duration;
             $return_data->{due_date} = $reduced_datedue;
         }
     }
@@ -1459,6 +1502,8 @@ sub AddIssue {
                 my ( $allowed, $message ) = CanBookBeReturned( $item_unblessed, C4::Context->userenv->{branch} );
                 return unless $allowed;
                 AddReturn( $item_object->barcode, C4::Context->userenv->{'branch'} );
+                # AddReturn certainly has side-effects, like onloan => undef
+                $item_object->discard_changes;
             }
 
             C4::Reserves::MoveReserve( $item_object->itemnumber, $borrower->{'borrowernumber'}, $cancelreserve );
@@ -1496,14 +1541,6 @@ sub AddIssue {
                 $auto_renew = $rule->rule_value if $rule;
             }
 
-            # Record in the database the fact that the book was issued.
-            unless ($datedue) {
-                my $itype = $item_object->effective_itemtype;
-                $datedue = CalcDateDue( $issuedate, $itype, $branchcode, $borrower );
-
-            }
-            $datedue->truncate( to => 'minute' );
-
             my $issue_attributes = {
                 borrowernumber  => $borrower->{'borrowernumber'},
                 issuedate       => $issuedate->strftime('%Y-%m-%d %H:%M:%S'),
@@ -1513,6 +1550,19 @@ sub AddIssue {
                 auto_renew      => $auto_renew ? 1 : 0,
             };
 
+            # Get ID of logged in user.  if called from a batch job,
+            # no user session exists and C4::Context->userenv() returns
+            # the scalar '0'. Only do this if the syspref says so
+            if ( C4::Context->preference('RecordStaffUserOnCheckout') ) {
+                my $userenv = C4::Context->userenv();
+                my $usernumber = (ref($userenv) eq 'HASH') ? $userenv->{'number'} : undef;
+                if ($usernumber) {
+                    $issue_attributes->{issuer_id} = $usernumber;
+                }
+            }
+
+            # In the case that the borrower has an on-site checkout
+            # and SwitchOnSiteCheckouts is enabled this converts it to a regular checkout
             $issue = Koha::Checkouts->find( { itemnumber => $item_object->itemnumber } );
             if ($issue) {
                 $issue->set($issue_attributes)->store;
@@ -1525,6 +1575,7 @@ sub AddIssue {
                     }
                 )->store;
             }
+            $issue->discard_changes;
             if ( $item_object->location && $item_object->location eq 'CART'
                 && ( !$item_object->permanent_location || $item_object->permanent_location ne 'CART' ) ) {
             ## Item was moved to cart via UpdateItemLocationOnCheckin, anything issued should be taken off the cart.
@@ -1535,13 +1586,43 @@ sub AddIssue {
                 UpdateTotalIssues( $item_object->biblionumber, 1 );
             }
 
+            # Record if item was lost
+            my $was_lost = $item_object->itemlost;
+
             $item_object->issues( ( $item_object->issues || 0 ) + 1);
             $item_object->holdingbranch(C4::Context->userenv->{'branch'});
             $item_object->itemlost(0);
             $item_object->onloan($datedue->ymd());
             $item_object->datelastborrowed( dt_from_string()->ymd() );
+            $item_object->datelastseen( dt_from_string()->ymd() );
             $item_object->store({log_action => 0});
-            ModDateLastSeen( $item_object->itemnumber );
+
+            # If the item was lost, it has now been found, charge the overdue if necessary
+            if ($was_lost) {
+                if ( $item_object->{_charge} ) {
+                    $actualissue //= Koha::Old::Checkouts->search(
+                        { itemnumber => $item_unblessed->{itemnumber} },
+                        {
+                            order_by => { '-desc' => 'returndate' },
+                            rows     => 1
+                        }
+                    )->single;
+                    unless ( exists( $borrower->{branchcode} ) ) {
+                        my $patron = $actualissue->patron;
+                        $borrower = $patron->unblessed;
+                    }
+                    _CalculateAndUpdateFine(
+                        {
+                            issue       => $actualissue,
+                            item        => $item_unblessed,
+                            borrower    => $borrower,
+                            return_date => $issuedate
+                        }
+                    );
+                    _FixOverduesOnReturn( $borrower->{borrowernumber},
+                        $item_object->itemnumber, undef, 'RENEWED' );
+                }
+            }
 
             # If it costs to borrow this book, charge it to the patron's account.
             my ( $charge, $itemtype ) = GetIssuingCharges( $item_object->itemnumber, $borrower->{'borrowernumber'} );
@@ -1748,9 +1829,10 @@ branch and item type, regardless of patron category.
 The return value is a hashref containing the following keys:
 
 holdallowed => Hold policy for this branch and itemtype. Possible values:
-  0: No holds allowed.
-  1: Holds allowed only by patrons that have the same homebranch as the item.
-  2: Holds allowed from any patron.
+  not_allowed:           No holds allowed.
+  from_home_library:     Holds allowed only by patrons that have the same homebranch as the item.
+  from_any_library:      Holds allowed from any patron.
+  from_local_hold_group: Holds allowed from libraries in hold group
 
 returnbranch => branch to which to return item.  Possible values:
   noreturn: do not return, let item remain where checked in (floating collections)
@@ -1775,22 +1857,22 @@ sub GetBranchItemRule {
     my $holdallowed_rule = Koha::CirculationRules->get_effective_rule(
         {
             branchcode => $branchcode,
-            itemtype => $itemtype,
-            rule_name => 'holdallowed',
+            itemtype   => $itemtype,
+            rule_name  => 'holdallowed',
         }
     );
     my $hold_fulfillment_policy_rule = Koha::CirculationRules->get_effective_rule(
         {
             branchcode => $branchcode,
-            itemtype => $itemtype,
-            rule_name => 'hold_fulfillment_policy',
+            itemtype   => $itemtype,
+            rule_name  => 'hold_fulfillment_policy',
         }
     );
     my $returnbranch_rule = Koha::CirculationRules->get_effective_rule(
         {
             branchcode => $branchcode,
-            itemtype => $itemtype,
-            rule_name => 'returnbranch',
+            itemtype   => $itemtype,
+            rule_name  => 'returnbranch',
         }
     );
 
@@ -1798,7 +1880,7 @@ sub GetBranchItemRule {
     my $rules;
     $rules->{holdallowed} = defined $holdallowed_rule
         ? $holdallowed_rule->rule_value
-        : 2;
+        : 'from_any_library';
     $rules->{hold_fulfillment_policy} = defined $hold_fulfillment_policy_rule
         ? $hold_fulfillment_policy_rule->rule_value
         : 'any';
@@ -1936,7 +2018,7 @@ sub AddReturn {
     my $borrowernumber = $patron ? $patron->borrowernumber : undef;    # we don't know if we had a borrower or not
     my $patron_unblessed = $patron ? $patron->unblessed : {};
 
-    my $update_loc_rules = get_yaml_pref_hash('UpdateItemLocationOnCheckin');
+    my $update_loc_rules = Koha::Config::SysPrefs->find('UpdateItemLocationOnCheckin')->get_yaml_pref_hash();
     map { $update_loc_rules->{$_} = $update_loc_rules->{$_}[0] } keys %$update_loc_rules; #We can only move to one location so we flatten the arrays
     if ($update_loc_rules) {
         if (defined $update_loc_rules->{_ALL_}) {
@@ -1964,7 +2046,7 @@ sub AddReturn {
     if ($yaml) {
         $yaml = "$yaml\n\n";  # YAML is anal on ending \n. Surplus does not hurt
         my $rules;
-        eval { $rules = YAML::Load($yaml); };
+        eval { $rules = YAML::XS::Load(Encode::encode_utf8($yaml)); };
         if ($@) {
             warn "Unable to parse UpdateNotForLoanStatusOnCheckin syspref : $@";
         }
@@ -1986,6 +2068,7 @@ sub AddReturn {
             Wrongbranch => $branch,
             Rightbranch => $message
         };
+        $doreturn = 0;
         my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX });
         $indexer->index_records( $item->biblionumber, "specialUpdate", "biblioserver" );
         return ( $doreturn, $messages, $issue, $patron_unblessed);
@@ -2021,7 +2104,7 @@ sub AddReturn {
                     _CalculateAndUpdateFine( { issue => $issue, item => $item->unblessed, borrower => $patron_unblessed, return_date => $return_date } );
                 }
             } else {
-                carp "The checkin for the following issue failed, Please go to the about page, section 'data corrupted' to know how to fix this problem ($@)" . Dumper( $issue->unblessed );
+                carp "The checkin for the following issue failed, Please go to the about page and check all messages on the 'System information' to see if there are configuration / data issues ($@)" . Dumper( $issue->unblessed );
 
                 my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX });
                 $indexer->index_records( $item->biblionumber, "specialUpdate", "biblioserver" );
@@ -2048,31 +2131,61 @@ sub AddReturn {
     my $updated_item = ModDateLastSeen( $item->itemnumber, $leave_item_lost, { skip_record_index => 1 } ); # will unset itemlost if needed
 
     # fix up the accounts.....
-    if ( $item_was_lost ) {
+    if ($item_was_lost) {
         $messages->{'WasLost'} = 1;
         unless ( C4::Context->preference("BlockReturnOfLostItems") ) {
             $messages->{'LostItemFeeRefunded'} = $updated_item->{_refunded};
+            $messages->{'LostItemFeeRestored'} = $updated_item->{_restored};
+
+            if ( $updated_item->{_charge} ) {
+                $issue //= Koha::Old::Checkouts->search(
+                    { itemnumber => $item->itemnumber },
+                    { order_by   => { '-desc' => 'returndate' }, rows => 1 } )
+                  ->single;
+                unless ( exists( $patron_unblessed->{branchcode} ) ) {
+                    my $patron = $issue->patron;
+                    $patron_unblessed = $patron->unblessed;
+                }
+                _CalculateAndUpdateFine(
+                    {
+                        issue       => $issue,
+                        item        => $item->unblessed,
+                        borrower    => $patron_unblessed,
+                        return_date => $return_date
+                    }
+                );
+                _FixOverduesOnReturn( $patron_unblessed->{borrowernumber},
+                    $item->itemnumber, undef, 'RETURNED' );
+                $messages->{'LostItemFeeCharged'} = 1;
+            }
         }
     }
 
     # check if we have a transfer for this document
-    my ($datesent,$frombranch,$tobranch) = GetTransfers( $item->itemnumber );
+    my $transfer = $item->get_transfer;
 
     # if we have a transfer to complete, we update the line of transfers with the datearrived
-    my $is_in_rotating_collection = C4::RotatingCollections::isItemInAnyCollection( $item->itemnumber );
-    if ($datesent) {
-        # At this point we will either fill the transfer or it is a wrong transfer
-        # either way we should not now generate a new transfer
+    if ($transfer) {
         $validTransfer = 0;
-        if ( $tobranch eq $branch ) {
-            my $sth = C4::Context->dbh->prepare(
-                "UPDATE branchtransfers SET datearrived = now() WHERE itemnumber= ? AND datearrived IS NULL"
-            );
-            $sth->execute( $item->itemnumber );
-            $messages->{'TransferArrived'} = $frombranch;
-        } else {
-            $messages->{'WrongTransfer'}     = $tobranch;
-            $messages->{'WrongTransferItem'} = $item->itemnumber;
+        if ( $transfer->in_transit ) {
+            if ( $transfer->tobranch eq $branch ) {
+                $transfer->receive;
+                $messages->{'TransferArrived'} = $transfer->frombranch;
+            }
+            else {
+                $messages->{'WrongTransfer'}     = $transfer->tobranch;
+                $messages->{'WrongTransferItem'} = $item->itemnumber;
+            }
+        }
+        else {
+            if ( $transfer->tobranch eq $branch ) {
+                $transfer->receive;
+                $messages->{'TransferArrived'} = $transfer->frombranch;
+            }
+            else {
+                $messages->{'WasTransfered'}   = $transfer->tobranch;
+                $messages->{'TransferTrigger'} = $transfer->reason;
+            }
         }
     }
 
@@ -2163,7 +2276,13 @@ sub AddReturn {
     }
 
     # Transfer to returnbranch if Automatic transfer set or append message NeedsTransfer
-    if ($validTransfer && !$is_in_rotating_collection && ($doreturn or $messages->{'NotIssued'}) and !$resfound and ($branch ne $returnbranch) ){
+    if ( $validTransfer && !C4::RotatingCollections::isItemInAnyCollection( $item->itemnumber )
+        && ( $doreturn or $messages->{'NotIssued'} )
+        and !$resfound
+        and ( $branch ne $returnbranch )
+        and not $messages->{'WrongTransfer'}
+        and not $messages->{'WasTransfered'} )
+    {
         my $BranchTransferLimitsType = C4::Context->preference("BranchTransferLimitsType") eq 'itemtype' ? 'effective_itemtype' : 'ccode';
         if  (C4::Context->preference("AutomaticItemReturn"    ) or
             (C4::Context->preference("UseBranchTransferLimits") and
@@ -2172,7 +2291,8 @@ sub AddReturn {
             $debug and warn sprintf "about to call ModItemTransfer(%s, %s, %s, %s)", $item->itemnumber,$branch, $returnbranch, $transfer_trigger;
             $debug and warn "item: " . Dumper($item->unblessed);
             ModItemTransfer($item->itemnumber, $branch, $returnbranch, $transfer_trigger, { skip_record_index => 1 });
-            $messages->{'WasTransfered'} = 1;
+            $messages->{'WasTransfered'} = $returnbranch;
+            $messages->{'TransferTrigger'} = $transfer_trigger;
         } else {
             $messages->{'NeedsTransfer'} = $returnbranch;
             $messages->{'TransferTrigger'} = $transfer_trigger;
@@ -2351,7 +2471,7 @@ sub _calculate_new_debar_dt {
 
     # grace period is measured in the same units as the loan
     my $grace =
-      DateTime::Duration->new( $unit => $issuing_rule->{firstremind} );
+      DateTime::Duration->new( $unit => $issuing_rule->{firstremind} // 0);
 
     my $deltadays = DateTime::Duration->new(
         days => $chargeable_units
@@ -2392,7 +2512,7 @@ sub _calculate_new_debar_dt {
                 branchcode => $branchcode,
                 days_mode  => 'Calendar'
             );
-            $new_debar_dt = $calendar->addDate( $return_date, $suspension_days );
+            $new_debar_dt = $calendar->addDuration( $return_date, $suspension_days );
         }
         else {
             $new_debar_dt = $return_date->clone()->add_duration($suspension_days);
@@ -2481,6 +2601,7 @@ sub _FixOverduesOnReturn {
             my $amountoutstanding = $accountline->amountoutstanding;
             if ( $accountline->amount == 0 && $payments->count == 0 ) {
                 $accountline->delete;
+                return 0; # no warning, we've just removed a zero value fine (backdated return)
             } elsif ($exemptfine && ($amountoutstanding != 0)) {
                 my $account = Koha::Account->new({patron_id => $borrowernumber});
                 my $credit = $account->add_credit(
@@ -2499,14 +2620,10 @@ sub _FixOverduesOnReturn {
                 if (C4::Context->preference("FinesLog")) {
                     &logaction("FINES", 'MODIFY',$borrowernumber,"Overdue forgiven: item $item");
                 }
-
-                $accountline->status('FORGIVEN');
-                $accountline->store();
-            } else {
-                $accountline->status($status);
-                $accountline->store();
-
             }
+
+            $accountline->status($status);
+            return $accountline->store();
         }
     );
 
@@ -2572,50 +2689,6 @@ sub GetOpenIssue {
 
 }
 
-=head2 GetBiblioIssues
-
-  $issues = GetBiblioIssues($biblionumber);
-
-this function get all issues from a biblionumber.
-
-Return:
-C<$issues> is a reference to array which each value is ref-to-hash. This ref-to-hash contains all column from
-tables issues and the firstname,surname & cardnumber from borrowers.
-
-=cut
-
-sub GetBiblioIssues {
-    my $biblionumber = shift;
-    return unless $biblionumber;
-    my $dbh   = C4::Context->dbh;
-    my $query = "
-        SELECT issues.*,items.barcode,biblio.biblionumber,biblio.title, biblio.author,borrowers.cardnumber,borrowers.surname,borrowers.firstname
-        FROM issues
-            LEFT JOIN borrowers ON borrowers.borrowernumber = issues.borrowernumber
-            LEFT JOIN items ON issues.itemnumber = items.itemnumber
-            LEFT JOIN biblioitems ON items.itemnumber = biblioitems.biblioitemnumber
-            LEFT JOIN biblio ON biblio.biblionumber = items.biblionumber
-        WHERE biblio.biblionumber = ?
-        UNION ALL
-        SELECT old_issues.*,items.barcode,biblio.biblionumber,biblio.title, biblio.author,borrowers.cardnumber,borrowers.surname,borrowers.firstname
-        FROM old_issues
-            LEFT JOIN borrowers ON borrowers.borrowernumber = old_issues.borrowernumber
-            LEFT JOIN items ON old_issues.itemnumber = items.itemnumber
-            LEFT JOIN biblioitems ON items.itemnumber = biblioitems.biblioitemnumber
-            LEFT JOIN biblio ON biblio.biblionumber = items.biblionumber
-        WHERE biblio.biblionumber = ?
-        ORDER BY timestamp
-    ";
-    my $sth = $dbh->prepare($query);
-    $sth->execute($biblionumber, $biblionumber);
-
-    my @issues;
-    while ( my $data = $sth->fetchrow_hashref ) {
-        push @issues, $data;
-    }
-    return \@issues;
-}
-
 =head2 GetUpcomingDueIssues
 
   my $upcoming_dues = GetUpcomingDueIssues( { days_in_advance => 4 } );
@@ -2627,19 +2700,15 @@ sub GetUpcomingDueIssues {
 
     $params->{'days_in_advance'} = 7 unless exists $params->{'days_in_advance'};
     my $dbh = C4::Context->dbh;
-
-    my $statement = <<END_SQL;
-SELECT *
-FROM (
-    SELECT issues.*, items.itype as itemtype, items.homebranch, TO_DAYS( date_due )-TO_DAYS( NOW() ) as days_until_due, branches.branchemail
-    FROM issues
-    LEFT JOIN items USING (itemnumber)
-    LEFT OUTER JOIN branches USING (branchcode)
-    WHERE returndate is NULL
-) tmp
-WHERE days_until_due >= 0 AND days_until_due <= ?
-END_SQL
-
+    my $statement;
+    $statement = q{
+        SELECT issues.*, items.itype as itemtype, items.homebranch, TO_DAYS( date_due )-TO_DAYS( NOW() ) as days_until_due, branches.branchemail
+        FROM issues
+        LEFT JOIN items USING (itemnumber)
+        LEFT JOIN branches ON branches.branchcode =
+    };
+    $statement .= $params->{'owning_library'} ? " items.homebranch " : " issues.branchcode ";
+    $statement .= " WHERE returndate is NULL AND TO_DAYS( date_due )-TO_DAYS( NOW() ) BETWEEN 0 AND ?";
     my @bind_parameters = ( $params->{'days_in_advance'} );
     
     my $sth = $dbh->prepare( $statement );
@@ -2701,6 +2770,7 @@ sub CanBookBeRenewed {
                     'no_auto_renewal_after_hard_limit',
                     'lengthunit',
                     'norenewalbefore',
+                    'unseen_renewals_allowed'
                 ]
             }
         );
@@ -2708,6 +2778,11 @@ sub CanBookBeRenewed {
         return ( 0, "too_many" )
           if not $issuing_rule->{renewalsallowed} or $issuing_rule->{renewalsallowed} <= $issue->renewals;
 
+        return ( 0, "too_unseen" )
+          if C4::Context->preference('UnseenRenewals') &&
+            $issuing_rule->{unseen_renewals_allowed} &&
+            $issuing_rule->{unseen_renewals_allowed} <= $issue->unseen_renewals;
+
         my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing');
         my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing');
         $patron         = Koha::Patrons->find($borrowernumber); # FIXME Is this really useful?
@@ -2795,7 +2870,7 @@ sub CanBookBeRenewed {
         }
     }
 
-    my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber);
+    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} ) {
@@ -2811,9 +2886,7 @@ sub CanBookBeRenewed {
     if ( $resfound
         && C4::Context->preference('AllowRenewalIfOtherItemsAvailable') )
     {
-        my $schema = Koha::Database->new()->schema();
-
-        my $item_holds = $schema->resultset('Reserve')->search( { itemnumber => $itemnumber, found => undef } )->count();
+        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;
@@ -2821,51 +2894,37 @@ sub CanBookBeRenewed {
         else {
 
             # Get all other items that could possibly fill reserves
-            my @itemnumbers = $schema->resultset('Item')->search(
-                {
-                    biblionumber => $resrec->{biblionumber},
-                    onloan       => undef,
-                    notforloan   => 0,
-                    -not         => { itemnumber => $itemnumber }
-                },
-                { columns => 'itemnumber' }
-            )->get_column('itemnumber')->all();
+            my $items = Koha::Items->search({
+                biblionumber => $resrec->{biblionumber},
+                onloan       => undef,
+                notforloan   => 0,
+                -not         => { itemnumber => $itemnumber }
+            });
 
             # Get all other reserves that could have been filled by this item
-            my @borrowernumbers;
-            while (1) {
-                my ( $reserve_found, $reserve, undef ) =
-                  C4::Reserves::CheckReserves( $itemnumber, undef, undef, \@borrowernumbers );
-
-                if ($reserve_found) {
-                    push( @borrowernumbers, $reserve->{borrowernumber} );
-                }
-                else {
-                    last;
-                }
-            }
+            my @borrowernumbers = map { $_->{borrowernumber} } @$possible_reserves;
+            my $patrons = Koha::Patrons->search({
+                borrowernumber => { -in => \@borrowernumbers }
+            });
 
             # If the count of the union of the lists of reservable items for each borrower
             # is equal or greater than the number of borrowers, we know that all reserves
             # can be filled with available items. We can get the union of the sets simply
             # by pushing all the elements onto an array and removing the duplicates.
             my @reservable;
-            my %patrons;
-            ITEM: foreach my $itemnumber (@itemnumbers) {
-                my $item = Koha::Items->find( $itemnumber );
-                next if IsItemOnHoldAndFound( $itemnumber );
-                for my $borrowernumber (@borrowernumbers) {
-                    my $patron = $patrons{$borrowernumber} //= Koha::Patrons->find( $borrowernumber );
+            ITEM: while ( my $item = $items->next ) {
+                next if IsItemOnHoldAndFound( $item->itemnumber );
+                while ( my $patron = $patrons->next ) {
                     next unless IsAvailableForItemLevelRequest($item, $patron);
-                    next unless CanItemBeReserved($borrowernumber,$itemnumber);
-
-                    push @reservable, $itemnumber;
+                    next unless CanItemBeReserved($patron->borrowernumber,$item->itemnumber,undef,{ignore_hold_counts=>1})->{status} eq 'OK';
+                    push @reservable, $item->itemnumber;
                     if (@reservable >= @borrowernumbers) {
                         $resfound = 0;
                         last ITEM;
                     }
                     last;
                 }
+                $patrons->reset;
             }
         }
     }
@@ -2884,7 +2943,7 @@ sub CanBookBeRenewed {
 
 =head2 AddRenewal
 
-  &AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate]);
+  &AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate], [$seen]);
 
 Renews a loan.
 
@@ -2909,6 +2968,10 @@ syspref)
 If C<$datedue> is the empty string, C<&AddRenewal> will calculate the due date automatically
 from the book's item type.
 
+C<$seen> is a boolean flag indicating if the item was seen or not during the renewal. This
+informs the incrementing of the unseen_renewals column. If this flag is not supplied, we
+fallback to a true value
+
 =cut
 
 sub AddRenewal {
@@ -2918,6 +2981,10 @@ sub AddRenewal {
     my $datedue         = shift;
     my $lastreneweddate = shift || dt_from_string();
     my $skipfinecalc    = shift;
+    my $seen            = shift;
+
+    # Fallback on a 'seen' renewal
+    $seen = defined $seen && $seen == 0 ? 0 : 1;
 
     my $item_object   = Koha::Items->find($itemnumber) or return;
     my $biblio = $item_object->biblio;
@@ -2970,15 +3037,39 @@ sub AddRenewal {
             }
         );
 
+        # Increment the unseen renewals, if appropriate
+        # We only do so if the syspref is enabled and
+        # a maximum value has been set in the circ rules
+        my $unseen_renewals = $issue->unseen_renewals;
+        if (C4::Context->preference('UnseenRenewals')) {
+            my $rule = Koha::CirculationRules->get_effective_rule(
+                {   categorycode => $patron->categorycode,
+                    itemtype     => $item_object->effective_itemtype,
+                    branchcode   => $circ_library->branchcode,
+                    rule_name    => 'unseen_renewals_allowed'
+                }
+            );
+            if (!$seen && $rule && $rule->rule_value) {
+                $unseen_renewals++;
+            } else {
+                # If the renewal is seen, unseen should revert to 0
+                $unseen_renewals = 0;
+            }
+        }
+
         # Update the issues record to have the new due date, and a new count
         # of how many times it has been renewed.
         my $renews = ( $issue->renewals || 0 ) + 1;
-        my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ?
-                                WHERE borrowernumber=?
-                                AND itemnumber=?"
-        );
+        my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, unseen_renewals = ?, lastreneweddate = ? WHERE issue_id = ?");
 
-        $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $lastreneweddate, $borrowernumber, $itemnumber );
+        eval{
+            $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $unseen_renewals, $lastreneweddate, $issue->issue_id );
+        };
+        if( $sth->err ){
+            Koha::Exceptions::Checkout::FailedRenewal->throw(
+                error => 'Update of issue# ' . $issue->issue_id . ' failed with error: ' . $sth->errstr
+            );
+        }
 
         # Update the renewal count on the item, and tell zebra to reindex
         $renews = ( $item_object->renewals || 0 ) + 1;
@@ -3065,13 +3156,16 @@ sub GetRenewCount {
     my ( $bornum, $itemno ) = @_;
     my $dbh           = C4::Context->dbh;
     my $renewcount    = 0;
+    my $unseencount    = 0;
     my $renewsallowed = 0;
+    my $unseenallowed = 0;
     my $renewsleft    = 0;
+    my $unseenleft    = 0;
 
     my $patron = Koha::Patrons->find( $bornum );
     my $item   = Koha::Items->find($itemno);
 
-    return (0, 0, 0) unless $patron or $item; # Wrong call, no renewal allowed
+    return (0, 0, 0, 0, 0, 0) unless $patron or $item; # Wrong call, no renewal allowed
 
     # Look in the issues table for this item, lent to this borrower,
     # and not yet returned.
@@ -3085,22 +3179,34 @@ sub GetRenewCount {
     $sth->execute( $bornum, $itemno );
     my $data = $sth->fetchrow_hashref;
     $renewcount = $data->{'renewals'} if $data->{'renewals'};
+    $unseencount = $data->{'unseen_renewals'} if $data->{'unseen_renewals'};
     # $item and $borrower should be calculated
     my $branchcode = _GetCircControlBranch($item->unblessed, $patron->unblessed);
 
-    my $rule = Koha::CirculationRules->get_effective_rule(
+    my $rules = Koha::CirculationRules->get_effective_rules(
         {
             categorycode => $patron->categorycode,
             itemtype     => $item->effective_itemtype,
             branchcode   => $branchcode,
-            rule_name    => 'renewalsallowed',
+            rules        => [ 'renewalsallowed', 'unseen_renewals_allowed' ]
         }
     );
-
-    $renewsallowed = $rule ? $rule->rule_value : 0;
+    $renewsallowed = $rules ? $rules->{renewalsallowed} : 0;
+    $unseenallowed = $rules->{unseen_renewals_allowed} ?
+        $rules->{unseen_renewals_allowed} :
+        0;
     $renewsleft    = $renewsallowed - $renewcount;
+    $unseenleft    = $unseenallowed - $unseencount;
     if($renewsleft < 0){ $renewsleft = 0; }
-    return ( $renewcount, $renewsallowed, $renewsleft );
+    if($unseenleft < 0){ $unseenleft = 0; }
+    return (
+        $renewcount,
+        $renewsallowed,
+        $renewsleft,
+        $unseencount,
+        $unseenallowed,
+        $unseenleft
+    );
 }
 
 =head2 GetSoonestRenewDate
@@ -3273,12 +3379,17 @@ sub GetIssuingCharges {
     if ( my $item_data = $sth->fetchrow_hashref ) {
         $item_type = $item_data->{itemtype};
         $charge    = $item_data->{rentalcharge};
+        # FIXME This should follow CircControl
         my $branch = C4::Context::mybranch();
         my $patron = Koha::Patrons->find( $borrowernumber );
-        my $discount = _get_discount_from_rule($patron->categorycode, $branch, $item_type);
+        my $discount = Koha::CirculationRules->get_effective_rule({
+            categorycode => $patron->categorycode,
+            branchcode   => $branch,
+            itemtype     => $item_type,
+            rule_name    => 'rentaldiscount'
+        });
         if ($discount) {
-            # We may have multiple rules so get the most specific
-            $charge = ( $charge * ( 100 - $discount ) ) / 100;
+            $charge = ( $charge * ( 100 - $discount->rule_value ) ) / 100;
         }
         if ($charge) {
             $charge = sprintf '%.2f', $charge; # ensure no fractions of a penny returned
@@ -3288,49 +3399,6 @@ sub GetIssuingCharges {
     return ( $charge, $item_type );
 }
 
-# Select most appropriate discount rule from those returned
-sub _get_discount_from_rule {
-    my ($categorycode, $branchcode, $itemtype) = @_;
-
-    # Set search precedences
-    my @params = (
-        {
-            branchcode   => $branchcode,
-            itemtype     => $itemtype,
-            categorycode => $categorycode,
-        },
-        {
-            branchcode   => undef,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-        },
-        {
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => undef,
-        },
-        {
-            branchcode   => undef,
-            categorycode => $categorycode,
-            itemtype     => undef,
-        },
-    );
-
-    foreach my $params (@params) {
-        my $rule = Koha::CirculationRules->search(
-            {
-                rule_name => 'rentaldiscount',
-                %$params,
-            }
-        )->next();
-
-        return $rule->rule_value if $rule;
-    }
-
-    # none of the above
-    return 0;
-}
-
 =head2 AddIssuingCharge
 
   &AddIssuingCharge( $checkout, $charge, $type )
@@ -3372,10 +3440,13 @@ sub GetTransfers {
         SELECT datesent,
                frombranch,
                tobranch,
-               branchtransfer_id
+               branchtransfer_id,
+               daterequested,
+               reason
         FROM branchtransfers
         WHERE itemnumber = ?
           AND datearrived IS NULL
+          AND datecancelled IS NULL
         ';
     my $sth = $dbh->prepare($query);
     $sth->execute($itemnumber);
@@ -3400,6 +3471,8 @@ sub GetTransfersFromTo {
         FROM   branchtransfers
         WHERE  frombranch=?
           AND  tobranch=?
+          AND datecancelled IS NULL
+          AND datesent IS NOT NULL
           AND datearrived IS NULL
     ";
     my $sth = $dbh->prepare($query);
@@ -3412,24 +3485,6 @@ sub GetTransfersFromTo {
     return (@gettransfers);
 }
 
-=head2 DeleteTransfer
-
-  &DeleteTransfer($itemnumber);
-
-=cut
-
-sub DeleteTransfer {
-    my ($itemnumber) = @_;
-    return unless $itemnumber;
-    my $dbh          = C4::Context->dbh;
-    my $sth          = $dbh->prepare(
-        "DELETE FROM branchtransfers
-         WHERE itemnumber=?
-         AND datearrived IS NULL "
-    );
-    return $sth->execute($itemnumber);
-}
-
 =head2 SendCirculationAlert
 
 Send out a C<check-in> or C<checkout> alert using the messaging system.
@@ -3508,7 +3563,6 @@ sub SendCirculationAlert {
             }
         ) or next;
 
-        $schema->storage->txn_begin;
         C4::Context->dbh->do(q|LOCK TABLE message_queue READ|) unless $do_not_lock;
         C4::Context->dbh->do(q|LOCK TABLE message_queue WRITE|) unless $do_not_lock;
         my $message = C4::Message->find_last_message($borrower, $type, $mtt);
@@ -3520,7 +3574,6 @@ sub SendCirculationAlert {
             $message->update;
         }
         C4::Context->dbh->do(q|UNLOCK TABLES|) unless $do_not_lock;
-        $schema->storage->txn_commit;
     }
 
     return;
@@ -3617,7 +3670,7 @@ sub CalcDateDue {
             $dur = DateTime::Duration->new( days => $loanlength->{$length_key});
         }
         my $calendar = Koha::Calendar->new( branchcode => $branch, days_mode => $daysmode );
-        $datedue = $calendar->addDate( $datedue, $dur, $loanlength->{lengthunit} );
+        $datedue = $calendar->addDuration( $datedue, $dur, $loanlength->{lengthunit} );
         if ($loanlength->{lengthunit} eq 'days') {
             $datedue->set_hour(23);
             $datedue->set_minute(59);
@@ -3821,14 +3874,15 @@ sub LostItem{
             #warn " $issues->{'borrowernumber'}  /  $itemnumber ";
         }
 
-        MarkIssueReturned($borrowernumber,$itemnumber,undef,$patron->privacy) if $mark_returned;
+        MarkIssueReturned($borrowernumber,$itemnumber,undef,$patron->privacy,$params) if $mark_returned;
     }
 
-    #When item is marked lost automatically cancel its outstanding transfers and set items holdingbranch to the transfer source branch (frombranch)
-    if (my ( $datesent,$frombranch,$tobranch ) = GetTransfers($itemnumber)) {
-        Koha::Items->find($itemnumber)->holdingbranch($frombranch)->store({ skip_record_index => $params->{skip_record_index} });
+    # When an item is marked as lost, we should automatically cancel its outstanding transfers.
+    my $item = Koha::Items->find($itemnumber);
+    my $transfers = $item->get_transfers;
+    while (my $transfer = $transfers->next) {
+        $transfer->cancel({ reason => 'ItemLost', force => 1 });
     }
-    my $transferdeleted = DeleteTransfer($itemnumber);
 }
 
 sub GetOfflineOperations {