Bug 24394: Typo when adding a new cash register
[koha-ffzg.git] / C4 / Circulation.pm
index 8db38ca..d250627 100644 (file)
@@ -18,9 +18,7 @@ package C4::Circulation;
 # You should have received a copy of the GNU General Public License
 # along with Koha; if not, see <http://www.gnu.org/licenses>.
 
-
-use strict;
-#use warnings; FIXME - Bug 2505
+use Modern::Perl;
 use DateTime;
 use POSIX qw( floor );
 use Koha::DateUtils;
@@ -60,6 +58,7 @@ use Koha::Account::Offsets;
 use Koha::Config::SysPrefs;
 use Koha::Charges::Fees;
 use Koha::Util::SystemPreferences;
+use Koha::Checkouts::ReturnClaims;
 use Carp;
 use List::MoreUtils qw( uniq any );
 use Scalar::Util qw( looks_like_number );
@@ -318,7 +317,6 @@ sub transferbook {
     }
 
     my $itemnumber = $item->itemnumber;
-    my $issue = GetOpenIssue($itemnumber);
     # get branches of book...
     my $hbr = $item->homebranch;
     my $fbr = $item->holdingbranch;
@@ -344,6 +342,7 @@ sub transferbook {
     }
 
     # check if it is still issued to someone, return it...
+    my $issue = Koha::Checkouts->find({ itemnumber => $itemnumber });
     if ( $issue ) {
         AddReturn( $barcode, $fbr );
         $messages->{'WasReturned'} = $issue->borrowernumber;
@@ -749,11 +748,11 @@ sub CanBookBeIssued {
         return( { STATS => 1 }, {});
     }
 
-    if ( $patron->gonenoaddress == 1 ) {
+    if ( $patron->gonenoaddress && $patron->gonenoaddress == 1 ) {
         $issuingimpossible{GNA} = 1;
     }
 
-    if ( $patron->lost == 1 ) {
+    if ( $patron->lost && $patron->lost == 1 ) {
         $issuingimpossible{CARD_LOST} = 1;
     }
     if ( $patron->is_debarred ) {
@@ -1431,7 +1430,8 @@ sub AddIssue {
                     }
                 )->store;
             }
-            if ( $item_object->location eq 'CART' && $item_object->permanent_location ne 'CART'  ) {
+            if ( $item_object->location && $item_object->location eq 'CART'
+                && ( !$item_object->permanent_location || $item_object->permanent_location ne 'CART' ) ) {
             ## Item was moved to cart via UpdateItemLocationOnCheckin, anything issued should be taken off the cart.
                 CartToShelf( $item_object->itemnumber );
             }
@@ -1459,7 +1459,7 @@ sub AddIssue {
 
             ModItem(
                 {
-                    issues        => $item_object->issues + 1,
+                    issues        => ( $item_object->issues || 0 ) + 1,
                     holdingbranch => C4::Context->userenv->{'branch'},
                     itemlost      => 0,
                     onloan        => $datedue->ymd(),
@@ -1473,7 +1473,7 @@ sub AddIssue {
 
             # If it costs to borrow this book, charge it to the patron's account.
             my ( $charge, $itemtype ) = GetIssuingCharges( $item_object->itemnumber, $borrower->{'borrowernumber'} );
-            if ( $charge > 0 ) {
+            if ( $charge && $charge > 0 ) {
                 AddIssuingCharge( $issue, $charge, 'RENT' );
             }
 
@@ -1952,17 +1952,15 @@ sub AddReturn {
 
     # case of a return of document (deal with issues and holdingbranch)
     if ($doreturn) {
-        my $is_overdue;
         die "The item is not issed and cannot be returned" unless $issue; # Just in case...
         $patron or warn "AddReturn without current borrower";
-        $is_overdue = $issue->is_overdue( $return_date );
 
         if ($patron) {
             eval {
                 MarkIssueReturned( $borrowernumber, $item->itemnumber, $return_date, $patron->privacy );
             };
             unless ( $@ ) {
-                if ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue && !$item->itemlost ) {
+                if ( C4::Context->preference('CalculateFinesOnReturn') && !$item->itemlost ) {
                     _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed, return_date => $return_date } );
                 }
             } else {
@@ -2127,6 +2125,19 @@ sub AddReturn {
         }
     }
 
+    if ( C4::Context->preference('ClaimReturnedLostValue') ) {
+        my $claims = Koha::Checkouts::ReturnClaims->search(
+           {
+               itemnumber => $item->id,
+               resolution => undef,
+           }
+        );
+
+        if ( $claims->count ) {
+            $messages->{ReturnClaims} = $claims;
+        }
+    }
+
     return ( $doreturn, $messages, $issue, ( $patron ? $patron->unblessed : {} ));
 }
 
@@ -2158,7 +2169,7 @@ sub MarkIssueReturned {
     my $issue_id = $issue->issue_id;
 
     my $anonymouspatron;
-    if ( $privacy == 2 ) {
+    if ( $privacy && $privacy == 2 ) {
         # The default of 0 will not work due to foreign key constraints
         # The anonymisation will fail if AnonymousPatron is not a valid entry
         # We need to check if the anonymous patron exist, Koha will fail loudly if it does not
@@ -2185,7 +2196,7 @@ sub MarkIssueReturned {
         my $old_checkout = Koha::Old::Checkout->new($issue->unblessed)->store;
 
         # anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber
-        if ( $privacy == 2) {
+        if ( $privacy && $privacy == 2) {
             $old_checkout->borrowernumber($anonymouspatron)->store;
         }
 
@@ -2221,14 +2232,15 @@ Internal function, called only by AddReturn that calculates and updates
 
 Should only be called for overdue returns
 
+Calculation of the debarment date has been moved to a separate subroutine _calculate_new_debar_dt
+to ease testing.
+
 =cut
 
-sub _debar_user_on_return {
+sub _calculate_new_debar_dt {
     my ( $borrower, $item, $dt_due, $return_date ) = @_;
 
     my $branchcode = _GetCircControlBranch( $item, $borrower );
-    $return_date //= dt_from_string();
-
     my $circcontrol = C4::Context->preference('CircControl');
     my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
         {   categorycode => $borrower->{categorycode},
@@ -2240,81 +2252,92 @@ sub _debar_user_on_return {
     my $unit     = $issuing_rule ? $issuing_rule->lengthunit : undef;
     my $chargeable_units = C4::Overdues::get_chargeable_units($unit, $dt_due, $return_date, $branchcode);
 
-    if ($finedays) {
+    return unless $finedays;
 
-        # finedays is in days, so hourly loans must multiply by 24
-        # thus 1 hour late equals 1 day suspension * finedays rate
-        $finedays = $finedays * 24 if ( $unit eq 'hours' );
+    # finedays is in days, so hourly loans must multiply by 24
+    # thus 1 hour late equals 1 day suspension * finedays rate
+    $finedays = $finedays * 24 if ( $unit eq 'hours' );
 
-        # grace period is measured in the same units as the loan
-        my $grace =
-          DateTime::Duration->new( $unit => $issuing_rule->firstremind );
+    # grace period is measured in the same units as the loan
+    my $grace =
+      DateTime::Duration->new( $unit => $issuing_rule->firstremind );
 
-        my $deltadays = DateTime::Duration->new(
-            days => $chargeable_units
-        );
-        if ( $deltadays->subtract($grace)->is_positive() ) {
-            my $suspension_days = $deltadays * $finedays;
-
-            # 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 ) {
-                $max_sd = DateTime::Duration->new( days => $max_sd );
-                $suspension_days = $max_sd
-                  if DateTime::Duration->compare( $max_sd, $suspension_days ) < 0;
-            }
+    my $deltadays = DateTime::Duration->new(
+        days => $chargeable_units
+    );
 
-            my ( $has_been_extended, $is_a_reminder );
-            if ( C4::Context->preference('CumulativeRestrictionPeriods') and $borrower->{debarred} ) {
-                my $debarment = @{ GetDebarments( { borrowernumber => $borrower->{borrowernumber}, type => 'SUSPENSION' } ) }[0];
-                if ( $debarment ) {
-                    $return_date = dt_from_string( $debarment->{expiration}, 'sql' );
-                    $has_been_extended = 1;
-                }
-            }
+    if ( $deltadays->subtract($grace)->is_positive() ) {
+        my $suspension_days = $deltadays * $finedays;
 
-            if ( $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 )
-                );
-            }
+        if ( $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 )
+            );
+        }
 
-            my $new_debar_dt;
-            # Use the calendar or not to calculate the debarment date
-            if ( C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed' ) {
-                my $calendar = Koha::Calendar->new(
-                    branchcode => $branchcode,
-                    days_mode  => 'Calendar'
-                );
-                $new_debar_dt = $calendar->addDate( $return_date, $suspension_days );
-            }
-            else {
-                $new_debar_dt = $return_date->clone()->add_duration($suspension_days);
-            }
+        # 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 ) {
+            $max_sd = DateTime::Duration->new( days => $max_sd );
+            $suspension_days = $max_sd
+              if DateTime::Duration->compare( $max_sd, $suspension_days ) < 0;
+        }
 
-            Koha::Patron::Debarments::AddUniqueDebarment({
-                borrowernumber => $borrower->{borrowernumber},
-                expiration     => $new_debar_dt->ymd(),
-                type           => 'SUSPENSION',
-            });
-            # if borrower was already debarred but does not get an extra debarment
-            my $patron = Koha::Patrons->find( $borrower->{borrowernumber} );
-            my $new_debarment_str;
-            if ( $borrower->{debarred} eq $patron->is_debarred ) {
-                $is_a_reminder = 1;
-                $new_debarment_str = $borrower->{debarred};
-            } else {
-                $new_debarment_str = $new_debar_dt->ymd();
+        my ( $has_been_extended );
+        if ( C4::Context->preference('CumulativeRestrictionPeriods') and $borrower->{debarred} ) {
+            my $debarment = @{ GetDebarments( { borrowernumber => $borrower->{borrowernumber}, type => 'SUSPENSION' } ) }[0];
+            if ( $debarment ) {
+                $return_date = dt_from_string( $debarment->{expiration}, 'sql' );
+                $has_been_extended = 1;
             }
-            # FIXME Should return a DateTime object
-            return $new_debarment_str, $is_a_reminder;
         }
+
+        my $new_debar_dt;
+        # Use the calendar or not to calculate the debarment date
+        if ( C4::Context->preference('SuspensionsCalendar') eq 'noSuspensionsWhenClosed' ) {
+            my $calendar = Koha::Calendar->new(
+                branchcode => $branchcode,
+                days_mode  => 'Calendar'
+            );
+            $new_debar_dt = $calendar->addDate( $return_date, $suspension_days );
+        }
+        else {
+            $new_debar_dt = $return_date->clone()->add_duration($suspension_days);
+        }
+        return $new_debar_dt;
     }
     return;
 }
 
+sub _debar_user_on_return {
+    my ( $borrower, $item, $dt_due, $return_date ) = @_;
+
+    $return_date //= dt_from_string();
+
+    my $new_debar_dt = _calculate_new_debar_dt ($borrower, $item, $dt_due, $return_date);
+
+    return unless $new_debar_dt;
+
+    Koha::Patron::Debarments::AddUniqueDebarment({
+        borrowernumber => $borrower->{borrowernumber},
+        expiration     => $new_debar_dt->ymd(),
+        type           => 'SUSPENSION',
+    });
+    # if borrower was already debarred but does not get an extra debarment
+    my $patron = Koha::Patrons->find( $borrower->{borrowernumber} );
+    my ($new_debarment_str, $is_a_reminder);
+    if ( $borrower->{debarred} && $borrower->{debarred} eq $patron->is_debarred ) {
+        $is_a_reminder = 1;
+        $new_debarment_str = $borrower->{debarred};
+    } else {
+        $new_debarment_str = $new_debar_dt->ymd();
+    }
+    # FIXME Should return a DateTime object
+    return $new_debarment_str, $is_a_reminder;
+}
+
 =head2 _FixOverduesOnReturn
 
    &_FixOverduesOnReturn($borrowernumber, $itemnumber, $exemptfine, $status);
@@ -2774,7 +2797,10 @@ sub CanBookBeRenewed {
 
         if ( C4::Context->preference('OPACFineNoRenewalsBlockAutoRenew') ) {
             my $fine_no_renewals = C4::Context->preference("OPACFineNoRenewals");
-            my $amountoutstanding = $patron->account->balance;
+            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" );
             }
@@ -2873,7 +2899,7 @@ sub AddRenewal {
     my $schema = Koha::Database->schema;
     $schema->txn_do(sub{
 
-        if ( C4::Context->preference('CalculateFinesOnReturn') && $issue->is_overdue ) {
+        if ( C4::Context->preference('CalculateFinesOnReturn') ) {
             _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed } );
         }
         _FixOverduesOnReturn( $borrowernumber, $itemnumber, undef, 'RENEWED' );
@@ -2902,7 +2928,7 @@ sub AddRenewal {
 
         # Update the issues record to have the new due date, and a new count
         # of how many times it has been renewed.
-        my $renews = $issue->renewals + 1;
+        my $renews = ( $issue->renewals || 0 ) + 1;
         my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ?
                                 WHERE borrowernumber=?
                                 AND itemnumber=?"
@@ -2911,7 +2937,7 @@ sub AddRenewal {
         $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $lastreneweddate, $borrowernumber, $itemnumber );
 
         # Update the renewal count on the item, and tell zebra to reindex
-        $renews = $item_object->renewals + 1;
+        $renews = ( $item_object->renewals || 0 ) + 1;
         ModItem( { renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item_object->biblionumber, $itemnumber, { log_action => 0 } );
 
         # Charge a new rental fee, if applicable