From f747d38aead414e77e5c367742c9e39a8042f689 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 10 17 - Repeat 3, patron is blocked 18 - Repeat 3 with child barcode, child is blocked Signed-off-by: David Nind Signed-off-by: Martin Renvoize Signed-off-by: Fridolin Somers --- 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 7d77b7e9be..de2b1ddade 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.39.5