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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/C4/Members.pm b/C4/Members.pm
index c7f2277..a501041 100644 (file)
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -62,7 +62,6 @@ BEGIN {
     push @EXPORT, qw(
         &GetMember
 
-        &GetMemberIssuesAndFines
         &GetPendingIssues
         &GetAllIssues
 
@@ -342,47 +341,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 (file)
--- 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 (file)
--- 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 (executable)
--- 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 de701ff..d0a0027 100755 (executable)
--- a/circ/returns.pl
+++ b/circ/returns.pl
@@ -324,9 +324,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 55b0901..c6ce1cd 100755 (executable)
--- 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 );
@@ -321,7 +326,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 37d90d9..8a63bde 100755 (executable)
--- a/t/db_dependent/Reserves.t
+++ b/t/db_dependent/Reserves.t
@@ -571,7 +571,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.
@@ -598,7 +599,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.
@@ -622,7 +623,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.
@@ -644,7 +645,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 6714614..767e8d6 100755 (executable)
--- 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;
 }