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 <lisetteslatah@gmail.com>

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

diff --git a/C4/Accounts.pm b/C4/Accounts.pm
index ddddaa0..d2c597a 100644 (file)
--- 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 017e9d7..483a104 100644 (file)
--- 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 78e8f6a..0fbe310 100644 (file)
--- 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 eeb87a5..33c5234 100644 (file)
--- 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 f76f85c..3813dea 100644 (file)
--- 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 {