From 74c03a404a8a3aafd748787d938f4d61b1b1afe8 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Mon, 30 Dec 2019 19:27:26 +0000 Subject: [PATCH] Bug 19014: [19.11.x] Prevent auto_renew notices form sending if too early to renew Bug 19014: Unit tests Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Martin Renvoize Bug 19014: Return auto_too_soon before on_reserve Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Martin Renvoize Bug 19014: on_reserve blocks auto_renew Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Martin Renvoize --- C4/Circulation.pm | 202 ++++++++++++++++++++++--------------------- t/db_dependent/Circulation.t | 20 ++++- 2 files changed, 123 insertions(+), 99 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 57fa54ba76..26e7f6dac0 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2672,6 +2672,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' ); @@ -2680,6 +2681,108 @@ 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::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 ) { + $auto_renew = 1; + } + } + + # Fallback for automatic renewals: + # If norenewalbefore is undef, don't renew before due date. + if ( $issue->auto_renew && !$auto_renew ) { + 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 @@ -2746,104 +2849,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 ); } diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 8bf9e56aff..10b41fdb07 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -277,7 +277,7 @@ $dbh->do( my ( $reused_itemnumber_1, $reused_itemnumber_2 ); subtest "CanBookBeRenewed tests" => sub { - plan tests => 71; + plan tests => 77; C4::Context->set_preference('ItemsDeniedRenewal',''); # Generate test biblio @@ -606,6 +606,24 @@ subtest "CanBookBeRenewed tests" => sub { is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic and premature' ); is( $error, 'auto_too_soon', 'Bug 14101: Cannot renew, renewal is automatic and premature, "No renewal before" = undef (returned code is auto_too_soon)' ); + AddReserve( + $branch, $reserving_borrowernumber, $biblio->biblionumber, + $bibitems, $priority, $resdate, $expdate, $notes, + 'a title', $item_4->itemnumber, $found + ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); + is( $renewokay, 0, 'Still should not be able to renew' ); + is( $error, 'auto_too_soon', 'returned code is auto_too_soon, reserve not checked' ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1 ); + is( $renewokay, 0, 'Still should not be able to renew' ); + is( $error, 'on_reserve', 'returned code is on_reserve, auto_too_soon limit is overridden' ); + $dbh->do('UPDATE circulation_rules SET rule_value = 0 where rule_name = "norenewalbefore"'); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1 ); + is( $renewokay, 0, 'Still should not be able to renew' ); + is( $error, 'on_reserve', 'returned code is on_reserve, auto_renew only happens if not on reserve' ); + ModReserveCancelAll($item_4->itemnumber, $reserving_borrowernumber); + + # Bug 7413 # Test premature manual renewal -- 2.11.0