From 4e78f1000de2cfa6a9cc434a4605d3e4ed6742d1 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 7 Nov 2016 16:16:04 +0000 Subject: [PATCH] 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 Signed-off-by: Nick Clemens Signed-off-by: Kyle M Hall --- C4/Members.pm | 4 +--- .../opac-tmpl/bootstrap/en/modules/opac-user.tt | 12 ++++++------ opac/opac-reserve.pl | 5 +++-- opac/opac-user.pl | 14 ++++++++------ t/db_dependent/Members.t | 15 +-------------- 5 files changed, 19 insertions(+), 31 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index 9a3a5794a3..c53b5d7239 100644 --- 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 47f3b4d6d5..ef24771eb7 100644 --- 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 %]
  • Relatives' checkouts
    • [% END %] [% IF ( overdues_count ) %]
  • Overdue ([% overdues_count %])
    • [% END %] [% IF ( OPACFinesTab ) %] - [% IF ( BORROWER_INFO.amountoverfive ) %]
  • Fines ([% BORROWER_INFO.amountoutstanding | $Price %])
    • [% END %] - [% IF ( BORROWER_INFO.amountoverzero ) %]
  • Fines ([% BORROWER_INFO.amountoutstanding | $Price %])
    • [% END %] - [% IF ( BORROWER_INFO.amountlessthanzero ) %]
  • Credits ([% BORROWER_INFO.amountoutstanding | $Price %])
    • [% END %] + [% IF ( BORROWER_INFO.amountoverfive ) %]
  • Fines ([% amountoutstanding | $Price %])
    • [% END %] + [% IF ( BORROWER_INFO.amountoverzero ) %]
  • Fines ([% amountoutstanding | $Price %])
    • [% END %] + [% IF ( BORROWER_INFO.amountlessthanzero ) %]
  • Credits ([% amountoutstanding | $Price %])
    • [% END %] [% END %] [% IF ( RESERVES.count ) %]
  • Holds ([% RESERVES.count %])
    • [% END %] [% IF Koha.Preference('ArticleRequests') && borrower.article_requests_current %]
  • Article requests ([% borrower.article_requests_current.count %])
    • [% END %] @@ -319,7 +319,7 @@ Using this account is not recommended because some parts of Koha will not functi You currently owe fines and charges amounting to: - [% BORROWER_INFO.amountoutstanding | $Price %] + [% amountoutstanding | $Price %] @@ -333,7 +333,7 @@ Using this account is not recommended because some parts of Koha will not functi You currently owe fines and charges amounting to: - [% BORROWER_INFO.amountoutstanding %] + [% amountoutstanding %] @@ -346,7 +346,7 @@ Using this account is not recommended because some parts of Koha will not functi Amount - You have a credit of:[% BORROWER_INFO.amountoutstanding %] + You have a credit of:[% amountoutstanding %] diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 42ee5983e7..85426ab60c 100755 --- 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 84ee6604dd..020e0f8043 100755 --- 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 6ea49cbde6..d293cbce6f 100755 --- 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' ); -- 2.39.5