From 7a15a7ee6f876e0ac1dd6d0eaa24bb2b3e03a925 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Mon, 20 Feb 2012 08:01:46 -0500 Subject: [PATCH] Bug 7560 - SIP Self Checkout Ignoring Fines Thresholds for Not Being able to Check Out Books Add checks in C4::ILS::Patron and C4::ILS::Transaction::Renew to make sure the patron is below the noissuescharge system pref for issues and OPACFineNoRenewals for renewals. Signed-off-by: Chris Cormack Patch ammended post sign-off --- C4/Accounts.pm | 57 ++++++++++++++++++++++++++++------------- C4/Overdues.pm | 2 ++ C4/SIP/ILS/Patron.pm | 4 +-- C4/SIP/ILS/Transaction/Renew.pm | 8 ++++++ t/db_dependent/Accounts.t | 48 +++++++++++++++++++++++++++++++++- 5 files changed, 98 insertions(+), 21 deletions(-) diff --git a/C4/Accounts.pm b/C4/Accounts.pm index 5243712..cfce592 100644 --- a/C4/Accounts.pm +++ b/C4/Accounts.pm @@ -31,24 +31,25 @@ use Data::Dumper qw(Dumper); use vars qw(@ISA @EXPORT); BEGIN { - require Exporter; - @ISA = qw(Exporter); - @EXPORT = qw( - &recordpayment - &makepayment - &manualinvoice - &getnextacctno - &getcharges - &ModNote - &getcredits - &getrefunds - &chargelostitem - &ReversePayment - &makepartialpayment - &recordpayment_selectaccts - &WriteOffFee - &purge_zero_balance_fees - ); + require Exporter; + @ISA = qw(Exporter); + @EXPORT = qw( + &recordpayment + &makepayment + &manualinvoice + &getnextacctno + &getcharges + &ModNote + &getcredits + &getrefunds + &chargelostitem + &ReversePayment + &makepartialpayment + &recordpayment_selectaccts + &WriteOffFee + &purge_zero_balance_fees + &GetTotalFines + ); } =head1 NAME @@ -853,6 +854,26 @@ sub purge_zero_balance_fees { $sth->execute($days) or die $dbh->errstr; } +=head2 GetTotalFines + + my $total_fines_owed = GetTotalFines( $borrowernumber ); + + Returns the total amount owed by the given borrower. + +=cut + +sub GetTotalFines { + my ( $borrowernumber ) = @_; + my $dbh = C4::Context->dbh(); + my $query = "SELECT SUM( amountoutstanding ) AS total FROM accountlines WHERE borrowernumber = ?"; + my $sth = $dbh->prepare( $query ); + $sth->execute( $borrowernumber ); + my $row = $sth->fetchrow_hashref(); + my $total = $row->{'total'}; + $total = $total || 0; + return $total; +} + END { } # module clean-up code here (global destructor) 1; diff --git a/C4/Overdues.pm b/C4/Overdues.pm index cc67b16..38fade5 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -52,6 +52,7 @@ BEGIN { &AmountNotify &UpdateFine &GetFine + &GetTotalFines &get_chargeable_units &CheckItemNotify &GetOverduesForBranch @@ -686,6 +687,7 @@ sub GetFine { return 0; } + =head2 NumberNotifyId (@notify) = &NumberNotifyId($borrowernumber); diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm index d61b097..ec062d8 100644 --- a/C4/SIP/ILS/Patron.pm +++ b/C4/SIP/ILS/Patron.pm @@ -80,8 +80,8 @@ sub new { address => $adr, home_phone => $kp->{phone}, email_addr => $kp->{email}, - charge_ok => ( !$debarred && !$expired && !$fine_blocked), - renew_ok => ( !$debarred && !$expired && !$fine_blocked), + charge_ok => ( !$debarred && !$expired && !$fine_blocked && ( $fines_amount < C4::Context->preference('noissuescharge') ) ), + renew_ok => ( !$debarred && !$expired && !$fine_blocked && ( $fines_amount < C4::Context->preference('OPACFineNoRenewals') ) ), recall_ok => ( !$debarred && !$expired && !$fine_blocked), hold_ok => ( !$debarred && !$expired && !$fine_blocked), card_lost => ( $kp->{lost} || $kp->{gonenoaddress} || $flags->{LOST} ), diff --git a/C4/SIP/ILS/Transaction/Renew.pm b/C4/SIP/ILS/Transaction/Renew.pm index 699f566..6826e59 100644 --- a/C4/SIP/ILS/Transaction/Renew.pm +++ b/C4/SIP/ILS/Transaction/Renew.pm @@ -33,6 +33,7 @@ sub do_renew_for { my $self = shift; my $borrower = shift; my ($renewokay,$renewerror) = CanBookBeRenewed($borrower->{borrowernumber},$self->{item}->{itemnumber}); + if ($renewokay) { # ok so far check charges my ($fee, undef) = GetIssuingCharges($self->{item}->{itemnumber}, $self->{patron}->{borrowernumber}); if ($fee > 0) { @@ -44,6 +45,12 @@ sub do_renew_for { } } + + unless ( C4::Accounts::GetTotalFines($borrower->{borrowernumber}) < C4::Context->preference('OPACFineNoRenewals') ) { + $renewokay = 0; + $renewerror = 'fines_exceed_OPACFineNoRenewals' + } + if ($renewokay){ my $issue = AddIssue( $borrower, $self->{item}->id, undef, 0 ); $self->{due} = $self->duedatefromissue($issue, $self->{item}->{itemnumber}); @@ -51,6 +58,7 @@ sub do_renew_for { } else { $renewerror=~s/on_reserve/Item unavailable due to outstanding holds/; $renewerror=~s/too_many/Item has reached maximum renewals/; + $renewerror=~s/fines_exceed_OPACFineNoRenewals/Total fines owed exceeds amount that would allow renewal/; $self->screen_msg($renewerror); $self->renewal_ok(0); } diff --git a/t/db_dependent/Accounts.t b/t/db_dependent/Accounts.t index ee92a48..3368b47 100644 --- a/t/db_dependent/Accounts.t +++ b/t/db_dependent/Accounts.t @@ -18,7 +18,7 @@ use Modern::Perl; -use Test::More tests => 19; +use Test::More tests => 20; use Test::MockModule; use Test::Warn; @@ -357,4 +357,50 @@ subtest "makepartialpayment() tests" => sub { } }; +subtest "GetTotalFines tests" => sub { + + plan tests => 1; + + my $categorycode = + $builder->build( { source => 'Category' } )->{categorycode}; + + my $borrower = $builder->build( + { + surname => 'Hall', + firstname => 'Kyle', + categorycode => $categorycode, + branchcode => $branchcode, + source => 'Borrower', + value => { + categorycode => $categorycode, + branchcode => $branchcode, + } + } + ); + + my $accountline1 = $builder->build( + { + source => 'Accountline', + value => { + borrowernumber => $borrower->{borrowernumber}, + amountoutstanding => '1.00', + } + } + ); + + my $accountline2 = $builder->build( + { + source => 'Accountline', + value => { + borrowernumber => $borrower->{borrowernumber}, + amountoutstanding => '2.00', + } + } + ); + + is( GetTotalFines( $borrower->{borrowernumber} ), '3.000000', "GetTotalFines functions correctly" ); +}; + +$dbh->rollback; + 1; -- 2.1.4