From c6c2fe81a273cbb70af69dfa0b51b2f07b49a76c Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Fri, 13 Jan 2023 19:27:04 +0000 Subject: [PATCH] Bug 32624: Don't include guarantee or guarantor fines in BV (fines amount) in SIP messages This patch chanegs the code to report only the patron's personal fines, and to report a block from other fines in the screen message This is to prevent overpayment on accounts from SIP machines To test: 1 - prove t/db_dependent/SIP/Patron.t Signed-off-by: David Nind Signed-off-by: Martin Renvoize Signed-off-by: Tomas Cohen Arazi --- C4/SIP/ILS/Patron.pm | 15 +++++++++++---- t/db_dependent/SIP/Patron.t | 19 ++++++++++++------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm index f0320bc720..88afb4ac00 100644 --- a/C4/SIP/ILS/Patron.pm +++ b/C4/SIP/ILS/Patron.pm @@ -78,16 +78,23 @@ sub new { # Get fines and add fines for guarantees (depends on preference NoIssuesChargeGuarantees) my $fines_amount = ($patron->account->balance > 0) ? $patron->account->non_issues_charges : 0; + my $personal_fines_amount = $fines_amount; my $fee_limit = _fee_limit(); - my $fine_blocked = $fines_amount > $fee_limit; my $noissueschargeguarantorswithguarantees = C4::Context->preference('NoIssuesChargeGuarantorsWithGuarantees'); + my $fines_msg = ""; + my $fine_blocked = 0; my $noissueschargeguarantees = C4::Context->preference('NoIssuesChargeGuarantees'); - if ( $noissueschargeguarantorswithguarantees ) { + if( $fines_amount > $fee_limit ){ + $fine_blocked = 1; + $fines_msg .= " -- " . "Patron blocked by fines" if $fine_blocked; + } elsif ( $noissueschargeguarantorswithguarantees ) { $fines_amount += $patron->relationships_debt({ include_guarantors => 1, only_this_guarantor => 0, include_this_patron => 0 }); $fine_blocked ||= $fines_amount > $noissueschargeguarantorswithguarantees; + $fines_msg .= " -- " . "Patron blocked by fines ($fines_amount) on related accounts"; } elsif ( $noissueschargeguarantees ) { $fines_amount += $patron->relationships_debt({ include_guarantors => 0, only_this_guarantor => 0, include_this_patron => 0 }); $fine_blocked ||= $fines_amount > $noissueschargeguarantees; + $fines_msg .= " -- " . "Patron blocked by fines ($fines_amount) on guaranteed accounts"; } my $circ_blocked =( C4::Context->preference('OverduesBlockCirc') ne "noblock" && defined $flags->{ODUES}->{itemlist} ) ? 1 : 0; @@ -114,11 +121,11 @@ sub new { hold_ok => ( !$debarred && !$expired && !$fine_blocked), card_lost => ( $kp->{lost} || $kp->{gonenoaddress} || $flags->{LOST} ), claims_returned => 0, - fines => $fines_amount, + fines => $personal_fines_amount, fees => 0, # currently not distinct from fines recall_overdue => 0, items_billed => 0, - screen_msg => 'Greetings from Koha. ' . $kp->{opacnote}, + screen_msg => 'Greetings from Koha. ' . $kp->{opacnote} . $fines_msg, print_line => '', items => [], hold_items => $flags->{WAITING}->{itemlist}, diff --git a/t/db_dependent/SIP/Patron.t b/t/db_dependent/SIP/Patron.t index 1c718bd1f1..de3526df58 100755 --- a/t/db_dependent/SIP/Patron.t +++ b/t/db_dependent/SIP/Patron.t @@ -276,7 +276,7 @@ $schema->storage->txn_rollback; subtest "NoIssuesChargeGuarantees tests" => sub { - plan tests => 4; + plan tests => 5; t::lib::Mocks::mock_preference( 'borrowerRelationship', 'parent' ); @@ -286,7 +286,8 @@ subtest "NoIssuesChargeGuarantees tests" => sub { my $child = $builder->build_object({ class => 'Koha::Patrons' }); $child->add_guarantor({ guarantor_id => $patron->borrowernumber, relationship => 'parent' }); - t::lib::Mocks::mock_preference('NoIssuesChargeGuarantees', 1); + 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( @@ -311,8 +312,9 @@ subtest "NoIssuesChargeGuarantees tests" => sub { my $sip_patron = C4::SIP::ILS::Patron->new( $patron->cardnumber ); - is( $sip_patron->fines_amount, 11.11,"Guarantor fines correctly included"); + is( $sip_patron->fines_amount, 11, "Only patron's fines are reported in total"); ok( !$sip_patron->charge_ok, "Guarantor blocked"); + like( $sip_patron->screen_msg, qr/Patron blocked by fines \(11\.11\) on guaranteed accounts/,"Screen message includes related fines total"); $sip_patron = C4::SIP::ILS::Patron->new( $child->cardnumber ); @@ -324,7 +326,7 @@ subtest "NoIssuesChargeGuarantees tests" => sub { subtest "NoIssuesChargeGuarantorsWithGuarantees tests" => sub { - plan tests => 4; + plan tests => 6; t::lib::Mocks::mock_preference( 'borrowerRelationship', 'parent' ); @@ -334,7 +336,8 @@ subtest "NoIssuesChargeGuarantorsWithGuarantees tests" => sub { my $child = $builder->build_object({ class => 'Koha::Patrons' }); $child->add_guarantor({ guarantor_id => $patron->borrowernumber, relationship => 'parent' }); - t::lib::Mocks::mock_preference('NoIssuesChargeGuarantorsWithGuarantees', 1); + t::lib::Mocks::mock_preference('noissuescharge', 50); + t::lib::Mocks::mock_preference('NoIssuesChargeGuarantorsWithGuarantees', 11.01); my $fee1 = $builder->build_object( { @@ -358,13 +361,15 @@ 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"); + is( $sip_patron->fines_amount, 11, "Guarantee fines correctly included"); ok( !$sip_patron->charge_ok, "Guarantor blocked"); + like( $sip_patron->screen_msg, qr/Patron blocked by fines \(11\.11\) on related accounts/,"Screen message includes related fines total"); $sip_patron = C4::SIP::ILS::Patron->new( $child->cardnumber ); - is( $sip_patron->fines_amount, 11.11,"Guarantor fines correctly included"); + is( $sip_patron->fines_amount, 0.11, "Guarantor fines correctly included"); ok( !$sip_patron->charge_ok, "Guarantee blocked"); + like( $sip_patron->screen_msg, qr/Patron blocked by fines \(11\.11\) on related accounts/,"Screen message includes related fines total"); $schema->storage->txn_rollback; }; -- 2.39.5