From 4d28bc7d3cf2a4e6cb8f384d38b5db984759b9b6 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Thu, 25 Oct 2018 16:55:32 -0300
Subject: [PATCH] Bug 21673: Use Koha::Account::Lines->total_amountoutstanding
 when needed

There are several times the same pattern to retrieve the sum of
amountoutstanding columns for Koha::Account::Line set.
We should use Koha::Account::Lines->total_outstanding instead.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
---
 Koha/Account.pm       | 23 ++++-------------------
 Koha/Account/Lines.pm | 14 +++++++++++---
 opac/opac-user.pl     |  9 ++-------
 3 files changed, 17 insertions(+), 29 deletions(-)

diff --git a/Koha/Account.pm b/Koha/Account.pm
index 6f41b3bbd2..a5b424ec35 100644
--- a/Koha/Account.pm
+++ b/Koha/Account.pm
@@ -421,19 +421,11 @@ Return the balance (sum of amountoutstanding columns)
 
 sub balance {
     my ($self) = @_;
-    my $fines = Koha::Account::Lines->search(
+    return Koha::Account::Lines->search(
         {
             borrowernumber => $self->{patron_id},
-        },
-        {
-            select => [ { sum => 'amountoutstanding' } ],
-            as => ['total_amountoutstanding'],
         }
-    );
-
-    return ( $fines->count )
-      ? $fines->next->get_column('total_amountoutstanding') + 0
-      : 0;
+    )->total_outstanding;
 }
 
 =head3 outstanding_debits
@@ -509,19 +501,12 @@ sub non_issues_charges {
     }
     @not_fines = map { substr( $_, 0, $ACCOUNT_TYPE_LENGTH ) } uniq(@not_fines);
 
-    my $non_issues_charges = Koha::Account::Lines->search(
+    return Koha::Account::Lines->search(
         {
             borrowernumber => $self->{patron_id},
             accounttype    => { -not_in => \@not_fines }
         },
-        {
-            select => [ { sum => 'amountoutstanding' } ],
-            as     => ['non_issues_charges'],
-        }
-    );
-    return $non_issues_charges->count
-      ? $non_issues_charges->next->get_column('non_issues_charges') + 0
-      : 0;
+    )->total_outstanding;
 }
 
 =head3 lines
diff --git a/Koha/Account/Lines.pm b/Koha/Account/Lines.pm
index fde90cd99c..66a91c9959 100644
--- a/Koha/Account/Lines.pm
+++ b/Koha/Account/Lines.pm
@@ -46,9 +46,17 @@ empty it returns 0.
 sub total_outstanding {
     my ( $self ) = @_;
 
-    my $total = sum0( $self->get_column('amountoutstanding') );
-
-    return $total;
+    my $lines = $self->search(
+        {},
+        {
+            select => [ { sum => 'amountoutstanding' } ],
+            as => ['total_amountoutstanding'],
+        }
+    );
+
+    return $lines->count
+      ? $lines->next->get_column('total_amountoutstanding') + 0
+      : 0;
 }
 
 =head2 Internal methods
diff --git a/opac/opac-user.pl b/opac/opac-user.pl
index bc159da4db..d5dc41767f 100755
--- a/opac/opac-user.pl
+++ b/opac/opac-user.pl
@@ -199,9 +199,8 @@ if ( $pending_checkouts->count ) { # Useless test
                 accounttype       => [ 'F', 'FU', 'L' ],
                 itemnumber        => $issue->{itemnumber}
             },
-            { select => [ { sum => 'amountoutstanding' } ], as => ['charges'] }
         );
-        $issue->{charges} = $charges->count ? $charges->next->get_column('charges') : 0;
+        $issue->{charges} = $charges->total_outstanding;
 
         my $rental_fines = Koha::Account::Lines->search(
             {
@@ -209,13 +208,9 @@ if ( $pending_checkouts->count ) { # Useless test
                 amountoutstanding => { '>' => 0 },
                 accounttype       => 'Rent',
                 itemnumber        => $issue->{itemnumber}
-            },
-            {
-                select => [ { sum => 'amountoutstanding' } ],
-                as     => ['rental_fines']
             }
         );
-        $issue->{rentalfines} = $rental_fines->count ? $rental_fines->next->get_column('rental_fines') : 0;
+        $issue->{rentalfines} = $rental_fines->total_outstanding
 
         my $marcrecord = GetMarcBiblio({
             biblionumber => $issue->{'biblionumber'},
-- 
2.20.1