From b0c007d66c20a6e17fdf7e484fc019aa1cb0307b 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 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..3404582375 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 current day + 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