Bug 17578: GetMemberDetails - Remove amountoutstanding

The amountoutstanding value set by GetMemberDetails was only used in a
few places. In that case it makes sense to only retrieve it when needed.

Test plan:
1/ Add fines to a patron, on the OPAC patron info page, you should see a
"Fines" tab
2/ Add credit to a patron, you should see the credit displayed
3/ Set the pref maxoutstanding to 3
4/ Add a fine of 4 to a patron
5/ Try to place an hold for this patron
=> You should get a "too much oweing" message

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/C4/Members.pm b/C4/Members.pm
index 9a3a579..c53b5d7 100644 (file)
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -184,9 +184,7 @@ sub GetMemberDetails {
     }
     my $borrower = $sth->fetchrow_hashref;
     return unless $borrower;
-    my ($amount) = GetMemberAccountRecords($borrower->{borrowernumber});
-    $borrower->{'amountoutstanding'} = $amount;
-    # FIXME - patronflags calls GetMemberAccountRecords... just have patronflags return $amount
+
     my $flags = patronflags( $borrower);
     $borrower->{'flags'}     = $flags;
 

diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
index 47f3b4d..ef24771 100644 (file)
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
@@ -129,9 +129,9 @@ Using this account is not recommended because some parts of Koha will not functi
                             [% IF relatives %]<li><a href="#opac-user-relative-issues">Relatives' checkouts</a></li>[% END %]
                             [% IF ( overdues_count ) %]<li><a href="#opac-user-overdues">Overdue ([% overdues_count %])</a></li>[% END %]
                             [% IF ( OPACFinesTab ) %]
-                                [% IF ( BORROWER_INFO.amountoverfive ) %]<li><a href="#opac-user-fines">Fines ([% BORROWER_INFO.amountoutstanding | $Price %])</a></li>[% END %]
-                                [% IF ( BORROWER_INFO.amountoverzero ) %]<li><a href="#opac-user-fines">Fines ([% BORROWER_INFO.amountoutstanding | $Price %])</a></li>[% END %]
-                                [% IF ( BORROWER_INFO.amountlessthanzero ) %]<li><a href="#opac-user-fines">Credits ([% BORROWER_INFO.amountoutstanding | $Price %])</a></li>[% END %]
+                                [% IF ( BORROWER_INFO.amountoverfive ) %]<li><a href="#opac-user-fines">Fines ([% amountoutstanding | $Price %])</a></li>[% END %]
+                                [% IF ( BORROWER_INFO.amountoverzero ) %]<li><a href="#opac-user-fines">Fines ([% amountoutstanding | $Price %])</a></li>[% END %]
+                                [% IF ( BORROWER_INFO.amountlessthanzero ) %]<li><a href="#opac-user-fines">Credits ([% amountoutstanding | $Price %])</a></li>[% END %]
                             [% END %]
                             [% IF ( RESERVES.count ) %]<li><a href="#opac-user-holds">Holds ([% RESERVES.count %])</a></li>[% END %]
                             [% IF Koha.Preference('ArticleRequests') && borrower.article_requests_current %]<li><a href="#opac-user-article-requests">Article requests ([% borrower.article_requests_current.count %])</a></li>[% END %]
@@ -319,7 +319,7 @@ Using this account is not recommended because some parts of Koha will not functi
                                         <tbody>
                                             <tr>
                                                 <td>You currently owe fines and charges amounting to:</td>
-                                                <td><a href="/cgi-bin/koha/opac-account.pl">[% BORROWER_INFO.amountoutstanding | $Price %]</a></td>
+                                                <td><a href="/cgi-bin/koha/opac-account.pl">[% amountoutstanding | $Price %]</a></td>
                                             </tr>
                                         </tbody>
                                     </table>
@@ -333,7 +333,7 @@ Using this account is not recommended because some parts of Koha will not functi
                                         <tbody>
                                             <tr>
                                                 <td>You currently owe fines and charges amounting to:</td>
-                                                <td><a href="/cgi-bin/koha/opac-account.pl">[% BORROWER_INFO.amountoutstanding %]</a></td>
+                                                <td><a href="/cgi-bin/koha/opac-account.pl">[% amountoutstanding %]</a></td>
                                             </tr>
                                         </tbody>
                                     </table>
@@ -346,7 +346,7 @@ Using this account is not recommended because some parts of Koha will not functi
                                         <thead><tr><th colspan="2">Amount</th></tr></thead>
                                         <tbody>
                                             <tr>
-                                                <td>You have a credit of:</td><td><a href="/cgi-bin/koha/opac-account.pl">[% BORROWER_INFO.amountoutstanding %]</a></td>
+                                                <td>You have a credit of:</td><td><a href="/cgi-bin/koha/opac-account.pl">[% amountoutstanding %]</a></td>
                                             </tr>
                                         </tbody>
                                     </table>

diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
index 42ee598..85426ab 100755 (executable)
--- a/opac/opac-reserve.pl
+++ b/opac/opac-reserve.pl
@@ -305,8 +305,9 @@ if ( $query->param('place_reserve') ) {
 my $noreserves     = 0;
 my $maxoutstanding = C4::Context->preference("maxoutstanding");
 $template->param( noreserve => 1 ) unless $maxoutstanding;
-if ( $borr->{'amountoutstanding'} && ($borr->{'amountoutstanding'} > $maxoutstanding) ) {
-    my $amount = sprintf "%.02f", $borr->{'amountoutstanding'};
+my $amountoutstanding = GetMemberAccountRecords($borrowernumber);
+if ( $amountoutstanding && ($amountoutstanding > $maxoutstanding) ) {
+    my $amount = sprintf "%.02f", $amountoutstanding;
     $template->param( message => 1 );
     $noreserves = 1;
     $template->param( too_much_oweing => $amount );

diff --git a/opac/opac-user.pl b/opac/opac-user.pl
index 84ee660..020e0f8 100755 (executable)
--- a/opac/opac-user.pl
+++ b/opac/opac-user.pl
@@ -113,10 +113,11 @@ if ( $userdebarred || $borr->{'gonenoaddress'} || $borr->{'lost'} ) {
     $canrenew = 0;
 }
 
-if ( $borr->{'amountoutstanding'} > 5 ) {
+my ( $amountoutstanding ) = GetMemberAccountRecords($borrowernumber);
+if ( $amountoutstanding > 5 ) {
     $borr->{'amountoverfive'} = 1;
 }
-if ( 5 >= $borr->{'amountoutstanding'} && $borr->{'amountoutstanding'} > 0 ) {
+if ( 5 >= $amountoutstanding && $amountoutstanding > 0 ) {
     $borr->{'amountoverzero'} = 1;
 }
 my $no_renewal_amt = C4::Context->preference( 'OPACFineNoRenewals' );
@@ -124,19 +125,19 @@ $no_renewal_amt = undef unless looks_like_number( $no_renewal_amt );
 
 if (   C4::Context->preference('OpacRenewalAllowed')
     && defined($no_renewal_amt)
-    && $borr->{amountoutstanding} > $no_renewal_amt )
+    && $amountoutstanding > $no_renewal_amt )
 {
     $borr->{'flagged'} = 1;
     $canrenew = 0;
     $template->param(
         renewal_blocked_fines => $no_renewal_amt,
-        renewal_blocked_fines_amountoutstanding => $borr->{amountoutstanding},
+        renewal_blocked_fines_amountoutstanding => $amountoutstanding,
     );
 }
 
-if ( $borr->{'amountoutstanding'} < 0 ) {
+if ( $amountoutstanding < 0 ) {
     $borr->{'amountlessthanzero'} = 1;
-    $borr->{'amountoutstanding'} = -1 * ( $borr->{'amountoutstanding'} );
+    $amountoutstanding = -1 * ( $amountoutstanding );
 }
 
 # Warningdate is the date that the warning starts appearing
@@ -158,6 +159,7 @@ if ( $borr->{'dateexpiry'} && C4::Context->preference('NotifyBorrowerDeparture')
 my $renew_error = $query->param('renew_error');
 
 $template->param(   BORROWER_INFO     => $borr,
+                    amountoutstanding => $amountoutstanding,
                     borrowernumber    => $borrowernumber,
                     patron_flagged    => $borr->{flagged},
                     OPACMySummaryHTML => (C4::Context->preference("OPACMySummaryHTML")) ? 1 : 0,

diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t
index 6ea49cb..d293cbc 100755 (executable)
--- a/t/db_dependent/Members.t
+++ b/t/db_dependent/Members.t
@@ -416,7 +416,7 @@ subtest 'GetMemberAccountRecords' => sub {
 
 subtest 'GetMemberAccountBalance' => sub {
 
-    plan tests => 10;
+    plan tests => 6;
 
     my $members_mock = new Test::MockModule('C4::Members');
     $members_mock->mock( 'GetMemberAccountRecords', sub {
@@ -434,19 +434,6 @@ subtest 'GetMemberAccountBalance' => sub {
         }
     });
 
-    my $person = GetMemberDetails(undef,undef);
-    ok( !$person , 'Expected no member details from undef,undef' );
-    $person = GetMemberDetails(undef,'987654321');
-    is( $person->{amountoutstanding}, 15,
-        'Expected 15 outstanding for cardnumber.');
-    $borrowernumber = $person->{borrowernumber};
-    $person = GetMemberDetails($borrowernumber,undef);
-    is( $person->{amountoutstanding}, 15,
-        'Expected 15 outstanding for borrowernumber.');
-    $person = GetMemberDetails($borrowernumber,'987654321');
-    is( $person->{amountoutstanding}, 15,
-        'Expected 15 outstanding for both borrowernumber and cardnumber.');
-
     # do not count holds charges
     t::lib::Mocks::mock_preference( 'HoldsInNoissuesCharge', '1' );
     t::lib::Mocks::mock_preference( 'ManInvInNoissuesCharge', '0' );