From 897ddc82b34b18576e4a2dd459691111836d6357 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 Signed-off-by: Josef Moravec Signed-off-by: Martin Renvoize Signed-off-by: Nick Clemens --- C4/Accounts.pm | 8 ++++++ C4/Circulation.pm | 30 +++++++++++++++------- C4/Reserves.pm | 25 +++++++++++++------ Koha/Account.pm | 4 +-- t/db_dependent/Accounts.t | 52 +++++++++++++++++++++++++++++++++++++-- 5 files changed, 99 insertions(+), 20 deletions(-) diff --git a/C4/Accounts.pm b/C4/Accounts.pm index ddddaa0ea0..d2c597a08a 100644 --- a/C4/Accounts.pm +++ b/C4/Accounts.pm @@ -107,6 +107,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 @@ -137,6 +140,7 @@ sub chargelostitem{ itemnumber => $itemnumber, note => $processingfeenote, manager_id => C4::Context->userenv ? C4::Context->userenv->{'number'} : 0, + branchcode => $branchcode, } )->store(); @@ -177,6 +181,7 @@ sub chargelostitem{ amountoutstanding => $replacementprice, itemnumber => $itemnumber, manager_id => C4::Context->userenv ? C4::Context->userenv->{'number'} : 0, + branchcode => $branchcode, } )->store(); @@ -241,6 +246,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, @@ -253,6 +260,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 017e9d7df0..483a104b7d 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; @@ -2878,15 +2879,23 @@ sub AddRenewal { 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 @@ -3220,6 +3229,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, @@ -3229,6 +3240,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 78e8f6a360..0fbe310903 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -49,6 +49,7 @@ use Koha::Items; use Koha::ItemTypes; use Koha::Patrons; use Koha::CirculationRules; +use Koha::Account::Lines; use List::MoreUtils qw( firstidx any ); use Carp; @@ -566,13 +567,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 eeb87a546c..33c5234733 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -81,6 +81,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} ); @@ -219,6 +220,7 @@ sub pay { payment_type => $payment_type, amountoutstanding => 0 - $balance_remaining, manager_id => $manager_id, + branchcode => $library_id, note => $note, } )->store(); @@ -228,8 +230,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 f76f85c6f0..3813dea6f0 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; @@ -69,6 +69,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 @@ -134,7 +181,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 }; @@ -257,6 +304,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.39.5