From a74ac1ff8dfa35fe80192bd57f5a88c319f7ffc5 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 2 Mar 2026 08:11:57 +0000 Subject: [PATCH] Bug 4858: (QA follow-up) Miscellaneous fixes - Add print_notice_charge to Koha::Patron::Category to_api_mapping. - Remove over-defensive branchcode and letter_code regex validation in gather_print_notices.pl::apply_print_notice_charge; the data is internal (from GetPrintMessages DB query) and the regex could silently skip charges for libraries with spaces or special characters in their branchcode. - Fix cronlogaction indentation inside apply_print_notice_charge. - Move test file from t/db_dependent/Koha/Account/PrintNoticeCharges.t to t/db_dependent/Koha/Patron/PrintNoticeCharge.t to match the class being tested (Koha::Patron, not Koha::Account::PrintNoticeCharges). - Add missing trailing newline to t/db_dependent/PrintNoticeCharging_EndToEnd.t. Sponsored-by: OpenFifth Signed-off-by: Jackie Usher --- Koha/Patron/Category.pm | 3 +- misc/cronjobs/gather_print_notices.pl | 57 ++- .../Koha/Account/PrintNoticeCharges.t | 274 -------------- .../Koha/Patron/PrintNoticeCharge.t | 309 +++++++++++++++ t/db_dependent/PrintNoticeCharging_EndToEnd.t | 351 ++++++++++-------- 5 files changed, 530 insertions(+), 464 deletions(-) delete mode 100755 t/db_dependent/Koha/Account/PrintNoticeCharges.t create mode 100755 t/db_dependent/Koha/Patron/PrintNoticeCharge.t diff --git a/Koha/Patron/Category.pm b/Koha/Patron/Category.pm index cc78f750e1d..74591908248 100644 --- a/Koha/Patron/Category.pm +++ b/Koha/Patron/Category.pm @@ -325,7 +325,8 @@ sub to_api_mapping { exclude_from_local_holds_priority => 'exclude_from_local_holds_priority', noissuescharge => 'no_issues_charge', noissueschargeguarantees => 'no_issues_charge_guarantees', - noissueschargeguarantorswithguarantees => 'no_issues_charge_guarantors_with_guarantees' + noissueschargeguarantorswithguarantees => 'no_issues_charge_guarantors_with_guarantees', + print_notice_charge => 'print_notice_charge' }; } diff --git a/misc/cronjobs/gather_print_notices.pl b/misc/cronjobs/gather_print_notices.pl index 4e7b16e4add..a0694ac30a1 100755 --- a/misc/cronjobs/gather_print_notices.pl +++ b/misc/cronjobs/gather_print_notices.pl @@ -202,10 +202,11 @@ sub print_notices { ); } - if ( $send ) { - foreach my $message ( @$branch_messages ) { + if ($send) { + foreach my $message (@$branch_messages) { + # Apply print notice charges if enabled - if (!$skip_charges) { + if ( !$skip_charges ) { apply_print_notice_charge($message); } @@ -363,50 +364,38 @@ sub apply_print_notice_charge { my ($message) = @_; # Check for valid message data - unless ($message && ref($message) eq 'HASH') { + unless ( $message && ref($message) eq 'HASH' && $message->{borrowernumber} ) { warn "Invalid message data passed to apply_print_notice_charge"; return; } - # Validate borrowernumber is numeric to prevent injection - unless ($message->{borrowernumber} && $message->{borrowernumber} =~ /^\d+$/) { - warn "Invalid or missing borrowernumber in message for print notice charge"; - return; - } - - # Validate branchcode format if present - if (defined $message->{branchcode} && $message->{branchcode} !~ /^[A-Za-z0-9_-]*$/) { - warn "Invalid branchcode format in message: " . $message->{branchcode}; - return; - } - - # Validate letter_code format if present - if (defined $message->{letter_code} && $message->{letter_code} !~ /^[A-Za-z0-9_-]*$/) { - warn "Invalid letter_code format in message: " . $message->{letter_code}; - return; - } - - # Skip if patron not found (using validated borrowernumber) - my $patron = Koha::Patrons->find($message->{borrowernumber}); + my $patron = Koha::Patrons->find( $message->{borrowernumber} ); unless ($patron) { warn "Patron " . $message->{borrowernumber} . " not found for print notice charge"; return; } eval { - my $result = $patron->add_print_notice_charge_if_needed({ - notice_code => $message->{letter_code}, - library_id => $message->{branchcode}, - }); + my $result = $patron->add_print_notice_charge_if_needed( + { + notice_code => $message->{letter_code}, + library_id => $message->{branchcode}, + } + ); # Log successful charge application if ($result) { - cronlogaction({ - action => 'Print notice charge', - info => "Applied charge for patron " . $message->{borrowernumber} . - " notice " . ($message->{letter_code} || 'unknown') . - " (charge ID: " . $result->id . ")" - }); + cronlogaction( + { + action => 'Print notice charge', + info => "Applied charge for patron " + . $message->{borrowernumber} + . " notice " + . ( $message->{letter_code} || 'unknown' ) + . " (charge ID: " + . $result->id . ")" + } + ); } }; diff --git a/t/db_dependent/Koha/Account/PrintNoticeCharges.t b/t/db_dependent/Koha/Account/PrintNoticeCharges.t deleted file mode 100755 index da2da3c4a7d..00000000000 --- a/t/db_dependent/Koha/Account/PrintNoticeCharges.t +++ /dev/null @@ -1,274 +0,0 @@ -#!/usr/bin/perl - -# Copyright 2024 Koha Development team -# -# This file is part of Koha -# -# Koha is free software; you can redistribute it and/or modify it -# under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 3 of the License, or -# (at your option) any later version. -# -# Koha is distributed in the hope that it will be useful, but -# WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Koha; if not, see - -use Modern::Perl; - -use Test::More tests => 8; -use Test::MockModule; -use Test::Exception; -use Test::Warn; - -use C4::Context; -use Koha::Account; -use Koha::Account::Lines; -use Koha::Patrons; - -use t::lib::Mocks; -use t::lib::TestBuilder; - -my $schema = Koha::Database->new->schema; -$schema->storage->dbh->{PrintError} = 0; -my $builder = t::lib::TestBuilder->new; -C4::Context->interface('commandline'); - -subtest 'add_print_notice_charge_if_needed with charging disabled' => sub { - plan tests => 3; - - $schema->storage->txn_begin; - - # Create category with print notice charging disabled (0.00) - my $category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.00 } }); - my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { categorycode => $category->categorycode } }); - my $account = Koha::Account->new({ patron_id => $patron->borrowernumber }); - - my $initial_balance = $account->balance; - - my $result = $patron->add_print_notice_charge_if_needed({ - notice_code => 'ODUE', - library_id => $patron->branchcode - }); - - is($result, undef, 'No charge applied when charging disabled'); - is($account->balance, $initial_balance, 'Account balance unchanged'); - is($account->lines->count, 0, 'No account lines created'); - - $schema->storage->txn_rollback; -}; - -subtest 'add_print_notice_charge_if_needed with charging enabled' => sub { - plan tests => 7; - - $schema->storage->txn_begin; - - # Create category with print notice charging enabled (0.75) - my $category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.75 } }); - my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { categorycode => $category->categorycode } }); - my $account = Koha::Account->new({ patron_id => $patron->borrowernumber }); - - my $initial_balance = $account->balance; - - my $result = $patron->add_print_notice_charge_if_needed({ - notice_code => 'ODUE', - library_id => $patron->branchcode - }); - - isa_ok($result, 'Koha::Account::Line', 'Returns account line object'); - is($account->balance, $initial_balance + 0.75, 'Account balance increased by charge amount'); - is($result->debit_type_code, 'PRINT_NOTICE', 'Correct debit type applied'); - cmp_ok($result->amount, '==', 0.75, 'Correct amount charged'); - cmp_ok($result->amountoutstanding, '==', 0.75, 'Full amount outstanding'); - is($result->borrowernumber, $patron->borrowernumber, 'Correct patron charged'); - is($result->interface, 'commandline', 'Correct interface recorded'); - - $schema->storage->txn_rollback; -}; - -subtest 'add_print_notice_charge_if_needed with custom amount' => sub { - plan tests => 3; - - $schema->storage->txn_begin; - - # Create category with print notice charging enabled (0.50) - my $category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.50 } }); - my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { categorycode => $category->categorycode } }); - my $account = Koha::Account->new({ patron_id => $patron->borrowernumber }); - - my $custom_amount = 1.25; - my $result = $patron->add_print_notice_charge_if_needed({ - notice_code => 'PREDUE', - library_id => $patron->branchcode, - amount => $custom_amount - }); - - isa_ok($result, 'Koha::Account::Line', 'Returns account line object'); - is($result->amount, $custom_amount, 'Custom amount used instead of system preference'); - is($account->balance, $custom_amount, 'Account balance reflects custom amount'); - - $schema->storage->txn_rollback; -}; - -subtest 'add_print_notice_charge_if_needed with zero amount' => sub { - plan tests => 3; - - $schema->storage->txn_begin; - - # Create category with charging disabled (0.00) - my $category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.00 } }); - my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { categorycode => $category->categorycode } }); - my $account = Koha::Account->new({ patron_id => $patron->borrowernumber }); - - my $result = $patron->add_print_notice_charge_if_needed({ - notice_code => 'HOLD', - library_id => $patron->branchcode - }); - - is($result, undef, 'No charge applied when amount is zero'); - is($account->balance, 0, 'Account balance unchanged'); - is($account->lines->count, 0, 'No account lines created'); - - $schema->storage->txn_rollback; -}; - -subtest 'add_print_notice_charge_if_needed validation tests' => sub { - plan tests => 3; - - $schema->storage->txn_begin; - - # Create category with print notice charging enabled - my $category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.50 } }); - my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { categorycode => $category->categorycode } }); - - # Test with invalid negative amount - my $result1 = $patron->add_print_notice_charge_if_needed({ - notice_code => 'ODUE', - library_id => $patron->branchcode, - amount => -0.50 - }); - is($result1, undef, 'No charge applied for negative amount'); - - # Test with invalid non-numeric amount - my $result2 = $patron->add_print_notice_charge_if_needed({ - notice_code => 'ODUE', - library_id => $patron->branchcode, - amount => 'invalid' - }); - is($result2, undef, 'No charge applied for non-numeric amount'); - - # Test with valid amount works - my $result3 = $patron->add_print_notice_charge_if_needed({ - notice_code => 'ODUE', - library_id => $patron->branchcode, - amount => 1.00 - }); - isa_ok($result3, 'Koha::Account::Line', 'Valid amount works correctly'); - - $schema->storage->txn_rollback; -}; - -subtest 'add_print_notice_charge_if_needed without notice code' => sub { - plan tests => 2; - - $schema->storage->txn_begin; - - # Create category with print notice charging enabled (0.50) - my $category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.50 } }); - my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { categorycode => $category->categorycode } }); - my $account = Koha::Account->new({ patron_id => $patron->borrowernumber }); - - my $result = $patron->add_print_notice_charge_if_needed({ - library_id => $patron->branchcode - }); - - isa_ok($result, 'Koha::Account::Line', 'Returns account line object even without notice code'); - cmp_ok($result->amount, '==', 0.50, 'Category amount used'); - - $schema->storage->txn_rollback; -}; - -subtest 'add_print_notice_charge_if_needed library_id handling' => sub { - plan tests => 4; - - $schema->storage->txn_begin; - - # Create category with print notice charging enabled (0.50) - my $category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.50 } }); - my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { categorycode => $category->categorycode } }); - my $account = Koha::Account->new({ patron_id => $patron->borrowernumber }); - - # Get the patron's library for testing - my $patron_library = $patron->branchcode; - - # Mock userenv for default library - my $mock_context = Test::MockModule->new('C4::Context'); - $mock_context->mock('userenv', sub { - return { branch => $patron_library }; - }); - - # Test with explicit library_id - my $result1 = $patron->add_print_notice_charge_if_needed({ - notice_code => 'ODUE', - library_id => $patron_library - }); - is($result1->branchcode, $patron_library, 'Explicit library_id used when provided'); - - # Test without library_id (should use userenv) - my $result2 = $patron->add_print_notice_charge_if_needed({ - notice_code => 'PREDUE' - }); - is($result2->branchcode, $patron_library, 'Default library from userenv used when not provided'); - - # Test with undefined userenv - $mock_context->mock('userenv', sub { return; }); - my $result3 = $patron->add_print_notice_charge_if_needed({ - notice_code => 'HOLD' - }); - isa_ok($result3, 'Koha::Account::Line', 'Still works with undefined userenv'); - is($result3->branchcode, undef, 'Library_id is undef when no userenv'); - - $schema->storage->txn_rollback; -}; - -subtest 'add_print_notice_charge_if_needed category isolation test' => sub { - plan tests => 6; - - $schema->storage->txn_begin; - - # Create two categories - one with charging enabled, one disabled - my $category_enabled = $builder->build_object({ class => 'Koha::Patron::Categories', value => { print_notice_charge => 1.25 } }); - my $category_disabled = $builder->build_object({ class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.00 } }); - - # Create patrons in each category - my $patron_enabled = $builder->build_object({ class => 'Koha::Patrons', value => { categorycode => $category_enabled->categorycode } }); - my $patron_disabled = $builder->build_object({ class => 'Koha::Patrons', value => { categorycode => $category_disabled->categorycode } }); - - my $account_enabled = Koha::Account->new({ patron_id => $patron_enabled->borrowernumber }); - my $account_disabled = Koha::Account->new({ patron_id => $patron_disabled->borrowernumber }); - - # Test patron with enabled category gets charged - my $result_enabled = $patron_enabled->add_print_notice_charge_if_needed({ - notice_code => 'ODUE', - library_id => $patron_enabled->branchcode - }); - - isa_ok($result_enabled, 'Koha::Account::Line', 'Patron with enabled category gets charged'); - cmp_ok($result_enabled->amount, '==', 1.25, 'Correct amount from enabled category'); - cmp_ok($account_enabled->balance, '==', 1.25, 'Enabled patron account balance updated'); - - # Test patron with disabled category does not get charged - my $result_disabled = $patron_disabled->add_print_notice_charge_if_needed({ - notice_code => 'ODUE', - library_id => $patron_disabled->branchcode - }); - - is($result_disabled, undef, 'Patron with disabled category not charged'); - cmp_ok($account_disabled->balance, '==', 0, 'Disabled patron account balance unchanged'); - is($account_disabled->lines->count, 0, 'No account lines for disabled category patron'); - - $schema->storage->txn_rollback; -}; diff --git a/t/db_dependent/Koha/Patron/PrintNoticeCharge.t b/t/db_dependent/Koha/Patron/PrintNoticeCharge.t new file mode 100755 index 00000000000..7e6b643b3ee --- /dev/null +++ b/t/db_dependent/Koha/Patron/PrintNoticeCharge.t @@ -0,0 +1,309 @@ +#!/usr/bin/perl + +# Copyright 2024 Koha Development team +# +# This file is part of Koha +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see + +use Modern::Perl; + +use Test::More tests => 8; +use Test::MockModule; +use Test::Exception; +use Test::Warn; + +use C4::Context; +use Koha::Account; +use Koha::Account::Lines; +use Koha::Patrons; + +use t::lib::Mocks; +use t::lib::TestBuilder; + +my $schema = Koha::Database->new->schema; +$schema->storage->dbh->{PrintError} = 0; +my $builder = t::lib::TestBuilder->new; +C4::Context->interface('commandline'); + +subtest 'add_print_notice_charge_if_needed with charging disabled' => sub { + plan tests => 3; + + $schema->storage->txn_begin; + + # Create category with print notice charging disabled (0.00) + my $category = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.00 } } ); + my $patron = + $builder->build_object( { class => 'Koha::Patrons', value => { categorycode => $category->categorycode } } ); + my $account = Koha::Account->new( { patron_id => $patron->borrowernumber } ); + + my $initial_balance = $account->balance; + + my $result = $patron->add_print_notice_charge_if_needed( + { + notice_code => 'ODUE', + library_id => $patron->branchcode + } + ); + + is( $result, undef, 'No charge applied when charging disabled' ); + is( $account->balance, $initial_balance, 'Account balance unchanged' ); + is( $account->lines->count, 0, 'No account lines created' ); + + $schema->storage->txn_rollback; +}; + +subtest 'add_print_notice_charge_if_needed with charging enabled' => sub { + plan tests => 7; + + $schema->storage->txn_begin; + + # Create category with print notice charging enabled (0.75) + my $category = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.75 } } ); + my $patron = + $builder->build_object( { class => 'Koha::Patrons', value => { categorycode => $category->categorycode } } ); + my $account = Koha::Account->new( { patron_id => $patron->borrowernumber } ); + + my $initial_balance = $account->balance; + + my $result = $patron->add_print_notice_charge_if_needed( + { + notice_code => 'ODUE', + library_id => $patron->branchcode + } + ); + + isa_ok( $result, 'Koha::Account::Line', 'Returns account line object' ); + is( $account->balance, $initial_balance + 0.75, 'Account balance increased by charge amount' ); + is( $result->debit_type_code, 'PRINT_NOTICE', 'Correct debit type applied' ); + cmp_ok( $result->amount, '==', 0.75, 'Correct amount charged' ); + cmp_ok( $result->amountoutstanding, '==', 0.75, 'Full amount outstanding' ); + is( $result->borrowernumber, $patron->borrowernumber, 'Correct patron charged' ); + is( $result->interface, 'commandline', 'Correct interface recorded' ); + + $schema->storage->txn_rollback; +}; + +subtest 'add_print_notice_charge_if_needed with custom amount' => sub { + plan tests => 3; + + $schema->storage->txn_begin; + + # Create category with print notice charging enabled (0.50) + my $category = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.50 } } ); + my $patron = + $builder->build_object( { class => 'Koha::Patrons', value => { categorycode => $category->categorycode } } ); + my $account = Koha::Account->new( { patron_id => $patron->borrowernumber } ); + + my $custom_amount = 1.25; + my $result = $patron->add_print_notice_charge_if_needed( + { + notice_code => 'PREDUE', + library_id => $patron->branchcode, + amount => $custom_amount + } + ); + + isa_ok( $result, 'Koha::Account::Line', 'Returns account line object' ); + is( $result->amount, $custom_amount, 'Custom amount used instead of system preference' ); + is( $account->balance, $custom_amount, 'Account balance reflects custom amount' ); + + $schema->storage->txn_rollback; +}; + +subtest 'add_print_notice_charge_if_needed with zero amount' => sub { + plan tests => 3; + + $schema->storage->txn_begin; + + # Create category with charging disabled (0.00) + my $category = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.00 } } ); + my $patron = + $builder->build_object( { class => 'Koha::Patrons', value => { categorycode => $category->categorycode } } ); + my $account = Koha::Account->new( { patron_id => $patron->borrowernumber } ); + + my $result = $patron->add_print_notice_charge_if_needed( + { + notice_code => 'HOLD', + library_id => $patron->branchcode + } + ); + + is( $result, undef, 'No charge applied when amount is zero' ); + is( $account->balance, 0, 'Account balance unchanged' ); + is( $account->lines->count, 0, 'No account lines created' ); + + $schema->storage->txn_rollback; +}; + +subtest 'add_print_notice_charge_if_needed validation tests' => sub { + plan tests => 3; + + $schema->storage->txn_begin; + + # Create category with print notice charging enabled + my $category = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.50 } } ); + my $patron = + $builder->build_object( { class => 'Koha::Patrons', value => { categorycode => $category->categorycode } } ); + + # Test with invalid negative amount + my $result1 = $patron->add_print_notice_charge_if_needed( + { + notice_code => 'ODUE', + library_id => $patron->branchcode, + amount => -0.50 + } + ); + is( $result1, undef, 'No charge applied for negative amount' ); + + # Test with invalid non-numeric amount + my $result2 = $patron->add_print_notice_charge_if_needed( + { + notice_code => 'ODUE', + library_id => $patron->branchcode, + amount => 'invalid' + } + ); + is( $result2, undef, 'No charge applied for non-numeric amount' ); + + # Test with valid amount works + my $result3 = $patron->add_print_notice_charge_if_needed( + { + notice_code => 'ODUE', + library_id => $patron->branchcode, + amount => 1.00 + } + ); + isa_ok( $result3, 'Koha::Account::Line', 'Valid amount works correctly' ); + + $schema->storage->txn_rollback; +}; + +subtest 'add_print_notice_charge_if_needed without notice code' => sub { + plan tests => 2; + + $schema->storage->txn_begin; + + # Create category with print notice charging enabled (0.50) + my $category = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.50 } } ); + my $patron = + $builder->build_object( { class => 'Koha::Patrons', value => { categorycode => $category->categorycode } } ); + my $account = Koha::Account->new( { patron_id => $patron->borrowernumber } ); + + my $result = $patron->add_print_notice_charge_if_needed( { library_id => $patron->branchcode } ); + + isa_ok( $result, 'Koha::Account::Line', 'Returns account line object even without notice code' ); + cmp_ok( $result->amount, '==', 0.50, 'Category amount used' ); + + $schema->storage->txn_rollback; +}; + +subtest 'add_print_notice_charge_if_needed library_id handling' => sub { + plan tests => 4; + + $schema->storage->txn_begin; + + # Create category with print notice charging enabled (0.50) + my $category = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.50 } } ); + my $patron = + $builder->build_object( { class => 'Koha::Patrons', value => { categorycode => $category->categorycode } } ); + my $account = Koha::Account->new( { patron_id => $patron->borrowernumber } ); + + # Get the patron's library for testing + my $patron_library = $patron->branchcode; + + # Mock userenv for default library + my $mock_context = Test::MockModule->new('C4::Context'); + $mock_context->mock( + 'userenv', + sub { + return { branch => $patron_library }; + } + ); + + # Test with explicit library_id + my $result1 = $patron->add_print_notice_charge_if_needed( + { + notice_code => 'ODUE', + library_id => $patron_library + } + ); + is( $result1->branchcode, $patron_library, 'Explicit library_id used when provided' ); + + # Test without library_id (should use userenv) + my $result2 = $patron->add_print_notice_charge_if_needed( { notice_code => 'PREDUE' } ); + is( $result2->branchcode, $patron_library, 'Default library from userenv used when not provided' ); + + # Test with undefined userenv + $mock_context->mock( 'userenv', sub { return; } ); + my $result3 = $patron->add_print_notice_charge_if_needed( { notice_code => 'HOLD' } ); + isa_ok( $result3, 'Koha::Account::Line', 'Still works with undefined userenv' ); + is( $result3->branchcode, undef, 'Library_id is undef when no userenv' ); + + $schema->storage->txn_rollback; +}; + +subtest 'add_print_notice_charge_if_needed category isolation test' => sub { + plan tests => 6; + + $schema->storage->txn_begin; + + # Create two categories - one with charging enabled, one disabled + my $category_enabled = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { print_notice_charge => 1.25 } } ); + my $category_disabled = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.00 } } ); + + # Create patrons in each category + my $patron_enabled = $builder->build_object( + { class => 'Koha::Patrons', value => { categorycode => $category_enabled->categorycode } } ); + my $patron_disabled = $builder->build_object( + { class => 'Koha::Patrons', value => { categorycode => $category_disabled->categorycode } } ); + + my $account_enabled = Koha::Account->new( { patron_id => $patron_enabled->borrowernumber } ); + my $account_disabled = Koha::Account->new( { patron_id => $patron_disabled->borrowernumber } ); + + # Test patron with enabled category gets charged + my $result_enabled = $patron_enabled->add_print_notice_charge_if_needed( + { + notice_code => 'ODUE', + library_id => $patron_enabled->branchcode + } + ); + + isa_ok( $result_enabled, 'Koha::Account::Line', 'Patron with enabled category gets charged' ); + cmp_ok( $result_enabled->amount, '==', 1.25, 'Correct amount from enabled category' ); + cmp_ok( $account_enabled->balance, '==', 1.25, 'Enabled patron account balance updated' ); + + # Test patron with disabled category does not get charged + my $result_disabled = $patron_disabled->add_print_notice_charge_if_needed( + { + notice_code => 'ODUE', + library_id => $patron_disabled->branchcode + } + ); + + is( $result_disabled, undef, 'Patron with disabled category not charged' ); + cmp_ok( $account_disabled->balance, '==', 0, 'Disabled patron account balance unchanged' ); + is( $account_disabled->lines->count, 0, 'No account lines for disabled category patron' ); + + $schema->storage->txn_rollback; +}; diff --git a/t/db_dependent/PrintNoticeCharging_EndToEnd.t b/t/db_dependent/PrintNoticeCharging_EndToEnd.t index 6dcca3704fb..102cefc6e21 100755 --- a/t/db_dependent/PrintNoticeCharging_EndToEnd.t +++ b/t/db_dependent/PrintNoticeCharging_EndToEnd.t @@ -43,52 +43,59 @@ subtest 'print notice charging workflow with charging enabled' => sub { $schema->storage->txn_begin; # Create category with print notice charging enabled (1.00) - my $category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { print_notice_charge => 1.00 } }); + my $category = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { print_notice_charge => 1.00 } } ); # Create test patron and library - my $library = $builder->build_object({ class => 'Koha::Libraries' }); - my $patron = $builder->build_object({ - class => 'Koha::Patrons', - value => { - branchcode => $library->branchcode, - categorycode => $category->categorycode + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + branchcode => $library->branchcode, + categorycode => $category->categorycode + } } - }); + ); # Create a print notice in message queue - my $message = Koha::Notice::Message->new({ - borrowernumber => $patron->borrowernumber, - subject => 'Test Notice', - content => 'This is a test print notice', - message_transport_type => 'print', - status => 'pending', - letter_code => 'ODUE', - to_address => $patron->address, - })->store(); + my $message = Koha::Notice::Message->new( + { + borrowernumber => $patron->borrowernumber, + subject => 'Test Notice', + content => 'This is a test print notice', + message_transport_type => 'print', + status => 'pending', + letter_code => 'ODUE', + to_address => $patron->address, + } + )->store(); - my $account = $patron->account; + my $account = $patron->account; my $initial_balance = $account->balance; - my $initial_lines = $account->lines->count; + my $initial_lines = $account->lines->count; # Simulate what gather_print_notices.pl does - is($message->status, 'pending', 'Message starts as pending'); + is( $message->status, 'pending', 'Message starts as pending' ); # Apply print notice charge (this is what our enhanced gather_print_notices.pl does) - my $charge_result = $patron->add_print_notice_charge_if_needed({ - notice_code => $message->letter_code, - library_id => $library->branchcode, - }); + my $charge_result = $patron->add_print_notice_charge_if_needed( + { + notice_code => $message->letter_code, + library_id => $library->branchcode, + } + ); # Update message status to sent (as gather_print_notices.pl would do) $message->status('sent')->store(); # Verify the charge was applied - isa_ok($charge_result, 'Koha::Account::Line', 'Charge was created'); - is($account->balance, $initial_balance + 1.00, 'Account balance increased by charge amount'); - is($account->lines->count, $initial_lines + 1, 'New account line created'); - is($charge_result->debit_type_code, 'PRINT_NOTICE', 'Correct debit type'); - is($charge_result->borrowernumber, $patron->borrowernumber, 'Correct patron charged'); - is($message->status, 'sent', 'Message marked as sent'); + isa_ok( $charge_result, 'Koha::Account::Line', 'Charge was created' ); + is( $account->balance, $initial_balance + 1.00, 'Account balance increased by charge amount' ); + is( $account->lines->count, $initial_lines + 1, 'New account line created' ); + is( $charge_result->debit_type_code, 'PRINT_NOTICE', 'Correct debit type' ); + is( $charge_result->borrowernumber, $patron->borrowernumber, 'Correct patron charged' ); + is( $message->status, 'sent', 'Message marked as sent' ); $schema->storage->txn_rollback; }; @@ -99,45 +106,52 @@ subtest 'print notice charging workflow with charging disabled' => sub { $schema->storage->txn_begin; # Create category with print notice charging disabled (0.00) - my $category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.00 } }); + my $category = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.00 } } ); # Create test patron and library - my $library = $builder->build_object({ class => 'Koha::Libraries' }); - my $patron = $builder->build_object({ - class => 'Koha::Patrons', - value => { - branchcode => $library->branchcode, - categorycode => $category->categorycode + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + branchcode => $library->branchcode, + categorycode => $category->categorycode + } } - }); + ); # Create a print notice in message queue - my $message = Koha::Notice::Message->new({ - borrowernumber => $patron->borrowernumber, - subject => 'Test Notice', - content => 'This is a test print notice', - message_transport_type => 'print', - status => 'pending', - letter_code => 'ODUE', - to_address => $patron->address, - })->store(); + my $message = Koha::Notice::Message->new( + { + borrowernumber => $patron->borrowernumber, + subject => 'Test Notice', + content => 'This is a test print notice', + message_transport_type => 'print', + status => 'pending', + letter_code => 'ODUE', + to_address => $patron->address, + } + )->store(); - my $account = $patron->account; + my $account = $patron->account; my $initial_balance = $account->balance; - my $initial_lines = $account->lines->count; + my $initial_lines = $account->lines->count; # Simulate what gather_print_notices.pl does when charging disabled - my $charge_result = $patron->add_print_notice_charge_if_needed({ - notice_code => $message->letter_code, - library_id => $library->branchcode, - }); + my $charge_result = $patron->add_print_notice_charge_if_needed( + { + notice_code => $message->letter_code, + library_id => $library->branchcode, + } + ); $message->status('sent')->store(); # Verify no charge was applied - is($charge_result, undef, 'No charge created when charging disabled'); - is($account->balance, $initial_balance, 'Account balance unchanged'); - is($account->lines->count, $initial_lines, 'No new account lines created'); - is($message->status, 'sent', 'Message still marked as sent'); + is( $charge_result, undef, 'No charge created when charging disabled' ); + is( $account->balance, $initial_balance, 'Account balance unchanged' ); + is( $account->lines->count, $initial_lines, 'No new account lines created' ); + is( $message->status, 'sent', 'Message still marked as sent' ); $schema->storage->txn_rollback; }; @@ -148,48 +162,55 @@ subtest 'multiple print notices for same patron' => sub { $schema->storage->txn_begin; # Create category with print notice charging enabled (0.50) - my $category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.50 } }); + my $category = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.50 } } ); # Create test patron - my $patron = $builder->build_object({ - class => 'Koha::Patrons', - value => { categorycode => $category->categorycode } - }); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { categorycode => $category->categorycode } + } + ); my $account = $patron->account; # Create multiple print notices - my @notice_codes = ('ODUE', 'PREDUE', 'HOLD'); + my @notice_codes = ( 'ODUE', 'PREDUE', 'HOLD' ); my @charges; foreach my $code (@notice_codes) { - my $message = Koha::Notice::Message->new({ - borrowernumber => $patron->borrowernumber, - subject => "Test $code Notice", - content => "This is a test $code print notice", - message_transport_type => 'print', - status => 'pending', - letter_code => $code, - to_address => $patron->address, - })->store(); + my $message = Koha::Notice::Message->new( + { + borrowernumber => $patron->borrowernumber, + subject => "Test $code Notice", + content => "This is a test $code print notice", + message_transport_type => 'print', + status => 'pending', + letter_code => $code, + to_address => $patron->address, + } + )->store(); # Apply charge for each notice - my $charge = $patron->add_print_notice_charge_if_needed({ - notice_code => $code, - library_id => $patron->branchcode, - }); + my $charge = $patron->add_print_notice_charge_if_needed( + { + notice_code => $code, + library_id => $patron->branchcode, + } + ); push @charges, $charge; $message->status('sent')->store(); } # Verify all charges were applied - is(scalar @charges, 3, 'Three charges created'); - is($account->balance, 1.50, 'Total balance is 3 × $0.50 = $1.50'); - is($account->lines->count, 3, 'Three account lines created'); + is( scalar @charges, 3, 'Three charges created' ); + is( $account->balance, 1.50, 'Total balance is 3 × $0.50 = $1.50' ); + is( $account->lines->count, 3, 'Three account lines created' ); # Verify each charge has correct debit type - my @lines = $account->lines->as_list; + my @lines = $account->lines->as_list; my @debit_types = map { $_->debit_type_code } @lines; - is(scalar(grep { $_ eq 'PRINT_NOTICE' } @debit_types), 3, 'All charges have PRINT_NOTICE debit type'); + is( scalar( grep { $_ eq 'PRINT_NOTICE' } @debit_types ), 3, 'All charges have PRINT_NOTICE debit type' ); $schema->storage->txn_rollback; }; @@ -200,48 +221,52 @@ subtest 'print notice charging with missing patron data' => sub { $schema->storage->txn_begin; # Create category with print notice charging enabled (1.00) - my $category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { print_notice_charge => 1.00 } }); + my $category = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { print_notice_charge => 1.00 } } ); # Create a patron then delete it to simulate missing data - my $patron = $builder->build_object({ - class => 'Koha::Patrons', - value => { categorycode => $category->categorycode } - }); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { categorycode => $category->categorycode } + } + ); my $borrowernumber = $patron->borrowernumber; $patron->delete(); # Create a message with reference to deleted patron my $message = { borrowernumber => $borrowernumber, - letter_code => 'ODUE', + letter_code => 'ODUE', }; # Test our apply_print_notice_charge function behavior # This simulates what would happen in gather_print_notices.pl my $found_patron = Koha::Patrons->find($borrowernumber); - is($found_patron, undef, 'Patron not found as expected'); + is( $found_patron, undef, 'Patron not found as expected' ); # The function should handle missing patrons gracefully my $result; warnings_are { if ($found_patron) { - my $account = Koha::Account->new({ patron_id => $borrowernumber }); - $result = $patron->add_print_notice_charge_if_needed({ - notice_code => $message->{letter_code}, - library_id => 'CPL', # Use a default library for this test - }); + my $account = Koha::Account->new( { patron_id => $borrowernumber } ); + $result = $patron->add_print_notice_charge_if_needed( + { + notice_code => $message->{letter_code}, + library_id => 'CPL', # Use a default library for this test + } + ); } else { $result = undef; } - } [], 'No warnings when patron not found'; + } + [], 'No warnings when patron not found'; - is($result, undef, 'No charge attempted for missing patron'); + is( $result, undef, 'No charge attempted for missing patron' ); # Verify no orphaned account lines - my $orphaned_lines = Koha::Account::Lines->search({ - borrowernumber => $borrowernumber - }); - is($orphaned_lines->count, 0, 'No orphaned account lines created'); + my $orphaned_lines = Koha::Account::Lines->search( { borrowernumber => $borrowernumber } ); + is( $orphaned_lines->count, 0, 'No orphaned account lines created' ); $schema->storage->txn_rollback; }; @@ -252,48 +277,59 @@ subtest 'print notice charging with different amounts' => sub { $schema->storage->txn_begin; # Create category with print notice charging enabled (0.75) - my $category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.75 } }); + my $category = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.75 } } ); - my $patron = $builder->build_object({ - class => 'Koha::Patrons', - value => { categorycode => $category->categorycode } - }); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { categorycode => $category->categorycode } + } + ); my $account = $patron->account; # Test 1: Use system preference amount - my $charge1 = $patron->add_print_notice_charge_if_needed({ - notice_code => 'ODUE', - library_id => $patron->branchcode, - }); - cmp_ok($charge1->amount, '==', 0.75, 'Category amount used when no custom amount'); + my $charge1 = $patron->add_print_notice_charge_if_needed( + { + notice_code => 'ODUE', + library_id => $patron->branchcode, + } + ); + cmp_ok( $charge1->amount, '==', 0.75, 'Category amount used when no custom amount' ); # Test 2: Use custom amount - my $charge2 = $patron->add_print_notice_charge_if_needed({ - notice_code => 'PREDUE', - library_id => $patron->branchcode, - amount => 1.50, - }); - is($charge2->amount, 1.50, 'Custom amount used when provided'); + my $charge2 = $patron->add_print_notice_charge_if_needed( + { + notice_code => 'PREDUE', + library_id => $patron->branchcode, + amount => 1.50, + } + ); + is( $charge2->amount, 1.50, 'Custom amount used when provided' ); # Test 3: Zero custom amount should not create charge - my $charge3 = $patron->add_print_notice_charge_if_needed({ - notice_code => 'HOLD', - library_id => $patron->branchcode, - amount => 0.00, - }); - is($charge3, undef, 'No charge created for zero amount'); + my $charge3 = $patron->add_print_notice_charge_if_needed( + { + notice_code => 'HOLD', + library_id => $patron->branchcode, + amount => 0.00, + } + ); + is( $charge3, undef, 'No charge created for zero amount' ); # Test 4: Very small amount - my $charge4 = $patron->add_print_notice_charge_if_needed({ - notice_code => 'RENEWAL', - library_id => $patron->branchcode, - amount => 0.01, - }); - is($charge4->amount, 0.01, 'Very small amounts work correctly'); + my $charge4 = $patron->add_print_notice_charge_if_needed( + { + notice_code => 'RENEWAL', + library_id => $patron->branchcode, + amount => 0.01, + } + ); + is( $charge4->amount, 0.01, 'Very small amounts work correctly' ); # Verify total balance - is($account->balance, 2.26, 'Total balance is 0.75 + 1.50 + 0.01 = 2.26'); - is($account->lines->count, 3, 'Three charges created (zero amount not counted)'); + is( $account->balance, 2.26, 'Total balance is 0.75 + 1.50 + 0.01 = 2.26' ); + is( $account->lines->count, 3, 'Three charges created (zero amount not counted)' ); $schema->storage->txn_rollback; }; @@ -304,19 +340,22 @@ subtest 'gather_print_notices.pl helper function tests' => sub { $schema->storage->txn_begin; # Create category with print notice charging enabled (0.60) - my $category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.60 } }); + my $category = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { print_notice_charge => 0.60 } } ); # Create test patron - my $patron = $builder->build_object({ - class => 'Koha::Patrons', - value => { categorycode => $category->categorycode } - }); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { categorycode => $category->categorycode } + } + ); my $account = $patron->account; # Test valid message hash my $valid_message = { borrowernumber => $patron->borrowernumber, - letter_code => 'ODUE', + letter_code => 'ODUE', }; # Simulate the apply_print_notice_charge function from gather_print_notices.pl @@ -325,15 +364,17 @@ subtest 'gather_print_notices.pl helper function tests' => sub { return unless $message->{borrowernumber}; - my $patron = Koha::Patrons->find($message->{borrowernumber}); + my $patron = Koha::Patrons->find( $message->{borrowernumber} ); return unless $patron; eval { - my $account = Koha::Account->new({ patron_id => $patron->borrowernumber }); - $patron->add_print_notice_charge_if_needed({ - notice_code => $message->{letter_code}, - library_id => $patron->branchcode, - }); + my $account = Koha::Account->new( { patron_id => $patron->borrowernumber } ); + $patron->add_print_notice_charge_if_needed( + { + notice_code => $message->{letter_code}, + library_id => $patron->branchcode, + } + ); }; if ($@) { @@ -345,42 +386,42 @@ subtest 'gather_print_notices.pl helper function tests' => sub { # Test 1: Valid message my $result1 = $apply_charge->($valid_message); - is($result1, 1, 'Valid message processed successfully'); - is($account->balance, 0.60, 'Charge applied correctly'); + is( $result1, 1, 'Valid message processed successfully' ); + is( $account->balance, 0.60, 'Charge applied correctly' ); # Test 2: Message with missing borrowernumber my $invalid_message1 = { letter_code => 'ODUE', - branchcode => $patron->branchcode, + branchcode => $patron->branchcode, }; my $result2 = $apply_charge->($invalid_message1); - is($result2, undef, 'Message without borrowernumber handled gracefully'); + is( $result2, undef, 'Message without borrowernumber handled gracefully' ); # Test 3: Message with invalid borrowernumber my $invalid_message2 = { borrowernumber => 999999, - letter_code => 'ODUE', - branchcode => $patron->branchcode, + letter_code => 'ODUE', + branchcode => $patron->branchcode, }; my $result3 = $apply_charge->($invalid_message2); - is($result3, undef, 'Message with invalid borrowernumber handled gracefully'); + is( $result3, undef, 'Message with invalid borrowernumber handled gracefully' ); # Test 4: Another valid message to same patron my $valid_message2 = { borrowernumber => $patron->borrowernumber, - letter_code => 'PREDUE', - branchcode => $patron->branchcode, + letter_code => 'PREDUE', + branchcode => $patron->branchcode, }; my $result4 = $apply_charge->($valid_message2); - is($result4, 1, 'Second valid message processed successfully'); - is($account->balance, 1.20, 'Second charge applied correctly'); + is( $result4, 1, 'Second valid message processed successfully' ); + is( $account->balance, 1.20, 'Second charge applied correctly' ); # Test 5: Verify account lines my @lines = $account->lines->as_list; - is(scalar @lines, 2, 'Two account lines created'); + is( scalar @lines, 2, 'Two account lines created' ); my @debit_types = map { $_->debit_type_code } @lines; - is(scalar(grep { $_ eq 'PRINT_NOTICE' } @debit_types), 2, 'Both charges have PRINT_NOTICE debit type'); + is( scalar( grep { $_ eq 'PRINT_NOTICE' } @debit_types ), 2, 'Both charges have PRINT_NOTICE debit type' ); $schema->storage->txn_rollback; -}; \ No newline at end of file +}; -- 2.53.0