Bugzilla – Attachment 76632 Details for
Bug 20946
Cannot pay fines for patrons with credits
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20946: (QA follow-up) make outstanding_debits return the account lines only
Bug-20946-QA-follow-up-make-outstandingdebits-retu.patch (text/plain), 12.42 KB, created by
Tomás Cohen Arazi (tcohen)
on 2018-07-02 15:21:01 UTC
(
hide
)
Description:
Bug 20946: (QA follow-up) make outstanding_debits return the account lines only
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2018-07-02 15:21:01 UTC
Size:
12.42 KB
patch
obsolete
>From 81a5365eed9ea76aaa0ee9db58ffe84bcef3c964 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Mon, 2 Jul 2018 12:05:55 -0300 >Subject: [PATCH] Bug 20946: (QA follow-up) make outstanding_debits return the > account lines only > >This patch was discussed with Jonathan on a QA conversation. It is >better to keep this simpler and more reusable. And is the right approach >in this case. > >This patch makes Koha::Account::outstanding_debits return the account >lines, and a method is added to Koha::Account::Lines so the outstanding >amount is calculated on the resultset. This is done the dame way it was >done before, and the tests got adjusted. > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > Koha/Account.pm | 7 +- > Koha/Account/Lines.pm | 22 ++++- > members/pay.pl | 3 +- > members/paycollect.pl | 5 +- > t/db_dependent/Koha/Account.t | 23 ++--- > t/db_dependent/Koha/Account/Lines.t | 127 +++++++++++++++++++++------- > 6 files changed, 135 insertions(+), 52 deletions(-) > >diff --git a/Koha/Account.pm b/Koha/Account.pm >index 4b48d692ae..8dee72be24 100644 >--- a/Koha/Account.pm >+++ b/Koha/Account.pm >@@ -22,7 +22,6 @@ use Modern::Perl; > use Carp; > use Data::Dumper; > use List::MoreUtils qw( uniq ); >-use List::Util qw( sum0 ); > > use C4::Log qw( logaction ); > use C4::Stats qw( UpdateStats ); >@@ -291,7 +290,7 @@ sub balance { > > =head3 outstanding_debits > >-my ( $total, $lines ) = Koha::Account->new({ patron_id => $patron_id })->outstanding_debits; >+my $lines = Koha::Account->new({ patron_id => $patron_id })->outstanding_debits; > > =cut > >@@ -305,9 +304,7 @@ sub outstanding_debits { > } > ); > >- my $total = sum0( $lines->get_column('amountoutstanding') ); >- >- return ( $total, $lines ); >+ return $lines; > } > > =head3 non_issues_charges >diff --git a/Koha/Account/Lines.pm b/Koha/Account/Lines.pm >index 0e0677ca85..a36e182339 100644 >--- a/Koha/Account/Lines.pm >+++ b/Koha/Account/Lines.pm >@@ -18,9 +18,9 @@ package Koha::Account::Lines; > use Modern::Perl; > > use Carp; >+use List::Util qw( sum0 ); > > use Koha::Database; >- > use Koha::Account::Line; > > use base qw(Koha::Objects); >@@ -34,9 +34,27 @@ Koha::Account::Lines - Koha Account Line Object set class > > =head2 Class Methods > >+=head3 total_outstanding >+ >+ my $lines = Koha::Account::Lines->search({ ... }); >+ my $total = $lines->total_outstanding; >+ >+Returns the sum of the outstanding amounts of the resultset. If the resultset is >+empty it returns 0. >+ > =cut > >-=head3 type >+sub total_outstanding { >+ my ( $self ) = @_; >+ >+ my $total = sum0( $self->get_column('amountoutstanding') ); >+ >+ return $total; >+} >+ >+=head2 Internal methods >+ >+=head3 _type > > =cut > >diff --git a/members/pay.pl b/members/pay.pl >index 8245f38b2e..1d4c5b0c22 100755 >--- a/members/pay.pl >+++ b/members/pay.pl >@@ -139,7 +139,8 @@ output_html_with_http_headers $input, $cookie, $template->output; > sub add_accounts_to_template { > > my $patron = Koha::Patrons->find( $borrowernumber ); >- my ( $total, $account_lines ) = Koha::Account->new( { patron_id => $borrowernumber } )->outstanding_debits; >+ my $account_lines = $patron->account->outstanding_debits; >+ my $total = $account_lines->total_outstanding; > my @accounts; > while ( my $account_line = $account_lines->next ) { > $account_line = $account_line->unblessed; >diff --git a/members/paycollect.pl b/members/paycollect.pl >index 167617778f..f1f9f16e97 100755 >--- a/members/paycollect.pl >+++ b/members/paycollect.pl >@@ -58,8 +58,9 @@ my $borrower = $patron->unblessed; > my $category = $patron->category; > my $user = $input->remote_user; > >-my $branch = C4::Context->userenv->{'branch'}; >-my ( $total_due ) = Koha::Account->new( { patron_id => $borrowernumber } )->outstanding_debits; >+my $branch = C4::Context->userenv->{'branch'}; >+my $total_due = $patron->account->outstanding_debits->total_outstanding; >+ > my $total_paid = $input->param('paid'); > > my $individual = $input->param('pay_individual'); >diff --git a/t/db_dependent/Koha/Account.t b/t/db_dependent/Koha/Account.t >index c0cce13609..6ef46afa99 100755 >--- a/t/db_dependent/Koha/Account.t >+++ b/t/db_dependent/Koha/Account.t >@@ -43,10 +43,10 @@ subtest 'outstanding_debits() tests' => sub { > push @generated_lines, Koha::Account::Line->new({ borrowernumber => $patron->id, amountoutstanding => 3 })->store; > push @generated_lines, Koha::Account::Line->new({ borrowernumber => $patron->id, amountoutstanding => 4 })->store; > >- my $account = Koha::Account->new({ patron_id => $patron->id }); >- my ( $total, $lines ) = $account->outstanding_debits(); >+ my $account = $patron->account; >+ my $lines = $account->outstanding_debits(); > >- is( $total, 10, 'Outstandig debits total is correctly calculated' ); >+ is( $lines->total_outstanding, 10, 'Outstandig debits total is correctly calculated' ); > > my $i = 0; > foreach my $line ( @{ $lines->as_list } ) { >@@ -55,16 +55,12 @@ subtest 'outstanding_debits() tests' => sub { > $i++; > } > >- ( $total, $lines ) = Koha::Account->new({ patron_id => 'InvalidBorrowernumber' })->outstanding_debits(); >- is( $total, 0, "Total if no outstanding debits is 0" ); >- is( $lines->count, 0, "With no outstanding debits, we get back a Lines object with 0 lines" ); >- > my $patron_2 = $builder->build_object({ class => 'Koha::Patrons' }); > Koha::Account::Line->new({ borrowernumber => $patron_2->id, amountoutstanding => -2 })->store; > my $just_one = Koha::Account::Line->new({ borrowernumber => $patron_2->id, amountoutstanding => 3 })->store; > Koha::Account::Line->new({ borrowernumber => $patron_2->id, amountoutstanding => -6 })->store; >- ( $total, $lines ) = Koha::Account->new({ patron_id => $patron_2->id })->outstanding_debits(); >- is( $total, 3, "Total if some outstanding debits and some credits is only debits" ); >+ $lines = $patron_2->account->outstanding_debits(); >+ is( $lines->total_outstanding, 3, "Total if some outstanding debits and some credits is only debits" ); > is( $lines->count, 1, "With 1 outstanding debits, we get back a Lines object with 1 lines" ); > my $the_line = Koha::Account::Lines->find( $just_one->id ); > is_deeply( $the_line->unblessed, $lines->next->unblessed, "We get back the one correct line"); >@@ -73,9 +69,14 @@ subtest 'outstanding_debits() tests' => sub { > Koha::Account::Line->new({ borrowernumber => $patron_2->id, amountoutstanding => -2 })->store; > Koha::Account::Line->new({ borrowernumber => $patron_2->id, amountoutstanding => -20 })->store; > Koha::Account::Line->new({ borrowernumber => $patron_2->id, amountoutstanding => -200 })->store; >- ( $total, $lines ) = Koha::Account->new({ patron_id => $patron_3->id })->outstanding_debits(); >- is( $total, 0, "Total if no outstanding debits total is 0" ); >+ $lines = $patron_3->account->outstanding_debits(); >+ is( $lines->total_outstanding, 0, "Total if no outstanding debits total is 0" ); > is( $lines->count, 0, "With 0 outstanding debits, we get back a Lines object with 0 lines" ); > >+ my $patron_4 = $builder->build_object({ class => 'Koha::Patrons' }); >+ $lines = $patron_4->account->outstanding_debits(); >+ is( $lines->total_outstanding, 0, "Total if no outstanding debits is 0" ); >+ is( $lines->count, 0, "With no outstanding debits, we get back a Lines object with 0 lines" ); >+ > $schema->storage->txn_rollback; > }; >diff --git a/t/db_dependent/Koha/Account/Lines.t b/t/db_dependent/Koha/Account/Lines.t >index 0c3287d8c3..11c8df121f 100755 >--- a/t/db_dependent/Koha/Account/Lines.t >+++ b/t/db_dependent/Koha/Account/Lines.t >@@ -19,7 +19,7 @@ > > use Modern::Perl; > >-use Test::More tests => 1; >+use Test::More tests => 2; > > use Koha::Account::Lines; > use Koha::Items; >@@ -27,39 +27,104 @@ use Koha::Items; > use t::lib::TestBuilder; > > my $schema = Koha::Database->new->schema; >-$schema->storage->txn_begin; >- > my $builder = t::lib::TestBuilder->new; > > subtest 'item' => sub { >- plan tests => 2; >- >- my $library = $builder->build( { source => 'Branch' } ); >- my $biblioitem = $builder->build( { source => 'Biblioitem' } ); >- my $patron = $builder->build( { source => 'Borrower' } ); >- my $item = Koha::Item->new( >- { >- biblionumber => $biblioitem->{biblionumber}, >- biblioitemnumber => $biblioitem->{biblioitemnumber}, >- homebranch => $library->{branchcode}, >- holdingbranch => $library->{branchcode}, >- barcode => 'some_barcode_12', >- itype => 'BK', >- })->store; >- >- my $line = Koha::Account::Line->new( >- { >- borrowernumber => $patron->{borrowernumber}, >- itemnumber => $item->itemnumber, >- accounttype => "F", >- amount => 10, >- })->store; >- >- my $account_line_item = $line->item; >- is( ref( $account_line_item ), 'Koha::Item', 'Koha::Account::Line->item should return a Koha::Item' ); >- is( $line->itemnumber, $account_line_item->itemnumber, 'Koha::Account::Line->item should return the correct item' ); >+ >+ plan tests => 2; >+ >+ $schema->storage->txn_begin; >+ >+ my $library = $builder->build( { source => 'Branch' } ); >+ my $biblioitem = $builder->build( { source => 'Biblioitem' } ); >+ my $patron = $builder->build( { source => 'Borrower' } ); >+ my $item = Koha::Item->new( >+ { >+ biblionumber => $biblioitem->{biblionumber}, >+ biblioitemnumber => $biblioitem->{biblioitemnumber}, >+ homebranch => $library->{branchcode}, >+ holdingbranch => $library->{branchcode}, >+ barcode => 'some_barcode_12', >+ itype => 'BK', >+ })->store; >+ >+ my $line = Koha::Account::Line->new( >+ { >+ borrowernumber => $patron->{borrowernumber}, >+ itemnumber => $item->itemnumber, >+ accounttype => "F", >+ amount => 10, >+ })->store; >+ >+ my $account_line_item = $line->item; >+ is( ref( $account_line_item ), 'Koha::Item', 'Koha::Account::Line->item should return a Koha::Item' ); >+ is( $line->itemnumber, $account_line_item->itemnumber, 'Koha::Account::Line->item should return the correct item' ); >+ >+ $schema->storage->txn_rollback; > }; > >-$schema->storage->txn_rollback; >+subtest 'total_outstanding' => sub { >+ >+ plan tests => 5; >+ >+ $schema->storage->txn_begin; >+ >+ my $patron = $builder->build_object({ class => 'Koha::Patrons' }); >+ >+ my $lines = Koha::Account::Lines->search({ borrowernumber => $patron->id }); >+ is( $lines->total_outstanding, 0, 'total_outstanding returns 0 if no lines (undef case)' ); > >-1; >+ my $debit_1 = Koha::Account::Line->new( >+ { borrowernumber => $patron->id, >+ accounttype => "F", >+ amount => 10, >+ amountoutstanding => 10 >+ } >+ )->store; >+ >+ my $debit_2 = Koha::Account::Line->new( >+ { borrowernumber => $patron->id, >+ accounttype => "F", >+ amount => 10, >+ amountoutstanding => 10 >+ } >+ )->store; >+ >+ $lines = Koha::Account::Lines->search({ borrowernumber => $patron->id }); >+ is( $lines->total_outstanding, 20, 'total_outstanding sums correctly' ); >+ >+ my $credit_1 = Koha::Account::Line->new( >+ { borrowernumber => $patron->id, >+ accounttype => "F", >+ amount => -10, >+ amountoutstanding => -10 >+ } >+ )->store; >+ >+ $lines = Koha::Account::Lines->search({ borrowernumber => $patron->id }); >+ is( $lines->total_outstanding, 10, 'total_outstanding sums correctly' ); >+ >+ my $credit_2 = Koha::Account::Line->new( >+ { borrowernumber => $patron->id, >+ accounttype => "F", >+ amount => -10, >+ amountoutstanding => -10 >+ } >+ )->store; >+ >+ $lines = Koha::Account::Lines->search({ borrowernumber => $patron->id }); >+ is( $lines->total_outstanding, 0, 'total_outstanding sums correctly' ); >+ >+ my $credit_3 = Koha::Account::Line->new( >+ { borrowernumber => $patron->id, >+ accounttype => "F", >+ amount => -100, >+ amountoutstanding => -100 >+ } >+ )->store; >+ >+ $lines = Koha::Account::Lines->search({ borrowernumber => $patron->id }); >+ is( $lines->total_outstanding, -100, 'total_outstanding sums correctly' ); >+ >+ $schema->storage->txn_rollback; >+}; >-- >2.18.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20946
:
76116
|
76317
|
76318
|
76394
|
76395
|
76432
|
76433
|
76434
|
76435
|
76437
|
76519
|
76521
|
76522
| 76632