From 848f75124a916007ca1d835a02908f055e9295ae Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 10 Feb 2022 14:30:02 +0000 Subject: [PATCH] Bug 29755: Check each NoIssuesCharge separately This patch updates SIP patron code to use account methods to calculate balances over the patronflags returns. It also checks if patron should be blocked for each 'No Issues charge' preference Tests are added for NoIssuesChargeGuarantees To test: 1 - Set noissuescharge preference to 5 2 - Add a $10 charge to a patron 3 - perl misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL -m patron_information --patron BARCODE 4 - Note the 64 message starts with Y's that mean patron is blocked 5 - Set noissuescharge to 11 6 - Repeat 3, patron is no longer blocked 7 - Set NoIssuesChargeGuarantees to 8 8 - Repeat 3, patron is blocked 9 - Pay $3 on patron so they owe 7 10 - Repeat 3, patron is not blocked 11 - Add a child account with patron as guarantor 12 - Repeat 3, patron is not blocked 13 - Add a $4 charge to child 14 - Repeat 3, patron is blocked 15 - Repeat 3, but with child barcode, child is not blocked 16 - Set NoIssuesChargeGuarantorsWithGuarantees to 11 17 - Repeat 3, patron is blocked 18 - Repeat 3 with child barcode, child is blocked --- C4/SIP/ILS/Patron.pm | 18 ++++++------ t/db_dependent/SIP/Patron.t | 57 +++++++++++++++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 10 deletions(-) diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm index 95166d3da8..767feffffe 100644 --- a/C4/SIP/ILS/Patron.pm +++ b/C4/SIP/ILS/Patron.pm @@ -77,17 +77,19 @@ sub new { $dexpiry and $dexpiry =~ s/-//g; # YYYYMMDD # Get fines and add fines for guarantees (depends on preference NoIssuesChargeGuarantees) - my $fines_amount = $flags->{CHARGES}->{amount}; #TODO Replace with $patron->account->non_issues_charges - $fines_amount = ($fines_amount and $fines_amount > 0) ? $fines_amount : 0; - if ( C4::Context->preference('NoIssuesChargeGuarantorsWithGuarantees') ) { + my $fines_amount = ($patron->account->balance > 0) ? $patron->account->non_issues_charges : 0; + my $fee_limit = _fee_limit(); + my $fine_blocked = $fines_amount > $fee_limit; + my $noissueschargeguarantorswithguarantees = C4::Context->preference('NoIssuesChargeGuarantorsWithGuarantees'); + my $noissueschargeguarantees = C4::Context->preference('NoIssuesChargeGuarantees'); + if ( $noissueschargeguarantorswithguarantees ) { $fines_amount += $patron->relationships_debt({ include_guarantors => 1, only_this_guarantor => 0, include_this_patron => 0 }); - } else { - my $guarantees_fines_amount = $flags->{CHARGES_GUARANTEES} ? $flags->{CHARGES_GUARANTEES}->{amount} : 0; #TODO: Replace with $patron->relationships_debt - $fines_amount += $guarantees_fines_amount; + $fine_blocked ||= $fines_amount > $noissueschargeguarantorswithguarantees; + } elsif ( $noissueschargeguarantees ) { + $fines_amount += $patron->relationships_debt({ include_guarantors => 0, only_this_guarantor => 0, include_this_patron => 0 }); + $fine_blocked ||= $fines_amount > $noissueschargeguarantees; } - my $fee_limit = _fee_limit(); - my $fine_blocked = $fines_amount > $fee_limit; my $circ_blocked =( C4::Context->preference('OverduesBlockCirc') ne "noblock" && defined $flags->{ODUES}->{itemlist} ) ? 1 : 0; { no warnings; # any of these $kp->{fields} being concat'd could be undef diff --git a/t/db_dependent/SIP/Patron.t b/t/db_dependent/SIP/Patron.t index 15b8827da2..f307e29a4f 100755 --- a/t/db_dependent/SIP/Patron.t +++ b/t/db_dependent/SIP/Patron.t @@ -4,7 +4,7 @@ # This needs to be extended! Your help is appreciated.. use Modern::Perl; -use Test::More tests => 9; +use Test::More tests => 10; use t::lib::Mocks; use t::lib::TestBuilder; @@ -274,9 +274,56 @@ subtest "fine_items tests" => sub { $schema->storage->txn_rollback; +subtest "NoIssuesChargeGuarantees tests" => sub { + + plan tests => 4; + + t::lib::Mocks::mock_preference( 'borrowerRelationship', 'parent' ); + + $schema->storage->txn_begin; + + my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + my $child = $builder->build_object({ class => 'Koha::Patrons' }); + $child->add_guarantor({ guarantor_id => $patron->borrowernumber, relationship => 'parent' }); + + t::lib::Mocks::mock_preference('NoIssuesChargeGuarantees', 1); + + my $fee1 = $builder->build_object( + { + class => 'Koha::Account::Lines', + value => { + borrowernumber => $patron->borrowernumber, + amountoutstanding => 11, + } + } + )->store; + + my $fee2 = $builder->build_object( + { + class => 'Koha::Account::Lines', + value => { + borrowernumber => $child->borrowernumber, + amountoutstanding => 0.11, + } + } + )->store; + + my $sip_patron = C4::SIP::ILS::Patron->new( $patron->cardnumber ); + + is( $sip_patron->fines_amount, 11.11,"Guarantor fines correctly included"); + ok( !$sip_patron->charge_ok, "Guarantor blocked"); + + $sip_patron = C4::SIP::ILS::Patron->new( $child->cardnumber ); + + is( $sip_patron->fines_amount, 0.11,"Guarantee only fines correctly counted"); + ok( $sip_patron->charge_ok, "Guarantee not blocked by guarantor fines"); + + $schema->storage->txn_rollback; +}; + subtest "NoIssuesChargeGuarantorsWithGuarantees tests" => sub { - plan tests => 1; + plan tests => 4; t::lib::Mocks::mock_preference( 'borrowerRelationship', 'parent' ); @@ -311,6 +358,12 @@ subtest "NoIssuesChargeGuarantorsWithGuarantees tests" => sub { my $sip_patron = C4::SIP::ILS::Patron->new( $patron->cardnumber ); is( $sip_patron->fines_amount, 11.11,"Guarantee fines correctly included"); + ok( !$sip_patron->charge_ok, "Guarantor blocked"); + + $sip_patron = C4::SIP::ILS::Patron->new( $child->cardnumber ); + + is( $sip_patron->fines_amount, 11.11,"Guarantor fines correctly included"); + ok( !$sip_patron->charge_ok, "Guarantee blocked"); $schema->storage->txn_rollback; }; -- 2.30.2