From 5778eb26ea2a2ee1562515e3adb25fc0eda37e91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= 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 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 --- C4/Circulation.pm | 10 ++++- t/db_dependent/Circulation.t | 81 +++++++++++++++++++++++++++--------- 2 files changed, 70 insertions(+), 21 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index c2edf81f94..28742e2f4e 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,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 3000753a92..2981430bdc 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 if no renewalbefore' + ); +}; + $schema->storage->txn_rollback; C4::Context->clear_syspref_cache(); $branches = Koha::Libraries->search(); -- 2.20.1