From 336f07ec320b986b3ea295b54458909215e7ef28 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 18 Nov 2021 16:11:24 +0000 Subject: [PATCH] Bug 29483: Unit tests This patch updates the AllowRenewalIfOtherItemsAvailable tests to remove deletion of all data, and create specific circ rules for this test. It adjust several other tests that were relying on the rules from this test, so thy too create their opwn specific rules. Additionally, we add tests to cover the case of mutliple items on the record, and some items cannot fill some reserves. What is uncovered here is that the same patron is checked twice, so two holds can be filled, but they only satisfy a single patron --- t/db_dependent/Circulation.t | 167 +++++++++++++++++++++++++++-------- 1 file changed, 130 insertions(+), 37 deletions(-) diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 3000753a92..13cf657092 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -1574,16 +1574,30 @@ subtest "Bug 13841 - Do not create new 0 amount fines" => sub { }; subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { - $dbh->do('DELETE FROM issues'); - $dbh->do('DELETE FROM items'); - $dbh->do('DELETE FROM circulation_rules'); + plan tests => 9; + my $biblio = $builder->build_sample_biblio(); + my $item_1 = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + library => $library2->{branchcode}, + } + ); + my $item_2= $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + library => $library2->{branchcode}, + itype => $item_1->effective_itemtype, + } + ); + Koha::CirculationRules->set_rules( { categorycode => undef, - itemtype => undef, + itemtype => $item_1->effective_itemtype, branchcode => undef, rules => { reservesallowed => 25, + holds_per_record => 25, issuelength => 14, lengthunit => 'days', renewalsallowed => 1, @@ -1596,23 +1610,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { } } ); - my $biblio = $builder->build_sample_biblio(); - my $item_1 = $builder->build_sample_item( - { - biblionumber => $biblio->biblionumber, - library => $library2->{branchcode}, - itype => $itemtype, - } - ); - - my $item_2= $builder->build_sample_item( - { - biblionumber => $biblio->biblionumber, - library => $library2->{branchcode}, - itype => $itemtype, - } - ); my $borrowernumber1 = Koha::Patron->new({ firstname => 'Kyle', @@ -1626,6 +1624,22 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { categorycode => $patron_category->{categorycode}, branchcode => $library2->{branchcode}, })->store->borrowernumber; + my $patron_category_2 = $builder->build( + { + source => 'Category', + value => { + category_type => 'P', + enrolmentfee => 0, + BlockExpiredPatronOpacActions => -1, # Pick the pref value + } + } + ); + my $borrowernumber3 = Koha::Patron->new({ + firstname => 'Carnegie', + surname => 'Hall', + categorycode => $patron_category_2->{categorycode}, + branchcode => $library2->{branchcode}, + })->store->borrowernumber; my $borrower1 = Koha::Patrons->find( $borrowernumber1 )->unblessed; my $borrower2 = Koha::Patrons->find( $borrowernumber2 )->unblessed; @@ -1647,7 +1661,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { Koha::CirculationRules->set_rules( { categorycode => undef, - itemtype => undef, + itemtype => $item_1->effective_itemtype, branchcode => undef, rules => { onshelfholds => 0, @@ -1658,16 +1672,6 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); is( $renewokay, 0, 'Bug 14337 - Verify the borrower cannot renew with a hold on the record if AllowRenewalIfOtherItemsAvailable and onshelfholds are disabled' ); - Koha::CirculationRules->set_rules( - { - categorycode => undef, - itemtype => undef, - branchcode => undef, - rules => { - onshelfholds => 0, - } - } - ); t::lib::Mocks::mock_preference( 'AllowRenewalIfOtherItemsAvailable', 1 ); ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); is( $renewokay, 0, 'Bug 14337 - Verify the borrower cannot renew with a hold on the record if AllowRenewalIfOtherItemsAvailable is enabled and onshelfholds is disabled' ); @@ -1675,7 +1679,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { Koha::CirculationRules->set_rules( { categorycode => undef, - itemtype => undef, + itemtype => $item_1->effective_itemtype, branchcode => undef, rules => { onshelfholds => 1, @@ -1686,25 +1690,55 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); is( $renewokay, 0, 'Bug 14337 - Verify the borrower cannot renew with a hold on the record if AllowRenewalIfOtherItemsAvailable is disabled and onshelfhold is enabled' ); + t::lib::Mocks::mock_preference( 'AllowRenewalIfOtherItemsAvailable', 1 ); + ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); + is( $renewokay, 1, 'Bug 14337 - Verify the borrower can renew with a hold on the record if AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled' ); + + AddReserve( + { + branchcode => $library2->{branchcode}, + borrowernumber => $borrowernumber3, + biblionumber => $biblio->biblionumber, + priority => 1, + } + ); + + ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); + is( $renewokay, 0, 'Verify the borrower cannot renew with 2 holds on the record if AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled and one other item on record' ); + + my $item_3= $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + library => $library2->{branchcode}, + itype => $item_1->effective_itemtype, + } + ); + + ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); + is( $renewokay, 1, 'Verify the borrower cannot renew with 2 holds on the record if AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled and two other items on record' ); + Koha::CirculationRules->set_rules( { - categorycode => undef, - itemtype => undef, + categorycode => $patron_category_2->{categorycode}, + itemtype => $item_1->effective_itemtype, branchcode => undef, rules => { - onshelfholds => 1, + reservesallowed => 0, } } ); - t::lib::Mocks::mock_preference( 'AllowRenewalIfOtherItemsAvailable', 1 ); + ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); - is( $renewokay, 1, 'Bug 14337 - Verify the borrower can renew with a hold on the record if AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled' ); + is( $renewokay, 0, 'Verify the borrower cannot renew with 2 holds on the record, but only one of those holds can be filled when AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled and two other items on record' ); # Setting item not checked out to be not for loan but holdable $item_2->notforloan(-1)->store; ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); is( $renewokay, 0, 'Bug 14337 - Verify the borrower can not renew with a hold on the record if AllowRenewalIfOtherItemsAvailable is enabled but the only available item is notforloan' ); + +$schema->storage->txn_rollback; +die "barf"; }; { @@ -1789,6 +1823,25 @@ subtest 'CanBookBeIssued & AllowReturnToBranch' => sub { holdingbranch => $holdingbranch->{branchcode}, } ); + Koha::CirculationRules->set_rules( + { + categorycode => undef, + itemtype => $item->effective_itemtype, + branchcode => undef, + rules => { + reservesallowed => 25, + issuelength => 14, + lengthunit => 'days', + renewalsallowed => 1, + renewalperiod => 7, + norenewalbefore => undef, + auto_renew => 0, + fine => .10, + chargeperiod => 1, + maxissueqty => 20 + } + } + ); set_userenv($holdingbranch); @@ -1942,6 +1995,26 @@ subtest 'CanBookBeIssued + Koha::Patron->is_debarred|has_overdues' => sub { library => $library->{branchcode}, } ); + Koha::CirculationRules->set_rules( + { + categorycode => undef, + itemtype => undef, + branchcode => $library->{branchcode}, + rules => { + reservesallowed => 25, + issuelength => 14, + lengthunit => 'days', + renewalsallowed => 1, + renewalperiod => 7, + norenewalbefore => undef, + auto_renew => 0, + fine => .10, + chargeperiod => 1, + maxissueqty => 20 + } + } + ); + my ( $error, $question, $alerts ); @@ -2139,6 +2212,26 @@ subtest 'CanBookBeIssued + AllowMultipleIssuesOnABiblio' => sub { } ); + Koha::CirculationRules->set_rules( + { + categorycode => undef, + itemtype => undef, + branchcode => $library->{branchcode}, + rules => { + reservesallowed => 25, + issuelength => 14, + lengthunit => 'days', + renewalsallowed => 1, + renewalperiod => 7, + norenewalbefore => undef, + auto_renew => 0, + fine => .10, + chargeperiod => 1, + maxissueqty => 20 + } + } + ); + my ( $error, $question, $alerts ); my $issue = AddIssue( $patron->unblessed, $item_1->barcode, dt_from_string->add( days => 1 ) ); -- 2.20.1