From 5220047ce1a17a5957b2128e105f0174ba1e91b7 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 Signed-off-by: Lisette Scheer Signed-off-by: Lisette Scheer --- 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 f087ba6..0b47603 100644 --- a/C4/Accounts.pm +++ b/C4/Accounts.pm @@ -140,6 +140,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 @@ -167,6 +170,7 @@ sub chargelostitem{ itemnumber => $itemnumber, note => $processingfeenote, manager_id => C4::Context->userenv ? C4::Context->userenv->{'number'} : 0, + branchcode => $branchcode, } )->store(); @@ -206,6 +210,7 @@ sub chargelostitem{ amountoutstanding => $replacementprice, itemnumber => $itemnumber, manager_id => C4::Context->userenv ? C4::Context->userenv->{'number'} : 0, + branchcode => $branchcode, } )->store(); @@ -270,6 +275,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, @@ -282,6 +289,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 dce77ea..ce95078 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; @@ -2851,18 +2852,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 @@ -3196,6 +3205,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, @@ -3205,6 +3216,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 9bac1e9..415c6d4 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; @@ -556,13 +557,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 4926f9e..9d6b158 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 26a789c..f8073d0 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.1.4