From c76dc7fb08f4db244e6c7e2a6675a13d12cddba4 Mon Sep 17 00:00:00 2001 From: Matt Blenkinsop Date: Tue, 23 Jan 2024 14:29:35 +0000 Subject: [PATCH] Bug 28924: Add unit tests for new methods and update existing tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds unit tests for the two new methods introduced and also updates all unit tests that use the noissuecharge sysprefs Test plan: 1. Choose a patron and note their patron category 2. Create a manual invoice on that patron for 6.00 3. Try to check an item out to the patron and it will show a message saying that checkout is blocked because the fine balance is over the limit. 4. This is because the default global value for the ‘noissuescharge’ is 5.00 and we are now over that limit 5. Navigate to Administration > Patron categories and click to edit the category relevant to your patron 6. At the bottom of the form will be fields called No Issues Charge, No Issues Charge Guarantees and No Issues Charge Guarantors With Guarantees. Set those fields to 7 7. Now try and checkout again - this time you will be allowed to checkout as the category level limit is higher than the fine we set. 8. Choose a different patron (patron 2) with a category that is different to the first patron (patron 1). 9. Repeat steps 2 and 3. This time checkout will be blocked again because the limit for this patron category is still the global value of 5.00 10. Choose a third patron with a category of child or similar (patron 3) 11. Make patron 1 a guarantor for patron 3 12. Edit patron 3’s patron category and set the limit for No Issues Charge, No Issues Charge Guarantees and No Issues Charge Guarantors With Guarantees to 5. 13. Try to check out to patron 3 and it will be blocked due to the fines on patron 1’s account 14. Try to checkout to patron 1 - it should still be possible 15. Add a fine to patron 3’s account for 2.00 16. Try to check out to patron 1 - the checkout will be blocked due to the total of 8 now owed by the patron and its guarantees Test plan: prove t/db_dependent/SIP/Transaction.t prove t/db_dependent/SIP/Patron.t prove t/db_dependent/Circulation/NoIssuesChargeGuarantees.t prove t/db_dependent/Koha/Patron.t Sponsored-by: Cuyahoga County Public Library Signed-off-by: David Nind Signed-off-by: Nick Clemens Signed-off-by: Katrin Fischer --- .../Circulation/NoIssuesChargeGuarantees.t | 16 +- t/db_dependent/Koha/Patron.t | 242 +++++++++++++++++- t/db_dependent/SIP/Patron.t | 47 +++- t/db_dependent/SIP/Transaction.t | 23 ++ 4 files changed, 322 insertions(+), 6 deletions(-) diff --git a/t/db_dependent/Circulation/NoIssuesChargeGuarantees.t b/t/db_dependent/Circulation/NoIssuesChargeGuarantees.t index 4d24c93f5f..af88af096f 100755 --- a/t/db_dependent/Circulation/NoIssuesChargeGuarantees.t +++ b/t/db_dependent/Circulation/NoIssuesChargeGuarantees.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 6; +use Test::More tests => 7; use t::lib::TestBuilder; use t::lib::Mocks; @@ -35,7 +35,15 @@ my $builder = t::lib::TestBuilder->new(); my $item = $builder->build_sample_item; -my $patron_category = $builder->build({ source => 'Category', value => { categorycode => 'NOT_X', category_type => 'P', enrolmentfee => 0 } }); +my $patron_category = $builder->build( + { + source => 'Category', + value => { + categorycode => 'NOT_X', category_type => 'P', enrolmentfee => 0, noissueschargeguarantees => 0, + noissuescharge => 0, noissueschargeguarantorswithguarantees => 0 + } + } +); my $patron = $builder->build_object( { class => 'Koha::Patrons', @@ -76,6 +84,10 @@ $account->add_debit({ amount => 10.00, type => 'LOST', interface => 'test' }); ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $patron, $item->barcode ); is( $issuingimpossible->{DEBT_GUARANTEES} + 0, '10.00' + 0, "Patron cannot check out item due to debt for guarantee" ); +$patron->category->noissueschargeguarantees(11); +( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $patron, $item->barcode ); +is( $issuingimpossible->{DEBT_GUARANTEES}, undef, "Patron can check out item as the patron category limit is now higher than 10" ); + my $accountline = Koha::Account::Lines->search({ borrowernumber => $guarantee->id })->next(); is( $accountline->amountoutstanding+0, 10, "Found 10.00 amount outstanding" ); is( $accountline->debit_type_code, "LOST", "Debit type is LOST" ); diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index bad1cc1bfa..bfc1938996 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 33; +use Test::More tests => 35; use Test::Exception; use Test::Warn; use Time::Fake; @@ -2246,6 +2246,246 @@ subtest 'guarantor requirements tests' => sub { throws_ok { $child2->store(); } 'Koha::Exceptions::Patron::Relationship::NoGuarantor', 'Exception thrown when guarantor is deleted.'; + + $schema->storage->txn_rollback; +}; + +subtest 'can_borrow() tests' => sub { + plan tests => 11; + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'borrowerRelationship', 'parent' ); + + my $patron_category = $builder->build( + { + source => 'Category', + value => { + categorycode => 'NOT_X', category_type => 'P', enrolmentfee => 0, noissueschargeguarantees => 0, + noissuescharge => 10, noissueschargeguarantorswithguarantees => 0 + } + } + ); + + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $patron_category->{categorycode}, + } + } + ); + my $patron_borrowing_status; + + $patron->debarred(1); + $patron_borrowing_status = $patron->can_borrow( { patron => $patron } ); + is( $patron_borrowing_status->{can_borrow}, 0, 'Debarred patron blocked from borrowing' ); + is( $patron_borrowing_status->{debarred}, 1, 'Blocker correctly identified and returned' ); + $patron->debarred(0); + + $patron->dateexpiry( dt_from_string->subtract( days => 1 ) ); + $patron_borrowing_status = $patron->can_borrow( { patron => $patron } ); + is( $patron_borrowing_status->{can_borrow}, 0, 'Expired patron blocked from borrowing' ); + is( $patron_borrowing_status->{expired}, 1, 'Blocker correctly identified and returned' ); + $patron->dateexpiry(undef); + + my $child = $builder->build_object( { class => 'Koha::Patrons' } ); + my $sibling = $builder->build_object( { class => 'Koha::Patrons' } ); + $child->add_guarantor( { guarantor_id => $patron->borrowernumber, relationship => 'parent' } ); + $sibling->add_guarantor( { guarantor_id => $patron->borrowernumber, relationship => 'parent' } ); + + t::lib::Mocks::mock_preference( 'noissuescharge', 50 ); + + my $fee1 = $builder->build_object( + { + class => 'Koha::Account::Lines', + value => { + borrowernumber => $patron->borrowernumber, + amountoutstanding => 11, + debit_type_code => 'OVERDUE', + } + } + )->store; + + my $fee2 = $builder->build_object( + { + class => 'Koha::Account::Lines', + value => { + borrowernumber => $child->borrowernumber, + amountoutstanding => 0.11, + debit_type_code => 'OVERDUE', + } + } + )->store; + + my $fee3 = $builder->build_object( + { + class => 'Koha::Account::Lines', + value => { + borrowernumber => $sibling->borrowernumber, + amountoutstanding => 11.11, + debit_type_code => 'OVERDUE', + } + } + )->store; + + $patron_borrowing_status = $patron->can_borrow( { patron => $patron } ); + + is( $patron_borrowing_status->{noissuescharge}->{charge}, 11, "Only patron's fines are reported in total" ); + is( $patron_borrowing_status->{noissuescharge}->{limit}, 10, "Limit correctly identified at category level" ); + is( $patron_borrowing_status->{noissuescharge}->{overlimit}, 1, "Patron is over the charge limit" ); + is( $patron_borrowing_status->{can_borrow}, 0, "Patron is over the charge limit and is blocked from borrowing" ); + $patron->category->noissuescharge(undef); + + $patron_borrowing_status = $patron->can_borrow( { patron => $patron } ); + is( $patron_borrowing_status->{noissuescharge}->{limit}, 50, "Limit correctly identified at global syspref level" ); + is( $patron_borrowing_status->{noissuescharge}->{overlimit}, 0, "Patron is within the charge limit" ); + is( $patron_borrowing_status->{can_borrow}, 1, "Patron is within the charge limit and can borrow" ); + + $schema->storage->txn_rollback; +}; + +subtest 'is_patron_inside_charge_limits() tests' => sub { + plan tests => 21; + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'borrowerRelationship', 'parent' ); + + my $patron_category = $builder->build( + { + source => 'Category', + value => { + categorycode => 'NOT_X', category_type => 'P', enrolmentfee => 0, noissueschargeguarantees => 0, + noissuescharge => 10, noissueschargeguarantorswithguarantees => 0 + } + } + ); + + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $patron_category->{categorycode}, + } + } + ); + + my $child = $builder->build_object( { class => 'Koha::Patrons' } ); + my $sibling = $builder->build_object( { class => 'Koha::Patrons' } ); + $child->add_guarantor( { guarantor_id => $patron->borrowernumber, relationship => 'parent' } ); + $sibling->add_guarantor( { guarantor_id => $patron->borrowernumber, relationship => 'parent' } ); + + t::lib::Mocks::mock_preference( 'noissuescharge', 50 ); + t::lib::Mocks::mock_preference( 'NoIssuesChargeGuarantees', 11.01 ); + t::lib::Mocks::mock_preference( 'NoIssuesChargeGuarantorsWithGuarantees', undef ); + + my $fee1 = $builder->build_object( + { + class => 'Koha::Account::Lines', + value => { + borrowernumber => $patron->borrowernumber, + amountoutstanding => 11, + debit_type_code => 'OVERDUE', + } + } + )->store; + + my $fee2 = $builder->build_object( + { + class => 'Koha::Account::Lines', + value => { + borrowernumber => $child->borrowernumber, + amountoutstanding => 0.11, + debit_type_code => 'OVERDUE', + } + } + )->store; + + my $fee3 = $builder->build_object( + { + class => 'Koha::Account::Lines', + value => { + borrowernumber => $sibling->borrowernumber, + amountoutstanding => 11.11, + debit_type_code => 'OVERDUE', + } + } + )->store; + + my $patron_borrowing_status; + $patron_borrowing_status = $patron->is_patron_inside_charge_limits( { patron => $patron } ); + + is( $patron_borrowing_status->{noissuescharge}->{charge}, 11, "Only patron's fines are reported in total" ); + is( $patron_borrowing_status->{noissuescharge}->{limit}, 10, "Limit correctly identified at category level" ); + is( $patron_borrowing_status->{noissuescharge}->{overlimit}, 1, "Patron is over the charge limit" ); + + $patron->category->noissuescharge(undef); + $patron_borrowing_status = $patron->is_patron_inside_charge_limits( { patron => $patron } ); + is( $patron_borrowing_status->{noissuescharge}->{limit}, 50, "Limit correctly identified at global syspref level" ); + is( $patron_borrowing_status->{noissuescharge}->{charge}, 11, "Charges correctly identified" ); + is( $patron_borrowing_status->{noissuescharge}->{overlimit}, 0, "Patron is within the charge limit" ); + + is( + $patron_borrowing_status->{NoIssuesChargeGuarantees}->{limit}, 11.01, + "Limit correctly identified at global syspref level" + ); + is( $patron_borrowing_status->{NoIssuesChargeGuarantees}->{charge}, 11.22, "Charges correctly identified" ); + is( $patron_borrowing_status->{NoIssuesChargeGuarantees}->{overlimit}, 1, "Patron is over the charge limit" ); + + $patron->category->noissueschargeguarantees(12); + $patron_borrowing_status = $patron->is_patron_inside_charge_limits( { patron => $patron } ); + is( + $patron_borrowing_status->{NoIssuesChargeGuarantees}->{limit}, 12, + "Limit correctly identified at patron category level" + ); + is( $patron_borrowing_status->{NoIssuesChargeGuarantees}->{charge}, 11.22, "Charges correctly identified" ); + is( $patron_borrowing_status->{NoIssuesChargeGuarantees}->{overlimit}, 0, "Patron is inside the charge limit" ); + + is( + $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{limit}, 0, + "Limit correctly identified as not set at either patron category or global syspref level" + ); + is( + $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{charge}, 22.22, + "Charges correctly identified" + ); + is( + $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{overlimit}, 0, + "Patron is inside the charge limit as no limit has been set" + ); + + $patron->category->noissueschargeguarantorswithguarantees(23); + t::lib::Mocks::mock_preference( 'NoIssuesChargeGuarantorsWithGuarantees', 20 ); + $patron_borrowing_status = $patron->is_patron_inside_charge_limits( { patron => $patron } ); + is( + $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{limit}, 23, + "Limit correctly identified at patron category level" + ); + is( + $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{charge}, 22.22, + "Charges correctly identified" + ); + is( + $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{overlimit}, 0, + "Patron is inside the charge limit" + ); + + $patron->category->noissueschargeguarantorswithguarantees(undef); + t::lib::Mocks::mock_preference( 'NoIssuesChargeGuarantorsWithGuarantees', 20 ); + $patron_borrowing_status = $patron->is_patron_inside_charge_limits( { patron => $patron } ); + is( + $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{limit}, 20, + "Limit correctly defaults to global syspref" + ); + is( + $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{charge}, 22.22, + "Charges correctly identified" + ); + is( + $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{overlimit}, 1, + "Patron is inside the charge limit" + ); + + $schema->storage->txn_rollback; }; subtest 'Scrub the note fields' => sub { diff --git a/t/db_dependent/SIP/Patron.t b/t/db_dependent/SIP/Patron.t index 67e81124bf..2c95245ed2 100755 --- a/t/db_dependent/SIP/Patron.t +++ b/t/db_dependent/SIP/Patron.t @@ -316,7 +316,24 @@ subtest "NoIssuesChargeGuarantees tests" => sub { $schema->storage->txn_begin; - my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + my $patron_category = $builder->build( + { + source => 'Category', + value => { + categorycode => 'NOT_X', category_type => 'P', enrolmentfee => 0, noissueschargeguarantees => 0, + noissuescharge => 0, noissueschargeguarantorswithguarantees => 0 + } + } + ); + + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $patron_category->{categorycode}, + } + } + ); my $child = $builder->build_object({ class => 'Koha::Patrons' }); my $sibling = $builder->build_object({ class => 'Koha::Patrons' }); $child->add_guarantor({ guarantor_id => $patron->borrowernumber, relationship => 'parent' }); @@ -382,8 +399,32 @@ subtest "NoIssuesChargeGuarantorsWithGuarantees tests" => sub { $schema->storage->txn_begin; - my $patron = $builder->build_object({ class => 'Koha::Patrons' }); - my $child = $builder->build_object({ class => 'Koha::Patrons' }); + my $patron_category = $builder->build( + { + source => 'Category', + value => { + categorycode => 'NOT_X', category_type => 'P', enrolmentfee => 0, noissueschargeguarantees => 0, + noissuescharge => 0, noissueschargeguarantorswithguarantees => 0 + } + } + ); + + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $patron_category->{categorycode}, + } + } + ); + my $child = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $patron_category->{categorycode}, + } + } + ); $child->add_guarantor({ guarantor_id => $patron->borrowernumber, relationship => 'parent' }); t::lib::Mocks::mock_preference('noissuescharge', 50); diff --git a/t/db_dependent/SIP/Transaction.t b/t/db_dependent/SIP/Transaction.t index b23b002545..4b2c231598 100755 --- a/t/db_dependent/SIP/Transaction.t +++ b/t/db_dependent/SIP/Transaction.t @@ -645,11 +645,22 @@ subtest do_checkout_with_sysprefs_override => sub { } ); + my $patron_category = $builder->build( + { + source => 'Category', + value => { + categorycode => 'NOT_X', category_type => 'P', enrolmentfee => 0, noissueschargeguarantees => 0, + noissuescharge => 0, noissueschargeguarantorswithguarantees => 0 + } + } + ); + my $patron_under_noissuescharge = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library->branchcode, + categorycode => $patron_category->{categorycode}, } } ); @@ -671,6 +682,7 @@ subtest do_checkout_with_sysprefs_override => sub { class => 'Koha::Patrons', value => { branchcode => $library->branchcode, + categorycode => $patron_category->{categorycode}, } } ); @@ -780,6 +792,16 @@ subtest do_checkout_with_patron_blocked => sub { } ); + my $patron_category = $builder->build( + { + source => 'Category', + value => { + categorycode => 'NOT_X2', category_type => 'P', enrolmentfee => 0, noissueschargeguarantees => 0, + noissuescharge => 0, noissueschargeguarantorswithguarantees => 0 + } + } + ); + my $expired_patron = $builder->build_object( { class => 'Koha::Patrons', @@ -797,6 +819,7 @@ subtest do_checkout_with_patron_blocked => sub { class => 'Koha::Patrons', value => { branchcode => $library->branchcode, + categorycode => $patron_category->{categorycode}, } } );