Bug 29476: Correct soonest renewal date calculation for checkouts with auto-renewal

If a checkout with auto-renewal enabled doesn't have a
"norenewalbefore" circulation rule set the code in CanBookBeRenewed()
falls back to using due date (to verify this please look for the
string "auto_too_soon" in C4/Circulation.pm), the calculation result
of GetSoonestRenewDate() however didn't do this, though luckily it was
not used in CanBookBeRenewed so we didn't get any issues
there. However, GetSoonestRenewDate() is used for displaying the
soonest renewal date in the staff interface on the circ/renew.pl page
so you would have gotten wrong results there.

This patch moves additionally the tests made for Bug 14395 under a new
subtest for GetSoonestRenewDate() as they should have been like that
already before.

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

Signed-off-by: David Nind <david@davidnind.com>
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>

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index c2edf81..28742e2 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -3183,7 +3183,6 @@ sub GetSoonestRenewDate {
     );
 
     my $now = dt_from_string;
-    return $now unless $issuing_rule;
 
     if ( defined $issuing_rule->{norenewalbefore}
         and $issuing_rule->{norenewalbefore} ne "" )
@@ -3198,6 +3197,15 @@ sub GetSoonestRenewDate {
             $soonestrenewal->truncate( to => 'day' );
         }
         return $soonestrenewal if $now < $soonestrenewal;
+    } elsif ( $itemissue->auto_renew && $patron->autorenew_checkouts ) {
+        # Checkouts with auto-renewing fall back to due date
+        my $soonestrenewal = return dt_from_string( $itemissue->date_due );
+        if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date'
+            and $issuing_rule->{lengthunit} eq 'days' )
+        {
+            $soonestrenewal->truncate( to => 'day' );
+        }
+        return $soonestrenewal;
     }
     return $now;
 }

diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
index 3000753..2981430 100755 (executable)
--- a/t/db_dependent/Circulation.t
+++ b/t/db_dependent/Circulation.t
@@ -18,7 +18,7 @@
 use Modern::Perl;
 use utf8;
 
-use Test::More tests => 56;
+use Test::More tests => 57;
 use Test::Exception;
 use Test::MockModule;
 use Test::Deep qw( cmp_deeply );
@@ -418,7 +418,7 @@ subtest "GetIssuingCharges tests" => sub {
 
 my ( $reused_itemnumber_1, $reused_itemnumber_2 );
 subtest "CanBookBeRenewed tests" => sub {
-    plan tests => 95;
+    plan tests => 93;
 
     C4::Context->set_preference('ItemsDeniedRenewal','');
     # Generate test biblio
@@ -856,24 +856,6 @@ subtest "CanBookBeRenewed tests" => sub {
     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)');
 
-    # Bug 14395
-    # Test 'exact time' setting for syspref NoRenewalBeforePrecision
-    t::lib::Mocks::mock_preference( 'NoRenewalBeforePrecision', 'exact_time' );
-    is(
-        GetSoonestRenewDate( $renewing_borrowernumber, $item_1->itemnumber ),
-        $datedue->clone->add( days => -7 ),
-        'Bug 14395: Renewals permitted 7 days before due date, as expected'
-    );
-
-    # Bug 14395
-    # Test 'date' setting for syspref NoRenewalBeforePrecision
-    t::lib::Mocks::mock_preference( 'NoRenewalBeforePrecision', 'date' );
-    is(
-        GetSoonestRenewDate( $renewing_borrowernumber, $item_1->itemnumber ),
-        $datedue->clone->add( days => -7 )->truncate( to => 'day' ),
-        'Bug 14395: Renewals permitted 7 days before due date, as expected'
-    );
-
     # Bug 14101
     # Test premature automatic renewal
     ( $renewokay, $error ) =
@@ -4974,6 +4956,65 @@ subtest "SendCirculationAlert" => sub {
 
 };
 
+subtest "GetSoonestRenewDate tests" => sub {
+    plan tests => 4;
+    Koha::CirculationRules->set_rule(
+        {
+            categorycode => undef,
+            branchcode   => undef,
+            itemtype     => undef,
+            rule_name    => 'norenewalbefore',
+            rule_value   => '7',
+        }
+    );
+    my $patron = $builder->build_object({ class => 'Koha::Patrons' });
+    my $item = $builder->build_sample_item();
+    my $issue = AddIssue( $patron->unblessed, $item->barcode);
+    my $datedue = dt_from_string( $issue->date_due() );
+
+    # Bug 14395
+    # Test 'exact time' setting for syspref NoRenewalBeforePrecision
+    t::lib::Mocks::mock_preference( 'NoRenewalBeforePrecision', 'exact_time' );
+    is(
+        GetSoonestRenewDate( $patron->id, $item->itemnumber ),
+        $datedue->clone->add( days => -7 ),
+        'Bug 14395: Renewals permitted 7 days before due date, as expected'
+    );
+
+    # Bug 14395
+    # Test 'date' setting for syspref NoRenewalBeforePrecision
+    t::lib::Mocks::mock_preference( 'NoRenewalBeforePrecision', 'date' );
+    is(
+        GetSoonestRenewDate( $patron->id, $item->itemnumber ),
+        $datedue->clone->add( days => -7 )->truncate( to => 'day' ),
+        'Bug 14395: Renewals permitted 7 days before due date, as expected'
+    );
+
+
+    Koha::CirculationRules->set_rule(
+        {
+            categorycode => undef,
+            branchcode   => undef,
+            itemtype     => undef,
+            rule_name    => 'norenewalbefore',
+            rule_value   => undef,
+        }
+    );
+
+    is(
+        GetSoonestRenewDate( $patron->id, $item->itemnumber ),
+        dt_from_string,
+        'Checkouts without auto-renewal can be renewed immediately if no norenewalbefore'
+    );
+
+    $issue->auto_renew(1)->store;
+    is(
+        GetSoonestRenewDate( $patron->id, $item->itemnumber ),
+        $datedue,
+        'Checkouts with auto-renewal can be renewed earliest on due date if no renewalbefore'
+    );
+};
+
 $schema->storage->txn_rollback;
 C4::Context->clear_syspref_cache();
 $branches = Koha::Libraries->search();