Bugzilla – Attachment 57820 Details for
Bug 17578
Replace GetMemberDetails with GetMember
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[SIGNED-OFF] Bug 17578: GetMemberDetails - Remove amountoutstanding
SIGNED-OFF-Bug-17578-GetMemberDetails---Remove-amo.patch (text/plain), 10.07 KB, created by
Josef Moravec
on 2016-11-30 07:17:40 UTC
(
hide
)
Description:
[SIGNED-OFF] Bug 17578: GetMemberDetails - Remove amountoutstanding
Filename:
MIME Type:
Creator:
Josef Moravec
Created:
2016-11-30 07:17:40 UTC
Size:
10.07 KB
patch
obsolete
>From a7917d23af7755e4d7150ab630a755dd58b48718 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Mon, 7 Nov 2016 16:16:04 +0000 >Subject: [PATCH] [SIGNED-OFF] 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> >--- > C4/Members.pm | 4 +--- > koha-tmpl/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 5fbbe1c..2104b73 100644 >--- a/C4/Members.pm >+++ b/C4/Members.pm >@@ -188,9 +188,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 47f3b4d6..ef24771 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 %]<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 >--- 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 >--- 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 963d790..188e388 100755 >--- a/t/db_dependent/Members.t >+++ b/t/db_dependent/Members.t >@@ -412,7 +412,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 { >@@ -430,19 +430,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.1.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 17578
:
57299
|
57300
|
57301
|
57302
|
57303
|
57304
|
57305
|
57306
|
57307
|
57308
|
57322
|
57323
|
57324
|
57325
|
57326
|
57793
|
57794
|
57795
|
57796
|
57797
|
57798
|
57799
|
57800
|
57801
|
57808
|
57817
|
57818
|
57819
|
57820
|
57821
|
57822
|
57823
|
57824
|
57825
|
57826
|
57972
|
57973
|
57974
|
57975
|
57976
|
57977
|
57978
|
57979
|
57980
|
57981
|
57982
|
57983
|
57984