From 0dccc8ec5b2a4347cbc78e08aa905b79f1136787 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Tue, 12 Jul 2016 14:48:03 +0000 Subject: [PATCH] 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 Signed-off-by: Tomas Cohen Arazi Signed-off-by: Jonathan Druart --- C4/Accounts.pm | 161 +++++++++++++++++++++++++---------------- C4/Circulation.pm | 177 ++++++++++++++++++++++------------------------ C4/Overdues.pm | 17 +++++ Koha/Account.pm | 58 +++++++++++---- 4 files changed, 248 insertions(+), 165 deletions(-) diff --git a/C4/Accounts.pm b/C4/Accounts.pm index 0a1e779632..2a9c3af080 100644 --- a/C4/Accounts.pm +++ b/C4/Accounts.pm @@ -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'}; - - 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 $accountline = Koha::Account::Lines->find($accountlines_id); + my $amount_outstanding = $accountline->amountoutstanding; + + 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 diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 48d768b9cc..9aee06efd9 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -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); - } - } - $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; + my $accountline = Koha::Account::Lines->search( + { + itemnumber => $itemnumber, + accounttype => { -in => [ 'L', 'Rep', 'W' ] }, + }, + { + order_by => { -desc => [ 'date', 'accountno' ] } + } + )->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 diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 65b1296295..07c9bbe030 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -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 diff --git a/Koha/Account.pm b/Koha/Account.pm index 9ec1c1b2f9..616ebebf71 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -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( -- 2.39.5