From 858fbeaf9af4e67b8e76dc9a13427339caeb269c Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Fri, 4 Sep 2020 12:47:58 +0000 Subject: [PATCH] [19.11] 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 --- C4/Circulation.pm | 27 +++++++++++++++------------ misc/cronjobs/automatic_renewals.pl | 2 +- t/db_dependent/Circulation.t | 10 ++++++++-- 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 0cedf80d9c..3ab8a16301 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2683,11 +2683,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' ); @@ -2778,22 +2778,18 @@ sub CanBookBeRenewed { if ( $soonestrenewal > dt_from_string() ) { - return ( 0, "auto_too_soon" ) if $issue->auto_renew; - return ( 0, "too_soon" ); - } - elsif ( $issue->auto_renew ) { - $auto_renew = 1; + $auto_renew = ($issue->auto_renew) ? "auto_too_soon" : "too_soon"; } } # 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"; } } } @@ -2863,8 +2859,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 ); } diff --git a/misc/cronjobs/automatic_renewals.pl b/misc/cronjobs/automatic_renewals.pl index fb1cf03ba9..92ed6f0ad8 100755 --- a/misc/cronjobs/automatic_renewals.pl +++ b/misc/cronjobs/automatic_renewals.pl @@ -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; diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 9b53952774..6d18724b34 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -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' ); -- 2.20.1