Bug 9394: QA Followup

[koha.git] / C4 / Circulation.pm

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 185575e..ca66931 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -826,9 +826,15 @@ sub CanBookBeIssued {
         $needsconfirmation{PATRON_CANT} = 1;
     } else {
         if($max_loans_allowed){
-            $needsconfirmation{TOO_MANY} = 1;
-            $needsconfirmation{current_loan_count} = $current_loan_count;
-            $needsconfirmation{max_loans_allowed} = $max_loans_allowed;
+            if ( C4::Context->preference("AllowTooManyOverride") ) {
+                $needsconfirmation{TOO_MANY} = 1;
+                $needsconfirmation{current_loan_count} = $current_loan_count;
+                $needsconfirmation{max_loans_allowed} = $max_loans_allowed;
+            } else {
+                $issuingimpossible{TOO_MANY} = 1;
+                $issuingimpossible{current_loan_count} = $current_loan_count;
+                $issuingimpossible{max_loans_allowed} = $max_loans_allowed;
+            }
         }
     }
 
@@ -887,7 +893,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
@@ -1785,19 +1791,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'});
@@ -1967,6 +1990,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 = ?
@@ -2171,7 +2195,7 @@ sub _FixAccountForLostAndReturned {
             # FIXME: move prepares outside while loop!
             my $usth = $dbh->prepare("UPDATE accountlines SET amountoutstanding= ?
                     WHERE (accountlines_id = ?)");
-            $usth->execute($newamtos,'$thisacct');    # FIXME: '$thisacct' is a string literal!
+            $usth->execute($newamtos,$thisacct);
             $usth = $dbh->prepare("INSERT INTO accountoffsets
                 (borrowernumber, accountno, offsetaccount,  offsetamount)
                 VALUES
@@ -2255,7 +2279,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=?");
@@ -2401,8 +2425,8 @@ SELECT issues.*, items.itype as itemtype, items.homebranch, TO_DAYS( date_due )-
 FROM issues 
 LEFT JOIN items USING (itemnumber)
 LEFT OUTER JOIN branches USING (branchcode)
-WhERE returndate is NULL
-AND ( TO_DAYS( NOW() )-TO_DAYS( date_due ) ) < ?
+WHERE returndate is NULL
+HAVING days_until_due > 0 AND days_until_due < ?
 END_SQL
 
     my @bind_parameters = ( $params->{'days_in_advance'} );
@@ -2525,7 +2549,7 @@ sub AddRenewal {
         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, 'is a renewal');
     }
@@ -2821,7 +2845,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>.
@@ -2829,7 +2853,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
 
@@ -2856,8 +2880,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