Bugzilla – Attachment 71806 Details for
Bug 19933
Move C4::Members::patronflags to the Koha namespace - part 1
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 19933: Remove patronflags - tricky ones
Bug-19933-Remove-patronflags---tricky-ones.patch (text/plain), 13.47 KB, created by
Jonathan Druart
on 2018-02-16 14:44:51 UTC
(
hide
)
Description:
Bug 19933: Remove patronflags - tricky ones
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2018-02-16 14:44:51 UTC
Size:
13.47 KB
patch
obsolete
>From cd844544f5efff2a1c1949e030de0757c16569b4 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Sun, 7 Jan 2018 14:57:23 -0300 >Subject: [PATCH] Bug 19933: Remove patronflags - tricky ones > >Here we are, patronflags is used in a couple of places where (almost) all flags >were really useful: C4::SIP::ILS::Patron->new and circulation.pl > >This patch only deal with the circulation code as I am not convident >enough with SIP code. > >The change does not seems trivial because of the complexity of the >existing code, but the logic is the same. We send a variable to the >template depending on the situation of the patron. > >I guess only code eyes ball could catch issue in this patch quickly. > >Maybe we need to find a good place in a Koha module to move this code >and provide code coverage (especially when C4::SIP::ILS::Patron will >reuse it). >--- > C4/Members.pm | 1 + > C4/SIP/ILS/Patron.pm | 18 +-- > circ/circulation.pl | 137 +++++++++------------ > .../prog/en/modules/circ/circulation.tt | 6 +- > .../en/modules/circ/circulation_batch_checkouts.tt | 4 +- > 5 files changed, 77 insertions(+), 89 deletions(-) > >diff --git a/C4/Members.pm b/C4/Members.pm >index 58f6e8a3de..4c0b4cbcd5 100644 >--- a/C4/Members.pm >+++ b/C4/Members.pm >@@ -172,6 +172,7 @@ The "message" field that comes from the DB is OK. > > # TODO: use {anonymous => hashes} instead of a dozen %flaginfo > # FIXME rename this function. >+# DEPRECATED Do not use this subroutine! > sub patronflags { > my %flags; > my ( $patroninformation) = @_; >diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm >index 17034c0d0f..45708405e2 100644 >--- a/C4/SIP/ILS/Patron.pm >+++ b/C4/SIP/ILS/Patron.pm >@@ -32,18 +32,18 @@ sub new { > my ($class, $patron_id) = @_; > my $type = ref($class) || $class; > my $self; >- $kp = Koha::Patrons->find( { cardnumber => $patron_id } ) >+ my $patron = Koha::Patrons->find( { cardnumber => $patron_id } ) > or Koha::Patrons->find( { userid => $patron_id } ); >- $debug and warn "new Patron: " . Dumper($kp->unblessed) if $kp; >- unless ($kp) { >+ $debug and warn "new Patron: " . Dumper($patron->unblessed) if $patron; >+ unless ($patron) { > syslog("LOG_DEBUG", "new ILS::Patron(%s): no such patron", $patron_id); > return; > } >- $kp = $kp->unblessed; >+ $kp = $patron->unblessed; > my $pw = $kp->{password}; > my $flags = C4::Members::patronflags( $kp ); >- my $debarred = defined($flags->{DBARRED}); >- $debug and warn sprintf("Debarred = %s : ", ($debarred||'undef')) . Dumper(%$flags); >+ my $debarred = $patron->is_debarred; >+ $debug and warn sprintf("Debarred = %s : ", ($debarred||'undef')); # Do we need more debug info here? > my ($day, $month, $year) = (localtime)[3,4,5]; > my $today = sprintf '%04d-%02d-%02d', $year+1900, $month+1, $day; > my $expired = ($today gt $kp->{dateexpiry}) ? 1 : 0; >@@ -59,7 +59,7 @@ sub new { > $dob and $dob =~ s/-//g; # YYYYMMDD > my $dexpiry = $kp->{dateexpiry}; > $dexpiry and $dexpiry =~ s/-//g; # YYYYMMDD >- my $fines_amount = $flags->{CHARGES}->{amount}; >+ my $fines_amount = $patron->account->balance; > $fines_amount = ($fines_amount and $fines_amount > 0) ? $fines_amount : 0; > my $fee_limit = _fee_limit(); > my $fine_blocked = $fines_amount > $fee_limit; >@@ -107,6 +107,10 @@ sub new { > ); > } > $debug and warn "patron fines: $ilspatron{fines} ... amountoutstanding: $kp->{amountoutstanding} ... CHARGES->amount: $flags->{CHARGES}->{amount}"; >+ >+ if ( $patron->is_debarred and $patron->debarredcomment ) { >+ $ilspatron{screen_msg} .= " -- " . $patron->debarredcomment; >+ } > for (qw(EXPIRED CHARGES CREDITS GNA LOST DBARRED NOTES)) { > ($flags->{$_}) or next; > if ($_ ne 'NOTES' and $flags->{$_}->{message}) { >diff --git a/circ/circulation.pl b/circ/circulation.pl >index 7581224565..26fd3ce294 100755 >--- a/circ/circulation.pl >+++ b/circ/circulation.pl >@@ -28,6 +28,7 @@ use Modern::Perl; > use CGI qw ( -utf8 ); > use DateTime; > use DateTime::Duration; >+use Scalar::Util qw( looks_like_number ); > use C4::Output; > use C4::Print; > use C4::Auth qw/:DEFAULT get_session haspermission/; >@@ -272,6 +273,7 @@ if ($findborrower) { > } > > # get the borrower information..... >+my $balance = 0; > $patron ||= Koha::Patrons->find( $borrowernumber ) if $borrowernumber; > if ($patron) { > >@@ -280,7 +282,7 @@ if ($patron) { > > my $overdues = $patron->get_overdues; > my $issues = $patron->checkouts; >- my $balance = $patron->account->balance; >+ $balance = $patron->account->balance; > > > # if the expiry date is before today ie they have expired >@@ -471,91 +473,72 @@ if ($patron) { > $template->param( adultborrower => 1 ) if ( $category_type eq 'A' || $category_type eq 'I' ); > } > >-#title >-my $flags = $patron ? C4::Members::patronflags( $patron->unblessed ) : {}; >-foreach my $flag ( sort keys %$flags ) { >- $flags->{$flag}->{'message'} =~ s#\n#<br />#g; >- if ( $flags->{$flag}->{'noissues'} ) { >+if ( $patron ) { >+ my $noissues; >+ if ( $patron->gonenoaddress ) { >+ $template->param( gna => 1 ); >+ $noissues = 1; >+ } >+ if ( $patron->lost ) { >+ $template->param( lost=> 1 ); >+ $noissues = 1; >+ } >+ if ( $patron->is_debarred ) { >+ $template->param( dbarred=> 1 ); >+ $noissues = 1; >+ } >+ my $account = $patron->account; >+ if( ( my $owing = $account->non_issues_charges ) > 0 ) { >+ my $noissuescharge = C4::Context->preference("noissuescharge") || 5; # FIXME If noissuescharge == 0 then 5, why?? >+ $noissues = ( not C4::Context->preference("AllowFineOverride") and ( $owing > $noissuescharge ) ); > $template->param( >- noissues => ($force_allow_issue) ? 0 : 'true', >- forceallow => $force_allow_issue, >+ charges => 1, >+ chargesamount => $owing, >+ ) >+ } elsif ( $balance < 0 ) { >+ $template->param( >+ credits => 1, >+ creditsamount => -$balance, > ); >- if ( $flag eq 'GNA' ) { >- $template->param( gna => 'true' ); >- } >- elsif ( $flag eq 'LOST' ) { >- $template->param( lost => 'true' ); >- } >- elsif ( $flag eq 'DBARRED' ) { >- $template->param( dbarred => 'true' ); >- } >- elsif ( $flag eq 'CHARGES' ) { >- $template->param( >- charges => 'true', >- chargesmsg => $flags->{'CHARGES'}->{'message'}, >- chargesamount => $flags->{'CHARGES'}->{'amount'}, >- charges_is_blocker => 1 >- ); >- } >- elsif ( $flag eq 'CHARGES_GUARANTEES' ) { >- $template->param( >- charges_guarantees => 'true', >- chargesmsg_guarantees => $flags->{'CHARGES_GUARANTEES'}->{'message'}, >- chargesamount_guarantees => $flags->{'CHARGES_GUARANTEES'}->{'amount'}, >- charges_guarantees_is_blocker => 1 >- ); >- } >- elsif ( $flag eq 'CREDITS' ) { >- $template->param( >- credits => 'true', >- creditsmsg => $flags->{'CREDITS'}->{'message'}, >- creditsamount => sprintf("%.02f", -($flags->{'CREDITS'}->{'amount'})), # from patron's pov >- ); >- } > } >- else { >- if ( $flag eq 'CHARGES' ) { >- $template->param( >- charges => 'true', >- chargesmsg => $flags->{'CHARGES'}->{'message'}, >- chargesamount => $flags->{'CHARGES'}->{'amount'}, >- ); >- } >- elsif ( $flag eq 'CHARGES_GUARANTEES' ) { >- $template->param( >- charges_guarantees => 'true', >- chargesmsg_guarantees => $flags->{'CHARGES_GUARANTEES'}->{'message'}, >- chargesamount_guarantees => $flags->{'CHARGES_GUARANTEES'}->{'amount'}, >- ); >- } >- elsif ( $flag eq 'CREDITS' ) { >- $template->param( >- credits => 'true', >- creditsmsg => $flags->{'CREDITS'}->{'message'}, >- creditsamount => sprintf("%.02f", -($flags->{'CREDITS'}->{'amount'})), # from patron's pov >- ); >- } >- elsif ( $flag eq 'ODUES' ) { >- $template->param( >- odues => 'true', >- oduesmsg => $flags->{'ODUES'}->{'message'} >- ); > >- my $items = $flags->{$flag}->{'itemlist'}; >- if ( ! $query->param('module') || $query->param('module') ne 'returns' ) { >- $template->param( nonreturns => 'true' ); >- } >+ my $no_issues_charge_guarantees = C4::Context->preference("NoIssuesChargeGuarantees"); >+ $no_issues_charge_guarantees = undef unless looks_like_number( $no_issues_charge_guarantees ); >+ if ( defined $no_issues_charge_guarantees ) { >+ my $guarantees_non_issues_charges = 0; >+ my $guarantees = $patron->guarantees; >+ while ( my $g = $guarantees->next ) { >+ $guarantees_non_issues_charges += $g->account->non_issues_charges; > } >- elsif ( $flag eq 'NOTES' ) { >+ if ( $guarantees_non_issues_charges > $no_issues_charge_guarantees ) { > $template->param( >- notes => 'true', >- notesmsg => $flags->{'NOTES'}->{'message'} >+ charges_guarantees => 1, >+ chargesamount_guarantees => $guarantees_non_issues_charges, > ); >+ $noissues = 1 unless C4::Context->preference("allowfineoverride"); > } > } >-} > >-my $total = $patron ? $patron->account->balance : 0; >+ if ( $patron->has_overdues ) { >+ $template->param( odues => 1 ); >+ } >+ >+ if ( $patron->borrowernotes ) { >+ my $borrowernotes = $patron->borrowernotes; >+ $borrowernotes =~ s#\n#<br />#g; >+ $template->param( >+ notes =>1, >+ notesmsg => $borrowernotes, >+ ) >+ } >+ >+ if ( $noissues ) { >+ $template->param( >+ noissues => ($force_allow_issue) ? 0 : 'true', >+ forceallow => $force_allow_issue, >+ ); >+ } >+} > > if ( $patron && $patron->category->category_type eq 'C') { > my $patron_categories = Koha::Patron::Categories->search_limited({ category_type => 'A' }, {order_by => ['categorycode']}); >@@ -641,7 +624,7 @@ $template->param( > duedatespec => $duedatespec, > restoreduedatespec => $restoreduedatespec, > message => $message, >- totaldue => sprintf('%.2f', $total), >+ totaldue => sprintf('%.2f', $balance), # FIXME not used in template? > inprocess => $inprocess, > $view => 1, > batch_allowed => $batch_allowed, >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt >index 3c6a8cfe65..aeaca57962 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt >@@ -707,7 +707,7 @@ No patron matched <span class="ex">[% message | html %]</span> > </li> > [% END %] > >- [% IF ( odues ) %]<li>[% IF ( nonreturns ) %]<span class="circ-hlt">Overdues: Patron has ITEMS OVERDUE.</span> <a href="#checkouts">See highlighted items below</a>[% END %]</li> >+ [% IF ( odues ) %]<li><span class="circ-hlt">Overdues: Patron has ITEMS OVERDUE.</span> <a href="#checkouts">See highlighted items below</a></li> > [% END %] > > [% IF ( charges ) %] >@@ -717,7 +717,7 @@ No patron matched <span class="ex">[% message | html %]</span> > [% IF ( charges_guarantees ) %] > <li> > <span class="circ-hlt">Fees & Charges:</span> Patron's guarantees collectively owe [% chargesamount_guarantees | $Price %]. >- [% IF ( charges_guarantees_is_blocker ) %] >+ [% IF noissues %] > <span class="circ-hlt">Checkouts are BLOCKED because fine balance is OVER THE LIMIT.</span> > [% END %] > </li> >@@ -726,7 +726,7 @@ No patron matched <span class="ex">[% message | html %]</span> > > [% IF ( credits ) %] > <li> >- <span class="circ-hlt">Credits:</span> Patron has a credit[% IF ( creditsamount ) %] of [% creditsamount %][% END %] >+ <span class="circ-hlt">Credits:</span> Patron has a credit[% IF ( creditsamount ) %] of [% creditsamount | $Price %][% END %] > </li> > [% END %] > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation_batch_checkouts.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation_batch_checkouts.tt >index 0dc32d9b5e..f2336c9398 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation_batch_checkouts.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation_batch_checkouts.tt >@@ -43,10 +43,10 @@ > [% ELSIF borrower and noissues and not checkout_infos %] > <div class="dialog alert"> > Cannot check out! >- [% IF charges_is_blocker %] >+ [% IF charges %] > <span class="circ-hlt">Checkouts are BLOCKED because fine balance is OVER THE LIMIT.</span> > [% END %] >- [% IF charges_guarantees_is_blocker %] >+ [% IF charges_guarantees %] > <li> > <span class="circ-hlt">Fees & Charges:</span> Patron's guarantees collectively owe [% chargesamount_guarantees | $Price %]. > </li> >-- >2.11.0
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 19933
:
70347
|
70348
|
70349
|
70350
|
71803
|
71804
|
71805
|
71806
|
72004
|
72005
|
72006
|
72007
|
72094
|
72095
|
72350
|
72351
|
72352
|
72353
|
72354
|
72355
|
72356
|
72472
|
72473
|
72474
|
72475
|
72476
|
72477
|
72478