Bug 17600: Standardize our EXPORT_OK
[srvgit] / C4 / Circulation.pm
index 6880a82..0549230 100644 (file)
@@ -21,33 +21,34 @@ package C4::Circulation;
 use Modern::Perl;
 use DateTime;
 use POSIX qw( floor );
-use Koha::DateUtils;
+use YAML::XS;
+use Encode;
+
+use Koha::DateUtils qw( dt_from_string output_pref );
 use C4::Context;
-use C4::Stats;
-use C4::Reserves;
-use C4::Biblio;
-use C4::Items;
-use C4::Members;
+use C4::Stats qw( UpdateStats );
+use C4::Reserves qw( CheckReserves CanItemBeReserved MoveReserve ModReserve ModReserveMinusPriority RevertWaitingStatus IsItemOnHoldAndFound IsAvailableForItemLevelRequest );
+use C4::Biblio qw( UpdateTotalIssues );
+use C4::Items qw( ModItemTransfer ModDateLastSeen CartToShelf );
 use C4::Accounts;
 use C4::ItemCirculationAlertPreference;
 use C4::Message;
-use C4::Debug;
-use C4::Log; # logaction
-use C4::Overdues qw(CalcFine UpdateFine get_chargeable_units);
+use C4::Log qw( logaction ); # logaction
+use C4::Overdues;
 use C4::RotatingCollections qw(GetCollectionItemBranches);
-use Algorithm::CheckDigits;
+use Algorithm::CheckDigits qw( CheckDigits );
 
-use Data::Dumper;
+use Data::Dumper qw( Dumper );
 use Koha::Account;
 use Koha::AuthorisedValues;
 use Koha::Biblioitems;
-use Koha::DateUtils;
+use Koha::DateUtils qw( dt_from_string output_pref );
 use Koha::Calendar;
 use Koha::Checkouts;
 use Koha::Illrequests;
 use Koha::Items;
 use Koha::Patrons;
-use Koha::Patron::Debarments;
+use Koha::Patron::Debarments qw( DelUniqueDebarment GetDebarments );
 use Koha::Database;
 use Koha::Libraries;
 use Koha::Account::Lines;
@@ -56,82 +57,72 @@ 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 Carp;
-use List::MoreUtils qw( uniq any );
+use Koha::Exceptions::Checkout;
+use Carp qw( carp );
+use List::MoreUtils qw( any );
 use Scalar::Util qw( looks_like_number );
-use Try::Tiny;
-use Date::Calc qw(
-  Today
-  Today_and_Now
-  Add_Delta_YM
-  Add_Delta_DHMS
-  Date_to_Days
-  Day_of_Week
-  Add_Delta_Days
-);
-use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
-
+use Date::Calc qw( Date_to_Days );
+our (@ISA, @EXPORT_OK);
 BEGIN {
-       require Exporter;
-       @ISA    = qw(Exporter);
-
-       # FIXME subs that should probably be elsewhere
-       push @EXPORT, qw(
-               &barcodedecode
-        &LostItem
-        &ReturnLostItem
-        &GetPendingOnSiteCheckouts
-       );
-
-       # subs to deal with issuing a book
-       push @EXPORT, qw(
-               &CanBookBeIssued
-               &CanBookBeRenewed
-               &AddIssue
-               &AddRenewal
-               &GetRenewCount
-        &GetSoonestRenewDate
-        &GetLatestAutoRenewDate
-               &GetIssuingCharges
-        &GetBranchBorrowerCircRule
-        &GetBranchItemRule
-               &GetBiblioIssues
-               &GetOpenIssue
-        &CheckIfIssuedToPatron
-        &IsItemIssued
-        GetTopIssues
-       );
-
-       # subs to deal with returns
-       push @EXPORT, qw(
-               &AddReturn
-        &MarkIssueReturned
-       );
-
-       # subs to deal with transfers
-       push @EXPORT, qw(
-               &transferbook
-               &GetTransfers
-               &GetTransfersFromTo
-               &updateWrongTransfer
-               &DeleteTransfer
-                &IsBranchTransferAllowed
-                &CreateBranchTransferLimit
-                &DeleteBranchTransferLimits
-        &TransferSlip
-       );
-
-    # subs to deal with offline circulation
-    push @EXPORT, qw(
-      &GetOfflineOperations
-      &GetOfflineOperation
-      &AddOfflineOperation
-      &DeleteOfflineOperation
-      &ProcessOfflineOperation
+
+    require Exporter;
+    @ISA = qw(Exporter);
+
+    # FIXME subs that should probably be elsewhere
+    push @EXPORT_OK, qw(
+      barcodedecode
+      LostItem
+      ReturnLostItem
+      GetPendingOnSiteCheckouts
+
+      CanBookBeIssued
+      checkHighHolds
+      CanBookBeRenewed
+      AddIssue
+      GetLoanLength
+      GetHardDueDate
+      AddRenewal
+      GetRenewCount
+      GetSoonestRenewDate
+      GetLatestAutoRenewDate
+      GetIssuingCharges
+      AddIssuingCharge
+      GetBranchBorrowerCircRule
+      GetBranchItemRule
+      GetBiblioIssues
+      GetOpenIssue
+      GetUpcomingDueIssues
+      CheckIfIssuedToPatron
+      IsItemIssued
+      GetAgeRestriction
+      GetTopIssues
+
+      AddReturn
+      MarkIssueReturned
+
+      transferbook
+      TooMany
+      GetTransfers
+      GetTransfersFromTo
+      updateWrongTransfer
+      CalcDateDue
+      CheckValidBarcode
+      IsBranchTransferAllowed
+      CreateBranchTransferLimit
+      DeleteBranchTransferLimits
+      TransferSlip
+
+      GetOfflineOperations
+      GetOfflineOperation
+      AddOfflineOperation
+      DeleteOfflineOperation
+      ProcessOfflineOperation
+      ProcessOfflinePayment
     );
+    push @EXPORT_OK, '_GetCircControlBranch';    # This is wrong!
 }
 
 =head1 NAME
@@ -180,7 +171,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 +202,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 +214,7 @@ or Javascript based decoding on the client side.
 
 =cut
 
-sub decode {
+sub _decode {
     my ($encoded) = @_;
     my $seq =
       'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789+-';
@@ -373,10 +364,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 +375,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 );
@@ -717,6 +708,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
@@ -841,7 +836,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;
         }
@@ -1129,6 +1124,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};
+                }
             }
         }
     }
@@ -1330,16 +1347,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;
         }
     }
@@ -1460,6 +1491,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 );
@@ -1506,6 +1539,17 @@ 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 } );
@@ -1520,6 +1564,8 @@ sub AddIssue {
                     }
                 )->store;
             }
+            $issue->discard_changes;
+            C4::Auth::track_login_daily( $borrower->{userid} );
             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.
@@ -1530,6 +1576,9 @@ 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);
@@ -1538,6 +1587,33 @@ sub AddIssue {
             $item_object->datelastseen( dt_from_string()->ymd() );
             $item_object->store({log_action => 0});
 
+            # 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'} );
             if ( $charge && $charge > 0 ) {
@@ -1743,9 +1819,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)
@@ -1770,22 +1847,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',
         }
     );
 
@@ -1793,7 +1870,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';
@@ -1931,7 +2008,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_}) {
@@ -1959,7 +2036,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 : $@";
         }
@@ -2017,7 +2094,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" );
@@ -2044,31 +2121,66 @@ 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;
+                # validTransfer=1 allows us returning the item back if the reserve is cancelled
+                $validTransfer = 1 if $transfer->reason eq 'Reserve';
+            }
+            else {
+                $messages->{'WrongTransfer'}     = $transfer->tobranch;
+                $messages->{'WrongTransferItem'} = $item->itemnumber;
+                $messages->{'TransferTrigger'}   = $transfer->reason;
+            }
+        }
+        else {
+            if ( $transfer->tobranch eq $branch ) {
+                $transfer->receive;
+                $messages->{'TransferArrived'} = $transfer->frombranch;
+                # validTransfer=1 allows us returning the item back if the reserve is cancelled
+                $validTransfer = 1 if $transfer->reason eq 'Reserve';
+            }
+            else {
+                $messages->{'WasTransfered'}   = $transfer->tobranch;
+                $messages->{'TransferTrigger'} = $transfer->reason;
+            }
         }
     }
 
@@ -2159,16 +2271,21 @@ 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
              ! IsBranchTransferAllowed($branch, $returnbranch, $item->$BranchTransferLimitsType )
            )) {
-            $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;
@@ -2388,7 +2505,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);
@@ -2477,6 +2594,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(
@@ -2495,14 +2613,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();
         }
     );
 
@@ -2568,50 +2682,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 } );
@@ -2623,19 +2693,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 );
@@ -2697,6 +2763,7 @@ sub CanBookBeRenewed {
                     'no_auto_renewal_after_hard_limit',
                     'lengthunit',
                     'norenewalbefore',
+                    'unseen_renewals_allowed'
                 ]
             }
         );
@@ -2704,6 +2771,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?
@@ -2791,7 +2863,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} ) {
@@ -2807,9 +2879,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;
@@ -2817,51 +2887,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;
             }
         }
     }
@@ -2880,7 +2936,7 @@ sub CanBookBeRenewed {
 
 =head2 AddRenewal
 
-  &AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate]);
+  &AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate], [$seen]);
 
 Renews a loan.
 
@@ -2905,6 +2961,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 {
@@ -2914,6 +2974,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;
@@ -2966,15 +3030,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;
@@ -3061,13 +3149,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.
@@ -3081,22 +3172,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
@@ -3269,12 +3372,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
@@ -3284,49 +3392,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 )
@@ -3368,10 +3433,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);
@@ -3396,6 +3464,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);
@@ -3408,24 +3478,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.
@@ -3504,7 +3556,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);
@@ -3516,7 +3567,6 @@ sub SendCirculationAlert {
             $message->update;
         }
         C4::Context->dbh->do(q|UNLOCK TABLES|) unless $do_not_lock;
-        $schema->storage->txn_commit;
     }
 
     return;
@@ -3532,19 +3582,24 @@ This function validate the line of brachtransfer but with the wrong destination
 
 sub updateWrongTransfer {
        my ( $itemNumber,$waitingAtLibrary,$FromLibrary ) = @_;
-       my $dbh = C4::Context->dbh;     
-# first step validate the actual line of transfert .
-       my $sth =
-               $dbh->prepare(
-                       "update branchtransfers set datearrived = now(),tobranch=?,comments='wrongtransfer' where itemnumber= ? AND datearrived IS NULL"
-               );
-               $sth->execute($FromLibrary,$itemNumber);
-
-# second step create a new line of branchtransfer to the right location .
-       ModItemTransfer($itemNumber, $FromLibrary, $waitingAtLibrary);
-
-#third step changing holdingbranch of item
-    my $item = Koha::Items->find($itemNumber)->holdingbranch($FromLibrary)->store;
+
+    # first step: cancel the original transfer
+    my $item = Koha::Items->find($itemNumber);
+    my $transfer = $item->get_transfer;
+    $transfer->set({ datecancelled => dt_from_string, cancellation_reason => 'WrongTransfer' })->store();
+
+    # second step: create a new transfer to the right location
+    my $new_transfer = $item->request_transfer(
+        {
+            to            => $transfer->to_library,
+            reason        => $transfer->reason,
+            comment       => $transfer->comments,
+            ignore_limits => 1,
+            enqueue       => 1
+        }
+    );
+
+    return $new_transfer;
 }
 
 =head2 CalcDateDue
@@ -3613,7 +3668,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);
@@ -3817,14 +3872,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 {