Bugzilla – Attachment 190700 Details for
Bug 41404
No need to check related guarantor/guarantee charges when the limits are not set
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 41404: (QA follow-up) Add tests for empty string and zero edge cases
f778c5d.patch (text/plain), 6.79 KB, created by
Martin Renvoize (ashimema)
on 2025-12-23 14:24:42 UTC
(
hide
)
Description:
Bug 41404: (QA follow-up) Add tests for empty string and zero edge cases
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-12-23 14:24:42 UTC
Size:
6.79 KB
patch
obsolete
>From f778c5deedfd13304c7fcb9fe8f42b7c8e1d7559 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Tue, 23 Dec 2025 14:18:52 +0000 >Subject: [PATCH] Bug 41404: (QA follow-up) Add tests for empty string and zero > edge cases > >This QA follow-up adds comprehensive test coverage for edge cases that >the original patch handles: > >1. Empty string values ('') - the default value in sysprefs > - Verifies that empty strings are treated as "not set" > - Confirms charge calculations are skipped (performance optimization) > - Ensures patrons are not blocked > >2. Zero values (0) - valid numeric limit > - Verifies that zero triggers charge calculation (valid number) > - Confirms patrons are not blocked (0 means "no limit") > - Documents intended behavior that 0 disables blocking > >3. Positive numeric values - normal case > - Confirms charges are calculated and blocking works correctly > >These tests validate that looks_like_number() is the correct approach >rather than simple truthiness checking, as it properly distinguishes >between: >- Empty string '' (not a number, skip calculation) >- Zero 0 (is a number, calculate but don't block) > >Test plan: >1. Apply this patch >2. Run the tests: > prove t/db_dependent/Koha/Patron.t >3. Verify all tests pass > >Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> >--- > t/db_dependent/Koha/Patron.t | 127 ++++++++++++++++++++++++++++++++++- > 1 file changed, 126 insertions(+), 1 deletion(-) > >diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t >index 8a6769cde00..9f166cc52c6 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 => 45; >+use Test::More tests => 46; > use Test::NoWarnings; > use Test::Exception; > use Test::Warn; >@@ -2952,6 +2952,131 @@ subtest 'is_patron_inside_charge_limits() tests' => sub { > $schema->storage->txn_rollback; > }; > >+subtest 'is_patron_inside_charge_limits - edge cases for empty string and zero' => sub { >+ plan tests => 14; >+ >+ $schema->storage->txn_begin; >+ >+ my $patron_category = $builder->build( >+ { >+ source => 'Category', >+ value => { >+ categorycode => 'TEST_CAT', >+ category_type => 'P', >+ enrolmentfee => 0, >+ noissuescharge => undef, >+ noissueschargeguarantees => undef, >+ noissueschargeguarantorswithguarantees => undef >+ } >+ } >+ ); >+ >+ my $patron = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { categorycode => $patron_category->{categorycode} } >+ } >+ ); >+ >+ my $child = $builder->build_object( { class => 'Koha::Patrons' } ); >+ $child->add_guarantor( { guarantor_id => $patron->borrowernumber, relationship => 'parent' } ); >+ >+ # Add some charges to the child >+ my $fee = $builder->build_object( >+ { >+ class => 'Koha::Account::Lines', >+ value => { >+ borrowernumber => $child->borrowernumber, >+ amountoutstanding => 5.50, >+ debit_type_code => 'OVERDUE', >+ } >+ } >+ )->store; >+ >+ # Test 1: Empty string values should be treated as "not set" >+ # Empty string is the default value in sysprefs for these preferences >+ t::lib::Mocks::mock_preference( 'NoIssuesChargeGuarantees', '' ); >+ t::lib::Mocks::mock_preference( 'NoIssuesChargeGuarantorsWithGuarantees', '' ); >+ >+ my $patron_borrowing_status = $patron->is_patron_inside_charge_limits(); >+ >+ is( >+ $patron_borrowing_status->{NoIssuesChargeGuarantees}->{limit}, '', >+ "Empty string limit is preserved in return value" >+ ); >+ is( >+ $patron_borrowing_status->{NoIssuesChargeGuarantees}->{charge}, 0, >+ "Charges should be 0 when limit is empty string (calculation skipped)" >+ ); >+ is( >+ $patron_borrowing_status->{NoIssuesChargeGuarantees}->{overlimit}, 0, >+ "Patron should not be overlimit when preference is empty string" >+ ); >+ >+ is( >+ $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{limit}, '', >+ "Empty string limit is preserved for guarantors preference" >+ ); >+ is( >+ $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{charge}, 0, >+ "Guarantors charges should be 0 when limit is empty string (calculation skipped)" >+ ); >+ is( >+ $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{overlimit}, 0, >+ "Patron should not be overlimit when guarantors preference is empty string" >+ ); >+ >+ # Test 2: Zero values should trigger calculation but not block >+ t::lib::Mocks::mock_preference( 'NoIssuesChargeGuarantees', 0 ); >+ t::lib::Mocks::mock_preference( 'NoIssuesChargeGuarantorsWithGuarantees', 0 ); >+ >+ $patron_borrowing_status = $patron->is_patron_inside_charge_limits(); >+ >+ is( >+ $patron_borrowing_status->{NoIssuesChargeGuarantees}->{limit}, 0, >+ "Zero limit is preserved in return value" >+ ); >+ is( >+ $patron_borrowing_status->{NoIssuesChargeGuarantees}->{charge}, 5.50, >+ "Charges should be calculated when limit is 0 (valid number)" >+ ); >+ is( >+ $patron_borrowing_status->{NoIssuesChargeGuarantees}->{overlimit}, 0, >+ "Patron should not be overlimit when limit is 0 (0 means no blocking)" >+ ); >+ >+ is( >+ $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{limit}, 0, >+ "Zero limit is preserved for guarantors preference" >+ ); >+ >+ # Note: This will be 5.50 because the child is included in guarantor debt calculation >+ ok( >+ $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{charge} > 0, >+ "Guarantors charges should be calculated when limit is 0 (valid number)" >+ ); >+ is( >+ $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{overlimit}, 0, >+ "Patron should not be overlimit when guarantors limit is 0 (0 means no blocking)" >+ ); >+ >+ # Test 3: Verify that a small positive number triggers calculation and can block >+ t::lib::Mocks::mock_preference( 'NoIssuesChargeGuarantees', 5.00 ); >+ >+ $patron_borrowing_status = $patron->is_patron_inside_charge_limits(); >+ >+ is( >+ $patron_borrowing_status->{NoIssuesChargeGuarantees}->{charge}, 5.50, >+ "Charges are calculated with positive limit" >+ ); >+ is( >+ $patron_borrowing_status->{NoIssuesChargeGuarantees}->{overlimit}, 1, >+ "Patron is correctly marked overlimit when charges exceed limit" >+ ); >+ >+ $schema->storage->txn_rollback; >+}; >+ > subtest 'Scrub the note fields' => sub { > plan tests => 4; > >-- >2.52.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 41404
:
190307
|
190313
|
190322
|
190699
| 190700