From 3572fd14045ddf0d5025c932c23380671d623569 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 22 Jun 2018 12:24:33 -0300 Subject: [PATCH] Bug 20980: (follow-up) Only use UpdateStats for 'payment' and 'writeoff' This patch makes Koha::Account::add_credit record statistics logs (through C4::Stats::UpdateStats) only for the 'payment' and 'writeoff' credit types. Both credit types are whitelisted in UpdateStats, so we keep the current behaviour. Another option would've been to add new account types to the white list, but with the account offsets it seems that using the statistics table for account changes won't be useful for too long, as offsets contain the same (and more) information. To test: - Apply the patch - Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Josef Moravec Signed-off-by: Kyle M Hall Signed-off-by: Nick Clemens (cherry picked from commit 7b6c242d972e3edf5b4d75d6023a0ae1dc9d1f9d) Signed-off-by: Martin Renvoize --- Koha/Account.pm | 115 ++++++++++++++++++++++++++++++++++ t/db_dependent/Koha/Account.t | 78 ++++++++++++++++++++++- 2 files changed, 192 insertions(+), 1 deletion(-) diff --git a/Koha/Account.pm b/Koha/Account.pm index 8dee72be24..382d6931b1 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -263,6 +263,121 @@ sub pay { return $payment->id; } +=head3 add_credit + +This method allows adding credits to a patron's account + +my $credit_line = Koha::Account->new({ patron_id => $patron_id })->add_credit( + { + amount => $amount, + description => $description, + note => $note, + user_id => $user_id, + library_id => $library_id, + sip => $sip, + payment_type => $payment_type, + type => $credit_type, + item_id => $item_id + } +); + +$credit_type can be any of: + - 'credit' + - 'payment' + - 'forgiven' + - 'lost_item_return' + - 'writeoff' + +=cut + +sub add_credit { + + my ( $self, $params ) = @_; + + # amount is passed as a positive value, but we store credit as negative values + my $amount = $params->{amount} * -1; + my $description = $params->{description} // q{}; + my $note = $params->{note} // q{}; + my $user_id = $params->{user_id}; + my $library_id = $params->{library_id}; + my $sip = $params->{sip}; + my $payment_type = $params->{payment_type}; + my $type = $params->{type} || 'payment'; + my $item_id = $params->{item_id}; + + my $schema = Koha::Database->new->schema; + + my $account_type = $Koha::Account::account_type->{$type}; + $account_type .= $sip + if defined $sip && + $type eq 'payment'; + + my $line; + + $schema->txn_do( + sub { + # We should remove accountno, it is no longer needed + my $last = Koha::Account::Lines->search( { borrowernumber => $self->{patron_id} }, + { order_by => 'accountno' } )->next(); + my $accountno = $last ? $last->accountno + 1 : 1; + + # Insert the account line + $line = Koha::Account::Line->new( + { borrowernumber => $self->{patron_id}, + date => \'NOW()', + amount => $amount, + description => $description, + accounttype => $account_type, + amountoutstanding => $amount, + payment_type => $payment_type, + note => $note, + manager_id => $user_id, + itemnumber => $item_id + } + )->store(); + + # Record the account offset + my $account_offset = Koha::Account::Offset->new( + { credit_id => $line->id, + type => $Koha::Account::offset_type->{$type}, + amount => $amount + } + )->store(); + + UpdateStats( + { branch => $library_id, + type => $type, + amount => $amount, + borrowernumber => $self->{patron_id}, + accountno => $accountno, + } + ) if grep { $type eq $_ } ('payment', 'writeoff') ; + + if ( C4::Context->preference("FinesLog") ) { + logaction( + "FINES", 'CREATE', + $self->{patron_id}, + Dumper( + { action => "create_$type", + borrowernumber => $self->{patron_id}, + accountno => $accountno, + amount => $amount, + description => $description, + amountoutstanding => $amount, + accounttype => $account_type, + note => $note, + itemnumber => $item_id, + manager_id => $user_id, + } + ) + ); + } + } + ); + + return $line; +} + =head3 balance my $balance = $self->balance diff --git a/t/db_dependent/Koha/Account.t b/t/db_dependent/Koha/Account.t index 6ef46afa99..d18d7de7a6 100755 --- a/t/db_dependent/Koha/Account.t +++ b/t/db_dependent/Koha/Account.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 1; +use Test::More tests => 2; use Koha::Account; use Koha::Account::Lines; @@ -80,3 +80,79 @@ subtest 'outstanding_debits() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'add_credit() tests' => sub { + + plan tests => 13; + + $schema->storage->txn_begin; + + # delete logs and statistics + my $action_logs = $schema->resultset('ActionLog')->search()->count; + my $statistics = $schema->resultset('Statistic')->search()->count; + + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $account = Koha::Account->new( { patron_id => $patron->borrowernumber } ); + + is( $account->balance, 0, 'Patron has no balance' ); + + # Disable logs + t::lib::Mocks::mock_preference( 'FinesLog', 0 ); + + my $line_1 = $account->add_credit( + { amount => 25, + description => 'Payment of 25', + library_id => $patron->branchcode, + note => 'not really important', + type => 'payment', + user_id => $patron->id + } + ); + + is( $account->balance, -25, 'Patron has a balance of -25' ); + is( $schema->resultset('ActionLog')->count(), $action_logs + 0, 'No log was added' ); + is( $schema->resultset('Statistic')->count(), $statistics + 1, 'Action added to statistics' ); + is( $line_1->accounttype, $Koha::Account::account_type->{'payment'}, 'Account type is correctly set' ); + + # Enable logs + t::lib::Mocks::mock_preference( 'FinesLog', 1 ); + + my $sip_code = "1"; + my $line_2 = $account->add_credit( + { amount => 37, + description => 'Payment of 37', + library_id => $patron->branchcode, + note => 'not really important', + user_id => $patron->id, + sip => $sip_code + } + ); + + is( $account->balance, -62, 'Patron has a balance of -25' ); + is( $schema->resultset('ActionLog')->count(), $action_logs + 1, 'Log was added' ); + is( $schema->resultset('Statistic')->count(), $statistics + 2, 'Action added to statistics' ); + is( $line_2->accounttype, $Koha::Account::account_type->{'payment'} . $sip_code, 'Account type is correctly set' ); + + # offsets have the credit_id set to accountlines_id, and debit_id is undef + my $offset_1 = Koha::Account::Offsets->search({ credit_id => $line_1->id })->next; + my $offset_2 = Koha::Account::Offsets->search({ credit_id => $line_2->id })->next; + + is( $offset_1->credit_id, $line_1->id, 'No debit_id is set for credits' ); + is( $offset_1->debit_id, undef, 'No debit_id is set for credits' ); + is( $offset_2->credit_id, $line_2->id, 'No debit_id is set for credits' ); + is( $offset_2->debit_id, undef, 'No debit_id is set for credits' ); + + my $line_3 = $account->add_credit( + { amount => 20, + description => 'Manual credit applied', + library_id => $patron->branchcode, + user_id => $patron->id, + type => 'forgiven' + } + ); + + is( $schema->resultset('ActionLog')->count(), 2, 'Log was added' ); + is( $schema->resultset('Statistic')->count(), 2, 'No action added to statistics, because of credit type' ); + + $schema->storage->txn_rollback; +}; -- 2.20.1