From a2751c4dc4d40a39a139e295ea29c6317c0a738c 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 | 33 ++++++++++++++++++++---------- C4/Reserves.pm | 25 ++++++++++++++++------- Koha/Account.pm | 5 +++-- t/db_dependent/Accounts.t | 51 +++++++++++++++++++++++++++++++++++++++++++++-- 5 files changed, 100 insertions(+), 22 deletions(-) diff --git a/C4/Accounts.pm b/C4/Accounts.pm index b84627b..cd91ede 100644 --- a/C4/Accounts.pm +++ b/C4/Accounts.pm @@ -159,6 +159,7 @@ sub chargelostitem{ unless ($existing_charges) { #add processing fee if ($processfee && $processfee > 0){ + my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; my $accountline = Koha::Account::Line->new( { borrowernumber => $borrowernumber, @@ -171,6 +172,7 @@ sub chargelostitem{ itemnumber => $itemnumber, note => $processingfeenote, manager_id => C4::Context->userenv ? C4::Context->userenv->{'number'} : 0, + branchcode => $branchcode, } )->store(); @@ -194,6 +196,7 @@ sub chargelostitem{ note => $accountline->note, itemnumber => $accountline->itemnumber, manager_id => $accountline->manager_id, + branchcode => $branchcode, })); } } @@ -274,6 +277,10 @@ sub manualinvoice { my $accountno = getnextacctno($borrowernumber); my $amountleft = $amount; + $itemnum ||= undef; + + my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; + my $accountline = Koha::Account::Line->new( { borrowernumber => $borrowernumber, @@ -286,6 +293,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 2b7722c..1f303f5 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -51,6 +51,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; @@ -2859,18 +2860,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 @@ -3202,6 +3211,7 @@ 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 => $borrowernumber, @@ -3210,13 +3220,14 @@ sub AddIssuingCharge { amount => $charge, amountoutstanding => $charge, manager_id => $manager_id, + branchcode => $branchcode, description => 'Rental', accounttype => 'Rent', date => \'NOW()', } )->store(); - Koha::Account::Offset->new( + return Koha::Account::Offset->new( { debit_id => $accountline->id, type => 'Rental Fee', diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 216ecec..69fc634 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; @@ -508,13 +509,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 ed1fd3c..4ad46f7 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -75,6 +75,8 @@ sub pay { my $account_type = $params->{account_type}; my $offset_type = $params->{offset_type} || $type eq 'writeoff' ? 'Writeoff' : 'Payment'; + $library_id ||= C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; + my $userenv = C4::Context->userenv; # We should remove accountno, it is no longer needed @@ -217,6 +219,7 @@ sub pay { accounttype => $account_type, amountoutstanding => 0 - $balance_remaining, manager_id => $manager_id, + branchcode => $library_id, note => $note, } )->store(); @@ -226,8 +229,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 51c094a..021cdb3 100644 --- a/t/db_dependent/Accounts.t +++ b/t/db_dependent/Accounts.t @@ -18,7 +18,7 @@ use Modern::Perl; -use Test::More tests => 23; +use Test::More tests => 36; use Test::MockModule; use Test::Warn; @@ -72,6 +72,52 @@ $context->mock( 'userenv', sub { branch => $branchcode, }; }); +my $userenv_branchcode = $branchcode; + +# Test chargelostitem +my $item = $builder->build( { source => 'Item' } ); +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 @@ -137,7 +183,7 @@ $dbh->do(q|DELETE FROM accountlines|); subtest "Koha::Account::pay tests" => sub { - plan tests => 12; + plan tests => 13; # Create a borrower my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; @@ -257,6 +303,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.10.2