Bugzilla – Attachment 146329 Details for
Bug 32624
Patrons fines are not accurate in SIP2 when NoIssuesChargeGuarantorsWithGuarantees or NoIssuesChargeGuarantees are enabled
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 32624: Don't include guarantee or guarantor fines in BV (fines amount) in SIP messages
Bug-32624-Dont-include-guarantee-or-guarantor-fine.patch (text/plain), 6.56 KB, created by
Martin Renvoize (ashimema)
on 2023-02-07 13:31:25 UTC
(
hide
)
Description:
Bug 32624: Don't include guarantee or guarantor fines in BV (fines amount) in SIP messages
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2023-02-07 13:31:25 UTC
Size:
6.56 KB
patch
obsolete
>From bb059ce7a060014bf6b4a7983c891dd8af1580c9 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >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 <david@davidnind.com> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > 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 f0320bc7206..88afb4ac00a 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 1c718bd1f15..de3526df582 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.1
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=146329">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 32624
:
145304
|
145345
| 146329