Bug 14826: Resurrect account offsets table

The account offsets table should be used to track increments and
decrements of fines via payments and credits, as well as fine accruals.
It should be able to match fees to payments and visa versa, so we can
know which fee was paid by a given payment, and which payments applied
to a given fee.

Test Plan:
1) Apply this patch
2) Run updatedatabase
3) Note the table accountoffsets has been renamed to account_offsets
4) Ensure fine generation creates offsets
5) Ensure creating a manual invoice creates an offset
6) Ensure a lost item charge creates an offset
7) Ensure Reverse Payment creates an offset
8) Ensure a payment creates an offset
9) Ensure a payment for multiple fees creates an offset for each
10) Ensure writeoffs create offsets

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Kyle M Hall 2016-07-12 14:48:03 +00:00 committed by Jonathan Druart
parent ca34994f23
commit 0dccc8ec5b
4 changed files with 246 additions and 163 deletions

View File

@ -26,7 +26,9 @@ use C4::Members;
use C4::Circulation qw(ReturnLostItem);
use C4::Log qw(logaction);
use Koha::Account;
use Koha::Account::Line;
use Koha::Account::Lines;
use Koha::Account::Offset;
use Data::Dumper qw(Dumper);
@ -118,33 +120,61 @@ EOT
=cut
=head2 chargelostitem
In a default install of Koha the following lost values are set
1 = Lost
2 = Long overdue
3 = Lost and paid for
FIXME: itemlost should be set to 3 after payment is made, should be a warning to the interface that a charge has been added
FIXME : if no replacement price, borrower just doesn't get charged?
=cut
sub chargelostitem{
# lost ==1 Lost, lost==2 longoverdue, lost==3 lost and paid for
# FIXME: itemlost should be set to 3 after payment is made, should be a warning to the interface that
# a charge has been added
# FIXME : if no replacement price, borrower just doesn't get charged?
my $dbh = C4::Context->dbh();
my ($borrowernumber, $itemnumber, $amount, $description) = @_;
# first make sure the borrower hasn't already been charged for this item
my $sth1=$dbh->prepare("SELECT * from accountlines
WHERE borrowernumber=? AND itemnumber=? and accounttype='L'");
$sth1->execute($borrowernumber,$itemnumber);
my $existing_charge_hashref=$sth1->fetchrow_hashref();
my $existing_charges = Koha::Account::Lines->search(
{
borrowernumber => $borrowernumber,
itemnumber => $itemnumber,
accounttype => 'L',
}
)->count();
# OK, they haven't
unless ($existing_charge_hashref) {
unless ($existing_charges) {
my $manager_id = 0;
$manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv;
# This item is on issue ... add replacement cost to the borrower's record and mark it returned
# Note that we add this to the account even if there's no replacement price, allowing some other
# process (or person) to update it, since we don't handle any defaults for replacement prices.
my $accountno = getnextacctno($borrowernumber);
my $sth2=$dbh->prepare("INSERT INTO accountlines
(borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding,itemnumber,manager_id)
VALUES (?,?,now(),?,?,'L',?,?,?)");
$sth2->execute($borrowernumber,$accountno,$amount,
$description,$amount,$itemnumber,$manager_id);
my $accountline = Koha::Account::Line->new(
{
borrowernumber => $borrowernumber,
accountno => $accountno,
date => \'NOW()',
amount => $amount,
description => $description,
accounttype => 'L',
amountoutstanding => $amount,
itemnumber => $itemnumber,
manager_id => $manager_id,
}
)->store();
my $account_offset = Koha::Account::Offset->new(
{
debit_id => $accountline->id,
type => 'Lost Item',
amount => $amount,
}
)->store();
if ( C4::Context->preference("FinesLog") ) {
logaction("FINES", 'CREATE', $borrowernumber, Dumper({
@ -208,21 +238,29 @@ sub manualinvoice {
$notifyid = 1;
}
if ( $itemnum ) {
$desc .= ' ' . $itemnum;
my $sth = $dbh->prepare(
'INSERT INTO accountlines
(borrowernumber, accountno, date, amount, description, accounttype, amountoutstanding, itemnumber,notify_id, note, manager_id)
VALUES (?, ?, now(), ?,?, ?,?,?,?,?,?)');
$sth->execute($borrowernumber, $accountno, $amount, $desc, $type, $amountleft, $itemnum,$notifyid, $note, $manager_id) || return $sth->errstr;
} else {
my $sth=$dbh->prepare("INSERT INTO accountlines
(borrowernumber, accountno, date, amount, description, accounttype, amountoutstanding,notify_id, note, manager_id)
VALUES (?, ?, now(), ?, ?, ?, ?,?,?,?)"
);
$sth->execute( $borrowernumber, $accountno, $amount, $desc, $type,
$amountleft, $notifyid, $note, $manager_id );
}
my $accountline = Koha::Account::Line->new(
{
borrowernumber => $borrowernumber,
accountno => $accountno,
date => \'NOW()',
amount => $amount,
description => $desc,
accounttype => $type,
amountoutstanding => $amountleft,
itemnumber => $itemnum || undef,
notify_id => $notifyid,
note => $note,
manager_id => $manager_id,
}
)->store();
my $account_offset = Koha::Account::Offset->new(
{
debit_id => $accountline->id,
type => 'Manual Debit',
amount => $amount,
}
)->store();
if ( C4::Context->preference("FinesLog") ) {
logaction("FINES", 'CREATE',$borrowernumber,Dumper({
@ -308,45 +346,50 @@ sub getrefunds {
return (@results);
}
#FIXME: ReversePayment should be replaced with a Void Payment feature
sub ReversePayment {
my ( $accountlines_id ) = @_;
my ($accountlines_id) = @_;
my $dbh = C4::Context->dbh;
my $sth = $dbh->prepare('SELECT * FROM accountlines WHERE accountlines_id = ?');
$sth->execute( $accountlines_id );
my $row = $sth->fetchrow_hashref();
my $amount_outstanding = $row->{'amountoutstanding'};
my $accountline = Koha::Account::Lines->find($accountlines_id);
my $amount_outstanding = $accountline->amountoutstanding;
if ( $amount_outstanding <= 0 ) {
$sth = $dbh->prepare('UPDATE accountlines SET amountoutstanding = amount * -1, description = CONCAT( description, " Reversed -" ) WHERE accountlines_id = ?');
$sth->execute( $accountlines_id );
} else {
$sth = $dbh->prepare('UPDATE accountlines SET amountoutstanding = 0, description = CONCAT( description, " Reversed -" ) WHERE accountlines_id = ?');
$sth->execute( $accountlines_id );
}
my $new_amountoutstanding =
$amount_outstanding <= 0 ? $accountline->amount * -1 : 0;
$accountline->description( $accountline->description . " Reversed -" );
$accountline->amountoutstanding($new_amountoutstanding);
$accountline->store();
my $account_offset = Koha::Account::Offset->new(
{
credit_id => $accountline->id,
type => 'Reverse Payment',
amount => $amount_outstanding - $new_amountoutstanding,
}
)->store();
if ( C4::Context->preference("FinesLog") ) {
my $manager_id = 0;
$manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv;
if ( $amount_outstanding <= 0 ) {
$row->{'amountoutstanding'} *= -1;
} else {
$row->{'amountoutstanding'} = '0';
}
$row->{'description'} .= ' Reversed -';
logaction("FINES", 'MODIFY', $row->{'borrowernumber'}, Dumper({
action => 'reverse_fee_payment',
borrowernumber => $row->{'borrowernumber'},
old_amountoutstanding => $row->{'amountoutstanding'},
new_amountoutstanding => 0 - $amount_outstanding,,
accountlines_id => $row->{'accountlines_id'},
accountno => $row->{'accountno'},
manager_id => $manager_id,
}));
logaction(
"FINES", 'MODIFY',
$accountline->borrowernumber,
Dumper(
{
action => 'reverse_fee_payment',
borrowernumber => $accountline->borrowernumber,
old_amountoutstanding => $amount_outstanding,
new_amountoutstanding => $new_amountoutstanding,
,
accountlines_id => $accountline->id,
accountno => $accountline->accountno,
manager_id => $manager_id,
}
)
);
}
}
=head2 purge_zero_balance_fees

View File

@ -54,6 +54,9 @@ use Koha::Libraries;
use Koha::Holds;
use Koha::RefundLostItemFeeRule;
use Koha::RefundLostItemFeeRules;
use Koha::Account::Lines;
use Koha::Account::Line;
use Koha::Account::Offset;
use Carp;
use List::MoreUtils qw( uniq );
use Scalar::Util qw( looks_like_number );
@ -2318,39 +2321,65 @@ sub _FixOverduesOnReturn {
my $dbh = C4::Context->dbh;
# check for overdue fine
my $sth = $dbh->prepare(
"SELECT * FROM accountlines WHERE (borrowernumber = ?) AND (itemnumber = ?) AND (accounttype='FU' OR accounttype='O')"
);
$sth->execute( $borrowernumber, $item );
# alter fine to show that the book has been returned
my $data = $sth->fetchrow_hashref;
return 0 unless $data; # no warning, there's just nothing to fix
my $accountline = Koha::Account::Lines->search(
{
borrowernumber => $borrowernumber,
itemnumber => $item,
-or => [
accounttype => 'FU',
accounttype => 'O',
],
}
)->next();
return 0 unless $accountline; # no warning, there's just nothing to fix
my $uquery;
my @bind = ($data->{'accountlines_id'});
if ($exemptfine) {
$uquery = "update accountlines set accounttype='FFOR', amountoutstanding=0";
my $amountoutstanding = $accountline->amountoutstanding;
$accountline->accounttype('FFOR');
$accountline->amountoutstanding(0);
Koha::Account::Offset->new(
{
debit_id => $accountline->id,
type => 'Forgiven',
amount => $amountoutstanding * -1,
}
);
if (C4::Context->preference("FinesLog")) {
&logaction("FINES", 'MODIFY',$borrowernumber,"Overdue forgiven: item $item");
}
} elsif ($dropbox && $data->{lastincrement}) {
my $outstanding = $data->{amountoutstanding} - $data->{lastincrement} ;
my $amt = $data->{amount} - $data->{lastincrement} ;
if (C4::Context->preference("FinesLog")) {
&logaction("FINES", 'MODIFY',$borrowernumber,"Dropbox adjustment $amt, item $item");
} elsif ($dropbox && $accountline->lastincrement) {
my $outstanding = $accountline->amountoutstanding - $accountline->lastincrement;
my $amt = $accountline->amount - $accountline->lastincrement;
Koha::Account::Offset->new(
{
debit_id => $accountline->id,
type => 'Dropbox',
amount => $accountline->lastincrement * -1,
}
);
if ( C4::Context->preference("FinesLog") ) {
&logaction( "FINES", 'MODIFY', $borrowernumber,
"Dropbox adjustment $amt, item $item" );
}
$uquery = "update accountlines set accounttype='F' ";
if($outstanding >= 0 && $amt >=0) {
$uquery .= ", amount = ? , amountoutstanding=? ";
unshift @bind, ($amt, $outstanding) ;
$accountline->accounttype('F');
if ( $outstanding >= 0 && $amt >= 0 ) {
$accountline->amount($amt);
$accountline->amountoutstanding($outstanding);
}
} else {
$uquery = "update accountlines set accounttype='F' ";
$accountline->accounttype('F');
}
$uquery .= " where (accountlines_id = ?)";
my $usth = $dbh->prepare($uquery);
return $usth->execute(@bind);
return $accountline->store();
}
=head2 _FixAccountForLostAndReturned
@ -2361,83 +2390,45 @@ Calculates the charge for a book lost and returned.
Internal function, not exported, called only by AddReturn.
FIXME: This function reflects how inscrutable fines logic is. Fix both.
FIXME: Give a positive return value on success. It might be the $borrowernumber who received credit, or the amount forgiven.
=cut
sub _FixAccountForLostAndReturned {
my $itemnumber = shift or return;
my $borrowernumber = @_ ? shift : undef;
my $item_id = @_ ? shift : $itemnumber; # Send the barcode if you want that logged in the description
my $dbh = C4::Context->dbh;
# check for charge made for lost book
my $sth = $dbh->prepare("SELECT * FROM accountlines WHERE itemnumber = ? AND accounttype IN ('L', 'Rep', 'W') ORDER BY date DESC, accountno DESC");
$sth->execute($itemnumber);
my $data = $sth->fetchrow_hashref;
$data or return; # bail if there is nothing to do
$data->{accounttype} eq 'W' and return; # Written off
# writeoff this amount
my $offset;
my $amount = $data->{'amount'};
my $acctno = $data->{'accountno'};
my $amountleft; # Starts off undef/zero.
if ($data->{'amountoutstanding'} == $amount) {
$offset = $data->{'amount'};
$amountleft = 0; # Hey, it's zero here, too.
} else {
$offset = $amount - $data->{'amountoutstanding'}; # Um, isn't this the same as ZERO? We just tested those two things are ==
$amountleft = $data->{'amountoutstanding'} - $amount; # Um, isn't this the same as ZERO? We just tested those two things are ==
}
my $usth = $dbh->prepare("UPDATE accountlines SET accounttype = 'LR',amountoutstanding='0'
WHERE (accountlines_id = ?)");
$usth->execute($data->{'accountlines_id'}); # We might be adjusting an account for some OTHER borrowernumber now. Not the one we passed in.
#check if any credit is left if so writeoff other accounts
my $nextaccntno = getnextacctno($data->{'borrowernumber'});
$amountleft *= -1 if ($amountleft < 0);
if ($amountleft > 0) {
my $msth = $dbh->prepare("SELECT * FROM accountlines WHERE (borrowernumber = ?)
AND (amountoutstanding >0) ORDER BY date"); # might want to order by amountoustanding ASC (pay smallest first)
$msth->execute($data->{'borrowernumber'});
# offset transactions
my $newamtos;
my $accdata;
while (($accdata=$msth->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'};
# FIXME: move prepares outside while loop!
my $usth = $dbh->prepare("UPDATE accountlines SET amountoutstanding= ?
WHERE (accountlines_id = ?)");
$usth->execute($newamtos,$thisacct);
$usth = $dbh->prepare("INSERT INTO accountoffsets
(borrowernumber, accountno, offsetaccount, offsetamount)
VALUES
(?,?,?,?)");
$usth->execute($data->{'borrowernumber'},$accdata->{'accountno'},$nextaccntno,$newamtos);
# check for charge made for lost book
my $accountline = Koha::Account::Lines->search(
{
itemnumber => $itemnumber,
accounttype => { -in => [ 'L', 'Rep', 'W' ] },
},
{
order_by => { -desc => [ 'date', 'accountno' ] }
}
}
$amountleft *= -1 if ($amountleft > 0);
my $desc = "Item Returned " . $item_id;
$usth = $dbh->prepare("INSERT INTO accountlines
(borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding)
VALUES (?,?,now(),?,?,'CR',?)");
$usth->execute($data->{'borrowernumber'},$nextaccntno,0-$amount,$desc,$amountleft);
if ($borrowernumber) {
# FIXME: same as query above. use 1 sth for both
$usth = $dbh->prepare("INSERT INTO accountoffsets
(borrowernumber, accountno, offsetaccount, offsetamount)
VALUES (?,?,?,?)");
$usth->execute($borrowernumber, $data->{'accountno'}, $nextaccntno, $offset);
}
ModItem({ paidfor => '' }, undef, $itemnumber);
return;
)->next();
return unless $accountline;
return if $accountline->accounttype eq 'W'; # Written off
$accountline->accounttype('LR');
$accountline->store();
my $account = Koha::Account->new( { patron_id => $accountline->borrowernumber } );
my $credit_id = $account->pay(
{
amount => $accountline->amount,
description => "Item Returned " . $item_id,
account_type => 'CR',
offset_type => 'Lost Item Return',
accounlines => [$accountline],
}
);
ModItem( { paidfor => '' }, undef, $itemnumber );
return $credit_id;
}
=head2 _GetCircControlBranch

View File

@ -36,6 +36,7 @@ use C4::Debug;
use Koha::DateUtils;
use Koha::Account::Line;
use Koha::Account::Lines;
use Koha::Account::Offset;
use Koha::IssuingRules;
use Koha::Libraries;
@ -591,6 +592,14 @@ sub UpdateFine {
accounttype => 'FU',
}
)->store();
Koha::Account::Offset->new(
{
debit_id => $accountline->id,
type => 'Fine Update',
amount => $diff,
}
)->store();
}
} else {
if ( $amount ) { # Don't add new fines with an amount of 0
@ -618,6 +627,14 @@ sub UpdateFine {
issue_id => $issue_id,
}
)->store();
Koha::Account::Offset->new(
{
debit_id => $accountline->id,
type => 'Fine',
amount => $amount,
}
)->store();
}
}
# logging action

View File

@ -27,6 +27,7 @@ use C4::Stats qw( UpdateStats );
use Koha::Account::Line;
use Koha::Account::Lines;
use Koha::Account::Offset;
use Koha::DateUtils qw( dt_from_string );
=head1 NAME
@ -49,11 +50,14 @@ This method allows payments to be made against fees/fines
Koha::Account->new( { patron_id => $borrowernumber } )->pay(
{
amount => $amount,
sip => $sipmode,
note => $note,
library_id => $branchcode,
lines => $lines, # Arrayref of Koha::Account::Line objects to pay
amount => $amount,
sip => $sipmode,
note => $note,
description => $description,
library_id => $branchcode,
lines => $lines, # Arrayref of Koha::Account::Line objects to pay
account_type => $type, # accounttype code
offset_type => $offset_type, # offset type code
}
);
@ -62,12 +66,15 @@ Koha::Account->new( { patron_id => $borrowernumber } )->pay(
sub pay {
my ( $self, $params ) = @_;
my $amount = $params->{amount};
my $sip = $params->{sip};
my $note = $params->{note} || q{};
my $library_id = $params->{library_id};
my $lines = $params->{lines};
my $type = $params->{type} || 'payment';
my $amount = $params->{amount};
my $sip = $params->{sip};
my $description = $params->{description};
my $note = $params->{note} || q{};
my $library_id = $params->{library_id};
my $lines = $params->{lines};
my $type = $params->{type} || 'payment';
my $account_type = $params->{account_type};
my $offset_type = $params->{offset_type} || $type eq 'writeoff' ? 'Writeoff' : 'Payment';
my $userenv = C4::Context->userenv;
@ -89,6 +96,8 @@ sub pay {
my $balance_remaining = $amount; # Set it now so we can adjust the amount if necessary
$balance_remaining ||= 0;
my @account_offsets;
# We were passed a specific line to pay
foreach my $fine ( @$lines ) {
my $amount_to_pay =
@ -106,6 +115,15 @@ sub pay {
C4::Circulation::ReturnLostItem( $self->{patron_id}, $fine->itemnumber );
}
my $account_offset = Koha::Account::Offset->new(
{
debit_id => $fine->id,
type => $offset_type,
amount => $amount_to_pay * -1,
}
);
push( @account_offsets, $account_offset );
if ( C4::Context->preference("FinesLog") ) {
logaction(
"FINES", 'MODIFY',
@ -149,6 +167,15 @@ sub pay {
$fine->amountoutstanding( $old_amountoutstanding - $amount_to_pay );
$fine->store();
my $account_offset = Koha::Account::Offset->new(
{
debit_id => $fine->id,
type => $offset_type,
amount => $amount_to_pay * -1,
}
);
push( @account_offsets, $account_offset );
if ( C4::Context->preference("FinesLog") ) {
logaction(
"FINES", 'MODIFY',
@ -174,12 +201,12 @@ sub pay {
last unless $balance_remaining > 0;
}
my $account_type =
$account_type ||=
$type eq 'writeoff' ? 'W'
: defined($sip) ? "Pay$sip"
: 'Pay';
my $description = $type eq 'writeoff' ? 'Writeoff' : q{};
$description ||= $type eq 'writeoff' ? 'Writeoff' : q{};
my $payment = Koha::Account::Line->new(
{
@ -195,6 +222,11 @@ sub pay {
}
)->store();
foreach my $o ( @account_offsets ) {
$o->credit_id( $payment->id() );
$o->store();
}
$library_id ||= $userenv ? $userenv->{'branch'} : undef;
UpdateStats(