X-Git-Url: http://koha-dev.rot13.org:8081/gitweb/?a=blobdiff_plain;f=C4%2FCirculation.pm;h=d2506275e3a2e7095f41d49a23b0a5a5a1b7b3f1;hb=4fcf7af117153690cc4aca55eb47290dfc5814f6;hp=815bc7a0a0fb700ebc02c78f6484a889ad1ad2b1;hpb=4c853b0034e20d127a42149dd9d6239867d9df67;p=koha-ffzg.git diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 815bc7a0a0..d2506275e3 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -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 . - -use strict; -#use warnings; FIXME - Bug 2505 +use Modern::Perl; use DateTime; use POSIX qw( floor ); use Koha::DateUtils; @@ -750,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 ) { @@ -1432,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 ); } @@ -1460,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(), @@ -1474,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' ); } @@ -2170,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 @@ -2197,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; } @@ -2233,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}, @@ -2252,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('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); - } + # 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); @@ -2917,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=?" @@ -2926,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