From 8e5a80027bbe4b2b4adfd639a2e195cfc18f517b Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Wed, 14 Jun 2017 14:10:08 -0400 Subject: [PATCH] Bug 18805 - Add ability to use up account credits Right now, in Koha, credits always have a static amount outstanding. So if a patron owes $10 in fines, and has a $10 credit, they technically owe nothing, but yet they still owe $10. This seems rather confusing. Instead, we should add the ability for credits to be used to pay down closed out fines and other fees. Test Plan: 1) Apply this patch and dependencies 2) Create a manual credit of some amount 3) Create a manual invoice 4) Note the manual invoice was automatically paid down using the balance from the manual credit! --- C4/Accounts.pm | 2 + C4/Circulation.pm | 2 + Koha/Account.pm | 123 +++++++++++++++++++++++++------------- t/db_dependent/Accounts.t | 38 +++++++++++- 4 files changed, 123 insertions(+), 42 deletions(-) diff --git a/C4/Accounts.pm b/C4/Accounts.pm index f087ba6b1a..9d42cd1587 100644 --- a/C4/Accounts.pm +++ b/C4/Accounts.pm @@ -308,6 +308,8 @@ sub manualinvoice { })); } + Koha::Account->new({ patron_id => $borrowernumber })->normalize_balance(); + return 0; } diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 9c8d5ba13e..15e04bff7f 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2374,6 +2374,8 @@ sub _FixOverduesOnReturn { $accountline->accounttype('F'); } + Koha::Account->new({ patron_id => $borrowernumber })->normalize_balance(); + return $accountline->store(); } diff --git a/Koha/Account.pm b/Koha/Account.pm index 2db8d0d182..7a0570deaf 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -60,7 +60,8 @@ Koha::Account->new( { patron_id => $borrowernumber } )->pay( library_id => $branchcode, lines => $lines, # Arrayref of Koha::Account::Line objects to pay account_type => $type, # accounttype code - offset_type => $offset_type, # offset type code + offset_type => $offset_type, # offset type code + credit_id => credit_id, # pay from balance of existing credit } ); @@ -79,6 +80,7 @@ sub pay { my $payment_type = $params->{payment_type} || undef; my $account_type = $params->{account_type}; my $offset_type = $params->{offset_type} || $type eq 'writeoff' ? 'Writeoff' : 'Payment'; + my $credit_id = $params->{credit_id}; my $userenv = C4::Context->userenv; @@ -214,20 +216,28 @@ sub pay { $description ||= $type eq 'writeoff' ? 'Writeoff' : q{}; - my $payment = Koha::Account::Line->new( - { - borrowernumber => $self->{patron_id}, - accountno => $accountno, - date => dt_from_string(), - amount => 0 - $amount, - description => $description, - accounttype => $account_type, - payment_type => $payment_type, - amountoutstanding => 0 - $balance_remaining, - manager_id => $manager_id, - note => $note, - } - )->store(); + my $payment; + if ($credit_id) { + $payment = Koha::Account::Lines->find($credit_id); + $payment->amountoutstanding( $balance_remaining * -1 ); + $payment->store(); + } + else { + $payment = Koha::Account::Line->new( + { + borrowernumber => $self->{patron_id}, + accountno => $accountno, + date => dt_from_string(), + amount => 0 - $amount, + description => $description, + accounttype => $account_type, + payment_type => $payment_type, + amountoutstanding => 0 - $balance_remaining, + manager_id => $manager_id, + note => $note, + } + )->store(); + } foreach my $o ( @account_offsets ) { $o->credit_id( $payment->id() ); @@ -236,33 +246,35 @@ sub pay { $library_id ||= $userenv ? $userenv->{'branch'} : undef; - UpdateStats( - { - branch => $library_id, - type => $type, - amount => $amount, - borrowernumber => $self->{patron_id}, - accountno => $accountno, - } - ); - - if ( C4::Context->preference("FinesLog") ) { - logaction( - "FINES", 'CREATE', - $self->{patron_id}, - Dumper( - { - action => "create_$type", - borrowernumber => $self->{patron_id}, - accountno => $accountno, - amount => 0 - $amount, - amountoutstanding => 0 - $balance_remaining, - accounttype => $account_type, - accountlines_paid => \@fines_paid, - manager_id => $manager_id, - } - ) + unless ( $credit_id ) { + UpdateStats( + { + branch => $library_id, + type => $type, + amount => $amount, + borrowernumber => $self->{patron_id}, + accountno => $accountno, + } ); + + if ( C4::Context->preference("FinesLog") ) { + logaction( + "FINES", 'CREATE', + $self->{patron_id}, + Dumper( + { + action => "create_$type", + borrowernumber => $self->{patron_id}, + accountno => $accountno, + amount => 0 - $amount, + amountoutstanding => 0 - $balance_remaining, + accounttype => $account_type, + accountlines_paid => \@fines_paid, + manager_id => $manager_id, + } + ) + ); + } } if ( C4::Context->preference('UseEmailReceipts') ) { @@ -524,6 +536,35 @@ sub non_issues_charges { : 0; } +=head3 normalize_balance + +$account->normalize_balance(); + +Find outstanding credits and use them to pay outstanding debits + +=cut + +sub normalize_balance { + my ($self) = @_; + my @credits = Koha::Account::Lines->search( + { + borrowernumber => $self->{patron_id}, + amountoutstanding => { '<' => 0 }, + } + ); + + foreach my $credit (@credits) { + $self->pay( + { + credit_id => $credit->id, + amount => $credit->amountoutstanding * -1, + } + ); + } + + return $self; +} + 1; =head2 Name mappings diff --git a/t/db_dependent/Accounts.t b/t/db_dependent/Accounts.t index 26a789c96a..b9cc861bf0 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 => 29; use Test::MockModule; use Test::Warn; @@ -1018,4 +1018,40 @@ subtest "Payment notice tests" => sub { is( $notice->content, 'A writeoff of 13.00 has been applied to your account.', 'Notice content is correct for writeoff' ); }; +subtest "Koha::Account::normalize_balance tests" => sub { + + plan tests => 6; + + # Create a borrower + my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; + my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; + + my $borrower = Koha::Patron->new( { + cardnumber => 'kyliehall', + surname => 'Hall', + firstname => 'Kylie', + } ); + $borrower->categorycode( $categorycode ); + $borrower->branchcode( $branchcode ); + $borrower->store; + + my $account = Koha::Account->new({ patron_id => $borrower->id }); + + my $line1 = Koha::Account::Line->new({ borrowernumber => $borrower->borrowernumber, amount => -10, amountoutstanding => -10 })->store(); + my $line2 = Koha::Account::Line->new({ borrowernumber => $borrower->borrowernumber, amount => 10, amountoutstanding => 10 })->store(); + + is( $account->balance(), 0, "Account balance is 0" ); + is( $line1->amountoutstanding, "-10", 'Credit has amount outstanding of -10' ); + is( $line2->amountoutstanding, "10", 'Fee has amount outstanding of 10' ); + + $account->normalize_balance(); + + is( $account->balance(), 0, "Account balance is 0" ); + + $line1->_result->discard_changes(); + $line2->_result->discard_changes(); + is( $line1->amountoutstanding, '0.000000', 'First fee has amount outstanding of 0' ); + is( $line2->amountoutstanding, '0.000000', 'Second fee has amount outstanding of 0' ); +}; + 1; -- 2.17.1 (Apple Git-112)