From 6f7554754063518cc3399a45b2aaa72d520c8326 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Joonas=20Kylm=C3=A4l=C3=A4?= <joonas.kylmala@iki.fi>
Date: Sun, 14 Nov 2021 12:05:16 +0000
Subject: [PATCH] Bug 29474: Decouple renewal checks of auto-renewed issues
 from non-autorenewed

The too_soon check does not relate at all to auto-renewed issues and
should be moved outside the _CanBookBeAutoRenewed() function.

To test:
   1) prove t/db_dependent/Circulation.t

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
---
 C4/Circulation.pm | 40 +++++++++++++---------------------------
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 200ec64fdb..d5fa0fb7ba 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -2795,7 +2795,7 @@ sub CanBookBeRenewed {
             branchcode => $branchcode,
             issue      => $issue
         });
-        return ( 0, $auto_renew  ) if $auto_renew =~ 'too_soon' && $cron;
+        return ( 0, $auto_renew  ) 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';
@@ -2864,6 +2864,9 @@ 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" ) if $auto_renew eq "ok" && !$override_limit; # 0 if auto-renewal should not succeed
 
@@ -4319,35 +4322,18 @@ sub _CanBookBeAutoRenewed {
                 return "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 > dt_from_string() )
-        {
-            return ($issue->auto_renew && $patron->autorenew_checkouts) ? "auto_too_soon" : "too_soon";
-        }
-        elsif ( $issue->auto_renew && $patron->autorenew_checkouts ) {
-            return "ok";
+        if ( defined $issuing_rule->{norenewalbefore}
+            and $issuing_rule->{norenewalbefore} ne "" ) {
+            if ( GetSoonestRenewDate($patron->id, $item->id) > dt_from_string()) {
+                return "auto_too_soon";
+            } else {
+                return "ok";
+            }
         }
-    }
 
-    # Fallback for automatic renewals:
-    # If norenewalbefore is undef, don't renew before due date.
-    if ( $issue->auto_renew && $patron->autorenew_checkouts ) {
+        # Fallback for automatic renewals:
+        # If norenewalbefore is undef, don't renew before due date.
         my $now = dt_from_string;
         if ( $now >= dt_from_string( $issue->date_due, 'sql' ) ){
             return "ok";
-- 
2.39.5