Bugzilla – Attachment 120258 Details for
Bug 27636
Replace Koha::Account::pay with a simpler method
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 27636: (QA follow-up) Fix tests and validate
Bug-27636-QA-follow-up-Fix-tests-and-validate.patch (text/plain), 13.47 KB, created by
Martin Renvoize (ashimema)
on 2021-04-28 09:27:38 UTC
(
hide
)
Description:
Bug 27636: (QA follow-up) Fix tests and validate
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2021-04-28 09:27:38 UTC
Size:
13.47 KB
patch
obsolete
>From 3fc26f85f027f57e0a8c6f6120870592a9d4de05 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >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 <martin.renvoize@ptfs-europe.com> >--- > 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 @@ > <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> > >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 @@ > <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"> >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 @@ > > <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> >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; >-- >2.20.1
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=120258">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 27636
:
116393
|
118471
|
118515
|
118516
|
118522
|
118523
|
119205
|
119206
|
119207
|
119242
|
119243
|
119244
|
120022
|
120023
|
120024
|
120089
|
120090
|
120091
|
120257
|
120258
|
120259
|
120262
|
120263
|
120270
|
120271
|
120272
|
120276
|
120294
|
120343
|
120344
|
120345