From 4c10f83dc00ff1b94810a56cf4e7f3a5f97b3c95 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Fri, 4 Sep 2020 12:47:58 +0000 Subject: [PATCH] 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 Signed-off-by: Katrin Fischer Signed-off-by: Jonathan Druart (cherry picked from commit ff08e99965ccb60092b5bdd5181fd517e1fab161) Signed-off-by: Lucas Gass --- C4/Circulation.pm | 26 ++++++++++++++++---------- misc/cronjobs/automatic_renewals.pl | 2 +- t/db_dependent/Circulation.t | 10 ++++++++-- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 5afb8f421f..39ef3fff55 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2725,11 +2725,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' ); @@ -2827,22 +2827,21 @@ sub CanBookBeRenewed { if ( $soonestrenewal > dt_from_string() ) { - return ( 0, "auto_too_soon" ) if $issue->auto_renew && $patron->autorenew_checkouts; - return ( 0, "too_soon" ); + $auto_renew = ($issue->auto_renew && $patron->autorenew_checkouts) ? "auto_too_soon" : "too_soon"; } elsif ( $issue->auto_renew && $patron->autorenew_checkouts ) { - $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 && $patron->autorenew_checkouts ) { + if ( $issue->auto_renew && $auto_renew eq "no" && $patron->autorenew_checkouts ) { 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"; } } } @@ -2912,8 +2911,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 a76f7dfb35..1c47cfd680 100755 --- a/misc/cronjobs/automatic_renewals.pl +++ b/misc/cronjobs/automatic_renewals.pl @@ -88,7 +88,7 @@ print "Test run only\n" unless $confirm; 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' ) { if ($verbose) { say sprintf "Issue id: %s for borrower: %s and item: %s ". ( $confirm ? 'will' : 'would') . " be renewed.", diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index f9c1696b06..519aece41f 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -277,7 +277,7 @@ Koha::CirculationRules->set_rules( my ( $reused_itemnumber_1, $reused_itemnumber_2 ); subtest "CanBookBeRenewed tests" => sub { - plan tests => 83; + plan tests => 87; C4::Context->set_preference('ItemsDeniedRenewal',''); # Generate test biblio @@ -648,10 +648,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.39.5