Bug 24161: Add the Koha::Acquisition::Order::Claim[s] classes
[koha-ffzg.git] / C4 / Circulation.pm
index 9fe6625..8db887d 100644 (file)
@@ -44,7 +44,7 @@ use Koha::Biblioitems;
 use Koha::DateUtils;
 use Koha::Calendar;
 use Koha::Checkouts;
-use Koha::IssuingRules;
+use Koha::Illrequests;
 use Koha::Items;
 use Koha::Patrons;
 use Koha::Patron::Debarments;
@@ -252,7 +252,7 @@ sub decode {
 =head2 transferbook
 
   ($dotransfer, $messages, $iteminformation) = &transferbook($newbranch, 
-                                            $barcode, $ignore_reserves);
+                                            $barcode, $ignore_reserves, $trigger);
 
 Transfers an item to a new branch. If the item is currently on loan, it is automatically returned before the actual transfer.
 
@@ -263,6 +263,8 @@ C<$barcode> is the barcode of the item to be transferred.
 If C<$ignore_reserves> is true, C<&transferbook> ignores reserves.
 Otherwise, if an item is reserved, the transfer fails.
 
+C<$trigger> is the enum value for what triggered the transfer.
+
 Returns three values:
 
 =over
@@ -304,7 +306,7 @@ The item was eligible to be transferred. Barring problems communicating with the
 =cut
 
 sub transferbook {
-    my ( $tbr, $barcode, $ignoreRs ) = @_;
+    my ( $tbr, $barcode, $ignoreRs, $trigger ) = @_;
     my $messages;
     my $dotransfer      = 1;
     my $item = Koha::Items->find( { barcode => $barcode } );
@@ -354,14 +356,13 @@ sub transferbook {
       CheckReserves( $itemnumber );
     if ( $resfound and not $ignoreRs ) {
         $resrec->{'ResFound'} = $resfound;
-
-        #         $messages->{'ResFound'} = $resrec;
+        $messages->{'ResFound'} = $resrec;
         $dotransfer = 1;
     }
 
     #actually do the transfer....
     if ($dotransfer) {
-        ModItemTransfer( $itemnumber, $fbr, $tbr );
+        ModItemTransfer( $itemnumber, $fbr, $tbr, $trigger );
 
         # don't need to update MARC anymore, we do it in batch now
         $messages->{'WasTransfered'} = 1;
@@ -423,18 +424,20 @@ sub TooMany {
             # specific rule
             if (C4::Context->preference('item-level_itypes')) {
                 $count_query .= " WHERE items.itype NOT IN (
-                                    SELECT itemtype FROM issuingrules
+                                    SELECT itemtype FROM circulation_rules
                                     WHERE branchcode = ?
                                     AND   (categorycode = ? OR categorycode = ?)
-                                    AND   itemtype <> '*'
+                                    AND   itemtype IS NOT NULL
+                                    AND   rule_name = 'maxissueqty'
                                   ) ";
-            } else { 
-                $count_query .= " JOIN  biblioitems USING (biblionumber) 
+            } else {
+                $count_query .= " JOIN  biblioitems USING (biblionumber)
                                   WHERE biblioitems.itemtype NOT IN (
-                                    SELECT itemtype FROM issuingrules
+                                    SELECT itemtype FROM circulation_rules
                                     WHERE branchcode = ?
                                     AND   (categorycode = ? OR categorycode = ?)
-                                    AND   itemtype <> '*'
+                                    AND   itemtype IS NOT NULL
+                                    AND   rule_name = 'maxissueqty'
                                   ) ";
             }
             push @bind_params, $maxissueqty_rule->branchcode;
@@ -472,7 +475,7 @@ sub TooMany {
         my $max_checkouts_allowed = $maxissueqty_rule ? $maxissueqty_rule->rule_value : undef;
         my $max_onsite_checkouts_allowed = $maxonsiteissueqty_rule ? $maxonsiteissueqty_rule->rule_value : undef;
 
-        if ( $onsite_checkout and defined $max_onsite_checkouts_allowed ) {
+        if ( $onsite_checkout and $max_onsite_checkouts_allowed ne '' ) {
             if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
                 return {
                     reason => 'TOO_MANY_ONSITE_CHECKOUTS',
@@ -702,7 +705,7 @@ sub CanBookBeIssued {
     if ($duedate && ref $duedate ne 'DateTime') {
         $duedate = dt_from_string($duedate);
     }
-    my $now = DateTime->now( time_zone => C4::Context->tz() );
+    my $now = dt_from_string();
     unless ( $duedate ) {
         my $issuedate = $now->clone();
 
@@ -958,7 +961,7 @@ sub CanBookBeIssued {
         }
         else {
             my $itemtype = Koha::ItemTypes->find($biblioitem->itemtype);
-            if ( $itemtype and $itemtype->notforloan == 1){
+            if ( $itemtype && defined $itemtype->notforloan && $itemtype->notforloan == 1){
                 if (!C4::Context->preference("AllowNotForLoanOverride")) {
                     $issuingimpossible{NOT_FOR_LOAN} = 1;
                     $issuingimpossible{itemtype_notforloan} = $effective_itemtype;
@@ -1233,7 +1236,7 @@ sub checkHighHolds {
             }
         }
 
-        my $issuedate = DateTime->now( time_zone => C4::Context->tz() );
+        my $issuedate = dt_from_string();
 
         my $calendar = Koha::Calendar->new( branchcode => $branchcode );
 
@@ -1312,7 +1315,7 @@ sub AddIssue {
 
     # $issuedate defaults to today.
     if ( !defined $issuedate ) {
-        $issuedate = DateTime->now( time_zone => C4::Context->tz() );
+        $issuedate = dt_from_string();
     }
     else {
         if ( ref $issuedate ne 'DateTime' ) {
@@ -1391,14 +1394,16 @@ sub AddIssue {
 
             # If automatic renewal wasn't selected while issuing, set the value according to the issuing rule.
             unless ($auto_renew) {
-                my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
-                    {   categorycode => $borrower->{categorycode},
+                my $rule = Koha::CirculationRules->get_effective_rule(
+                    {
+                        categorycode => $borrower->{categorycode},
                         itemtype     => $item_object->effective_itemtype,
-                        branchcode   => $branchcode
+                        branchcode   => $branchcode,
+                        rule_name    => 'auto_renew'
                     }
                 );
 
-                $auto_renew = $issuing_rule->auto_renew if $issuing_rule;
+                $auto_renew = $rule->rule_value if $rule;
             }
 
             # Record in the database the fact that the book was issued.
@@ -1452,23 +1457,17 @@ sub AddIssue {
                     )
                   )
                 {
-                    _FixAccountForLostAndReturned( $item_object->itemnumber, undef,
+                    _FixAccountForLostAndFound( $item_object->itemnumber, undef,
                         $item_object->barcode );
                 }
             }
 
-            ModItem(
-                {
-                    issues        => ( $item_object->issues || 0 ) + 1,
-                    holdingbranch => C4::Context->userenv->{'branch'},
-                    itemlost      => 0,
-                    onloan        => $datedue->ymd(),
-                    datelastborrowed => DateTime->now( time_zone => C4::Context->tz() )->ymd(),
-                },
-                $item_object->biblionumber,
-                $item_object->itemnumber,
-                { log_action => 0 }
-            );
+            $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->store({log_action => 0});
             ModDateLastSeen( $item_object->itemnumber );
 
             # If it costs to borrow this book, charge it to the patron's account.
@@ -1539,67 +1538,77 @@ Get loan length for an itemtype, a borrower type and a branch
 =cut
 
 sub GetLoanLength {
-    my ( $borrowertype, $itemtype, $branchcode ) = @_;
-    my $dbh = C4::Context->dbh;
-    my $sth = $dbh->prepare(qq{
-        SELECT issuelength, lengthunit, renewalperiod
-        FROM issuingrules
-        WHERE   categorycode=?
-            AND itemtype=?
-            AND branchcode=?
-            AND issuelength IS NOT NULL
-    });
+    my ( $categorycode, $itemtype, $branchcode ) = @_;
 
-    # try to find issuelength & return the 1st available.
-    # check with borrowertype, itemtype and branchcode, then without one of those parameters
-    $sth->execute( $borrowertype, $itemtype, $branchcode );
-    my $loanlength = $sth->fetchrow_hashref;
-
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    $sth->execute( $borrowertype, '*', $branchcode );
-    $loanlength = $sth->fetchrow_hashref;
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    $sth->execute( '*', $itemtype, $branchcode );
-    $loanlength = $sth->fetchrow_hashref;
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    $sth->execute( '*', '*', $branchcode );
-    $loanlength = $sth->fetchrow_hashref;
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    $sth->execute( $borrowertype, $itemtype, '*' );
-    $loanlength = $sth->fetchrow_hashref;
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    $sth->execute( $borrowertype, '*', '*' );
-    $loanlength = $sth->fetchrow_hashref;
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    $sth->execute( '*', $itemtype, '*' );
-    $loanlength = $sth->fetchrow_hashref;
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    $sth->execute( '*', '*', '*' );
-    $loanlength = $sth->fetchrow_hashref;
-    return $loanlength
-      if defined($loanlength) && defined $loanlength->{issuelength};
-
-    # if no rule is set => 0 day (hardcoded)
-    return {
-        issuelength => 0,
+    # Set search precedences
+    my @params = (
+        {
+            categorycode => $categorycode,
+            itemtype     => $itemtype,
+            branchcode   => $branchcode,
+        },
+        {
+            categorycode => $categorycode,
+            itemtype     => undef,
+            branchcode   => $branchcode,
+        },
+        {
+            categorycode => undef,
+            itemtype     => $itemtype,
+            branchcode   => $branchcode,
+        },
+        {
+            categorycode => undef,
+            itemtype     => undef,
+            branchcode   => $branchcode,
+        },
+        {
+            categorycode => $categorycode,
+            itemtype     => $itemtype,
+            branchcode   => undef,
+        },
+        {
+            categorycode => $categorycode,
+            itemtype     => undef,
+            branchcode   => undef,
+        },
+        {
+            categorycode => undef,
+            itemtype     => $itemtype,
+            branchcode   => undef,
+        },
+        {
+            categorycode => undef,
+            itemtype     => undef,
+            branchcode   => undef,
+        },
+    );
+
+    # Initialize default values
+    my $rules = {
+        issuelength   => 0,
         renewalperiod => 0,
-        lengthunit => 'days',
+        lengthunit    => 'days',
     };
 
+    # Search for rules!
+    foreach my $rule_name (qw( issuelength renewalperiod lengthunit )) {
+        foreach my $params (@params) {
+            my $rule = Koha::CirculationRules->search(
+                {
+                    rule_name => $rule_name,
+                    %$params,
+                }
+            )->next();
+
+            if ($rule) {
+                $rules->{$rule_name} = $rule->rule_value;
+                last;
+            }
+        }
+    }
+
+    return $rules;
 }
 
 
@@ -1614,19 +1623,21 @@ Get the Hard Due Date and it's comparison for an itemtype, a borrower type and a
 sub GetHardDueDate {
     my ( $borrowertype, $itemtype, $branchcode ) = @_;
 
-    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
-        {   categorycode => $borrowertype,
+    my $rules = Koha::CirculationRules->get_effective_rules(
+        {
+            categorycode => $borrowertype,
             itemtype     => $itemtype,
-            branchcode   => $branchcode
+            branchcode   => $branchcode,
+            rules        => [ 'hardduedate', 'hardduedatecompare' ],
         }
     );
 
-
-    if ( defined( $issuing_rule ) ) {
-        if ( $issuing_rule->hardduedate ) {
-            return (dt_from_string($issuing_rule->hardduedate, 'iso'),$issuing_rule->hardduedatecompare);
-        else {
-            return (undef, undef);
+    if ( defined( $rules->{hardduedate} ) ) {
+        if ( $rules->{hardduedate} ) {
+            return ( dt_from_string( $rules->{hardduedate}, 'iso' ), $rules->{hardduedatecompare} );
+        }
+        else {
+            return ( undef, undef );
         }
     }
 }
@@ -1843,6 +1854,7 @@ sub AddReturn {
         undef $branch;
     }
     $branch = C4::Context->userenv->{'branch'} unless $branch;  # we trust userenv to be a safe fallback/default
+    my $return_date_specified = !!$return_date;
     $return_date //= dt_from_string();
     my $messages;
     my $patron;
@@ -1866,7 +1878,8 @@ sub AddReturn {
                 . Dumper($issue->unblessed) . "\n";
     } else {
         $messages->{'NotIssued'} = $barcode;
-        ModItem({ onloan => undef }, $item->biblionumber, $item->itemnumber) if defined $item->onloan;
+        $item->onloan(undef)->store if defined $item->onloan;
+
         # even though item is not on loan, it may still be transferred;  therefore, get current branch info
         $doreturn = 0;
         # No issue, no borrowernumber.  ONLY if $doreturn, *might* you have a $borrower later.
@@ -1877,12 +1890,12 @@ sub AddReturn {
         }
     }
 
-    my $item_unblessed = $item->unblessed;
         # full item data, but no borrowernumber or checkout info (no issue)
     my $hbr = GetBranchItemRule($item->homebranch, $itemtype)->{'returnbranch'} || "homebranch";
         # 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)
+    my $transfer_trigger = $hbr eq 'homebranch' ? 'ReturnToHome' : $hbr eq 'holdingbranch' ? 'ReturnToHolding' : undef;
 
     my $borrowernumber = $patron ? $patron->borrowernumber : undef;    # we don't know if we had a borrower or not
     my $patron_unblessed = $patron ? $patron->unblessed : {};
@@ -1895,7 +1908,7 @@ sub AddReturn {
             if ($update_loc_rules->{_ALL_} eq '_BLANK_') { $update_loc_rules->{_ALL_} = ''; }
             if ( $item->location ne $update_loc_rules->{_ALL_}) {
                 $messages->{'ItemLocationUpdated'} = { from => $item->location, to => $update_loc_rules->{_ALL_} };
-                ModItem( { location => $update_loc_rules->{_ALL_} }, undef, $itemnumber );
+                $item->location($update_loc_rules->{_ALL_})->store;
             }
         }
         else {
@@ -1904,7 +1917,7 @@ sub AddReturn {
                 if ( $update_loc_rules->{$key} eq '_BLANK_') { $update_loc_rules->{$key} = '' ;}
                 if ( ($item->location eq $key && $item->location ne $update_loc_rules->{$key}) || ($key eq '_BLANK_' && $item->location eq '' && $update_loc_rules->{$key} ne '') ) {
                     $messages->{'ItemLocationUpdated'} = { from => $item->location, to => $update_loc_rules->{$key} };
-                    ModItem( { location => $update_loc_rules->{$key} }, undef, $itemnumber );
+                    $item->location($update_loc_rules->{$key})->store;
                     last;
                 }
             }
@@ -1923,7 +1936,7 @@ sub AddReturn {
             foreach my $key ( keys %$rules ) {
                 if ( $item->notforloan eq $key ) {
                     $messages->{'NotForLoanStatusUpdated'} = { from => $item->notforloan, to => $rules->{$key} };
-                    ModItem( { notforloan => $rules->{$key} }, undef, $itemnumber, { log_action => 0 } );
+                    $item->notforloan($rules->{$key})->store({ log_action => 0 });
                     last;
                 }
             }
@@ -1931,7 +1944,7 @@ sub AddReturn {
     }
 
     # check if the return is allowed at this branch
-    my ($returnallowed, $message) = CanBookBeReturned($item_unblessed, $branch);
+    my ($returnallowed, $message) = CanBookBeReturned($item->unblessed, $branch);
     unless ($returnallowed){
         $messages->{'Wrongbranch'} = {
             Wrongbranch => $branch,
@@ -1960,8 +1973,15 @@ sub AddReturn {
                 MarkIssueReturned( $borrowernumber, $item->itemnumber, $return_date, $patron->privacy );
             };
             unless ( $@ ) {
-                if ( C4::Context->preference('CalculateFinesOnReturn') && !$item->itemlost ) {
-                    _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed, return_date => $return_date } );
+                if (
+                    (
+                        C4::Context->preference('CalculateFinesOnReturn')
+                        || ( $return_date_specified && C4::Context->preference('CalculateFinesOnBackdate') )
+                    )
+                    && !$item->itemlost
+                  )
+                {
+                    _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 );
@@ -1974,15 +1994,14 @@ sub AddReturn {
 
         }
 
-        ModItem( { onloan => undef }, $item->biblionumber, $item->itemnumber, { log_action => 0 } );
+        $item->onloan(undef)->store({ log_action => 0 });
     }
 
     # the holdingbranch is updated if the document is returned to another location.
     # this is always done regardless of whether the item was on loan or not
     my $item_holding_branch = $item->holdingbranch;
     if ($item->holdingbranch ne $branch) {
-        UpdateHoldingbranch($branch, $item->itemnumber);
-        $item_unblessed->{'holdingbranch'} = $branch; # update item data holdingbranch too # FIXME I guess this is for the _debar_user_on_return call later
+        $item->holdingbranch($branch)->store;
     }
 
     my $leave_item_lost = C4::Context->preference("BlockReturnOfLostItems") ? 1 : 0;
@@ -2022,7 +2041,7 @@ sub AddReturn {
                 )
               )
             {
-                _FixAccountForLostAndReturned( $item->itemnumber,
+                _FixAccountForLostAndFound( $item->itemnumber,
                     $borrowernumber, $barcode );
                 $messages->{'LostItemFeeRefunded'} = 1;
             }
@@ -2036,7 +2055,7 @@ sub AddReturn {
 
         if ( $issue and $issue->is_overdue ) {
         # fix fine days
-            my ($debardate,$reminder) = _debar_user_on_return( $patron_unblessed, $item_unblessed, dt_from_string($issue->date_due), $return_date );
+            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 {
@@ -2058,10 +2077,16 @@ sub AddReturn {
     }
 
     # find reserves.....
-    # if we don't have a reserve with the status W, we launch the Checkreserves routine
+    # launch the Checkreserves routine to find any holds
     my ($resfound, $resrec);
     my $lookahead= C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds
     ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->itemnumber, undef, $lookahead ) unless ( $item->withdrawn );
+    # if a hold is found and is waiting at another branch, change the priority back to 1 and trigger the hold (this will trigger a transfer and update the hold status properly)
+    if ( $resfound eq "Waiting" and $branch ne $resrec->{branchcode} ) {
+        my $hold = C4::Reserves::RevertWaitingStatus( { itemnumber => $item->itemnumber } );
+        $resfound = 'Reserved';
+        $resrec = $hold->unblessed;
+    }
     if ($resfound) {
           $resrec->{'ResFound'} = $resfound;
         $messages->{'ResFound'} = $resrec;
@@ -2089,7 +2114,7 @@ sub AddReturn {
         if ($doreturn && $circulation_alert->is_enabled_for(\%conditions)) {
             SendCirculationAlert({
                 type     => 'CHECKIN',
-                item     => $item_unblessed,
+                item     => $item->unblessed,
                 borrower => $patron->unblessed,
                 branch   => $branch,
             });
@@ -2109,6 +2134,15 @@ sub AddReturn {
         DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
     }
 
+    # Check if this item belongs to a biblio record that is attached to an
+    # ILL request, if it is we need to update the ILL request's status
+    if (C4::Context->preference('CirculateILL')) {
+        my $request = Koha::Illrequests->find(
+            { biblio_id => $item->biblio->biblionumber }
+        );
+        $request->status('RET') if $request;
+    }
+
     # Transfer to returnbranch if Automatic transfer set or append message NeedsTransfer
     if (!$is_in_rotating_collection && ($doreturn or $messages->{'NotIssued'}) and !$resfound and ($branch ne $returnbranch) and not $messages->{'WrongTransfer'}){
         my $BranchTransferLimitsType = C4::Context->preference("BranchTransferLimitsType") eq 'itemtype' ? 'effective_itemtype' : 'ccode';
@@ -2116,12 +2150,13 @@ sub AddReturn {
             (C4::Context->preference("UseBranchTransferLimits") and
              ! IsBranchTransferAllowed($branch, $returnbranch, $item->$BranchTransferLimitsType )
            )) {
-            $debug and warn sprintf "about to call ModItemTransfer(%s, %s, %s)", $item->itemnumber,$branch, $returnbranch;
-            $debug and warn "item: " . Dumper($item_unblessed);
-            ModItemTransfer($item->itemnumber, $branch, $returnbranch);
+            $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);
             $messages->{'WasTransfered'} = 1;
         } else {
             $messages->{'NeedsTransfer'} = $returnbranch;
+            $messages->{'TransferTrigger'} = $transfer_trigger;
         }
     }
 
@@ -2206,7 +2241,7 @@ sub MarkIssueReturned {
         # And finally delete the issue
         $issue->delete;
 
-        ModItem( { 'onloan' => undef }, undef, $itemnumber, { log_action => 0 } );
+        $issue->item->onloan(undef)->store({ log_action => 0 });
 
         if ( C4::Context->preference('StoreLastBorrower') ) {
             my $item = Koha::Items->find( $itemnumber );
@@ -2245,14 +2280,21 @@ sub _calculate_new_debar_dt {
 
     my $branchcode = _GetCircControlBranch( $item, $borrower );
     my $circcontrol = C4::Context->preference('CircControl');
-    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
+    my $issuing_rule = Koha::CirculationRules->get_effective_rules(
         {   categorycode => $borrower->{categorycode},
             itemtype     => $item->{itype},
-            branchcode   => $branchcode
+            branchcode   => $branchcode,
+            rules => [
+                'finedays',
+                'lengthunit',
+                'firstremind',
+                'maxsuspensiondays',
+                'suspension_chargeperiod',
+            ]
         }
     );
-    my $finedays = $issuing_rule ? $issuing_rule->finedays : undef;
-    my $unit     = $issuing_rule ? $issuing_rule->lengthunit : undef;
+    my $finedays = $issuing_rule ? $issuing_rule->{finedays} : undef;
+    my $unit     = $issuing_rule ? $issuing_rule->{lengthunit} : undef;
     my $chargeable_units = C4::Overdues::get_chargeable_units($unit, $dt_due, $return_date, $branchcode);
 
     return unless $finedays;
@@ -2263,7 +2305,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} );
 
     my $deltadays = DateTime::Duration->new(
         days => $chargeable_units
@@ -2272,17 +2314,17 @@ sub _calculate_new_debar_dt {
     if ( $deltadays->subtract($grace)->is_positive() ) {
         my $suspension_days = $deltadays * $finedays;
 
-        if ( $issuing_rule->suspension_chargeperiod > 1 ) {
+        if ( defined $issuing_rule->{suspension_chargeperiod} && $issuing_rule->{suspension_chargeperiod} > 1 ) {
             # No need to / 1 and do not consider / 0
             $suspension_days = DateTime::Duration->new(
-                days => floor( $suspension_days->in_units('days') / $issuing_rule->suspension_chargeperiod )
+                days => floor( $suspension_days->in_units('days') / $issuing_rule->{suspension_chargeperiod} )
             );
         }
 
         # If the max suspension days is < than the suspension days
         # the suspension days is limited to this maximum period.
-        my $max_sd = $issuing_rule->maxsuspensiondays;
-        if ( defined $max_sd ) {
+        my $max_sd = $issuing_rule->{maxsuspensiondays};
+        if ( defined $max_sd && $max_sd ne '' ) {
             $max_sd = DateTime::Duration->new( days => $max_sd );
             $suspension_days = $max_sd
               if DateTime::Duration->compare( $max_sd, $suspension_days ) < 0;
@@ -2421,9 +2463,9 @@ sub _FixOverduesOnReturn {
     return $result;
 }
 
-=head2 _FixAccountForLostAndReturned
+=head2 _FixAccountForLostAndFound
 
-  &_FixAccountForLostAndReturned($itemnumber, [$borrowernumber, $barcode]);
+  &_FixAccountForLostAndFound($itemnumber, [$borrowernumber, $barcode]);
 
 Finds the most recent lost item charge for this item and refunds the borrower
 appropriatly, taking into account any payments or writeoffs already applied
@@ -2433,7 +2475,7 @@ Internal function, not exported, called only by AddReturn.
 
 =cut
 
-sub _FixAccountForLostAndReturned {
+sub _FixAccountForLostAndFound {
     my $itemnumber     = shift or return;
     my $borrowernumber = @_ ? shift : undef;
     my $item_id        = @_ ? shift : $itemnumber;  # Send the barcode if you want that logged in the description
@@ -2445,7 +2487,7 @@ sub _FixAccountForLostAndReturned {
         {
             itemnumber      => $itemnumber,
             debit_type_code => 'LOST',
-            status          => [ undef, { '<>' => 'RETURNED' } ]
+            status          => [ undef, { '<>' => 'FOUND' } ]
         },
         {
             order_by => { -desc => [ 'date', 'accountlines_id' ] }
@@ -2484,11 +2526,13 @@ sub _FixAccountForLostAndReturned {
     if ( $credit_total > 0 ) {
         my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;
         $credit = $account->add_credit(
-            {   amount      => $credit_total,
-                description => 'Item Returned ' . $item_id,
-                type        => 'LOST_RETURN',
+            {
+                amount      => $credit_total,
+                description => 'Item found ' . $item_id,
+                type        => 'LOST_FOUND',
                 interface   => C4::Context->interface,
-                library_id  => $branchcode
+                library_id  => $branchcode,
+                item_id     => $itemnumber
             }
         );
 
@@ -2496,9 +2540,11 @@ sub _FixAccountForLostAndReturned {
     }
 
     # Update the account status
-    $accountline->discard_changes->status('RETURNED');
+    $accountline->discard_changes->status('FOUND');
     $accountline->store;
 
+    $accountline->item->paidfor('')->store({ log_action => 0 });
+
     if ( defined $account and C4::Context->preference('AccountAutoReconcile') ) {
         $account->reconcile_balance;
     }
@@ -2671,6 +2717,7 @@ sub CanBookBeRenewed {
 
     my $dbh    = C4::Context->dbh;
     my $renews = 1;
+    my $auto_renew = 0;
 
     my $item      = Koha::Items->find($itemnumber)      or return ( 0, 'no_item' );
     my $issue = $item->checkout or return ( 0, 'no_checkout' );
@@ -2679,6 +2726,115 @@ sub CanBookBeRenewed {
 
     my $patron = $issue->patron or return;
 
+    # override_limit will override anything else except on_reserve
+    unless ( $override_limit ){
+        my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed );
+        my $issuing_rule = Koha::CirculationRules->get_effective_rules(
+            {
+                categorycode => $patron->categorycode,
+                itemtype     => $item->effective_itemtype,
+                branchcode   => $branchcode,
+                rules => [
+                    'renewalsallowed',
+                    'no_auto_renewal_after',
+                    'no_auto_renewal_after_hard_limit',
+                    'lengthunit',
+                    'norenewalbefore',
+                ]
+            }
+        );
+
+        return ( 0, "too_many" )
+          if not $issuing_rule->{renewalsallowed} or $issuing_rule->{renewalsallowed} <= $issue->renewals;
+
+        my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing');
+        my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing');
+        $patron         = Koha::Patrons->find($borrowernumber); # FIXME Is this really useful?
+        my $restricted  = $patron->is_debarred;
+        my $hasoverdues = $patron->has_overdues;
+
+        if ( $restricted and $restrictionblockrenewing ) {
+            return ( 0, 'restriction');
+        } elsif ( ($hasoverdues and $overduesblockrenewing eq 'block') || ($issue->is_overdue and $overduesblockrenewing eq 'blockitem') ) {
+            return ( 0, 'overdue');
+        }
+
+        if ( $issue->auto_renew && $patron->autorenew_checkouts ) {
+
+            if ( $patron->category->effective_BlockExpiredPatronOpacActions and $patron->is_expired ) {
+                return ( 0, 'auto_account_expired' );
+            }
+
+            if ( defined $issuing_rule->{no_auto_renewal_after}
+                    and $issuing_rule->{no_auto_renewal_after} ne "" ) {
+                # Get issue_date and add no_auto_renewal_after
+                # If this is greater than today, it's too late for renewal.
+                my $maximum_renewal_date = dt_from_string($issue->issuedate, 'sql');
+                $maximum_renewal_date->add(
+                    $issuing_rule->{lengthunit} => $issuing_rule->{no_auto_renewal_after}
+                );
+                my $now = dt_from_string;
+                if ( $now >= $maximum_renewal_date ) {
+                    return ( 0, "auto_too_late" );
+                }
+            }
+            if ( defined $issuing_rule->{no_auto_renewal_after_hard_limit}
+                          and $issuing_rule->{no_auto_renewal_after_hard_limit} ne "" ) {
+                # If no_auto_renewal_after_hard_limit is >= today, it's also too late for renewal
+                if ( dt_from_string >= dt_from_string( $issuing_rule->{no_auto_renewal_after_hard_limit} ) ) {
+                    return ( 0, "auto_too_late" );
+                }
+            }
+
+            if ( C4::Context->preference('OPACFineNoRenewalsBlockAutoRenew') ) {
+                my $fine_no_renewals = C4::Context->preference("OPACFineNoRenewals");
+                my $amountoutstanding =
+                  C4::Context->preference("OPACFineNoRenewalsIncludeCredit")
+                  ? $patron->account->balance
+                  : $patron->account->outstanding_debits->total_outstanding;
+                if ( $amountoutstanding and $amountoutstanding > $fine_no_renewals ) {
+                    return ( 0, "auto_too_much_oweing" );
+                }
+            }
+        }
+
+        if ( defined $issuing_rule->{norenewalbefore}
+            and $issuing_rule->{norenewalbefore} ne "" )
+        {
+
+            # Calculate soonest renewal by subtracting 'No renewal before' from due date
+            my $soonestrenewal = dt_from_string( $issue->date_due, 'sql' )->subtract(
+                $issuing_rule->{lengthunit} => $issuing_rule->{norenewalbefore} );
+
+            # Depending on syspref reset the exact time, only check the date
+            if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date'
+                and $issuing_rule->{lengthunit} eq 'days' )
+            {
+                $soonestrenewal->truncate( to => 'day' );
+            }
+
+            if ( $soonestrenewal > dt_from_string() )
+            {
+                return ( 0, "auto_too_soon" ) if $issue->auto_renew && $patron->autorenew_checkouts;
+                return ( 0, "too_soon" );
+            }
+            elsif ( $issue->auto_renew && $patron->autorenew_checkouts ) {
+                $auto_renew = 1;
+            }
+        }
+
+        # Fallback for automatic renewals:
+        # If norenewalbefore is undef, don't renew before due date.
+        if ( $issue->auto_renew && !$auto_renew && $patron->autorenew_checkouts ) {
+            my $now = dt_from_string;
+            if ( $now >= dt_from_string( $issue->date_due, 'sql' ) ){
+                $auto_renew = 1;
+            } else {
+                return ( 0, "auto_too_soon" );
+            }
+        }
+    }
+
     my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber);
 
     # This item can fill one or more unfilled reserve, can those unfilled reserves
@@ -2745,104 +2901,7 @@ sub CanBookBeRenewed {
         }
     }
     return ( 0, "on_reserve" ) if $resfound;    # '' when no hold was found
-
-    return ( 1, undef ) if $override_limit;
-
-    my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed );
-    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
-        {   categorycode => $patron->categorycode,
-            itemtype     => $item->effective_itemtype,
-            branchcode   => $branchcode
-        }
-    );
-
-    return ( 0, "too_many" )
-      if not $issuing_rule or $issuing_rule->renewalsallowed <= $issue->renewals;
-
-    my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing');
-    my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing');
-    $patron         = Koha::Patrons->find($borrowernumber); # FIXME Is this really useful?
-    my $restricted  = $patron->is_debarred;
-    my $hasoverdues = $patron->has_overdues;
-
-    if ( $restricted and $restrictionblockrenewing ) {
-        return ( 0, 'restriction');
-    } elsif ( ($hasoverdues and $overduesblockrenewing eq 'block') || ($issue->is_overdue and $overduesblockrenewing eq 'blockitem') ) {
-        return ( 0, 'overdue');
-    }
-
-    if ( $issue->auto_renew ) {
-
-        if ( $patron->category->effective_BlockExpiredPatronOpacActions and $patron->is_expired ) {
-            return ( 0, 'auto_account_expired' );
-        }
-
-        if ( defined $issuing_rule->no_auto_renewal_after
-                and $issuing_rule->no_auto_renewal_after ne "" ) {
-            # Get issue_date and add no_auto_renewal_after
-            # If this is greater than today, it's too late for renewal.
-            my $maximum_renewal_date = dt_from_string($issue->issuedate, 'sql');
-            $maximum_renewal_date->add(
-                $issuing_rule->lengthunit => $issuing_rule->no_auto_renewal_after
-            );
-            my $now = dt_from_string;
-            if ( $now >= $maximum_renewal_date ) {
-                return ( 0, "auto_too_late" );
-            }
-        }
-        if ( defined $issuing_rule->no_auto_renewal_after_hard_limit
-                      and $issuing_rule->no_auto_renewal_after_hard_limit ne "" ) {
-            # If no_auto_renewal_after_hard_limit is >= today, it's also too late for renewal
-            if ( dt_from_string >= dt_from_string( $issuing_rule->no_auto_renewal_after_hard_limit ) ) {
-                return ( 0, "auto_too_late" );
-            }
-        }
-
-        if ( C4::Context->preference('OPACFineNoRenewalsBlockAutoRenew') ) {
-            my $fine_no_renewals = C4::Context->preference("OPACFineNoRenewals");
-            my $amountoutstanding =
-              C4::Context->preference("OPACFineNoRenewalsIncludeCredit")
-              ? $patron->account->balance
-              : $patron->account->outstanding_debits->total_outstanding;
-            if ( $amountoutstanding and $amountoutstanding > $fine_no_renewals ) {
-                return ( 0, "auto_too_much_oweing" );
-            }
-        }
-    }
-
-    if ( defined $issuing_rule->norenewalbefore
-        and $issuing_rule->norenewalbefore ne "" )
-    {
-
-        # Calculate soonest renewal by subtracting 'No renewal before' from due date
-        my $soonestrenewal = dt_from_string( $issue->date_due, 'sql' )->subtract(
-            $issuing_rule->lengthunit => $issuing_rule->norenewalbefore );
-
-        # Depending on syspref reset the exact time, only check the date
-        if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date'
-            and $issuing_rule->lengthunit eq 'days' )
-        {
-            $soonestrenewal->truncate( to => 'day' );
-        }
-
-        if ( $soonestrenewal > DateTime->now( time_zone => C4::Context->tz() ) )
-        {
-            return ( 0, "auto_too_soon" ) if $issue->auto_renew;
-            return ( 0, "too_soon" );
-        }
-        elsif ( $issue->auto_renew ) {
-            return ( 0, "auto_renew" );
-        }
-    }
-
-    # Fallback for automatic renewals:
-    # If norenewalbefore is undef, don't renew before due date.
-    if ( $issue->auto_renew ) {
-        my $now = dt_from_string;
-        return ( 0, "auto_renew" )
-          if $now >= dt_from_string( $issue->date_due, 'sql' );
-        return ( 0, "auto_too_soon" );
-    }
+    return ( 0, "auto_renew" ) if $auto_renew && !$override_limit; # 0 if auto-renewal should not succeed
 
     return ( 1, undef );
 }
@@ -2866,6 +2925,11 @@ C<$datedue> can be a DateTime object used to set the due date.
 C<$lastreneweddate> is an optional ISO-formatted date used to set issues.lastreneweddate.  If
 this parameter is not supplied, lastreneweddate is set to the current date.
 
+C<$skipfinecalc> is an optional boolean. There may be circumstances where, even if the
+CalculateFinesOnReturn syspref is enabled, we don't want to calculate fines upon renew,
+for example, when we're renewing as a result of a fine being paid (see RenewAccruingItemWhenPaid
+syspref)
+
 If C<$datedue> is the empty string, C<&AddRenewal> will calculate the due date automatically
 from the book's item type.
 
@@ -2876,7 +2940,8 @@ sub AddRenewal {
     my $itemnumber      = shift or return;
     my $branch          = shift;
     my $datedue         = shift;
-    my $lastreneweddate = shift || DateTime->now(time_zone => C4::Context->tz);
+    my $lastreneweddate = shift || dt_from_string();
+    my $skipfinecalc    = shift;
 
     my $item_object   = Koha::Items->find($itemnumber) or return;
     my $biblio = $item_object->biblio;
@@ -2902,7 +2967,7 @@ sub AddRenewal {
     my $schema = Koha::Database->schema;
     $schema->txn_do(sub{
 
-        if ( C4::Context->preference('CalculateFinesOnReturn') ) {
+        if ( !$skipfinecalc && C4::Context->preference('CalculateFinesOnReturn') ) {
             _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed } );
         }
         _FixOverduesOnReturn( $borrowernumber, $itemnumber, undef, 'RENEWED' );
@@ -2915,7 +2980,7 @@ sub AddRenewal {
 
             $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?
                                             dt_from_string( $issue->date_due, 'sql' ) :
-                                            DateTime->now( time_zone => C4::Context->tz());
+                                            dt_from_string();
             $datedue =  CalcDateDue($datedue, $itemtype, $circ_library->branchcode, $patron_unblessed, 'is a renewal');
         }
 
@@ -2941,7 +3006,9 @@ sub AddRenewal {
 
         # Update the renewal count on the item, and tell zebra to reindex
         $renews = ( $item_object->renewals || 0 ) + 1;
-        ModItem( { renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item_object->biblionumber, $itemnumber, { log_action => 0 } );
+        $item_object->renewals($renews);
+        $item_object->onloan($datedue);
+        $item_object->store({ log_action => 0 });
 
         # Charge a new rental fee, if applicable
         my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
@@ -2989,14 +3056,10 @@ sub AddRenewal {
             DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
         }
 
-        unless ( C4::Context->interface eq 'opac' ) { #if from opac we are obeying OpacRenewalBranch as calculated in opac-renew.pl
-            $branch = ( C4::Context->userenv && defined C4::Context->userenv->{branch} ) ? C4::Context->userenv->{branch} : $branch;
-        }
-
         # Add the renewal to stats
         UpdateStats(
             {
-                branch         => $branch,
+                branch         => $item_object->renewal_branchcode({branch => $branch}),
                 type           => 'renew',
                 amount         => $charge,
                 itemnumber     => $itemnumber,
@@ -3042,14 +3105,16 @@ sub GetRenewCount {
     # $item and $borrower should be calculated
     my $branchcode = _GetCircControlBranch($item->unblessed, $patron->unblessed);
 
-    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
-        {   categorycode => $patron->categorycode,
+    my $rule = Koha::CirculationRules->get_effective_rule(
+        {
+            categorycode => $patron->categorycode,
             itemtype     => $item->effective_itemtype,
-            branchcode   => $branchcode
+            branchcode   => $branchcode,
+            rule_name    => 'renewalsallowed',
         }
     );
 
-    $renewsallowed = $issuing_rule ? $issuing_rule->renewalsallowed : 0;
+    $renewsallowed = $rule ? $rule->rule_value : 0;
     $renewsleft    = $renewsallowed - $renewcount;
     if($renewsleft < 0){ $renewsleft = 0; }
     return ( $renewcount, $renewsallowed, $renewsleft );
@@ -3087,25 +3152,29 @@ sub GetSoonestRenewDate {
       or return;
 
     my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed );
-    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
+    my $issuing_rule = Koha::CirculationRules->get_effective_rules(
         {   categorycode => $patron->categorycode,
             itemtype     => $item->effective_itemtype,
-            branchcode   => $branchcode
+            branchcode   => $branchcode,
+            rules => [
+                'norenewalbefore',
+                'lengthunit',
+            ]
         }
     );
 
     my $now = dt_from_string;
     return $now unless $issuing_rule;
 
-    if ( defined $issuing_rule->norenewalbefore
-        and $issuing_rule->norenewalbefore ne "" )
+    if ( defined $issuing_rule->{norenewalbefore}
+        and $issuing_rule->{norenewalbefore} ne "" )
     {
         my $soonestrenewal =
           dt_from_string( $itemissue->date_due )->subtract(
-            $issuing_rule->lengthunit => $issuing_rule->norenewalbefore );
+            $issuing_rule->{lengthunit} => $issuing_rule->{norenewalbefore} );
 
         if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date'
-            and $issuing_rule->lengthunit eq 'days' )
+            and $issuing_rule->{lengthunit} eq 'days' )
         {
             $soonestrenewal->truncate( to => 'day' );
         }
@@ -3146,30 +3215,36 @@ sub GetLatestAutoRenewDate {
       or return;
 
     my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed );
-    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
-        {   categorycode => $patron->categorycode,
+    my $circulation_rules = Koha::CirculationRules->get_effective_rules(
+        {
+            categorycode => $patron->categorycode,
             itemtype     => $item->effective_itemtype,
-            branchcode   => $branchcode
+            branchcode   => $branchcode,
+            rules => [
+                'no_auto_renewal_after',
+                'no_auto_renewal_after_hard_limit',
+                'lengthunit',
+            ]
         }
     );
 
-    return unless $issuing_rule;
+    return unless $circulation_rules;
     return
-      if ( not $issuing_rule->no_auto_renewal_after
-            or $issuing_rule->no_auto_renewal_after eq '' )
-      and ( not $issuing_rule->no_auto_renewal_after_hard_limit
-             or $issuing_rule->no_auto_renewal_after_hard_limit eq '' );
+      if ( not $circulation_rules->{no_auto_renewal_after}
+            or $circulation_rules->{no_auto_renewal_after} eq '' )
+      and ( not $circulation_rules->{no_auto_renewal_after_hard_limit}
+             or $circulation_rules->{no_auto_renewal_after_hard_limit} eq '' );
 
     my $maximum_renewal_date;
-    if ( $issuing_rule->no_auto_renewal_after ) {
+    if ( $circulation_rules->{no_auto_renewal_after} ) {
         $maximum_renewal_date = dt_from_string($itemissue->issuedate);
         $maximum_renewal_date->add(
-            $issuing_rule->lengthunit => $issuing_rule->no_auto_renewal_after
+            $circulation_rules->{lengthunit} => $circulation_rules->{no_auto_renewal_after}
         );
     }
 
-    if ( $issuing_rule->no_auto_renewal_after_hard_limit ) {
-        my $dt = dt_from_string( $issuing_rule->no_auto_renewal_after_hard_limit );
+    if ( $circulation_rules->{no_auto_renewal_after_hard_limit} ) {
+        my $dt = dt_from_string( $circulation_rules->{no_auto_renewal_after_hard_limit} );
         $maximum_renewal_date = $dt if not $maximum_renewal_date or $maximum_renewal_date > $dt;
     }
     return $maximum_renewal_date;
@@ -3216,19 +3291,10 @@ sub GetIssuingCharges {
         $item_type = $item_data->{itemtype};
         $charge    = $item_data->{rentalcharge};
         my $branch = C4::Context::mybranch();
-        my $discount_query = q|SELECT rentaldiscount,
-            issuingrules.itemtype, issuingrules.branchcode
-            FROM borrowers
-            LEFT JOIN issuingrules ON borrowers.categorycode = issuingrules.categorycode
-            WHERE borrowers.borrowernumber = ?
-            AND (issuingrules.itemtype = ? OR issuingrules.itemtype = '*')
-            AND (issuingrules.branchcode = ? OR issuingrules.branchcode = '*')|;
-        my $discount_sth = $dbh->prepare($discount_query);
-        $discount_sth->execute( $borrowernumber, $item_type, $branch );
-        my $discount_rules = $discount_sth->fetchall_arrayref({});
-        if (@{$discount_rules}) {
+        my $patron = Koha::Patrons->find( $borrowernumber );
+        my $discount = _get_discount_from_rule($patron->categorycode, $branch, $item_type);
+        if ($discount) {
             # We may have multiple rules so get the most specific
-            my $discount = _get_discount_from_rule($discount_rules, $branch, $item_type);
             $charge = ( $charge * ( 100 - $discount ) ) / 100;
         }
         if ($charge) {
@@ -3241,37 +3307,43 @@ sub GetIssuingCharges {
 
 # Select most appropriate discount rule from those returned
 sub _get_discount_from_rule {
-    my ($rules_ref, $branch, $itemtype) = @_;
-    my $discount;
+    my ($categorycode, $branchcode, $itemtype) = @_;
 
-    if (@{$rules_ref} == 1) { # only 1 applicable rule use it
-        $discount = $rules_ref->[0]->{rentaldiscount};
-        return (defined $discount) ? $discount : 0;
-    }
-    # could have up to 4 does one match $branch and $itemtype
-    my @d = grep { $_->{branchcode} eq $branch && $_->{itemtype} eq $itemtype } @{$rules_ref};
-    if (@d) {
-        $discount = $d[0]->{rentaldiscount};
-        return (defined $discount) ? $discount : 0;
-    }
-    # do we have item type + all branches
-    @d = grep { $_->{branchcode} eq q{*} && $_->{itemtype} eq $itemtype } @{$rules_ref};
-    if (@d) {
-        $discount = $d[0]->{rentaldiscount};
-        return (defined $discount) ? $discount : 0;
-    }
-    # do we all item types + this branch
-    @d = grep { $_->{branchcode} eq $branch && $_->{itemtype} eq q{*} } @{$rules_ref};
-    if (@d) {
-        $discount = $d[0]->{rentaldiscount};
-        return (defined $discount) ? $discount : 0;
-    }
-    # so all and all (surely we wont get here)
-    @d = grep { $_->{branchcode} eq q{*} && $_->{itemtype} eq q{*} } @{$rules_ref};
-    if (@d) {
-        $discount = $d[0]->{rentaldiscount};
-        return (defined $discount) ? $discount : 0;
+    # 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;
 }
@@ -3434,7 +3506,7 @@ sub SendCirculationAlert {
     # LOCK TABLES is not transaction-safe and implicitly commits any active transaction before attempting to lock the tables.
     # If the LOCK/UNLOCK statements are executed from tests, the current transaction will be committed.
     # To avoid that we need to guess if this code is execute from tests or not (yes it is a bit hacky)
-    my $do_not_lock = ( exists $ENV{_} && $ENV{_} =~ m|prove| ) || $ENV{KOHA_NO_TABLE_LOCKS};
+    my $do_not_lock = ( exists $ENV{_} && $ENV{_} =~ m|prove| ) || $ENV{KOHA_TESTING};
 
     for my $mtt (@transports) {
         my $letter =  C4::Letters::GetPreparedLetter (
@@ -3493,20 +3565,7 @@ sub updateWrongTransfer {
        ModItemTransfer($itemNumber, $FromLibrary, $waitingAtLibrary);
 
 #third step changing holdingbranch of item
-       UpdateHoldingbranch($FromLibrary,$itemNumber);
-}
-
-=head2 UpdateHoldingbranch
-
-  $items = UpdateHoldingbranch($branch,$itmenumber);
-
-Simple methode for updating hodlingbranch in items BDD line
-
-=cut
-
-sub UpdateHoldingbranch {
-       my ( $branch,$itemnumber ) = @_;
-    ModItem({ holdingbranch => $branch }, undef, $itemnumber);
+    my $item = Koha::Items->find($itemNumber)->holdingbranch($FromLibrary)->store;
 }
 
 =head2 CalcDateDue
@@ -3544,9 +3603,7 @@ sub CalcDateDue {
             $datedue = $startdate->clone;
         }
     } else {
-        $datedue =
-          DateTime->now( time_zone => C4::Context->tz() )
-          ->truncate( to => 'minute' );
+        $datedue = dt_from_string()->truncate( to => 'minute' );
     }
 
 
@@ -3745,7 +3802,16 @@ sub LostItem{
         defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $itemnumber...) failed!";  # zero is OK, check defined
 
         if (C4::Context->preference('WhenLostChargeReplacementFee')){
-            C4::Accounts::chargelostitem($borrowernumber, $itemnumber, $issues->{'replacementprice'}, "$issues->{'title'} $issues->{'barcode'} $issues->{'itemcallnumber'}");
+            C4::Accounts::chargelostitem(
+                $borrowernumber,
+                $itemnumber,
+                $issues->{'replacementprice'},
+                sprintf( "%s %s %s",
+                    $issues->{'title'}          || q{},
+                    $issues->{'barcode'}        || q{},
+                    $issues->{'itemcallnumber'} || q{},
+                ),
+            );
             #FIXME : Should probably have a way to distinguish this from an item that really was returned.
             #warn " $issues->{'borrowernumber'}  /  $itemnumber ";
         }
@@ -3755,7 +3821,7 @@ sub LostItem{
 
     #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)) {
-        ModItem({holdingbranch => $frombranch}, undef, $itemnumber);
+        Koha::Items->find($itemnumber)->holdingbranch($frombranch)->store;
     }
     my $transferdeleted = DeleteTransfer($itemnumber);
 }
@@ -3818,17 +3884,16 @@ sub ProcessOfflineReturn {
         my $itemnumber = $item->itemnumber;
         my $issue = GetOpenIssue( $itemnumber );
         if ( $issue ) {
+            my $leave_item_lost = C4::Context->preference("BlockReturnOfLostItems") ? 1 : 0;
+            ModDateLastSeen( $itemnumber, $leave_item_lost );
             MarkIssueReturned(
                 $issue->{borrowernumber},
                 $itemnumber,
                 $operation->{timestamp},
             );
-            ModItem(
-                { renewals => 0, onloan => undef },
-                $issue->{'biblionumber'},
-                $itemnumber,
-                { log_action => 0 }
-            );
+            $item->renewals(0);
+            $item->onloan(undef);
+            $item->store({ log_action => 0 });
             return "Success.";
         } else {
             return "Item not issued.";
@@ -4024,7 +4089,7 @@ sub GetAgeRestriction {
             }
 
             #Get how many days the borrower has to reach the age restriction
-            my @Today = split /-/, DateTime->today->ymd();
+            my @Today = split /-/, dt_from_string()->ymd();
             my $daysToAgeRestriction = Date_to_Days(@alloweddate) - Date_to_Days(@Today);
             #Negative days means the borrower went past the age restriction age
             return ($restriction_year, $daysToAgeRestriction);