Bug 17588: Koha::Patrons - Move GetMemberIssuesAndFines
The GetMemberIssuesAndFines subroutine used to retrieve the issues, overdues and fines for a given patron. Most of the time, only 1 or 2 of these values were used. This patch removes this subroutine and uses the new get_issues, get_overdues and get_balance method from Koha::Patron and Koha::Account::Lines. Test plan: 1/ Add overdues, issues and fines to different patrons 2/ On the checkout, checkin and patron search result and the patron detail pages, these 3 informations, if displayed before this patch, must be correctly displayed. 3/ Use the batch patron deletion tool and make sure that patrons with a balance > 0 are not deleted Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This commit is contained in:
Jonathan Druart
Kyle M Hall
parent
42bef19fe9
commit
45cee0cec8
|
|
@ -62,7 +62,6 @@ BEGIN {
|
|||
push @EXPORT, qw(
|
||||
&GetMember
|
||||
|
||||
&GetMemberIssuesAndFines
|
||||
&GetPendingIssues
|
||||
&GetAllIssues
|
||||
|
||||
|
|
@ -342,47 +341,6 @@ sub GetMember {
|
|||
return;
|
||||
}
|
||||
|
||||
=head2 GetMemberIssuesAndFines
|
||||
|
||||
($overdue_count, $issue_count, $total_fines) = &GetMemberIssuesAndFines($borrowernumber);
|
||||
|
||||
Returns aggregate data about items borrowed by the patron with the
|
||||
given borrowernumber.
|
||||
|
||||
C<&GetMemberIssuesAndFines> returns a three-element array. C<$overdue_count> is the
|
||||
number of overdue items the patron currently has borrowed. C<$issue_count> is the
|
||||
number of books the patron currently has borrowed. C<$total_fines> is
|
||||
the total fine currently due by the borrower.
|
||||
|
||||
=cut
|
||||
|
||||
#'
|
||||
sub GetMemberIssuesAndFines {
|
||||
my ( $borrowernumber ) = @_;
|
||||
my $dbh = C4::Context->dbh;
|
||||
my $query = "SELECT COUNT(*) FROM issues WHERE borrowernumber = ?";
|
||||
|
||||
$debug and warn $query."\n";
|
||||
my $sth = $dbh->prepare($query);
|
||||
$sth->execute($borrowernumber);
|
||||
my $issue_count = $sth->fetchrow_arrayref->[0];
|
||||
|
||||
$sth = $dbh->prepare(
|
||||
"SELECT COUNT(*) FROM issues
|
||||
WHERE borrowernumber = ?
|
||||
AND date_due < now()"
|
||||
);
|
||||
$sth->execute($borrowernumber);
|
||||
my $overdue_count = $sth->fetchrow_arrayref->[0];
|
||||
|
||||
$sth = $dbh->prepare("SELECT SUM(amountoutstanding) FROM accountlines WHERE borrowernumber = ?");
|
||||
$sth->execute($borrowernumber);
|
||||
my $total_fines = $sth->fetchrow_arrayref->[0];
|
||||
|
||||
return ($overdue_count, $issue_count, $total_fines);
|
||||
}
|
||||
|
||||
|
||||
=head2 ModMember
|
||||
|
||||
my $success = ModMember(borrowernumber => $borrowernumber,
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@ package C4::Utils::DataTables::Members;
|
|||
|
||||
use Modern::Perl;
|
||||
use C4::Context;
|
||||
use C4::Members qw/GetMemberIssuesAndFines/;
|
||||
use C4::Utils::DataTables;
|
||||
use Koha::DateUtils;
|
||||
|
||||
|
|
@ -169,14 +168,18 @@ sub search {
|
|||
|
||||
# Get some information on patrons
|
||||
foreach my $patron (@$patrons) {
|
||||
($patron->{overdues}, $patron->{issues}, $patron->{fines}) =
|
||||
GetMemberIssuesAndFines($patron->{borrowernumber});
|
||||
my $patron_object = Koha::Patrons->find( $patron->{borrowernumber} );
|
||||
$patron->{overdues} = $patron_object->get_overdues->count;
|
||||
$patron->{issues} = $patron_object->get_issues->count;
|
||||
my $balance = $patron_object->get_account_lines->get_balance;
|
||||
# FIXME Should be formatted from the template
|
||||
$patron->{fines} = sprintf("%.2f", $balance);
|
||||
|
||||
if($patron->{dateexpiry} and $patron->{dateexpiry} ne '0000-00-00') {
|
||||
$patron->{dateexpiry} = output_pref( { dt => dt_from_string( $patron->{dateexpiry}, 'iso'), dateonly => 1} );
|
||||
} else {
|
||||
$patron->{dateexpiry} = '';
|
||||
}
|
||||
$patron->{fines} = sprintf("%.2f", $patron->{fines} || 0);
|
||||
}
|
||||
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@ package C4::Utils::DataTables::VirtualShelves;
|
|||
|
||||
use Modern::Perl;
|
||||
use C4::Context;
|
||||
use C4::Members qw/GetMemberIssuesAndFines/;
|
||||
use C4::Utils::DataTables;
|
||||
use Koha::Virtualshelves;
|
||||
|
||||
|
|
|
|||
|
|
@ -266,7 +266,10 @@ my $patron;
|
|||
if ($borrowernumber) {
|
||||
$patron = Koha::Patrons->find( $borrowernumber );
|
||||
$borrower = GetMember( borrowernumber => $borrowernumber );
|
||||
my ( $od, $issue, $fines ) = GetMemberIssuesAndFines( $borrowernumber );
|
||||
my $overdues = $patron->get_overdues;
|
||||
my $issues = $patron->get_issues;
|
||||
my $balance = $patron->get_account_lines->get_balance;
|
||||
|
||||
|
||||
# if the expiry date is before today ie they have expired
|
||||
if ( $patron->is_expired ) {
|
||||
|
|
@ -287,9 +290,9 @@ if ($borrowernumber) {
|
|||
}
|
||||
}
|
||||
$template->param(
|
||||
overduecount => $od,
|
||||
issuecount => $issue,
|
||||
finetotal => $fines
|
||||
overduecount => $overdues->count,
|
||||
issuecount => $issues->count,
|
||||
finetotal => $balance,
|
||||
);
|
||||
|
||||
if ( $patron and $patron->is_debarred ) {
|
||||
|
|
@ -407,9 +410,6 @@ if (@$barcodes) {
|
|||
}
|
||||
}
|
||||
|
||||
# FIXME If the issue is confirmed, we launch another time GetMemberIssuesAndFines, now display the issue count after issue
|
||||
my ( $od, $issue, $fines ) = GetMemberIssuesAndFines($borrowernumber);
|
||||
|
||||
if ($question->{RESERVE_WAITING} or $question->{RESERVED}){
|
||||
$template->param(
|
||||
reserveborrowernumber => $question->{'resborrowernumber'}
|
||||
|
|
@ -421,8 +421,9 @@ if (@$barcodes) {
|
|||
);
|
||||
|
||||
|
||||
|
||||
$template_params->{issuecount} = $issue;
|
||||
# FIXME If the issue is confirmed, we launch another time get_issues->count, now display the issue count after issue
|
||||
$patron = Koha::Patrons->find( $borrowernumber );
|
||||
$template_params->{issuecount} = $patron->get_issues->count;
|
||||
|
||||
if ( $iteminfo ) {
|
||||
$iteminfo->{subtitle} = GetRecordValue('subtitle', GetMarcBiblio($iteminfo->{biblionumber}), GetFrameworkCode($iteminfo->{biblionumber}));
|
||||
|
|
|
|||
|
|
@ -324,9 +324,12 @@ if ($barcode) {
|
|||
push( @inputloop, \%input );
|
||||
|
||||
if ( C4::Context->preference("FineNotifyAtCheckin") ) {
|
||||
my ( $od, $issue, $fines ) = GetMemberIssuesAndFines( $borrower->{'borrowernumber'} );
|
||||
if ($fines && $fines > 0) {
|
||||
$template->param( fines => sprintf("%.2f",$fines) );
|
||||
my $patron = Koha::Patrons->find( $borrower->{borrowernumber} );
|
||||
my $account_lines = $patron->get_account_lines;
|
||||
my $balance = $patron->get_account_lines->get_balance;
|
||||
|
||||
if ($balance > 0) {
|
||||
$template->param( fines => sprintf("%.2f", $balance) );
|
||||
$template->param( fineborrowernumber => $borrower->{'borrowernumber'} );
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -119,8 +119,14 @@ my $borrowernumber = $input->param('borrowernumber');
|
|||
my $error = $input->param('error');
|
||||
$template->param( error => $error ) if ( $error );
|
||||
|
||||
my ( $od, $issue, $fines ) = GetMemberIssuesAndFines($borrowernumber);
|
||||
$template->param( issuecount => $issue, fines => $fines );
|
||||
my $patron = Koha::Patrons->find($borrowernumber);
|
||||
my $issues = $patron->get_issues;
|
||||
my $balance = $patron->get_account_lines->get_balance;
|
||||
$template->param(
|
||||
issuecount => $issues->count,
|
||||
fines => $balance,
|
||||
);
|
||||
|
||||
|
||||
my $data = GetMember( 'borrowernumber' => $borrowernumber );
|
||||
|
||||
|
|
@ -148,7 +154,7 @@ for (qw(gonenoaddress lost borrowernotes)) {
|
|||
$data->{$_} and $template->param(flagged => 1) and last;
|
||||
}
|
||||
|
||||
if ( Koha::Patrons->find( $borrowernumber )->is_debarred ) {
|
||||
if ( $patron->is_debarred ) {
|
||||
$template->param( 'userdebarred' => 1, 'flagged' => 1 );
|
||||
my $debar = $data->{'debarred'};
|
||||
if ( $debar ne "9999-12-31" ) {
|
||||
|
|
@ -165,7 +171,6 @@ if ( $category_type eq 'C') {
|
|||
$template->param( 'catcode' => $patron_categories->next ) if $patron_categories->count == 1;
|
||||
}
|
||||
|
||||
my $patron = Koha::Patrons->find($data->{borrowernumber});
|
||||
my @relatives;
|
||||
if ( my $guarantor = $patron->guarantor ) {
|
||||
$template->param( guarantor => $guarantor );
|
||||
|
|
@ -321,7 +326,7 @@ if (C4::Context->preference('EnhancedMessagingPreferences')) {
|
|||
$template->param(TalkingTechItivaPhone => C4::Context->preference("TalkingTechItivaPhoneNotification"));
|
||||
}
|
||||
|
||||
# in template <TMPL_IF name="I"> => instutitional (A for Adult, C for children)
|
||||
# in template <TMPL_IF name="I"> => instutitional (A for Adult, C for children)
|
||||
$template->param( $data->{'categorycode'} => 1 );
|
||||
$template->param(
|
||||
patron => $patron,
|
||||
|
|
|
|||
|
|
@ -571,7 +571,8 @@ ok( !C4::Reserves::OnShelfHoldsAllowed($item, $borrower), "OnShelfHoldsAllowed()
|
|||
# Tests for bug 14464
|
||||
|
||||
$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum));
|
||||
my ( undef, undef, $bz14464_fines ) = GetMemberIssuesAndFines( $borrowernumber );
|
||||
my $patron = Koha::Patrons->find( $borrowernumber );
|
||||
my $bz14464_fines = $patron->get_account_lines->get_balance;
|
||||
is( !$bz14464_fines || $bz14464_fines==0, 1, 'Bug 14464 - No fines at beginning' );
|
||||
|
||||
# First, test cancelling a reserve when there's no charge configured.
|
||||
|
|
@ -598,7 +599,7 @@ CancelReserve({ reserve_id => $bz14464_reserve, charge_cancel_fee => 1 });
|
|||
my $old_reserve = Koha::Database->new()->schema()->resultset('OldReserve')->find( $bz14464_reserve );
|
||||
is($old_reserve->get_column('found'), 'W', 'Bug 14968 - Keep found column from reserve');
|
||||
|
||||
( undef, undef, $bz14464_fines ) = GetMemberIssuesAndFines( $borrowernumber );
|
||||
$bz14464_fines = $patron->get_account_lines->get_balance;
|
||||
is( !$bz14464_fines || $bz14464_fines==0, 1, 'Bug 14464 - No fines after cancelling reserve with no charge configured' );
|
||||
|
||||
# Then, test cancelling a reserve when there's no charge desired.
|
||||
|
|
@ -622,7 +623,7 @@ ok( $bz14464_reserve, 'Bug 14464 - 2nd reserve correctly created' );
|
|||
|
||||
CancelReserve({ reserve_id => $bz14464_reserve });
|
||||
|
||||
( undef, undef, $bz14464_fines ) = GetMemberIssuesAndFines( $borrowernumber );
|
||||
$bz14464_fines = $patron->get_account_lines->get_balance;
|
||||
is( !$bz14464_fines || $bz14464_fines==0, 1, 'Bug 14464 - No fines after cancelling reserve with no charge desired' );
|
||||
|
||||
# Finally, test cancelling a reserve when there's a charge desired and configured.
|
||||
|
|
@ -644,7 +645,7 @@ ok( $bz14464_reserve, 'Bug 14464 - 1st reserve correctly created' );
|
|||
|
||||
CancelReserve({ reserve_id => $bz14464_reserve, charge_cancel_fee => 1 });
|
||||
|
||||
( undef, undef, $bz14464_fines ) = GetMemberIssuesAndFines( $borrowernumber );
|
||||
$bz14464_fines = $patron->get_account_lines->get_balance;
|
||||
is( int( $bz14464_fines ), 42, 'Bug 14464 - Fine applied after cancelling reserve with charge desired and configured' );
|
||||
|
||||
# tests for MoveReserve in relation to ConfirmFutureHolds (BZ 14526)
|
||||
|
|
|
|||
|
|
@ -199,7 +199,8 @@ sub _skip_borrowers_with_nonzero_balance {
|
|||
my $borrowers = shift;
|
||||
my $balance;
|
||||
@$borrowers = map {
|
||||
(undef, undef, $balance) = GetMemberIssuesAndFines( $_->{borrowernumber} );
|
||||
my $patron = Koha::Patrons->find( $_->{borrowernumber} );
|
||||
my $balance = $patron->get_account_lines->get_balance;
|
||||
(defined $balance && $balance != 0) ? (): ($_);
|
||||
} @$borrowers;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue