Bug 10529: Remove hardcoded dollar from patron message
[koha.git] / C4 / Circulation.pm
index 48b7b4a..30cbe2f 100644 (file)
@@ -36,7 +36,11 @@ use C4::Message;
 use C4::Debug;
 use C4::Branch; # GetBranches
 use C4::Log; # logaction
-use C4::Koha qw(GetAuthorisedValueByCode);
+use C4::Koha qw(
+    GetAuthorisedValueByCode
+    GetAuthValCode
+    GetKohaAuthorisedValueLib
+);
 use C4::Overdues qw(CalcFine UpdateFine);
 use Algorithm::CheckDigits;
 
@@ -83,6 +87,7 @@ BEGIN {
                &GetBiblioIssues
                &GetOpenIssue
                &AnonymiseIssueHistory
+        &CheckIfIssuedToPatron
        );
 
        # subs to deal with returns
@@ -732,7 +737,7 @@ sub CanBookBeIssued {
     #
     if ( $borrower->{'category_type'} eq 'X' && (  $item->{barcode}  )) { 
        # stats only borrower -- add entry to statistics table, and return issuingimpossible{STATS} = 1  .
-        &UpdateStats(C4::Context->userenv->{'branch'},'localuse','','',$item->{'itemnumber'},$item->{'itemtype'},$borrower->{'borrowernumber'});
+        &UpdateStats(C4::Context->userenv->{'branch'},'localuse','','',$item->{'itemnumber'},$item->{'itemtype'},$borrower->{'borrowernumber'}, undef, $item->{'ccode'});
         ModDateLastSeen( $item->{'itemnumber'} );
         return( { STATS => 1 }, {});
     }
@@ -771,29 +776,32 @@ sub CanBookBeIssued {
     #
 
     # DEBTS
-    my ($amount) =
-      C4::Members::GetMemberAccountRecords( $borrower->{'borrowernumber'}, '' && $duedate->ymd() );
+    my ($balance, $non_issue_charges, $other_charges) =
+      C4::Members::GetMemberAccountBalance( $borrower->{'borrowernumber'} );
     my $amountlimit = C4::Context->preference("noissuescharge");
     my $allowfineoverride = C4::Context->preference("AllowFineOverride");
     my $allfinesneedoverride = C4::Context->preference("AllFinesNeedOverride");
     if ( C4::Context->preference("IssuingInProcess") ) {
-        if ( $amount > $amountlimit && !$inprocess && !$allowfineoverride) {
-            $issuingimpossible{DEBT} = sprintf( "%.2f", $amount );
-        } elsif ( $amount > $amountlimit && !$inprocess && $allowfineoverride) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $amount );
-        } elsif ( $allfinesneedoverride && $amount > 0 && $amount <= $amountlimit && !$inprocess ) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $amount );
+        if ( $non_issue_charges > $amountlimit && !$inprocess && !$allowfineoverride) {
+            $issuingimpossible{DEBT} = sprintf( "%.2f", $non_issue_charges );
+        } elsif ( $non_issue_charges > $amountlimit && !$inprocess && $allowfineoverride) {
+            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_charges );
+        } elsif ( $allfinesneedoverride && $non_issue_charges > 0 && $non_issue_charges <= $amountlimit && !$inprocess ) {
+            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_charges );
         }
     }
     else {
-        if ( $amount > $amountlimit && $allowfineoverride ) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $amount );
-        } elsif ( $amount > $amountlimit && !$allowfineoverride) {
-            $issuingimpossible{DEBT} = sprintf( "%.2f", $amount );
-        } elsif ( $amount > 0 && $allfinesneedoverride ) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $amount );
+        if ( $non_issue_charges > $amountlimit && $allowfineoverride ) {
+            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_charges );
+        } elsif ( $non_issue_charges > $amountlimit && !$allowfineoverride) {
+            $issuingimpossible{DEBT} = sprintf( "%.2f", $non_issue_charges );
+        } elsif ( $non_issue_charges > 0 && $allfinesneedoverride ) {
+            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_charges );
         }
     }
+    if ($balance > 0 && $other_charges > 0) {
+        $alerts{OTHER_CHARGES} = sprintf( "%.2f", $other_charges );
+    }
 
     my ($blocktype, $count) = C4::Members::IsMemberBlocked($borrower->{'borrowernumber'});
     if ($blocktype == -1) {
@@ -827,16 +835,17 @@ sub CanBookBeIssued {
     #
     # ITEM CHECKING
     #
-    if (   $item->{'notforloan'}
-        && $item->{'notforloan'} > 0 )
+    if ( $item->{'notforloan'} )
     {
         if(!C4::Context->preference("AllowNotForLoanOverride")){
             $issuingimpossible{NOT_FOR_LOAN} = 1;
+            $issuingimpossible{item_notforloan} = $item->{'notforloan'};
         }else{
             $needsconfirmation{NOT_FOR_LOAN_FORCING} = 1;
+            $needsconfirmation{item_notforloan} = $item->{'notforloan'};
         }
     }
-    elsif ( !$item->{'notforloan'} ){
+    els{
         # we have to check itemtypes.notforloan also
         if (C4::Context->preference('item-level_itypes')){
             # this should probably be a subroutine
@@ -847,16 +856,20 @@ sub CanBookBeIssued {
             if ($notforloan->{'notforloan'}) {
                 if (!C4::Context->preference("AllowNotForLoanOverride")) {
                     $issuingimpossible{NOT_FOR_LOAN} = 1;
+                    $issuingimpossible{itemtype_notforloan} = $item->{'itype'};
                 } else {
                     $needsconfirmation{NOT_FOR_LOAN_FORCING} = 1;
+                    $needsconfirmation{itemtype_notforloan} = $item->{'itype'};
                 }
             }
         }
         elsif ($biblioitem->{'notforloan'} == 1){
             if (!C4::Context->preference("AllowNotForLoanOverride")) {
                 $issuingimpossible{NOT_FOR_LOAN} = 1;
+                $issuingimpossible{itemtype_notforloan} = $biblioitem->{'itemtype'};
             } else {
                 $needsconfirmation{NOT_FOR_LOAN_FORCING} = 1;
+                $needsconfirmation{itemtype_notforloan} = $biblioitem->{'itemtype'};
             }
         }
     }
@@ -874,7 +887,7 @@ sub CanBookBeIssued {
         $needsconfirmation{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'confirm' );
         $alerts{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'alert' );
     }
-    if ( C4::Context->preference("IndependantBranches") ) {
+    if ( C4::Context->preference("IndependentBranches") ) {
         my $userenv = C4::Context->userenv;
         if ( ($userenv) && ( $userenv->{flags} % 2 != 1 ) ) {
             $issuingimpossible{ITEMNOTSAMEBRANCH} = 1
@@ -982,16 +995,16 @@ sub CanBookBeIssued {
         # Index points to the next value
         my $restrictionyear = 0;
         if (($take <= $#values) && ($take >= 0)){
-            $restrictionyear += @values[$take];
+            $restrictionyear += $values[$take];
         }
 
         if ($restrictionyear > 0) {
             if ( $borrower->{'dateofbirth'}  ) {
                 my @alloweddate =  split /-/,$borrower->{'dateofbirth'} ;
-                @alloweddate[0] += $restrictionyear;
+                $alloweddate[0] += $restrictionyear;
                 #Prevent runime eror on leap year (invalid date)
-                if ((@alloweddate[1] == 2) && (@alloweddate[2] == 29)) {
-                    @alloweddate[2] = 28;
+                if (($alloweddate[1] == 2) && ($alloweddate[2] == 29)) {
+                    $alloweddate[2] = 28;
                 }
 
                 if ( Date_to_Days(Today) <  Date_to_Days(@alloweddate) -1  ) {
@@ -1005,9 +1018,118 @@ sub CanBookBeIssued {
             }
         }
     }
+
+## check for high holds decreasing loan period
+    my $decrease_loan = C4::Context->preference('decreaseLoanHighHolds');
+    if ( $decrease_loan && $decrease_loan == 1 ) {
+        my ( $reserved, $num, $duration, $returndate ) =
+          checkHighHolds( $item, $borrower );
+
+        if ( $num >= C4::Context->preference('decreaseLoanHighHoldsValue') ) {
+            $needsconfirmation{HIGHHOLDS} = {
+                num_holds  => $num,
+                duration   => $duration,
+                returndate => output_pref($returndate),
+            };
+        }
+    }
+
     return ( \%issuingimpossible, \%needsconfirmation, \%alerts );
 }
 
+=head2 CanBookBeReturned
+
+  ($returnallowed, $message) = CanBookBeReturned($item, $branch)
+
+Check whether the item can be returned to the provided branch
+
+=over 4
+
+=item C<$item> is a hash of item information as returned from GetItem
+
+=item C<$branch> is the branchcode where the return is taking place
+
+=back
+
+Returns:
+
+=over 4
+
+=item C<$returnallowed> is 0 or 1, corresponding to whether the return is allowed (1) or not (0)
+
+=item C<$message> is the branchcode where the item SHOULD be returned, if the return is not allowed
+
+=back
+
+=cut
+
+sub CanBookBeReturned {
+  my ($item, $branch) = @_;
+  my $allowreturntobranch = C4::Context->preference("AllowReturnToBranch") || 'anywhere';
+
+  # assume return is allowed to start
+  my $allowed = 1;
+  my $message;
+
+  # identify all cases where return is forbidden
+  if ($allowreturntobranch eq 'homebranch' && $branch ne $item->{'homebranch'}) {
+     $allowed = 0;
+     $message = $item->{'homebranch'};
+  } elsif ($allowreturntobranch eq 'holdingbranch' && $branch ne $item->{'holdingbranch'}) {
+     $allowed = 0;
+     $message = $item->{'holdingbranch'};
+  } elsif ($allowreturntobranch eq 'homeorholdingbranch' && $branch ne $item->{'homebranch'} && $branch ne $item->{'holdingbranch'}) {
+     $allowed = 0;
+     $message = $item->{'homebranch'}; # FIXME: choice of homebranch is arbitrary
+  }
+
+  return ($allowed, $message);
+}
+
+=head2 CheckHighHolds
+
+    used when syspref decreaseLoanHighHolds is active. Returns 1 or 0 to define whether the minimum value held in
+    decreaseLoanHighHoldsValue is exceeded, the total number of outstanding holds, the number of days the loan
+    has been decreased to (held in syspref decreaseLoanHighHoldsValue), and the new due date
+
+=cut
+
+sub checkHighHolds {
+    my ( $item, $borrower ) = @_;
+    my $biblio = GetBiblioFromItemNumber( $item->{itemnumber} );
+    my $branch = _GetCircControlBranch( $item, $borrower );
+    my $dbh    = C4::Context->dbh;
+    my $sth    = $dbh->prepare(
+'select count(borrowernumber) as num_holds from reserves where biblionumber=?'
+    );
+    $sth->execute( $item->{'biblionumber'} );
+    my ($holds) = $sth->fetchrow_array;
+    if ($holds) {
+        my $issuedate = DateTime->now( time_zone => C4::Context->tz() );
+
+        my $calendar = Koha::Calendar->new( branchcode => $branch );
+
+        my $itype =
+          ( C4::Context->preference('item-level_itypes') )
+          ? $biblio->{'itype'}
+          : $biblio->{'itemtype'};
+        my $orig_due =
+          C4::Circulation::CalcDateDue( $issuedate, $itype, $branch,
+            $borrower );
+
+        my $reduced_datedue =
+          $calendar->addDate( $issuedate,
+            C4::Context->preference('decreaseLoanHighHoldsDuration') );
+
+        if ( DateTime->compare( $reduced_datedue, $orig_due ) == -1 ) {
+            return ( 1, $holds,
+                C4::Context->preference('decreaseLoanHighHoldsDuration'),
+                $reduced_datedue );
+        }
+    }
+    return ( 0, 0, 0, undef );
+}
+
 =head2 AddIssue
 
   &AddIssue($borrower, $barcode, [$datedue], [$cancelreserve], [$issuedate])
@@ -1065,7 +1187,7 @@ sub AddIssue {
     }
        if ($borrower and $barcode and $barcodecheck ne '0'){#??? wtf
                # find which item we issue
-               my $item = GetItem('', $barcode) or return undef;       # if we don't get an Item, abort.
+               my $item = GetItem('', $barcode) or return;     # if we don't get an Item, abort.
                my $branch = _GetCircControlBranch($item,$borrower);
                
                # get actual issuing if there is one
@@ -1143,7 +1265,9 @@ sub AddIssue {
 
         ## If item was lost, it has now been found, reverse any list item charges if neccessary.
         if ( $item->{'itemlost'} ) {
-            _FixAccountForLostAndReturned( $item->{'itemnumber'}, undef, $item->{'barcode'} );
+            if ( C4::Context->preference('RefundLostItemFeeOnReturn' ) ) {
+                _FixAccountForLostAndReturned( $item->{'itemnumber'}, undef, $item->{'barcode'} );
+            }
         }
 
         ModItem({ issues           => $item->{'issues'},
@@ -1172,7 +1296,7 @@ sub AddIssue {
             C4::Context->userenv->{'branch'},
             'issue', $charge,
             ($sipmode ? "SIP-$sipmode" : ''), $item->{'itemnumber'},
-            $item->{'itype'}, $borrower->{'borrowernumber'}
+            $item->{'itype'}, $borrower->{'borrowernumber'}, undef, $item->{'ccode'}
         );
 
         # Send a checkout slip.
@@ -1193,7 +1317,7 @@ sub AddIssue {
         }
     }
 
-    logaction("CIRCULATION", "ISSUE", $borrower->{'borrowernumber'}, $biblio->{'biblionumber'})
+    logaction("CIRCULATION", "ISSUE", $borrower->{'borrowernumber'}, $biblio->{'itemnumber'})
         if C4::Context->preference("IssueLog");
   }
   return ($datedue);   # not necessarily the same as when it came in!
@@ -1210,15 +1334,20 @@ Get loan length for an itemtype, a borrower type and a branch
 sub GetLoanLength {
     my ( $borrowertype, $itemtype, $branchcode ) = @_;
     my $dbh = C4::Context->dbh;
-    my $sth =
-      $dbh->prepare(
-'select issuelength, lengthunit from issuingrules where categorycode=? and itemtype=? and branchcode=? and issuelength is not null'
-      );
-# warn "in get loan lenght $borrowertype $itemtype $branchcode ";
-# try to find issuelength & return the 1st available.
-# check with borrowertype, itemtype and branchcode, then without one of those parameters
+    my $sth = $dbh->prepare(qq{
+        SELECT issuelength, lengthunit, renewalperiod
+        FROM issuingrules
+        WHERE   categorycode=?
+            AND itemtype=?
+            AND branchcode=?
+            AND issuelength IS NOT NULL
+    });
+
+    # 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) && $loanlength->{issuelength};
 
@@ -1260,6 +1389,7 @@ sub GetLoanLength {
     # if no rule is set => 21 days (hardcoded)
     return {
         issuelength => 21,
+        renewalperiod => 21,
         lengthunit => 'days',
     };
 
@@ -1294,7 +1424,7 @@ sub GetHardDueDate {
 
 FIXME - This is a copy-paste of GetLoanLength
 as a stop-gap.  Do not wish to change API for GetLoanLength 
-this close to release, however, Overdues::GetIssuingRules is broken.
+this close to release.
 
 Get the issuing rule for an itemtype, a borrower type and a branch
 Returns a hashref from the issuingrules table.
@@ -1340,7 +1470,7 @@ sub GetIssuingRule {
     return $irule if defined($irule) ;
 
     # if no rule matches,
-    return undef;
+    return;
 }
 
 =head2 GetBranchBorrowerCircRule
@@ -1623,24 +1753,20 @@ sub AddReturn {
         $branches->{$hbr}->{PE} and $messages->{'IsPermanent'} = $hbr;
     }
 
-    # if indy branches and returning to different branch, refuse the return unless canreservefromotherbranches is turned on
-    if ($hbr ne $branch && C4::Context->preference("IndependantBranches") && !(C4::Context->preference("canreservefromotherbranches"))){
+    # check if the return is allowed at this branch
+    my ($returnallowed, $message) = CanBookBeReturned($item, $branch);
+    unless ($returnallowed){
         $messages->{'Wrongbranch'} = {
             Wrongbranch => $branch,
-            Rightbranch => $hbr,
+            Rightbranch => $message
         };
         $doreturn = 0;
-        # bailing out here - in this case, current desired behavior
-        # is to act as if no return ever happened at all.
-        # FIXME - even in an indy branches situation, there should
-        # still be an option for the library to accept the item
-        # and transfer it to its owning library.
         return ( $doreturn, $messages, $issue, $borrower );
     }
 
     if ( $item->{'wthdrawn'} ) { # book has been cancelled
         $messages->{'wthdrawn'} = 1;
-        $doreturn = 0;
+        $doreturn = 0 if C4::Context->preference("BlockReturnOfWithdrawnItems");
     }
 
     # case of a return of document (deal with issues and holdingbranch)
@@ -1659,19 +1785,36 @@ sub AddReturn {
         }
 
         if ($borrowernumber) {
-        if($issue->{'overdue'}){
-                my ( $amount, $type, $unitcounttotal ) = C4::Overdues::CalcFine( $item, $borrower->{categorycode},$branch, $datedue, $today );
+            if( C4::Context->preference('CalculateFinesOnReturn') && $issue->{'overdue'}){
+            # we only need to calculate and change the fines if we want to do that on return
+            # Should be on for hourly loans
+                my $control = C4::Context->preference('CircControl');
+                my $control_branchcode =
+                    ( $control eq 'ItemHomeLibrary' ) ? $item->{homebranch}
+                  : ( $control eq 'PatronLibrary' )   ? $borrower->{branchcode}
+                  :                                     $issue->{branchcode};
+
+                my ( $amount, $type, $unitcounttotal ) =
+                  C4::Overdues::CalcFine( $item, $borrower->{categorycode},
+                    $control_branchcode, $datedue, $today );
+
                 $type ||= q{};
-        if ( $amount > 0 && ( C4::Context->preference('finesMode') eq 'production' )) {
-          C4::Overdues::UpdateFine(
-              $issue->{itemnumber},
-              $issue->{borrowernumber},
-                      $amount, $type, output_pref($datedue)
-              );
-        }
+
+                if ( $amount > 0
+                    && C4::Context->preference('finesMode') eq 'production' )
+                {
+                    C4::Overdues::UpdateFine( $issue->{itemnumber},
+                        $issue->{borrowernumber},
+                        $amount, $type, output_pref($datedue) );
+                }
             }
-            MarkIssueReturned($borrowernumber, $item->{'itemnumber'}, $circControlBranch, '', $borrower->{'privacy'});
-            $messages->{'WasReturned'} = 1;    # FIXME is the "= 1" right?  This could be the borrower hash.
+
+            MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
+                $circControlBranch, '', $borrower->{'privacy'} );
+
+            # FIXME is the "= 1" right?  This could be the borrower hash.
+            $messages->{'WasReturned'} = 1;
+
         }
 
         ModItem({ onloan => undef }, $issue->{'biblionumber'}, $item->{'itemnumber'});
@@ -1708,9 +1851,13 @@ sub AddReturn {
     }
 
     # fix up the accounts.....
-    if ($item->{'itemlost'}) {
-        _FixAccountForLostAndReturned($item->{'itemnumber'}, $borrowernumber, $barcode);    # can tolerate undef $borrowernumber
+    if ( $item->{'itemlost'} ) {
         $messages->{'WasLost'} = 1;
+
+        if ( C4::Context->preference('RefundLostItemFeeOnReturn' ) ) {
+            _FixAccountForLostAndReturned($item->{'itemnumber'}, $borrowernumber, $barcode);    # can tolerate undef $borrowernumber
+            $messages->{'LostItemFeeRefunded'} = 1;
+        }
     }
 
     # fix up the overdues in accounts...
@@ -1728,7 +1875,8 @@ sub AddReturn {
 
     # find reserves.....
     # if we don't have a reserve with the status W, we launch the Checkreserves routine
-    my ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
+    my ($resfound, $resrec);
+    ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->{'itemnumber'} ) unless ( $item->{'wthdrawn'} );
     if ($resfound) {
           $resrec->{'ResFound'} = $resfound;
         $messages->{'ResFound'} = $resrec;
@@ -1740,7 +1888,7 @@ sub AddReturn {
         $branch, $stat_type, '0', '',
         $item->{'itemnumber'},
         $biblio->{'itemtype'},
-        $borrowernumber
+        $borrowernumber, undef, $item->{'ccode'}
     );
 
     # Send a check-in slip. # NOTE: borrower may be undef.  probably shouldn't try to send messages then.
@@ -1760,7 +1908,7 @@ sub AddReturn {
         });
     }
     
-    logaction("CIRCULATION", "RETURN", $borrowernumber, $item->{'biblionumber'})
+    logaction("CIRCULATION", "RETURN", $borrowernumber, $item->{'itemnumber'})
         if C4::Context->preference("ReturnLog");
     
     # FIXME: make this comment intelligible.
@@ -1836,6 +1984,7 @@ sub MarkIssueReturned {
     if ( $privacy == 2) {
         # The default of 0 does not work due to foreign key constraints
         # The anonymisation will fail quietly if AnonymousPatron is not a valid entry
+        # FIXME the above is unacceptable - bug 9942 relates
         my $anonymouspatron = (C4::Context->preference('AnonymousPatron')) ? C4::Context->preference('AnonymousPatron') : 0;
         my $sth_ano = $dbh->prepare("UPDATE old_issues SET borrowernumber=?
                                   WHERE borrowernumber = ?
@@ -1876,7 +2025,7 @@ sub _debar_user_on_return {
     # $deltadays is a DateTime::Duration object
     my $deltadays = $calendar->days_between( $dt_due, $dt_today );
 
-    my $circcontrol = C4::Context::preference('CircControl');
+    my $circcontrol = C4::Context->preference('CircControl');
     my $issuingrule =
       GetIssuingRule( $borrower->{categorycode}, $item->{itype}, $branchcode );
     my $finedays = $issuingrule->{finedays};
@@ -1953,7 +2102,7 @@ sub _FixOverduesOnReturn {
     return 0 unless $data;    # no warning, there's just nothing to fix
 
     my $uquery;
-    my @bind = ($borrowernumber, $item, $data->{'accountno'});
+    my @bind = ($data->{'accountlines_id'});
     if ($exemptfine) {
         $uquery = "update accountlines set accounttype='FFOR', amountoutstanding=0";
         if (C4::Context->preference("FinesLog")) {
@@ -1973,7 +2122,7 @@ sub _FixOverduesOnReturn {
     } else {
         $uquery = "update accountlines set accounttype='F' ";
     }
-    $uquery .= " where (borrowernumber = ?) and (itemnumber = ?) and (accountno = ?)";
+    $uquery .= " where (accountlines_id = ?)";
     my $usth = $dbh->prepare($uquery);
     return $usth->execute(@bind);
 }
@@ -2016,9 +2165,8 @@ sub _FixAccountForLostAndReturned {
         $amountleft = $data->{'amountoutstanding'} - $amount;   # Um, isn't this the same as ZERO?  We just tested those two things are ==
     }
     my $usth = $dbh->prepare("UPDATE accountlines SET accounttype = 'LR',amountoutstanding='0'
-        WHERE (borrowernumber = ?)
-        AND (itemnumber = ?) AND (accountno = ?) ");
-    $usth->execute($data->{'borrowernumber'},$itemnumber,$acctno);      # We might be adjusting an account for some OTHER borrowernumber now.  Not the one we passed in.  
+        WHERE (accountlines_id = ?)");
+    $usth->execute($data->{'accountlines_id'});      # We might be adjusting an account for some OTHER borrowernumber now.  Not the one we passed in.
     #check if any credit is left if so writeoff other accounts
     my $nextaccntno = getnextacctno($data->{'borrowernumber'});
     $amountleft *= -1 if ($amountleft < 0);
@@ -2037,12 +2185,11 @@ sub _FixAccountForLostAndReturned {
                 $newamtos = $accdata->{'amountoutstanding'} - $amountleft;
                 $amountleft = 0;
             }
-            my $thisacct = $accdata->{'accountno'};
+            my $thisacct = $accdata->{'accountlines_id'};
             # FIXME: move prepares outside while loop!
             my $usth = $dbh->prepare("UPDATE accountlines SET amountoutstanding= ?
-                    WHERE (borrowernumber = ?)
-                    AND (accountno=?)");
-            $usth->execute($newamtos,$data->{'borrowernumber'},'$thisacct');    # FIXME: '$thisacct' is a string literal!
+                    WHERE (accountlines_id = ?)");
+            $usth->execute($newamtos,$thisacct);
             $usth = $dbh->prepare("INSERT INTO accountoffsets
                 (borrowernumber, accountno, offsetaccount,  offsetamount)
                 VALUES
@@ -2126,7 +2273,7 @@ sub GetItemIssue {
     my ($itemnumber) = @_;
     return unless $itemnumber;
     my $sth = C4::Context->dbh->prepare(
-        "SELECT *
+        "SELECT items.*, issues.*
         FROM issues
         LEFT JOIN items ON issues.itemnumber=items.itemnumber
         WHERE issues.itemnumber=?");
@@ -2225,7 +2372,7 @@ tables issues and the firstname,surname & cardnumber from borrowers.
 
 sub GetBiblioIssues {
     my $biblionumber = shift;
-    return undef unless $biblionumber;
+    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
@@ -2292,8 +2439,6 @@ END_SQL
 
 Find out whether a borrowed item may be renewed.
 
-C<$dbh> is a DBI handle to the Koha database.
-
 C<$borrowernumber> is the borrower number of the patron who currently
 has the item on loan.
 
@@ -2303,7 +2448,7 @@ C<$override_limit>, if supplied with a true value, causes
 the limit on the number of times that the loan can be renewed
 (as controlled by the item type) to be ignored.
 
-C<$CanBookBeRenewed> returns a true value iff the item may be renewed. The
+C<$CanBookBeRenewed> returns a true value if the item may be renewed. The
 item must currently be on loan to the specified borrower; renewals
 must be allowed for the item's type; and the borrower must not have
 already renewed the loan. $error will contain the reason the renewal can not proceed
@@ -2317,65 +2462,29 @@ sub CanBookBeRenewed {
     my $dbh       = C4::Context->dbh;
     my $renews    = 1;
     my $renewokay = 0;
-       my $error;
+    my $error;
 
-    # Look in the issues table for this item, lent to this borrower,
-    # and not yet returned.
+    my $borrower    = C4::Members::GetMemberDetails( $borrowernumber, 0 )   or return;
+    my $item        = GetItem($itemnumber)                                  or return;
+    my $itemissue   = GetItemIssue($itemnumber)                             or return;
 
-    # Look in the issues table for this item, lent to this borrower,
-    # and not yet returned.
-    my %branch = (
-            'ItemHomeLibrary' => 'items.homebranch',
-            'PickupLibrary'   => 'items.holdingbranch',
-            'PatronLibrary'   => 'borrowers.branchcode'
-            );
-    my $controlbranch = $branch{C4::Context->preference('CircControl')};
-    my $itype         = C4::Context->preference('item-level_itypes') ? 'items.itype' : 'biblioitems.itemtype';
-    
-    my $sthcount = $dbh->prepare("
-                   SELECT 
-                    borrowers.categorycode, biblioitems.itemtype, issues.renewals, renewalsallowed, $controlbranch
-                   FROM  issuingrules, 
-                   issues
-                   LEFT JOIN items USING (itemnumber) 
-                   LEFT JOIN borrowers USING (borrowernumber) 
-                   LEFT JOIN biblioitems USING (biblioitemnumber)
-                   
-                   WHERE
-                    (issuingrules.categorycode = borrowers.categorycode OR issuingrules.categorycode = '*')
-                   AND
-                    (issuingrules.itemtype = $itype OR issuingrules.itemtype = '*')
-                   AND
-                    (issuingrules.branchcode = $controlbranch OR issuingrules.branchcode = '*') 
-                   AND 
-                    borrowernumber = ? 
-                   AND
-                    itemnumber = ?
-                   ORDER BY
-                    issuingrules.categorycode desc,
-                    issuingrules.itemtype desc,
-                    issuingrules.branchcode desc
-                   LIMIT 1;
-                  ");
-
-    $sthcount->execute( $borrowernumber, $itemnumber );
-    if ( my $data1 = $sthcount->fetchrow_hashref ) {
-        
-        if ( ( $data1->{renewalsallowed} && $data1->{renewalsallowed} > $data1->{renewals} ) || $override_limit ) {
-            $renewokay = 1;
-        }
-        else {
-                       $error="too_many";
-               }
-               
-        my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber);
-        if ($resfound) {
-            $renewokay = 0;
-                       $error="on_reserve"
-        }
+    my $branchcode  = _GetCircControlBranch($item, $borrower);
+
+    my $issuingrule = GetIssuingRule($borrower->{categorycode}, $item->{itype}, $branchcode);
+
+    if ( ( $issuingrule->{renewalsallowed} > $itemissue->{renewals} ) || $override_limit ) {
+        $renewokay = 1;
+    } else {
+        $error = "too_many";
+    }
 
+    my $resstatus = C4::Reserves::GetReserveStatus($itemnumber);
+    if ( $resstatus eq "Waiting" or $resstatus eq "Reserved" ) {
+        $renewokay = 0;
+        $error = "on_reserve";
     }
-    return ($renewokay,$error);
+
+    return ( $renewokay, $error );
 }
 
 =head2 AddRenewal
@@ -2403,13 +2512,13 @@ from the book's item type.
 =cut
 
 sub AddRenewal {
-    my $borrowernumber  = shift or return undef;
-    my $itemnumber      = shift or return undef;
+    my $borrowernumber  = shift or return;
+    my $itemnumber      = shift or return;
     my $branch          = shift;
     my $datedue         = shift;
     my $lastreneweddate = shift || DateTime->now(time_zone => C4::Context->tz)->ymd();
-    my $item   = GetItem($itemnumber) or return undef;
-    my $biblio = GetBiblioFromItemNumber($itemnumber) or return undef;
+    my $item   = GetItem($itemnumber) or return;
+    my $biblio = GetBiblioFromItemNumber($itemnumber) or return;
 
     my $dbh = C4::Context->dbh;
     # Find the issues record for this book
@@ -2430,13 +2539,13 @@ sub AddRenewal {
     # based on the value of the RenewalPeriodBase syspref.
     unless ($datedue) {
 
-        my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ) or return undef;
+        my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ) or return;
         my $itemtype = (C4::Context->preference('item-level_itypes')) ? $biblio->{'itype'} : $biblio->{'itemtype'};
 
         $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?
-                                        $issuedata->{date_due} :
+                                        dt_from_string( $issuedata->{date_due} ) :
                                         DateTime->now( time_zone => C4::Context->tz());
-        $datedue =  CalcDateDue($datedue,$itemtype,$issuedata->{'branchcode'},$borrower);
+        $datedue =  CalcDateDue($datedue, $itemtype, $issuedata->{'branchcode'}, $borrower, 'is a renewal');
     }
 
     # Update the issues record to have the new due date, and a new count
@@ -2471,8 +2580,29 @@ sub AddRenewal {
             "Renewal of Rental Item $item->{'title'} $item->{'barcode'}",
             'Rent', $charge, $itemnumber );
     }
+
+    # Send a renewal slip according to checkout alert preferencei
+    if ( C4::Context->preference('RenewalSendNotice') eq '1') {
+       my $borrower = C4::Members::GetMemberDetails( $borrowernumber, 0 );
+       my $circulation_alert = 'C4::ItemCirculationAlertPreference';
+       my %conditions = (
+               branchcode   => $branch,
+               categorycode => $borrower->{categorycode},
+               item_type    => $item->{itype},
+               notification => 'CHECKOUT',
+       );
+       if ($circulation_alert->is_enabled_for(\%conditions)) {
+               SendCirculationAlert({
+                       type     => 'RENEWAL',
+                       item     => $item,
+               borrower => $borrower,
+               branch   => $branch,
+               });
+       }
+    }
+
     # Log the renewal
-    UpdateStats( $branch, 'renew', $charge, '', $itemnumber, $item->{itype}, $borrowernumber);
+    UpdateStats( $branch, 'renew', $charge, '', $itemnumber, $item->{itype}, $borrowernumber, undef, $item->{'ccode'});
        return $datedue;
 }
 
@@ -2709,7 +2839,7 @@ sub DeleteTransfer {
 
 =head2 AnonymiseIssueHistory
 
-  $rows = AnonymiseIssueHistory($date,$borrowernumber)
+  ($rows,$err_history_not_deleted) = AnonymiseIssueHistory($date,$borrowernumber)
 
 This function write NULL instead of C<$borrowernumber> given on input arg into the table issues.
 if C<$borrowernumber> is not set, it will delete the issue history for all borrower older than C<$date>.
@@ -2717,7 +2847,7 @@ if C<$borrowernumber> is not set, it will delete the issue history for all borro
 If c<$borrowernumber> is set, it will delete issue history for only that borrower, regardless of their opac privacy
 setting (force delete).
 
-return the number of affected rows.
+return the number of affected rows and a value that evaluates to true if an error occurred deleting the history.
 
 =cut
 
@@ -2744,8 +2874,9 @@ sub AnonymiseIssueHistory {
     }
     my $sth = $dbh->prepare($query);
     $sth->execute(@bind_params);
+    my $anonymisation_err = $dbh->err;
     my $rows_affected = $sth->rows;  ### doublecheck row count return function
-    return $rows_affected;
+    return ($rows_affected, $anonymisation_err);
 }
 
 =head2 SendCirculationAlert
@@ -2792,12 +2923,13 @@ sub SendCirculationAlert {
     my %message_name = (
         CHECKIN  => 'Item_Check_in',
         CHECKOUT => 'Item_Checkout',
+       RENEWAL  => 'Item_Checkout',
     );
     my $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences({
         borrowernumber => $borrower->{borrowernumber},
         message_name   => $message_name{$type},
     });
-    my $issues_table = ( $type eq 'CHECKOUT' ) ? 'issues' : 'old_issues';
+    my $issues_table = ( $type eq 'CHECKOUT' || $type eq 'RENEWAL' ) ? 'issues' : 'old_issues';
     my $letter =  C4::Letters::GetPreparedLetter (
         module => 'circulation',
         letter_code => $type,
@@ -2879,61 +3011,60 @@ C<$startdate>   = C4::Dates object representing start date of loan period (assum
 C<$itemtype>  = itemtype code of item in question
 C<$branch>  = location whose calendar to use
 C<$borrower> = Borrower object
+C<$isrenewal> = Boolean: is true if we want to calculate the date due for a renewal. Else is false.
 
 =cut
 
 sub CalcDateDue {
-    my ( $startdate, $itemtype, $branch, $borrower ) = @_;
+    my ( $startdate, $itemtype, $branch, $borrower, $isrenewal ) = @_;
+
+    $isrenewal ||= 0;
 
     # loanlength now a href
     my $loanlength =
-      GetLoanLength( $borrower->{'categorycode'}, $itemtype, $branch );
+            GetLoanLength( $borrower->{'categorycode'}, $itemtype, $branch );
 
-    my $datedue;
+    my $length_key = ( $isrenewal and defined $loanlength->{renewalperiod} )
+            ? qq{renewalperiod}
+            : qq{issuelength};
 
-    # if globalDueDate ON the datedue is set to that date
-    if (C4::Context->preference('globalDueDate')
-        && ( C4::Context->preference('globalDueDate') =~
-            C4::Dates->regexp('syspref') )
-      ) {
-        $datedue = dt_from_string(
-            C4::Context->preference('globalDueDate'),
-            C4::Context->preference('dateformat')
-        );
+    my $datedue;
+    if ( $startdate ) {
+        if (ref $startdate ne 'DateTime' ) {
+            $datedue = dt_from_string($datedue);
+        } else {
+            $datedue = $startdate->clone;
+        }
     } else {
+        $datedue =
+          DateTime->now( time_zone => C4::Context->tz() )
+          ->truncate( to => 'minute' );
+    }
 
-        # otherwise, calculate the datedue as normal
-        if ( C4::Context->preference('useDaysMode') eq 'Days' )
-        {    # ignoring calendar
-            my $dt =
-              DateTime->now( time_zone => C4::Context->tz() )
-              ->truncate( to => 'minute' );
-            if ( $loanlength->{lengthunit} eq 'hours' ) {
-                $dt->add( hours => $loanlength->{issuelength} );
-                return $dt;
-            } else {    # days
-                $dt->add( days => $loanlength->{issuelength} );
-                $dt->set_hour(23);
-                $dt->set_minute(59);
-                return $dt;
-            }
-        } else {
-            my $dur;
-            if ($loanlength->{lengthunit} eq 'hours') {
-                $dur = DateTime::Duration->new( hours => $loanlength->{issuelength});
-            }
-            else { # days
-                $dur = DateTime::Duration->new( days => $loanlength->{issuelength});
-            }
-            if (ref $startdate ne 'DateTime' ) {
-                $startdate = dt_from_string($startdate);
-            }
-            my $calendar = Koha::Calendar->new( branchcode => $branch );
-            $datedue = $calendar->addDate( $startdate, $dur, $loanlength->{lengthunit} );
-            if ($loanlength->{lengthunit} eq 'days') {
-                $datedue->set_hour(23);
-                $datedue->set_minute(59);
-            }
+
+    # calculate the datedue as normal
+    if ( C4::Context->preference('useDaysMode') eq 'Days' )
+    {    # ignoring calendar
+        if ( $loanlength->{lengthunit} eq 'hours' ) {
+            $datedue->add( hours => $loanlength->{$length_key} );
+        } else {    # days
+            $datedue->add( days => $loanlength->{$length_key} );
+            $datedue->set_hour(23);
+            $datedue->set_minute(59);
+        }
+    } else {
+        my $dur;
+        if ($loanlength->{lengthunit} eq 'hours') {
+            $dur = DateTime::Duration->new( hours => $loanlength->{$length_key});
+        }
+        else { # days
+            $dur = DateTime::Duration->new( days => $loanlength->{$length_key});
+        }
+        my $calendar = Koha::Calendar->new( branchcode => $branch );
+        $datedue = $calendar->addDate( $datedue, $dur, $loanlength->{lengthunit} );
+        if ($loanlength->{lengthunit} eq 'days') {
+            $datedue->set_hour(23);
+            $datedue->set_minute(59);
         }
     }
 
@@ -2954,11 +3085,13 @@ sub CalcDateDue {
         }
 
         # in all other cases, keep the date due as it is
+
     }
 
     # if ReturnBeforeExpiry ON the datedue can't be after borrower expirydate
     if ( C4::Context->preference('ReturnBeforeExpiry') ) {
         my $expiry_dt = dt_from_string( $borrower->{dateexpiry}, 'iso' );
+        $expiry_dt->set( hour => 23, minute => 59);
         if ( DateTime->compare( $datedue, $expiry_dt ) == 1 ) {
             $datedue = $expiry_dt->clone;
         }
@@ -3191,9 +3324,10 @@ sub GetOfflineOperation {
 }
 
 sub AddOfflineOperation {
+    my ( $userid, $branchcode, $timestamp, $action, $barcode, $cardnumber, $amount ) = @_;
     my $dbh = C4::Context->dbh;
-    my $sth = $dbh->prepare("INSERT INTO pending_offline_operations (userid, branchcode, timestamp, action, barcode, cardnumber) VALUES(?,?,?,?,?,?)");
-    $sth->execute( @_ );
+    my $sth = $dbh->prepare("INSERT INTO pending_offline_operations (userid, branchcode, timestamp, action, barcode, cardnumber, amount) VALUES(?,?,?,?,?,?,?)");
+    $sth->execute( $userid, $branchcode, $timestamp, $action, $barcode, $cardnumber, $amount );
     return "Added.";
 }
 
@@ -3212,6 +3346,8 @@ sub ProcessOfflineOperation {
         $report = ProcessOfflineReturn( $operation );
     } elsif ( $operation->{action} eq 'issue' ) {
         $report = ProcessOfflineIssue( $operation );
+    } elsif ( $operation->{action} eq 'payment' ) {
+        $report = ProcessOfflinePayment( $operation );
     }
 
     DeleteOfflineOperation( $operation->{operationid} ) if $operation->{operationid};
@@ -3281,6 +3417,16 @@ sub ProcessOfflineIssue {
     }
 }
 
+sub ProcessOfflinePayment {
+    my $operation = shift;
+
+    my $borrower = C4::Members::GetMemberDetails( undef, $operation->{cardnumber} ); # Get borrower from operation cardnumber
+    my $amount = $operation->{amount};
+
+    recordpayment( $borrower->{borrowernumber}, $amount );
+
+    return "Success."
+}
 
 
 =head2 TransferSlip
@@ -3311,6 +3457,26 @@ sub TransferSlip {
     );
 }
 
+=head2 CheckIfIssuedToPatron
+
+  CheckIfIssuedToPatron($borrowernumber, $biblionumber)
+
+  Return 1 if any record item is issued to patron, otherwise return 0
+
+=cut
+
+sub CheckIfIssuedToPatron {
+    my ($borrowernumber, $biblionumber) = @_;
+
+    my $items = GetItemsByBiblioitemnumber($biblionumber);
+
+    foreach my $item (@{$items}) {
+        return 1 if ($item->{borrowernumber} && $item->{borrowernumber} eq $borrowernumber);
+    }
+
+    return;
+}
+
 
 1;