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). Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Jonathan Druart
parent
8b61e7145e
commit
bddfed7591
|
|
@ -173,6 +173,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) = @_;
|
||||
|
|
|
|||
|
|
@ -38,18 +38,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 } )
|
||||
|| 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;
|
||||
|
|
@ -65,7 +65,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;
|
||||
|
|
@ -112,6 +112,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}) {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
@ -468,92 +470,73 @@ if ($patron) {
|
|||
);
|
||||
}
|
||||
|
||||
#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(
|
||||
charges => 1,
|
||||
chargesamount => $owing,
|
||||
)
|
||||
} elsif ( $balance < 0 ) {
|
||||
$template->param(
|
||||
credits => 1,
|
||||
creditsamount => -$balance,
|
||||
);
|
||||
}
|
||||
|
||||
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;
|
||||
}
|
||||
if ( $guarantees_non_issues_charges > $no_issues_charge_guarantees ) {
|
||||
$template->param(
|
||||
charges_guarantees => 1,
|
||||
chargesamount_guarantees => $guarantees_non_issues_charges,
|
||||
);
|
||||
$noissues = 1 unless C4::Context->preference("allowfineoverride");
|
||||
}
|
||||
}
|
||||
|
||||
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 ( $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' );
|
||||
}
|
||||
}
|
||||
elsif ( $flag eq 'NOTES' ) {
|
||||
$template->param(
|
||||
notes => 'true',
|
||||
notesmsg => $flags->{'NOTES'}->{'message'}
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
my $total = $patron ? $patron->account->balance : 0;
|
||||
|
||||
if ( $patron && $patron->is_child) {
|
||||
my $patron_categories = Koha::Patron::Categories->search_limited({ category_type => 'A' }, {order_by => ['categorycode']});
|
||||
$template->param( 'CATCODE_MULTI' => 1) if $patron_categories->count > 1;
|
||||
|
|
@ -634,7 +617,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,
|
||||
|
|
|
|||
|
|
@ -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 %]
|
||||
|
||||
|
|
|
|||
|
|
@ -43,10 +43,10 @@
|
|||
[% ELSIF patron 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>
|
||||
|
|
|
|||
Loading…
Reference in New Issue