From b594c51a3a9042ee10b6d2998b1a294363228725 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Wed, 9 Aug 2017 16:55:56 +0000 Subject: [PATCH] Bug 19066: Add branchcode to accountlines For the purposes of statistics, it appears that it would help many libraries to have branchcode recorded in the accountlines table. For payments, the field would contain the code for the branch the payment was made at. For manual invoices, it would be the code of the library that created the invoice. Test Plan: 1) Apply this patch set 2) Create and pay some fees 3) Note the branchcode for those fees and payments is set to your logged in branch --- C4/Accounts.pm | 8 ++++++++ C4/Circulation.pm | 32 ++++++++++++++++++++--------- C4/Reserves.pm | 25 ++++++++++++++++------- Koha/Account.pm | 4 ++-- t/db_dependent/Accounts.t | 52 +++++++++++++++++++++++++++++++++++++++++++++-- 5 files changed, 100 insertions(+), 21 deletions(-) diff --git a/C4/Accounts.pm b/C4/Accounts.pm index eef9766b20..5dc0eb5146 100644 --- a/C4/Accounts.pm +++ b/C4/Accounts.pm @@ -141,6 +141,9 @@ sub chargelostitem{ if ($usedefaultreplacementcost && $amount == 0 && $defaultreplacecost){ $replacementprice = $defaultreplacecost; } + + my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; + # first make sure the borrower hasn't already been charged for this item # FIXME this should be more exact # there is no reason a user can't lose an item, find and return it, and lost it again @@ -168,6 +171,7 @@ sub chargelostitem{ itemnumber => $itemnumber, note => $processingfeenote, manager_id => C4::Context->userenv ? C4::Context->userenv->{'number'} : 0, + branchcode => $branchcode, } )->store(); @@ -207,6 +211,7 @@ sub chargelostitem{ amountoutstanding => $replacementprice, itemnumber => $itemnumber, manager_id => C4::Context->userenv ? C4::Context->userenv->{'number'} : 0, + branchcode => $branchcode, } )->store(); @@ -271,6 +276,8 @@ sub manualinvoice { my $accountno = getnextacctno($borrowernumber); my $amountleft = $amount; + my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; + my $accountline = Koha::Account::Line->new( { borrowernumber => $borrowernumber, @@ -283,6 +290,7 @@ sub manualinvoice { itemnumber => $itemnum || undef, note => $note, manager_id => $manager_id, + branchcode => $branchcode, } )->store(); diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 87812a3ce1..1ebd90b557 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -52,6 +52,7 @@ use Koha::Patrons; use Koha::Patron::Debarments; use Koha::Database; use Koha::Libraries; +use Koha::Account::Lines; use Koha::Holds; use Koha::RefundLostItemFeeRule; use Koha::RefundLostItemFeeRules; @@ -2847,18 +2848,26 @@ sub AddRenewal { # Charge a new rental fee, if applicable? my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber ); if ( $charge > 0 ) { - my $accountno = getnextacctno( $borrowernumber ); + my $accountno = C4::Accounts::getnextacctno( $borrowernumber ); my $manager_id = 0; $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv; - $sth = $dbh->prepare( - "INSERT INTO accountlines - (date, borrowernumber, accountno, amount, manager_id, - description,accounttype, amountoutstanding, itemnumber) - VALUES (now(),?,?,?,?,?,?,?,?)" - ); - $sth->execute( $borrowernumber, $accountno, $charge, $manager_id, - "Renewal of Rental Item " . $biblio->title . " $item->{'barcode'}", - 'Rent', $charge, $itemnumber ); + my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; + Koha::Account::Line->new( + { + date => dt_from_string(), + borrowernumber => $borrowernumber, + accountno => $accountno, + amount => $charge, + manager_id => $manager_id, + accounttype => 'Rent', + amountoutstanding => $charge, + itemnumber => $itemnumber, + branchcode => $branchcode, + description => 'Renewal of Rental Item ' + . $biblio->title + . " $item->{'barcode'}", + } + )->store(); } # Send a renewal slip according to checkout alert preferencei @@ -3192,6 +3201,8 @@ sub AddIssuingCharge { my $manager_id = 0; $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv; + my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; + my $accountline = Koha::Account::Line->new( { borrowernumber => $checkout->borrowernumber, @@ -3201,6 +3212,7 @@ sub AddIssuingCharge { amount => $charge, amountoutstanding => $charge, manager_id => $manager_id, + branchcode => $branchcode, description => 'Rental', accounttype => 'Rent', date => \'NOW()', diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 359fd984b6..f974aae5f9 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -48,6 +48,7 @@ use Koha::IssuingRules; use Koha::Items; use Koha::ItemTypes; use Koha::Patrons; +use Koha::Account::Lines; use List::MoreUtils qw( firstidx any ); use Carp; @@ -519,13 +520,23 @@ sub GetOtherReserves { sub ChargeReserveFee { my ( $borrowernumber, $fee, $title ) = @_; - return if !$fee || $fee==0; # the last test is needed to include 0.00 - my $accquery = qq{ -INSERT INTO accountlines ( borrowernumber, accountno, date, amount, description, accounttype, amountoutstanding ) VALUES (?, ?, NOW(), ?, ?, 'Res', ?) - }; - my $dbh = C4::Context->dbh; - my $nextacctno = &getnextacctno( $borrowernumber ); - $dbh->do( $accquery, undef, ( $borrowernumber, $nextacctno, $fee, "Reserve Charge - $title", $fee ) ); + + return if !$fee || $fee == 0; # the last test is needed to include 0.00 + + my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; + my $nextacctno = C4::Accounts::getnextacctno($borrowernumber); + + Koha::Account::Line->new( + { + borrowernumber => $borrowernumber, + accountno => $nextacctno, + date => dt_from_string(), + amount => $fee, + description => "Reserve Charge - $title", + accounttype => 'Res', + amountoutstanding => $fee, + } + )->store(); } =head2 GetReserveFee diff --git a/Koha/Account.pm b/Koha/Account.pm index 4926f9e2e8..9d6b15853e 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -79,6 +79,7 @@ sub pay { my $offset_type = $params->{offset_type} || $type eq 'writeoff' ? 'Writeoff' : 'Payment'; my $userenv = C4::Context->userenv; + $library_id ||= $userenv ? $userenv->{branch} : undef; my $patron = Koha::Patrons->find( $self->{patron_id} ); @@ -223,6 +224,7 @@ sub pay { payment_type => $payment_type, amountoutstanding => 0 - $balance_remaining, manager_id => $manager_id, + branchcode => $library_id, note => $note, } )->store(); @@ -232,8 +234,6 @@ sub pay { $o->store(); } - $library_id ||= $userenv ? $userenv->{'branch'} : undef; - UpdateStats( { branch => $library_id, diff --git a/t/db_dependent/Accounts.t b/t/db_dependent/Accounts.t index 26a789c96a..f8073d008f 100644 --- a/t/db_dependent/Accounts.t +++ b/t/db_dependent/Accounts.t @@ -18,7 +18,7 @@ use Modern::Perl; -use Test::More tests => 28; +use Test::More tests => 41; use Test::MockModule; use Test::Warn; @@ -71,6 +71,53 @@ $context->mock( 'userenv', sub { branch => $branchcode, }; }); +my $userenv_branchcode = $branchcode; + +# Test chargelostitem +my $itemtype = $builder->build( { source => 'Itemtype' } ); +my $item = $builder->build( { source => 'Item', value => { itype => $itemtype->{itemtype} } } ); +my $patron = $builder->build( { source => 'Borrower' } ); +my $amount = '5.000000'; +my $description = "Test fee!"; +chargelostitem( $patron->{borrowernumber}, $item->{itemnumber}, $amount, $description ); +my ($accountline) = Koha::Account::Lines->search( + { + borrowernumber => $patron->{borrowernumber} + } +); +is( $accountline->amount, $amount, 'Accountline amount set correctly for chargelostitem' ); +is( $accountline->description, $description, 'Accountline description set correctly for chargelostitem' ); +is( $accountline->branchcode, $branchcode, 'Accountline branchcode set correctly for chargelostitem' ); +$dbh->do(q|DELETE FROM accountlines|); + +# Test manualinvoice, reuse some of the vars from testing chargelostitem +my $type = 'L'; +my $note = 'Test note!'; +manualinvoice( $patron->{borrowernumber}, $item->{itemnumber}, $description, $type, $amount, $note ); +($accountline) = Koha::Account::Lines->search( + { + borrowernumber => $patron->{borrowernumber} + } +); +is( $accountline->accounttype, $type, 'Accountline type set correctly for manualinvoice' ); +is( $accountline->amount, $amount, 'Accountline amount set correctly for manualinvoice' ); +ok( $accountline->description =~ /^$description/, 'Accountline description set correctly for manualinvoice' ); +is( $accountline->note, $note, 'Accountline note set correctly for manualinvoice' ); +is( $accountline->branchcode, $branchcode, 'Accountline branchcode set correctly for manualinvoice' ); + +# Test _FixAccountForLostAndReturned, use the accountline from the manualinvoice to test +C4::Circulation::_FixAccountForLostAndReturned( $item->{itemnumber} ); +my ( $accountline_fee, $accountline_payment ) = Koha::Account::Lines->search( + { + borrowernumber => $patron->{borrowernumber} + } +); +is( $accountline_fee->accounttype, 'LR', 'Lost item fee account type updated to LR' ); +is( $accountline_fee->amountoutstanding, '0.000000', 'Lost item fee amount outstanding updated to 0' ); +is( $accountline_payment->accounttype, 'CR', 'Lost item fee account type is CR' ); +is( $accountline_payment->amount, "-$amount", 'Lost item refund amount is correct' ); +is( $accountline_payment->branchcode, $branchcode, 'Lost item refund branchcode is set correctly' ); +$dbh->do(q|DELETE FROM accountlines|); # Testing purge_zero_balance_fees @@ -136,7 +183,7 @@ $dbh->do(q|DELETE FROM accountlines|); subtest "Koha::Account::pay tests" => sub { - plan tests => 13; + plan tests => 14; # Create a borrower my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; @@ -259,6 +306,7 @@ subtest "Koha::Account::pay tests" => sub { is( $payment->amount(), '-42.000000', "Payment paid the specified fine" ); $line3 = Koha::Account::Lines->find( $line3->id ); is( $line3->amountoutstanding, '0.000000', "Specified fine is paid" ); + is( $payment->branchcode, $userenv_branchcode, 'Branchcode set correctly' ); }; subtest "Koha::Account::pay particular line tests" => sub { -- 2.11.0