Bugzilla – Attachment 57992 Details for
Bug 17588
Move GetMemberIssuesAndFines to Koha::Patron
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 17588: Koha::Patrons - Move GetMemberIssuesAndFines
Bug-17588-KohaPatrons---Move-GetMemberIssuesAndFin.patch (text/plain), 12.33 KB, created by
Jonathan Druart
on 2016-12-06 08:24:08 UTC
(
hide
)
Description:
Bug 17588: Koha::Patrons - Move GetMemberIssuesAndFines
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2016-12-06 08:24:08 UTC
Size:
12.33 KB
patch
obsolete
>From 361f657ccf784931d3201046950ebaf35b7f957e Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Tue, 8 Nov 2016 13:52:56 +0000 >Subject: [PATCH] Bug 17588: Koha::Patrons - Move GetMemberIssuesAndFines > >The GetMemberIssuesAndFines subroutine used to retrieve the issues, >overdues and fines for a given patron. Most of the time, only 1 or 2 of >these values were used. >This patch removes this subroutine and uses the new get_issues, >get_overdues and get_balance method from Koha::Patron and Koha::Account::Lines. > >Test plan: >1/ Add overdues, issues and fines to different patrons >2/ On the checkout, checkin and patron search result and the patron >detail pages, these 3 informations, if displayed before this patch, must be >correctly displayed. >3/ Use the batch patron deletion tool and make sure that patrons with a >balance > 0 are not deleted >--- > C4/Members.pm | 42 ----------------------------------- > C4/Utils/DataTables/Members.pm | 11 +++++---- > C4/Utils/DataTables/VirtualShelves.pm | 1 - > circ/circulation.pl | 19 ++++++++-------- > circ/returns.pl | 9 +++++--- > members/moremember.pl | 15 ++++++++----- > t/db_dependent/Reserves.t | 9 ++++---- > tools/cleanborrowers.pl | 3 ++- > 8 files changed, 40 insertions(+), 69 deletions(-) > >diff --git a/C4/Members.pm b/C4/Members.pm >index 586761b..b95fb1b 100644 >--- a/C4/Members.pm >+++ b/C4/Members.pm >@@ -62,7 +62,6 @@ BEGIN { > push @EXPORT, qw( > &GetMember > >- &GetMemberIssuesAndFines > &GetPendingIssues > &GetAllIssues > >@@ -349,47 +348,6 @@ sub GetMember { > return; > } > >-=head2 GetMemberIssuesAndFines >- >- ($overdue_count, $issue_count, $total_fines) = &GetMemberIssuesAndFines($borrowernumber); >- >-Returns aggregate data about items borrowed by the patron with the >-given borrowernumber. >- >-C<&GetMemberIssuesAndFines> returns a three-element array. C<$overdue_count> is the >-number of overdue items the patron currently has borrowed. C<$issue_count> is the >-number of books the patron currently has borrowed. C<$total_fines> is >-the total fine currently due by the borrower. >- >-=cut >- >-#' >-sub GetMemberIssuesAndFines { >- my ( $borrowernumber ) = @_; >- my $dbh = C4::Context->dbh; >- my $query = "SELECT COUNT(*) FROM issues WHERE borrowernumber = ?"; >- >- $debug and warn $query."\n"; >- my $sth = $dbh->prepare($query); >- $sth->execute($borrowernumber); >- my $issue_count = $sth->fetchrow_arrayref->[0]; >- >- $sth = $dbh->prepare( >- "SELECT COUNT(*) FROM issues >- WHERE borrowernumber = ? >- AND date_due < now()" >- ); >- $sth->execute($borrowernumber); >- my $overdue_count = $sth->fetchrow_arrayref->[0]; >- >- $sth = $dbh->prepare("SELECT SUM(amountoutstanding) FROM accountlines WHERE borrowernumber = ?"); >- $sth->execute($borrowernumber); >- my $total_fines = $sth->fetchrow_arrayref->[0]; >- >- return ($overdue_count, $issue_count, $total_fines); >-} >- >- > =head2 ModMember > > my $success = ModMember(borrowernumber => $borrowernumber, >diff --git a/C4/Utils/DataTables/Members.pm b/C4/Utils/DataTables/Members.pm >index 2446703..e4f12cc 100644 >--- a/C4/Utils/DataTables/Members.pm >+++ b/C4/Utils/DataTables/Members.pm >@@ -2,7 +2,6 @@ package C4::Utils::DataTables::Members; > > use Modern::Perl; > use C4::Context; >-use C4::Members qw/GetMemberIssuesAndFines/; > use C4::Utils::DataTables; > use Koha::DateUtils; > >@@ -169,14 +168,18 @@ sub search { > > # Get some information on patrons > foreach my $patron (@$patrons) { >- ($patron->{overdues}, $patron->{issues}, $patron->{fines}) = >- GetMemberIssuesAndFines($patron->{borrowernumber}); >+ my $patron_object = Koha::Patrons->find( $patron->{borrowernumber} ); >+ $patron->{overdues} = $patron_object->get_overdues->count; >+ $patron->{issues} = $patron_object->get_issues->count; >+ my $balance = $patron_object->get_account_lines->get_balance; >+ # FIXME Should be formatted from the template >+ $patron->{fines} = sprintf("%.2f", $balance); >+ > if($patron->{dateexpiry} and $patron->{dateexpiry} ne '0000-00-00') { > $patron->{dateexpiry} = output_pref( { dt => dt_from_string( $patron->{dateexpiry}, 'iso'), dateonly => 1} ); > } else { > $patron->{dateexpiry} = ''; > } >- $patron->{fines} = sprintf("%.2f", $patron->{fines} || 0); > } > > return { >diff --git a/C4/Utils/DataTables/VirtualShelves.pm b/C4/Utils/DataTables/VirtualShelves.pm >index 2b37706..28f579f 100644 >--- a/C4/Utils/DataTables/VirtualShelves.pm >+++ b/C4/Utils/DataTables/VirtualShelves.pm >@@ -2,7 +2,6 @@ package C4::Utils::DataTables::VirtualShelves; > > use Modern::Perl; > use C4::Context; >-use C4::Members qw/GetMemberIssuesAndFines/; > use C4::Utils::DataTables; > use Koha::Virtualshelves; > >diff --git a/circ/circulation.pl b/circ/circulation.pl >index b065267..82cc89e 100755 >--- a/circ/circulation.pl >+++ b/circ/circulation.pl >@@ -266,7 +266,10 @@ my $patron; > if ($borrowernumber) { > $patron = Koha::Patrons->find( $borrowernumber ); > $borrower = GetMember( borrowernumber => $borrowernumber ); >- my ( $od, $issue, $fines ) = GetMemberIssuesAndFines( $borrowernumber ); >+ my $overdues = $patron->get_overdues; >+ my $issues = $patron->get_issues; >+ my $balance = $patron->get_account_lines->get_balance; >+ > > # if the expiry date is before today ie they have expired > if ( $patron->is_expired ) { >@@ -287,9 +290,9 @@ if ($borrowernumber) { > } > } > $template->param( >- overduecount => $od, >- issuecount => $issue, >- finetotal => $fines >+ overduecount => $overdues->count, >+ issuecount => $issues->count, >+ finetotal => $balance, > ); > > if ( $patron and $patron->is_debarred ) { >@@ -407,9 +410,6 @@ if (@$barcodes) { > } > } > >- # FIXME If the issue is confirmed, we launch another time GetMemberIssuesAndFines, now display the issue count after issue >- my ( $od, $issue, $fines ) = GetMemberIssuesAndFines($borrowernumber); >- > if ($question->{RESERVE_WAITING} or $question->{RESERVED}){ > $template->param( > reserveborrowernumber => $question->{'resborrowernumber'} >@@ -421,8 +421,9 @@ if (@$barcodes) { > ); > > >- >- $template_params->{issuecount} = $issue; >+ # FIXME If the issue is confirmed, we launch another time get_issues->count, now display the issue count after issue >+ $patron = Koha::Patrons->find( $borrowernumber ); >+ $template_params->{issuecount} = $patron->get_issues->count; > > if ( $iteminfo ) { > $iteminfo->{subtitle} = GetRecordValue('subtitle', GetMarcBiblio($iteminfo->{biblionumber}), GetFrameworkCode($iteminfo->{biblionumber})); >diff --git a/circ/returns.pl b/circ/returns.pl >index 5927759..25715e4 100755 >--- a/circ/returns.pl >+++ b/circ/returns.pl >@@ -323,9 +323,12 @@ if ($barcode) { > push( @inputloop, \%input ); > > if ( C4::Context->preference("FineNotifyAtCheckin") ) { >- my ( $od, $issue, $fines ) = GetMemberIssuesAndFines( $borrower->{'borrowernumber'} ); >- if ($fines && $fines > 0) { >- $template->param( fines => sprintf("%.2f",$fines) ); >+ my $patron = Koha::Patrons->find( $borrower->{borrowernumber} ); >+ my $account_lines = $patron->get_account_lines; >+ my $balance = $patron->get_account_lines->get_balance; >+ >+ if ($balance > 0) { >+ $template->param( fines => sprintf("%.2f", $balance) ); > $template->param( fineborrowernumber => $borrower->{'borrowernumber'} ); > } > } >diff --git a/members/moremember.pl b/members/moremember.pl >index 70aed84..3955655 100755 >--- a/members/moremember.pl >+++ b/members/moremember.pl >@@ -119,8 +119,14 @@ my $borrowernumber = $input->param('borrowernumber'); > my $error = $input->param('error'); > $template->param( error => $error ) if ( $error ); > >-my ( $od, $issue, $fines ) = GetMemberIssuesAndFines($borrowernumber); >-$template->param( issuecount => $issue, fines => $fines ); >+my $patron = Koha::Patrons->find($borrowernumber); >+my $issues = $patron->get_issues; >+my $balance = $patron->get_account_lines->get_balance; >+$template->param( >+ issuecount => $issues->count, >+ fines => $balance, >+); >+ > > my $data = GetMember( 'borrowernumber' => $borrowernumber ); > >@@ -148,7 +154,7 @@ for (qw(gonenoaddress lost borrowernotes)) { > $data->{$_} and $template->param(flagged => 1) and last; > } > >-if ( Koha::Patrons->find( $borrowernumber )->is_debarred ) { >+if ( $patron->is_debarred ) { > $template->param( 'userdebarred' => 1, 'flagged' => 1 ); > my $debar = $data->{'debarred'}; > if ( $debar ne "9999-12-31" ) { >@@ -165,7 +171,6 @@ if ( $category_type eq 'C') { > $template->param( 'catcode' => $patron_categories->next ) if $patron_categories->count == 1; > } > >-my $patron = Koha::Patrons->find($data->{borrowernumber}); > my @relatives; > if ( my $guarantor = $patron->guarantor ) { > $template->param( guarantor => $guarantor ); >@@ -323,7 +328,7 @@ if (C4::Context->preference('EnhancedMessagingPreferences')) { > $template->param(TalkingTechItivaPhone => C4::Context->preference("TalkingTechItivaPhoneNotification")); > } > >-# in template <TMPL_IF name="I"> => instutitional (A for Adult, C for children) >+# in template <TMPL_IF name="I"> => instutitional (A for Adult, C for children) > $template->param( $data->{'categorycode'} => 1 ); > $template->param( > patron => $patron, >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index 29f6b00..327b7e0 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -572,7 +572,8 @@ ok( !C4::Reserves::OnShelfHoldsAllowed($item, $borrower), "OnShelfHoldsAllowed() > # Tests for bug 14464 > > $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum)); >-my ( undef, undef, $bz14464_fines ) = GetMemberIssuesAndFines( $borrowernumber ); >+my $patron = Koha::Patrons->find( $borrowernumber ); >+my $bz14464_fines = $patron->get_account_lines->get_balance; > is( !$bz14464_fines || $bz14464_fines==0, 1, 'Bug 14464 - No fines at beginning' ); > > # First, test cancelling a reserve when there's no charge configured. >@@ -599,7 +600,7 @@ CancelReserve({ reserve_id => $bz14464_reserve, charge_cancel_fee => 1 }); > my $old_reserve = Koha::Database->new()->schema()->resultset('OldReserve')->find( $bz14464_reserve ); > is($old_reserve->get_column('found'), 'W', 'Bug 14968 - Keep found column from reserve'); > >-( undef, undef, $bz14464_fines ) = GetMemberIssuesAndFines( $borrowernumber ); >+$bz14464_fines = $patron->get_account_lines->get_balance; > is( !$bz14464_fines || $bz14464_fines==0, 1, 'Bug 14464 - No fines after cancelling reserve with no charge configured' ); > > # Then, test cancelling a reserve when there's no charge desired. >@@ -623,7 +624,7 @@ ok( $bz14464_reserve, 'Bug 14464 - 2nd reserve correctly created' ); > > CancelReserve({ reserve_id => $bz14464_reserve }); > >-( undef, undef, $bz14464_fines ) = GetMemberIssuesAndFines( $borrowernumber ); >+$bz14464_fines = $patron->get_account_lines->get_balance; > is( !$bz14464_fines || $bz14464_fines==0, 1, 'Bug 14464 - No fines after cancelling reserve with no charge desired' ); > > # Finally, test cancelling a reserve when there's a charge desired and configured. >@@ -645,7 +646,7 @@ ok( $bz14464_reserve, 'Bug 14464 - 1st reserve correctly created' ); > > CancelReserve({ reserve_id => $bz14464_reserve, charge_cancel_fee => 1 }); > >-( undef, undef, $bz14464_fines ) = GetMemberIssuesAndFines( $borrowernumber ); >+$bz14464_fines = $patron->get_account_lines->get_balance; > is( int( $bz14464_fines ), 42, 'Bug 14464 - Fine applied after cancelling reserve with charge desired and configured' ); > > # tests for MoveReserve in relation to ConfirmFutureHolds (BZ 14526) >diff --git a/tools/cleanborrowers.pl b/tools/cleanborrowers.pl >index a1c2135..d2713b5 100755 >--- a/tools/cleanborrowers.pl >+++ b/tools/cleanborrowers.pl >@@ -199,7 +199,8 @@ sub _skip_borrowers_with_nonzero_balance { > my $borrowers = shift; > my $balance; > @$borrowers = map { >- (undef, undef, $balance) = GetMemberIssuesAndFines( $_->{borrowernumber} ); >+ my $patron = Koha::Patrons->find( $_->{borrowernumber} ); >+ my $balance = $patron->get_account_lines->get_balance; > (defined $balance && $balance != 0) ? (): ($_); > } @$borrowers; > } >-- >2.9.3
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 17588
:
57339
|
57992
|
57993
|
58430
|
58759
|
58760
|
58761
|
59154
|
59155
|
59156