Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 27636: (QA follow-up) Fix tests and validate

Browse source 
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 <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Martin Renvoize 2021-04-28 10:21:14 +01:00 committed by Jonathan Druart
parent 80c13cfb31
commit 7c88df8289
6 changed files with 23 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
Koha/Account.pm
View file

@ -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}}
}
);
}

2
koha-tmpl/intranet-tmpl/prog/en/modules/members/mancredit.tt
View file

@ -66,7 +66,7 @@
<li><label for="barcode">Barcode: </label><input type="text" name="barcode" id="barcode" /></li>
<li><label for="desc">Description: </label><input type="text" name="desc" size="50" id="desc" /></li>
<li><label for="note">Note: </label><input type="text" name="note" size="50" id="note" /></li>
<li><label for="amount">Amount: </label><input type="text" inputmode="decimal" pattern="^\d+(\.\d{2})?$" name="amount" id="amount" required="required" value=""/> Example: 5.00</li>
<li><label for="amount">Amount: </label><input type="text" inputmode="decimal" pattern="^\d+(\.\d{2})?$" name="amount" id="amount" required="required" min="0" value=""/> Example: 5.00</li>
</ol>
</fieldset>

2
koha-tmpl/intranet-tmpl/prog/en/modules/members/maninvoice.tt
View file

@ -79,7 +79,7 @@
<li><label for="barcode">Barcode: </label><input type="text" name="barcode" id="barcode" value="[% barcode | html %]" /></li>
<li><label for="desc">Description: </label><input type="text" name="desc" id="desc" size="50" value="[% desc | html %]" /></li>
<li><label for="note">Note: </label><input type="text" name="note" size="50" id="note" value="[% note | html %]" /></li>
<li><label for="amount">Amount: </label><input type="text" inputmode="decimal" pattern="^\d+(\.\d{2})?$" name="amount" id="amount" required="required" value="[% amount | $Price on_editing => 1 %]" /> Example: 5.00</li>
<li><label for="amount">Amount: </label><input type="text" inputmode="decimal" pattern="^\d+(\.\d{2})?$" name="amount" id="amount" required="required" min="0" value="[% amount | $Price on_editing => 1 %]" /> Example: 5.00</li>
</ol>
</fieldset>
<fieldset class="action">

8
koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt
View file

@ -155,11 +155,11 @@
<li>
<label for="paid">Amount being paid: </label>
<input name="paid" id="paid" type="text" step="0.01" value="[% amountoutstanding | $Price on_editing => 1 %]"/>
<input name="paid" id="paid" type="text" step="0.01" min="0" value="[% amountoutstanding | $Price on_editing => 1 %]"/>
</li>
<li>
<label for="collected">Amount tendered: </label>
<input name="collected" id="collected" type="text" step="0.01" value="[% amountoutstanding | $Price on_editing => 1 %]"/>
<input name="collected" id="collected" type="text" step="0.01" min="0" value="[% amountoutstanding | $Price on_editing => 1 %]"/>
</li>
<li>
<label>Change to give: </label>
@ -285,12 +285,12 @@
[% ELSE %]
<label for="paid">Amount being paid: </label>
[% END %]
<input name="paid" id="paid" type="text" step="0.01" value="[% total | $Price on_editing => 1 %]"/>
<input name="paid" id="paid" type="text" step="0.01" min="0" value="[% total | $Price on_editing => 1 %]"/>
</li>
[% IF type != 'WRITEOFF' %]
<li>
<label for="collected">Amount tendered: </label>
<input name="collected" id="collected" type="text" step="0.01" value="[% total | $Price on_editing => 1 %]"/>
<input name="collected" id="collected" type="text" step="0.01" min="0" value="[% total | $Price on_editing => 1 %]"/>
</li>
<li>
<label>Change to give: </label>

35
t/db_dependent/Accounts.t
View file

@ -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" );
}
};

8
t/db_dependent/Koha/Account.t
View file

@ -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;