Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 17578: GetMemberDetails - Remove amountoutstanding

Browse source 
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>
This commit is contained in:
Jonathan Druart 2016-11-07 16:16:04 +00:00 committed by Kyle M Hall
parent d1e7bbc602
commit 4e78f1000d
5 changed files with 19 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
C4/Members.pm
View file

@ -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;

12
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
View file

@ -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>

5
opac/opac-reserve.pl
View file

@ -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 );

14
opac/opac-user.pl
View file

@ -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,

15
t/db_dependent/Members.t
View file

@ -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' );