Bug 2900: fix GetPendingIssues.
GetPendingIssues did several bad things: ~ select * on a 4 table join, ~ including multiple namespace collisions, ~ including large fields marc and marcxml from biblioitems, ~ return ($count, \@array_being_counted). Not everything is fixed here (see FIXMEs), but the situation is improved considerably, with bug 2900 resolved. The "timestamp" namespace collision in query should be resolved by separate patch. Signed-off-by: Galen Charlton <galen.charlton@liblime.com>
This commit is contained in:
Joe Atzberger
Henri-Damien LAURENT
parent
c1d3b08049
commit
267fe06f0a
|
|
@ -994,44 +994,60 @@ sub UpdateGuarantees {
|
|||
}
|
||||
=head2 GetPendingIssues
|
||||
|
||||
($count, $issues) = &GetPendingIssues($borrowernumber);
|
||||
my $issues = &GetPendingIssues($borrowernumber);
|
||||
|
||||
Looks up what the patron with the given borrowernumber has borrowed.
|
||||
|
||||
C<&GetPendingIssues> returns a two-element array. C<$issues> is a
|
||||
reference-to-array, where each element is a reference-to-hash; the
|
||||
keys are the fields from the C<issues>, C<biblio>, and C<items> tables
|
||||
in the Koha database. C<$count> is the number of elements in
|
||||
C<$issues>.
|
||||
C<&GetPendingIssues> returns a
|
||||
reference-to-array where each element is a reference-to-hash; the
|
||||
keys are the fields from the C<issues>, C<biblio>, and C<items> tables.
|
||||
The keys include C<biblioitems> fields except marc and marcxml.
|
||||
|
||||
=cut
|
||||
|
||||
#'
|
||||
sub GetPendingIssues {
|
||||
my ($borrowernumber) = @_;
|
||||
my $dbh = C4::Context->dbh;
|
||||
|
||||
my $sth = $dbh->prepare(
|
||||
"SELECT *,issues.timestamp as timestamp FROM issues
|
||||
LEFT JOIN items ON issues.itemnumber=items.itemnumber
|
||||
LEFT JOIN biblio ON items.biblionumber=biblio.biblionumber
|
||||
LEFT JOIN biblioitems ON items.biblioitemnumber=biblioitems.biblioitemnumber
|
||||
# must avoid biblioitems.* to prevent large marc and marcxml fields from killing performance
|
||||
# FIXME: namespace collision: each table has "timestamp" fields. Which one is "timestamp" ?
|
||||
# FIXME: circ/ciculation.pl tries to sort by timestamp!
|
||||
# FIXME: C4::Print::printslip tries to sort by timestamp!
|
||||
# FIXME: namespace collision: other collisions possible.
|
||||
# FIXME: most of this data isn't really being used by callers.
|
||||
my $sth = C4::Context->dbh->prepare(
|
||||
"SELECT issues.*,
|
||||
items.*,
|
||||
biblio.*,
|
||||
biblioitems.volume,
|
||||
biblioitems.number,
|
||||
biblioitems.itemtype,
|
||||
biblioitems.isbn,
|
||||
biblioitems.issn,
|
||||
biblioitems.publicationyear,
|
||||
biblioitems.publishercode,
|
||||
biblioitems.volumedate,
|
||||
biblioitems.volumedesc,
|
||||
biblioitems.lccn,
|
||||
biblioitems.url,
|
||||
issues.timestamp AS timestamp,
|
||||
issues.renewals AS renewals,
|
||||
items.renewals AS totalrenewals
|
||||
FROM issues
|
||||
LEFT JOIN items ON items.itemnumber = issues.itemnumber
|
||||
LEFT JOIN biblio ON items.biblionumber = biblio.biblionumber
|
||||
LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
|
||||
WHERE
|
||||
borrowernumber=?
|
||||
borrowernumber=?
|
||||
ORDER BY issues.issuedate"
|
||||
);
|
||||
$sth->execute($borrowernumber);
|
||||
my $data = $sth->fetchall_arrayref({});
|
||||
my $today = POSIX::strftime("%Y%m%d", localtime);
|
||||
foreach( @$data ) {
|
||||
my $datedue = $_->{'date_due'};
|
||||
$datedue =~ s/-//g;
|
||||
if ( $datedue < $today ) {
|
||||
$_->{'overdue'} = 1;
|
||||
}
|
||||
my $today = C4::Dates->new->output('iso');
|
||||
foreach (@$data) {
|
||||
$_->{date_due} or next;
|
||||
($_->{date_due} < $today) and $_->{overdue} = 1;
|
||||
}
|
||||
$sth->finish;
|
||||
return ( scalar(@$data), $data );
|
||||
return $data;
|
||||
}
|
||||
|
||||
=head2 GetAllIssues
|
||||
|
|
|
|||
|
|
@ -177,9 +177,9 @@ EOF
|
|||
|
||||
#'
|
||||
sub printslip ($) {
|
||||
my ( $borrowernumber ) = shift;
|
||||
my ( $borrower ) = GetMemberDetails( $borrowernumber);
|
||||
my ($countissues,$issueslist) = GetPendingIssues($borrowernumber);
|
||||
my $borrowernumber = shift;
|
||||
my $borrower = GetMemberDetails($borrowernumber);
|
||||
my $issueslist = GetPendingIssues($borrowernumber);
|
||||
foreach my $it (@$issueslist){
|
||||
$it->{'date_due'}=format_date($it->{'date_due'});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -102,8 +102,7 @@ sub new {
|
|||
# FIXME: populate items fine_items recall_items
|
||||
# $ilspatron{hold_items} = (GetReservesFromBorrowernumber($kp->{borrowernumber},'F'));
|
||||
$ilspatron{unavail_holds} = [(GetReservesFromBorrowernumber($kp->{borrowernumber}))];
|
||||
my ($count,$issues) = GetPendingIssues($kp->{borrowernumber});
|
||||
$ilspatron{items} = $issues;
|
||||
$ilspatron{items} = GetPendingIssues($kp->{borrowernumber});
|
||||
$self = \%ilspatron;
|
||||
$debug and warn Dumper($self);
|
||||
syslog("LOG_DEBUG", "new ILS::Patron(%s): found patron '%s'", $patron_id,$self->{id});
|
||||
|
|
|
|||
|
|
@ -420,7 +420,7 @@ my @issued_itemtypes_count_loop;
|
|||
|
||||
if ($borrower) {
|
||||
# get each issue of the borrower & separate them in todayissues & previous issues
|
||||
my ($countissues,$issueslist) = GetPendingIssues($borrower->{'borrowernumber'});
|
||||
my ($issueslist) = GetPendingIssues($borrower->{'borrowernumber'});
|
||||
|
||||
# split in 2 arrays for today & previous
|
||||
foreach my $it ( @$issueslist ) {
|
||||
|
|
@ -480,7 +480,7 @@ FROM issuingrules
|
|||
WHERE categorycode=?
|
||||
" );
|
||||
#my @issued_itemtypes_count; # huh?
|
||||
$issueqty_sth->execute("*"); # FIXME: Why have a WHERE clause at all with a hardcoded "*"?
|
||||
$issueqty_sth->execute("*"); # This is a literal asterisk, not a wildcard.
|
||||
|
||||
while ( my $data = $issueqty_sth->fetchrow_hashref() ) {
|
||||
|
||||
|
|
|
|||
|
|
@ -1,11 +1,9 @@
|
|||
#!/usr/bin/perl
|
||||
|
||||
|
||||
#script to delete items
|
||||
#written 2/5/00
|
||||
#by chris@katipo.co.nz
|
||||
|
||||
|
||||
# Copyright 2000-2002 Katipo Communications
|
||||
#
|
||||
# This file is part of Koha.
|
||||
|
|
@ -31,11 +29,8 @@ use C4::Output;
|
|||
use C4::Auth;
|
||||
use C4::Members;
|
||||
|
||||
|
||||
my $input = new CGI;
|
||||
|
||||
my $flagsrequired;
|
||||
$flagsrequired->{borrowers}=1;
|
||||
my ($template, $borrowernumber, $cookie)
|
||||
= get_template_and_user({template_name => "members/deletemem.tmpl",
|
||||
query => $input,
|
||||
|
|
@ -47,9 +42,8 @@ my ($template, $borrowernumber, $cookie)
|
|||
|
||||
#print $input->header;
|
||||
my $member=$input->param('member');
|
||||
my %member2;
|
||||
$member2{'borrowernumber'}=$member;
|
||||
my ($countissues,$issues)=GetPendingIssues($member);
|
||||
my $issues = GetPendingIssues($member); # FIXME: wasteful call when really, we only want the count
|
||||
my $countissues = scalar(@$issues);
|
||||
|
||||
my ($bor)=GetMemberDetails($member,'');
|
||||
my $flags=$bor->{flags};
|
||||
|
|
@ -74,7 +68,6 @@ my $dbh = C4::Context->dbh;
|
|||
my $sth=$dbh->prepare("Select * from borrowers where guarantorid=?");
|
||||
$sth->execute($member);
|
||||
my $data=$sth->fetchrow_hashref;
|
||||
$sth->finish;
|
||||
if ($countissues > 0 or $flags->{'CHARGES'} or $data->{'borrowernumber'}){
|
||||
# print $input->header;
|
||||
$template->param(borrowernumber => $member);
|
||||
|
|
@ -84,7 +77,7 @@ if ($countissues > 0 or $flags->{'CHARGES'} or $data->{'borrowernumber'}){
|
|||
if ($flags->{'CHARGES'} ne '') {
|
||||
$template->param(charges => $flags->{'CHARGES'}->{'amount'});
|
||||
}
|
||||
if ($data ne '') {
|
||||
if ($data) {
|
||||
$template->param(guarantees => 1);
|
||||
}
|
||||
output_html_with_http_headers $input, $cookie, $template->output;
|
||||
|
|
|
|||
|
|
@ -216,7 +216,8 @@ my $lib2 = &GetSortDetails( "Bsort2", $data->{'sort2'} );
|
|||
|
||||
# current issues
|
||||
#
|
||||
my ( $count, $issue ) = GetPendingIssues($borrowernumber);
|
||||
my $issue = GetPendingIssues($borrowernumber);
|
||||
my $count = scalar(@$issue);
|
||||
my $roaddetails = &GetRoadTypeDetails( $data->{'streettype'} );
|
||||
my $today = POSIX::strftime("%Y-%m-%d", localtime); # iso format
|
||||
my @issuedata;
|
||||
|
|
|
|||
|
|
@ -51,7 +51,7 @@ my ( $borr ) = GetMemberDetails( $borrowernumber );
|
|||
my $calendar = Data::ICal->new();
|
||||
|
||||
# get issued items ....
|
||||
my ($countissues,$issues) = GetPendingIssues($borrowernumber);
|
||||
my ($issues) = GetPendingIssues($borrowernumber);
|
||||
|
||||
foreach my $issue ( @$issues ) {
|
||||
my $vevent = Data::ICal::Entry::Event->new();
|
||||
|
|
|
|||
|
|
@ -89,7 +89,7 @@ $template->param( BORROWER_INFO => \@bordat,
|
|||
);
|
||||
|
||||
#get issued items ....
|
||||
my ($countissues,$issues) = GetPendingIssues($borrowernumber);
|
||||
my ($issues) = GetPendingIssues($borrowernumber);
|
||||
my @issue_list = sort { $b->{'date_due'} cmp $a->{'date_due'} } @$issues;
|
||||
|
||||
my $count = 0;
|
||||
|
|
|
|||
|
|
@ -132,7 +132,7 @@ if ($borrower->{cardnumber}) {
|
|||
my $bornum = $borrower->{borrowernumber};
|
||||
my $borrowername = $borrower->{firstname} . " " . $borrower->{surname};
|
||||
my @issues;
|
||||
my ($countissues,$issueslist) = GetPendingIssues($borrower->{'borrowernumber'});
|
||||
my ($issueslist) = GetPendingIssues($borrower->{'borrowernumber'});
|
||||
foreach my $it ( @$issueslist ) {
|
||||
push @issues, $it;
|
||||
$cnt++;
|
||||
|
|
|
|||
Loading…
Reference in New Issue