From 2e1317014b22c15dfedd351e6e402f53496fb3dd Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Fri, 29 Jan 2021 15:19:26 +0000 Subject: [PATCH] Bug 18146: Fix tests for OverduesBlockRenewing This pref was supposedly covered by tests, but the conditions were wrong and we didn't test the reasons we were failing so the code was being missed To test: 1 - Add a warn around in the conditional at line 2748: } elsif ( ($hasoverdues and $overduesblockrenewing eq 'block') || ($itemissue->{overdue} and $overduesblockrenewing eq 'blockitem') ) { warn "SUCCESS!" return ( 0, 'overdue'); } 2 - prove -v t/db_dependent/Circulation.t | grep SUCCESS 3 - No output 4 - Apply patch 5 - Repeat 6 - SUCCESS! https://bugs.koha-community.org/show_bug.cgi?id=18416 Signed-off-by: David Nind --- t/db_dependent/Circulation.t | 47 ++++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 6258ed80a8..97ca6492e2 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -329,7 +329,7 @@ subtest "GetIssuingCharges tests" => sub { my ( $reused_itemnumber_1, $reused_itemnumber_2 ); subtest "CanBookBeRenewed tests" => sub { - plan tests => 89; + plan tests => 95; C4::Context->set_preference('ItemsDeniedRenewal',''); # Generate test biblio @@ -464,6 +464,15 @@ subtest "CanBookBeRenewed tests" => sub { rule_value => '1', } ); + Koha::CirculationRules->set_rule( + { + categorycode => undef, + branchcode => undef, + itemtype => undef, + rule_name => 'renewalsallowed', + rule_value => '5', + } + ); t::lib::Mocks::mock_preference('AllowRenewalIfOtherItemsAvailable', 1 ); ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); is( $renewokay, 1, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); @@ -567,6 +576,7 @@ subtest "CanBookBeRenewed tests" => sub { is( $renewokay, 1, '(Bug 8236), Can renew, user is not restricted'); ( $renewokay, $error ) = CanBookBeRenewed($restricted_borrowernumber, $item_5->itemnumber); is( $renewokay, 0, '(Bug 8236), Cannot renew, user is restricted'); + is( $error, 'restriction', "Correct error returned"); # Users cannot renew an overdue item my $item_6 = $builder->build_sample_item( @@ -643,6 +653,27 @@ subtest "CanBookBeRenewed tests" => sub { isnt( $fines->next->status, 'UNRETURNED', 'Fine on renewed item is closed out properly' ); $fines->delete(); + t::lib::Mocks::mock_preference('OverduesBlockRenewing','allow'); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber); + is( $renewokay, 1, '(Bug 8236), Can renew, this item is not overdue'); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber); + is( $renewokay, 1, '(Bug 8236), Can renew, this item is overdue but not pref does not block'); + + t::lib::Mocks::mock_preference('OverduesBlockRenewing','block'); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber); + is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is not overdue but patron has overdues'); + is( $error, 'overdue', "Correct error returned"); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber); + is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is overdue so patron has overdues'); + is( $error, 'overdue', "Correct error returned"); + + t::lib::Mocks::mock_preference('OverduesBlockRenewing','blockitem'); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber); + is( $renewokay, 1, '(Bug 8236), Can renew, this item is not overdue'); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber); + is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is overdue'); + is( $error, 'overdue', "Correct error returned"); + my $old_issue_log_size = Koha::ActionLogs->count( \%params_issue ); my $old_renew_log_size = Koha::ActionLogs->count( \%params_renewal ); @@ -655,13 +686,6 @@ subtest "CanBookBeRenewed tests" => sub { $fines = Koha::Account::Lines->search( { borrowernumber => $renewing_borrower->{borrowernumber}, itemnumber => $item_7->itemnumber } ); $fines->delete(); - t::lib::Mocks::mock_preference('OverduesBlockRenewing','blockitem'); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber); - is( $renewokay, 1, '(Bug 8236), Can renew, this item is not overdue'); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber); - is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is overdue'); - - $hold = Koha::Holds->search({ biblionumber => $biblio->biblionumber, borrowernumber => $reserving_borrowernumber })->next; $hold->cancel; @@ -1316,13 +1340,6 @@ subtest "CanBookBeRenewed tests" => sub { my $units = C4::Overdues::get_chargeable_units('days', $future, $now, $library2->{branchcode}); ok( $units == 0, '_get_chargeable_units returns 0 for items not past due date (Bug 12596)' ); - # Users cannot renew any item if there is an overdue item - t::lib::Mocks::mock_preference('OverduesBlockRenewing','block'); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber); - is( $renewokay, 0, '(Bug 8236), Cannot renew, one of the items is overdue'); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber); - is( $renewokay, 0, '(Bug 8236), Cannot renew, one of the items is overdue'); - my $manager = $builder->build_object({ class => "Koha::Patrons" }); t::lib::Mocks::mock_userenv({ patron => $manager,branchcode => $manager->branchcode }); t::lib::Mocks::mock_preference('WhenLostChargeReplacementFee','1'); -- 2.11.0