From 2a80d866e920019cd383ff1454e561ff077a5aff Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 23 Feb 2022 16:01:22 +0000 Subject: [PATCH] Bug 30167: Return soonest renewal date when renewal is to soon This patch adds an 'info' return param to CanBookBeRenewed and passes the soonest renewal date when returning too_soon errors To test: 1 - prove -v t/db_dependent/Circulation.t Fix whitespace Signed-off-by: David Nind --- C4/Circulation.pm | 25 +++++++++++++++---------- t/db_dependent/Circulation.t | 24 ++++++++++++++++-------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index d0f292650d..bba60d0618 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2818,7 +2818,7 @@ sub GetUpcomingDueIssues { =head2 CanBookBeRenewed - ($ok,$error) = &CanBookBeRenewed($borrowernumber, $itemnumber[, $override_limit]); + ($ok,$error,$info) = &CanBookBeRenewed($borrowernumber, $itemnumber[, $override_limit]); Find out whether a borrowed item may be renewed. @@ -2836,7 +2836,9 @@ that are automatically renewed. 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 +already renewed the loan. + $error will contain the reason the renewal can not proceed + $info will contain the soonest renewal date if the error is 'too soon' =cut @@ -2844,6 +2846,7 @@ sub CanBookBeRenewed { my ( $borrowernumber, $itemnumber, $override_limit, $cron ) = @_; my $auto_renew = "no"; + my $soonest; my $item = Koha::Items->find($itemnumber) or return ( 0, 'no_item' ); my $issue = $item->checkout or return ( 0, 'no_checkout' ); @@ -2888,13 +2891,13 @@ sub CanBookBeRenewed { return ( 0, 'overdue'); } - $auto_renew = _CanBookBeAutoRenewed({ + ( $auto_renew, $soonest ) = _CanBookBeAutoRenewed({ patron => $patron, item => $item, branchcode => $branchcode, issue => $issue }); - return ( 0, $auto_renew ) if $auto_renew =~ 'auto_too_soon' && $cron; + return ( 0, $auto_renew, $soonest ) if $auto_renew =~ 'auto_too_soon' && $cron; # cron wants 'too_soon' over 'on_reserve' for performance and to avoid # extra notices being sent. Cron also implies no override return ( 0, $auto_renew ) if $auto_renew =~ 'auto_account_expired'; @@ -2997,9 +3000,10 @@ sub CanBookBeRenewed { } return ( 0, "on_reserve" ) if $resfound; # '' when no hold was found - return ( 0, $auto_renew ) if $auto_renew =~ 'too_soon';#$auto_renew ne "no" && $auto_renew ne "ok"; - if ( GetSoonestRenewDate($borrowernumber, $itemnumber) > dt_from_string() ){ - return (0, "too_soon") unless $override_limit; + return ( 0, $auto_renew, $soonest ) if $auto_renew =~ 'too_soon';#$auto_renew ne "no" && $auto_renew ne "ok"; + $soonest = GetSoonestRenewDate($borrowernumber, $itemnumber); + if ( $soonest > dt_from_string() ){ + return (0, "too_soon", $soonest ) unless $override_limit; } return ( 0, "auto_renew" ) if $auto_renew eq "ok" && !$override_limit; # 0 if auto-renewal should not succeed @@ -4459,8 +4463,9 @@ sub _CanBookBeAutoRenewed { if ( defined $issuing_rule->{norenewalbefore} and $issuing_rule->{norenewalbefore} ne "" ) { - if ( GetSoonestRenewDate($patron->id, $item->id) > dt_from_string()) { - return "auto_too_soon"; + my $soonest = GetSoonestRenewDate($patron->id, $item->id); + if ( $soonest > dt_from_string()) { + return ( "auto_too_soon", $soonest ); } else { return "ok"; } @@ -4472,7 +4477,7 @@ sub _CanBookBeAutoRenewed { if ( $now >= dt_from_string( $issue->date_due, 'sql' ) ){ return "ok"; } else { - return "auto_too_soon"; + return ( "auto_too_soon", $issue->date_due ); } } diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index cf54a4f720..fcfcf90d71 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -418,7 +418,7 @@ subtest "GetIssuingCharges tests" => sub { my ( $reused_itemnumber_1, $reused_itemnumber_2 ); subtest "CanBookBeRenewed tests" => sub { - plan tests => 97; + plan tests => 104; C4::Context->set_preference('ItemsDeniedRenewal',''); # Generate test biblio @@ -794,11 +794,13 @@ subtest "CanBookBeRenewed tests" => sub { ); $issue = AddIssue( $renewing_borrower, $item_4->barcode, undef, undef, undef, undef, { auto_renew => 1 } ); - ( $renewokay, $error ) = + my $info; + ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); 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)' ); + is( $info, $issue->date_due, "Due date is returned as earliest renewal date when error is 'auto_too_soon'" ); AddReserve( { branchcode => $branch, @@ -817,9 +819,10 @@ subtest "CanBookBeRenewed tests" => sub { ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); is( $renewokay, 0, 'Still should not be able to renew' ); is( $error, 'on_reserve', 'returned code is on_reserve, reserve checked when not checking for cron' ); - ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, undef, 1 ); + ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, undef, 1 ); is( $renewokay, 0, 'Still should not be able to renew' ); is( $error, 'auto_too_soon', 'returned code is auto_too_soon, reserve not checked when checking for cron' ); + is( $info, $issue->date_due, "Due date is returned as earliest renewal date when error is 'auto_too_soon'" ); ( $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' ); @@ -852,39 +855,44 @@ subtest "CanBookBeRenewed tests" => sub { } ); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); + ( $renewokay, $error, $info ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); is( $renewokay, 0, 'Bug 7413: Cannot renew, renewal is premature'); is( $error, 'too_soon', 'Bug 7413: Cannot renew, renewal is premature (returned code is too_soon)'); + is( $info, dt_from_string($issue->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'too_soon'"); # Bug 14101 # Test premature automatic renewal - ( $renewokay, $error ) = + ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); 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 (returned code is auto_too_soon)' ); + is( $info, dt_from_string($issue->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'auto_too_soon'"); $renewing_borrower_obj->autorenew_checkouts(0)->store; - ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); + ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); is( $renewokay, 0, 'No renewal before is 7, patron opted out of auto_renewal still cannot renew early' ); is( $error, 'too_soon', 'Error is too_soon, no auto' ); + is( $info, dt_from_string($issue->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'too_soon'"); $renewing_borrower_obj->autorenew_checkouts(1)->store; # Change policy so that loans can only be renewed exactly on due date (0 days prior to due date) # and test automatic renewal again $dbh->do(q{UPDATE circulation_rules SET rule_value = '0' WHERE rule_name = 'norenewalbefore'}); - ( $renewokay, $error ) = + ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); 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" = 0 (returned code is auto_too_soon)' ); + is( $info, dt_from_string($issue->date_due), "Soonest renew date returned when error is 'auto_too_soon'"); $renewing_borrower_obj->autorenew_checkouts(0)->store; - ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); + ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); is( $renewokay, 0, 'No renewal before is 0, patron opted out of auto_renewal still cannot renew early' ); is( $error, 'too_soon', 'Error is too_soon, no auto' ); + is( $info, dt_from_string($issue->date_due), "Soonest renew date returned when error is 'auto_too_soon'"); $renewing_borrower_obj->autorenew_checkouts(1)->store; # Change policy so that loans can be renewed 99 days prior to the due date -- 2.30.2