From 76bc87c1125cdf58f3464d0df458f3a517241b9e Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Fri, 19 Feb 2016 19:57:12 +0000 Subject: [PATCH] Bug 15895 - Add Koha::Account module, use Koha::Account::pay internally for recordpayment This is the first patch in a series to unify all payment functions into a single method. Test Plan: 1) Apply this patch 2) prove t/db_dependent/Accounts.t 3) Test fine payment via the "Pay amount" button --- C4/Accounts.pm | 92 +------------------------ Koha/Account.pm | 171 ++++++++++++++++++++++++++++++++++++++++++++++ Koha/Account/Line.pm | 2 +- Koha/Account/Lines.pm | 2 +- t/db_dependent/Accounts.t | 27 ++++---- 5 files changed, 189 insertions(+), 105 deletions(-) create mode 100644 Koha/Account.pm diff --git a/C4/Accounts.pm b/C4/Accounts.pm index 4f5f8d4..0631855 100644 --- a/C4/Accounts.pm +++ b/C4/Accounts.pm @@ -25,6 +25,7 @@ use C4::Stats; use C4::Members; use C4::Circulation qw(ReturnLostItem); use C4::Log qw(logaction); +use Koha::Account; use Data::Dumper qw(Dumper); @@ -91,96 +92,9 @@ sub recordpayment { #here we update the account lines my ( $borrowernumber, $data, $sip_paytype, $payment_note ) = @_; - my $dbh = C4::Context->dbh; - my $newamtos = 0; - my $accdata = ""; - my $branch = C4::Context->userenv->{'branch'}; - my $amountleft = $data; - my $manager_id = 0; - $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv; - - $payment_note //= ""; - - # begin transaction - my $nextaccntno = getnextacctno($borrowernumber); - - # get lines with outstanding amounts to offset - my $sth = $dbh->prepare( - "SELECT * FROM accountlines - WHERE (borrowernumber = ?) AND (amountoutstanding<>0) - ORDER BY date" - ); - $sth->execute($borrowernumber); - - # offset transactions - my @ids; - while ( ( $accdata = $sth->fetchrow_hashref ) and ( $amountleft > 0 ) ) { - if ( $accdata->{'amountoutstanding'} < $amountleft ) { - $newamtos = 0; - $amountleft -= $accdata->{'amountoutstanding'}; - } - else { - $newamtos = $accdata->{'amountoutstanding'} - $amountleft; - $amountleft = 0; - } - my $thisacct = $accdata->{accountlines_id}; - my $usth = $dbh->prepare( - "UPDATE accountlines SET amountoutstanding= ? - WHERE (accountlines_id = ?)" - ); - $usth->execute( $newamtos, $thisacct ); - - if ( C4::Context->preference("FinesLog") ) { - $accdata->{'amountoutstanding_new'} = $newamtos; - logaction("FINES", 'MODIFY', $borrowernumber, Dumper({ - action => 'fee_payment', - borrowernumber => $accdata->{'borrowernumber'}, - old_amountoutstanding => $accdata->{'amountoutstanding'}, - new_amountoutstanding => $newamtos, - amount_paid => $accdata->{'amountoutstanding'} - $newamtos, - accountlines_id => $accdata->{'accountlines_id'}, - accountno => $accdata->{'accountno'}, - manager_id => $manager_id, - note => $payment_note, - })); - push( @ids, $accdata->{'accountlines_id'} ); - } - } - - # create new line - my $usth = $dbh->prepare( - "INSERT INTO accountlines - (borrowernumber, accountno,date,amount,description,accounttype,amountoutstanding,manager_id, note) - VALUES (?,?,now(),?,'',?,?,?,?)" - ); - - my $paytype = "Pay"; - $paytype .= $sip_paytype if defined $sip_paytype; - $usth->execute( $borrowernumber, $nextaccntno, 0 - $data, $paytype, 0 - $amountleft, $manager_id, $payment_note ); - $usth->finish; - - UpdateStats({ - branch => $branch, - type =>'payment', - amount => $data, - borrowernumber => $borrowernumber, - accountno => $nextaccntno } - ); - - if ( C4::Context->preference("FinesLog") ) { - $accdata->{'amountoutstanding_new'} = $newamtos; - logaction("FINES", 'CREATE',$borrowernumber,Dumper({ - action => 'create_payment', - borrowernumber => $borrowernumber, - accountno => $nextaccntno, - amount => $data * -1, - amountoutstanding => $amountleft * -1, - accounttype => 'Pay', - accountlines_paid => \@ids, - manager_id => $manager_id, - })); - } + return Koha::Account->new( { patron_id => $borrowernumber } ) + ->pay( { amount => $data, sip => $sip_paytype, note => $payment_note } ); } =head2 makepayment diff --git a/Koha/Account.pm b/Koha/Account.pm new file mode 100644 index 0000000..b7341b1 --- /dev/null +++ b/Koha/Account.pm @@ -0,0 +1,171 @@ +package Koha::Account; + +# Copyright 2016 ByWater Solutions +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Carp; +use Data::Dumper; + +use C4::Log qw( logaction ); +use C4::Stats qw( UpdateStats ); + +use Koha::Account::Line; +use Koha::Account::Lines; +use Koha::DateUtils qw( dt_from_string ); + +=head1 NAME + +Koha::Accounts - Module for managing payments and fees for patrons + +=cut + +sub new { + my ( $class, $params ) = @_; + + Carp::croak("No patron id passed in!") unless $params->{patron_id}; + + return bless( $params, $class ); +} + +=head2 pay + +This method allows payments to be made against feees + +=cut + +sub pay { + my ( $self, $params ) = @_; + + my $amount = $params->{amount}; + my $sip = $params->{sip}; + my $note = $params->{note} || q{}; + + my $userenv = C4::Context->userenv; + + # We should remove accountno, it is no longer needed + my $last = Koha::Account::Lines->search( + { + borrowernumber => $self->{patron_id} + }, + { + order_by => 'accountno' + } + )->next(); + my $accountno = $last ? $last->accountno + 1 : 1; + + my $manager_id = $userenv ? $userenv->{number} : 0; + + my @outstanding_fines = Koha::Account::Lines->search( + { + borrowernumber => $self->{patron_id}, + amountoutstanding => { '>' => 0 }, + } + ); + + my $balance_remaining = $amount; + my @fines_paid; + foreach my $fine (@outstanding_fines) { + my $amount_to_pay = + $fine->amountoutstanding > $balance_remaining + ? $balance_remaining + : $fine->amountoutstanding; + + my $old_amountoutstanding = $fine->amountoutstanding; + $fine->amountoutstanding( $old_amountoutstanding - $amount_to_pay ); + $fine->store(); + + if ( C4::Context->preference("FinesLog") ) { + logaction( + "FINES", 'MODIFY', + $self->{patron_id}, + Dumper( + { + action => 'fee_payment', + borrowernumber => $fine->borrowernumber, + old_amountoutstanding => $old_amountoutstanding, + new_amountoutstanding => $fine->amountoutstanding, + amount_paid => $amount_to_pay, + accountlines_id => $fine->id, + accountno => $fine->accountno, + manager_id => $manager_id, + note => $note, + } + ) + ); + push( @fines_paid, $fine->id ); + } + + $balance_remaining = $balance_remaining - $amount_to_pay; + last unless $balance_remaining > 0; + } + + my $account_type = defined($sip) ? "Pay$sip" : 'Pay'; + + my $payment = Koha::Account::Line->new( + { + borrowernumber => $self->{patron_id}, + accountno => $accountno, + date => dt_from_string(), + amount => 0 - $amount, + description => q{}, + accounttype => $account_type, + amountoutstanding => 0 - $balance_remaining, + manager_id => $manager_id, + note => $note, + } + )->store(); + + my $branch = $userenv ? $userenv->{'branch'} : undef; + UpdateStats( + { + branch => $branch, + type => 'payment', + amount => $amount, + borrowernumber => $self->{patron_id}, + accountno => $accountno, + } + ); + + if ( C4::Context->preference("FinesLog") ) { + logaction( + "FINES", 'CREATE', + $self->{patron_id}, + Dumper( + { + action => 'create_payment', + borrowernumber => $self->{patron_id}, + accountno => $accountno, + amount => 0 - $amount, + amountoutstanding => 0 - $balance_remaining, + accounttype => 'Pay', + accountlines_paid => \@fines_paid, + manager_id => $manager_id, + } + ) + ); + } +} + +1; + +=head1 AUTHOR + +Kyle M Hall + +=cut diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm index 773ad20..f7578e7 100644 --- a/Koha/Account/Line.pm +++ b/Koha/Account/Line.pm @@ -37,7 +37,7 @@ Koha::Account::Lines - Koha accountline Object class =cut -sub type { +sub _type { return 'Accountline'; } diff --git a/Koha/Account/Lines.pm b/Koha/Account/Lines.pm index ae32774..0e0677c 100644 --- a/Koha/Account/Lines.pm +++ b/Koha/Account/Lines.pm @@ -40,7 +40,7 @@ Koha::Account::Lines - Koha Account Line Object set class =cut -sub type { +sub _type { return 'Accountline'; } diff --git a/t/db_dependent/Accounts.t b/t/db_dependent/Accounts.t index 5b9c764..442b5d8 100644 --- a/t/db_dependent/Accounts.t +++ b/t/db_dependent/Accounts.t @@ -24,6 +24,9 @@ use Test::Warn; use t::lib::TestBuilder; +use Koha::Account::Lines; +use Koha::Account::Line; + BEGIN { use_ok('C4::Accounts'); use_ok('Koha::Object'); @@ -153,19 +156,15 @@ subtest "recordpayment() tests" => sub { $borrower->branchcode( $branchcode ); $borrower->store; - my $sth = $dbh->prepare( - "INSERT INTO accountlines ( - borrowernumber, - amountoutstanding ) - VALUES ( ?, ? )" - ); - $sth->execute($borrower->borrowernumber, '100'); - $sth->execute($borrower->borrowernumber, '200'); + my $line1 = Koha::Account::Line->new({ borrowernumber => $borrower->borrowernumber, amountoutstanding => 100 })->store(); + my $line2 = Koha::Account::Line->new({ borrowernumber => $borrower->borrowernumber, amountoutstanding => 200 })->store(); + $line1->_result->discard_changes; + $line2->_result->discard_changes; $sth = $dbh->prepare("SELECT count(*) FROM accountlines"); $sth->execute; my $count = $sth->fetchrow_array; - is ($count, 2, 'There is 2 lines as expected'); + is($count, 2, 'There is 2 lines as expected'); # Testing recordpayment ------------------------- # There is $100 in the account @@ -175,7 +174,7 @@ subtest "recordpayment() tests" => sub { for my $line ( @$amountoutstanding ) { $amountleft += $line; } - ok($amountleft == 300, 'The account has 300$ as expected' ); + is($amountleft, 300, 'The account has 300$ as expected' ); # We make a $20 payment my $borrowernumber = $borrower->borrowernumber; @@ -190,7 +189,7 @@ subtest "recordpayment() tests" => sub { for my $line ( @$amountoutstanding ) { $amountleft += $line; } - ok($amountleft == 280, 'The account has $280 as expected' ); + is($amountleft, 280, 'The account has $280 as expected' ); # Is the payment note well registered $sth = $dbh->prepare("SELECT note FROM accountlines WHERE borrowernumber=? ORDER BY accountlines_id DESC LIMIT 1"); $sth->execute($borrower->borrowernumber); @@ -208,7 +207,7 @@ subtest "recordpayment() tests" => sub { for my $line ( @$amountoutstanding ) { $amountleft += $line; } - ok($amountleft == 310, 'The account has $310 as expected' ); + is($amountleft, 310, 'The account has $310 as expected' ); # Is the payment note well registered $sth = $dbh->prepare("SELECT note FROM accountlines WHERE borrowernumber=? ORDER BY accountlines_id DESC LIMIT 1"); $sth->execute($borrower->borrowernumber); @@ -226,7 +225,7 @@ subtest "recordpayment() tests" => sub { for my $line ( @$amountoutstanding ) { $amountleft += $line; } - ok($amountleft == 160, 'The account has $160 as expected' ); + is($amountleft, 160, 'The account has $160 as expected' ); # Is the payment note well registered $sth = $dbh->prepare("SELECT note FROM accountlines WHERE borrowernumber=? ORDER BY accountlines_id DESC LIMIT 1"); $sth->execute($borrower->borrowernumber); @@ -244,7 +243,7 @@ subtest "recordpayment() tests" => sub { for my $line ( @$amountoutstanding ) { $amountleft += $line; } - ok($amountleft == -40, 'The account has -$40 as expected, (credit situation)' ); + is($amountleft, -40, 'The account has -$40 as expected, (credit situation)' ); # Is the payment note well registered $sth = $dbh->prepare("SELECT note FROM accountlines WHERE borrowernumber=? ORDER BY accountlines_id DESC LIMIT 1"); $sth->execute($borrower->borrowernumber); -- 2.1.4