Attachment 127619 Details for Bug 29476 – Bug 29476: Correct soonest renewal date calculation for checkouts with auto-renewal

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

Bug-29476-Correct-soonest-renewal-date-calculation.patch (text/plain), 6.22 KB, created by Joonas Kylmälä on 2021-11-14 14:37:01 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Joonas Kylmälä

Created: 2021-11-14 14:37:01 UTC

Size: 6.22 KB

patch

obsolete

>From 82400df37fe64d1256a59a8d95bbf12e61a7bf88 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= <joonas.kylmala@iki.fi>
>Date: Sun, 14 Nov 2021 14:19:08 +0000
>Subject: [PATCH] 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 display 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
>---
> C4/Circulation.pm            | 14 ++++++-
> t/db_dependent/Circulation.t | 81 +++++++++++++++++++++++++++---------
> 2 files changed, 73 insertions(+), 22 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index c2edf81f94..78e4894b93 100644
>--- 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,8 +3197,19 @@ 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;
>+    } else {
>+        # Checkouts without auto-renewing fall back to current moment
>+        return $now;
>     }
>-    return $now;
> }
> 
> =head2 GetLatestAutoRenewDate
>diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
>index 3000753a92..e7fd227d98 100755
>--- 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'
>+    );
>+};
>+
> $schema->storage->txn_rollback;
> C4::Context->clear_syspref_cache();
> $branches = Koha::Libraries->search();
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 29476: 127618 | 127619 | 127620 | 127900 | 128096 | 128477 | 128478 | 128552 | 128553