Bug 25758: Return on_reserve over too_soon when not calling from automatic_renewals cron

Bug 19014 altered CanBookBeRenewed to return (auto_)too_soon over on_reserve

For cron purposes this is the correct behaviour.

For display purposes we wish to see on_reserve over too_soon

This patchset adds a switch to 'CanBookBeRenewed' to alter the priority of these statuses

To test:
 1 - set NoRenewalBeforePrecision to date only
 2 - set a circ rule to auto-renewal=yes, no renewal before=0, checkout period to 7 days
 3 - check item out
 4 - confirm item shows Scheduled For Automatic Renewal in issues table
 5 - place a hold on the item for another patron
 6 - reload issues table for patron 1, confirm checkout still shows "scheduled for automatic renewal" rather than "on hold"
 7 - change No Renewal Before value to 7
 8 - reload issues table for patron 1, confirm checkout now shows "on hold"
 9 - Apply patch
10 - restart_all
11 - Reload the issues table - confirm 'on_hold' still shows
12 - Change No Renewal Before to 0
13 - Refresh issues table, still shows 'On hold'
14 - perl misc/cronjobs/automatic_renewals.pl -v
15 - Result shows 'auto_too_soon'
16 - prove -v t/db_dependent/Circulation.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
This commit is contained in:
Nick Clemens 2020-09-04 12:47:58 +00:00 committed by Aleisha Amohia
parent 4a2fcd6604
commit 321afcfe74
3 changed files with 25 additions and 13 deletions

View File

@ -2685,11 +2685,11 @@ already renewed the loan. $error will contain the reason the renewal can not pro
=cut
sub CanBookBeRenewed {
my ( $borrowernumber, $itemnumber, $override_limit ) = @_;
my ( $borrowernumber, $itemnumber, $override_limit, $cron ) = @_;
my $dbh = C4::Context->dbh;
my $renews = 1;
my $auto_renew = 0;
my $auto_renew = "no";
my $item = Koha::Items->find($itemnumber) or return ( 0, 'no_item' );
my $issue = $item->checkout or return ( 0, 'no_checkout' );
@ -2780,22 +2780,21 @@ sub CanBookBeRenewed {
if ( $soonestrenewal > dt_from_string() )
{
return ( 0, "auto_too_soon" ) if $issue->auto_renew;
return ( 0, "too_soon" );
$auto_renew = ($issue->auto_renew) ? "auto_too_soon" : "too_soon";
}
elsif ( $issue->auto_renew ) {
$auto_renew = 1;
$auto_renew = 'ok';
}
}
# Fallback for automatic renewals:
# If norenewalbefore is undef, don't renew before due date.
if ( $issue->auto_renew && !$auto_renew ) {
if ( $issue->auto_renew && $auto_renew eq "no" ) {
my $now = dt_from_string;
if ( $now >= dt_from_string( $issue->date_due, 'sql' ) ){
$auto_renew = 1;
$auto_renew = "ok";
} else {
return ( 0, "auto_too_soon" );
$auto_renew = "auto_too_soon";
}
}
}
@ -2865,8 +2864,15 @@ sub CanBookBeRenewed {
}
}
}
return ( 0, "on_reserve" ) if $resfound; # '' when no hold was found
return ( 0, "auto_renew" ) if $auto_renew && !$override_limit; # 0 if auto-renewal should not succeed
if( $cron ) { #The cron wants to return 'too_soon' over 'on_reserve'
return ( 0, $auto_renew ) if $auto_renew =~ 'too_soon';#$auto_renew ne "no" && $auto_renew ne "ok";
return ( 0, "on_reserve" ) if $resfound; # '' when no hold was found
} else { # For other purposes we want 'on_reserve' before 'too_soon'
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";
}
return ( 0, "auto_renew" ) if $auto_renew eq "ok" && !$override_limit; # 0 if auto-renewal should not succeed
return ( 1, undef );
}

View File

@ -76,7 +76,7 @@ my %report;
while ( my $auto_renew = $auto_renews->next ) {
# CanBookBeRenewed returns 'auto_renew' when the renewal should be done by this script
my ( $ok, $error ) = CanBookBeRenewed( $auto_renew->borrowernumber, $auto_renew->itemnumber );
my ( $ok, $error ) = CanBookBeRenewed( $auto_renew->borrowernumber, $auto_renew->itemnumber, undef, 1 );
if ( $error eq 'auto_renew' ) {
my $date_due = AddRenewal( $auto_renew->borrowernumber, $auto_renew->itemnumber, $auto_renew->branchcode );
$auto_renew->auto_renew_error(undef)->store;

View File

@ -279,7 +279,7 @@ $dbh->do(
my ( $reused_itemnumber_1, $reused_itemnumber_2 );
subtest "CanBookBeRenewed tests" => sub {
plan tests => 77;
plan tests => 81;
C4::Context->set_preference('ItemsDeniedRenewal','');
# Generate test biblio
@ -615,10 +615,16 @@ subtest "CanBookBeRenewed tests" => sub {
);
( $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' );
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 );
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' );
( $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' );
( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1, 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' );