From 7c88df82893521d9fad106f39c9c39db9cfd68c7 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 28 Apr 2021 10:21:14 +0100 Subject: [PATCH] Bug 27636: (QA follow-up) Fix tests and validate This patch fixes the tests for when a negative amount is passed to the pay method. Prior to now, a negative amount would have been passed through and recorded. This was inconsistent with all other accounts methods and has been deprecated to ensure consistent amounts handling. This patch also introduces basic validation to prevent negatives being entered into the UI. Signed-off-by: Martin Renvoize Signed-off-by: Kyle M Hall Signed-off-by: Jonathan Druart --- Koha/Account.pm | 5 ++- .../prog/en/modules/members/mancredit.tt | 2 +- .../prog/en/modules/members/maninvoice.tt | 2 +- .../prog/en/modules/members/paycollect.tt | 8 ++--- t/db_dependent/Accounts.t | 35 ++++++------------- t/db_dependent/Koha/Account.t | 8 ++--- 6 files changed, 23 insertions(+), 37 deletions(-) diff --git a/Koha/Account.pm b/Koha/Account.pm index 7c01fba0ce..16bff39073 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -81,7 +81,6 @@ sub pay { my $lines = $params->{lines}; my $type = $params->{type} || 'PAYMENT'; my $payment_type = $params->{payment_type} || undef; - my $offset_type = $params->{offset_type} || $type eq 'WRITEOFF' ? 'Writeoff' : 'Payment'; my $cash_register = $params->{cash_register}; my $item_id = $params->{item_id}; @@ -358,7 +357,7 @@ sub payin_amount { $credit = $credit->apply( { debits => $params->{debits}, - offset_type => $params->{type} + offset_type => $Koha::Account::offset_type->{$params->{type}} } ); } @@ -370,7 +369,7 @@ sub payin_amount { $credit = $credit->apply( { debits => [ $self->outstanding_debits->as_list ], - offset_type => $params->{type} + offset_type => $Koha::Account::offset_type->{$params->{type}} } ); } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/mancredit.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/mancredit.tt index e4cccecb7e..0a7e81e940 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/mancredit.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/mancredit.tt @@ -66,7 +66,7 @@
    • -
  • Example: 5.00
    • +
  • Example: 5.00
    • diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/maninvoice.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/maninvoice.tt index 7849d763b8..3020366c0a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/maninvoice.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/maninvoice.tt @@ -79,7 +79,7 @@
    • -
  • Example: 5.00
    • +
  • Example: 5.00
    • diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt index a11b8b38fb..d06e076405 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt @@ -155,11 +155,11 @@
  • - +
  • - +
  • @@ -285,12 +285,12 @@ [% ELSE %] [% END %] - +
    • [% IF type != 'WRITEOFF' %]
  • - +
  • diff --git a/t/db_dependent/Accounts.t b/t/db_dependent/Accounts.t index 489ae7de36..7480664a90 100755 --- a/t/db_dependent/Accounts.t +++ b/t/db_dependent/Accounts.t @@ -20,6 +20,7 @@ use Modern::Perl; use Test::More tests => 27; use Test::MockModule; +use Test::Exception; use Test::Warn; use t::lib::TestBuilder; @@ -143,7 +144,7 @@ $dbh->do(q|DELETE FROM accountlines|); subtest "Koha::Account::pay tests" => sub { - plan tests => 14; + plan tests => 13; # Create a borrower my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; @@ -201,38 +202,24 @@ subtest "Koha::Account::pay tests" => sub { my $note = $sth->fetchrow_array; is($note,'$20.00 payment note', '$20.00 payment note is registered'); - # We make a -$30 payment (a NEGATIVE payment) + # We attempt to make a -$30 payment (a NEGATIVE payment) $data = '-30.00'; $payment_note = '-$30.00 payment note'; - $account->pay( { amount => $data, note => $payment_note } ); - - # There is now $310 in the account - $sth = $dbh->prepare("SELECT amountoutstanding FROM accountlines WHERE borrowernumber=?"); - $amountoutstanding = $dbh->selectcol_arrayref($sth, {}, $borrower->borrowernumber); - $amountleft = 0; - for my $line ( @$amountoutstanding ) { - $amountleft += $line; - } - is($amountleft, 310, 'The account has $310 as expected' ); - # Is the payment note well registered - $sth = $dbh->prepare("SELECT note FROM accountlines WHERE borrowernumber=? ORDER BY accountlines_id DESC LIMIT 1"); - $sth->execute($borrower->borrowernumber); - $note = $sth->fetchrow_array; - is($note,'-$30.00 payment note', '-$30.00 payment note is registered'); + throws_ok { $account->pay( { amount => $data, note => $payment_note } ) } qr//, 'Croaked on call to pay with negative amount'; #We make a $150 payment ( > 1stLine ) $data = '150.00'; $payment_note = '$150.00 payment note'; $account->pay( { amount => $data, note => $payment_note } ); - # There is now $160 in the account + # There is now $130 in the account $sth = $dbh->prepare("SELECT amountoutstanding FROM accountlines WHERE borrowernumber=?"); $amountoutstanding = $dbh->selectcol_arrayref($sth, {}, $borrower->borrowernumber); $amountleft = 0; for my $line ( @$amountoutstanding ) { $amountleft += $line; } - is($amountleft, 160, 'The account has $160 as expected' ); + is($amountleft, 130, 'The account has $130 as expected' ); # Is the payment note well registered $sth = $dbh->prepare("SELECT note FROM accountlines WHERE borrowernumber=? ORDER BY accountlines_id DESC LIMIT 1"); @@ -245,14 +232,14 @@ subtest "Koha::Account::pay tests" => sub { $payment_note = '$200.00 payment note'; $account->pay( { amount => $data, note => $payment_note } ); - # There is now -$40 in the account + # There is now -$70 in the account $sth = $dbh->prepare("SELECT amountoutstanding FROM accountlines WHERE borrowernumber=?"); $amountoutstanding = $dbh->selectcol_arrayref($sth, {}, $borrower->borrowernumber); $amountleft = 0; for my $line ( @$amountoutstanding ) { $amountleft += $line; } - is($amountleft, -40, 'The account has -$40 as expected, (credit situation)' ); + is($amountleft, -70, 'The account has -$70 as expected, (credit situation)' ); # Is the payment note well registered $sth = $dbh->prepare("SELECT note FROM accountlines WHERE borrowernumber=? ORDER BY accountlines_id DESC LIMIT 1"); @@ -352,7 +339,7 @@ subtest "Koha::Account::pay writeoff tests" => sub { my $writeoff = Koha::Account::Lines->find( $id ); is( $writeoff->credit_type_code, 'WRITEOFF', 'Type is correct for WRITEOFF' ); - is( $writeoff->description, 'Writeoff', 'Description is correct' ); + is( $writeoff->description, '', 'Description is correct' ); is( $writeoff->amount+0, -42, 'Amount is correct' ); }; @@ -411,7 +398,7 @@ subtest "More Koha::Account::pay tests" => sub { is( $stat->type, 'payment', "Correct statistic type" ); is( $stat->branch, $branch, "Correct branch logged to statistics" ); is( $stat->borrowernumber, $borrowernumber, "Correct borrowernumber logged to statistics" ); - is( $stat->value+0, $amount, "Correct amount logged to statistics" ); + is( $stat->value+0, -$amount, "Correct amount logged to statistics" ); } }; @@ -471,7 +458,7 @@ subtest "Even more Koha::Account::pay tests" => sub { is( $stat->type, 'payment', "Correct statistic type" ); is( $stat->branch, $branch, "Correct branch logged to statistics" ); is( $stat->borrowernumber, $borrowernumber, "Correct borrowernumber logged to statistics" ); - is( $stat->value+0, $partialamount, "Correct amount logged to statistics" ); + is( $stat->value+0, -$partialamount, "Correct amount logged to statistics" ); } }; diff --git a/t/db_dependent/Koha/Account.t b/t/db_dependent/Koha/Account.t index f685b1e704..f8e32a97b5 100755 --- a/t/db_dependent/Koha/Account.t +++ b/t/db_dependent/Koha/Account.t @@ -1395,15 +1395,15 @@ subtest 'Koha::Account::payin_amount() tests' => sub { is( $offset->amount * 1, -10, 'Correct offset amount recorded' ); $offset = $offsets->next; is( $offset->debit_id, $debit_1->id, "Offset added against debit_1"); - is( $offset->type, 'PAYMENT', "Payment used for offset_type" ); + is( $offset->type, 'Payment', "Payment used for offset_type" ); is( $offset->amount * 1, -2, 'Correct amount offset against debit_1' ); $offset = $offsets->next; is( $offset->debit_id, $debit_2->id, "Offset added against debit_2"); - is( $offset->type, 'PAYMENT', "Payment used for offset_type" ); + is( $offset->type, 'Payment', "Payment used for offset_type" ); is( $offset->amount * 1, -3, 'Correct amount offset against debit_2' ); $offset = $offsets->next; is( $offset->debit_id, $debit_3->id, "Offset added against debit_3"); - is( $offset->type, 'PAYMENT', "Payment used for offset_type" ); + is( $offset->type, 'Payment', "Payment used for offset_type" ); is( $offset->amount * 1, -5, 'Correct amount offset against debit_3' ); my $debit_5 = $account->add_debit( { amount => 5, interface => 'commandline', type => 'OVERDUE' } ); @@ -1428,7 +1428,7 @@ subtest 'Koha::Account::payin_amount() tests' => sub { is( $offset->amount * 1, -2.50, 'Correct offset amount recorded' ); $offset = $offsets->next; is( $offset->debit_id, $debit_5->id, "Offset added against debit_5"); - is( $offset->type, 'PAYMENT', "PAYMENT used for offset_type" ); + is( $offset->type, 'Payment', "Payment used for offset_type" ); is( $offset->amount * 1, -2.50, 'Correct amount offset against debit_5' ); $schema->storage->txn_rollback;