From 8aa24c0ceb8dcd5315aba90f3a661e65776c3ad4 Mon Sep 17 00:00:00 2001 From: =?utf8?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 Signed-off-by: David Nind Signed-off-by: Nick Clemens Signed-off-by: Jonathan Druart Signed-off-by: Fridolin Somers Signed-off-by: Kyle M Hall (cherry picked from commit cea2b217c5288c77aae7cbaf5ac791ee249b9812) Signed-off-by: Andrew Fuerste-Henry --- 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 3f5b8b6eeb..4d89924eab 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3263,7 +3263,6 @@ sub GetSoonestRenewDate { ); my $now = dt_from_string; - return $now unless $issuing_rule; if ( defined $issuing_rule->{norenewalbefore} and $issuing_rule->{norenewalbefore} ne "" ) @@ -3278,6 +3277,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 b5d70db411..fd8024a3ca 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 ); @@ -416,7 +416,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 @@ -854,24 +854,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 ) = @@ -4975,6 +4957,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.39.5