Bugzilla – Attachment 71852 Details for
Bug 12001
GetMemberAccountRecords slows down display of patron details and checkout pages
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 12001: Move GetMemberAccountRecords to the Koha namespace
Bug-12001-Move-GetMemberAccountRecords-to-the-Koha.patch (text/plain), 38.41 KB, created by
Tomás Cohen Arazi (tcohen)
on 2018-02-16 18:22:39 UTC
(
hide
)
Description:
Bug 12001: Move GetMemberAccountRecords to the Koha namespace
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2018-02-16 18:22:39 UTC
Size:
38.41 KB
patch
obsolete
>From 9d3ef557fca60f1f975e5082ff433fe7916fda49 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Fri, 5 Jan 2018 16:18:37 -0300 >Subject: [PATCH] Bug 12001: Move GetMemberAccountRecords to the Koha namespace > >The GetMemberAccountRecords may be a perf killer, it retrieves all the >account lines of a patron and then the related item and biblio >information. >Most of the time we only want to know how much the patron owns to the >library (sum of amountoutstanding). We already have this information in >Koha::Patron->account->balance. > >This patch replaces the occurrences of this subroutine by fetching only >the information we need, either the balance, the detail, or both. >It removes the formatting done in the module, to use the TT plugin >'Price' instead. >There is a very weird and error-prone behavior/feature in >GetMemberAccountBalance (FIXME): as the accountlines.accounttype is a >varchar(5), the value of the authorised value used for the >ManInvInNoissuesCharge pref (category MANUAL_INV) is truncated to the 5 >first characters. That could lead to unexpected behaviors. > >On the way, this patchset also replace the GetMemberAccountBalance >subroutine, which returns the balance, the non issues charges and the >other charges. We only need to have the balance and the non issues >charges to calcul the third one. > >Test plan: >Add several fees for a patron and play with HoldsInNoissuesCharge, >RentalsInNoissuesCharge and ManInvInNoissuesCharge. >The information (biblio and item info, as well as the account line) must >be correctly displayed on the different screens: 'Fines' module, fine >slips, circulation module > >Note that this patchset could introduce regression on price formatting, >but will be easy to fix using the TT plugin. > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > C4/Circulation.pm | 2 +- > C4/Members.pm | 64 ++++--------------- > C4/SIP/ILS/Patron.pm | 2 +- > Koha/Account.pm | 3 +- > Koha/Account/Line.pm | 15 ++++- > circ/circulation.pl | 2 +- > .../prog/en/modules/members/boraccount.tt | 2 +- > .../prog/en/modules/members/moremember-print.tt | 5 +- > members/boraccount.pl | 21 ++++++- > members/moremember.pl | 16 ++--- > members/pay.pl | 3 +- > members/paycollect.pl | 2 +- > members/printfeercpt.pl | 73 +++++++++------------- > members/printinvoice.pl | 72 ++++++++------------- > members/summary-print.pl | 13 ++-- > opac/opac-account.pl | 39 ++++++------ > opac/opac-main.pl | 24 +++---- > opac/opac-reserve.pl | 2 +- > opac/opac-user.pl | 47 ++++++++------ > reserve/request.pl | 3 +- > t/db_dependent/Koha/Patrons.t | 20 +++--- > t/db_dependent/Members.t | 31 +-------- > 22 files changed, 196 insertions(+), 265 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 2f75d8c5a1..43585168c0 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -2703,7 +2703,7 @@ sub CanBookBeRenewed { > > if ( C4::Context->preference('OPACFineNoRenewalsBlockAutoRenew') ) { > my $fine_no_renewals = C4::Context->preference("OPACFineNoRenewals"); >- my ( $amountoutstanding ) = C4::Members::GetMemberAccountRecords($patron->borrowernumber); >+ my $amountoutstanding = $patron->account->balance; > if ( $amountoutstanding and $amountoutstanding > $fine_no_renewals ) { > return ( 0, "auto_too_much_oweing" ); > } >diff --git a/C4/Members.pm b/C4/Members.pm >index 90ebb54afe..727a7af6de 100644 >--- a/C4/Members.pm >+++ b/C4/Members.pm >@@ -26,6 +26,7 @@ use C4::Context; > use String::Random qw( random_string ); > use Scalar::Util qw( looks_like_number ); > use Date::Calc qw/Today check_date Date_to_Days/; >+use List::MoreUtils qw( uniq ); > use C4::Log; # logaction > use C4::Overdues; > use C4::Reserves; >@@ -64,8 +65,6 @@ BEGIN { > &GetPendingIssues > &GetAllIssues > >- &GetMemberAccountRecords >- > &GetBorrowersToExpunge > > &IssueSlip >@@ -736,49 +735,6 @@ sub GetAllIssues { > } > > >-=head2 GetMemberAccountRecords >- >- ($total, $acctlines, $count) = &GetMemberAccountRecords($borrowernumber); >- >-Looks up accounting data for the patron with the given borrowernumber. >- >-C<&GetMemberAccountRecords> returns a three-element array. C<$acctlines> is a >-reference-to-array, where each element is a reference-to-hash; the >-keys are the fields of the C<accountlines> table in the Koha database. >-C<$count> is the number of elements in C<$acctlines>. C<$total> is the >-total amount outstanding for all of the account lines. >- >-=cut >- >-sub GetMemberAccountRecords { >- my ($borrowernumber) = @_; >- my $dbh = C4::Context->dbh; >- my @acctlines; >- my $numlines = 0; >- my $strsth = qq( >- SELECT * >- FROM accountlines >- WHERE borrowernumber=?); >- $strsth.=" ORDER BY accountlines_id desc"; >- my $sth= $dbh->prepare( $strsth ); >- $sth->execute( $borrowernumber ); >- >- my $total = 0; >- while ( my $data = $sth->fetchrow_hashref ) { >- if ( $data->{itemnumber} ) { >- my $item = Koha::Items->find( $data->{itemnumber} ); >- my $biblio = $item->biblio; >- $data->{biblionumber} = $biblio->biblionumber; >- $data->{title} = $biblio->title; >- } >- $acctlines[$numlines] = $data; >- $numlines++; >- $total += sprintf "%.0f", 1000*$data->{amountoutstanding}; # convert float to integer to avoid round-off errors >- } >- $total /= 1000; >- return ( $total, \@acctlines,$numlines); >-} >- > =head2 GetMemberAccountBalance > > ($total_balance, $non_issue_balance, $other_charges) = &GetMemberAccountBalance($borrowernumber); >@@ -795,6 +751,7 @@ Charges exempt from non-issue are: > sub GetMemberAccountBalance { > my ($borrowernumber) = @_; > >+ # FIXME REMOVE And add a warning in the about page + update DB if length(MANUAL_INV) > 5 > my $ACCOUNT_TYPE_LENGTH = 5; # this is plain ridiculous... > > my @not_fines; >@@ -802,16 +759,17 @@ sub GetMemberAccountBalance { > push @not_fines, 'Rent' unless C4::Context->preference('RentalsInNoissuesCharge'); > unless ( C4::Context->preference('ManInvInNoissuesCharge') ) { > my $dbh = C4::Context->dbh; >- my $man_inv_types = $dbh->selectcol_arrayref(qq{SELECT authorised_value FROM authorised_values WHERE category = 'MANUAL_INV'}); >- push @not_fines, map substr($_, 0, $ACCOUNT_TYPE_LENGTH), @$man_inv_types; >+ push @not_fines, @{ $dbh->selectcol_arrayref(qq{SELECT authorised_value FROM authorised_values WHERE category = 'MANUAL_INV'}) }; > } >- my %not_fine = map {$_ => 1} @not_fines; >+ @not_fines = map { substr($_, 0, $ACCOUNT_TYPE_LENGTH) } uniq (@not_fines); > >- my ($total, $acctlines) = GetMemberAccountRecords($borrowernumber); >- my $other_charges = 0; >- foreach (@$acctlines) { >- $other_charges += $_->{amountoutstanding} if $not_fine{ substr($_->{accounttype}, 0, $ACCOUNT_TYPE_LENGTH) }; >- } >+ my $patron = Koha::Patrons->find( $borrowernumber ); >+ my $total = $patron->account->balance; >+ my $other_charges = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber, accounttype => { -in => \@not_fines } }, { >+ select => [ { sum => 'amountoutstanding' } ], >+ as => ['total_other_charges'], >+ }); >+ $other_charges = $other_charges->count ? $other_charges->next->get_column('total_other_charges') : 0; > > return ( $total, $total - $other_charges, $other_charges); > } >diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm >index 6cd3300eee..17034c0d0f 100644 >--- a/C4/SIP/ILS/Patron.pm >+++ b/C4/SIP/ILS/Patron.pm >@@ -88,7 +88,7 @@ sub new { > hold_ok => ( !$debarred && !$expired && !$fine_blocked), > card_lost => ( $kp->{lost} || $kp->{gonenoaddress} || $flags->{LOST} ), > claims_returned => 0, >- fines => $fines_amount, # GetMemberAccountRecords($kp->{borrowernumber}) >+ fines => $fines_amount, > fees => 0, # currently not distinct from fines > recall_overdue => 0, > items_billed => 0, >diff --git a/Koha/Account.pm b/Koha/Account.pm >index ed1fd3cea7..ba79f6600d 100644 >--- a/Koha/Account.pm >+++ b/Koha/Account.pm >@@ -279,7 +279,8 @@ sub balance { > as => ['total_amountoutstanding'], > } > ); >- return $fines->count >+ >+ my $total = $fines->count > ? $fines->next->get_column('total_amountoutstanding') > : 0; > } >diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm >index f7578e74f9..2065de7e2f 100644 >--- a/Koha/Account/Line.pm >+++ b/Koha/Account/Line.pm >@@ -20,6 +20,7 @@ use Modern::Perl; > use Carp; > > use Koha::Database; >+use Koha::Items; > > use base qw(Koha::Object); > >@@ -33,7 +34,19 @@ Koha::Account::Lines - Koha accountline Object class > > =cut > >-=head3 type >+=head3 item >+ >+Return the item linked to this account line if exists >+ >+=cut >+ >+sub item { >+ my ( $self ) = @_; >+ my $rs = $self->_result->itemnumber; >+ return Koha::Item->_new_from_dbic( $rs ); >+} >+ >+=head3 _type > > =cut > >diff --git a/circ/circulation.pl b/circ/circulation.pl >index aa98f9af1f..ee73f6adbe 100755 >--- a/circ/circulation.pl >+++ b/circ/circulation.pl >@@ -558,7 +558,7 @@ foreach my $flag ( sort keys %$flags ) { > my $amountold = $flags ? $flags->{'CHARGES'}->{'message'} || 0 : 0; > $amountold =~ s/^.*\$//; # remove upto the $, if any > >-my ( $total, $accts, $numaccts) = GetMemberAccountRecords( $borrowernumber ); >+my $total = $patron ? $patron->account->balance : 0; > > if ( $patron && $patron->category->category_type eq 'C') { > my $patron_categories = Koha::Patron::Categories->search_limited({ category_type => 'A' }, {order_by => ['categorycode']}); >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt >index aef11d34b3..dbf0ac0098 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt >@@ -78,7 +78,7 @@ > [% CASE %][% account.accounttype %] > [%- END -%] > [%- IF account.description %], [% account.description %][% END %] >- [% IF ( account.itemnumber ) %]<a href="/cgi-bin/koha/catalogue/moredetail.pl?biblionumber=[% account.biblionumber %]&itemnumber=[% account.itemnumber %]">[% account.title |html %]</a>[% END %]</td> >+ [% IF ( account.itemnumber ) %]<a href="/cgi-bin/koha/catalogue/moredetail.pl?biblionumber=[% account.item.biblionumber %]&itemnumber=[% account.itemnumber %]">[% account.item.biblio.title |html %]</a>[% END %]</td> > <td>[% account.note | html_line_break %]</td> > [% IF ( account.amountcredit ) %]<td class="credit" style="text-align: right;">[% ELSE %]<td class="debit" style="text-align: right;">[% END %][% account.amount | $Price %]</td> > [% IF ( account.amountoutstandingcredit ) %]<td class="credit" style="text-align: right;">[% ELSE %]<td class="debit" style="text-align: right;">[% END %][% account.amountoutstanding | $Price %]</td> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-print.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-print.tt >index 35a841056d..2ad6104447 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-print.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-print.tt >@@ -103,11 +103,10 @@ > </tr> > > [% FOREACH account IN accounts %] >- [% NEXT IF account.amountoutstanding == 0 %] > <tr> > <td> >- [% IF ( account.itemnumber ) %]<a href="/cgi-bin/koha/catalogue/moredetail.pl?biblionumber=[% account.biblionumber %]&itemnumber=[% account.itemnumber %]">[% END %] >- [% account.description %] [% IF ( account.printtitle ) %] [% account.title |html %][% END %] >+ [% IF ( account.itemnumber ) %]<a href="/cgi-bin/koha/catalogue/moredetail.pl?biblionumber=[% account.item.biblionumber %]&itemnumber=[% account.itemnumber %]">[% END %] >+ [% account.description %] [% IF account.item AND account.accounttype != 'F' AND account.accounttype != 'FU' %] [% account.item.biblio.title |html %][% END %] > [% IF ( account.itemnumber ) %]</a>[% END %] > </td> > <td>[% account.date | $KohaDates %]</td> >diff --git a/members/boraccount.pl b/members/boraccount.pl >index 7f6bfda11c..8277ab180d 100755 >--- a/members/boraccount.pl >+++ b/members/boraccount.pl >@@ -71,14 +71,23 @@ if ( $patron->category->category_type eq 'C') { > } > > #get account details >-my ($total,$accts,undef)=GetMemberAccountRecords($borrowernumber); >+my $total = $patron->account->balance; >+ >+my $accts = Koha::Account::Lines->search( >+ { borrowernumber => $patron->borrowernumber }, >+ { order_by => { -desc => 'accountlines_id' } } >+); >+ > my $totalcredit; > if($total <= 0){ > $totalcredit = 1; > } > > my $reverse_col = 0; # Flag whether we need to show the reverse column >-foreach my $accountline ( @{$accts}) { >+my @accountlines; >+while ( my $line = $accts->next ) { >+ # FIXME We should pass the $accts iterator to the template and do this formatting part there >+ my $accountline = $line->unblessed; > $accountline->{amount} += 0.00; > if ($accountline->{amount} <= 0 ) { > $accountline->{amountcredit} = 1; >@@ -94,6 +103,12 @@ foreach my $accountline ( @{$accts}) { > $accountline->{payment} = 1; > $reverse_col = 1; > } >+ >+ if ( $accountline->{itemnumber} ) { >+ # Because we will not have access to the object from the template >+ $accountline->{item} = { biblionumber => $line->item->biblionumber, }; >+ } >+ push @accountlines, $accountline; > } > > $template->param( adultborrower => 1 ) if ( $patron->category->category_type =~ /^(A|I)$/ ); >@@ -117,7 +132,7 @@ $template->param( > totalcredit => $totalcredit, > is_child => ($patron->category->category_type eq 'C'), > reverse_col => $reverse_col, >- accounts => $accts, >+ accounts => \@accountlines, > ); > > output_html_with_http_headers $input, $cookie, $template->output; >diff --git a/members/moremember.pl b/members/moremember.pl >index 29eea1fe4e..d06831ff3a 100755 >--- a/members/moremember.pl >+++ b/members/moremember.pl >@@ -50,6 +50,7 @@ use C4::Biblio; > use C4::Form::MessagingPreferences; > use List::MoreUtils qw/uniq/; > use C4::Members::Attributes qw(GetBorrowerAttributes); >+use Koha::Account::Lines; > use Koha::AuthorisedValues; > use Koha::CsvProfiles; > use Koha::Patron::Debarments qw(GetDebarments); >@@ -218,18 +219,12 @@ else { > my $library = Koha::Libraries->find( $data->{branchcode})->unblessed; > @{$data}{keys %$library} = values %$library; # merge in all branch columns > >-my ( $total, $accts, $numaccts) = GetMemberAccountRecords( $borrowernumber ); >- > # If printing a page, send the account informations to the template > if ($print eq "page") { >- foreach my $accountline (@$accts) { >- $accountline->{amount} = sprintf '%.2f', $accountline->{amount}; >- $accountline->{amountoutstanding} = sprintf '%.2f', $accountline->{amountoutstanding}; >- >- if ($accountline->{accounttype} ne 'F' && $accountline->{accounttype} ne 'FU'){ >- $accountline->{printtitle} = 1; >- } >- } >+ my $accts = Koha::Account::Lines->search( >+ { borrowernumber => $patron->borrowernumber, amountoutstanding => { '>' => 0 } }, >+ { order_by => { -desc => 'accountlines_id' } } >+ ); > $template->param( accounts => $accts ); > } > >@@ -348,6 +343,7 @@ my $patron_messages = Koha::Patron::Messages->search( > my ( $subtag, $region ) = split '-', $patron->lang; > my $translated_language = C4::Languages::language_get_description( $subtag, $subtag, 'language' ); > >+my $total = $patron->account->balance; > $template->param( > patron => $patron, > translated_language => $translated_language, >diff --git a/members/pay.pl b/members/pay.pl >index f51d56d551..f2f5a76e93 100755 >--- a/members/pay.pl >+++ b/members/pay.pl >@@ -129,7 +129,8 @@ output_html_with_http_headers $input, $cookie, $template->output; > > sub add_accounts_to_template { > >- my ( $total, undef, undef ) = GetMemberAccountRecords($borrowernumber); >+ my $patron = Koha::Patrons->find( $borrowernumber ); >+ my $total = $patron->account->balance; > my $account_lines = Koha::Account::Lines->search({ borrowernumber => $borrowernumber, amountoutstanding => { '!=' => 0 } }, { order_by => ['accounttype'] }); > my @accounts; > while ( my $account_line = $account_lines->next ) { >diff --git a/members/paycollect.pl b/members/paycollect.pl >index 1eb418437f..f9a2ac9f3d 100755 >--- a/members/paycollect.pl >+++ b/members/paycollect.pl >@@ -61,7 +61,7 @@ my $user = $input->remote_user; > > my $branch = C4::Context->userenv->{'branch'}; > >-my ( $total_due, $accts, $numaccts ) = GetMemberAccountRecords($borrowernumber); >+my $total_due = $patron->account->balance; > my $total_paid = $input->param('paid'); > > my $individual = $input->param('pay_individual'); >diff --git a/members/printfeercpt.pl b/members/printfeercpt.pl >index 748242f530..d39198857e 100755 >--- a/members/printfeercpt.pl >+++ b/members/printfeercpt.pl >@@ -29,6 +29,7 @@ use C4::Output; > use CGI qw ( -utf8 ); > use C4::Members; > use C4::Accounts; >+use Koha::Account::Lines; > use Koha::DateUtils; > use Koha::Patrons; > use Koha::Patron::Categories; >@@ -69,54 +70,39 @@ if ( $data->{'category_type'} eq 'C') { > } > > #get account details >-my ($total,$accts,$numaccts)=GetMemberAccountRecords($borrowernumber); >+my $total = $patron->account->balance; >+ >+# FIXME This whole stuff is ugly and should be rewritten >+# FIXME We should pass the $accts iterator to the template and do this formatting part there >+my $accountline = Koha::Account::Lines->find($accountlines_id)->unblessed; > my $totalcredit; > if($total <= 0){ > $totalcredit = 1; > } >-my @accountrows; # this is for the tmpl-loop >- >-my $toggle; >-for (my $i=0;$i<$numaccts;$i++){ >- next if ( $accts->[$i]{'accountlines_id'} ne $accountlines_id ); >- if($i%2){ >- $toggle = 0; >- } else { >- $toggle = 1; >- } >- $accts->[$i]{'toggle'} = $toggle; >- $accts->[$i]{'amount'}+=0.00; >- if($accts->[$i]{'amount'} <= 0){ >- $accts->[$i]{'amountcredit'} = 1; >- $accts->[$i]{'amount'}*=-1.00; >- } >- $accts->[$i]{'amountoutstanding'}+=0.00; >- if($accts->[$i]{'amountoutstanding'} <= 0){ >- $accts->[$i]{'amountoutstandingcredit'} = 1; >- } >- >- my %row = ( 'date' => dt_from_string( $accts->[$i]{'date'} ), >- 'amountcredit' => $accts->[$i]{'amountcredit'}, >- 'amountoutstandingcredit' => $accts->[$i]{'amountoutstandingcredit'}, >- 'toggle' => $accts->[$i]{'toggle'}, >- 'description' => $accts->[$i]{'description'}, >- 'itemnumber' => $accts->[$i]{'itemnumber'}, >- 'biblionumber' => $accts->[$i]{'biblionumber'}, >- 'amount' => sprintf("%.2f",$accts->[$i]{'amount'}), >- 'amountoutstanding' => sprintf("%.2f",$accts->[$i]{'amountoutstanding'}), >- 'accountno' => $accts->[$i]{'accountno'}, >- accounttype => $accts->[$i]{accounttype}, >- 'note' => $accts->[$i]{'note'}, >- ); >- >- if ($accts->[$i]{'accounttype'} ne 'F' && $accts->[$i]{'accounttype'} ne 'FU'){ >- $row{'printtitle'}=1; >- $row{'title'} = $accts->[$i]{'title'}; >- } >- >- push(@accountrows, \%row); >+ >+$accountline->{'amount'} += 0.00; >+if ( $accountline->{'amount'} <= 0 ) { >+ $accountline->{'amountcredit'} = 1; >+ $accountline->{'amount'} *= -1.00; >+} >+$accountline->{'amountoutstanding'} += 0.00; >+if ( $accountline->{'amountoutstanding'} <= 0 ) { >+ $accountline->{'amountoutstandingcredit'} = 1; > } > >+my %row = ( >+ 'date' => dt_from_string( $accountline->{'date'} ), >+ 'amountcredit' => $accountline->{'amountcredit'}, >+ 'amountoutstandingcredit' => $accountline->{'amountoutstandingcredit'}, >+ 'description' => $accountline->{'description'}, >+ 'amount' => sprintf( "%.2f", $accountline->{'amount'} ), >+ 'amountoutstanding' => >+ sprintf( "%.2f", $accountline->{'amountoutstanding'} ), >+ 'accountno' => $accountline->{'accountno'}, >+ accounttype => $accountline->{accounttype}, >+ 'note' => $accountline->{'note'}, >+); >+ > $template->param( adultborrower => 1 ) if ( $data->{'category_type'} eq 'A' || $data->{'category_type'} eq 'I' ); > > $template->param( picture => 1 ) if $patron->image; >@@ -142,6 +128,7 @@ $template->param( > total => sprintf("%.2f",$total), > totalcredit => $totalcredit, > is_child => ($data->{'category_type'} eq 'C'), >- accounts => \@accountrows ); >+ accounts => [$accountline], # FIXME There is always only 1 row! >+); > > output_html_with_http_headers $input, $cookie, $template->output; >diff --git a/members/printinvoice.pl b/members/printinvoice.pl >index 66c19c2a0b..6cb3fe8881 100755 >--- a/members/printinvoice.pl >+++ b/members/printinvoice.pl >@@ -29,6 +29,7 @@ use CGI qw ( -utf8 ); > use C4::Members; > use C4::Accounts; > >+use Koha::Account::Lines; > use Koha::Patrons; > use Koha::Patron::Categories; > >@@ -64,58 +65,37 @@ if ( $data->{'category_type'} eq 'C' ) { > } > > #get account details >-my ( $total, $accts, $numaccts ) = GetMemberAccountRecords($borrowernumber); >+my $total = $patron->account->balance; >+my $accountline = Koha::Account::Lines->find($accountlines_id)->unblessed; >+ > my $totalcredit; > if ( $total <= 0 ) { > $totalcredit = 1; > } > >-my @accountrows; # this is for the tmpl-loop >- >-my $toggle; >-for ( my $i = 0 ; $i < $numaccts ; $i++ ) { >- next if ( $accts->[$i]{'accountlines_id'} ne $accountlines_id ); >- >- if ( $i % 2 ) { >- $toggle = 0; >- } else { >- $toggle = 1; >- } >- >- $accts->[$i]{'toggle'} = $toggle; >- $accts->[$i]{'amount'} += 0.00; >- >- if ( $accts->[$i]{'amount'} <= 0 ) { >- $accts->[$i]{'amountcredit'} = 1; >- } >- >- $accts->[$i]{'amountoutstanding'} += 0.00; >- if ( $accts->[$i]{'amountoutstanding'} <= 0 ) { >- $accts->[$i]{'amountoutstandingcredit'} = 1; >- } >- >- my %row = ( >- 'date' => output_pref({ dt => dt_from_string( $accts->[$i]{'date'} ), dateonly => 1 }), >- 'amountcredit' => $accts->[$i]{'amountcredit'}, >- 'amountoutstandingcredit' => $accts->[$i]{'amountoutstandingcredit'}, >- 'toggle' => $accts->[$i]{'toggle'}, >- 'description' => $accts->[$i]{'description'}, >- 'itemnumber' => $accts->[$i]{'itemnumber'}, >- 'biblionumber' => $accts->[$i]{'biblionumber'}, >- 'amount' => sprintf( "%.2f", $accts->[$i]{'amount'} ), >- 'amountoutstanding' => sprintf( "%.2f", $accts->[$i]{'amountoutstanding'} ), >- 'accountno' => $accts->[$i]{'accountno'}, >- accounttype => $accts->[$i]{accounttype}, >- 'note' => $accts->[$i]{'note'}, >- ); >- >- if ( $accts->[$i]{'accounttype'} ne 'F' && $accts->[$i]{'accounttype'} ne 'FU' ) { >- $row{'printtitle'} = 1; >- $row{'title'} = $accts->[$i]{'title'}; >- } > >- push( @accountrows, \%row ); >+$accountline->{'amount'} += 0.00; >+if ( $accountline->{'amount'} <= 0 ) { >+ $accountline->{'amountcredit'} = 1; >+ $accountline->{'amount'} *= -1.00; > } >+$accountline->{'amountoutstanding'} += 0.00; >+if ( $accountline->{'amountoutstanding'} <= 0 ) { >+ $accountline->{'amountoutstandingcredit'} = 1; >+} >+ >+my %row = ( >+ 'date' => dt_from_string( $accountline->{'date'}, dateonly => 1 ), >+ 'amountcredit' => $accountline->{'amountcredit'}, >+ 'amountoutstandingcredit' => $accountline->{'amountoutstandingcredit'}, >+ 'description' => $accountline->{'description'}, >+ 'amount' => sprintf( "%.2f", $accountline->{'amount'} ), >+ 'amountoutstanding' => >+ sprintf( "%.2f", $accountline->{'amountoutstanding'} ), >+ 'accountno' => $accountline->{'accountno'}, >+ accounttype => $accountline->{accounttype}, >+ 'note' => $accountline->{'note'}, >+); > > $template->param( adultborrower => 1 ) if ( $data->{'category_type'} eq 'A' || $data->{'category_type'} eq 'I' ); > >@@ -141,7 +121,7 @@ $template->param( > total => sprintf( "%.2f", $total ), > totalcredit => $totalcredit, > is_child => ( $data->{'category_type'} eq 'C' ), >- accounts => \@accountrows >+ accounts => [$accountline], # FIXME There is always only 1 row! > ); > > output_html_with_http_headers $input, $cookie, $template->output; >diff --git a/members/summary-print.pl b/members/summary-print.pl >index 9c1c6122eb..c4209be39c 100755 >--- a/members/summary-print.pl >+++ b/members/summary-print.pl >@@ -52,14 +52,11 @@ my $data = $patron->unblessed; > $data->{description} = $category->description; > $data->{category_type} = $category->category_type; > >-my ( $total, $accts, $numaccts ) = GetMemberAccountRecords($borrowernumber); >-foreach my $accountline (@$accts) { >- if ( $accountline->{accounttype} ne 'F' >- && $accountline->{accounttype} ne 'FU' ) >- { >- $accountline->{printtitle} = 1; >- } >-} >+my $total = $patron->account->balance; >+my $accts = Koha::Account::Lines->search( >+ { borrowernumber => $patron->borrowernumber }, >+ { order_by => { -desc => 'accountlines_id' } } >+); > > our $totalprice = 0; > >diff --git a/opac/opac-account.pl b/opac/opac-account.pl >index c93b5b74da..9611554b58 100755 >--- a/opac/opac-account.pl >+++ b/opac/opac-account.pl >@@ -24,6 +24,7 @@ use CGI qw ( -utf8 ); > use C4::Members; > use C4::Auth; > use C4::Output; >+use Koha::Account::Lines; > use Koha::Patrons; > use Koha::Plugins; > >@@ -45,32 +46,30 @@ $borrower->{description} = $category->description; > $borrower->{category_type} = $category->category_type; > $template->param( BORROWER_INFO => $borrower ); > >-#get account details >-my ( $total , $accts, $numaccts) = GetMemberAccountRecords( $borrowernumber ); >+my $total = $patron->account->balance; >+my $accts = Koha::Account::Lines->search( >+ { borrowernumber => $patron->borrowernumber }, >+ { order_by => { -desc => 'accountlines_id' } } >+); > >-for ( my $i = 0 ; $i < $numaccts ; $i++ ) { >- $accts->[$i]{'amount'} = sprintf( "%.2f", $accts->[$i]{'amount'} || '0.00'); >- if ( $accts->[$i]{'amount'} >= 0 ) { >- $accts->[$i]{'amountcredit'} = 1; >+my @accountlines; >+while ( my $line = $accts->next ) { >+ my $accountline = $line->unblessed; >+ $accountline->{'amount'} = sprintf( "%.2f", $accountline->{'amount'} || '0.00'); >+ if ( $accountline->{'amount'} >= 0 ) { >+ $accountline->{'amountcredit'} = 1; > } >- $accts->[$i]{'amountoutstanding'} = >- sprintf( "%.2f", $accts->[$i]{'amountoutstanding'} || '0.00' ); >- if ( $accts->[$i]{'amountoutstanding'} >= 0 ) { >- $accts->[$i]{'amountoutstandingcredit'} = 1; >+ $accountline->{'amountoutstanding'} = >+ sprintf( "%.2f", $accountline->{'amountoutstanding'} || '0.00' ); >+ if ( $accountline->{'amountoutstanding'} >= 0 ) { >+ $accountline->{'amountoutstandingcredit'} = 1; > } >-} >- >-# add the row parity >-my $num = 0; >-foreach my $row (@$accts) { >- $row->{'even'} = 1 if $num % 2 == 0; >- $row->{'odd'} = 1 if $num % 2 == 1; >- $num++; >+ push @accountlines, $accountline; > } > > $template->param( >- ACCOUNT_LINES => $accts, >- total => sprintf( "%.2f", $total ), >+ ACCOUNT_LINES => \@accountlines, >+ total => sprintf( "%.2f", $total ), # FIXME Use TT plugin Price > accountview => 1, > message => scalar $query->param('message') || q{}, > message_value => scalar $query->param('message_value') || q{}, >diff --git a/opac/opac-main.pl b/opac/opac-main.pl >index b007b81ab5..723dced66c 100755 >--- a/opac/opac-main.pl >+++ b/opac/opac-main.pl >@@ -67,24 +67,26 @@ my $koha_news_count = scalar @$all_koha_news; > my $quote = GetDailyQuote(); # other options are to pass in an exact quote id or select a random quote each pass... see perldoc C4::Koha > > # For dashboard >-if ( defined $borrowernumber ){ >+my $patron = Koha::Patrons->find( $borrowernumber ); >+ >+if ( $patron ) { > my $checkouts = Koha::Checkouts->search({ borrowernumber => $borrowernumber })->count; > my ( $overdues_count, $overdues ) = checkoverdues($borrowernumber); > my $holds_pending = Koha::Holds->search({ borrowernumber => $borrowernumber, found => undef })->count; > my $holds_waiting = Koha::Holds->search({ borrowernumber => $borrowernumber })->waiting->count; >- my ( $total , $accts, $numaccts) = GetMemberAccountRecords( $borrowernumber ); >+ >+ my $total = $patron->account->balance; > > if ( $checkouts > 0 || $overdues_count > 0 || $holds_pending > 0 || $holds_waiting > 0 || $total > 0 ) { >- $template->param( dashboard_info => 1 ); >+ $template->param( >+ dashboard_info => 1, >+ checkouts => $checkouts, >+ overdues => $overdues_count, >+ holds_pending => $holds_pending, >+ holds_waiting => $holds_waiting, >+ total_owing => $total, >+ ); > } >- >- $template->param( >- checkouts => $checkouts, >- overdues => $overdues_count, >- holds_pending => $holds_pending, >- holds_waiting => $holds_waiting, >- total_owing => $total, >- ); > } > > $template->param( >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index dcaec358af..3f22eded50 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -330,7 +330,7 @@ if ( $query->param('place_reserve') ) { > my $noreserves = 0; > my $maxoutstanding = C4::Context->preference("maxoutstanding"); > $template->param( noreserve => 1 ) unless $maxoutstanding; >-my ( $amountoutstanding ) = GetMemberAccountRecords($borrowernumber); >+my $amountoutstanding = $patron->account->balance; > if ( $amountoutstanding && ($amountoutstanding > $maxoutstanding) ) { > my $amount = sprintf "%.02f", $amountoutstanding; > $template->param( message => 1 ); >diff --git a/opac/opac-user.pl b/opac/opac-user.pl >index 55c04a3a78..ac91d4f06a 100755 >--- a/opac/opac-user.pl >+++ b/opac/opac-user.pl >@@ -33,6 +33,7 @@ use C4::Output; > use C4::Biblio; > use C4::Items; > use C4::Letters; >+use Koha::Account::Lines; > use Koha::Libraries; > use Koha::DateUtils; > use Koha::Holds; >@@ -88,7 +89,8 @@ if (!$borrowernumber) { > } > > # get borrower information .... >-my $borr = Koha::Patrons->find( $borrowernumber )->unblessed; >+my $patron = Koha::Patrons->find( $borrowernumber ); >+my $borr = $patron->unblessed; > > my ( $today_year, $today_month, $today_day) = Today(); > my ($warning_year, $warning_month, $warning_day) = split /-/, $borr->{'dateexpiry'}; >@@ -116,7 +118,7 @@ if ( $userdebarred || $borr->{'gonenoaddress'} || $borr->{'lost'} ) { > $canrenew = 0; > } > >-my ( $amountoutstanding ) = GetMemberAccountRecords($borrowernumber); >+my $amountoutstanding = $patron->account->balance; > if ( $amountoutstanding > 5 ) { > $borr->{'amountoverfive'} = 1; > } >@@ -187,23 +189,32 @@ if ($issues){ > $issue->{'reserved'} = 1; > } > >- my ( $total , $accts, $numaccts) = GetMemberAccountRecords( $borrowernumber ); >- my $charges = 0; >- my $rentalfines = 0; >- foreach my $ac (@$accts) { >- if ( $ac->{'itemnumber'} == $issue->{'itemnumber'} ) { >- $charges += $ac->{'amountoutstanding'} >- if $ac->{'accounttype'} eq 'F'; >- $charges += $ac->{'amountoutstanding'} >- if $ac->{'accounttype'} eq 'FU'; >- $charges += $ac->{'amountoutstanding'} >- if $ac->{'accounttype'} eq 'L'; >- $rentalfines += $ac->{'amountoutstanding'} >- if $ac->{'accounttype'} eq 'Rent'; >+ # Must be moved in a module if reused >+ my $charges = Koha::Account::Lines->search( >+ { >+ borrowernumber => $patron->borrowernumber, >+ amountoutstanding => { '>' => 0 }, >+ accounttype => [ 'F', 'FU', 'L' ], >+ itemnumber => $issue->{itemnumber} >+ }, >+ { select => [ { sum => 'amountoutstanding' } ], as => ['charges'] } >+ ); >+ $issue->{charges} = $charges->count ? $charges->next->get_column('charges') : 0; >+ >+ my $rental_fines = Koha::Account::Lines->search( >+ { >+ borrowernumber => $patron->borrowernumber, >+ amountoutstanding => { '>' => 0 }, >+ accounttype => 'Rent', >+ itemnumber => $issue->{itemnumber} >+ }, >+ { >+ select => [ { sum => 'amountoutstanding' } ], >+ as => ['rental_fines'] > } >- } >- $issue->{'charges'} = $charges; >- $issue->{'rentalfines'} = $rentalfines; >+ ); >+ $issue->{rentalfines} = $charges->count ? $charges->next->get_column('rental_fines') : 0; >+ > my $marcrecord = GetMarcBiblio({ biblionumber => $issue->{'biblionumber'} }); > $issue->{'subtitle'} = GetRecordValue('subtitle', $marcrecord, GetFrameworkCode($issue->{'biblionumber'})); > # check if item is renewable >diff --git a/reserve/request.pl b/reserve/request.pl >index d66be33a1c..18d7b0f509 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -180,6 +180,7 @@ if ($borrowernumber_hold && !$action) { > } > > my $is_debarred = $patron->is_debarred; >+ my $amount_outstanding = $patron->account->balance; > $template->param( > borrowernumber => $patron->borrowernumber, > borrowersurname => $patron->surname, >@@ -198,7 +199,7 @@ if ($borrowernumber_hold && !$action) { > messages => $messages, > warnings => $warnings, > restricted => $is_debarred, >- amount_outstanding => GetMemberAccountRecords($patron->borrowernumber), >+ amount_outstanding => $amount_outstanding, > ); > } > >diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t >index edda6540a6..6d2edfd064 100644 >--- a/t/db_dependent/Koha/Patrons.t >+++ b/t/db_dependent/Koha/Patrons.t >@@ -400,14 +400,15 @@ subtest 'add_enrolment_fee_if_needed' => sub { > my $borrowernumber = C4::Members::AddMember(%borrower_data); > $borrower_data{borrowernumber} = $borrowernumber; > >- my ($total) = C4::Members::GetMemberAccountRecords($borrowernumber); >- is( $total, $enrolmentfee_K, "New kid pay $enrolmentfee_K" ); >+ my $patron = Koha::Patrons->find( $borrowernumber ); >+ my $total = $patron->account->balance; >+ is( int($total), int($enrolmentfee_K), "New kid pay $enrolmentfee_K" ); > > t::lib::Mocks::mock_preference( 'FeeOnChangePatronCategory', 0 ); > $borrower_data{categorycode} = 'J'; > C4::Members::ModMember(%borrower_data); >- ($total) = C4::Members::GetMemberAccountRecords($borrowernumber); >- is( $total, $enrolmentfee_K, "Kid growing and become a juvenile, but shouldn't pay for the upgrade " ); >+ $total = $patron->account->balance; >+ is( int($total), int($enrolmentfee_K), "Kid growing and become a juvenile, but shouldn't pay for the upgrade " ); > > $borrower_data{categorycode} = 'K'; > C4::Members::ModMember(%borrower_data); >@@ -415,16 +416,15 @@ subtest 'add_enrolment_fee_if_needed' => sub { > > $borrower_data{categorycode} = 'J'; > C4::Members::ModMember(%borrower_data); >- ($total) = C4::Members::GetMemberAccountRecords($borrowernumber); >- is( $total, $enrolmentfee_K + $enrolmentfee_J, "Kid growing and become a juvenile, they should pay " . ( $enrolmentfee_K + $enrolmentfee_J ) ); >+ $total = $patron->account->balance; >+ is( int($total), int($enrolmentfee_K + $enrolmentfee_J), "Kid growing and become a juvenile, they should pay " . ( $enrolmentfee_K + $enrolmentfee_J ) ); > > # Check with calling directly Koha::Patron->get_enrolment_fee_if_needed >- my $patron = Koha::Patrons->find($borrowernumber); > $patron->categorycode('YA')->store; > my $fee = $patron->add_enrolment_fee_if_needed; >- ($total) = C4::Members::GetMemberAccountRecords($borrowernumber); >- is( $total, >- $enrolmentfee_K + $enrolmentfee_J + $enrolmentfee_YA, >+ $total = $patron->account->balance; >+ is( int($total), >+ int($enrolmentfee_K + $enrolmentfee_J + $enrolmentfee_YA), > "Juvenile growing and become an young adult, they should pay " . ( $enrolmentfee_K + $enrolmentfee_J + $enrolmentfee_YA ) > ); > >diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t >index ac928b2352..aaf1262139 100755 >--- a/t/db_dependent/Members.t >+++ b/t/db_dependent/Members.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 62; >+use Test::More tests => 61; > use Test::MockModule; > use Test::Exception; > >@@ -373,35 +373,6 @@ ok( $borrower->{userid}, 'A userid should have been generated correctly' ); > is( Check_Userid( C4::Context->config('user'), '' ), 0, > 'Check_Userid should return 0 for the DB user (Bug 12226)'); > >-subtest 'GetMemberAccountRecords' => sub { >- >- plan tests => 2; >- >- my $borrowernumber = $builder->build({ source => 'Borrower' })->{ borrowernumber }; >- my $accountline_1 = $builder->build({ >- source => 'Accountline', >- value => { >- borrowernumber => $borrowernumber, >- amountoutstanding => 64.60 >- } >- }); >- >- my ($total,undef,undef) = GetMemberAccountRecords( $borrowernumber ); >- is( $total , 64.60, "Rounding works correctly in total calculation (single value)" ); >- >- my $accountline_2 = $builder->build({ >- source => 'Accountline', >- value => { >- borrowernumber => $borrowernumber, >- amountoutstanding => 10.65 >- } >- }); >- >- ($total,undef,undef) = GetMemberAccountRecords( $borrowernumber ); >- is( $total , 75.25, "Rounding works correctly in total calculation (multiple values)" ); >- >-}; >- > subtest 'GetMemberAccountBalance' => sub { > > plan tests => 6; >-- >2.14.1
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 12001
:
70314
|
70342
|
70343
|
70344
|
70345
|
70346
|
71798
|
71799
|
71800
|
71801
|
71802
|
71852
|
71853
|
71854
|
71855
|
71856
|
71857
|
71979
|
71980
|
71981
|
71982
|
71983
|
71984
|
71985
|
71986
|
71987
|
71988
|
77066
|
77136
|
77278
|
77279