From 9a9576dbd1b325800994520c2a436c8be282d78f Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 14 Oct 2019 18:01:55 +0100 Subject: [PATCH] Bug 23805: Drop type lookup as it's now a foreign key Signed-off-by: Kyle Hall Signed-off-by: Tomas Cohen Arazi Signed-off-by: Martin Renvoize --- Koha/Account.pm | 164 ++++++++++++++++++---------------- t/db_dependent/Koha/Account.t | 4 +- 2 files changed, 89 insertions(+), 79 deletions(-) diff --git a/Koha/Account.pm b/Koha/Account.pm index dd233ec718..04bbd083e7 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -334,8 +334,22 @@ 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; + # check for mandatory params + my @mandatory = ( 'interface', 'amount' ); + for my $param (@mandatory) { + unless ( defined( $params->{$param} ) ) { + Koha::Exceptions::MissingParameter->throw( + error => "The $param parameter is mandatory" ); + } + } + + # amount should always be passed as a positive value + my $amount = $params->{amount} * -1; + unless ( $amount < 0 ) { + Koha::Exceptions::Account::AmountNotPositive->throw( + error => 'Debit amount passed is not positive' ); + } + my $description = $params->{description} // q{}; my $note = $params->{note} // q{}; my $user_id = $params->{user_id}; @@ -343,85 +357,93 @@ sub add_credit { my $library_id = $params->{library_id}; my $cash_register = $params->{cash_register}; my $payment_type = $params->{payment_type}; - my $type = $params->{type} || 'PAYMENT'; + my $credit_type = $params->{type} || 'PAYMENT'; my $item_id = $params->{item_id}; - unless ( $interface ) { - Koha::Exceptions::MissingParameter->throw( - error => 'The interface parameter is mandatory' - ); - } - Koha::Exceptions::Account::RegisterRequired->throw() if ( C4::Context->preference("UseCashRegisters") && defined($payment_type) && ( $payment_type eq 'CASH' ) && !defined($cash_register) ); - my $schema = Koha::Database->new->schema; - - my $credit_type = $Koha::Account::account_type_credit->{$type}; my $line; + my $schema = Koha::Database->new->schema; + try { + $schema->txn_do( + sub { - $schema->txn_do( - sub { + # Insert the account line + $line = Koha::Account::Line->new( + { + borrowernumber => $self->{patron_id}, + date => \'NOW()', + amount => $amount, + description => $description, + credit_type_code => $credit_type, + amountoutstanding => $amount, + payment_type => $payment_type, + note => $note, + manager_id => $user_id, + interface => $interface, + branchcode => $library_id, + register_id => $cash_register, + itemnumber => $item_id, + } + )->store(); - # Insert the account line - $line = Koha::Account::Line->new( - { borrowernumber => $self->{patron_id}, - date => \'NOW()', - amount => $amount, - description => $description, - credit_type_code => $credit_type, - amountoutstanding => $amount, - payment_type => $payment_type, - note => $note, - manager_id => $user_id, - interface => $interface, - branchcode => $library_id, - register_id => $cash_register, - itemnumber => $item_id, - } - )->store(); + # Record the account offset + my $account_offset = Koha::Account::Offset->new( + { + credit_id => $line->id, + type => $Koha::Account::offset_type->{$credit_type}, + amount => $amount + } + )->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 => $credit_type, + amount => $amount, + borrowernumber => $self->{patron_id}, + } + ) if grep { $credit_type eq $_ } ( 'PAYMENT', 'WRITEOFF' ); - UpdateStats( - { branch => $library_id, - type => $type, - amount => $amount, - borrowernumber => $self->{patron_id}, + if ( C4::Context->preference("FinesLog") ) { + logaction( + "FINES", 'CREATE', + $self->{patron_id}, + Dumper( + { + action => "create_$credit_type", + borrowernumber => $self->{patron_id}, + amount => $amount, + description => $description, + amountoutstanding => $amount, + credit_type_code => $credit_type, + note => $note, + itemnumber => $item_id, + manager_id => $user_id, + branchcode => $library_id, + } + ), + $interface + ); } - ) 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}, - amount => $amount, - description => $description, - amountoutstanding => $amount, - credit_type_code => $credit_type, - note => $note, - itemnumber => $item_id, - manager_id => $user_id, - branchcode => $library_id, - } - ), - $interface - ); + } + ); + } + catch { + if ( ref($_) eq 'Koha::Exceptions::Object::FKConstraint' ) { + if ( $_->broken_fk eq 'credit_type_code' ) { + Koha::Exceptions::Account::UnrecognisedType->throw( + error => 'Type of credit not recognised' ); + } + else { + $_->rethrow; } } - ); + }; return $line; } @@ -734,18 +756,6 @@ our $offset_type = { 'RESERVE_EXPIRED' => 'Hold Expired' }; -=head3 $account_type_credit - -=cut - -our $account_type_credit = { - 'CREDIT' => 'CREDIT', - 'FORGIVEN' => 'FORGIVEN', - 'LOST_RETURN' => 'LOST_RETURN', - 'PAYMENT' => 'PAYMENT', - 'WRITEOFF' => 'WRITEOFF' -}; - =head1 AUTHORS =encoding utf8 diff --git a/t/db_dependent/Koha/Account.t b/t/db_dependent/Koha/Account.t index a4853b52df..02383c7d80 100755 --- a/t/db_dependent/Koha/Account.t +++ b/t/db_dependent/Koha/Account.t @@ -241,7 +241,7 @@ subtest 'add_credit() tests' => sub { 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->credit_type_code, $Koha::Account::account_type_credit->{'PAYMENT'}, 'Account type is correctly set' ); + is( $line_1->credit_type_code, 'PAYMENT', 'Account type is correctly set' ); # Enable logs t::lib::Mocks::mock_preference( 'FinesLog', 1 ); @@ -259,7 +259,7 @@ subtest 'add_credit() tests' => sub { 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->credit_type_code, $Koha::Account::account_type_credit->{'PAYMENT'}, 'Account type is correctly set' ); + is( $line_2->credit_type_code, 'PAYMENT', '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; -- 2.39.5